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

["Jürgen Lambrecht" <j.lambrecht@televic.com>]

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