[PATCH 4/4] dmaengine: stm32-dma3: introduce ddata2dev helper

Amelie Delaunay posted 4 patches 3 months, 1 week ago
There is a newer version of this series
[PATCH 4/4] dmaengine: stm32-dma3: introduce ddata2dev helper
Posted by Amelie Delaunay 3 months, 1 week ago
ddata2dev helper returns the device pointer from struct dma_device stored
in stm32_dma3_ddata structure.
Device pointer from struct dma_device has been initialized with &pdev->dev,
so the ddata2dev helper returns &pdev->dev.

Signed-off-by: Amelie Delaunay <amelie.delaunay@foss.st.com>
---
 drivers/dma/stm32/stm32-dma3.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/dma/stm32/stm32-dma3.c b/drivers/dma/stm32/stm32-dma3.c
index 29ea510fa539..9f49ef8e2972 100644
--- a/drivers/dma/stm32/stm32-dma3.c
+++ b/drivers/dma/stm32/stm32-dma3.c
@@ -333,6 +333,11 @@ static struct device *chan2dev(struct stm32_dma3_chan *chan)
 	return &chan->vchan.chan.dev->device;
 }
 
+static struct device *ddata2dev(struct stm32_dma3_ddata *ddata)
+{
+	return ddata->dma_dev.dev;
+}
+
 static void stm32_dma3_chan_dump_reg(struct stm32_dma3_chan *chan)
 {
 	struct stm32_dma3_ddata *ddata = to_stm32_dma3_ddata(chan);
@@ -392,6 +397,7 @@ static void stm32_dma3_chan_dump_hwdesc(struct stm32_dma3_chan *chan,
 	} else {
 		dev_dbg(chan2dev(chan), "X\n");
 	}
+
 }
 
 static struct stm32_dma3_swdesc *stm32_dma3_chan_desc_alloc(struct stm32_dma3_chan *chan, u32 count)
@@ -1110,7 +1116,7 @@ static int stm32_dma3_alloc_chan_resources(struct dma_chan *c)
 	struct stm32_dma3_ddata *ddata = to_stm32_dma3_ddata(chan);
 	int ret;
 
-	ret = pm_runtime_resume_and_get(ddata->dma_dev.dev);
+	ret = pm_runtime_resume_and_get(ddata2dev(ddata));
 	if (ret < 0)
 		return ret;
 
@@ -1144,7 +1150,7 @@ static int stm32_dma3_alloc_chan_resources(struct dma_chan *c)
 	chan->lli_pool = NULL;
 
 err_put_sync:
-	pm_runtime_put_sync(ddata->dma_dev.dev);
+	pm_runtime_put_sync(ddata2dev(ddata));
 
 	return ret;
 }
@@ -1170,7 +1176,7 @@ static void stm32_dma3_free_chan_resources(struct dma_chan *c)
 	if (chan->semaphore_mode)
 		stm32_dma3_put_chan_sem(chan);
 
-	pm_runtime_put_sync(ddata->dma_dev.dev);
+	pm_runtime_put_sync(ddata2dev(ddata));
 
 	/* Reset configuration */
 	memset(&chan->dt_config, 0, sizeof(chan->dt_config));
@@ -1610,11 +1616,11 @@ static bool stm32_dma3_filter_fn(struct dma_chan *c, void *fn_param)
 		if (!(mask & BIT(chan->id)))
 			return false;
 
-	ret = pm_runtime_resume_and_get(ddata->dma_dev.dev);
+	ret = pm_runtime_resume_and_get(ddata2dev(ddata));
 	if (ret < 0)
 		return false;
 	semcr = readl_relaxed(ddata->base + STM32_DMA3_CSEMCR(chan->id));
-	pm_runtime_put_sync(ddata->dma_dev.dev);
+	pm_runtime_put_sync(ddata2dev(ddata));
 
 	/* Check if chan is free */
 	if (semcr & CSEMCR_SEM_MUTEX)
