Re: [PATCH v6 3/3] arm64: dts: qcom: x1e80100-dell-xps13-9345: introduce EC

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] platform: arm64: dell-xps-ec: permanently stopped thermal polling on resume error
- [Low] i2c: dell_xps_ec_i2c_id initialized without named initializers
--

commit bd6f1bc93a902f49663602dd218ea3f2d7526203
Author: Aleksandrs Vinarskis <alex@vinarskis.com>

platform: arm64: dell-xps-ec: new driver

This commit introduces a new Embedded Controller driver for the Dell XPS
13 9345. Its primary function is to unblock thermal management by
providing the EC with temperature readings needed to control fans and
peripheral power.

> diff --git a/drivers/platform/arm64/dell-xps-ec.c b/drivers/platform/arm64/dell-xps-ec.c
> new file mode 100644
> index 0000000000000..cf2dd41b1aa68
> --- /dev/null
> +++ b/drivers/platform/arm64/dell-xps-ec.c

[ ... ]

> +static int dell_xps_ec_resume(struct device *dev)
> +{
> +	struct dell_xps_ec *ec = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = dell_xps_ec_suspend_cmd(ec, false);
> +	if (ret)
> +		return ret;

[Severity: Medium]
If a transient I2C bus error occurs during resume, dell_xps_ec_resume() returns
early here without calling schedule_delayed_work().

Does this mean the background temperature polling loop (ec->temp_work) is never
restarted? If so, could this cause the Embedded Controller to stop receiving
thermal updates for the remainder of the boot, potentially leading to incorrect
fan curves or unexpected thermal shutdowns until the next suspend/resume cycle?

> +
> +	schedule_delayed_work(&ec->temp_work, DELL_XPS_EC_TEMP_POLL_JIFFIES);
> +
> +	return 0;
> +}

[ ... ]

> +static const struct i2c_device_id dell_xps_ec_i2c_id[] = {
> +	{ "dell-xps-ec", (kernel_ulong_t)&dell_xps13_9345_data },

[Severity: Low]
This isn't a bug, but should the dell_xps_ec_i2c_id array use named initializers
instead of positional ones? The I2C subsystem guidelines mandate that struct
i2c_device_id arrays must use named initializers.

> +	{}
> +};
> +MODULE_DEVICE_TABLE(i2c, dell_xps_ec_i2c_id);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261011-dell-xps-9345-ec-v6-0-2a66a364fe70@vinarskis.com?part=3