[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <d1aac0b0-c985-40c2-8a6f-29a4617edaf8@denx.de>
Date: Mon, 3 Jun 2024 16:26:19 +0200
From: Marek Vasut <marex@...x.de>
To: Sai Krishna Gajula <saikrishnag@...vell.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Cc: "David S. Miller" <davem@...emloft.net>,
Alexandre Torgue <alexandre.torgue@...s.st.com>,
Christophe Roullier <christophe.roullier@...s.st.com>,
Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
Jose Abreu <joabreu@...opsys.com>,
Maxime Coquelin <mcoquelin.stm32@...il.com>, Paolo Abeni
<pabeni@...hat.com>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linux-stm32@...md-mailman.stormreply.com"
<linux-stm32@...md-mailman.stormreply.com>
Subject: Re: [net-next,RFC,PATCH 1/5] net: stmmac: dwmac-stm32: Separate out
external clock rate validation
On 4/29/24 9:19 AM, Sai Krishna Gajula wrote:
>
>> -----Original Message-----
>> From: Marek Vasut <marex@...x.de>
>> Sent: Sunday, April 28, 2024 3:21 AM
>> To: netdev@...r.kernel.org
>> Cc: Marek Vasut <marex@...x.de>; David S. Miller <davem@...emloft.net>;
>> Alexandre Torgue <alexandre.torgue@...s.st.com>; Christophe Roullier
>> <christophe.roullier@...s.st.com>; Eric Dumazet <edumazet@...gle.com>;
>> Jakub Kicinski <kuba@...nel.org>; Jose Abreu <joabreu@...opsys.com>;
>> Maxime Coquelin <mcoquelin.stm32@...il.com>; Paolo Abeni
>> <pabeni@...hat.com>; linux-arm-kernel@...ts.infradead.org; linux-
>> stm32@...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@...x.de>
>> ---
>> Cc: "David S. Miller" <davem@...emloft.net>
>> Cc: Alexandre Torgue <alexandre.torgue@...s.st.com>
>> Cc: Christophe Roullier <christophe.roullier@...s.st.com>
>> Cc: Eric Dumazet <edumazet@...gle.com>
>> Cc: Jakub Kicinski <kuba@...nel.org>
>> Cc: Jose Abreu <joabreu@...opsys.com>
>> Cc: Maxime Coquelin <mcoquelin.stm32@...il.com>
>> Cc: Paolo Abeni <pabeni@...hat.com>
>> Cc: linux-arm-kernel@...ts.infradead.org
>> Cc: linux-stm32@...md-mailman.stormreply.com
>> Cc: netdev@...r.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 !
Powered by blists - more mailing lists