On Fri, 5 Sep 2025 16:39:05 +0300 Andy Shevchenko <andy.shevche...@gmail.com> wrote:
> On Fri, Sep 5, 2025 at 2:01 PM Duje Mihanović <d...@dujemihanovic.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/ Consistency is indeed good, but I'm always careful about putting up barriers to submission. So I'd prefer starting with guidance and general review comments about consistent style, but probably not pushing back too hard initially at least on new code where a consistent slightly different style is applied. e.g. there is inconsistency in this patch as Volt and Ampere get capitals but not Ohm (which should given all 3 are named after people) To actually move to such a style we'd need to fix up at least some existing comments but that would be a trade off against churn. I've been useless for years about putting an IIO maintainer entry profile in place. https://docs.kernel.org/doc-guide/maintainer-profile.html but that is where probably where this sort of extra documentation of specific requirements would belong unless we can get broad agreement across subsystems where comments with units are common. Before moving to any standard on this I think we'd want to make sure we at least don't 'clash' with what is accepted in other subsystems where these units are common. +CC some relevant maintainers. Jonathan > > ... > > > +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"); > >
