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

[Tom Rini]

On Fri, Mar 08, 2024 at 01:21:15PM +0000, Conor Dooley wrote:
> 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.

I think that's perhaps the crux of the issue. Since the kernel is the
definitive source of the dtb files too, cleanups of the sources can
impact other projects (U-Boot, OpenBSD, etc) who might not have been
aware the binding was deprecated/etc.

> > 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.

Yes, fair.

> > 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.

OK.

> > 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
> ;)

It's all tricky and complex, yup :(

> 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.

I think the first steps will just be trying to be more aware of the
other users and communicate earlier rather than later about changes. And
thanks for keeping an eye out for things.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20240308/9ec86523/attachment.sig>