All of lore.kernel.org
 help / color / mirror / Atom feed
* DSA: external phy address and port number of switch conflicts
@ 2018-12-14  0:12 John Rama
  2018-12-14  5:08 ` Florian Fainelli
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: John Rama @ 2018-12-14  0:12 UTC (permalink / raw)
  To: netdev

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>;

        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.

- Question
    Is this system configuration is not supported by DSA ?
    Or am I misunderstand something ?
    Any insights are really appreciated.

John

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: DSA: external phy address and port number of switch conflicts
  2018-12-14  0:12 DSA: external phy address and port number of switch conflicts John Rama
@ 2018-12-14  5:08 ` Florian Fainelli
  2018-12-17 22:55   ` John Rama
  2018-12-14  8:38 ` Andrew Lunn
  2018-12-14 16:24 ` Vivien Didelot
  2 siblings, 1 reply; 9+ messages in thread
From: Florian Fainelli @ 2018-12-14  5:08 UTC (permalink / raw)
  To: John Rama, netdev, Andrew Lunn, Vivien Didelot

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
> 


-- 
Florian

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: DSA: external phy address and port number of switch conflicts
  2018-12-14  0:12 DSA: external phy address and port number of switch conflicts John Rama
  2018-12-14  5:08 ` Florian Fainelli
@ 2018-12-14  8:38 ` Andrew Lunn
  2018-12-14 16:24 ` Vivien Didelot
  2 siblings, 0 replies; 9+ messages in thread
From: Andrew Lunn @ 2018-12-14  8:38 UTC (permalink / raw)
  To: John Rama; +Cc: netdev

On Thu, Dec 13, 2018 at 07:12:05PM -0500, John Rama wrote:
> 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.

88e5050 is a new switch to me. So i don't know anything about its
register layout.

> 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                        
>     I'm using a little bit old kernel,4.1.15.

Since you have the switch in single address mode, i don't see why this
should not work. But as Florian said, please update to a new kernel,
and swap to using the new binding with the properties inside the IMX
MDIO bus.

    Andrew

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: DSA: external phy address and port number of switch conflicts
  2018-12-14  0:12 DSA: external phy address and port number of switch conflicts John Rama
  2018-12-14  5:08 ` Florian Fainelli
  2018-12-14  8:38 ` Andrew Lunn
@ 2018-12-14 16:24 ` Vivien Didelot
  2018-12-14 16:31   ` John Rama
  2 siblings, 1 reply; 9+ messages in thread
From: Vivien Didelot @ 2018-12-14 16:24 UTC (permalink / raw)
  To: John Rama; +Cc: netdev

Hi John,

On Thu, 13 Dec 2018 19:12:05 -0500, John Rama <john.rama01@gmail.com> wrote:

> 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 can see documentation for 88E6050 and 88Q5050, but no 88E5050. Are
you sure about the product description?

Thanks,
Vivien

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: DSA: external phy address and port number of switch conflicts
  2018-12-14 16:24 ` Vivien Didelot
@ 2018-12-14 16:31   ` John Rama
  0 siblings, 0 replies; 9+ messages in thread
From: John Rama @ 2018-12-14 16:31 UTC (permalink / raw)
  To: Vivien Didelot; +Cc: netdev

Ops, sorry, Viven. 

It should be 88Q5050.

John.

On 2018/12/14 11:24, Vivien Didelot wrote:
> Hi John,
> 
> On Thu, 13 Dec 2018 19:12:05 -0500, John Rama <john.rama01@gmail.com> wrote:
> 
>> 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 can see documentation for 88E6050 and 88Q5050, but no 88E5050. Are
> you sure about the product description?
> 
> Thanks,
> Vivien
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: DSA: external phy address and port number of switch conflicts
  2018-12-14  5:08 ` Florian Fainelli
@ 2018-12-17 22:55   ` John Rama
  2018-12-17 23:03     ` Florian Fainelli
  0 siblings, 1 reply; 9+ messages in thread
From: John Rama @ 2018-12-17 22:55 UTC (permalink / raw)
  To: Florian Fainelli, netdev, Andrew Lunn, Vivien Didelot

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
>>
> 
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: DSA: external phy address and port number of switch conflicts
  2018-12-17 22:55   ` John Rama
@ 2018-12-17 23:03     ` Florian Fainelli
  2018-12-18 18:55       ` John Rama
  0 siblings, 1 reply; 9+ messages in thread
From: Florian Fainelli @ 2018-12-17 23:03 UTC (permalink / raw)
  To: John Rama, netdev, Andrew Lunn, Vivien Didelot

On 12/17/18 2:55 PM, John Rama wrote:
> 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
>     }
> }

That was later fixed with this commit:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=399ba77a94e1ee01f747b168c429a121164ac962

can you try to backport that change and see if that helps?

> 
>>
>> 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
>>>
>>
>>
> 


-- 
Florian

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: DSA: external phy address and port number of switch conflicts
  2018-12-17 23:03     ` Florian Fainelli
@ 2018-12-18 18:55       ` John Rama
  0 siblings, 0 replies; 9+ messages in thread
From: John Rama @ 2018-12-18 18:55 UTC (permalink / raw)
  To: Florian Fainelli, netdev, Andrew Lunn, Vivien Didelot

Hi Florian, 

On 2018/12/17 18:03, Florian Fainelli wrote:
> On 12/17/18 2:55 PM, John Rama wrote:
>> 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
>>     }
>> }
> 
> That was later fixed with this commit:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=399ba77a94e1ee01f747b168c429a121164ac962
> 
> can you try to backport that change and see if that helps?
> 

Yes, that's the one !!
I confirmed that the problem is solved by backporting this patch to both kernel 4.9.0 and 4.1.15.

Thanks a lot.

John

^ permalink raw reply	[flat|nested] 9+ messages in thread

* DSA: external phy address and port number of switch conflicts
@ 2018-12-13 23:51 John Rama
  0 siblings, 0 replies; 9+ messages in thread
From: John Rama @ 2018-12-13 23:51 UTC (permalink / raw)
  To: netdev; +Cc: john.rama01

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 havea problem and 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>;

        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.

- Question
    Is this system configuration is not supported by DSA ?
    Or am I misunderstand something ?
    Any insights are really appreciated.

John

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-12-18 18:55 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-14  0:12 DSA: external phy address and port number of switch conflicts John Rama
2018-12-14  5:08 ` Florian Fainelli
2018-12-17 22:55   ` John Rama
2018-12-17 23:03     ` Florian Fainelli
2018-12-18 18:55       ` John Rama
2018-12-14  8:38 ` Andrew Lunn
2018-12-14 16:24 ` Vivien Didelot
2018-12-14 16:31   ` John Rama
  -- strict thread matches above, loose matches on Subject: below --
2018-12-13 23:51 John Rama

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.