[PATCH 0/6] media: Fix warnings for smatch and sparse

[PATCH 0/6] media: Fix warnings for smatch and sparse

[Ricardo Ribalda]

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
Ricardo Ribalda (6):
      media: usb: siano: Fix allocation of urbs
      media: cxd2880: Replaze kmalloc with kzalloc
      media: platform: sti: hva: clk_unprepare unconditionally
      media: v4l2-ctrls-core.c: Do not use iterator outside loop
      media: adv7180: Only request valids IRQs
      media: touchscreen: sur40: convert le16 to cpu before use

 drivers/input/touchscreen/sur40.c          |  2 +-
 drivers/media/i2c/adv7180.c                |  2 +-
 drivers/media/platform/st/sti/hva/hva-hw.c |  3 +--
 drivers/media/spi/cxd2880-spi.c            |  2 +-
 drivers/media/usb/siano/smsusb.c           | 28 +++++++++++++++++--------
 drivers/media/v4l2-core/v4l2-ctrls-api.c   | 33 +++++++++++++++++-------------
 6 files changed, 43 insertions(+), 27 deletions(-)
---
base-commit: 34d7bf1c8e59f5fbf438ee32c96389ebe41ca2e8
change-id: 20240410-smatch-8f235d50753d

Best regards,
-- 
Ricardo Ribalda <ribalda@chromium.org>

[PATCH 1/6] media: usb: siano: Fix allocation of urbs

[Ricardo Ribalda]

USB urbs must be allocated with usb_alloc_urb. Quoting the manual

Only use this function (usb_init_urb) if you _really_ understand what you
are doing.

Fix the following smatch error:

