* [PATCH v1] drivers:adp8870_bl: check the return value of adp8870_write
@ 2022-09-21 21:50 ` Li Zhong
0 siblings, 0 replies; 4+ messages in thread
From: Li Zhong @ 2022-09-21 21:50 UTC (permalink / raw)
To: linux-fbdev, dri-devel; +Cc: deller, jingoohan1, daniel.thompson, lee, Li Zhong
Check and propagate the return value of adp8870_write() when it fails,
which is possible when SMBus writing byte fails.
Signed-off-by: Li Zhong <floridsleeves@gmail.com>
---
drivers/video/backlight/adp8870_bl.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/video/backlight/adp8870_bl.c b/drivers/video/backlight/adp8870_bl.c
index 8b5213a39527..0eb4ae2ff592 100644
--- a/drivers/video/backlight/adp8870_bl.c
+++ b/drivers/video/backlight/adp8870_bl.c
@@ -567,9 +567,13 @@ static ssize_t adp8870_store(struct device *dev, const char *buf,
return ret;
mutex_lock(&data->lock);
- adp8870_write(data->client, reg, val);
+ ret = adp8870_write(data->client, reg, val);
mutex_unlock(&data->lock);
+ if (ret) {
+ return ret;
+ }
+
return count;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH v1] drivers:adp8870_bl: check the return value of adp8870_write
@ 2022-09-21 21:50 ` Li Zhong
0 siblings, 0 replies; 4+ messages in thread
From: Li Zhong @ 2022-09-21 21:50 UTC (permalink / raw)
To: linux-fbdev, dri-devel; +Cc: jingoohan1, deller, lee, daniel.thompson, Li Zhong
Check and propagate the return value of adp8870_write() when it fails,
which is possible when SMBus writing byte fails.
Signed-off-by: Li Zhong <floridsleeves@gmail.com>
---
drivers/video/backlight/adp8870_bl.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/video/backlight/adp8870_bl.c b/drivers/video/backlight/adp8870_bl.c
index 8b5213a39527..0eb4ae2ff592 100644
--- a/drivers/video/backlight/adp8870_bl.c
+++ b/drivers/video/backlight/adp8870_bl.c
@@ -567,9 +567,13 @@ static ssize_t adp8870_store(struct device *dev, const char *buf,
return ret;
mutex_lock(&data->lock);
- adp8870_write(data->client, reg, val);
+ ret = adp8870_write(data->client, reg, val);
mutex_unlock(&data->lock);
+ if (ret) {
+ return ret;
+ }
+
return count;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v1] drivers:adp8870_bl: check the return value of adp8870_write
2022-09-21 21:50 ` Li Zhong
@ 2022-09-22 9:34 ` Daniel Thompson
-1 siblings, 0 replies; 4+ messages in thread
From: Daniel Thompson @ 2022-09-22 9:34 UTC (permalink / raw)
To: Li Zhong; +Cc: linux-fbdev, dri-devel, deller, jingoohan1, lee
On Wed, Sep 21, 2022 at 02:50:49PM -0700, Li Zhong wrote:
> Subject: [PATCH v1] drivers:adp8870_bl: check the return value of
> adp8870_write
^^^^^^^^^^^^^^^^^^^^
Should be backlight: adp8870_bl.
> Check and propagate the return value of adp8870_write() when it fails,
> which is possible when SMBus writing byte fails.
This looks like a sensible change, however...
When writing patches like this please review the whole file for similar
concerns and fix all instances of the same issue. In this case there is another
unchecked call to adp8870_write() in
adp8870_bl_ambient_light_zone_store() (this function also contains
other unchecked calls).
Note that the unchecked use in adp8870_led_work() because there is no
way to propogate the error from this function (and adp8870_write()
already logged the error).
It would also be good to review and fix adp8860_bl.c at the same time
since these drivers are very similar (ideally the identical code in
these drivers should be factored out).
> Signed-off-by: Li Zhong <floridsleeves@gmail.com>
> ---
> drivers/video/backlight/adp8870_bl.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/backlight/adp8870_bl.c b/drivers/video/backlight/adp8870_bl.c
> index 8b5213a39527..0eb4ae2ff592 100644
> --- a/drivers/video/backlight/adp8870_bl.c
> +++ b/drivers/video/backlight/adp8870_bl.c
> @@ -567,9 +567,13 @@ static ssize_t adp8870_store(struct device *dev, const char *buf,
> return ret;
>
> mutex_lock(&data->lock);
> - adp8870_write(data->client, reg, val);
> + ret = adp8870_write(data->client, reg, val);
> mutex_unlock(&data->lock);
>
> + if (ret) {
> + return ret;
> + }
No need for braces here.
> +
> return count;
> }
>
Thanks
Daniel.
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v1] drivers:adp8870_bl: check the return value of adp8870_write
@ 2022-09-22 9:34 ` Daniel Thompson
0 siblings, 0 replies; 4+ messages in thread
From: Daniel Thompson @ 2022-09-22 9:34 UTC (permalink / raw)
To: Li Zhong; +Cc: jingoohan1, linux-fbdev, lee, deller, dri-devel
On Wed, Sep 21, 2022 at 02:50:49PM -0700, Li Zhong wrote:
> Subject: [PATCH v1] drivers:adp8870_bl: check the return value of
> adp8870_write
^^^^^^^^^^^^^^^^^^^^
Should be backlight: adp8870_bl.
> Check and propagate the return value of adp8870_write() when it fails,
> which is possible when SMBus writing byte fails.
This looks like a sensible change, however...
When writing patches like this please review the whole file for similar
concerns and fix all instances of the same issue. In this case there is another
unchecked call to adp8870_write() in
adp8870_bl_ambient_light_zone_store() (this function also contains
other unchecked calls).
Note that the unchecked use in adp8870_led_work() because there is no
way to propogate the error from this function (and adp8870_write()
already logged the error).
It would also be good to review and fix adp8860_bl.c at the same time
since these drivers are very similar (ideally the identical code in
these drivers should be factored out).
> Signed-off-by: Li Zhong <floridsleeves@gmail.com>
> ---
> drivers/video/backlight/adp8870_bl.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/backlight/adp8870_bl.c b/drivers/video/backlight/adp8870_bl.c
> index 8b5213a39527..0eb4ae2ff592 100644
> --- a/drivers/video/backlight/adp8870_bl.c
> +++ b/drivers/video/backlight/adp8870_bl.c
> @@ -567,9 +567,13 @@ static ssize_t adp8870_store(struct device *dev, const char *buf,
> return ret;
>
> mutex_lock(&data->lock);
> - adp8870_write(data->client, reg, val);
> + ret = adp8870_write(data->client, reg, val);
> mutex_unlock(&data->lock);
>
> + if (ret) {
> + return ret;
> + }
No need for braces here.
> +
> return count;
> }
>
Thanks
Daniel.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-09-22 9:34 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-21 21:50 [PATCH v1] drivers:adp8870_bl: check the return value of adp8870_write Li Zhong
2022-09-21 21:50 ` Li Zhong
2022-09-22 9:34 ` Daniel Thompson
2022-09-22 9:34 ` 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.