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

[Andre Przywara <andre.przywara@arm.com>]

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
>