i.MX6S/DL and QCA8334 switch using DSA driver - CPU port not working

i.MX6S/DL and QCA8334 switch using DSA driver - CPU port not working

[Michal Vokáč]

Ahoj,

Sorry for bothering you guys but I could not come up with a more appropriate
place to ask my question.

I would very much appreciate some adivce to the DSA/QCA8K driver usage.
I am not (yet) very educated in networking internals so my wording may not be
right and my expectations may be wrong.

I am working on custom boards based on i.MX6S and i.MX6DL with QCA8334 four
port switch chip. The switch is connected to CPU using MDIO and RGMII
interface.
   _____                 _____________
  |     |               |   QCA8334   |
  |     |  RGMII        |        port2| eth2 <---> LAN (DHCP server)
  | CPU | <------> eth0 |port0        |
  |     |   MDIO        |        port3| eth1 <---> Some device
  |_____| <------>      |_____________|

HW issue is out of question as we are using the HW for a long time with
older kernel and a custom PHY driver for the switch.

I am now in a process of upgrading the kernel from 3.14 to 4.9 and I would
like to get rid of our ugly driver and utilize the mainline code.

I went through a lot of documentation/slides/video regarding the DSA subsystem.
I am still having a hard time to make the switch work as I expect with DSA and
qca8k driver.

I will describe my problem in three steps:

  - 1. The old version - this is what we were using so far.
  - 2. Current state - this is what I was able to come up with.
  - 3. Questions

------------------
1. The old version
------------------

  - Linux 3.14 (Freescale 3.14-1.1.x-imx branch)
  - Custom PHY driver for the switch chip

Device tree snippet:

&fec {
	pinctrl-names = "default";
	pinctrl-0 = <&pinctrl_enet_4>;
	phy-mode = "rgmii";
	phy-reset-gpios = <&gpio1 25 0>;
	phy-reset-duration = <0x14>;
	phy-supply = <&sw2_reg>;
	phy-handle = <&ethphy0>;
	status = "okay";

	mdio {
		#address-cells = <1>;
		#size-cells = <0>;
		ethphy0: ethernet-phy@0 {
			compatible = "ethernet-phy-id004d.d036";
			reg = <0>;
			max-speed = <1000>;
		};
	};
};

In my old setup I see only one ethernet interface.
The driver leaves most internal registers of the switch in its default state.
The major change is that forwarding is enabled not just across all the user
ports (HW default) but to the CPU port as well.

	qca8k_write(priv, 0x0624, 0x007F7F7F);

Regardless of state of the eth0 interface, communication is possible through
the switch. Device connected to port3 can get IP address from local DHCP server.
This is what you get by default without touching a single bit of the switch
internal registers.

Then when I bring up the eth0 interface also the CPU itself can get IP address
from local DHCP server and communicate to LAN or with the device.

----------------
2. Current state
----------------

  - Linux 4.9.84 (Freescale 4.9-1.0.x-imx branch)
  - Mainline DSA drivers
  - Mainline qca8k driver

The Freescale branch does not introduce any changes to the DSA nor to the QCA8K
drivers from mainline.

The QCA8K driver currently supports only the seven-port variant QCA8337.
AFAIK the only difference between the QCA8337 and QCA8334 we use is the number
of ports - 7 vs 4 respectively. So I added "qca,qca8334" compatible string to
the qca8k_of_match table in qca8k.c driver.

These corresponding configs are enabled:

  CONFIG_HAVE_NET_DSA=y
  CONFIG_NET_DSA=y
  CONFIG_NET_DSA_TAG_QCA=y
  CONFIG_NET_DSA_QCA8K=y

Device tree snippet:

