Received-SPF: pass (google.com: domain of linux-kernel+bounces-149704-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Thu, 18 Apr 2024 11:17:15 +0300
From: Serge Semin <fancer.lancer@gmail.com>
To: Christophe Roullier <christophe.roullier@foss.st.com>
Cc: "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>, 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
Subject: Re: [PATCH 03/11] net: ethernet: stmmac: rework glue to simplify
 management for next stm32
Message-ID: <peqz2hl6efyu25j26rpt6rqi5ovq4ymjvivfhczihnz4hf7w2o@akaiyvzuqjgo>
References: <20240411143658.1049706-1-christophe.roullier@foss.st.com>
 <20240411143658.1049706-4-christophe.roullier@foss.st.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240411143658.1049706-4-christophe.roullier@foss.st.com>

On Thu, Apr 11, 2024 at 04:36:50PM +0200, 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>

[PATCH 03/11] net: ethernet: stmmac: rework glue to simplify management for next stm32

Just use the dwmac-stm32 prefix so you won't need to have the "for
next stm32" clause in all the commits. For instance like this:

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

the same for the patches 4 to 7.

-Serge(y)


> ---
>  .../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 c92dfc4ecf570..68a02de25ac76 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;
> +};
> +
>  struct stm32_ops {
>  	int (*set_mode)(struct plat_stmmacenet_data *plat_dat);
>  	int (*suspend)(struct stm32_dwmac *dwmac);
>  	void (*resume)(struct stm32_dwmac *dwmac);
>  	int (*parse_data)(struct stm32_dwmac *dwmac,
>  			  struct device *dev);
> -	u32 syscfg_eth_mask;
> +	u32 syscfg_clr_off;
> +	struct stm32_syscfg_pmcsetr pmcsetr;
>  	bool clk_rx_enable_in_suspend;
>  };
>  
> @@ -161,7 +168,7 @@ static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat)
>  {
>  	struct stm32_dwmac *dwmac = plat_dat->bsp_priv;
>  	u32 reg = dwmac->mode_reg, clk_rate;
> -	int val;
> +	int val = 0;
>  
>  	clk_rate = clk_get_rate(dwmac->clk_eth_ck);
>  	dwmac->enable_eth_ck = false;
> @@ -169,7 +176,7 @@ static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat)
>  	case PHY_INTERFACE_MODE_MII:
>  		if (clk_rate == ETH_CK_F_25M && dwmac->ext_phyclk)
>  			dwmac->enable_eth_ck = true;
> -		val = SYSCFG_PMCR_ETH_SEL_MII;
> +		val = dwmac->ops->pmcsetr.eth1_selmii;
>  		pr_debug("SYSCFG init : PHY_INTERFACE_MODE_MII\n");
>  		break;
>  	case PHY_INTERFACE_MODE_GMII:
> @@ -177,16 +184,17 @@ static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat)
>  		if (clk_rate == ETH_CK_F_25M &&
>  		    (dwmac->eth_clk_sel_reg || dwmac->ext_phyclk)) {
>  			dwmac->enable_eth_ck = true;
> -			val |= SYSCFG_PMCR_ETH_CLK_SEL;
> +			val |= dwmac->ops->pmcsetr.eth1_clk_sel;
>  		}
>  		pr_debug("SYSCFG init : PHY_INTERFACE_MODE_GMII\n");
>  		break;
>  	case PHY_INTERFACE_MODE_RMII:
> -		val = SYSCFG_PMCR_ETH_SEL_RMII;
> +		val = dwmac->ops->pmcsetr.eth1_sel_rmii | dwmac->ops->pmcsetr.eth2_sel_rmii;
>  		if ((clk_rate == ETH_CK_F_25M || clk_rate == ETH_CK_F_50M) &&
>  		    (dwmac->eth_ref_clk_sel_reg || dwmac->ext_phyclk)) {
>  			dwmac->enable_eth_ck = true;
> -			val |= SYSCFG_PMCR_ETH_REF_CLK_SEL;
> +			val |= dwmac->ops->pmcsetr.eth1_ref_clk_sel;
> +			val |= dwmac->ops->pmcsetr.eth2_ref_clk_sel;
>  		}
>  		pr_debug("SYSCFG init : PHY_INTERFACE_MODE_RMII\n");
>  		break;
> @@ -194,11 +202,12 @@ static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat)
>  	case PHY_INTERFACE_MODE_RGMII_ID:
>  	case PHY_INTERFACE_MODE_RGMII_RXID:
>  	case PHY_INTERFACE_MODE_RGMII_TXID:
> -		val = SYSCFG_PMCR_ETH_SEL_RGMII;
> +		val = dwmac->ops->pmcsetr.eth1_sel_rgmii | dwmac->ops->pmcsetr.eth2_sel_rgmii;
>  		if ((clk_rate == ETH_CK_F_25M || clk_rate == ETH_CK_F_125M) &&
>  		    (dwmac->eth_clk_sel_reg || dwmac->ext_phyclk)) {
>  			dwmac->enable_eth_ck = true;
> -			val |= SYSCFG_PMCR_ETH_CLK_SEL;
> +			val |= dwmac->ops->pmcsetr.eth1_clk_sel;
> +			val |= dwmac->ops->pmcsetr.eth2_clk_sel;
>  		}
>  		pr_debug("SYSCFG init : PHY_INTERFACE_MODE_RGMII\n");
>  		break;
> @@ -210,12 +219,12 @@ static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat)
>  	}
>  
>  	/* Need to update PMCCLRR (clear register) */
> -	regmap_write(dwmac->regmap, reg + SYSCFG_PMCCLRR_OFFSET,
> -		     dwmac->ops->syscfg_eth_mask);
> +	regmap_write(dwmac->regmap, reg + dwmac->ops->syscfg_clr_off,
> +		     dwmac->mode_mask);
>  
>  	/* Update PMCSETR (set register) */
>  	return regmap_update_bits(dwmac->regmap, reg,
> -				 dwmac->ops->syscfg_eth_mask, val);
> +				 dwmac->mode_mask, val);
>  }
>  
>  static int stm32mcu_set_mode(struct plat_stmmacenet_data *plat_dat)
> @@ -241,7 +250,7 @@ static int stm32mcu_set_mode(struct plat_stmmacenet_data *plat_dat)
>  	}
>  
>  	return regmap_update_bits(dwmac->regmap, reg,
> -				 dwmac->ops->syscfg_eth_mask, val << 23);
> +				 SYSCFG_MCU_ETH_MASK, val << 23);
>  }
>  
>  static void stm32_dwmac_clk_disable(struct stm32_dwmac *dwmac, bool suspend)
> @@ -286,10 +295,17 @@ static int stm32_dwmac_parse_data(struct stm32_dwmac *dwmac,
>  		return PTR_ERR(dwmac->regmap);
>  
>  	err = of_property_read_u32_index(np, "st,syscon", 1, &dwmac->mode_reg);
> +	if (err) {
> +		dev_err(dev, "Can't get sysconfig register offset (%d)\n", err);
> +		return err;
> +	}
> +
> +	dwmac->mode_mask = SYSCFG_MP1_ETH_MASK;
> +	err = of_property_read_u32_index(np, "st,syscon", 2, &dwmac->mode_mask);
>  	if (err)
> -		dev_err(dev, "Can't get sysconfig mode offset (%d)\n", err);
> +		pr_debug("Warning sysconfig register mask not set\n");
>  
> -	return err;
> +	return 0;
>  }
>  
>  static int stm32mp1_parse_data(struct stm32_dwmac *dwmac,
> @@ -478,8 +494,7 @@ static SIMPLE_DEV_PM_OPS(stm32_dwmac_pm_ops,
>  	stm32_dwmac_suspend, stm32_dwmac_resume);
>  
>  static struct stm32_ops stm32mcu_dwmac_data = {
> -	.set_mode = stm32mcu_set_mode,
> -	.syscfg_eth_mask = SYSCFG_MCU_ETH_MASK
> +	.set_mode = stm32mcu_set_mode
>  };
>  
>  static struct stm32_ops stm32mp1_dwmac_data = {
> @@ -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
> +	}
>  };
>  
>  static const struct of_device_id stm32_dwmac_match[] = {
> -- 
> 2.25.1
> 
>