* [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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).