mbox series

[net-next,v1,0/7] port asix ax88772 to the PHYlib

Message ID 20210604134244.2467-1-o.rempel@pengutronix.de
Headers show
Series port asix ax88772 to the PHYlib | expand

Message

Oleksij Rempel June 4, 2021, 1:42 p.m. UTC 
Port ax88772 part of asix driver to the phylib to be able to use more
advanced external PHY attached to this controller.

Oleksij Rempel (7):
  net: usb: asix: ax88772_bind: use devm_kzalloc() instead of kzalloc()
  net: usb: asix: ax88772: add phylib support
  net: usb/phy: asix: add support for ax88772A/C PHYs
  net: usb: asix: ax88772: add generic selftest support
  net: usb: asix: add error handling for asix_mdio_* functions
  net: phy: do not print dump stack if device was removed
  usbnet: run unbind() before unregister_netdev()

 drivers/net/phy/ax88796b.c     |  74 ++++++++++++++++-
 drivers/net/phy/phy.c          |   3 +
 drivers/net/usb/Kconfig        |   2 +
 drivers/net/usb/asix.h         |  10 +++
 drivers/net/usb/asix_common.c  |  68 +++++++++++++--
 drivers/net/usb/asix_devices.c | 148 +++++++++++++++++++++++----------
 drivers/net/usb/ax88172a.c     |  14 ----
 drivers/net/usb/usbnet.c       |   6 +-
 8 files changed, 253 insertions(+), 72 deletions(-)

Comments

Andrew Lunn June 4, 2021, 11:24 p.m. UTC | #1 
On Fri, Jun 04, 2021 at 03:42:41PM +0200, Oleksij Rempel wrote:
> With working phylib support we are able now to use generic selftests.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew
Andrew Lunn June 4, 2021, 11:41 p.m. UTC | #2 
On Fri, Jun 04, 2021 at 03:42:44PM +0200, Oleksij Rempel wrote:
> unbind() is the proper place to disconnect PHY, but it will fail if
> netdev is already unregistered.

O.K, this partially answers the question i was about to ask for the
previous patch.

void phy_start(struct phy_device *phydev)
{
	mutex_lock(&phydev->lock);

	if (phydev->state != PHY_READY && phydev->state != PHY_HALTED) {
		WARN(1, "called from state %s\n",
		     phy_state_to_str(phydev->state));
		goto out;
	}

By skipping phy_error(), phydev->state is not set to PHY_HALTED. So if
you try to start the phy again, without disconnecting it, it looks
like there could be a problem.

But with this patch, i assume the PHY will always be disconnected and
later reconnected when the device is replugged.

      Andrew
Oleksij Rempel June 7, 2021, 7:55 a.m. UTC | #3 
On Sat, Jun 05, 2021 at 01:31:14AM +0200, Andrew Lunn wrote:
> > -void asix_mdio_write(struct net_device *netdev, int phy_id, int loc, int val)
> > +static int __asix_mdio_write(struct net_device *netdev, int phy_id, int loc,
> > +			     int val)
> >  {
> >  	struct usbnet *dev = netdev_priv(netdev);
> >  	__le16 res = cpu_to_le16(val);
> > @@ -517,13 +522,24 @@ void asix_mdio_write(struct net_device *netdev, int phy_id, int loc, int val)
> >  	} while (!(smsr & AX_HOST_EN) && (i++ < 30) && (ret != -ENODEV));
> >  	if (ret == -ENODEV) {
> >  		mutex_unlock(&dev->phy_mutex);
> > -		return;
> > +		return ret;
> 
> Now that you have added an out: it might be better to use a goto?

ack, done

> Otherwise
> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> 
>     Andrew
> 
>
Oleksij Rempel June 7, 2021, 8:04 a.m. UTC | #4 
On Sat, Jun 05, 2021 at 01:41:15AM +0200, Andrew Lunn wrote:
> On Fri, Jun 04, 2021 at 03:42:44PM +0200, Oleksij Rempel wrote:
> > unbind() is the proper place to disconnect PHY, but it will fail if
> > netdev is already unregistered.
> 
> O.K, this partially answers the question i was about to ask for the
> previous patch.
> 
> void phy_start(struct phy_device *phydev)
> {
> 	mutex_lock(&phydev->lock);
> 
> 	if (phydev->state != PHY_READY && phydev->state != PHY_HALTED) {
> 		WARN(1, "called from state %s\n",
> 		     phy_state_to_str(phydev->state));
> 		goto out;
> 	}
> 
> By skipping phy_error(), phydev->state is not set to PHY_HALTED. So if
> you try to start the phy again, without disconnecting it, it looks
> like there could be a problem.
> 
> But with this patch, i assume the PHY will always be disconnected and
> later reconnected when the device is replugged.

Yes. The PHY is disconnected and the PHY driver is unbinded.

Regards,
Oleksij