mbox series

[00/49] regmap-irq cleanups and refactoring

Message ID 20220620200644.1961936-1-aidanmacdonald.0x0@gmail.com (mailing list archive)
Headers show
Series regmap-irq cleanups and refactoring | expand

Message

Aidan MacDonald June 20, 2022, 8:05 p.m. UTC
Hi Mark,

Here's a bunch of cleanups for regmap-irq focused on simplifying the API
and generalizing it a bit. It's broken up into three refactors, focusing
on one area at a time.

* Patches 01 and 02 are straightforward bugfixes, independent of the
  rest of the series. Neither of the bugs are triggered by in-tree
  drivers but they might be worth picking up early anyhow.

* Patches 03-13 clean up everything related to configuring IRQ types.

* Patches 14-45 deal with mask/unmask registers. First, make unmask
  registers behave more intuitively and usefully, and get rid of the
  mask_invert flag in favor of describing inverted mask registers as
  unmask registers. Second, make the mask_writeonly flag more useful
  and enable it for two chips where it makes sense.

* Patches 46-49 refactor sub_irq_reg() as a get_irq_reg() callback,
  and use that to eliminate the not_fixed_stride flag.

The approach I used when refactoring is pretty simple: (1) introduce new
functionality in regmap-irq, (2) convert the drivers, and (3) remove any
old code. Nothing should break in the middle.

The patches can be re-ordered to some extent if that's preferable, but
it's best to add get_irq_reg() last to avoid having to think about how
it interacts with features that'll be removed anyway.

I can't test most of the devices affected by this series so a lot of the
code is only build tested. I've tested on real hardware with my AXP192
patchset[1], although it only provides limited code coverage.

qcom-pm8008 in particular deserves careful testing - it used all of the
features touched by the refactors and required the most changes. Other
drivers only required trivial changes but there are three of them worth
mentioning: wcd943x, wcd9335, and wcd938x. They have suspicious looking
IRQ type definitions and I'm pretty sure aren't working properly, but
I can't fix them myself. The refactor shouldn't affect their behavior
so how / when / if they get fixed shouldn't be much of an issue.

Oh, and I added the 'mask_writeonly' flag and volatile ranges to the
stpmic1 driver based on its datasheet[2] as a small optimization. It's
probably fine but testing would be a good idea.

[1]: https://lore.kernel.org/linux-iio/20220618214009.2178567-1-aidanmacdonald.0x0@gmailcom/
[2]: https://www.st.com/resource/en/datasheet/stpmic1.pdf

