[PATCH 2/2] arm64: dts: imx8mm: Add i.MX8M Mini Toradex Verdin based Menlo board

[Marek Vasut]

On 4/13/22 11:44, Francesco Dolcini wrote:
> Hello Marek

Hi,

> On Sun, Apr 10, 2022 at 11:37:52PM +0200, Marek Vasut wrote:
>> On 4/10/22 10:46, Francesco Dolcini wrote:
>>> On Fri, Apr 08, 2022 at 05:02:15PM +0200, Marek Vasut wrote:
>>>> On 4/8/22 08:46, Francesco Dolcini wrote:
>>>>>> +	/delete-node/ gpio-keys;
>>>>> would it be better if we had a label in the imx8mm-verdin.dtsi and we
>>>>> could just set status=disabled here?
>>>>
>>>> It would be better if there was Verdin SoM dtsi and Verdin carrier board dts
>>>> which includes the SoM dtsi. Right now, it seems these two things are
>>>> conflated a bit.
>>>>
>>>> There are no GPIO buttons on the Verdin SoM, there are some on the Dahlia
>>>> carrier board I think.
>>>
>>> The GPIO keys, for example the suspend button, are part of Verdin family
>>> SODIMM connector pinout/definition (see related datasheets). In the SoM
>>> dtsi we implement our standard family definition.
>>>
>>> Of course, you are free to redefine this in any way you prefer. In
>>> general the way we envision this is to just enable/disable in the
>>> carrier board or overlay dts, this is the reason I proposed to add
>>> a label there. I do not see any real value on deleting the node at all,
>>> just some potential for additional maintenance burden.
>>
>> There are two reasons for not adding DT nodes for hardware which is not
>> populated:
>> - You are polluting the DT with unused nodes representing hardware which
>>    can never be present on the system, that makes the DT bigger and more
>>    complex, for no reason.
>> - Once DTOs enter the picture, these so far only useless nodes and
>>    properties actively become a problem. You cannot delete either node or
>>    property from a base DT using a DTO, because neither /delete-node/ nor
>>    /delete-property/ can be encoded into the DTO blob .
> 
> Ok, I understand your arguments and I agree they are fully valid.
> We (Toradex) had some discussion about in the past and we still see
> benefit on the way we are doing it never the less.
> 
> - The SoM dtsi representing not only the functionality implemented into
>    the SoM, but the whole connector pinout to the carrier makes very easy
>    to just include a different som.dtsi in the carrier board dts and just
>    switch SoM, for example from a colibri-imx6 to a colibri-imx7.
> - We avoid code duplication
> - Even if the DTO cannot `delete`, it can disable a node. We do our
>    best to have label and keep nodes disabled when functionality is not
>    self-contained in the SoM.

You missed one important problem here, superfluous properties in the SoM DT:

&reg_usb_otg2_vbus {
	/delete-property/ enable-active-high; // <----- HERE
};

You cannot get rid of those using DTOs.

> This is working for us pretty well so far and the majority of the users
> of ours modules rely on that, changing it now would just be too
> disruptive.
> 
> I would propose that you go with the delete-node as you are doing and we
> keep the verdin.dtsi as it is.

Let's do that, thanks.

[...]