Re: [PATCH v3 2/4] Documentation: ABI: Add ABI doc for versal edac sysfs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] The sysfs entries implement a stateful RPC command-response interface, causing data races.
- [Medium] Multiple sysfs attributes output complex, multiline blocks instead of single values.
- [Low] Malformed ABI Documentation Syntax.
- [Low] Duplicate and conflicting entries in ABI Documentation.
--

commit 9760cee2db24f4eb5850706de75cd24db0ae078c
Author: Rama devi Veggalam <rama.devi.veggalam@amd.com>

Documentation: ABI: Add ABI doc for versal edac sysfs

Add documentation for the sysfs entries created for
versal edac (XilSEM).

> diff --git a/Documentation/ABI/testing/sysfs-driver-versal-edac b/Documentation/ABI/testing/sysfs-driver-versal-edac
> new file mode 100644
> index 0000000000000..c35d864fcbe2c
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-versal-edac
> @@ -0,0 +1,303 @@
> +What:		/sys/devices/system/edac/versal_xilsem/xsem_scan_control
> +Date:		What:		/sys/devices/system/edac/mc/mc0/xsem_scan_control
> +Date:		June 2026

[Severity: Low]
Is there malformed syntax in this documentation block? The second line has a 
duplicate "What:" definition inside the "Date:" field.

> +Contact:	rama.devi.veggalam@amd.com
> +Description:
> +		It is a read/write file.
> +		Writing to this file causes the software to initiate a
> +		request to the firmware for doing requested scan operation in
> +		Configuration RAM (CRAM) or NPI of Versal mono/SSIT devices.
> +		User needs to provide scan operation id (init, start, stop) and
> +		SLR id (Versal SSIT device) details. For Versal mono devices,
> +		the SLR id should be 0.
> +		The scan operation id values are as given below:
> +		1 - Initialize the scan
> +		2 - Start CRAM scan
> +		3 - Stop CRAM scan
> +		5 - Start NPI scan
> +		6 - Stop NPI scan
> +		7 - Inject NPI error in first descriptor
> +
> +		When read, it shows the current scan status with error code.

[Severity: Medium]
Could this stateful command-response interface introduce data races?

In drivers/edac/versal_edac.c, xsem_scan_control_store() writes the command
result directly to the shared, lockless array:

    ret = zynqmp_pm_xilsem_cntrl_ops(cmd, slrid, priv->xsem_rtca->scan_ctrl_status);

Then xsem_scan_control_show() reads from it:

    return sprintf(data, "[0x%x][0x%x][0x%x][0x%x][0x%x][0x%x][0x%x]\n\r",
                   priv->xsem_rtca->scan_ctrl_status[0],

Will concurrent writes to xsem_scan_control clobber the shared buffers
in priv->xsem_rtca->scan_ctrl_status before either process reads the
attribute, leading to incorrect results?

[ ... ]

> +What:		/sys/devices/system/edac/mc/mc0/xsem_read_ssit_status
> +Date:		June 2026
> +Contact:	rama.devi.veggalam@amd.com
> +Description:
> +		It is a read/write file.
> +		Writing to this file causes the software to initiate a
> +		request to read CRAM and NPI scan status information in a given SLR.
> +		User needs to provide SLR id in Versal SSIT device for which
> +		XilSEM status to be read. When read, it shows
> +		the XilSEM status of the requested SLR.
> +		The format is: <Header> <SLR id> <NPI status info> <CRAM status info>
> +		Example: Read SLR Status Cmd:[0x1030d]
> +		SLR ID:[0x3]
> +		NPI status:[0xa01]

[Severity: Medium]
Will this sysfs attribute violate the single-value-per-file guideline?

Looking at xsem_read_ssit_status_show() in drivers/edac/versal_edac.c, it
generates a multiline block instead of a single value:

    offset += sprintf(data + offset, "Read SLR Status Cmd:[0x%x]\n\r",
                      priv->xsem_rtca->xilsem_ssit_status[0]);
    offset += sprintf(data + offset, "SLR ID:[0x%x]\n\r",
                      priv->xsem_rtca->xilsem_ssit_status[1]);
    offset += sprintf(data + offset, "NPI status:[0x%x]\n\r",

Does this complex multiline output break standard userspace tools that
parse sysfs?

[ ... ]

> +What:		/sys/devices/system/edac/versal_xilsem/xsem_cram_injecterr
> +Date:		July 2025
> +Contact:	rama.devi.veggalam@amd.com

[Severity: Low]
Are there duplicate and conflicting entries in the documentation?

The path /sys/devices/system/edac/versal_xilsem/xsem_cram_injecterr 
listed here appears to conflict with the /sys/devices/system/edac/mc/mc0/ 
path documented earlier for the same attribute.

> +Description:
> +		It is a read/write file.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260624212545.2850787-1-rama.devi.veggalam@amd.com?part=2