From: Sascha Hauer <s.hauer@pengutronix.de> To: Michael Tretter <m.tretter@pengutronix.de> Cc: Barebox List <barebox@lists.infradead.org> Subject: Re: [PATCH 12/17] blspec: Rework firmware load Date: Mon, 15 Mar 2021 11:25:51 +0100 [thread overview] Message-ID: <20210315102551.GS23724@pengutronix.de> (raw) In-Reply-To: <20210312111505.GN12621@pengutronix.de> On Fri, Mar 12, 2021 at 12:15:05PM +0100, Michael Tretter wrote: > On Wed, 10 Mar 2021 14:28:24 +0100, Sascha Hauer wrote: > > Applying overlays in blspec currently works in two steps. First > > of_firmware_load_overlay() is called which doesn't load an overlay, > > but instead loads firmware when one is needed by the overlay. This > > is done on the live tree, because that was needed to find the firmware > > manager. The second step is to call of_register_overlay() to apply > > the overlay to the kernel device tree when the fixups are executed. > > > > Instead of using a separate step to load the firmware, load the firmware > > as part of the of_fixups. > > Wouldn't this result in the firmware being loaded whenever of_fix_tree is > called? Then, every use of the of_dump or of_diff commands would result in a > reload of the firmware. You are right, that is undesired. Ahmad just suggested a dryrun parameter to the of_fixup callbacks. That could be useful elsewhere. A dryrun would mean "Do the device tree fixups, but don't change anything else". > > -int of_firmware_load_overlay(struct device_node *overlay) > > +int of_overlay_load_firmware(struct device_node *root, struct device_node *overlay) > > { > > - int err; > > - struct device_node *root; > > - struct device_node *resolved; > > - struct device_node *ovl; > > - > > - root = of_get_root_node(); > > - resolved = of_resolve_phandles(root, overlay); > > - /* > > - * If the overlay cannot be resolved, use the load_firmware callback > > - * with the unresolved overlay to verify that the overlay does not > > - * depend on a firmware to be loaded. If a required firmware cannot be > > - * loaded, the overlay must not be applied. > > - */ > > - ovl = resolved ? resolved : overlay; > > - > > - err = of_process_overlay(root, ovl, load_firmware, NULL); > > - > > - if (resolved) > > - of_delete_node(resolved); > > - > > - return err; > > + return of_process_overlay(root, overlay, load_firmware, NULL); > > This drops the check, if the overlay depends on firmware, which cannot be > loaded, because Barebox could not resolve the overlay. This might be OK, > because in this case, we also don't know if the target is an fpga-region. > Looks fragile to me, anyway. > > > } > > diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c > > index c9ede614a6..d42e15ed1c 100644 > > --- a/drivers/of/overlay.c > > +++ b/drivers/of/overlay.c > > @@ -221,6 +221,8 @@ int of_overlay_apply_tree(struct device_node *root, > > pr_warn("failed to apply %s\n", fragment->name); > > } > > > > + err = of_overlay_load_firmware(root, resolved); > > The firmware must be loaded before the overlay is applied. If the firmware > cannot be loaded, the device tree must not be modified. OK, I will load the firmware before applying the overlay. Sascha -- 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 | _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
next prev parent reply other threads:[~2021-03-15 10:27 UTC|newest] Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-03-10 13:28 [PATCH 00/17] Apply device tree overlays to kernel tree Sascha Hauer 2021-03-10 13:28 ` [PATCH 01/17] fdt: Check blob size during unflattening Sascha Hauer 2021-03-10 13:28 ` [PATCH 02/17] firmware: make device_node argument non const Sascha Hauer 2021-03-10 13:28 ` [PATCH 03/17] libbb: Add find_path function Sascha Hauer 2021-03-10 13:28 ` [PATCH 04/17] firmware: consolidate ifdefs Sascha Hauer 2021-03-10 13:28 ` [PATCH 05/17] firmware: Add search path Sascha Hauer 2021-03-10 13:28 ` [PATCH 06/17] firmware: Fix device_node matching Sascha Hauer 2021-03-10 13:28 ` [PATCH 07/17] firmware: recognize by reproducible name Sascha Hauer 2021-03-10 13:28 ` [PATCH 08/17] blspec: Set firmware searchpath Sascha Hauer 2021-03-10 13:28 ` [PATCH 09/17] overlay: only apply compatible trees Sascha Hauer 2021-03-10 14:34 ` Ahmad Fatoum 2021-03-10 13:28 ` [PATCH 10/17] overlay: Add of_overlay_apply_file() Sascha Hauer 2021-03-10 13:28 ` [PATCH 11/17] firmware: Load from global search path Sascha Hauer 2021-03-10 14:59 ` Ahmad Fatoum 2021-03-10 13:28 ` [PATCH 12/17] blspec: Rework firmware load Sascha Hauer 2021-03-12 11:15 ` Michael Tretter 2021-03-15 10:25 ` Sascha Hauer [this message] 2021-03-10 13:28 ` [PATCH 13/17] of_overlay: apply overlays during booting Sascha Hauer 2021-03-10 13:28 ` [PATCH 14/17] blspec: Apply overlays from rootfs Sascha Hauer 2021-03-10 13:28 ` [PATCH 15/17] doc: devicetree: Refer to internal device tree also as live tree Sascha Hauer 2021-03-10 13:28 ` [PATCH 16/17] Documentation: Add documentation for device tree overlays Sascha Hauer 2021-03-10 13:28 ` [PATCH 17/17] of_firmware: Fix handling of firmware-name property Sascha Hauer 2021-06-23 5:16 [PATCH v2 00/17] Apply device tree overlays to kernel tree Sascha Hauer 2021-06-23 5:16 ` [PATCH 12/17] blspec: Rework firmware load Sascha Hauer
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=20210315102551.GS23724@pengutronix.de \ --to=s.hauer@pengutronix.de \ --cc=barebox@lists.infradead.org \ --cc=m.tretter@pengutronix.de \ --subject='Re: [PATCH 12/17] blspec: Rework firmware load' \ /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
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox