netdev - Re: [PATCH v2 03/11] net: stmmac: dwmac-stm32: rework glue to simplify management

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4da0ce80-2120-4d67-aaaa-7dbf13b1da73@foss.st.com>
Date: Mon, 13 May 2024 14:48:26 +0200
From: Christophe ROULLIER <christophe.roullier@...s.st.com>
To: Marek Vasut <marex@...x.de>, "David S . Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
        Paolo
 Abeni <pabeni@...hat.com>, Rob Herring <robh+dt@...nel.org>,
        Krzysztof
 Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley
	<conor+dt@...nel.org>,
        Maxime Coquelin <mcoquelin.stm32@...il.com>,
        Alexandre
 Torgue <alexandre.torgue@...s.st.com>,
        Richard Cochran
	<richardcochran@...il.com>,
        Jose Abreu <joabreu@...opsys.com>,
        Liam Girdwood
	<lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>
CC: <netdev@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-stm32@...md-mailman.stormreply.com>,
        <linux-arm-kernel@...ts.infradead.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 03/11] net: stmmac: dwmac-stm32: rework glue to
 simplify management

Hi

On 4/26/24 16:53, Marek Vasut wrote:
> On 4/26/24 2:56 PM, Christophe Roullier wrote:
>> Change glue to be more generic and manage easily next stm32 products.
>> The goal of this commit is to have one stm32mp1_set_mode function which
>> can manage different STM32 SOC. SOC can have different SYSCFG register
>> bitfields. so in pmcsetr we defined the bitfields corresponding to 
>> the SOC.
>>
>> Signed-off-by: Christophe Roullier <christophe.roullier@...s.st.com>
>> ---
>>   .../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 76 +++++++++++++------
>>   1 file changed, 51 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c 
>> b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
>> index c92dfc4ecf57..68a02de25ac7 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
>> @@ -23,10 +23,6 @@
>>     #define SYSCFG_MCU_ETH_MASK        BIT(23)
>>   #define SYSCFG_MP1_ETH_MASK        GENMASK(23, 16)
>> -#define SYSCFG_PMCCLRR_OFFSET        0x40
>> -
>> -#define SYSCFG_PMCR_ETH_CLK_SEL        BIT(16)
>> -#define SYSCFG_PMCR_ETH_REF_CLK_SEL    BIT(17)
>>     /* CLOCK feed to PHY*/
>>   #define ETH_CK_F_25M    25000000
>> @@ -46,9 +42,6 @@
>>    * RMII  |   1     |   0      |   0       |  n/a  |
>>    *------------------------------------------
>>    */
>> -#define SYSCFG_PMCR_ETH_SEL_MII        BIT(20)
>> -#define SYSCFG_PMCR_ETH_SEL_RGMII    BIT(21)
>> -#define SYSCFG_PMCR_ETH_SEL_RMII    BIT(23)
>>   #define SYSCFG_PMCR_ETH_SEL_GMII    0
>>   #define SYSCFG_MCU_ETH_SEL_MII        0
>>   #define SYSCFG_MCU_ETH_SEL_RMII        1
>> @@ -90,19 +83,33 @@ struct stm32_dwmac {
>>       int eth_ref_clk_sel_reg;
>>       int irq_pwr_wakeup;
>>       u32 mode_reg;         /* MAC glue-logic mode register */
>> +    u32 mode_mask;
>>       struct regmap *regmap;
>>       u32 speed;
>>       const struct stm32_ops *ops;
>>       struct device *dev;
>>   };
>>   +struct stm32_syscfg_pmcsetr {
>> +    u32 eth1_clk_sel;
>> +    u32 eth1_ref_clk_sel;
>> +    u32 eth1_selmii;
>> +    u32 eth1_sel_rgmii;
>> +    u32 eth1_sel_rmii;
>> +    u32 eth2_clk_sel;
>> +    u32 eth2_ref_clk_sel;
>> +    u32 eth2_sel_rgmii;
>> +    u32 eth2_sel_rmii;
>> +};
>
> [...]
>
>> @@ -487,8 +502,19 @@ static struct stm32_ops stm32mp1_dwmac_data = {
>>       .suspend = stm32mp1_suspend,
>>       .resume = stm32mp1_resume,
>>       .parse_data = stm32mp1_parse_data,
>> -    .syscfg_eth_mask = SYSCFG_MP1_ETH_MASK,
>> -    .clk_rx_enable_in_suspend = true
>> +    .clk_rx_enable_in_suspend = true,
>> +    .syscfg_clr_off = 0x44,
>> +    .pmcsetr = {
>> +        .eth1_clk_sel        = BIT(16),
>> +        .eth1_ref_clk_sel    = BIT(17),
>> +        .eth1_selmii        = BIT(20),
>> +        .eth1_sel_rgmii        = BIT(21),
>> +        .eth1_sel_rmii        = BIT(23),
>> +        .eth2_clk_sel        = 0,
>> +        .eth2_ref_clk_sel    = 0,
>> +        .eth2_sel_rgmii        = 0,
>> +        .eth2_sel_rmii        = 0
>> +    }
>>   };
>
> Is this structure really necessary ?
>
I prefer to keep this implementation for the moment, as it is working 
fine. Maybe at a later stage, I will send some optimizations.

> It seems the MP15 single ethernet config bitfield is at offset 16.
> MP13 has two bitfields, one at offset 16, the other at offset 24 .
>
> All you need to do is figure out which of the two MACs you are 
> configuring, and then shift the bitfield mask by 16 or 24, since the 
> bits are at the same offset for both bitfields.
>
> See the matching upstream U-Boot commit for how this shift can be done:
> a440d19c6c91 ("net: dwc_eth_qos: Add DT parsing for STM32MP13xx 
> platform")
>