[PATCH 0/9] media: Fix all missing cocci warnings

[PATCH 0/9] media: Fix all missing cocci warnings

[Ricardo Ribalda]

With the introduction of 7.2, cocci is triggering some new warnings in
our codebase. This series takes care of the new warnings and refloats an
old fix for dvb_frontend.

./platform/amd/isp4/isp4_subdev.c:394:6-25: WARNING: atomic_dec_and_test variation before object free at line 395.
./dvb-frontends/helene.c:1049:2-7: WARNING: invalid free of devm_ allocated data
./dvb-frontends/helene.c:1013:2-7: WARNING: invalid free of devm_ allocated data
./dvb-core/dvb_frontend.c:2897:1-7: preceding lock on line 2776
./dvb-core/dvb_frontend.c:2897:1-7: preceding lock on line 2786
./dvb-core/dvb_frontend.c:2897:1-7: preceding lock on line 2809
./test-drivers/vimc/vimc-sensor.c:107:1-7: WARNING: do_div() does a 64-by-32 division, please consider using div64_u64 instead.
./platform/qcom/iris/iris_vpu_buffer.c:703:13-15: WARNING opportunity for max()
./platform/qcom/iris/iris_vpu_buffer.c:583:23-25: WARNING opportunity for max()
./usb/em28xx/em28xx-cards.c:4085:2-3: Unneeded semicolon
./usb/em28xx/em28xx-core.c:635:2-3: Unneeded semicolon

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
Ricardo Ribalda (9):
      media: em28xx-video: Remove unneeded semicolons
      media: iris: Replace ternary conditionals with max()
      media: vimc: Fix prototype of vimc_sensor_update_frame_timing
      media: vimc: Ensure that pixel_rate fits in 32 bits
      media: platform: amd: use refcount_t instead of atomic_t
      media: dvb-frontends/helene: Rename priv variable
      media: drivers/media/dvb-core: Split dvb_frontend_open()
      media: drivers/media/dvb-core: Refactor dvb_frontend_open locking
      media: drivers/media/dvb-core: CodeStyle for dvb_frontend_open()

 drivers/media/dvb-core/dvb_frontend.c              | 159 ++++++++++++---------
 drivers/media/dvb-frontends/helene.c               |  56 ++++----
 drivers/media/platform/amd/isp4/isp4_interface.c   |   4 +-
 drivers/media/platform/amd/isp4/isp4_interface.h   |   2 +-
 drivers/media/platform/amd/isp4/isp4_subdev.c      |   2 +-
 drivers/media/platform/qcom/iris/iris_vpu_buffer.c |   4 +-
 drivers/media/test-drivers/vimc/vimc-sensor.c      |  12 +-
 drivers/media/usb/em28xx/em28xx-cards.c            |   2 +-
 drivers/media/usb/em28xx/em28xx-core.c             |   2 +-
 9 files changed, 134 insertions(+), 109 deletions(-)
---
base-commit: 253355887a1ab0ac8f33b356c7c1140eee554d18
change-id: 20260629-cocci-7-2-7d30217e6526

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

[PATCH 1/9] media: em28xx-video: Remove unneeded semicolons

[Ricardo Ribalda]

There is no need to add a semicolon after a switch statement.

It also makes cocci a bit uneasy. It triggers the following warnings:

./usb/em28xx/em28xx-cards.c:4085:2-3: Unneeded semicolon
./usb/em28xx/em28xx-core.c:635:2-3: Unneeded semicolon

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/usb/em28xx/em28xx-cards.c | 2 +-
 drivers/media/usb/em28xx/em28xx-core.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
index fbfb74eab475..133bc1082d57 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -4082,7 +4082,7 @@ static void em28xx_check_usb_descriptor(struct em28xx *dev,
 			dev->analog_ep_bulk = e->bEndpointAddress;
 		}
 		return;
