linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Oleksij Rempel <o.rempel@pengutronix.de>
To: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Heiner Kallweit <hkallweit1@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>, Andrew Lunn <andrew@lunn.ch>,
	Russell King <linux@armlinux.org.uk>,
	kernel@pengutronix.de, linux-kernel@vger.kernel.org,
	linux-usb@vger.kernel.org, netdev@vger.kernel.org
Subject: Re: [PATCH net-next v2 4/8] net: usb: asix: ax88772: add phylib support
Date: Wed, 23 Jun 2021 09:06:18 +0200	[thread overview]
Message-ID: <20210623070618.nfv4yizuijbrv575@pengutronix.de> (raw)
In-Reply-To: <2d0bdf2e-49bc-60c0-789e-b909cf1e2667@samsung.com>

Hi Marek,

On Mon, Jun 21, 2021 at 08:05:49AM +0200, Marek Szyprowski wrote:
> Hi Oleksij,
> 
> On 18.06.2021 15:20, Oleksij Rempel wrote:
> > On Fri, Jun 18, 2021 at 01:11:41PM +0200, Marek Szyprowski wrote:
> >> On 18.06.2021 13:04, Heiner Kallweit wrote:
> >>> On 18.06.2021 12:13, Oleksij Rempel wrote:
> >>>> thank you for your feedback.
> >>>>
> >>>> On Fri, Jun 18, 2021 at 10:39:12AM +0200, Marek Szyprowski wrote:
> >>>>> On 07.06.2021 10:27, Oleksij Rempel wrote:
> >>>>>> To be able to use ax88772 with external PHYs and use advantage of
> >>>>>> existing PHY drivers, we need to port at least ax88772 part of asix
> >>>>>> driver to the phylib framework.
> >>>>>>
> >>>>>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> >>>>> I found one more issue with this patch. On one of my test boards
> >>>>> (Samsung Exynos5250 SoC based Arndale) system fails to establish network
> >>>>> connection just after starting the kernel when the driver is build-in.
> >>>>>
> >>> If you build in the MAC driver, do you also build in the PHY driver?
> >>> If the PHY driver is still a module this could explain why genphy
> >>> driver is used.
> >>> And your dmesg filtering suppresses the phy_attached_info() output
> >>> that would tell us the truth.
> >> Here is a bit more complete log:
> >>
> >> # dmesg | grep -i Asix
> >> [    2.412966] usbcore: registered new interface driver asix
> >> [    4.620094] usb 1-3.2.4: Manufacturer: ASIX Elec. Corp.
> >> [    4.641797] asix 1-3.2.4:1.0 (unnamed net_device) (uninitialized):
> >> invalid hw address, using random
> >> [    5.657009] libphy: Asix MDIO Bus: probed
> >> [    5.750584] Asix Electronics AX88772A usb-001:004:10: attached PHY
> >> driver (mii_bus:phy_addr=usb-001:004:10, irq=POLL)
> >> [    5.763908] asix 1-3.2.4:1.0 eth0: register 'asix' at
> >> usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet, fe:a5:29:e2:97:3e
> >> [    9.090270] asix 1-3.2.4:1.0 eth0: Link is Up - 100Mbps/Full - flow
> >> control off
> >>
> >> This seems to be something different than missing PHY driver.
> > Can you please test it:
> >
> > diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
> > index aec97b021a73..7897108a1a42 100644
> > --- a/drivers/net/usb/asix_devices.c
> > +++ b/drivers/net/usb/asix_devices.c
> > @@ -453,6 +453,7 @@ static int ax88772a_hw_reset(struct usbnet *dev, int in_pm)
> >   	u16 rx_ctl, phy14h, phy15h, phy16h;
> >   	u8 chipcode = 0;
> >   
> > +	netdev_info(dev->net, "ax88772a_hw_reset\n");
> >   	ret = asix_write_gpio(dev, AX_GPIO_RSE, 5, in_pm);
> >   	if (ret < 0)
> >   		goto out;
> > @@ -509,31 +510,7 @@ static int ax88772a_hw_reset(struct usbnet *dev, int in_pm)
> >   			goto out;
> >   		}
> >   	} else if ((chipcode & AX_CHIPCODE_MASK) == AX_AX88772A_CHIPCODE) {
> > -		/* Check if the PHY registers have default settings */
> > -		phy14h = asix_mdio_read_nopm(dev->net, dev->mii.phy_id,
> > -					     AX88772A_PHY14H);
> > -		phy15h = asix_mdio_read_nopm(dev->net, dev->mii.phy_id,
> > -					     AX88772A_PHY15H);
> > -		phy16h = asix_mdio_read_nopm(dev->net, dev->mii.phy_id,
> > -					     AX88772A_PHY16H);
> > -
> > -		netdev_dbg(dev->net,
> > -			   "772a_hw_reset: MR20=0x%x MR21=0x%x MR22=0x%x\n",
> > -			   phy14h, phy15h, phy16h);
> > -
> > -		/* Restore PHY registers default setting if not */
> > -		if (phy14h != AX88772A_PHY14H_DEFAULT)
> > -			asix_mdio_write_nopm(dev->net, dev->mii.phy_id,
> > -					     AX88772A_PHY14H,
> > -					     AX88772A_PHY14H_DEFAULT);
> > -		if (phy15h != AX88772A_PHY15H_DEFAULT)
> > -			asix_mdio_write_nopm(dev->net, dev->mii.phy_id,
> > -					     AX88772A_PHY15H,
> > -					     AX88772A_PHY15H_DEFAULT);
> > -		if (phy16h != AX88772A_PHY16H_DEFAULT)
> > -			asix_mdio_write_nopm(dev->net, dev->mii.phy_id,
> > -					     AX88772A_PHY16H,
> > -					     AX88772A_PHY16H_DEFAULT);
> > +		netdev_info(dev->net, "do not touch PHY regs\n");
> >   	}
> >   
> >   	ret = asix_write_cmd(dev, AX_CMD_WRITE_IPG0,
> 
> This doesn't help for this issue.

Ok.
So far I was not able to see obvious differences between:
probe -> ip link set dev eth1 up

and

probe -> ip link set dev eth1 up;
	 ip link set dev eth1 down;
	 ip link set dev eth1 up


Except of PHY sate. By default the PHY is in resumed state after probe
and is able to negotiate the link even if the MAC is down.
After ip link set dev eth1 down, the PHY is in suspend state, as
expected.

Can you please test this change?

diff --git a/drivers/net/usb/asix_devices.c b/drivers/net/usb/asix_devices.c
index aec97b021a73..2c115216420a 100644
--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -701,6 +701,7 @@ static int ax88772_init_phy(struct usbnet *dev)
 		return ret;
 	}
 
