[PATCH 3/4] [RFC] ARM: dts: stm32: Add mux for ETHRX clock

[Alexandre TORGUE]

On 1/26/21 4:42 PM, Marek Vasut wrote:
> On 1/26/21 4:40 PM, Alexandre TORGUE wrote:
>>
>>
>> On 1/26/21 1:59 PM, Marek Vasut wrote:
>>> On 1/26/21 11:54 AM, Alexandre TORGUE wrote:
>>> [...]
>>>>>>>>>>> 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.