* [PATCH RFC] watchdog: sbsa: Update the W_IIDR Implementer bit mask to 0xFFF
@ 2025-11-04 6:39 Naina Mehta
2025-11-04 12:00 ` Andre Przywara
0 siblings, 1 reply; 4+ messages in thread
From: Naina Mehta @ 2025-11-04 6:39 UTC (permalink / raw)
To: wim, linux; +Cc: linux-watchdog, linux-kernel, Naina Mehta
We noticed that the implementer mask defined in the driver [1] captures
bits 0-10, whereas section C.4.2 of BSA specification [2] indicates that
bits 0-11 of the W_IIDR register represent the implementer JEP106 code.
We were hoping to understand if there is a specific reason for using
11-bits in the driver implementation.
Looking forward to your insights.
[1] #define SBSA_GWDT_IMPL_MASK 0x7FF
[2] Implementer, bits [11:0]
Contains the JEP106 code of the company that implemented the Generic
Watchdog:
Bits[11:8] The JEP106 continuation code of the implementer.
Bit[7] Always 0
Bits [6:0] The JEP106 identity code of the implementer.
Signed-off-by: Naina Mehta <naina.mehta@oss.qualcomm.com>
---
drivers/watchdog/sbsa_gwdt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/watchdog/sbsa_gwdt.c b/drivers/watchdog/sbsa_gwdt.c
index 6ce1bfb39064..80cb166582df 100644
--- a/drivers/watchdog/sbsa_gwdt.c
+++ b/drivers/watchdog/sbsa_gwdt.c
@@ -75,7 +75,7 @@
#define SBSA_GWDT_VERSION_MASK 0xF
#define SBSA_GWDT_VERSION_SHIFT 16
-#define SBSA_GWDT_IMPL_MASK 0x7FF
+#define SBSA_GWDT_IMPL_MASK 0xFFF
#define SBSA_GWDT_IMPL_SHIFT 0
#define SBSA_GWDT_IMPL_MEDIATEK 0x426
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH RFC] watchdog: sbsa: Update the W_IIDR Implementer bit mask to 0xFFF
2025-11-04 6:39 [PATCH RFC] watchdog: sbsa: Update the W_IIDR Implementer bit mask to 0xFFF Naina Mehta
@ 2025-11-04 12:00 ` Andre Przywara
2025-11-05 16:37 ` Aaron Plattner
2025-11-05 17:02 ` Aaron Plattner
0 siblings, 2 replies; 4+ messages in thread
From: Andre Przywara @ 2025-11-04 12:00 UTC (permalink / raw)
To: Naina Mehta, wim, linux; +Cc: linux-watchdog, linux-kernel
Hi,
On 04/11/2025 06:39, Naina Mehta wrote:
> We noticed that the implementer mask defined in the driver [1] captures
> bits 0-10, whereas section C.4.2 of BSA specification [2] indicates that
> bits 0-11 of the W_IIDR register represent the implementer JEP106 code.
>
> We were hoping to understand if there is a specific reason for using
> 11-bits in the driver implementation.
>
> Looking forward to your insights.
Well, looks like a simple off-by-one bug, doesn't it? And nobody noticed
because it only affects vendors in the later JEP banks, and the only
user so far is comparing with 0x426, so it's not affected.
>
> [1] #define SBSA_GWDT_IMPL_MASK 0x7FF
>
> [2] Implementer, bits [11:0]
> Contains the JEP106 code of the company that implemented the Generic
> Watchdog:
> Bits[11:8] The JEP106 continuation code of the implementer.
> Bit[7] Always 0
> Bits [6:0] The JEP106 identity code of the implementer.
>
> Signed-off-by: Naina Mehta <naina.mehta@oss.qualcomm.com>
> ---
> drivers/watchdog/sbsa_gwdt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/watchdog/sbsa_gwdt.c b/drivers/watchdog/sbsa_gwdt.c
> index 6ce1bfb39064..80cb166582df 100644
> --- a/drivers/watchdog/sbsa_gwdt.c
> +++ b/drivers/watchdog/sbsa_gwdt.c
> @@ -75,7 +75,7 @@
> #define SBSA_GWDT_VERSION_MASK 0xF
> #define SBSA_GWDT_VERSION_SHIFT 16
>
> -#define SBSA_GWDT_IMPL_MASK 0x7FF
> +#define SBSA_GWDT_IMPL_MASK 0xFFF
Can we please use GENMASK here? This probably would have avoided the
problem in the first place.
Cheers,
Andre
> #define SBSA_GWDT_IMPL_SHIFT 0
> #define SBSA_GWDT_IMPL_MEDIATEK 0x426
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH RFC] watchdog: sbsa: Update the W_IIDR Implementer bit mask to 0xFFF
2025-11-04 12:00 ` Andre Przywara
@ 2025-11-05 16:37 ` Aaron Plattner
2025-11-05 17:02 ` Aaron Plattner
1 sibling, 0 replies; 4+ messages in thread
From: Aaron Plattner @ 2025-11-05 16:37 UTC (permalink / raw)
To: Andre Przywara, Naina Mehta, wim, linux; +Cc: linux-watchdog, linux-kernel
On 11/4/25 4:00 AM, Andre Przywara wrote:
> Hi,
>
> On 04/11/2025 06:39, Naina Mehta wrote:
>> We noticed that the implementer mask defined in the driver [1] captures
>> bits 0-10, whereas section C.4.2 of BSA specification [2] indicates that
>> bits 0-11 of the W_IIDR register represent the implementer JEP106 code.
>>
>> We were hoping to understand if there is a specific reason for using
>> 11-bits in the driver implementation.
>>
>> Looking forward to your insights.
>
> Well, looks like a simple off-by-one bug, doesn't it? And nobody
> noticed because it only affects vendors in the later JEP banks, and
> the only user so far is comparing with 0x426, so it's not affected.
Yes, I think you're right. Sorry about that.
As far as I know, there isn't a 0xc26 assigned so I doubt anyone will
run into this. And if they do, the workaround it applies has very low
impact.
>
>>
>> [1] #define SBSA_GWDT_IMPL_MASK 0x7FF
>>
>> [2] Implementer, bits [11:0]
>> Contains the JEP106 code of the company that implemented the
>> Generic
>> Watchdog:
>> Bits[11:8] The JEP106 continuation code of the implementer.
>> Bit[7] Always 0
>> Bits [6:0] The JEP106 identity code of the implementer.
>>
>> Signed-off-by: Naina Mehta <naina.mehta@oss.qualcomm.com>
>> ---
>> drivers/watchdog/sbsa_gwdt.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/watchdog/sbsa_gwdt.c b/drivers/watchdog/sbsa_gwdt.c
>> index 6ce1bfb39064..80cb166582df 100644
>> --- a/drivers/watchdog/sbsa_gwdt.c
>> +++ b/drivers/watchdog/sbsa_gwdt.c
>> @@ -75,7 +75,7 @@
>> #define SBSA_GWDT_VERSION_MASK 0xF
>> #define SBSA_GWDT_VERSION_SHIFT 16
>> -#define SBSA_GWDT_IMPL_MASK 0x7FF
>> +#define SBSA_GWDT_IMPL_MASK 0xFFF
>
> Can we please use GENMASK here? This probably would have avoided the
> problem in the first place.
I'll leave that up to the maintainers, but assuming they're okay with
this quick fix,
Acked-by: Aaron Plattner <aplattner@nvidia.com>
-- Aaron
>
> Cheers,
> Andre
>
>
>> #define SBSA_GWDT_IMPL_SHIFT 0
>> #define SBSA_GWDT_IMPL_MEDIATEK 0x426
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH RFC] watchdog: sbsa: Update the W_IIDR Implementer bit mask to 0xFFF
2025-11-04 12:00 ` Andre Przywara
2025-11-05 16:37 ` Aaron Plattner
@ 2025-11-05 17:02 ` Aaron Plattner
1 sibling, 0 replies; 4+ messages in thread
From: Aaron Plattner @ 2025-11-05 17:02 UTC (permalink / raw)
To: Andre Przywara, Naina Mehta, wim, linux; +Cc: linux-watchdog, linux-kernel
On 11/4/25 4:00 AM, Andre Przywara wrote:
> Hi,
>
> On 04/11/2025 06:39, Naina Mehta wrote:
>> We noticed that the implementer mask defined in the driver [1] captures
>> bits 0-10, whereas section C.4.2 of BSA specification [2] indicates that
>> bits 0-11 of the W_IIDR register represent the implementer JEP106 code.
>>
>> We were hoping to understand if there is a specific reason for using
>> 11-bits in the driver implementation.
>>
>> Looking forward to your insights.
>
> Well, looks like a simple off-by-one bug, doesn't it? And nobody noticed
> because it only affects vendors in the later JEP banks, and the only
> user so far is comparing with 0x426, so it's not affected.
Yes, I think you're right. Sorry about that.
As far as I know, there isn't a 0xc26 assigned so I doubt anyone will
run into this. And if they do, the workaround it applies has very low
impact.
>>
>> [1] #define SBSA_GWDT_IMPL_MASK 0x7FF
>>
>> [2] Implementer, bits [11:0]
>> Contains the JEP106 code of the company that implemented the Generic
>> Watchdog:
>> Bits[11:8] The JEP106 continuation code of the implementer.
>> Bit[7] Always 0
>> Bits [6:0] The JEP106 identity code of the implementer.
>>
>> Signed-off-by: Naina Mehta <naina.mehta@oss.qualcomm.com>
>> ---
>> drivers/watchdog/sbsa_gwdt.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/watchdog/sbsa_gwdt.c b/drivers/watchdog/sbsa_gwdt.c
>> index 6ce1bfb39064..80cb166582df 100644
>> --- a/drivers/watchdog/sbsa_gwdt.c
>> +++ b/drivers/watchdog/sbsa_gwdt.c
>> @@ -75,7 +75,7 @@
>> #define SBSA_GWDT_VERSION_MASK 0xF
>> #define SBSA_GWDT_VERSION_SHIFT 16
>> -#define SBSA_GWDT_IMPL_MASK 0x7FF
>> +#define SBSA_GWDT_IMPL_MASK 0xFFF
>
> Can we please use GENMASK here? This probably would have avoided the
> problem in the first place.
I'll leave that up to the maintainers, but assuming they're okay with
this quick fix,
Acked-by: Aaron Plattner <aplattner@nvidia.com>
-- Aaron
> Cheers,
> Andre
>
>
>> #define SBSA_GWDT_IMPL_SHIFT 0
>> #define SBSA_GWDT_IMPL_MEDIATEK 0x426
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-11-05 17:03 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-04 6:39 [PATCH RFC] watchdog: sbsa: Update the W_IIDR Implementer bit mask to 0xFFF Naina Mehta
2025-11-04 12:00 ` Andre Przywara
2025-11-05 16:37 ` Aaron Plattner
2025-11-05 17:02 ` Aaron Plattner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox