net: dsa: mv88e6xxx: error parsing ethernet node from dts

net: dsa: mv88e6xxx: error parsing ethernet node from dts

[Jürgen Lambrecht]

Hi,

I extended the drivers/net/dsa/mv88e6xxx to add a new switch to the 6250 family, the 6071, see below. I will properly submit a patch once the driver is up and running.
I branched mainline on 4d856f72c10e Linux 5.3.

The problem is that I do not succeed to get the network interfaces specified in DTS. Here the interesting part from my dts, copied from vf610-zii-ssmb-spu3.dts, also imx6qdl-zii-rdu2.dtsi is almost the same. My dts includes imx6ul.dtsi.

&fec1 {
    pinctrl-names = "default";
    pinctrl-0 = <&pinctrl_enet1>;
    phy-mode = "rmii";
    status = "okay";

    fixed-link {
        speed = <100>;
        full-duplex;
    };

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

        switch: switch@0 {
            compatible = "marvell,mv88e6250";
            reg = <0>;
            reset-gpios = <&gpio4 17 GPIO_ACTIVE_LOW>;

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

                port@0 {
                    reg = <0>;
                    label = "eth0";
                };

                port@1 {
                    reg = <1>;
                    label = "x1";
                };

                port@2 {
                    reg = <2>;
                    label = "x2";
                };

                port@5 {
                    reg = <5>;
                    label = "cpu";
                    ethernet = <&fec1>;  //<-- here error

                    fixed-link {
                        speed = <100>;
                        full-duplex;
                    };
                };
            };
        };
    };
};

Here parts of dmesg (no error reported):

[    1.992342] libphy: Fixed MDIO Bus: probed
[    2.009532] pps pps0: new PPS source ptp0
[    2.014387] libphy: fec_enet_mii_bus: probed
[    2.017159] mv88e6085 2188000.ethernet-1:00: switch 0x710 detected: Marvell 88E6071, revision 5
[    2.125616] libphy: mv88e6xxx SMI: probed
[    2.134450] fec 2188000.ethernet eth0: registered PHC device 0
...
[   11.366359] Generic PHY fixed-0:00: attached PHY driver [Generic PHY] (mii_bus:phy_addr=fixed-0:00, irq=POLL)
[   11.366722] fec 2188000.ethernet eth0: Link is Up - 100Mbps/Full - flow control off

When I enable debugging in the source code, I see that mv88e6xxx_probe() fails, because *'of_find_net_device_by_node(ethernet);'* fails. But why?, &fec1 does exist, although I find it strange to specify &fec1 inside &fec1 itself. But according to documentation and other examples, that is the way it is done.

Here some more details:

- The detect, and register read/write is OK: with #define _DEBUG I see a lot of successful register read/write (to global and phy registers).

- When I do not add the first fixed-link in DTS, the kernel hangs, or oopses, or runs but with warnings (..._sendmsg, probably trying to send dhcp requests)

- mv88e6xxx_probe() gives the error above because mv88e6xxx_register_switch(chip) fails, with 'goto_mdio'.

-------------------------------------------------------------------------------
diff --git a/drivers/net/dsa/mv88e6xxx/port.h b/drivers/net/dsa/mv88e6xxx/port.h

index 8d5a6cd6fb19..9c0b84fb69b0 100644
--- a/drivers/net/dsa/mv88e6xxx/port.h
+++ b/drivers/net/dsa/mv88e6xxx/port.h
@@ -100,6 +100,9 @@
 /* Offset 0x03: Switch Identifier Register */
 #define MV88E6XXX_PORT_SWITCH_ID               0x03
 #define MV88E6XXX_PORT_SWITCH_ID_PROD_MASK     0xfff0
+#define MV88E6XXX_PORT_SWITCH_ID_PROD_6020     0x0200
+#define MV88E6XXX_PORT_SWITCH_ID_PROD_6070     0x0700
+#define MV88E6XXX_PORT_SWITCH_ID_PROD_6071     0x0710
 #define MV88E6XXX_PORT_SWITCH_ID_PROD_6085     0x04a0
 #define MV88E6XXX_PORT_SWITCH_ID_PROD_6095     0x0950
 #define MV88E6XXX_PORT_SWITCH_ID_PROD_6097     0x0990