drivers/media/usb/siano/smsusb.c:53:38: warning: array of flexible structures

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/usb/siano/smsusb.c | 28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/media/usb/siano/smsusb.c b/drivers/media/usb/siano/smsusb.c
index 723510520d092..d85308e0785db 100644
--- a/drivers/media/usb/siano/smsusb.c
+++ b/drivers/media/usb/siano/smsusb.c
@@ -40,7 +40,7 @@ struct smsusb_urb_t {
 	struct smscore_buffer_t *cb;
 	struct smsusb_device_t *dev;
 
-	struct urb urb;
+	struct urb *urb;
 
 	/* For the bottom half */
 	struct work_struct wq;
@@ -160,7 +160,7 @@ static int smsusb_submit_urb(struct smsusb_device_t *dev,
 	}
 
 	usb_fill_bulk_urb(
-		&surb->urb,
+		surb->urb,
 		dev->udev,
 		usb_rcvbulkpipe(dev->udev, dev->in_ep),
 		surb->cb->p,
@@ -168,9 +168,9 @@ static int smsusb_submit_urb(struct smsusb_device_t *dev,
 		smsusb_onresponse,
 		surb
 	);
-	surb->urb.transfer_flags |= URB_FREE_BUFFER;
+	surb->urb->transfer_flags |= URB_FREE_BUFFER;
 
-	return usb_submit_urb(&surb->urb, GFP_ATOMIC);
+	return usb_submit_urb(surb->urb, GFP_ATOMIC);
 }
 
 static void smsusb_stop_streaming(struct smsusb_device_t *dev)
@@ -178,7 +178,7 @@ static void smsusb_stop_streaming(struct smsusb_device_t *dev)
 	int i;
 
 	for (i = 0; i < MAX_URBS; i++) {
-		usb_kill_urb(&dev->surbs[i].urb);
+		usb_kill_urb(dev->surbs[i].urb);
 		if (dev->surbs[i].wq.func)
 			cancel_work_sync(&dev->surbs[i].wq);
 
@@ -338,6 +338,8 @@ static void smsusb_term_device(struct usb_interface *intf)
 	struct smsusb_device_t *dev = usb_get_intfdata(intf);
 
 	if (dev) {
+		int i;
+
 		dev->state = SMSUSB_DISCONNECTED;
 
 		smsusb_stop_streaming(dev);
@@ -346,6 +348,9 @@ static void smsusb_term_device(struct usb_interface *intf)
 		if (dev->coredev)
 			smscore_unregister_device(dev->coredev);
 
+		for (i = 0; i < MAX_URBS; i++)
+			usb_free_urb(dev->surbs[i].urb);
+
 		pr_debug("device 0x%p destroyed\n", dev);
 		kfree(dev);
 	}
@@ -390,6 +395,7 @@ static int smsusb_init_device(struct usb_interface *intf, int board_id)
 	void *mdev;
 	int i, rc;
 	int align = 0;
+	int n_urb = 0;
 
 	/* create device object */
 	dev = kzalloc(sizeof(struct smsusb_device_t), GFP_KERNEL);
@@ -461,9 +467,11 @@ static int smsusb_init_device(struct usb_interface *intf, int board_id)
 	dev->coredev->is_usb_device = true;
 
 	/* initialize urbs */
-	for (i = 0; i < MAX_URBS; i++) {
-		dev->surbs[i].dev = dev;
-		usb_init_urb(&dev->surbs[i].urb);
+	for (n_urb = 0; n_urb < MAX_URBS; n_urb++) {
+		dev->surbs[n_urb].dev = dev;
+		dev->surbs[n_urb].urb = usb_alloc_urb(0, GFP_KERNEL);
+		if (!dev->surbs[n_urb].urb)
+			goto free_urbs;
 	}
 
 	pr_debug("smsusb_start_streaming(...).\n");
@@ -485,6 +493,10 @@ static int smsusb_init_device(struct usb_interface *intf, int board_id)
 
 	return rc;
 
+free_urbs:
+	for (i = 0; i < n_urb; i++)
+		usb_free_urb(dev->surbs[n_urb].urb);
+
 err_unregister_device:
 	smsusb_term_device(intf);
 #ifdef CONFIG_MEDIA_CONTROLLER_DVB

-- 
2.44.0.478.gd926399ef9-goog

Re: [PATCH 1/6] media: usb: siano: Fix allocation of urbs

[Hans Verkuil]

On 10/04/2024 23:54, Ricardo Ribalda wrote:
> USB urbs must be allocated with usb_alloc_urb. Quoting the manual
> 
> Only use this function (usb_init_urb) if you _really_ understand what you
> are doing.
> 
> Fix the following smatch error:
> 
> drivers/media/usb/siano/smsusb.c:53:38: warning: array of flexible structures
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  drivers/media/usb/siano/smsusb.c | 28 ++++++++++++++++++++--------
>  1 file changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/usb/siano/smsusb.c b/drivers/media/usb/siano/smsusb.c
> index 723510520d092..d85308e0785db 100644
> --- a/drivers/media/usb/siano/smsusb.c
> +++ b/drivers/media/usb/siano/smsusb.c
> @@ -40,7 +40,7 @@ struct smsusb_urb_t {
>  	struct smscore_buffer_t *cb;
>  	struct smsusb_device_t *dev;
>  
> -	struct urb urb;
> +	struct urb *urb;
>  
>  	/* For the bottom half */
>  	struct work_struct wq;
> @@ -160,7 +160,7 @@ static int smsusb_submit_urb(struct smsusb_device_t *dev,
>  	}
>  
>  	usb_fill_bulk_urb(
> -		&surb->urb,
> +		surb->urb,
>  		dev->udev,
>  		usb_rcvbulkpipe(dev->udev, dev->in_ep),
>  		surb->cb->p,
> @@ -168,9 +168,9 @@ static int smsusb_submit_urb(struct smsusb_device_t *dev,
>  		smsusb_onresponse,
>  		surb
>  	);
> -	surb->urb.transfer_flags |= URB_FREE_BUFFER;
> +	surb->urb->transfer_flags |= URB_FREE_BUFFER;
>  
> -	return usb_submit_urb(&surb->urb, GFP_ATOMIC);
> +	return usb_submit_urb(surb->urb, GFP_ATOMIC);
>  }
>  
>  static void smsusb_stop_streaming(struct smsusb_device_t *dev)
> @@ -178,7 +178,7 @@ static void smsusb_stop_streaming(struct smsusb_device_t *dev)
>  	int i;
>  
>  	for (i = 0; i < MAX_URBS; i++) {
> -		usb_kill_urb(&dev->surbs[i].urb);
> +		usb_kill_urb(dev->surbs[i].urb);
>  		if (dev->surbs[i].wq.func)
>  			cancel_work_sync(&dev->surbs[i].wq);
>  
> @@ -338,6 +338,8 @@ static void smsusb_term_device(struct usb_interface *intf)
>  	struct smsusb_device_t *dev = usb_get_intfdata(intf);
>  
>  	if (dev) {
> +		int i;
> +
>  		dev->state = SMSUSB_DISCONNECTED;
>  
>  		smsusb_stop_streaming(dev);
> @@ -346,6 +348,9 @@ static void smsusb_term_device(struct usb_interface *intf)
>  		if (dev->coredev)
>  			smscore_unregister_device(dev->coredev);
>  
> +		for (i = 0; i < MAX_URBS; i++)
> +			usb_free_urb(dev->surbs[i].urb);
> +
>  		pr_debug("device 0x%p destroyed\n", dev);
>  		kfree(dev);
>  	}
> @@ -390,6 +395,7 @@ static int smsusb_init_device(struct usb_interface *intf, int board_id)
>  	void *mdev;
>  	int i, rc;
>  	int align = 0;
> +	int n_urb = 0;
>  
>  	/* create device object */
>  	dev = kzalloc(sizeof(struct smsusb_device_t), GFP_KERNEL);
> @@ -461,9 +467,11 @@ static int smsusb_init_device(struct usb_interface *intf, int board_id)
>  	dev->coredev->is_usb_device = true;
>  
>  	/* initialize urbs */
> -	for (i = 0; i < MAX_URBS; i++) {
> -		dev->surbs[i].dev = dev;
> -		usb_init_urb(&dev->surbs[i].urb);
> +	for (n_urb = 0; n_urb < MAX_URBS; n_urb++) {
> +		dev->surbs[n_urb].dev = dev;
> +		dev->surbs[n_urb].urb = usb_alloc_urb(0, GFP_KERNEL);
> +		if (!dev->surbs[n_urb].urb)
> +			goto free_urbs;
>  	}

After allocating the URBs there are a few more error paths that do
'goto err_unregister_device;' instead of 'goto free_urbs;'. From what
I can see, those need to go through 'free_urbs' as well.

>  
>  	pr_debug("smsusb_start_streaming(...).\n");
> @@ -485,6 +493,10 @@ static int smsusb_init_device(struct usb_interface *intf, int board_id)
>  
>  	return rc;
>  
> +free_urbs:
> +	for (i = 0; i < n_urb; i++)
> +		usb_free_urb(dev->surbs[n_urb].urb);

Would it be better to also assign NULL to dev->surbs[n_urb].urb?
That way there are no invalid pointers that can mess up things.

Regards,

	Hans

> +
>  err_unregister_device:
>  	smsusb_term_device(intf);
>  #ifdef CONFIG_MEDIA_CONTROLLER_DVB
> 

Re: [PATCH 1/6] media: usb: siano: Fix allocation of urbs

[Hans Verkuil]

On 15/04/2024 12:08, Hans Verkuil wrote:
> On 10/04/2024 23:54, Ricardo Ribalda wrote:
>> USB urbs must be allocated with usb_alloc_urb. Quoting the manual
>>
>> Only use this function (usb_init_urb) if you _really_ understand what you
>> are doing.
>>
>> Fix the following smatch error:
>>
>> drivers/media/usb/siano/smsusb.c:53:38: warning: array of flexible structures
>>
>> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
>> ---
>>  drivers/media/usb/siano/smsusb.c | 28 ++++++++++++++++++++--------
>>  1 file changed, 20 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/media/usb/siano/smsusb.c b/drivers/media/usb/siano/smsusb.c
>> index 723510520d092..d85308e0785db 100644
>> --- a/drivers/media/usb/siano/smsusb.c
>> +++ b/drivers/media/usb/siano/smsusb.c
>> @@ -40,7 +40,7 @@ struct smsusb_urb_t {
>>  	struct smscore_buffer_t *cb;
>>  	struct smsusb_device_t *dev;
>>  
>> -	struct urb urb;
>> +	struct urb *urb;
>>  
>>  	/* For the bottom half */
>>  	struct work_struct wq;
>> @@ -160,7 +160,7 @@ static int smsusb_submit_urb(struct smsusb_device_t *dev,
>>  	}
>>  
>>  	usb_fill_bulk_urb(
>> -		&surb->urb,
>> +		surb->urb,
>>  		dev->udev,
>>  		usb_rcvbulkpipe(dev->udev, dev->in_ep),
>>  		surb->cb->p,
>> @@ -168,9 +168,9 @@ static int smsusb_submit_urb(struct smsusb_device_t *dev,
>>  		smsusb_onresponse,
>>  		surb
>>  	);
>> -	surb->urb.transfer_flags |= URB_FREE_BUFFER;
>> +	surb->urb->transfer_flags |= URB_FREE_BUFFER;
>>  
>> -	return usb_submit_urb(&surb->urb, GFP_ATOMIC);
>> +	return usb_submit_urb(surb->urb, GFP_ATOMIC);
>>  }
>>  
>>  static void smsusb_stop_streaming(struct smsusb_device_t *dev)
>> @@ -178,7 +178,7 @@ static void smsusb_stop_streaming(struct smsusb_device_t *dev)
>>  	int i;
>>  
>>  	for (i = 0; i < MAX_URBS; i++) {
>> -		usb_kill_urb(&dev->surbs[i].urb);
>> +		usb_kill_urb(dev->surbs[i].urb);
>>  		if (dev->surbs[i].wq.func)
>>  			cancel_work_sync(&dev->surbs[i].wq);
>>  
>> @@ -338,6 +338,8 @@ static void smsusb_term_device(struct usb_interface *intf)
>>  	struct smsusb_device_t *dev = usb_get_intfdata(intf);
>>  
>>  	if (dev) {
>> +		int i;
>> +
>>  		dev->state = SMSUSB_DISCONNECTED;
>>  
>>  		smsusb_stop_streaming(dev);
>> @@ -346,6 +348,9 @@ static void smsusb_term_device(struct usb_interface *intf)
>>  		if (dev->coredev)
>>  			smscore_unregister_device(dev->coredev);
>>  
>> +		for (i = 0; i < MAX_URBS; i++)
>> +			usb_free_urb(dev->surbs[i].urb);
>> +
>>  		pr_debug("device 0x%p destroyed\n", dev);
>>  		kfree(dev);
>>  	}
>> @@ -390,6 +395,7 @@ static int smsusb_init_device(struct usb_interface *intf, int board_id)
>>  	void *mdev;
>>  	int i, rc;
>>  	int align = 0;
>> +	int n_urb = 0;
>>  
>>  	/* create device object */
>>  	dev = kzalloc(sizeof(struct smsusb_device_t), GFP_KERNEL);
>> @@ -461,9 +467,11 @@ static int smsusb_init_device(struct usb_interface *intf, int board_id)
>>  	dev->coredev->is_usb_device = true;
>>  
>>  	/* initialize urbs */
>> -	for (i = 0; i < MAX_URBS; i++) {
>> -		dev->surbs[i].dev = dev;
>> -		usb_init_urb(&dev->surbs[i].urb);
>> +	for (n_urb = 0; n_urb < MAX_URBS; n_urb++) {
>> +		dev->surbs[n_urb].dev = dev;
>> +		dev->surbs[n_urb].urb = usb_alloc_urb(0, GFP_KERNEL);
>> +		if (!dev->surbs[n_urb].urb)
>> +			goto free_urbs;
>>  	}
> 
> After allocating the URBs there are a few more error paths that do
> 'goto err_unregister_device;' instead of 'goto free_urbs;'. From what
> I can see, those need to go through 'free_urbs' as well.
> 
>>  
>>  	pr_debug("smsusb_start_streaming(...).\n");
>> @@ -485,6 +493,10 @@ static int smsusb_init_device(struct usb_interface *intf, int board_id)
>>  
>>  	return rc;
>>  
>> +free_urbs:
>> +	for (i = 0; i < n_urb; i++)
>> +		usb_free_urb(dev->surbs[n_urb].urb);
> 
> Would it be better to also assign NULL to dev->surbs[n_urb].urb?
> That way there are no invalid pointers that can mess up things.

Ricardo, just post a v2 of this patch. I posted a PR for the other
patches. Nice work, BTW!

Regards,

	Hans

> 
> Regards,
> 
> 	Hans
> 
>> +
>>  err_unregister_device:
>>  	smsusb_term_device(intf);
>>  #ifdef CONFIG_MEDIA_CONTROLLER_DVB
>>
> 
> 

[PATCH 2/6] media: cxd2880: Replaze kmalloc with kzalloc

[Ricardo Ribalda]

Fix smatch error:
drivers/media/spi/cxd2880-spi.c:391 cxd2880_start_feed() warn: Please consider using kzalloc instead of kmalloc

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/spi/cxd2880-spi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/spi/cxd2880-spi.c b/drivers/media/spi/cxd2880-spi.c
index 6be4e5528879f..65fa7f857fcaf 100644
--- a/drivers/media/spi/cxd2880-spi.c
+++ b/drivers/media/spi/cxd2880-spi.c
@@ -388,7 +388,7 @@ static int cxd2880_start_feed(struct dvb_demux_feed *feed)
 
 	if (dvb_spi->feed_count == 0) {
 		dvb_spi->ts_buf =
-			kmalloc(MAX_TRANS_PKT * 188,
+			kzalloc(MAX_TRANS_PKT * 188,
 				GFP_KERNEL | GFP_DMA);
 		if (!dvb_spi->ts_buf) {
 			pr_err("ts buffer allocate failed\n");

-- 
2.44.0.478.gd926399ef9-goog

[PATCH 3/6] media: platform: sti: hva: clk_unprepare unconditionally

[Ricardo Ribalda]

hva->clk cannot be NULL at this point. Simplify the code and make smatch
happy:

drivers/media/platform/st/sti/hva/hva-hw.c:412 hva_hw_probe() warn: 'hva->clk' from clk_prepare() not released on lines: 412

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/platform/st/sti/hva/hva-hw.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/platform/st/sti/hva/hva-hw.c b/drivers/media/platform/st/sti/hva/hva-hw.c
index fe4ea2e7f37e3..fcb18fb52fdd7 100644
--- a/drivers/media/platform/st/sti/hva/hva-hw.c
+++ b/drivers/media/platform/st/sti/hva/hva-hw.c
@@ -406,8 +406,7 @@ int hva_hw_probe(struct platform_device *pdev, struct hva_dev *hva)
 err_disable:
 	pm_runtime_disable(dev);
 err_clk:
-	if (hva->clk)
-		clk_unprepare(hva->clk);
+	clk_unprepare(hva->clk);
 
 	return ret;
 }

-- 
2.44.0.478.gd926399ef9-goog

[PATCH 4/6] media: v4l2-ctrls-core.c: Do not use iterator outside loop

[Ricardo Ribalda]

Simplify a bit the code introducing a new variable for iterating through
the control list.

It also makes smatch happy:

drivers/media/v4l2-core/v4l2-ctrls-api.c:1091 v4l2_query_ext_ctrl() warn: iterator used outside loop: 'ref'

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/v4l2-core/v4l2-ctrls-api.c | 33 ++++++++++++++++++--------------
 1 file changed, 19 insertions(+), 14 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls-api.c b/drivers/media/v4l2-core/v4l2-ctrls-api.c
index d9a422017bd9d..42b7a45bfa79c 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls-api.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls-api.c
@@ -1052,35 +1052,40 @@ int v4l2_query_ext_ctrl(struct v4l2_ctrl_handler *hdl, struct v4l2_query_ext_ctr
 		if (id >= node2id(hdl->ctrl_refs.prev)) {
 			ref = NULL; /* Yes, so there is no next control */
 		} else if (ref) {
+			struct v4l2_ctrl_ref *pos  = ref;
+
 			/*
 			 * We found a control with the given ID, so just get
 			 * the next valid one in the list.
 			 */
-			list_for_each_entry_continue(ref, &hdl->ctrl_refs, node) {
-				is_compound = ref->ctrl->is_array ||
-					ref->ctrl->type >= V4L2_CTRL_COMPOUND_TYPES;
-				if (id < ref->ctrl->id &&
-				    (is_compound & mask) == match)
+			ref = NULL;
+			list_for_each_entry_continue(pos, &hdl->ctrl_refs, node) {
+				is_compound = pos->ctrl->is_array ||
+					pos->ctrl->type >= V4L2_CTRL_COMPOUND_TYPES;
+				if (id < pos->ctrl->id &&
+				    (is_compound & mask) == match) {
+					ref = pos;
 					break;
+				}
 			}
-			if (&ref->node == &hdl->ctrl_refs)
-				ref = NULL;
 		} else {
+			struct v4l2_ctrl_ref *pos;
+
 			/*
 			 * No control with the given ID exists, so start
 			 * searching for the next largest ID. We know there
 			 * is one, otherwise the first 'if' above would have
 			 * been true.
 			 */
-			list_for_each_entry(ref, &hdl->ctrl_refs, node) {
-				is_compound = ref->ctrl->is_array ||
-					ref->ctrl->type >= V4L2_CTRL_COMPOUND_TYPES;
-				if (id < ref->ctrl->id &&
-				    (is_compound & mask) == match)
+			list_for_each_entry(pos, &hdl->ctrl_refs, node) {
+				is_compound = pos->ctrl->is_array ||
+					pos->ctrl->type >= V4L2_CTRL_COMPOUND_TYPES;
+				if (id < pos->ctrl->id &&
+				    (is_compound & mask) == match) {
+					ref = pos;
 					break;
+				}
 			}
-			if (&ref->node == &hdl->ctrl_refs)
-				ref = NULL;
 		}
 	}
 	mutex_unlock(hdl->lock);

-- 
2.44.0.478.gd926399ef9-goog

[PATCH 5/6] media: adv7180: Only request valids IRQs

[Ricardo Ribalda]

i2c_device_probe(), seems to assume that irq = 0 means that there is no
irq to request.

The driver also believes that on the clean path. So lets be consistent
here.

Also make smatch happy.

Fix:
drivers/media/i2c/adv7180.c:1526 adv7180_probe() warn: 'client->irq' from request_threaded_irq() not released on lines: 1526

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/i2c/adv7180.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 4829cbe324198..819ff9f7c90fe 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -1486,7 +1486,7 @@ static int adv7180_probe(struct i2c_client *client)
 	if (ret)
 		goto err_media_entity_cleanup;
 
-	if (state->irq) {
+	if (state->irq > 0) {
 		ret = request_threaded_irq(client->irq, NULL, adv7180_irq,
 					   IRQF_ONESHOT | IRQF_TRIGGER_FALLING,
 					   KBUILD_MODNAME, state);

-- 
2.44.0.478.gd926399ef9-goog

[PATCH 6/6] media: touchscreen: sur40: convert le16 to cpu before use

[Ricardo Ribalda]

Smatch found this issue:
drivers/input/touchscreen/sur40.c:424:55: warning: incorrect type in argument 2 (different base types)
drivers/input/touchscreen/sur40.c:424:55:    expected int key
drivers/input/touchscreen/sur40.c:424:55:    got restricted __le16 [usertype] blob_id

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/input/touchscreen/sur40.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index ae3aab4283370..5f2cf8881e724 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -421,7 +421,7 @@ static void sur40_report_blob(struct sur40_blob *blob, struct input_dev *input)
 	if (blob->type != SUR40_TOUCH)
 		return;
 
-	slotnum = input_mt_get_slot_by_key(input, blob->blob_id);
+	slotnum = input_mt_get_slot_by_key(input, le16_to_cpu(blob->blob_id));
 	if (slotnum < 0 || slotnum >= MAX_CONTACTS)
 		return;
 

-- 
2.44.0.478.gd926399ef9-goog

Re: [PATCH 6/6] media: touchscreen: sur40: convert le16 to cpu before use

[Dmitry Torokhov]

On Wed, Apr 10, 2024 at 09:54:43PM +0000, Ricardo Ribalda wrote:
> Smatch found this issue:
> drivers/input/touchscreen/sur40.c:424:55: warning: incorrect type in argument 2 (different base types)
> drivers/input/touchscreen/sur40.c:424:55:    expected int key
> drivers/input/touchscreen/sur40.c:424:55:    got restricted __le16 [usertype] blob_id
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  drivers/input/touchscreen/sur40.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
> index ae3aab4283370..5f2cf8881e724 100644
> --- a/drivers/input/touchscreen/sur40.c
> +++ b/drivers/input/touchscreen/sur40.c
> @@ -421,7 +421,7 @@ static void sur40_report_blob(struct sur40_blob *blob, struct input_dev *input)
>  	if (blob->type != SUR40_TOUCH)
>  		return;
>  
> -	slotnum = input_mt_get_slot_by_key(input, blob->blob_id);
> +	slotnum = input_mt_get_slot_by_key(input, le16_to_cpu(blob->blob_id));

We are not really using the real value of the ID for computations but only
as an opaque "key", so doing the conversion is not strictly necessary,
but it is cheap so we may as well do it.

>  	if (slotnum < 0 || slotnum >= MAX_CONTACTS)
>  		return;

Applied, thank you.

-- 
Dmitry