Re: [PATCH v5] ASoC: dt-bindings: omap-mcpdm: Convert to DT schema

[Péter Ujfalusi]

On 22/05/2024 17:16, Krzysztof Kozlowski wrote:
> On 22/05/2024 15:56, Péter Ujfalusi wrote:
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +  - interrupts
>>> +  - ti,hwmods
>>> +  - clocks
>>> +  - clock-names
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>>> +    pdm@40132000 {
>>
>> The original label and name is preferred to be used.
> 
> Label is not used here.
> 
> About node name, not:
> 
> Node names should be generic. See also an explanation and list of
> examples (not exhaustive) in DT specification:
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> 
> 
>>
>>> +      compatible = "ti,omap4-mcpdm";
>>> +      reg = <0x40132000 0x7f>, /* MPU private access */
>>> +            <0x49032000 0x7f>; /* L3 Interconnect */
>>> +      interrupts = <GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>;
>>> +      interrupt-parent = <&gic>;
>>> +      ti,hwmods = "mcpdm";
>>> +      clocks = <&twl6040>;
>>> +      clock-names = "pdmclk";
>>
>> The clocks cannot be added at the time when the node is defined, it is
>> board specific. This way you imply that it is OK to have it in main dtsi
>> file. It is not.
> 
> Wait, what? That's example and pretty standard. Example should be
> complete. This is not an exceptional binding.

The fclk for the McPDM is coming from external source, and the McPDM is
designed in pair with twl6040/6041, there were plan for other codecs to
support the McPDM protocol and in those cases the clock would come from
the connected codec.

The example (as the original binding was bit rot) is missing reg-names,
dmas and dma-names to be complete.

> 
> Best regards,
> Krzysztof
> 

-- 
Péter