Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <28932c5e2250c68e07ef5fafe3bee42653fd62f5.camel@inka.de>
Subject: Re: [PATCH v2 4/7] HID: ft260: support i2c reads greater than HID
 report size
From:   Enrik Berkhan <Enrik.Berkhan@inka.de>
To:     Michael Zaidman <michael.zaidman@gmail.com>, jikos@kernel.org
Cc:     linux-kernel@vger.kernel.org, linux-input@vger.kernel.org,
        Guillaume Champagne <champagne.guillaume.c@gmail.com>
Date:   Tue, 04 Oct 2022 20:11:37 +0200
In-Reply-To: <20220928144854.5580-5-michael.zaidman@gmail.com>
References: <20220928144854.5580-1-michael.zaidman@gmail.com>
         <20220928144854.5580-5-michael.zaidman@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
User-Agent: Evolution 3.44.1-0ubuntu1 
MIME-Version: 1.0
Precedence: bulk

Hi Michael,

On Wed, 2022-09-28 at 17:48 +0300, Michael Zaidman wrote:
> diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
> index bfda5b191a3a..cb8f1782d1f0 100644
> --- a/drivers/hid/hid-ft260.c
> +++ b/drivers/hid/hid-ft260.c
> @@ -460,49 +460,68 @@ static int ft260_smbus_write(struct ft260_device *dev, u8 addr, u8 cmd,
>  static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data,
>  			  u16 len, u8 flag)
>  {
> +	u16 rd_len;
> +	int timeout, ret;
>  	struct ft260_i2c_read_request_report rep;
>  	struct hid_device *hdev = dev->hdev;
> -	int timeout;
> -	int ret;
> +	bool first = true;
>  
> -	if (len > FT260_RD_DATA_MAX) {
> -		hid_err(hdev, "%s: unsupported rd len: %d\n", __func__, len);
> -		return -EINVAL;
> -	}
> +	do {
> +		if (first) {
> +			if (flag & FT260_FLAG_START_REPEATED)

I think the test should be

    if ((flag & FT260_FLAG_START_REPEATED) == FT260_FLAG_START_REPEATED)

Otherwise, flag will never be FT260_FLAG_START ( = 2), as
FT260_FLAG_START_REPEATED (= 3) has tow bits set. It looks as if bit 0
actually means "repeated", bit 1 is "start" and bit 2 is "stop".

Cheers,
Enrik