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

[Marek Vasut]

On 1/29/21 4:19 PM, Alexandre TORGUE wrote:
> 
> 
> On 1/26/21 8:11 PM, Marek Vasut wrote:
>> On 1/26/21 5:47 PM, Alexandre TORGUE wrote:
>>>
>>>
>>> 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.
>>
>> 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.