Re: [PATCH -next v2 2/4] HID: amd_sfh: Add operating-mode telemetry sysfs attributes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] hid: non-atomic copy into mismatched struct for SRA data
- [Low] hid: build failure due to non-existent c2p3_data member
--

commit ee7fce24f1e5a01f6f7e6e6cd19e09582b434ae4
Author: Basavaraj Natikar <Basavaraj.Natikar@amd.com>

HID: amd_sfh: Add operating-mode telemetry sysfs attributes

The Sensor Fusion Hub reports additional operating-mode telemetry through
the SRA sensor. This commit exposes these states read-only through sysfs
so that userspace can consume them.

> diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_sysfs.c b/drivers/hid/amd-sfh-hid/amd_sfh_sysfs.c
> index db1cc205d25b9..8af4df09760f0 100644
> --- a/drivers/hid/amd-sfh-hid/amd_sfh_sysfs.c
> +++ b/drivers/hid/amd-sfh-hid/amd_sfh_sysfs.c
[ ... ]
> @@ -41,19 +43,78 @@ static ssize_t hpd_store(struct device *dev,
>  }
>  static DEVICE_ATTR_RW(hpd);
>  
> +/* Operating-mode telemetry reported by the SRA sensor, exposed read-only. */
> +static int amd_sfh_read_op_mode(struct amd_mp2_dev *mp2, struct sfh_op_mode *mode)
> +{
> +	struct sfh_accel_data accel_data;
> +	void __iomem *sensoraddr;
> +
> +	if (!mp2->sfh1_1_ops || !mp2->dev_en.is_sra_present)
> +		return -ENODEV;
> +
> +	if (mp2->mp2_ver >= MP2_VER_1_2) {
> +		sensoraddr = mp2->vsbase +
> +			(SRA_IDX * SENSOR_DATA_MEM_SIZE_DEFAULT) +
> +			OFFSET_SENSOR_DATA_DEFAULT;
> +		memcpy_fromio(&accel_data, sensoraddr, sizeof(struct sfh_accel_data));

[Severity: Medium]
Does amd_sfh_read_op_mode() perform a multi-byte, non-atomic bulk I/O copy
from shared hardware memory using the wrong struct type (accelerometer instead
of SRA)?

Reading any of the new SRA telemetry sysfs attributes on a device with MP2
version >= 1.2 could risk torn reads from concurrent hardware updates.

Could a targeted, atomic readl() be used instead to read the single 32-bit
register value?

> +		mode->val = accel_data.c2p3_data;

[Severity: Low]
Does struct sfh_accel_data have a c2p3_data member?

It appears this member does not exist, which causes an immediate compilation
failure when compiling the amd_sfh module.

> +	} else {
> +		mode->val = readl(mp2->mmio + amd_get_c2p_val(mp2, 3));
> +	}
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261006103754.3614316-1-Basavaraj.Natikar@amd.com?part=2