mirror of https://lore.kernel.org/linux-amlogic/
 help / color / mirror / Atom feed
* [PATCH 0/6] media: Fix last smatch warnings
@ 2024-08-13 12:13 Ricardo Ribalda
  2024-08-13 12:13 ` [PATCH 1/6] media: ar0521: Refactor ar0521_power_off() Ricardo Ribalda
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Ricardo Ribalda @ 2024-08-13 12:13 UTC (permalink / raw)
  To: Sakari Ailus, Krzysztof Hałasa, Mauro Carvalho Chehab,
	Sylwester Nawrocki, Andrzej Hajda, Neil Armstrong,
	Greg Kroah-Hartman, Kevin Hilman, Jerome Brunet,
	Martin Blumenstingl
  Cc: Hans Verkuil, linux-media, linux-kernel, linux-amlogic,
	linux-staging, linux-arm-kernel, Ricardo Ribalda

This series completes the smatch warning cleanout.

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
Ricardo Ribalda (6):
      media: ar0521: Refactor ar0521_power_off()
      media: i2c: ov5645: Refactor ov5645_set_power_off()
      media: i2c: s5c73m3: Move clk_prepare to its own function
      media: tc358746: Move clk_prepare to its own function
      media: meson: vdec_1: Refactor vdec_1_stop()
      media: meson: vdec: hevc: Refactor vdec_hevc_start and vdec_hevc_stop

 drivers/media/i2c/ar0521.c                   | 15 ++++++++--
 drivers/media/i2c/ov5645.c                   | 15 ++++++++--
 drivers/media/i2c/s5c73m3/s5c73m3-core.c     | 13 ++++++++-
 drivers/media/i2c/tc358746.c                 | 12 +++++++-
 drivers/staging/media/meson/vdec/vdec_1.c    | 16 ++++++++---
 drivers/staging/media/meson/vdec/vdec_hevc.c | 43 +++++++++++++++++++++-------
 6 files changed, 92 insertions(+), 22 deletions(-)
---
base-commit: c80bfa4f9e0ebfce6ac909488d412347acbcb4f9
change-id: 20240813-smatch-clock-2fa3b1174857

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


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/6] media: ar0521: Refactor ar0521_power_off()
  2024-08-13 12:13 [PATCH 0/6] media: Fix last smatch warnings Ricardo Ribalda
@ 2024-08-13 12:13 ` Ricardo Ribalda
  2024-08-13 12:54   ` Dan Carpenter
  2024-08-13 12:13 ` [PATCH 2/6] media: i2c: ov5645: Refactor ov5645_set_power_off() Ricardo Ribalda
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 10+ messages in thread
From: Ricardo Ribalda @ 2024-08-13 12:13 UTC (permalink / raw)
  To: Sakari Ailus, Krzysztof Hałasa, Mauro Carvalho Chehab,
	Sylwester Nawrocki, Andrzej Hajda, Neil Armstrong,
	Greg Kroah-Hartman, Kevin Hilman, Jerome Brunet,
	Martin Blumenstingl
  Cc: Hans Verkuil, linux-media, linux-kernel, linux-amlogic,
	linux-staging, linux-arm-kernel, Ricardo Ribalda

Factor out all the power off logic, except the clk_disable_unprepare(),
to a new function __ar0521_power_off().

This allows ar0521_power_on() to explicitly clean-out the clock during
the error-path.

The following smatch warning is fixed:
drivers/media/i2c/ar0521.c:912 ar0521_power_on() warn: 'sensor->extclk' from clk_prepare_enable() not released on lines: 912.

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/i2c/ar0521.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/ar0521.c b/drivers/media/i2c/ar0521.c
index 09331cf95c62..2c528db31ba6 100644
--- a/drivers/media/i2c/ar0521.c
+++ b/drivers/media/i2c/ar0521.c
@@ -835,7 +835,7 @@ static const struct initial_reg {
 	     be(0x0707)), /* 3F44: couple k factor 2 */
 };
 
-static int ar0521_power_off(struct device *dev)
+static void __ar0521_power_off(struct device *dev)
 {
 	struct v4l2_subdev *sd = dev_get_drvdata(dev);
 	struct ar0521_dev *sensor = to_ar0521_dev(sd);
@@ -850,6 +850,16 @@ static int ar0521_power_off(struct device *dev)
 		if (sensor->supplies[i])
 			regulator_disable(sensor->supplies[i]);
 	}