Aidan MacDonald (49):
  regmap-irq: Fix a bug in regmap_irq_enable() for type_in_mask chips
  regmap-irq: Fix offset/index mismatch in read_sub_irq_data()
  regmap-irq: Remove an unnecessary restriction on type_in_mask
  regmap-irq: Introduce config registers for irq types
  mfd: qcom-pm8008: Convert irq chip to config regs
  mfd: wcd934x: Convert irq chip to config regs
  sound: soc: codecs: wcd9335: Convert irq chip to config regs
  sound: soc: codecs: wcd938x: Remove spurious type_base from irq chip
  mfd: max77650: Remove useless type_invert flag
  regmap-irq: Remove virtual registers support
  regmap-irq: Remove old type register support, refactor
  regmap-irq: Remove unused type_reg_stride field
  regmap-irq: Remove unused type_invert flag
  regmap-irq: Do not use regmap_irq_update_bits() for wake regs
  regmap-irq: Change the behavior of mask_writeonly
  regmap-irq: Rename regmap_irq_update_bits()
  regmap-irq: Add broken_mask_unmask flag
  mfd: qcom-pm8008: Add broken_mask_unmask irq chip flag
  mfd: stpmic1: Add broken_mask_unmask irq chip flag
  regmap-irq: Fix inverted handling of unmask registers
  mfd: tps65090: replace irqchip mask_invert with unmask_base
  mfd: sun4i-gpadc: replace irqchip mask_invert with unmask_base
  mfd: sprd-sc27xx-spi: replace irqchip mask_invert with unmask_base
  mfd: rt5033: replace irqchip mask_invert with unmask_base
  mfd: rohm-bd71828: replace irqchip mask_invert with unmask_base
  mfd: rn5t618: replace irqchip mask_invert with unmask_base
  mfd: gateworks-gsc: replace irqchip mask_invert with unmask_base
  mfd: axp20x: replace irqchip mask_invert with unmask_base
  mfd: atc260x: replace irqchip mask_invert with unmask_base
  mfd: 88pm800: replace irqchip mask_invert with unmask_base
  mfd: max14577: replace irqchip mask_invert with unmask_base
  mfd: max77693: replace irqchip mask_invert with unmask_base
  mfd: rohm-bd718x7: drop useless mask_invert flag on irqchip
  mfd: max77843: drop useless mask_invert flag on irqchip
  extcon: max77843: replace irqchip mask_invert with unmask_base
  extcon: sm5502: drop useless mask_invert flag on irqchip
  extcon: rt8973a: drop useless mask_invert flag on irqchip
  irqchip: sl28cpld: replace irqchip mask_invert with unmask_base
  gpio: sl28cpld: replace irqchip mask_invert with unmask_base
  mfd: stpmic1: Fix broken mask/unmask in irq chip
  mfd: stpmic1: Enable mask_writeonly flag for irq chip
  mfd: qcom-pm8008: Fix broken mask/unmask in irq chip
  mfd: qcom-pm8008: Enable mask_writeonly flag for irq chip
  regmap-irq: Remove broken_mask_unmask flag
  regmap-irq: Remove mask_invert flag
  regmap-irq: Refactor checks for status bulk read support
  regmap-irq: Add get_irq_reg() callback
  mfd: qcom-pm8008: Use get_irq_reg() for irq chip
  regmap-irq: Remove not_fixed_stride flag

 drivers/base/regmap/regmap-irq.c | 457 ++++++++++++++-----------------
 drivers/extcon/extcon-max77843.c |   3 +-
 drivers/extcon/extcon-rt8973a.c  |   1 -
 drivers/extcon/extcon-sm5502.c   |   2 -
 drivers/gpio/gpio-sl28cpld.c     |   3 +-
 drivers/irqchip/irq-sl28cpld.c   |   3 +-
 drivers/mfd/88pm800.c            |   3 +-
 drivers/mfd/atc260x-core.c       |   6 +-
 drivers/mfd/axp20x.c             |  21 +-
 drivers/mfd/gateworks-gsc.c      |   3 +-
 drivers/mfd/max14577.c           |   7 +-
 drivers/mfd/max77650.c           |   1 -
 drivers/mfd/max77693.c           |   6 +-
 drivers/mfd/max77843.c           |   1 -
 drivers/mfd/qcom-pm8008.c        | 131 ++++-----
 drivers/mfd/rn5t618.c            |   3 +-
 drivers/mfd/rohm-bd71828.c       |   6 +-
 drivers/mfd/rohm-bd718x7.c       |   1 -
 drivers/mfd/rt5033.c             |   3 +-
 drivers/mfd/sprd-sc27xx-spi.c    |   3 +-
 drivers/mfd/stpmic1.c            |   7 +-
 drivers/mfd/sun4i-gpadc.c        |   3 +-
 drivers/mfd/tps65090.c           |   3 +-
 drivers/mfd/wcd934x.c            |  11 +-
 include/linux/regmap.h           |  59 ++--
 sound/soc/codecs/wcd9335.c       |  10 +-
 sound/soc/codecs/wcd938x.c       |   1 -
 27 files changed, 332 insertions(+), 426 deletions(-)

Comments

Andy Shevchenko June 21, 2022, 9:25 a.m. UTC | #1
On Mon, Jun 20, 2022 at 10:07 PM Aidan MacDonald
<aidanmacdonald.0x0@gmail.com> wrote:
>
> Hi Mark,
>
> Here's a bunch of cleanups for regmap-irq focused on simplifying the API
> and generalizing it a bit. It's broken up into three refactors, focusing
> on one area at a time.
>
> * Patches 01 and 02 are straightforward bugfixes, independent of the
>   rest of the series. Neither of the bugs are triggered by in-tree
>   drivers but they might be worth picking up early anyhow.
>
> * Patches 03-13 clean up everything related to configuring IRQ types.
>
> * Patches 14-45 deal with mask/unmask registers. First, make unmask
>   registers behave more intuitively and usefully, and get rid of the
>   mask_invert flag in favor of describing inverted mask registers as
>   unmask registers. Second, make the mask_writeonly flag more useful
>   and enable it for two chips where it makes sense.
>
> * Patches 46-49 refactor sub_irq_reg() as a get_irq_reg() callback,
>   and use that to eliminate the not_fixed_stride flag.
>
> The approach I used when refactoring is pretty simple: (1) introduce new
> functionality in regmap-irq, (2) convert the drivers, and (3) remove any
> old code. Nothing should break in the middle.
>
> The patches can be re-ordered to some extent if that's preferable, but
> it's best to add get_irq_reg() last to avoid having to think about how
> it interacts with features that'll be removed anyway.
>
> I can't test most of the devices affected by this series so a lot of the
> code is only build tested. I've tested on real hardware with my AXP192
> patchset[1], although it only provides limited code coverage.
>
> qcom-pm8008 in particular deserves careful testing - it used all of the
> features touched by the refactors and required the most changes. Other
> drivers only required trivial changes but there are three of them worth
> mentioning: wcd943x, wcd9335, and wcd938x. They have suspicious looking
> IRQ type definitions and I'm pretty sure aren't working properly, but
> I can't fix them myself. The refactor shouldn't affect their behavior
> so how / when / if they get fixed shouldn't be much of an issue.
>
> Oh, and I added the 'mask_writeonly' flag and volatile ranges to the
> stpmic1 driver based on its datasheet[2] as a small optimization. It's
> probably fine but testing would be a good idea.
>
> [1]: https://lore.kernel.org/linux-iio/20220618214009.2178567-1-aidanmacdonald.0x0@gmailcom/
> [2]: https://www.st.com/resource/en/datasheet/stpmic1.pdf

Cool series, thanks for cleaning this up!
Mark Brown June 21, 2022, 5:08 p.m. UTC | #2
On Mon, Jun 20, 2022 at 09:05:55PM +0100, Aidan MacDonald wrote:

> Here's a bunch of cleanups for regmap-irq focused on simplifying the API
> and generalizing it a bit. It's broken up into three refactors, focusing
> on one area at a time.

This series is very large and the way it is interleaving patches for
several different subsystems adds to the difficulty managing it.  As
you've identified there's several different subserieses in here, if you
need to resend any of this (I've not even started looking at the actual
patches yet) it would be easier to digest with some combination of
sending as separate serieses and reordering things so that all the
things for each subsystem are grouped together.  That'd help with both
review and with merging, both large serieses and cross subsystem
dependencies tend to slow things down.
Aidan MacDonald June 21, 2022, 9:04 p.m. UTC | #3
Mark Brown <broonie@kernel.org> writes:

> On Mon, Jun 20, 2022 at 09:05:55PM +0100, Aidan MacDonald wrote:
>
>> Here's a bunch of cleanups for regmap-irq focused on simplifying the API
>> and generalizing it a bit. It's broken up into three refactors, focusing
>> on one area at a time.
>
> This series is very large and the way it is interleaving patches for
> several different subsystems adds to the difficulty managing it.  As
> you've identified there's several different subserieses in here, if you
> need to resend any of this (I've not even started looking at the actual
> patches yet) it would be easier to digest with some combination of
> sending as separate serieses and reordering things so that all the
> things for each subsystem are grouped together.  That'd help with both
> review and with merging, both large serieses and cross subsystem
> dependencies tend to slow things down.

Thanks for the advice. After reading this and some of Andy's comments I
think I understand how to organize this better.

I'll send a trimmed down series including only regmap changes, and
instead of removing things right away I'll just mark them deprecated.
Then the driver patches can go by subsystem (one series per subsystem?)
and once everything is merged the deprecated stuff in regmap-irq can be
removed at a later date.
Mark Brown June 22, 2022, 3:16 p.m. UTC | #4
On Mon, 20 Jun 2022 21:05:55 +0100, Aidan MacDonald wrote:
> Here's a bunch of cleanups for regmap-irq focused on simplifying the API
> and generalizing it a bit. It's broken up into three refactors, focusing
> on one area at a time.
> 
> * Patches 01 and 02 are straightforward bugfixes, independent of the
>   rest of the series. Neither of the bugs are triggered by in-tree
>   drivers but they might be worth picking up early anyhow.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git for-next

Thanks!

[01/49] regmap-irq: Fix a bug in regmap_irq_enable() for type_in_mask chips
        commit: 485037ae9a095491beb7f893c909a76cc4f9d1e7
[02/49] regmap-irq: Fix offset/index mismatch in read_sub_irq_data()
        commit: 3f05010f243be06478a9b11cfce0ce994f5a0890

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark