Hi dee Ho peeps! Sorry for the late reply. pe 10. kesäk. 2022 klo 18.43 Aidan MacDonald (aidanmacdonald.0x0@xxxxxxxxx) kirjoitti: > > Mark Brown <broonie@xxxxxxxxxx> writes: > > > On Tue, Jun 07, 2022 at 04:53:09PM +0100, Aidan MacDonald wrote: > > > >> - if (!chip->sub_reg_offsets || !chip->not_fixed_stride) { > >> + if (chip->get_irq_reg) { > >> + reg = chip->get_irq_reg(base_reg, i); > >> + } else if (!chip->sub_reg_offsets || !chip->not_fixed_stride) { > > > > It seems like it would be cleaner and clearer to refactor things so that > > we always have a get_irq_reg() with standard chips getting given a > > default implementation which implements the current behaviour. > > I don't think that is a good way to clean things up. I only intended > get_irq_reg() to be a quick hack to solve a problem; in my opinion it > would be a poor abstraction to base the API around. > > What I'd suggest is something that will simplify regmap-irq. Instead of > defining the base registers, etc. in the chip, introduce a new struct > to describe a register group: > > struct regmap_irq_reg_group { > unsigned int status_base; > unsigned int mask_base; > ... > > unsigned int irq_reg_stride; > > int num_regs; > }; > > The idea is that the registers in a group are linearly mapped using the > formula "base + (i * irq_reg_stride)". Then it's possible to allow for > multiple register groups in regmap_irq_chip: > > struct regmap_irq_chip { > const struct regmap_irq_reg_group *groups; > unsigned int num_groups; > > unsigned int main_status_base; > unsigned int num_main_status_bits; > int num_main_regs; > > ... > }; > > It should be straightforward to fit existing chips into this model. > > - Chips that use a main status + sub-block IRQ layout will define > one register group for each sub-block and continue to describe the > location of the main status registers inside of regmap_irq_chip. > A group will only get polled if the corresponding main status bit > is set -- n'th group is polled if n'th bit is set. Does this work for devices where a single main status bit can flag IRQs in more than one sub-registers? Best Regards -- Matti -- Matti Vaittinen Linux kernel developer at ROHM Semiconductors Oulu Finland ~~ When things go utterly wrong vim users can always type :help! ~~ Discuss - Estimate - Plan - Report and finally accomplish this: void do_work(int time) __attribute__ ((const));