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

[Frank Li]

On Fri, Jul 10, 2026 at 05:15:09PM +0900, Koichiro Den wrote:
> A DesignWare eDMA instance may represent only a subset of channels 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 and uses
> the limited quiesce path in probe() and 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.
>
> Partial instances also skip interrupt-emulation doorbell allocation: the
> emulated doorbell is a controller-level resource, and a partial owner
> must not claim it on behalf of the whole block.
>
> 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 owned
> at channel granularity.
>
> Signed-off-by: Koichiro Den <den@xxxxxxxxxxxxx>
> ---
> Changes in v4:
>   - Fix and revise commit message. (Frank)
>   - Move partial-ownership validation into dw_edma_check_partial().
>     (Frank)
>   - While at it, add a small source comment that explains why local
>     variables hw_{wr,rd}_ch_cnt are introduced separately.
>   - Quiesce represented resources during partial probe as well as
>     remove, draining stale channel state from a previous owner without
>     resetting controller-wide state.
>
>  drivers/dma/dw-edma/dw-edma-core.c | 75 ++++++++++++++++++++++++++----
>  include/linux/dma/edma.h           |  7 +++
>  2 files changed, 72 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index fb17074917df..0d38de4480a0 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -831,6 +831,9 @@ static int dw_edma_emul_irq_alloc(struct dw_edma *dw)
>  	chip->db_irq = 0;
>  	chip->db_offset = ~0;
>
> +	if (chip->flags & DW_EDMA_CHIP_PARTIAL)
> +		return 0;
> +
>  	/*
>  	 * Only meaningful when the core provides the deassert sequence
>  	 * for interrupt emulation.
> @@ -1188,10 +1191,33 @@ static int dw_edma_irq_request(struct dw_edma *dw,
>  	return err;
>  }
>
> +static int dw_edma_check_partial(struct dw_edma_chip *chip,
> +				 u16 hw_wr_ch_cnt, u16 hw_rd_ch_cnt)
> +{
> +	if (!(chip->flags & DW_EDMA_CHIP_PARTIAL))
> +		return 0;
> +
> +	if (chip->mf != EDMA_MF_EDMA_UNROLL &&
> +	    chip->mf != EDMA_MF_HDMA_COMPAT)
> +		return 0;
> +
> +	/*
> +	 * 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;
> +
> +	return 0;
> +}
> +
>  int dw_edma_probe(struct dw_edma_chip *chip)
>  {
>  	struct device *dev;
>  	struct dw_edma *dw;
> +	u16 hw_wr_ch_cnt;
> +	u16 hw_rd_ch_cnt;
>  	u32 wr_alloc = 0;
>  	u32 rd_alloc = 0;
>  	int i, err;
> @@ -1203,6 +1229,17 @@ int dw_edma_probe(struct dw_edma_chip *chip)
>  	if (!dev || !chip->ops)
>  		return -EINVAL;
>
> +	if (chip->flags & DW_EDMA_CHIP_PARTIAL) {
> +		switch (chip->mf) {
> +		case EDMA_MF_EDMA_UNROLL:
> +		case EDMA_MF_HDMA_COMPAT:
> +		case EDMA_MF_HDMA_NATIVE:
> +			break;
> +		default:
> +			return -EOPNOTSUPP;
> +		}
> +	}
> +
>  	dw = devm_kzalloc(dev, sizeof(*dw), GFP_KERNEL);
>  	if (!dw)
>  		return -ENOMEM;
> @@ -1216,13 +1253,21 @@ 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);
> +	/*
> +	 * chip->ll_*_cnt describes the channels exposed by this instance. Keep
> +	 * the usable hardware counts separate for partial ownership checks.
> +	 */
> +	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);

If need respin patch,  now it is safe to use min()

Reviewed-by: Frank Li <Frank.Li@xxxxxxx>

> +
> +	err = dw_edma_check_partial(chip, hw_wr_ch_cnt, hw_rd_ch_cnt);
> +	if (err)
> +		return err;
>
> -	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);
> +	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;
> @@ -1239,8 +1284,16 @@ 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) {
> +		/*
> +		 * Do not reset the shared controller, but drain stale state
> +		 * from resources represented by this instance.
> +		 */
> +		dw_edma_core_quiesce(dw);
> +	} else {
> +		/* Disable eDMA only when this instance owns the controller. */
> +		dw_edma_core_off(dw);
> +	}
>
>  	/*
>  	 * Deferred IRQ works are queued from the hard IRQ handlers, so the
> @@ -1296,8 +1349,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 1007122d4123..3c33d12d1cdb 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
>