Re: [PATCH RFC 05/15] ims-msi: Add mask/unmask routines

Thomas Gleixner
2020-04-25T21:49:58Z

Dave Jiang <dave.jiang@intel.com> 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