+}
+
+static int ar0521_power_off(struct device *dev)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct ar0521_dev *sensor = to_ar0521_dev(sd);
+
+	clk_disable_unprepare(sensor->extclk);
+	__ar0521_power_off(dev);
+
 	return 0;
 }
 
@@ -908,7 +918,8 @@ static int ar0521_power_on(struct device *dev)
 
 	return 0;
 off:
-	ar0521_power_off(dev);
+	clk_disable_unprepare(sensor->extclk);
+	__ar0521_power_off(dev);
 	return ret;
 }
 

-- 
2.46.0.76.ge559c4bf1a-goog


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/6] media: i2c: ov5645: Refactor ov5645_set_power_off()
  2024-08-13 12:13 [PATCH 0/6] media: Fix last smatch warnings Ricardo Ribalda
  2024-08-13 12:13 ` [PATCH 1/6] media: ar0521: Refactor ar0521_power_off() Ricardo Ribalda
@ 2024-08-13 12:13 ` Ricardo Ribalda
  2024-08-13 12:13 ` [PATCH 3/6] media: i2c: s5c73m3: Move clk_prepare to its own function Ricardo Ribalda
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Ricardo Ribalda @ 2024-08-13 12:13 UTC (permalink / raw)
  To: Sakari Ailus, Krzysztof Hałasa, Mauro Carvalho Chehab,
	Sylwester Nawrocki, Andrzej Hajda, Neil Armstrong,
	Greg Kroah-Hartman, Kevin Hilman, Jerome Brunet,
	Martin Blumenstingl
  Cc: Hans Verkuil, linux-media, linux-kernel, linux-amlogic,
	linux-staging, linux-arm-kernel, Ricardo Ribalda

Factor out all the power off logic, except clk_disable_unprepare(), to a
new function __ov5645_set_power_off().

This allows ov5645_set_power_on() to excplicitly clean-out the clock
during the error-path.

The following smatch warning is fixed:
drivers/media/i2c/ov5645.c:690 ov5645_set_power_on() warn: 'ov5645->xclk' from clk_prepare_enable() not released on lines: 690.

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/i2c/ov5645.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/ov5645.c b/drivers/media/i2c/ov5645.c
index 6c2d221f6973..0c32bd2940ec 100644
--- a/drivers/media/i2c/ov5645.c
+++ b/drivers/media/i2c/ov5645.c
@@ -635,7 +635,7 @@ static int ov5645_set_register_array(struct ov5645 *ov5645,
 	return 0;
 }
 
-static int ov5645_set_power_off(struct device *dev)
+static void __ov5645_set_power_off(struct device *dev)
 {
 	struct v4l2_subdev *sd = dev_get_drvdata(dev);
 	struct ov5645 *ov5645 = to_ov5645(sd);
@@ -643,8 +643,16 @@ static int ov5645_set_power_off(struct device *dev)
 	ov5645_write_reg(ov5645, OV5645_IO_MIPI_CTRL00, 0x58);
 	gpiod_set_value_cansleep(ov5645->rst_gpio, 1);
 	gpiod_set_value_cansleep(ov5645->enable_gpio, 0);
-	clk_disable_unprepare(ov5645->xclk);
 	regulator_bulk_disable(OV5645_NUM_SUPPLIES, ov5645->supplies);
+}
+
+static int ov5645_set_power_off(struct device *dev)
+{
+	struct v4l2_subdev *sd = dev_get_drvdata(dev);
+	struct ov5645 *ov5645 = to_ov5645(sd);
+
+	__ov5645_set_power_off(dev);
+	clk_disable_unprepare(ov5645->xclk);
 
 	return 0;
 }
@@ -686,7 +694,8 @@ static int ov5645_set_power_on(struct device *dev)
 	return 0;
 
 exit:
-	ov5645_set_power_off(dev);
+	__ov5645_set_power_off(dev);
+	clk_disable_unprepare(ov5645->xclk);
 	return ret;
 }
 

-- 
2.46.0.76.ge559c4bf1a-goog


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 3/6] media: i2c: s5c73m3: Move clk_prepare to its own function
  2024-08-13 12:13 [PATCH 0/6] media: Fix last smatch warnings Ricardo Ribalda
  2024-08-13 12:13 ` [PATCH 1/6] media: ar0521: Refactor ar0521_power_off() Ricardo Ribalda
  2024-08-13 12:13 ` [PATCH 2/6] media: i2c: ov5645: Refactor ov5645_set_power_off() Ricardo Ribalda
@ 2024-08-13 12:13 ` Ricardo Ribalda
  2024-08-13 12:56   ` Dan Carpenter
  2024-08-13 12:13 ` [PATCH 4/6] media: tc358746: " Ricardo Ribalda
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 10+ messages in thread
From: Ricardo Ribalda @ 2024-08-13 12:13 UTC (permalink / raw)
  To: Sakari Ailus, Krzysztof Hałasa, Mauro Carvalho Chehab,
	Sylwester Nawrocki, Andrzej Hajda, Neil Armstrong,
	Greg Kroah-Hartman, Kevin Hilman, Jerome Brunet,
	Martin Blumenstingl
  Cc: Hans Verkuil, linux-media, linux-kernel, linux-amlogic,
	linux-staging, linux-arm-kernel, Ricardo Ribalda

