| Message ID | 20240604143502.154463-8-christophe.roullier@foss.st.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Series to deliver Ethernet for STM32MP13 | expand |
On 6/4/24 4:34 PM, Christophe Roullier wrote: > Add Ethernet support for STM32MP13. > STM32MP13 is STM32 SOC with 2 GMACs instances. > GMAC IP version is SNPS 4.20. > GMAC IP configure with 1 RX and 1 TX queue. > DMA HW capability register supported > RX Checksum Offload Engine supported > TX Checksum insertion supported > Wake-Up On Lan supported > TSO supported > > Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com> > --- > .../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 50 +++++++++++++++---- > 1 file changed, 40 insertions(+), 10 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c > index bed2be129b2d2..e59f8a845e01e 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c > @@ -84,12 +84,14 @@ struct stm32_dwmac { > struct clk *clk_eth_ck; > struct clk *clk_ethstp; > struct clk *syscfg_clk; > + bool is_mp13; > int ext_phyclk; > int enable_eth_ck; > int eth_clk_sel_reg; > 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; > @@ -102,8 +104,8 @@ struct stm32_ops { > void (*resume)(struct stm32_dwmac *dwmac); > int (*parse_data)(struct stm32_dwmac *dwmac, > struct device *dev); > - u32 syscfg_eth_mask; > bool clk_rx_enable_in_suspend; > + u32 syscfg_clr_off; > }; > > static int stm32_dwmac_clk_enable(struct stm32_dwmac *dwmac, bool resume) > @@ -227,7 +229,14 @@ static int stm32mp1_configure_pmcr(struct plat_stmmacenet_data *plat_dat) > > switch (plat_dat->mac_interface) { > case PHY_INTERFACE_MODE_MII: > - val = SYSCFG_PMCR_ETH_SEL_MII; > + /* > + * STM32MP15xx supports both MII and GMII, STM32MP13xx MII only. > + * SYSCFG_PMCSETR ETH_SELMII is present only on STM32MP15xx and > + * acts as a selector between 0:GMII and 1:MII. As STM32MP13xx > + * supports only MII, ETH_SELMII is not present. > + */ > + if (!dwmac->is_mp13) /* Select MII mode on STM32MP15xx */ > + val |= SYSCFG_PMCR_ETH_SEL_MII; > break; > case PHY_INTERFACE_MODE_GMII: > val = SYSCFG_PMCR_ETH_SEL_GMII; > @@ -256,13 +265,16 @@ static int stm32mp1_configure_pmcr(struct plat_stmmacenet_data *plat_dat) > > dev_dbg(dwmac->dev, "Mode %s", phy_modes(plat_dat->mac_interface)); > > + /* Shift value at correct ethernet MAC offset in SYSCFG_PMCSETR */ > + val <<= ffs(dwmac->mode_mask) - ffs(SYSCFG_MP1_ETH_MASK); > + > /* Need to update PMCCLRR (clear register) */ > - regmap_write(dwmac->regmap, reg + SYSCFG_PMCCLRR_OFFSET, > - dwmac->ops->syscfg_eth_mask); > + regmap_write(dwmac->regmap, 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 stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat) > @@ -303,7 +315,7 @@ static int stm32mcu_set_mode(struct plat_stmmacenet_data *plat_dat) > dev_dbg(dwmac->dev, "Mode %s", phy_modes(plat_dat->mac_interface)); > > 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) > @@ -348,8 +360,15 @@ 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) > + if (err) { > dev_err(dev, "Can't get sysconfig mode 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) > + pr_debug("Warning sysconfig register mask not set\n"); I _think_ you need to left-shift the mode mask by 8 for STM32MP13xx second GMAC somewhere in here, right ? > return err; > } > @@ -361,6 +380,8 @@ static int stm32mp1_parse_data(struct stm32_dwmac *dwmac, > struct device_node *np = dev->of_node; > int err = 0; > > + dwmac->is_mp13 = of_device_is_compatible(np, "st,stm32mp13-dwmac"); You could make is_mp13 part of struct stm32_ops {} just like syscfg_clr_off is part of struct stm32_ops {} . > /* Ethernet PHY have no crystal */ > dwmac->ext_phyclk = of_property_read_bool(np, "st,ext-phyclk"); > > @@ -540,8 +561,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 It is not necessary to remove the trailing comma ',' > }; > > static struct stm32_ops stm32mp1_dwmac_data = { > @@ -549,13 +569,23 @@ 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, > + .syscfg_clr_off = 0x44, > + .clk_rx_enable_in_suspend = true > +}; > + > +static struct stm32_ops stm32mp13_dwmac_data = { > + .set_mode = stm32mp1_set_mode, > + .suspend = stm32mp1_suspend, > + .resume = stm32mp1_resume, > + .parse_data = stm32mp1_parse_data, > + .syscfg_clr_off = 0x08, > .clk_rx_enable_in_suspend = true > }; > > static const struct of_device_id stm32_dwmac_match[] = { > { .compatible = "st,stm32-dwmac", .data = &stm32mcu_dwmac_data}, > { .compatible = "st,stm32mp1-dwmac", .data = &stm32mp1_dwmac_data}, > + { .compatible = "st,stm32mp13-dwmac", .data = &stm32mp13_dwmac_data}, > { } > }; > MODULE_DEVICE_TABLE(of, stm32_dwmac_match); This patch definitely looks MUCH better than what this series started with, it is much easier to grasp the MP13 specific changes. You could possibly improve this further and split the dwmac->ops->syscfg_eth_mask to dwmac->mode_mask conversion into separate preparatory patch (as a 6.5/11 in context of this series), and then add the few MP13 changes on top (as 7/11 patch).
Hi Christophe, kernel test robot noticed the following build warnings: [auto build test WARNING on cd0057ad75116bacf16fea82e48c1db642971136] url: https://github.com/intel-lab-lkp/linux/commits/Christophe-Roullier/dt-bindings-net-add-STM32MP13-compatible-in-documentation-for-stm32/20240604-224324 base: cd0057ad75116bacf16fea82e48c1db642971136 patch link: https://lore.kernel.org/r/20240604143502.154463-8-christophe.roullier%40foss.st.com patch subject: [PATCH v4 07/11] net: ethernet: stmmac: add management of stm32mp13 for stm32 config: arm-defconfig (https://download.01.org/0day-ci/archive/20240605/202406050248.rGgTkevY-lkp@intel.com/config) compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240605/202406050248.rGgTkevY-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202406050248.rGgTkevY-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c:239:4: warning: variable 'val' is uninitialized when used here [-Wuninitialized] val |= SYSCFG_PMCR_ETH_SEL_MII; ^~~ drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c:228:9: note: initialize the variable 'val' to silence this warning int val; ^ = 0 1 warning generated. vim +/val +239 drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c 223 224 static int stm32mp1_configure_pmcr(struct plat_stmmacenet_data *plat_dat) 225 { 226 struct stm32_dwmac *dwmac = plat_dat->bsp_priv; 227 u32 reg = dwmac->mode_reg; 228 int val; 229 230 switch (plat_dat->mac_interface) { 231 case PHY_INTERFACE_MODE_MII: 232 /* 233 * STM32MP15xx supports both MII and GMII, STM32MP13xx MII only. 234 * SYSCFG_PMCSETR ETH_SELMII is present only on STM32MP15xx and 235 * acts as a selector between 0:GMII and 1:MII. As STM32MP13xx 236 * supports only MII, ETH_SELMII is not present. 237 */ 238 if (!dwmac->is_mp13) /* Select MII mode on STM32MP15xx */ > 239 val |= SYSCFG_PMCR_ETH_SEL_MII; 240 break; 241 case PHY_INTERFACE_MODE_GMII: 242 val = SYSCFG_PMCR_ETH_SEL_GMII; 243 if (dwmac->enable_eth_ck) 244 val |= SYSCFG_PMCR_ETH_CLK_SEL; 245 break; 246 case PHY_INTERFACE_MODE_RMII: 247 val = SYSCFG_PMCR_ETH_SEL_RMII; 248 if (dwmac->enable_eth_ck) 249 val |= SYSCFG_PMCR_ETH_REF_CLK_SEL; 250 break; 251 case PHY_INTERFACE_MODE_RGMII: 252 case PHY_INTERFACE_MODE_RGMII_ID: 253 case PHY_INTERFACE_MODE_RGMII_RXID: 254 case PHY_INTERFACE_MODE_RGMII_TXID: 255 val = SYSCFG_PMCR_ETH_SEL_RGMII; 256 if (dwmac->enable_eth_ck) 257 val |= SYSCFG_PMCR_ETH_CLK_SEL; 258 break; 259 default: 260 dev_err(dwmac->dev, "Mode %s not supported", 261 phy_modes(plat_dat->mac_interface)); 262 /* Do not manage others interfaces */ 263 return -EINVAL; 264 } 265 266 dev_dbg(dwmac->dev, "Mode %s", phy_modes(plat_dat->mac_interface)); 267 268 /* Shift value at correct ethernet MAC offset in SYSCFG_PMCSETR */ 269 val <<= ffs(dwmac->mode_mask) - ffs(SYSCFG_MP1_ETH_MASK); 270 271 /* Need to update PMCCLRR (clear register) */ 272 regmap_write(dwmac->regmap, dwmac->ops->syscfg_clr_off, 273 dwmac->mode_mask); 274 275 /* Update PMCSETR (set register) */ 276 return regmap_update_bits(dwmac->regmap, reg, 277 dwmac->mode_mask, val); 278 } 279
> Add Ethernet support for STM32MP13. > STM32MP13 is STM32 SOC with 2 GMACs instances. > GMAC IP version is SNPS 4.20. > GMAC IP configure with 1 RX and 1 TX queue. > DMA HW capability register supported > RX Checksum Offload Engine supported > TX Checksum insertion supported > Wake-Up On Lan supported > TSO supported > > Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com> > --- > .../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 50 +++++++++++++++---- > 1 file changed, 40 insertions(+), 10 deletions(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c > index bed2be129b2d2..e59f8a845e01e 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c > @@ -84,12 +84,14 @@ struct stm32_dwmac { > struct clk *clk_eth_ck; > struct clk *clk_ethstp; > struct clk *syscfg_clk; > + bool is_mp13; > int ext_phyclk; > int enable_eth_ck; > int eth_clk_sel_reg; > 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; > @@ -102,8 +104,8 @@ struct stm32_ops { > void (*resume)(struct stm32_dwmac *dwmac); > int (*parse_data)(struct stm32_dwmac *dwmac, > struct device *dev); > - u32 syscfg_eth_mask; > bool clk_rx_enable_in_suspend; > + u32 syscfg_clr_off; > }; > > static int stm32_dwmac_clk_enable(struct stm32_dwmac *dwmac, bool resume) > @@ -227,7 +229,14 @@ static int stm32mp1_configure_pmcr(struct plat_stmmacenet_data *plat_dat) > > switch (plat_dat->mac_interface) { > case PHY_INTERFACE_MODE_MII: > - val = SYSCFG_PMCR_ETH_SEL_MII; > + /* > + * STM32MP15xx supports both MII and GMII, STM32MP13xx MII only. > + * SYSCFG_PMCSETR ETH_SELMII is present only on STM32MP15xx and > + * acts as a selector between 0:GMII and 1:MII. As STM32MP13xx > + * supports only MII, ETH_SELMII is not present. > + */ > + if (!dwmac->is_mp13) /* Select MII mode on STM32MP15xx */ > + val |= SYSCFG_PMCR_ETH_SEL_MII; > break; > case PHY_INTERFACE_MODE_GMII: > val = SYSCFG_PMCR_ETH_SEL_GMII; > @@ -256,13 +265,16 @@ static int stm32mp1_configure_pmcr(struct plat_stmmacenet_data *plat_dat) > > dev_dbg(dwmac->dev, "Mode %s", phy_modes(plat_dat->mac_interface)); > > + /* Shift value at correct ethernet MAC offset in SYSCFG_PMCSETR */ > + val <<= ffs(dwmac->mode_mask) - ffs(SYSCFG_MP1_ETH_MASK); > + > /* Need to update PMCCLRR (clear register) */ > - regmap_write(dwmac->regmap, reg + SYSCFG_PMCCLRR_OFFSET, > - dwmac->ops->syscfg_eth_mask); > + regmap_write(dwmac->regmap, 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 stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat) > @@ -303,7 +315,7 @@ static int stm32mcu_set_mode(struct plat_stmmacenet_data *plat_dat) > dev_dbg(dwmac->dev, "Mode %s", phy_modes(plat_dat->mac_interface)); > > 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) > @@ -348,8 +360,15 @@ 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) > + if (err) { > dev_err(dev, "Can't get sysconfig mode offset (%d)\n", err); Shouldn't we decrement the refcount of np (of_node_put) before returning from this point? > + return err; > + } > + > + dwmac->mode_mask = SYSCFG_MP1_ETH_MASK; > + err = of_property_read_u32_index(np, "st,syscon", 2, &dwmac->mode_mask); > + if (err) > + pr_debug("Warning sysconfig register mask not set\n"); > > return err; > } > @@ -361,6 +380,8 @@ static int stm32mp1_parse_data(struct stm32_dwmac *dwmac, > struct device_node *np = dev->of_node; > int err = 0; > > + dwmac->is_mp13 = of_device_is_compatible(np, "st,stm32mp13-dwmac"); > + > /* Ethernet PHY have no crystal */ > dwmac->ext_phyclk = of_property_read_bool(np, "st,ext-phyclk"); > > @@ -540,8 +561,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 = { > @@ -549,13 +569,23 @@ 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, > + .syscfg_clr_off = 0x44, > + .clk_rx_enable_in_suspend = true > +}; > + > +static struct stm32_ops stm32mp13_dwmac_data = { > + .set_mode = stm32mp1_set_mode, > + .suspend = stm32mp1_suspend, > + .resume = stm32mp1_resume, > + .parse_data = stm32mp1_parse_data, > + .syscfg_clr_off = 0x08, > .clk_rx_enable_in_suspend = true > }; > > static const struct of_device_id stm32_dwmac_match[] = { > { .compatible = "st,stm32-dwmac", .data = &stm32mcu_dwmac_data}, > { .compatible = "st,stm32mp1-dwmac", .data = &stm32mp1_dwmac_data}, > + { .compatible = "st,stm32mp13-dwmac", .data = &stm32mp13_dwmac_data}, > { } > }; > MODULE_DEVICE_TABLE(of, stm32_dwmac_match);
On 6/4/24 19:05, Marek Vasut wrote: > On 6/4/24 4:34 PM, Christophe Roullier wrote: >> Add Ethernet support for STM32MP13. >> STM32MP13 is STM32 SOC with 2 GMACs instances. >> GMAC IP version is SNPS 4.20. >> GMAC IP configure with 1 RX and 1 TX queue. >> DMA HW capability register supported >> RX Checksum Offload Engine supported >> TX Checksum insertion supported >> Wake-Up On Lan supported >> TSO supported >> >> Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com> >> --- >> .../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 50 +++++++++++++++---- >> 1 file changed, 40 insertions(+), 10 deletions(-) >> >> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c >> b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c >> index bed2be129b2d2..e59f8a845e01e 100644 >> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c >> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c >> @@ -84,12 +84,14 @@ struct stm32_dwmac { >> struct clk *clk_eth_ck; >> struct clk *clk_ethstp; >> struct clk *syscfg_clk; >> + bool is_mp13; >> int ext_phyclk; >> int enable_eth_ck; >> int eth_clk_sel_reg; >> 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; >> @@ -102,8 +104,8 @@ struct stm32_ops { >> void (*resume)(struct stm32_dwmac *dwmac); >> int (*parse_data)(struct stm32_dwmac *dwmac, >> struct device *dev); >> - u32 syscfg_eth_mask; >> bool clk_rx_enable_in_suspend; >> + u32 syscfg_clr_off; >> }; >> static int stm32_dwmac_clk_enable(struct stm32_dwmac *dwmac, bool >> resume) >> @@ -227,7 +229,14 @@ static int stm32mp1_configure_pmcr(struct >> plat_stmmacenet_data *plat_dat) >> switch (plat_dat->mac_interface) { >> case PHY_INTERFACE_MODE_MII: >> - val = SYSCFG_PMCR_ETH_SEL_MII; >> + /* >> + * STM32MP15xx supports both MII and GMII, STM32MP13xx MII >> only. >> + * SYSCFG_PMCSETR ETH_SELMII is present only on STM32MP15xx and >> + * acts as a selector between 0:GMII and 1:MII. As STM32MP13xx >> + * supports only MII, ETH_SELMII is not present. >> + */ >> + if (!dwmac->is_mp13) /* Select MII mode on STM32MP15xx */ >> + val |= SYSCFG_PMCR_ETH_SEL_MII; >> break; >> case PHY_INTERFACE_MODE_GMII: >> val = SYSCFG_PMCR_ETH_SEL_GMII; >> @@ -256,13 +265,16 @@ static int stm32mp1_configure_pmcr(struct >> plat_stmmacenet_data *plat_dat) >> dev_dbg(dwmac->dev, "Mode %s", >> phy_modes(plat_dat->mac_interface)); >> + /* Shift value at correct ethernet MAC offset in >> SYSCFG_PMCSETR */ >> + val <<= ffs(dwmac->mode_mask) - ffs(SYSCFG_MP1_ETH_MASK); >> + >> /* Need to update PMCCLRR (clear register) */ >> - regmap_write(dwmac->regmap, reg + SYSCFG_PMCCLRR_OFFSET, >> - dwmac->ops->syscfg_eth_mask); >> + regmap_write(dwmac->regmap, 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 stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat) >> @@ -303,7 +315,7 @@ static int stm32mcu_set_mode(struct >> plat_stmmacenet_data *plat_dat) >> dev_dbg(dwmac->dev, "Mode %s", >> phy_modes(plat_dat->mac_interface)); >> 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) >> @@ -348,8 +360,15 @@ 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) >> + if (err) { >> dev_err(dev, "Can't get sysconfig mode 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) >> + pr_debug("Warning sysconfig register mask not set\n"); > > I _think_ you need to left-shift the mode mask by 8 for STM32MP13xx > second GMAC somewhere in here, right ? > The shift is performed in function stm32mp1_configure_pmcr: /* Shift value at correct ethernet MAC offset in SYSCFG_PMCSETR */ val <<= ffs(dwmac->mode_mask) - ffs(SYSCFG_MP1_ETH_MASK); In case of MP13 Ethernet1 or MP15, shift equal 0 In case of MP13 Ethernet2 , shift equal 8 ;-) >> return err; >> } >> @@ -361,6 +380,8 @@ static int stm32mp1_parse_data(struct stm32_dwmac >> *dwmac, >> struct device_node *np = dev->of_node; >> int err = 0; >> + dwmac->is_mp13 = of_device_is_compatible(np, >> "st,stm32mp13-dwmac"); > > You could make is_mp13 part of struct stm32_ops {} just like > syscfg_clr_off is part of struct stm32_ops {} . ok > >> /* Ethernet PHY have no crystal */ >> dwmac->ext_phyclk = of_property_read_bool(np, "st,ext-phyclk"); >> @@ -540,8 +561,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 > > It is not necessary to remove the trailing comma ',' ok > >> }; >> static struct stm32_ops stm32mp1_dwmac_data = { >> @@ -549,13 +569,23 @@ 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, >> + .syscfg_clr_off = 0x44, >> + .clk_rx_enable_in_suspend = true >> +}; >> + >> +static struct stm32_ops stm32mp13_dwmac_data = { >> + .set_mode = stm32mp1_set_mode, >> + .suspend = stm32mp1_suspend, >> + .resume = stm32mp1_resume, >> + .parse_data = stm32mp1_parse_data, >> + .syscfg_clr_off = 0x08, >> .clk_rx_enable_in_suspend = true >> }; >> static const struct of_device_id stm32_dwmac_match[] = { >> { .compatible = "st,stm32-dwmac", .data = &stm32mcu_dwmac_data}, >> { .compatible = "st,stm32mp1-dwmac", .data = >> &stm32mp1_dwmac_data}, >> + { .compatible = "st,stm32mp13-dwmac", .data = >> &stm32mp13_dwmac_data}, >> { } >> }; >> MODULE_DEVICE_TABLE(of, stm32_dwmac_match); > > This patch definitely looks MUCH better than what this series started > with, it is much easier to grasp the MP13 specific changes. > > You could possibly improve this further and split the > dwmac->ops->syscfg_eth_mask to dwmac->mode_mask conversion into > separate preparatory patch (as a 6.5/11 in context of this series), > and then add the few MP13 changes on top (as 7/11 patch). ok
On 6/6/24 4:19 PM, Christophe ROULLIER wrote: Hi, >>> @@ -348,8 +360,15 @@ 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) >>> + if (err) { >>> dev_err(dev, "Can't get sysconfig mode 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) >>> + pr_debug("Warning sysconfig register mask not set\n"); >> >> I _think_ you need to left-shift the mode mask by 8 for STM32MP13xx >> second GMAC somewhere in here, right ? >> > The shift is performed in function stm32mp1_configure_pmcr: > > /* Shift value at correct ethernet MAC offset in SYSCFG_PMCSETR */ > val <<= ffs(dwmac->mode_mask) - ffs(SYSCFG_MP1_ETH_MASK); > > In case of MP13 Ethernet1 or MP15, shift equal 0 > > In case of MP13 Ethernet2 , shift equal 8 ;-) Oh, good, thanks !
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c index bed2be129b2d2..e59f8a845e01e 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c @@ -84,12 +84,14 @@ struct stm32_dwmac { struct clk *clk_eth_ck; struct clk *clk_ethstp; struct clk *syscfg_clk; + bool is_mp13; int ext_phyclk; int enable_eth_ck; int eth_clk_sel_reg; 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; @@ -102,8 +104,8 @@ struct stm32_ops { void (*resume)(struct stm32_dwmac *dwmac); int (*parse_data)(struct stm32_dwmac *dwmac, struct device *dev); - u32 syscfg_eth_mask; bool clk_rx_enable_in_suspend; + u32 syscfg_clr_off; }; static int stm32_dwmac_clk_enable(struct stm32_dwmac *dwmac, bool resume) @@ -227,7 +229,14 @@ static int stm32mp1_configure_pmcr(struct plat_stmmacenet_data *plat_dat) switch (plat_dat->mac_interface) { case PHY_INTERFACE_MODE_MII: - val = SYSCFG_PMCR_ETH_SEL_MII; + /* + * STM32MP15xx supports both MII and GMII, STM32MP13xx MII only. + * SYSCFG_PMCSETR ETH_SELMII is present only on STM32MP15xx and + * acts as a selector between 0:GMII and 1:MII. As STM32MP13xx + * supports only MII, ETH_SELMII is not present. + */ + if (!dwmac->is_mp13) /* Select MII mode on STM32MP15xx */ + val |= SYSCFG_PMCR_ETH_SEL_MII; break; case PHY_INTERFACE_MODE_GMII: val = SYSCFG_PMCR_ETH_SEL_GMII; @@ -256,13 +265,16 @@ static int stm32mp1_configure_pmcr(struct plat_stmmacenet_data *plat_dat) dev_dbg(dwmac->dev, "Mode %s", phy_modes(plat_dat->mac_interface)); + /* Shift value at correct ethernet MAC offset in SYSCFG_PMCSETR */ + val <<= ffs(dwmac->mode_mask) - ffs(SYSCFG_MP1_ETH_MASK); + /* Need to update PMCCLRR (clear register) */ - regmap_write(dwmac->regmap, reg + SYSCFG_PMCCLRR_OFFSET, - dwmac->ops->syscfg_eth_mask); + regmap_write(dwmac->regmap, 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 stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat) @@ -303,7 +315,7 @@ static int stm32mcu_set_mode(struct plat_stmmacenet_data *plat_dat) dev_dbg(dwmac->dev, "Mode %s", phy_modes(plat_dat->mac_interface)); 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) @@ -348,8 +360,15 @@ 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) + if (err) { dev_err(dev, "Can't get sysconfig mode 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) + pr_debug("Warning sysconfig register mask not set\n"); return err; } @@ -361,6 +380,8 @@ static int stm32mp1_parse_data(struct stm32_dwmac *dwmac, struct device_node *np = dev->of_node; int err = 0; + dwmac->is_mp13 = of_device_is_compatible(np, "st,stm32mp13-dwmac"); + /* Ethernet PHY have no crystal */ dwmac->ext_phyclk = of_property_read_bool(np, "st,ext-phyclk"); @@ -540,8 +561,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 = { @@ -549,13 +569,23 @@ 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, + .syscfg_clr_off = 0x44, + .clk_rx_enable_in_suspend = true +}; + +static struct stm32_ops stm32mp13_dwmac_data = { + .set_mode = stm32mp1_set_mode, + .suspend = stm32mp1_suspend, + .resume = stm32mp1_resume, + .parse_data = stm32mp1_parse_data, + .syscfg_clr_off = 0x08, .clk_rx_enable_in_suspend = true }; static const struct of_device_id stm32_dwmac_match[] = { { .compatible = "st,stm32-dwmac", .data = &stm32mcu_dwmac_data}, { .compatible = "st,stm32mp1-dwmac", .data = &stm32mp1_dwmac_data}, + { .compatible = "st,stm32mp13-dwmac", .data = &stm32mp13_dwmac_data}, { } }; MODULE_DEVICE_TABLE(of, stm32_dwmac_match);
Add Ethernet support for STM32MP13. STM32MP13 is STM32 SOC with 2 GMACs instances. GMAC IP version is SNPS 4.20. GMAC IP configure with 1 RX and 1 TX queue. DMA HW capability register supported RX Checksum Offload Engine supported TX Checksum insertion supported Wake-Up On Lan supported TSO supported Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com> --- .../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 50 +++++++++++++++---- 1 file changed, 40 insertions(+), 10 deletions(-)