On Mon, 15 Apr 2024 18:05:54 +0300 Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote: > On Sun, Apr 14, 2024 at 8:57 PM Aren Moynihan <aren@xxxxxxxxxxxxxxxxx> 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