@@ -117,6 +120,7 @@
 #define MV88E6XXX_PORT_SWITCH_ID_PROD_6190     0x1900
 #define MV88E6XXX_PORT_SWITCH_ID_PROD_6191     0x1910
 #define MV88E6XXX_PORT_SWITCH_ID_PROD_6185     0x1a70
+#define MV88E6XXX_PORT_SWITCH_ID_PROD_6220     0x2200
 #define MV88E6XXX_PORT_SWITCH_ID_PROD_6240     0x2400
 #define MV88E6XXX_PORT_SWITCH_ID_PROD_6250     0x2500
 #define MV88E6XXX_PORT_SWITCH_ID_PROD_6290     0x2900

diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index 4646e46d47f2..207b777da98e 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -41,6 +41,7 @@ enum mv88e6xxx_frame_mode {
 
 /* List of supported models */
 enum mv88e6xxx_model {
+       MV88E6071,
        MV88E6085,
        MV88E6095,
        MV88E6097,
@@ -77,7 +78,7 @@ enum mv88e6xxx_family {
        MV88E6XXX_FAMILY_6097,  /* 6046 6085 6096 6097 */
        MV88E6XXX_FAMILY_6165,  /* 6123 6161 6165 */
        MV88E6XXX_FAMILY_6185,  /* 6108 6121 6122 6131 6152 6155 6182 6185 */
-       MV88E6XXX_FAMILY_6250,  /* 6250 */
+       MV88E6XXX_FAMILY_6250,  /* 6020 6070 6071 6220 6250 */
        MV88E6XXX_FAMILY_6320,  /* 6320 6321 */
        MV88E6XXX_FAMILY_6341,  /* 6141 6341 */
        MV88E6XXX_FAMILY_6351,  /* 6171 6175 6350 6351 */

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index d0a97eb73a37..c697ebd6e336 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -3881,13 +3881,34 @@ static const struct mv88e6xxx_ops mv88e6390x_ops = {
 };
 
 static const struct mv88e6xxx_info mv88e6xxx_table[] = {
+       [MV88E6071] = {
+               .prod_num = MV88E6XXX_PORT_SWITCH_ID_PROD_6071,
+               .family = MV88E6XXX_FAMILY_6250,
+               .name = "Marvell 88E6071",
+               .num_databases = 64,
+               .num_ports = 7,
+               .num_internal_phys = 5,
+               .max_vid = 4095,
+               .port_base_addr = 0x08,
+               .phy_base_addr = 0x00,
+               .global1_addr = 0x0f,
+               .global2_addr = 0x07,
+               .age_time_coeff = 15000,
+               .g1_irqs = 9,
+               .g2_irqs = 10,
+               .atu_move_port_mask = 0xf,
+               .dual_chip = true,
+               .tag_protocol = DSA_TAG_PROTO_DSA,
+               .ops = &mv88e6250_ops,
+       },
+
        [MV88E6085] = {
                .prod_num = MV88E6XXX_PORT_SWITCH_ID_PROD_6085,
                .family = MV88E6XXX_FAMILY_6097,
                .name = "Marvell 88E6085",
-               .num_databases = 4096,
+               .num_databases = 64,
                .num_ports = 10,
-               .num_internal_phys = 5,
+               .num_internal_phys = 8,
                .max_vid = 4095,
                .port_base_addr = 0x10,
                .phy_base_addr = 0x0,
@@ -3909,7 +3930,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
                .name = "Marvell 88E6095/88E6095F",
                .num_databases = 256,
                .num_ports = 11,
-               .num_internal_phys = 0,
+               .num_internal_phys = 8,
                .max_vid = 4095,
                .port_base_addr = 0x10,
                .phy_base_addr = 0x0,

-- 

Kind Regards,

Jürgen Lambrecht
R&D Associate

Re: net: dsa: mv88e6xxx: error parsing ethernet node from dts

[Andrew Lunn]

> Here parts of dmesg (no error reported):
> 
> [    1.992342] libphy: Fixed MDIO Bus: probed
> [    2.009532] pps pps0: new PPS source ptp0
> [    2.014387] libphy: fec_enet_mii_bus: probed
> [    2.017159] mv88e6085 2188000.ethernet-1:00: switch 0x710 detected: Marvell 88E6071, revision 5
> [    2.125616] libphy: mv88e6xxx SMI: probed
> [    2.134450] fec 2188000.ethernet eth0: registered PHC device 0
> ...
> [   11.366359] Generic PHY fixed-0:00: attached PHY driver [Generic PHY] (mii_bus:phy_addr=fixed-0:00, irq=POLL)
> [   11.366722] fec 2188000.ethernet eth0: Link is Up - 100Mbps/Full - flow control off
> 
> When I enable debugging in the source code, I see that mv88e6xxx_probe() fails, because *'of_find_net_device_by_node(ethernet);'* fails. But why?,

That always happens the first time. There is a chicken/egg
problem. The MDIO bus is registered by the FEC driver, the switch is
probed, and the DSA core looks for the ethernet interface. But the FEC
driver has not yet registered the interface, it is still busy
registering the MDIO bus. So you get an EPRODE_DEFFER from the switch
probe. The FEC then completes its probe, registering the
interface. Sometime later Linux retries the switch probe, and this
time it works.

What you are seeing here is the first attempt. There should be a
second go later in the log.

       Andrew

Re: net: dsa: mv88e6xxx: error parsing ethernet node from dts

[Jürgen Lambrecht]

On 12/4/19 4:38 PM, Andrew Lunn wrote:
>> Here parts of dmesg (no error reported):
>>
>> [    1.992342] libphy: Fixed MDIO Bus: probed
>> [    2.009532] pps pps0: new PPS source ptp0
>> [    2.014387] libphy: fec_enet_mii_bus: probed
>> [    2.017159] mv88e6085 2188000.ethernet-1:00: switch 0x710 detected: Marvell 88E6071, revision 5
>> [    2.125616] libphy: mv88e6xxx SMI: probed
>> [    2.134450] fec 2188000.ethernet eth0: registered PHC device 0
>> ...
>> [   11.366359] Generic PHY fixed-0:00: attached PHY driver [Generic PHY] (mii_bus:phy_addr=fixed-0:00, irq=POLL)
>> [   11.366722] fec 2188000.ethernet eth0: Link is Up - 100Mbps/Full - flow control off
>>
>> When I enable debugging in the source code, I see that mv88e6xxx_probe() fails, because *'of_find_net_device_by_node(ethernet);'* fails. But why?,
> That always happens the first time. There is a chicken/egg
> problem. The MDIO bus is registered by the FEC driver, the switch is
> probed, and the DSA core looks for the ethernet interface. But the FEC
> driver has not yet registered the interface, it is still busy
> registering the MDIO bus. So you get an EPRODE_DEFFER from the switch
> probe. The FEC then completes its probe, registering the
> interface. Sometime later Linux retries the switch probe, and this
> time it works.
>
> What you are seeing here is the first attempt. There should be a
> second go later in the log.
>
>        Andrew

Indeed, but that also fails because this second time, reading the switch ID (macreg 3 at addr 8) fails, it returns 0x0000!??

Last register read/write was 800ms before, disabling interrupts.

Shortly after the first ID read (that succeeded 1 second before), all MAC ports have been disabled (macreg 4, PortState from forwarding to disabled). But that should have no influence on reading the ID. FYI, The switch is configured in CPU attached mode.

Any ideas?

Tomorrow I will try to run a user-space program to read/write the registers.

Thanks a lot for clarifying this,

Jürgen

Re: net: dsa: mv88e6xxx: error parsing ethernet node from dts

[Andrew Lunn]

On Wed, Dec 04, 2019 at 05:20:04PM +0100, Jürgen Lambrecht wrote:
> On 12/4/19 4:38 PM, Andrew Lunn wrote:
> >> Here parts of dmesg (no error reported):
> >>
> >> [    1.992342] libphy: Fixed MDIO Bus: probed
> >> [    2.009532] pps pps0: new PPS source ptp0
> >> [    2.014387] libphy: fec_enet_mii_bus: probed
> >> [    2.017159] mv88e6085 2188000.ethernet-1:00: switch 0x710 detected: Marvell 88E6071, revision 5
> >> [    2.125616] libphy: mv88e6xxx SMI: probed
> >> [    2.134450] fec 2188000.ethernet eth0: registered PHC device 0
> >> ...
> >> [   11.366359] Generic PHY fixed-0:00: attached PHY driver [Generic PHY] (mii_bus:phy_addr=fixed-0:00, irq=POLL)
> >> [   11.366722] fec 2188000.ethernet eth0: Link is Up - 100Mbps/Full - flow control off
> >>
> >> When I enable debugging in the source code, I see that mv88e6xxx_probe() fails, because *'of_find_net_device_by_node(ethernet);'* fails. But why?,
> > That always happens the first time. There is a chicken/egg
> > problem. The MDIO bus is registered by the FEC driver, the switch is
> > probed, and the DSA core looks for the ethernet interface. But the FEC
> > driver has not yet registered the interface, it is still busy
> > registering the MDIO bus. So you get an EPRODE_DEFFER from the switch
> > probe. The FEC then completes its probe, registering the
> > interface. Sometime later Linux retries the switch probe, and this
> > time it works.
> >
> > What you are seeing here is the first attempt. There should be a
> > second go later in the log.
> >
> >        Andrew
> 

> Indeed, but that also fails because this second time, reading the
> switch ID (macreg 3 at addr 8) fails, it returns 0x0000!??

So this is the real problem.

Try removing the reset GPIO line from DT. If there is an EEPROM, and
the EEPROM contains a lot of instructions, we have seen it take a long
time to reset, load the EEPROM, and then start responding on the MDIO
bus. If you take away the GPIO, it will only do a software reset,
which is much faster. Even if you don't have an EEPROM, it is worth a
try.

But returning 0x0000 is odd. Normally, if an MDIO device does not
respond, you read 0xffff, because of the pull up resistor on the bus.

The fact you find it once means it is something like this, some minor
configuration problem, power management, etc.

	 Andrew

Re: net: dsa: mv88e6xxx: error parsing ethernet node from dts

[Jürgen Lambrecht]

On 12/4/19 6:13 PM, Andrew Lunn wrote:
> CAUTION: This Email originated from outside Televic. Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
>
> On Wed, Dec 04, 2019 at 05:20:04PM +0100, Jürgen Lambrecht wrote:
>> On 12/4/19 4:38 PM, Andrew Lunn wrote:
>>>> Here parts of dmesg (no error reported):
>>>>
>>>> [    1.992342] libphy: Fixed MDIO Bus: probed
>>>> [    2.009532] pps pps0: new PPS source ptp0
>>>> [    2.014387] libphy: fec_enet_mii_bus: probed
>>>> [    2.017159] mv88e6085 2188000.ethernet-1:00: switch 0x710 detected: Marvell 88E6071, revision 5
>>>> [    2.125616] libphy: mv88e6xxx SMI: probed
>>>> [    2.134450] fec 2188000.ethernet eth0: registered PHC device 0
>>>> ...
>>>> [   11.366359] Generic PHY fixed-0:00: attached PHY driver [Generic PHY] (mii_bus:phy_addr=fixed-0:00, irq=POLL)
>>>> [   11.366722] fec 2188000.ethernet eth0: Link is Up - 100Mbps/Full - flow control off
>>>>
>>>> When I enable debugging in the source code, I see that mv88e6xxx_probe() fails, because *'of_find_net_device_by_node(ethernet);'* fails. But why?,
>>> That always happens the first time. There is a chicken/egg
>>> problem. The MDIO bus is registered by the FEC driver, the switch is
>>> probed, and the DSA core looks for the ethernet interface. But the FEC
>>> driver has not yet registered the interface, it is still busy
>>> registering the MDIO bus. So you get an EPRODE_DEFFER from the switch
>>> probe. The FEC then completes its probe, registering the
>>> interface. Sometime later Linux retries the switch probe, and this
>>> time it works.
>>>
>>> What you are seeing here is the first attempt. There should be a
>>> second go later in the log.
>>>
>>>        Andrew
>> Indeed, but that also fails because this second time, reading the
>> switch ID (macreg 3 at addr 8) fails, it returns 0x0000!??
> So this is the real problem.
>
> Try removing the reset GPIO line from DT. If there is an EEPROM, and
> the EEPROM contains a lot of instructions, we have seen it take a long
> time to reset, load the EEPROM, and then start responding on the MDIO
> bus. If you take away the GPIO, it will only do a software reset,
> which is much faster. Even if you don't have an EEPROM, it is worth a
> try.
No didn't help
>
> But returning 0x0000 is odd. Normally, if an MDIO device does not
> respond, you read 0xffff, because of the pull up resistor on the bus.
Indeed
>
> The fact you find it once means it is something like this, some minor
> configuration problem, power management, etc.
>
>          Andrew

Thanks. I will have to look further in that direction. There must be sth obvious that I don't see.

A strange thing to me however is why - in my dts and in vf610-zii-ssmb-spu3.dts - there is 2 times a 'fixed-link' declaration? Moreover, when I omit the first declaration, the kernel crashes (oops).

I have the impression the first 'fixed-link' is used at the end of booting (at 11s, see first email) to configure the kernel in a fixed-phy configuration without a switch in between - and maybe that is the obvious that I don't see: why that fixed-phy config happens there, and even more, why does the kernel crash if it cannot config when I omit the info from dts?

I checked the kernel menuconfig, and there are fixed-phy configs enabled, but I cannot disable them, because they are selected by the DSA and freescale configs:

FIXED_PHY <= OF_MDIO [=y] && OF [=y] && PHYLIB [=y]
  PHYLIB <= FEC ... & ARCH_MXC
         <= PHYLINK && NETDEVICES
              PHYLINK <= NET_DSA && NET && HAVE_NET_DSA

Kind regards,

Jürgen

Re: net: dsa: mv88e6xxx: error parsing ethernet node from dts

[Andrew Lunn]

> A strange thing to me however is why - in my dts and in
> vf610-zii-ssmb-spu3.dts - there is 2 times a 'fixed-link'
> declaration? Moreover, when I omit the first declaration, the kernel
> crashes (oops).

The FEC driver expect there to be a PHY connected to the MAC, and the
PHY should indicate what speed the link is running at. However, the
FEC is directly connected to the switch, there is no PHY involved. To
keep the FEC happy, and to tell it what speed to use, a fixed-link PHY
is used. It uses the same API as a real PHY, and indicates the link is
using the speed as defined in DT. So the FEC runs at 100Mbps.

The DSA driver defaults to configuring the CPU port at its maximum
speed. For this chip, that is 1Gbps. However, the Vybrid FEC can only
support 100Mbps. So we need to force the CPU port to the slower
speed. Hence the fixed link in the CPU node.

       Andrew

Re: net: dsa: mv88e6xxx: error parsing ethernet node from dts

[Jürgen Lambrecht]

On 12/4/19 6:13 PM, Andrew Lunn wrote:
> But returning 0x0000 is odd. Normally, if an MDIO device does not
> respond, you read 0xffff, because of the pull up resistor on the bus.
>
> The fact you find it ones means it is something like this, some minor
> configuration problem, power management, etc.

Hi Adrew,

to close this issue: you were right: the Marvell clock, that comes from the iMX clocking block ENET1_REF_CLK_25M suddenly stops running:

an oscilloscope showed that the Marvell main clock stops shortly after the first probe, and only comes back 5s later at the end of booting when the fixed-phy is configured.
It is not the fec that stops the clock, because if fec1 is "disabled" also the clock stops, but then does not come back.

We did not found yet how to keep the clock enabled (independent of the fec), so if you have any hints - more than welcome.


Best regards,

Jürgen

Re: net: dsa: mv88e6xxx: error parsing ethernet node from dts

[Andrew Lunn]

On Tue, Dec 24, 2019 at 11:28:27AM +0100, Jürgen Lambrecht wrote:
> On 12/4/19 6:13 PM, Andrew Lunn wrote:
> > But returning 0x0000 is odd. Normally, if an MDIO device does not
> > respond, you read 0xffff, because of the pull up resistor on the bus.
> >
> > The fact you find it ones means it is something like this, some minor
> > configuration problem, power management, etc.
> 
> Hi Adrew,
> 
> to close this issue: you were right: the Marvell clock, that comes from the iMX clocking block ENET1_REF_CLK_25M suddenly stops running:
> 
> an oscilloscope showed that the Marvell main clock stops shortly after the first probe, and only comes back 5s later at the end of booting when the fixed-phy is configured.
> It is not the fec that stops the clock, because if fec1 is "disabled" also the clock stops, but then does not come back.
> 
> We did not found yet how to keep the clock enabled (independent of the fec), so if you have any hints - more than welcome.

Let me make sure i understand your design correct.

I think you are saying your switch does not have a crystal connected
to XTAL_OUT/XTAL_IN. Instead you want the iMX to generate a 25MHz
clock, which you are feeding to the switch?

All the designs i've used have the crystal connected to the
switch. The FEC clock line is used as an input, either driven from a
PHY connected to the other FEC port, or the clock output from the
switch.

So for your design, you need to ensure the 25MHz clock output is
always ticking. Looking at the FEC driver, you see the optional clock
fep->clk_enet_out. This clock is enabled when the FEC interface is
opened, i.e. configured up. It is disabled when the FEC is closed. It
is enabled during probe, but turned off at the end of the probe to
save power. The FEC also has runtime suspend/resume support. This
actually does not touch the clk_enet_out, but it does enable/disable
clocks needed for MDIO to work. I had to fix this many years ago.

It appears this clock is just a plain SOC clock.

In imx7d.dtsi we see:

                clocks = <&clks IMX7D_ENET2_IPG_ROOT_CLK>,
                        <&clks IMX7D_ENET_AXI_ROOT_CLK>,
                        <&clks IMX7D_ENET2_TIME_ROOT_CLK>,
                        <&clks IMX7D_PLL_ENET_MAIN_125M_CLK>,
                        <&clks IMX7D_ENET_PHY_REF_ROOT_CLK>;
                clock-names = "ipg", "ahb", "ptp",
                        "enet_clk_ref", "enet_out";

The mapping between clock-names and clocks seem a bit odd. But there
is some room for error here, since the FEC driver mostly just enables
them all or disables them all. But you need one specific clock.

What i suggest you do is add clock support to DSA. Allow DSA to look
up a clock in DT, and call clk_prepare_enable() and
clk_disable_unprepare(). The clock framework uses reference
counting. So if DSA turns a clock on, it does not matter what the FEC
does, it will stay on. It will only go off when all users of the clock
turn it off.

I'm not sure if this can be done in a generic way for all DSA drivers,
or if you need to add it to the mv88e6xxx driver. The DSA core only
gets involved once the probe of the switch is over. And you probably
need the clock reliably ticking during probe. So maybe it needs to be
in the mv88e6xxx driver.

   Andrew

Re: net: dsa: mv88e6xxx: error parsing ethernet node from dts

[Jürgen Lambrecht]

On 12/24/19 12:19 PM, Andrew Lunn wrote:
> CAUTION: This Email originated from outside Televic. Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
>
> On Tue, Dec 24, 2019 at 11:28:27AM +0100, Jürgen Lambrecht wrote:
>> On 12/4/19 6:13 PM, Andrew Lunn wrote:
>>> But returning 0x0000 is odd. Normally, if an MDIO device does not
>>> respond, you read 0xffff, because of the pull up resistor on the bus.
>>>
>>> The fact you find it ones means it is something like this, some minor
>>> configuration problem, power management, etc.
>> Hi Adrew,
>>
>> to close this issue: you were right: the Marvell clock, that comes from the iMX clocking block ENET1_REF_CLK_25M suddenly stops running:
>>
>> an oscilloscope showed that the Marvell main clock stops shortly after the first probe, and only comes back 5s later at the end of booting when the fixed-phy is configured.
>> It is not the fec that stops the clock, because if fec1 is "disabled" also the clock stops, but then does not come back.
>>
>> We did not found yet how to keep the clock enabled (independent of the fec), so if you have any hints - more than welcome.
> Let me make sure i understand your design correct.
>
> I think you are saying your switch does not have a crystal connected
> to XTAL_OUT/XTAL_IN. Instead you want the iMX to generate a 25MHz
> clock, which you are feeding to the switch?
indeed.
> All the designs i've used have the crystal connected to the
> switch. The FEC clock line is used as an input, either driven from a
> PHY connected to the other FEC port, or the clock output from the
> switch.
>
> So for your design, you need to ensure the 25MHz clock output is
> always ticking. Looking at the FEC driver, you see the optional clock
> fep->clk_enet_out. This clock is enabled when the FEC interface is
> opened, i.e. configured up. It is disabled when the FEC is closed. It
> is enabled during probe, but turned off at the end of the probe to
> save power. The FEC also has runtime suspend/resume support. This
> actually does not touch the clk_enet_out, but it does enable/disable
> clocks needed for MDIO to work. I had to fix this many years ago.
>
> It appears this clock is just a plain SOC clock.
>
> In imx7d.dtsi we see:
>
>                 clocks = <&clks IMX7D_ENET2_IPG_ROOT_CLK>,
>                         <&clks IMX7D_ENET_AXI_ROOT_CLK>,
>                         <&clks IMX7D_ENET2_TIME_ROOT_CLK>,
>                         <&clks IMX7D_PLL_ENET_MAIN_125M_CLK>,
>                         <&clks IMX7D_ENET_PHY_REF_ROOT_CLK>;
>                 clock-names = "ipg", "ahb", "ptp",
>                         "enet_clk_ref", "enet_out";
>
> The mapping between clock-names and clocks seem a bit odd. But there
> is some room for error here, since the FEC driver mostly just enables
> them all or disables them all. But you need one specific clock.
>
> What i suggest you do is add clock support to DSA. Allow DSA to look
> up a clock in DT, and call clk_prepare_enable() and
> clk_disable_unprepare(). The clock framework uses reference
> counting. So if DSA turns a clock on, it does not matter what the FEC
> does, it will stay on. It will only go off when all users of the clock
> turn it off.
>
> I'm not sure if this can be done in a generic way for all DSA drivers,
> or if you need to add it to the mv88e6xxx driver. The DSA core only
> gets involved once the probe of the switch is over. And you probably
> need the clock reliably ticking during probe. So maybe it needs to be
> in the mv88e6xxx driver.

Thanks for the hint. Now I know what to work on next year :-), because we are closed now for a week.

Jürgen

>  :-
>    Andrew

Re: net: dsa: mv88e6xxx: error parsing ethernet node from dts

[Jürgen Lambrecht]

On 12/24/19 2:57 PM, Jürgen Lambrecht wrote:
> CAUTION: This Email originated from outside Televic. Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
>
> On 12/24/19 12:19 PM, Andrew Lunn wrote:
>> CAUTION: This Email originated from outside Televic. Do not click links or open attachments unless you recognize the sender and know the content is safe.
>>
>>
>> On Tue, Dec 24, 2019 at 11:28:27AM +0100, Jürgen Lambrecht wrote:
>>> On 12/4/19 6:13 PM, Andrew Lunn wrote:
>>>> But returning 0x0000 is odd. Normally, if an MDIO device does not
>>>> respond, you read 0xffff, because of the pull up resistor on the bus.
>>>>
>>>> The fact you find it ones means it is something like this, some minor
>>>> configuration problem, power management, etc.
>>> Hi Adrew,
>>>
>>> to close this issue: you were right: the Marvell clock, that comes from the iMX clocking block ENET1_REF_CLK_25M suddenly stops running:
>>>
>>> an oscilloscope showed that the Marvell main clock stops shortly after the first probe, and only comes back 5s later at the end of booting when the fixed-phy is configured.
>>> It is not the fec that stops the clock, because if fec1 is "disabled" also the clock stops, but then does not come back.
>>>
>>> We did not found yet how to keep the clock enabled (independent of the fec), so if you have any hints - more than welcome.
>> Let me make sure i understand your design correct.
>>
>> I think you are saying your switch does not have a crystal connected
>> to XTAL_OUT/XTAL_IN. Instead you want the iMX to generate a 25MHz
>> clock, which you are feeding to the switch?
> indeed.
>> All the designs i've used have the crystal connected to the
>> switch. The FEC clock line is used as an input, either driven from a
>> PHY connected to the other FEC port, or the clock output from the
>> switch.
>>
>> So for your design, you need to ensure the 25MHz clock output is
>> always ticking. Looking at the FEC driver, you see the optional clock
>> fep->clk_enet_out. This clock is enabled when the FEC interface is
>> opened, i.e. configured up. It is disabled when the FEC is closed. It
>> is enabled during probe, but turned off at the end of the probe to
>> save power. The FEC also has runtime suspend/resume support. This
>> actually does not touch the clk_enet_out, but it does enable/disable
>> clocks needed for MDIO to work. I had to fix this many years ago.

You mean you added then the code that touches clk_enet_out: 'ret = clk_prepare_enable(fep->clk_enet_out);' and 'clk_disable_unprepare(fep->clk_enet_out);' ?

For the PHY, this can be the PHY chip clock, so it must be running for MDIO to work.
(and for us it is the switch clock)

>>
>> It appears this clock is just a plain SOC clock.
>>
>> In imx7d.dtsi we see:
>>
>>                 clocks = <&clks IMX7D_ENET2_IPG_ROOT_CLK>,
>>                         <&clks IMX7D_ENET_AXI_ROOT_CLK>,
>>                         <&clks IMX7D_ENET2_TIME_ROOT_CLK>,
>>                         <&clks IMX7D_PLL_ENET_MAIN_125M_CLK>,
>>                         <&clks IMX7D_ENET_PHY_REF_ROOT_CLK>;
>>                 clock-names = "ipg", "ahb", "ptp",
>>                         "enet_clk_ref", "enet_out";
>>
>> The mapping between clock-names and clocks seem a bit odd. But there
>> is some room for error here, since the FEC driver mostly just enables
>> them all or disables them all. But you need one specific clock.
>>
>> What i suggest you do is add clock support to DSA. Allow DSA to look
>> up a clock in DT, and call clk_prepare_enable() and
>> clk_disable_unprepare(). The clock framework uses reference
>> counting. So if DSA turns a clock on, it does not matter what the FEC
>> does, it will stay on. It will only go off when all users of the clock
>> turn it off.
>>
>> I'm not sure if this can be done in a generic way for all DSA drivers,
>> or if you need to add it to the mv88e6xxx driver. The DSA core only
>> gets involved once the probe of the switch is over. And you probably
>> need the clock reliably ticking during probe. So maybe it needs to be
>> in the mv88e6xxx driver.

We could do that, but actually, we prefer that the master clock of the switch chip is running always, independently of any kernel code.

So we would like to add support to be able to specify in DTS a clock output that should be always running. Because on our board that Ethernet clock output of the iMX6 is used to clock the switch, but it could also be used to clock any other chip that needs 25MHz.
We already tested such a driver and it works fine now.
What do you think?

I would then send a patch proposal to the arm mailing list.

(FYI: it has no use to remove that clock from the fec1 entry in imx6ul.dtsi, because then (after a patch) the clock is not touched anymore, but the power-management puts the clocking block in power-down when MDIO finishes and its clock is turned off, but the same block generates several clocks, so it is not possible to ignore the kernel because the bootloader already enables the clock.)

Kind regards,

Jürgen