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

[Marek Vasut <marex@denx.de>]

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 ?

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