[PATCH 0/3] iio: adc: add mt6397 PMIC AUXADC support

Ryan Brue posted 3 patches 1 week, 2 days ago
There is a newer version of this series
.../bindings/iio/adc/mediatek,mt6359-auxadc.yaml   |   1 +
MAINTAINERS                                        |   7 +
drivers/iio/adc/Kconfig                            |  11 +
drivers/iio/adc/Makefile                           |   1 +
drivers/iio/adc/mt6397-auxadc.c                    | 340 +++++++++++++++++++++
drivers/mfd/mt6397-core.c                          |   3 +
.../dt-bindings/iio/adc/mediatek,mt6397-auxadc.h   |   9 +
7 files changed, 372 insertions(+)
[PATCH 0/3] iio: adc: add mt6397 PMIC AUXADC support
Posted by Ryan Brue 1 week, 2 days ago
The MediaTek mt6397 PMIC has a 10-bit AUXADC that nothing in-tree can
reach. On boards built around it that ADC is the only path to the battery:
the SoC's AUXADC is wired to board thermistors, and the charger ICs these
boards use have no ADC at all, so without it there is no pack voltage and
no state of charge.

Patch 1 adds the compatible to the existing MediaTek PMIC AUXADC schema and
the header naming the channels, patch 2 the driver, patch 3 the MFD cell
that instantiates it. The cell comes last so that no commit in between
registers a child no driver can claim.

The mt6397 is not given mediatek,mt6359-auxadc as a fallback because the
two are not register compatible; patch 1 has the detail.

Only the two channels a board needs for its pack are described, so patch 1
ships a header naming them: a channel ID is an index into the driver's array
and not the PMIC's channel number, as in mt6323-auxadc. The register
sequences are the ones the vendor kernel's pmic_auxadc.c programs.

One note on patch 3: it adds a cell to mt6397_devs[], which Luca Leonardo
Scorcia's MT6392 series converts to the MFD_CELL_* macros in its patch
4/9 [1]. Whichever of the two lands second needs a respin. Mine would
become a single MFD_CELL_OF(), and I can do a respin if that patch arrives
first.
Cc'd Luca.

[1] https://lore.kernel.org/all/20260902193351.185771-5-l.scorcia@gmail.com/

Tested on an Amazon Fire HD 10 (2017), an mt8173 with an mt6397. Both
channels are exercised: the battery channel by a fuel gauge, which reads 817
counts, 3830 mV with the pack at rest, and the thermistor channel by an
ntc-thermistor consuming it.

I have two things in the driver different from the vendor (Amazon
Fire OS based on a 3.18 kernel). Verified with over 150 interleaved reads
per setting:

 - Leaving the ISENSE input enabled between reads, as the vendor does,
   makes no difference to the value, so the driver switches it off again
   and leaves both ADC input enables clear when idle.

 - The chip's sample accumulator makes no difference at 4, 8, 16 or 31
   samples and costs no measurable time, so it is left at one. Software
   averaging helps, but only by about a quarter: 0.92 LSB standard
   deviation for one conversion against 0.67 for sixteen, where independent
   samples would have given 0.23. Conversions in a burst are correlated.

The reading was checked against the charger, which regulates the pack to
a programmed voltage in constant-voltage mode. Over twelve such voltages
from 3904 to 4080 mV, and charge currents from 111 to 778 mA, the driver's
reading is a mean of 0.9 mV from the regulated value, well inside
the charger's regulation tolerance. That also settles reading the chip's
trimmed result register rather than the raw one - the raw register is
20.7 mV low at every one of the twelve points.

Signed-off-by: Ryan Brue <ryanbrue.dev@gmail.com>
---
Ryan Brue (3):
      dt-bindings: iio: adc: mediatek,mt6359-auxadc: add mt6397 PMIC AUXADC
      iio: adc: mt6397-auxadc: add mt6397 PMIC AUXADC driver
      mfd: mt6397-core: Add mt6397 AUXADC support

 .../bindings/iio/adc/mediatek,mt6359-auxadc.yaml   |   1 +
 MAINTAINERS                                        |   7 +
 drivers/iio/adc/Kconfig                            |  11 +
 drivers/iio/adc/Makefile                           |   1 +
 drivers/iio/adc/mt6397-auxadc.c                    | 340 +++++++++++++++++++++
 drivers/mfd/mt6397-core.c                          |   3 +
 .../dt-bindings/iio/adc/mediatek,mt6397-auxadc.h   |   9 +
 7 files changed, 372 insertions(+)
---
base-commit: fd73f4a6659897191fa0d40695fe370925dd3780
change-id: 20260914-rbrue-suez-upstreaming-mt6397-auxadc-5e1228025c50

Best regards,
--  
Ryan Brue <ryanbrue.dev@gmail.com>
Re: [PATCH 0/3] iio: adc: add mt6397 PMIC AUXADC support
Posted by Andy Shevchenko 1 week, 1 day ago
On Tue, Sep 15, 2026 at 11:15:25PM -0500, Ryan Brue wrote:
> The MediaTek mt6397 PMIC has a 10-bit AUXADC that nothing in-tree can
> reach. On boards built around it that ADC is the only path to the battery:
> the SoC's AUXADC is wired to board thermistors, and the charger ICs these
> boards use have no ADC at all, so without it there is no pack voltage and
> no state of charge.

