* [PATCH net] net: ethernet: mediatek: Fix bit field in mtk_set_queue_speed()
@ 2025-03-10 10:48 Dan Carpenter
2025-03-10 12:45 ` Michal Kubiak
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2025-03-10 10:48 UTC (permalink / raw)
To: Felix Fietkau
Cc: Sean Wang, Lorenzo Bianconi, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Matthias Brugger,
AngeloGioacchino Del Regno, netdev, linux-kernel,
linux-arm-kernel, linux-mediatek, kernel-janitors
This was supposed to set "FIELD_PREP(MTK_QTX_SCH_MAX_RATE_WEIGHT, 1)"
but there was typo and the | operation was missing and which turned
it into a no-op.
Fixes: f63959c7eec3 ("net: ethernet: mtk_eth_soc: implement multi-queue support for per-port queues")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
From static analysis, not tested.
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 922330b3f4d7..9efef0e860da 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -757,7 +757,7 @@ static void mtk_set_queue_speed(struct mtk_eth *eth, unsigned int idx,
case SPEED_100:
val |= MTK_QTX_SCH_MAX_RATE_EN |
FIELD_PREP(MTK_QTX_SCH_MAX_RATE_MAN, 1) |
- FIELD_PREP(MTK_QTX_SCH_MAX_RATE_EXP, 5);
+ FIELD_PREP(MTK_QTX_SCH_MAX_RATE_EXP, 5) |
FIELD_PREP(MTK_QTX_SCH_MAX_RATE_WEIGHT, 1);
break;
case SPEED_1000:
--
2.47.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net] net: ethernet: mediatek: Fix bit field in mtk_set_queue_speed()
2025-03-10 10:48 [PATCH net] net: ethernet: mediatek: Fix bit field in mtk_set_queue_speed() Dan Carpenter
@ 2025-03-10 12:45 ` Michal Kubiak
2025-03-10 19:27 ` Dan Carpenter
0 siblings, 1 reply; 4+ messages in thread
From: Michal Kubiak @ 2025-03-10 12:45 UTC (permalink / raw)
To: Dan Carpenter
Cc: Felix Fietkau, Sean Wang, Lorenzo Bianconi, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Matthias Brugger, AngeloGioacchino Del Regno, netdev,
linux-kernel, linux-arm-kernel, linux-mediatek, kernel-janitors
On Mon, Mar 10, 2025 at 01:48:27PM +0300, Dan Carpenter wrote:
> This was supposed to set "FIELD_PREP(MTK_QTX_SCH_MAX_RATE_WEIGHT, 1)"
> but there was typo and the | operation was missing and which turned
> it into a no-op.
>
> Fixes: f63959c7eec3 ("net: ethernet: mtk_eth_soc: implement multi-queue support for per-port queues")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
> From static analysis, not tested.
>
> drivers/net/ethernet/mediatek/mtk_eth_soc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> index 922330b3f4d7..9efef0e860da 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -757,7 +757,7 @@ static void mtk_set_queue_speed(struct mtk_eth *eth, unsigned int idx,
> case SPEED_100:
> val |= MTK_QTX_SCH_MAX_RATE_EN |
> FIELD_PREP(MTK_QTX_SCH_MAX_RATE_MAN, 1) |
> - FIELD_PREP(MTK_QTX_SCH_MAX_RATE_EXP, 5);
> + FIELD_PREP(MTK_QTX_SCH_MAX_RATE_EXP, 5) |
> FIELD_PREP(MTK_QTX_SCH_MAX_RATE_WEIGHT, 1);
> break;
> case SPEED_1000:
There's a similar bug a few lines above (line #737):
case SPEED_100:
val |= MTK_QTX_SCH_MAX_RATE_EN |
FIELD_PREP(MTK_QTX_SCH_MAX_RATE_MAN, 103) |
FIELD_PREP(MTK_QTX_SCH_MAX_RATE_EXP, 3);
FIELD_PREP(MTK_QTX_SCH_MAX_RATE_WEIGHT, 1);
break;
I think it would be reasonable to fix that too in the same patch.
Thanks,
Michal
> --
> 2.47.2
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net] net: ethernet: mediatek: Fix bit field in mtk_set_queue_speed()
2025-03-10 12:45 ` Michal Kubiak
@ 2025-03-10 19:27 ` Dan Carpenter
2025-03-10 20:00 ` Dan Carpenter
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2025-03-10 19:27 UTC (permalink / raw)
To: Michal Kubiak
Cc: Felix Fietkau, Sean Wang, Lorenzo Bianconi, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Matthias Brugger, AngeloGioacchino Del Regno, netdev,
linux-kernel, linux-arm-kernel, linux-mediatek, kernel-janitors
On Mon, Mar 10, 2025 at 01:45:35PM +0100, Michal Kubiak wrote:
> On Mon, Mar 10, 2025 at 01:48:27PM +0300, Dan Carpenter wrote:
> > This was supposed to set "FIELD_PREP(MTK_QTX_SCH_MAX_RATE_WEIGHT, 1)"
> > but there was typo and the | operation was missing and which turned
> > it into a no-op.
> >
> > Fixes: f63959c7eec3 ("net: ethernet: mtk_eth_soc: implement multi-queue support for per-port queues")
> > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> > ---
> > From static analysis, not tested.
> >
> > drivers/net/ethernet/mediatek/mtk_eth_soc.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > index 922330b3f4d7..9efef0e860da 100644
> > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > @@ -757,7 +757,7 @@ static void mtk_set_queue_speed(struct mtk_eth *eth, unsigned int idx,
> > case SPEED_100:
> > val |= MTK_QTX_SCH_MAX_RATE_EN |
> > FIELD_PREP(MTK_QTX_SCH_MAX_RATE_MAN, 1) |
> > - FIELD_PREP(MTK_QTX_SCH_MAX_RATE_EXP, 5);
> > + FIELD_PREP(MTK_QTX_SCH_MAX_RATE_EXP, 5) |
> > FIELD_PREP(MTK_QTX_SCH_MAX_RATE_WEIGHT, 1);
> > break;
> > case SPEED_1000:
>
>
> There's a similar bug a few lines above (line #737):
>
> case SPEED_100:
> val |= MTK_QTX_SCH_MAX_RATE_EN |
> FIELD_PREP(MTK_QTX_SCH_MAX_RATE_MAN, 103) |
> FIELD_PREP(MTK_QTX_SCH_MAX_RATE_EXP, 3);
> FIELD_PREP(MTK_QTX_SCH_MAX_RATE_WEIGHT, 1);
> break;
>
> I think it would be reasonable to fix that too in the same patch.
Yes. You're of course correct. I'm trying to figure out why my
static checker found the one instance and not the other. I will
send a v2.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net] net: ethernet: mediatek: Fix bit field in mtk_set_queue_speed()
2025-03-10 19:27 ` Dan Carpenter
@ 2025-03-10 20:00 ` Dan Carpenter
0 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2025-03-10 20:00 UTC (permalink / raw)
To: Michal Kubiak
Cc: Felix Fietkau, Sean Wang, Lorenzo Bianconi, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Matthias Brugger, AngeloGioacchino Del Regno, netdev,
linux-kernel, linux-arm-kernel, linux-mediatek, kernel-janitors
On Mon, Mar 10, 2025 at 10:27:34PM +0300, Dan Carpenter wrote:
> On Mon, Mar 10, 2025 at 01:45:35PM +0100, Michal Kubiak wrote:
> > On Mon, Mar 10, 2025 at 01:48:27PM +0300, Dan Carpenter wrote:
> > > This was supposed to set "FIELD_PREP(MTK_QTX_SCH_MAX_RATE_WEIGHT, 1)"
> > > but there was typo and the | operation was missing and which turned
> > > it into a no-op.
> > >
> > > Fixes: f63959c7eec3 ("net: ethernet: mtk_eth_soc: implement multi-queue support for per-port queues")
> > > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> > > ---
> > > From static analysis, not tested.
> > >
> > > drivers/net/ethernet/mediatek/mtk_eth_soc.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > index 922330b3f4d7..9efef0e860da 100644
> > > --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> > > @@ -757,7 +757,7 @@ static void mtk_set_queue_speed(struct mtk_eth *eth, unsigned int idx,
> > > case SPEED_100:
> > > val |= MTK_QTX_SCH_MAX_RATE_EN |
> > > FIELD_PREP(MTK_QTX_SCH_MAX_RATE_MAN, 1) |
> > > - FIELD_PREP(MTK_QTX_SCH_MAX_RATE_EXP, 5);
> > > + FIELD_PREP(MTK_QTX_SCH_MAX_RATE_EXP, 5) |
> > > FIELD_PREP(MTK_QTX_SCH_MAX_RATE_WEIGHT, 1);
> > > break;
> > > case SPEED_1000:
> >
> >
> > There's a similar bug a few lines above (line #737):
> >
> > case SPEED_100:
> > val |= MTK_QTX_SCH_MAX_RATE_EN |
> > FIELD_PREP(MTK_QTX_SCH_MAX_RATE_MAN, 103) |
> > FIELD_PREP(MTK_QTX_SCH_MAX_RATE_EXP, 3);
> > FIELD_PREP(MTK_QTX_SCH_MAX_RATE_WEIGHT, 1);
> > break;
> >
> > I think it would be reasonable to fix that too in the same patch.
>
> Yes. You're of course correct. I'm trying to figure out why my
> static checker found the one instance and not the other. I will
> send a v2.
Oh... Duh. if (IS_ENABLED(CONFIG_SOC_MT7621)) is false for my config so
first line wasn't reachable.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-03-10 20:00 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-10 10:48 [PATCH net] net: ethernet: mediatek: Fix bit field in mtk_set_queue_speed() Dan Carpenter
2025-03-10 12:45 ` Michal Kubiak
2025-03-10 19:27 ` Dan Carpenter
2025-03-10 20:00 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).