Re: [PATCH 1/1] net: phy: marvell: avoid bringing down fibre link when autoneg is bypassed

[<Parthiban.Veerasooran@microchip.com>]

Hi,

On 03/10/24 7:55 am, Qingtao Cao wrote:
> [You don't often get email from qingtao.cao.au@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 88E151x the SGMII autoneg bypass mode defaults to be enabled. When it is
> activated, the device assumes a link-up status with existing configuration
> in BMCR, avoid bringing down the fibre link in this case
> 
> Test case:
> 1. Two 88E151x connected with SFP, both enable autoneg, link is up with speed
>     1000M
> 2. Disable autoneg on one device and explicitly set its speed to 1000M
> 3. The fibre link can still up with this change, otherwise not.
> 
> Signed-off-by: Qingtao Cao <qingtao.cao@digi.com>
> ---
>   drivers/net/phy/marvell.c | 23 ++++++++++++++++++++++-
>   1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
> index 9964bf3dea2f..535c6e679ff7 100644
> --- a/drivers/net/phy/marvell.c
> +++ b/drivers/net/phy/marvell.c
> @@ -195,6 +195,10 @@
> 
>   #define MII_88E1510_MSCR_2             0x15
> 
> +#define MII_88E1510_FSCR2              0x1a
> +#define MII_88E1510_FSCR2_BYPASS_ENABLE        (1<<6)
I think you can use BIT(6)
> +#define MII_88E1510_FSCR2_BYPASS_STATUS        (1<<5)
Here as well BIT(5)
> +
>   #define MII_VCT5_TX_RX_MDI0_COUPLING   0x10
>   #define MII_VCT5_TX_RX_MDI1_COUPLING   0x11
>   #define MII_VCT5_TX_RX_MDI2_COUPLING   0x12
> @@ -1625,9 +1629,26 @@ static int marvell_read_status_page_an(struct phy_device *phydev,
>   {
>          int lpa;
>          int err;
> +       int fscr2;
Follow reverse xmas tree ordering.

https://www.kernel.org/doc/Documentation/process/maintainer-netdev.rst#:~:text=Local%20variable%20ordering%20(%22reverse%20xmas%20tree%22%2C%20%22RCS%22)

Best regards,
Parthiban V
> 
>          if (!(status & MII_M1011_PHY_STATUS_RESOLVED)) {
> -               phydev->link = 0;
> +               if (!fiber) {
> +                       phydev->link = 0;
> +               } else {
> +                       fscr2 = phy_read(phydev, MII_88E1510_FSCR2);
> +                       if (fscr2 > 0) {
> +                               if ((fscr2 & MII_88E1510_FSCR2_BYPASS_ENABLE) &&
> +                                   (fscr2 & MII_88E1510_FSCR2_BYPASS_STATUS)) {
> +                                       if (genphy_read_status_fixed(phydev) < 0)
> +                                               phydev->link = 0;
> +                               } else {
> +                                       phydev->link = 0;
> +                               }
> +                       } else {
> +                               phydev->link = 0;
> +                       }
> +               }
> +
>                  return 0;
>          }
> 
> --
> 2.34.1
> 
>