Linux-ide Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] ata: pata_arasam_cf: Use dma_request_chan() instead dma_request_slave_channel()
@ 2019-12-17 10:50 Peter Ujfalusi
  2019-12-17 11:19 ` Viresh Kumar
  0 siblings, 1 reply; 4+ messages in thread
From: Peter Ujfalusi @ 2019-12-17 10:50 UTC (permalink / raw)
  To: vireshk, b.zolnierkie, axboe; +Cc: vkoul, linux-ide, linux-kernel

dma_request_slave_channel() is a wrapper on top of dma_request_chan()
eating up the error code.

By using dma_request_chan() directly the driver can support deferred
probing against DMA.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---
 drivers/ata/pata_arasan_cf.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/pata_arasan_cf.c b/drivers/ata/pata_arasan_cf.c
index 135173c8d138..69b555d83f68 100644
--- a/drivers/ata/pata_arasan_cf.c
+++ b/drivers/ata/pata_arasan_cf.c
@@ -526,9 +526,10 @@ static void data_xfer(struct work_struct *work)
 
 	/* request dma channels */
 	/* dma_request_channel may sleep, so calling from process context */
-	acdev->dma_chan = dma_request_slave_channel(acdev->host->dev, "data");
-	if (!acdev->dma_chan) {
+	acdev->dma_chan = dma_request_chan(acdev->host->dev, "data");
+	if (IS_ERR(acdev->dma_chan)) {
 		dev_err(acdev->host->dev, "Unable to get dma_chan\n");
+		acdev->dma_chan = NULL;
 		goto chan_request_fail;
 	}
 
@@ -539,6 +540,7 @@ static void data_xfer(struct work_struct *work)
 	}
 
 	dma_release_channel(acdev->dma_chan);
+	acdev->dma_chan = NULL;
 
 	/* data xferred successfully */
 	if (!ret) {
-- 
Peter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ata: pata_arasam_cf: Use dma_request_chan() instead dma_request_slave_channel()
  2019-12-17 10:50 [PATCH] ata: pata_arasam_cf: Use dma_request_chan() instead dma_request_slave_channel() Peter Ujfalusi
@ 2019-12-17 11:19 ` Viresh Kumar
  2020-01-13 11:31   ` Bartlomiej Zolnierkiewicz
  0 siblings, 1 reply; 4+ messages in thread
From: Viresh Kumar @ 2019-12-17 11:19 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: vireshk, b.zolnierkie, axboe, vkoul, linux-ide, linux-kernel

On 17-12-19, 12:50, Peter Ujfalusi wrote:
> dma_request_slave_channel() is a wrapper on top of dma_request_chan()
> eating up the error code.
> 
> By using dma_request_chan() directly the driver can support deferred
> probing against DMA.
> 
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> ---
>  drivers/ata/pata_arasan_cf.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/ata/pata_arasan_cf.c b/drivers/ata/pata_arasan_cf.c
> index 135173c8d138..69b555d83f68 100644
> --- a/drivers/ata/pata_arasan_cf.c
> +++ b/drivers/ata/pata_arasan_cf.c
> @@ -526,9 +526,10 @@ static void data_xfer(struct work_struct *work)
>  
>  	/* request dma channels */
>  	/* dma_request_channel may sleep, so calling from process context */
> -	acdev->dma_chan = dma_request_slave_channel(acdev->host->dev, "data");
> -	if (!acdev->dma_chan) {
> +	acdev->dma_chan = dma_request_chan(acdev->host->dev, "data");
> +	if (IS_ERR(acdev->dma_chan)) {
>  		dev_err(acdev->host->dev, "Unable to get dma_chan\n");
> +		acdev->dma_chan = NULL;
>  		goto chan_request_fail;
>  	}
>  
> @@ -539,6 +540,7 @@ static void data_xfer(struct work_struct *work)
>  	}
>  
>  	dma_release_channel(acdev->dma_chan);
> +	acdev->dma_chan = NULL;
>  
>  	/* data xferred successfully */
>  	if (!ret) {

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ata: pata_arasam_cf: Use dma_request_chan() instead dma_request_slave_channel()
  2019-12-17 11:19 ` Viresh Kumar
@ 2020-01-13 11:31   ` Bartlomiej Zolnierkiewicz
  2020-01-13 14:33     ` Peter Ujfalusi
  0 siblings, 1 reply; 4+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2020-01-13 11:31 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: Viresh Kumar, vireshk, axboe, vkoul, linux-ide, linux-kernel


On 12/17/19 12:19 PM, Viresh Kumar wrote:
> On 17-12-19, 12:50, Peter Ujfalusi wrote:
>> dma_request_slave_channel() is a wrapper on top of dma_request_chan()
>> eating up the error code.
>>
>> By using dma_request_chan() directly the driver can support deferred
>> probing against DMA.

It doesn't seem to be the case as DMA channel is requested at the start
of the data transfer (which happens after the driver has been successfully
probed).

