net: stmmac: DT clock parameter and behavior?

net: stmmac: DT clock parameter and behavior?

[Chen-Yu Tsai]

I am currently porting the Linux stmmac driver to an AllWinner SoC
board. The AllWinner A20 SoC has a GMAC, and the drivers they provided
seems to be an early version of stmmac, with the names replaced.
I assume the IP in the SoC is an early version of dwmac. So far I have
managed to get a running system. I would like to clarify a few details
to clean up the code.

The stmmac core code references a clock named "stmmaceth".
This clock does not seem to be documented in "networking/stmmac.txt"
nor in "devicetree/bindings/net/stmmac.txt".
STMicroelectronics SPEAr board DTs do not mention this clock.
But I do see Altera SoCFPGA and Samsung Exynos DTs defining it.

The latest patch series for dwmac-sti on netdev suggests that this
clock is a 25MHz clock for the phy device. Is this true?

Then the driver does not consider that the dwmac might have a gated
clock that needs to be enabled as well. I ran into problems when
the dwmac was not initialized by the bootloader. I assume this can
be dealt with using AUXDATA and .init/.exit callbacks?

Also, earlier versions of stmmac seemed to only give a warning if
"stmmaceth" clock was not found. But in

    6a81c26f  net/stmmac: remove conditional compilation of clk code

the behavior changed to failing completely. Was this intended?

Last, given the supported device is actually Synopsys DesignWare
Ethernet MAC, is there any chance the driver could be renamed to dwmac?


Thank You

Chen-Yu Tsai

Re: net: stmmac: DT clock parameter and behavior?

[Giuseppe CAVALLARO]

Hi

On 11/19/2013 3:17 PM, Chen-Yu Tsai wrote:
> I am currently porting the Linux stmmac driver to an AllWinner SoC
> board. The AllWinner A20 SoC has a GMAC, and the drivers they provided
> seems to be an early version of stmmac, with the names replaced.
> I assume the IP in the SoC is an early version of dwmac. So far I have
> managed to get a running system. I would like to clarify a few details
> to clean up the code.
>
> The stmmac core code references a clock named "stmmaceth".
> This clock does not seem to be documented in "networking/stmmac.txt"

yes you are right and I will improve the doc  for this clk soon.

Let me provide you some details. I hope useful to go head on your
porting.

The MAC Control Interface, CSR, and Station Management Agent of the MAC
run on the clock named "stmmaceth".

