Re: [RFC v7 3/6] irqchip: add mpfs gpio interrupt mux

[Thomas Gleixner]

On Tue, Jul 23 2024 at 12:27, Conor Dooley wrote:
> +
> +struct mpfs_irq_mux_bank_config {
> +	u32 mask;
> +	u8 shift;
> +};

Please see:

  https://www.kernel.org/doc/html/latest/process/maintainer-tip.html

vs. coding style.

> +/*
> + * Returns an unsigned long, where a set bit indicates the corresponding
> + * interrupt is in non-direct/muxed mode for that bank/GPIO controller.
> + */
> +static inline unsigned long mpfs_irq_mux_get_muxed_irqs(struct mpfs_irq_mux *priv,
> +							unsigned int bank)
> +{
> +	unsigned long mux_config = priv->mux_config, muxed_irqs = -1;
> +	struct mpfs_irq_mux_bank_config bank_config = mpfs_irq_mux_bank_configs[bank];
> +
> +	/*
> +	 * If a bit is set in the mux, GPIO the corresponding interrupt from
> +	 * controller 2 is direct and that controllers 0 or 1 is muxed.

This is not a coherent sentence.

> +	 * Invert the bits in the configuration register, so that set bits
> +	 * equate to non-direct mode, for GPIO controller 2.
> +	 */
> +	if (bank == 2u)
> +		mux_config = ~mux_config;
> +

> +static int mpfs_irq_mux_nondirect_alloc(struct irq_domain *d, unsigned int virq,
> +					struct irq_fwspec *fwspec, struct mpfs_irq_mux *priv)
> +{
> +	unsigned int bank = fwspec->param[0] / MPFS_MAX_IRQS_PER_GPIO;
> +
> +	if (bank > 2)
> +		return -EINVAL;
> +
> +	priv->nondirect_irqchips[bank].domain = d;
> +
> +	irq_domain_set_hwirq_and_chip(d, virq, fwspec->param[0],
> +				      &mpfs_irq_mux_nondirect_irq_chip, priv);
> +	irq_set_chained_handler_and_data(virq, handle_untracked_irq,

Why does this use handle_untracked_irq()? This sets up a chained handler
but handle_untracked_irq() is a regular interrupt handler.

> +					 &priv->nondirect_irqchips[bank]);

Thanks,

        tglx