On 1/30/21 7:36 PM, Marek Vasut wrote:

On 1/29/21 4:19 PM, Alexandre TORGUE wrote:

quoted

On 1/26/21 8:11 PM, Marek Vasut wrote:

quoted

On 1/26/21 5:47 PM, Alexandre TORGUE wrote:

quoted

On 1/26/21 4:42 PM, Marek Vasut wrote:

quoted

On 1/26/21 4:40 PM, Alexandre TORGUE wrote:

quoted

On 1/26/21 1:59 PM, Marek Vasut wrote:

quoted

On 1/26/21 11:54 AM, Alexandre TORGUE wrote:
[...]

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

The implementation of ETH_RX_CLK/ETH_REF_CLK handling 
currently does not
permit selecting the clock input from SoC pad. To make 
things worse, the
implementation of this is partly present and is split 
between the clock
driver and dwmac4 driver. Moreover, the ETHRX clock 
parent is incorrect.

Sorry but I don't understand which configuration is 
missing. I think we can handle all possible cases for 
RMII. At the glue layer (dwmac-stm32.c) clocks gates and 
syscfg are set regarding device tree binding (see the tab 
in dwmac-stm32.c). You could have a look here for more 
details: 
https://wiki.st.com/stm32mpu/wiki/Ethernet_device_tree_configuration 


Regarding the clock parent, yes it is not at the well 
frequency if you want to select this path. Our current 
"clock tree" is done to fit with our ST reference boards 
(we have more peripherals than PLL outputs so we have to 
make choices). So yes for customer/partners boards this 
clock tree has to be modified to better fit with the need 
(either using assigned-clock-parent or by modifying 
bootloader clock tree (tf-a or u-boot)).

I don't think you handle all the configuration options, but 
I might also be confused.

See Figure 83. Peripheral clock distribution for Ethernet 
in the MP1 datasheet for the below.

The current setup I have needs 50 MHz on SoC pad PA1 to 
drive the PHY clock, and uses eth_clk_fb to supply 
ETH_RX_CLK. However, the 50 MHz is sourced directly from 
PLL4P, which then has to run at 50 MHz and that in turn 
reduces clock frequency for other blocks connected to PLL4P 
(e.g. SDMMC, where the impact is noticable).

Ok that's the common path to clock a PHY a 50MHz without 
using the ref_clk coming from the PHY. And yes I can 
understand that the drawback is huge).

So lets fix it.

There is no issue in code. It is just clock tree configuration 
issue. Either you don't use PLL4P for Ethernet (what you're 
doing) or you don't use PLL4P for SDMMC. But yes, there are 
not a lot of possibilities.

I am supplying MCO2 with PLL4P, that is PLL4P->MCO2->ETHRX . To 
enable this entire chain of clock, I need the correct clock 
tree. Currently that cannot be modeled, can it?

Maybe I miss something, I thought your setup was like that:

First clock path to your PHY:
--------------------

PLL4P ---> MCO2 ---> X1 (PHY input clock which replaces crystal)
It is not directly linked to the dwmac-stm32. You "just" provide 
a clock to MCO2. After that you can use MCO2 pins for any usages.

Second clock patch:
--------------------

50MHz (refclk coming from phy) --> ETH_REF_CLK pad
This one is already covered in dwmac-stm32.

Why do you want to link the both clock paths ?

Because the X1 (MCO2 output) is the same net as 50 MHz 
ETH_REF_CLK input. MCO2 output is routed on a SoC pin and that is 
connected with a wire to ETH_REF_CLK SoC pin (input).

Ok I see, but I don't think you have to link both clocks.

If I don't, then MCO2 will not have any consumer and would be 
turned off by the kernel.

I agree, but IMO the MCO clock should be declared with 
CLK_IGNORE_UNUSED flag in stm32mp1 clock driver.

Why? It can be safely turned off if it is only used to supply ETHRX. 
And if the clock tree is correctly modeled, that is what happens.

You're right. I think we could only add an optional clock inside dwmac 
stm32 glue to take this phy clock (here MCO2)

But you already do have clock in the glue, it's the ETHRX clock. There 
are no additional clock that have to be added to the glue.

Yes this one is for ETHRX/REFCLK, but the one I talk about is the one to 
clock the PHY (which replaces the crystal). In your case it is the same 
net but it is not always the case.

cheers
alex


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel