Re: [PATCH] driver: input :touchscreen : add Raydium I2C touch driver

[Dmitry Torokhov]

Hi Jeffrey,

On Fri, Apr 29, 2016 at 05:45:13PM +0800, jeffrey.lin wrote:
> Raydium I2C touch driver.
> 

In my previous e-mail I requested you to enumerate changes that are made
to the driver, compared to the previous version(s). There were also a
few questions that I did not get answer for.

Also:

> +static int raydium_i2c_query_ts_info(struct raydium_data *ts)
> +{
> +	struct i2c_client *client = ts->client;
> +	int error, retry_cnt;
> +
> +	for (retry_cnt = 0; retry_cnt < MAX_RETRIES; retry_cnt++) {
> +		error = raydium_i2c_read(client, CMD_DATA_BANK,
> +			sizeof(ts->obj), (void *)&ts->obj);
> +			ts->obj.data_bank_addr =
> +				get_unaligned_le32(&ts->obj.data_bank_addr);
> +
> +		if (!error) {
> +			error = raydium_i2c_read(client, CMD_QUERY_BANK,
> +				sizeof(ts->query_bank_info),
> +				(void *)&ts->query_bank_info);
> +			if (!error) {
> +				error = raydium_i2c_read_message(client,
> +					ts->query_bank_info, sizeof(ts->info),
> +					(void *)&ts->info);
> +
> +				ts->info.hw_ver =
> +					get_unaligned_le32(&ts->info.hw_ver);
> +				ts->info.ft_ver =
> +					get_unaligned_le16(&ts->info.ft_ver);
> +				ts->info.x_max =
> +					get_unaligned_le16(&ts->info.x_max);
> +				ts->info.y_max =
> +					get_unaligned_le16(&ts->info.y_max);
> +				return 0;
> +			}
> +		}
> +	}
> +	dev_err(&client->dev, "Get touch data failed: %d\n", error);
> +
> +	return -EINVAL;
> +}
> +
> +static int raydium_i2c_fastboot(struct i2c_client *client)
> +{
> +	static const u8 boot_cmd[] = { 0x50, 0x00, 0x06, 0x20 };
> +	u8 buf[HEADER_SIZE];
> +	int error;
> +
> +	error = raydium_i2c_read_message(client,
> +		get_unaligned_be32(boot_cmd),
> +		sizeof(boot_cmd), buf);

I still can't figure out the logic in read/write mesage handling. Here
we see that device is supposedly reporting data_bank_addr as LE integer,
but query_bank_info is returned in native CPU endianness. With fastboot
command we assume that we are dealing with BE-encoded data and convert
it to CPU-endianness before using it.

However in raydium_i2c_read_message() we convert from CPU endianness to
BE and this comfuses me (does the device really return data in one
endianness but accepts in another endianness?

By the way, why do you have raydium_i2c_read_message() handle reads
above MAX_PKG_SIZE? There are no callers that want it.

Thanks.

-- 
Dmitry