This doesn't explain why a brand new driver? Perhaps we have existing code that
may be updated to support this device?

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH 0/3] iio: adc: add mt6397 PMIC AUXADC support
Posted by Ryan Brue 1 week ago
On 9/16/26 4:50 AM, Andy Shevchenko wrote:
> On Tue, Sep 15, 2026 at 11:15:25PM -0500, Ryan Brue wrote:
>> The MediaTek mt6397 PMIC has a 10-bit AUXADC that nothing in-tree can
>> reach. On boards built around it that ADC is the only path to the battery:
>> the SoC's AUXADC is wired to board thermistors, and the charger ICs these
>> boards use have no ADC at all, so without it there is no pack voltage and
>> no state of charge.
> This doesn't explain why a brand new driver? Perhaps we have existing code that
> may be updated to support this device?
I considered adding mt6397 support to either mt6323-auxadc or 
mt6359-auxadc, and both had problems.

Both mt6323-auxadc and mt6359-auxadc select channels through a request 
register (1 bit per channel), while mt6397 uses a 4-bit numeric field 
CHSEL in CON1 (10:7), and then pulses a START bit (CON1 bit 0). That was 
the biggest reason I made the new driver.

For mt6323-auxadc, which is the closest I could find to the mt6397 
(CON0..CON27), it has 13 more registers than the mt6397 (CON0..CON14). 
It uses CON22 for its request register, and reads the result value from 
the same register as the ready bit. We don't do that - the mt6397 has a 
factory calibrated value for each channel at 0x16 higher than the raw 
value. mt6323 also has a 1800 mV / 15 bit scale / resolution while we 
have 1200 mV / 10 bits. We also have some per-channel preparation that 
we have to do before the burst, that the mt6323 doesn't have to do.

For mt6359-auxadc, it has a more generic framework for describing the 
AUXADC, but it assumes requests are channel-per-bit, and so we would 
have to basically ignore req_idx, req_mask, rdy_idx, and rdy_mask.

We also have our own software sampling, which the vendor does too 
(Amazon Fire OS based on Linux 3.18). We'd have to have our own sampling 
callback to do it.

I drafted two other versions of these patches adding mt6397 support to 
both of those drivers, but the differences meant I had to add a lot of 
extra boilerplate to each driver and to me it didn't make sense.

In v2 I will add the justification to the cover letter and commits for 
why I chose a new driver.

If you'd like me to instead send the exploratory patches I made adapting 
mt6323-auxadc or mt6359-auxadc, let me know. I'm fine if it ends up 
seeming like we should adapt one of the existing drivers, but I think 
the mechanism for controlling this AUXADC is unique and merits its own 
driver.

Thanks again for the review, I am going through each one, and sorry for 
the delay. I'm rather new to kernel development.

Best regards,
Ryan
Re: [PATCH 0/3] iio: adc: add mt6397 PMIC AUXADC support
Posted by Andy Shevchenko 6 days, 22 hours ago
On Thu, Sep 17, 2026 at 03:24:05PM -0500, Ryan Brue wrote:
> On 9/16/26 4:50 AM, Andy Shevchenko wrote:
> > On Tue, Sep 15, 2026 at 11:15:25PM -0500, Ryan Brue wrote:
> > > The MediaTek mt6397 PMIC has a 10-bit AUXADC that nothing in-tree can
> > > reach. On boards built around it that ADC is the only path to the battery:
> > > the SoC's AUXADC is wired to board thermistors, and the charger ICs these
> > > boards use have no ADC at all, so without it there is no pack voltage and
> > > no state of charge.
> > This doesn't explain why a brand new driver? Perhaps we have existing code that
> > may be updated to support this device?
> I considered adding mt6397 support to either mt6323-auxadc or mt6359-auxadc,
> and both had problems.
> 
> Both mt6323-auxadc and mt6359-auxadc select channels through a request
> register (1 bit per channel), while mt6397 uses a 4-bit numeric field CHSEL
> in CON1 (10:7), and then pulses a START bit (CON1 bit 0). That was the
> biggest reason I made the new driver.
> 
> For mt6323-auxadc, which is the closest I could find to the mt6397
> (CON0..CON27), it has 13 more registers than the mt6397 (CON0..CON14). It
> uses CON22 for its request register, and reads the result value from the
> same register as the ready bit. We don't do that - the mt6397 has a factory
> calibrated value for each channel at 0x16 higher than the raw value. mt6323
> also has a 1800 mV / 15 bit scale / resolution while we have 1200 mV / 10
> bits. We also have some per-channel preparation that we have to do before
> the burst, that the mt6323 doesn't have to do.
> 
> For mt6359-auxadc, it has a more generic framework for describing the
> AUXADC, but it assumes requests are channel-per-bit, and so we would have to
> basically ignore req_idx, req_mask, rdy_idx, and rdy_mask.

> We also have our own software sampling, which the vendor does too (Amazon
> Fire OS based on Linux 3.18). We'd have to have our own sampling callback to
> do it.

If it's not an upstream, we do not care.

> I drafted two other versions of these patches adding mt6397 support to both
> of those drivers, but the differences meant I had to add a lot of extra
> boilerplate to each driver and to me it didn't make sense.
> 
> In v2 I will add the justification to the cover letter and commits for why I
> chose a new driver.

Yes, please. Always do that when submitting a brand new driver that might sound
like a (partially) compatible with something already existing.

> If you'd like me to instead send the exploratory patches I made adapting
> mt6323-auxadc or mt6359-auxadc, let me know. I'm fine if it ends up seeming
> like we should adapt one of the existing drivers, but I think the mechanism
> for controlling this AUXADC is unique and merits its own driver.
> 
> Thanks again for the review, I am going through each one, and sorry for the
> delay. I'm rather new to kernel development.

No problem, and welcome!

-- 
With Best Regards,
Andy Shevchenko