* [PATCH] backlight: (adp8870) fix a missing check for adp8870_write
@ 2018-12-25 6:21 ` Kangjie Lu
0 siblings, 0 replies; 6+ messages in thread
From: Kangjie Lu @ 2018-12-25 6:21 UTC (permalink / raw)
To: kjlu
Cc: pakki001, Lee Jones, Daniel Thompson, Jingoo Han,
Bartlomiej Zolnierkiewicz, dri-devel, linux-fbdev, linux-kernel
adp8870_write() may fail. This fix checks if adp8870_write fails, and if
so, returns its error code.
Signed-off-by: Kangjie Lu <kjlu@umn.edu>
---
drivers/video/backlight/adp8870_bl.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/video/backlight/adp8870_bl.c b/drivers/video/backlight/adp8870_bl.c
index 8d50e0299578..79901fb4fcd1 100644
--- a/drivers/video/backlight/adp8870_bl.c
+++ b/drivers/video/backlight/adp8870_bl.c
@@ -811,9 +811,14 @@ static ssize_t adp8870_bl_ambient_light_zone_store(struct device *dev,
if (!ret) {
reg_val &= ~(CFGR_BLV_MASK << CFGR_BLV_SHIFT);
reg_val |= (val - 1) << CFGR_BLV_SHIFT;
- adp8870_write(data->client, ADP8870_CFGR, reg_val);
- }
- mutex_unlock(&data->lock);
+ ret = adp8870_write(data->client,
+ ADP8870_CFGR, reg_val);
+ if (ret) {
+ mutex_unlock(&data->lock);
+ return ret;
+ }
+ } else
+ mutex_unlock(&data->lock);
}
return count;
--
2.17.2 (Apple Git-113)
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH] backlight: (adp8870) fix a missing check for adp8870_write
@ 2018-12-25 6:21 ` Kangjie Lu
0 siblings, 0 replies; 6+ messages in thread
From: Kangjie Lu @ 2018-12-25 6:21 UTC (permalink / raw)
To: kjlu
Cc: pakki001, Lee Jones, Daniel Thompson, Jingoo Han,
Bartlomiej Zolnierkiewicz, dri-devel, linux-fbdev, linux-kernel
adp8870_write() may fail. This fix checks if adp8870_write fails, and if
so, returns its error code.
Signed-off-by: Kangjie Lu <kjlu@umn.edu>
---
drivers/video/backlight/adp8870_bl.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/video/backlight/adp8870_bl.c b/drivers/video/backlight/adp8870_bl.c
index 8d50e0299578..79901fb4fcd1 100644
--- a/drivers/video/backlight/adp8870_bl.c
+++ b/drivers/video/backlight/adp8870_bl.c
@@ -811,9 +811,14 @@ static ssize_t adp8870_bl_ambient_light_zone_store(struct device *dev,
if (!ret) {
reg_val &= ~(CFGR_BLV_MASK << CFGR_BLV_SHIFT);
reg_val |= (val - 1) << CFGR_BLV_SHIFT;
- adp8870_write(data->client, ADP8870_CFGR, reg_val);
- }
- mutex_unlock(&data->lock);
+ ret = adp8870_write(data->client,
+ ADP8870_CFGR, reg_val);
+ if (ret) {
+ mutex_unlock(&data->lock);
+ return ret;
+ }
+ } else
+ mutex_unlock(&data->lock);
}
return count;
--
2.17.2 (Apple Git-113)
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] backlight: (adp8870) fix a missing check for adp8870_write
2018-12-25 6:21 ` Kangjie Lu
@ 2018-12-27 9:39 ` Sam Ravnborg
-1 siblings, 0 replies; 6+ messages in thread
From: Sam Ravnborg @ 2018-12-27 9:39 UTC (permalink / raw)
To: Kangjie Lu
Cc: Daniel Thompson, Bartlomiej Zolnierkiewicz, Jingoo Han,
linux-fbdev, dri-devel, linux-kernel, pakki001, Lee Jones
Hi Kangjie
> adp8870_write() may fail. This fix checks if adp8870_write fails, and if
> so, returns its error code.
>
> Signed-off-by: Kangjie Lu <kjlu@umn.edu>
> ---
> drivers/video/backlight/adp8870_bl.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/backlight/adp8870_bl.c b/drivers/video/backlight/adp8870_bl.c
> index 8d50e0299578..79901fb4fcd1 100644
> --- a/drivers/video/backlight/adp8870_bl.c
> +++ b/drivers/video/backlight/adp8870_bl.c
> @@ -811,9 +811,14 @@ static ssize_t adp8870_bl_ambient_light_zone_store(struct device *dev,
> if (!ret) {
> reg_val &= ~(CFGR_BLV_MASK << CFGR_BLV_SHIFT);
> reg_val |= (val - 1) << CFGR_BLV_SHIFT;
> - adp8870_write(data->client, ADP8870_CFGR, reg_val);
> - }
> - mutex_unlock(&data->lock);
> + ret = adp8870_write(data->client,
> + ADP8870_CFGR, reg_val);
> + if (ret) {
> + mutex_unlock(&data->lock);
> + return ret;
> + }
> + } else
> + mutex_unlock(&data->lock);
> }
Something looks wrong with the indent.
If you have braces around first part of if () then use
barces also after else. Then it is easier to follow the code.
In this case please consider another approach where you
have only a single mutex_unlock() both in the good and
in the error case.
Sam
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] backlight: (adp8870) fix a missing check for adp8870_write
@ 2018-12-27 9:39 ` Sam Ravnborg
0 siblings, 0 replies; 6+ messages in thread
From: Sam Ravnborg @ 2018-12-27 9:39 UTC (permalink / raw)
To: Kangjie Lu
Cc: Daniel Thompson, Bartlomiej Zolnierkiewicz, Jingoo Han,
linux-fbdev, dri-devel, linux-kernel, pakki001, Lee Jones
Hi Kangjie
> adp8870_write() may fail. This fix checks if adp8870_write fails, and if
> so, returns its error code.
>
> Signed-off-by: Kangjie Lu <kjlu@umn.edu>
> ---
> drivers/video/backlight/adp8870_bl.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/backlight/adp8870_bl.c b/drivers/video/backlight/adp8870_bl.c
> index 8d50e0299578..79901fb4fcd1 100644
> --- a/drivers/video/backlight/adp8870_bl.c
> +++ b/drivers/video/backlight/adp8870_bl.c
> @@ -811,9 +811,14 @@ static ssize_t adp8870_bl_ambient_light_zone_store(struct device *dev,
> if (!ret) {
> reg_val &= ~(CFGR_BLV_MASK << CFGR_BLV_SHIFT);
> reg_val |= (val - 1) << CFGR_BLV_SHIFT;
> - adp8870_write(data->client, ADP8870_CFGR, reg_val);
> - }
> - mutex_unlock(&data->lock);
> + ret = adp8870_write(data->client,
> + ADP8870_CFGR, reg_val);
> + if (ret) {
> + mutex_unlock(&data->lock);
> + return ret;
> + }
> + } else
> + mutex_unlock(&data->lock);
> }
Something looks wrong with the indent.
If you have braces around first part of if () then use
barces also after else. Then it is easier to follow the code.
In this case please consider another approach where you
have only a single mutex_unlock() both in the good and
in the error case.
Sam
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] backlight: (adp8870) fix a missing check for adp8870_write
2018-12-25 6:21 ` Kangjie Lu
@ 2019-01-03 16:48 ` Daniel Thompson
-1 siblings, 0 replies; 6+ messages in thread
From: Daniel Thompson @ 2019-01-03 16:48 UTC (permalink / raw)
To: Kangjie Lu
Cc: linux-fbdev, Bartlomiej Zolnierkiewicz, Jingoo Han, linux-kernel,
dri-devel, pakki001, Lee Jones
On Tue, Dec 25, 2018 at 12:21:09AM -0600, Kangjie Lu wrote:
> adp8870_write() may fail. This fix checks if adp8870_write fails, and if
> so, returns its error code.
>
> Signed-off-by: Kangjie Lu <kjlu@umn.edu>
> ---
> drivers/video/backlight/adp8870_bl.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/backlight/adp8870_bl.c b/drivers/video/backlight/adp8870_bl.c
> index 8d50e0299578..79901fb4fcd1 100644
> --- a/drivers/video/backlight/adp8870_bl.c
> +++ b/drivers/video/backlight/adp8870_bl.c
> @@ -811,9 +811,14 @@ static ssize_t adp8870_bl_ambient_light_zone_store(struct device *dev,
> if (!ret) {
> reg_val &= ~(CFGR_BLV_MASK << CFGR_BLV_SHIFT);
> reg_val |= (val - 1) << CFGR_BLV_SHIFT;
> - adp8870_write(data->client, ADP8870_CFGR, reg_val);
> - }
> - mutex_unlock(&data->lock);
> + ret = adp8870_write(data->client,
> + ADP8870_CFGR, reg_val);
> + if (ret) {
> + mutex_unlock(&data->lock);
> + return ret;
> + }
> + } else
> + mutex_unlock(&data->lock);
Please don't unbalance the lock paths. Instead check the value of ret *after*
the existing mutex unlock (which will also means that the failed
adp8870_read() isn't silently ignored).
Also there's not much point in fixing this site without also fixing the
missing return checks on adp8870_set_bits() and adp8870_clr_bits() in
the same function.
Daniel.
> }
>
> return count;
> --
> 2.17.2 (Apple Git-113)
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] backlight: (adp8870) fix a missing check for adp8870_write
@ 2019-01-03 16:48 ` Daniel Thompson
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Thompson @ 2019-01-03 16:48 UTC (permalink / raw)
To: Kangjie Lu
Cc: linux-fbdev, Bartlomiej Zolnierkiewicz, Jingoo Han, linux-kernel,
dri-devel, pakki001, Lee Jones
On Tue, Dec 25, 2018 at 12:21:09AM -0600, Kangjie Lu wrote:
> adp8870_write() may fail. This fix checks if adp8870_write fails, and if
> so, returns its error code.
>
> Signed-off-by: Kangjie Lu <kjlu@umn.edu>
> ---
> drivers/video/backlight/adp8870_bl.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/backlight/adp8870_bl.c b/drivers/video/backlight/adp8870_bl.c
> index 8d50e0299578..79901fb4fcd1 100644
> --- a/drivers/video/backlight/adp8870_bl.c
> +++ b/drivers/video/backlight/adp8870_bl.c
> @@ -811,9 +811,14 @@ static ssize_t adp8870_bl_ambient_light_zone_store(struct device *dev,
> if (!ret) {
> reg_val &= ~(CFGR_BLV_MASK << CFGR_BLV_SHIFT);
> reg_val |= (val - 1) << CFGR_BLV_SHIFT;
> - adp8870_write(data->client, ADP8870_CFGR, reg_val);
> - }
> - mutex_unlock(&data->lock);
> + ret = adp8870_write(data->client,
> + ADP8870_CFGR, reg_val);
> + if (ret) {
> + mutex_unlock(&data->lock);
> + return ret;
> + }
> + } else
> + mutex_unlock(&data->lock);
Please don't unbalance the lock paths. Instead check the value of ret *after*
the existing mutex unlock (which will also means that the failed
adp8870_read() isn't silently ignored).
Also there's not much point in fixing this site without also fixing the
missing return checks on adp8870_set_bits() and adp8870_clr_bits() in
the same function.
Daniel.
> }
>
> return count;
> --
> 2.17.2 (Apple Git-113)
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-01-03 16:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-12-25 6:21 [PATCH] backlight: (adp8870) fix a missing check for adp8870_write Kangjie Lu
2018-12-25 6:21 ` Kangjie Lu
2018-12-27 9:39 ` Sam Ravnborg
2018-12-27 9:39 ` Sam Ravnborg
2019-01-03 16:48 ` Daniel Thompson
2019-01-03 16:48 ` Daniel Thompson
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.