Dave Jiang <dave.jiang@xxxxxxxxx> writes: > > +static u32 __dev_ims_desc_mask_irq(struct msi_desc *desc, u32 flag) ...mask_irq()? This is doing both mask and unmask depending on the availability of the ops callbacks. > +{ > + u32 mask_bits = desc->platform.masked; > + const struct platform_msi_ops *ops; > + > + ops = desc->platform.msi_priv_data->ops; > + if (!ops) > + return 0; > + > + if (flag) { flag? Darn, this has a clear boolean meaning of mask or unmask and 'u32 flag' is the most natural and obvious self explaining expression for this, right? > + if (ops->irq_mask) > + mask_bits = ops->irq_mask(desc); > + } else { > + if (ops->irq_unmask) > + mask_bits = ops->irq_unmask(desc); > + } > + > + return mask_bits; What's mask_bits? This is about _ONE_ IMS interrupt. Can it have multiple mask bits and if so then the explanation which I decoded by crystal ball probably looks like this: Bit 0: Don't know whether it's masked Bit 1: Perhaps it's masked Bit 2: Probably it's masked Bit 3: Mostly masked ... Bit 31: Fully masked Or something like that. Makes a lot of sense in a XKCD cartoon at least. > +} > + > +/** > + * dev_ims_mask_irq - Generic irq chip callback to mask IMS interrupts > + * @data: pointer to irqdata associated to that interrupt > + */ > +static void dev_ims_mask_irq(struct irq_data *data) > +{ > + struct msi_desc *desc = irq_data_get_msi_desc(data); > + > + desc->platform.masked = __dev_ims_desc_mask_irq(desc, 1); The purpose of this masked information is? Thanks, tglx