Re: [PATCH v2 2/3] platform: arm64: Add Lenovo ThinkPad X13s EC driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] i2c: missing I2C_FUNC_PROTOCOL_MANGLING check for I2C_M_STOP
--

commit a8937fc74e70704582392fdfcb89327bf751206f
Author: Clover <alex@ironrobin.net>
Subject: platform: arm64: Add Lenovo ThinkPad X13s EC driver

This commit adds a standalone driver for the Lenovo ThinkPad X13s embedded
controller. It handles keyboard-backlight control, hardware brightness
change notifications, and system sleep power sequencing.

> diff --git a/drivers/platform/arm64/lenovo-thinkpad-x13s.c b/drivers/platform/arm64/lenovo-thinkpad-x13s.c
> new file mode 100644
> index 0000000000000..708f909f78780
> --- /dev/null
> +++ b/drivers/platform/arm64/lenovo-thinkpad-x13s.c
[ ... ]
> @@ -0,0 +1,408 @@
[ ... ]
> +static int x13s_read(struct x13s_ec *ec, u8 cmd, u8 reg, u8 *value)
> +{
> +	struct i2c_client *client = ec->client;
> +	u8 buf[] = { cmd, reg, 0, 1 };
> +	struct i2c_msg request = {
> +		.addr = client->addr,
> +		.flags = I2C_M_STOP,

[Severity: Low]
Is the I2C_M_STOP flag necessary here?

Single-message transfers inherently end with a STOP condition. If the
adapter requires this explicit flag, the driver needs to verify that the
adapter supports protocol mangling.

[ ... ]
> +static int x13s_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	struct x13s_ec *ec;
> +	int ret;
> +
> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
> +		return -EOPNOTSUPP;

[Severity: Low]
If the I2C_M_STOP flag is intentionally required in x13s_read(), should this
functionality check also include I2C_FUNC_PROTOCOL_MANGLING?

The I2C API contract dictates that I2C_M_STOP must only be used if the
adapter advertises protocol mangling.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260926195415.5995-1-alex@ironrobin.net?part=2