linux-kernel - Re: [PATCH v3 2/3] iio: adc: Add driver for Marvell 88PM886 PMIC ADC

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHp75VewzAN+tcJ9GrWESY38Uq+qeA4QH=qeaEV8g35H67GcMg@mail.gmail.com>
Date: Fri, 5 Sep 2025 16:39:05 +0300
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Duje Mihanović <duje@...emihanovic.xyz>
Cc: Jonathan Cameron <jic23@...nel.org>, David Lechner <dlechner@...libre.com>, 
	Nuno Sá <nuno.sa@...log.com>, 
	Andy Shevchenko <andy@...nel.org>, Karel Balej <balejk@...fyz.cz>, Lee Jones <lee@...nel.org>, 
	Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	David Wronek <david@...nlining.org>, phone-devel@...r.kernel.org, 
	~postmarketos/upstreaming@...ts.sr.ht, linux-kernel@...r.kernel.org, 
	linux-iio@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH v3 2/3] iio: adc: Add driver for Marvell 88PM886 PMIC ADC

On Fri, Sep 5, 2025 at 2:01 PM Duje Mihanović <duje@...emihanovic.xyz> wrote:
>
> Marvell's 88PM886 PMIC has a so-called General Purpose ADC used for
> monitoring various system voltages and temperatures. Add the relevant
> register definitions to the MFD header and a driver for the ADC.

Jonathan et al. The Q here is do we want to somehow make values
spelling (in the comments and documentation) be unified with the
Scientific Style [1]? Personally I find the article very useful and
one style for the whole subsystem would be good to enforce (in my
humble opinion). Thoughts?

[1]: https://poynton.ca/notes/units/

...

> +static int gpadc_get_raw(struct iio_dev *iio, enum pm886_gpadc_channel chan)
> +{
> +       struct pm886_gpadc *gpadc = iio_priv(iio);
> +       __be16 buf;
> +       int ret;
> +
> +       ret = regmap_bulk_read(gpadc->map, pm886_gpadc_regs[chan], &buf, sizeof(buf));
> +       if (ret)
> +               return ret;
> +
> +       return be16_to_cpu(buf) >> 4;

+ asm/byteorder.h (after linux/* but before linux/iio/*)

> +}

...

> +               /*
> +                * Vendor kernel errors out above 1.25V, but testing shows that

1.25 V

> +                * the resistance of the battery detection channel (GPADC2 on
> +                * coreprimevelte) reaches about 1.4Mohm when the battery is

1.4 MOhm or even 1.4 MΩ


> +                * removed, which can't be measured with such a low upper
> +                * limit. Therefore, to be able to detect the battery without
> +                * ugly externs as used in the vendor fuelgauge driver,

fuel gauge

> +                * increase this limit a bit.
> +                */
> +               if (WARN_ON(*raw_uV > 1500 * (MICRO / MILLI)))

+ bug.h

> +                       return -EIO;

...

> +               /*
> +                * Vendor kernel errors out under 300mV, but for the same

300 mV

> +                * reason as above (except the channel hovers around 3.5kohm

3.5 kOhm / 3.5 kΩ

> +                * with battery present) reduce this limit.
> +                */
> +               if (*raw_uV < 200 * (MICRO / MILLI)) {
> +                       dev_dbg(&iio->dev, "bad bias for chan %d: %duA @ %duV\n", chan->channel,

Also add spaces before units.

> +                               *raw_uA, *raw_uV);
> +                       continue;
> +               }
> +
> +               dev_dbg(&iio->dev, "good bias for chan %d: %duA @ %duV\n", chan->channel,
> +                       *raw_uA, *raw_uV);

Ditto.

> +               return 0;
> +       }
> +
> +       dev_err(&iio->dev, "failed to find good bias for chan %d\n", chan->channel);
> +       return -EINVAL;
> +}

...

> +       ret = DIV_ROUND_CLOSEST(raw_uV, raw_uA);

+ math.h

...

> +       page = devm_i2c_new_dummy_device(dev, client->adapter,
> +                                        client->addr + PM886_PAGE_OFFSET_GPADC);
> +       if (IS_ERR(page))

+ err.h

> +               return dev_err_probe(dev, PTR_ERR(page), "Failed to initialize GPADC page\n");


-- 
With Best Regards,
Andy Shevchenko