Re: [PATCH v3 00/16] Add support for AXP192 PMIC

[Jonathan Cameron]

On Sun, 19 Jun 2022 00:43:07 +0200
Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote:

> On Saturday, June 18, 2022, Aidan MacDonald <aidanmacdonald.0x0@xxxxxxxxx>
> wrote:
> 
> > Changes in v3:
> >
> > * Update pinctrl driver to address Andy Shevchenko's review comments
> >   from v1, and fix a few other  
> 
> 
> I believe I gave more comments than just against pin control driver. Even
> though, some comments are still not addressed in the series, including pin
> control. Am I mistaken?

Hi Andy,

Maybe, it's a question of clarity/misunderstanding? You had some 'global' comments
at the end of the pinctrl review. Perhaps not clear enough you meant
they should apply to the rest of the patch series (and more generally to
the driver being modified I think).

Just guessing!

Jonathan

> 
> 
> > * Add gpio-ranges property and example snippet to gpio DT bindings.
> > * Update commit message of patch 01/16 to point out that all register
> >   addresses are obtained using sub_irq_reg().
> > * Document ccc_table in axp20x_battery. Also update commit message to
> >   note a small fix that is part of that patch.
> > * Drop axp20x_adc consolidation patch in favor of using separate adc_raw
> >   functions. It's a minor code size optimization that may not be worth
> >   the effort due to implementation complexity.
> > * Use the FIELD_GET macro in axp20x_adc to further clarify intent.
> > * Fix a typo in the regulator driver where an AXP20X regulator ID was
> >   mistakenly used instead of an AXP192 regulator ID. Also carry over
> >   an Acked-by: tag from v1. Hope that's okay.
> > * Accumulate Acked-by: tags from v1 on DT patches.
> > * Accumulate Acked-by: tags from v2.
> >
> > Note that regmap maintainer Mark Brown has said the first two patches to
> > regmap-irq aren't suitable for inclusion into the kernel in their current
> > state. I'm including them for v3 so the series remains testable.
> >
> > Changes in v2:
> >
> > * Do a little cleanup of axp20x_adc suggested by Jonathan Cameron
> > * Consolidate ADC read functions in axp20x_adc
> > * Drop the axp192's read_label callback in axp20x_adc
> > * Clean up the axp192-gpio dt bindings
> > * Rewrite a problematic bit of code in axp20x_usb_power reported
> >   by kernel test robot
> > * Support AXP192 in axp20x_battery
> > * Split up regmap-irq changes to two separate patches
> >
> > Cover letter from v1:
> >
> > Hi all,
> >
> > This patch series adds support for the X-Powers AXP192 PMIC to the
> > AXP20x driver framework.
> >
> > The first patch is a small change to regmap-irq to support the AXP192's
> > unusual IRQ register layout. It isn't possible to include all of the
> > IRQ registers in one regmap-irq chip without this.
> >
> > The rest of the changes are pretty straightforward, I think the only
> > notable parts are the axp20x_adc driver where there seems to be some
> > opportunities for code reuse (the axp192 is nearly a duplicate of the
> > axp20x) and the addition of a new pinctrl driver for the axp192, since
> > the axp20x pinctrl driver was not very easy to adapt.
> >
> > Aidan MacDonald (16):
> >   regmap-irq: Use sub_irq_reg() to calculate unmask register address
> >   regmap-irq: Add get_irq_reg to support unusual register layouts
> >   dt-bindings: mfd: add bindings for AXP192 MFD device
> >   dt-bindings: iio: adc: axp209: Add AXP192 compatible
> >   dt-bindings: power: supply: axp20x: Add AXP192 compatible
> >   dt-bindings: gpio: Add AXP192 GPIO bindings
> >   dt-bindings: power: axp20x-battery: Add AXP192 compatible
> >   mfd: axp20x: Add support for AXP192
> >   regulator: axp20x: Add support for AXP192
> >   iio: adc: axp20x_adc: Minor code cleanups
> >   iio: adc: axp20x_adc: Add support for AXP192
> >   power: supply: axp20x_usb_power: Add support for AXP192
> >   pinctrl: Add AXP192 pin control driver
> >   power: axp20x_battery: Add constant charge current table
> >   power: axp20x_battery: Support battery status without fuel gauge
> >   power: axp20x_battery: Add support for AXP192
> >
> >  .../bindings/gpio/x-powers,axp192-gpio.yaml   |  68 +++
> >  .../bindings/iio/adc/x-powers,axp209-adc.yaml |  18 +
> >  .../bindings/mfd/x-powers,axp152.yaml         |   1 +
> >  .../x-powers,axp20x-battery-power-supply.yaml |   1 +
> >  .../x-powers,axp20x-usb-power-supply.yaml     |   1 +
> >  drivers/base/regmap/regmap-irq.c              |  19 +-
> >  drivers/iio/adc/axp20x_adc.c                  | 359 +++++++++--
> >  drivers/mfd/axp20x-i2c.c                      |   2 +
> >  drivers/mfd/axp20x.c                          | 153 +++++
> >  drivers/pinctrl/Kconfig                       |  14 +
> >  drivers/pinctrl/Makefile                      |   1 +
> >  drivers/pinctrl/pinctrl-axp192.c              | 562 ++++++++++++++++++
> >  drivers/power/supply/axp20x_battery.c         | 143 ++++-
> >  drivers/power/supply/axp20x_usb_power.c       |  80 ++-
> >  drivers/regulator/axp20x-regulator.c          | 101 +++-
> >  include/linux/mfd/axp20x.h                    |  84 +++
> >  include/linux/regmap.h                        |   5 +
> >  17 files changed, 1529 insertions(+), 83 deletions(-)
> >  create mode 100644 Documentation/devicetree/
> > bindings/gpio/x-powers,axp192-gpio.yaml
> >  create mode 100644 drivers/pinctrl/pinctrl-axp192.c
> >
> > --
> > 2.35.1
> >
> >  
>