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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250111125616.2b12f03e@jic23-huawei>
Date: Sat, 11 Jan 2025 12:56:16 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Yu-Hsian Yang <j2anfernee@...il.com>
Cc: marcelo.schmitt@...log.com, olivier.moysan@...s.st.com,
 avifishman70@...il.com, tmaimon77@...il.com, tali.perry1@...il.com,
 venture@...gle.com, yuenn@...gle.com, benjaminfair@...gle.com,
 lars@...afoo.de, robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
 nuno.sa@...log.com, dlechner@...libre.com, javier.carrasco.cruz@...il.com,
 andriy.shevchenko@...ux.intel.com, mitrutzceclan@...il.com,
 tgamblin@...libre.com, matteomartelli3@...il.com, alisadariana@...il.com,
 gstols@...libre.com, thomas.bonnefille@...tlin.com,
 herve.codina@...tlin.com, chanh@...amperecomputing.com, KWLIU@...oton.com,
 yhyang2@...oton.com, openbmc@...ts.ozlabs.org, linux-iio@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 2/2] iio: adc: add Nuvoton NCT7201 ADC driver


<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