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

[Marek Vasut]

On 4/29/24 9:19 AM, Sai Krishna Gajula wrote:
> 
>> -----Original Message-----
>> From: Marek Vasut <marex at denx.de>
>> Sent: Sunday, April 28, 2024 3:21 AM
>> To: netdev at vger.kernel.org
>> Cc: Marek Vasut <marex at denx.de>; David S. Miller <davem at davemloft.net>;
>> Alexandre Torgue <alexandre.torgue at foss.st.com>; Christophe Roullier
>> <christophe.roullier at foss.st.com>; Eric Dumazet <edumazet at google.com>;
>> Jakub Kicinski <kuba at kernel.org>; Jose Abreu <joabreu at synopsys.com>;
>> Maxime Coquelin <mcoquelin.stm32 at gmail.com>; Paolo Abeni
>> <pabeni at redhat.com>; linux-arm-kernel at lists.infradead.org; linux-
>> stm32 at st-md-mailman.stormreply.com
>> Subject: [net-next,RFC,PATCH 1/5] net: stmmac: dwmac-stm32:
>> Separate out external clock rate validation
>>
>> 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 at denx.de>
>> ---
>> Cc: "David S. Miller" <davem at davemloft.net>
>> Cc: Alexandre Torgue <alexandre.torgue at foss.st.com>
>> Cc: Christophe Roullier <christophe.roullier at foss.st.com>
>> Cc: Eric Dumazet <edumazet at google.com>
>> Cc: Jakub Kicinski <kuba at kernel.org>
>> Cc: Jose Abreu <joabreu at synopsys.com>
>> Cc: Maxime Coquelin <mcoquelin.stm32 at gmail.com>
>> Cc: Paolo Abeni <pabeni at redhat.com>
>> Cc: linux-arm-kernel at lists.infradead.org
>> Cc: linux-stm32 at st-md-mailman.stormreply.com
>> Cc: netdev at vger.kernel.org
>> ---
>>   .../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);
> 
> 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;
> 
> Please check, whether we can combine the two cases..

I hope those would be addressed in v4 of:

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

Thanks !