* (Optional?) DMA vs. PIO
@ 2020-10-08 15:05 Andy Shevchenko
2020-10-08 15:29 ` Stephen Warren
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Andy Shevchenko @ 2020-10-08 15:05 UTC (permalink / raw
To: Stephen Warren, Lars-Peter Clausen
Cc: alsa-devel, Mark Brown, Pierre-Louis Bossart,
Sit, Michael Wei Hong
Hi!
During internal review of one patch I have been puzzled with the following code
and Pierre suggested to ask mailing list for help.
My main concern is what was the idea behind? Does it mean we support optional
DMA in such case? If now, why not to return an error code directly?
---8<---8<---8<---
> Why ASoC core has the following code in the first place:
>
> 387 chan = dma_request_chan(dev, name);
> 388 if (IS_ERR(chan)) {
> 389 if (PTR_ERR(chan) == -EPROBE_DEFER)
> 390 return -EPROBE_DEFER;
> 391 pcm->chan[i] = NULL;
> 392 } else {
> 393 pcm->chan[i] = chan;
> 394 }
>
> (note lines 389-391).
> If PIO fallback is not okay, why not to return an error there?
no idea, the code has been this way since 2013
(5eda87b890f867b098e5566b5543642851e8b9c3)
It's worth asking the question on the mailing list, I don't know if this is a
bug or a feature.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: (Optional?) DMA vs. PIO
2020-10-08 15:05 (Optional?) DMA vs. PIO Andy Shevchenko
@ 2020-10-08 15:29 ` Stephen Warren
2020-10-08 15:42 ` Lars-Peter Clausen
2020-10-08 15:46 ` Mark Brown
2 siblings, 0 replies; 4+ messages in thread
From: Stephen Warren @ 2020-10-08 15:29 UTC (permalink / raw
To: Andy Shevchenko, Lars-Peter Clausen
Cc: alsa-devel@alsa-project.org, Mark Brown, Pierre-Louis Bossart,
Sit, Michael Wei Hong
Andy Shevchenko wrote at Thursday, October 8, 2020 9:06 AM:
> During internal review of one patch I have been puzzled with the following code
> and Pierre suggested to ask mailing list for help.
>
> My main concern is what was the idea behind? Does it mean we support optional
> DMA in such case? If now, why not to return an error code directly?
>
> ---8<---8<---8<---
>
> > Why ASoC core has the following code in the first place:
> >
> > 387 chan = dma_request_chan(dev, name);
> > 388 if (IS_ERR(chan)) {
> > 389 if (PTR_ERR(chan) == -EPROBE_DEFER)
> > 390 return -EPROBE_DEFER;
> > 391 pcm->chan[i] = NULL;
> > 392 } else {
> > 393 pcm->chan[i] = chan;
> > 394 }
> >
> > (note lines 389-391).
> > If PIO fallback is not okay, why not to return an error there?
>
> no idea, the code has been this way since 2013
> (5eda87b890f867b098e5566b5543642851e8b9c3)
It's been there a bit longer, in fact since the file was created:
commit 28c4468b00a1e55e08cc20117de968f7c6275441
Author: Lars-Peter Clausen <lars@metafoo.de>
Date: Mon Apr 15 19:19:50 2013 +0200
ASoC: Add a generic dmaengine_pcm driver
+ pcm->chan[i] = of_dma_request_slave_channel(dev->of_node,
+ dmaengine_pcm_dma_channel_names[i]);
The commit you mentioned above simply prevents the code from taking the "no DMA
available" path if deferred probe occurs.
> It's worth asking the question on the mailing list, I don't know if this is a
> bug or a feature.
This does seem like a bug, but there are a few places in the code which explicitly check
for a NULL chan or dma_dev (derived from chan) object, so it seems deliberate.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: (Optional?) DMA vs. PIO
2020-10-08 15:05 (Optional?) DMA vs. PIO Andy Shevchenko
2020-10-08 15:29 ` Stephen Warren
@ 2020-10-08 15:42 ` Lars-Peter Clausen
2020-10-08 15:46 ` Mark Brown
2 siblings, 0 replies; 4+ messages in thread
From: Lars-Peter Clausen @ 2020-10-08 15:42 UTC (permalink / raw
To: Andy Shevchenko, Stephen Warren
Cc: alsa-devel, Mark Brown, Pierre-Louis Bossart,
Sit, Michael Wei Hong
On 10/8/20 5:05 PM, Andy Shevchenko wrote:
> Hi!
>
> During internal review of one patch I have been puzzled with the following code
> and Pierre suggested to ask mailing list for help.
>
> My main concern is what was the idea behind? Does it mean we support optional
> DMA in such case? If now, why not to return an error code directly?
>
> ---8<---8<---8<---
>
>> Why ASoC core has the following code in the first place:
>>
>> 387 chan = dma_request_chan(dev, name);
>> 388 if (IS_ERR(chan)) {
>> 389 if (PTR_ERR(chan) == -EPROBE_DEFER)
>> 390 return -EPROBE_DEFER;
>> 391 pcm->chan[i] = NULL;
>> 392 } else {
>> 393 pcm->chan[i] = chan;
>> 394 }
>>
>> (note lines 389-391).
>> If PIO fallback is not okay, why not to return an error there?
> no idea, the code has been this way since 2013
> (5eda87b890f867b098e5566b5543642851e8b9c3)
>
> It's worth asking the question on the mailing list, I don't know if this is a
> bug or a feature.
>
This is to handle the case where a only TX or only RX is supported. In
that case it is not an error if only one DMA channel is specified.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: (Optional?) DMA vs. PIO
2020-10-08 15:05 (Optional?) DMA vs. PIO Andy Shevchenko
2020-10-08 15:29 ` Stephen Warren
2020-10-08 15:42 ` Lars-Peter Clausen
@ 2020-10-08 15:46 ` Mark Brown
2 siblings, 0 replies; 4+ messages in thread
From: Mark Brown @ 2020-10-08 15:46 UTC (permalink / raw
To: Andy Shevchenko
Cc: alsa-devel, Lars-Peter Clausen, Stephen Warren,
Pierre-Louis Bossart, Sit, Michael Wei Hong
[-- Attachment #1: Type: text/plain, Size: 801 bytes --]
On Thu, Oct 08, 2020 at 06:05:39PM +0300, Andy Shevchenko wrote:
> My main concern is what was the idea behind? Does it mean we support optional
> DMA in such case? If now, why not to return an error code directly?
...
> no idea, the code has been this way since 2013
> (5eda87b890f867b098e5566b5543642851e8b9c3)
That's "ASoC: dmaengine: support deferred probe for DMA channels" for
those playing at home. It's been that way since before then since
previously we ignored errors entirely.
> It's worth asking the question on the mailing list, I don't know if this is a
> bug or a feature.
I'm fairly sure it's intentional for systems with limited DMA channels
available but ICBW, it's obviously been quite some time. In retrospect
a comment explaining the decision would have been a good idea.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-10-08 15:47 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-10-08 15:05 (Optional?) DMA vs. PIO Andy Shevchenko
2020-10-08 15:29 ` Stephen Warren
2020-10-08 15:42 ` Lars-Peter Clausen
2020-10-08 15:46 ` Mark Brown
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.