PS there is a typo in the patch summary (it should "pata_arasan_cf").

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
>> ---
>>  drivers/ata/pata_arasan_cf.c | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/ata/pata_arasan_cf.c b/drivers/ata/pata_arasan_cf.c
>> index 135173c8d138..69b555d83f68 100644
>> --- a/drivers/ata/pata_arasan_cf.c
>> +++ b/drivers/ata/pata_arasan_cf.c
>> @@ -526,9 +526,10 @@ static void data_xfer(struct work_struct *work)
>>  
>>  	/* request dma channels */
>>  	/* dma_request_channel may sleep, so calling from process context */
>> -	acdev->dma_chan = dma_request_slave_channel(acdev->host->dev, "data");
>> -	if (!acdev->dma_chan) {
>> +	acdev->dma_chan = dma_request_chan(acdev->host->dev, "data");
>> +	if (IS_ERR(acdev->dma_chan)) {
>>  		dev_err(acdev->host->dev, "Unable to get dma_chan\n");
>> +		acdev->dma_chan = NULL;
>>  		goto chan_request_fail;
>>  	}
>>  
>> @@ -539,6 +540,7 @@ static void data_xfer(struct work_struct *work)
>>  	}
>>  
>>  	dma_release_channel(acdev->dma_chan);
>> +	acdev->dma_chan = NULL;
>>  
>>  	/* data xferred successfully */
>>  	if (!ret) {
> 
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ata: pata_arasam_cf: Use dma_request_chan() instead dma_request_slave_channel()
  2020-01-13 11:31   ` Bartlomiej Zolnierkiewicz
@ 2020-01-13 14:33     ` Peter Ujfalusi
  0 siblings, 0 replies; 4+ messages in thread
From: Peter Ujfalusi @ 2020-01-13 14:33 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz
  Cc: Viresh Kumar, vireshk, axboe, vkoul, linux-ide, linux-kernel



On 13/01/2020 13.31, Bartlomiej Zolnierkiewicz wrote:
> 
> On 12/17/19 12:19 PM, Viresh Kumar wrote:
>> On 17-12-19, 12:50, Peter Ujfalusi wrote:
>>> dma_request_slave_channel() is a wrapper on top of dma_request_chan()
>>> eating up the error code.
>>>
>>> By using dma_request_chan() directly the driver can support deferred
>>> probing against DMA.
> 
> It doesn't seem to be the case as DMA channel is requested at the start
> of the data transfer (which happens after the driver has been successfully
> probed).

True, I have updated the commit message to remove the reference to
deferred probing.
If the DMA is requested upfront (at probe time, device open time?) the
driver would save quite a bit of time by not allocating and freeing the
DMA resources repeatedly for each transfer, thus most likely giving a
boost to throughput...

> PS there is a typo in the patch summary (it should "pata_arasan_cf").

Ah, fixed this as well in v2.

Thanks for catching them,
- Péter

> 
> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
> 
>>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
>>> ---
>>>  drivers/ata/pata_arasan_cf.c | 6 ++++--
>>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/ata/pata_arasan_cf.c b/drivers/ata/pata_arasan_cf.c
>>> index 135173c8d138..69b555d83f68 100644
>>> --- a/drivers/ata/pata_arasan_cf.c
>>> +++ b/drivers/ata/pata_arasan_cf.c
>>> @@ -526,9 +526,10 @@ static void data_xfer(struct work_struct *work)
>>>  
>>>  	/* request dma channels */
>>>  	/* dma_request_channel may sleep, so calling from process context */
>>> -	acdev->dma_chan = dma_request_slave_channel(acdev->host->dev, "data");
>>> -	if (!acdev->dma_chan) {
>>> +	acdev->dma_chan = dma_request_chan(acdev->host->dev, "data");
>>> +	if (IS_ERR(acdev->dma_chan)) {
>>>  		dev_err(acdev->host->dev, "Unable to get dma_chan\n");
>>> +		acdev->dma_chan = NULL;
>>>  		goto chan_request_fail;
>>>  	}
>>>  
>>> @@ -539,6 +540,7 @@ static void data_xfer(struct work_struct *work)
>>>  	}
>>>  
>>>  	dma_release_channel(acdev->dma_chan);
>>> +	acdev->dma_chan = NULL;
>>>  
>>>  	/* data xferred successfully */
>>>  	if (!ret) {
>>
>> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
>>
> 

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-17 10:50 [PATCH] ata: pata_arasam_cf: Use dma_request_chan() instead dma_request_slave_channel() Peter Ujfalusi
2019-12-17 11:19 ` Viresh Kumar
2020-01-13 11:31   ` Bartlomiej Zolnierkiewicz
2020-01-13 14:33     ` Peter Ujfalusi

Linux-ide Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-ide/0 linux-ide/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-ide linux-ide/ https://lore.kernel.org/linux-ide \
		linux-ide@vger.kernel.org
	public-inbox-index linux-ide

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-ide


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git