| Message ID | 20240414175716.958831-3-aren@peacevolution.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: light: stk3310: support powering off during suspend | expand |
On Sun, Apr 14, 2024 at 8:57 PM Aren Moynihan <aren@peacevolution.org> wrote: > > If the chip isn't powered, this call is likely to return an error. > Without a log here the driver will silently fail to probe. Common errors > are ENXIO (when the chip isn't powered) and ETIMEDOUT (when the i2c bus > isn't powered). > ret = regmap_read(data->regmap, STK3310_REG_ID, &chipid); > - if (ret < 0) > + if (ret < 0) { > + dev_err(&client->dev, "failed to read chip id: %d", ret); > return ret; > + } Briefly looking at the code it seems that this one is strictly part of the probe phase, which means we may use return dev_err_probe(...); pattern. Yet, you may add another patch to clean up all of them: _probe(), _init(), _regmap_init() to use the same pattern everywhere.
On Mon, 15 Apr 2024 18:05:54 +0300 Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Sun, Apr 14, 2024 at 8:57 PM Aren Moynihan <aren@peacevolution.org> wrote: > > > > If the chip isn't powered, this call is likely to return an error. > > Without a log here the driver will silently fail to probe. Common errors > > are ENXIO (when the chip isn't powered) and ETIMEDOUT (when the i2c bus > > isn't powered). > > > ret = regmap_read(data->regmap, STK3310_REG_ID, &chipid); > > - if (ret < 0) > > + if (ret < 0) { > > + dev_err(&client->dev, "failed to read chip id: %d", ret); > > return ret; > > + } > > Briefly looking at the code it seems that this one is strictly part of > the probe phase, which means we may use > > return dev_err_probe(...); > > pattern. Yet, you may add another patch to clean up all of them: > _probe(), _init(), _regmap_init() to use the same pattern everywhere. > Yes, a precursor patch to use dev_err_probe() throughout the probe only functions in this driver would be excellent. Jonathan
diff --git a/drivers/iio/light/stk3310.c b/drivers/iio/light/stk3310.c index bfa090538df7..c0954a63a143 100644 --- a/drivers/iio/light/stk3310.c +++ b/drivers/iio/light/stk3310.c @@ -472,8 +472,10 @@ static int stk3310_init(struct iio_dev *indio_dev) struct i2c_client *client = data->client; ret = regmap_read(data->regmap, STK3310_REG_ID, &chipid); - if (ret < 0) + if (ret < 0) { + dev_err(&client->dev, "failed to read chip id: %d", ret); return ret; + } if (chipid != STK3310_CHIP_ID_VAL && chipid != STK3311_CHIP_ID_VAL &&
If the chip isn't powered, this call is likely to return an error. Without a log here the driver will silently fail to probe. Common errors are ENXIO (when the chip isn't powered) and ETIMEDOUT (when the i2c bus isn't powered). Signed-off-by: Aren Moynihan <aren@peacevolution.org> --- drivers/iio/light/stk3310.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)