Smatch is very confused by a clk_prepare_enable() being called in an
error-path. Fix this warning by moving the clk_prepare_enable() to its
own function.

drivers/media/i2c/s5c73m3/s5c73m3-core.c:1425 __s5c73m3_power_off() warn: 'state->clock' from clk_prepare_enable() not released on lines: 1425.

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/i2c/s5c73m3/s5c73m3-core.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/s5c73m3/s5c73m3-core.c b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
index e89e888f028e..7716dfe2b8c9 100644
--- a/drivers/media/i2c/s5c73m3/s5c73m3-core.c
+++ b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
@@ -1392,6 +1392,16 @@ static int __s5c73m3_power_on(struct s5c73m3 *state)
 	return ret;
 }
 
+/*
+ * This function has been created just to avoid a smatch warning,
+ * please do not merge into __s5c73m3_power_off() until you have
+ * confirmed that it does not introduce a new warning.
+ */
+static void s5c73m3_enable_clk(struct s5c73m3 *state)
+{
+	clk_prepare_enable(state->clock);
+}
+
 static int __s5c73m3_power_off(struct s5c73m3 *state)
 {
 	int i, ret;
@@ -1421,7 +1431,8 @@ static int __s5c73m3_power_off(struct s5c73m3 *state)
 				 state->supplies[i].supply, r);
 	}
 
-	clk_prepare_enable(state->clock);
+	s5c73m3_enable_clk(state);
+
 	return ret;
 }
 

-- 
2.46.0.76.ge559c4bf1a-goog


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 4/6] media: tc358746: Move clk_prepare to its own function
  2024-08-13 12:13 [PATCH 0/6] media: Fix last smatch warnings Ricardo Ribalda
                   ` (2 preceding siblings ...)
  2024-08-13 12:13 ` [PATCH 3/6] media: i2c: s5c73m3: Move clk_prepare to its own function Ricardo Ribalda
