[alsa-devel] [PATCH] ALSA: pcm_dmaengine: correct the error handler of dmaengine_prep_dma_cyclic

[Barry Song]

2014-12-25 17:45 GMT+08:00 Lars-Peter Clausen <lars at metafoo.de>:
> On 12/25/2014 10:08 AM, Barry Song wrote:
>>
>> 2014-12-25 17:02 GMT+08:00 Lars-Peter Clausen <lars at metafoo.de>:
>>>
>>> On 12/25/2014 05:41 AM, Barry Song wrote:
>>>>
>>>>
>>>> From: Barry Song <Baohua.Song at csr.com>
>>>>
>>>> preparing cyclic DMA description can fail either due to DMA desc list
>>>> is full(-ENOMEM), or due to the coming DMA configuration is illegal or
>>>> not supported by the acting DMA hardware(other ERR codes).
>>>
>>>
>>>
>>> According to the API definition this returns either NULL or a valid
>>> descriptor, so the behavior in pcm_dmaengine is correct. Maybe your
>>> particular dmaengine driver as a incorrect implementation.
>>
>>
>> i think it is something wrong. an functions returns pointer should return
>> one of
>> (1) right pointer
>> (2) NULL
>> (3) a wrong pointer from error CODES
>>
>> if for 2&3, we both return NULL, it actually means we are taking
>> something wrong.
>>
>> this should be a generic rule as from clk and other subsystems. why
>> does DMA want to do something special?
>
>
> I think the dmaengine API simply predates the widespread use of ERR_PTR.
>
> And it probably makes sense to update it to use ERR_PTR to have more
> meaningful error codes. But this needs to be done in a way that makes sure
> that all providers and all consumers agree on the same semantics. E.g. first
> update all consumers to handle ERR_PTR or NULL, then update all providers to
> return a ERR_PTR instead of NULL, then update all consumers to just handle
> ERR_PTR.

i agree. API should be small but can't be smaller.
but for this case, this api has been too small and has not reported
enough information to users.
this lost information is preparing_dma can fill due to all kinds of
hardware or software reasons.
users should not be kept in the dark for these .

>
>
> - Lars

-barry