RE: [PATCH net v2 2/2] net: stmmac: move fixed-link support fixup code

From: Sit, Michael Wei Hong
Date: Tue Mar 21 2023 - 06:17:57 EST




> -----Original Message-----
> From: Andrew Lunn <andrew@xxxxxxx>
> Sent: Saturday, March 18, 2023 4:03 AM
> To: Sit, Michael Wei Hong <michael.wei.hong.sit@xxxxxxxxx>
> Cc: Giuseppe Cavallaro <peppe.cavallaro@xxxxxx>; Alexandre
> Torgue <alexandre.torgue@xxxxxxxxxxx>; Jose Abreu
> <joabreu@xxxxxxxxxxxx>; David S . Miller
> <davem@xxxxxxxxxxxxx>; Eric Dumazet
> <edumazet@xxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>;
> Paolo Abeni <pabeni@xxxxxxxxxx>; Maxime Coquelin
> <mcoquelin.stm32@xxxxxxxxx>; Ong, Boon Leong
> <boon.leong.ong@xxxxxxxxx>; netdev@xxxxxxxxxxxxxxx; linux-
> stm32@xxxxxxxxxxxxxxxxxxxxxxxxxxxx; linux-arm-
> kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Looi,
> Hong Aun <hong.aun.looi@xxxxxxxxx>; Voon, Weifeng
> <weifeng.voon@xxxxxxxxx>; Lai, Peter Jun Ann
> <peter.jun.ann.lai@xxxxxxxxx>
> Subject: Re: [PATCH net v2 2/2] net: stmmac: move fixed-link
> support fixup code
>
> On Tue, Mar 14, 2023 at 03:02:08PM +0800, Michael Sit Wei Hong
> wrote:
> > xpcs_an_inband value is updated in the speed_mode_2500
> function which
> > turns on the xpcs_an_inband mode.
> >
> > Moving the fixed-link fixup code to right before phylink setup
> to
> > ensure no more fixup will affect the fixed-link mode
> configurations.
>
> Please could you explain why this is correct, when you could
> simple not set priv->plat->mdio_bus_data->xpcs_an_inband =
> true; in intel_speed_mode_2500()?
>
> This all seems like hacks, rather than a clean design.
>
> Andrew
Makes sense, will test out with your suggestion and submit
the changes in next version, will feedback if there is any
findings.