Re: [PATCH net-next 8/9] net: phy: mediatek: Change mtk-ge-soc.c line wrapping

[SkyLake Huang (黃啟澤)]

On Fri, 2024-10-04 at 13:07 +0100, Russell King (Oracle) wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  Hi,
> 
> On Fri, Oct 04, 2024 at 06:24:12PM +0800, Sky Huang wrote:
> > diff --git a/drivers/net/phy/mediatek/mtk-ge-soc.c
> b/drivers/net/phy/mediatek/mtk-ge-soc.c
> > index 26c2183..cb6838b 100644
> > --- a/drivers/net/phy/mediatek/mtk-ge-soc.c
> > +++ b/drivers/net/phy/mediatek/mtk-ge-soc.c
> > @@ -295,7 +295,8 @@ static int cal_cycle(struct phy_device *phydev,
> int devad,
> >  ret = phy_read_mmd_poll_timeout(phydev, MDIO_MMD_VEND1,
> >  MTK_PHY_RG_AD_CAL_CLK, reg_val,
> >  reg_val & MTK_PHY_DA_CAL_CLK, 500,
> > -ANALOG_INTERNAL_OPERATION_MAX_US, false);
> > +ANALOG_INTERNAL_OPERATION_MAX_US,
> > +false);
> 
> This is fine.
> 
> >  if (ret) {
> >  phydev_err(phydev, "Calibration cycle timeout\n");
> >  return ret;
> > @@ -304,7 +305,7 @@ static int cal_cycle(struct phy_device *phydev,
> int devad,
> >  phy_clear_bits_mmd(phydev, MDIO_MMD_VEND1, MTK_PHY_RG_AD_CALIN,
> >     MTK_PHY_DA_CALIN_FLAG);
> >  ret = phy_read_mmd(phydev, MDIO_MMD_VEND1, MTK_PHY_RG_AD_CAL_COMP)
> >>
> > -   MTK_PHY_AD_CAL_COMP_OUT_SHIFT;
> > +      MTK_PHY_AD_CAL_COMP_OUT_SHIFT;
> 
> Before cleaning this up, please first make it propagate any error
> code
> correctly (a bug fix):
> 
> ret = phy_read_mmd(phydev, MDIO_MMD_VEND1, MTK_PHY_RG_AD_CAL_COMP);
> if (ret < 0)
> return ret;
> 
> ret >>= MTK_PHY_AD_CAL_COMP_OUT_SHIFT;
> 
> and then you won't need to change it in this patch. A better solution
> to
> the shift would be to look at FIELD_GET().
> 
> >  phydev_dbg(phydev, "cal_val: 0x%x, ret: %d\n", cal_val, ret);
> >  
> >  return ret;
> > @@ -394,38 +395,46 @@ static int tx_amp_fill_result(struct
> phy_device *phydev, u16 *buf)
> >  }
> >  
> >  phy_modify_mmd(phydev, MDIO_MMD_VEND1, MTK_PHY_TXVLD_DA_RG,
> > -       MTK_PHY_DA_TX_I2MPB_A_GBE_MASK, (buf[0] + bias[0]) << 10);
> > +       MTK_PHY_DA_TX_I2MPB_A_GBE_MASK,
> > +       (buf[0] + bias[0]) << 10);
> 
> Another cleanup would be to use FIELD_PREP() for these.
> 
> > -static const unsigned long supported_triggers =
> (BIT(TRIGGER_NETDEV_FULL_DUPLEX) |
> > - BIT(TRIGGER_NETDEV_HALF_DUPLEX) |
> > - BIT(TRIGGER_NETDEV_LINK)        |
> > - BIT(TRIGGER_NETDEV_LINK_10)     |
> > - BIT(TRIGGER_NETDEV_LINK_100)    |
> > - BIT(TRIGGER_NETDEV_LINK_1000)   |
> > - BIT(TRIGGER_NETDEV_RX)          |
> > - BIT(TRIGGER_NETDEV_TX));
> > +static const unsigned long supported_triggers =
> > +(BIT(TRIGGER_NETDEV_FULL_DUPLEX) |
> > + BIT(TRIGGER_NETDEV_HALF_DUPLEX) |
> > + BIT(TRIGGER_NETDEV_LINK)        |
> > + BIT(TRIGGER_NETDEV_LINK_10)     |
> > + BIT(TRIGGER_NETDEV_LINK_100)    |
> > + BIT(TRIGGER_NETDEV_LINK_1000)   |
> > + BIT(TRIGGER_NETDEV_RX)          |
> > + BIT(TRIGGER_NETDEV_TX));
> 
> The outer parens are unnecessary, and thus could be removed.
> 
> Thanks.
> 
> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Thanks. I think I'll isolate this patch and send another cleanup patch
for mediatek-ge-soc.c first.

BRs,
Sky