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 18:42:40 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Péter Ujfalusi <peter.ujfalusi@...il.com>,
 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:01, Péter Ujfalusi wrote:
> 
> 
> 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.

Yep. And testing DTS should clearly show that conversion leads to
incomplete binding.

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

Mithil Bavishi,
Are you sure you tested the DTS?

Best regards,
Krzysztof