Re: [PATCH v3 05/13] dmaengine: dw-edma: Add partial channel ownership mode

[Koichiro Den]

On Mon, Jun 22, 2026 at 10:59:26AM -0500, Frank Li wrote:
> On Sun, Jun 21, 2026 at 02:00:32AM +0900, Koichiro Den wrote:
> > A DesignWare eDMA instance may represent only a subset of a controller
> 
> s/a subset of a controller/a subset of channels

Will fix.

> 
> > that is also initialized by another OS instance, such as an
> > endpoint-side OS. Add a partial ownership flag for instances that must
> > preserve controller-wide state owned by that peer.
> >
> > In partial ownership mode, dw-edma skips the initial core reset in
> > probe() and uses the limited quiesce path in remove() instead of the
> > full core-off path. The flag also makes the driver validate the
> > ownership granularity required by each register layout before
> > registering channels.
> >
> > For EDMA_MF_EDMA_UNROLL and EDMA_MF_HDMA_COMPAT, the driver programs
> > per-direction registers, such as DMA_{WRITE,READ}_INT_MASK_OFF and
> > DMA_{WRITE,READ}_INT_CLEAR_OFF. These register layouts have at most
> > EDMA_MAX_{WR,RD}_CH channels per direction, so the capped hardware
> > channel count still represents the whole direction. A partial instance
> > can therefore expose write or read channels only if it owns every
> > channel in that direction; otherwise two OS instances could update the
> > same direction-wide registers without a shared locking protocol.
> >
> > In contrast, HDMA native uses per-channel registers, so it can be shared
> > at channel granularity.
> 
> Not "shared", each channel can be owned by local or remote indepently?

Right, that wording is misleading. Will fix.

> 
> >
> > Signed-off-by: Koichiro Den <den@xxxxxxxxxxxxx>
> > ---
> > Changes in v3:
> >   - Allow partial ownership for HDMA native, which has per-channel
> >     registers.
> >   - Quiesce represented resources on remove; v2 only skipped core_off(),
> >     which could leave those channels or directions running.
> >   - Revise the commit message.
> >
> >  drivers/dma/dw-edma/dw-edma-core.c | 52 ++++++++++++++++++++++++------
> >  include/linux/dma/edma.h           |  7 ++++
> >  2 files changed, 49 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> > index c782eaa12021..d87791205837 100644
> > --- a/drivers/dma/dw-edma/dw-edma-core.c
> > +++ b/drivers/dma/dw-edma/dw-edma-core.c
[snip]
> > @@ -1105,13 +1121,25 @@ int dw_edma_probe(struct dw_edma_chip *chip)
> >
> >  	raw_spin_lock_init(&dw->lock);
> >
> > -	dw->wr_ch_cnt = min_t(u16, chip->ll_wr_cnt,
> > -			      dw_edma_core_ch_count(dw, EDMA_DIR_WRITE));
> > -	dw->wr_ch_cnt = min_t(u16, dw->wr_ch_cnt, EDMA_MAX_WR_CH);
> > +	hw_wr_ch_cnt = min_t(u16, dw_edma_core_ch_count(dw, EDMA_DIR_WRITE),
> > +			     EDMA_MAX_WR_CH);
> > +	hw_rd_ch_cnt = min_t(u16, dw_edma_core_ch_count(dw, EDMA_DIR_READ),
> > +			     EDMA_MAX_RD_CH);
> >
> > -	dw->rd_ch_cnt = min_t(u16, chip->ll_rd_cnt,
> > -			      dw_edma_core_ch_count(dw, EDMA_DIR_READ));
> > -	dw->rd_ch_cnt = min_t(u16, dw->rd_ch_cnt, EDMA_MAX_RD_CH);
> > +	if ((chip->flags & DW_EDMA_CHIP_PARTIAL) &&
> > +	    (chip->mf == EDMA_MF_EDMA_UNROLL ||
> > +	     chip->mf == EDMA_MF_HDMA_COMPAT)) {
> > +		/*
> > +		 * Direction-wide registers are shared by all channels in that
> > +		 * direction, so a direction must have a single owner.
> > +		 */
> > +		if ((chip->ll_wr_cnt && chip->ll_wr_cnt != hw_wr_ch_cnt) ||
> > +		    (chip->ll_rd_cnt && chip->ll_rd_cnt != hw_rd_ch_cnt))
> > +			return -EOPNOTSUPP;
> > +	}
> 
> move this check logic to helper function.

Sure, I will do so.

Thanks for the review,
Koichiro

> 
> Frank
> > +
> > +	dw->wr_ch_cnt = min_t(u16, chip->ll_wr_cnt, hw_wr_ch_cnt);
> > +	dw->rd_ch_cnt = min_t(u16, chip->ll_rd_cnt, hw_rd_ch_cnt);
> >
> >  	if (!dw->wr_ch_cnt && !dw->rd_ch_cnt)
> >  		return -EINVAL;
> > @@ -1128,8 +1156,10 @@ int dw_edma_probe(struct dw_edma_chip *chip)
> >  	snprintf(dw->name, sizeof(dw->name), "dw-edma-core:%s",
> >  		 dev_name(chip->dev));
> >
> > -	/* Disable eDMA, only to establish the ideal initial conditions */
> > -	dw_edma_core_off(dw);
> > +	if (!(chip->flags & DW_EDMA_CHIP_PARTIAL)) {
> > +		/* Disable eDMA only when this instance owns the controller. */
> > +		dw_edma_core_off(dw);
> > +	}
> >
> >  	/* Request IRQs */
> >  	err = dw_edma_irq_request(dw, &wr_alloc, &rd_alloc);
> > @@ -1173,8 +1203,10 @@ int dw_edma_remove(struct dw_edma_chip *chip)
> >  	if (!dw)
> >  		return -ENODEV;
> >
> > -	/* Disable eDMA */
> > -	dw_edma_core_off(dw);
> > +	if (chip->flags & DW_EDMA_CHIP_PARTIAL)
> > +		dw_edma_core_quiesce(dw);
> > +	else
> > +		dw_edma_core_off(dw);
> >
> >  	/* Free irqs */
> >  	for (i = (dw->nr_irqs - 1); i >= 0; i--)
> > diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
> > index 0ba8a1143fb2..3c730c88f0ab 100644
> > --- a/include/linux/dma/edma.h
> > +++ b/include/linux/dma/edma.h
> > @@ -55,9 +55,16 @@ enum dw_edma_map_format {
> >  /**
> >   * enum dw_edma_chip_flags - Flags specific to an eDMA chip
> >   * @DW_EDMA_CHIP_LOCAL:		eDMA is used locally by an endpoint
> > + * @DW_EDMA_CHIP_PARTIAL:	Only channels described by this instance are
> > + *				owned by this driver. Controller-wide state
> > + *				must be preserved, and layouts with shared
> > + *				direction-wide registers must only be shared at
> > + *				direction granularity. Layouts with per-channel
> > + *				registers may be shared at channel granularity.
> >   */
> >  enum dw_edma_chip_flags {
> >  	DW_EDMA_CHIP_LOCAL	= BIT(0),
> > +	DW_EDMA_CHIP_PARTIAL	= BIT(1),
> >  };
> >
> >  /**
> > --
> > 2.51.0
> >