On Thu, 2021-10-14 at 11:34 +0200, Michael Tretter wrote:
> On Wed, 13 Oct 2021 10:05:30 +0200, Michael Tretter wrote:
> > On Tue, 12 Oct 2021 15:27:11 +0200, Philipp Zabel wrote:
[...]
> > > > +static int isl7998x_wait_power_on(struct isl7998x *isl7998x)
> > > > +{
> > > > + struct device *dev = isl7998x->subdev.dev;
> > > > + unsigned int retry;
> > > > + u32 chip_id;
> > > > + int ret = -ETIMEDOUT;
> > > > +
> > > > + for (retry = 10; ret && retry > 0; retry--) {
> > > > + ret = regmap_read(isl7998x->regmap,
> > > > + ISL7998x_REG_P0_PRODUCT_ID_CODE, &chip_id);
> > > > + usleep_range(1000, 2000);
> > > > + }
> > >
> > > Consider using regmap_read_poll_timeout() here.
> >
> > Ack. I forgot about this function.
>
> regmap_read_poll_timeout() cannot be used here, because it returns if
> regmap_read() returns an error. The driver uses the return value of
> regmap_read() to detect, if the chip is available, and should continue polling
> if regmap_read() failed. I can implement the necessary behavior with
> read_poll_timeout(), but am not sure if it is really worth it.
Oh, right. I still think so, but your call.
regards
Philipp
