| Message ID | 20241118060207.141048-4-jacky_chou@aspeedtech.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Add Aspeed G7 FTGMAC100 support | expand |
On Mon, Nov 18, 2024, at 07:02, Jacky Chou wrote: > @@ -1969,10 +1971,29 @@ static int ftgmac100_probe(struct platform_device *pdev) > } > > if (priv->is_aspeed) { > + struct reset_control *rst; > + > err = ftgmac100_setup_clk(priv); > if (err) > goto err_phy_connect; > > + rst = devm_reset_control_get_optional(priv->dev, NULL); > + if (IS_ERR(rst)) > + goto err_register_netdev; > + priv->rst = rst; > + > + err = reset_control_assert(priv->rst); Since that reset line is optional, how about making it part of the normal probe procedure, not just the if(aspeed) section? It seems this does nothing for older devices but may help for future ones regardless of the SoC family. Arnd
Hi Arnd, Thank you for you reply. > > } > > > > if (priv->is_aspeed) { > > + struct reset_control *rst; > > + > > err = ftgmac100_setup_clk(priv); > > if (err) > > goto err_phy_connect; > > > > + rst = devm_reset_control_get_optional(priv->dev, NULL); > > + if (IS_ERR(rst)) > > + goto err_register_netdev; > > + priv->rst = rst; > > + > > + err = reset_control_assert(priv->rst); > > Since that reset line is optional, how about making it part of the normal probe > procedure, not just the if(aspeed) section? It seems this does nothing for older > devices but may help for future ones regardless of the SoC family. Agree. Because it is optional, even if reset line does not exist on other SoCs, it will not affect the behavior of the code. Thank you for pointing this out. I will adjust it to normal probe procedure in next version. Or I will separate it from Aspeed AST2700 support series. Jacky
On Mo, 2024-11-18 at 14:02 +0800, Jacky Chou wrote: > Toggle the SCU reset before hardware initialization. > > Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com> > --- > drivers/net/ethernet/faraday/ftgmac100.c | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c > index 17ec35e75a65..cae23b712a6d 100644 > --- a/drivers/net/ethernet/faraday/ftgmac100.c > +++ b/drivers/net/ethernet/faraday/ftgmac100.c > @@ -9,6 +9,7 @@ > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > #include <linux/clk.h> > +#include <linux/reset.h> > #include <linux/dma-mapping.h> > #include <linux/etherdevice.h> > #include <linux/ethtool.h> > @@ -98,6 +99,7 @@ struct ftgmac100 { > struct work_struct reset_task; > struct mii_bus *mii_bus; > struct clk *clk; > + struct reset_control *rst; > > /* AST2500/AST2600 RMII ref clock gate */ > struct clk *rclk; > @@ -1969,10 +1971,29 @@ static int ftgmac100_probe(struct platform_device *pdev) > } > > if (priv->is_aspeed) { > + struct reset_control *rst; > + > err = ftgmac100_setup_clk(priv); > if (err) > goto err_phy_connect; > > + rst = devm_reset_control_get_optional(priv->dev, NULL); Please use devm_reset_control_get_optional_exclusive() directly. regards Philipp
Hi Philipp Thank you for your reply. > > Toggle the SCU reset before hardware initialization. > > > > Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com> > > --- > > drivers/net/ethernet/faraday/ftgmac100.c | 21 +++++++++++++++++++++ > > 1 file changed, 21 insertions(+) > > > > diff --git a/drivers/net/ethernet/faraday/ftgmac100.c > b/drivers/net/ethernet/faraday/ftgmac100.c > > index 17ec35e75a65..cae23b712a6d 100644 > > --- a/drivers/net/ethernet/faraday/ftgmac100.c > > +++ b/drivers/net/ethernet/faraday/ftgmac100.c > > @@ -9,6 +9,7 @@ > > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > > > #include <linux/clk.h> > > +#include <linux/reset.h> > > #include <linux/dma-mapping.h> > > #include <linux/etherdevice.h> > > #include <linux/ethtool.h> > > @@ -98,6 +99,7 @@ struct ftgmac100 { > > struct work_struct reset_task; > > struct mii_bus *mii_bus; > > struct clk *clk; > > + struct reset_control *rst; > > > > /* AST2500/AST2600 RMII ref clock gate */ > > struct clk *rclk; > > @@ -1969,10 +1971,29 @@ static int ftgmac100_probe(struct > platform_device *pdev) > > } > > > > if (priv->is_aspeed) { > > + struct reset_control *rst; > > + > > err = ftgmac100_setup_clk(priv); > > if (err) > > goto err_phy_connect; > > > > + rst = devm_reset_control_get_optional(priv->dev, NULL); > > Please use devm_reset_control_get_optional_exclusive() directly. Got it. Thanks, Jacky
diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c index 17ec35e75a65..cae23b712a6d 100644 --- a/drivers/net/ethernet/faraday/ftgmac100.c +++ b/drivers/net/ethernet/faraday/ftgmac100.c @@ -9,6 +9,7 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt #include <linux/clk.h> +#include <linux/reset.h> #include <linux/dma-mapping.h> #include <linux/etherdevice.h> #include <linux/ethtool.h> @@ -98,6 +99,7 @@ struct ftgmac100 { struct work_struct reset_task; struct mii_bus *mii_bus; struct clk *clk; + struct reset_control *rst; /* AST2500/AST2600 RMII ref clock gate */ struct clk *rclk; @@ -1969,10 +1971,29 @@ static int ftgmac100_probe(struct platform_device *pdev) } if (priv->is_aspeed) { + struct reset_control *rst; + err = ftgmac100_setup_clk(priv); if (err) goto err_phy_connect; + rst = devm_reset_control_get_optional(priv->dev, NULL); + if (IS_ERR(rst)) + goto err_register_netdev; + priv->rst = rst; + + err = reset_control_assert(priv->rst); + if (err) { + dev_err(priv->dev, "Failed to reset mac (%d)\n", err); + goto err_register_netdev; + } + usleep_range(10000, 20000); + err = reset_control_deassert(priv->rst); + if (err) { + dev_err(priv->dev, "Failed to deassert mac reset (%d)\n", err); + goto err_register_netdev; + } + /* Disable ast2600 problematic HW arbitration */ if (of_device_is_compatible(np, "aspeed,ast2600-mac")) iowrite32(FTGMAC100_TM_DEFAULT,
Toggle the SCU reset before hardware initialization. Signed-off-by: Jacky Chou <jacky_chou@aspeedtech.com> --- drivers/net/ethernet/faraday/ftgmac100.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)