&fec {
	pinctrl-names = "default";
	pinctrl-0 = <&pinctrl_enet_4>;
	phy-mode = "rgmii";
	phy-reset-gpios = <&gpio1 25 0>;
	phy-reset-duration = <0x14>;
	phy-supply = <&sw2_reg>;
	phy-handle = <&ethphy0>;
	status = "okay";

	mdio {
		#address-cells = <1>;
		#size-cells = <0>;

		phy_port2: phy@1 {
		    reg = <1>;
		};

		phy_port3: phy@2 {
		    reg = <2>;
		};

		switch@0 {
			compatible = "qca,qca8334";
			#address-cells = <1>;
			#size-cells = <0>;
			reg = <0>;

			ports {
				#address-cells = <1>;
				#size-cells = <0>;
				ethphy0: port@0 {
					reg = <0>;
					label = "cpu";
					phy-mode = "rgmii";
					ethernet = <&fec>;
					fixed-link {
						speed = <1000>;
						full-duplex;
					};
				};
				ethphy2: port@2 {
					reg = <2>;
					label = "eth2";
					phy-handle = <&phy_port2>;
				};

				ethphy3: port@3 {
					reg = <3>;
					label = "eth1";
					phy-handle = <&phy_port3>;
				};
			};
		};
	};
};

  # dmesg
  [    1.954903] libphy: fec_enet_mii_bus: probed
  [    1.955457] mdio_bus 2188000.ethernet-1:00: mdio_device_register
  [    1.955763] dsa2: enabling port: 2
  [    1.955770] dsa2: enabling port: 3
  [    1.956493] fec 2188000.ethernet eth0: registered PHC device 0
  ...
  [    2.920810] dsa2: enabling port: 2
  [    2.920817] dsa2: enabling port: 3
  [    2.920878] DSA: switch 0 0 parsed
  [    2.920884] DSA: tree 0 parsed
  [    2.927628] libphy: dsa slave smi: probed
  [    2.928161] Generic PHY 2188000.ethernet-1:01: attached PHY driver [Generic PHY] (mii_bus:phy_addr=2188000.ethernet-1:01, irq=-1)
  [    2.929326] Generic PHY dsa-0.0:02: attached PHY driver [Generic PHY] (mii_bus:phy_addr=dsa-0.0:02, irq=-1)
  
  # ip a
  2: eth0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN group default qlen 1000
       link/ether 68:7c:d5:04:01:5a brd ff:ff:ff:ff:ff:ff
  3: eth2@eth0: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noop state DOWN group default qlen 1000
       link/ether 68:7c:d5:04:01:5a brd ff:ff:ff:ff:ff:ff
  4: eth1@eth0: <BROADCAST,MULTICAST,M-DOWN> mtu 1500 qdisc noop state DOWN group default qlen 1000
       link/ether 68:7c:d5:04:01:5a brd ff:ff:ff:ff:ff:ff

At this point I am able to bring up all three interfaces, create a bridge, add
eth1 and eth2 to the bridge and bring up the bridge. But the bridge does not
work. No communication is possible from one port to the other.

To make the bridge work I need to enable forwarding across all the switch ports
at setup.

--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -578,12 +578,12 @@ qca8k_setup(struct dsa_switch *ds)
    		if (ds->enabled_port_mask & BIT(i))
    			qca8k_port_set_status(priv, i, 0);
    
