Re: [PATCH] HID: i2c-hid: Refactor _DSM helper and add i2c-hid-acpi-prp0001 driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing `acpi_device_fix_up_power()` before device probe may leave the touchpad unpowered.
- [Low] Compilation error due to omitted parameter name in C function definition.
--

commit a6cad016330e4f17120f69020be7c1d0cb8f3e5d
Author: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>

HID: i2c-hid: Refactor _DSM helper and add i2c-hid-acpi-prp0001 driver

This patch adds a new i2c-hid-acpi-prp0001 driver to support devices with
_HID "PRP0001" that lack the "hid-descr-addr" property but provide the HID
descriptor address via an ACPI _DSM. It also refactors the _DSM helper out
of i2c-hid-acpi.c into a shared header.

> diff --git a/drivers/hid/i2c-hid/i2c-hid-acpi-prp0001.c b/drivers/hid/i2c-hid/i2c-hid-acpi-prp0001.c
> new file mode 100644
> index 0000000000000..35cbbab347acf
> --- /dev/null
> +++ b/drivers/hid/i2c-hid/i2c-hid-acpi-prp0001.c
[ ... ]
> +static int i2c_hid_acpi_prp0001_power_up(struct i2chid_ops *)
> +{

[Severity: Low]
Does omitting the parameter name here cause a compilation error? 

Standard C requires parameter names in function definitions, and omitting
them generally results in a build failure.

> +	/* give the device time to power up */
> +	msleep(250);
> +	return 0;
> +}
[ ... ]
> +	adev = ACPI_COMPANION(dev);
> +	if (!adev)
> +		return -ENODEV;
> +
> +	ret = i2c_hid_acpi_get_descriptor(adev);
> +	if (ret < 0)
> +		return ret;
> +	hid_descriptor_address = ret;
> +

[Severity: High]
Should acpi_device_fix_up_power() be called on the ACPI companion device
before calling i2c_hid_core_probe()?

ACPI I2C HID devices frequently lack the _PSC (Power State Current) object. 
For such devices, standard ACPI PM attach assumes the device is already in
D0 and skips executing the _PS0 method, leaving the device unpowered.

The core i2c-hid-acpi driver explicitly works around this by calling
acpi_device_fix_up_power(). 

Without it here, will affected touchpads remain unpowered and result in I2C
timeouts during probe?

> +	return i2c_hid_core_probe(client, &i2c_hid_acpi_prp0001_ops,
> +				  hid_descriptor_address, 0);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/tencent_9F8936ABE7C35AED6FF779A83DFF2729D005@qq.com?part=1