devicetree-compiler.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: Yves-Alexis Perez <corsac@debian.org>,
	devicetree-compiler@vger.kernel.org, entwicklung@pengutronix.de
Subject: Re: [PATCH v2] libfdt: overlay: ensure that existing phandles are not overwritten
Date: Fri, 23 Feb 2024 19:15:40 +1100	[thread overview]
Message-ID: <ZdhULACva9hAAwy3@zatzit> (raw)
In-Reply-To: <sdlw3yyquseg2a6muznsll25x2z2thks4jyq3pjh5anliuctkz@ozehzhgk5b4x>

[-- Attachment #1: Type: text/plain, Size: 2461 bytes --]

On Fri, Feb 23, 2024 at 08:53:37AM +0100, Uwe Kleine-König wrote:
> On Fri, Feb 23, 2024 at 03:41:40PM +1100, David Gibson wrote:
> > On Thu, Feb 22, 2024 at 10:38:54AM +0100, Uwe Kleine-König wrote:
> > > On Thu, Feb 22, 2024 at 05:32:10PM +1100, David Gibson wrote:
> > > > And here you can use fdt32_st() on (tree_val + poffset), avoiding
> > > > quite some complexity.
> > > 
> > > There is some complexity because tree_val is a const char * (as this is
> > > what fdt_getprop() returns) and fdt32_st() obviously doens't take a
> > > const pointer.
> > 
> > Ah, right.  You can use fdt_getprop_w() to get a writable pointer
> > instead.  If there was a way to have fdt_getprop() return a const
> > pointer only if the fdt pointer it was given was const, I'd do that.
> 
> Look at how container_of_const() is defined in the kernel[1], this
> requires C11 though. See

Right.  Just like we avoid allocations to allow libfdt to be used in
constrained environments like bootloaders, we're also very
conservative with which compiler features we want to rely on.

> https://en.cppreference.com/w/c/language/generic for some docs.
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/container_of.h?h=v6.7#n32
>  
> > > If you want to play around with that, the code is mostly a copy of
> > > overlay_update_local_node_references() which could benefit from your
> > > suggestions in the same way.
> > 
> > That's quite plausible.
> > 
> > > Can I lure you in improving overlay_update_local_node_references()? Then
> > 
> > Fair point..
> > 
> > https://github.com/dgibson/dtc/commit/24f60011fd43683d8e3916435c4c726e9baac9c9
> 
> I gave feedback on that one. Looks good.
> 
> > > I could copy from the improved function for my patch. (Or maybe refactor
> > > the function to take a function as parameter which is
> > 
> > Eh... I'd prefer to avoid higher order functions in something this low
> > level.
> 
> OK.

To elaborate here, I can also image certain platform + environment +
compiler combinations where indirect calls don't work properly in an
early boot environment.  They should, of course, but you'd be
astonished how many messed up things I've seen in firmwares.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

      reply	other threads:[~2024-02-23  8:16 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-16 18:19 [PATCH v2] libfdt: overlay: ensure that existing phandles are not overwritten Uwe Kleine-König
2024-02-16 21:06 ` Uwe Kleine-König
2024-02-22  6:33   ` David Gibson
2024-02-22  6:32 ` David Gibson
2024-02-22  7:22   ` Uwe Kleine-König
2024-02-22  9:02     ` David Gibson
2024-02-22  9:38   ` Uwe Kleine-König
2024-02-23  4:41     ` David Gibson
2024-02-23  7:53       ` Uwe Kleine-König
2024-02-23  8:15         ` David Gibson [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZdhULACva9hAAwy3@zatzit \
    --to=david@gibson.dropbear.id.au \
    --cc=corsac@debian.org \
    --cc=devicetree-compiler@vger.kernel.org \
    --cc=entwicklung@pengutronix.de \
    --cc=u.kleine-koenig@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).