Hi Tom, On Sat, 27 Dec 2025 at 13:30, Tom Rini <[email protected]> wrote: > > On Sat, Dec 27, 2025 at 11:51:02AM -0700, Simon Glass wrote: > > Hi Tom, > > > > On Sat, 27 Dec 2025 at 08:06, Tom Rini <[email protected]> wrote: > > > > > > On Sat, Dec 27, 2025 at 07:58:10AM -0700, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Sat, 27 Dec 2025 at 07:48, Tom Rini <[email protected]> wrote: > > > > > > > > > > On Sat, Dec 27, 2025 at 04:44:07AM -0700, Simon Glass wrote: > > > > > > Hi, > > > > > > > > > > > > On Mon, 1 Dec 2025 at 12:49, Simon Glass <[email protected]> wrote: > > > > > > > > > > > > > > When there is no device tree there is not point in trying to find nodes, > > > > > > > etc. since they will all be null. > > > > > > > > > > > > > > Add static inlines to skip the code in that case. > > > > > > > > > > > > > > Unfortunately this makes the file a little convoluted and there are > > > > > > > two inlines for ofnode_is_enabled() and ofnode_first/next_subnode(). But > > > > > > > it seems better than the alternative. > > > > > > > > > > > > > > We could also consider splitting up the header file. > > > > > > > > > > > > > > Also add a rule in drivers/Makefile to compile ofnode.o when OF_REAL is > > > > > > > enabled but DM is not (for kontron-sl-mx6ul) and move the > > > > > > > ofnode_for_each_compatible_node/prop() macros outside the OF_REAL > > > > > > > condition, since they only use functions that have stubs. > > > > > > > > > > > > > > Co-developed-by: Claude <[email protected]> > > > > > > > Signed-off-by: Simon Glass <[email protected]> > > > > > > > --- > > > > > > > > > > > > > > drivers/Makefile | 4 + > > > > > > > drivers/core/Makefile | 3 +- > > > > > > > include/dm/ofnode.h | 714 +++++++++++++++++++++++++++++++++++++----- > > > > > > > 3 files changed, 641 insertions(+), 80 deletions(-) > > > > > > > > > > > > Any thoughts on this patch, please? > > > > > > > > > > I'm not sure what you're expecting for AI-developed patches after we > > > > > talked on the community call about not doing that. > > > > > > > > I haven't been able to join recently but should be able to join the > > > > one in a few weeks. Which meeting was it? > > > > > > The last call you joined, where you asked about briging up AI, and I > > > explained why I didn't think it was a good idea, and we should at least > > > wait until after kernel summit, which was going to talk about it, before > > > we do anything else. > > > > Oh, that call. Yes I heard what you said. I didn't got to plumbers, > > but Marek might have heard something. > > > > See this patch for the current state: > > > > https://lore.kernel.org/ksummit/[email protected]/ > > > > There will likely be a discussion next year about which tag to use. > > But I suppose this is relevant too: > > > > https://www.linuxfoundation.org/legal/generative-ai > > Yes, and there's an LWN article about the summit portion. I also thought > the need to not worry about slop yet since it hasn't happened yet funny, > in light how of much slop gets posted everywhere else. And the comment > about it just reproducing kernel code anyways so no need to worry about > copyright reminded me of https://github.com/ocaml/ocaml/pull/14369 yet > again. > > But the huge point was "no AI", which you ignored.
That's quite entertaining! I had vaguely heard about that one before. It shows the perils of people using AI when they know nothing about the code base, or even maybe software development. I hope you are not suggesting it has any relevance to my use of AI here! Since we are talking about the same meeting that I was in, we discussed AI and you suggested delaying a formal decision until the kernel maintainers summit, to see what they come up with. I agreed with that. It seems they are landing on 'disclosure', but certainly not a ban. For reference, the LWN article is here <https://lwn.net/Articles/1049830/>. I read through some of the comments. I recently read a book called 'Don't Be a Luddite: Learning to Love Change in a World that Fears It' (it's an 'airport' book, so I'm not necessarily recommending it) which has loads of examples about how each new technology generally hit huge resistance at first. > > > > > > That absolute briefest review I can give is that it didn't solve the > > > > > problem either, and so you didn't test it correctly. > > > > > > > > > > I was just planning to ignore this, rather than get in to another long > > > > > thread with you that I suspect few people would read. > > > > > > > > The problem solved is in the commit message. Is that the problem you > > > > are referring to, or is there another one? We can discuss it in the > > > > next call if you prefer. > > > > > > It seems like you forgot why you made this patch. But your patch also > > > doesn't solve the problem it states, which you would have also found if > > > you did more in-depth testing. > > > > > > This patch is I think a great example of why as a project U-Boot should > > > have a "Don't use AI" policy. If with all your experience with the > > > project this is the best you can get I fear others will have even worse > > > quality output. > > > > It seems like you have reviewed the patch and have some feedback, but > > you want me to guess what it might be? Is that correct? > > Well, that's half correct. I could send along what a friend of mine had > Claude review it and say needs to be done, or what Gemini Pro "thinking" > told me needs to be done. But it also doesn't solve the problem you were > going to solve, because you didn't like Marek's approach. And you then > didn't test the available failure case. > > And of course both of the AI reviews start from the premise that your > patch correctly does what it describes, and that it's a good idea to do > what you describe. Neither of which are good assumptions to make for > actual review. > > > That patch took quite a while to put together and is fairly ugly. I am > > sure I could split it up, particularly if it doesn't have to be > > bisectable. But it could easily become 10 patches and a few separate > > files, given the current state of ofnode.h > > > > But no, I don't know what is wrong with it. If you would like to tell > > me I can take a look. > > I'm not sure what the point would be. It doesn't solve the problem that > lead you down this path (I've now pushed the patches that do). The next > problem is the seemingly countless times you've been asked to provide > changes in a reviewable state, of which this patch is not. Finally, if > you had reviewed the outputs you might have noticed and then wondered > how this could result in size growth of the platforms that should see a > size reduction because they don't set OF_REAL. > > But really, most critically, the current policy is Don't use AI. It is > producing lower quality work that is difficult to review. Please see above. BTW I often ask Claude to do a code review, particularly with Python. It does a pretty good job of the basics. It can undertake a refactoring, but struggles to see when one is needed/useful. Anyway, my commit message is: "When there is no device tree there is no(t) point in trying to find nodes, etc. since they will all be null. Add static inlines to skip the code in that case." So that is the problem I am trying to solve. Probably what I would like to do is turn it into a series which splits up ofnode.h so that the inline portions are not in the same file. I haven't tried that approach, though - it is just an idea. What do you think? Re the size issue, I see this in exactly two boards: am335x_evm and kontron-sl-mx6ul. I honestly didn't notice. There are quite a few combinations to consider and it seems I didn't get them all. I will take a look. Regards, Simon
