mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: "huangguangbin (A)" <huangguangbin2@huawei.com>
Cc: davem@davemloft.net, kuba@kernel.org, o.rempel@pengutronix.de,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	lipeng321@huawei.com, chenhao288@hisilicon.com
Subject: Re: [PATCH] net: phy: genphy_loopback: fix loopback failed when speed is unknown
Date: Thu, 31 Mar 2022 16:26:47 +0200	[thread overview]
Message-ID: <YkW6J9rM6O/cb/lv@lunn.ch> (raw)
In-Reply-To: <130bb780-0dc1-3819-8f6d-f2daf4d9ece9@huawei.com>

> In this case, as speed and duplex both are unknown, ctl is just set to 0x4000.
> However, the follow code sets mask to ~0 for function phy_modify():
> int genphy_loopback(struct phy_device *phydev, bool enable)
> {
> 	if (enable) {
> 		...
> 		phy_modify(phydev, MII_BMCR, ~0, ctl);
> 		...
> }
> so all other bits of BMCR will be cleared and just set bit 14, I use phy trace to
> prove that:
> 
> $ cat /sys/kernel/debug/tracing/trace
> # tracer: nop
> #
> # entries-in-buffer/entries-written: 923/923   #P:128
> #
> #                                _-----=> irqs-off/BH-disabled
> #                               / _----=> need-resched
> #                              | / _---=> hardirq/softirq
> #                              || / _--=> preempt-depth
> #                              ||| / _-=> migrate-disable
> #                              |||| /     delay
> #           TASK-PID     CPU#  |||||  TIMESTAMP  FUNCTION
> #              | |         |   |||||     |         |
>   kworker/u257:2-694     [015] .....   209.263912: mdio_access: mii-0000:bd:00.1 read  phy:0x03 reg:0x00 val:0x1040
>   kworker/u257:2-694     [015] .....   209.263951: mdio_access: mii-0000:bd:00.1 read  phy:0x03 reg:0x01 val:0x7989
>   kworker/u257:2-694     [015] .....   209.263990: mdio_access: mii-0000:bd:00.1 read  phy:0x03 reg:0x01 val:0x7989
>   kworker/u257:2-694     [015] .....   209.264028: mdio_access: mii-0000:bd:00.1 read  phy:0x03 reg:0x09 val:0x0200
>   kworker/u257:2-694     [015] .....   209.264067: mdio_access: mii-0000:bd:00.1 read  phy:0x03 reg:0x0a val:0x0000
>          ethtool-1148    [007] .....   209.665693: mdio_access: mii-0000:bd:00.1 read  phy:0x03 reg:0x00 val:0x1040
>          ethtool-1148    [007] .....   209.665706: mdio_access: mii-0000:bd:00.1 write phy:0x03 reg:0x00 val:0x1840
>          ethtool-1148    [007] .....   210.588139: mdio_access: mii-0000:bd:00.1 read  phy:0x03 reg:0x00 val:0x1840
>          ethtool-1148    [007] .....   210.588152: mdio_access: mii-0000:bd:00.1 write phy:0x03 reg:0x00 val:0x1040
>          ethtool-1148    [007] .....   210.615900: mdio_access: mii-0000:bd:00.1 read  phy:0x03 reg:0x00 val:0x1040
>          ethtool-1148    [007] .....   210.615912: mdio_access: mii-0000:bd:00.1 write phy:0x03 reg:0x00 val:0x4000 //here just set bit 14
> 
> So phy speed will be set to 10M in this case, if previous speed of
> device before going down is 10M, loopback test is pass. Only
> previous speed is 100M or 1000M, loopback test is failed.

O.K. So it should be set into 10M half duplex. But why does this cause
it not to loopback packets? Does the PHY you are using not actually
support 10 Half? Why does it need to be the same speed as when the
link was up? And why does it actually set LSTATUS indicating there is
link?

Is this a generic problem, all PHYs are like this, or is this specific
to the PHY you are using? Maybe this PHY needs its own loopback
function because it does something odd?

   Andrew


  reply	other threads:[~2022-03-31 14:27 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-31 11:48 Guangbin Huang
2022-03-31 12:23 ` Andrew Lunn
2022-03-31 13:57   ` huangguangbin (A)
2022-03-31 14:26     ` Andrew Lunn [this message]
2022-04-01  6:40       ` Oleksij Rempel
2022-04-01 12:14         ` Andrew Lunn
2022-04-07 13:54           ` huangguangbin (A)
2022-04-07 14:45             ` Andrew Lunn
2022-04-08  8:18               ` Oleksij Rempel
2022-04-08 13:04                 ` Andrew Lunn

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=YkW6J9rM6O/cb/lv@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=chenhao288@hisilicon.com \
    --cc=davem@davemloft.net \
    --cc=huangguangbin2@huawei.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lipeng321@huawei.com \
    --cc=netdev@vger.kernel.org \
    --cc=o.rempel@pengutronix.de \
    /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

all inboxes | Powered by JetHome®