Re: [PATCH v3 02/11] net: stmmac: dwmac-stm32: Separate out external clock rate validation

From: Marek Vasut
Date: Mon Jun 03 2024 - 10:31:43 EST


On 6/3/24 11:27 AM, Christophe Roullier wrote:
From: Marek Vasut <marex@xxxxxxx>

Pull the external clock frequency validation into a separate function,
to avoid conflating it with external clock DT property decoding and
clock mux register configuration. This should make the code easier to
read and understand.

This does change the code behavior slightly. The clock mux PMCR register
setting now depends solely on the DT properties which configure the clock
mux between external clock and internal RCC generated clock. The mux PMCR
register settings no longer depend on the supplied clock frequency, that
supplied clock frequency is now only validated, and if the clock frequency
is invalid for a mode, it is rejected.

Previously, the code would switch the PMCR register clock mux to internal
RCC generated clock if external clock couldn't provide suitable frequency,
without checking whether the RCC generated clock frequency is correct. Such
behavior is risky at best, user should have configured their clock correctly
in the first place, so this behavior is removed here.

Signed-off-by: Marek Vasut <marex@xxxxxxx>
---
.../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 54 +++++++++++++++----
1 file changed, 44 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
index c92dfc4ecf570..43340a5573c64 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
@@ -157,25 +157,57 @@ static int stm32_dwmac_init(struct plat_stmmacenet_data *plat_dat, bool resume)
return stm32_dwmac_clk_enable(dwmac, resume);
}
+static int stm32mp1_validate_ethck_rate(struct plat_stmmacenet_data *plat_dat)
+{
+ struct stm32_dwmac *dwmac = plat_dat->bsp_priv;
+ const u32 clk_rate = clk_get_rate(dwmac->clk_eth_ck);

From Sai in
Re: [net-next,RFC,PATCH 1/5] net: stmmac: dwmac-stm32: Separate out external clock rate validation

Please check reverse x-mass tree is followed for these variables, if possible.

+ switch (plat_dat->mac_interface) {
+ case PHY_INTERFACE_MODE_MII:
+ if (clk_rate == ETH_CK_F_25M)
+ return 0;
+ break;
+ case PHY_INTERFACE_MODE_GMII:
+ if (clk_rate == ETH_CK_F_25M)
+ return 0;
+ break;

From Sai in
Re: [net-next,RFC,PATCH 1/5] net: stmmac: dwmac-stm32: Separate out external clock rate validation

Please check, whether we can combine the two cases..

+ case PHY_INTERFACE_MODE_RMII:
+ if (clk_rate == ETH_CK_F_25M || clk_rate == ETH_CK_F_50M)
+ return 0;
+ break;
+ case PHY_INTERFACE_MODE_RGMII:
+ case PHY_INTERFACE_MODE_RGMII_ID:
+ case PHY_INTERFACE_MODE_RGMII_RXID:
+ case PHY_INTERFACE_MODE_RGMII_TXID:
+ if (clk_rate == ETH_CK_F_25M || clk_rate == ETH_CK_F_125M)
+ return 0;
+ break;
+ default:
+ break;
+ }
+
+ dev_err(dwmac->dev, "Mode %s does not match eth-ck frequency %d Hz",
+ phy_modes(plat_dat->mac_interface), clk_rate);
+ return -EINVAL;
+}

[...]