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 19:01:52 +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 18:22, Krzysztof Kozlowski wrote:
> On 22/05/2024 16:43, Péter Ujfalusi wrote:
>>>>
>>>>> +      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.
> 
> None of these properties are allowed by the binding and during these
> five/six revisions of the patchset no one raised missing properties.

I just by accident spotted this patch, I was not in Cc.

The reg-names must be set to 'mpu' and 'dma'
The dma-names should be 'up_link' and 'dn_link'

These names go back for a long time (~2012) and have been mandatory ever
since.

Yes, the binding document was neglected pretty badly but when converting
to yaml it has to be correct since that will have ripple effect on
existing dts/dtsi files.

> I assume the DTS was validated with the binding. Isn't the case here?
> 
> Best regards,
> Krzysztof
> 

-- 
Péter