-	/* Forward all unknown frames to CPU port for Linux processing */
+	/* Forward all unknown frames to all pors */
    	qca8k_write(priv, QCA8K_REG_GLOBAL_FW_CTRL1,
    		    BIT(0) << QCA8K_GLOBAL_FW_CTRL1_IGMP_DP_S |
-		    BIT(0) << QCA8K_GLOBAL_FW_CTRL1_BC_DP_S |
-		    BIT(0) << QCA8K_GLOBAL_FW_CTRL1_MC_DP_S |
-		    BIT(0) << QCA8K_GLOBAL_FW_CTRL1_UC_DP_S);
+		    0x7f << QCA8K_GLOBAL_FW_CTRL1_BC_DP_S |
+		    0x7f << QCA8K_GLOBAL_FW_CTRL1_MC_DP_S |
+		    0x7f << QCA8K_GLOBAL_FW_CTRL1_UC_DP_S);
    
    	/* Setup connection between CPU port & user ports */
    	for (i = 0; i < DSA_MAX_PORTS; i++) {
--

Then when I do:

  # ifconfig eth0 up
  # ifconfig eth1 up
  # ifconfig eth2 up
  # brctl addbr br0
  # brctl addif eth1
  # brctl addif eth2
  # ifconfig br0 up

the bridge works fine.
But I am still not able to make work the CPU port though.

  # udhcpc -i eth2
  Sending discover...
  [FOREVER]

The same for eth1, eth2 and br0.

I suspect the problem may be at different levels:

  - The RGMII interface is not properly configured
   -- at the CPU side, or
   -- at the switch chip side.
  - Some setup that I have not done needs to be done (in userspace).

------------
3. Questions
------------

Q: What is the correct interface that the CPU should use to talk to the outside
world? I am convinced any of the eth1, eth2, br0 interfaces can be used.

Q: Is there something else that needs to be done to get networking going on the
CPU interface?

Q: Is my device tree correct?

Q: Lets assume the CPU port is working. Is NFS boot of the CPU possible from at
least one port of the switch?

Any hints how to deal with that will be much appreciated.
BR, Michal

Re: i.MX6S/DL and QCA8334 switch using DSA driver - CPU port not working

[Andrew Lunn]

On Thu, Apr 26, 2018 at 03:37:33PM +0200, Michal Vokáč wrote:
> 
>  - Linux 4.9.84 (Freescale 4.9-1.0.x-imx branch)

Hi Michal

Please use mainline, not the freescale fork. For DSA, there is nothing
you need in the freescale fork. Once it works with mainline, you can
then figure out what needs to be done to make the fork work.

>  - Mainline DSA drivers
>  - Mainline qca8k driver
> 
> The Freescale branch does not introduce any changes to the DSA nor to the QCA8K
> drivers from mainline.

Does it have 
fbbeefdd2104 ("net: fec: Allow reception of frames bigger than 1522 bytes")

> To make the bridge work I need to enable forwarding across all the switch ports
> at setup.
> 
> --- a/drivers/net/dsa/qca8k.c
> +++ b/drivers/net/dsa/qca8k.c
> @@ -578,12 +578,12 @@ qca8k_setup(struct dsa_switch *ds)
>    		if (ds->enabled_port_mask & BIT(i))
>    			qca8k_port_set_status(priv, i, 0);
> -	/* Forward all unknown frames to CPU port for Linux processing */
> +	/* Forward all unknown frames to all pors */
>    	qca8k_write(priv, QCA8K_REG_GLOBAL_FW_CTRL1,
>    		    BIT(0) << QCA8K_GLOBAL_FW_CTRL1_IGMP_DP_S |
> -		    BIT(0) << QCA8K_GLOBAL_FW_CTRL1_BC_DP_S |
> -		    BIT(0) << QCA8K_GLOBAL_FW_CTRL1_MC_DP_S |
> -		    BIT(0) << QCA8K_GLOBAL_FW_CTRL1_UC_DP_S);
> +		    0x7f << QCA8K_GLOBAL_FW_CTRL1_BC_DP_S |
> +		    0x7f << QCA8K_GLOBAL_FW_CTRL1_MC_DP_S |
> +		    0x7f << QCA8K_GLOBAL_FW_CTRL1_UC_DP_S);
>    	/* Setup connection between CPU port & user ports */
>    	for (i = 0; i < DSA_MAX_PORTS; i++) {
> --

This is probably because you don't have a working CPU port.  If that
worked, all unknown frames would be passed to the software bridge. It
would then either flood them out all ports, or if it knows the
destination MAC address, out one specific port. The should be enough
to make the destination reply, at which point the switch learns the
MAC address, and it is no longer unknown.

So lets leave this alone for the moment.

> But I am still not able to make work the CPU port though.
> 
>  # udhcpc -i eth2
>  Sending discover...
>  [FOREVER]
> 
> The same for eth1, eth2 and br0.
> 
> I suspect the problem may be at different levels:
> 
>  - The RGMII interface is not properly configured
>   -- at the CPU side, or
>   -- at the switch chip side.
>  - Some setup that I have not done needs to be done (in userspace).

Your user space setup look O.K.

Try playing with RGMII delays. Set the phy-mode to rgmii-id.

    Andrew

Re: i.MX6S/DL and QCA8334 switch using DSA driver - CPU port not working

[Michal Vokáč]

On 26.4.2018 16:06, Andrew Lunn wrote:
> On Thu, Apr 26, 2018 at 03:37:33PM +0200, Michal Vokáč wrote:
>>
>>   - Linux 4.9.84 (Freescale 4.9-1.0.x-imx branch)
> 
> Hi Michal
> 
> Please use mainline, not the freescale fork. For DSA, there is nothing
> you need in the freescale fork. Once it works with mainline, you can
> then figure out what needs to be done to make the fork work.

Hi Andrew,
Thanks for such a quick reply!

OK, good point, I will go this way - mainline first, fork then.

>> The Freescale branch does not introduce any changes to the DSA nor to the QCA8K
>> drivers from mainline.
> 
> Does it have
> fbbeefdd2104 ("net: fec: Allow reception of frames bigger than 1522 bytes")

Yes, this one was backported to 4.9.74 and is in the freescale branch too.

>> To make the bridge work I need to enable forwarding across all the switch ports
>> at setup.
>>
>> --- a/drivers/net/dsa/qca8k.c
>> +++ b/drivers/net/dsa/qca8k.c
>> @@ -578,12 +578,12 @@ qca8k_setup(struct dsa_switch *ds)
>>     		if (ds->enabled_port_mask & BIT(i))
>>     			qca8k_port_set_status(priv, i, 0);
>> -	/* Forward all unknown frames to CPU port for Linux processing */
>> +	/* Forward all unknown frames to all pors */
>>     	qca8k_write(priv, QCA8K_REG_GLOBAL_FW_CTRL1,
>>     		    BIT(0) << QCA8K_GLOBAL_FW_CTRL1_IGMP_DP_S |
>> -		    BIT(0) << QCA8K_GLOBAL_FW_CTRL1_BC_DP_S |
>> -		    BIT(0) << QCA8K_GLOBAL_FW_CTRL1_MC_DP_S |
>> -		    BIT(0) << QCA8K_GLOBAL_FW_CTRL1_UC_DP_S);
>> +		    0x7f << QCA8K_GLOBAL_FW_CTRL1_BC_DP_S |
>> +		    0x7f << QCA8K_GLOBAL_FW_CTRL1_MC_DP_S |
>> +		    0x7f << QCA8K_GLOBAL_FW_CTRL1_UC_DP_S);
>>     	/* Setup connection between CPU port & user ports */
>>     	for (i = 0; i < DSA_MAX_PORTS; i++) {
>> --
> 
> This is probably because you don't have a working CPU port.  If that
> worked, all unknown frames would be passed to the software bridge. It
> would then either flood them out all ports, or if it knows the
> destination MAC address, out one specific port. The should be enough
> to make the destination reply, at which point the switch learns the
> MAC address, and it is no longer unknown.
> 
> So lets leave this alone for the moment.

First attempt - pure mainline 4.9.84 without my patch.
CPU port not working, bridge not working.

>> But I am still not able to make work the CPU port though.
>>
>>   # udhcpc -i eth2
>>   Sending discover...
>>   [FOREVER]
>>
>> The same for eth1, eth2 and br0.
>>
>> I suspect the problem may be at different levels:
>>
>>   - The RGMII interface is not properly configured
>>    -- at the CPU side, or
>>    -- at the switch chip side.
>>   - Some setup that I have not done needs to be done (in userspace).
> 
> Your user space setup look O.K.
> 
> Try playing with RGMII delays. Set the phy-mode to rgmii-id.

OK, I will try some combinations and also to tune the numbers in the driver.
That part is actually quite confusing to me. phy-mode can be set for the fec
and for the port. For the fec I am now using rgmii as that is what we were
using before and it worked. Though from my understanding of the ethernet
binding doc it totally make sense to use rgmii-id for the fec.
I tried that and it did not help.

Using rgmii-id for the port is not valid as the qca8k driver does not support
that mode. It only supports rgmii and sgmii. I think this is actually not
correct. When phy-mode is set to rgmii for port the qca8k driver configures
internal delays in the switch. So it behaves like rgmii-id I think.

Should not it be:

--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -474,7 +474,7 @@ qca8k_set_pad_ctrl(struct qca8k_priv *priv, int port, int mode)
  	 * PHY or MAC.
  	 */
  	switch (mode) {
-	case PHY_INTERFACE_MODE_RGMII:
+	case PHY_INTERFACE_MODE_RGMII_ID:
  	qca8k_write(priv, reg,
		    QCA8K_PORT_PAD_RGMII_EN |
		    QCA8K_PORT_PAD_RGMII_TX_DELAY(3) |

Re: i.MX6S/DL and QCA8334 switch using DSA driver - CPU port not working

[Andrew Lunn]

> Using rgmii-id for the port is not valid as the qca8k driver does not support
> that mode. It only supports rgmii and sgmii. I think this is actually not
> correct. When phy-mode is set to rgmii for port the qca8k driver configures
> internal delays in the switch. So it behaves like rgmii-id I think.
> 
> Should not it be:
> 
> --- a/drivers/net/dsa/qca8k.c
> +++ b/drivers/net/dsa/qca8k.c
> @@ -474,7 +474,7 @@ qca8k_set_pad_ctrl(struct qca8k_priv *priv, int port, int mode)
>  	 * PHY or MAC.
>  	 */
>  	switch (mode) {
> -	case PHY_INTERFACE_MODE_RGMII:
> +	case PHY_INTERFACE_MODE_RGMII_ID:
>  	qca8k_write(priv, reg,
> 		    QCA8K_PORT_PAD_RGMII_EN |
> 		    QCA8K_PORT_PAD_RGMII_TX_DELAY(3) |

We have to be careful cleaning this up. It has the potential to break
existing boards when using an old device tree blob.

	 Andrew

Re: i.MX6S/DL and QCA8334 switch using DSA driver - CPU port not working

[Michal Vokáč]

On 30.4.2018 15:20, Andrew Lunn wrote:
>> Using rgmii-id for the port is not valid as the qca8k driver does not support
>> that mode. It only supports rgmii and sgmii. I think this is actually not
>> correct. When phy-mode is set to rgmii for port the qca8k driver configures
>> internal delays in the switch. So it behaves like rgmii-id I think.
>>
>> Should not it be:
>>
>> --- a/drivers/net/dsa/qca8k.c
>> +++ b/drivers/net/dsa/qca8k.c
>> @@ -474,7 +474,7 @@ qca8k_set_pad_ctrl(struct qca8k_priv *priv, int port, int mode)
>>   	 * PHY or MAC.
>>   	 */
>>   	switch (mode) {
>> -	case PHY_INTERFACE_MODE_RGMII:
>> +	case PHY_INTERFACE_MODE_RGMII_ID:
>>   	qca8k_write(priv, reg,
>> 		    QCA8K_PORT_PAD_RGMII_EN |
>> 		    QCA8K_PORT_PAD_RGMII_TX_DELAY(3) |
> 
> We have to be careful cleaning this up. It has the potential to break
> existing boards when using an old device tree blob.

Oh, I see. Thanks for pointing this out.

Some news to the problem with the non-working CPU port.
Andrew, thank you very mych for the ideas how to debug the issue.
I tried what you suggested but have no luck. FYI Now I am doing all my tests
with linux-stable.

First of all I tried to make work my old phy driver for the switch with latest
kernel. It works on v4.1.46 but did not on v4.17-rc2 - no IP@ on eth0.
So a very same issue as I have with the DSA. Bisecting the kernel picked:

d5c3d84 ("net: phy: Avoid polling PHY with PHY_IGNORE_INTERRUPTS")

Fixed that by using PHY_POLL in my driver. I was hoping that I may have similar
issue when using DSA but it looks OK. This is with the DSA enabled:

  # dmesg | grep PHY
  [    3.452536] Generic PHY 2188000.ethernet-1:01: attached PHY driver [Generic PHY] (mii_bus:phy_addr=2188000.ethernet-1:01, irq=POLL)
  [    3.453437] Generic PHY 2188000.ethernet-1:02: attached PHY driver [Generic PHY] (mii_bus:phy_addr=2188000.ethernet-1:02, irq=POLL)
  [   20.769281] Generic PHY fixed-0:00: attached PHY driver [Generic PHY] (mii_bus:phy_addr=fixed-0:00, irq=POLL)

Anyway, now I am sure that I can use RGMII interface with mainline when I am
not using DSA and phy-mode is set to rgmii and I use QCA8K_PORT_PAD_RGMII_TX_DELAY(2)
and QCA8K_PORT_PAD_RGMII_RX_DELAY(2).

To debug the non-working CPU port with DSA I tried these kernel versions:

  - v4.8-rc6-1085-g6b93fb4 - NOT OK
    - Can not go lower than this version. qca8k driver was introduced here.
  - 4.9.84 - NOT OK
  - 4.17-rc2 - NOT OK

Some RGMII delay tunning attempts with v4.17-rc2:

phy-mode (fec)	Rx/Tx delay	result
--------------------------------------
rgmii		0/0		NOT OK
rgmii		1/1		NOT OK
rgmii		2/2		NOT OK
rgmii		3/3		NOT OK
rgmii-id	0/0		NOT OK
rgmii-id	1/1		NOT OK
rgmii-id	2/2		NOT OK
rgmii-id	3/3		NOT OK

I am out of ideas how to further debug this.
Any additional adivce will be much appreciated.

Thanks, Michal.

Re: i.MX6S/DL and QCA8334 switch using DSA driver - CPU port not working

[Andrew Lunn]

> Some RGMII delay tunning attempts with v4.17-rc2:
> 
> phy-mode (fec)	Rx/Tx delay	result
> --------------------------------------
> rgmii		0/0		NOT OK
> rgmii		1/1		NOT OK
> rgmii		2/2		NOT OK
> rgmii		3/3		NOT OK
> rgmii-id	0/0		NOT OK
> rgmii-id	1/1		NOT OK
> rgmii-id	2/2		NOT OK
> rgmii-id	3/3		NOT OK
> 
> I am out of ideas how to further debug this.
> Any additional adivce will be much appreciated.

I would suggest looking at the statistics counters.  ethtool -S. Try
it for both the slave interfaces, and the master interface. The master
interfaces statistics should have both the host counters, and the
switches counters. Do you see packets being sent but not received? Are
there errors reported?

      Andrew

Re: i.MX6S/DL and QCA8334 switch using DSA driver - CPU port not working

[Michal Vokáč]

On 4.5.2018 15:30, Andrew Lunn wrote:
>> I am out of ideas how to further debug this.
>> Any additional adivce will be much appreciated.
> 
> I would suggest looking at the statistics counters.  ethtool -S. Try
> it for both the slave interfaces, and the master interface. The master
> interfaces statistics should have both the host counters, and the
> switches counters. Do you see packets being sent but not received? Are
> there errors reported?

Thanks Andrew, now I am getting closer!
I briefly looked at the statistics some time ago but could not interpret
the numbers correctly. Now I went through the relevant source code and it
shed some light to it.

This is what I get from the numbers:

Slaves - switch counters: some packets received, none transmitted. No errors.
Slaves - DSA counters: some packets transmitted, none received. No error cntrs.

Master - switch counters: some packets transmitted, none received. No errors.
Master - host/fec counters: some packets transmitted, none received. No errors,
	 but IEEE_rx_drop counter shows frames are being dropped!

So this is most probably the issue. From the i.MX6 RM:
"Counter increments if a frame with invalid or missing SFD character is
detected and has been dropped. None of the other counters increments if
this counter increments."

At first I thought that this could be caused by the atheros header.
But the atheros header is inserted in the frame by the switch after the SA.
So it can not be the issue as the frame is discarded even before the DA and SA
is processed.

So both the CPU and the switch are trying to talk to each other but their
frames are not delivered. This still looks like RGMII configuration problem.
As it works with my old PHY driver I suspect the problem is at the switches
side somewhere in qca8k_set_pad_ctrl or qca8k_setup in the qca8k driver.

Any other ideas?

BR, Michal

Re: i.MX6S/DL and QCA8334 switch using DSA driver - CPU port not working

[Andrew Lunn]

> So both the CPU and the switch are trying to talk to each other but their
> frames are not delivered. This still looks like RGMII configuration problem.

Yes, i would say it is.

> As it works with my old PHY driver I suspect the problem is at the switches
> side somewhere in qca8k_set_pad_ctrl or qca8k_setup in the qca8k driver.
> 
> Any other ideas?

I would probably add code to dump all the qca8k registers. Compare the
values for your working setup and your non-working setup. Hopefully
they are not too different, and you can quickly get to the bits which
matter.

     Andrew

Re: i.MX6S/DL and QCA8334 switch using DSA driver - CPU port not working

[Michal Vokáč]

On 10.5.2018 16:29, Andrew Lunn wrote:
> I would probably add code to dump all the qca8k registers. Compare the
> values for your working setup and your non-working setup. Hopefully
> they are not too different, and you can quickly get to the bits which
> matter.

Perfect! Thanks to your suggestion I did that again and much more carefully.
After some tedious comparison I think I finally found the problem.

The RGMII works only if I write 0x7e to the PORT0_STATUS (0x7c) register
from setup. Then I found out that this setup is also described in
Qualcomm QCA8334 Q&A document. When I do that, everything work as expected.

Both PORT0 and PORT6 may be configured as xGMII, xMII and SerDes and their
functions may be exchanged. In all cases the port status register should be
set to 0x7X where X depends on the link speed setup.

Translated into register bits this means:
  - clear BIT(12) - disable MAC flow control auto-negotiation (set by default)
  - clear BIT(7)  - disable MAC Tx flow control in half-duplex (set by default)
  - set BIT(6)    - use full-duplex
  - set BIT(5,4)  - enable Rx/Tx flow control
  - set BIT(3)    - enable Rx MAC - this one is tricky, the bit is described as
		   R/O in datasheet but it does not work when not set.
  - set BIT(2)    - enable Tx MAC
  - set BIT(1,0)  - set speed to 1000Mb

In general the fixed-link subnode is not handled in qca8k driver. That is why
the link speed and flow control is not set properly for the CPU port.
And obviously autonegotiation can not be used here.

I wonder whether there are some users of this driver and what may be their
setup that they are not affected by that?

I would like to have confirmed that I understand it correctly and that
the problem is really in the driver not handling fixed-link.

Then I can try to prepare a patch. It will be a small challenge for me
but I would like to do the work. I will look at the other drivers where
I think this is already implemented but any advice on how this should be
done will be appreciated.

Please confirm or correct my suggestions.

Thanks, Michal

Re: i.MX6S/DL and QCA8334 switch using DSA driver - CPU port not working

[Andrew Lunn]

On Tue, May 15, 2018 at 04:25:49PM +0200, Michal Vokáč wrote:
> On 10.5.2018 16:29, Andrew Lunn wrote:
> >I would probably add code to dump all the qca8k registers. Compare the
> >values for your working setup and your non-working setup. Hopefully
> >they are not too different, and you can quickly get to the bits which
> >matter.
> 
> Perfect! Thanks to your suggestion I did that again and much more carefully.
> After some tedious comparison I think I finally found the problem.
> 
> The RGMII works only if I write 0x7e to the PORT0_STATUS (0x7c) register
> from setup. Then I found out that this setup is also described in
> Qualcomm QCA8334 Q&A document. When I do that, everything work as expected.

Great.

> 
> Both PORT0 and PORT6 may be configured as xGMII, xMII and SerDes and their
> functions may be exchanged. In all cases the port status register should be
> set to 0x7X where X depends on the link speed setup.
> 
> Translated into register bits this means:
>  - clear BIT(12) - disable MAC flow control auto-negotiation (set by default)
>  - clear BIT(7)  - disable MAC Tx flow control in half-duplex (set by default)
>  - set BIT(6)    - use full-duplex
>  - set BIT(5,4)  - enable Rx/Tx flow control
>  - set BIT(3)    - enable Rx MAC - this one is tricky, the bit is described as
> 		   R/O in datasheet but it does not work when not set.
>  - set BIT(2)    - enable Tx MAC
>  - set BIT(1,0)  - set speed to 1000Mb

The Marvell devices have something similar. What we do there is for
the cpu port is to always set the port up, full duplex, and the
fastest speed it supports. This covers 95% of boards. We have a few
boards where the SoC on the other end can only do 100Mbps, not
1G. Then we use a fixed-phy.
 
> I wonder whether there are some users of this driver and what may be their
> setup that they are not affected by that?

It could be the bootloader is setting up the CPU port? I don't really
like that.

> I would like to have confirmed that I understand it correctly and that
> the problem is really in the driver not handling fixed-link.

I would actually skip fixed-link, if you don't need it. Just hardwire
the CPU port, like the Marvell driver does:

https://elixir.bootlin.com/linux/latest/source/drivers/net/dsa/mv88e6xxx/chip.c#L1780

I would do it here:

https://elixir.bootlin.com/linux/latest/source/drivers/net/dsa/qca8k.c#L518

	Andrew

Re: i.MX6S/DL and QCA8334 switch using DSA driver - CPU port not working

[Florian Fainelli]

On 05/15/2018 09:08 AM, Andrew Lunn wrote:
> On Tue, May 15, 2018 at 04:25:49PM +0200, Michal Vokáč wrote:
>> On 10.5.2018 16:29, Andrew Lunn wrote:
>>> I would probably add code to dump all the qca8k registers. Compare the
>>> values for your working setup and your non-working setup. Hopefully
>>> they are not too different, and you can quickly get to the bits which
>>> matter.
>>
>> Perfect! Thanks to your suggestion I did that again and much more carefully.
>> After some tedious comparison I think I finally found the problem.
>>
>> The RGMII works only if I write 0x7e to the PORT0_STATUS (0x7c) register
>> from setup. Then I found out that this setup is also described in
>> Qualcomm QCA8334 Q&A document. When I do that, everything work as expected.
> 
> Great.
> 
>>
>> Both PORT0 and PORT6 may be configured as xGMII, xMII and SerDes and their
>> functions may be exchanged. In all cases the port status register should be
>> set to 0x7X where X depends on the link speed setup.
>>
>> Translated into register bits this means:
>>  - clear BIT(12) - disable MAC flow control auto-negotiation (set by default)
>>  - clear BIT(7)  - disable MAC Tx flow control in half-duplex (set by default)
>>  - set BIT(6)    - use full-duplex
>>  - set BIT(5,4)  - enable Rx/Tx flow control
>>  - set BIT(3)    - enable Rx MAC - this one is tricky, the bit is described as
>> 		   R/O in datasheet but it does not work when not set.
>>  - set BIT(2)    - enable Tx MAC
>>  - set BIT(1,0)  - set speed to 1000Mb
> 
> The Marvell devices have something similar. What we do there is for
> the cpu port is to always set the port up, full duplex, and the
> fastest speed it supports. This covers 95% of boards. We have a few
> boards where the SoC on the other end can only do 100Mbps, not
> 1G. Then we use a fixed-phy.
>  
>> I wonder whether there are some users of this driver and what may be their
>> setup that they are not affected by that?
> 
> It could be the bootloader is setting up the CPU port? I don't really
> like that.

This is unfortunately not uncommon...

> 
>> I would like to have confirmed that I understand it correctly and that
>> the problem is really in the driver not handling fixed-link.
> 
> I would actually skip fixed-link, if you don't need it. Just hardwire
> the CPU port, like the Marvell driver does:
> 
> https://elixir.bootlin.com/linux/latest/source/drivers/net/dsa/mv88e6xxx/chip.c#L1780
> 
> I would do it here:
> 
> https://elixir.bootlin.com/linux/latest/source/drivers/net/dsa/qca8k.c#L518

Agreed with Andrew here, though if you can implement fixed-link, this
should be more future proof.

As far as people using this driver, John submitted it, but we have not
see many fixes/enhancements, so I am not clear who is actually using it
these days... glad you are though!
-- 
Florian

Re: i.MX6S/DL and QCA8334 switch using DSA driver - CPU port not working

[Michal Vokáč]

On 15.5.2018 18:17, Florian Fainelli wrote:
>>> I would like to have confirmed that I understand it correctly and that
>>> the problem is really in the driver not handling fixed-link.
>>
>> I would actually skip fixed-link, if you don't need it. Just hardwire
>> the CPU port, like the Marvell driver does:
>>
>> https://elixir.bootlin.com/linux/latest/source/drivers/net/dsa/mv88e6xxx/chip.c#L1780
>>
>> I would do it here:
>>
>> https://elixir.bootlin.com/linux/latest/source/drivers/net/dsa/qca8k.c#L518

Thanks for the references, I will look there.
> 
> Agreed with Andrew here, though if you can implement fixed-link, this
> should be more future proof.

I do not actually need fixed-link but I agree with Florian that it is much
better to have that option. I see it a as way to override defaults that
do not work for the user. That is what I was hoping for but it did not work.

So as the fixed-link subnode is optional we still should force some sensible
defaults if it is not used. Same as Marvell drives does. Can I say that we
agreed that the current CPU port setting is not correct and the fastest link
speed and duplex supported by the chip should be set as a default?

> As far as people using this driver, John submitted it, but we have not
> see many fixes/enhancements, so I am not clear who is actually using it
> these days... glad you are though!

Thanks.

To the compatibility of the driver with the QCA8334 that I use. I am not sure
what should be the correct way to deal with that. For example Marvell binding
uses only two compatible strings for a large range of chips in one family.
While Broadcom has one compatible string for each chip. As I mentioned earlier
in this thread the QCA8334 switch has four ports while the already supported
QCA8337 has seven ports. That is the only difference. Register address space
is the same.

Should I:
  - add a new compatible string "qualcomm,qca8334" and update the docs,
  - or just update the documentation and describe the compatibility?

As I need to update the docs I understand it should go as a separate patch.
Is it enough to CC devicetree list with the whole series? Any other
list/maintainer?

Thank you for your time,
Michal

Re: i.MX6S/DL and QCA8334 switch using DSA driver - CPU port not working

[Andrew Lunn]

> So as the fixed-link subnode is optional we still should force some sensible
> defaults if it is not used. Same as Marvell drives does. Can I say that we
> agreed that the current CPU port setting is not correct and the fastest link
> speed and duplex supported by the chip should be set as a default?

That is a sensible default.

> >As far as people using this driver, John submitted it, but we have not
> >see many fixes/enhancements, so I am not clear who is actually using it
> >these days... glad you are though!
> 
> Thanks.
> 
> To the compatibility of the driver with the QCA8334 that I use. I am not sure
> what should be the correct way to deal with that. For example Marvell binding
> uses only two compatible strings for a large range of chips in one family.
> While Broadcom has one compatible string for each chip.

With the Marvell devices, there is an ID register which tells you the
model. The compatible string tells us the information we need in order
to find that ID register. Once we know the ID, we have all the
information we need, so don't need a more specific compatible string.

> As I mentioned earlier
> in this thread the QCA8334 switch has four ports while the already supported
> QCA8337 has seven ports. That is the only difference. Register address space
> is the same.

Can you identify the exact model using some ID register? If yes, than
the existing compatible string is sufficient. If no, you need an
additional compatible string.

	Andrew

Re: i.MX6S/DL and QCA8334 switch using DSA driver - CPU port not working

[Michal Vokáč]

On 16.5.2018 15:17, Andrew Lunn wrote:
>> So as the fixed-link subnode is optional we still should force some sensible
>> defaults if it is not used. Same as Marvell drives does. Can I say that we
>> agreed that the current CPU port setting is not correct and the fastest link
>> speed and duplex supported by the chip should be set as a default?
> 
> That is a sensible default.

OK.

>>> As far as people using this driver, John submitted it, but we have not
>>> see many fixes/enhancements, so I am not clear who is actually using it
>>> these days... glad you are though!
>>
>> Thanks.
>>
>> To the compatibility of the driver with the QCA8334 that I use. I am not sure
>> what should be the correct way to deal with that. For example Marvell binding
>> uses only two compatible strings for a large range of chips in one family.
>> While Broadcom has one compatible string for each chip.
> 
> With the Marvell devices, there is an ID register which tells you the
> model. The compatible string tells us the information we need in order
> to find that ID register. Once we know the ID, we have all the
> information we need, so don't need a more specific compatible string.
> 
>> As I mentioned earlier
>> in this thread the QCA8334 switch has four ports while the already supported
>> QCA8337 has seven ports. That is the only difference. Register address space
>> is the same.
> 
> Can you identify the exact model using some ID register? If yes, than
> the existing compatible string is sufficient. If no, you need an
> additional compatible string.

Nope. Device ID is the same for the whole family so we need a new compatible.
--	
Michal