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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d0b28e331cb91f8b1e5d8f94441c7860f841f5e6.camel@mediatek.com>
Date: Mon, 7 Oct 2024 10:52:53 +0000
From: SkyLake Huang (黃啟澤)
	<SkyLake.Huang@...iatek.com>
To: "linux@...linux.org.uk" <linux@...linux.org.uk>
CC: "andrew@...n.ch" <andrew@...n.ch>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "linux-mediatek@...ts.infradead.org"
	<linux-mediatek@...ts.infradead.org>, "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "kuba@...nel.org" <kuba@...nel.org>,
	"pabeni@...hat.com" <pabeni@...hat.com>, "edumazet@...gle.com"
	<edumazet@...gle.com>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"dqfext@...il.com" <dqfext@...il.com>,
	Steven Liu (劉人豪) <steven.liu@...iatek.com>,
	"matthias.bgg@...il.com" <matthias.bgg@...il.com>, "davem@...emloft.net"
	<davem@...emloft.net>, "hkallweit1@...il.com" <hkallweit1@...il.com>,
	"daniel@...rotopia.org" <daniel@...rotopia.org>, AngeloGioacchino Del Regno
	<angelogioacchino.delregno@...labora.com>
Subject: Re: [PATCH net-next 8/9] net: phy: mediatek: Change mtk-ge-soc.c line
 wrapping

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