Re: [PATCH] ASoC: pcm512x: Add support for data formats RJ and LJ

[Peter Ujfalusi]

Hi Kirill,

On 12/11/2020 9.57, Kirill Marinushkin wrote:
>> The set_fmt callback is there to set the bus format, it has nothing to
>> do (in most cases) with the sample format (hw_params). Bus coding, clock
>> source has nothing to do with hw_params.
>>
>> When you bind a link you will use set_fmt for the two sides to see if
>> they can agree, that both can support what has been asked.
>>
>> The pcm512x driver just saves the fmt and say back to that card:
>> whatever, I'm fine with it. But runtime during hw_params it can fail due
>> to unsupported bus format, which it actually acked to be ok.
>>
>> This is the difference.
>>
>> Sure, some device have constraint based on the fmt towards the hw_params
>> and it is perfectly OK to do such a checks and rejections or build
>> rules/constraints based on fmt, but failing hw_params just because
>> set_fmt did not checked that the bus format is not even supported is not
>> a nice thing to do.
> 
> Those are good arguments
> 
>>> Would you agree?
>>
>> I don't have a device to test, I'm just trying to point out what is the
>> right thing to do.
> 
> I have a device to test. I will move format checks into `pcm512x_set_fmt()`,
> ensure that it works properly, and submit as patch v3.

Thank you. I know it is slightly more work.

When you send v3 please do not use --in-reply-to, let it be itself.

Regards,
- Péter

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