mailarchive of the pengutronix oss-tools mailing list
 help / color / mirror / Atom feed
From: Michael Olbrich <m.olbrich@pengutronix.de>
To: Ahmad Fatoum <a.fatoum@pengutronix.de>
Cc: oss-tools@pengutronix.de
Subject: Re: [OSS-Tools] [PATCH v3 3/3] state: automatically find state.dtb in the ESP
Date: Wed, 11 May 2022 10:02:34 +0200	[thread overview]
Message-ID: <20220511080234.GK26745@pengutronix.de> (raw)
In-Reply-To: <7cc43cd0-c9d3-e34d-a463-061fc3c5c896@pengutronix.de>

On Tue, May 10, 2022 at 03:59:24PM +0200, Ahmad Fatoum wrote:
> Hello Michael,
> 
> On 10.05.22 08:57, Michael Olbrich wrote:
> > Systemd mounts the EFI System Partition (ESP) to /boot or /efi.
> > So look there for the state.dtb when the devicetree in sysfs/procfs is
> > not available.
> > 
> > This way barebox-state can be used on EFI systems without manually
> > specifying the devicetree file.
> > 
> > Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
> > ---
> >  src/barebox-state.c | 24 ++++++++++++++++++++++++
> >  1 file changed, 24 insertions(+)
> > 
> > diff --git a/src/barebox-state.c b/src/barebox-state.c
> > index 334aed6f3d43..bf67340d4dc6 100644
> > --- a/src/barebox-state.c
> > +++ b/src/barebox-state.c
> > @@ -342,6 +342,30 @@ struct state *state_get(const char *name, const char *filename, bool readonly, b
> >  		}
> >  	} else {
> >  		root = of_read_proc_devicetree();
> > +
> > +		/* No device-tree in procfs / sysfs, try dtb file in the ESP */
> > +		if (-PTR_ERR(root) == ENOENT) {
> > +			const char *paths[] = {
> > +				/* default mount paths used by systemd */
> > +				"/boot/EFI/BAREBOX/state.dtb",
> > +				"/efi/EFI/BAREBOX/state.dtb",
> 
> On my Debian, it is /boot/efi/EFI. Do you mind appending that one to the list? :-)

ok.

> > +				NULL
> > +			};
> > +			void *fdt;
> > +			int i;
> > +
> > +			for (i = 0; paths[i]; ++i) {
> > +				fdt = read_file(paths[i], NULL);
> > +				if (fdt)
> > +					break;
> > +			}
> > +			if (fdt) {
> > +				root = of_unflatten_dtb(fdt);
> > +				free(fdt);
> > +			}
> > +			else
> > +				root = ERR_PTR(-ENOENT);
> 
> This duplicates code in the first if clause. Can you move this into
> a common helper (just a static function above state_get suffices)
> and use it?

It's similar but not the same. Here we loop over multiple files and take
the first that works, so there should be no error messages when some of
them fail.
A share function would require conditional error messages and at that
point, I don't think it actually simplifies the code any more.

Michael

> > +		}
> >  		if (IS_ERR(root)) {
> >  			pr_err("Unable to read devicetree. %s\n",
> >  			       strerror(-PTR_ERR(root)));
> 
> 
> -- 
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



      reply	other threads:[~2022-05-11  8:02 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-10  6:57 [OSS-Tools] [PATCH v3 0/3] improve barebox-state support on EFI system Michael Olbrich
2022-05-10  6:57 ` [OSS-Tools] [PATCH v3 1/3] libdt: only requires a partname for mtd Michael Olbrich
2022-05-10  6:57 ` [OSS-Tools] [PATCH v3 2/3] libdt: add support for barebox, storage-by-uuid Michael Olbrich
2022-05-10 14:02   ` Ahmad Fatoum
2022-05-10  6:57 ` [OSS-Tools] [PATCH v3 3/3] state: automatically find state.dtb in the ESP Michael Olbrich
2022-05-10 13:59   ` Ahmad Fatoum
2022-05-11  8:02     ` Michael Olbrich [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=20220511080234.GK26745@pengutronix.de \
    --to=m.olbrich@pengutronix.de \
    --cc=a.fatoum@pengutronix.de \
    --cc=oss-tools@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