Skip to contentSkip to conversation

DSA: external phy address and port number of switch conflicts

DISCUSSION John Rama 4 participants 8 messages Dec 14, 2018 – Dec 18, 2018

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

John Rama

author
#0 lore raw

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

#1 lore raw

Hello,

Le 12/13/18 à 4:12 PM, John Rama a écrit :

Quoted 34 lines

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

Quoted 9 lines
    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.

Quoted 61 lines
        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]

Quoted 8 lines

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.

Quoted 3 lines

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

Quoted 5 lines

Or am I misunderstand something ?
Any insights are really appreciated.

John

Signature · 1 line
Florian
author
#2 lore raw

Hi, Andrew, Florian and Viven

thank you your feedback !!

Quoted 9 lines
Quoted 9 lines

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.

Quoted 1 line

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.

Quoted 2 lines

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
    }
}
Quoted 3 lines

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

Quoted 150 lines · Florian Fainelli

Hello,

Quoted 34 lines · John Rama

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

Quoted 9 lines
    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.

Quoted 61 lines
        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]

Quoted 8 lines

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.

Quoted 3 lines

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

Quoted 5 lines

Or am I misunderstand something ?
Any insights are really appreciated.

John

#3 lore raw
Quoted 27 lines · John Rama

Hi, Andrew, Florian and Viven

thank you your feedback !!

Quoted 9 lines
Quoted 9 lines

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.

Quoted 1 line

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.

Quoted 2 lines

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() {

Quoted 9 lines
  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?

Quoted 172 lines
Quoted 4 lines

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

Quoted 150 lines · Florian Fainelli

Hello,

Quoted 34 lines · John Rama

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

Quoted 9 lines
    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.

Quoted 61 lines
        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]

Quoted 8 lines

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.

Quoted 3 lines

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

Quoted 5 lines

Or am I misunderstand something ?
Any insights are really appreciated.

John

Signature · 1 line
Florian
author
#4 lore raw

Hi Florian,

Quoted 38 lines · Florian Fainelli
Quoted 27 lines · John Rama

Hi, Andrew, Florian and Viven

thank you your feedback !!

Quoted 9 lines
Quoted 9 lines

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.

Quoted 1 line

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.

Quoted 2 lines

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() {

Quoted 9 lines
  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
    }
}
Quoted 6 lines

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

#5 lore raw
In reply to John Rama
Quoted 6 lines · John Rama

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.

Quoted 18 lines

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

Quoted 8 lines

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

#6 lore raw
In reply to John Rama

Hi John,

Quoted 2 lines · John Rama

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

author
#7 lore raw

Ops, sorry, Viven.

It should be 88Q5050.

John.

Quoted 13 lines · Vivien Didelot

Hi John,

Quoted 2 lines · John Rama

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