Re: [PATCH v2 03/12] dmaengine: dw-edma: Add partial channel ownership mode

[Frank Li]

On Mon, May 25, 2026 at 03:24:11PM +0900, Koichiro Den wrote:
> Some endpoint DMA frontends expose only a subset of a controller that is
> also initialized by the endpoint-side OS. Add a partial ownership flag
> so dw-edma does not reset controller-wide state in probe() or remove().
>
> Keep the mode conservative. Do not enable interrupt-emulation doorbells,
> and reject partial instances for map formats that this driver cannot safely
> share. For EDMA_MF_EDMA_UNROLL and EDMA_MF_HDMA_COMPAT, require ownership
> of all channels in each exposed direction. The driver updates registers
> shared by all channels in a direction, such as interrupt masks and
> linked-list error enables, so two independent OS instances cannot safely
> split one direction without a shared locking protocol, which is
> unrealistic.
>
> The frontend must still quiesce delegated channels before removing a
> partial instance. The flag only keeps probe() and remove() from
> resetting controller-wide state that may belong to a peer OS instance.
>
> Signed-off-by: Koichiro Den <den@xxxxxxxxxxxxx>
> ---
> Changes in v2:
>   - Reject partial ownership for unsupported map formats up front,
>     keep direction-granularity validation limited to supported formats.
>   - Revise the commit message accordingly.
>
>  drivers/dma/dw-edma/dw-edma-core.c | 47 +++++++++++++++++++++++-------
>  include/linux/dma/edma.h           |  6 ++++
>  2 files changed, 43 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index a70e0640d082..fcef9a27b6ce 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -794,6 +794,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.
> @@ -1135,6 +1138,8 @@ 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;
> @@ -1146,6 +1151,16 @@ 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:
> +			break;
> +		default:
> +			return -EOPNOTSUPP;
> +		}
> +	}
> +
>  	dw = devm_kzalloc(dev, sizeof(*dw), GFP_KERNEL);
>  	if (!dw)
>  		return -ENOMEM;
> @@ -1159,13 +1174,23 @@ 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);
> +
> +	if (chip->flags & DW_EDMA_CHIP_PARTIAL) {
> +		/*
> +		 * 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;
> +	}
>
> -	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;
> @@ -1182,8 +1207,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);
> @@ -1227,8 +1254,8 @@ 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_off(dw);

Can we simplely prevent dma driver remove? If attached to pci host,
remove edma driver always be risk because RC may write data at any time.

And it doesn't make sense to remove EP and EDMA driver after linkup.

Frank

>
>  	/* 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 2bf2298711e1..84f0e728d300 100644
> --- a/include/linux/dma/edma.h
> +++ b/include/linux/dma/edma.h
> @@ -55,9 +55,15 @@ 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.
>   */
>  enum dw_edma_chip_flags {
>  	DW_EDMA_CHIP_LOCAL	= BIT(0),
> +	DW_EDMA_CHIP_PARTIAL	= BIT(1),
>  };
>
>  /**
> --
> 2.51.0
>