@ 2024-08-13 12:13 ` Ricardo Ribalda
  2024-08-13 12:13 ` [PATCH 5/6] media: meson: vdec_1: Refactor vdec_1_stop() Ricardo Ribalda
  2024-08-13 12:13 ` [PATCH 6/6] media: meson: vdec: hevc: Refactor vdec_hevc_start and vdec_hevc_stop Ricardo Ribalda
  5 siblings, 0 replies; 10+ messages in thread
From: Ricardo Ribalda @ 2024-08-13 12:13 UTC (permalink / raw)
  To: Sakari Ailus, Krzysztof Hałasa, Mauro Carvalho Chehab,
	Sylwester Nawrocki, Andrzej Hajda, Neil Armstrong,
	Greg Kroah-Hartman, Kevin Hilman, Jerome Brunet,
	Martin Blumenstingl
  Cc: Hans Verkuil, linux-media, linux-kernel, linux-amlogic,
	linux-staging, linux-arm-kernel, Ricardo Ribalda

Smatch is very confused by a clk_prepare_enable() being called in an
error-path. Fix this warning by moving the clk_prepare_enable() to its
own function.

drivers/media/i2c/tc358746.c:1631 tc358746_suspend() warn: 'tc358746->refclk' from clk_prepare_enable() not released on lines: 1631.

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

diff --git a/drivers/media/i2c/tc358746.c b/drivers/media/i2c/tc358746.c
index edf79107adc5..389582420ba7 100644
--- a/drivers/media/i2c/tc358746.c
+++ b/drivers/media/i2c/tc358746.c
@@ -1616,6 +1616,16 @@ static void tc358746_remove(struct i2c_client *client)
 	pm_runtime_dont_use_autosuspend(sd->dev);
 }
 
+/*
+ * This function has been created just to avoid a smatch warning,
+ * please do not merge it into tc358746_suspend until you have
+ * confirmed that it does not introduce a new warning.
+ */
+static void tc358746_clk_enable(struct tc358746 *tc358746)
+{
+	clk_prepare_enable(tc358746->refclk);
+}
+
 static int tc358746_suspend(struct device *dev)
 {
 	struct tc358746 *tc358746 = dev_get_drvdata(dev);
@@ -1626,7 +1636,7 @@ static int tc358746_suspend(struct device *dev)
 	err = regulator_bulk_disable(ARRAY_SIZE(tc358746_supplies),
 				     tc358746->supplies);
 	if (err)
-		clk_prepare_enable(tc358746->refclk);
+		tc358746_clk_enable(tc358746);
 
 	return err;
 }

-- 
2.46.0.76.ge559c4bf1a-goog


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 5/6] media: meson: vdec_1: Refactor vdec_1_stop()
  2024-08-13 12:13 [PATCH 0/6] media: Fix last smatch warnings Ricardo Ribalda
                   ` (3 preceding siblings ...)
  2024-08-13 12:13 ` [PATCH 4/6] media: tc358746: " Ricardo Ribalda
@ 2024-08-13 12:13 ` Ricardo Ribalda
  2024-08-13 12:13 ` [PATCH 6/6] media: meson: vdec: hevc: Refactor vdec_hevc_start and vdec_hevc_stop Ricardo Ribalda
  5 siblings, 0 replies; 10+ messages in thread
From: Ricardo Ribalda @ 2024-08-13 12:13 UTC (permalink / raw)
  To: Sakari Ailus, Krzysztof Hałasa, Mauro Carvalho Chehab,
	Sylwester Nawrocki, Andrzej Hajda, Neil Armstrong,
	Greg Kroah-Hartman, Kevin Hilman, Jerome Brunet,
	Martin Blumenstingl
  Cc: Hans Verkuil, linux-media, linux-kernel, linux-amlogic,
	linux-staging, linux-arm-kernel, Ricardo Ribalda

Factor out all the power off logic, except the clk_disable_unprepare(),
to a new function __vdec_1_stop().

This allows vdec_1_start() to explicitly clean-out the clock during the
error-path.

The following smatch warning is fixed:
drivers/staging/media/meson/vdec/vdec_1.c:239 vdec_1_start() warn: 'core->vdec_1_clk' from clk_prepare_enable() not released on lines: 239.

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/staging/media/meson/vdec/vdec_1.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/meson/vdec/vdec_1.c b/drivers/staging/media/meson/vdec/vdec_1.c
index 3fe2de0c9331..a65cb4959446 100644
--- a/drivers/staging/media/meson/vdec/vdec_1.c
+++ b/drivers/staging/media/meson/vdec/vdec_1.c
@@ -129,7 +129,7 @@ static u32 vdec_1_vififo_level(struct amvdec_session *sess)
 	return amvdec_read_dos(core, VLD_MEM_VIFIFO_LEVEL);
 }
 
-static int vdec_1_stop(struct amvdec_session *sess)
+static void __vdec_1_stop(struct amvdec_session *sess)
 {
 	struct amvdec_core *core = sess->core;
 	struct amvdec_codec_ops *codec_ops = sess->fmt_out->codec_ops;
@@ -158,10 +158,17 @@ static int vdec_1_stop(struct amvdec_session *sess)
 		regmap_update_bits(core->regmap_ao, AO_RTI_GEN_PWR_SLEEP0,
 				   GEN_PWR_VDEC_1, GEN_PWR_VDEC_1);
 
-	clk_disable_unprepare(core->vdec_1_clk);
-
 	if (sess->priv)
 		codec_ops->stop(sess);
+}
+
+static int vdec_1_stop(struct amvdec_session *sess)
+{
+	struct amvdec_core *core = sess->core;
+
+	__vdec_1_stop(sess);
+
+	clk_disable_unprepare(core->vdec_1_clk);
 
 	return 0;
 }
@@ -235,7 +242,8 @@ static int vdec_1_start(struct amvdec_session *sess)
 	return 0;
 
 stop:
-	vdec_1_stop(sess);
+	__vdec_1_stop(sess);
+	clk_disable_unprepare(core->vdec_1_clk);
 	return ret;
 }
 

-- 
2.46.0.76.ge559c4bf1a-goog


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 6/6] media: meson: vdec: hevc: Refactor vdec_hevc_start and vdec_hevc_stop
  2024-08-13 12:13 [PATCH 0/6] media: Fix last smatch warnings Ricardo Ribalda
                   ` (4 preceding siblings ...)
  2024-08-13 12:13 ` [PATCH 5/6] media: meson: vdec_1: Refactor vdec_1_stop() Ricardo Ribalda
@ 2024-08-13 12:13 ` Ricardo Ribalda
  5 siblings, 0 replies; 10+ messages in thread
