Re: [PATCH v2 6/7] spmi: apple: use IRQ for RX FIFO if possible

[Janne Grunau <j@jannau.net>]

On Tue, Jul 28, 2026 at 11:28:12AM +0200, Sasha Finkelstein wrote:
> From: Alba Mendez <me@alba.sh>
> 
> The IRQ fires as soon as the reply is available, which is usually takes
> a few us instead of the 10ms sleep interval for polling
> 
> Signed-off-by: Alba Mendez <me@alba.sh>
> Signed-off-by: Sasha Finkelstein <k@chaosmail.tech>
> ---
>  drivers/spmi/spmi-apple-controller.c | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 93 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/spmi/spmi-apple-controller.c b/drivers/spmi/spmi-apple-controller.c
> index 2cd4ed1803d4..109c5d2c735d 100644
> --- a/drivers/spmi/spmi-apple-controller.c
> +++ b/drivers/spmi/spmi-apple-controller.c
> @@ -11,6 +11,8 @@
>   *		spmi-pmic-arb.c Copyright (c) 2021, The Linux Foundation.
>   */
>  
> +#include <linux/completion.h>
> +#include <linux/interrupt.h>
>  #include <linux/io.h>
>  #include <linux/iopoll.h>
>  #include <linux/module.h>
> @@ -23,6 +25,12 @@
>  #define SPMI_CMD_REG 0x4
>  #define SPMI_RSP_REG 0x8
>  
> +#define SPMI_IRQ_MASK_BASE 0x20
> +#define SPMI_IRQ_ACK_BASE 0x60
> +#define SPMI_IRQ_USER_SIZE 0x20
> +
> +#define SPMI_IRQ_FIFO_RX 0
> +
>  /* SPMI_RSP_REG reply word */
>  #define SPMI_REPLY_FRAME_PARITY_OFFSET 16
>  #define SPMI_REPLY_ACK BIT(15)
> @@ -37,6 +45,8 @@
>  struct apple_spmi {
>  	void __iomem *regs;
>  	struct mutex fifo_lock;
> +	bool fifo_rx_irq;
> +	struct completion fifo_rx;

please reorder the fields to avoid a hole in the struct

>  };
>  
>  #define poll_reg(spmi, reg, val, cond) \
> @@ -55,7 +65,19 @@ static int apple_spmi_wait_rx_not_empty(struct spmi_controller *ctrl)
>  	int ret;
>  	u32 status;
>  
> -	ret = poll_reg(spmi, SPMI_STATUS_REG, status, !(status & SPMI_RX_FIFO_EMPTY));
> +	if (spmi->fifo_rx_irq) {
> +		ret = wait_for_completion_timeout(&spmi->fifo_rx,
> +			usecs_to_jiffies(REG_POLL_TIMEOUT_US));
> +		if (!ret)
> +			ret = -ETIMEDOUT;
> +		else if (readl(spmi->regs + SPMI_STATUS_REG) & SPMI_RX_FIFO_EMPTY)
> +			ret = -EIO;
> +		else
> +			ret = 0;
> +	} else {
> +		ret = poll_reg(spmi, SPMI_STATUS_REG, status, !(status & SPMI_RX_FIFO_EMPTY));
> +	}
> +
>  	if (ret) {
>  		dev_err(&ctrl->dev,
>  			"failed to wait for RX FIFO not empty\n");
> @@ -77,6 +99,8 @@ static int spmi_raw_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid,
>  
>  	mutex_lock(&spmi->fifo_lock);
>  
> +	reinit_completion(&spmi->fifo_rx);
> +
>  	writel(spmi_cmd, spmi->regs + SPMI_CMD_REG);
>  
>  	while (i < len) {
> @@ -181,10 +205,68 @@ static int spmi_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid)
>  	return -EINVAL;
>  }
>  
> +static void apple_spmi_irq_ack_raw(struct apple_spmi *spmi, u32 irq)
> +{
> +	u32 __iomem *reg = spmi->regs + SPMI_IRQ_ACK_BASE + (irq / 32) * 4;
> +
> +	writel(BIT(irq % 32), reg);
> +}
> +
q> +static void apple_spmi_irq_mask_raw(struct apple_spmi *spmi, u32 irq)
> +{
> +	u32 __iomem *reg = spmi->regs + SPMI_IRQ_MASK_BASE + (irq / 32) * 4;
> +
> +	writel(readl(reg) & ~BIT(irq % 32), reg);
> +}
> +
> +static void apple_spmi_irq_unmask_raw(struct apple_spmi *spmi, u32 irq)
> +{
> +	u32 __iomem *reg = spmi->regs + SPMI_IRQ_MASK_BASE + (irq / 32) * 4;
> +
> +	writel(readl(reg) | BIT(irq % 32), reg);
> +}
> +
> +static irqreturn_t apple_spmi_irq_handler(int irq, void *dev_id)
> +{
> +	struct apple_spmi *spmi = dev_id;
> +	bool handled = false;
> +	u32 val;
> +
> +	val = readl(spmi->regs + SPMI_IRQ_ACK_BASE + SPMI_IRQ_USER_SIZE);
> +	if (val & BIT(SPMI_IRQ_FIFO_RX)) {
> +		apple_spmi_irq_ack_raw(spmi, SPMI_IRQ_USER_SIZE * 8 + SPMI_IRQ_FIFO_RX);
> +		complete(&spmi->fifo_rx);
> +		handled = true;
> +	}
> +
> +	return handled ? IRQ_HANDLED : IRQ_NONE;
> +}
> +
> +static int apple_spmi_init_irq(struct platform_device *pdev,
> +			  struct apple_spmi *spmi, int irq)
> +{
> +	int ret;
> +
> +	for (size_t offset = 0; offset < SPMI_IRQ_USER_SIZE + 4; offset += 4) {
> +		writel(0, spmi->regs + SPMI_IRQ_MASK_BASE + offset);
> +		writel(U32_MAX, spmi->regs + SPMI_IRQ_ACK_BASE + offset);
> +	}
> +
> +	spmi->fifo_rx_irq = true;
> +	apple_spmi_irq_unmask_raw(spmi, SPMI_IRQ_USER_SIZE * 8 + SPMI_IRQ_FIFO_RX);

can we use

#define SPMI_NUM_PERIPHERAL_IRQS	256
#define SPMI_NUM_IRQS			(SPMI_NUM_PERIPHERAL_IRQS + 32)

#define SPMI_IRQ_FIFO_RX		256

and get rid of SPMI_IRQ_USER_SIZE?

Janne