On Sat, 24 Jan 2026, Artur Weber wrote: > On 13.11.2025 14:26, Lee Jones wrote: > > On Wed, 29 Oct 2025, Artur Weber wrote: > > > > > On 23.10.2025 15:03, Lee Jones wrote: > > > > On Mon, 13 Oct 2025, Artur Weber wrote: > > > > > > > > > The BCM590XX supports up to 128 internal interrupts, which are used by > > > > > various parts of the chip. Add regmap_irq-based interrupt handling and > > > > > helper functions to allow subdevice drivers to easily use the > > > > > interrupts. > > > > > > > > > > Signed-off-by: Artur Weber <[email protected]> > > > > > > > > > > (...)>> > > > > > diff --git a/drivers/mfd/bcm590xx.c b/drivers/mfd/bcm590xx.c > > > > > index 5a8456bbd63f..fb6afe277ebf 100644 > > > > > --- a/drivers/mfd/bcm590xx.c > > > > > +++ b/drivers/mfd/bcm590xx.c > > > > > @@ -26,16 +26,29 @@ > > > > > #define BCM590XX_PMUREV_ANA_MASK 0xF0 > > > > > #define BCM590XX_PMUREV_ANA_SHIFT 4 > > > > > +#define BCM590XX_REG_IRQ1 0x20 > > > > > +#define BCM590XX_REG_IRQ1MASK 0x30 > > > > > > > > This isn't better. > > > > > > > > And now the nomenclature is inconsistent with the one above. > > > > > > > > What is a mask register? I don't understand. > > > > > > The IRQxMASK registers store the interrupt masks for each interrupt. To > > > explain this more clearly: > > > > > > The BCM590xx chips have up to 128 internal interrupts (the exact number > > > is different between the BCM59054 and BCM59056, but both reserve the > > > exact same amount of registers for them). > > > > > > The status of each interrupt is stored in the IRQx registers > > > (0x20-0x2f), and each bit represents a single interrupt. > > > > > > The interrupt masks (that is, whether the interrupt is enabled or > > > disabled) are stored in the IRQx_MASK registers (0x30-0x3f), and each > > > bit represents the mask for a single interrupt, in the same order as the > > > IRQx registers. (...would IRQMASKx be more consistent?) > > > > > > Each register stores 8 bits of data, meaning the {status, mask} for 8 > > > interrupts can fit into one {status, mask} register. > > > > Okay, so the "MASK" thing is just silly naming by the H/W designers? > > > > STATUS or ENABLE sounds like it would be better, since a "mask" to me is > > a software term which describes the methods for manipulating these kinds > > of groups of bits. > > > > As far as I can tell, "interrupt masking" is the standard term for > enabling/disabling interrupts[1]. It's even consistent with regmap_irq setup > code - if you scroll down to the part where this register gets used, it's > passed to the struct "regmap_irq_chip" to the field ".mask_base": > > > +static const struct regmap_irq_chip bcm59054_irq_chip = { > > + .name = "bcm59054-irq", > > + .irqs = bcm59054_regmap_irqs, > > + .num_irqs = BCM59054_IRQ_MAX, > > + .num_regs = 16, > > + .status_base = BCM590XX_REG_IRQ1, > > + .mask_base = BCM590XX_REG_IRQ1MASK, > > + .clear_on_unmask = true, > > +}; > > I find it a bit reductive to just call it "silly naming by the H/W > designers"... > > ...but I won't waste any more of your time about it :). I'll rename it to > IRQENABLE in the next version (though I do have mixed feelings about > completely changing a hw register's name...) > > [1] https://en.wikipedia.org/wiki/Interrupt#Masking
I know how masking works. However, I was unaware of the nomenclature used by Regmap. If the registers are called that in the datasheet, you should keep them. -- Lee Jones [李琼斯]
