| Message ID | 20210521194805.2078135-1-trix@redhat.com |
|---|---|
| State | Accepted |
| Commit | 544ee7306d9e3a7ff675e87f151723ff30efd9a1 |
| Headers | show |
| Series | media: staging: atomisp: improve error handling in gc2235_detect() | expand |
Hi Tom, On Fri, May 21, 2021 at 12:48:05PM -0700, trix@redhat.com wrote: > From: Tom Rix <trix@redhat.com> > > Static analysis reports this representative problem > > atomisp-gc2235.c:867:20: warning: The right operand > of '|' is a garbage value > id = ((high << 8) | low); > ^ ~~~ > When gc2235_read_reg() fails, its return val is never written. > > For gc2235_detect(), high and low are or-ed and compared > with GC2235_ID, 0x2235. Initialize both to 0 and skip > checking the read returns, it's errors are not passed up, only > -ENODEV is. > > Signed-off-by: Tom Rix <trix@redhat.com> > --- > drivers/staging/media/atomisp/i2c/atomisp-gc2235.c | 13 +++---------- > 1 file changed, 3 insertions(+), 10 deletions(-) > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c > index 38defa0f81513..3b6e02b1f45d1 100644 > --- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c > +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c > @@ -849,21 +849,14 @@ static int gc2235_get_fmt(struct v4l2_subdev *sd, > static int gc2235_detect(struct i2c_client *client) > { > struct i2c_adapter *adapter = client->adapter; > - u16 high, low; > - int ret; > + u16 high = 0, low = 0; > u16 id; > > if (!i2c_check_functionality(adapter, I2C_FUNC_I2C)) > return -ENODEV; > > - ret = gc2235_read_reg(client, GC2235_8BIT, > - GC2235_SENSOR_ID_H, &high); > - if (ret) { > - dev_err(&client->dev, "sensor_id_high = 0x%x\n", high); > - return -ENODEV; It'd be nice if the function returned a different value for different errors, such as there was an I/O error and that the ID was wrong. > - } > - ret = gc2235_read_reg(client, GC2235_8BIT, > - GC2235_SENSOR_ID_L, &low); > + gc2235_read_reg(client, GC2235_8BIT, GC2235_SENSOR_ID_H, &high); > + gc2235_read_reg(client, GC2235_8BIT, GC2235_SENSOR_ID_L, &low); > id = ((high << 8) | low); > > if (id != GC2235_ID) { -- Sakari Ailus
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c index 38defa0f81513..3b6e02b1f45d1 100644 --- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c @@ -849,21 +849,14 @@ static int gc2235_get_fmt(struct v4l2_subdev *sd, static int gc2235_detect(struct i2c_client *client) { struct i2c_adapter *adapter = client->adapter; - u16 high, low; - int ret; + u16 high = 0, low = 0; u16 id; if (!i2c_check_functionality(adapter, I2C_FUNC_I2C)) return -ENODEV; - ret = gc2235_read_reg(client, GC2235_8BIT, - GC2235_SENSOR_ID_H, &high); - if (ret) { - dev_err(&client->dev, "sensor_id_high = 0x%x\n", high); - return -ENODEV; - } - ret = gc2235_read_reg(client, GC2235_8BIT, - GC2235_SENSOR_ID_L, &low); + gc2235_read_reg(client, GC2235_8BIT, GC2235_SENSOR_ID_H, &high); + gc2235_read_reg(client, GC2235_8BIT, GC2235_SENSOR_ID_L, &low); id = ((high << 8) | low); if (id != GC2235_ID) {