Re: Toggling link state breaks network connectivity
On 15/06/2017 12:19, Måns Rullgård wrote: > Now I did notice one thing. When the interrupts from the loopback > frames are handled, the rx interrupt is all but disabled for NAPI poll > mode. Of course, NAPI isn't active, so the rx interrupt never gets > re-enabled. We should probably do this in ndo_open just to be sure. I have tried everything I could think of (whether reasonable, or just random). Setting the link down always hoses the board's network connectivity, 100% reproducible on Vantage 1218. The single HW difference that I'm aware of is that the Atheros PHY used to be clocked at 25 MHz (I think) on 1172 and 1191 boards, but it is clocked at 100 MHz on this 1218 board. I'll try tweaking this setting. For the time being, my test script is: ip addr add 172.27.64.77/18 brd 172.27.127.255 dev eth0 ip link set eth0 up sleep 6 ## autoneg should be complete ntpdate 172.27.64.1 sleep 1 date +%T && ping -c 3 172.27.64.1 > /tmp/ping_before sleep 1 ip link set eth0 down sleep 2 ip link set eth0 up sleep 5 date +%T && ping -c 3 172.27.64.1 > /tmp/ping_after # cat /tmp/ping_before PING 172.27.64.1 (172.27.64.1): 56 data bytes 64 bytes from 172.27.64.1: seq=0 ttl=64 time=31.442 ms 64 bytes from 172.27.64.1: seq=1 ttl=64 time=31.359 ms 64 bytes from 172.27.64.1: seq=2 ttl=64 time=31.379 ms --- 172.27.64.1 ping statistics --- 3 packets transmitted, 3 packets received, 0% packet loss round-trip min/avg/max = 31.359/31.393/31.442 ms => These RTT numbers are crazy high for a LAN connection through a switch. (I get 0.250 ms from my desktop.) => What the server saw was: 15:23:30.400066 IP 172.27.64.77 > 172.27.64.1: ICMP echo request, id 41987, seq 0, length 64 15:23:30.400094 IP 172.27.64.1 > 172.27.64.77: ICMP echo reply, id 41987, seq 0, length 64 15:23:31.431479 IP 172.27.64.77 > 172.27.64.1: ICMP echo request, id 41987, seq 1, length 64 15:23:31.431500 IP 172.27.64.1 > 172.27.64.77: ICMP echo reply, id 41987, seq 1, length 64 15:23:32.462844 IP 172.27.64.77 > 172.27.64.1: ICMP echo request, id 41987, seq 2, length 64 15:23:32.462867 IP 172.27.64.1 > 172.27.64.77: ICMP echo reply, id 41987, seq 2, length 64 # cat /tmp/ping_after PING 172.27.64.1 (172.27.64.1): 56 data bytes --- 172.27.64.1 ping statistics --- 3 packets transmitted, 0 packets received, 100% packet loss => What the server saw was: 15:23:41.180068 ARP, Request who-has 172.27.64.1 tell 172.27.64.77, length 46 15:23:41.180081 ARP, Reply 172.27.64.1 is-at 00:15:17:24:e0:81, length 28 15:23:42.194409 ARP, Request who-has 172.27.64.1 tell 172.27.64.77, length 46 15:23:42.194419 ARP, Reply 172.27.64.1 is-at 00:15:17:24:e0:81, length 28 15:23:43.207700 ARP, Request who-has 172.27.64.1 tell 172.27.64.77, length 46 15:23:43.207710 ARP, Reply 172.27.64.1 is-at 00:15:17:24:e0:81, length 28 Below are the "raw" logs (of what I thought might be useful) Regards. [0.898538] libphy: Fixed MDIO Bus: probed [0.902734] ENTER nb8800_probe [0.905863] ++ETH++ gw8 0xf0026424 0x00 [0.919601] ++ETH++ gw8 0xf0026424 0x01 [0.923550] ++ETH++ gw16 0xf0026420 0x0050 [0.927749] libphy: nb8800-mii: probed [0.933736] ENTER nb8800_hw_init [0.936991] ++ETH++ gw8 0xf0026000 0x1c [0.940952] ++ETH++ gw8 0xf0026001 0x05 [0.944897] ++ETH++ gw8 0xf0026004 0x22 [0.948839] ++ETH++ gw8 0xf0026008 0x04 [0.952792] ++ETH++ gw8 0xf0026014 0x0c [0.956740] ++ETH++ gw8 0xf0026051 0x00 [0.960693] ++ETH++ gw8 0xf0026052 0xff [0.964637] ++ETH++ gw8 0xf0026054 0x40 [0.968579] ++ETH++ gw32 0xf0026100 0x02fe [0.973058] ++ETH++ gw32 0xf0026118 0x003cc4a4 [0.977527] ++ETH++ gw32 0xf0026200 0x02fe [0.981994] ++ETH++ gw32 0xf0026218 0x4dc8 [0.986473] ++ETH++ gw8 0xf002602e 0x00 [0.990417] EXIT nb8800_hw_init [0.993576] ++ETH++ gw8 0xf0026400 0x01 [0.997518] ++ETH++ gw32 0xf0026200 0x028e [1.001988] ++ETH++ gw8 0xf002606a 0x00 [1.005930] ++ETH++ gw8 0xf002606b 0x16 [1.009873] ++ETH++ gw8 0xf002606c 0xe8 [1.013815] ++ETH++ gw8 0xf002606d 0x4d [1.017757] ++ETH++ gw8 0xf002606e 0x7f [1.021699] ++ETH++ gw8 0xf002606f 0xc4 [1.025641] ++ETH++ gw8 0xf002603c 0x00 [1.029584] ++ETH++ gw8 0xf002603d 0x16 [1.033526] ++ETH++ gw8 0xf002603e 0xe8 [1.037468] ++ETH++ gw8 0xf002603f 0x4d [1.041411] ++ETH++ gw8 0xf0026040 0x7f [1.045354] ++ETH++ gw8 0xf0026041 0xc4 [1.049772] nb8800 26000.ethernet eth0: MAC address 00:16:e8:4d:7f:c4 [1.056259] EXIT nb8800_probe # ip addr add 172.27.64.77/18 brd 172.27.127.255 dev eth0 # ip link set eth0 up [ 16.172093] ENTER nb8800_open [ 16.175096] ++ETH++ gw32 0xf0026204 0x000f [ 16.179574] ++ETH++ gw32 0xf0026104 0x000f [ 16.184063] ENTER nb8800_dma_init [ 16.188000] ++ETH++ gw32 0xf002620c 0x9dc4c000 [ 16.192478] EXIT nb8800_dma_init [ 16.195851] ++ETH++ gw32 0xf0026218 0x4dc8 [ 16.200334] ++ETH++ gw8 0xf0026004 0x23 [ 16.204280] ++ETH++ gw8 0x
Quirks of the Atheros 8035 PHY
Hello, I've discussed this subject in the past, but we never reached a conclusion, AFAIR. The Atheros 8035 GigE PHY has IMO 2 quirks wrt to clock delays. https://www.redeszone.net/app/uploads/2014/04/AR8035.pdf 1) RX clock delay Commit 2e5f9f281ee8369f56d403b4a52942f19b6978f8 In fact, RX clock delay is *ENABLED* by default at reset. So if a board actually required it *disabled* then we actually need to set the bit to 0. Reference: 4.2.25 rgmii rx clock delay control 2) TX clock delay Commit 1ca6d1b1aef4628ff0fe458135ddb008d134ad8f TX clock delay is DISABLED by default, so no surprises there... except that it is only DISABLED after *HW reset*. After a SW reset, such as that performed in Linux IIUC, the PHY retains the value previously set. So if a bootloader such a Uboot enabled TX delay, then Linux would "inherit" the setting, even if it performs a reset. If the PHY setting calls for no TX clock delay, the Linux driver would have to actively disable it. Reference: 4.2.26 rgmii tx clock delay control I can (of course) send a patch fixing both issues, but what was said last time was that "it's too late to fix it now, since the fix risks breaking working setups". Is that an accurate paraphrase? 3) There's also a RGMII GTX_CLK documented as "RGMII transmit clock, 125 MHz digital. Adding a 22 ohm damping resistor is recommended for EMI design near MAC side" => Is that TX_CLK? There's a 2-bit related field called Gtx_dly_val which allows tweaking the delay Select the delay of gtx_clk. 00: 0.25ns 01: 1.3ns 10: 2.4ns 11: 3.4ns (DEFAULT 10b = 2.4 ns, BUT Retain val on SW reset, so inherit bootloader value) I'm not sure the current DT allows such fine-grained tweaking? Should we enhance it? 4) There's the whole mess of having clock delays supported both by the PHY *AND* the MAC. If both add a delay, things won't work as expected. What's the recommended approach there? Regards.
Re: Quirks of the Atheros 8035 PHY
Mugunthan's address bounces. Removing it. Adding Grygorii's address instead. On 14/07/2017 23:08, Mason wrote: > Hello, > > I've discussed this subject in the past, but we never reached > a conclusion, AFAIR. > > The Atheros 8035 GigE PHY has IMO 2 quirks wrt to clock delays. > > https://www.redeszone.net/app/uploads/2014/04/AR8035.pdf > > > 1) RX clock delay > > Commit 2e5f9f281ee8369f56d403b4a52942f19b6978f8 > > In fact, RX clock delay is *ENABLED* by default at > reset. So if a board actually required it *disabled* > then we actually need to set the bit to 0. > > Reference: > 4.2.25 rgmii rx clock delay control > > > 2) TX clock delay > > Commit 1ca6d1b1aef4628ff0fe458135ddb008d134ad8f > > TX clock delay is DISABLED by default, so no surprises > there... except that it is only DISABLED after *HW reset*. > > After a SW reset, such as that performed in Linux IIUC, > the PHY retains the value previously set. > > So if a bootloader such a Uboot enabled TX delay, then > Linux would "inherit" the setting, even if it performs > a reset. If the PHY setting calls for no TX clock delay, > the Linux driver would have to actively disable it. > > Reference: > 4.2.26 rgmii tx clock delay control > > > I can (of course) send a patch fixing both issues, but > what was said last time was that "it's too late to > fix it now, since the fix risks breaking working > setups". Is that an accurate paraphrase? > > > 3) There's also a RGMII GTX_CLK documented as > "RGMII transmit clock, 125 MHz digital. Adding a 22 ohm > damping resistor is recommended for EMI design near MAC side" > => Is that TX_CLK? > There's a 2-bit related field called Gtx_dly_val > which allows tweaking the delay > > Select the delay of gtx_clk. > 00: 0.25ns > 01: 1.3ns > 10: 2.4ns > 11: 3.4ns > (DEFAULT 10b = 2.4 ns, BUT Retain val on SW reset, > so inherit bootloader value) > I'm not sure the current DT allows such fine-grained > tweaking? Should we enhance it? > > > 4) There's the whole mess of having clock delays > supported both by the PHY *AND* the MAC. If both > add a delay, things won't work as expected. > What's the recommended approach there? > > > Regards.
Re: Quirks of the Atheros 8035 PHY
On 14/07/2017 23:28, Florian Fainelli wrote:
> On 07/14/2017 02:08 PM, Mason wrote:
>
>> I've discussed this subject in the past, but we never reached
>> a conclusion, AFAIR.
>>
>> The Atheros 8035 GigE PHY has IMO 2 quirks wrt to clock delays.
>>
>> https://www.redeszone.net/app/uploads/2014/04/AR8035.pdf
>>
>>
>> 1) RX clock delay
>>
>> Commit 2e5f9f281ee8369f56d403b4a52942f19b6978f8
>>
>> In fact, RX clock delay is *ENABLED* by default at
>> reset. So if a board actually required it *disabled*
>> then we actually need to set the bit to 0.
>>
>> Reference:
>> 4.2.25 rgmii rx clock delay control
>>
>>
>> 2) TX clock delay
>>
>> Commit 1ca6d1b1aef4628ff0fe458135ddb008d134ad8f
>>
>> TX clock delay is DISABLED by default, so no surprises
>> there... except that it is only DISABLED after *HW reset*.
>>
>> After a SW reset, such as that performed in Linux IIUC,
>> the PHY retains the value previously set.
>>
>> So if a bootloader such a Uboot enabled TX delay, then
>> Linux would "inherit" the setting, even if it performs
>> a reset. If the PHY setting calls for no TX clock delay,
>> the Linux driver would have to actively disable it.
>>
>> Reference:
>> 4.2.26 rgmii tx clock delay control
>>
>>
>> I can (of course) send a patch fixing both issues, but
>> what was said last time was that "it's too late to
>> fix it now, since the fix risks breaking working
>> setups". Is that an accurate paraphrase?
>
> More or less, this particular problematic PHY has come up with some many
> different platforms, and people and typically no one being able to have
> insights on its internal design that it is really hard to comment on
> what would break, it's already apparently pretty broken.
When you say broken, do you mean the situation,
or the HW? I don't think the HW is broken.
The API is confusing, and violates the principle of
least astonishment, which led several SW devs to make
implementation errors, but the documentation is
pretty clear and well-written, as far as docs go.
The fix is straight-forward:
if (PHY_INTERFACE_MODE_RGMII)
disable_rx_delay;
disable_tx_delay;
if (PHY_INTERFACE_MODE_RGMII_RXID)
enable_rx_delay;
disable_tx_delay;
if (PHY_INTERFACE_MODE_RGMII_TXID)
disable_rx_delay;
enable_tx_delay;
if (PHY_INTERFACE_MODE_RGMII_ID)
enable_rx_delay;
enable_tx_delay;
Then the setting will be as requested in the DT.
>> 3) There's also a RGMII GTX_CLK documented as
>> "RGMII transmit clock, 125 MHz digital. Adding a 22 ohm
>> damping resistor is recommended for EMI design near MAC side"
>> => Is that TX_CLK?
>> There's a 2-bit related field called Gtx_dly_val
>> which allows tweaking the delay
>>
>> Select the delay of gtx_clk.
>> 00: 0.25ns
>> 01: 1.3ns
>> 10: 2.4ns
>> 11: 3.4ns
>> (DEFAULT 10b = 2.4 ns, BUT Retain val on SW reset,
>> so inherit bootloader value)
>> I'm not sure the current DT allows such fine-grained
>> tweaking? Should we enhance it?
>
> What is "the current DT" in that context? There is no binding for
> at8033x defined and there is not one either for nb8800.
I meant
Documentation/devicetree/bindings/net/ethernet.txt
Documentation/devicetree/bindings/net/phy.txt
There is no prop to define the required delay,
in case the specific PHY supports multiple
delays, as the AR8035 seems to do.
This is the current DT
http://elixir.free-electrons.com/linux/latest/source/arch/arm/boot/dts/tango4-vantage-1172.dts
ð0 {
phy-connection-type = "rgmii";
phy-handle = <ð0_phy>;
#address-cells = <1>;
#size-cells = <0>;
/* Atheros AR8035 */
eth0_phy: ethernet-phy@4 {
compatible = "ethernet-phy-id004d.d072",
"ethernet-phy-ieee802.3-c22";
interrupts = <37 IRQ_TYPE_EDGE_RISING>;
reg = <4>;
};
};
phy-connection-type is wrong, since I need both delays.
So I will change to rgmii-id, and fix the MAC driver to
honor the doc blurb:
* "rgmii" (RX and TX delays are added by the MAC when required)
* "rgmii-id" (RGMII with internal RX and TX delays provided by the PHY, the
MAC should not add the RX or TX delays in this case)
* "rgmii-rxid" (RGMII with internal RX delay provided by the PHY, the MAC
should not add an RX delay in this case)
* "rgmii-txid" (RGMII with internal TX delay provided by the PHY, the MAC
should not add an TX delay in this
Re: [PATCH 2/2] net: ethernet: nb8800: Fix RGMII TX clock delay setup
On 19/07/2017 19:17, Måns Rullgård wrote:
> Marc Gonzalez writes:
>
>> According to commit e5f3a4a56ce2a707b2fb8ce37e4414dcac89c672
>> ("Documentation: devicetree: clarify usage of the RGMII phy-modes")
>> there are 4 RGMII phy-modes to handle:
>>
>> "rgmii" (RX and TX delays are added by the MAC when required)
>> "rgmii-id" (RGMII with internal RX and TX delays provided by the PHY,
>> the MAC should not add the RX or TX delays in this case)
>> "rgmii-rxid" (RGMII with internal RX delay provided by the PHY,
>> the MAC should not add an RX delay in this case)
>> "rgmii-txid" (RGMII with internal TX delay provided by the PHY,
>> the MAC should not add an TX delay in this case)
>>
>> Let the MAC handle TX clock delay for rgmii and rgmii-rxid.
>>
>> Signed-off-by: Marc Gonzalez
>> ---
>> drivers/net/ethernet/aurora/nb8800.c | 8 +---
>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/aurora/nb8800.c
>> b/drivers/net/ethernet/aurora/nb8800.c
>> index 041cfb7952f8..f3ed320eb4ad 100644
>> --- a/drivers/net/ethernet/aurora/nb8800.c
>> +++ b/drivers/net/ethernet/aurora/nb8800.c
>> @@ -609,7 +609,7 @@ static void nb8800_mac_config(struct net_device *dev)
>> mac_mode |= HALF_DUPLEX;
>>
>> if (gigabit) {
>> -if (priv->phy_mode == PHY_INTERFACE_MODE_RGMII)
>> +if (phy_interface_is_rgmii(dev->phydev))
>> mac_mode |= RGMII_MODE;
>>
>> mac_mode |= GMAC_MODE;
>
> This is a separate issue, and the change is obviously correct.
>
>> @@ -1268,11 +1268,13 @@ static int nb8800_tangox_init(struct net_device *dev)
>> break;
>>
>> case PHY_INTERFACE_MODE_RGMII:
>> -pad_mode = PAD_MODE_RGMII;
>> +case PHY_INTERFACE_MODE_RGMII_RXID:
>> +pad_mode = PAD_MODE_RGMII | PAD_MODE_GTX_CLK_DELAY;
>> break;
>>
>> +case PHY_INTERFACE_MODE_RGMII_ID:
>> case PHY_INTERFACE_MODE_RGMII_TXID:
>> -pad_mode = PAD_MODE_RGMII | PAD_MODE_GTX_CLK_DELAY;
>> +pad_mode = PAD_MODE_RGMII;
>> break;
>
> Won't this just make it break in a different set of circumstances?
I don't think so, and here's my reasoning:
AFAIU, the HW block always requires a TX clock delay
(I don't know what the "safe" interval is. PHY adds
2.4 ns, MAC adds ~1 ns, both work.)
RX clock delay seems to be "Don't Care" (tested both
enabled and disabled by PHY)
By "tested", I mean ability to ping remote system.
If phy-mode is RGMII or RGMII_RXID, then don't add
TX clock delay from PHY, therefore add it from MAC.
If phy_mode is RGMII_ID or RGMII_TXID, then do add
TX clock delay from PHY, therefore don't add it from MAC.
What set of circumstances would create an issue?
Regards.
Re: [PATCH 2/2] net: ethernet: nb8800: Fix RGMII TX clock delay setup
On 19/07/2017 20:30, Florian Fainelli wrote:
> On 07/19/2017 10:36 AM, Mason wrote:
>> On 19/07/2017 19:17, Måns Rullgård wrote:
>>
>>> Marc Gonzalez writes:
>>>
>>>> According to commit e5f3a4a56ce2a707b2fb8ce37e4414dcac89c672
>>>> ("Documentation: devicetree: clarify usage of the RGMII phy-modes")
>>>> there are 4 RGMII phy-modes to handle:
>>>>
>>>> "rgmii" (RX and TX delays are added by the MAC when required)
>>>> "rgmii-id" (RGMII with internal RX and TX delays provided by the PHY,
>>>>the MAC should not add the RX or TX delays in this case)
>>>> "rgmii-rxid" (RGMII with internal RX delay provided by the PHY,
>>>>the MAC should not add an RX delay in this case)
>>>> "rgmii-txid" (RGMII with internal TX delay provided by the PHY,
>>>>the MAC should not add an TX delay in this case)
>>>>
>>>> Let the MAC handle TX clock delay for rgmii and rgmii-rxid.
>>>>
>>>> Signed-off-by: Marc Gonzalez
>>>> ---
>>>> drivers/net/ethernet/aurora/nb8800.c | 8 +---
>>>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/net/ethernet/aurora/nb8800.c
>>>> b/drivers/net/ethernet/aurora/nb8800.c
>>>> index 041cfb7952f8..f3ed320eb4ad 100644
>>>> --- a/drivers/net/ethernet/aurora/nb8800.c
>>>> +++ b/drivers/net/ethernet/aurora/nb8800.c
>>>> @@ -609,7 +609,7 @@ static void nb8800_mac_config(struct net_device *dev)
>>>>mac_mode |= HALF_DUPLEX;
>>>>
>>>>if (gigabit) {
>>>> - if (priv->phy_mode == PHY_INTERFACE_MODE_RGMII)
>>>> + if (phy_interface_is_rgmii(dev->phydev))
>>>>mac_mode |= RGMII_MODE;
>>>>
>>>>mac_mode |= GMAC_MODE;
>>>
>>> This is a separate issue, and the change is obviously correct.
>>>
>>>> @@ -1268,11 +1268,13 @@ static int nb8800_tangox_init(struct net_device
>>>> *dev)
>>>>break;
>>>>
>>>>case PHY_INTERFACE_MODE_RGMII:
>>>> - pad_mode = PAD_MODE_RGMII;
>>>> + case PHY_INTERFACE_MODE_RGMII_RXID:
>>>> + pad_mode = PAD_MODE_RGMII | PAD_MODE_GTX_CLK_DELAY;
>>>>break;
>>>>
>>>> + case PHY_INTERFACE_MODE_RGMII_ID:
>>>>case PHY_INTERFACE_MODE_RGMII_TXID:
>>>> - pad_mode = PAD_MODE_RGMII | PAD_MODE_GTX_CLK_DELAY;
>>>> + pad_mode = PAD_MODE_RGMII;
>>>>break;
>>>
>>> Won't this just make it break in a different set of circumstances?
>>
>> I don't think so, and here's my reasoning:
>>
>> AFAIU, the HW block always requires a TX clock delay
>> (I don't know what the "safe" interval is. PHY adds
>> 2.4 ns, MAC adds ~1 ns, both work.)
>
> The nominal delay should be 2ns because that's exactly what a 90 degrees
> shift at a 125Mhz would be. The RGMII specification defines the following:
>
> TskewT - Data to Clock output Skew (At Transmitter) Min: -500ns, Nom: 0,
> Max: + 500 ns
> TskewR - Data to Clock input Skew (At Receiver) Min: 1ns, Nom: 0, Max:
> 2.6ns (see note 1)
>
> note 1: This implies that PC board design will require clocks to be
> routed such that an additional trace delay of greater than 1.5ns and
> less than 2.0ns will be added to the associated clock signal. For 10/100
> the Max value is unspecified.
>
> So it seems to me like you are borderline spec in both delays you gave
> here and the "HW block always requires a TX clock delay" statement is
> true for a given board design only.
I must confess that my understanding of clock delays,
clock skew, routing, traces, etc is nil.
Is TskewT the TX clock delay?
And TskewR the RX clock delay?
Doesn't wire delay factor in too?
(So longer wires require more delay.)
>> RX clock delay seems to be "Don't Care" (tested both
>> enabled and disabled by PHY)
>> By "tested", I mean ability to ping remote system.
>
> Can you do something a bit more stressful than just a ping, also if you
> have the ability to change the inter-packet gap, do it, and see if you
> start seeing FCS or any other decoding errors.
Errr... "Inter-packet gap"?
Is there supposed to be a HW knob to tweak how long
the HW waits between sending two frames?
>> If phy-mode is RGMII or RGMII_RXID, then don't add
>> TX clock delay from PHY, therefore add it from MAC.
>>
>> If phy_mode is RGMII_ID or RGMII_TXID, then do add
>> TX clock delay from PHY, therefore don't add it from MAC.
>>
>> What set of circumstances would create an issue?
>
> Existing Device Tree sources that do not correspond to that description
> you just did, I suppose they are all out of tree?
The problem with PHY drivers is that there is no
simple compatible string to grep for.
The tango boards use "ethernet-phy-id004d.d072"
but not a single other DT uses that string.
For example, am335x-evm.dts doesn't seem to name the PHY.
Hmmm, how does the at803x probe function match for that
board?
How does one estimate the impact of driver changes in
the eth PHY layer?
Regards.
Re: [PATCH 1/2] net: phy: at803x: Fix RGMII RX and TX clock delays setup
On 19/07/2017 21:30, Florian Fainelli wrote:
> On 07/19/2017 12:24 PM, Grygorii Strashko wrote:
>> Hi
>>
>> On 07/19/2017 10:31 AM, Marc Gonzalez wrote:
>>> The current code supports enabling RGMII RX and TX clock delays.
>>> The unstated assumption is that these settings are disabled by
>>> default at reset, which is not the case.
>>>
>>> RX clock delay is enabled at reset. And TX clock delay "survives"
>>> across SW resets. Thus, if the bootloader enables TX clock delay,
>>> it will remain enabled at reset in Linux.
>>>
>>> Provide disable functions to configure the RGMII clock delays
>>> exactly as specified in the fwspec.
>>>
>>> Signed-off-by: Marc Gonzalez
>>> ---
>>> drivers/net/phy/at803x.c | 32
>>> 1 file changed, 24 insertions(+), 8 deletions(-)
>> This patch breaks am335x-evm networking.
>>
>> To restore it I've had to apply below diff:
>> diff --git a/arch/arm/boot/dts/am335x-evm.dts
>> b/arch/arm/boot/dts/am335x-evm.dts
>> index 200d6ab..9578bdf 100644
>> --- a/arch/arm/boot/dts/am335x-evm.dts
>> +++ b/arch/arm/boot/dts/am335x-evm.dts
>> @@ -724,12 +724,12 @@
>>
>> &cpsw_emac0 {
>> phy_id = <&davinci_mdio>, <0>;
>> - phy-mode = "rgmii-txid";
>> + phy-mode = "rgmii-id";
>> };
>>
>> &cpsw_emac1 {
>> phy_id = <&davinci_mdio>, <1>;
>> - phy-mode = "rgmii-txid";
>> + phy-mode = "rgmii-id";
>> };
>>
>> &tscadc {
>>
>> Sry, can't comment here to much - not E-PHY expert.
>
> It's useful feedback, since we had poorly defined "phy-mode" semantics
> for too long, this is totally expected, Marc this is exactly why Mans is
> suggesting additional MAC-specific properties to define delays.
In the current situation, it is impossible to configure
the at803x to disable RX clock delay or TX clock delay
(in case the boot loader enabled it).
Are you saying that, because no one has had a problem
so far, it is not possible to fix it now, as it would
break boards like am335x-evm.dts which didn't request
RX clock delay, but got one anyway?
Does that mean we cannot support boards using AR8035
that need the RX and TX clock delays disabled?
I'm not sure how the MAC-specific properties can save
the day?
Regards.
Re: [PATCH 2/2] net: ethernet: nb8800: Fix RGMII TX clock delay setup
On 19/07/2017 23:34, Florian Fainelli wrote:
> How about you start reading the RGMII specification so we can at least,
> if nothing else agree on the terminology? It's public:
>
> http://web.archive.org/web/20160303171328/http://www.hp.com/rnd/pdfs/RGMIIv2_0_final_hp.pdf
Thanks for linking the spec. Having no EE training,
I am ill-equipped to interpret the timings table.
As you pointed out, the spec states that the
"Data to Clock input Skew (at Receiver)"
must be within [ 1.0, 2.6 ] ns.
I understand that 2 ns is 1/4 of a 125 MHz period,
but it's not clear to me why the above interval is
centered at 1.8 instead of 2.0 ns.
Also, the AR8035 PHY offers 4 possible TX clock delays:
{ 0.25, 1.3, 2.4, 3.4 } according to their doc.
The two extremes are outside the interval, when would
they be useful? In case the transmitter adds "bad" skew?
Why doesn't the PHY support 1.8/2.0? Is it perhaps
unable to, because of PLL limitations?
It's also not clear to me if wire length has any
influence on the required skew. I would say "no".
I think signal propagation time has nothing to do
with clock skew (as long as both wires are roughly
the same length).
> Some Ethernet controllers let you change it, some don't, if nb8800
> allows it, it's good for testing in that it packs more frames per
> quantum of time. If not, do you have at least a FCS error counter?
I'll have a closer look, and test with iPerf3.
Or is there a better benchmark? I will look for
an inter-packet gap knob and FCS error counter.
> I completely understand what you want to solve but I suspect you will
> have to do it in a way where you either accept that you may not be fully
> compliant with the now clarified "phy-mode" description, in order not to
> break other people's set up that were already non-compliant (can't blame
> them, they did not know back then), or you will have to use additional
> MAC properties to override the delay settings on the MAC or PHY side.
I think I need to give up the notion of "fixing"
the at803x driver. Some boards rely on the fact
that RX clock delay is enabled by default, like
am335x-evm using "rgmii-txid" instead of "rgmii-id".
My board needs to enable both internal delays,
so I don't need the PHY patch. I will only fix
the MAC driver and the DTS.
Regards.
Problem with PHY state machine when using interrupts
Hello,
When I set the link down via 'ip link set eth0 down'
(as opposed to pulling the Ethernet cable) things don't happen as expected:
The driver's adjust_link() callback is never called, and doesn't
get a chance make some required changes. And when I set the link
up again, there is no network connectivity.
I get this problem only if I enable interrupts on my PHY.
If I use polling, things work as expected.
When I set the link down, devinet_ioctl() eventually calls
ndo_set_rx_mode() and ndo_stop()
In ndo_stop() the driver calls
phy_stop(phydev);
which disables interrupts and sets the state to HALTED.
In phy_state_machine()
the PHY_HALTED case does call the adjust_link() callback:
if (phydev->link) {
phydev->link = 0;
netif_carrier_off(phydev->attached_dev);
phy_adjust_link(phydev);
do_suspend = true;
}
But it's not called when I use interrupts...
Perhaps because there are no interrupts generated?
Or even if there were, they have been turned off by phy_stop?
Basically, it seems like when I use interrupts,
the phy_state_machine() is not called on link down,
which breaks the MAC driver's expectations.
Am I barking up the wrong tree?
Regards.
Re: Problem with PHY state machine when using interrupts
On 24/07/2017 13:07, Mason wrote:
> When I set the link down via 'ip link set eth0 down'
> (as opposed to pulling the Ethernet cable) things don't happen as expected:
>
> The driver's adjust_link() callback is never called, and doesn't
> get a chance make some required changes. And when I set the link
> up again, there is no network connectivity.
>
> I get this problem only if I enable interrupts on my PHY.
> If I use polling, things work as expected.
>
>
> When I set the link down, devinet_ioctl() eventually calls
> ndo_set_rx_mode() and ndo_stop()
>
> In ndo_stop() the driver calls
> phy_stop(phydev);
> which disables interrupts and sets the state to HALTED.
>
> In phy_state_machine()
> the PHY_HALTED case does call the adjust_link() callback:
>
> if (phydev->link) {
> phydev->link = 0;
> netif_carrier_off(phydev->attached_dev);
> phy_adjust_link(phydev);
> do_suspend = true;
> }
>
> But it's not called when I use interrupts...
>
> Perhaps because there are no interrupts generated?
> Or even if there were, they have been turned off by phy_stop?
>
> Basically, it seems like when I use interrupts,
> the phy_state_machine() is not called on link down,
> which breaks the MAC driver's expectations.
>
> Am I barking up the wrong tree?
FWIW, the patch below solves my issue.
Basically, we reset the MAC in open(), instead of probe().
I also had to solve the issue of adjust_link() not being
called by calling it explicitly in stop() instead of
relying on phy_stop() to do it indirectly.
With this code, I think it is easy to handle suspend/resume:
on suspend, I will stop() and on resume, I will start(),
and everything should work as expected.
I'd like to hear comments on the patch, so I can turn it
into a formal submission.
Regards.
For the record, here is the debug output printed:
# ip addr add 172.27.64.45/18 brd 172.27.127.255 dev eth0
# ip link set eth0 up
[ 10.460952] ENTER nb8800_tangox_reset
[ 10.464680] ++ETH++ gw8 reg=f0026424 val=00
[ 10.478521] ++ETH++ gw8 reg=f0026424 val=01
[ 10.482837] ++ETH++ gw16 reg=f0026420 val=0050
[ 10.487325] ENTER nb8800_hw_init
[ 10.490571] ++ETH++ gw8 reg=f0026000 val=1c
[ 10.494878] ++ETH++ gw8 reg=f0026001 val=05
[ 10.499176] ++ETH++ gw8 reg=f0026004 val=22
[ 10.503481] ++ETH++ gw8 reg=f0026008 val=04
[ 10.50] ++ETH++ gw8 reg=f0026014 val=0c
[ 10.512082] ++ETH++ gw8 reg=f0026051 val=00
[ 10.516377] ++ETH++ gw8 reg=f0026052 val=ff
[ 10.520672] ++ETH++ gw8 reg=f0026054 val=40
[ 10.524967] ++ETH++ gw8 reg=f0026060 val=00
[ 10.529261] ++ETH++ gw8 reg=f0026061 val=c3
[ 10.533555] ENTER nb8800_mc_init
[ 10.536801] ++ETH++ gw8 reg=f002602e val=00
[ 10.541094] ENTER nb8800_tangox_init
[ 10.544690] ++ETH++ gw8 reg=f0026400 val=01
[ 10.548985] ENTER nb8800_tango4_init
[ 10.552580] ENTER nb8800_update_mac_addr
[ 10.556523] ++ETH++ gw8 reg=f002606a val=00
[ 10.560818] ++ETH++ gw8 reg=f002606b val=16
[ 10.565112] ++ETH++ gw8 reg=f002606c val=e8
[ 10.569407] ++ETH++ gw8 reg=f002606d val=5e
[ 10.573700] ++ETH++ gw8 reg=f002606e val=65
[ 10.577994] ++ETH++ gw8 reg=f002606f val=bc
[ 10.582288] ++ETH++ gw8 reg=f002603c val=00
[ 10.586582] ++ETH++ gw8 reg=f002603d val=16
[ 10.590876] ++ETH++ gw8 reg=f002603e val=e8
[ 10.595171] ++ETH++ gw8 reg=f002603f val=5e
[ 10.599465] ++ETH++ gw8 reg=f0026040 val=65
[ 10.603759] ++ETH++ gw8 reg=f0026041 val=bc
[ 10.608051] ENTER nb8800_open
[ 10.611034] ENTER nb8800_dma_init
[ 10.614951] ++ETH++ gw8 reg=f0026004 val=23
[ 10.619255] ++ETH++ gw8 reg=f0026000 val=1d
[ 10.688912] ENTER nb8800_set_rx_mode
[ 10.692515] ENTER nb8800_mc_init
[ 10.695762] ++ETH++ gw8 reg=f002602e val=00
[ 10.700384] PHY state change UP -> AN
[ 10.704118] ENTER nb8800_set_rx_mode
[ 10.707717] ENTER nb8800_mc_init
[ 10.710963] ++ETH++ gw8 reg=f002602e val=00
[ 10.715257] ++ETH++ gw8 reg=f0026028 val=01
[ 10.719550] ++ETH++ gw8 reg=f0026029 val=00
[ 10.723843] ++ETH++ gw8 reg=f002602a val=5e
[ 10.728135] ++ETH++ gw8 reg=f002602b val=00
[ 10.732428] ++ETH++ gw8 reg=f002602c val=00
[ 10.736721] ++ETH++ gw8 reg=f002602d val=01
[ 10.741013] ENTER nb8800_mc_init
[ 10.744258] ++ETH++ gw8 reg=f002602e val=ff
[ 14.141948] ENTER nb8800_link_reconfigure
[ 14.145988] PRIV link=0 speed=0 duplex=0
[ 14.150121] PHYDEV link=1 speed=1000 duplex=1
[ 14.154589] ENTER nb8800_mac_config
[ 14.158164] ++ETH++ gw8 reg=f0026050 val=01
[ 14.162527] ++ETH++ gw8 reg=f002601c val=ff
[ 14.166882] ++ETH++ gw8 reg=f0026044 val=81
[ 14.171233] ENTER nb8800_pause_config
[ 14.174981] ++ETH++ gw8 reg=f0026004 val=2b
[ 14.179342] nb8800 26000.ether
Re: Problem with PHY state machine when using interrupts
On 24/07/2017 18:49, Florian Fainelli wrote:
> On 07/24/2017 08:01 AM, Mason wrote:
>
>>> When I set the link down via 'ip link set eth0 down'
>>> (as opposed to pulling the Ethernet cable) things don't happen as expected:
>>>
>>> The driver's adjust_link() callback is never called, and doesn't
>>> get a chance make some required changes. And when I set the link
>>> up again, there is no network connectivity.
>>>
>>> I get this problem only if I enable interrupts on my PHY.
>>> If I use polling, things work as expected.
>>>
>>>
>>> When I set the link down, devinet_ioctl() eventually calls
>>> ndo_set_rx_mode() and ndo_stop()
>>>
>>> In ndo_stop() the driver calls
>>> phy_stop(phydev);
>>> which disables interrupts and sets the state to HALTED.
>>>
>>> In phy_state_machine()
>>> the PHY_HALTED case does call the adjust_link() callback:
>>>
>>> if (phydev->link) {
>>> phydev->link = 0;
>>> netif_carrier_off(phydev->attached_dev);
>>> phy_adjust_link(phydev);
>>> do_suspend = true;
>>> }
>>>
>>> But it's not called when I use interrupts...
>>>
>>> Perhaps because there are no interrupts generated?
>>> Or even if there were, they have been turned off by phy_stop?
>>>
>>> Basically, it seems like when I use interrupts,
>>> the phy_state_machine() is not called on link down,
>>> which breaks the MAC driver's expectations.
>>>
>>> Am I barking up the wrong tree?
>>
>> FWIW, the patch below solves my issue.
>> Basically, we reset the MAC in open(), instead of probe().
>>
>> I also had to solve the issue of adjust_link() not being
>> called by calling it explicitly in stop() instead of
>> relying on phy_stop() to do it indirectly.
>
> Which is of course absolutely not how it is intended to be used.
> phy_stop() does the following:
>
> - if the PHY was already HALTED do nothing and exit
> - if it was not and an interrupt is valid for this PHY: disable and
> clear these interrupts
> - set state to PHY_HALTED
>
> somehow an interrupt should be generated from doing this such that
> phy_change(), invoked from phy_interrupt() should have a chance to run
> and make the PHY state machine transition properly to PHY_HALTED.
I'm totally confused. Are you saying that phy_stop itself
should trigger an interrupt, or that the process of setting
the link down should generate an interrupt *before* we reach
phy_stop?
I'm also perplex over this synchronous IRQ business.
Should I be looking for a way to trigger an IRQ in
software in the Atheros PHY?
Before I forget: there is also an issue when using the PHY
in polling mode. The ndo_stop callback runs through phy_stop
and phy_disconnect too fast for the adjust_link() callback
to be called. My patch fixed that too, by calling
nb8800_link_reconfigure() explicitly.
> So from there can you check a few things:
>
> - is such an interrupt actually generated?
> - if you turn on dynamic debug prints for drivers/net/phy/phy.c where do
> we leave the PHY state machine and what state is it in when you call
> ifconfig up again?
The only interrupts I've ever seen the PHY generate are
on plugging/unplugging the Ethernet cable.
Looking at the driver and datasheet...
http://elixir.free-electrons.com/linux/v4.13-rc2/source/drivers/net/phy/at803x.c#L312
value |= AT803X_INTR_ENABLE_AUTONEG_ERR;
value |= AT803X_INTR_ENABLE_SPEED_CHANGED;
value |= AT803X_INTR_ENABLE_DUPLEX_CHANGED;
value |= AT803X_INTR_ENABLE_LINK_FAIL;
value |= AT803X_INTR_ENABLE_LINK_SUCCESS;
And the interrupts reasons supported by the PHY are:
#define AT803X_INTR_ENABLE_AUTONEG_ERR BIT(15)
#define AT803X_INTR_ENABLE_SPEED_CHANGEDBIT(14)
#define AT803X_INTR_ENABLE_DUPLEX_CHANGED BIT(13)
#define AT803X_INTR_ENABLE_PAGE_RECEIVEDBIT(12)
#define AT803X_INTR_ENABLE_LINK_FAILBIT(11)
#define AT803X_INTR_ENABLE_LINK_SUCCESS BIT(10)
#define AT803X_INTR_ENABLE_WIRESPEED_DOWNGRADE BIT(5)
#define AT803X_INTR_ENABLE_POLARITY_CHANGED BIT(1)
#define AT803X_INTR_ENABLE_WOL BIT(0)
These all seem to be external reasons (from the peer).
I did enable debug logs in drivers/net/phy/phy.c
to trace the state machine, and it is not called
at all on set link down, so it remains in state
RUNNING (both in polling and interrupt modes).
IIRC, this used to work on the 2-core board, but breaks
on the 4-core board. Could this be some kind of race?
Regards.
Re: Problem with PHY state machine when using interrupts
On 24/07/2017 21:53, Florian Fainelli wrote: > Well now that I see the possible interrupts generated, I indeed don't > see how you can get a link down notification unless you somehow force > the link down yourself, which would certainly happen in phy_suspend() > when we set BMCR.pwrdwn, but that may be too late. > > You should still expect the adjust_link() function to be called though > with PHY_HALTED being set and that takes care of doing phydev->link = 0 > and netif_carrier_off(). If that still does not work, then see whether > removing the call to phy_stop() does help (it really should). The only functions setting phydev->state to PHY_HALTED are phy_error() and phy_stop() AFAICT. I am aware that when phy_state_machine() handles the PHY_HALTED state, it will set phydev->link = 0; and call netif_carrier_off() -- because that's where I copied that code from. My issue is that phy_state_machine() does not run when I run 'ip set link dev eth0 down' from the command line. If I'm reading the code right, phy_disconnect() actually stops the state machine. In interrupt mode, phy_state_machine() doesn't run because no interrupt is generated. In polling mode, phy_state_machine() doesn't run because phy_disconnect() stops the state machine. Introducing a sleep before phy_disconnect() gives the state machine a chance to run in polling mode, but it doesn't feel right, and doesn't fix the other mode, which I'm using. Looking at bcm_enet_stop() it calls phy_stop() and phy_disconnect() just like the nb8800 driver... I'm stumped. Regards.
Re: [PATCH 2/2] net: ethernet: nb8800: Fix RGMII TX clock delay setup
On 20/07/2017 14:33, Mason wrote:
> As [Florian] pointed out, the spec states that the
> "Data to Clock input Skew (at Receiver)"
> must be within [ 1.0, 2.6 ] ns.
>
> I understand that 2 ns is 1/4 of a 125 MHz period,
> but it's not clear to me why the above interval is
> centered at 1.8 instead of 2.0 ns.
>
> Also, the AR8035 PHY offers 4 possible TX clock delays:
> { 0.25, 1.3, 2.4, 3.4 } according to their doc.
> The two extremes are outside the interval, when would
> they be useful? In case the transmitter adds "bad" skew?
>
> Why doesn't the PHY support 1.8/2.0? Is it perhaps
> unable to, because of PLL limitations?
I haven't yet found answers for these questions.
- Why is the interval centered at 1.8 instead of 2.0 ns?
- What use are 0.25 ns and 3.4 ns skew?
- Why doesn't the PHY support a "recommended" value like 1.8 ns?
Does anyone have pointers to good resources?
Regards.
Re: [PATCH 2/2] net: ethernet: nb8800: Fix RGMII TX clock delay setup
On 24/07/2017 23:49, Florian Fainelli wrote:
> On 07/24/2017 02:21 PM, Mason wrote:
>> On 20/07/2017 14:33, Mason wrote:
>>
>>> As [Florian] pointed out, the spec states that the
>>> "Data to Clock input Skew (at Receiver)"
>>> must be within [ 1.0, 2.6 ] ns.
>>>
>>> I understand that 2 ns is 1/4 of a 125 MHz period,
>>> but it's not clear to me why the above interval is
>>> centered at 1.8 instead of 2.0 ns.
>>>
>>> Also, the AR8035 PHY offers 4 possible TX clock delays:
>>> { 0.25, 1.3, 2.4, 3.4 } according to their doc.
>>> The two extremes are outside the interval, when would
>>> they be useful? In case the transmitter adds "bad" skew?
>>>
>>> Why doesn't the PHY support 1.8/2.0? Is it perhaps
>>> unable to, because of PLL limitations?
>>
>> I haven't yet found answers for these questions.
>>
>> - Why is the interval centered at 1.8 instead of 2.0 ns?
>
> Presumably because this is almost the middle of the available range and
> it still provides a value that is within the specification...
I was talking about the RGMII spec.
If - theoretically - the best results are achieved
by having a 2 ns skew between clock and data,
it seems odd for the RGMII spec to define an
interval of [ 1.0, 2.6 ] ns for acceptable values.
I would have expected [ 1.2, 2.8 ] ns.
>> - What use are 0.25 ns and 3.4 ns skew?
>
> Accounting for extreme PCB traces lengths possibly, or just exposing the
> raw values that the HW supports by increments of 0.25 ns, just because
> the HW supports it.
The AR8035 doesn't support increments of 0.25 ns,
it supports just 4 values: 0.25, 1.3, 2.4, 3.4
Two of which are outside the acceptable range
defined in the RGMII spec. Odd.
Giving it more thought, I don't think trace length
factors in, unless the data and clock lines have
very different length (signal propagation).
>> - Why doesn't the PHY support a "recommended" value like 1.8 ns?
>>
>> Does anyone have pointers to good resources?
>
> The PHY datasheet and the RGMII specification really ought to be the
> starting points, there is not much more to it. Maybe go ask your support
> person at Qualcomm/Atheros about their PHY design?
Sadly, I rarely have access to support for the blocks
we use. I had to download the datasheet off the internet.
But I was only asking out of personal curiosity, since
this is outside my field. I don't think any customer
has complained about the default settings.
Regards.
Re: Problem with PHY state machine when using interrupts
On 25/07/2017 00:36, Florian Fainelli wrote: > On 07/24/2017 02:20 PM, Mason wrote: >> On 24/07/2017 21:53, Florian Fainelli wrote: >> >>> Well now that I see the possible interrupts generated, I indeed don't >>> see how you can get a link down notification unless you somehow force >>> the link down yourself, which would certainly happen in phy_suspend() >>> when we set BMCR.pwrdwn, but that may be too late. >>> >>> You should still expect the adjust_link() function to be called though >>> with PHY_HALTED being set and that takes care of doing phydev->link = 0 >>> and netif_carrier_off(). If that still does not work, then see whether >>> removing the call to phy_stop() does help (it really should). >> >> The only functions setting phydev->state to PHY_HALTED >> are phy_error() and phy_stop() AFAICT. >> >> I am aware that when phy_state_machine() handles the >> PHY_HALTED state, it will set phydev->link = 0; >> and call netif_carrier_off() -- because that's where >> I copied that code from. >> >> My issue is that phy_state_machine() does not run when >> I run 'ip set link dev eth0 down' from the command line. > > Yes, that much is clear, which is why I suggested earlier you try the > patch at the end now. > >> >> If I'm reading the code right, phy_disconnect() actually >> stops the state machine. >> >> In interrupt mode, phy_state_machine() doesn't run >> because no interrupt is generated. >> >> In polling mode, phy_state_machine() doesn't run >> because phy_disconnect() stops the state machine. >> >> Introducing a sleep before phy_disconnect() gives >> the state machine a chance to run in polling mode, >> but it doesn't feel right, and doesn't fix the >> other mode, which I'm using. > > There are several problems it seems: > > - the PHY state machine cannot move solely based on the PHY generating > interrupts during phy_stop() if none of the changing conditions for the > HW have changed, come to think about it, I doubt any PHY would be > capable of doing something like that > > - there is an expectation from your driver to have adjust_link() run > sometime during ndo_stop() to do something, but why? > > What is special about nb8800 that interrupts should be generated during > ndo_stop(), and why do you think this is going to solve your problem? > >> >> Looking at bcm_enet_stop() it calls phy_stop() and >> phy_disconnect() just like the nb8800 driver... >> >> I'm stumped. > > My suggestion of not using phy_stop() was not a good one, but > functionally there is little difference in doing phy_stop() + > phy_disconnect() or just phy_disconnect() alone. What matters is that we > restart the PHY properly when phy_connect() or phy_start() is called. > > What I understand now from your "bug report" is that you want > adjust_link to run with phydev->link = 0 to do something during > ndo_close() but you have not explained why, but I suspect such that upon > ndo_open() things work again. > > What I suspect your bug is, is that the really was no change in link > status, no interrupt was generated because there should not be one, yet > some of what nb8800_stop() does is not properly balanced by > nb8800_open(). How about the following patch: > > diff --git a/drivers/net/ethernet/aurora/nb8800.c > b/drivers/net/ethernet/aurora/nb8800.c > index 041cfb7952f8..b07dea3ab019 100644 > --- a/drivers/net/ethernet/aurora/nb8800.c > +++ b/drivers/net/ethernet/aurora/nb8800.c > @@ -972,6 +972,10 @@ static int nb8800_open(struct net_device *dev) > nb8800_mac_rx(dev, true); > nb8800_mac_tx(dev, true); > > + priv->speed = -1; > + priv->link = -1; > + priv->duplex = -1; > + > phydev = of_phy_connect(dev, priv->phy_node, > nb8800_link_reconfigure, 0, > priv->phy_mode); > I will test your two patches ASAP. For the record, I have two (maybe related) issues: 1) After a sequence of ip set link dev eth0 up ip set link dev eth0 down ip set link dev eth0 up RX engine is hosed, thus network connectivity is borked. The work-around is resetting the MAC in ndo_open => mac_init in my proposed patch. This is by far the biggest issue. Also, resetting the MAC in ndo_open will make it easy to implement suspend/resume, as I can just ndo_stop in suspend, and ndo_open in resume. 2) The system does not print "Link down" when I run 'ip set link dev eth0 down' This is a symptom of nb8800_link_reconfigure() not being called at all (or being called with phydev->link == priv->link when I tried skipping phy_stop) Again, thanks for your patches and suggestions, which I will test in the morning. I will also try to understand why I didn't have these issues on the other board... Regards.
Re: Problem with PHY state machine when using interrupts
On 25/07/2017 00:39, Florian Fainelli wrote: > diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c > index d0626bf5c540..652e24b53f3f 100644 > --- a/drivers/net/phy/phy.c > +++ b/drivers/net/phy/phy.c > @@ -968,6 +968,8 @@ void phy_stop(struct phy_device *phydev) > * of rtnl_lock(), but PHY_HALTED shall guarantee phy_change() > * will not reenable interrupts. > */ > + if (phy_interrupt_is_valid(phydev)) > + phy_change(phydev); > } > EXPORT_SYMBOL(phy_stop); > When setting link down: - kernel logs Unbalanced enable for IRQ 30 (PHY interrupt) - serial console hangs # ip link set eth0 up [ 125.800057] ENTER nb8800_tangox_reset [ 125.803790] ++ETH++ gw8 reg=f0026424 val=00 [ 125.817446] ++ETH++ gw8 reg=f0026424 val=01 [ 125.821768] ++ETH++ gw16 reg=f0026420 val=0050 [ 125.826241] ENTER nb8800_hw_init [ 125.829507] ++ETH++ gw8 reg=f0026000 val=1c [ 125.833808] ++ETH++ gw8 reg=f0026001 val=05 [ 125.838122] ++ETH++ gw8 reg=f0026004 val=22 [ 125.842421] ++ETH++ gw8 reg=f0026008 val=04 [ 125.846717] ++ETH++ gw8 reg=f0026014 val=0c [ 125.851015] ++ETH++ gw8 reg=f0026051 val=00 [ 125.855310] ++ETH++ gw8 reg=f0026052 val=ff [ 125.859607] ++ETH++ gw8 reg=f0026054 val=40 [ 125.863903] ++ETH++ gw8 reg=f0026060 val=00 [ 125.868226] ++ETH++ gw8 reg=f0026061 val=c3 [ 125.872534] ENTER nb8800_mc_init [ 125.875800] ++ETH++ gw8 reg=f002602e val=00 [ 125.880096] ENTER nb8800_tangox_init [ 125.883697] ++ETH++ gw8 reg=f0026400 val=01 [ 125.887993] ENTER nb8800_tango4_init [ 125.891592] ENTER nb8800_update_mac_addr [ 125.895538] ++ETH++ gw8 reg=f002606a val=00 [ 125.899835] ++ETH++ gw8 reg=f002606b val=16 [ 125.904132] ++ETH++ gw8 reg=f002606c val=e8 [ 125.908429] ++ETH++ gw8 reg=f002606d val=5e [ 125.912725] ++ETH++ gw8 reg=f002606e val=65 [ 125.917022] ++ETH++ gw8 reg=f002606f val=bc [ 125.921319] ++ETH++ gw8 reg=f002603c val=00 [ 125.925618] ++ETH++ gw8 reg=f002603d val=16 [ 125.929912] ++ETH++ gw8 reg=f002603e val=e8 [ 125.934210] ++ETH++ gw8 reg=f002603f val=5e [ 125.938505] ++ETH++ gw8 reg=f0026040 val=65 [ 125.942802] ++ETH++ gw8 reg=f0026041 val=bc [ 125.947095] ENTER nb8800_open [ 125.950082] ENTER nb8800_dma_init [ 125.954025] ++ETH++ gw8 reg=f0026004 val=23 [ 125.958331] ++ETH++ gw8 reg=f0026000 val=1d [ 126.027848] ENTER nb8800_set_rx_mode [ 126.031451] ENTER nb8800_mc_init [ 126.034702] ++ETH++ gw8 reg=f002602e val=00 [ 126.039334] Atheros 8035 ethernet 26000.nb8800-mii:04: PHY state change UP -> AN [ 126.046838] ENTER nb8800_set_rx_mode [ 126.050438] ENTER nb8800_mc_init [ 126.053688] ++ETH++ gw8 reg=f002602e val=00 [ 126.057983] ++ETH++ gw8 reg=f0026028 val=01 [ 126.062279] ++ETH++ gw8 reg=f0026029 val=00 [ 126.066573] ++ETH++ gw8 reg=f002602a val=5e [ 126.070868] ++ETH++ gw8 reg=f002602b val=00 [ 126.075162] ++ETH++ gw8 reg=f002602c val=00 [ 126.079457] ++ETH++ gw8 reg=f002602d val=01 [ 126.083751] ENTER nb8800_mc_init [ 126.086997] ++ETH++ gw8 reg=f002602e val=ff [ 129.426741] ENTER nb8800_link_reconfigure [ 129.430800] PRIV link=0 speed=0 duplex=0 [ 129.434753] PHYDEV link=1 speed=1000 duplex=1 [ 129.439141] ENTER nb8800_mac_config [ 129.442662] ++ETH++ gw8 reg=f0026050 val=01 [ 129.446962] ++ETH++ gw8 reg=f002601c val=ff [ 129.451262] ++ETH++ gw8 reg=f0026044 val=81 [ 129.455563] ENTER nb8800_pause_config [ 129.459252] ++ETH++ gw8 reg=f0026004 val=2b [ 129.463558] nb8800 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx [ 129.471355] Atheros 8035 ethernet 26000.nb8800-mii:04: PHY state change AN -> RUNNING # ip link set eth0 down [ 170.933454] ENTER nb8800_set_rx_mode [ 170.937067] ENTER nb8800_mc_init [ 170.940318] ++ETH++ gw8 reg=f002602e val=00 [ 170.944613] ++ETH++ gw8 reg=f0026028 val=01 [ 170.948907] ++ETH++ gw8 reg=f0026029 val=00 [ 170.953199] ++ETH++ gw8 reg=f002602a val=5e [ 170.957494] ++ETH++ gw8 reg=f002602b val=00 [ 170.961786] ++ETH++ gw8 reg=f002602c val=00 [ 170.966079] ++ETH++ gw8 reg=f002602d val=01 [ 170.970371] ENTER nb8800_mc_init [ 170.973616] ++ETH++ gw8 reg=f002602e val=ff [ 170.977962] ENTER nb8800_stop [ 170.981210] [ cut here ] [ 170.985860] WARNING: CPU: 0 PID: 964 at kernel/irq/manage.c:498 __enable_irq+0x44/0x68 [ 170.993815] Unbalanced enable for IRQ 30 [ 170.997751] Modules linked in: [ 171.000821] CPU: 0 PID: 964 Comm: ip Not tainted 4.13.0-rc1 #154 [ 171.006854] Hardware name: Sigma Tango DT [ 171.010896] [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [ 171.018686] [] (show_stack) from [] (dump_stack+0x78/0x8c) [ 171.025950] [] (dump_stack) from [] (__warn+0xe8/0x100) [ 171.032948] [] (__warn) from [] (warn_slowpath_fmt+0x38/0x48) [ 171.040471] [] (warn_slowpath_fmt) from [] (__enable_irq+0x44/0x68) [ 171.048519] [] (__enable_irq) from [] (enable_irq+0x34/0x6c) [ 171.055956] [] (enable_irq) from [] (phy_change+0x98/0x13c)
Re: Problem with PHY state machine when using interrupts
On 25/07/2017 12:51, Mason wrote:
> Moving the call to phy_stop() down after all the MAC tear down
> avoids the hang.
>
> As far as I understand, when we are shutting everything down,
> we don't need the phy_state_machine to run asynchronously.
> We can run it synchronously one last time after the delayed
> stuff has been disabled.
Below is my current WIP diff. (It conflates the two issues
I've been discussing. Splitting the diff required.)
Tested in interrupt mode:
# ip link set eth0 up
[ 11.107547] Atheros 8035 ethernet 26000.nb8800-mii:04: PHY state change UP
-> AN
[ 14.530329] nb8800 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow
control rx/tx
[ 14.538136] Atheros 8035 ethernet 26000.nb8800-mii:04: PHY state change AN
-> RUNNING
# ip link set eth0 down
[ 23.801018] nb8800 26000.ethernet eth0: Link is Down
# ip link set eth0 up
[ 28.740870] Atheros 8035 ethernet 26000.nb8800-mii:04: PHY state change UP
-> AN
[ 31.431528] nb8800 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow
control rx/tx
[ 31.439350] Atheros 8035 ethernet 26000.nb8800-mii:04: PHY state change AN
-> RUNNING
Works as expected.
Tested in polling mode:
# ip link set eth0 up
[ 23.001199] Atheros 8035 ethernet 26000.nb8800-mii:04: PHY state change UP
-> AN
[ 24.024315] Atheros 8035 ethernet 26000.nb8800-mii:04: PHY state change AN
-> NOLINK
[ 27.064355] nb8800 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow
control rx/tx
[ 27.072156] Atheros 8035 ethernet 26000.nb8800-mii:04: PHY state change
NOLINK -> RUNNING
# ip link set eth0 down
[ 42.134617] nb8800 26000.ethernet eth0: Link is Down
# ip link set eth0 up
[ 48.381185] Atheros 8035 ethernet 26000.nb8800-mii:04: PHY state change UP
-> AN
[ 49.410976] Atheros 8035 ethernet 26000.nb8800-mii:04: PHY state change AN
-> NOLINK
[ 51.437686] nb8800 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow
control rx/tx
[ 51.445486] Atheros 8035 ethernet 26000.nb8800-mii:04: PHY state change
NOLINK -> RUNNING
Works as expected.
Also tested on my old board, no regression seen.
Can you confirm that the changes to drivers/net/phy/phy.c
look reasonable?
Regards.
diff --git a/drivers/net/ethernet/aurora/nb8800.c
b/drivers/net/ethernet/aurora/nb8800.c
index e94159507847..b5eba7958871 100644
--- a/drivers/net/ethernet/aurora/nb8800.c
+++ b/drivers/net/ethernet/aurora/nb8800.c
@@ -41,6 +41,7 @@
static void nb8800_tx_done(struct net_device *dev);
static int nb8800_dma_stop(struct net_device *dev);
+static int nb8800_init(struct net_device *dev);
static inline u8 nb8800_readb(struct nb8800_priv *priv, int reg)
{
@@ -957,6 +958,9 @@ static int nb8800_open(struct net_device *dev)
struct phy_device *phydev;
int err;
+ nb8800_init(dev);
+ priv->speed = 0;
+
/* clear any pending interrupts */
nb8800_writel(priv, NB8800_RXC_SR, 0xf);
nb8800_writel(priv, NB8800_TXC_SR, 0xf);
@@ -1004,8 +1008,6 @@ static int nb8800_stop(struct net_device *dev)
struct nb8800_priv *priv = netdev_priv(dev);
struct phy_device *phydev = dev->phydev;
- phy_stop(phydev);
-
netif_stop_queue(dev);
napi_disable(&priv->napi);
@@ -1013,6 +1015,7 @@ static int nb8800_stop(struct net_device *dev)
nb8800_mac_rx(dev, false);
nb8800_mac_tx(dev, false);
+ phy_stop(phydev);
phy_disconnect(phydev);
free_irq(dev->irq, dev);
@@ -1350,6 +1353,21 @@ static int nb8800_tango4_init(struct net_device *dev)
};
MODULE_DEVICE_TABLE(of, nb8800_dt_ids);
+static int nb8800_init(struct net_device *dev)
+{
+#ifndef RESET_IN_PROBE
+ struct nb8800_priv *priv = netdev_priv(dev);
+ const struct nb8800_ops *ops = priv->ops;
+
+ ops->reset(dev);
+ nb8800_hw_init(dev);
+ ops->init(dev);
+ nb8800_update_mac_addr(dev);
+#endif
+
+ return 0;
+}
+
static int nb8800_probe(struct platform_device *pdev)
{
const struct of_device_id *match;
@@ -1389,6 +1407,7 @@ static int nb8800_probe(struct platform_device *pdev)
priv = netdev_priv(dev);
priv->base = base;
+ priv->ops = ops;
priv->phy_mode = of_get_phy_mode(pdev->dev.of_node);
if (priv->phy_mode < 0)
@@ -1407,11 +1426,13 @@ static int nb8800_probe(struct platform_device *pdev)
spin_lock_init(&priv->tx_lock);
+#ifdef RESET_IN_PROBE
if (ops && ops->reset) {
ret = ops->reset(dev);
if (ret)
goto err_disable_clk;
}
+#endif
bus = devm_mdiobus_alloc(&pdev->dev);
if (!bus) {
@@ -1454,6 +1475,7 @@ static int nb8800_probe(struct platform_device *pdev)
priv->mii_bus = bus;
+#ifdef RESET_IN_PROBE
ret = nb8800_hw_init(dev);
if (ret)
goto err_deregister_fixed_
Re: [RFC PATCH v1] net: ethernet: nb8800: Reset HW block in ndo_open
On 28/07/2017 20:56, Måns Rullgård wrote:
> Marc Gonzalez writes:
>
>> On 28/07/2017 18:17, Måns Rullgård wrote:
>>
>>> Marc Gonzalez wrote:
>>>
ndo_stop breaks RX in a way that ndo_open is unable to undo.
>>>
>>> Please elaborate. Why can't it be fixed in a less heavy-handed way?
>>
>> I'm not sure what "elaborate" means. After we've been through
>> ndo_stop once, the board can send packets, but it doesn't see
>> any replies from remote systems. RX is wedged.
>
> So you say, but you have not explained why this happens. Until we know
> why, we can't decide on the proper fix.
I'll try adding delays in strategic places, and see if
I can trigger the same bug on tango4. If I can't, then
this work around is all we've got.
And I need nb8800_init for resume anyway, so I might
as well use it in ndo_open.
TODO: test power savings from holding HW in reset.
>> I think ndo_stop is rare enough an event that doing a full
>> reset is not an issue, in terms of performance.
>
> Performance isn't the issue. Doing the right thing is.
I don't have always have time to figure out exactly how
broken HW is broken. It's already bad enough that disabling
DMA requires sending a fake packet through the loop back...
>>> I'm pretty sure this doesn't preserve everything it should.
>>
>> Hmmm, we're supposed to start fresh ("full reset").
>> What could there be to preserve?
>> You mentioned flow control and multicast elsewhere.
>> I will take a closer look. Thanks for the heads up.
>
> Yes, those settings are definitely lost with your patch. Now I'm not
> sure whether the networking core expects these to survive a stop/start
> cycle, so please check that. There might also be other less obvious
> things that need to be preserved.
The original code calls nb8800_pause_config() every
time the link comes up. The proposed patch doesn't
change that.
Regards.
Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"
On 31/08/2017 18:36, David Daney wrote:
> On 08/31/2017 05:29 AM, Marc Gonzalez wrote:
>> On 31/08/2017 02:49, Florian Fainelli wrote:
>>
>>> This reverts commit 7ad813f208533cebfcc32d3d7474dc1677d1b09a ("net: phy:
>>> Correctly process PHY_HALTED in phy_stop_machine()") because it is
>>> creating the possibility for a NULL pointer dereference.
>>>
>>> David Daney provide the following call trace and diagram of events:
>>>
>>> When ndo_stop() is called we call:
>>>
>>> phy_disconnect()
>>> +---> phy_stop_interrupts() implies: phydev->irq = PHY_POLL;
>>
>> What does this mean?
>
> I meant that after the call to phy_stop_interrupts(), phydev->irq =
> PHY_POLL;
I must be missing something.
http://elixir.free-electrons.com/linux/latest/source/drivers/net/phy/phy.c#L868
phy_stop_interrupts() doesn't change phydev->irq right?
Only phy_start_interrupts() sets phydev->irq to
PHY_POLL if it cannot set up interrupt mode.
Regards.
Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"
On 31/08/2017 18:57, Florian Fainelli wrote: > And the race is between phy_detach() setting phydev->attached_dev = NULL > and phy_state_machine() running in PHY_HALTED state and calling > netif_carrier_off(). I must be missing something. (Since a thread cannot race against itself.) phy_disconnect calls phy_stop_machine which 1) stops the work queue from running in a separate thread 2) calls phy_state_machine *synchronously* which runs the PHY_HALTED case with everything well-defined end of phy_stop_machine phy_disconnect only then calls phy_detach() which makes future calls of phy_state_machine perilous. This all happens in the same thread, so I'm not yet seeing where the race happens? Regards.
Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"
On 31/08/2017 19:53, Florian Fainelli wrote:
> On 08/31/2017 10:49 AM, Mason wrote:
>> On 31/08/2017 18:57, Florian Fainelli wrote:
>>> And the race is between phy_detach() setting phydev->attached_dev = NULL
>>> and phy_state_machine() running in PHY_HALTED state and calling
>>> netif_carrier_off().
>>
>> I must be missing something.
>> (Since a thread cannot race against itself.)
>>
>> phy_disconnect calls phy_stop_machine which
>> 1) stops the work queue from running in a separate thread
>> 2) calls phy_state_machine *synchronously*
>> which runs the PHY_HALTED case with everything well-defined
>> end of phy_stop_machine
>>
>> phy_disconnect only then calls phy_detach()
>> which makes future calls of phy_state_machine perilous.
>>
>> This all happens in the same thread, so I'm not yet
>> seeing where the race happens?
>
> The race is as described in David's earlier email, so let's recap:
>
> Thread 1 Thread 2
> phy_disconnect()
> phy_stop_interrupts()
> phy_stop_machine()
> phy_state_machine()
> -> queue_delayed_work()
> phy_detach()
> phy_state_machine()
> -> netif_carrier_off()
>
> If phy_detach() finishes earlier than the workqueue had a chance to be
> scheduled and process PHY_HALTED again, then we trigger the NULL pointer
> de-reference.
>
> workqueues are not tasklets, the CPU scheduling them gets no guarantee
> they will run on the same CPU.
Something does not add up.
The synchronous call to phy_state_machine() does:
case PHY_HALTED:
if (phydev->link) {
phydev->link = 0;
netif_carrier_off(phydev->attached_dev);
phy_adjust_link(phydev);
do_suspend = true;
}
then sets phydev->link = 0; therefore subsequent calls to
phy_state_machin() will be no-op.
Also, queue_delayed_work() is only called in polling mode.
David stated that he's using interrupt mode.
Regards.
Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"
On 31/08/2017 19:03, Florian Fainelli wrote: > On 08/31/2017 05:29 AM, Marc Gonzalez wrote: > >> On 31/08/2017 02:49, Florian Fainelli wrote: >> >>> The original motivation for this change originated from Marc Gonzalez >>> indicating that his network driver did not have its adjust_link callback >>> executing with phydev->link = 0 while he was expecting it. >> >> I expect the core to call phy_adjust_link() for link changes. >> This used to work back in 3.4 and was broken somewhere along >> the way. > > If that was working correctly in 3.4 surely we can look at the diff and > figure out what changed, even maybe find the offending commit, can you > do that? Bisecting would a be a huge pain because my platform was not upstream until v4.4 You mentioned the guarantees made by PHYLIB. When is the adjust_link callback guaranteed to be called? >>> PHYLIB has never made any such guarantees ever because phy_stop() merely >>> just tells the workqueue to move into PHY_HALTED state which will happen >>> asynchronously. >> >> My original proposal was to fix the issue in the driver. >> I'll try locating it in my archives. > > Yes I remember you telling that, by the way I don't think you ever > provided a clear explanation why this is absolutely necessary for your > driver though? 1) nb8800_link_reconfigure() calls phy_print_status() which prints the "Link down" and "Link up" messages to the console. With the patch reverted, nothing is printed when the link goes down, and the result is random when the link comes up. Sometimes, we get down + up, sometimes just up. 2) nb8800_link_reconfigure() does some HW init when the link state changes. If we miss some notifications, we might not perform some HW init, and stuff breaks. Regards.
Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"
On 31/08/2017 20:29, Florian Fainelli wrote:
On 08/31/2017 11:12 AM, Mason wrote:
On 31/08/2017 19:53, Florian Fainelli wrote:
On 08/31/2017 10:49 AM, Mason wrote:
On 31/08/2017 18:57, Florian Fainelli wrote:
And the race is between phy_detach() setting phydev->attached_dev = NULL
and phy_state_machine() running in PHY_HALTED state and calling
netif_carrier_off().
I must be missing something.
(Since a thread cannot race against itself.)
phy_disconnect calls phy_stop_machine which
1) stops the work queue from running in a separate thread
2) calls phy_state_machine *synchronously*
which runs the PHY_HALTED case with everything well-defined
end of phy_stop_machine
phy_disconnect only then calls phy_detach()
which makes future calls of phy_state_machine perilous.
This all happens in the same thread, so I'm not yet
seeing where the race happens?
The race is as described in David's earlier email, so let's recap:
Thread 1Thread 2
phy_disconnect()
phy_stop_interrupts()
phy_stop_machine()
phy_state_machine()
-> queue_delayed_work()
phy_detach()
phy_state_machine()
-> netif_carrier_off()
If phy_detach() finishes earlier than the workqueue had a chance to be
scheduled and process PHY_HALTED again, then we trigger the NULL pointer
de-reference.
workqueues are not tasklets, the CPU scheduling them gets no guarantee
they will run on the same CPU.
Something does not add up.
The synchronous call to phy_state_machine() does:
case PHY_HALTED:
if (phydev->link) {
phydev->link = 0;
netif_carrier_off(phydev->attached_dev);
phy_adjust_link(phydev);
do_suspend = true;
}
then sets phydev->link = 0; therefore subsequent calls to
phy_state_machin() will be no-op.
Actually you are right, once phydev->link is set to 0 these would become
no-ops. Still scratching my head as to what happens for David then...
Also, queue_delayed_work() is only called in polling mode.
David stated that he's using interrupt mode.
Right that's confusing too now. David can you check if you tree has:
49d52e8108a21749dc2114b924c907db43358984 ("net: phy: handle state
correctly in phy_stop_machine")
Hello David,
A week ago, you wrote about my patch:
"This is broken. Please revert."
I assume you tested the revert locally, and that reverting did make
the crash disappear. Is that correct?
The reason I ask is because the analysis you provided contains some
flaws, as noted above. But, if reverting my patch did fix your issue,
then perhaps understanding *why* is unimportant.
I'm a bit baffled that it took less than 90 minutes for your request
to be approved, and the patch reverted in all branches, before I even
had a chance to comment.
Regards.
Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"
On 31/08/2017 21:18, Florian Fainelli wrote: On 08/31/2017 12:09 PM, Mason wrote: 1) nb8800_link_reconfigure() calls phy_print_status() which prints the "Link down" and "Link up" messages to the console. With the patch reverted, nothing is printed when the link goes down, and the result is random when the link comes up. Sometimes, we get down + up, sometimes just up. Nothing printed when you bring down the network interface as a result of not signaling the link down, there is a small nuance here. Let me first focus on the "Link down" message. Do you agree that such a message should be printed when the link goes down, not when the link comes up? Perhaps the issue is that the 2 following cases need to be handled differently: A) operator sets link down on the command-line B) asynchronous event makes link go down (peer is dead, cable is cut, etc) In B) the PHY state machine keeps on running, and eventually calls adjust_link() In A) the driver calls phy_stop() and phy_disconnect() and therefore adjust_link() will not be called? Regards.
Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"
On 31/08/2017 21:18, Florian Fainelli wrote: On 08/31/2017 12:09 PM, Mason wrote: On 31/08/2017 19:03, Florian Fainelli wrote: On 08/31/2017 05:29 AM, Marc Gonzalez wrote: On 31/08/2017 02:49, Florian Fainelli wrote: The original motivation for this change originated from Marc Gonzalez indicating that his network driver did not have its adjust_link callback executing with phydev->link = 0 while he was expecting it. I expect the core to call phy_adjust_link() for link changes. This used to work back in 3.4 and was broken somewhere along the way. If that was working correctly in 3.4 surely we can look at the diff and figure out what changed, even maybe find the offending commit, can you do that? Bisecting would a be a huge pain because my platform was not upstream until v4.4 Then just diff the file and try to pinpoint which commit may have changed that? Running 'ip link set eth0 down' on the command-line. In v3.4 => adjust_link() callback is called In v4.5 => adjust_link() callback is NOT called $ git log --oneline --no-merges v3.4..v4.5 drivers/net/phy/phy.c | wc -l 59 I'm not sure what "just diff the file" entails. I can't move 3.4 up, nor move 4.5 down. I'm not even sure the problem comes from drivers/net/phy/phy.c to be honest. Regards.
Re: [PATCH net] Revert "net: phy: Correctly process PHY_HALTED in phy_stop_machine()"
On 06/09/2017 20:00, David Daney wrote:
> On 08/31/2017 11:29 AM, Florian Fainelli wrote:
>> On 08/31/2017 11:12 AM, Mason wrote:
>>> On 31/08/2017 19:53, Florian Fainelli wrote:
>>>> On 08/31/2017 10:49 AM, Mason wrote:
>>>>> On 31/08/2017 18:57, Florian Fainelli wrote:
>>>>>> And the race is between phy_detach() setting phydev->attached_dev = NULL
>>>>>> and phy_state_machine() running in PHY_HALTED state and calling
>>>>>> netif_carrier_off().
>>>>>
>>>>> I must be missing something.
>>>>> (Since a thread cannot race against itself.)
>>>>>
>>>>> phy_disconnect calls phy_stop_machine which
>>>>> 1) stops the work queue from running in a separate thread
>>>>> 2) calls phy_state_machine *synchronously*
>>>>> which runs the PHY_HALTED case with everything well-defined
>>>>> end of phy_stop_machine
>>>>>
>>>>> phy_disconnect only then calls phy_detach()
>>>>> which makes future calls of phy_state_machine perilous.
>>>>>
>>>>> This all happens in the same thread, so I'm not yet
>>>>> seeing where the race happens?
>>>>
>>>> The race is as described in David's earlier email, so let's recap:
>>>>
>>>> Thread 1 Thread 2
>>>> phy_disconnect()
>>>> phy_stop_interrupts()
>>>> phy_stop_machine()
>>>> phy_state_machine()
>>>> -> queue_delayed_work()
>>>> phy_detach()
>>>>phy_state_machine()
>>>>-> netif_carrier_off()
>>>>
>>>> If phy_detach() finishes earlier than the workqueue had a chance to be
>>>> scheduled and process PHY_HALTED again, then we trigger the NULL pointer
>>>> de-reference.
>>>>
>>>> workqueues are not tasklets, the CPU scheduling them gets no guarantee
>>>> they will run on the same CPU.
>>>
>>> Something does not add up.
>>>
>>> The synchronous call to phy_state_machine() does:
>>>
>>> case PHY_HALTED:
>>> if (phydev->link) {
>>> phydev->link = 0;
>>> netif_carrier_off(phydev->attached_dev);
>>> phy_adjust_link(phydev);
>>> do_suspend = true;
>>> }
>>>
>>> then sets phydev->link = 0; therefore subsequent calls to
>>> phy_state_machin() will be no-op.
>>
>> Actually you are right, once phydev->link is set to 0 these would become
>> no-ops. Still scratching my head as to what happens for David then...
>>
>>>
>>> Also, queue_delayed_work() is only called in polling mode.
>>> David stated that he's using interrupt mode.
>
> Did you see what I wrote?
>
> phy_disconnect() calls phy_stop_interrupts() which puts it into polling
> mode. So the polling work gets queued unconditionally.
I did address that remark in
https://www.mail-archive.com/netdev@vger.kernel.org/msg186336.html
int phy_stop_interrupts(struct phy_device *phydev)
{
int err = phy_disable_interrupts(phydev);
if (err)
phy_error(phydev);
free_irq(phydev->irq, phydev);
/* If work indeed has been cancelled, disable_irq() will have
* been left unbalanced from phy_interrupt() and enable_irq()
* has to be called so that other devices on the line work.
*/
while (atomic_dec_return(&phydev->irq_disable) >= 0)
enable_irq(phydev->irq);
return err;
}
Which part of this function changes phydev->irq to PHY_POLL?
Perhaps phydev->drv->config_intr?
What PHY are you using?
Regards.
Waiting for the PHY to complete auto-negotiation
Hello, I've been trying to wrap my head around Ethernet auto-negotiation, vs actual / real packets seen at the MAC layer. I found the relevant Wikipedia article to be fairly informative: https://en.wikipedia.org/wiki/Autonegotiation The reason I care is that my Ethernet HW does not allow changing the flow control setting once the MAC has started (more specifically, once RX DMA has been enabled). In nb8800_open(), the code currently works in this order: nb8800_start_rx(dev); phy_start(phydev); The first line enables the MAC (and DMA). The second enables the PHY and starts auto-negotiation. This is a problem: I would like for PHY auto-negotiation to be /complete/ before I enable the MAC. What is the recommended way to wait for the PHY? AFAICT, the PHY layer calls back into the eth driver through the adjust_link() callback registered through of_phy_connect(). It seems like this might be a good place to enable the MAC? (When some other conditions are true.) Regards.
Re: Waiting for the PHY to complete auto-negotiation
On 06/12/2017 17:59, Andrew Lunn wrote: > On Wed, Dec 06, 2017 at 05:39:00PM +0100, Mason wrote: > >> I've been trying to wrap my head around Ethernet auto-negotiation, >> vs actual / real packets seen at the MAC layer. I found the relevant >> Wikipedia article to be fairly informative: >> >> https://en.wikipedia.org/wiki/Autonegotiation >> >> The reason I care is that my Ethernet HW does not allow changing the >> flow control setting once the MAC has started (more specifically, once >> RX DMA has been enabled). >> >> In nb8800_open(), the code currently works in this order: >> >> nb8800_start_rx(dev); >> phy_start(phydev); >> >> The first line enables the MAC (and DMA). >> The second enables the PHY and starts auto-negotiation. >> >> This is a problem: I would like for PHY auto-negotiation to be >> /complete/ before I enable the MAC. >> >> What is the recommended way to wait for the PHY? > >> AFAICT, the PHY layer calls back into the eth driver through the >> adjust_link() callback registered through of_phy_connect(). >> It seems like this might be a good place to enable the MAC? > > That probably works, but you might have a few corner cases to handle. > I'm not sure changes at the PHY always transition through down. So you > could for example get a callback saying the link is up, 1Gbps, then a > second call saying it has dropped to 100Mbps, if your > cables/connectors are bad. > > If your hardware has problems, it might be safest to stop everything > in the callback, make configuration changes, and they start everything > back up. A link negotiation change is not something you expect to > happen often. So making it slow but robust is O.K. What you've described is, in fact, the existing implementation! ;-) nb8800_pause_config() checks for netif_running() and, when true, tries to disable everything, make the change, then re-enable everything. The problem with this is the following mind-boggling quirk of the hardware: once RX DMA is enabled, there is no supported way to disable it! Thus, I'm trying to find a clean way to set the control flow parameter BEFORE enabling the MAC. Regards.
Re: Waiting for the PHY to complete auto-negotiation
On 06/12/2017 19:26, Andrew Lunn wrote: > On 06/12/2017 19:03, Mason wrote: > >> The problem with this is the following mind-boggling quirk of >> the hardware: once RX DMA is enabled, there is no supported >> way to disable it! Thus, I'm trying to find a clean way to set >> the control flow parameter BEFORE enabling the MAC. > > There is no solution. I can pull the cable out, and plug it into a > different computer. The first could use flow control, the second not. > > If you cannot disable RX DMA, you should probably disable unloading of > the module. Also kexec. It is not possible to /selectively/ disable RX DMA, but after a reset, DMA is disabled. So the solution I propose is this: When we detect link down, we put the chip in reset, so that we will repeat initialization when the link comes back up. Mans has been reluctant to adopt this approach, but this is what I implemented in my forked version. I'm trying to push my changes upstream, to minimize the differences. Regards.
Re: Waiting for the PHY to complete auto-negotiation
On 06/12/2017 20:07, Andrew Lunn wrote: > By chip, you mean the Ethernet controller? Not the whole SoC? Doh! Yes. Let me rephrase. When we detect link down, we put the ethernet HW block in reset, and repeat initialization when the link comes back up. Hmmm, however, at the moment, I only reset on an administrative (user-requested) link down, i.e. through ndo_stop. I would probably have to handle cable unplug/replug events as well. Or just consider the quirk to make flow control too complicated to implement correctly... Regards.
Re: Waiting for the PHY to complete auto-negotiation
On 07/12/2017 00:00, Florian Fainelli wrote: > On 12/06/2017 11:25 AM, Mason wrote: > >> When we detect link down, we put the ethernet HW block in reset, >> and repeat initialization when the link comes back up. >> >> Hmmm, however, at the moment, I only reset on an administrative >> (user-requested) link down, i.e. through ndo_stop. I would probably >> have to handle cable unplug/replug events as well. >> >> Or just consider the quirk to make flow control too complicated >> to implement correctly... > > I suppose your procedure is fine, but don't you have a better way to > resolve that by trying to place a special RX DMA ring entry that allows > your RX DMA not to be entirely stopped, but intentionally looped through > a buffer that you control? As long as you can stop the Ethernet MAC RX, > working with such a limitation is probably fine, but this really sounds > like a huge pain in the butt and a major HW flaw. Could you elaborate a bit on your suggestion? (Special ring entry, looped through a buffer under my control) Is this a typical thing to do to stop DMA? Currently the driver tries to stop DMA in nb8800_dma_stop(), which does the following: http://elixir.free-electrons.com/linux/latest/source/drivers/net/ethernet/aurora/nb8800.c#L881 1) poll until TX finishes (I assume the system no longer accepts new frames to send at this point) 2) set the EOC (end of chain) bit on all descriptors (could there be a problem if we receive a frame at that moment? Don't we need some kind of lock?) 3) disable address filtering (need to check what this does) 4) enable loop-back mode 5) send up to 5 "fake" packets in order to hit an EOC descriptor The reason I'm trying to move away from this method is that it doesn't work on our new SoC; and when pressed, the HW dev said it had never been supported. (Also I find it somewhat hackish, but that's a matter of taste.) Regards.
Re: Waiting for the PHY to complete auto-negotiation
+ Mans On 09/12/2017 19:49, Florian Fainelli wrote: > Having any HW state machine requiring X number of clock cycles to > guarantee a full transition to a stopped state is not unusual, however, > the fact that you need to send 5 packets to guarantee an EOC descriptor > is hit is completely unusual. Ideally there is a single bit that tells > the DMA engine: you are enabled, do your thing, or you are now disabled, > and you must stop all accesses to DRAM *now*. > > So what would be the correct way to quiesce that controller according to > your HW folks? He (it's a single person) offered to run some RTL-level simulations, but then moved on to more important tasks. At some point he wrote: > If you reset the DMA enable in the middle of a DMA, the hardware > state machine doesn't return to the IDLE state if it has more > descriptors to process. It should be noted that the RX DMA has been > designed by our IP vendor with no intention of stopping the DMA in > the middle of its operation. While the documentation for the IP states: > Receive DMA Channel Disabling > > When the entire receive frame has been read from the Receive FIFO and > sent over the AMBA bus, the DMA operation ends, and the Receive DMA > Channel is automatically disabled. To do this, hardware resets the > Enable bit in the Receive Channel Control Register to "0" after the > last data has been read from the Receive FIFO and sent over the AMBA > bus. > > When operating in descriptor mode, upon completion of a receive frame > DMA operation, if the descriptor chain has not ended when a receive > frame DMA operation completes, the next receive frame DMA operation > begins. The last descriptor in a descriptor chain is indicated by > having its End Of Chain- EOC, flag set to "1". If this EOC flag is > "0", to begin the next receive frame DMA operation, the next > descriptor is automatically retrieved and used to configure the > Receive DMA Channel. The Receive DMA Channel is then automatically > re-enabled and the next receive frame DMA operation begins. > > In descriptor mode, an AMBA bus error can occur when reading receive > descriptor data. If this happens, receive descriptor processing ends > and the Receive DMA Channel is turned off. The Descriptor Error bit > in the Receive Status Register is set to "1". I don't see how sending "fake" packets through the loop back would be considered "resetting the DMA enable in the middle of a DMA". (I'm afraid the HW dev didn't grasp what the driver is doing.) I suppose I should test forcefully setting the enable bit to 0 in the driver, and see if hell breaks loose. Regards.
Re: Waiting for the PHY to complete auto-negotiation
On 11/12/2017 15:36, Måns Rullgård wrote: > Mason writes: > >> I suppose I should test forcefully setting the enable bit to 0 in >> the driver, and see if hell breaks loose. > > You can't. When the enable bit is 1, writes to that register are > ignored. It goes back to 0 automatically when the hw runs out of > descriptors. I don't think they are ignored, because toggling the control flow bit actually breaks RX. Regards.
Re: [RFC PATCH v1] net: ethernet: nb8800: Reset HW block in ndo_open
On 29/07/2017 13:24, Måns Rullgård wrote: > Until you figure out why it's getting stuck, we can't be sure > it isn't caused by something that could trigger at any time. Would you take a look at it, if I can reproduce on tango4? I have identified a 100% reproducible flaw. I have proposed a work-around that brings this down to 0 (tested 1000 cycles of link up / ping / link down). In my opinion, upstream should consider this work-around for inclusion. I'd like to hear David's and Florian's opinion on the topic. It's always a pain to maintain out-of-tree patches. > Yes, but by then you've reset those parameters to the defaults. Good catch. There is some non HW-related init in nb8800_hw_init(). I'll take this opportunity to change flow control to off by default (it breaks several 100 Mbps switches). I'll take a closer look at priv assignments on Monday. Regards.
Re: [RFC PATCH v1] net: ethernet: nb8800: Reset HW block in ndo_open
On 29/07/2017 14:05, Måns Rullgård wrote: > Mason writes: > >> I'll take this opportunity to change flow control to >> off by default (it breaks several 100 Mbps switches). > > I was told to have it on by default. This is what most other drivers > do too. If you have faulty switches, that's your problem. Or maybe the fault is in the HW implementation, and it is the board's fault that the connection is hosed. How many switches did you test the driver on? We tested 4 switches, and DHCP failed on 3 of them. Disabling pause frames "fixed" that. I could spend weeks testing dozens of switches, but I have to prioritize. Ethernet flow control is simply not an important feature in the market the SoC is intended for. Regards.
Re: [RFC PATCH v1] net: ethernet: nb8800: Reset HW block in ndo_open
On 29/07/2017 22:15, Florian Fainelli wrote: > On 07/29/2017 05:44 AM, Mason wrote: > >> We tested 4 switches, and DHCP failed on 3 of them. >> Disabling pause frames "fixed" that. > > OK, so it is this problem that you reported about before? The "Ethernet flow control / pause frames" issue is separate from the "link down wedges RX" issue. We discussed the former back in November 2016: https://www.mail-archive.com/netdev@vger.kernel.org/msg137094.html https://patchwork.ozlabs.org/patch/694577/ Wait a second... I see that you and Mans had the following exchange: https://www.mail-archive.com/netdev@vger.kernel.org/msg138175.html Mans mentions disabling DMA to be able to change the flow control bits. The current theory is that it is disabling DMA in ndo_stop that wedges RX. So maybe the two issues are related after all... I hate all these hardware quirks. Why can't HW engineers make stuff that "just works"... > Pause frames are tricky in that receiving pause frames means you > should backpressure your transmitter and sending pause frames happens > when your receiver cannot keep up. It is somewhat conceivable that > your HW implementation is bogus and that you can get the HW in a > state where it gets permanently backpressured for instance? And then > only a full re-init would get you out of this stuck state presumably? > Are there significant differences at the DMA/Ethernet controller > level between Tango 3 (is that the one Mans worked on?) and Tango 4 > for instance that could explain a behavioral difference? I'll have to take a look at the issue in light of the new information. FWIW, Mans has tango3&4 boards. I work on newer boards. The HW dev *swears* there have been no functional differences in the eth block "forever". However, bus accesses are faster in recent chips, which could change who wins specific races. Regards.
Re: [RFC PATCH v1] net: ethernet: nb8800: Reset HW block in ndo_open
On 29/07/2017 17:18, Florian Fainelli wrote: > On 07/29/2017 05:02 AM, Mason wrote: > >> I have identified a 100% reproducible flaw. >> I have proposed a work-around that brings this down to 0 >> (tested 1000 cycles of link up / ping / link down). > > Can you also try to get help from your HW resources to eventually help > you find out what is going on here? The patch I proposed /is/ based on the feedback from the HW team :-( "Just reset the HW block, and everything will work as expected." >> In my opinion, upstream should consider this work-around >> for inclusion. I'd like to hear David's and Florian's >> opinion on the topic. It's always a pain to maintain >> out-of-tree patches. > > I have to agree with Mans here that the commit message explanation is > not good enough to understand how the RX path is hosed after a call to > ndo_stop() it would be good, both for you and for the people maintaining > this driver to understand what happens exactly so the fix is correct, > understood and maintainable. The patch itself looks reasonable with the > limited description given, but it's the description itself that needs > changing. I have logged all register reads/writes occurring while nb8800_stop() is executing. 1) BOARD A -- EVERYTHING WORKS AS EXPECTED # test_eth.sh [ 13.293669] nb8800 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx 172.27.64.1 : xmt/rcv/%loss = 1/1/0%, min/avg/max = 0.41/0.41/0.41 [ 15.874627] nb8800_stop from __dev_close_many [ 15.879044] ++ETH++ gw32 reg=f0026020 val=0092 [ 15.883900] ++ETH++ gw32 reg=f0026020 val=8092 [ 15.888969] ++ETH++ gr32 reg=f0026024 val=ec00 [ 15.893809] ++ETH++ gw32 reg=f0026020 val=0492 [ 15.898697] ++ETH++ gw32 reg=f0026020 val=8492 [ 15.903582] ++ETH++ gw32 reg=f0026020 val=0093 [ 15.908423] ++ETH++ gw32 reg=f0026020 val=8093 [ 15.913272] ++ETH++ gr32 reg=f0026024 val= [ 15.918160] ++ETH++ gr8 reg=f0026004 val=2b [ 15.922459] ++ETH++ gw8 reg=f0026004 val=0b [ 15.926782] ++ETH++ gr8 reg=f0026044 val=81 [ 15.931123] ++ETH++ gw8 reg=f0026044 val=85 [ 15.935457] ++ETH++ gw32 reg=f002610c val=9de74000 [ 15.940317] ++ETH++ gw32 reg=f0026100 val=005c0aff [ 15.945187] ENTER nb8800_irq [ 15.948077] ++ETH++ gr32 reg=f0026104 val=0004 [ 15.952887] ++ETH++ gw32 reg=f0026104 val=0004 [ 15.957697] ++ETH++ gr32 reg=f0026204 val=0004 [ 15.962507] ++ETH++ gw32 reg=f0026204 val=0004 [ 15.967316] ++ETH++ gw32 reg=f0026218 val=003cc4a4 [ 15.972149] ENTER nb8800_irq [ 15.975039] ++ETH++ gr32 reg=f0026104 val=0001 [ 15.979848] ++ETH++ gw32 reg=f0026104 val=0001 [ 15.984658] ++ETH++ gr32 reg=f0026204 val= [ 16.045509] ++ETH++ gw32 reg=f002610c val=9de74000 [ 16.050329] ++ETH++ gw32 reg=f0026100 val=005c0aff [ 16.055150] ENTER nb8800_irq [ 16.058042] ++ETH++ gr32 reg=f0026104 val=0004 [ 16.062852] ++ETH++ gw32 reg=f0026104 val=0004 [ 16.067662] ++ETH++ gr32 reg=f0026204 val=0004 [ 16.072470] ++ETH++ gw32 reg=f0026204 val=0004 [ 16.077279] ++ETH++ gw32 reg=f0026218 val=003cc4a4 [ 16.082100] ENTER nb8800_irq [ 16.084993] ++ETH++ gr32 reg=f0026104 val=0001 [ 16.089802] ++ETH++ gw32 reg=f0026104 val=0001 [ 16.094611] ++ETH++ gr32 reg=f0026204 val= [ 16.099454] ++ETH++ gr8 reg=f0026004 val=0b [ 16.103752] ++ETH++ gw8 reg=f0026004 val=2b [ 16.108057] ++ETH++ gr8 reg=f0026044 val=85 [ 16.112353] ++ETH++ gw8 reg=f0026044 val=81 [ 16.116699] ++ETH++ gw32 reg=f002620c val=9f7b2000 [ 16.121528] ++ETH++ gr8 reg=f0026004 val=2b [ 16.125827] ++ETH++ gw8 reg=f0026004 val=2a [ 16.130126] ++ETH++ gr32 reg=f0026100 val=00080afe [ 16.134945] ++ETH++ gr8 reg=f0026000 val=1d [ 16.139238] ++ETH++ gw8 reg=f0026000 val=1c [ 16.143534] ++ETH++ gw32 reg=f0026020 val=0092 [ 16.148363] ++ETH++ gw32 reg=f0026020 val=8092 [ 16.153209] ++ETH++ gr32 reg=f0026024 val= [ 16.158027] ++ETH++ gw32 reg=f0026020 val=0492 [ 16.162856] ++ETH++ gw32 reg=f0026020 val=8492 [ 16.167702] ++ETH++ gw32 reg=f0026020 val=0093 [ 16.172531] ++ETH++ gw32 reg=f0026020 val=8093 [ 16.177377] ++ETH++ gr32 reg=f0026024 val= [ 16.182338] nb8800 26000.ethernet eth0: Link is Down [ 16.187361] ++ETH++ gw32 reg=f0026020 val=0092 [ 16.192194] ++ETH++ gw32 reg=f0026020 val=8092 [ 16.197052] ++ETH++ gr32 reg=f0026024 val= [ 16.201887] ++ETH++ gw32 reg=f0026020 val=0092 [ 16.206717] ++ETH++ gw32 reg=f0026020 val=8092 [ 16.211575] ++ETH++ gr32 reg=f0026024 val= [ 16.216394] ++ETH++ gw32 reg=f0026020 val=0080 [ 16.221235] ++ETH++ gw32 reg=f0026020 val=8080 [ 16.226084] ++ETH++ gr32 reg=f0026024 val=1000 [ 16.230913] ++ETH++ gw32 reg=f0026020 val=04801800 [ 16.235742] ++ETH++
Re: [RFC PATCH v1] net: ethernet: nb8800: Reset HW block in ndo_open
On 31/07/2017 13:59, Måns Rullgård wrote:
> Mason writes:
>
>> On 29/07/2017 17:18, Florian Fainelli wrote:
>>
>>> On 07/29/2017 05:02 AM, Mason wrote:
>>>
>>>> I have identified a 100% reproducible flaw.
>>>> I have proposed a work-around that brings this down to 0
>>>> (tested 1000 cycles of link up / ping / link down).
>>>
>>> Can you also try to get help from your HW resources to eventually help
>>> you find out what is going on here?
>>
>> The patch I proposed /is/ based on the feedback from the HW team :-(
>> "Just reset the HW block, and everything will work as expected."
>
> Nobody is saying a reset won't recover the lockup. The problem is that
> we don't know what caused it to lock up in the first place. How do we
> know it can't happen during normal operation? If we knew the cause, it
> might also be possible to avoid the situation entirely.
How does one prove that something "can't happen during normal operation"?
The "put adapter in loop-back mode so we can send ourselves fake packets"
shenanigans seems completely insane, if you ask me.
Other things make no sense to me, for example in nb8800_dma_stop()
there is a polling loop:
do {
mdelay(100);
nb8800_writel(priv, NB8800_TX_DESC_ADDR, txb->dma_desc);
wmb();
mdelay(100);
nb8800_writel(priv, NB8800_TXC_CR, txcr | TCR_EN);
mdelay(5500);
err = readl_poll_timeout_atomic(priv->base + NB8800_RXC_CR,
rxcr, !(rxcr & RCR_EN),
1000, 10);
printk("err=%d retry=%d\n", err, retry);
} while (err && --retry);
(It was me who added the delays.)
*Whatever* delays I insert, it always goes 3 times through the loop.
[ 29.654492] ++ETH++ gw32 reg=f002610c val=9ecc8000
[ 29.759320] ++ETH++ gw32 reg=f0026100 val=005c0aff
[ 35.364705] err=-110 retry=5
[ 35.467609] ++ETH++ gw32 reg=f002610c val=9ecc8000
[ 35.572436] ++ETH++ gw32 reg=f0026100 val=005c0aff
[ 41.177822] err=-110 retry=4
[ 41.280726] ++ETH++ gw32 reg=f002610c val=9ecc8000
[ 41.385553] ++ETH++ gw32 reg=f0026100 val=005c0aff
[ 46.890907] err=0 retry=3
How is that possible?
I've tried using spinlocks and delays to get parallel execution
down to a minimum, and have the same logs on both boards.
Regards.
Re: [RFC PATCH v1] net: ethernet: nb8800: Reset HW block in ndo_open
On 31/07/2017 16:08, Mason wrote:
> Other things make no sense to me, for example in nb8800_dma_stop()
> there is a polling loop:
>
> do {
> mdelay(100);
> nb8800_writel(priv, NB8800_TX_DESC_ADDR, txb->dma_desc);
> wmb();
> mdelay(100);
> nb8800_writel(priv, NB8800_TXC_CR, txcr | TCR_EN);
>
> mdelay(5500);
>
> err = readl_poll_timeout_atomic(priv->base + NB8800_RXC_CR,
> rxcr, !(rxcr & RCR_EN),
> 1000, 10);
> printk("err=%d retry=%d\n", err, retry);
> } while (err && --retry);
>
>
> (It was me who added the delays.)
>
> *Whatever* delays I insert, it always goes 3 times through the loop.
>
> [ 29.654492] ++ETH++ gw32 reg=f002610c val=9ecc8000
> [ 29.759320] ++ETH++ gw32 reg=f0026100 val=005c0aff
> [ 35.364705] err=-110 retry=5
> [ 35.467609] ++ETH++ gw32 reg=f002610c val=9ecc8000
> [ 35.572436] ++ETH++ gw32 reg=f0026100 val=005c0aff
> [ 41.177822] err=-110 retry=4
> [ 41.280726] ++ETH++ gw32 reg=f002610c val=9ecc8000
> [ 41.385553] ++ETH++ gw32 reg=f0026100 val=005c0aff
> [ 46.890907] err=0 retry=3
>
> How is that possible?
First time through the loop, it doesn't matter how long we poll,
it *always* times out. Second time as well (only on BOARD B).
Third time, it succeeds quickly (first or second poll).
(This explains why various delays had no impact.)
In fact, requesting the transfer 3 times *before* polling
makes the polling succeed quickly:
nb8800_writel(priv, NB8800_TX_DESC_ADDR, txb->dma_desc);
wmb();
nb8800_writel(priv, NB8800_TXC_CR, txcr | TCR_EN);
[ 16.464596] ++ETH++ gw32 reg=f002610c val=9ef28000
[ 16.469414] ++ETH++ gw32 reg=f0026100 val=005c0aff
[ 16.474231] ++ETH++ gw32 reg=f002610c val=9ef28000
[ 16.479048] ++ETH++ gw32 reg=f0026100 val=005c0aff
[ 16.483865] ++ETH++ gw32 reg=f002610c val=9ef28000
[ 16.488682] ++ETH++ gw32 reg=f0026100 val=005c0aff
[ 16.493500] ++ETH++ POLL reg=f0026200 val=06100a8f
[ 16.499317] ++ETH++ POLL reg=f0026200 val=06100a8e
[ 16.504134] err=0 retry=5
With my changes, I get *exactly* the same logs on BOARD A
and BOARD B (modulo the descriptors addresses).
Yet BOARD A stays functional, but BOARD B is hosed...
Depressing. I've run out of ideas.
BOARD A LOGS:
# test_eth.sh
[ 18.037782] nb8800 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow
control rx/tx
172.27.64.1 : xmt/rcv/%loss = 1/1/0%, min/avg/max = 0.39/0.39/0.39
[ 20.617917] nb8800_stop from __dev_close_many
[ 20.622314] ++ETH++ gw32 reg=f0026020 val=0092
[ 20.627135] ++ETH++ gw32 reg=f0026020 val=8092
[ 20.631973] ++ETH++ gr32 reg=f0026024 val=ec00
[ 20.636782] ++ETH++ gw32 reg=f0026020 val=0492
[ 20.641601] ++ETH++ gw32 reg=f0026020 val=8492
[ 20.646440] ++ETH++ gw32 reg=f0026020 val=0093
[ 20.651258] ++ETH++ gw32 reg=f0026020 val=8093
[ 20.656095] ++ETH++ gr32 reg=f0026024 val=
[ 20.660909] ++ETH++ POLL reg=f0026100 val=005c0afe
[ 20.665743] ++ETH++ gr8 reg=f0026004 val=2b
[ 20.670028] ++ETH++ gw8 reg=f0026004 val=0b
[ 20.674313] ++ETH++ gr8 reg=f0026044 val=81
[ 20.678598] ++ETH++ gw8 reg=f0026044 val=85
[ 20.682883] ++ETH++ gw32 reg=f002610c val=9de0c000
[ 20.687693] ++ETH++ gw32 reg=f0026100 val=005c0aff
[ 20.692503] ++ETH++ gw32 reg=f002610c val=9de0c000
[ 20.697312] ++ETH++ gw32 reg=f0026100 val=005c0aff
[ 20.702121] ++ETH++ gw32 reg=f002610c val=9de0c000
[ 20.706929] ++ETH++ gw32 reg=f0026100 val=005c0aff
[ 20.712738] ++ETH++ POLL reg=f0026200 val=06100a8e
[ 20.717547] err=0 retry=5
[ 20.720172] ++ETH++ gr8 reg=f0026004 val=0b
[ 20.724456] ++ETH++ gw8 reg=f0026004 val=2b
[ 20.728742] ++ETH++ gr8 reg=f0026044 val=85
[ 20.733026] ++ETH++ gw8 reg=f0026044 val=81
[ 20.737349] ++ETH++ gw32 reg=f002620c val=9de04000
[ 20.742158] nb8800_dma_stop=0
[ 21.845224] ENTER nb8800_irq
[ 21.848116] ++ETH++ gr32 reg=f0026104 val=0005
[ 21.852927] ++ETH++ gw32 reg=f0026104 val=0005
[ 21.857738] ++ETH++ gr32 reg=f0026204 val=0004
[ 21.862547] ++ETH++ gw32 reg=f0026204 val=0004
[ 21.867356] ++ETH++ gw32 reg=f0026218 val=003cc4a4
[ 21.872164] EXIT nb8800_irq
[ 24.373448] ++ETH++ gr8 reg=f0026004 val=2b
[ 24.377755] ++ETH++ gw8 reg=f0026004 val=2a
[ 24.382054] ++ETH++ gr32 reg=f0026100 val=00080afe
[ 24.386876] ++ETH++ gr8 reg=f0026000 val=1d
[ 24.391192] ++ETH++ gw8 reg=f0026000 val=1c
[ 25.706747] ++ETH++ gw32 reg=f0026020 val=0092
[ 25.711578] ++ETH++ gw32 reg=f0026020 val=8092
[ 25.716427] ++ETH++ gr32 reg=f0026024 val=
[ 25.721248] ++ETH++ gw32 reg=f0026020 val=0492
[ 25.726091] ++ETH++ gw32 reg=f002602
[RFC PATCH v2 0/2] nb8800 suspend/resume support
Hello, I need suspend/resume support in the nb8800 driver. On tango platforms, suspend loses all context (MMIO registers). To make the task easy, we just close the device on suspend, and open it again on resume. This requires properly resetting the HW on resume. Patch 1 moves all the HW init to nb8800_init() Patch 2 adds suspend/resume support Regards.
[RFC PATCH v2 1/2] net: ethernet: nb8800: Reset HW block in ndo_open
Move all HW initializations to nb8800_init.
This provides the basis for suspend/resume support.
---
drivers/net/ethernet/aurora/nb8800.c | 50 +---
drivers/net/ethernet/aurora/nb8800.h | 1 +
2 files changed, 25 insertions(+), 26 deletions(-)
diff --git a/drivers/net/ethernet/aurora/nb8800.c
b/drivers/net/ethernet/aurora/nb8800.c
index e94159507847..aa18ea25d91f 100644
--- a/drivers/net/ethernet/aurora/nb8800.c
+++ b/drivers/net/ethernet/aurora/nb8800.c
@@ -39,6 +39,7 @@
#include "nb8800.h"
+static void nb8800_init(struct net_device *dev);
static void nb8800_tx_done(struct net_device *dev);
static int nb8800_dma_stop(struct net_device *dev);
@@ -957,6 +958,8 @@ static int nb8800_open(struct net_device *dev)
struct phy_device *phydev;
int err;
+ nb8800_init(dev);
+
/* clear any pending interrupts */
nb8800_writel(priv, NB8800_RXC_SR, 0xf);
nb8800_writel(priv, NB8800_TXC_SR, 0xf);
@@ -1246,11 +1249,6 @@ static int nb8800_hw_init(struct net_device *dev)
nb8800_writeb(priv, NB8800_PQ1, val >> 8);
nb8800_writeb(priv, NB8800_PQ2, val & 0xff);
- /* Auto-negotiate by default */
- priv->pause_aneg = true;
- priv->pause_rx = true;
- priv->pause_tx = true;
-
nb8800_mc_init(dev, 0);
return 0;
@@ -1350,6 +1348,20 @@ static const struct of_device_id nb8800_dt_ids[] = {
};
MODULE_DEVICE_TABLE(of, nb8800_dt_ids);
+static void nb8800_init(struct net_device *dev)
+{
+ struct nb8800_priv *priv = netdev_priv(dev);
+ const struct nb8800_ops *ops = priv->ops;
+
+ if (ops && ops->reset)
+ ops->reset(dev);
+ nb8800_hw_init(dev);
+ if (ops && ops->init)
+ ops->init(dev);
+ nb8800_update_mac_addr(dev);
+ priv->speed = 0;
+}
+
static int nb8800_probe(struct platform_device *pdev)
{
const struct of_device_id *match;
@@ -1389,6 +1401,7 @@ static int nb8800_probe(struct platform_device *pdev)
priv = netdev_priv(dev);
priv->base = base;
+ priv->ops = ops;
priv->phy_mode = of_get_phy_mode(pdev->dev.of_node);
if (priv->phy_mode < 0)
@@ -1407,12 +1420,6 @@ static int nb8800_probe(struct platform_device *pdev)
spin_lock_init(&priv->tx_lock);
- if (ops && ops->reset) {
- ret = ops->reset(dev);
- if (ret)
- goto err_disable_clk;
- }
-
bus = devm_mdiobus_alloc(&pdev->dev);
if (!bus) {
ret = -ENOMEM;
@@ -1454,21 +1461,16 @@ static int nb8800_probe(struct platform_device *pdev)
priv->mii_bus = bus;
- ret = nb8800_hw_init(dev);
- if (ret)
- goto err_deregister_fixed_link;
-
- if (ops && ops->init) {
- ret = ops->init(dev);
- if (ret)
- goto err_deregister_fixed_link;
- }
-
dev->netdev_ops = &nb8800_netdev_ops;
dev->ethtool_ops = &nb8800_ethtool_ops;
dev->flags |= IFF_MULTICAST;
dev->irq = irq;
+ /* Auto-negotiate by default */
+ priv->pause_aneg = true;
+ priv->pause_rx = true;
+ priv->pause_tx = true;
+
mac = of_get_mac_address(pdev->dev.of_node);
if (mac)
ether_addr_copy(dev->dev_addr, mac);
@@ -1476,14 +1478,12 @@ static int nb8800_probe(struct platform_device *pdev)
if (!is_valid_ether_addr(dev->dev_addr))
eth_hw_addr_random(dev);
- nb8800_update_mac_addr(dev);
-
netif_carrier_off(dev);
ret = register_netdev(dev);
if (ret) {
netdev_err(dev, "failed to register netdev\n");
- goto err_free_dma;
+ goto err_deregister_fixed_link;
}
netif_napi_add(dev, &priv->napi, nb8800_poll, NAPI_POLL_WEIGHT);
@@ -1492,8 +1492,6 @@ static int nb8800_probe(struct platform_device *pdev)
return 0;
-err_free_dma:
- nb8800_dma_free(dev);
err_deregister_fixed_link:
if (of_phy_is_fixed_link(pdev->dev.of_node))
of_phy_deregister_fixed_link(pdev->dev.of_node);
diff --git a/drivers/net/ethernet/aurora/nb8800.h
b/drivers/net/ethernet/aurora/nb8800.h
index 6ec4a956e1e5..d5f4481a2c7b 100644
--- a/drivers/net/ethernet/aurora/nb8800.h
+++ b/drivers/net/ethernet/aurora/nb8800.h
@@ -305,6 +305,7 @@ struct nb8800_priv {
dma_addr_t tx_desc_dma;
struct clk *clk;
+ const struct nb8800_ops *ops;
};
struct nb8800_ops {
--
2.11.0
[RFC PATCH v2 2/2] net: ethernet: nb8800: Add suspend/resume support
Wrappers around nb8800_stop and nb8800_open.
---
drivers/net/ethernet/aurora/nb8800.c | 23 ++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/aurora/nb8800.c
b/drivers/net/ethernet/aurora/nb8800.c
index aa18ea25d91f..607064a6d7a1 100644
--- a/drivers/net/ethernet/aurora/nb8800.c
+++ b/drivers/net/ethernet/aurora/nb8800.c
@@ -1012,7 +1012,6 @@ static int nb8800_stop(struct net_device *dev)
netif_stop_queue(dev);
napi_disable(&priv->napi);
- nb8800_dma_stop(dev);
nb8800_mac_rx(dev, false);
nb8800_mac_tx(dev, false);
@@ -1526,6 +1525,26 @@ static int nb8800_remove(struct platform_device *pdev)
return 0;
}
+static int nb8800_suspend(struct platform_device *pdev, pm_message_t state)
+{
+ struct net_device *dev = platform_get_drvdata(pdev);
+
+ if (netif_running(dev))
+ nb8800_stop(dev);
+
+ return 0;
+}
+
+static int nb8800_resume(struct platform_device *pdev)
+{
+ struct net_device *dev = platform_get_drvdata(pdev);
+
+ if (netif_running(dev))
+ nb8800_open(dev);
+
+ return 0;
+}
+
static struct platform_driver nb8800_driver = {
.driver = {
.name = "nb8800",
@@ -1533,6 +1552,8 @@ static struct platform_driver nb8800_driver = {
},
.probe = nb8800_probe,
.remove = nb8800_remove,
+ .suspend = nb8800_suspend,
+ .resume = nb8800_resume,
};
module_platform_driver(nb8800_driver);
--
2.11.0
Re: [RFC PATCH v2 1/2] net: ethernet: nb8800: Reset HW block in ndo_open
On 02/08/2017 13:02, Måns Rullgård wrote: > Mason wrote: > >> Move all HW initializations to nb8800_init. >> This provides the basis for suspend/resume support. >> --- >> drivers/net/ethernet/aurora/nb8800.c | 50 >> +--- >> drivers/net/ethernet/aurora/nb8800.h | 1 + >> 2 files changed, 25 insertions(+), 26 deletions(-) > > You're still not doing anything to preserve flow control and multicast > configuration, and those are probably not the only ones. I did handle flow control (as far as I can tell). The current settings are stored in: priv->pause_aneg, priv->pause_rx, priv->pause_tx and nb8800_pause_config() is called from nb8800_link_reconfigure() I'll take a closer look at multicast settings. > Worse, you're now never tearing down dma properly, which means the > hardware can be left with active pointers to memory no longer allocated > to it. You're making it sound like there is a risk those pointers might be used somehow. There is no such risk AFAICT. The PHY is disconnected, NAPI is stopped, RX and TX have been disabled, and the ISR has been removed. I have considered putting the HW block in reset (clock gated) at the end of nb8800_stop() to save power, which would make the issue you point out moot. The reason I removed nb8800_dma_stop() in nb8800_stop() is because it hangs when called from nb8800_suspend(). (It requires interrupts to make progress, and interrupts seem to be disabled when nb8800_suspend() is called.) Broader question for netdev: I've been wondering about costly close operations. If closing a device is complex (in terms of code), at which point does it become simpler to just reset the block, and avoid writing/maintaining/debugging the code performing said close operation? > Finally, you still haven't explained why the hw needs to be reset in > ndo_open(). Whatever is causing your lockup can almost certainly be > triggered in some other way too. I will not accept this side-stepping > of the issue. (I was not aware that you were the final authority on which patches are accepted upstream.) FWIW, I have placed a formal request for the HW dev to investigate, as I could not reproduce on tango4, despite several days wasted on this issue. In the mean time, the driver in its current form does not support suspend/resume. How would *you* implement it? (IMO, my way is great in its simplicity and code reuse.) Regards.
Re: [RFC PATCH v2 0/2] nb8800 suspend/resume support
On 01/08/2017 18:32, Mason wrote: > I need suspend/resume support in the nb8800 driver. > On tango platforms, suspend loses all context (MMIO registers). > To make the task easy, we just close the device on suspend, > and open it again on resume. This requires properly resetting > the HW on resume. > > Patch 1 moves all the HW init to nb8800_init() > Patch 2 adds suspend/resume support I have now confirmed that the "flow control" issue I reported in another thread has nothing to do with flow control per se. The problem is that nb8800_pause_config() calls nb8800_dma_stop() and when it does, RX is borked. On a GigE switch: [ 21.444268] ENTER nb8800_pause_config [ 21.448604] rxcr=06100a8f pause_rx=1 pause_tx=0 pause=1 asym_pause=1 [ 21.455020] nb8800 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx In this case, pause_tx and RCR_FL match, so we skip the silly dance. On a FastE switch: [ 11.611613] ENTER nb8800_pause_config [ 11.615942] rxcr=06100a8f pause_rx=1 pause_tx=1 pause=1 asym_pause=0 [ 11.825535] nb8800 26000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx In this case pause_tx and RCR_FL don't match, so we hit nb8800_dma_stop() and the HW block becomes deaf. In conclusion, two issues I've been discussing: A) RX wedged after ndo_close B) flow control breaks on FastE switches are in fact two symptoms of the same root issue. Regards.
Re: [RFC PATCH v2 0/2] nb8800 suspend/resume support
On 02/08/2017 17:36, Måns Rullgård wrote: > Mason wrote: > >> Looking at the tango-specific integration, I note this nugget: >> >> 1.5.4 Stopping & Starting the DMA >> >> This feature has been added to allow the software to stop and start >> the DMA without any issues. >> >> Procedure: >> 1- STOP: >> 2- Stop RX core; >> 3- Set OWN bit of all descriptor of the chain to 1; >> 4- Stop DMA by writing dma_stop bit to 1 in RX_DMA_Stop register >> 5- Wait around 100 clock cycles. >> >> The pending packets are held until the system will re-start. >> >> RE-START: >> 1- Clear dma_stop bit (note that if at the time of stopping the DMA, >> the next packet in the FIFO was an UDP packet, when clearing dma_stop, >> this packet will directly start being written in the DRAM since UDP >> packets are not controlled by the descriptor mechanism); >> 2- Program a new chain of descriptor; >> 3- Re-enable DMA (rx_ctrl register) >> >> rx_dma_stop: >> Software control to stop the Rx DMA. >> A write to this bit with "1" will gracefully stop the Rx DMA by after >> transferring the current packet. If more packets are pending they will >> be held until the software clears this bit. >> >> Hmmm, what do you think? This looks promising... > > This is only available in the more recent Sigma versions. Although it > is nicer, I didn't think it was worth the trouble to support both > methods since the older method should work on all chips. Well, from my perspective, the older method does not work on newer chips :-) The "new" method is available on the SMP8670 onward (released Jan 2011). I might try implementing it for "sigma,smp8734-ethernet" boards. Regards.
Re: [RFC PATCH v2 0/2] nb8800 suspend/resume support
On 02/08/2017 17:56, Måns Rullgård wrote: > Mason writes: > >> From my perspective, the older method does not work on newer chips :-) > > It does work on tango4. Agreed. > What does the tango5 do if you flood it with packets faster than the > kernel can keep up with? That would make it hit the end of the rx > chain, which is apparently what makes it miserable with the current dma > stop code. The simplest way to test this would be sending tiny packets as fast as possible, right? So ping -f on a GigE link should fit the bill? Regards.
Re: [RFC PATCH v2 0/2] nb8800 suspend/resume support
On 02/08/2017 18:10, Måns Rullgård wrote: > Mason writes: > >> On 02/08/2017 17:56, Måns Rullgård wrote: >> >>> What does the tango5 do if you flood it with packets faster than the >>> kernel can keep up with? That would make it hit the end of the rx >>> chain, which is apparently what makes it miserable with the current dma >>> stop code. >> >> The simplest way to test this would be sending tiny packets >> as fast as possible, right? So ping -f on a GigE link should >> fit the bill? > > ping -f is limited to 100 packets per second. Use something like iperf > in UDP mode instead. ping -f can go 100 times faster than 100 pps: # ping -f -q -c 15 -s 300 172.27.64.45 PING 172.27.64.45 (172.27.64.45) 300(328) bytes of data. --- 172.27.64.45 ping statistics --- 15 packets transmitted, 15 received, 0% packet loss, time 15035ms rtt min/avg/max/mdev = 0.065/0.084/0.537/0.014 ms, ipg/ewma 0.100/0.087 ms 150,000 packets in 15 seconds = 10,000 pps (172.27.64.45 is the tango5 board) Ergo, dealing with 10,000 packets per second does not hose RX. Regards.
Re: [RFC PATCH v2 0/2] nb8800 suspend/resume support
On 02/08/2017 18:10, Måns Rullgård wrote: > ping -f is limited to 100 packets per second. > Use something like iperf in UDP mode instead. CLIENT: DESKTOP # iperf3 -c 172.27.64.45 -u -b 950M Connecting to host 172.27.64.45, port 5201 [ 4] local 172.27.64.1 port 55533 connected to 172.27.64.45 port 5201 [ ID] Interval Transfer Bandwidth Total Datagrams [ 4] 0.00-1.00 sec 102 MBytes 858 Mbits/sec 13091 [ 4] 1.00-2.00 sec 114 MBytes 953 Mbits/sec 14541 [ 4] 2.00-3.00 sec 114 MBytes 953 Mbits/sec 14542 [ 4] 3.00-4.00 sec 114 MBytes 953 Mbits/sec 14541 [ 4] 4.00-5.00 sec 114 MBytes 953 Mbits/sec 14542 [ 4] 5.00-6.00 sec 114 MBytes 953 Mbits/sec 14541 [ 4] 6.00-7.00 sec 114 MBytes 953 Mbits/sec 14535 [ 4] 7.00-8.00 sec 114 MBytes 953 Mbits/sec 14536 [ 4] 8.00-9.00 sec 114 MBytes 953 Mbits/sec 14543 [ 4] 9.00-10.00 sec 114 MBytes 953 Mbits/sec 14541 - - - - - - - - - - - - - - - - - - - - - - - - - [ ID] Interval Transfer Bandwidth JitterLost/Total Datagrams [ 4] 0.00-10.00 sec 1.10 GBytes 943 Mbits/sec 0.247 ms 132176/143788 (92%) [ 4] Sent 143788 datagrams iperf Done. SERVER: TANGO BOARD # iperf3 -s Accepted connection from 172.27.64.1, port 44995 [ 5] local 172.27.64.45 port 5201 connected to 172.27.64.1 port 55533 [ ID] Interval Transfer Bandwidth JitterLost/Total Datagrams [ 5] 0.00-1.00 sec 7.90 MBytes 66.2 Mbits/sec 0.218 ms 11365/12376 (92%) [ 5] 1.00-2.00 sec 9.13 MBytes 76.6 Mbits/sec 0.230 ms 13381/14550 (92%) [ 5] 2.00-3.00 sec 9.12 MBytes 76.5 Mbits/sec 0.290 ms 13372/14540 (92%) [ 5] 3.00-4.00 sec 9.11 MBytes 76.4 Mbits/sec 0.292 ms 13369/14535 (92%) [ 5] 4.00-5.00 sec 9.18 MBytes 77.0 Mbits/sec 0.178 ms 13374/14549 (92%) [ 5] 5.00-6.00 sec 9.10 MBytes 76.3 Mbits/sec 0.228 ms 13367/14532 (92%) [ 5] 6.00-7.00 sec 9.26 MBytes 77.7 Mbits/sec 0.607 ms 13356/14541 (92%) [ 5] 7.00-8.00 sec 9.23 MBytes 77.4 Mbits/sec 0.507 ms 13364/14545 (92%) [ 5] 8.00-9.00 sec 9.20 MBytes 77.1 Mbits/sec 0.215 ms 13351/14528 (92%) [ 5] 9.00-10.00 sec 9.16 MBytes 76.9 Mbits/sec 0.188 ms 13356/14529 (92%) [ 5] 10.00-10.04 sec 336 KBytes 72.2 Mbits/sec 0.247 ms 521/563 (93%) - - - - - - - - - - - - - - - - - - - - - - - - - [ ID] Interval Transfer Bandwidth JitterLost/Total Datagrams [ 5] 0.00-10.04 sec 0.00 Bytes 0.00 bits/sec 0.247 ms 132176/143788 (92%) There is some packet loss, but the board doesn't lose connectivity. With smaller packets... # iperf3 -c 172.27.64.45 -u -b 950M -l 800 Connecting to host 172.27.64.45, port 5201 [ 4] local 172.27.64.1 port 35197 connected to 172.27.64.45 port 5201 [ ID] Interval Transfer Bandwidth Total Datagrams [ 4] 0.00-1.00 sec 90.6 MBytes 760 Mbits/sec 118724 [ 4] 1.00-2.00 sec 107 MBytes 894 Mbits/sec 139718 [ 4] 2.00-3.00 sec 106 MBytes 889 Mbits/sec 138918 [ 4] 3.00-4.00 sec 107 MBytes 895 Mbits/sec 139768 [ 4] 4.00-5.00 sec 106 MBytes 891 Mbits/sec 139275 [ 4] 5.00-6.00 sec 107 MBytes 895 Mbits/sec 139862 [ 4] 6.00-7.00 sec 107 MBytes 895 Mbits/sec 139825 [ 4] 7.00-8.00 sec 107 MBytes 895 Mbits/sec 139775 [ 4] 8.00-9.00 sec 107 MBytes 895 Mbits/sec 139849 [ 4] 9.00-10.00 sec 107 MBytes 895 Mbits/sec 139835 - - - - - - - - - - - - - - - - - - - - - - - - - [ ID] Interval Transfer Bandwidth JitterLost/Total Datagrams [ 4] 0.00-10.00 sec 1.02 GBytes 880 Mbits/sec 0.009 ms 564117/1375520 (41%) [ 4] Sent 1375520 datagrams iperf Done. Accepted connection from 172.27.64.1, port 46151 [ 5] local 172.27.64.45 port 5201 connected to 172.27.64.1 port 35197 [ ID] Interval Transfer Bandwidth JitterLost/Total Datagrams [ 5] 0.00-1.00 sec 60.8 MBytes 510 Mbits/sec 0.004 ms 33508/113252 (30%) iperf3: OUT OF ORDER - incoming packet = 147146 and received packet = 0 AND SP = 147497 iperf3: OUT OF ORDER - incoming packet = 146128 and received packet = 0 AND SP = 147690 iperf3: OUT OF ORDER - incoming packet = 146067 and received packet = 0 AND SP = 147863 iperf3: OUT OF ORDER - incoming packet = 147242 and received packet = 0 AND SP = 148039 iperf3: OUT OF ORDER - incoming packet = 163837 and received packet = 0 AND SP = 164363 [ 5] 1.00-2.00 sec 61.0 MBytes 511 Mbits/sec 0.198 ms 59635/139649 (43%) iperf3: OUT OF ORDER - incoming packet = 286437 and received packet = 0 AND SP = 287226 iperf3: OUT OF ORDER - incoming packet = 302990 and received packet = 0 AND SP = 305710 [ 5] 2.00-3.00 sec 61.5 MBytes 517 Mbits/sec 0.005 ms 58369/138944 (42%) iperf3: OUT OF ORDER - incoming packet =
Re: [RFC PATCH v2 0/2] nb8800 suspend/resume support
On 02/08/2017 19:31, Mason wrote: > # iperf3 -c 172.27.64.45 -u -b 950M > Connecting to host 172.27.64.45, port 5201 > [ 4] local 172.27.64.1 port 55533 connected to 172.27.64.45 port 5201 > [ ID] Interval Transfer Bandwidth Total Datagrams > [ 4] 0.00-1.00 sec 102 MBytes 858 Mbits/sec 13091 > [ 4] 1.00-2.00 sec 114 MBytes 953 Mbits/sec 14541 114 MB in 14541 packets => 7840 bytes per packet Is iperf3 sending jumbo frames?? In the nb8800 driver, RX_BUF_SIZE is only 1552, how would it deal with jumbo frames... truncate? > # iperf3 -c 172.27.64.45 -u -b 950M -l 800 > Connecting to host 172.27.64.45, port 5201 > [ 4] local 172.27.64.1 port 35197 connected to 172.27.64.45 port 5201 > [ ID] Interval Transfer Bandwidth Total Datagrams > [ 4] 0.00-1.00 sec 90.6 MBytes 760 Mbits/sec 118724 > [ 4] 1.00-2.00 sec 107 MBytes 894 Mbits/sec 139718 107 MB in 139718 packets => 766 bytes per packet 800 - 8 (UDP) - 20 (IPv4) = 772 bytes per packet I suppose that's close enough... 772 * 139718 = 107.86 MB I need to run the test slightly slower, to prevent packet loss at the sender. Perhaps -b 0 or -b 800M Regards.
Re: [RFC PATCH v2 0/2] nb8800 suspend/resume support
On 02/08/2017 22:02, Mason wrote: > I need to run the test slightly slower, to prevent packet loss > at the sender. # iperf3 -c 172.27.64.45 -u -b 0 -l 1000 Connecting to host 172.27.64.45, port 5201 [ 4] local 172.27.64.1 port 42607 connected to 172.27.64.45 port 5201 [ ID] Interval Transfer Bandwidth Total Datagrams [ 4] 0.00-1.00 sec 111 MBytes 931 Mbits/sec 116420 [ 4] 1.00-2.00 sec 111 MBytes 931 Mbits/sec 116390 [ 4] 2.00-3.00 sec 111 MBytes 930 Mbits/sec 116220 [ 4] 3.00-4.00 sec 111 MBytes 930 Mbits/sec 116310 [ 4] 4.00-5.00 sec 111 MBytes 931 Mbits/sec 116380 [ 4] 5.00-6.00 sec 111 MBytes 930 Mbits/sec 116280 [ 4] 6.00-7.00 sec 111 MBytes 931 Mbits/sec 116390 [ 4] 7.00-8.00 sec 111 MBytes 931 Mbits/sec 116370 [ 4] 8.00-9.00 sec 111 MBytes 931 Mbits/sec 116340 [ 4] 9.00-10.00 sec 111 MBytes 930 Mbits/sec 116310 - - - - - - - - - - - - - - - - - - - - - - - - - [ ID] Interval Transfer Bandwidth JitterLost/Total Datagrams [ 4] 0.00-10.00 sec 1.08 GBytes 931 Mbits/sec 0.009 ms 278644/1163363 (24%) [ 4] Sent 1163363 datagrams iperf Done. # iperf3 -s Accepted connection from 172.27.64.1, port 42966 [ 5] local 172.27.64.45 port 5201 connected to 172.27.64.1 port 42607 [ ID] Interval Transfer Bandwidth JitterLost/Total Datagrams [ 5] 0.00-1.00 sec 81.1 MBytes 681 Mbits/sec 0.017 ms 26834/111909 (24%) [ 5] 1.00-2.00 sec 84.2 MBytes 706 Mbits/sec 0.019 ms 28127/116384 (24%) [ 5] 2.00-3.00 sec 84.2 MBytes 706 Mbits/sec 0.013 ms 27946/116204 (24%) [ 5] 3.00-4.00 sec 84.5 MBytes 709 Mbits/sec 0.013 ms 27674/116311 (24%) [ 5] 4.00-5.00 sec 84.6 MBytes 709 Mbits/sec 0.015 ms 27712/116387 (24%) [ 5] 5.00-6.00 sec 84.5 MBytes 709 Mbits/sec 0.010 ms 27649/116265 (24%) [ 5] 6.00-7.00 sec 84.3 MBytes 707 Mbits/sec 0.011 ms 27995/116382 (24%) [ 5] 7.00-8.00 sec 84.3 MBytes 707 Mbits/sec 0.013 ms 27972/116387 (24%) [ 5] 8.00-9.00 sec 84.3 MBytes 708 Mbits/sec 0.020 ms 27899/116343 (24%) [ 5] 9.00-10.00 sec 84.4 MBytes 708 Mbits/sec 0.014 ms 27759/116305 (24%) [ 5] 10.00-10.04 sec 3.25 MBytes 710 Mbits/sec 0.009 ms 1077/4486 (24%) - - - - - - - - - - - - - - - - - - - - - - - - - [ ID] Interval Transfer Bandwidth JitterLost/Total Datagrams [ 5] 0.00-10.04 sec 0.00 Bytes 0.00 bits/sec 0.009 ms 278644/1163363 (24%) IIUC, sender (desktop system) sends datagrams as fast as possible. Receiver (tango board) drops around 24% of all datagrams. I think this invalidates the theory that exhausting RX descriptors wedges RX DMA. Regards.
Toggling link state breaks network connectivity
Hello, I am using the following drivers for Ethernet connectivity. drivers/net/ethernet/aurora/nb8800.c drivers/net/phy/at803x.c Pulling the cable and plugging it back works as expected. (I can ping both before and after.) However, if I toggle the link state in software (using ip link set), the board loses network connectivity. # Statically assign IP address ip addr add 172.27.64.77/18 brd 172.27.127.255 dev eth0 # Set link state to "up" ip link set eth0 up # ping -c 3 172.27.64.1 > /tmp/v1 PING 172.27.64.1 (172.27.64.1): 56 data bytes 64 bytes from 172.27.64.1: seq=0 ttl=64 time=18.321 ms --- 172.27.64.1 ping statistics --- 3 packets transmitted, 1 packets received, 66% packet loss round-trip min/avg/max = 18.321/18.321/18.321 ms 172.27.64.1 is a desktop system. Running % tcpdump -n -i eth1-boards ether host 00:16:e8:4d:7f:c4 on the desktop, I get: 15:01:45.187346 ARP, Request who-has 172.27.64.1 tell 172.27.64.77, length 46 15:01:45.187359 ARP, Reply 172.27.64.1 is-at 00:15:17:24:e0:81, length 28 15:01:45.194633 IP 172.27.64.77 > 172.27.64.1: ICMP echo request, id 41219, seq 0, length 64 15:01:45.194662 IP 172.27.64.1 > 172.27.64.77: ICMP echo reply, id 41219, seq 0, length 64 15:01:50.198564 ARP, Request who-has 172.27.64.77 tell 172.27.64.1, length 28 15:01:50.205929 IP 172.27.64.77 > 172.27.64.1: ICMP echo request, id 41219, seq 1, length 64 15:01:50.205951 IP 172.27.64.1 > 172.27.64.77: ICMP echo reply, id 41219, seq 1, length 64 15:01:50.213217 IP 172.27.64.77 > 172.27.64.1: ICMP echo request, id 41219, seq 2, length 64 15:01:50.213232 IP 172.27.64.1 > 172.27.64.77: ICMP echo reply, id 41219, seq 2, length 64 15:01:51.198563 ARP, Request who-has 172.27.64.77 tell 172.27.64.1, length 28 15:01:51.209586 ARP, Reply 172.27.64.77 is-at 00:16:e8:4d:7f:c4, length 46 15:01:51.209598 ARP, Reply 172.27.64.77 is-at 00:16:e8:4d:7f:c4, length 46 Packet #1: the board asks for the desktop's MAC address Packet #2: the desktop replies instantly Packet #3: the board sends the first ping Packet #4: the desktop replies instantly Then the board goes quiet for a long time (why???) Packet #5: the desktop asks for the board's MAC address (doesn't it have it already?) Packet #6: this seems to unwedge the board, which sends the second ping Packet #7: the desktop replies instantly Packet #8: the board sends the third ping Packet #9: the desktop replies instantly Packet #10: the desktop asks again for the board's MAC address Packet #11 and #12: the board answers twice (for the old and new requests?) Some oddities, but it seems to work. Now toggle the link state: % ip link set eth0 down % ip link set eth0 up % ping -c 3 172.27.64.1 > /tmp/v2 PING 172.27.64.1 (172.27.64.1): 56 data bytes --- 172.27.64.1 ping statistics --- 3 packets transmitted, 0 packets received, 100% packet loss On the desktop, I see 15:14:03.900162 ARP, Request who-has 172.27.64.1 tell 172.27.64.77, length 46 15:14:03.900175 ARP, Reply 172.27.64.1 is-at 00:15:17:24:e0:81, length 28 15:14:05.017189 ARP, Request who-has 172.27.64.1 tell 172.27.64.77, length 46 15:14:05.017200 ARP, Reply 172.27.64.1 is-at 00:15:17:24:e0:81, length 28 15:14:06.030531 ARP, Request who-has 172.27.64.1 tell 172.27.64.77, length 46 15:14:06.030541 ARP, Reply 172.27.64.1 is-at 00:15:17:24:e0:81, length 28 So basically, the board is asking the desktop for its MAC address, and the desktop is answering immediately. But the board doesn't seem to be getting the replies... Any ideas, or words of wisdom, as they say? Regards.
Re: Toggling link state breaks network connectivity
Hello Florian, On 12/06/2017 18:38, Florian Fainelli wrote: > On 06/12/2017 06:22 AM, Mason wrote: > >> I am using the following drivers for Ethernet connectivity. >> drivers/net/ethernet/aurora/nb8800.c >> drivers/net/phy/at803x.c >> >> Pulling the cable and plugging it back works as expected. >> (I can ping both before and after.) >> >> However, if I toggle the link state in software (using ip link set), >> the board loses network connectivity. >> >> # Statically assign IP address >> ip addr add 172.27.64.77/18 brd 172.27.127.255 dev eth0 >> # Set link state to "up" >> ip link set eth0 up >> # ping -c 3 172.27.64.1 > /tmp/v1 >> >> PING 172.27.64.1 (172.27.64.1): 56 data bytes >> 64 bytes from 172.27.64.1: seq=0 ttl=64 time=18.321 ms > > This delay seems abnormally long unless you are purposely introducing > delay (e.g: with cls_netem) or this is a really remote host, does not > seem to be based on your traces later on. 172.27.64.1 and 172.27.64.77 are connected to the same switch. Purely local traffic. It seems to me that the ARP request/reply could explain the delay. Start op at 45.187346 Receive ICMP echo reply at 45.194662 Hmmm, that's only 7 ms >> 172.27.64.1 is a desktop system. >> Running >> % tcpdump -n -i eth1-boards ether host 00:16:e8:4d:7f:c4 >> on the desktop, I get: >> >> 15:01:45.187346 ARP, Request who-has 172.27.64.1 tell 172.27.64.77, length 46 >> 15:01:45.187359 ARP, Reply 172.27.64.1 is-at 00:15:17:24:e0:81, length 28 >> 15:01:45.194633 IP 172.27.64.77 > 172.27.64.1: ICMP echo request, id 41219, >> seq 0, length 64 >> 15:01:45.194662 IP 172.27.64.1 > 172.27.64.77: ICMP echo reply, id 41219, >> seq 0, length 64 >> 15:01:50.198564 ARP, Request who-has 172.27.64.77 tell 172.27.64.1, length 28 >> 15:01:50.205929 IP 172.27.64.77 > 172.27.64.1: ICMP echo request, id 41219, >> seq 1, length 64 >> 15:01:50.205951 IP 172.27.64.1 > 172.27.64.77: ICMP echo reply, id 41219, >> seq 1, length 64 >> 15:01:50.213217 IP 172.27.64.77 > 172.27.64.1: ICMP echo request, id 41219, >> seq 2, length 64 >> 15:01:50.213232 IP 172.27.64.1 > 172.27.64.77: ICMP echo reply, id 41219, >> seq 2, length 64 >> 15:01:51.198563 ARP, Request who-has 172.27.64.77 tell 172.27.64.1, length 28 >> 15:01:51.209586 ARP, Reply 172.27.64.77 is-at 00:16:e8:4d:7f:c4, length 46 >> 15:01:51.209598 ARP, Reply 172.27.64.77 is-at 00:16:e8:4d:7f:c4, length 46 >> >> Packet #1: the board asks for the desktop's MAC address >> Packet #2: the desktop replies instantly >> Packet #3: the board sends the first ping >> Packet #4: the desktop replies instantly >> Then the board goes quiet for a long time (why???) >> Packet #5: the desktop asks for the board's MAC address (doesn't it have it >> already?) >> Packet #6: this seems to unwedge the board, which sends the second ping >> Packet #7: the desktop replies instantly >> Packet #8: the board sends the third ping >> Packet #9: the desktop replies instantly >> Packet #10: the desktop asks again for the board's MAC address >> Packet #11 and #12: the board answers twice (for the old and new requests?) >> >> Some oddities, but it seems to work. >> >> Now toggle the link state: >> >> % ip link set eth0 down >> % ip link set eth0 up >> % ping -c 3 172.27.64.1 > /tmp/v2 >> >> PING 172.27.64.1 (172.27.64.1): 56 data bytes >> >> --- 172.27.64.1 ping statistics --- >> 3 packets transmitted, 0 packets received, 100% packet loss >> >> >> On the desktop, I see >> >> 15:14:03.900162 ARP, Request who-has 172.27.64.1 tell 172.27.64.77, length 46 >> 15:14:03.900175 ARP, Reply 172.27.64.1 is-at 00:15:17:24:e0:81, length 28 >> 15:14:05.017189 ARP, Request who-has 172.27.64.1 tell 172.27.64.77, length 46 >> 15:14:05.017200 ARP, Reply 172.27.64.1 is-at 00:15:17:24:e0:81, length 28 >> 15:14:06.030531 ARP, Request who-has 172.27.64.1 tell 172.27.64.77, length 46 >> 15:14:06.030541 ARP, Reply 172.27.64.1 is-at 00:15:17:24:e0:81, length 28 >> >> So basically, the board is asking the desktop for its MAC address, >> and the desktop is answering immediately. But the board doesn't seem >> to be getting the replies... Any ideas, or words of wisdom, as they say? > > - check the Ethernet MAC counters to see if there is packet loss, or > error, or both I'll take a look, but I don't expect any packet loss (LAN traffic on an idle switch). > - consult with your HW engineers for possible flaws in your > ndo_open/ndo_close paths and possible interactions with the MAC/PHY > clocks, or reset etc. (The HW engineers have no knowledge of Linux use-cases.) The crazy thing is that I can use the same driver on the previous chip, and I don't see this behavior... Will retest tomorrow to be sure. What does change between the two chips are a few clock frequencies though. So maybe some race is now consistently lost on the new chip... > - see if your PHY needs a complete re-init after an up/down sequence and > if you are doing this properly Thanks for these suggestions. I'll take a closer look tomorrow. Regards.
Re: Toggling link state breaks network connectivity
On 13/06/2017 11:39, Matthias May wrote: > On 12/06/17 15:22, Mason wrote: >> Hello, >> >> I am using the following drivers for Ethernet connectivity. >> drivers/net/ethernet/aurora/nb8800.c >> drivers/net/phy/at803x.c >> >> Pulling the cable and plugging it back works as expected. >> (I can ping both before and after.) >> >> However, if I toggle the link state in software (using ip link set), >> the board loses network connectivity. >> >> # Statically assign IP address >> ip addr add 172.27.64.77/18 brd 172.27.127.255 dev eth0 >> # Set link state to "up" >> ip link set eth0 up >> # ping -c 3 172.27.64.1 > /tmp/v1 >> >> PING 172.27.64.1 (172.27.64.1): 56 data bytes >> 64 bytes from 172.27.64.1: seq=0 ttl=64 time=18.321 ms >> >> --- 172.27.64.1 ping statistics --- >> 3 packets transmitted, 1 packets received, 66% packet loss >> round-trip min/avg/max = 18.321/18.321/18.321 ms >> >> >> 172.27.64.1 is a desktop system. >> Running >> % tcpdump -n -i eth1-boards ether host 00:16:e8:4d:7f:c4 >> on the desktop, I get: >> >> 15:01:45.187346 ARP, Request who-has 172.27.64.1 tell 172.27.64.77, length 46 >> 15:01:45.187359 ARP, Reply 172.27.64.1 is-at 00:15:17:24:e0:81, length 28 >> 15:01:45.194633 IP 172.27.64.77 > 172.27.64.1: ICMP echo request, id 41219, >> seq 0, length 64 >> 15:01:45.194662 IP 172.27.64.1 > 172.27.64.77: ICMP echo reply, id 41219, >> seq 0, length 64 >> 15:01:50.198564 ARP, Request who-has 172.27.64.77 tell 172.27.64.1, length 28 >> 15:01:50.205929 IP 172.27.64.77 > 172.27.64.1: ICMP echo request, id 41219, >> seq 1, length 64 >> 15:01:50.205951 IP 172.27.64.1 > 172.27.64.77: ICMP echo reply, id 41219, >> seq 1, length 64 >> 15:01:50.213217 IP 172.27.64.77 > 172.27.64.1: ICMP echo request, id 41219, >> seq 2, length 64 >> 15:01:50.213232 IP 172.27.64.1 > 172.27.64.77: ICMP echo reply, id 41219, >> seq 2, length 64 >> 15:01:51.198563 ARP, Request who-has 172.27.64.77 tell 172.27.64.1, length 28 >> 15:01:51.209586 ARP, Reply 172.27.64.77 is-at 00:16:e8:4d:7f:c4, length 46 >> 15:01:51.209598 ARP, Reply 172.27.64.77 is-at 00:16:e8:4d:7f:c4, length 46 >> >> Packet #1: the board asks for the desktop's MAC address >> Packet #2: the desktop replies instantly >> Packet #3: the board sends the first ping >> Packet #4: the desktop replies instantly >> Then the board goes quiet for a long time (why???) >> Packet #5: the desktop asks for the board's MAC address (doesn't it have it >> already?) >> Packet #6: this seems to unwedge the board, which sends the second ping >> Packet #7: the desktop replies instantly >> Packet #8: the board sends the third ping >> Packet #9: the desktop replies instantly >> Packet #10: the desktop asks again for the board's MAC address >> Packet #11 and #12: the board answers twice (for the old and new requests?) >> >> Some oddities, but it seems to work. >> >> Now toggle the link state: >> >> % ip link set eth0 down >> % ip link set eth0 up >> % ping -c 3 172.27.64.1 > /tmp/v2 >> >> PING 172.27.64.1 (172.27.64.1): 56 data bytes >> >> --- 172.27.64.1 ping statistics --- >> 3 packets transmitted, 0 packets received, 100% packet loss >> >> >> On the desktop, I see >> >> 15:14:03.900162 ARP, Request who-has 172.27.64.1 tell 172.27.64.77, length 46 >> 15:14:03.900175 ARP, Reply 172.27.64.1 is-at 00:15:17:24:e0:81, length 28 >> 15:14:05.017189 ARP, Request who-has 172.27.64.1 tell 172.27.64.77, length 46 >> 15:14:05.017200 ARP, Reply 172.27.64.1 is-at 00:15:17:24:e0:81, length 28 >> 15:14:06.030531 ARP, Request who-has 172.27.64.1 tell 172.27.64.77, length 46 >> 15:14:06.030541 ARP, Reply 172.27.64.1 is-at 00:15:17:24:e0:81, length 28 >> >> So basically, the board is asking the desktop for its MAC address, >> and the desktop is answering immediately. But the board doesn't seem >> to be getting the replies... Any ideas, or words of wisdom, as they say? > > You might want to read this thread: > https://www.mail-archive.com/netdev@vger.kernel.org/msg133896.html > The symptoms you describe are pretty much what we had at one point. Hello Matthias, Yes, I remember discussing the issue with Zefir. https://www.mail-archive.com/netdev@vger.kernel.org/msg147369.html Do you think I should apply Zefir's patch? In an attempt to remove the Atheros driver from the equation, I used the generic PHY driver (I didn't build the Atheros driver at all). But this had no impact on the issue. Regards.
Re: Toggling link state breaks network connectivity
On 12/06/2017 18:38, Florian Fainelli wrote: > On 06/12/2017 06:22 AM, Mason wrote: > >> I am using the following drivers for Ethernet connectivity. >> drivers/net/ethernet/aurora/nb8800.c >> drivers/net/phy/at803x.c >> >> Pulling the cable and plugging it back works as expected. >> (I can ping both before and after.) >> >> However, if I toggle the link state in software (using ip link set), >> the board loses network connectivity. >> >> # Statically assign IP address >> ip addr add 172.27.64.77/18 brd 172.27.127.255 dev eth0 >> # Set link state to "up" >> ip link set eth0 up >> # ping -c 3 172.27.64.1 > /tmp/v1 >> >> PING 172.27.64.1 (172.27.64.1): 56 data bytes >> 64 bytes from 172.27.64.1: seq=0 ttl=64 time=18.321 ms > > This delay seems abnormally long unless you are purposely introducing > delay (e.g: with cls_netem) or this is a really remote host, does not > seem to be based on your traces later on. I think the delay is due to calling ping before the link is actually up. For example, if I ping immediately after setting the link up, the first 4 packets are lost. PING 172.27.64.1 (172.27.64.1): 56 data bytes 64 bytes from 172.27.64.1: seq=4 ttl=64 time=0.235 ms 64 bytes from 172.27.64.1: seq=5 ttl=64 time=0.142 ms 64 bytes from 172.27.64.1: seq=6 ttl=64 time=0.110 ms 64 bytes from 172.27.64.1: seq=7 ttl=64 time=0.095 ms 64 bytes from 172.27.64.1: seq=8 ttl=64 time=0.139 ms 64 bytes from 172.27.64.1: seq=9 ttl=64 time=0.120 ms --- 172.27.64.1 ping statistics --- 10 packets transmitted, 6 packets received, 40% packet loss round-trip min/avg/max = 0.095/0.140/0.235 ms >> So basically, the board is asking the desktop for its MAC address, >> and the desktop is answering immediately. But the board doesn't seem >> to be getting the replies... Any ideas, or words of wisdom, as they say? > > - check the Ethernet MAC counters to see if there is packet loss, or > error, or both > > - consult with your HW engineers for possible flaws in your > ndo_open/ndo_close paths and possible interactions with the MAC/PHY > clocks, or reset etc. > > - see if your PHY needs a complete re-init after an up/down sequence and > if you are doing this properly I'm using the following test script: ip addr add 172.27.64.77/18 brd 172.27.127.255 dev eth0 ip link set eth0 up sleep 3 ## hopefully autoneg is complete ethtool -S eth0 > /tmp/s0 ping -c 10 172.27.64.1 > /tmp/v1 ethtool -S eth0 > /tmp/s1 diff -U0 /tmp/s0 /tmp/s1 ip link set eth0 down sleep 1 ip link set eth0 up sleep 1 ethtool -S eth0 > /tmp/s0 ping -c 10 172.27.64.1 > /tmp/v2 ethtool -S eth0 > /tmp/s1 diff -U0 /tmp/s0 /tmp/s1 Testing with a generic PHY driver (no Atheros 8035 support built). Apparently, ethtool doesn't report any packet loss or error. First time: # diff -U0 /tmp/s0 /tmp/s1 --- /tmp/s0 +++ /tmp/s1 @@ -2,2 +2,2 @@ - rx_bytes_ok: 0 - rx_frames_ok: 0 + rx_bytes_ok: 1084 + rx_frames_ok: 11 @@ -6,2 +6,2 @@ - rx_64_byte_frames: 0 - rx_127_byte_frames: 0 + rx_64_byte_frames: 1 + rx_127_byte_frames: 10 @@ -22,6 +22,6 @@ - rx_bytes: 0 - rx_frames: 0 - tx_bytes_ok: 0 - tx_frames_ok: 0 - tx_64_byte_frames: 0 - tx_127_byte_frames: 0 + rx_bytes: 1084 + rx_frames: 11 + tx_bytes_ok: 1084 + tx_frames_ok: 11 + tx_64_byte_frames: 1 + tx_127_byte_frames: 10 @@ -33 +33 @@ - tx_broadcast_frames: 0 + tx_broadcast_frames: 1 @@ -43,2 +43,2 @@ - tx_bytes: 0 - tx_frames: 0 + tx_bytes: 1084 + tx_frames: 11 Second time: # diff -U0 /tmp/s0 /tmp/s1 --- /tmp/s0 +++ /tmp/s1 @@ -2,2 +2,2 @@ - rx_bytes_ok: 1276 - rx_frames_ok: 14 + rx_bytes_ok: 1779 + rx_frames_ok: 19 @@ -6 +6 @@ - rx_64_byte_frames: 4 + rx_64_byte_frames: 8 @@ -8 +8 @@ - rx_255_byte_frames: 0 + rx_255_byte_frames: 1 @@ -14 +14 @@ - rx_broadcast_frames: 0 + rx_broadcast_frames: 1 @@ -22,5 +22,5 @@ - rx_bytes: 1276 - rx_frames: 14 - tx_bytes_ok: 1276 - tx_frames_ok: 14 - tx_64_byte_frames: 4 + rx_bytes: 1779 + rx_frames: 19 + tx_bytes_ok: 1724 + tx_frames_ok: 21 + tx_64_byte_frames: 11 @@ -33 +33 @@ - tx_broadcast_frames: 1 + tx_broadcast_frames: 8 @@ -43,2 +43,2 @@ - tx_bytes: 1276 - tx_frames: 14 + tx_bytes: 1724 + tx_frames: 21 I did note something that seems important. If I toggle the link state in software, then connectivity breaks. If I unplug the ethernet cable, and replug, connectivity remains. The difference is that plugging/unplugging doesn't call the .ndo_stop callback. But 'ip link set eth0 down' does call it. Should the .ndo_stop callback be symmetric to the .ndo_open callback? In other words, should .ndo_open(); .ndo_stop(); be a NOP? Regards.
Re: Toggling link state breaks network connectivity
On 13/06/2017 17:07, Mason wrote: > I did note something that seems important. > > If I toggle the link state in software, then connectivity breaks. > > If I unplug the ethernet cable, and replug, connectivity remains. > > The difference is that plugging/unplugging doesn't call the > .ndo_stop callback. But 'ip link set eth0 down' does call it. > > Should the .ndo_stop callback be symmetric to the .ndo_open callback? > In other words, should .ndo_open(); .ndo_stop(); be a NOP? I changed the ndo_open callback to a wrapper that calls: nb8800_open(dev); nb8800_stop(dev); nb8800_open(dev); With this change, connectivity is broken from the start. Valid ARP requests are correctly *sent* but the corresponding ARP replies are not received. I'm hoping this limits the scope of what needs to be investigated. Regards.
Re: Toggling link state breaks network connectivity
On 13/06/2017 17:36, Florian Fainelli wrote: > On 06/13/2017 08:07 AM, Mason wrote: > >> I did note something that seems important. >> If I toggle the link state in software, then connectivity breaks. >> If I unplug the ethernet cable, and replug, connectivity remains. > > What does that actually mean? If you disconnect the cable a link state > should be notified and another link state should be notified, even if > that happens faster than the PHYLIB polling time (1s). Sorry for being unclear. If I unplug/replug the cable, ping still works afterward. (Packet RX appears to be *not* wedged) If I toggle the link state in SW (set link down/set link up), I can no longer ping afterward. (Packet RX appears to be wedged, so ARP replies are not seen) Maybe the two experiments are too unrelated to consider the different results relevant in any way? Regards.
Re: Toggling link state breaks network connectivity
On 13/06/2017 17:50, Florian Fainelli wrote: > On 06/13/2017 08:47 AM, Mason wrote: > >> If I unplug/replug the cable, ping still works afterward. >> (Packet RX appears to be *not* wedged) >> >> If I toggle the link state in SW (set link down/set link up), >> I can no longer ping afterward. >> (Packet RX appears to be wedged, so ARP replies are not seen) >> >> Maybe the two experiments are too unrelated to consider >> the different results relevant in any way? > > No, they are not, this really tells you that whatever your ndo_open() > and ndo_stop() functions do, they are currently broken with your > particular HW. I have seen numerous problems on some of our older > platforms using bcmgenet where the PHY needs to be reset *before* any > MAC activity occurs, and any MAC activity even qualifies as a reset of > the MAC itself, we can perform a PHY reset there because the PHY is > internal, that may not be an option, and your HW is different anyway. I have logged *all* register writes, to compare state after probe, after open, and after close. I do see a few odd values... I'm still deep in the maze, but I may find an exit eventually. What state is probe supposed to leave the HW in? What state is open supposed to leave the HW in? I'm thinking that I could reset the entire block in close. Regards. Found ethernet phy : AR8035 [0.852746] libphy: Fixed MDIO Bus: probed [0.856975] ++ETH++ gw8 0xf0026424 0x00 # ETH_SW_RESET [0.870942] ++ETH++ gw8 0xf0026424 0x01 # ETH_SW_RESET [0.874894] ++ETH++ gw16 0xf0026420 0x0050 # ETH_MDIO_CLK_DIV [0.879101] libphy: nb8800-mii: probed [0.885005] ++ETH++ gw8 0xf0026000 0x1c # ETH_MAC_CORE_TRANSMIT_CONTROL [0.888967] ++ETH++ gw8 0xf0026001 0x05 # ETH_MAC_CORE_TRANSMIT_CONTROL [0.892911] ++ETH++ gw8 0xf0026004 0x22 # ETH_MAC_CORE_RECEIVE_CONTROL [0.896855] ++ETH++ gw8 0xf0026008 0x04 # ETH_MAC_CORE_RANDOM_SEED [0.900815] ++ETH++ gw8 0xf0026014 0x0c # ETH_MAC_CORE_TRANSMIT_SINGLE_DEFERRAL [0.904758] ++ETH++ gw8 0xf0026051 0x00 # ETH_MAC_CORE_THRESHOLDS [0.908700] ++ETH++ gw8 0xf0026052 0xff # ETH_MAC_CORE_THRESHOLDS [0.912643] ++ETH++ gw8 0xf0026054 0x40 # ETH_MAC_CORE_BUFFER_SIZE_TX_AND_ETH_MAC_CORE_FIFO_CONTROL [0.916586] ++ETH++ gw32 0xf0026100 0x02fe # TRANSMIT_CHANNEL_CONTROL [0.921054] ++ETH++ gw32 0xf0026118 0x003cc4a4 # TRANSMIT_INTERRUPT_TIME [0.925521] ++ETH++ gw32 0xf0026200 0x02fe # RECEIVE_CHANNEL_CONTROL [0.929990] ++ETH++ gw32 0xf0026218 0x4dc8 # RECEIVE_INTERRUPT_TIME [0.934456] ++ETH++ gw8 0xf0026060 0x00 # ETH_MAC_CORE_PAUSE_QUANTA [0.938398] ++ETH++ gw8 0xf0026061 0xc3 # ETH_MAC_CORE_PAUSE_QUANTA [0.942338] ++ETH++ gw8 0xf002602e 0x00 # ETH_MAC_CORE_MULTICAST_ADDRESS1 [0.946283] ++ETH++ gw8 0xf0026400 0x01 # ETH_GPIO_PAD_MODE [0.950226] ++ETH++ gw32 0xf0026200 0x028e # RECEIVE_CHANNEL_CONTROL [0.954695] ++ETH++ gw8 0xf002606a 0x00 # ETH_MAC_CORE_SOURCE_ADDRESS0 [0.958638] ++ETH++ gw8 0xf002606b 0x16 # ETH_MAC_CORE_SOURCE_ADDRESS0 [0.962581] ++ETH++ gw8 0xf002606c 0xe8 # ETH_MAC_CORE_SOURCE_ADDRESS1 [0.966523] ++ETH++ gw8 0xf002606d 0x4d # ETH_MAC_CORE_SOURCE_ADDRESS1 [0.970465] ++ETH++ gw8 0xf002606e 0x7f # ETH_MAC_CORE_SOURCE_ADDRESS1 [0.974408] ++ETH++ gw8 0xf002606f 0xc4 # ETH_MAC_CORE_SOURCE_ADDRESS1 [0.978349] ++ETH++ gw8 0xf002603c 0x00 # ETH_MAC_CORE_UNICAST_ADDRESS0 [0.982291] ++ETH++ gw8 0xf002603d 0x16 # ETH_MAC_CORE_UNICAST_ADDRESS0 [0.986234] ++ETH++ gw8 0xf002603e 0xe8 # ETH_MAC_CORE_UNICAST_ADDRESS0 [0.990176] ++ETH++ gw8 0xf002603f 0x4d # ETH_MAC_CORE_UNICAST_ADDRESS0 [0.994118] ++ETH++ gw8 0xf0026040 0x7f # ETH_MAC_CORE_UNICAST_ADDRESS1 [0.998060] ++ETH++ gw8 0xf0026041 0xc4 # ETH_MAC_CORE_UNICAST_ADDRESS1 [1.002435] nb8800 26000.ethernet eth0: MAC address 00:16:e8:4d:7f:c4 # ip link set eth0 up [ 33.202058] ENTER nb8800_open [ 33.205134] ++ETH++ gw32 0xf0026204 0x000f # RECEIVE_CHANNEL_STATUS [ 33.209610] ++ETH++ gw32 0xf0026104 0x000f # TRANSMIT_CHANNEL_STATUS [ 33.214626] ++ETH++ gw32 0xf002620c 0x9de36000 # RECEIVE_DESCRIPTOR_ADDRESS [ 33.219138] ++ETH++ gw8 0xf0026004 0x23 # ETH_MAC_CORE_RECEIVE_CONTROL [ 33.223097] ++ETH++ gw8 0xf0026000 0x1d # ETH_MAC_CORE_TRANSMIT_CONTROL [ 33.227054] nb8800_mdio_write: reg=0 val=8000# by genphy_soft_reset() [ 33.339579] nb8800_mdio_write: reg=0 val=# by at
Re: [PATCH v3] net: ethernet: support "fixed-link" DT node on nb8800 driver
On 08/02/2016 14:37, Måns Rullgård wrote: > Sebastian Frias wrote: > >> By the way, I know some people like the command line, email, etc. but >> there ought to be other tools better suited for patch review... > > Some kernel subsystems use http://patchwork.ozlabs.org/ to track status > of various patches. There's also a kernel bugzilla, but it may be for actual bugs. https://bugzilla.kernel.org/
Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB
[ CCing a few devs who might be interested ]
On 16/03/2016 18:25, Sebastian Frias wrote:
> Commit 687908c2b649 ("net: phy: at803x: simplify using
> devm_gpiod_get_optional and its 4th argument") introduced a dependency
> on GPIOLIB that was not there before.
>
> This commit removes such dependency by checking the return code and
> comparing it against ENOSYS which is returned when GPIOLIB is not
> selected.
>
> Fixes: 687908c2b649 ("net: phy: at803x: simplify using
> devm_gpiod_get_optional and its 4th argument")
>
> Signed-off-by: Sebastian Frias
> ---
> drivers/net/phy/at803x.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
> index 2174ec9..88b7ff3 100644
> --- a/drivers/net/phy/at803x.c
> +++ b/drivers/net/phy/at803x.c
> @@ -252,7 +252,9 @@ static int at803x_probe(struct phy_device *phydev)
> return -ENOMEM;
>
> gpiod_reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> - if (IS_ERR(gpiod_reset))
> + if (PTR_ERR(gpiod_reset) == -ENOSYS)
> + gpiod_reset = NULL;
> + else if (IS_ERR(gpiod_reset))
> return PTR_ERR(gpiod_reset);
>
> priv->gpiod_reset = gpiod_reset;
>
Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB
On 18/03/2016 21:11, Uwe Kleine-König wrote: > Hello, > > On Fri, Mar 18, 2016 at 08:31:20PM +0100, Mason wrote: > >> On 18/03/2016 20:12, Uwe Kleine-König wrote: >> >>> On Fri, Mar 18, 2016 at 04:56:21PM +0100, Sebastian Frias wrote: >>> >>>> What would you think of making at803x_link_change_notify() print a >>>> message every time it should do a reset but does not has a way to do it? >>> >>> Then this question is obsolete because the device doesn't probe. >> >> I don't understand this statement. >> >> What does it mean for a question to be obsolete? > > If the driver doesn't probe because it cannot control the reset line, > you don't need to think about how it should behave in > at803x_link_change_notify without control of the reset line, because > this code isn't reached then. If I understand correctly, it is possible to soft-reset the PHY by writing to a specific register. The GPIO pin is useful only to force a hardware-reset when the PHY is wedged by some random event. (Or am I completely off the mark?) Regards.
Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB
On 18/03/2016 20:12, Uwe Kleine-König wrote: > On Fri, Mar 18, 2016 at 04:56:21PM +0100, Sebastian Frias wrote: > >> What would you think of making at803x_link_change_notify() print a >> message every time it should do a reset but does not has a way to do it? > > Then this question is obsolete because the device doesn't probe. I don't understand this statement. What does it mean for a question to be obsolete? Regards.
Re: [PATCH] net: phy: at803x: don't depend on GPIOLIB
On 22/03/2016 20:42, Uwe Kleine-König wrote:
> Preconditions:
> - Some of the devices a given driver handles have a reset line and
>others don't.
> - A non-empty subset (maybe all) of the devices that have a reset line
>require that this reset line is used.
>
> Then the way to handle this in the driver should be done as follows:
>
> unless reset_handling_not_necessary():
> gpio = gpiod_get_optional("reset")
> if IS_ERR(gpio):
> return PTR_ERR(gpio)
>
> Checking for -ENOSYS or GPIOLIB=n is not allowed because the device
> you're currently handling might need the GPIO, so you must not continue
> without the ability to control the line.
>
> So the options you have (as you have a phy that doesn't need the reset
> handling):
>
> - enable GPIOLIB (either in your .config or introduce a Kconfig
>dependency)
> - improve reset_handling_not_necessary() to return true for your case
>
> There is nothing else.
Here are some numbers for GPIOLIB, on an ARM build:
textdata bss dec hex filename
1830 0 01830 726 devres.o
627 0 0 627 273 gpiolib-legacy.o
11018 40 4 110622b36 gpiolib.o
1598 0 01598 63e gpiolib-of.o
15073 40 4 151173b0d built-in.o
So ~15 kilobytes.
By the way, since the "reset-by-GPIO" solution is used only for
the Atheros 8030, would it be possible to make the
devm_gpiod_get_optional conditional on ATH8030_PHY_ID?
I'm thinking of something along these lines, for illustration:
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 2d020a3ec0b5..576e7873e049 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -198,12 +198,16 @@ static int at803x_probe(struct phy_device *phydev)
if (!priv)
return -ENOMEM;
+ if (phydev->drv->phy_id != ATH8030_PHY_ID)
+ goto no_gpio;
+
gpiod_reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
if (IS_ERR(gpiod_reset))
return PTR_ERR(gpiod_reset);
priv->gpiod_reset = gpiod_reset;
+no_gpio:
phydev->priv = priv;
return 0;
Regards.
WARNING: CPU: 0 PID: 0 at net/ipv4/af_inet.c:155 inet_sock_destruct+0x1c4/0x1dc
Hello, I was testing suspend/resume sequences where the suspend operation fails and returns without having suspended the platform. # echo mem > /sys/power/state [ 90.322264] PM: Syncing filesystems ... done. [ 90.328758] Freezing user space processes ... (elapsed 0.001 seconds) done. [ 90.337092] Double checking all user space processes after OOM killer disable... (elapsed 0.000 seconds) [ 90.346765] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done. [ 90.355357] Suspending console(s) (use no_console_suspend to debug) [ 90.364590] PM: suspend of devices complete after 2.068 msecs [ 90.365554] PM: late suspend of devices complete after 0.954 msecs [ 90.366223] PM: noirq suspend of devices complete after 0.662 msecs [ 90.366227] Disabling non-boot CPUs ... [ 90.379004] CPU1: shutdown [ 90.412661] Enabling non-boot CPUs ... [ 90.450385] CPU1 is up [ 90.450979] PM: noirq resume of devices complete after 0.584 msecs [ 90.451672] PM: early resume of devices complete after 0.667 msecs [ 90.453149] nb8800 26000.ethernet eth0: Link is Down [ 90.453264] PM: resume of devices complete after 1.583 msecs [ 90.508180] Restarting tasks ... done. -sh: echo: write error: Input/output error [ 93.860411] nb8800 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx (The error message is expected, as my suspend routine returns -EIO on failure.) I left the system to idle at the prompt; then 5 minutes later, the system printed the following trace. [ 400.718491] [ cut here ] [ 400.723175] WARNING: CPU: 0 PID: 0 at net/ipv4/af_inet.c:155 inet_sock_destruct+0x1c4/0x1dc [ 400.731582] Modules linked in: [ 400.734689] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.7.0-rc6-00010-gd07031bdc433 #1 [ 400.742646] Hardware name: Sigma Tango DT [ 400.746671] Backtrace: [ 400.749141] [] (dump_backtrace) from [] (show_stack+0x18/0x1c) [ 400.756747] r7:6113 r6:c080ea84 r5: r4:c080ea84 [ 400.762454] [] (show_stack) from [] (dump_stack+0x80/0x94) [ 400.769722] [] (dump_stack) from [] (__warn+0xec/0x104) [ 400.776717] r7:0009 r6:c05e3fbc r5: r4: [ 400.782417] [] (__warn) from [] (warn_slowpath_null+0x28/0x30) [ 400.790022] r9:dfbdd4e0 r8:000a r7:c0801de8 r6:df6f9514 r5:df5df144 r4:df5df040 [ 400.797825] [] (warn_slowpath_null) from [] (inet_sock_destruct+0x1c4/0x1dc) [ 400.806661] [] (inet_sock_destruct) from [] (__sk_destruct+0x28/0xe0) [ 400.814878] r7:c0801de8 r6:df6f9514 r5:df5df040 r4:df5df1ec [ 400.820584] [] (__sk_destruct) from [] (rcu_process_callbacks+0x488/0x59c) [ 400.829237] r5: r4: [ 400.832836] [] (rcu_process_callbacks) from [] (__do_softirq+0x138/0x264) [ 400.841402] r10:c08020a0 r9:4001 r8:0101 r7:c080 r6:c08020a4 r5:0009 [ 400.849285] r4: [ 400.851829] [] (__do_softirq) from [] (irq_exit+0xc8/0x104) [ 400.859172] r10:c0801f10 r9:df402400 r8:0001 r7: r6:0013 r5: [ 400.867053] r4:c0735428 [ 400.869601] [] (irq_exit) from [] (__handle_domain_irq+0x88/0xf4) [ 400.877473] [] (__handle_domain_irq) from [] (gic_handle_irq+0x50/0x94) [ 400.885865] r10:dfffcdc0 r9:e0803100 r8:e0802100 r7:c0801f10 r6:e080210c r5:c080277c [ 400.893747] r4:c080eca0 r3:c0801f10 [ 400.897342] [] (gic_handle_irq) from [] (__irq_svc+0x54/0x90) [ 400.904861] Exception stack(0xc0801f10 to 0xc0801f58) [ 400.909936] 1f00: 826a c0117c80 [ 400.918156] 1f20: c080 c08024f8 c0802494 c081e2d6 c05b954c c07268c0 dfffcdc0 c0801f6c [ 400.926376] 1f40: c0801f70 c0801f60 c01086b0 c01086b4 6013 [ 400.933020] r9:c07268c0 r8:c05b954c r7:c0801f44 r6: r5:6013 r4:c01086b4 [ 400.940826] [] (arch_cpu_idle) from [] (default_idle_call+0x28/0x34) [ 400.948960] [] (default_idle_call) from [] (cpu_startup_entry+0x128/0x17c) [ 400.957620] [] (cpu_startup_entry) from [] (rest_init+0x8c/0x90) [ 400.965400] r7: r4:0002 [ 400.969005] [] (rest_init) from [] (start_kernel+0x310/0x31c) [ 400.976522] r5:c081e4c0 r4:0001 [ 400.980121] [] (start_kernel) from [<8000807c>] (0x8000807c) [ 400.986716] ---[ end trace f8deb50d1b3d3c7a ]--- Did I implement something incorrectly? Regards.
Re: WARNING: CPU: 0 PID: 0 at net/ipv4/af_inet.c:155 inet_sock_destruct+0x1c4/0x1dc
On 05/07/2016 15:33, Mason wrote: > I was testing suspend/resume sequences where the suspend operation > fails and returns without having suspended the platform. > > # echo mem > /sys/power/state > [ 90.322264] PM: Syncing filesystems ... done. > [ 90.328758] Freezing user space processes ... (elapsed 0.001 seconds) done. > [ 90.337092] Double checking all user space processes after OOM killer > disable... (elapsed 0.000 seconds) > [ 90.346765] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) > done. > [ 90.355357] Suspending console(s) (use no_console_suspend to debug) > [ 90.364590] PM: suspend of devices complete after 2.068 msecs > [ 90.365554] PM: late suspend of devices complete after 0.954 msecs > [ 90.366223] PM: noirq suspend of devices complete after 0.662 msecs > [ 90.366227] Disabling non-boot CPUs ... > [ 90.379004] CPU1: shutdown > [ 90.412661] Enabling non-boot CPUs ... > [ 90.450385] CPU1 is up > [ 90.450979] PM: noirq resume of devices complete after 0.584 msecs > [ 90.451672] PM: early resume of devices complete after 0.667 msecs > [ 90.453149] nb8800 26000.ethernet eth0: Link is Down > [ 90.453264] PM: resume of devices complete after 1.583 msecs > [ 90.508180] Restarting tasks ... done. > -sh: echo: write error: Input/output error > [ 93.860411] nb8800 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow > control rx/tx > > (The error message is expected, as my suspend routine returns -EIO > on failure.) > > I left the system to idle at the prompt; then 5 minutes later, > the system printed the following trace. > > [ 400.718491] [ cut here ] > [ 400.723175] WARNING: CPU: 0 PID: 0 at net/ipv4/af_inet.c:155 > inet_sock_destruct+0x1c4/0x1dc > [ 400.731582] Modules linked in: > [ 400.734689] CPU: 0 PID: 0 Comm: swapper/0 Not tainted > 4.7.0-rc6-00010-gd07031bdc433 #1 > [ 400.742646] Hardware name: Sigma Tango DT > [ 400.746671] Backtrace: > [ 400.749141] [] (dump_backtrace) from [] > (show_stack+0x18/0x1c) > [ 400.756747] r7:6113 r6:c080ea84 r5: r4:c080ea84 > [ 400.762454] [] (show_stack) from [] > (dump_stack+0x80/0x94) > [ 400.769722] [] (dump_stack) from [] (__warn+0xec/0x104) > [ 400.776717] r7:0009 r6:c05e3fbc r5: r4: > [ 400.782417] [] (__warn) from [] > (warn_slowpath_null+0x28/0x30) > [ 400.790022] r9:dfbdd4e0 r8:000a r7:c0801de8 r6:df6f9514 r5:df5df144 > r4:df5df040 > [ 400.797825] [] (warn_slowpath_null) from [] > (inet_sock_destruct+0x1c4/0x1dc) > [ 400.806661] [] (inet_sock_destruct) from [] > (__sk_destruct+0x28/0xe0) > [ 400.814878] r7:c0801de8 r6:df6f9514 r5:df5df040 r4:df5df1ec > [ 400.820584] [] (__sk_destruct) from [] > (rcu_process_callbacks+0x488/0x59c) > [ 400.829237] r5: r4: > [ 400.832836] [] (rcu_process_callbacks) from [] > (__do_softirq+0x138/0x264) > [ 400.841402] r10:c08020a0 r9:4001 r8:0101 r7:c080 r6:c08020a4 > r5:0009 > [ 400.849285] r4: > [ 400.851829] [] (__do_softirq) from [] > (irq_exit+0xc8/0x104) > [ 400.859172] r10:c0801f10 r9:df402400 r8:0001 r7: r6:0013 > r5: > [ 400.867053] r4:c0735428 > [ 400.869601] [] (irq_exit) from [] > (__handle_domain_irq+0x88/0xf4) > [ 400.877473] [] (__handle_domain_irq) from [] > (gic_handle_irq+0x50/0x94) > [ 400.885865] r10:dfffcdc0 r9:e0803100 r8:e0802100 r7:c0801f10 r6:e080210c > r5:c080277c > [ 400.893747] r4:c080eca0 r3:c0801f10 > [ 400.897342] [] (gic_handle_irq) from [] > (__irq_svc+0x54/0x90) > [ 400.904861] Exception stack(0xc0801f10 to 0xc0801f58) > [ 400.909936] 1f00: > 826a c0117c80 > [ 400.918156] 1f20: c080 c08024f8 c0802494 c081e2d6 c05b954c c07268c0 > dfffcdc0 c0801f6c > [ 400.926376] 1f40: c0801f70 c0801f60 c01086b0 c01086b4 6013 > [ 400.933020] r9:c07268c0 r8:c05b954c r7:c0801f44 r6: r5:6013 > r4:c01086b4 > [ 400.940826] [] (arch_cpu_idle) from [] > (default_idle_call+0x28/0x34) > [ 400.948960] [] (default_idle_call) from [] > (cpu_startup_entry+0x128/0x17c) > [ 400.957620] [] (cpu_startup_entry) from [] > (rest_init+0x8c/0x90) > [ 400.965400] r7: r4:0002 > [ 400.969005] [] (rest_init) from [] > (start_kernel+0x310/0x31c) > [ 400.976522] r5:c081e4c0 r4:0001 > [ 400.980121] [] (start_kernel) from [<8000807c>] (0x8000807c) > [ 400.986716] ---[ end trace f8deb50d1b3d3c7a ]--- NB: The warning shows up 310 seconds after the suspend attempt. I rebooted, tried the same operation, and hit the same warning still 310 seconds later: # echo mem
Re: WARNING: CPU: 0 PID: 0 at net/ipv4/af_inet.c:155 inet_sock_destruct+0x1c4/0x1dc
On 05/07/2016 17:28, Florian Fainelli wrote: > nb8800.c does not currently show suspend/resume hooks implemented, are > you positive that when you suspend, you properly tear down all HW, stop > transmit queues, etc. and do the opposite upon resumption? I am currently testing the error path for my suspend routine. Firmware is, in fact, denying the suspend request, and immediately returns control to Linux, without having powered anything down. I expected not having to save any context in that situation. Am I mistaken? You mention "stop transmit queues". Can you say more about this? > Is your system clocksource also correctly saved/restored, or if you go > through a firmware in-between could it be changing the counter values > and make Linux think that more time as elapsed than it really happened? Thanks for pointing this out, I was not aware I was supposed to save and restore the tick counter on suspend/resume. (This is not an issue in this specific situation, as the platform is NOT suspended.) However, your remark has brought some more confusion to my mind. Linux is expecting time to stand still when it suspends? What if the tick counter is in an always-on power domain, and other processors depend on the counter? I can't just overwrite the reg when Linux resumes... Regards.
Re: WARNING: CPU: 0 PID: 0 at net/ipv4/af_inet.c:155 inet_sock_destruct+0x1c4/0x1dc
On 05/07/2016 18:20, Florian Fainelli wrote:
> On 07/05/2016 08:56 AM, Mason wrote:
>> On 05/07/2016 17:28, Florian Fainelli wrote:
>>
>>> nb8800.c does not currently show suspend/resume hooks implemented, are
>>> you positive that when you suspend, you properly tear down all HW, stop
>>> transmit queues, etc. and do the opposite upon resumption?
>>
>> I am currently testing the error path for my suspend routine.
>> Firmware is, in fact, denying the suspend request, and immediately
>> returns control to Linux, without having powered anything down.
>>
>> I expected not having to save any context in that situation.
>> Am I mistaken?
>
> It depends what power state you are going to and resuming from, and how
> much of this is platform dependent, on the platforms I work with S2
> preserves register states for our On/Off domain, while S3 only keeps an
> always-on power island and shuts off the On/Off domain, you therefore
> need to have your drivers in the On/Off domain suspend any activity and
> preserve important register states, or re-initialize them from scratch
> whichever is the most convenient.
Thanks for bringing these details to my attention, they will
definitely prove useful when I test an actual suspend/resume
sequence. However, I must stress that the platform did NOT
power down in my test case, because the firmware currently
denies all suspend requests.
Therefore, loss of context cannot possibly explain the
warning I am seeing.
>> You mention "stop transmit queues". Can you say more about this?
>
> See drivers/net/ethernet/broadcom/genet/bcmgenet.c which is a driver
> that takes care of that for instance, look for bcmgenet_{suspend,resume}
Thanks. I will look into it.
If I understand correctly, something is missing in the
network interface code? (My system is using an NFS root
filesystem, so network is an important subsystem.)
>>> Is your system clocksource also correctly saved/restored, or if you go
>>> through a firmware in-between could it be changing the counter values
>>> and make Linux think that more time as elapsed than it really happened?
>>
>> Thanks for pointing this out, I was not aware I was supposed to save
>> and restore the tick counter on suspend/resume. (This is not an issue
>> in this specific situation, as the platform is NOT suspended.)
>
> You don't have to save and restore the clocksource counter, although if
> you want proper time accounting to be done across suspend states, you
> would want to use a clocksource which is persistent across these suspend
> states.
The clocksource is a 27 MHz 32-bit tick counter. In other words,
the counter wraps around every 159 seconds. If Linux suspends
for several hours, how can it determine how much time went by?
Regards.
Re: WARNING: CPU: 0 PID: 0 at net/ipv4/af_inet.c:155 inet_sock_destruct+0x1c4/0x1dc
On 05/07/2016 23:22, Florian Fainelli wrote:
> On 07/05/2016 01:26 PM, Mason wrote:
>> On 05/07/2016 18:20, Florian Fainelli wrote:
>>> On 07/05/2016 08:56 AM, Mason wrote:
>>>> On 05/07/2016 17:28, Florian Fainelli wrote:
>>>>
>>>>> nb8800.c does not currently show suspend/resume hooks implemented, are
>>>>> you positive that when you suspend, you properly tear down all HW, stop
>>>>> transmit queues, etc. and do the opposite upon resumption?
>>>>
>>>> I am currently testing the error path for my suspend routine.
>>>> Firmware is, in fact, denying the suspend request, and immediately
>>>> returns control to Linux, without having powered anything down.
>>>>
>>>> I expected not having to save any context in that situation.
>>>> Am I mistaken?
>>>
>>> It depends what power state you are going to and resuming from, and how
>>> much of this is platform dependent, on the platforms I work with S2
>>> preserves register states for our On/Off domain, while S3 only keeps an
>>> always-on power island and shuts off the On/Off domain, you therefore
>>> need to have your drivers in the On/Off domain suspend any activity and
>>> preserve important register states, or re-initialize them from scratch
>>> whichever is the most convenient.
>>
>> Thanks for bringing these details to my attention, they will
>> definitely prove useful when I test an actual suspend/resume
>> sequence. However, I must stress that the platform did NOT
>> power down in my test case, because the firmware currently
>> denies all suspend requests.
>>
>> Therefore, loss of context cannot possibly explain the
>> warning I am seeing.
>
> No, but if you go all the way down to trying to suspend and the last
> step is the firmware failing, anything you have suspended needs to be
> unwinded, for your ethernet driver that means that you went through a
> successful suspend then resume cycle even if it failed down later when
> the platform attempted to suspend.
So it is the driver's responsibility to "shut down" on resume?
(I had the vague impression that the suspend framework would
"disable" the device through the appropriate callback.)
>>> See drivers/net/ethernet/broadcom/genet/bcmgenet.c which is a driver
>>> that takes care of that for instance, look for bcmgenet_{suspend,resume}
>>
>> Thanks. I will look into it.
>>
>> If I understand correctly, something is missing in the
>> network interface code? (My system is using an NFS root
>> filesystem, so network is an important subsystem.)
>
> The typical things are detaching the network device and stopping
> transmit queues, but without knowing what changes you have done to
> nb8800.c, hard to tell what else is needed.
I'm using the driver unaltered. So I guess I need to figure out
the exact steps required for suspending a network device.
(I'll look at bcmgenet.c tomorrow.)
>>>>> Is your system clocksource also correctly saved/restored, or if you go
>>>>> through a firmware in-between could it be changing the counter values
>>>>> and make Linux think that more time as elapsed than it really happened?
>>>>
>>>> Thanks for pointing this out, I was not aware I was supposed to save
>>>> and restore the tick counter on suspend/resume. (This is not an issue
>>>> in this specific situation, as the platform is NOT suspended.)
>>>
>>> You don't have to save and restore the clocksource counter, although if
>>> you want proper time accounting to be done across suspend states, you
>>> would want to use a clocksource which is persistent across these suspend
>>> states.
>>
>> The clocksource is a 27 MHz 32-bit tick counter. In other words,
>> the counter wraps around every 159 seconds. If Linux suspends
>> for several hours, how can it determine how much time went by?
>
> Well, that's unfortunate, then you are pretty much either doomed to
> accepting to lose time in between and rely on e.g: NTP to resync your
> time upon resumption, or, if you had smarter hardware you could have a
> prescaler or something that makes this counter wrap far ahead (like
> years or days after).
Maybe the hardware devs thought of that problem, because they
"widened" the counter to 64 bits on newer platforms.
Regards.
[PATCH v6] net: ethernet: nb8800: support fixed-link DT node
From: Sebastian Frias
Under some circumstances, e.g. when connecting to a switch, the ethernet
port will not be connected to a PHY. In that case a "fixed-link" DT node
can be used to replace it.
https://stackoverflow.com/questions/31046172/device-tree-for-phy-less-connection-to-a-dsa-switch
This patch adds support for the "fixed-link" node to the nb8800 driver.
Signed-off-by: Sebastian Frias
Acked-by: Mans Rullgard
Cc: Mason
---
There were spurious spaces in the previous patch.
---
drivers/net/ethernet/aurora/nb8800.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/aurora/nb8800.c
b/drivers/net/ethernet/aurora/nb8800.c
index f71ab2647a3b..08a23e6b60e9 100644
--- a/drivers/net/ethernet/aurora/nb8800.c
+++ b/drivers/net/ethernet/aurora/nb8800.c
@@ -1460,7 +1460,19 @@ static int nb8800_probe(struct platform_device *pdev)
goto err_disable_clk;
}
- priv->phy_node = of_parse_phandle(pdev->dev.of_node, "phy-handle", 0);
+ if (of_phy_is_fixed_link(pdev->dev.of_node)) {
+ ret = of_phy_register_fixed_link(pdev->dev.of_node);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "bad fixed-link spec\n");
+ goto err_free_bus;
+ }
+ priv->phy_node = of_node_get(pdev->dev.of_node);
+ }
+
+ if (!priv->phy_node)
+ priv->phy_node = of_parse_phandle(pdev->dev.of_node,
+ "phy-handle", 0);
+
if (!priv->phy_node) {
dev_err(&pdev->dev, "no PHY specified\n");
ret = -ENODEV;
--
2.7.0
Re: [PATCH v5] net: ethernet: nb8800: support fixed-link DT node
On 16/02/2016 21:04, David Miller wrote: > This doesn't apply, please respin against my tree. I fixed several formatting issues with Sebastian's patch, and submitted v6. Regards.
Re: Small improvements for the at803x PHY driver
[ CCing people who might be interested in this patch series ] On 26/12/2015 01:26, Martin Blumenstingl wrote: > while trying to debug a problem on a board with an AR8030 PHY (which turned > out to be an incorrectly configured MDC clock) I made a few changes to the > at803x driver. > Due to lack of other hardware I could only test these changes with an > AR8030 chip. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] net: phy: at803x: Allow specifying the RGMII RX clock delay via phy mode
On 27/12/2015 04:28, Florian Fainelli wrote: > Le 25/12/2015 16:27, Martin Blumenstingl wrote: > >> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c >> index f566b6e..0b262a2 100644 >> --- a/drivers/net/phy/at803x.c >> +++ b/drivers/net/phy/at803x.c >> @@ -36,8 +36,10 @@ >> #define AT803X_INSR 0x0013 >> #define AT803X_DEBUG_ADDR 0x1D >> #define AT803X_DEBUG_DATA 0x1E >> -#define AT803X_DEBUG_SYSTEM_MODE_CTRL 0x05 >> -#define AT803X_DEBUG_RGMII_TX_CLK_DLY BIT(8) >> +#define AT803X_DEBUG_REG_0 0x00 > > Seems like the previous register name might have been clearer that the > new name you suggest here, did that come from a different GPL tarball or > documentation? http://www.redeszone.net/app/uploads/2014/04/AR8035.pdf According to the 8035 data sheet, the debug register at offset 0 is just "Debug register 0". In fact, the only non-reserved bit is "rgmii rx clock delay enable/disable" So the SYSTEM_MODE_CTRL name is misleading. Unless the register has different semantics on the other PHYs? Regards. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Legacy features in PCI Express devices
Hello, There are two revisions of our PCI Express controller. Rev 1 did not support the following features: 1) legacy PCI interrupt delivery (INTx signals) 2) I/O address space Internally, someone stated that such missing support would prevent some PCIe cards from working with our controller. Are there really modern PCIe cards that require 1) and/or 2) to function? Can someone provide examples of such cards, so that I may test them on both revisions? I was told to check ath9k-based cards. Any other examples? Looking around, I came across this thread: http://lists.infradead.org/pipermail/linux-arm-kernel/2016-March/418254.html "i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity" IIUC, although some PCIe boards do support MSI, the driver might not put in the work to use that infrastructure, and instead reverts to legacy interrupts. (So it is a SW issue, in a sense.) Regards.
Re: Legacy features in PCI Express devices
On 13/03/2017 18:12, Robin Murphy wrote: > On 13/03/17 16:10, Mason wrote: > >> There are two revisions of our PCI Express controller. >> >> Rev 1 did not support the following features: >> >> 1) legacy PCI interrupt delivery (INTx signals) >> 2) I/O address space >> >> Internally, someone stated that such missing support would prevent >> some PCIe cards from working with our controller. >> >> Are there really modern PCIe cards that require 1) and/or 2) >> to function? >> >> Can someone provide examples of such cards, so that I may test them >> on both revisions? >> >> I was told to check ath9k-based cards. Any other examples? >> >> Looking around, I came across this thread: >> http://lists.infradead.org/pipermail/linux-arm-kernel/2016-March/418254.html >> "i.MX6 PCIe: Fix imx6_pcie_deassert_core_reset() polarity" >> >> IIUC, although some PCIe boards do support MSI, the driver might not >> put in the work to use that infrastructure, and instead reverts to >> legacy interrupts. (So it is a SW issue, in a sense.) > > Secondary to that category is endpoints which nominally support MSI, but > in a way which is unreliable or otherwise broken. My experience shows > that the Silicon Image SiI 3132 (as integrated on ARM Juno boards, but > seemingly also relatively common on 'generic' 2-port SATA cards) falls > into that category - using the command-line parameter to force MSIs > instead of legacy interrupts leads to the the machine barely reaching > userspace before something goes horribly wrong: Do drivers typically support *both* MSI and INTx? Specifically, would the xhci driver support both? If I remove MSI support from my kernel, I might be able to test legacy interrupt support that way, right? Regards.
Setting link down hangs the kernel
Hello, I'm using kernel v4.9 on a dev board. I built a small kernel + rootfs with buildroot 2016.11.1 eth0 is driven by drivers/net/ethernet/aurora/nb8800.c After booting, I just run udhcpc (busybox version) Then I set the link down, and the kernel hangs. Here's the console output: [1.116707] Freeing unused kernel memory: 3072K (c060 - c090) Starting logging: OK Initializing random number generator... [1.217335] random: dd: uninitialized urandom read (512 bytes read) done. Starting network: OK Welcome to Buildroot buildroot login: root # udhcpc udhcpc: started, v1.25.1 udhcpc: sending discover udhcpc: sending discover [ 13.840512] nb8800 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx udhcpc: sending discover udhcpc: sending select for 172.27.64.51 udhcpc: lease of 172.27.64.51 obtained, lease time 604800 deleting routers adding dns 172.27.0.17 # ip link set dev eth0 down [ 65.520187] nb8800 26000.ethernet eth0: Link is Down ^C^C^C [ 85.790557] sysrq: SysRq : Show backtrace of all active CPUs [ 85.796260] NMI backtrace for cpu 0 [ 85.799773] CPU: 0 PID: 866 Comm: ip Not tainted 4.9.0 #143 [ 85.805376] Hardware name: Sigma Tango DT [ 85.809405] Backtrace: [ 85.811893] [] (dump_backtrace) from [] (show_stack+0x18/0x1c) [ 85.819510] r7: r6:600c0193 r5: r4:c0910af4 [ 85.825216] [] (show_stack) from [] (dump_stack+0x84/0xa0) [ 85.832491] [] (dump_stack) from [] (nmi_cpu_backtrace+0x98/0xb8) [ 85.840370] r7: r6:c010c9e4 r5: r4: [ 85.846074] [] (nmi_cpu_backtrace) from [] (nmi_trigger_cpumask_backtrace+0xc4/0x17c) [ 85.855696] r5: r4:c09032e4 [ 85.859304] [] (nmi_trigger_cpumask_backtrace) from [] (arch_trigger_cpumask_backtrace+0x14/0x1c) [ 85.869981] r9:0100 r8:0001 r7:006c r6:000a r5:c090ac48 r4:c0911458 [ 85.877783] [] (arch_trigger_cpumask_backtrace) from [] (sysrq_handle_showallcpus+0x18/0x20) [ 85.888027] [] (sysrq_handle_showallcpus) from [] (__handle_sysrq+0x90/0x12c) [ 85.896958] [] (__handle_sysrq) from [] (handle_sysrq+0x28/0x2c) [ 85.904751] r9:0100 r8: r7:006c r6:0061 r5:0061 r4:006c [ 85.912553] [] (handle_sysrq) from [] (serial8250_rx_chars+0x178/0x1d8) [ 85.920952] r5:0061 r4:c094e654 [ 85.924557] [] (serial8250_rx_chars) from [] (serial8250_handle_irq+0x8c/0xdc) [ 85.933575] r10:0014 r9:cfbea6cc r8:cfbea6c0 r7:600c0193 r6:0061 r5:00cc [ 85.941448] r4:c094e654 [ 85.944002] [] (serial8250_handle_irq) from [] (serial8250_default_handle_irq+0x30/0x44) [ 85.953888] r7: r6: r5: r4:c094e654 [ 85.959588] [] (serial8250_default_handle_irq) from [] (serial8250_interrupt+0x3c/0xbc) [ 85.969383] r5: r4:c094e768 [ 85.972993] [] (serial8250_interrupt) from [] (__handle_irq_event_percpu+0x40/0x10c) [ 85.982534] r10:c090c7b5 r9:cfa57c54 r8:cf866a00 r7:0001 r6:0014 r5: [ 85.990409] r4:cfbea680 r3:c02dce30 [ 85.994015] [] (__handle_irq_event_percpu) from [] (handle_irq_event_percpu+0x24/0x60) [ 86.003730] r10: r9:cfa56000 r8:0002 r7:0001 r6:cf866a10 r5:cf866a00 [ 86.011602] r4:cf866a00 [ 86.014159] [] (handle_irq_event_percpu) from [] (handle_irq_event+0x40/0x64) [ 86.023082] r5:cf866a60 r4:cf866a00 [ 86.026686] [] (handle_irq_event) from [] (handle_level_irq+0xdc/0x118) [ 86.035087] r7:0001 r6:cf866a10 r5:cf866a60 r4:cf866a00 [ 86.040787] [] (handle_level_irq) from [] (generic_handle_irq+0x20/0x30) [ 86.049275] r7:0001 r6: r5:cf802600 r4:0001 [ 86.054976] [] (generic_handle_irq) from [] (tangox_dispatch_irqs+0x4c/0x58) [ 86.063820] [] (tangox_dispatch_irqs) from [] (tangox_irq_handler+0x7c/0xa4) [ 86.072662] r9:cfa56000 r8:cf802400 r7: r6:c0903310 r5:cf802600 r4:cf804010 [ 86.080458] [] (tangox_irq_handler) from [] (generic_handle_irq+0x20/0x30) [ 86.089121] r7:0010 r6:c0866edc r5: r4: [ 86.094822] [] (generic_handle_irq) from [] (__handle_domain_irq+0x94/0xbc) [ 86.103578] [] (__handle_domain_irq) from [] (gic_handle_irq+0x50/0x7c) [ 86.111984] r9:cfa56000 r8: r7:f0701100 r6:cfa57d60 r5:c0903310 r4:f0700100 [ 86.119778] [] (gic_handle_irq) from [] (__irq_svc+0x6c/0xa8) [ 86.127303] Exception stack(0xcfa57d60 to 0xcfa57da8) [ 86.132390] 7d60: cfb2f800 f0026000 00240aff cfb2f800 cfbe1400 1042 1003 [ 86.140621] 7d80: cfa57dbc cfa57dc0 cfa57db0 c0312e8c c0310ad4 [ 86.148843] 7da0: 200c0013 [ 86.152354] r7:cfa57d94 r6: r5:200c0013 r4:c0310ad4 [ 86.158058] [] (nb8800_mac_tx) from [] (nb8800_stop+0x60/0x84) [ 86.165683] [] (nb8800_stop) from [] (__dev_close_many+0x9c/0xc0) [ 86.173558] r5:cfa57df0 r4:cfb2f800 [ 86.177160] [] (__dev_close_many) from [] (__dev_close+0x30/0x48) [ 86.185035] r5:0001 r4:cfb2f800 [ 86
Re: Setting link down hangs the kernel
On 10/01/2017 15:36, Mason wrote: > I'm using kernel v4.9 on a [new] dev board. > I built a small kernel + rootfs with buildroot 2016.11.1 > eth0 is driven by drivers/net/ethernet/aurora/nb8800.c > > After booting, I just run udhcpc (busybox version) > Then I set the link down, and the kernel hangs. So far, I've been unable to reproduce this issue on an older smp8758 dev board. # while true > do > ip link set dev eth0 down > sleep 3 > ip link set dev eth0 up > sleep 3 > let "++XX" > echo $XX > done The output is strange nonetheless: 1 2 3 4 5 [ 179.002757] nb8800 26000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx 6 7 [ 192.538648] nb8800 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx 8 9 10 11 12 [ 222.284936] nb8800 26000.ethernet eth0: Link is Up - 100Mbps/Full - flow control rx/tx 13 14 ^C # [ 235.800851] nb8800 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx A) I would expect a "Link is Up" message for every "link up" command. I get only 4 messages for 14 commands. Is that expected? B) Twice, the PHY was only able to negotiate 100 Mbps, although the peer supports 1 Gbps. What could cause this? Regards.
Re: Setting link down hangs the kernel
On 10/01/2017 16:28, Mason wrote: > On 10/01/2017 15:36, Mason wrote: > >> I'm using kernel v4.9 on a [new] dev board. >> I built a small kernel + rootfs with buildroot 2016.11.1 >> eth0 is driven by drivers/net/ethernet/aurora/nb8800.c >> >> After booting, I just run udhcpc (busybox version) >> Then I set the link down, and the kernel hangs. > > So far, I've been unable to reproduce this issue on an older > smp8758 dev board. And another symptom (which exists only on the new board) : After the first "link down", udhcpc no longer works. Also, the OS prints "link down" only when udhcpc requests "link up". I guess I have lots of "fun" things to investigate tomorrow... Any pointers to where I should look first? I think some kind of "deferred worker thread" is spawned when the PHY state machine changes state? Could there be some kind of race there? Or is it more likely something is broken in the HW? Welcome to Buildroot buildroot login: root # udhcpc && ip addr udhcpc: started, v1.25.1 udhcpc: sending discover udhcpc: sending discover [ 28.427174] nb8800 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx udhcpc: sending discover udhcpc: sending select for 172.27.64.46 udhcpc: lease of 172.27.64.46 obtained, lease time 604800 deleting routers adding dns 172.27.0.17 1: lo: mtu 65536 qdisc noqueue qlen 1 link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00 inet 127.0.0.1/8 scope host lo valid_lft forever preferred_lft forever 2: eth0: mtu 1500 qdisc pfifo_fast qlen 1000 link/ether 00:16:e8:78:2c:b3 brd ff:ff:ff:ff:ff:ff inet 172.27.64.46/18 brd 172.27.127.255 scope global eth0 valid_lft forever preferred_lft forever # ip link set dev eth0 down && sleep 10 && udhcpc -T 1 -A 3 udhcpc: started, v1.25.1 udhcpc: sending discover [ 55.040450] nb8800 26000.ethernet eth0: Link is Down udhcpc: sending discover udhcpc: sending discover [ 57.067154] nb8800 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx udhcpc: sending discover udhcpc: sending discover udhcpc: sending discover udhcpc: sending discover udhcpc: sending discover udhcpc: sending discover udhcpc: sending discover udhcpc: sending discover udhcpc: sending discover udhcpc: sending discover udhcpc: sending discover udhcpc: sending discover udhcpc: sending discover udhcpc: sending discover udhcpc: sending discover udhcpc: sending discover udhcpc: sending discover udhcpc: sending discover udhcpc: sending discover udhcpc: sending discover udhcpc: sending discover udhcpc: sending discover udhcpc: sending discover ^C Regards.
Setting link down or up in software
Hello, I'm wondering what are the semantics of calling ip link set dev eth0 down I was expecting that to somehow instruct the device's ethernet driver to shut everything down, have the PHY tell the peer that it's going away, maybe even put the PHY in some low-power mode, etc. But it doesn't seem to be doing any of that on my HW. So what exactly is it supposed to do? And on top of that, I am seeing random occurrences of nb8800 26000.ethernet eth0: Link is Down Sometimes it is printed immediately. Sometimes it is printed as soon as I run "ip link set dev eth0 up" (?!) Sometimes it is not printed at all. I find this erratic behavior very confusing. Is it the symptom of some deeper bug? Regards.
Re: Setting link down or up in software
On 12/01/2017 14:05, Mason wrote: > I'm wondering what are the semantics of calling > > ip link set dev eth0 down > > I was expecting that to somehow instruct the device's ethernet driver > to shut everything down, have the PHY tell the peer that it's going > away, maybe even put the PHY in some low-power mode, etc. > > But it doesn't seem to be doing any of that on my HW. > > So what exactly is it supposed to do? > > > And on top of that, I am seeing random occurrences of > > nb8800 26000.ethernet eth0: Link is Down > > Sometimes it is printed immediately. > Sometimes it is printed as soon as I run "ip link set dev eth0 up" (?!) > Sometimes it is not printed at all. > > I find this erratic behavior very confusing. > > Is it the symptom of some deeper bug? Here's an example of "Link is Down" printed when I set link up: At [ 62.750220] I run ip link set dev eth0 down Then leave the system idle for 10 minutes. At [ 646.263041] I run ip link set dev eth0 up At [ 647.364079] it prints "Link is Down" At [ 649.417434] it prints "Link is Up - 1Gbps/Full - flow control rx/tx" I think whether I set up the PHY to use interrupts or polling does have an influence on the weirdness I observe. AFAICT, changing the interface flags is done in dev_change_flags which calls __dev_change_flags and __dev_notify_flags Is one of these supposed to call the device driver through a callback at some point? How/when is the phy_state_machine notified of the change in interface flags? Regards.
Re: Setting link down or up in software
On 12/01/2017 16:28, Andrew Lunn wrote: > Mason wrote: > >> Here's an example of "Link is Down" printed when I set link up: >> >> At [ 62.750220] I run ip link set dev eth0 down >> Then leave the system idle for 10 minutes. >> At [ 646.263041] I run ip link set dev eth0 up >> At [ 647.364079] it prints "Link is Down" >> At [ 649.417434] it prints "Link is Up - 1Gbps/Full - flow control rx/tx" > > Purely a guess, but when you up the interface, it starts auto > negotiation. That often involves resetting the PHY. If the PHY has > already once completed autoneg, e.g. because of the boot loader, it > will be initially UP. The reset will put it DOWN, and then once > autoneg is complete, it will be Up again. > > Pure guess. Go read the code and see if i'm write. Thanks for giving me some food for thought, although the net framework is far from easy to navigate. (So I'm not sure "go read the code" will take me anywhere in the short term.) Whatever the reason for the symptoms I'm seeing, some kind of race condition must be involved, because it occurs randomly. Regards.
Re: Setting link down or up in software
On 13/01/2017 10:20, Zefir Kurtisi wrote: > On 01/12/2017 04:16 PM, Mason wrote: >> On 12/01/2017 14:05, Mason wrote: >> >>> I'm wondering what are the semantics of calling >>> >>> ip link set dev eth0 down >>> >>> I was expecting that to somehow instruct the device's ethernet driver >>> to shut everything down, have the PHY tell the peer that it's going >>> away, maybe even put the PHY in some low-power mode, etc. >>> >>> But it doesn't seem to be doing any of that on my HW. >>> >>> So what exactly is it supposed to do? >>> >>> >>> And on top of that, I am seeing random occurrences of >>> >>> nb8800 26000.ethernet eth0: Link is Down >>> >>> Sometimes it is printed immediately. >>> Sometimes it is printed as soon as I run "ip link set dev eth0 up" (?!) >>> Sometimes it is not printed at all. >>> >>> I find this erratic behavior very confusing. >>> >>> Is it the symptom of some deeper bug? >> >> Here's an example of "Link is Down" printed when I set link up: >> >> At [ 62.750220] I run ip link set dev eth0 down >> Then leave the system idle for 10 minutes. >> At [ 646.263041] I run ip link set dev eth0 up >> At [ 647.364079] it prints "Link is Down" >> At [ 649.417434] it prints "Link is Up - 1Gbps/Full - flow control rx/tx" >> >> I think whether I set up the PHY to use interrupts or polling >> does have an influence on the weirdness I observe. >> >> AFAICT, changing the interface flags is done in dev_change_flags >> which calls __dev_change_flags and __dev_notify_flags >> >> Is one of these supposed to call the device driver through a >> callback at some point? >> >> How/when is the phy_state_machine notified of the change in >> interface flags? >> >> Regards. >> > Hm, reminds me of something at my side that I recently fixed with [1]. For me > it > was pulling the cable got randomly unnoticed at PHY layer - but might be > related. > > Do you by chance have some component that polls the link states over the > ethtool > interface very often (like once per second)? At my side it was a snmpd agent > that > pro-actively updated the interface states every second and with that 'stole' > the > link change information from the phy link state machine. What you need to > have to > run in such a failing situation is: > 1) an ETH driver that updates link status in ethtool GSET path (e.g. dsa does) > 2) some component that continuously polls states via ethtool GSET > > > Cheers, > Zefir > > > [1] https://patchwork.ozlabs.org/patch/711839/ Hello Zefir, Thanks for the insightful comment. This is a minimal buildroot system, with no frills, and not much running. There definitely is no SNMP daemon running, but I can't be 100% sure that busybox isn't polling the link state once in a while. (It's unlikely.) I'm surprised that there are still bugs lurking in the phy state machine, I expected this to be a "solved problem", but I suppose power management has broken many assumptions that were once safe... By the way, I did come across code paths where phy->state was read or written without taking the lock. Isn't that never supposed to happen? Regards.
Re: Setting link down or up in software
On 13/01/2017 17:28, Zefir Kurtisi wrote:
> As for your specific problem: since I fought myself with the PHY/ETH
> subsystems
> over the past months, I might remember something relevant to your issue.
> Could you
> give some more info on your setup (PHY driver, opmode (SGMII, RGMII, etc.),
> ETH).
Hello Zefir,
My boards are using these drivers:
http://lxr.free-electrons.com/source/drivers/net/ethernet/aurora/nb8800.c
http://lxr.free-electrons.com/source/drivers/net/phy/at803x.c
The relevant device tree nodes are:
eth0: ethernet@26000 {
compatible = "sigma,smp8734-ethernet";
reg = <0x26000 0x800>;
interrupts = <38 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clkgen SYS_CLK>;
};
ð0 {
phy-connection-type = "rgmii";
phy-handle = <ð0_phy>;
#address-cells = <1>;
#size-cells = <0>;
/* Atheros AR8035 */
eth0_phy: ethernet-phy@4 {
compatible = "ethernet-phy-id004d.d072",
"ethernet-phy-ieee802.3-c22";
interrupts = <37 IRQ_TYPE_EDGE_RISING>;
reg = <4>;
};
};
If I comment the PHY "interrupts" property, then the PHY framework
falls back to polling.
Am I forgetting important information?
Regards.
Initializing MAC address at run-time
Hello,
When my system boots up, eth0 is given a seemingly random MAC address.
[0.950734] nb8800 26000.ethernet eth0: MAC address ba:de:d6:38:b8:38
[0.957334] nb8800 26000.ethernet eth0: MAC address 6e:f1:48:de:d6:c4
The DT node for eth0 is:
eth0: ethernet@26000 {
compatible = "sigma,smp8734-ethernet";
reg = <0x26000 0x800>;
interrupts = <38 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clkgen SYS_CLK>;
};
Documentation/devicetree/bindings/net/ethernet.txt mentions
- local-mac-address: array of 6 bytes, specifies the MAC address that was
assigned to the network device;
And indeed, if I define this property, eth0 ends up with the MAC address
I specify in the device tree. But of course, I don't want all my boards
to share the same MAC address. Every interface has a unique MAC address.
In fact, the boot loader (not Uboot, a custom non-DT boot loader) stores
the MAC address somewhere in MMIO space, in some weird custom format.
So, at init, I can find the MAC address, and dynamically insert the
"local-mac-address" property in the eth0 node.
Is there another (better) way to do this?
I'll post my code below, for illustration purpose.
Mark suggested this can be done from user-space, but I can't do that,
because I'm using an NFS rootfs, so I need the network before I even
have a user-space. And the DHCP server is configured to serve different
root filesystems, based on the MAC address.
I need to do something similar with the NAND partitions. The boot loader
stores the partition offsets somewhere, and I need to pass this info
to the NAND framework, so I assumed that inserting the corresponding
properties at run-time was the correct way to do it.
Regards.
#include
#include
#include
#define XENV_LRRW_ADDR 0x61a00
#define XENV_LRRW_SIZE 628
static u8 xenv[XENV_LRRW_SIZE] __initdata;
static void __init *xenv_lookup(void *addr, const char *key, int keylen)
{
u32 len = le32_to_cpup(addr);
void *end = addr + len;
if (len > XENV_LRRW_SIZE)
return NULL;
for (addr += 36; addr < end; addr += len)
{
len = get_unaligned_be16(addr) & 0xfff;
if (strcmp(key, addr + 2) == 0)
return addr + 2 + keylen;
}
return NULL;
}
static struct property prop;
static u8 mac[6];
static const char mac_lo[] __initconst = "lrrw.maclo";
static const char mac_hi[] __initconst = "lrrw.machi";
static int __init tango_get_mac_address(void *xenv)
{
struct device_node *np = of_find_node_by_path("eth0");
u8 *lo = xenv_lookup(xenv, mac_lo, sizeof mac_lo);
u8 *hi = xenv_lookup(xenv, mac_hi, sizeof mac_hi);
if (np == NULL || lo == NULL || hi == NULL) return -ENODEV;
mac[0] = hi[1];
mac[1] = hi[0];
mac[2] = lo[3];
mac[3] = lo[2];
mac[4] = lo[1];
mac[5] = lo[0];
prop.name = "local-mac-address";
prop.length = sizeof mac;
prop.value = mac;
return of_update_property(np, &prop);
}
static int __init mac_fixup(void)
{
void __iomem *xenv_orig = ioremap(XENV_LRRW_ADDR, XENV_LRRW_SIZE);
memcpy_fromio(xenv, xenv_orig, XENV_LRRW_SIZE);
iounmap(xenv_orig);
return tango_get_mac_address(xenv);
}
device_initcall(mac_fixup);
Re: Initializing MAC address at run-time
Hello Uwe,
On 18/01/2017 19:54, Uwe Kleine-König wrote:
> On Wed, Jan 18, 2017 at 03:03:57PM +0100, Mason wrote:
>
>> When my system boots up, eth0 is given a seemingly random MAC address.
>>
>> [0.950734] nb8800 26000.ethernet eth0: MAC address ba:de:d6:38:b8:38
>> [0.957334] nb8800 26000.ethernet eth0: MAC address 6e:f1:48:de:d6:c4
>>
>>
>> The DT node for eth0 is:
>>
>> eth0: ethernet@26000 {
>> compatible = "sigma,smp8734-ethernet";
>> reg = <0x26000 0x800>;
>> interrupts = <38 IRQ_TYPE_LEVEL_HIGH>;
>> clocks = <&clkgen SYS_CLK>;
>> };
>>
>> Documentation/devicetree/bindings/net/ethernet.txt mentions
>> - local-mac-address: array of 6 bytes, specifies the MAC address that was
>> assigned to the network device;
>>
>> And indeed, if I define this property, eth0 ends up with the MAC address
>> I specify in the device tree. But of course, I don't want all my boards
>> to share the same MAC address. Every interface has a unique MAC address.
>>
>> In fact, the boot loader (not Uboot, a custom non-DT boot loader) stores
>> the MAC address somewhere in MMIO space, in some weird custom format.
>
> Where does your machine get the dtb from? You write it to the boot
> medium at a certain point of time I assume.
The DTB is appended to the kernel uImage.
(It's never written to the board's storage.)
> So AFAICT you have the following options (in no particular order):
>
> a) Describe in the dtb how to find out how the MAC address is stored
> (already pointed out Mark and Robin)
> b) Make your bootloader dt aware and let it provide the
> local-mac-address property.
Do you agree that such boot loader would execute code that is roughly
identical to the one posted for illustration purposes?
1. find the MAC address to use for eth0
2. find the eth0 node in the DT
3. insert the right prop in the eth0 node
In which case, it seems a waste to add the DT library to the boot
loader, when the operation can be done in Linux, which requires the
DT library anyway. (Additionally, adding DT support to some custom
legacy boot loader might be a complex task.)
> c) Adapt the dtb before it is written to the boot medium.
This is not applicable, as the DTB is not written to the board.
> d) Let the bootloader configure the device and teach the driver to pick
> up the mac from the device's address space.
I'm not sure what you call "the device" ?
The local RAM where the MAC is stored? the eth controller?
> e) Accept that the mac address is random during development, and make
> Userspace configure the MAC address, which is early enough for
> production use.
During development, some devs configure the DHCP server to provide
a specific uImage and/or rootfs to their board, based on the MAC
address. This scheme would fall apart with a random MAC.
> Not sure d) is considered ok today, but some drivers have this feature.
> I'd say b) is the best choice.
In my mind, doing it early in Linux is similar in spirit to doing it
at the boot loader stage, in that it's neatly separated from the rest
of the setup.
>> I need to do something similar with the NAND partitions. The boot loader
>> stores the partition offsets somewhere, and I need to pass this info
>> to the NAND framework, so I assumed that inserting the corresponding
>> properties at run-time was the correct way to do it.
>
> The list of options here is similar to the list above. d) doesn't work,
> but instead you can pass the partitioning on the kernel commandline.
Yes, I will test the command line approach. Thanks.
Regards.
Re: Setting link down or up in software
On 18/01/2017 11:29, Zefir Kurtisi wrote:
> On 01/13/2017 06:35 PM, Mason wrote:
>> On 13/01/2017 17:28, Zefir Kurtisi wrote:
>>
>>> As for your specific problem: since I fought myself with the PHY/ETH
>>> subsystems
>>> over the past months, I might remember something relevant to your issue.
>>> Could you
>>> give some more info on your setup (PHY driver, opmode (SGMII, RGMII, etc.),
>>> ETH).
>>
>> Hello Zefir,
>>
>> My boards are using these drivers:
>>
>> http://lxr.free-electrons.com/source/drivers/net/ethernet/aurora/nb8800.c
>> http://lxr.free-electrons.com/source/drivers/net/phy/at803x.c
>>
>> The relevant device tree nodes are:
>>
>> eth0: ethernet@26000 {
>> compatible = "sigma,smp8734-ethernet";
>> reg = <0x26000 0x800>;
>> interrupts = <38 IRQ_TYPE_LEVEL_HIGH>;
>> clocks = <&clkgen SYS_CLK>;
>> };
>>
>> ð0 {
>> phy-connection-type = "rgmii";
>> phy-handle = <ð0_phy>;
>> #address-cells = <1>;
>> #size-cells = <0>;
>>
>> /* Atheros AR8035 */
>> eth0_phy: ethernet-phy@4 {
>> compatible = "ethernet-phy-id004d.d072",
>> "ethernet-phy-ieee802.3-c22";
>> interrupts = <37 IRQ_TYPE_EDGE_RISING>;
>> reg = <4>;
>> };
>> };
>>
>> If I comment the PHY "interrupts" property, then the PHY framework
>> falls back to polling.
>>
>> Am I forgetting important information?
>
> In our system we attach the at8031 over SGMII to the gianfar (Freescale
> eTSEC) and
> to fibre optics transceivers, which operate in fixed speeds. Getting this
> setup to
> work reliably was challenging for various reasons, maybe worth to note
>
> 1) fixed SGMII speed not working: link is up on both ends, but no data is
> passed
> 2) known issue with SGMII link not completing autonegotiation correctly, see
> [1]
> 3) once autoneg is started or chip is reset, MII_CTRL1000 can not be written
> to
> until autoneg is completed => breaks phy state machine when the driver loads
> with
> unplugged cable and tries to set fixed speed
>
> Unless you are using fixed speed links in your setup, none of those should
> affect
> it. My experience with at8031 attached to RGMII is that it is genphy
> compliant,
> therefore I would a) disable interrupts, and b) prevent loading at803x.ko and
> try
> the genphy instead. Yours is an at8035, results may vary.
>
> [1] https://www.spinics.net/lists/netdev/msg400804.html
Hello Zefir,
Thanks for having a look at my issue.
I must confess that I'm not up to speed on the different *MIIs
(MII, RMII, GMII, RGMII, SGMII, etc)
One thing I find confusing... If the 8035 could work with the generic
PHY driver, what's the point of the at803x.ko driver?
Regards.
[PATCH] NAPI support for Sibyte MAC
[ This is a re-post, but the patch still applies and works fine against the linux-mips.org tip. We'd really like to get this in. -Mark] This patch completes the NAPI functionality for SB1250 MAC, including making NAPI a kernel option that can be turned on or off and adds the "sbmac_poll" routine. Signed off by: Mark Mason ([EMAIL PROTECTED]) Signed off by: Dan Krejsa ([EMAIL PROTECTED]) Signed off by: Steve Yang ([EMAIL PROTECTED]) Index: linux-2.6.14-cgl/drivers/net/Kconfig === --- linux-2.6.14-cgl.orig/drivers/net/Kconfig 2006-09-20 14:58:54.0 -0700 +++ linux-2.6.14-cgl/drivers/net/Kconfig2006-09-20 17:04:31.0 -0700 @@ -2031,6 +2031,23 @@ tristate "SB1250 Ethernet support" depends on SIBYTE_SB1xxx_SOC +config SBMAC_NAPI + bool "SBMAC: Use Rx Polling (NAPI) (EXPERIMENTAL)" + depends on NET_SB1250_MAC && EXPERIMENTAL + help + NAPI is a new driver API designed to reduce CPU and interrupt load + when the driver is receiving lots of packets from the card. It is + still somewhat experimental and thus not yet enabled by default. + + If your estimated Rx load is 10kpps or more, or if the card will be + deployed on potentially unfriendly networks (e.g. in a firewall), + then say Y here. + + See for more + information. + + If in doubt, say y. + config R8169_VLAN bool "VLAN support" depends on R8169 && VLAN_8021Q @@ -2826,3 +2843,5 @@ def_bool NETPOLL endmenu + + Index: linux-2.6.14-cgl/drivers/net/sb1250-mac.c === --- linux-2.6.14-cgl.orig/drivers/net/sb1250-mac.c 2006-09-20 14:59:00.0 -0700 +++ linux-2.6.14-cgl/drivers/net/sb1250-mac.c 2006-09-20 20:16:27.0 -0700 @@ -95,19 +95,28 @@ #endif #ifdef CONFIG_SBMAC_COALESCE -static int int_pktcnt = 0; -module_param(int_pktcnt, int, S_IRUGO); -MODULE_PARM_DESC(int_pktcnt, "Packet count"); - -static int int_timeout = 0; -module_param(int_timeout, int, S_IRUGO); -MODULE_PARM_DESC(int_timeout, "Timeout value"); +static int int_pktcnt_tx = 255; +module_param(int_pktcnt_tx, int, S_IRUGO); +MODULE_PARM_DESC(int_pktcnt_tx, "TX packet count"); + +static int int_timeout_tx = 255; +module_param(int_timeout_tx, int, S_IRUGO); +MODULE_PARM_DESC(int_timeout_tx, "TX timeout value"); + +static int int_pktcnt_rx = 64; +module_param(int_pktcnt_rx, int, S_IRUGO); +MODULE_PARM_DESC(int_pktcnt_rx, "RX packet count"); + +static int int_timeout_rx = 64; +module_param(int_timeout_rx, int, S_IRUGO); +MODULE_PARM_DESC(int_timeout_rx, "RX timeout value"); #endif #include #if defined(CONFIG_SIBYTE_BCM1x55) || defined(CONFIG_SIBYTE_BCM1x80) #include #include +#define R_MAC_DMA_OODPKTLOST_RXR_MAC_DMA_OODPKTLOST #elif defined(CONFIG_SIBYTE_SB1250) || defined(CONFIG_SIBYTE_BCM112X) #include #include @@ -155,8 +164,8 @@ #define NUMCACHEBLKS(x) (((x)+SMP_CACHE_BYTES-1)/SMP_CACHE_BYTES) -#define SBMAC_MAX_TXDESCR 32 -#define SBMAC_MAX_RXDESCR 32 +#define SBMAC_MAX_TXDESCR 256 +#define SBMAC_MAX_RXDESCR 256 #define ETHER_ALIGN2 #define ETHER_ADDR_LEN 6 @@ -185,10 +194,10 @@ * associated with it. */ - struct sbmac_softc *sbdma_eth; /* back pointer to associated MAC */ - int sbdma_channel; /* channel number */ + struct sbmac_softc *sbdma_eth; /* back pointer to associated MAC */ + int sbdma_channel; /* channel number */ int sbdma_txdir; /* direction (1=transmit) */ - int sbdma_maxdescr;/* total # of descriptors in ring */ + int sbdma_maxdescr;/* total # of descriptors in ring */ #ifdef CONFIG_SBMAC_COALESCE int sbdma_int_pktcnt; /* # descriptors rx/tx before interrupt*/ int sbdma_int_timeout; /* # usec rx/tx interrupt */ @@ -197,13 +206,16 @@ volatile void __iomem *sbdma_config0; /* DMA config register 0 */ volatile void __iomem *sbdma_config1; /* DMA config register 1 */ volatile void __iomem *sbdma_dscrbase; /* Descriptor base address */ - volatile void __iomem *sbdma_dscrcnt; /* Descriptor count register */ + volatile void __iomem *sbdma_dscrcnt; /* Descriptor count register */ volatile void __iomem *sbdma_curdscr; /* current descriptor address */ + volatile void __iomem *sbdma_oodpktlost;/* pkt drop (rx only) */ + /* * This stuff is for maintenance of the ring */ + sbdmadscr_t *sbdma_dscrtable_unaligned; sbdmadscr_t *sbdma_dscrtable; /* base of descript
Re: WARNING: CPU: 0 PID: 0 at net/ipv4/af_inet.c:155 inet_sock_destruct+0x1c4/0x1dc
On 05/07/2016 17:28, Florian Fainelli wrote: > Le 05/07/2016 07:50, Mason wrote: > >> On 05/07/2016 15:33, Mason wrote: >> >>> I was testing suspend/resume sequences where the suspend operation >>> fails and returns without having suspended the platform. Please forget I ever mentioned suspend, that was a red herring. The warning is displayed even if I never suspend. (Dropping linux-pm from this discussion.) >> I rebooted, tried the same operation, and hit the same warning >> still 310 seconds later: However, the 310 seconds time span still seems to be relevant. Steps to reproduce: I booted the system, logged in as root, mounted an NFS file system, then left the system idling at the prompt. (I don't remember seeing this warning in v4.1 and v4.4) What's going wrong here? Is it related to NFS? Here is the defconfig I'm using http://pastebin.ubuntu.com/19160299/ Regards. [ 317.940133] [ cut here ] [ 317.944815] WARNING: CPU: 1 PID: 0 at net/ipv4/af_inet.c:155 inet_sock_destruct+0x1c4/0x1dc [ 317.953223] Modules linked in: [ 317.956305] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.7.0-rc6-00010-gd07031bdc433-dirty #2 [ 317.964784] Hardware name: Sigma Tango DT [ 317.968809] Backtrace: [ 317.971279] [] (dump_backtrace) from [] (show_stack+0x18/0x1c) [ 317.978884] r7:6113 r6:c080ea84 r5: r4:c080ea84 [ 317.984590] [] (show_stack) from [] (dump_stack+0x80/0x94) [ 317.991856] [] (dump_stack) from [] (__warn+0xec/0x104) [ 317.998849] r7:0009 r6:c05e3fc8 r5: r4: [ 318.004549] [] (__warn) from [] (warn_slowpath_null+0x28/0x30) [ 318.012154] r9:dfbea4e0 r8:000a r7:df45fe30 r6:dec19594 r5:df68f144 r4:df68f040 [ 318.019954] [] (warn_slowpath_null) from [] (inet_sock_destruct+0x1c4/0x1dc) [ 318.028788] [] (inet_sock_destruct) from [] (__sk_destruct+0x28/0xe0) [ 318.037005] r7:df45fe30 r6:dec19594 r5:df68f040 r4:df68f1ec [ 318.042710] [] (__sk_destruct) from [] (rcu_process_callbacks+0x488/0x59c) [ 318.051363] r5: r4: [ 318.054962] [] (rcu_process_callbacks) from [] (__do_softirq+0x138/0x264) [ 318.063527] r10:c08020a0 r9:4001 r8:0101 r7:df45e000 r6:c08020a4 r5:0009 [ 318.071408] r4: [ 318.073953] [] (__do_softirq) from [] (irq_exit+0xc8/0x104) [ 318.081296] r10:df45ff58 r9:df402400 r8:0001 r7: r6:0013 r5: [ 318.089176] r4:c0735428 [ 318.091723] [] (irq_exit) from [] (__handle_domain_irq+0x88/0xf4) [ 318.099595] [] (__handle_domain_irq) from [] (gic_handle_irq+0x50/0x94) [ 318.107986] r10: r9:e0803100 r8:e0802100 r7:df45ff58 r6:e080210c r5:c080277c [ 318.115865] r4:c080eca0 r3:df45ff58 [ 318.119461] [] (gic_handle_irq) from [] (__irq_svc+0x54/0x90) [ 318.126980] Exception stack(0xdf45ff58 to 0xdf45ffa0) [ 318.132053] ff40: 0001 [ 318.140273] ff60: ab80 c0117c80 df45e000 c08024f8 c0802494 c081e2d6 c05b9550 413fc090 [ 318.148492] ff80: df45ffb4 df45ffb8 df45ffa8 c01086b0 c01086b4 6013 [ 318.156709] r9:413fc090 r8:c05b9550 r7:df45ff8c r6: r5:6013 r4:c01086b4 [ 318.164512] [] (arch_cpu_idle) from [] (default_idle_call+0x28/0x34) [ 318.172646] [] (default_idle_call) from [] (cpu_startup_entry+0x128/0x17c) [ 318.181305] [] (cpu_startup_entry) from [] (secondary_start_kernel+0x158/0x164) [ 318.190395] r7:c081e7c8 r4:c080b4f0 [ 318.193993] [] (secondary_start_kernel) from [<8010158c>] (0x8010158c) [ 318.201423] r5:0051 r4:9f44006a [ 318.205024] ---[ end trace 6e04001434b19cb9 ]--- Just to be sure, I performed the same steps a second time: [ 316.238527] [ cut here ] [ 316.243210] WARNING: CPU: 1 PID: 0 at net/ipv4/af_inet.c:155 inet_sock_destruct+0x1c4/0x1dc [ 316.251619] Modules linked in: [ 316.254702] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.7.0-rc6-00010-gd07031bdc433-dirty #2 [ 316.263182] Hardware name: Sigma Tango DT [ 316.267206] Backtrace: [ 316.269675] [] (dump_backtrace) from [] (show_stack+0x18/0x1c) [ 316.277280] r7:6113 r6:c080ea84 r5: r4:c080ea84 [ 316.282986] [] (show_stack) from [] (dump_stack+0x80/0x94) [ 316.290254] [] (dump_stack) from [] (__warn+0xec/0x104) [ 316.297247] r7:0009 r6:c05e3fc8 r5: r4: [ 316.302947] [] (__warn) from [] (warn_slowpath_null+0x28/0x30) [ 316.310552] r9:dfbea4e0 r8:000a r7:df45fe30 r6:dec15694 r5:df6063c4 r4:df6062c0 [ 316.318354] [] (warn_slowpath_null) from [] (inet_sock_destruct+0x1c4/0x1dc) [ 316.327190] [] (inet_sock_destruct) from [] (__sk_destruct+0x28/0xe0) [ 316.335406] r7:df45fe30 r6:dec15694 r5:df6062c0 r4:df60646c [ 316.341112] [] (__sk_destruct) from [] (rcu_process_callbacks+0x488/0x59c) [ 316.349765] r5: r4: [ 316.353363] [] (rcu_process_callbacks) from [] (
Re: WARNING: CPU: 0 PID: 0 at net/ipv4/af_inet.c:155 inet_sock_destruct+0x1c4/0x1dc
On 12/07/2016 11:53, Mason wrote: > However, the 310 seconds time span still seems to be relevant. > > Steps to reproduce: I booted the system, logged in as root, > mounted an NFS file system, then left the system idling at > the prompt. > > (I don't remember seeing this warning in v4.1 and v4.4) > > What's going wrong here? Is it related to NFS? > > Here is the defconfig I'm using > http://pastebin.ubuntu.com/19160299/ > > > [ 317.940133] [ cut here ] > [ 317.944815] WARNING: CPU: 1 PID: 0 at net/ipv4/af_inet.c:155 > inet_sock_destruct+0x1c4/0x1dc > [ 317.953223] Modules linked in: > [ 317.956305] CPU: 1 PID: 0 Comm: swapper/1 Not tainted > 4.7.0-rc6-00010-gd07031bdc433-dirty #2 > [ 317.964784] Hardware name: Sigma Tango DT > [ 317.968809] Backtrace: > [ 317.971279] [] (dump_backtrace) from [] > (show_stack+0x18/0x1c) > [ 317.978884] r7:6113 r6:c080ea84 r5: r4:c080ea84 > [ 317.984590] [] (show_stack) from [] > (dump_stack+0x80/0x94) > [ 317.991856] [] (dump_stack) from [] (__warn+0xec/0x104) > [ 317.998849] r7:0009 r6:c05e3fc8 r5: r4: > [ 318.004549] [] (__warn) from [] > (warn_slowpath_null+0x28/0x30) > [ 318.012154] r9:dfbea4e0 r8:000a r7:df45fe30 r6:dec19594 r5:df68f144 > r4:df68f040 > [ 318.019954] [] (warn_slowpath_null) from [] > (inet_sock_destruct+0x1c4/0x1dc) > [ 318.028788] [] (inet_sock_destruct) from [] > (__sk_destruct+0x28/0xe0) > [ 318.037005] r7:df45fe30 r6:dec19594 r5:df68f040 r4:df68f1ec > [ 318.042710] [] (__sk_destruct) from [] > (rcu_process_callbacks+0x488/0x59c) > [ 318.051363] r5: r4: > [ 318.054962] [] (rcu_process_callbacks) from [] > (__do_softirq+0x138/0x264) > [ 318.063527] r10:c08020a0 r9:4001 r8:0101 r7:df45e000 r6:c08020a4 > r5:0009 > [ 318.071408] r4: > [ 318.073953] [] (__do_softirq) from [] > (irq_exit+0xc8/0x104) > [ 318.081296] r10:df45ff58 r9:df402400 r8:0001 r7: r6:0013 > r5: > [ 318.089176] r4:c0735428 > [ 318.091723] [] (irq_exit) from [] > (__handle_domain_irq+0x88/0xf4) > [ 318.099595] [] (__handle_domain_irq) from [] > (gic_handle_irq+0x50/0x94) > [ 318.107986] r10: r9:e0803100 r8:e0802100 r7:df45ff58 r6:e080210c > r5:c080277c > [ 318.115865] r4:c080eca0 r3:df45ff58 > [ 318.119461] [] (gic_handle_irq) from [] > (__irq_svc+0x54/0x90) > [ 318.126980] Exception stack(0xdf45ff58 to 0xdf45ffa0) > [ 318.132053] ff40: > 0001 > [ 318.140273] ff60: ab80 c0117c80 df45e000 c08024f8 c0802494 c081e2d6 > c05b9550 413fc090 > [ 318.148492] ff80: df45ffb4 df45ffb8 df45ffa8 c01086b0 c01086b4 > 6013 > [ 318.156709] r9:413fc090 r8:c05b9550 r7:df45ff8c r6: r5:6013 > r4:c01086b4 > [ 318.164512] [] (arch_cpu_idle) from [] > (default_idle_call+0x28/0x34) > [ 318.172646] [] (default_idle_call) from [] > (cpu_startup_entry+0x128/0x17c) > [ 318.181305] [] (cpu_startup_entry) from [] > (secondary_start_kernel+0x158/0x164) > [ 318.190395] r7:c081e7c8 r4:c080b4f0 > [ 318.193993] [] (secondary_start_kernel) from [<8010158c>] > (0x8010158c) > [ 318.201423] r5:0051 r4:9f44006a > [ 318.205024] ---[ end trace 6e04001434b19cb9 ]--- > > > Just to be sure, I performed the same steps a second time: > > [ 316.238527] [ cut here ] > [ 316.243210] WARNING: CPU: 1 PID: 0 at net/ipv4/af_inet.c:155 > inet_sock_destruct+0x1c4/0x1dc > [ 316.251619] Modules linked in: > [ 316.254702] CPU: 1 PID: 0 Comm: swapper/1 Not tainted > 4.7.0-rc6-00010-gd07031bdc433-dirty #2 > [ 316.263182] Hardware name: Sigma Tango DT > [ 316.267206] Backtrace: > [ 316.269675] [] (dump_backtrace) from [] > (show_stack+0x18/0x1c) > [ 316.277280] r7:6113 r6:c080ea84 r5: r4:c080ea84 > [ 316.282986] [] (show_stack) from [] > (dump_stack+0x80/0x94) > [ 316.290254] [] (dump_stack) from [] (__warn+0xec/0x104) > [ 316.297247] r7:0009 r6:c05e3fc8 r5: r4: > [ 316.302947] [] (__warn) from [] > (warn_slowpath_null+0x28/0x30) > [ 316.310552] r9:dfbea4e0 r8:000a r7:df45fe30 r6:dec15694 r5:df6063c4 > r4:df6062c0 > [ 316.318354] [] (warn_slowpath_null) from [] > (inet_sock_destruct+0x1c4/0x1dc) > [ 316.327190] [] (inet_sock_destruct) from [] > (__sk_destruct+0x28/0xe0) > [ 316.335406] r7:df45fe30 r6:dec15694 r5:df6062c0 r4:df60646c > [ 316.341112] [] (__sk_destruct) from [] > (rcu_process_callbacks+0x488/0x59c) > [ 316.349765] r5: r4: > [ 316.353363] [] (rcu_proces
Re: WARNING: CPU: 0 PID: 0 at net/ipv4/af_inet.c:155 inet_sock_destruct+0x1c4/0x1dc
On 12/07/2016 16:25, Eric Dumazet wrote: > Could you try this debug patch ? Note: I've been unable to trigger the warning again. Dunno what has changed... With your patch applied, I get a warning at boot: [4.668309] nb8800 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx [4.688609] Sending DHCP requests ., OK [4.711935] IP-Config: Got DHCP answer from 172.27.200.1, my address is 172.27.64.49 [4.719956] IP-Config: Complete: [4.723221] device=eth0, hwaddr=00:16:e8:02:08:42, ipaddr=172.27.64.49, mask=255.255.192.0, gw=172.27.64.1 [4.733376] host=toto5, domain=france.foo.com sac.foo.com asic.foo.com soft.sde, nis-domain=france.foo.com [4.745279] bootserver=172.27.64.1, rootserver=172.27.64.1, rootpath=/export/roots/titi/6_2_0_8756,v3 nameserver0=172.27.0.17 [4.759725] [ cut here ] [4.764426] WARNING: CPU: 0 PID: 877 at net/core/sock.c:1468 sk_destruct+0x74/0x78 [4.772056] Modules linked in: [4.775133] CPU: 0 PID: 877 Comm: kworker/0:1H Not tainted 4.7.0-rc6-00010-gd07031bdc433-dirty #6 [4.784050] Hardware name: Sigma Tango DT [4.788084] Workqueue: rpciod rpc_async_schedule [4.792725] Backtrace: [4.795196] [] (dump_backtrace) from [] (show_stack+0x18/0x1c) [4.802802] r7:6013 r6:c080ea84 r5: r4:c080ea84 [4.808513] [] (show_stack) from [] (dump_stack+0x80/0x94) [4.815781] [] (dump_stack) from [] (__warn+0xec/0x104) [4.822776] r7:0009 r6:c05e4d04 r5: r4: [4.828482] [] (__warn) from [] (warn_slowpath_null+0x28/0x30) [4.836089] r9: r8:df5edb58 r7:df711364 r6:df006c80 r5:df5edb58 r4:df5eda40 [4.843898] [] (warn_slowpath_null) from [] (sk_destruct+0x74/0x78) [4.851945] [] (sk_destruct) from [] (__sk_free+0x50/0xbc) [4.859203] r5:df5edb58 r4:df5eda40 [4.862802] [] (__sk_free) from [] (sk_free+0x3c/0x40) [4.869710] r5:df5edb58 r4:df5eda40 [4.873310] [] (sk_free) from [] (sk_common_release+0xe8/0xf4) [4.880924] [] (sk_common_release) from [] (udp_lib_close+0x10/0x14) [4.889054] r5:df006c80 r4:df5eda40 [4.892657] [] (udp_lib_close) from [] (inet_release+0x4c/0x78) [4.900360] [] (inet_release) from [] (sock_release+0x28/0xa8) [4.907967] r5: r4:df006c80 [4.911575] [] (sock_release) from [] (xs_reset_transport+0xac/0xbc) [4.919706] r5:df5eda40 r4:df711000 [4.923306] [] (xs_reset_transport) from [] (xs_destroy+0x24/0x54) [4.931262] r9: r8:c049c614 r7:df7c3218 r6:df711000 r5: r4:df711000 [4.939070] [] (xs_destroy) from [] (xprt_destroy+0x88/0x8c) [4.946502] r5:df711218 r4:df711000 [4.950102] [] (xprt_destroy) from [] (xprt_put+0x40/0x44) [4.957358] r5:df7c3200 r4:df613d00 [4.960959] [] (xprt_put) from [] (rpc_task_release_client+0x7c/0x80) [4.969181] [] (rpc_task_release_client) from [] (rpc_release_resources_task+0x34/0x38) [4.978971] r7:c049c298 r6:0001 r5: r4:df613d00 [4.984674] [] (rpc_release_resources_task) from [] (__rpc_execute+0xb0/0x2a8) [4.993678] r5: r4:df613d00 [4.997277] [] (__rpc_execute) from [] (rpc_async_schedule+0x14/0x18) [5.005496] r10:df683500 r9: r8:dfbe4700 r7: r6:dfbdcc80 r5:df683500 [5.013383] r4:df613d24 [5.015935] [] (rpc_async_schedule) from [] (process_one_work+0x128/0x3fc) [5.024595] [] (process_one_work) from [] (worker_thread+0x58/0x574) [5.032726] r10:df683500 r9:df6d4000 r8:dfbdcc98 r7:c0802100 r6:0008 r5:df683518 [5.040614] r4:dfbdcc80 [5.043162] [] (worker_thread) from [] (kthread+0xe4/0xfc) [5.050419] r10: r9: r8: r7:c0132914 r6:df683500 r5:df6cc600 [5.058309] r4: [5.060858] [] (kthread) from [] (ret_from_fork+0x14/0x3c) [5.068116] r7: r6: r5:c013838c r4:df6cc600 [5.073846] ---[ end trace 05b24e2dedd2f2a0 ]--- [5.082009] VFS: Mounted root (nfs filesystem) readonly on device 0:11. [5.090328] Freeing unused kernel memory: 1024K (c070 - c080) [5.452552] random: udevd urandom read with 68 bits of entropy available
Re: WARNING: CPU: 0 PID: 0 at net/ipv4/af_inet.c:155 inet_sock_destruct+0x1c4/0x1dc
On 12/07/2016 16:38, Mason wrote: > With Eric's patch applied, I get this warning at boot: > > [4.668309] nb8800 26000.ethernet eth0: Link is Up - 1Gbps/Full - flow > control rx/tx > [4.688609] Sending DHCP requests ., OK > [4.711935] IP-Config: Got DHCP answer from 172.27.200.1, my address is > 172.27.64.49 > [4.719956] IP-Config: Complete: > [4.723221] device=eth0, hwaddr=00:16:e8:02:08:42, > ipaddr=172.27.64.49, mask=255.255.192.0, gw=172.27.64.1 > [4.733376] host=toto5, domain=france.foo.com sac.foo.com > asic.foo.com soft.sde, nis-domain=france.foo.com > [4.745279] bootserver=172.27.64.1, rootserver=172.27.64.1, > rootpath=/export/roots/titi/6_2_0_8756,v3 nameserver0=172.27.0.17 > [4.759725] [ cut here ] > [4.764426] WARNING: CPU: 0 PID: 877 at net/core/sock.c:1468 > sk_destruct+0x74/0x78 > [4.772056] Modules linked in: > [4.775133] CPU: 0 PID: 877 Comm: kworker/0:1H Not tainted > 4.7.0-rc6-00010-gd07031bdc433-dirty #6 > [4.784050] Hardware name: Sigma Tango DT > [4.788084] Workqueue: rpciod rpc_async_schedule > [4.792725] Backtrace: > [4.795196] [] (dump_backtrace) from [] > (show_stack+0x18/0x1c) > [4.802802] r7:6013 r6:c080ea84 r5: r4:c080ea84 > [4.808513] [] (show_stack) from [] > (dump_stack+0x80/0x94) > [4.815781] [] (dump_stack) from [] (__warn+0xec/0x104) > [4.822776] r7:0009 r6:c05e4d04 r5: r4: > [4.828482] [] (__warn) from [] > (warn_slowpath_null+0x28/0x30) > [4.836089] r9: r8:df5edb58 r7:df711364 r6:df006c80 r5:df5edb58 > r4:df5eda40 > [4.843898] [] (warn_slowpath_null) from [] > (sk_destruct+0x74/0x78) > [4.851945] [] (sk_destruct) from [] > (__sk_free+0x50/0xbc) > [4.859203] r5:df5edb58 r4:df5eda40 > [4.862802] [] (__sk_free) from [] (sk_free+0x3c/0x40) > [4.869710] r5:df5edb58 r4:df5eda40 > [4.873310] [] (sk_free) from [] > (sk_common_release+0xe8/0xf4) > [4.880924] [] (sk_common_release) from [] > (udp_lib_close+0x10/0x14) > [4.889054] r5:df006c80 r4:df5eda40 > [4.892657] [] (udp_lib_close) from [] > (inet_release+0x4c/0x78) > [4.900360] [] (inet_release) from [] > (sock_release+0x28/0xa8) > [4.907967] r5: r4:df006c80 > [4.911575] [] (sock_release) from [] > (xs_reset_transport+0xac/0xbc) > [4.919706] r5:df5eda40 r4:df711000 > [4.923306] [] (xs_reset_transport) from [] > (xs_destroy+0x24/0x54) > [4.931262] r9: r8:c049c614 r7:df7c3218 r6:df711000 r5: > r4:df711000 > [4.939070] [] (xs_destroy) from [] > (xprt_destroy+0x88/0x8c) > [4.946502] r5:df711218 r4:df711000 > [4.950102] [] (xprt_destroy) from [] > (xprt_put+0x40/0x44) > [4.957358] r5:df7c3200 r4:df613d00 > [4.960959] [] (xprt_put) from [] > (rpc_task_release_client+0x7c/0x80) > [4.969181] [] (rpc_task_release_client) from [] > (rpc_release_resources_task+0x34/0x38) > [4.978971] r7:c049c298 r6:0001 r5: r4:df613d00 > [4.984674] [] (rpc_release_resources_task) from [] > (__rpc_execute+0xb0/0x2a8) > [4.993678] r5: r4:df613d00 > [4.997277] [] (__rpc_execute) from [] > (rpc_async_schedule+0x14/0x18) > [5.005496] r10:df683500 r9: r8:dfbe4700 r7: r6:dfbdcc80 > r5:df683500 > [5.013383] r4:df613d24 > [5.015935] [] (rpc_async_schedule) from [] > (process_one_work+0x128/0x3fc) > [5.024595] [] (process_one_work) from [] > (worker_thread+0x58/0x574) > [5.032726] r10:df683500 r9:df6d4000 r8:dfbdcc98 r7:c0802100 r6:0008 > r5:df683518 > [5.040614] r4:dfbdcc80 > [5.043162] [] (worker_thread) from [] > (kthread+0xe4/0xfc) > [5.050419] r10: r9: r8: r7:c0132914 r6:df683500 > r5:df6cc600 > [5.058309] r4: > [5.060858] [] (kthread) from [] > (ret_from_fork+0x14/0x3c) > [5.068116] r7: r6: r5:c013838c r4:df6cc600 > [5.073846] ---[ end trace 05b24e2dedd2f2a0 ]--- > [5.082009] VFS: Mounted root (nfs filesystem) readonly on device 0:11. > [5.090328] Freeing unused kernel memory: 1024K (c070 - c080) > [5.452552] random: udevd urandom read with 68 bits of entropy available AFAICT, I get the same call stack for every boot. Is this an unexpected call sequence? Are there other tests I can run? Regards.
Ethernet not working on a different SoC with same eth HW
Hello everyone, I'm using these net drivers: drivers/net/ethernet/aurora/nb8800.c drivers/net/phy/at803x.c With a smp8758 board, they work great. I've been trying to use them on a different board: same eth PHY (Atheros AR8035) same eth MAC (Aurora SSN8800) different SoC (same base address for MAC block) It doesn't work, and I'm not sure where to look first... I put logs in some functions to get a rough idea. When the kernel boots, it prints: [0.798912] libphy: mdiobus_read: addr=0 regnum=0002 from get_phy_device [0.806515] libphy: mdiobus_read: addr=0 regnum=0003 from get_phy_device [0.813704] libphy: mdiobus_read: addr=1 regnum=0002 from get_phy_device [0.820923] libphy: mdiobus_read: addr=1 regnum=0003 from get_phy_device [0.828192] libphy: mdiobus_read: addr=2 regnum=0002 from get_phy_device [0.835373] libphy: mdiobus_read: addr=2 regnum=0003 from get_phy_device [0.842547] libphy: mdiobus_read: addr=3 regnum=0002 from get_phy_device [0.849681] libphy: mdiobus_read: addr=3 regnum=0003 from get_phy_device [0.856814] libphy: mdiobus_read: addr=4 regnum=0002 from get_phy_device [0.863944] libphy: mdiobus_read: addr=4 regnum=0003 from get_phy_device [0.871075] libphy: mdiobus_read: addr=5 regnum=0002 from get_phy_device [0.878202] libphy: mdiobus_read: addr=5 regnum=0003 from get_phy_device [0.885331] libphy: mdiobus_read: addr=6 regnum=0002 from get_phy_device [0.892458] libphy: mdiobus_read: addr=6 regnum=0003 from get_phy_device [0.899586] libphy: mdiobus_read: addr=7 regnum=0002 from get_phy_device [0.906715] libphy: mdiobus_read: addr=7 regnum=0003 from get_phy_device [0.913844] libphy: mdiobus_read: addr=8 regnum=0002 from get_phy_device [0.920972] libphy: mdiobus_read: addr=8 regnum=0003 from get_phy_device [0.928101] libphy: mdiobus_read: addr=9 regnum=0002 from get_phy_device [0.935230] libphy: mdiobus_read: addr=9 regnum=0003 from get_phy_device [0.942359] libphy: mdiobus_read: addr=10 regnum=0002 from get_phy_device [0.949576] libphy: mdiobus_read: addr=10 regnum=0003 from get_phy_device [0.956793] libphy: mdiobus_read: addr=11 regnum=0002 from get_phy_device [0.964009] libphy: mdiobus_read: addr=11 regnum=0003 from get_phy_device [0.971225] libphy: mdiobus_read: addr=12 regnum=0002 from get_phy_device [0.978441] libphy: mdiobus_read: addr=12 regnum=0003 from get_phy_device [0.985657] libphy: mdiobus_read: addr=13 regnum=0002 from get_phy_device [0.992873] libphy: mdiobus_read: addr=13 regnum=0003 from get_phy_device [1.89] libphy: mdiobus_read: addr=14 regnum=0002 from get_phy_device [1.007305] libphy: mdiobus_read: addr=14 regnum=0003 from get_phy_device [1.014522] libphy: mdiobus_read: addr=15 regnum=0002 from get_phy_device [1.021737] libphy: mdiobus_read: addr=15 regnum=0003 from get_phy_device [1.028953] libphy: mdiobus_read: addr=16 regnum=0002 from get_phy_device [1.036169] libphy: mdiobus_read: addr=16 regnum=0003 from get_phy_device [1.043385] libphy: mdiobus_read: addr=17 regnum=0002 from get_phy_device [1.050600] libphy: mdiobus_read: addr=17 regnum=0003 from get_phy_device [1.057817] libphy: mdiobus_read: addr=18 regnum=0002 from get_phy_device [1.065032] libphy: mdiobus_read: addr=18 regnum=0003 from get_phy_device [1.072250] libphy: mdiobus_read: addr=19 regnum=0002 from get_phy_device [1.079464] libphy: mdiobus_read: addr=19 regnum=0003 from get_phy_device [1.086679] libphy: mdiobus_read: addr=20 regnum=0002 from get_phy_device [1.093895] libphy: mdiobus_read: addr=20 regnum=0003 from get_phy_device [1.10] libphy: mdiobus_read: addr=21 regnum=0002 from get_phy_device [1.108327] libphy: mdiobus_read: addr=21 regnum=0003 from get_phy_device [1.115542] libphy: mdiobus_read: addr=22 regnum=0002 from get_phy_device [1.122757] libphy: mdiobus_read: addr=22 regnum=0003 from get_phy_device [1.129973] libphy: mdiobus_read: addr=23 regnum=0002 from get_phy_device [1.137189] libphy: mdiobus_read: addr=23 regnum=0003 from get_phy_device [1.144405] libphy: mdiobus_read: addr=24 regnum=0002 from get_phy_device [1.151620] libphy: mdiobus_read: addr=24 regnum=0003 from get_phy_device [1.158835] libphy: mdiobus_read: addr=25 regnum=0002 from get_phy_device [1.166050] libphy: mdiobus_read: addr=25 regnum=0003 from get_phy_device [1.173266] libphy: mdiobus_read: addr=26 regnum=0002 from get_phy_device [1.180481] libphy: mdiobus_read: addr=26 regnum=0003 from get_phy_device [1.187697] libphy: mdiobus_read: addr=27 regnum=0002 from get_phy_device [1.194911] libphy: mdiobus_read: addr=27 regnum=0003 from get_phy_device [1.202127] libphy
Re: Ethernet not working on a different SoC with same eth HW
On 31/10/2016 16:37, Andrew Lunn wrote:
>> The regnum=5,4,1,1,a,9 logs keep repeating, endlessly.
>> Is that expected?
>
> Yes, that is expected, if you are not using interrupts. The phylib
> state machine polls the state of the PHY once per second to see if
> there has been a link up/down.
Interesting. But the logs are showing accesses much more frequent
than once per second, it seems... (?)
And an interrupt for the PHY is configured in the device tree:
ð0 {
#address-cells = <1>;
#size-cells = <0>;
phy-connection-type = "rgmii";
phy-handle = <ð0_phy>;
/* Atheros AR8035 */
eth0_phy: ethernet-phy@4 {
compatible = "ethernet-phy-id004d.d072",
"ethernet-phy-ieee802.3-c22";
interrupts = <37 IRQ_TYPE_EDGE_RISING>;
reg = <4>;
};
};
I'll add a log for the request_irq call.
Regards.
Re: Ethernet not working on a different SoC with same eth HW
On 31/10/2016 16:53, Andrew Lunn wrote: >> I'll add a log for the request_irq call. > > And take a look at /proc/interrupts You're right, there does seem to be something wrong with the interrupts. # cat /proc/interrupts CPU0 20: 26285 GIC-0 29 Edge twd IPI0: 0 CPU wakeup interrupts IPI1: 0 Timer broadcast interrupts IPI2: 0 Rescheduling interrupts IPI3: 0 Function call interrupts IPI4: 0 CPU stop interrupts IPI5: 0 IRQ work interrupts IPI6: 0 completion interrupts Err: 0 None of the expected platform interrupts (eth, phy, uart) are registered. Thanks for pushing in a promising direction. Regards.
Re: [PATCH net-next v2 3/4] Documentation: net: phy: Add blurb about RGMII
On 28/11/2016 18:43, Florian Fainelli wrote: > On 11/28/2016 02:34 AM, Sebastian Frias wrote: > >> For what is worth, the Atheros at803x driver comes with RX delay enabled >> as per HW reset. > > Always, or is this a behavior possibly defined via a stra-pin that can > be changed? Here's the data sheet: http://www.redeszone.net/app/uploads/2014/04/AR8035.pdf 4.2.25 rgmii rx clock delay control Offset: 0x00 bit 15: Control bit for rgmii interface rx clock delay: 1 = rgmii rx clock delay enable 0 = rgmii rx clock delay disable HW Rst. 1 SW Rst. 1 As far as I can tell, rx delay is enabled by default, always. The "Features" list mentions "RGMII timing modes support internal delay and external delay on Rx path" (Not sure about the internal vs external distinction.) Table 3-6. RGMII AC Characteristics — no Internal Delay Table 3-7. RGMII AC Characteristics — with internal delay added (Default) Delay is 2 ns apparently. There's also 4.2.27 Hib ctrl and rgmii gtx clock delay register Offset: 0x0B bits 6:5 Gtx_dly_val Select the delay of gtx_clk. 00:0.25ns 01:1.3ns 10:2.4ns 11:3.4ns I don't know what that is used for. Maybe this is the external vs internal delay? Regards.
Re: [PATCH net 04/16] net: ethernet: aurora: nb8800: fix fixed-link phydev leaks
On 28/11/2016 19:24, Johan Hovold wrote:
> Make sure to deregister and free any fixed-link PHY registered using
> of_phy_register_fixed_link() on probe errors and on driver unbind.
>
> Fixes: c7dfe3abf40e ("net: ethernet: nb8800: support fixed-link DT node")
> Signed-off-by: Johan Hovold
> ---
> drivers/net/ethernet/aurora/nb8800.c | 9 +++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
Did you use scripts/get_maintainer.pl ?
Neither the author of the driver (Mans) nor the author of
the code in question (Sebastian) were CCed on this patch.
It looks like the CC list was truncated, the last entry being
Vivien Didelot <
Regards.
> diff --git a/drivers/net/ethernet/aurora/nb8800.c
> b/drivers/net/ethernet/aurora/nb8800.c
> index 00c38bf151e6..e078d8da978c 100644
> --- a/drivers/net/ethernet/aurora/nb8800.c
> +++ b/drivers/net/ethernet/aurora/nb8800.c
> @@ -1466,12 +1466,12 @@ static int nb8800_probe(struct platform_device *pdev)
>
> ret = nb8800_hw_init(dev);
> if (ret)
> - goto err_free_bus;
> + goto err_deregister_fixed_link;
>
> if (ops && ops->init) {
> ret = ops->init(dev);
> if (ret)
> - goto err_free_bus;
> + goto err_deregister_fixed_link;
> }
>
> dev->netdev_ops = &nb8800_netdev_ops;
> @@ -1504,6 +1504,9 @@ static int nb8800_probe(struct platform_device *pdev)
>
> err_free_dma:
> nb8800_dma_free(dev);
> +err_deregister_fixed_link:
> + if (of_phy_is_fixed_link(pdev->dev.of_node))
> + of_phy_deregister_fixed_link(pdev->dev.of_node);
> err_free_bus:
> of_node_put(priv->phy_node);
> mdiobus_unregister(bus);
> @@ -1521,6 +1524,8 @@ static int nb8800_remove(struct platform_device *pdev)
> struct nb8800_priv *priv = netdev_priv(ndev);
>
> unregister_netdev(ndev);
> + if (of_phy_is_fixed_link(pdev->dev.of_node))
> + of_phy_deregister_fixed_link(pdev->dev.of_node);
> of_node_put(priv->phy_node);
>
> mdiobus_unregister(priv->mii_bus);
Re: Ethernet not working on a different SoC with same eth HW
On 31/10/2016 17:28, Mason wrote:
> On 31/10/2016 16:53, Andrew Lunn wrote:
>
>>> I'll add a log for the request_irq call.
>>
>> And take a look at /proc/interrupts
>
> You're right, there does seem to be something wrong with the interrupts.
Having fixed that, I'm still unable to ping a box on the same
ethernet segment... Still investigating.
I think I may have spotted a potential issue in the Atheros driver.
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
ret = at803x_enable_rx_delay(phydev);
if (ret < 0)
return ret;
}
Looking at this code, one might believe that "rgmii rx clock delay"
is only enabled when the user requests it (through DT).
http://www.redeszone.net/app/uploads/2014/04/AR8035.pdf
cf. PDF page 48
*However* this bit is set to 1 at reset (both HW and SW resets).
Thus, "rgmii rx clock delay" is always enabled, whether the user
requests it or not.
Could someone knowledgeable comment on the expected behavior of
enabling rgmii rx (and tx) clock delay?
https://en.wikipedia.org/wiki/Media-independent_interface#Reduced_gigabit_media-independent_interface
Regards.
Re: Ethernet not working on a different SoC with same eth HW
On 04/11/2016 14:40, Måns Rullgård wrote:
> Mason writes:
>
>> On 31/10/2016 17:28, Mason wrote:
>>
>>> On 31/10/2016 16:53, Andrew Lunn wrote:
>>>
>>>>> I'll add a log for the request_irq call.
>>>>
>>>> And take a look at /proc/interrupts
>>>
>>> You're right, there does seem to be something wrong with the interrupts.
>>
>> Having fixed that, I'm still unable to ping a box on the same
>> ethernet segment... Still investigating.
>>
>> I think I may have spotted a potential issue in the Atheros driver.
>>
>> if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
>> phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
>> ret = at803x_enable_rx_delay(phydev);
>> if (ret < 0)
>> return ret;
>> }
>>
>> Looking at this code, one might believe that "rgmii rx clock delay"
>> is only enabled when the user requests it (through DT).
>>
>> http://www.redeszone.net/app/uploads/2014/04/AR8035.pdf
>> cf. PDF page 48
>>
>> *However* this bit is set to 1 at reset (both HW and SW resets).
>> Thus, "rgmii rx clock delay" is always enabled, whether the user
>> requests it or not.
>>
>> Could someone knowledgeable comment on the expected behavior of
>> enabling rgmii rx (and tx) clock delay?
>
> Clock delay is sometimes (depending on PCB layout) required to achieve
> the correct timing between clock and data signals. The delay can be
> applied at the MAC or the PHY. I'd start by finding out what the PCB
> design expects or check the signals with a fast oscilloscope if you have
> one.
Considering the ethernet DT bindings:
https://www.kernel.org/doc/Documentation/devicetree/bindings/net/ethernet.txt
Specifically, phy-mode values "rgmii", "rgmii-id", "rgmii-rxid", "rgmii-txid".
Assuming that "rxid" (rx internal delay) and "rx clock delay" are
in fact the same concept with different names, do you agree that
it would be unexpected for "rgmii rx clock delay" to be enabled
when a DTB specifies "rgmii" or "rgmii-txid" ?
Regards.
Re: Ethernet not working on a different SoC with same eth HW
On 04/11/2016 15:04, Måns Rullgård wrote: > Andrew Lunn writes: > >>> Considering the ethernet DT bindings: >>> >>> https://www.kernel.org/doc/Documentation/devicetree/bindings/net/ethernet.txt >>> >>> Specifically, phy-mode values "rgmii", "rgmii-id", "rgmii-rxid", >>> "rgmii-txid". >>> >>> Assuming that "rxid" (rx internal delay) and "rx clock delay" are >>> in fact the same concept with different names, do you agree that >>> it would be unexpected for "rgmii rx clock delay" to be enabled >>> when a DTB specifies "rgmii" or "rgmii-txid" ? >> >> I agree with you. But fixing it is likely to break boards which >> currently have "rgmii", but actually need the delay in order to work. > > Does the internal delay here refer to the PHY or the MAC? It's a > property of the MAC node after all. Isn't there a one-to-one correspondence between MAC and PHY? Can a MAC be connected to multiple PHYs? Can a PHY be connected to multiple MACs? If it is a one-to-one correspondence, then whether the property is specified in the MAC or PHY node is just a matter of presentation, IMHO. Regards.
Re: Ethernet not working on a different SoC with same eth HW
On 04/11/2016 17:55, Måns Rullgård wrote: > Florian Fainelli writes: > >> On 11/04/2016 08:22 AM, Måns Rullgård wrote: >>> Andrew Lunn writes: >>> >>>> On Fri, Nov 04, 2016 at 03:05:00PM +, Måns Rullgård wrote: >>>>> Andrew Lunn writes: >>>>> >>>>>>>> I agree with you. But fixing it is likely to break boards which >>>>>>>> currently have "rgmii", but actually need the delay in order to work. >>>>>>> >>>>>>> Does the internal delay here refer to the PHY or the MAC? It's a >>>>>>> property of the MAC node after all. >>>>>> >>>>>> It is the PHY which applies the delay. >>>>> >>>>> Says who? >>>> >>>> The source code. >>> >>> There's source code that disagrees with that. The Broadcom GENET >>> driver, for instance. >> >> Correct, and in the case where the MAC adds the delay while transmitting >> (because it supports that) the expectation is that the PHY would remove >> such a delay internally, conversely, the PHY would introduce a delay >> while transmitting back to the PHY, in order to produce the desired 90 >> degrees shift on the RGMII signals, and get reproduce the correct clock >> and data alignment internally. >> >>> >>>>> Some MACs can do it too. >>>> >>>> I'm sure they can. But look at the code. Nearly none do, and those >>>> that do are potentially broken. >>> >>> Those few drivers that do anything differently based on these values >>> enable clock delay in the MAC. That's why I wrote the NB8800 driver the >>> way I did. >>> >> >> I don't really what is wrong with the nb8800 driver at the moment, so >> maybe this is just a configuration issue with the Atheros PHY driver, >> it's not like it has not given people headache judging by the recent >> discussions... > > We don't even know if the problems Mason is having are caused by > incorrect clock skew in the first place. I'd suggest not patching > anything at all until he gets it working. All I said was: > Assuming that "rxid" (rx internal delay) and "rx clock delay" are > in fact the same concept with different names, do you agree that > it would be unexpected for "rgmii rx clock delay" to be enabled > when a DTB specifies "rgmii" or "rgmii-txid" ? In parallel, Sebastian changed the DT of a 8758 board from phy-connection-type = "rgmii"; to phy-connection-type = "rgmii-txid"; and this broke the Ethernet on 8758, although the "reference" legacy 3.4 kernel does enable tx clock delay. Regards.
Re: Ethernet not working on a different SoC with same eth HW
On 31/10/2016 16:29, Mason wrote: > I'm using these net drivers: > > drivers/net/ethernet/aurora/nb8800.c > drivers/net/phy/at803x.c > > With a smp8758 board, they work great. > I've been trying to use them on a different board: > > same eth PHY (Atheros AR8035) > same eth MAC (Aurora SSN8800) > different SoC (same base address for MAC block) > > It doesn't work, and I'm not sure where to look first... Sweet baby Jesus on a pogo stick... After oh-so-many days making increasingly random changes, hoping something would magically un-break, we did find a *local* commit for an exotic platform (chip emulator) that was causing the issue. Regards.
Re: Ethernet not working on a different SoC with same eth HW
On 08/11/2016 16:41, Mason wrote: > On 31/10/2016 16:29, Mason wrote: > >> I'm using these net drivers: >> >> drivers/net/ethernet/aurora/nb8800.c >> drivers/net/phy/at803x.c >> >> With a smp8758 board, they work great. >> I've been trying to use them on a different board: >> >> same eth PHY (Atheros AR8035) >> same eth MAC (Aurora SSN8800) >> different SoC (same base address for MAC block) >> >> It doesn't work, and I'm not sure where to look first... > > After oh-so-many days making increasingly random changes, > hoping something would magically un-break, we did find a > *local* commit for an exotic platform (chip emulator) that > was causing the issue. However... all is not well yet :-( A) When the board is connected to a Gigabit switch, it is able to complete the DHCP dance. But this takes around 5 seconds, (with several requests timing out). Whereas the same board running an ugly 3.4 kernel (which Mans called "quite hideous even by evil vendor standards") completes the DHCP dance in under a second. B) When the board is connected to a Fast Ethernet switch, the DHCP dance times out forever. (Whereas this works on the 3.4 kernel.) C) When the board is connected to a Gigabit switch, then plugged into the Fast Ethernet switch, then back into the Gigabit switch, network connectivity is lost forever. We've started examining the differences in phy and net frameworks between 3.4 and 4.9 but that's an atom in a haystack. To be continued... Regards.
