1. Patchew
  2. linux
  3. View series

[PATCH RFC] watchdog: sbsa: Update the W_IIDR Implementer bit mask to 0xFFF

Naina Mehta posted 1 patch 1 month, 2 weeks ago
There is a newer version of this series
drivers/watchdog/sbsa_gwdt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH RFC] watchdog: sbsa: Update the W_IIDR Implementer bit mask to 0xFFF
Posted by Naina Mehta 1 month, 2 weeks ago 
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
Re: [PATCH RFC] watchdog: sbsa: Update the W_IIDR Implementer bit mask to 0xFFF
Posted by Andre Przywara 1 month, 2 weeks ago 
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
>
Re: [PATCH RFC] watchdog: sbsa: Update the W_IIDR Implementer bit mask to 0xFFF
Posted by Aaron Plattner 1 month, 1 week ago 
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
> 
>
Re: [PATCH RFC] watchdog: sbsa: Update the W_IIDR Implementer bit mask to 0xFFF
Posted by Aaron Plattner 1 month, 1 week ago 
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
>
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.