[PATCH 1/2] opos6uldev: make the LCD work again

[Conor Dooley]

On Fri, Mar 01, 2024 at 01:54:13PM -0500, Tom Rini wrote:
> On Fri, Mar 01, 2024 at 01:32:53PM +0000, Conor Dooley wrote:
> > > > > > > > > On Wed, 28 Feb 2024 at 18:40, Tom Rini <trini at konsulko.com> wrote:
> > > > > > > > > > I think the commit in question can be
> > > > > > > > > > summarized as "remove a bunch of explicit HW information because there's
> > > > > > > > > > now a Linux Kernel driver that determines that dynamically". What do we
> > > > > > > > > > do next? The old information is in a presumably valid binding still, can
> > > > > > > > > > we just put it back and comment that consumers outside of Linux use this
> > > > > > > > > > still so it's not removed again later? Or am I just missing where we can
> > > > > > > > > > instead get this information from the DT still and not need to come up
> > > > > > > > > > with a new driver and subsystems?
> > 
> > I don't think this is an accurate summary. The "explict hw information"
> > was never allowed for an imx6ul, only for some older devices, so "the
> > old information is in a presumably valid binding" is not accurate.
> > See 7b920aae917d ("dt-bindings: mxsfb: Add new bindings for the MXSFB
> > driver") for when doing things that way was deprecated. The imx6ul was
> > only documented several years after it was introduced (so likely several
> > years after it started incorrectly using that binding).
> > 
> > Seeing that, I am not sure that I would even question the kernel patch
> > cited above, the change was done for binding compliance and I would not
> > be inclined to think twice, given the bindings are the ABI.

Reading this back today, it took me second to recall what I meant by
"question". What I meant was that during a review I would would see a
patch changing this to comply with the bindings and think nothing of it,
not that removing it was a unquestionably correct thing to do.

> So part of the problem I see here is that legacy platforms (which to me
> is a large chunk of 32bit ARM) are trickier. I'm not asking for anything
> more to be done in this example to be clear (I think the new panel-dpi
> binding is what U-Boot needs to implement and solve the issue here).
> 
> But I don't think the fact that the old binding was handled
> suboptimally means that its usage should be ignored.

What often happens is that Linux retains support for the old way of
doing things but the dts is updated to the documented way. I'm not
really sure how to deal with this kind of thing, other than being extra
diligent about reviewing cleanup work that may impact other users of the
dts sources and letting the linux platform maintainers know if something
gets broken in U-Boot.

> They were added in
> 2017 which is after they were deprecated and not caught then. I consider
> that to mean it was still valid.

I don't agree that that makes it valid, but I can see how it would be
assumed to be valid by other users of the dts file.

> And I also assume that today we have
> the tooling to catch that and not let it in, in the first place.

Unfortunately, not really. The tooling is capable of spotting these
things, but it totally depends on the effort people have put in to
that platform as to whether or not the single to noise ratio actually
allows a maintainer to spot when these things are introduced.
The RISC-V platforms, Samsung and some others have really good
compliance, but other platforms like aspeed or at91 have so many
warnings etc that it is very very difficult for the platform maintainers
to actually spot things like these being added.

> Time will tell now how bumpy a ride things end up being as I do agree
> that getting all the DTS files following the ABI correctly is an
> important goal.

Fixing up a platform is utterly mind-numbing work that requires
understanding datasheets and bindings for really varied hardware, so
it's best done by either the platform maintainers or piecemeal by people
that want to improve one particular board. It's gonna take a long time
to get "all" files following the ABI, particularly away from the ones
the linux dt-maintainers take more of an interest in.
Next time we get a new grad starting I might subject them to some
cleanup activities, they'll have to learn about dt at some point anyway
;)

Sorta unrelated to the above, but related to the OF_UPSTREAM stuff is
that I do notice a bit of an attitude of "U-Boot bundles the dtb with
the binary, so it's okay to break U-Boot because they have a copy of
the old dts and we can update both code and dts at the same time in
the 'future'". I can't cite anything off the top of my head, but I've
not been reviewing binding patches for all that long and it has been
said multiple times.
I'm not entirely sure how to handle that sort of situation, "force" the
submitter to send patches to U-Boot before I ack the binding?

I do skim all the patch subjects that get sent to the U-Boot ML, so I'll
at least try to keep an eye out for any fallout caused by the
OF_UPSTREAM stuff.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20240308/76d18ef9/attachment.sig>