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

[Francesco Dolcini <francesco.dolcini@toradex.com>]

Hello Marek

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.

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.

Thanks once more for the very good discussion, I hope that my proposal
works fine for you.

Francesco


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel