[v2,2/5] dmaengine: sun6i-dma: Add set_addr function pointer for variable address widths

Message ID 20260621-sun60i-a733-dma-v2-2-340f205891cc@gmail.com (mailing list archive)
State New
Headers
Series dmaengine: sun6i-dma: Add support for Allwinner A733 DMA controller |

Commit Message

Yuanshen Cao June 21, 2026, 9:40 p.m. UTC
The A733 DMA controller supports higher address (up to 32G) compared to
previous generations. The existing `sun6i_dma_set_addr` function uses a
hardcoded logic for setting the high-address bits in the LLI parameters.

By moving `set_addr` into the `sun6i_dma_config` structure, we can
provide specialized implementations for different hardware. This allows
the A733 to use a version of `set_addr` that correctly handles its
specific `SRC_HIGH_ADDR_32G` and `DST_HIGH_ADDR_32G` in the `set_addr`
register later in the series.

Changes:
- Added `set_addr` function pointer to `struct sun6i_dma_config`.
- Refactored `sun6i_dma_set_addr` and introduced
  `sun6i_dma_set_addr_a31/a100` (keeping the logic for previous
  generations).
- Updated all existing configuration structs to include the new
  `set_addr` pointer.
- Removed `has_high_addr` since the logic is replaced by
  `sun6i_dma_set_addr_a100`.

Signed-off-by: Yuanshen Cao <alex.caoys@gmail.com>
---
 drivers/dma/sun6i-dma.c | 35 +++++++++++++++++++++++++++++------
 1 file changed, 29 insertions(+), 6 deletions(-)
  

Comments

sashiko-bot@kernel.org June 21, 2026, 9:51 p.m. UTC | #1
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Use-After-Free during error path cleanup in sun6i_dma_prep_slave_sg and sun6i_dma_prep_dma_cyclic.
--

commit 9bdc9020fe709f46ed0cba2bea4a233b8e23c603
Author: Yuanshen Cao <alex.caoys@gmail.com>

dmaengine: sun6i-dma: Add set_addr function pointer for variable address widths

This commit refactors the sun6i DMA driver by moving the address setting logic
into a function pointer within the configuration structure. This allows
handling of different hardware variations, specifically adding support for
the A733's 32G high-address bits while preserving legacy behavior for
older generations.

> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index ef3052c4ab363..9984b9033cbb9 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -138,6 +139,8 @@ struct sun6i_dma_config {
>  	void (*set_burst_length)(u32 *p_cfg, s8 src_burst, s8 dst_burst);
>  	void (*set_drq)(u32 *p_cfg, s8 src_drq, s8 dst_drq);
>  	void (*set_mode)(u32 *p_cfg, s8 src_mode, s8 dst_mode);
> +	void (*set_addr)(struct sun6i_dma_dev *sdev, struct sun6i_dma_lli *v_lli,
> +		dma_addr_t src, dma_addr_t dst);
>  	void (*dump_com_regs)(struct sun6i_dma_dev *sdev);

[ ... ]

[Severity: High]
This is a pre-existing issue, but I noticed a potential use-after-free
in the error cleanup paths.

Looking at sun6i_dma_prep_slave_sg():

err_lli_free:
	for (p_lli = txd->p_lli, v_lli = txd->v_lli; v_lli;
	     p_lli = v_lli->p_lli_next, v_lli = v_lli->v_lli_next)
		dma_pool_free(sdev->pool, v_lli, p_lli);

Does this loop cause a use-after-free? The dma_pool_free() call frees
v_lli, but the loop increment step then dereferences it via
v_lli = v_lli->v_lli_next.

This exact same pattern also appears in sun6i_dma_prep_dma_cyclic():

err_lli_free:
	for (p_lli = txd->p_lli, v_lli = txd->v_lli; v_lli;
	     p_lli = v_lli->p_lli_next, v_lli = v_lli->v_lli_next)
		dma_pool_free(sdev->pool, v_lli, p_lli);
  
Frank Li June 21, 2026, 10:16 p.m. UTC | #2
On Sun, Jun 21, 2026 at 09:40:55PM +0000, Yuanshen Cao wrote:
> The A733 DMA controller supports higher address (up to 32G) compared to
> previous generations. The existing `sun6i_dma_set_addr` function uses a
> hardcoded logic for setting the high-address bits in the LLI parameters.
>
> By moving `set_addr` into the `sun6i_dma_config` structure, we can
> provide specialized implementations for different hardware. This allows
> the A733 to use a version of `set_addr` that correctly handles its
> specific `SRC_HIGH_ADDR_32G` and `DST_HIGH_ADDR_32G` in the `set_addr`
> register later in the series.
>
> Changes:
> - Added `set_addr` function pointer to `struct sun6i_dma_config`.
> - Refactored `sun6i_dma_set_addr` and introduced
>   `sun6i_dma_set_addr_a31/a100` (keeping the logic for previous
>   generations).
> - Updated all existing configuration structs to include the new
>   `set_addr` pointer.
> - Removed `has_high_addr` since the logic is replaced by
>   `sun6i_dma_set_addr_a100`.
>
> Signed-off-by: Yuanshen Cao <alex.caoys@gmail.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/dma/sun6i-dma.c | 35 +++++++++++++++++++++++++++++------
>  1 file changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index ef3052c4ab36..9984b9033cbb 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -112,6 +112,7 @@
>
>  /* forward declaration */
>  struct sun6i_dma_dev;
> +struct sun6i_dma_lli;
>
>  /*
>   * Hardware channels / ports representation
> @@ -138,6 +139,8 @@ struct sun6i_dma_config {
>  	void (*set_burst_length)(u32 *p_cfg, s8 src_burst, s8 dst_burst);
>  	void (*set_drq)(u32 *p_cfg, s8 src_drq, s8 dst_drq);
>  	void (*set_mode)(u32 *p_cfg, s8 src_mode, s8 dst_mode);
> +	void (*set_addr)(struct sun6i_dma_dev *sdev, struct sun6i_dma_lli *v_lli,
> +		dma_addr_t src, dma_addr_t dst);
>  	void (*dump_com_regs)(struct sun6i_dma_dev *sdev);
>  	u32 (*read_irq_en)(struct sun6i_dma_dev *sdev, u32 irq_reg);
>  	void (*write_irq_en)(struct sun6i_dma_dev *sdev, u32 irq_reg, u32 irq_val);
> @@ -147,7 +150,6 @@ struct sun6i_dma_config {
>  	u32 dst_burst_lengths;
>  	u32 src_addr_widths;
>  	u32 dst_addr_widths;
> -	bool has_high_addr;
>  	bool has_mbus_clk;
>  };
>
> @@ -673,16 +675,30 @@ static int set_config(struct sun6i_dma_dev *sdev,
>  	return 0;
>  }
>
> -static inline void sun6i_dma_set_addr(struct sun6i_dma_dev *sdev,
> +static void sun6i_dma_set_addr_a31(struct sun6i_dma_dev *sdev,
> +				      struct sun6i_dma_lli *v_lli,
> +				      dma_addr_t src, dma_addr_t dst)
> +{
> +	v_lli->src = lower_32_bits(src);
> +	v_lli->dst = lower_32_bits(dst);
> +}
> +
> +static void sun6i_dma_set_addr_a100(struct sun6i_dma_dev *sdev,
>  				      struct sun6i_dma_lli *v_lli,
>  				      dma_addr_t src, dma_addr_t dst)
>  {
>  	v_lli->src = lower_32_bits(src);
>  	v_lli->dst = lower_32_bits(dst);
>
> -	if (sdev->cfg->has_high_addr)
> -		v_lli->para |= SRC_HIGH_ADDR(upper_32_bits(src)) |
> -			       DST_HIGH_ADDR(upper_32_bits(dst));
> +	v_lli->para |= SRC_HIGH_ADDR(upper_32_bits(src)) |
> +				DST_HIGH_ADDR(upper_32_bits(dst));
> +}
> +
> +static inline void sun6i_dma_set_addr(struct sun6i_dma_dev *sdev,
> +				      struct sun6i_dma_lli *v_lli,
> +				      dma_addr_t src, dma_addr_t dst)
> +{
> +	sdev->cfg->set_addr(sdev, v_lli, src, dst);
>  }
>
>  static struct dma_async_tx_descriptor *sun6i_dma_prep_dma_memcpy(
> @@ -1156,6 +1172,7 @@ static struct sun6i_dma_config sun6i_a31_dma_cfg = {
>  	.set_burst_length = sun6i_set_burst_length_a31,
>  	.set_drq          = sun6i_set_drq_a31,
>  	.set_mode         = sun6i_set_mode_a31,
> +	.set_addr         = sun6i_dma_set_addr_a31,
>  	.src_burst_lengths = BIT(1) | BIT(8),
>  	.dst_burst_lengths = BIT(1) | BIT(8),
>  	.src_addr_widths   = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
> @@ -1180,6 +1197,7 @@ static struct sun6i_dma_config sun8i_a23_dma_cfg = {
>  	.set_burst_length = sun6i_set_burst_length_a31,
>  	.set_drq          = sun6i_set_drq_a31,
>  	.set_mode         = sun6i_set_mode_a31,
> +	.set_addr         = sun6i_dma_set_addr_a31,
>  	.src_burst_lengths = BIT(1) | BIT(8),
>  	.dst_burst_lengths = BIT(1) | BIT(8),
>  	.src_addr_widths   = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
> @@ -1199,6 +1217,7 @@ static struct sun6i_dma_config sun8i_a83t_dma_cfg = {
>  	.set_burst_length = sun6i_set_burst_length_a31,
>  	.set_drq          = sun6i_set_drq_a31,
>  	.set_mode         = sun6i_set_mode_a31,
> +	.set_addr         = sun6i_dma_set_addr_a31,
>  	.src_burst_lengths = BIT(1) | BIT(8),
>  	.dst_burst_lengths = BIT(1) | BIT(8),
>  	.src_addr_widths   = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
> @@ -1225,6 +1244,7 @@ static struct sun6i_dma_config sun8i_h3_dma_cfg = {
>  	.set_burst_length = sun6i_set_burst_length_h3,
>  	.set_drq          = sun6i_set_drq_a31,
>  	.set_mode         = sun6i_set_mode_a31,
> +	.set_addr         = sun6i_dma_set_addr_a31,
>  	.src_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
>  	.dst_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
>  	.src_addr_widths   = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
> @@ -1247,6 +1267,7 @@ static struct sun6i_dma_config sun50i_a64_dma_cfg = {
>  	.set_burst_length = sun6i_set_burst_length_h3,
>  	.set_drq          = sun6i_set_drq_a31,
>  	.set_mode         = sun6i_set_mode_a31,
> +	.set_addr         = sun6i_dma_set_addr_a31,
>  	.src_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
>  	.dst_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
>  	.src_addr_widths   = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
> @@ -1269,6 +1290,7 @@ static struct sun6i_dma_config sun50i_a100_dma_cfg = {
>  	.set_burst_length = sun6i_set_burst_length_h3,
>  	.set_drq          = sun6i_set_drq_h6,
>  	.set_mode         = sun6i_set_mode_h6,
> +	.set_addr         = sun6i_dma_set_addr_a100,
>  	.src_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
>  	.dst_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
>  	.src_addr_widths   = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
> @@ -1279,7 +1301,6 @@ static struct sun6i_dma_config sun50i_a100_dma_cfg = {
>  			     BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
>  			     BIT(DMA_SLAVE_BUSWIDTH_4_BYTES) |
>  			     BIT(DMA_SLAVE_BUSWIDTH_8_BYTES),
> -	.has_high_addr = true,
>  	.has_mbus_clk = true,
>  	SUN6I_DMA_IRQ_A31_COMMON_OPS
>  };
> @@ -1293,6 +1314,7 @@ static struct sun6i_dma_config sun50i_h6_dma_cfg = {
>  	.set_burst_length = sun6i_set_burst_length_h3,
>  	.set_drq          = sun6i_set_drq_h6,
>  	.set_mode         = sun6i_set_mode_h6,
> +	.set_addr         = sun6i_dma_set_addr_a31,
>  	.src_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
>  	.dst_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
>  	.src_addr_widths   = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
> @@ -1320,6 +1342,7 @@ static struct sun6i_dma_config sun8i_v3s_dma_cfg = {
>  	.set_burst_length = sun6i_set_burst_length_a31,
>  	.set_drq          = sun6i_set_drq_a31,
>  	.set_mode         = sun6i_set_mode_a31,
> +	.set_addr         = sun6i_dma_set_addr_a31,
>  	.src_burst_lengths = BIT(1) | BIT(8),
>  	.dst_burst_lengths = BIT(1) | BIT(8),
>  	.src_addr_widths   = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
>
> --
> 2.54.0
>
  

Patch

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index ef3052c4ab36..9984b9033cbb 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -112,6 +112,7 @@ 
 
 /* forward declaration */
 struct sun6i_dma_dev;
+struct sun6i_dma_lli;
 
 /*
  * Hardware channels / ports representation
@@ -138,6 +139,8 @@  struct sun6i_dma_config {
 	void (*set_burst_length)(u32 *p_cfg, s8 src_burst, s8 dst_burst);
 	void (*set_drq)(u32 *p_cfg, s8 src_drq, s8 dst_drq);
 	void (*set_mode)(u32 *p_cfg, s8 src_mode, s8 dst_mode);
+	void (*set_addr)(struct sun6i_dma_dev *sdev, struct sun6i_dma_lli *v_lli,
+		dma_addr_t src, dma_addr_t dst);
 	void (*dump_com_regs)(struct sun6i_dma_dev *sdev);
 	u32 (*read_irq_en)(struct sun6i_dma_dev *sdev, u32 irq_reg);
 	void (*write_irq_en)(struct sun6i_dma_dev *sdev, u32 irq_reg, u32 irq_val);
@@ -147,7 +150,6 @@  struct sun6i_dma_config {
 	u32 dst_burst_lengths;
 	u32 src_addr_widths;
 	u32 dst_addr_widths;
-	bool has_high_addr;
 	bool has_mbus_clk;
 };
 
@@ -673,16 +675,30 @@  static int set_config(struct sun6i_dma_dev *sdev,
 	return 0;
 }
 
-static inline void sun6i_dma_set_addr(struct sun6i_dma_dev *sdev,
+static void sun6i_dma_set_addr_a31(struct sun6i_dma_dev *sdev,
+				      struct sun6i_dma_lli *v_lli,
+				      dma_addr_t src, dma_addr_t dst)
+{
+	v_lli->src = lower_32_bits(src);
+	v_lli->dst = lower_32_bits(dst);
+}
+
+static void sun6i_dma_set_addr_a100(struct sun6i_dma_dev *sdev,
 				      struct sun6i_dma_lli *v_lli,
 				      dma_addr_t src, dma_addr_t dst)
 {
 	v_lli->src = lower_32_bits(src);
 	v_lli->dst = lower_32_bits(dst);
 
-	if (sdev->cfg->has_high_addr)
-		v_lli->para |= SRC_HIGH_ADDR(upper_32_bits(src)) |
-			       DST_HIGH_ADDR(upper_32_bits(dst));
+	v_lli->para |= SRC_HIGH_ADDR(upper_32_bits(src)) |
+				DST_HIGH_ADDR(upper_32_bits(dst));
+}
+
+static inline void sun6i_dma_set_addr(struct sun6i_dma_dev *sdev,
+				      struct sun6i_dma_lli *v_lli,
+				      dma_addr_t src, dma_addr_t dst)
+{
+	sdev->cfg->set_addr(sdev, v_lli, src, dst);
 }
 
 static struct dma_async_tx_descriptor *sun6i_dma_prep_dma_memcpy(
@@ -1156,6 +1172,7 @@  static struct sun6i_dma_config sun6i_a31_dma_cfg = {
 	.set_burst_length = sun6i_set_burst_length_a31,
 	.set_drq          = sun6i_set_drq_a31,
 	.set_mode         = sun6i_set_mode_a31,
+	.set_addr         = sun6i_dma_set_addr_a31,
 	.src_burst_lengths = BIT(1) | BIT(8),
 	.dst_burst_lengths = BIT(1) | BIT(8),
 	.src_addr_widths   = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
@@ -1180,6 +1197,7 @@  static struct sun6i_dma_config sun8i_a23_dma_cfg = {
 	.set_burst_length = sun6i_set_burst_length_a31,
 	.set_drq          = sun6i_set_drq_a31,
 	.set_mode         = sun6i_set_mode_a31,
+	.set_addr         = sun6i_dma_set_addr_a31,
 	.src_burst_lengths = BIT(1) | BIT(8),
 	.dst_burst_lengths = BIT(1) | BIT(8),
 	.src_addr_widths   = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
@@ -1199,6 +1217,7 @@  static struct sun6i_dma_config sun8i_a83t_dma_cfg = {
 	.set_burst_length = sun6i_set_burst_length_a31,
 	.set_drq          = sun6i_set_drq_a31,
 	.set_mode         = sun6i_set_mode_a31,
+	.set_addr         = sun6i_dma_set_addr_a31,
 	.src_burst_lengths = BIT(1) | BIT(8),
 	.dst_burst_lengths = BIT(1) | BIT(8),
 	.src_addr_widths   = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
@@ -1225,6 +1244,7 @@  static struct sun6i_dma_config sun8i_h3_dma_cfg = {
 	.set_burst_length = sun6i_set_burst_length_h3,
 	.set_drq          = sun6i_set_drq_a31,
 	.set_mode         = sun6i_set_mode_a31,
+	.set_addr         = sun6i_dma_set_addr_a31,
 	.src_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
 	.dst_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
 	.src_addr_widths   = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
@@ -1247,6 +1267,7 @@  static struct sun6i_dma_config sun50i_a64_dma_cfg = {
 	.set_burst_length = sun6i_set_burst_length_h3,
 	.set_drq          = sun6i_set_drq_a31,
 	.set_mode         = sun6i_set_mode_a31,
+	.set_addr         = sun6i_dma_set_addr_a31,
 	.src_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
 	.dst_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
 	.src_addr_widths   = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
@@ -1269,6 +1290,7 @@  static struct sun6i_dma_config sun50i_a100_dma_cfg = {
 	.set_burst_length = sun6i_set_burst_length_h3,
 	.set_drq          = sun6i_set_drq_h6,
 	.set_mode         = sun6i_set_mode_h6,
+	.set_addr         = sun6i_dma_set_addr_a100,
 	.src_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
 	.dst_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
 	.src_addr_widths   = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
@@ -1279,7 +1301,6 @@  static struct sun6i_dma_config sun50i_a100_dma_cfg = {
 			     BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
 			     BIT(DMA_SLAVE_BUSWIDTH_4_BYTES) |
 			     BIT(DMA_SLAVE_BUSWIDTH_8_BYTES),
-	.has_high_addr = true,
 	.has_mbus_clk = true,
 	SUN6I_DMA_IRQ_A31_COMMON_OPS
 };
@@ -1293,6 +1314,7 @@  static struct sun6i_dma_config sun50i_h6_dma_cfg = {
 	.set_burst_length = sun6i_set_burst_length_h3,
 	.set_drq          = sun6i_set_drq_h6,
 	.set_mode         = sun6i_set_mode_h6,
+	.set_addr         = sun6i_dma_set_addr_a31,
 	.src_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
 	.dst_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
 	.src_addr_widths   = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
@@ -1320,6 +1342,7 @@  static struct sun6i_dma_config sun8i_v3s_dma_cfg = {
 	.set_burst_length = sun6i_set_burst_length_a31,
 	.set_drq          = sun6i_set_drq_a31,
 	.set_mode         = sun6i_set_mode_a31,
+	.set_addr         = sun6i_dma_set_addr_a31,
 	.src_burst_lengths = BIT(1) | BIT(8),
 	.dst_burst_lengths = BIT(1) | BIT(8),
 	.src_addr_widths   = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |