| Message ID | 20220620200644.1961936-1-aidanmacdonald.0x0@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | regmap-irq cleanups and refactoring | expand |
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!
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.
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.
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