| Message ID | 1479257416-29389-7-git-send-email-andre.przywara@arm.com |
|---|---|
| State | Accepted |
| Commit | 429033659d574bec966718de19eb732b99f3a9af |
| Delegated to: | Tom Rini |
| Headers | show |
(Adding a few Marvell people to Cc) On 16.11.2016 01:50, Andre Przywara wrote: > Obviously the mask for the rx and tx select field cannot be right, > as it would overlap in one and exceed the 32-bit register in the other > case. From looking at the neighbouring bits it looks like the mask > should be really 4 bits wide instead of 8. > > Pointed out by a GCC 6.2 (default) warning. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > drivers/phy/marvell/comphy_a3700.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/phy/marvell/comphy_a3700.h b/drivers/phy/marvell/comphy_a3700.h > index eb2ed7b..dd60b88 100644 > --- a/drivers/phy/marvell/comphy_a3700.h > +++ b/drivers/phy/marvell/comphy_a3700.h > @@ -33,9 +33,9 @@ > #define rb_pin_pu_tx BIT(18) > #define rb_pin_tx_idle BIT(19) > #define rf_gen_rx_sel_shift 22 > -#define rf_gen_rx_select (0xFF << rf_gen_rx_sel_shift) > +#define rf_gen_rx_select (0x0F << rf_gen_rx_sel_shift) > #define rf_gen_tx_sel_shift 26 > -#define rf_gen_tx_select (0xFF << rf_gen_tx_sel_shift) > +#define rf_gen_tx_select (0x0F << rf_gen_tx_sel_shift) > #define rb_phy_rx_init BIT(30) > > #define COMPHY_PHY_STAT1_ADDR(lane) MVEBU_REG(0x018318 + (lane) * 0x28) Looks good to me, so: Reviewed-by: Stefan Roese <sr@denx.de> Thanks, Stefan
Agree, both fields are 4 bits wide. Kosta -----Original Message----- From: Stefan Roese [mailto:sr@denx.de] Sent: Wednesday, November 16, 2016 13:32 To: Andre Przywara Cc: u-boot@lists.denx.de; Nadav Haklai; Kostya Porotchkin; Hanna Hawa Subject: Re: [PATCH 06/12] marvell: comphy_a3700: fix bitmask (Adding a few Marvell people to Cc) On 16.11.2016 01:50, Andre Przywara wrote: > Obviously the mask for the rx and tx select field cannot be right, as > it would overlap in one and exceed the 32-bit register in the other > case. From looking at the neighbouring bits it looks like the mask > should be really 4 bits wide instead of 8. > > Pointed out by a GCC 6.2 (default) warning. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > drivers/phy/marvell/comphy_a3700.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/phy/marvell/comphy_a3700.h > b/drivers/phy/marvell/comphy_a3700.h > index eb2ed7b..dd60b88 100644 > --- a/drivers/phy/marvell/comphy_a3700.h > +++ b/drivers/phy/marvell/comphy_a3700.h > @@ -33,9 +33,9 @@ > #define rb_pin_pu_tx BIT(18) > #define rb_pin_tx_idle BIT(19) > #define rf_gen_rx_sel_shift 22 > -#define rf_gen_rx_select (0xFF << rf_gen_rx_sel_shift) > +#define rf_gen_rx_select (0x0F << rf_gen_rx_sel_shift) > #define rf_gen_tx_sel_shift 26 > -#define rf_gen_tx_select (0xFF << rf_gen_tx_sel_shift) > +#define rf_gen_tx_select (0x0F << rf_gen_tx_sel_shift) > #define rb_phy_rx_init BIT(30) > > #define COMPHY_PHY_STAT1_ADDR(lane) MVEBU_REG(0x018318 + (lane) * 0x28) Looks good to me, so: Reviewed-by: Stefan Roese <sr@denx.de> Thanks, Stefan
On Wed, Nov 16, 2016 at 12:50:10AM +0000, Andre Przywara wrote: > Obviously the mask for the rx and tx select field cannot be right, > as it would overlap in one and exceed the 32-bit register in the other > case. From looking at the neighbouring bits it looks like the mask > should be really 4 bits wide instead of 8. > > Pointed out by a GCC 6.2 (default) warning. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > Reviewed-by: Stefan Roese <sr@denx.de> > Reviewed-by: Stefan Roese <sr@denx.de> Applied to u-boot/master, thanks!
diff --git a/drivers/phy/marvell/comphy_a3700.h b/drivers/phy/marvell/comphy_a3700.h index eb2ed7b..dd60b88 100644 --- a/drivers/phy/marvell/comphy_a3700.h +++ b/drivers/phy/marvell/comphy_a3700.h @@ -33,9 +33,9 @@ #define rb_pin_pu_tx BIT(18) #define rb_pin_tx_idle BIT(19) #define rf_gen_rx_sel_shift 22 -#define rf_gen_rx_select (0xFF << rf_gen_rx_sel_shift) +#define rf_gen_rx_select (0x0F << rf_gen_rx_sel_shift) #define rf_gen_tx_sel_shift 26 -#define rf_gen_tx_select (0xFF << rf_gen_tx_sel_shift) +#define rf_gen_tx_select (0x0F << rf_gen_tx_sel_shift) #define rb_phy_rx_init BIT(30) #define COMPHY_PHY_STAT1_ADDR(lane) MVEBU_REG(0x018318 + (lane) * 0x28)
Obviously the mask for the rx and tx select field cannot be right, as it would overlap in one and exceed the 32-bit register in the other case. From looking at the neighbouring bits it looks like the mask should be really 4 bits wide instead of 8. Pointed out by a GCC 6.2 (default) warning. Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- drivers/phy/marvell/comphy_a3700.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)