On Tue, Jun 20, 2017 at 04:08:48PM -0700, Andrew Duggan wrote:
> The F54 driver is currently only using the first 6 bytes of F54 so there
> is no need to read all 27 bytes. Some Dell systems
> (Dell XP13 9333 and similar) have an issue with the touchpad or I2C bus
> when readiing reports larger then 16 bytes. Reads larger then 16 bytes
> are reported in two HID reports. Something about the back to back
> reports seems to cause the next read to report incorrect data. This
> results in F30 failing to load and the click button failing to work.
>
> Previous issues with the I2C controller or touchpad were addressed in:
> commit 5b65c2a02966 ("HID: rmi: check sanity of the incoming report")
>
> Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=195949
> Signed-off-by: Andrew Duggan <[email protected]>
Looks fine to me.
Reviewed-by: Nick Dyer <[email protected]>
> ---
> Also, in addition to changing the size of the read I moved the query
> buffer. I figured that since the query buffer is only acceessed in the
> rmi_f54_detect function it was not necessary keep it in f54_data. The
> parsed values are already being stored in f54_data.
>
> Thanks,
> Andrew
>
>
> drivers/input/rmi4/rmi_f54.c | 17 +++++++----------
> 1 file changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_f54.c b/drivers/input/rmi4/rmi_f54.c
> index dea63e2..f5206e2 100644
> --- a/drivers/input/rmi4/rmi_f54.c
> +++ b/drivers/input/rmi4/rmi_f54.c
> @@ -31,9 +31,6 @@
> #define F54_GET_REPORT 1
> #define F54_FORCE_CAL 2
>
> -/* Fixed sizes of reports */
> -#define F54_QUERY_LEN 27
> -
> /* F54 capabilities */
> #define F54_CAP_BASELINE (1 << 2)
> #define F54_CAP_IMAGE8 (1 << 3)
> @@ -95,7 +92,6 @@ struct rmi_f54_reports {
> struct f54_data {
> struct rmi_function *fn;
>
> - u8 qry[F54_QUERY_LEN];
> u8 num_rx_electrodes;
> u8 num_tx_electrodes;
> u8 capabilities;
> @@ -632,22 +628,23 @@ static int rmi_f54_detect(struct rmi_function *fn)
> {
> int error;
> struct f54_data *f54;
> + u8 buf[6];
>
> f54 = dev_get_drvdata(&fn->dev);
>
> error = rmi_read_block(fn->rmi_dev, fn->fd.query_base_addr,
> - &f54->qry, sizeof(f54->qry));
> + buf, sizeof(buf));
> if (error) {
> dev_err(&fn->dev, "%s: Failed to query F54 properties\n",
> __func__);
> return error;
> }
>
> - f54->num_rx_electrodes = f54->qry[0];
> - f54->num_tx_electrodes = f54->qry[1];
> - f54->capabilities = f54->qry[2];
> - f54->clock_rate = f54->qry[3] | (f54->qry[4] << 8);
> - f54->family = f54->qry[5];
> + f54->num_rx_electrodes = buf[0];
> + f54->num_tx_electrodes = buf[1];
> + f54->capabilities = buf[2];
> + f54->clock_rate = buf[3] | (buf[4] << 8);
> + f54->family = buf[5];
>
> rmi_dbg(RMI_DEBUG_FN, &fn->dev, "F54 num_rx_electrodes: %d\n",
> f54->num_rx_electrodes);
> --
> 2.7.4
>
