Re: [PATCH v5 2/2] Input: add Himax HX852x(ES) touchscreen driver

[Dmitry Torokhov]

Hi Stephan,

On Mon, Sep 15, 2025 at 04:19:57PM +0200, Stephan Gerhold wrote:
> +static int hx852x_i2c_read(struct hx852x *hx, u8 cmd, void *data, u16 len)
> +{
> +	struct i2c_client *client = hx->client;
> +	int ret;
> +
> +	struct i2c_msg msg[] = {
> +		{
> +			.addr = client->addr,
> +			.flags = 0,
> +			.len = 1,
> +			.buf = &cmd,
> +		},
> +		{
> +			.addr = client->addr,
> +			.flags = I2C_M_RD,
> +			.len = len,
> +			.buf = data,
> +		},
> +	};
> +
> +	ret = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg));
> +	if (ret != ARRAY_SIZE(msg)) {

Added
		error = ret < 0 ? ret : -EIO;

because theoretically i2c_transfer() can return 0 as number of messages
transferred.


> +
> +err_test_mode:
> +	error = i2c_smbus_write_byte_data(hx->client, HX852X_REG_SRAM_SWITCH, 0) ? : error;

You want to return the first error that happened, not the last one.
Changed to:

	error2 = i2c_smbus_write_byte_data(...);
	error = error ?: error2;

> +
> +static int hx852x_suspend(struct device *dev)
> +{
> +	struct hx852x *hx = dev_get_drvdata(dev);
> +	int error = 0;
> +
> +	mutex_lock(&hx->input_dev->mutex);

Changed to use

	guard(mutex)(&hx->input_dev->mutex);

style and applied.

Thanks.

-- 
Dmitry