@@ -1636,7 +1642,7 @@ static struct dma_chan *stm32_dma3_of_xlate(struct of_phandle_args *dma_spec, st
 	struct dma_chan *c;
 
 	if (dma_spec->args_count < 3) {
-		dev_err(ddata->dma_dev.dev, "Invalid args count\n");
+		dev_err(ddata2dev(ddata), "Invalid args count\n");
 		return NULL;
 	}
 
@@ -1645,14 +1651,14 @@ static struct dma_chan *stm32_dma3_of_xlate(struct of_phandle_args *dma_spec, st
 	conf.tr_conf = dma_spec->args[2];
 
 	if (conf.req_line >= ddata->dma_requests) {
-		dev_err(ddata->dma_dev.dev, "Invalid request line\n");
+		dev_err(ddata2dev(ddata), "Invalid request line\n");
 		return NULL;
 	}
 
 	/* Request dma channel among the generic dma controller list */
 	c = dma_request_channel(mask, stm32_dma3_filter_fn, &conf);
 	if (!c) {
-		dev_err(ddata->dma_dev.dev, "No suitable channel found\n");
+		dev_err(ddata2dev(ddata), "No suitable channel found\n");
 		return NULL;
 	}
 
@@ -1665,6 +1671,7 @@ static struct dma_chan *stm32_dma3_of_xlate(struct of_phandle_args *dma_spec, st
 
 static u32 stm32_dma3_check_rif(struct stm32_dma3_ddata *ddata)
 {
+	struct device *dev = ddata2dev(ddata);
 	u32 chan_reserved, mask = 0, i, ccidcfgr, invalid_cid = 0;
 
 	/* Reserve Secure channels */
@@ -1676,7 +1683,7 @@ static u32 stm32_dma3_check_rif(struct stm32_dma3_ddata *ddata)
 	 * In case CID filtering is not configured, dma-channel-mask property can be used to
 	 * specify available DMA channels to the kernel.
 	 */
-	of_property_read_u32(ddata->dma_dev.dev->of_node, "dma-channel-mask", &mask);
+	of_property_read_u32(dev->of_node, "dma-channel-mask", &mask);
 
 	/* Reserve !CID-filtered not in dma-channel-mask, static CID != CID1, CID1 not allowed */
 	for (i = 0; i < ddata->dma_channels; i++) {
@@ -1696,7 +1703,7 @@ static u32 stm32_dma3_check_rif(struct stm32_dma3_ddata *ddata)
 				ddata->chans[i].semaphore_mode = true;
 			}
 		}
-		dev_dbg(ddata->dma_dev.dev, "chan%d: %s mode, %s\n", i,
+		dev_dbg(dev, "chan%d: %s mode, %s\n", i,
 			!(ccidcfgr & CCIDCFGR_CFEN) ? "!CID-filtered" :
 			ddata->chans[i].semaphore_mode ? "Semaphore" : "Static CID",
 			(chan_reserved & BIT(i)) ? "denied" :
@@ -1704,7 +1711,7 @@ static u32 stm32_dma3_check_rif(struct stm32_dma3_ddata *ddata)
 	}
 
 	if (invalid_cid)
-		dev_warn(ddata->dma_dev.dev, "chan%*pbl have invalid CID configuration\n",
+		dev_warn(dev, "chan%*pbl have invalid CID configuration\n",
 			 ddata->dma_channels, &invalid_cid);
 
 	return chan_reserved;

-- 
2.43.0
Re: [PATCH 4/4] dmaengine: stm32-dma3: introduce ddata2dev helper
Posted by Eugen Hristev 2 months, 2 weeks ago

On 11/3/25 12:15, Amelie Delaunay wrote:
> ddata2dev helper returns the device pointer from struct dma_device stored
> in stm32_dma3_ddata structure.
> Device pointer from struct dma_device has been initialized with &pdev->dev,
> so the ddata2dev helper returns &pdev->dev.
> 
> Signed-off-by: Amelie Delaunay <amelie.delaunay@foss.st.com>
> ---
>  drivers/dma/stm32/stm32-dma3.c | 29 ++++++++++++++++++-----------
>  1 file changed, 18 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/dma/stm32/stm32-dma3.c b/drivers/dma/stm32/stm32-dma3.c
> index 29ea510fa539..9f49ef8e2972 100644
> --- a/drivers/dma/stm32/stm32-dma3.c
> +++ b/drivers/dma/stm32/stm32-dma3.c
> @@ -333,6 +333,11 @@ static struct device *chan2dev(struct stm32_dma3_chan *chan)
>  	return &chan->vchan.chan.dev->device;
>  }
>  
> +static struct device *ddata2dev(struct stm32_dma3_ddata *ddata)
> +{
> +	return ddata->dma_dev.dev;
> +}

Not really sure how much this helper actually simplifies the code. To me
it appears as if there is no improvement, but this is a personal
preference.> +
>  static void stm32_dma3_chan_dump_reg(struct stm32_dma3_chan *chan)
>  {
>  	struct stm32_dma3_ddata *ddata = to_stm32_dma3_ddata(chan);
> @@ -392,6 +397,7 @@ static void stm32_dma3_chan_dump_hwdesc(struct stm32_dma3_chan *chan,
>  	} else {
>  		dev_dbg(chan2dev(chan), "X\n");
>  	}
> +
This newline here appears to be unintended >  }
>  
>  static struct stm32_dma3_swdesc *stm32_dma3_chan_desc_alloc(struct stm32_dma3_chan *chan, u32 count)
> @@ -1110,7 +1116,7 @@ static int stm32_dma3_alloc_chan_resources(struct dma_chan *c)
>  	struct stm32_dma3_ddata *ddata = to_stm32_dma3_ddata(chan);
>  	int ret;
>  
> -	ret = pm_runtime_resume_and_get(ddata->dma_dev.dev);
> +	ret = pm_runtime_resume_and_get(ddata2dev(ddata));
>  	if (ret < 0)
>  		return ret;
>  
> @@ -1144,7 +1150,7 @@ static int stm32_dma3_alloc_chan_resources(struct dma_chan *c)
>  	chan->lli_pool = NULL;
>  
>  err_put_sync:
> -	pm_runtime_put_sync(ddata->dma_dev.dev);
> +	pm_runtime_put_sync(ddata2dev(ddata));
>  
>  	return ret;
>  }
> @@ -1170,7 +1176,7 @@ static void stm32_dma3_free_chan_resources(struct dma_chan *c)
>  	if (chan->semaphore_mode)
>  		stm32_dma3_put_chan_sem(chan);
>  
> -	pm_runtime_put_sync(ddata->dma_dev.dev);
> +	pm_runtime_put_sync(ddata2dev(ddata));
>  
>  	/* Reset configuration */
>  	memset(&chan->dt_config, 0, sizeof(chan->dt_config));
> @@ -1610,11 +1616,11 @@ static bool stm32_dma3_filter_fn(struct dma_chan *c, void *fn_param)
>  		if (!(mask & BIT(chan->id)))
>  			return false;
>  
> -	ret = pm_runtime_resume_and_get(ddata->dma_dev.dev);
> +	ret = pm_runtime_resume_and_get(ddata2dev(ddata));
>  	if (ret < 0)
>  		return false;
>  	semcr = readl_relaxed(ddata->base + STM32_DMA3_CSEMCR(chan->id));
> -	pm_runtime_put_sync(ddata->dma_dev.dev);
> +	pm_runtime_put_sync(ddata2dev(ddata));
>  
>  	/* Check if chan is free */
>  	if (semcr & CSEMCR_SEM_MUTEX)
> @@ -1636,7 +1642,7 @@ static struct dma_chan *stm32_dma3_of_xlate(struct of_phandle_args *dma_spec, st
>  	struct dma_chan *c;
>  
>  	if (dma_spec->args_count < 3) {
> -		dev_err(ddata->dma_dev.dev, "Invalid args count\n");
> +		dev_err(ddata2dev(ddata), "Invalid args count\n");
>  		return NULL;
>  	}
>  
> @@ -1645,14 +1651,14 @@ static struct dma_chan *stm32_dma3_of_xlate(struct of_phandle_args *dma_spec, st
>  	conf.tr_conf = dma_spec->args[2];
>  
>  	if (conf.req_line >= ddata->dma_requests) {
> -		dev_err(ddata->dma_dev.dev, "Invalid request line\n");
> +		dev_err(ddata2dev(ddata), "Invalid request line\n");
>  		return NULL;
>  	}
>  
>  	/* Request dma channel among the generic dma controller list */
>  	c = dma_request_channel(mask, stm32_dma3_filter_fn, &conf);
>  	if (!c) {
> -		dev_err(ddata->dma_dev.dev, "No suitable channel found\n");
> +		dev_err(ddata2dev(ddata), "No suitable channel found\n");
>  		return NULL;
>  	}
>  
> @@ -1665,6 +1671,7 @@ static struct dma_chan *stm32_dma3_of_xlate(struct of_phandle_args *dma_spec, st
>  
>  static u32 stm32_dma3_check_rif(struct stm32_dma3_ddata *ddata)
>  {
> +	struct device *dev = ddata2dev(ddata);
>  	u32 chan_reserved, mask = 0, i, ccidcfgr, invalid_cid = 0;
>  
>  	/* Reserve Secure channels */
> @@ -1676,7 +1683,7 @@ static u32 stm32_dma3_check_rif(struct stm32_dma3_ddata *ddata)
>  	 * In case CID filtering is not configured, dma-channel-mask property can be used to
>  	 * specify available DMA channels to the kernel.
>  	 */
> -	of_property_read_u32(ddata->dma_dev.dev->of_node, "dma-channel-mask", &mask);
> +	of_property_read_u32(dev->of_node, "dma-channel-mask", &mask);
>  
>  	/* Reserve !CID-filtered not in dma-channel-mask, static CID != CID1, CID1 not allowed */
>  	for (i = 0; i < ddata->dma_channels; i++) {
> @@ -1696,7 +1703,7 @@ static u32 stm32_dma3_check_rif(struct stm32_dma3_ddata *ddata)
>  				ddata->chans[i].semaphore_mode = true;
>  			}
>  		}
> -		dev_dbg(ddata->dma_dev.dev, "chan%d: %s mode, %s\n", i,
> +		dev_dbg(dev, "chan%d: %s mode, %s\n", i,
>  			!(ccidcfgr & CCIDCFGR_CFEN) ? "!CID-filtered" :
>  			ddata->chans[i].semaphore_mode ? "Semaphore" : "Static CID",
>  			(chan_reserved & BIT(i)) ? "denied" :
> @@ -1704,7 +1711,7 @@ static u32 stm32_dma3_check_rif(struct stm32_dma3_ddata *ddata)
>  	}
>  
>  	if (invalid_cid)
> -		dev_warn(ddata->dma_dev.dev, "chan%*pbl have invalid CID configuration\n",
> +		dev_warn(dev, "chan%*pbl have invalid CID configuration\n",
>  			 ddata->dma_channels, &invalid_cid);
>  
>  	return chan_reserved;
>
Re: [PATCH 4/4] dmaengine: stm32-dma3: introduce ddata2dev helper
Posted by Amelie Delaunay 2 months, 2 weeks ago

On 11/21/25 10:25, Eugen Hristev wrote:
> 
> 
> On 11/3/25 12:15, Amelie Delaunay wrote:
>> ddata2dev helper returns the device pointer from struct dma_device stored
>> in stm32_dma3_ddata structure.
>> Device pointer from struct dma_device has been initialized with &pdev->dev,
>> so the ddata2dev helper returns &pdev->dev.
>>
>> Signed-off-by: Amelie Delaunay <amelie.delaunay@foss.st.com>
>> ---
>>   drivers/dma/stm32/stm32-dma3.c | 29 ++++++++++++++++++-----------
>>   1 file changed, 18 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/dma/stm32/stm32-dma3.c b/drivers/dma/stm32/stm32-dma3.c
>> index 29ea510fa539..9f49ef8e2972 100644
>> --- a/drivers/dma/stm32/stm32-dma3.c
>> +++ b/drivers/dma/stm32/stm32-dma3.c
>> @@ -333,6 +333,11 @@ static struct device *chan2dev(struct stm32_dma3_chan *chan)
>>   	return &chan->vchan.chan.dev->device;
>>   }
>>   
>> +static struct device *ddata2dev(struct stm32_dma3_ddata *ddata)
>> +{
>> +	return ddata->dma_dev.dev;
>> +}
> 
> Not really sure how much this helper actually simplifies the code. To me
> it appears as if there is no improvement, but this is a personal
> preference.> +

Hi Eugen, thanks for your review.

Saving two characters cannot be considered an improvement, but the 
purpose of this helper is to 'standardize' device pointer retrieval, 
similar to the chan2dev() helper above.

>>   static void stm32_dma3_chan_dump_reg(struct stm32_dma3_chan *chan)
>>   {
>>   	struct stm32_dma3_ddata *ddata = to_stm32_dma3_ddata(chan);
>> @@ -392,6 +397,7 @@ static void stm32_dma3_chan_dump_hwdesc(struct stm32_dma3_chan *chan,
>>   	} else {
>>   		dev_dbg(chan2dev(chan), "X\n");
>>   	}
>> +
> This newline here appears to be unintended >  }

Indeed! I'll fix it in v2 Thanks for catching that.

Regards,
Amelie

>>   
>>   static struct stm32_dma3_swdesc *stm32_dma3_chan_desc_alloc(struct stm32_dma3_chan *chan, u32 count)
>> @@ -1110,7 +1116,7 @@ static int stm32_dma3_alloc_chan_resources(struct dma_chan *c)
>>   	struct stm32_dma3_ddata *ddata = to_stm32_dma3_ddata(chan);
>>   	int ret;
>>   
>> -	ret = pm_runtime_resume_and_get(ddata->dma_dev.dev);
>> +	ret = pm_runtime_resume_and_get(ddata2dev(ddata));
>>   	if (ret < 0)
>>   		return ret;
>>   
>> @@ -1144,7 +1150,7 @@ static int stm32_dma3_alloc_chan_resources(struct dma_chan *c)
>>   	chan->lli_pool = NULL;
>>   
>>   err_put_sync:
>> -	pm_runtime_put_sync(ddata->dma_dev.dev);
>> +	pm_runtime_put_sync(ddata2dev(ddata));
>>   
>>   	return ret;
>>   }
>> @@ -1170,7 +1176,7 @@ static void stm32_dma3_free_chan_resources(struct dma_chan *c)
>>   	if (chan->semaphore_mode)
>>   		stm32_dma3_put_chan_sem(chan);
>>   
>> -	pm_runtime_put_sync(ddata->dma_dev.dev);
>> +	pm_runtime_put_sync(ddata2dev(ddata));
>>   
>>   	/* Reset configuration */
>>   	memset(&chan->dt_config, 0, sizeof(chan->dt_config));
>> @@ -1610,11 +1616,11 @@ static bool stm32_dma3_filter_fn(struct dma_chan *c, void *fn_param)
>>   		if (!(mask & BIT(chan->id)))
>>   			return false;
>>   
>> -	ret = pm_runtime_resume_and_get(ddata->dma_dev.dev);
>> +	ret = pm_runtime_resume_and_get(ddata2dev(ddata));
>>   	if (ret < 0)
>>   		return false;
>>   	semcr = readl_relaxed(ddata->base + STM32_DMA3_CSEMCR(chan->id));
>> -	pm_runtime_put_sync(ddata->dma_dev.dev);
>> +	pm_runtime_put_sync(ddata2dev(ddata));
>>   
>>   	/* Check if chan is free */
>>   	if (semcr & CSEMCR_SEM_MUTEX)
>> @@ -1636,7 +1642,7 @@ static struct dma_chan *stm32_dma3_of_xlate(struct of_phandle_args *dma_spec, st
>>   	struct dma_chan *c;
>>   
>>   	if (dma_spec->args_count < 3) {
>> -		dev_err(ddata->dma_dev.dev, "Invalid args count\n");
>> +		dev_err(ddata2dev(ddata), "Invalid args count\n");
>>   		return NULL;
>>   	}
>>   
>> @@ -1645,14 +1651,14 @@ static struct dma_chan *stm32_dma3_of_xlate(struct of_phandle_args *dma_spec, st
>>   	conf.tr_conf = dma_spec->args[2];
>>   
>>   	if (conf.req_line >= ddata->dma_requests) {
>> -		dev_err(ddata->dma_dev.dev, "Invalid request line\n");
>> +		dev_err(ddata2dev(ddata), "Invalid request line\n");
>>   		return NULL;
>>   	}
>>   
>>   	/* Request dma channel among the generic dma controller list */
>>   	c = dma_request_channel(mask, stm32_dma3_filter_fn, &conf);
>>   	if (!c) {
>> -		dev_err(ddata->dma_dev.dev, "No suitable channel found\n");
>> +		dev_err(ddata2dev(ddata), "No suitable channel found\n");
>>   		return NULL;
>>   	}
>>   
>> @@ -1665,6 +1671,7 @@ static struct dma_chan *stm32_dma3_of_xlate(struct of_phandle_args *dma_spec, st
>>   
>>   static u32 stm32_dma3_check_rif(struct stm32_dma3_ddata *ddata)
>>   {
>> +	struct device *dev = ddata2dev(ddata);
>>   	u32 chan_reserved, mask = 0, i, ccidcfgr, invalid_cid = 0;
>>   
>>   	/* Reserve Secure channels */
>> @@ -1676,7 +1683,7 @@ static u32 stm32_dma3_check_rif(struct stm32_dma3_ddata *ddata)
>>   	 * In case CID filtering is not configured, dma-channel-mask property can be used to
>>   	 * specify available DMA channels to the kernel.
>>   	 */
>> -	of_property_read_u32(ddata->dma_dev.dev->of_node, "dma-channel-mask", &mask);
>> +	of_property_read_u32(dev->of_node, "dma-channel-mask", &mask);
>>   
>>   	/* Reserve !CID-filtered not in dma-channel-mask, static CID != CID1, CID1 not allowed */
>>   	for (i = 0; i < ddata->dma_channels; i++) {
>> @@ -1696,7 +1703,7 @@ static u32 stm32_dma3_check_rif(struct stm32_dma3_ddata *ddata)
>>   				ddata->chans[i].semaphore_mode = true;
>>   			}
>>   		}
>> -		dev_dbg(ddata->dma_dev.dev, "chan%d: %s mode, %s\n", i,
>> +		dev_dbg(dev, "chan%d: %s mode, %s\n", i,
>>   			!(ccidcfgr & CCIDCFGR_CFEN) ? "!CID-filtered" :
>>   			ddata->chans[i].semaphore_mode ? "Semaphore" : "Static CID",
>>   			(chan_reserved & BIT(i)) ? "denied" :
>> @@ -1704,7 +1711,7 @@ static u32 stm32_dma3_check_rif(struct stm32_dma3_ddata *ddata)
>>   	}
>>   
>>   	if (invalid_cid)
>> -		dev_warn(ddata->dma_dev.dev, "chan%*pbl have invalid CID configuration\n",
>> +		dev_warn(dev, "chan%*pbl have invalid CID configuration\n",
>>   			 ddata->dma_channels, &invalid_cid);
>>   
>>   	return chan_reserved;
>>
>