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 = <ðphy1>;
>>> 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
