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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Wed, 22 May 2024 17:43:55 +0300
From: Péter Ujfalusi <peter.ujfalusi@...il.com>
To: Krzysztof Kozlowski <krzk@...nel.org>, Mighty <bavishimithil@...il.com>
Cc: Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Lopez Cruz <misael.lopez@...com>,
 linux-sound@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5] ASoC: dt-bindings: omap-mcpdm: Convert to DT schema



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@...32000 {
>>
>> 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