[PATCH net 0/3] net: stmmac: Fix usage of maximum queue number macros

[PATCH net 0/3] net: stmmac: Fix usage of maximum queue number macros

[Kunihiko Hayashi]

The maximum number of Rx and Tx queues is defined by MTL_MAX_RX_QUEUES and
MTL_MAX_TX_QUEUES respectively.

There are some places where Rx and Tx are used in reverse. Currently these
two values as the same and there is no impact, but need to fix the usage
to keep consistency.

Kunihiko Hayashi (3):
  net: stmmac: Fix use of queue max macros for Rx interrupt name
  net: stmmac: Fix use of queue max macros for Rx coalesce
  net: stmmac: Fix use of queue max macros for irq statistics

 drivers/net/ethernet/stmicro/stmmac/common.h | 4 ++--
 drivers/net/ethernet/stmicro/stmmac/stmmac.h | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

-- 
2.25.1

[PATCH net 1/3] net: stmmac: Fix use of queue max macros for Rx interrupt name

[Kunihiko Hayashi]

Fix macros for maximum queue number to keep consistency.

Fixes: 8532f613bc78 ("net: stmmac: introduce MSI Interrupt routines for mac, safety, RX & TX")
Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index ea135203ff2e..86c071baedf2 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -354,7 +354,7 @@ struct stmmac_priv {
 	char int_name_sfty[IFNAMSIZ + 10];
 	char int_name_sfty_ce[IFNAMSIZ + 10];
 	char int_name_sfty_ue[IFNAMSIZ + 10];
-	char int_name_rx_irq[MTL_MAX_TX_QUEUES][IFNAMSIZ + 14];
+	char int_name_rx_irq[MTL_MAX_RX_QUEUES][IFNAMSIZ + 14];
 	char int_name_tx_irq[MTL_MAX_TX_QUEUES][IFNAMSIZ + 18];
 
 #ifdef CONFIG_DEBUG_FS
-- 
2.25.1

[PATCH net 2/3] net: stmmac: Fix use of queue max macros for Rx coalesce

[Kunihiko Hayashi]

Fix macros for maximum queue number to keep consistency.

Fixes: db2f2842e6f5 ("net: stmmac: add per-queue TX & RX coalesce ethtool support")
Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index 86c071baedf2..1793ddd69164 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -265,7 +265,7 @@ struct stmmac_priv {
 	/* Frequently used values are kept adjacent for cache effect */
 	u32 tx_coal_frames[MTL_MAX_TX_QUEUES];
 	u32 tx_coal_timer[MTL_MAX_TX_QUEUES];
-	u32 rx_coal_frames[MTL_MAX_TX_QUEUES];
+	u32 rx_coal_frames[MTL_MAX_RX_QUEUES];
 
 	int hwts_tx_en;
 	bool tx_path_in_lpi_mode;
@@ -275,7 +275,7 @@ struct stmmac_priv {
 	u32 sarc_type;
 
 	unsigned int rx_copybreak;
-	u32 rx_riwt[MTL_MAX_TX_QUEUES];
+	u32 rx_riwt[MTL_MAX_RX_QUEUES];
 	int hwts_rx_en;
 
 	void __iomem *ioaddr;
-- 
2.25.1

[PATCH net 3/3] net: stmmac: Fix use of queue max macros for irq statistics

[Kunihiko Hayashi]

Fix macros for maximum queue number to keep consistency.

Fixes: 38cc3c6dcc09 ("net: stmmac: protect updates of 64-bit statistics counters")
Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
 drivers/net/ethernet/stmicro/stmmac/common.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h b/drivers/net/ethernet/stmicro/stmmac/common.h
index 684489156dce..b6966218cb77 100644
--- a/drivers/net/ethernet/stmicro/stmmac/common.h
+++ b/drivers/net/ethernet/stmicro/stmmac/common.h
@@ -101,8 +101,8 @@ struct stmmac_rxq_stats {
 /* Updates on each CPU protected by not allowing nested irqs. */
 struct stmmac_pcpu_stats {
 	struct u64_stats_sync syncp;
-	u64_stats_t rx_normal_irq_n[MTL_MAX_TX_QUEUES];
-	u64_stats_t tx_normal_irq_n[MTL_MAX_RX_QUEUES];
+	u64_stats_t rx_normal_irq_n[MTL_MAX_RX_QUEUES];
+	u64_stats_t tx_normal_irq_n[MTL_MAX_TX_QUEUES];
 };
 
 /* Extra statistic and debug information exposed by ethtool */
-- 
2.25.1

Re: [PATCH net 0/3] net: stmmac: Fix usage of maximum queue number macros

[Russell King (Oracle)]

On Mon, Jan 27, 2025 at 06:24:47PM +0900, Kunihiko Hayashi wrote:
> The maximum number of Rx and Tx queues is defined by MTL_MAX_RX_QUEUES and
> MTL_MAX_TX_QUEUES respectively.
> 
> There are some places where Rx and Tx are used in reverse. Currently these
> two values as the same and there is no impact, but need to fix the usage
> to keep consistency.

I disagree that this should be targetting the net tree - I think it
should be the net-next tree. Nothing is currently broken, this isn't
fixing a regression, there is no urgent need to get it into mainline.
It is merely a cleanup because both macros have the same value:

include/linux/stmmac.h:#define MTL_MAX_RX_QUEUES        8
include/linux/stmmac.h:#define MTL_MAX_TX_QUEUES        8

Please re-send for net-next after the merge window and net-next has
re-opened.

In any case, for the whole series:

Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Thanks!

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net 0/3] net: stmmac: Fix usage of maximum queue number macros

[Huacai Chen]

Hi, Russell,

On Mon, Jan 27, 2025 at 5:52 PM Russell King (Oracle)
<linux@armlinux.org.uk> wrote:
>
> On Mon, Jan 27, 2025 at 06:24:47PM +0900, Kunihiko Hayashi wrote:
> > The maximum number of Rx and Tx queues is defined by MTL_MAX_RX_QUEUES and
> > MTL_MAX_TX_QUEUES respectively.
> >
> > There are some places where Rx and Tx are used in reverse. Currently these
> > two values as the same and there is no impact, but need to fix the usage
> > to keep consistency.
>
> I disagree that this should be targetting the net tree - I think it
> should be the net-next tree. Nothing is currently broken, this isn't
> fixing a regression, there is no urgent need to get it into mainline.
> It is merely a cleanup because both macros have the same value:
I'm not very familiar with the difference between net and net-next,
but I think this series should be backported to stable branches.

Reviewed-by: Huacai Chen <chenhuacai@kernel.org>

Huacai

>
> include/linux/stmmac.h:#define MTL_MAX_RX_QUEUES        8
> include/linux/stmmac.h:#define MTL_MAX_TX_QUEUES        8
>
> Please re-send for net-next after the merge window and net-next has
> re-opened.
>
> In any case, for the whole series:
>
> Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
>
> Thanks!
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
>

Re: [PATCH net 0/3] net: stmmac: Fix usage of maximum queue number macros

[Andrew Lunn]

> I'm not very familiar with the difference between net and net-next,
> but I think this series should be backported to stable branches.

https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html


  It must either fix a real bug that bothers people or just add a
  device ID.

Does this really bother people? Have we seen bug reports?

There is another aspect to this. We are adding warnings saying that
the device tree blob is broken. That should encourage users to upgrade
their device tree blob. But most won't find any newer version. If this
goes into net-next, the roll out will be a lot slower, developers on
the leading edge will find the DT issue and submit a DT patch. By the
time this is in a distro kernel, maybe most of the DT issues will
already be fixed?

	Andrew

Re: [PATCH net 0/3] net: stmmac: Fix usage of maximum queue number macros

[Huacai Chen]

Hi, Andrew,

On Mon, Jan 27, 2025 at 9:21 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > I'm not very familiar with the difference between net and net-next,
> > but I think this series should be backported to stable branches.
>
> https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
According to the rules a "bug" should break build or break runtime or
a security issue, but shouldn't be spelling fixes.

But from my point of view, this series is not just "spelling fixes",
and not "trivial fixes without benefit for users". It is obviously a
copy-paste error and may confuse developers, so I think the patches
really have "benefits".

>
>
>   It must either fix a real bug that bothers people or just add a
>   device ID.
>
> Does this really bother people? Have we seen bug reports?
No bug report is because MTL_MAX_RX_QUEUES is accidentally equal to
MTL_MAX_TX_QUEUES and it just works, not because the logic is correct.
And Kunihiko's patch can also be treated as a report.

>
> There is another aspect to this. We are adding warnings saying that
> the device tree blob is broken. That should encourage users to upgrade
> their device tree blob. But most won't find any newer version. If this
> goes into net-next, the roll out will be a lot slower, developers on
> the leading edge will find the DT issue and submit a DT patch. By the
> time this is in a distro kernel, maybe most of the DT issues will
> already be fixed?
Goto net or goto net-next are both fine to me, I just think this
series should be backported to stable branches. There are lots of
patches backported even though they are less important than this
series (maybe not in the network subsystem).

Huacai
>
>         Andrew

Re: [PATCH net 0/3] net: stmmac: Fix usage of maximum queue number macros

[Jakub Kicinski]

On Mon, 27 Jan 2025 21:47:29 +0800 Huacai Chen wrote:
> > There is another aspect to this. We are adding warnings saying that
> > the device tree blob is broken. That should encourage users to upgrade
> > their device tree blob. But most won't find any newer version. If this
> > goes into net-next, the roll out will be a lot slower, developers on
> > the leading edge will find the DT issue and submit a DT patch. By the
> > time this is in a distro kernel, maybe most of the DT issues will
> > already be fixed?  
> Goto net or goto net-next are both fine to me, I just think this
> series should be backported to stable branches. There are lots of
> patches backported even though they are less important than this
> series (maybe not in the network subsystem).

Please remove the Fixes tags when reposting.
-- 
pw-bot: defer

Re: [PATCH net 0/3] net: stmmac: Fix usage of maximum queue number macros

[Kunihiko Hayashi]

Hi Russell,

On 2025/01/27 18:51, Russell King (Oracle) wrote:
> On Mon, Jan 27, 2025 at 06:24:47PM +0900, Kunihiko Hayashi wrote:
>> The maximum number of Rx and Tx queues is defined by MTL_MAX_RX_QUEUES
> and
>> MTL_MAX_TX_QUEUES respectively.
>>
>> There are some places where Rx and Tx are used in reverse. Currently
> these
>> two values as the same and there is no impact, but need to fix the usage
>> to keep consistency.
> 
> I disagree that this should be targetting the net tree - I think it
> should be the net-next tree. Nothing is currently broken, this isn't
> fixing a regression, there is no urgent need to get it into mainline.
> It is merely a cleanup because both macros have the same value:
> 
> include/linux/stmmac.h:#define MTL_MAX_RX_QUEUES        8
> include/linux/stmmac.h:#define MTL_MAX_TX_QUEUES        8

I was a bit confused about how to choose net and net-next in this case,
but I understand what you are saying.

As I wrote:
>> Currently these two values as the same and there is no impact

this case isn't about fixing what is broken and also not required fixes
for the stable kernel, so I should post this series to net-next without
Fixes: tag.

> Please re-send for net-next after the merge window and net-next has
> re-opened.

I see. I'll take care and repost.

Thank you,

---
Best Regards
Kunihiko Hayashi

Re: [PATCH net 0/3] net: stmmac: Fix usage of maximum queue number macros

[Kunihiko Hayashi]

Hi Huacai, Andrew,

On 2025/01/27 22:47, Huacai Chen wrote:
> Hi, Andrew,
> 
> On Mon, Jan 27, 2025 at 9:21 PM Andrew Lunn <andrew@lunn.ch> wrote:
>>
>>> I'm not very familiar with the difference between net and net-next,
>>> but I think this series should be backported to stable branches.
>>
>> https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> According to the rules a "bug" should break build or break runtime or
> a security issue, but shouldn't be spelling fixes.
> 
> But from my point of view, this series is not just "spelling fixes",
> and not "trivial fixes without benefit for users". It is obviously a
> copy-paste error and may confuse developers, so I think the patches
> really have "benefits".
> 
>>
>>
>>    It must either fix a real bug that bothers people or just add a
>>    device ID.
>>
>> Does this really bother people? Have we seen bug reports?
> No bug report is because MTL_MAX_RX_QUEUES is accidentally equal to
> MTL_MAX_TX_QUEUES and it just works, not because the logic is correct.
> And Kunihiko's patch can also be treated as a report.
> 
>>
>> There is another aspect to this. We are adding warnings saying that
>> the device tree blob is broken. That should encourage users to upgrade
>> their device tree blob. But most won't find any newer version. If this
>> goes into net-next, the roll out will be a lot slower, developers on
>> the leading edge will find the DT issue and submit a DT patch. By the
>> time this is in a distro kernel, maybe most of the DT issues will
>> already be fixed?
> Goto net or goto net-next are both fine to me, I just think this
> series should be backported to stable branches. There are lots of
> patches backported even though they are less important than this
> series (maybe not in the network subsystem).

Currently both macros define the same value and there is no critical
break for previous kernels.

When different values are defined for these macros, this can cause
problems, however, I don't think that these different values are
adopted (backported) in the current stable kernels.

So I think it's reasonable to repost this series to net-next.

Thank you,

---
Best Regards
Kunihiko Hayashi