Subject: Re: DSA: external phy address and port number of switch conflicts

From: John Rama <[email-masked]>
Date: Mon, 17 Dec 2018 17:55:10 -0500

Hi, Andrew, Florian and Viven

thank you your feedback !!

>> I would recommend checking a newer kernel anyway which would have
>> support for the mv88e6xxx internal MDIO bus as it might allow you to
>> solve that specific problem. A kernel that contains
>> a3c53be55c955b7150cda17874c3fcb4eeb97a89 ("net: dsa: mv88e6xxx: Support
>> multiple MDIO busses") might work better and actually help here, though
>> I don't know enough about that specific switch model, Andrew or Vivien
>> would.
>> 
>>>

I tried with kernel 4.9.0, and confirmed that same problem happens.
(I wanted to try more latest one, but newer version is using phylink,
 which I need couple of time to understand how it works)
I think this is not the driver side issue, maybe dsa side (but not sure for newer kernel)
or I misunderstanding some concept.
 
> Since you have the switch in single address mode, i don't see why this
> should not work.

Why conflicts happens is because of the code at slave.c (@kernel 4.9.0)

/* pseudo code */
dsa_slave_phy_setup() {                                     
  phy_dn = of_parse_phandle(port_dn, "phy-handle", 0);      
    if (phy_dn) {                                             
	phy_id = of_mdio_parse_addr(&slave_dev->dev, phy_dn); 
	dsa_slave_phy_connect(p, slave_dev, phy_id); <= register with phy_id
    }
    if (!p->phy) {
	dsa_slave_phy_connect(p, slave_dev, p->port); <= register with port number
    }
}

> 
> 88e5050 is a new switch to me. So i don't know anything about its
> register layout.
Marvel 88Q5050 has similar architecture with mv88e6xxx, 
Use two MDIO mapped register(DATA, CMD) to get access to the PHY status, control register, etc..
But, I do not think this is driver side issue, as I stated above.

BTW, I can work-around this problem at 88Q5050 driver side by using different port number for port1,
then do something conversion, to map the port to the correct internal register.
But it will lack the generality(it just only for our custom board).

I want to clarify if the use case using same number for port and external phy_id is
expected to work or our of scope for DSA. Have we ever validated this use case before ?
If it's expected work, I want to know how it will be avoided for conflicting.

Thanks !!

John

On 2018/12/14 0:08, Florian Fainelli wrote:
> Hello,
> 
> Le 12/13/18 à 4:12 PM, John Rama a écrit :
>> Dear DSA experts
>>
>> My name is John.
>>
>> I'm working for our custom board which has the Marvel 88e5050 ethernet switch
>> as shown below, and trying to make the system works using DSA subsystem.
>>
>> I found one problem and have a question.
>>
>> - System
>>     - i.MX6 and Marvel 88e5050 Switch(PHY addr is 0x10)
>>     - port1-5 has integrated PHY
>>      - port7 is connected to Marvel 88e1510 PHY(PHY addr is 0x1)
>>
>>                            0x10
>>     +------------+         +-----------------+
>>     |            |         | Marvel 88e5050  |
>>     |            | RGMII   |           port1 |
>>     |   IMX6     +---------+ port6           |
>>     |            |         |           port2 |
>>     |            | MDIO    |                 |
>>     |            +-+-------+           port3 |
>>     |            | |       |                 |
>>     +------------+ |       |           port4 |
>>                    |       |                 |
>>                  +-+-+     |           port5 |
>>                  |   |SGMII|                 |
>>                  |PHY|-----| port7           |
>>                  |   |     |                 |
>>                  +---+     +-----------------+
>>               Marvel 88e1510
>>                    0x1                        
>>
>> - Device Tree setting
>>     In this configuration, our dts is as followings.
>>
>>     dsa@0 {
>>       compatible = "marvell,dsa";
>>         #address-cells = <2>;
>>         #size-cells = <0>;
>>
>>         dsa,ethernet = <&fec>;
>>         dsa,mii-bus = <&mdio>;
> 
> You are using the old binding here, you might want to switch to the new
> DSA binding which would be nearly identical, except that it would be
> placing the switch Device Tree node as a child node of the MDIO bus it
> is connected to.
> 
>>
>>         switch@0 {
>>             #address-cells = <1>;
>>             #size-cells = <0>;
>>             reg = <0x10 0>;    /* MDIO address 16, switch 0 in tree */
>>
>>             port@1 {
>>                 reg = <1>;
>>                 label = "port1";
>>             };
>>
>>             port@2 {
>>                 reg = <2>;
>>                 label = "port2";
>>             };
>>
>>             port@3 {
>>                 reg = <3>;
>>                 label = "port3";
>>             };
>>
>>             port@4 {
>>                 reg = <4>;
>>                 label = "port4";
>>             };
>>
>>             port@5 {
>>                 reg = <5>;
>>                 label = "port5";
>>             };
>>
>>             port@6 {
>>                 reg = <6>;
>>                 label = "cpu";
>>             };
>>
>>             port@7 {
>>                 reg = <7>;
>>                 label = "port7";
>>                 phy-handle = <&ethphy1>;
>>                 phy-mode = "sgmii";
>>             };                                       
>>         };
>>      };
>>
>>     mdio: mdio {
>>         #address-cells = <1>;
>>         #size-cells = <0>;
>>
>>         ethphy1: 88e1510{
>>             reg = <1>;
>>             phy-mode = "sgmii";
>>         };
>>
>>     };
>>                                 
>> - Problem
>>     dsa_slave_phy_connect() for port 7 results in attaching Generic PHY,
>>     as shown in the following log, which is unexpected result.
>>     dsa dsa@0 port7 (uninitialized): attached PHY at address 1 [Generic PHY]
>>
>>     This is because port7 PHY addr (reg=<1>) and the port1 port number (reg=<1)> is conflicting.
>>     If I comment out the port1 setting from DTS, I can get following log,
>>     everything working fine and
>>
>>     dsa dsa@0 port7 (uninitialized): attached PHY at address 1 [Marvel 88E1510]
>>
>>     I'm using a little bit old kernel,4.1.15.
>>     So I checked the latest code of linux-next, but it seems there is similar problem.
> 
> I would recommend checking a newer kernel anyway which would have
> support for the mv88e6xxx internal MDIO bus as it might allow you to
> solve that specific problem. A kernel that contains
> a3c53be55c955b7150cda17874c3fcb4eeb97a89 ("net: dsa: mv88e6xxx: Support
> multiple MDIO busses") might work better and actually help here, though
> I don't know enough about that specific switch model, Andrew or Vivien
> would.
> 
>>
>> - Question
>>     Is this system configuration is not supported by DSA ?
> 
> This does not appear to be a DSA specific problem, this is more about
> possible MDIO bus address conflicts. I am not sure how well this is
> going to work; but it might be possible to create a virtual mdio-mux
> driver which is pinmuxing the MDIO pins on the imx6 chip when there is a
> possible MDIO bus address conflict, such that you can access each
> address without conflicts. This may not be working given that the switch
> driver might be doing concurrent accesses to the bus while the external
> PHY is being polled...
> 
>>     Or am I misunderstand something ?
>>     Any insights are really appreciated.
>>
>> John
>>
> 
>