Message-ID: <p4ommmpcjegvb4lafzecf67tzmdodtuqexeoifcn5eh7xqyp2y@ss76d3ubbsw7>
Date: Thu, 5 Feb 2026 15:50:59 +0900
From: Koichiro Den <den@...inux.co.jp>
To: Frank Li <Frank.li@....com>
Cc: vkoul@...nel.org, mani@...nel.org, jingoohan1@...il.com, 
	lpieralisi@...nel.org, kwilczynski@...nel.org, robh@...nel.org, bhelgaas@...gle.com, 
	dmaengine@...r.kernel.org, linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 04/11] dmaengine: Add selfirq callback registration API

On Wed, Feb 04, 2026 at 12:46:36PM -0500, Frank Li wrote:
> On Wed, Feb 04, 2026 at 11:54:32PM +0900, Koichiro Den wrote:
> > Some DMA controllers can generate an interrupt by software writing to a
> > register, without updating the normal interrupt status bits. This can be
> > used as a doorbell mechanism when the DMA engine is remotely programmed,
> > or for self-tests.
> >
> > Add an optional per-DMA-device API to register/unregister callbacks for
> > such "selfirq" events. Providers may invoke these callbacks from their
> > interrupt handler when they detect an emulated interrupt.
> >
> > Callbacks are invoked in hardirq context and must not sleep.
> 
> Is it possible register shared irq handle with the same channel's irq
> number?

The proposed dmaengine_{register,unregister}_selfirq() APIs are device-wide
(i.e. not per channel), so I'm not sure which "channel" you refer to here.
Also, when chip->nr_irqs > 1 on EP, dw-edma distributes channels across
multiple IRQ vectors, and it's unclear (at least to me) which IRQ vector
the emulated interrupt ("fake irq") is expected to be delivered on.

That said, technically, yes I agree adding another handler should be
possible, as dw-edma currently requests its irq(s) with IRQF_SHARED.
However, for a consumer driver to do request_irq() on its own, I think it
would need a stable way to obtain the irq number. Today that mapping seems
platform-specific and hidden behind dw_edma_plat_ops->irq_vector().

Would you prefer exposing a helper for obtaining the irq number (or
exporting the mapping in some form) instead of adding the dmaengine selfirq
API, or did you have another approach in mind?

Thanks for the review,
Koichiro

> 
> Frank
> 
> >
> > Signed-off-by: Koichiro Den <den@...inux.co.jp>
> > ---
> >  include/linux/dmaengine.h | 70 +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 70 insertions(+)
> >
> > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> > index 71bc2674567f..9c6194e8bfe1 100644
> > --- a/include/linux/dmaengine.h
> > +++ b/include/linux/dmaengine.h
> > @@ -785,6 +785,17 @@ struct dma_filter {
> >  	const struct dma_slave_map *map;
> >  };
> >
> > +/**
> > + * dma_selfirq_fn - callback for emulated/self IRQ events
> > + * @dev: DMA device invoking the callback
> > + * @data: opaque pointer provided at registration time
> > + *
> > + * Providers may invoke this callback from their interrupt handler when an
> > + * emulated interrupt ("selfirq") might have occurred. The callback runs in
> > + * hardirq context and must not sleep.
> > + */
> > +typedef void (*dma_selfirq_fn)(struct dma_device *dev, void *data);
> > +
> >  /**
> >   * struct dma_device - info on the entity supplying DMA services
> >   * @ref: reference is taken and put every time a channel is allocated or freed
> > @@ -853,6 +864,10 @@ struct dma_filter {
> >   *	or an error code
> >   * @device_synchronize: Synchronizes the termination of a transfers to the
> >   *  current context.
> > + * @device_register_selfirq: optional callback registration for
> > + *	emulated/self IRQ events
> > + * @device_unregister_selfirq: unregister previously registered selfirq
> > + *	callback
> >   * @device_tx_status: poll for transaction completion, the optional
> >   *	txstate parameter can be supplied with a pointer to get a
> >   *	struct with auxiliary transfer status information, otherwise the call
> > @@ -951,6 +966,11 @@ struct dma_device {
> >  	int (*device_terminate_all)(struct dma_chan *chan);
> >  	void (*device_synchronize)(struct dma_chan *chan);
> >
> > +	int (*device_register_selfirq)(struct dma_device *dev,
> > +				       dma_selfirq_fn fn, void *data);
> > +	void (*device_unregister_selfirq)(struct dma_device *dev,
> > +					  dma_selfirq_fn fn, void *data);
> > +
> >  	enum dma_status (*device_tx_status)(struct dma_chan *chan,
> >  					    dma_cookie_t cookie,
> >  					    struct dma_tx_state *txstate);
> > @@ -1197,6 +1217,56 @@ static inline void dmaengine_synchronize(struct dma_chan *chan)
> >  		chan->device->device_synchronize(chan);
> >  }
> >
> > +/**
> > + * dmaengine_register_selfirq() - Register a callback for emulated/self IRQ
> > + *                                events
> > + * @dev: DMA device
> > + * @fn: callback invoked from the provider's IRQ handler
> > + * @data: opaque callback data
> > + *
> > + * Some DMA controllers can raise an interrupt by software writing to a
> > + * register without updating normal status bits. Providers may call
> > + * registered callbacks from their interrupt handler when such events may
> > + * have occurred.
> > + * Callbacks are invoked in hardirq context and must not sleep.
> > + *
> > + * Return: 0 on success, -EOPNOTSUPP if unsupported, -EINVAL on bad args,
> > + * or provider-specific -errno.
> > + */
> > +static inline int dmaengine_register_selfirq(struct dma_device *dev,
> > +					     dma_selfirq_fn fn, void *data)
> > +{
> > +	if (!dev || !fn)
> > +		return -EINVAL;
> > +	if (!dev->device_register_selfirq)
> > +		return -EOPNOTSUPP;
> > +
> > +	return dev->device_register_selfirq(dev, fn, data);
> > +}
> > +
> > +/**
> > + * dmaengine_unregister_selfirq() - Unregister a previously registered
> > + *                                  selfirq callback
> > + * @dev: DMA device
> > + * @fn: callback pointer used at registration time
> > + * @data: opaque pointer used at registration time
> > + *
> > + * Unregister a callback previously registered via
> > + * dmaengine_register_selfirq(). Providers may synchronize against
> > + * in-flight callbacks, therefore this function may sleep and must not be
> > + * called from atomic context.
> > + */
> > +static inline void dmaengine_unregister_selfirq(struct dma_device *dev,
> > +						dma_selfirq_fn fn, void *data)
> > +{
> > +	if (!dev || !fn)
> > +		return;
> > +	if (!dev->device_unregister_selfirq)
> > +		return;
> > +
> > +	dev->device_unregister_selfirq(dev, fn, data);
> > +}
> > +
> >  /**
> >   * dmaengine_terminate_sync() - Terminate all active DMA transfers
> >   * @chan: The channel for which to terminate the transfers
> > --
> > 2.51.0
> >

Powered by blists - more mailing lists