* [PATCH net-next] net: airoha: Fix EGRESS_RATE_METER_EN_MASK definition
@ 2024-10-04 21:51 Lorenzo Bianconi
2024-10-04 22:49 ` Jacob Keller
0 siblings, 1 reply; 4+ messages in thread
From: Lorenzo Bianconi @ 2024-10-04 21:51 UTC (permalink / raw)
To: Felix Fietkau, Sean Wang, Mark Lee, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Matthias Brugger,
AngeloGioacchino Del Regno
Cc: linux-arm-kernel, linux-mediatek, netdev, Lorenzo Bianconi
Fix typo in EGRESS_RATE_METER_EN_MASK mask definition. This bus was not
introducing any user visible problem.
Introduced by commit 23020f049327 ("net: airoha: Introduce ethernet support
for EN7581 SoC")
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
drivers/net/ethernet/mediatek/airoha_eth.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mediatek/airoha_eth.c b/drivers/net/ethernet/mediatek/airoha_eth.c
index 2e01abc70c170f32f4206b34e116b441c14c628e..a1cfdc146a41610a3a6b060bfdc6e1d9aad97d5d 100644
--- a/drivers/net/ethernet/mediatek/airoha_eth.c
+++ b/drivers/net/ethernet/mediatek/airoha_eth.c
@@ -554,7 +554,7 @@
#define FWD_DSCP_LOW_THR_MASK GENMASK(17, 0)
#define REG_EGRESS_RATE_METER_CFG 0x100c
-#define EGRESS_RATE_METER_EN_MASK BIT(29)
+#define EGRESS_RATE_METER_EN_MASK BIT(31)
#define EGRESS_RATE_METER_EQ_RATE_EN_MASK BIT(17)
#define EGRESS_RATE_METER_WINDOW_SZ_MASK GENMASK(16, 12)
#define EGRESS_RATE_METER_TIMESLICE_MASK GENMASK(10, 0)
---
base-commit: c55ff46aeebed1704a9a6861777b799f15ce594d
change-id: 20241004-airoha-fixes-8aaa8177b234
Best regards,
--
Lorenzo Bianconi <lorenzo@kernel.org>
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH net-next] net: airoha: Fix EGRESS_RATE_METER_EN_MASK definition
2024-10-04 21:51 [PATCH net-next] net: airoha: Fix EGRESS_RATE_METER_EN_MASK definition Lorenzo Bianconi
@ 2024-10-04 22:49 ` Jacob Keller
2024-10-05 10:24 ` Lorenzo Bianconi
0 siblings, 1 reply; 4+ messages in thread
From: Jacob Keller @ 2024-10-04 22:49 UTC (permalink / raw)
To: Lorenzo Bianconi, Felix Fietkau, Sean Wang, Mark Lee,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Matthias Brugger, AngeloGioacchino Del Regno
Cc: linux-arm-kernel, linux-mediatek, netdev
On 10/4/2024 2:51 PM, Lorenzo Bianconi wrote:
> Fix typo in EGRESS_RATE_METER_EN_MASK mask definition. This bus was not
> introducing any user visible problem.
>
I'm not sure I follow. This bit is used by airoha_qdma_init_qos which
sets the REG_EGRESS_RATE_METER_CFG register?
How does this not provide any user visible issues? It seems like an
incorrect enable bit likely means that QOS is not enabled? I'm guessing
bit 29 is reserved?
It would be good to understand why this is not considered a fix? The
offending commit is in the net branch already.
> Introduced by commit 23020f049327 ("net: airoha: Introduce ethernet support
> for EN7581 SoC")
>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
> drivers/net/ethernet/mediatek/airoha_eth.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/mediatek/airoha_eth.c b/drivers/net/ethernet/mediatek/airoha_eth.c
> index 2e01abc70c170f32f4206b34e116b441c14c628e..a1cfdc146a41610a3a6b060bfdc6e1d9aad97d5d 100644
> --- a/drivers/net/ethernet/mediatek/airoha_eth.c
> +++ b/drivers/net/ethernet/mediatek/airoha_eth.c
> @@ -554,7 +554,7 @@
> #define FWD_DSCP_LOW_THR_MASK GENMASK(17, 0)
>
> #define REG_EGRESS_RATE_METER_CFG 0x100c
> -#define EGRESS_RATE_METER_EN_MASK BIT(29)
> +#define EGRESS_RATE_METER_EN_MASK BIT(31)
> #define EGRESS_RATE_METER_EQ_RATE_EN_MASK BIT(17)
> #define EGRESS_RATE_METER_WINDOW_SZ_MASK GENMASK(16, 12)
> #define EGRESS_RATE_METER_TIMESLICE_MASK GENMASK(10, 0)
>
> ---
> base-commit: c55ff46aeebed1704a9a6861777b799f15ce594d
> change-id: 20241004-airoha-fixes-8aaa8177b234
>
> Best regards,
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH net-next] net: airoha: Fix EGRESS_RATE_METER_EN_MASK definition
2024-10-04 22:49 ` Jacob Keller
@ 2024-10-05 10:24 ` Lorenzo Bianconi
2024-10-08 16:46 ` Jacob Keller
0 siblings, 1 reply; 4+ messages in thread
From: Lorenzo Bianconi @ 2024-10-05 10:24 UTC (permalink / raw)
To: Jacob Keller
Cc: Felix Fietkau, Sean Wang, Mark Lee, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Matthias Brugger,
AngeloGioacchino Del Regno, linux-arm-kernel, linux-mediatek,
netdev
[-- Attachment #1: Type: text/plain, Size: 2417 bytes --]
>
>
> On 10/4/2024 2:51 PM, Lorenzo Bianconi wrote:
> > Fix typo in EGRESS_RATE_METER_EN_MASK mask definition. This bus was not
> > introducing any user visible problem.
> >
>
> I'm not sure I follow. This bit is used by airoha_qdma_init_qos which
> sets the REG_EGRESS_RATE_METER_CFG register?
>
> How does this not provide any user visible issues? It seems like an
> incorrect enable bit likely means that QOS is not enabled? I'm guessing
> bit 29 is reserved?
Hi Jacob,
even if we are setting EGRESS_RATE_METER_EN_MASK bit (with a wrong value) in
REG_EGRESS_RATE_METER_CFG register, egress QoS metering will not be supported
yet since we are missing some other configuration (token bucket rate, token
bucket size. Airoha folks please correct me if I am wrong). This is why I do
not think it is important to backport this patch and I did not added any Fixes
tag.
QoS hw ingress/egress metering is in my ToDo list. Here I have ported the basic
qos configuration I found in the vendor sdk. I will add more info in the commit
log in v2. Sorry for the confusion.
Regards,
Lorenzo
>
> It would be good to understand why this is not considered a fix? The
> offending commit is in the net branch already.
>
> > Introduced by commit 23020f049327 ("net: airoha: Introduce ethernet support
> > for EN7581 SoC")
> >
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > ---
> > drivers/net/ethernet/mediatek/airoha_eth.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/mediatek/airoha_eth.c b/drivers/net/ethernet/mediatek/airoha_eth.c
> > index 2e01abc70c170f32f4206b34e116b441c14c628e..a1cfdc146a41610a3a6b060bfdc6e1d9aad97d5d 100644
> > --- a/drivers/net/ethernet/mediatek/airoha_eth.c
> > +++ b/drivers/net/ethernet/mediatek/airoha_eth.c
> > @@ -554,7 +554,7 @@
> > #define FWD_DSCP_LOW_THR_MASK GENMASK(17, 0)
> >
> > #define REG_EGRESS_RATE_METER_CFG 0x100c
> > -#define EGRESS_RATE_METER_EN_MASK BIT(29)
> > +#define EGRESS_RATE_METER_EN_MASK BIT(31)
> > #define EGRESS_RATE_METER_EQ_RATE_EN_MASK BIT(17)
> > #define EGRESS_RATE_METER_WINDOW_SZ_MASK GENMASK(16, 12)
> > #define EGRESS_RATE_METER_TIMESLICE_MASK GENMASK(10, 0)
> >
> > ---
> > base-commit: c55ff46aeebed1704a9a6861777b799f15ce594d
> > change-id: 20241004-airoha-fixes-8aaa8177b234
> >
> > Best regards,
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH net-next] net: airoha: Fix EGRESS_RATE_METER_EN_MASK definition
2024-10-05 10:24 ` Lorenzo Bianconi
@ 2024-10-08 16:46 ` Jacob Keller
0 siblings, 0 replies; 4+ messages in thread
From: Jacob Keller @ 2024-10-08 16:46 UTC (permalink / raw)
To: Lorenzo Bianconi
Cc: Felix Fietkau, Sean Wang, Mark Lee, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Matthias Brugger,
AngeloGioacchino Del Regno, linux-arm-kernel, linux-mediatek,
netdev
On 10/5/2024 3:24 AM, Lorenzo Bianconi wrote:
>>
>>
>> On 10/4/2024 2:51 PM, Lorenzo Bianconi wrote:
>>> Fix typo in EGRESS_RATE_METER_EN_MASK mask definition. This bus was not
>>> introducing any user visible problem.
>>>
>>
>> I'm not sure I follow. This bit is used by airoha_qdma_init_qos which
>> sets the REG_EGRESS_RATE_METER_CFG register?
>>
>> How does this not provide any user visible issues? It seems like an
>> incorrect enable bit likely means that QOS is not enabled? I'm guessing
>> bit 29 is reserved?
>
> Hi Jacob,
>
> even if we are setting EGRESS_RATE_METER_EN_MASK bit (with a wrong value) in
> REG_EGRESS_RATE_METER_CFG register, egress QoS metering will not be supported
> yet since we are missing some other configuration (token bucket rate, token
> bucket size. Airoha folks please correct me if I am wrong). This is why I do
> not think it is important to backport this patch and I did not added any Fixes
> tag.
> QoS hw ingress/egress metering is in my ToDo list. Here I have ported the basic
> qos configuration I found in the vendor sdk. I will add more info in the commit
> log in v2. Sorry for the confusion.
>
> Regards,
> Lorenzo
Thanks for explaining.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-10-08 16:46 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-04 21:51 [PATCH net-next] net: airoha: Fix EGRESS_RATE_METER_EN_MASK definition Lorenzo Bianconi
2024-10-04 22:49 ` Jacob Keller
2024-10-05 10:24 ` Lorenzo Bianconi
2024-10-08 16:46 ` Jacob Keller
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).