| Message ID | 20220921215049.1658796-1-floridsleeves@gmail.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [v1] drivers:adp8870_bl: check the return value of adp8870_write | expand |
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.
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; }
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(-)