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

[Alexandre TORGUE <alexandre.torgue@foss.st.com>]

Hi Marek

On 1/6/21 9:43 PM, Marek Vasut 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)).

> 
> First, the ETHRX clock in clk-stm32mp1.c only represents the ETHRXEN gate,
> however it should represent also ETH_REF_CLK_SEL mux. The problem is that
> the ETH_REF_CLK_SEL mux is currently configured in the DWMAC4 driver and
> the ETH_REF_CLK_SEL bit is part of SYSCFG block, not the DWMAC4 or the
> clock block.

dwmac4-stm32 doesn't contain code for dwmac4 but it contains the glue 
around the dwmac4: syscfg, clocks ...

> 
> Second, the ETHRX parent clock is either eth_clk_fb (ETHCK_K) or external
> ETH_RX_CLK/ETH_REF_CLK_SEL, it is never CK_AXI.

Why CK_AXI ?

Regards
Alex

> 
> This patch attempts to address the clock selection by adding fixed factor
> clock to DT, which allows the user to select its upstream clock.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Alexandre Torgue <alexandre.torgue@st.com>
> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> Cc: Patrice Chotard <patrice.chotard@st.com>
> Cc: Patrick Delaunay <patrick.delaunay@st.com>
> Cc: linux-stm32@st-md-mailman.stormreply.com
> To: linux-arm-kernel@lists.infradead.org
> ---
>   arch/arm/boot/dts/stm32mp151.dtsi | 8 ++++++++
>   drivers/clk/clk-stm32mp1.c        | 2 +-
>   2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/stm32mp151.dtsi b/arch/arm/boot/dts/stm32mp151.dtsi
> index 32875eabd357..8c2a5d0875d8 100644
> --- a/arch/arm/boot/dts/stm32mp151.dtsi
> +++ b/arch/arm/boot/dts/stm32mp151.dtsi
> @@ -82,6 +82,14 @@ clk_csi: clk-csi {
>   			compatible = "fixed-clock";
>   			clock-frequency = <4000000>;
>   		};
> +
> +		clk_eth_rx: eth-rx-clk {
> +			compatible = "fixed-factor-clock";
> +			clocks = <&rcc ETHCK_K>;
> +			#clock-cells = <0>;
> +			clock-div = <1>;
> +			clock-mult = <1>;
> +		};
>   	};
>   
>   	thermal-zones {
> diff --git a/drivers/clk/clk-stm32mp1.c b/drivers/clk/clk-stm32mp1.c
> index a875649df8b8..63971d40f15c 100644
> --- a/drivers/clk/clk-stm32mp1.c
> +++ b/drivers/clk/clk-stm32mp1.c
> @@ -1892,7 +1892,7 @@ static const struct clock_config stm32mp1_clock_cfg[] = {
>   	PCLK(MDMA, "mdma", "ck_axi", 0, G_MDMA),
>   	PCLK(GPU, "gpu", "ck_axi", 0, G_GPU),
>   	PCLK(ETHTX, "ethtx", "ck_axi", 0, G_ETHTX),
> -	PCLK(ETHRX, "ethrx", "ck_axi", 0, G_ETHRX),
> +	PCLK(ETHRX, "ethrx", "eth-rx-clk", 0, G_ETHRX),
>   	PCLK(ETHMAC, "ethmac", "ck_axi", 0, G_ETHMAC),
>   	PCLK(FMC, "fmc", "ck_axi", CLK_IGNORE_UNUSED, G_FMC),
>   	PCLK(QSPI, "qspi", "ck_axi", CLK_IGNORE_UNUSED, G_QSPI),
> 

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