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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87le1k8oq2.ffs@tglx>
Date: Mon, 29 Jul 2024 12:41:25 +0200
From: Thomas Gleixner <tglx@...utronix.de>
To: Conor Dooley <conor.dooley@...rochip.com>, linux-kernel@...r.kernel.org
Cc: conor@...nel.org, conor.dooley@...rochip.com, Marc Zyngier
 <maz@...nel.org>, Daire McNamara <daire.mcnamara@...rochip.com>, Linus
 Walleij <linus.walleij@...aro.org>, Bartosz Golaszewski <brgl@...ev.pl>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt
 <palmer@...belt.com>, linux-riscv@...ts.infradead.org,
 linux-gpio@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [RFC v7 3/6] irqchip: add mpfs gpio interrupt mux

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