Re: [PATCH net-next] net: phy: introduce phydev->port

[Florian Fainelli <f.fainelli@gmail.com>]

On 2/10/21 3:20 AM, Michael Walle wrote:
> 
> Am 2021-02-09 17:38, schrieb Michael Walle:
>> --- a/drivers/net/phy/phy.c
>> +++ b/drivers/net/phy/phy.c
>> @@ -308,7 +308,7 @@ void phy_ethtool_ksettings_get(struct phy_device
>> *phydev,
>>      if (phydev->interface == PHY_INTERFACE_MODE_MOCA)
>>          cmd->base.port = PORT_BNC;
>>      else
>> -        cmd->base.port = PORT_MII;
>> +        cmd->base.port = phydev->port;
>>      cmd->base.transceiver = phy_is_internal(phydev) ?
>>                  XCVR_INTERNAL : XCVR_EXTERNAL;
>>      cmd->base.phy_address = phydev->mdio.addr;
> 
> Russell, the phylink has a similiar place where PORT_MII is set. I don't
> know
> if we'd have to change that, too.
> 
> Also, I wanted to look into the PHY_INTERFACE_MODE_MOCA thing and if we can
> get rid of the special case here and just set phydev->port to PORT_BNC
> in the
> driver. Florian, maybe you have a comment on this?

For GENET, it's simple because we can do this:

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index fcca023f22e5..34cbd008a3af 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -777,6 +777,8 @@ static int bcmgenet_get_link_ksettings(struct
net_device *dev,
                return -ENODEV;

        phy_ethtool_ksettings_get(dev->phydev, cmd);
+       if (dev->phydev->interface == PHY_INTERFACE_MODE_MOCA)
+               cmd->base.port = PORT_BNC;

        return 0;
 }

but for bcm_sf2.c, we would need to add plumbing between the DSA core
and the DSA driver in order to override the cmd structure with the
desired port and that would be most likely the only driver needing that,
should we really bother? There is also potentially a 3rd driver coming
down the road (bgmac) which would need to report MoCA/BNC.

I don't see this scaling very well nor being such a big issue to have
that in the PHYLIB and PHYLINK.
-- 
Florian