Received-SPF: pass (google.com: domain of linux-kernel+bounces-177581-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <4da0ce80-2120-4d67-aaaa-7dbf13b1da73@foss.st.com>
Date: Mon, 13 May 2024 14:48:26 +0200
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 03/11] net: stmmac: dwmac-stm32: rework glue to
 simplify management
To: Marek Vasut <marex@denx.de>, "David S . Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>,
        Paolo
 Abeni <pabeni@redhat.com>, Rob Herring <robh+dt@kernel.org>,
        Krzysztof
 Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley
	<conor+dt@kernel.org>,
        Maxime Coquelin <mcoquelin.stm32@gmail.com>,
        Alexandre
 Torgue <alexandre.torgue@foss.st.com>,
        Richard Cochran
	<richardcochran@gmail.com>,
        Jose Abreu <joabreu@synopsys.com>,
        Liam Girdwood
	<lgirdwood@gmail.com>, Mark Brown <broonie@kernel.org>
CC: <netdev@vger.kernel.org>, <devicetree@vger.kernel.org>,
        <linux-stm32@st-md-mailman.stormreply.com>,
        <linux-arm-kernel@lists.infradead.org>, <linux-kernel@vger.kernel.org>
References: <20240426125707.585269-1-christophe.roullier@foss.st.com>
 <20240426125707.585269-4-christophe.roullier@foss.st.com>
 <56f2d023-82d5-4910-8c4e-68e9d62bd1fe@denx.de>
Content-Language: en-US
From: Christophe ROULLIER <christophe.roullier@foss.st.com>
In-Reply-To: <56f2d023-82d5-4910-8c4e-68e9d62bd1fe@denx.de>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit

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@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")
>