Re: [PATCH v3] ASoC: dt-bindings: davinci-mcasp: convert McASP bindings to yaml schema

[Jayesh Choudhary <j-choudhary@ti.com>]

On 29/11/21 12:23 pm, Péter Ujfalusi wrote:
> 
> 
> On 26/11/2021 07:02, Jayesh Choudhary wrote:
>> Convert the bindings for McASP controllers for TI SOCs
>> from txt to YAML schema.
> 
> Can you CC the sound/soc/ti/ maintainer next time, I have found this
> patch in my Spam folder...

Okay. Also, I will add this file in the MAINTAINERS file under the 
heading 'TEXAS INSTRUMENTS ASoC DRIVERS'

> 
>> Adds additional properties 'clocks', 'clock-names', 'power-domains',
>> '#sound-dai-cells',
> 
>> 'num-serializer'
> 
> Which use was removed by 1427e660b49e87cd842dba94158b0fc73030c17e

The dts file of arm SOCs is not updated and was generating an error in 
dtbs_check. I will remove this property.

> 
>> and 'port'
> 
> And what this "port" is?

The mcasp node in the file 'arch/arm/boot/dts/am335x-sl50.dts' has this 
as child node.

> 
>> which were not there in the txt file.
> 
> Most likely for a reason?
> 
>> Adds 'dmas' and 'dma-names' in the example which were not there in
>> the txt file.
>> Changes 'interrupts' and 'interrupt-names' from optional to
>> required properties.
> 
> My biggest problem with the davinci-mcasp-audio.txt is that it was done
> in a wrong way and it just documented the pdata parameters as DT binding.
> If we convert it to yaml, this is going to be 'written in stone'.
> 
>>
>> Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
>> ---


>> +
>> +  tdm-slots:
> 
> description?

I will add description.

> 
>> +    maxItems: 1
>> +
>> +  serial-dir:
>> +    description:
>> +      A list of serializer configuration
>> +      Entry is indication for serializer pin direction
>> +      0 - Inactive, 1 - TX, 2 - RX
> 
> You should mention that _all_ AXR pins should be present in the array,
> even if they are no in use.
> 
>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>> +    minItems: 1
>> +    maxItems: 16
> 
> a McASP could have up to 25 AXR pins...
> 

Will update the description and the maximum.

>> +    items:
>> +      minimum: 0
>> +      maximum: 2
>> +      default: 0
>> +


>> +
>> +  tx-num-evt:
> 
> description?
> 
>> +    maxItems: 1
>> +
>> +  rx-num-evt:
> 
> description?
> 
>> +    maxItems: 1
>> +
>> +  dismod:
> 
> description?
> 

For the above three properties, is the description in the txt file 
sufficient?



>> +
>> +  sram-size-playback:
>> +    maxItems: 1
> 
> should be dropped, not used
> 
>> +
>> +  sram-size-capture:
>> +    maxItems: 1
> 
> not used, please drop
> 

Okay.

>> +
>> +  interrupts:
>> +    minItems: 1
>> +    items:
>> +      - description: TX FIFO interrupt
>> +      - description: RX FIFO interrupt
> 
> The 'common' does not deserve a description?
> 

Will add this.


>> +  gpio-controller: true
>> +
>> +  "#gpio-cells":
>> +    const: 2
>> +
>> +  function-gpios:
>> +    maxItems: 1
> 
> This is not McASP property, it was an example on how to use a pin as
> GPIO from the outside...
> 

Okay. will remove function-gpios.

>> +
>> +  clocks:
>> +    minItems: 1
>> +    maxItems: 3
>> +
>> +  clock-names:
>> +    minItems: 1
>> +    items:
>> +      - const: fck
>> +      - const: ahclkx
>> +      - const: ahclkr
> 
> I can not find any use in the code for ahclkx/r?
> 

Some arm SOCs had additional clocks in the DT nodes.


>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - reg-names
>> +  - dmas
>> +  - dma-names
>> +  - interrupts
>> +  - interrupt-names
>> +  - serial-dir
>> +  - op-mode
>> +  - tdm-slots
> 
> The last three is not needed if the McASP is used only as GPIO.
> The dmas and interrupts should not be needed in this case, but I think
> it is not taken care of atm.
> 
> The tdm-slots is ignored for DIT mode
> 

These were mentioned in txt file as required.
In light of this new knowledge, I will remove 'serial-dir', 'op-mode' 
and 'tdm-slots'.

>