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

Parthiban.Veerasooran
2024-10-03T04:39:59Z

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
>
>