Re: [PATCH v2] lan743x: add virtual PHY for PHY-less devices

[Sergej Bauer <sbauer@blackbox.su>]

On Saturday, January 23, 2021 2:23:25 AM MSK Andrew Lunn wrote:
> > > > @@ -1000,8 +1005,10 @@ static void lan743x_phy_close(struct
> > > > lan743x_adapter *adapter)>
> > > > 
> > > >  	struct net_device *netdev = adapter->netdev;
> > > >  	
> > > >  	phy_stop(netdev->phydev);
> > > > 
> > > > -	phy_disconnect(netdev->phydev);
> > > > -	netdev->phydev = NULL;
> > > > +	if (phy_is_pseudo_fixed_link(netdev->phydev))
> > > > +		lan743x_virtual_phy_disconnect(netdev->phydev);
> > > > +	else
> > > > +		phy_disconnect(netdev->phydev);
> > > 
> > > phy_disconnect() should work. You might want to call
> 
> There are drivers which call phy_disconnect() on a fixed_link. e.g.
> 
> https://elixir.bootlin.com/linux/v5.11-rc4/source/drivers/net/usb/lan78xx.c#
> L3555.
> 
> It could be your missing call to fixed_phy_unregister() is leaving
> behind bad state.
> 
fixed_phy_unregister() were inside of lan743x_virtual_phy_disconnect()

> > It was to make ethtool show full set of supported speeds and MII only in
> > supported ports (without TP and the no any ports in the bare card).
> 
> But fixed link does not support the full set of speed. It is fixed. It
> supports only one speed it is configured with.  And by setting it
> wrongly, you are going to allow the user to do odd things, like use
> ethtool force the link speed to a speed which is not actually
> supported.
when writing virtual phy i have used "Microchip AN2948 Configuration Registers
of LAN743x" document and the lan743x is designed only for LAN7430 either
LAN7431 as it pointed in the document and in lan743x_pcidev_tbl. which both
support speeds 10/100/1000 Mbps.


-- 
                                Regards,
                                    Sergej.