From: Ricardo Ribalda @ 2024-08-13 12:13 UTC (permalink / raw)
  To: Sakari Ailus, Krzysztof Hałasa, Mauro Carvalho Chehab,
	Sylwester Nawrocki, Andrzej Hajda, Neil Armstrong,
	Greg Kroah-Hartman, Kevin Hilman, Jerome Brunet,
	Martin Blumenstingl
  Cc: Hans Verkuil, linux-media, linux-kernel, linux-amlogic,
	linux-staging, linux-arm-kernel, Ricardo Ribalda

Make a new function __vdec_hevc_start(), that does all the
initialization, except the clock initialization for G12A and SM1.

Factor out all the stop logic, except the clk_disable_unprepare(), to a
new function __vdec_hevc_stop. This allows vdec_hevc_start() to
explicitly celan-out the clock during the error-path.

The following smatch warnings are fixed:

drivers/staging/media/meson/vdec/vdec_hevc.c:227 vdec_hevc_start() warn: 'core->vdec_hevc_clk' from clk_prepare_enable() not released on lines: 227.
drivers/staging/media/meson/vdec/vdec_hevc.c:227 vdec_hevc_start() warn: 'core->vdec_hevcf_clk' from clk_prepare_enable() not released on lines: 227.

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/staging/media/meson/vdec/vdec_hevc.c | 43 +++++++++++++++++++++-------
 1 file changed, 32 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/media/meson/vdec/vdec_hevc.c b/drivers/staging/media/meson/vdec/vdec_hevc.c
index afced435c907..1939c47def58 100644
--- a/drivers/staging/media/meson/vdec/vdec_hevc.c
+++ b/drivers/staging/media/meson/vdec/vdec_hevc.c
@@ -110,7 +110,7 @@ static u32 vdec_hevc_vififo_level(struct amvdec_session *sess)
 	return readl_relaxed(sess->core->dos_base + HEVC_STREAM_LEVEL);
 }
 
