On 29/11/2021 13:21, Jayesh Choudhary wrote: > > > 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' OK, thank you. I'm sure I have missed some other binding document... >>> 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. Yes, that dts file was added 2 years after the num-serializer got removed... > >> >>> 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. Right, it is there if McASP is used via the graph card. >>> + >>> + 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? I would add a bit more detail than just 'FIFO levels' >>> + >>> + sram-size-playback: >>> + maxItems: 1 >> >> should be dropped, not used >> >>> + >>> + sram-size-capture: >>> + maxItems: 1 >> >> not used, please drop >> > > Okay. Thanks > >>> + >>> + interrupts: >>> + minItems: 1 >>> + items: >>> + - description: TX FIFO interrupt >>> + - description: RX FIFO interrupt >> >> The 'common' does not deserve a description? >> > > Will add this. Great > > >>> + 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. It looks like dra7 family have it. On those the AHCLK x/r have other source option than from outside (ATL for example). I'm not certain if they are absolutely needed, but there were something about the optional clocks... Tony, can you recall? >>> + >>> +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'. Yes, I know. The trick is that serial-dir op-mode and tdm-slots only needed when audio is used and tdm-slots is only needed in I2S mode. I would check the dmas and interrupts, but from the hardware pow they are not needed in GPIO only mode. -- Péter