Re: [PATCH v1 4/7] zorro: warp mfd: add the dual-port RAM command channel and expose the board identity

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] zorro: warp-mfd: stale reg_cr irq flags not cleared in
  warp_send_msg()
--

commit 4b5b6213bfa8c6be415c4e81b0b8ac501eb8e890
Author: Paolo Pisati <p.pisati@gmail.com>

zorro: warp mfd: add the dual-port RAM command channel and expose the board identity

This commit adds the dual-port RAM command channel and interrupt handling
to communicate with the ARM co-processor, exposing boardname, fmwver, and
hwver in sysfs.

> diff --git a/drivers/zorro/warp-mfd.c b/drivers/zorro/warp-mfd.c
> index 94af7c9d0ea5e..61866f678b613 100644
> --- a/drivers/zorro/warp-mfd.c
> +++ b/drivers/zorro/warp-mfd.c

[ ... ]

> +/* send a message to the ARM coprocessor, optionally waiting for a reply */
> +static int warp_send_msg(struct warp_mfd *warp, bool wait_reply)
> +{
> +	struct completion *done = wait_reply ? &warp->reply : &warp->recv;
> +	long ret;
> +
> +	/* a previous exchange may have timed out and completed late */
> +	reinit_completion(done);
> +
> +	/* enable 68k & ARM interrupts */
> +	z_writel(DPREG_CR_SET | DPREG_CR_IE_68K | DPREG_CR_IE_ARM, warp->reg_cr);
> +
> +	/* send irq to ARM */
> +	z_writel(DPREG_CR_SET | DPREG_CR_MP_ARM, warp->reg_cr);
> +
> +	/* wait for a reply, or just wait for a receiver ack */
> +	ret = wait_for_completion_interruptible_timeout(done,
> +							WARP_MSG_TIMEOUT);
> +
> +	/* disable ARM & 68k interrupts */
> +	z_writel(DPREG_CR_CLR | DPREG_CR_IE_68K | DPREG_CR_IE_ARM, warp->reg_cr);

[Severity: Medium]
Can an interrupted or timed-out wait leave DPREG_CR_MR_ARM and
DPREG_CR_MP_68K latched in warp->reg_cr?

When wait_for_completion_interruptible_timeout() returns early (such as on
a signal during a sysfs read), warp_send_msg() clears DPREG_CR_IE_68K
while the ARM is still processing the command.  When the ARM finishes, its
status flags stay latched in warp->reg_cr because the interrupt is
disabled.  On the next command, enabling DPREG_CR_IE_68K after
reinit_completion(done) immediately triggers warp_irq() from the stale
flags before the ARM processes the new request, so warp_exec_cmd() reads
back the command opcode still in warp->dpram and warp_get_status() fails
with -EIO.

Clearing DPREG_CR_MR_ARM and DPREG_CR_MP_68K before reinit_completion()
and enabling DPREG_CR_IE_68K would avoid completing on a stale flag.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111163735.227286-1-p.pisati@gmail.com?part=4