Re: [PATCH v2 2/2] irqchip: add MSM8x60 MPM wakeup interrupt controller driver

[Sebastian Andrzej Siewior]

On 2026-05-31 06:09:11 [+0200], Herman van Hazendonk wrote:
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -525,6 +525,29 @@ config QCOM_MPM
>  	  MSM Power Manager driver to manage and configure wakeup
>  	  IRQs for Qualcomm Technologies Inc (QTI) mobile chips.
>  
> +config QCOM_MSM8660_MPM
> +	bool "MSM8x60 MPM wakeup interrupt controller"
> +	depends on ARCH_QCOM
> +	depends on MFD_SYSCON
> +	depends on MAILBOX
> +	select IRQ_DOMAIN_HIERARCHY
> +	default y

This shouldn't by y by default.

> +	help
> +	  Platform driver for the MSM Power Manager (MPM) wakeup interrupt
> +	  controller on the MSM8x60 family (MSM8260/MSM8660/APQ8060). The vMPM registers live inside
> +	  the RPM control block, which makes the generic QCOM_MPM driver
> +	  unusable here (it assumes a dedicated MPM SRAM region, an IPCC
> +	  mailbox, and uses IRQCHIP_DECLARE early-init that races platform
> +	  device creation).
> +
> +	  This driver replicates the legacy 2.6.35-palm arch/arm/mach-msm/
> +	  mpm.c mechanism as a regular platform driver, accessing the vMPM
> +	  registers via a syscon phandle to the RPM block and signaling the
> +	  MPM via the qcom-apcs-ipc mailbox (writing to GCC + 0x008 bit 1).

This is a reference to something that does not exist.

> +	  Required for cpuidle deep-sleep states (SPC/PC) and for
> +	  WoWLAN-style wake-from-suspend on MSM8x60 SoCs.
> +
>  config CSKY_MPINTC
>  	bool
>  	depends on CSKY
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index 26aa3b6ec99f..3e64591f0f5b 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -100,6 +100,7 @@ obj-$(CONFIG_MESON_IRQ_GPIO)		+= irq-meson-gpio.o
>  obj-$(CONFIG_GOLDFISH_PIC) 		+= irq-goldfish-pic.o
>  obj-$(CONFIG_QCOM_PDC)			+= qcom-pdc.o
>  obj-$(CONFIG_QCOM_MPM)			+= irq-qcom-mpm.o
> +obj-$(CONFIG_QCOM_MSM8660_MPM)		+= irq-msm8660-mpm.o
>  obj-$(CONFIG_CSKY_MPINTC)		+= irq-csky-mpintc.o
>  obj-$(CONFIG_CSKY_APB_INTC)		+= irq-csky-apb-intc.o
>  obj-$(CONFIG_RISCV_INTC)		+= irq-riscv-intc.o
> diff --git a/drivers/irqchip/irq-msm8660-mpm.c b/drivers/irqchip/irq-msm8660-mpm.c
> new file mode 100644
> index 000000000000..b8394d9c076d
> --- /dev/null
> +++ b/drivers/irqchip/irq-msm8660-mpm.c
> @@ -0,0 +1,740 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * MSM8x60 family (MSM8260/MSM8660/APQ8060) MPM (MSM Power Manager) wakeup interrupt controller
> + *
> + * The MPM is an always-on hardware block that keeps a small set of wake
> + * sources alive while the application processor is powered down for
> + * cpuidle Power Collapse or suspend-to-RAM. On MSM8x60 the
> + * vMPM (virtual MPM) registers live INSIDE the RPM's 4 KB control block
> + * at:
> + *
> + *   request (control) regs: RPM_BASE + 0x9d8  (ENABLE, DETECT_CTL,
> + *                                              POLARITY, CLEAR)
> + *   status (pending) regs:  RPM_BASE + 0xdf8  (== 0x9d8 + 0x420)
> + *
> + * The mainline qcom-mpm driver (drivers/irqchip/irq-qcom-mpm.c) is
> + * fundamentally incompatible with this layout:
> + *   - it assumes a dedicated MPM SRAM region separate from RPM;
> + *   - it assumes a mailbox controller (IPCC) for wake notification;
> + *   - it uses IRQCHIP_DECLARE which runs before platform devices exist,
> + *     so of_find_device_by_node() returns NULL and the init silently
> + *     hangs.
> + *
> + * This driver replicates the 2.6.35-palm `arch/arm/mach-msm/mpm.c`
> + * mechanism as a regular platform driver: probes after platform
> + * infrastructure is ready, ioremaps the vMPM sub-region of the RPM
> + * control block (the qcom,rpm driver maps the surrounding area for
> + * its own use; the two mappings overlap and neither claims exclusive
> + * ownership), and uses the qcom-apcs-ipc mailbox for wake notification
> + * (writing to GCC + 0x008 bit 1).

