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

[Christophe ROULLIER]

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 at foss.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")
>