Re: [PATCH v3 2/2] iio: adc: add Nuvoton NCT7201 ADC driver

[Jonathan Cameron <jic23@kernel.org>]

<snip>

> > > +
> > > +     /*
> > > +      * After about 25 msecs, the device should be ready and then
> > > +      * the Power Up bit will be set to 1. If not, wait for it.
> > > +      */
> > > +     mdelay(25);
> > > +     err = regmap_read(chip->regmap, NCT7201_REG_BUSY_STATUS, &value);
> > > +     if (err < 0)
> > > +             return err;
> > > +     if (!(value & NCT7201_BIT_PWR_UP))
> > > +             return dev_err_probe(&chip->client->dev, -EIO, "failed to power up after reset\n");
> > > +
> > > +     /* Enable Channel */
> > > +     guard(mutex)(&chip->access_lock);
> > > +     regmap_write(chip->regmap, NCT7201_REG_CHANNEL_ENABLE_1,
> > > +                  NCT7201_REG_CHANNEL_ENABLE_1_MASK);  
> >
> > Check return value.  This is over an I2C bus, not the most reliable of
> > transports!
> >
> > Consider doing this differently and using a bulk write for the larger
> > case.
> >
> >         if (chip->num_vin_channels <= 8)
> >                 ret = regmap_write();
> >         else
> >                 ret = regmap_bulk_write();
> >
> > However as you read ENABLE_2 unconditionally below, can you instead just
> > always use a bulk write here?
> >  
> 
> We can't use regmap_bulk_write() due to the chip's limit.
> regmap_bulk_write(chip->regmap, ..., ..., 2) ,
> the first byte is well written, but the second byte don't changed.

Find out why.  You may well need to set a few more parameters for
the configuration of the regmap to ensure the correct form of bulk
write. 

> 
> 
> > > +     if (chip->num_vin_channels == 12)
> > > +             regmap_write(chip->regmap, NCT7201_REG_CHANNEL_ENABLE_2,
> > > +                          NCT7201_REG_CHANNEL_ENABLE_2_MASK);
> > > +

Jonathan