Again, a file reference to something that does not exist.

> + * Register access is done with readl_relaxed/writel_relaxed rather than
> + * via the RPM syscon regmap. The IRQ core invokes our mask/unmask/
> + * set_type/set_wake callbacks with the irq_desc's raw_spinlock_t held,
> + * and syscon regmaps use a sleepable spinlock_t which on PREEMPT_RT
> + * would deadlock under that raw lock. Direct MMIO is also what every
> + * other SoC irqchip (qcom-pdc, gic-v3, ...) does.

Why is this kind of description here and not in the commit message.

> + * Two consumer interfaces:
> + *
> + *   1. Hierarchical irqdomain: for MPM pins that map to GIC SPIs (USB,
> + *      HDMI, ...). Consumers wire their interrupts through this
> + *      controller via interrupts-extended and the kernel manages
> + *      enable / mask / set_type / set_wake via the IRQ subsystem.
> + *
> + *   2. Raw-pin API: for MPM pins that do NOT correspond to a GIC IRQ
> + *      (SDC3_DAT1=21, SDC3_DAT3=22, SDC4_DAT1=23, SDC4_DAT3=24).
> + *      These are physical wake-signal lines monitored by MPM
> + *      directly. Consumers (mmci for SDC4 wake) call
> + *      msm8660_mpm_set_pin_wake() etc. The consumer API establishes
> + *      a device_link from consumer to producer so the MPM device
> + *      cannot disappear while a consumer holds a handle.
> + *
> + * Copyright (c) 2026 Herman van Hazendonk <github.com@xxxxxxxxxx>
> + * Copyright (c) 2010-2012, The Linux Foundation (legacy mpm.c reference)
> + */
> +
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/mailbox_client.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +
> +#include <soc/qcom/msm8660-mpm.h>
> +
> +/*
> + * vMPM register offsets (relative to the start of the ioremap'd window
> + * = RPM base + 0x9d8). Each register is two 32-bit slots because MPM
> + * exposes 64 wake pins.
> + */
> +#define MSM8660_MPM_REG_ENABLE		0x00
> +#define MSM8660_MPM_REG_DETECT_CTL	0x08
> +#define MSM8660_MPM_REG_POLARITY	0x10
> +#define MSM8660_MPM_REG_CLEAR		0x18
> +
> +/* Status registers at +0x420 from vMPM base (== RPM + 0xdf8). */
> +#define MSM8660_MPM_STATUS_OFFSET	0x420
> +
> +#define MSM8660_MPM_PIN_COUNT		64
> +#define MSM8660_MPM_REG_WIDTH		2
> +
> +struct msm8660_mpm_pin {
> +	int pin;
> +	int hwirq;
> +};
> +
> +struct msm8660_mpm {
> +	struct device *dev;
> +	void __iomem *base;
> +	struct irq_domain *domain;
> +	struct msm8660_mpm_pin *pin_map;
> +	unsigned int pin_map_count;
> +	int parent_irq;
> +	struct mbox_client mbox_client;
> +	struct mbox_chan *mbox_chan;
> +};
> +
> +/*
> + * Singleton - there is only one MPM instance per SoC. msm8660_mpm_get()
> + * returns this. Updates are serialised through the binding lifecycle so
> + * a plain pointer is sufficient.
> + */
> +static struct msm8660_mpm *msm8660_mpm_global;
> +
> +static u32 msm8660_mpm_read(struct msm8660_mpm *mpm, unsigned int reg)
> +{
> +	return readl_relaxed(mpm->base + reg);
> +}
> +
> +static void msm8660_mpm_write(struct msm8660_mpm *mpm, unsigned int reg,
> +			      u32 val)
> +{
> +	writel_relaxed(val, mpm->base + reg);
> +}
> +
> +/*
> + * Doorbell the RPM after touching the vMPM request registers. Without
> + * this the RPM keeps using its last cached copy of the enable/detect/
> + * polarity state and our configuration changes have no effect.
> + *
> + * Called from raw_spinlock_t-held contexts (irq_chip mask/unmask/
> + * set_type/set_wake), so the mailbox driver must accept that. The
> + * qcom-apcs-ipc mailbox just does a writel into the IPC trigger
> + * register; it is safe under a raw lock.
> + */

Where are these comments coming from?

> +static void msm8660_mpm_doorbell(struct msm8660_mpm *mpm)
…

Sebastian