1. Patchew
  2. linux
  3. [PATCH net-next v2 0/2] Refine stmmac code
  4. View patch

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

Tiezhu Yang posted 2 patches 2 months, 2 weeks ago
There is a newer version of this series
[PATCH net-next v2 1/2] net: stmmac: Return early if invalid in loongson_dwmac_fix_reset()
Posted by Tiezhu Yang 2 months, 2 weeks ago 
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
Re: [PATCH net-next v2 1/2] net: stmmac: Return early if invalid in loongson_dwmac_fix_reset()
Posted by Andrew Lunn 2 months, 2 weeks ago 
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()
Posted by Tiezhu Yang 2 months, 2 weeks ago 
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()
Posted by Tiezhu Yang 2 months, 1 week ago 
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()
Posted by Andrew Lunn 2 months, 1 week ago 
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

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.