Re: [PATCH 1/6] genirq: generic chip: add generic irq_mask_ack functions

[Doug Berger]

Mans, as the author of the only existing upstream user of this code,
should have received this as well.

-Doug

On 07/07/2017 12:20 PM, Doug Berger wrote:
> The irq_gc_mask_disable_reg_and_ack() function name implies that it
> provides the combined functions of irq_gc_mask_disable_reg() and
> irq_gc_ack().  However, the implementation does not actually do
> that since it writes the mask instead of the disable register. It
> also does not maintain the mask cache which makes it inappropriate
> to use with other masking functions.
> 
> In addition, commit 659fb32d1b67 ("genirq: replace irq_gc_ack() with
> {set,clr}_bit variants (fwd)") effectively renamed irq_gc_ack() to
> irq_gc_set_bit() so this function probably should have also been
> renamed at that time.
> 
> Since this generic chip code provides three mask functions and two
> ack functions, this commit provides generic implementations for all
> six combinations of the mask and ack functions suitable for use
> with the irq_mask_ack member of the struct irq_chip.
> 
> The '_reg' and '_bit' portions of the base function names were left
> out of the new combined function names in an attempt to keep the
> function name lengths manageable with the 80 character source code
> line length while still capturing the distinct aspects of each
> combination of functions.
> 
> Signed-off-by: Doug Berger <opendmb@xxxxxxxxx>
> ---
>  include/linux/irq.h       |   6 +++
>  kernel/irq/generic-chip.c | 120 ++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 126 insertions(+)
> 
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index 00db35b61e9e..23b9617bb682 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -1003,6 +1003,12 @@ void irq_gc_unmask_enable_reg(struct irq_data *d);
>  void irq_gc_ack_set_bit(struct irq_data *d);
>  void irq_gc_ack_clr_bit(struct irq_data *d);
>  void irq_gc_mask_disable_reg_and_ack(struct irq_data *d);
> +void irq_gc_mask_disable_and_ack_set(struct irq_data *d);
> +void irq_gc_mask_disable_and_ack_clr(struct irq_data *d);
> +void irq_gc_mask_set_and_ack_set(struct irq_data *d);
> +void irq_gc_mask_set_and_ack_clr(struct irq_data *d);
> +void irq_gc_mask_clr_and_ack_set(struct irq_data *d);
> +void irq_gc_mask_clr_and_ack_clr(struct irq_data *d);
>  void irq_gc_eoi(struct irq_data *d);
>  int irq_gc_set_wake(struct irq_data *d, unsigned int on);
>  
> diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
> index f7086b78ad6e..168887a81a29 100644
> --- a/kernel/irq/generic-chip.c
> +++ b/kernel/irq/generic-chip.c
> @@ -151,6 +151,126 @@ void irq_gc_mask_disable_reg_and_ack(struct irq_data *d)
>  }
>  
>  /**
> + * irq_gc_mask_disable_and_ack_set - Mask and ack pending interrupt
> + * @d: irq_data
> + *
> + * Chip has separate enable/disable registers instead of a single mask
> + * register and pending interrupt is acknowledged by setting a bit.
> + */
> +void irq_gc_mask_disable_and_ack_set(struct irq_data *d)
> +{
> +	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
> +	struct irq_chip_type *ct = irq_data_get_chip_type(d);
> +	u32 mask = d->mask;
> +
> +	irq_gc_lock(gc);
> +	irq_reg_writel(gc, mask, ct->regs.disable);
> +	*ct->mask_cache &= ~mask;
> +	irq_reg_writel(gc, mask, ct->regs.ack);
> +	irq_gc_unlock(gc);
> +}
> +
> +/**
> + * irq_gc_mask_disable_and_ack_clr - Mask and ack pending interrupt
> + * @d: irq_data
> + *
> + * Chip has separate enable/disable registers instead of a single mask
> + * register and pending interrupt is acknowledged by clearing a bit.
> + */
> +void irq_gc_mask_disable_and_ack_clr(struct irq_data *d)
> +{
> +	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
> +	struct irq_chip_type *ct = irq_data_get_chip_type(d);
> +	u32 mask = d->mask;
> +
> +	irq_gc_lock(gc);
> +	irq_reg_writel(gc, mask, ct->regs.disable);
> +	*ct->mask_cache &= ~mask;
> +	irq_reg_writel(gc, ~mask, ct->regs.ack);
> +	irq_gc_unlock(gc);
> +}
> +
> +/**
> + * irq_gc_mask_set_and_ack_set - Mask and ack pending interrupt
> + * @d: irq_data
> + *
> + * Chip has a single mask register where setting bits masks the interrupt
> + * and the pending interrupt is acknowledged by setting a bit.
> + */
> +void irq_gc_mask_set_and_ack_set(struct irq_data *d)
> +{
> +	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
> +	struct irq_chip_type *ct = irq_data_get_chip_type(d);
> +	u32 mask = d->mask;
> +
> +	irq_gc_lock(gc);
> +	*ct->mask_cache |= mask;
> +	irq_reg_writel(gc, *ct->mask_cache, ct->regs.mask);
> +	irq_reg_writel(gc, mask, ct->regs.ack);
> +	irq_gc_unlock(gc);
> +}
> +
> +/**
> + * irq_gc_mask_set_and_ack_clr - Mask and ack pending interrupt
> + * @d: irq_data
> + *
> + * Chip has a single mask register where setting bits masks the interrupt
> + * and the pending interrupt is acknowledged by clearing a bit.
> + */
> +void irq_gc_mask_set_and_ack_clr(struct irq_data *d)
> +{
> +	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
> +	struct irq_chip_type *ct = irq_data_get_chip_type(d);
> +	u32 mask = d->mask;
> +
> +	irq_gc_lock(gc);
> +	*ct->mask_cache |= mask;
> +	irq_reg_writel(gc, *ct->mask_cache, ct->regs.mask);
> +	irq_reg_writel(gc, ~mask, ct->regs.ack);
> +	irq_gc_unlock(gc);
> +}
> +
> +/**
> + * irq_gc_mask_clr_and_ack_set - Mask and ack pending interrupt
> + * @d: irq_data
> + *
> + * Chip has a single mask register where clearing bits masks the interrupt
> + * and the pending interrupt is acknowledged by setting a bit.
> + */
> +void irq_gc_mask_clr_and_ack_set(struct irq_data *d)
> +{
> +	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
> +	struct irq_chip_type *ct = irq_data_get_chip_type(d);
> +	u32 mask = d->mask;
> +
> +	irq_gc_lock(gc);
> +	*ct->mask_cache &= ~mask;
> +	irq_reg_writel(gc, *ct->mask_cache, ct->regs.mask);
> +	irq_reg_writel(gc, mask, ct->regs.ack);
> +	irq_gc_unlock(gc);
> +}
> +
> +/**
> + * irq_gc_mask_clr_and_ack_clr - Mask and ack pending interrupt
> + * @d: irq_data
> + *
> + * Chip has a single mask register where clearing bits masks the interrupt
> + * and the pending interrupt is acknowledged by clearing a bit.
> + */
> +void irq_gc_mask_clr_and_ack_clr(struct irq_data *d)
> +{
> +	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
> +	struct irq_chip_type *ct = irq_data_get_chip_type(d);
> +	u32 mask = ~d->mask;
> +
> +	irq_gc_lock(gc);
> +	*ct->mask_cache &= mask;
> +	irq_reg_writel(gc, *ct->mask_cache, ct->regs.mask);
> +	irq_reg_writel(gc, mask, ct->regs.ack);
> +	irq_gc_unlock(gc);
> +}
> +
> +/**
>   * irq_gc_eoi - EOI interrupt
>   * @d: irq_data
>   */
>