-	};
+	}
 }
 
 /*
diff --git a/drivers/media/usb/em28xx/em28xx-core.c b/drivers/media/usb/em28xx/em28xx-core.c
index 5bbb082dbed9..d4197e37f637 100644
--- a/drivers/media/usb/em28xx/em28xx-core.c
+++ b/drivers/media/usb/em28xx/em28xx-core.c
@@ -632,7 +632,7 @@ void em2828X_decoder_vmux(struct em28xx *dev, unsigned int vin)
 	default:
 		dev_dbg(&dev->intf->dev, "EM2828X_SVIDEO\n");
 		break;
-	};
+	}
 
 	em28xx_write_reg(dev, 0x24, 0x00);
 	em28xx_write_reg(dev, 0x25, 0x02);

-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH 2/9] media: iris: Replace ternary conditionals with max()

[Ricardo Ribalda]

The max() macro is simpler to read than the current construction, it
also makes cocci happier, which currently throws these warnings:

./platform/qcom/iris/iris_vpu_buffer.c:703:13-15: WARNING opportunity for max()
./platform/qcom/iris/iris_vpu_buffer.c:583:23-25: WARNING opportunity for max()

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/platform/qcom/iris/iris_vpu_buffer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/qcom/iris/iris_vpu_buffer.c b/drivers/media/platform/qcom/iris/iris_vpu_buffer.c
index fb6f1016415e..faebb5472866 100644
--- a/drivers/media/platform/qcom/iris/iris_vpu_buffer.c
+++ b/drivers/media/platform/qcom/iris/iris_vpu_buffer.c
@@ -580,7 +580,7 @@ static u32 hfi_buffer_line_av1d(u32 frame_width, u32 frame_height,
 		ALIGN(size_av1d_qp(frame_width, frame_height), DMA_ALIGNMENT);
 	opbwr8 = size_av1d_lb_opb_wr1_nv12_ubwc(frame_width, frame_height);
 	opbwr10 = size_av1d_lb_opb_wr1_tp10_ubwc(frame_width, frame_height);
-	opbwrbufsize = opbwr8 >= opbwr10 ? opbwr8 : opbwr10;
+	opbwrbufsize = max(opbwr8, opbwr10);
 	size = ALIGN((size + opbwrbufsize), DMA_ALIGNMENT);
 	if (is_opb) {
 		vpss_lb_size = size_vpss_lb(frame_width, frame_height);
@@ -700,7 +700,7 @@ static u32 hfi_buffer_ibc_av1d(u32 frame_width, u32 frame_height)
 
 	ibc8 = size_av1d_ibc_nv12_ubwc(frame_width, frame_height);
 	ibc10 = size_av1d_ibc_tp10_ubwc(frame_width, frame_height);
-	size = ibc8 >= ibc10 ? ibc8 : ibc10;
+	size = max(ibc8, ibc10);
 
 	return ALIGN(size, DMA_ALIGNMENT);
 }

-- 
2.55.0.rc0.799.gd6f94ed593-goog

Re: [PATCH 2/9] media: iris: Replace ternary conditionals with max()

[Bryan O'Donoghue]

On 29/06/2026 12:30, Ricardo Ribalda wrote:
> The max() macro is simpler to read than the current construction, it
> also makes cocci happier, which currently throws these warnings:
> 
> ./platform/qcom/iris/iris_vpu_buffer.c:703:13-15: WARNING opportunity for max()
> ./platform/qcom/iris/iris_vpu_buffer.c:583:23-25: WARNING opportunity for max()
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>   drivers/media/platform/qcom/iris/iris_vpu_buffer.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/iris/iris_vpu_buffer.c b/drivers/media/platform/qcom/iris/iris_vpu_buffer.c
> index fb6f1016415e..faebb5472866 100644
> --- a/drivers/media/platform/qcom/iris/iris_vpu_buffer.c
> +++ b/drivers/media/platform/qcom/iris/iris_vpu_buffer.c
> @@ -580,7 +580,7 @@ static u32 hfi_buffer_line_av1d(u32 frame_width, u32 frame_height,
>   		ALIGN(size_av1d_qp(frame_width, frame_height), DMA_ALIGNMENT);
>   	opbwr8 = size_av1d_lb_opb_wr1_nv12_ubwc(frame_width, frame_height);
>   	opbwr10 = size_av1d_lb_opb_wr1_tp10_ubwc(frame_width, frame_height);
> -	opbwrbufsize = opbwr8 >= opbwr10 ? opbwr8 : opbwr10;
> +	opbwrbufsize = max(opbwr8, opbwr10);
>   	size = ALIGN((size + opbwrbufsize), DMA_ALIGNMENT);
>   	if (is_opb) {
>   		vpss_lb_size = size_vpss_lb(frame_width, frame_height);
> @@ -700,7 +700,7 @@ static u32 hfi_buffer_ibc_av1d(u32 frame_width, u32 frame_height)
> 
>   	ibc8 = size_av1d_ibc_nv12_ubwc(frame_width, frame_height);
>   	ibc10 = size_av1d_ibc_tp10_ubwc(frame_width, frame_height);
> -	size = ibc8 >= ibc10 ? ibc8 : ibc10;
> +	size = max(ibc8, ibc10);
> 
>   	return ALIGN(size, DMA_ALIGNMENT);
>   }
> 
> --
> 2.55.0.rc0.799.gd6f94ed593-goog
> 

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

---
bod

[PATCH 3/9] media: vimc: Fix prototype of vimc_sensor_update_frame_timing

[Ricardo Ribalda]

The function does not return any value, make it into a void function.

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/test-drivers/vimc/vimc-sensor.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/media/test-drivers/vimc/vimc-sensor.c b/drivers/media/test-drivers/vimc/vimc-sensor.c
index 5deebcc78a33..d125a79fec8c 100644
--- a/drivers/media/test-drivers/vimc/vimc-sensor.c
+++ b/drivers/media/test-drivers/vimc/vimc-sensor.c
@@ -92,8 +92,8 @@ static void vimc_sensor_tpg_s_format(struct vimc_sensor_device *vsensor,
 	tpg_s_xfer_func(&vsensor->tpg, format->xfer_func);
 }
 
-static int vimc_sensor_update_frame_timing(struct v4l2_subdev *sd,
-					   u32 width, u32 height)
+static void vimc_sensor_update_frame_timing(struct v4l2_subdev *sd,
+					    u32 width, u32 height)
 {
 	struct vimc_sensor_device *vsensor =
 		container_of(sd, struct vimc_sensor_device, sd);
@@ -108,8 +108,6 @@ static int vimc_sensor_update_frame_timing(struct v4l2_subdev *sd,
 	vsensor->hw.fps_jiffies = nsecs_to_jiffies(frame_interval_ns);
 	if (vsensor->hw.fps_jiffies == 0)
 		vsensor->hw.fps_jiffies = 1;
-
-	return 0;
 }
 
 static void vimc_sensor_adjust_fmt(struct v4l2_mbus_framefmt *fmt)

-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH 4/9] media: vimc: Ensure that pixel_rate fits in 32 bits

[Ricardo Ribalda]

pixel_rate is set to VIMC_PIXEL_RATE_FIXED, which the code expects to
fit in 32 bits. Make that constraint into a WARN_ON, so if we ever break
that constraint a kernel warning will be triggered.

It also fixes the following cocci warning:
./test-drivers/vimc/vimc-sensor.c:107:1-7: WARNING: do_div() does a 64-by-32 division, please consider using div64_u64 instead.

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/test-drivers/vimc/vimc-sensor.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/media/test-drivers/vimc/vimc-sensor.c b/drivers/media/test-drivers/vimc/vimc-sensor.c
index d125a79fec8c..83dcc9d61ee0 100644
--- a/drivers/media/test-drivers/vimc/vimc-sensor.c
+++ b/drivers/media/test-drivers/vimc/vimc-sensor.c
@@ -103,8 +103,12 @@ static void vimc_sensor_update_frame_timing(struct v4l2_subdev *sd,
 	u64 total_pixels = (u64)hts * vts;
 	u64 frame_interval_ns;
 
+	/* Sanity check, pixel rate is fixed and fits in 32 bits. */
+	if (WARN_ON(pixel_rate >= 0x100000000))
+		return;
+
 	frame_interval_ns = total_pixels * NSEC_PER_SEC;
