Received-SPF: pass (google.com: domain of linux-kernel+bounces-186472-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <d999bc26-9bb1-44a8-92a3-bcbe14c5a1c3@gmail.com>
Date: Wed, 22 May 2024 19:01:52 +0300
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v5] ASoC: dt-bindings: omap-mcpdm: Convert to DT schema
To: Krzysztof Kozlowski <krzk@kernel.org>, Mighty <bavishimithil@gmail.com>
Cc: Liam Girdwood <lgirdwood@gmail.com>, Mark Brown <broonie@kernel.org>,
 Rob Herring <robh@kernel.org>, Krzysztof Kozlowski <krzk+dt@kernel.org>,
 Conor Dooley <conor+dt@kernel.org>, Lopez Cruz <misael.lopez@ti.com>,
 linux-sound@vger.kernel.org, devicetree@vger.kernel.org,
 linux-kernel@vger.kernel.org
References: <20240522075245.388-1-bavishimithil@gmail.com>
 <0594944d-c158-4840-8724-b3f2edaab1ca@gmail.com>
 <4f722e53-011f-4176-b6af-080522165007@kernel.org>
 <bb44d588-9316-4509-b545-9bbaa2d240cb@gmail.com>
 <3c6c5be1-fb8e-4bf0-9f58-cfb09672e8c1@kernel.org>
Content-Language: en-US
From: =?UTF-8?Q?P=C3=A9ter_Ujfalusi?= <peter.ujfalusi@gmail.com>
In-Reply-To: <3c6c5be1-fb8e-4bf0-9f58-cfb09672e8c1@kernel.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit


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