linux-kernel - Re: [PATCH v8 9/9] drivers: mfd: Add support for MediaTek SPMI PMICs and MT6363/73

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: <14451186.uLZWGnKmhe@workhorse>
Date: Thu, 09 Oct 2025 22:15:01 +0200
From: Nicolas Frattaroli <nicolas.frattaroli@...labora.com>
To: linux-mediatek@...ts.infradead.org,
 angelogioacchino.delregno@...labora.com
Cc: lee@...nel.org, robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
 matthias.bgg@...il.com, lgirdwood@...il.com, broonie@...nel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, kernel@...labora.com,
 wenst@...omium.org, igor.belwon@...tallysanemainliners.org,
 Nícolas F. R. A. Prado <nfraprado@...labora.com>,
 AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
Subject:
 Re: [PATCH v8 9/9] drivers: mfd: Add support for MediaTek SPMI PMICs and
 MT6363/73

On Friday, 3 October 2025 11:11:58 Central European Summer Time AngeloGioacchino Del Regno wrote:
> This driver adds support for the MediaTek SPMI PMICs and their
> interrupt controller (which is present in 95% of the cases).
> 
> Other than probing all of the sub-devices of a SPMI PMIC, this
> sets up a regmap from the relevant SPMI bus and initializes an
> interrupt controller with its irq domain and irqchip to handle
> chained interrupts, with the SPMI bus itself being its parent
> irq controller, and the PMIC being the outmost device.
> 
> This driver hence holds all of the information about a specific
> PMIC's interrupts and will properly handle them, calling the
> ISR for any subdevice that requested an interrupt.
> 
> As for the interrupt spec, this driver wants 3 interrupt cells,
> but ignores the first one: this is because of how this first
> revision of the MediaTek SPMI 2.0 Controller works, which does
> not hold irq number information in its register, but delegates
> that to the SPMI device - it's possible that this will change
> in the future with a newer revision of the controller IP, and
> this is the main reason for that.
> 
> To make use of this implementation, this driver also adds the
> required bits to support MediaTek MT6363 and MT6373 SPMI PMICs.
> 
> Reviewed-by: Nícolas F. R. A. Prado <nfraprado@...labora.com>
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
> ---
>  drivers/mfd/Kconfig         |  17 ++
>  drivers/mfd/Makefile        |   1 +
>  drivers/mfd/mtk-spmi-pmic.c | 410 ++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/mt6363.h  |  26 +++
>  include/linux/mfd/mt6373.h  |  21 ++
>  5 files changed, 475 insertions(+)
>  create mode 100644 drivers/mfd/mtk-spmi-pmic.c
>  create mode 100644 include/linux/mfd/mt6363.h
>  create mode 100644 include/linux/mfd/mt6373.h

Hi Angelo, came across something suspicious here while debugging why
I ran into IRQ parsing from DT issues here.

>
> [...]
> diff --git a/drivers/mfd/mtk-spmi-pmic.c b/drivers/mfd/mtk-spmi-pmic.c
> new file mode 100644
> index 000000000000..512b53bdb0d1
> --- /dev/null
> +++ b/drivers/mfd/mtk-spmi-pmic.c
> [...]
> +
> +static int mtk_spmi_pmic_irq_xlate(struct irq_domain *d, struct device_node *ctrlr,
> +				  const u32 *intspec, unsigned int intsize,
> +				  unsigned long *out_hwirq, unsigned int *out_type)
> +{
> +	struct mtk_spmi_pmic *pmic = d->host_data;
> +	struct device *dev = pmic->dev;
> +	struct irq_fwspec fwspec;
> +
> +	of_phandle_args_to_fwspec(ctrlr, intspec, intsize, &fwspec);
> +	if (WARN_ON(fwspec.param_count < 3))
> +		return -EINVAL;

What's the point of fwspec here? The caller in
irqdomain.c::irq_domain_translate has an fwspec, converts it to an
of_node with the intsize/intspec args, then passes it to this function,
which builds it back into an fwspec just to check fwspec.param_count,
which is equal intsize.

fwspec is then never used again.

Just check intsize instead, it's precisely that value.

spmi-mtk-pmif.c::mtk_spmi_rcs_irq_xlate does the same thing, which also
seems pointless.

> +
> +	/*
> +	 * The IRQ number in intspec[0] is ignored on purpose here!
> +	 *
> +	 * This is because of how at least the first revision of the SPMI 2.0
> +	 * controller works in MediaTek SoCs: the controller will raise an
> +	 * interrupt for each SID (but doesn't know the details!), and the
> +	 * specific IRQ number that got raised must be read from the PMIC or
> +	 * its sub-device driver.
> +	 * It's possible that this will change in the future with a newer
> +	 * revision of the SPMI controller, and this is why the devicetree
> +	 * holds the full intspec.
> +	 */
> +	*out_hwirq = intspec[1];
> +	*out_type = intspec[2] & IRQ_TYPE_SENSE_MASK;
> +
> +	dev_dbg(dev, "Found device IRQ %u chained from SPMI IRQ %x (map: 0x%lx)\n",
> +		intspec[1], intspec[0], *out_hwirq);
> +
> +	return 0;
> +}
> +
> +static const struct irq_domain_ops mtk_spmi_pmic_irq_domain_ops = {
> +	.map = mtk_spmi_pmic_irq_domain_map,
> +	.xlate = mtk_spmi_pmic_irq_xlate,
> +};
> +
> [...]

Kind regards,
Nicolas Frattaroli