+	phy_suspend(priv->phydev);
 	priv->phydev->mac_managed_pm = 1;
 
 	phy_attached_info(priv->phydev);

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

  reply	other threads:[~2021-06-23  7:06 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-07  8:27 [PATCH net-next v2 0/8] port asix ax88772 to the PHYlib Oleksij Rempel
2021-06-07  8:27 ` [PATCH net-next v2 1/8] net: usb: asix: ax88772_bind: use devm_kzalloc() instead of kzalloc() Oleksij Rempel
2021-06-07  8:27 ` [PATCH net-next v2 2/8] net: usb: asix: refactor asix_read_phy_addr() and handle errors on return Oleksij Rempel
2021-06-07  8:27 ` [PATCH net-next v2 3/8] net: usb/phy: asix: add support for ax88772A/C PHYs Oleksij Rempel
2021-06-07  8:27 ` [PATCH net-next v2 4/8] net: usb: asix: ax88772: add phylib support Oleksij Rempel
     [not found]   ` <CGME20210609095923eucas1p2e692c9a482151742d543316c91f29802@eucas1p2.samsung.com>
2021-06-09  9:59     ` Marek Szyprowski
2021-06-09 12:46       ` Oleksij Rempel
2021-06-09 13:12         ` Heiner Kallweit
2021-06-10 13:36           ` Oleksij Rempel
2021-06-10 10:31         ` Marek Szyprowski
2021-06-10 12:54       ` Jon Hunter
2021-06-10 14:22         ` Oleksij Rempel
     [not found]   ` <CGME20210618083914eucas1p240f88e7064a7bf15b68370b7506d24a9@eucas1p2.samsung.com>
2021-06-18  8:39     ` Marek Szyprowski
2021-06-18 10:13       ` Oleksij Rempel
2021-06-18 10:45         ` Marek Szyprowski
2021-06-18 10:57           ` Marek Szyprowski
2021-06-18 13:10             ` Oleksij Rempel
2021-06-18 11:04         ` Heiner Kallweit
2021-06-18 11:11           ` Marek Szyprowski
2021-06-18 13:20             ` Oleksij Rempel
2021-06-21  6:05               ` Marek Szyprowski
2021-06-23  7:06                 ` Oleksij Rempel [this message]
2021-06-28  8:27                   ` Marek Szyprowski
2021-06-07  8:27 ` [PATCH net-next v2 5/8] net: usb: asix: ax88772: add generic selftest support Oleksij Rempel
2021-06-07  8:27 ` [PATCH net-next v2 6/8] net: usb: asix: add error handling for asix_mdio_* functions Oleksij Rempel
2021-06-07  8:27 ` [PATCH net-next v2 7/8] net: phy: do not print dump stack if device was removed Oleksij Rempel
2021-06-07  8:27 ` [PATCH net-next v2 8/8] usbnet: run unbind() before unregister_netdev() Oleksij Rempel
2021-06-07 20:40 ` [PATCH net-next v2 0/8] port asix ax88772 to the PHYlib patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210623070618.nfv4yizuijbrv575@pengutronix.de \
    --to=o.rempel@pengutronix.de \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=hkallweit1@gmail.com \
    --cc=kernel@pengutronix.de \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=m.szyprowski@samsung.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).