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

[Jayesh Choudhary]

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@xxxxxx>
> > ---


> > +
> > +  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'.

>