-static int vdec_hevc_stop(struct amvdec_session *sess)
+static void __vdec_hevc_stop(struct amvdec_session *sess)
 {
 	struct amvdec_core *core = sess->core;
 	struct amvdec_codec_ops *codec_ops = sess->fmt_out->codec_ops;
@@ -142,6 +142,13 @@ static int vdec_hevc_stop(struct amvdec_session *sess)
 	else
 		regmap_update_bits(core->regmap_ao, AO_RTI_GEN_PWR_SLEEP0,
 				   GEN_PWR_VDEC_HEVC, GEN_PWR_VDEC_HEVC);
+}
+
+static int vdec_hevc_stop(struct amvdec_session *sess)
+{
+	struct amvdec_core *core = sess->core;
+
+	__vdec_hevc_stop(sess);
 
 	clk_disable_unprepare(core->vdec_hevc_clk);
 	if (core->platform->revision == VDEC_REVISION_G12A ||
@@ -151,20 +158,12 @@ static int vdec_hevc_stop(struct amvdec_session *sess)
 	return 0;
 }
 
-static int vdec_hevc_start(struct amvdec_session *sess)
+static int __vdec_hevc_start(struct amvdec_session *sess)
 {
 	int ret;
 	struct amvdec_core *core = sess->core;
 	struct amvdec_codec_ops *codec_ops = sess->fmt_out->codec_ops;
 
-	if (core->platform->revision == VDEC_REVISION_G12A ||
-	    core->platform->revision == VDEC_REVISION_SM1) {
-		clk_set_rate(core->vdec_hevcf_clk, 666666666);
-		ret = clk_prepare_enable(core->vdec_hevcf_clk);
-		if (ret)
-			return ret;
-	}
-
 	clk_set_rate(core->vdec_hevc_clk, 666666666);
 	ret = clk_prepare_enable(core->vdec_hevc_clk);
 	if (ret) {
@@ -223,10 +222,32 @@ static int vdec_hevc_start(struct amvdec_session *sess)
 	return 0;
 
 stop:
-	vdec_hevc_stop(sess);
+	__vdec_hevc_stop(sess);
+	clk_disable_unprepare(core->vdec_hevc_clk);
 	return ret;
 }
 
+static int vdec_hevc_start(struct amvdec_session *sess)
+{
+	struct amvdec_core *core = sess->core;
+	int ret;
+
+	if (core->platform->revision == VDEC_REVISION_G12A ||
+	    core->platform->revision == VDEC_REVISION_SM1) {
+		clk_set_rate(core->vdec_hevcf_clk, 666666666);
+		ret = clk_prepare_enable(core->vdec_hevcf_clk);
+		if (ret)
+			return ret;
+
+		ret = __vdec_hevc_start(sess);
+		if (ret)
+			clk_disable_unprepare(core->vdec_hevcf_clk);
+		return ret;
+	}
+
+	return __vdec_hevc_start(sess);
+}
+
 struct amvdec_ops vdec_hevc_ops = {
 	.start = vdec_hevc_start,
 	.stop = vdec_hevc_stop,

-- 
2.46.0.76.ge559c4bf1a-goog


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/6] media: ar0521: Refactor ar0521_power_off()
  2024-08-13 12:13 ` [PATCH 1/6] media: ar0521: Refactor ar0521_power_off() Ricardo Ribalda
@ 2024-08-13 12:54   ` Dan Carpenter
  2024-08-13 13:01     ` Ricardo Ribalda
  0 siblings, 1 reply; 10+ messages in thread
From: Dan Carpenter @ 2024-08-13 12:54 UTC (permalink / raw)
  To: Ricardo Ribalda
  Cc: Sakari Ailus, Krzysztof Hałasa, Mauro Carvalho Chehab,
	Sylwester Nawrocki, Andrzej Hajda, Neil Armstrong,
	Greg Kroah-Hartman, Kevin Hilman, Jerome Brunet,
	Martin Blumenstingl, Hans Verkuil, linux-media, linux-kernel,
	linux-amlogic, linux-staging, linux-arm-kernel

On Tue, Aug 13, 2024 at 12:13:48PM +0000, Ricardo Ribalda wrote:
> Factor out all the power off logic, except the clk_disable_unprepare(),
> to a new function __ar0521_power_off().
> 
> This allows ar0521_power_on() to explicitly clean-out the clock during
> the error-path.
> 
> The following smatch warning is fixed:
> drivers/media/i2c/ar0521.c:912 ar0521_power_on() warn: 'sensor->extclk' from clk_prepare_enable() not released on lines: 912.
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---

It's better to just ignore false positives...  The problem here is that
Smatch can't track that to_ar0521_dev(dev_get_drvdata(dev))->sensor is the same
as sensor.  What I could do is say that "this frees *something* unknown" so
let's silence the warning."

The problem is that check_unwind.c is not very granular.  It just marks things
as allocated and freed.  I could make it more granular so the free and the
alloc have to match.  Or we could match based on the type.  This frees a
"struct ar0521_dev" so mark all those as freed in the caller.

>  drivers/media/i2c/ar0521.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/i2c/ar0521.c b/drivers/media/i2c/ar0521.c
> index 09331cf95c62..2c528db31ba6 100644
> --- a/drivers/media/i2c/ar0521.c
> +++ b/drivers/media/i2c/ar0521.c
> @@ -835,7 +835,7 @@ static const struct initial_reg {
>  	     be(0x0707)), /* 3F44: couple k factor 2 */
>  };
>  
> -static int ar0521_power_off(struct device *dev)
> +static void __ar0521_power_off(struct device *dev)
>  {
>  	struct v4l2_subdev *sd = dev_get_drvdata(dev);
>  	struct ar0521_dev *sensor = to_ar0521_dev(sd);
> @@ -850,6 +850,16 @@ static int ar0521_power_off(struct device *dev)
>  		if (sensor->supplies[i])
>  			regulator_disable(sensor->supplies[i]);
>  	}
> +}
> +
> +static int ar0521_power_off(struct device *dev)
> +{
> +	struct v4l2_subdev *sd = dev_get_drvdata(dev);
> +	struct ar0521_dev *sensor = to_ar0521_dev(sd);
> +
> +	clk_disable_unprepare(sensor->extclk);
> +	__ar0521_power_off(dev);

You had intended to remove the clk_disable_unprepare() from __ar0521_power_off()
but forgot so these are double unprepares.

regards,
dan carpenter

> +
>  	return 0;
>  }
>  
> @@ -908,7 +918,8 @@ static int ar0521_power_on(struct device *dev)
>  
>  	return 0;
>  off:
> -	ar0521_power_off(dev);
> +	clk_disable_unprepare(sensor->extclk);
> +	__ar0521_power_off(dev);
>  	return ret;
>  }
>  
> 
> -- 
> 2.46.0.76.ge559c4bf1a-goog
> 

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/6] media: i2c: s5c73m3: Move clk_prepare to its own function
  2024-08-13 12:13 ` [PATCH 3/6] media: i2c: s5c73m3: Move clk_prepare to its own function Ricardo Ribalda
@ 2024-08-13 12:56   ` Dan Carpenter
  0 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2024-08-13 12:56 UTC (permalink / raw)
  To: Ricardo Ribalda
  Cc: Sakari Ailus, Krzysztof Hałasa, Mauro Carvalho Chehab,
	Sylwester Nawrocki, Andrzej Hajda, Neil Armstrong,
	Greg Kroah-Hartman, Kevin Hilman, Jerome Brunet,
	Martin Blumenstingl, Hans Verkuil, linux-media, linux-kernel,
	linux-amlogic, linux-staging, linux-arm-kernel

On Tue, Aug 13, 2024 at 12:13:50PM +0000, Ricardo Ribalda wrote:
> Smatch is very confused by a clk_prepare_enable() being called in an
> error-path. Fix this warning by moving the clk_prepare_enable() to its
> own function.
> 
> drivers/media/i2c/s5c73m3/s5c73m3-core.c:1425 __s5c73m3_power_off() warn: 'state->clock' from clk_prepare_enable() not released on lines: 1425.
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---

To be honest, maybe I should remove the clk_prepare_enable/disable() from the
check_unwind.c.  Or at least move them into their own check so that I can
tweak this kind of thing more easily.

regards,
dan carpenter


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/6] media: ar0521: Refactor ar0521_power_off()
  2024-08-13 12:54   ` Dan Carpenter
@ 2024-08-13 13:01     ` Ricardo Ribalda
  0 siblings, 0 replies; 10+ messages in thread
From: Ricardo Ribalda @ 2024-08-13 13:01 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Sakari Ailus, Krzysztof Hałasa, Mauro Carvalho Chehab,
	Sylwester Nawrocki, Andrzej Hajda, Neil Armstrong,
	Greg Kroah-Hartman, Kevin Hilman, Jerome Brunet,
	Martin Blumenstingl, Hans Verkuil, linux-media, linux-kernel,
	linux-amlogic, linux-staging, linux-arm-kernel

Hi Dan

On Tue, 13 Aug 2024 at 14:54, Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> On Tue, Aug 13, 2024 at 12:13:48PM +0000, Ricardo Ribalda wrote:
> > Factor out all the power off logic, except the clk_disable_unprepare(),
> > to a new function __ar0521_power_off().
> >
> > This allows ar0521_power_on() to explicitly clean-out the clock during
> > the error-path.
> >
> > The following smatch warning is fixed:
> > drivers/media/i2c/ar0521.c:912 ar0521_power_on() warn: 'sensor->extclk' from clk_prepare_enable() not released on lines: 912.
> >
> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > ---
>
> It's better to just ignore false positives...  The problem here is that
> Smatch can't track that to_ar0521_dev(dev_get_drvdata(dev))->sensor is the same
> as sensor.  What I could do is say that "this frees *something* unknown" so
> let's silence the warning."
>
> The problem is that check_unwind.c is not very granular.  It just marks things
> as allocated and freed.  I could make it more granular so the free and the
> alloc have to match.  Or we could match based on the type.  This frees a
> "struct ar0521_dev" so mark all those as freed in the caller.
>
> >  drivers/media/i2c/ar0521.c | 15 +++++++++++++--
> >  1 file changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/i2c/ar0521.c b/drivers/media/i2c/ar0521.c
> > index 09331cf95c62..2c528db31ba6 100644
> > --- a/drivers/media/i2c/ar0521.c
> > +++ b/drivers/media/i2c/ar0521.c
> > @@ -835,7 +835,7 @@ static const struct initial_reg {
> >            be(0x0707)), /* 3F44: couple k factor 2 */
> >  };
> >
> > -static int ar0521_power_off(struct device *dev)
> > +static void __ar0521_power_off(struct device *dev)
> >  {
> >       struct v4l2_subdev *sd = dev_get_drvdata(dev);
> >       struct ar0521_dev *sensor = to_ar0521_dev(sd);
> > @@ -850,6 +850,16 @@ static int ar0521_power_off(struct device *dev)
> >               if (sensor->supplies[i])
> >                       regulator_disable(sensor->supplies[i]);
> >       }
> > +}
> > +
> > +static int ar0521_power_off(struct device *dev)
> > +{
> > +     struct v4l2_subdev *sd = dev_get_drvdata(dev);
> > +     struct ar0521_dev *sensor = to_ar0521_dev(sd);
> > +
> > +     clk_disable_unprepare(sensor->extclk);
> > +     __ar0521_power_off(dev);
>
> You had intended to remove the clk_disable_unprepare() from __ar0521_power_off()
> but forgot so these are double unprepares.

Ups, thanks for catching it :)

Will send as v2, unless you fix it in smatch first ;P

Regards!

>
> regards,
> dan carpenter
>
> > +
> >       return 0;
> >  }
> >
> > @@ -908,7 +918,8 @@ static int ar0521_power_on(struct device *dev)
> >
> >       return 0;
> >  off:
> > -     ar0521_power_off(dev);
> > +     clk_disable_unprepare(sensor->extclk);
> > +     __ar0521_power_off(dev);
> >       return ret;
> >  }
> >
> >
> > --
> > 2.46.0.76.ge559c4bf1a-goog
> >



-- 
Ricardo Ribalda

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2024-08-13 13:02 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-08-13 12:13 [PATCH 0/6] media: Fix last smatch warnings Ricardo Ribalda
2024-08-13 12:13 ` [PATCH 1/6] media: ar0521: Refactor ar0521_power_off() Ricardo Ribalda
2024-08-13 12:54   ` Dan Carpenter
2024-08-13 13:01     ` Ricardo Ribalda
2024-08-13 12:13 ` [PATCH 2/6] media: i2c: ov5645: Refactor ov5645_set_power_off() Ricardo Ribalda
2024-08-13 12:13 ` [PATCH 3/6] media: i2c: s5c73m3: Move clk_prepare to its own function Ricardo Ribalda
2024-08-13 12:56   ` Dan Carpenter
2024-08-13 12:13 ` [PATCH 4/6] media: tc358746: " Ricardo Ribalda
2024-08-13 12:13 ` [PATCH 5/6] media: meson: vdec_1: Refactor vdec_1_stop() Ricardo Ribalda
2024-08-13 12:13 ` [PATCH 6/6] media: meson: vdec: hevc: Refactor vdec_hevc_start and vdec_hevc_stop Ricardo Ribalda

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®