Re: [PATCH v3 2/2] spi: qcom-geni: Add panic notifier to cancel and reset DMA during panic

[Praveen Talari]

Hi Mukesh

On 20-08-2026 01:02, Mukesh Savaliya wrote:
> On 8/18/2026 6:58 PM, Praveen Talari wrote:
> [...]
>
> > +static int spi_geni_panic_notifier(struct notifier_block *nb,
> > +                   unsigned long action, void *data)
> > +{
> > +    struct spi_geni_master *mas = container_of(nb, struct 
> > spi_geni_master, panic_nb);
> > +    struct spi_controller *spi = dev_get_drvdata(mas->dev);
> > +    struct geni_se *se = &mas->se;
> > +    u32 val;
> > +
> > +    if (!pm_runtime_active(mas->dev))
>
> Not completely sure, but why not NOTIFY_DONE ? we haven't handled 
> anything here.
My intention was to treat this as a successful no-op since no action is 
required when the device is runtime suspended.
> > +        return NOTIFY_OK;
> > +
> > +    if (mas->cur_xfer_mode == GENI_GPI_DMA) {
> > +        dmaengine_terminate_async(mas->tx);
> > +        dmaengine_terminate_async(mas->rx);
> > +        return NOTIFY_OK;
> > +    }
> > +
> > +    if (!(readl_relaxed(se->base + SE_GENI_STATUS) & 
> > M_GENI_CMD_ACTIVE))
> Shouldn't this be under FIFO mode check  ? As i can see SE DMA mode 
> and GPI mode conditions present but not for FIFO.
This code path is shared between FIFO and SE_DMA modes. The DMA reset 
sequence below is only required when the transfer is operating in DMA mode.
> > +        return NOTIFY_OK;
> > +
> > +    if (!spi->target) {
> > +        geni_se_cancel_m_cmd(se);
> > +        if (!readl_poll_timeout_atomic(se->base + 
> > SE_GENI_M_IRQ_STATUS, val,
> > +                           val & M_CMD_CANCEL_EN, 10, 50000)) {
> > +            writel_relaxed(M_CMD_CANCEL_EN, se->base + 
> > SE_GENI_M_IRQ_CLEAR);
> > +            return NOTIFY_OK;
> > +        }
> > +    }
> > +
> > +    geni_se_abort_m_cmd(se);
>
> why abort is done directly for target device and not for master ?
> shouldn't be combined with cancel failure ?

The target doesn't support cancel.

> > +    if (!readl_poll_timeout_atomic(se->base + SE_GENI_M_IRQ_STATUS, 
> > val,
> > +                       val & M_CMD_ABORT_EN, 10, 50000))
> > +        writel_relaxed(M_CMD_ABORT_EN, se->base + SE_GENI_M_IRQ_CLEAR);
> > +
> > +    if (mas->cur_xfer_mode == GENI_SE_DMA) {
> > +        writel_relaxed(1, se->base + SE_DMA_TX_FSM_RST);
> > +        readl_poll_timeout_atomic(se->base + SE_DMA_TX_IRQ_STAT, val,
> > +                      val & TX_RESET_DONE, 10, 50000);
> > +        writel_relaxed(val, se->base + SE_DMA_TX_IRQ_CLR);
> > +
> > +        writel_relaxed(1, se->base + SE_DMA_RX_FSM_RST);
> > +        readl_poll_timeout_atomic(se->base + SE_DMA_RX_IRQ_STAT, val,
> > +                      val & RX_RESET_DONE, 10, 50000);
> May be good to declar macro for 10usec and 50msec ? Multiple places 
> it's being used.> +        writel_relaxed(val, se->base + 
> SE_DMA_RX_IRQ_CLR);

Let me review and update.


Thanks,

Praveen Talari

> > +    }
> > +
> > +    return NOTIFY_OK;
> > +}
> > +[...]