Re: [RFC PATCH v2 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
issues to consider:
- [Low] riscv: sbi: commit subject uses OpenSBI sbi_ecall_interface
  prefix
- [Low] riscv: sbi: inconsistent WDT vs WATCHDOG naming in asm/sbi.h
--

commit 20a1be6707ce97918704c1778e1e39d1a180ed16
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 extension ID, function IDs, and attribute IDs for
the RISC-V SBI watchdog extension to arch/riscv/include/asm/sbi.h.

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

[Severity: Low]
The subject prefix "include: sbi_ecall_interface:" refers to OpenSBI's
include/sbi/sbi_ecall_interface.h header rather than a Linux kernel path
or subsystem.  Should this use "riscv: sbi:" or "RISC-V:" like other
commits touching arch/riscv/include/asm/sbi.h?

> diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
> index fd6266a7b3395..f2b0b0c9f6b13 100644
> --- a/arch/riscv/include/asm/sbi.h
> +++ b/arch/riscv/include/asm/sbi.h
> @@ -40,6 +40,7 @@ enum sbi_ext_id {
>  	SBI_EXT_FWFT = 0x46574654,
>  	SBI_EXT_MPXY = 0x4D505859,
>  	SBI_EXT_DBTR = 0x44425452,
> +	SBI_EXT_WDT = 0x574454,
>  
>  	/* Experimentals extensions must lie within this range */
>  	SBI_EXT_EXPERIMENTAL_START = 0x08000000,
> @@ -536,6 +537,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,
> +	SBI_EXT_WATCHDOG_START,
> +	SBI_EXT_WATCHDOG_STOP,
> +	SBI_EXT_WATCHDOG_PAT,
> +};
> +
> +/* SBI watchdog attributes */
> +enum sbi_ext_wdt_attribute_id {
> +	SBI_WATCHDOG_CAPABILITY			= 0x00000000,
> +	SBI_WATCHDOG_STATE			= 0x00000001,
> +	SBI_WATCHDOG_MIN_PERIOD			= 0x00000002,
> +	SBI_WATCHDOG_PERIOD			= 0x00000003,
> +	SBI_WATCHDOG_TIME_LEFT			= 0x00000004,
> +	SBI_WATCHDOG_NOTIF_TIME			= 0x00000005,
> +	SBI_WATCHDOG_NOTIF_MSI_ADDR_LOW		= 0x00000006,
> +	SBI_WATCHDOG_NOTIF_MSI_ADDR_HIGH	= 0x00000007,
> +	SBI_WATCHDOG_NOTIF_MSI_DATA		= 0x00000008,
> +	SBI_WATCHDOG_ATTR_MAX			= 0x00000009,
> +	SBI_WATCHDOG_ATTR_RESERVED_MAX		= 0xFFFFFFFF
> +};

[Severity: Low]
The extension ID is SBI_EXT_WDT and the enum types are sbi_ext_wdt_fid
and sbi_ext_wdt_attribute_id, whereas the enumerators use
SBI_EXT_WATCHDOG_* and SBI_WATCHDOG_*.  Other extensions in this header
use a single extension acronym across the extension ID, enum types, and
enumerators; should these enumerators use SBI_EXT_WDT_* and SBI_WDT_* for
consistency?

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