-	do_div(frame_interval_ns, pixel_rate);
+	do_div(frame_interval_ns, (u32)pixel_rate);
 	vsensor->hw.fps_jiffies = nsecs_to_jiffies(frame_interval_ns);
 	if (vsensor->hw.fps_jiffies == 0)
 		vsensor->hw.fps_jiffies = 1;

-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH 5/9] media: platform: amd: use refcount_t instead of atomic_t

[Ricardo Ribalda]

We are using the refcnt variable for refcounting. Use the refcount_t
type instead, as it has support for saturation and underflow.

This also makes cocci happier, as it will fix the following warning:
./platform/amd/isp4/isp4_subdev.c:394:6-25: WARNING: atomic_dec_and_test variation before object free at line 395.

Fixes: 4c5feef6a62c ("media: platform: amd: Add isp4 fw and hw interface")
Cc: stable@vger.kernel.org
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/platform/amd/isp4/isp4_interface.c | 4 ++--
 drivers/media/platform/amd/isp4/isp4_interface.h | 2 +-
 drivers/media/platform/amd/isp4/isp4_subdev.c    | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/amd/isp4/isp4_interface.c b/drivers/media/platform/amd/isp4/isp4_interface.c
index 8d73f66bb42c..00a817909292 100644
--- a/drivers/media/platform/amd/isp4/isp4_interface.c
+++ b/drivers/media/platform/amd/isp4/isp4_interface.c
@@ -375,7 +375,7 @@ static int isp4if_send_fw_cmd(struct isp4_interface *ispif, u32 cmd_id,
 			return -ENOMEM;
 
 		/* Get two references: one for the resp thread, one for us */
-		atomic_set(&ele->refcnt, 2);
+		refcount_set(&ele->refcnt, 2);
 		init_completion(&ele->cmd_done);
 	}
 
