This adds support for 10mbps speed in PHY device's
"supported" field which helps in autonegotiating
10mbps link from PHY side where PHY supports the speed
but not updated in PHY kernel framework.

One such example is AQR113C PHY.

Signed-off-by: Devang Vyas <devangnayanbhai.vyas@amd.com>
---
 drivers/net/phy/aquantia_main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/phy/aquantia_main.c b/drivers/net/phy/aquantia_main.c
index 334a6904ca5a..fed0a4cea651 100644
--- a/drivers/net/phy/aquantia_main.c
+++ b/drivers/net/phy/aquantia_main.c
@@ -556,6 +556,9 @@ static void aqr107_chip_info(struct phy_device *phydev)
 	build_id = FIELD_GET(VEND1_GLOBAL_RSVD_STAT1_FW_BUILD_ID, val);
 	prov_id = FIELD_GET(VEND1_GLOBAL_RSVD_STAT1_PROV_ID, val);
 
+	if (!test_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, phydev->supported))
+		linkmode_set_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT, phydev->supported);
+
 	phydev_dbg(phydev, "FW %u.%u, Build %u, Provisioning %u\n",
 		   fw_major, fw_minor, build_id, prov_id);
 }
-- 
2.25.1

On Wed, Apr 26, 2023 at 01:46:12PM +0530, Devang Vyas wrote:
> This adds support for 10mbps speed in PHY device's
> "supported" field which helps in autonegotiating
> 10mbps link from PHY side where PHY supports the speed
> but not updated in PHY kernel framework.

Are you saying it is not listed in BMSR that the PHY supports 10 Mbps?
Bits BMSR_10HALF and BMSR_10FULL are not set?

     Andrew

On Wed, 26 Apr 2023 14:54:01 +0200 Andrew Lunn wrote:
> On Wed, Apr 26, 2023 at 01:46:12PM +0530, Devang Vyas wrote:
> > This adds support for 10mbps speed in PHY device's
> > "supported" field which helps in autonegotiating
> > 10mbps link from PHY side where PHY supports the speed
> > but not updated in PHY kernel framework.  
> 
> Are you saying it is not listed in BMSR that the PHY supports 10 Mbps?
> Bits BMSR_10HALF and BMSR_10FULL are not set?

I didn't see any reply to Andrew's question so dropping this 
from patchwork for now. It feels like -next material too, so
please hold off any reposting until Monday.
-- 
pw-bot: defer

[AMD Official Use Only - General]

Hi Andrew,

We are using AQR113C Marvell PHY which is CL45 based and based on below check in phy_probe() function:
        if (phydrv->features)
                linkmode_copy(phydev->supported, phydrv->features);
        else if (phydrv->get_features)
                err = phydrv->get_features(phydev);
        else if (phydev->is_c45)
                err = genphy_c45_pma_read_abilities(phydev);    -> it reads capability from PMA register where 10M bit is read-only static and value is 0
        else
                err = genphy_read_abilities(phydev);

Based on PHY datasheet, it supports 10M and we have made the change for the same and verified successfully.

Below code should set the supported field under genphy_c45_pma_read_abilities(), but as the value is 0, we have to set the 10M mode explicitly.

                linkmode_mod_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT,
                                 phydev->supported,
                                 val & MDIO_PMA_EXTABLE_10BT);

Please share your inputs further.

Thanks & Regards,
Devang Vyas

-----Original Message-----
From: Jakub Kicinski <kuba@kernel.org> 
Sent: Wednesday, May 3, 2023 8:17 AM
To: Vyas, Devang nayanbhai <Devangnayanbhai.Vyas@amd.com>
Cc: Andrew Lunn <andrew@lunn.ch>; hkallweit1@gmail.com; linux@armlinux.org.uk; davem@davemloft.net; edumazet@google.com; pabeni@redhat.com; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH] net: phy: aquantia: Add 10mbps support

On Wed, 26 Apr 2023 14:54:01 +0200 Andrew Lunn wrote:
> On Wed, Apr 26, 2023 at 01:46:12PM +0530, Devang Vyas wrote:
> > This adds support for 10mbps speed in PHY device's "supported" field 
> > which helps in autonegotiating 10mbps link from PHY side where PHY 
> > supports the speed but not updated in PHY kernel framework.
> 
> Are you saying it is not listed in BMSR that the PHY supports 10 Mbps?
> Bits BMSR_10HALF and BMSR_10FULL are not set?

I didn't see any reply to Andrew's question so dropping this from patchwork for now. It feels like -next material too, so please hold off any reposting until Monday.
--
pw-bot: defer

On Thu, May 04, 2023 at 06:18:11AM +0000, Vyas, Devang nayanbhai wrote:
> [AMD Official Use Only - General]

Hi Devang

Please don't top post.

Also, wrap your emails at around 75 characters. Network Etiquette
rules apply for linux kernel mailling list.

> We are using AQR113C Marvell PHY which is CL45 based and based on below check in phy_probe() function:
>         if (phydrv->features)
>                 linkmode_copy(phydev->supported, phydrv->features);
>         else if (phydrv->get_features)
>                 err = phydrv->get_features(phydev);
>         else if (phydev->is_c45)
>                 err = genphy_c45_pma_read_abilities(phydev);    -> it reads capability from PMA register where 10M bit is read-only static and value is 0
>         else
>                 err = genphy_read_abilities(phydev);
> 
> Based on PHY datasheet, it supports 10M and we have made the change for the same and verified successfully.
> 
> Below code should set the supported field under genphy_c45_pma_read_abilities(), but as the value is 0, we have to set the 10M mode explicitly.

So the PHY is 'broken' in that one of its registers has the wrong
value. However, it can probably be fixed. aQuantia firmware is not
just code executed by its embedded uC. It also contains
`provisioning`. This blob sets the values of many registers, and i
think can be used to set registers which are read only. Maybe the blob
you have is incorrectly provisioning the MDIO_PMA_EXTABLE_10BT
register.

Please talk to Marvell about the provisioning blob you have.

       Andrew