* [PATCH] media: tw9910: add missed clk_disable_unprepare() on failure path
@ 2018-12-29 21:35 Alexey Khoroshilov
2018-12-30 9:49 ` Jacopo Mondi
0 siblings, 1 reply; 5+ messages in thread
From: Alexey Khoroshilov @ 2018-12-29 21:35 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Jacopo Mondi
Cc: Alexey Khoroshilov, linux-media, linux-kernel, ldv-project
If gpiod_get_optional() fails in tw9910_power_on(), clk is left undisabled.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
drivers/media/i2c/tw9910.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
index a54548cc4285..109770d678d2 100644
--- a/drivers/media/i2c/tw9910.c
+++ b/drivers/media/i2c/tw9910.c
@@ -610,6 +610,7 @@ static int tw9910_power_on(struct tw9910_priv *priv)
GPIOD_OUT_LOW);
if (IS_ERR(priv->rstb_gpio)) {
dev_info(&client->dev, "Unable to get GPIO \"rstb\"");
+ clk_disable_unprepare(priv->clk);
return PTR_ERR(priv->rstb_gpio);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] media: tw9910: add missed clk_disable_unprepare() on failure path
2018-12-29 21:35 [PATCH] media: tw9910: add missed clk_disable_unprepare() on failure path Alexey Khoroshilov
@ 2018-12-30 9:49 ` Jacopo Mondi
2018-12-30 11:17 ` Alexey Khoroshilov
0 siblings, 1 reply; 5+ messages in thread
From: Jacopo Mondi @ 2018-12-30 9:49 UTC (permalink / raw)
To: Alexey Khoroshilov
Cc: Mauro Carvalho Chehab, Jacopo Mondi, linux-media, linux-kernel,
ldv-project
[-- Attachment #1: Type: text/plain, Size: 1002 bytes --]
Hi Alexey,
thanks for the patch
On Sun, Dec 30, 2018 at 12:35:20AM +0300, Alexey Khoroshilov wrote:
> If gpiod_get_optional() fails in tw9910_power_on(), clk is left undisabled.
>
Correct, thanks for spotting this.
I think pdn_gpio should also be handled if rstb_gpio fails.
What's your opinion?
Thanks
j
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
> ---
> drivers/media/i2c/tw9910.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
> index a54548cc4285..109770d678d2 100644
> --- a/drivers/media/i2c/tw9910.c
> +++ b/drivers/media/i2c/tw9910.c
> @@ -610,6 +610,7 @@ static int tw9910_power_on(struct tw9910_priv *priv)
> GPIOD_OUT_LOW);
> if (IS_ERR(priv->rstb_gpio)) {
> dev_info(&client->dev, "Unable to get GPIO \"rstb\"");
> + clk_disable_unprepare(priv->clk);
> return PTR_ERR(priv->rstb_gpio);
> }
>
> --
> 2.7.4
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] media: tw9910: add missed clk_disable_unprepare() on failure path
2018-12-30 9:49 ` Jacopo Mondi
@ 2018-12-30 11:17 ` Alexey Khoroshilov
2018-12-30 11:41 ` [PATCH v2 1/2] media: tw9910: fix failure handling in tw9910_power_on() Alexey Khoroshilov
0 siblings, 1 reply; 5+ messages in thread
From: Alexey Khoroshilov @ 2018-12-30 11:17 UTC (permalink / raw)
To: Jacopo Mondi
Cc: Mauro Carvalho Chehab, Jacopo Mondi, linux-media, linux-kernel,
ldv-project
Hi Jacopo,
On 30.12.2018 12:49, Jacopo Mondi wrote:
> On Sun, Dec 30, 2018 at 12:35:20AM +0300, Alexey Khoroshilov wrote:
>> If gpiod_get_optional() fails in tw9910_power_on(), clk is left undisabled.
>
> Correct, thanks for spotting this.
>
> I think pdn_gpio should also be handled if rstb_gpio fails.
> What's your opinion?
I would agree. I'll send v2.
Thank you,
Alexey
>
>> Found by Linux Driver Verification project (linuxtesting.org).
>>
>> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
>> ---
>> drivers/media/i2c/tw9910.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
>> index a54548cc4285..109770d678d2 100644
>> --- a/drivers/media/i2c/tw9910.c
>> +++ b/drivers/media/i2c/tw9910.c
>> @@ -610,6 +610,7 @@ static int tw9910_power_on(struct tw9910_priv *priv)
>> GPIOD_OUT_LOW);
>> if (IS_ERR(priv->rstb_gpio)) {
>> dev_info(&client->dev, "Unable to get GPIO \"rstb\"");
>> + clk_disable_unprepare(priv->clk);
>> return PTR_ERR(priv->rstb_gpio);
>> }
>>
>> --
>> 2.7.4
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] media: tw9910: fix failure handling in tw9910_power_on()
2018-12-30 11:17 ` Alexey Khoroshilov
@ 2018-12-30 11:41 ` Alexey Khoroshilov
2018-12-30 11:41 ` [PATCH v2 2/2] media: tw9910: add helper function for setting gpiod value Alexey Khoroshilov
0 siblings, 1 reply; 5+ messages in thread
From: Alexey Khoroshilov @ 2018-12-30 11:41 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Jacopo Mondi
Cc: Alexey Khoroshilov, linux-media, linux-kernel, ldv-project
If gpiod_get_optional() fails in tw9910_power_on(), clk is left undisabled.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
v2: reset pdn_gpio as well as Jacopo Mondi suggested.
drivers/media/i2c/tw9910.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
index a54548cc4285..0971f8a34afb 100644
--- a/drivers/media/i2c/tw9910.c
+++ b/drivers/media/i2c/tw9910.c
@@ -610,6 +610,11 @@ static int tw9910_power_on(struct tw9910_priv *priv)
GPIOD_OUT_LOW);
if (IS_ERR(priv->rstb_gpio)) {
dev_info(&client->dev, "Unable to get GPIO \"rstb\"");
+ clk_disable_unprepare(priv->clk);
+ if (priv->pdn_gpio) {
+ gpiod_set_value(priv->pdn_gpio, 1);
+ usleep_range(500, 1000);
+ }
return PTR_ERR(priv->rstb_gpio);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] media: tw9910: add helper function for setting gpiod value
2018-12-30 11:41 ` [PATCH v2 1/2] media: tw9910: fix failure handling in tw9910_power_on() Alexey Khoroshilov
@ 2018-12-30 11:41 ` Alexey Khoroshilov
0 siblings, 0 replies; 5+ messages in thread
From: Alexey Khoroshilov @ 2018-12-30 11:41 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Jacopo Mondi
Cc: Alexey Khoroshilov, linux-media, linux-kernel, ldv-project
tw9910 driver tries to sleep for the smae period of time
after each gpiod_set_value(). The patch moves duplicated code
to a helper function.
Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
drivers/media/i2c/tw9910.c | 30 +++++++++++++-----------------
1 file changed, 13 insertions(+), 17 deletions(-)
diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
index 0971f8a34afb..8d1138e13803 100644
--- a/drivers/media/i2c/tw9910.c
+++ b/drivers/media/i2c/tw9910.c
@@ -584,6 +584,14 @@ static int tw9910_s_register(struct v4l2_subdev *sd,
}
#endif
+static void tw9910_set_gpio_value(struct gpio_desc *desc, int value)
+{
+ if (desc) {
+ gpiod_set_value(desc, value);
+ usleep_range(500, 1000);
+ }
+}
+
static int tw9910_power_on(struct tw9910_priv *priv)
{
struct i2c_client *client = v4l2_get_subdevdata(&priv->subdev);
@@ -595,10 +603,7 @@ static int tw9910_power_on(struct tw9910_priv *priv)
return ret;
}
- if (priv->pdn_gpio) {
- gpiod_set_value(priv->pdn_gpio, 0);
- usleep_range(500, 1000);
- }
+ tw9910_set_gpio_value(priv->pdn_gpio, 0);
/*
* FIXME: The reset signal is connected to a shared GPIO on some
@@ -611,18 +616,13 @@ static int tw9910_power_on(struct tw9910_priv *priv)
if (IS_ERR(priv->rstb_gpio)) {
dev_info(&client->dev, "Unable to get GPIO \"rstb\"");
clk_disable_unprepare(priv->clk);
- if (priv->pdn_gpio) {
- gpiod_set_value(priv->pdn_gpio, 1);
- usleep_range(500, 1000);
- }
+ tw9910_set_gpio_value(priv->pdn_gpio, 1);
return PTR_ERR(priv->rstb_gpio);
}
if (priv->rstb_gpio) {
- gpiod_set_value(priv->rstb_gpio, 1);
- usleep_range(500, 1000);
- gpiod_set_value(priv->rstb_gpio, 0);
- usleep_range(500, 1000);
+ tw9910_set_gpio_value(priv->rstb_gpio, 1);
+ tw9910_set_gpio_value(priv->rstb_gpio, 0);
gpiod_put(priv->rstb_gpio);
}
@@ -633,11 +633,7 @@ static int tw9910_power_on(struct tw9910_priv *priv)
static int tw9910_power_off(struct tw9910_priv *priv)
{
clk_disable_unprepare(priv->clk);
-
- if (priv->pdn_gpio) {
- gpiod_set_value(priv->pdn_gpio, 1);
- usleep_range(500, 1000);
- }
+ tw9910_set_gpio_value(priv->pdn_gpio, 1);
return 0;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-12-30 11:42 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-29 21:35 [PATCH] media: tw9910: add missed clk_disable_unprepare() on failure path Alexey Khoroshilov
2018-12-30 9:49 ` Jacopo Mondi
2018-12-30 11:17 ` Alexey Khoroshilov
2018-12-30 11:41 ` [PATCH v2 1/2] media: tw9910: fix failure handling in tw9910_power_on() Alexey Khoroshilov
2018-12-30 11:41 ` [PATCH v2 2/2] media: tw9910: add helper function for setting gpiod value Alexey Khoroshilov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.