| Message ID | 20170804081720.2g6yuthplhc4rv6b@mwanda |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Dan Carpenter > Sent: 04 August 2017 09:17 > This was supposed to be a bitwise OR but there is a || vs | typo. > > Fixes: 864dc729d528 ("net: phy: marvell: Refactor m88e1121 RGMII delay configuration") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c > index 361fe9927ef2..15cbcdba618a 100644 > --- a/drivers/net/phy/marvell.c > +++ b/drivers/net/phy/marvell.c > @@ -83,7 +83,7 @@ > #define MII_88E1121_PHY_MSCR_REG 21 > #define MII_88E1121_PHY_MSCR_RX_DELAY BIT(5) > #define MII_88E1121_PHY_MSCR_TX_DELAY BIT(4) > -#define MII_88E1121_PHY_MSCR_DELAY_MASK (~(BIT(5) || BIT(4))) > +#define MII_88E1121_PHY_MSCR_DELAY_MASK (~(BIT(5) | BIT(4))) Wouldn't: +#define MII_88E1121_PHY_MSCR_DELAY_MASK (~(MII_88E1121_PHY_MSCR_RX_DELAY | MII_88E1121_PHY_MSCR_TX_DELAY)) be more correct? If a little long? Actually the ~ looks odd here.... Reads code.... Kill the define and explicitly mask off the two values just before conditionally setting them. David
On Fri, Aug 04, 2017 at 11:17:21AM +0300, Dan Carpenter wrote: > This was supposed to be a bitwise OR but there is a || vs | typo. > > Fixes: 864dc729d528 ("net: phy: marvell: Refactor m88e1121 RGMII delay configuration") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> Hi Dan Thanks for the fix. Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
On Fri, Aug 04, 2017 at 02:54:45PM +0000, David Laight wrote: > From: Dan Carpenter > > Sent: 04 August 2017 09:17 > > This was supposed to be a bitwise OR but there is a || vs | typo. > > > > Fixes: 864dc729d528 ("net: phy: marvell: Refactor m88e1121 RGMII delay configuration") > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > > > diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c > > index 361fe9927ef2..15cbcdba618a 100644 > > --- a/drivers/net/phy/marvell.c > > +++ b/drivers/net/phy/marvell.c > > @@ -83,7 +83,7 @@ > > #define MII_88E1121_PHY_MSCR_REG 21 > > #define MII_88E1121_PHY_MSCR_RX_DELAY BIT(5) > > #define MII_88E1121_PHY_MSCR_TX_DELAY BIT(4) > > -#define MII_88E1121_PHY_MSCR_DELAY_MASK (~(BIT(5) || BIT(4))) > > +#define MII_88E1121_PHY_MSCR_DELAY_MASK (~(BIT(5) | BIT(4))) > > Wouldn't: > +#define MII_88E1121_PHY_MSCR_DELAY_MASK (~(MII_88E1121_PHY_MSCR_RX_DELAY | MII_88E1121_PHY_MSCR_TX_DELAY)) > be more correct? > If a little long? > Actually the ~ looks odd here.... > Reads code.... > Kill the define and explicitly mask off the two values just before > conditionally setting them. Hi David I will put this on my TODO list. But lets get Dan's fix included first. Andrew
From: Dan Carpenter <dan.carpenter@oracle.com> Date: Fri, 4 Aug 2017 11:17:21 +0300 > This was supposed to be a bitwise OR but there is a || vs | typo. > > Fixes: 864dc729d528 ("net: phy: marvell: Refactor m88e1121 RGMII delay configuration") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> Applied, but please specify "[PATCH net-next]" in your Subject line and make your target tree explicit in the future. Thanks.
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c index 361fe9927ef2..15cbcdba618a 100644 --- a/drivers/net/phy/marvell.c +++ b/drivers/net/phy/marvell.c @@ -83,7 +83,7 @@ #define MII_88E1121_PHY_MSCR_REG 21 #define MII_88E1121_PHY_MSCR_RX_DELAY BIT(5) #define MII_88E1121_PHY_MSCR_TX_DELAY BIT(4) -#define MII_88E1121_PHY_MSCR_DELAY_MASK (~(BIT(5) || BIT(4))) +#define MII_88E1121_PHY_MSCR_DELAY_MASK (~(BIT(5) | BIT(4))) #define MII_88E1121_MISC_TEST 0x1a #define MII_88E1510_MISC_TEST_TEMP_THRESHOLD_MASK 0x1f00
This was supposed to be a bitwise OR but there is a || vs | typo. Fixes: 864dc729d528 ("net: phy: marvell: Refactor m88e1121 RGMII delay configuration") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>