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 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. -- Tom
signature.asc
Description: PGP signature
