diff mbox

[v4,03/11] net: mvmdio: use GENMASK for masks

Message ID 20170614154911.14510-4-antoine.tenart@free-electrons.com (mailing list archive)
State New, archived
Headers show

Commit Message

Antoine Tenart June 14, 2017, 3:49 p.m. UTC 
Cosmetic patch to use the GENMASK helper for masks.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/marvell/mvmdio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

David Laight June 19, 2017, 11:57 a.m. UTC | #1 
From: Antoine Tenart
> Sent: 14 June 2017 16:49
> Cosmetic patch to use the GENMASK helper for masks.
...
> -	ret = val & 0xFFFF;
> +	ret = val & GENMASK(15, 0);

My 2c: It isn't at all clear to me that changes like this in anyway
improve the code readability.
In some sense the '15' should be a named constant - but that just makes
it even less obvious what is going on.

	David
David Miller June 19, 2017, 2:49 p.m. UTC | #2 
From: David Laight <David.Laight@ACULAB.COM>
Date: Mon, 19 Jun 2017 11:57:51 +0000

> From: Antoine Tenart
>> Sent: 14 June 2017 16:49
>> Cosmetic patch to use the GENMASK helper for masks.
> ...
>> -	ret = val & 0xFFFF;
>> +	ret = val & GENMASK(15, 0);
> 
> My 2c: It isn't at all clear to me that changes like this in anyway
> improve the code readability.
> In some sense the '15' should be a named constant - but that just makes
> it even less obvious what is going on.

I agree, a hexidecimal mask of 0xffff is 100 times more readable and
understandable to me than "SOME_MACRO(x, y)".
diff mbox

Patch

diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index 17b518b13ae3..583f1c5753c2 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -138,7 +138,7 @@  static int orion_mdio_read(struct mii_bus *bus, int mii_id,
 		goto out;
 	}
 
-	ret = val & 0xFFFF;
+	ret = val & GENMASK(15, 0);
 out:
 	mutex_unlock(&dev->lock);
 	return ret;