| Message ID | eaab1b7b-b33b-458b-a89a-81391bd2e6e8@stanley.mountain (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [net] net: ethernet: mediatek: Fix bit field in mtk_set_queue_speed() | expand |
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 > >
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
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
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:
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(-)