[PATCH net-next v2 0/2] Refine stmmac code

[PATCH net-next v2 0/2] Refine stmmac code

[Tiezhu Yang]

Here are two small patches to refine stmmac code when debugging and
testing the problem "Failed to reset the dma".

v2:
  -- Update the commit message of patch #1 to explain the background.
  -- Add Reviewed-by tag for patch #2, no code changes.

Tiezhu Yang (2):
  net: stmmac: Return early if invalid in loongson_dwmac_fix_reset()
  net: stmmac: Check stmmac_hw_setup() in stmmac_resume()

 drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c | 3 +++
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c    | 9 ++++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

-- 
2.42.0

[PATCH net-next v2 1/2] net: stmmac: Return early if invalid in loongson_dwmac_fix_reset()

[Tiezhu Yang]

If the MAC controller does not connect to any PHY interface, there is a
missing clock, then the DMA reset fails.

For this case, the DMA_BUS_MODE_SFT_RESET bit is 1 before software reset,
just return -EINVAL immediately to avoid waiting for the timeout when the
DMA reset fails in loongson_dwmac_fix_reset().

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
index e1591e6217d4..6d10077666c7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
@@ -513,6 +513,9 @@ static int loongson_dwmac_fix_reset(void *priv, void __iomem *ioaddr)
 {
 	u32 value = readl(ioaddr + DMA_BUS_MODE);
 
+	if (value & DMA_BUS_MODE_SFT_RESET)
+		return -EINVAL;
+
 	value |= DMA_BUS_MODE_SFT_RESET;
 	writel(value, ioaddr + DMA_BUS_MODE);
 
-- 
2.42.0

[PATCH net-next v2 2/2] net: stmmac: Check stmmac_hw_setup() in stmmac_resume()

[Tiezhu Yang]

stmmac_hw_setup() may return 0 on success and an appropriate negative
integer as defined in errno.h file on failure, just check it and then
return early if failed in stmmac_resume().

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Huacai Chen <chenhuacai@loongson.cn>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index b948df1bff9a..2bfacab71ab9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -7975,7 +7975,14 @@ int stmmac_resume(struct device *dev)
 	stmmac_free_tx_skbufs(priv);
 	stmmac_clear_descriptors(priv, &priv->dma_conf);
 
-	stmmac_hw_setup(ndev, false);
+	ret = stmmac_hw_setup(ndev, false);
+	if (ret < 0) {
+		netdev_err(priv->dev, "%s: Hw setup failed\n", __func__);
+		mutex_unlock(&priv->lock);
+		rtnl_unlock();
+		return ret;
+	}
+
 	stmmac_init_coalesce(priv);
 	phylink_rx_clk_stop_block(priv->phylink);
 	stmmac_set_rx_mode(ndev);
-- 
2.42.0

Re: [PATCH net-next v2 1/2] net: stmmac: Return early if invalid in loongson_dwmac_fix_reset()

[Andrew Lunn]

On Wed, Jul 23, 2025 at 06:00:55PM +0800, Tiezhu Yang wrote:
> If the MAC controller does not connect to any PHY interface, there is a
> missing clock, then the DMA reset fails.
> 
> For this case, the DMA_BUS_MODE_SFT_RESET bit is 1 before software reset,
> just return -EINVAL immediately to avoid waiting for the timeout when the
> DMA reset fails in loongson_dwmac_fix_reset().
> 
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> ---
>  drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> index e1591e6217d4..6d10077666c7 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> @@ -513,6 +513,9 @@ static int loongson_dwmac_fix_reset(void *priv, void __iomem *ioaddr)
>  {
>  	u32 value = readl(ioaddr + DMA_BUS_MODE);
>  
> +	if (value & DMA_BUS_MODE_SFT_RESET)
> +		return -EINVAL;

What happens with this return value? Do you get an error message which
gives a hint the PHY clock is missing? Would a netdev_err() make sense
here?

	Andrew

Re: [PATCH net-next v2 1/2] net: stmmac: Return early if invalid in loongson_dwmac_fix_reset()

[Tiezhu Yang]

On 2025/7/23 下午10:53, Andrew Lunn wrote:
> On Wed, Jul 23, 2025 at 06:00:55PM +0800, Tiezhu Yang wrote:
>> If the MAC controller does not connect to any PHY interface, there is a
>> missing clock, then the DMA reset fails.
>>
>> For this case, the DMA_BUS_MODE_SFT_RESET bit is 1 before software reset,
>> just return -EINVAL immediately to avoid waiting for the timeout when the
>> DMA reset fails in loongson_dwmac_fix_reset().
>>
>> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
>> ---
>>   drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
>> index e1591e6217d4..6d10077666c7 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
>> @@ -513,6 +513,9 @@ static int loongson_dwmac_fix_reset(void *priv, void __iomem *ioaddr)
>>   {
>>   	u32 value = readl(ioaddr + DMA_BUS_MODE);
>>   
>> +	if (value & DMA_BUS_MODE_SFT_RESET)
>> +		return -EINVAL;
> 
> What happens with this return value? Do you get an error message which
> gives a hint the PHY clock is missing? Would a netdev_err() make sense
> here?

Yes, I will use dev_err() rather than netdev_err() (because there is no
net_device member here) to do something like this:

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
index 6d10077666c7..4a7b2b11ecce 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
@@ -513,8 +513,11 @@ static int loongson_dwmac_fix_reset(void *priv, 
void __iomem *ioaddr)
  {
         u32 value = readl(ioaddr + DMA_BUS_MODE);

-       if (value & DMA_BUS_MODE_SFT_RESET)
+       if (value & DMA_BUS_MODE_SFT_RESET) {
+               struct plat_stmmacenet_data *plat = priv;
+               dev_err(&plat->pdev->dev, "the PHY clock is missing\n");
                 return -EINVAL;
+       }

         value |= DMA_BUS_MODE_SFT_RESET;
         writel(value, ioaddr + DMA_BUS_MODE);

Thanks,
Tiezhu

Re: [PATCH net-next v2 1/2] net: stmmac: Return early if invalid in loongson_dwmac_fix_reset()

[Tiezhu Yang]

On 2025/7/24 上午10:26, Tiezhu Yang wrote:
> On 2025/7/23 下午10:53, Andrew Lunn wrote:
>> On Wed, Jul 23, 2025 at 06:00:55PM +0800, Tiezhu Yang wrote:
>>> If the MAC controller does not connect to any PHY interface, there is a
>>> missing clock, then the DMA reset fails.

...

>>> +    if (value & DMA_BUS_MODE_SFT_RESET)
>>> +        return -EINVAL;
>>
>> What happens with this return value? Do you get an error message which
>> gives a hint the PHY clock is missing? Would a netdev_err() make sense
>> here?
> 
> Yes, I will use dev_err() rather than netdev_err() (because there is no
> net_device member here) to do something like this:
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c 
> b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> index 6d10077666c7..4a7b2b11ecce 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> @@ -513,8 +513,11 @@ static int loongson_dwmac_fix_reset(void *priv, 
> void __iomem *ioaddr)
>   {
>          u32 value = readl(ioaddr + DMA_BUS_MODE);
> 
> -       if (value & DMA_BUS_MODE_SFT_RESET)
> +       if (value & DMA_BUS_MODE_SFT_RESET) {
> +               struct plat_stmmacenet_data *plat = priv;
> +               dev_err(&plat->pdev->dev, "the PHY clock is missing\n");
>                  return -EINVAL;
> +       }
> 
>          value |= DMA_BUS_MODE_SFT_RESET;
>          writel(value, ioaddr + DMA_BUS_MODE);

Oops, the above changes can not work well.

It can not use netdev_err() or dev_err() to print message with device info
in loongson_dwmac_fix_reset() directly, this is because the type of "priv"
argument is struct plat_stmmacenet_data and the "pdev" member of "priv" is
NULL here, it will lead to the fatal error "Unable to handle kernel paging
request at virtual address" when printing message.

Based on the above analysis, in order to show an error message which gives
a hint the PHY clock is missing, it is proper to check the return value of
stmmac_reset() which calls loongson_dwmac_fix_reset().

With this patch, for the normal end user, the computer start faster with
reducing boot time for 2 seconds on the specified mainboard.

The final changes look something like this:

----->8-----
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
index e1591e6217d4..6d10077666c7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
@@ -513,6 +513,9 @@ static int loongson_dwmac_fix_reset(void *priv, void 
__iomem *ioaddr)
  {
         u32 value = readl(ioaddr + DMA_BUS_MODE);

+       if (value & DMA_BUS_MODE_SFT_RESET)
+               return -EINVAL;
+
         value |= DMA_BUS_MODE_SFT_RESET;
         writel(value, ioaddr + DMA_BUS_MODE);

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index b948df1bff9a..1a2610815847 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3133,6 +3133,9 @@ static int stmmac_init_dma_engine(struct 
stmmac_priv *priv)

         ret = stmmac_reset(priv, priv->ioaddr);
         if (ret) {
+               if (ret == -EINVAL)
+                       netdev_err(priv->dev, "the PHY clock is missing\n");
+
                 netdev_err(priv->dev, "Failed to reset the dma\n");
                 return ret;
         }

I will wait for more comments and send v3 after the merge window.

Thanks,
Tiezhu

Re: [PATCH net-next v2 1/2] net: stmmac: Return early if invalid in loongson_dwmac_fix_reset()

[Andrew Lunn]

On Thu, Jul 24, 2025 at 05:05:57PM +0800, Tiezhu Yang wrote:
> On 2025/7/24 上午10:26, Tiezhu Yang wrote:
> > On 2025/7/23 下午10:53, Andrew Lunn wrote:
> > > On Wed, Jul 23, 2025 at 06:00:55PM +0800, Tiezhu Yang wrote:
> > > > If the MAC controller does not connect to any PHY interface, there is a
> > > > missing clock, then the DMA reset fails.
> 
> ...
> 
> > > > +    if (value & DMA_BUS_MODE_SFT_RESET)
> > > > +        return -EINVAL;
> > > 
> > > What happens with this return value? Do you get an error message which
> > > gives a hint the PHY clock is missing? Would a netdev_err() make sense
> > > here?
> > 
> > Yes, I will use dev_err() rather than netdev_err() (because there is no
> > net_device member here) to do something like this:
> > 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> > b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> > index 6d10077666c7..4a7b2b11ecce 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> > @@ -513,8 +513,11 @@ static int loongson_dwmac_fix_reset(void *priv,
> > void __iomem *ioaddr)
> >   {
> >          u32 value = readl(ioaddr + DMA_BUS_MODE);
> > 
> > -       if (value & DMA_BUS_MODE_SFT_RESET)
> > +       if (value & DMA_BUS_MODE_SFT_RESET) {
> > +               struct plat_stmmacenet_data *plat = priv;
> > +               dev_err(&plat->pdev->dev, "the PHY clock is missing\n");
> >                  return -EINVAL;
> > +       }
> > 
> >          value |= DMA_BUS_MODE_SFT_RESET;
> >          writel(value, ioaddr + DMA_BUS_MODE);
> 
> Oops, the above changes can not work well.
> 
> It can not use netdev_err() or dev_err() to print message with device info
> in loongson_dwmac_fix_reset() directly, this is because the type of "priv"
> argument is struct plat_stmmacenet_data and the "pdev" member of "priv" is
> NULL here, it will lead to the fatal error "Unable to handle kernel paging
> request at virtual address" when printing message.

Maybe it would be better to change fix_soc_reset() to have struct
stmmac_priv * as its first parameter. There are not too many user of
it, so it is not too big a change.

>         ret = stmmac_reset(priv, priv->ioaddr);
>         if (ret) {
> +               if (ret == -EINVAL)
> +                       netdev_err(priv->dev, "the PHY clock is missing\n");
> +

The problem with this is, you have no idea if EINVAL might of come
from somewhere else.

	Andrew