For example the CSR is tuned according to this main clock (as poorly
documented in the stmmac.txt ... I'll improve it as well )

> nor in "devicetree/bindings/net/stmmac.txt".
> STMicroelectronics SPEAr board DTs do not mention this clock.
> But I do see Altera SoCFPGA and Samsung Exynos DTs defining it.
>
> The latest patch series for dwmac-sti on netdev suggests that this
> clock is a 25MHz clock for the phy device. Is this true?


this is reported in

Documentation/devicetree/bindings/net/sti-dwmac.txt

The problem is that, on ST platforms, e.g. in rgmii mode, it is
necessary to dynamically change the phy clk according to the
speed that is negotiated. (e.g. speed = 1000, frq = 125MHz
25 for 100 etc)

This is a quite complex logic that has been added to manage
the phy clock that can be routed by the SoC or PHY.

There is no relationship with the stmmaceth clk.

>
> Then the driver does not consider that the dwmac might have a gated
> clock that needs to be enabled as well. I ran into problems when
> the dwmac was not initialized by the bootloader. I assume this can
> be dealt with using AUXDATA and .init/.exit callbacks?

AUXDATA is not the preferred solution.


this how we are managing stmmac from DT

ethernet1: ethernet1 {
	#address-cells = <1>;
	#size-cells = <1>;
	status 		= "disabled";
	compatible		= "st,stih416-dwmac";
	reg			= <0x7f0 0x4>;
	st,syscon		= <&syscfg_sbc>;
	resets			= <&softreset STIH416_ETH1_SOFTRESET>;
	ranges;

	dwmac@fef08000 {
		device_type = "network";
		compatible	= "snps,dwmac", "snps,dwmac-3.710";
		status 		= "disabled";
		reg		= <0xfef08000 0x8000>;
		interrupts = <0 136 0>, <0 137 0>, <0 138 0>;
		interrupt-names = "macirq", "eth_wake_irq", "eth_lpi";

		snps,pbl	= <32>;
		snps,mixed-burst;

		pinctrl-names 	= "default";
		pinctrl-0	= <&pinctrl_mii1>;
		clock-names	= "stmmaceth";
		clocks		= <&CLK_S_ETH1_PHY>;
	};
};

> Also, earlier versions of stmmac seemed to only give a warning if
> "stmmaceth" clock was not found. But in
>
>      6a81c26f  net/stmmac: remove conditional compilation of clk code
>
> the behavior changed to failing completely. Was this intended?

in the beginning, when I pushed the stmmac, we used an old approach
to manage the tx/rx processes by using an external timer.
For example for SH4 we used the TMU channel 2.
The patch you mentioned (IIRC) was for a code that has been removed
long time ago because the logic behind it became obsolete due to the
introduction of the RX watchdog inside the SYNP.

> Last, given the supported device is actually Synopsys DesignWare
> Ethernet MAC, is there any chance the driver could be renamed to dwmac?

we discussed about that long time ago when the driver was moved to
ethernet/stmicro directory. We can reopen the discussion.
I accept it because, indeed, the stmmac is really generic and for
dwmac synp chips.

BR
peppe

Re: net: stmmac: DT clock parameter and behavior?

[Chen-Yu Tsai]

>> The stmmac core code references a clock named "stmmaceth".
>> This clock does not seem to be documented in "networking/stmmac.txt"
> The MAC Control Interface, CSR, and Station Management Agent of the MAC
> run on the clock named "stmmaceth".
>
> For example the CSR is tuned according to this main clock (as poorly
> documented in the stmmac.txt ... I'll improve it as well )

I assume the MDIO bus registers and MDC are also derived from this
main clock?

The issue I ran into is that in the clock is not enabled during *_probe,
stmmac_dvr_probe() to be exact. The stmmac driver probes the MAC hardware
version, feature flags, and the MDIO bus for PHY devices. If the clock was
not previously enabled by the boot loader, the driver gets bogus results.

In my opinion, either the clock has to be enabled before or in
stmmac_dvr_probe(), or the clock was intended for something else than
the MAC's main clock.

I was not provided with any documentation for the MAC.
What is the CSR for exactly?

>> The latest patch series for dwmac-sti on netdev suggests that this
>> clock is a 25MHz clock for the phy device. Is this true?
> this is reported in
> Documentation/devicetree/bindings/net/sti-dwmac.txt
>
> The problem is that, on ST platforms, e.g. in rgmii mode, it is
> necessary to dynamically change the phy clk according to the
> speed that is negotiated. (e.g. speed = 1000, frq = 125MHz
> 25 for 100 etc)
>
> This is a quite complex logic that has been added to manage
> the phy clock that can be routed by the SoC or PHY.

Are you refering to the "st,tx-retime-src" property for sti-dwmac?
The Allwinner SoCs have a similar mechanism for TX clock selection
and division.

My original question was, the DTs for STiH415 and ST416 both use
<&CLK_S_GMAC0_PHY> and <&CLK_S_ETH1_PHY> as "stmmaceth" clock.
Both of them are defined as 25 MHz fixed rate clocks.

Without schematics, I can only guess that they might be external
crystal oscillators that run the phy device's internals, but not
the MII/RGMII interface. Hence they are only enabled when the net
device is opened, i.e. when needed.

> AUXDATA is not the preferred solution.
> this how we are managing stmmac from DT

Adding a glue layer device seems like a good way.
Thank you for pointing me in that direction.

>> Also, earlier versions of stmmac seemed to only give a warning if
>> "stmmaceth" clock was not found. But in
>>
>>      6a81c26f  net/stmmac: remove conditional compilation of clk code
>>
>> the behavior changed to failing completely. Was this intended?
> in the beginning, when I pushed the stmmac, we used an old approach
> to manage the tx/rx processes by using an external timer.
> For example for SH4 we used the TMU channel 2.
> The patch you mentioned (IIRC) was for a code that has been removed
> long time ago because the logic behind it became obsolete due to the
> introduction of the RX watchdog inside the SYNP.

The patch looks more like common clock framework code cleanup to me.

>> Last, given the supported device is actually Synopsys DesignWare
>> Ethernet MAC, is there any chance the driver could be renamed to dwmac?
> we discussed about that long time ago when the driver was moved to
> ethernet/stmicro directory. We can reopen the discussion.
> I accept it because, indeed, the stmmac is really generic and for
> dwmac synp chips.

I see. We could discuss this after my code is is submitted and accepted.


Cheers
wens