Re: [RFC PATCH 1/2] include: sbi_ecall_interface: Add defines for the RISC-V SBI watchdog extension

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] riscv: sbi: overly generic enum name for watchdog attributes
- [Low] riscv: sbi: inconsistent prefixing for watchdog function IDs
--

commit 7817ac6d29fdb82db2284ee376b8c224893e23e3
Author: Prajwal N <prajwal.n@oss.qualcomm.com>

include: sbi_ecall_interface: Add defines for the RISC-V SBI watchdog extension

This commit adds the SBI watchdog extension ID, function IDs, and attribute
IDs. It enables supported operations like start, stop, and attribute
read/write.

> diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
> index 5725e0ca4dda3..b67ee507c517e 100644
> --- a/arch/riscv/include/asm/sbi.h
> +++ b/arch/riscv/include/asm/sbi.h

[ ... ]

> @@ -534,6 +535,31 @@ union sbi_dbtr_shmem_entry {
>  	struct sbi_dbtr_id_msg id;
>  };
>  
> +/* SBI watchdog function IDs */
> +enum sbi_ext_wdt_fid {
> +	SBI_EXT_WATCHDOG_READ_ATTRIBUTE = 0,
> +	SBI_EXT_WATCHDOG_WRITE_ATTRIBUTE,
> +	SBI_EXT_WATCHDOG_WRITE_NOTIF_MSI_MESSAGE,

[Severity: Low]
Is this prefix inconsistent? The extension is defined as SBI_EXT_WDT and the
attributes use SBI_WDT_. Could these function IDs use the SBI_EXT_WDT_ prefix
instead of SBI_EXT_WATCHDOG_?

> +	SBI_EXT_WATCHDOG_START,
> +	SBI_EXT_WATCHDOG_STOP,
> +	SBI_EXT_WATCHDOG_PAT,
> +};
> +
> +/* SBI watchdog attributes */
> +enum sbi_ext_attribute_id {

[Severity: Low]
Does this generic enum name risk namespace collisions with future SBI
extensions? Should it be named sbi_ext_wdt_attribute_id to reflect the
WDT-specific values it contains?

> +	SBI_WDT_CAPABILITY		= 0x00000000,
> +	SBI_WDT_STATE			= 0x00000001,
> +	SBI_WDT_MIN_PERIOD		= 0x00000002,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261002163758.2919372-1-prajwal.n@oss.qualcomm.com?part=1