@@ -455,7 +455,7 @@ static int isp4if_send_fw_cmd(struct isp4_interface *ispif, u32 cmd_id,
 
 put_ele_ref:
 	/* Don't free the command if we didn't put the last reference */
-	if (ele && atomic_dec_return(&ele->refcnt))
+	if (ele && !refcount_dec_and_test(&ele->refcnt))
 		ele = NULL;
 
 free_ele:
diff --git a/drivers/media/platform/amd/isp4/isp4_interface.h b/drivers/media/platform/amd/isp4/isp4_interface.h
index ce3ac9b9e5cd..04db71cd54e6 100644
--- a/drivers/media/platform/amd/isp4/isp4_interface.h
+++ b/drivers/media/platform/amd/isp4/isp4_interface.h
@@ -68,7 +68,7 @@ struct isp4if_cmd_element {
 	u32 seq_num;
 	u32 cmd_id;
 	struct completion cmd_done;
-	atomic_t refcnt;
+	refcount_t refcnt;
 };
 
 struct isp4_interface {
diff --git a/drivers/media/platform/amd/isp4/isp4_subdev.c b/drivers/media/platform/amd/isp4/isp4_subdev.c
index 48deea79ce6c..2a8bc1207843 100644
--- a/drivers/media/platform/amd/isp4/isp4_subdev.c
+++ b/drivers/media/platform/amd/isp4/isp4_subdev.c
@@ -391,7 +391,7 @@ static void isp4sd_fw_resp_cmd_done(struct isp4_subdev *isp_subdev,
 
 	if (ele) {
 		complete(&ele->cmd_done);
-		if (atomic_dec_and_test(&ele->refcnt))
+		if (refcount_dec_and_test(&ele->refcnt))
 			kfree(ele);
 	}
 }

-- 
2.55.0.rc0.799.gd6f94ed593-goog

Re: [PATCH 5/9] media: platform: amd: use refcount_t instead of atomic_t

[Nirujogi, Pratap]

On 6/29/2026 7:30 AM, Ricardo Ribalda wrote:
> [You don't often get email from ribalda@chromium.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> We are using the refcnt variable for refcounting. Use the refcount_t
> type instead, as it has support for saturation and underflow.
> 
> This also makes cocci happier, as it will fix the following warning:
> ./platform/amd/isp4/isp4_subdev.c:394:6-25: WARNING: atomic_dec_and_test variation before object free at line 395.
> 
> Fixes: 4c5feef6a62c ("media: platform: amd: Add isp4 fw and hw interface")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>   drivers/media/platform/amd/isp4/isp4_interface.c | 4 ++--
>   drivers/media/platform/amd/isp4/isp4_interface.h | 2 +-
>   drivers/media/platform/amd/isp4/isp4_subdev.c    | 2 +-
>   3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/platform/amd/isp4/isp4_interface.c b/drivers/media/platform/amd/isp4/isp4_interface.c
> index 8d73f66bb42c..00a817909292 100644
> --- a/drivers/media/platform/amd/isp4/isp4_interface.c
> +++ b/drivers/media/platform/amd/isp4/isp4_interface.c
> @@ -375,7 +375,7 @@ static int isp4if_send_fw_cmd(struct isp4_interface *ispif, u32 cmd_id,
>                          return -ENOMEM;
> 
>                  /* Get two references: one for the resp thread, one for us */
> -               atomic_set(&ele->refcnt, 2);
> +               refcount_set(&ele->refcnt, 2);
>                  init_completion(&ele->cmd_done);
>          }
> 
> @@ -455,7 +455,7 @@ static int isp4if_send_fw_cmd(struct isp4_interface *ispif, u32 cmd_id,
> 
>   put_ele_ref:
>          /* Don't free the command if we didn't put the last reference */
> -       if (ele && atomic_dec_return(&ele->refcnt))
> +       if (ele && !refcount_dec_and_test(&ele->refcnt))
>                  ele = NULL;
> 
>   free_ele:
> diff --git a/drivers/media/platform/amd/isp4/isp4_interface.h b/drivers/media/platform/amd/isp4/isp4_interface.h
> index ce3ac9b9e5cd..04db71cd54e6 100644
> --- a/drivers/media/platform/amd/isp4/isp4_interface.h
> +++ b/drivers/media/platform/amd/isp4/isp4_interface.h
> @@ -68,7 +68,7 @@ struct isp4if_cmd_element {
>          u32 seq_num;
>          u32 cmd_id;
>          struct completion cmd_done;
> -       atomic_t refcnt;
> +       refcount_t refcnt;
>   };
> 
>   struct isp4_interface {
> diff --git a/drivers/media/platform/amd/isp4/isp4_subdev.c b/drivers/media/platform/amd/isp4/isp4_subdev.c
> index 48deea79ce6c..2a8bc1207843 100644
> --- a/drivers/media/platform/amd/isp4/isp4_subdev.c
> +++ b/drivers/media/platform/amd/isp4/isp4_subdev.c
> @@ -391,7 +391,7 @@ static void isp4sd_fw_resp_cmd_done(struct isp4_subdev *isp_subdev,
> 
>          if (ele) {
>                  complete(&ele->cmd_done);
> -               if (atomic_dec_and_test(&ele->refcnt))
> +               if (refcount_dec_and_test(&ele->refcnt))
>                          kfree(ele);
>          }
>   }
> 
> --
> 2.55.0.rc0.799.gd6f94ed593-goog
> 

Reviewed-by: Pratap Nirujogi <pratap.nirujogi@amd.com>

Thanks,
Pratap

Re: [PATCH 5/9] media: platform: amd: use refcount_t instead of atomic_t

[Bin Du]

On 6/29/2026 11:51 PM, Nirujogi, Pratap wrote:
> 
> 
> On 6/29/2026 7:30 AM, Ricardo Ribalda wrote:
>> [You don't often get email from ribalda@chromium.org. Learn why this
>> is important at https://aka.ms/LearnAboutSenderIdentification ]
>>
>> Caution: This message originated from an External Source. Use proper
>> caution when opening attachments, clicking links, or responding.
>>
>>
>> We are using the refcnt variable for refcounting. Use the refcount_t
>> type instead, as it has support for saturation and underflow.
>>
>> This also makes cocci happier, as it will fix the following warning:
>> ./platform/amd/isp4/isp4_subdev.c:394:6-25: WARNING:
>> atomic_dec_and_test variation before object free at line 395.
>>
>> Fixes: 4c5feef6a62c ("media: platform: amd: Add isp4 fw and hw
>> interface")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
>> ---
>>   drivers/media/platform/amd/isp4/isp4_interface.c | 4 ++--
>>   drivers/media/platform/amd/isp4/isp4_interface.h | 2 +-
>>   drivers/media/platform/amd/isp4/isp4_subdev.c    | 2 +-
>>   3 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/media/platform/amd/isp4/isp4_interface.c b/
>> drivers/media/platform/amd/isp4/isp4_interface.c
>> index 8d73f66bb42c..00a817909292 100644
>> --- a/drivers/media/platform/amd/isp4/isp4_interface.c
>> +++ b/drivers/media/platform/amd/isp4/isp4_interface.c
>> @@ -375,7 +375,7 @@ static int isp4if_send_fw_cmd(struct
>> isp4_interface *ispif, u32 cmd_id,
>>                          return -ENOMEM;
>>
>>                  /* Get two references: one for the resp thread, one
>> for us */
>> -               atomic_set(&ele->refcnt, 2);
>> +               refcount_set(&ele->refcnt, 2);
>>                  init_completion(&ele->cmd_done);
>>          }
>>
>> @@ -455,7 +455,7 @@ static int isp4if_send_fw_cmd(struct
>> isp4_interface *ispif, u32 cmd_id,
>>
>>   put_ele_ref:
>>          /* Don't free the command if we didn't put the last reference */
>> -       if (ele && atomic_dec_return(&ele->refcnt))
>> +       if (ele && !refcount_dec_and_test(&ele->refcnt))
>>                  ele = NULL;
>>
>>   free_ele:
>> diff --git a/drivers/media/platform/amd/isp4/isp4_interface.h b/
>> drivers/media/platform/amd/isp4/isp4_interface.h
>> index ce3ac9b9e5cd..04db71cd54e6 100644
>> --- a/drivers/media/platform/amd/isp4/isp4_interface.h
>> +++ b/drivers/media/platform/amd/isp4/isp4_interface.h
>> @@ -68,7 +68,7 @@ struct isp4if_cmd_element {
>>          u32 seq_num;
>>          u32 cmd_id;
>>          struct completion cmd_done;
>> -       atomic_t refcnt;
>> +       refcount_t refcnt;
>>   };
>>
>>   struct isp4_interface {
>> diff --git a/drivers/media/platform/amd/isp4/isp4_subdev.c b/drivers/
>> media/platform/amd/isp4/isp4_subdev.c
>> index 48deea79ce6c..2a8bc1207843 100644
>> --- a/drivers/media/platform/amd/isp4/isp4_subdev.c
>> +++ b/drivers/media/platform/amd/isp4/isp4_subdev.c
>> @@ -391,7 +391,7 @@ static void isp4sd_fw_resp_cmd_done(struct
>> isp4_subdev *isp_subdev,
>>
>>          if (ele) {
>>                  complete(&ele->cmd_done);
>> -               if (atomic_dec_and_test(&ele->refcnt))
>> +               if (refcount_dec_and_test(&ele->refcnt))
>>                          kfree(ele);
>>          }
>>   }
>>
>> -- 
>> 2.55.0.rc0.799.gd6f94ed593-goog
>>
> 
> Reviewed-by: Pratap Nirujogi <pratap.nirujogi@amd.com>
> 
> Thanks,
> Pratap
> 
> 

Reviewed-by: Bin Du <bin.du@amd.com>

Regards,
Bin

[PATCH 6/9] media: dvb-frontends/helene: Rename priv variable

[Ricardo Ribalda]

Coccinelle triggers a false positive where it thinks that the priv
variable in helene_attach_s and helene_attach is the same variable as
helene_probe. This is due to a bad heuristic in cocci.

We have reported it to cocci, but until/if this is fixed, renaming a
local variable is a good compromise to fix this warning:

./dvb-frontends/helene.c:1049:2-7: WARNING: invalid free of devm_ allocated data
./dvb-frontends/helene.c:1013:2-7: WARNING: invalid free of devm_ allocated data

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/dvb-frontends/helene.c | 56 ++++++++++++++++++------------------
 1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/drivers/media/dvb-frontends/helene.c b/drivers/media/dvb-frontends/helene.c
index 993280fefc2c..5fbb466cc8af 100644
--- a/drivers/media/dvb-frontends/helene.c
+++ b/drivers/media/dvb-frontends/helene.c
@@ -995,22 +995,22 @@ struct dvb_frontend *helene_attach_s(struct dvb_frontend *fe,
 		const struct helene_config *config,
 		struct i2c_adapter *i2c)
 {
-	struct helene_priv *priv = NULL;
+	struct helene_priv *pr = NULL;
 
-	priv = kzalloc_obj(struct helene_priv);
-	if (priv == NULL)
+	pr = kzalloc_obj(struct helene_priv);
+	if (!pr)
 		return NULL;
-	priv->i2c_address = (config->i2c_address >> 1);
-	priv->i2c = i2c;
-	priv->set_tuner_data = config->set_tuner_priv;
-	priv->set_tuner = config->set_tuner_callback;
-	priv->xtal = config->xtal;
+	pr->i2c_address = (config->i2c_address >> 1);
+	pr->i2c = i2c;
+	pr->set_tuner_data = config->set_tuner_priv;
+	pr->set_tuner = config->set_tuner_callback;
+	pr->xtal = config->xtal;
 
 	if (fe->ops.i2c_gate_ctrl)
 		fe->ops.i2c_gate_ctrl(fe, 1);
 
-	if (helene_x_pon(priv) != 0) {
-		kfree(priv);
+	if (helene_x_pon(pr) != 0) {
+		kfree(pr);
 		return NULL;
 	}
 
@@ -1019,10 +1019,10 @@ struct dvb_frontend *helene_attach_s(struct dvb_frontend *fe,
 
 	memcpy(&fe->ops.tuner_ops, &helene_tuner_ops_s,
 			sizeof(struct dvb_tuner_ops));
-	fe->tuner_priv = priv;
-	dev_info(&priv->i2c->dev,
-			"Sony HELENE Sat attached on addr=%x at I2C adapter %p\n",
-			priv->i2c_address, priv->i2c);
+	fe->tuner_priv = pr;
+	dev_info(&pr->i2c->dev,
+		 "Sony HELENE Sat attached on addr=%x at I2C adapter %p\n",
+		 pr->i2c_address, pr->i2c);
 	return fe;
 }
 EXPORT_SYMBOL_GPL(helene_attach_s);
@@ -1031,22 +1031,22 @@ struct dvb_frontend *helene_attach(struct dvb_frontend *fe,
 		const struct helene_config *config,
 		struct i2c_adapter *i2c)
 {
-	struct helene_priv *priv = NULL;
+	struct helene_priv *pr = NULL;
 
-	priv = kzalloc_obj(struct helene_priv);
-	if (priv == NULL)
+	pr = kzalloc_obj(struct helene_priv);
+	if (!pr)
 		return NULL;
-	priv->i2c_address = (config->i2c_address >> 1);
-	priv->i2c = i2c;
-	priv->set_tuner_data = config->set_tuner_priv;
-	priv->set_tuner = config->set_tuner_callback;
-	priv->xtal = config->xtal;
+	pr->i2c_address = (config->i2c_address >> 1);
+	pr->i2c = i2c;
+	pr->set_tuner_data = config->set_tuner_priv;
+	pr->set_tuner = config->set_tuner_callback;
+	pr->xtal = config->xtal;
 
 	if (fe->ops.i2c_gate_ctrl)
 		fe->ops.i2c_gate_ctrl(fe, 1);
 
-	if (helene_x_pon(priv) != 0) {
-		kfree(priv);
+	if (helene_x_pon(pr) != 0) {
+		kfree(pr);
 		return NULL;
 	}
 
@@ -1055,10 +1055,10 @@ struct dvb_frontend *helene_attach(struct dvb_frontend *fe,
 
 	memcpy(&fe->ops.tuner_ops, &helene_tuner_ops_t,
 			sizeof(struct dvb_tuner_ops));
-	fe->tuner_priv = priv;
-	dev_info(&priv->i2c->dev,
-			"Sony HELENE Ter attached on addr=%x at I2C adapter %p\n",
-			priv->i2c_address, priv->i2c);
+	fe->tuner_priv = pr;
+	dev_info(&pr->i2c->dev,
+		 "Sony HELENE Ter attached on addr=%x at I2C adapter %p\n",
+		 pr->i2c_address, pr->i2c);
 	return fe;
 }
 EXPORT_SYMBOL_GPL(helene_attach);

-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH 7/9] media: drivers/media/dvb-core: Split dvb_frontend_open()

[Ricardo Ribalda]

Move the actual opening to its own function.

Not intended code change. This is a preparation for the next patch.

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/dvb-core/dvb_frontend.c | 148 ++++++++++++++++++----------------
 1 file changed, 80 insertions(+), 68 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
index d082b6c57c76..d99b0348df54 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -2760,77 +2760,25 @@ static __poll_t dvb_frontend_poll(struct file *file, struct poll_table_struct *w
 	return 0;
 }
 
-static int dvb_frontend_open(struct inode *inode, struct file *file)
+static int __dvb_frontend_open(struct inode *inode, struct file *file)
 {
 	struct dvb_device *dvbdev = file->private_data;
 	struct dvb_frontend *fe = dvbdev->priv;
 	struct dvb_frontend_private *fepriv = fe->frontend_priv;
-	struct dvb_adapter *adapter = fe->dvb;
 	int ret;
 
-	dev_dbg(fe->dvb->device, "%s:\n", __func__);
-	if (fe->exit == DVB_FE_DEVICE_REMOVED)
-		return -ENODEV;
-
-	if (adapter->mfe_shared == 2) {
-		mutex_lock(&adapter->mfe_lock);
-		if ((file->f_flags & O_ACCMODE) != O_RDONLY) {
-			if (adapter->mfe_dvbdev &&
-			    !adapter->mfe_dvbdev->writers) {
-				mutex_unlock(&adapter->mfe_lock);
-				return -EBUSY;
-			}
-			adapter->mfe_dvbdev = dvbdev;
-		}
-	} else if (adapter->mfe_shared) {
-		mutex_lock(&adapter->mfe_lock);
-
-		if (!adapter->mfe_dvbdev)
-			adapter->mfe_dvbdev = dvbdev;
-
-		else if (adapter->mfe_dvbdev != dvbdev) {
-			struct dvb_device
-				*mfedev = adapter->mfe_dvbdev;
-			struct dvb_frontend
-				*mfe = mfedev->priv;
-			struct dvb_frontend_private
-				*mfepriv = mfe->frontend_priv;
-			int mferetry = (dvb_mfe_wait_time << 1);
-
-			mutex_unlock(&adapter->mfe_lock);
-			while (mferetry-- && (mfedev->users != -1 ||
-					      mfepriv->thread)) {
-				if (msleep_interruptible(500)) {
-					if (signal_pending(current))
-						return -EINTR;
-				}
-			}
-
-			mutex_lock(&adapter->mfe_lock);
-			if (adapter->mfe_dvbdev != dvbdev) {
-				mfedev = adapter->mfe_dvbdev;
-				mfe = mfedev->priv;
-				mfepriv = mfe->frontend_priv;
-				if (mfedev->users != -1 ||
-				    mfepriv->thread) {
-					mutex_unlock(&adapter->mfe_lock);
-					return -EBUSY;
-				}
-				adapter->mfe_dvbdev = dvbdev;
-			}
-		}
-	}
-
 	if (dvbdev->users == -1 && fe->ops.ts_bus_ctrl) {
 		if ((ret = fe->ops.ts_bus_ctrl(fe, 1)) < 0)
-			goto err0;
-
-		/* If we took control of the bus, we need to force
-		   reinitialization.  This is because many ts_bus_ctrl()
-		   functions strobe the RESET pin on the demod, and if the
-		   frontend thread already exists then the dvb_init() routine
-		   won't get called (which is what usually does initial
-		   register configuration). */
+			return ret;
+
+		/*
+		 * If we took control of the bus, we need to force
+		 * reinitialization.  This is because many ts_bus_ctrl()
+		 * functions strobe the RESET pin on the demod, and if the
+		 * frontend thread already exists then the dvb_init() routine
+		 * won't get called (which is what usually does initial
+		 * register configuration).
+		 */
 		fepriv->reinitialise = 1;
 	}
 
@@ -2871,8 +2819,6 @@ static int dvb_frontend_open(struct inode *inode, struct file *file)
 
 	dvb_frontend_get(fe);
 
-	if (adapter->mfe_shared)
-		mutex_unlock(&adapter->mfe_lock);
 	return ret;
 
 err3:
@@ -2891,9 +2837,75 @@ static int dvb_frontend_open(struct inode *inode, struct file *file)
 err1:
 	if (dvbdev->users == -1 && fe->ops.ts_bus_ctrl)
 		fe->ops.ts_bus_ctrl(fe, 0);
-err0:
-	if (adapter->mfe_shared)
-		mutex_unlock(&adapter->mfe_lock);
+
+	return ret;
+}
+
+static int dvb_frontend_open(struct inode *inode, struct file *file)
+{
+	struct dvb_device *dvbdev = file->private_data;
+	struct dvb_frontend *fe = dvbdev->priv;
+	struct dvb_adapter *adapter = fe->dvb;
+	int ret;
+
+	dev_dbg(fe->dvb->device, "%s:\n", __func__);
+	if (fe->exit == DVB_FE_DEVICE_REMOVED)
+		return -ENODEV;
+
+	if (!adapter->mfe_shared)
+		return __dvb_frontend_open(inode, file);
+
+	if (adapter->mfe_shared == 2) {
+		mutex_lock(&adapter->mfe_lock);
+		if ((file->f_flags & O_ACCMODE) != O_RDONLY) {
+			if (adapter->mfe_dvbdev &&
+			    !adapter->mfe_dvbdev->writers) {
+				mutex_unlock(&adapter->mfe_lock);
+				return -EBUSY;
+			}
+			adapter->mfe_dvbdev = dvbdev;
+		}
+	} else {
+		mutex_lock(&adapter->mfe_lock);
+
+		if (!adapter->mfe_dvbdev) {
+			adapter->mfe_dvbdev = dvbdev;
+		} else if (adapter->mfe_dvbdev != dvbdev) {
+			struct dvb_device
+				*mfedev = adapter->mfe_dvbdev;
+			struct dvb_frontend
+				*mfe = mfedev->priv;
+			struct dvb_frontend_private
+				*mfepriv = mfe->frontend_priv;
+			int mferetry = (dvb_mfe_wait_time << 1);
+
+			mutex_unlock(&adapter->mfe_lock);
+			while (mferetry-- && (mfedev->users != -1 ||
+					      mfepriv->thread)) {
+				if (msleep_interruptible(500)) {
+					if (signal_pending(current))
+						return -EINTR;
+				}
+			}
+
+			mutex_lock(&adapter->mfe_lock);
+			if (adapter->mfe_dvbdev != dvbdev) {
+				mfedev = adapter->mfe_dvbdev;
+				mfe = mfedev->priv;
+				mfepriv = mfe->frontend_priv;
+				if (mfedev->users != -1 ||
+				    mfepriv->thread) {
+					mutex_unlock(&adapter->mfe_lock);
+					return -EBUSY;
+				}
+				adapter->mfe_dvbdev = dvbdev;
+			}
+		}
+	}
+
+	ret = __dvb_frontend_open(inode, file);
+	mutex_unlock(&adapter->mfe_lock);
+
 	return ret;
 }
 

-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH 8/9] media: drivers/media/dvb-core: Refactor dvb_frontend_open locking

[Ricardo Ribalda]

Split out the wait function, and introduce some new toys: guard and
lockdep.

This fixes the following cocci warnings:
drivers/media/dvb-core/dvb_frontend.c:2897:1-7: preceding lock on line 2776
drivers/media/dvb-core/dvb_frontend.c:2897:1-7: preceding lock on line 2786
drivers/media/dvb-core/dvb_frontend.c:2897:1-7: preceding lock on line 2809

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/dvb-core/dvb_frontend.c | 58 ++++++++++++++++++++++-------------
 1 file changed, 37 insertions(+), 21 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
index d99b0348df54..791834e088b6 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -30,6 +30,7 @@
 #include <linux/kthread.h>
 #include <linux/ktime.h>
 #include <linux/compat.h>
+#include <linux/lockdep.h>
 #include <asm/processor.h>
 
 #include <media/dvb_frontend.h>
@@ -2841,6 +2842,34 @@ static int __dvb_frontend_open(struct inode *inode, struct file *file)
 	return ret;
 }
 
+static int wait_dvb_frontend(struct dvb_adapter *adapter,
+			     struct dvb_device *mfedev)
+{
+	struct dvb_frontend *mfe = mfedev->priv;
+	struct dvb_frontend_private *mfepriv = mfe->frontend_priv;
+	int mferetry = (dvb_mfe_wait_time << 1);
+	int ret = 0;
+
+	lockdep_assert_held(&adapter->mfe_lock);
+
+	if (mfedev->users == -1 && !mfepriv->thread)
+		return 0;
+
+	mutex_unlock(&adapter->mfe_lock);
+
+	while (mferetry-- && (mfedev->users != -1 || mfepriv->thread)) {
+		if (msleep_interruptible(500))
+			if (signal_pending(current)) {
+				ret = -EINTR;
+				break;
+			}
+	}
+
+	mutex_lock(&adapter->mfe_lock);
+
+	return ret;
+}
+
 static int dvb_frontend_open(struct inode *inode, struct file *file)
 {
 	struct dvb_device *dvbdev = file->private_data;
@@ -2855,19 +2884,16 @@ static int dvb_frontend_open(struct inode *inode, struct file *file)
 	if (!adapter->mfe_shared)
 		return __dvb_frontend_open(inode, file);
 
+	guard(mutex)(&adapter->mfe_lock);
+
 	if (adapter->mfe_shared == 2) {
-		mutex_lock(&adapter->mfe_lock);
 		if ((file->f_flags & O_ACCMODE) != O_RDONLY) {
 			if (adapter->mfe_dvbdev &&
-			    !adapter->mfe_dvbdev->writers) {
-				mutex_unlock(&adapter->mfe_lock);
+			    !adapter->mfe_dvbdev->writers)
 				return -EBUSY;
-			}
 			adapter->mfe_dvbdev = dvbdev;
 		}
 	} else {
-		mutex_lock(&adapter->mfe_lock);
-
 		if (!adapter->mfe_dvbdev) {
 			adapter->mfe_dvbdev = dvbdev;
 		} else if (adapter->mfe_dvbdev != dvbdev) {
@@ -2877,34 +2903,24 @@ static int dvb_frontend_open(struct inode *inode, struct file *file)
 				*mfe = mfedev->priv;
 			struct dvb_frontend_private
 				*mfepriv = mfe->frontend_priv;
-			int mferetry = (dvb_mfe_wait_time << 1);
-
-			mutex_unlock(&adapter->mfe_lock);
-			while (mferetry-- && (mfedev->users != -1 ||
-					      mfepriv->thread)) {
-				if (msleep_interruptible(500)) {
-					if (signal_pending(current))
-						return -EINTR;
-				}
-			}
 
-			mutex_lock(&adapter->mfe_lock);
+			ret = wait_dvb_frontend(adapter, mfedev);
+			if (ret)
+				return ret;
+
 			if (adapter->mfe_dvbdev != dvbdev) {
 				mfedev = adapter->mfe_dvbdev;
 				mfe = mfedev->priv;
 				mfepriv = mfe->frontend_priv;
 				if (mfedev->users != -1 ||
-				    mfepriv->thread) {
-					mutex_unlock(&adapter->mfe_lock);
+				    mfepriv->thread)
 					return -EBUSY;
-				}
 				adapter->mfe_dvbdev = dvbdev;
 			}
 		}
 	}
 
 	ret = __dvb_frontend_open(inode, file);
-	mutex_unlock(&adapter->mfe_lock);
 
 	return ret;
 }

-- 
2.55.0.rc0.799.gd6f94ed593-goog

[PATCH 9/9] media: drivers/media/dvb-core: CodeStyle for dvb_frontend_open()

[Ricardo Ribalda]

We can rearrange a bit the function to reduce the indentation levels.

No functional change added to this patch.

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/dvb-core/dvb_frontend.c | 49 ++++++++++++++++-------------------
 1 file changed, 22 insertions(+), 27 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
index 791834e088b6..0286da57f382 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -2875,7 +2875,6 @@ static int dvb_frontend_open(struct inode *inode, struct file *file)
 	struct dvb_device *dvbdev = file->private_data;
 	struct dvb_frontend *fe = dvbdev->priv;
 	struct dvb_adapter *adapter = fe->dvb;
-	int ret;
 
 	dev_dbg(fe->dvb->device, "%s:\n", __func__);
 	if (fe->exit == DVB_FE_DEVICE_REMOVED)
@@ -2893,36 +2892,32 @@ static int dvb_frontend_open(struct inode *inode, struct file *file)
 				return -EBUSY;
 			adapter->mfe_dvbdev = dvbdev;
 		}
-	} else {
-		if (!adapter->mfe_dvbdev) {
+		return __dvb_frontend_open(inode, file);
+	}
+
+	if (!adapter->mfe_dvbdev) {
+		adapter->mfe_dvbdev = dvbdev;
+	} else if (adapter->mfe_dvbdev != dvbdev) {
+		struct dvb_device *mfedev = adapter->mfe_dvbdev;
+		struct dvb_frontend *mfe = mfedev->priv;
+		struct dvb_frontend_private *mfepriv = mfe->frontend_priv;
+		int ret;
+
+		ret = wait_dvb_frontend(adapter, mfedev);
+		if (ret)
+			return ret;
+
+		if (adapter->mfe_dvbdev != dvbdev) {
+			mfedev = adapter->mfe_dvbdev;
+			mfe = mfedev->priv;
+			mfepriv = mfe->frontend_priv;
+			if (mfedev->users != -1 || mfepriv->thread)
+				return -EBUSY;
 			adapter->mfe_dvbdev = dvbdev;
-		} else if (adapter->mfe_dvbdev != dvbdev) {
-			struct dvb_device
-				*mfedev = adapter->mfe_dvbdev;
-			struct dvb_frontend
-				*mfe = mfedev->priv;
-			struct dvb_frontend_private
-				*mfepriv = mfe->frontend_priv;
-
-			ret = wait_dvb_frontend(adapter, mfedev);
-			if (ret)
-				return ret;
-
-			if (adapter->mfe_dvbdev != dvbdev) {
-				mfedev = adapter->mfe_dvbdev;
-				mfe = mfedev->priv;
-				mfepriv = mfe->frontend_priv;
-				if (mfedev->users != -1 ||
-				    mfepriv->thread)
-					return -EBUSY;
-				adapter->mfe_dvbdev = dvbdev;
-			}
 		}
 	}
 
-	ret = __dvb_frontend_open(inode, file);
-
-	return ret;
+	return __dvb_frontend_open(inode, file);
 }
 
 static int dvb_frontend_release(struct inode *inode, struct file *file)

-- 
2.55.0.rc0.799.gd6f94ed593-goog