fitimage: Allow match against config node name

[Hans Christian Lønstad]

Great, re-issued PR with the proper wording …

Hans Christian

> 17. sep. 2022 kl. 12:13 skrev Ahmad Fatoum <a.fatoum at pengutronix.de>:
> 
> Hi Hans,
> 
> On 17.09.22 07:51, Hans Christian Lønstad wrote:
>> Thank you for pointing out the existing option, it resolves the issue.
> 
> :)
> 
>> Regarding patch submissions:
>> Are GitHub pull requests an option? Actually have one pending that is needed on USB for IMX8MP.
> 
> Mailing list is the usual contribution method, but Sascha has accepted
> Github PRs in the past.
> 
> Regarding your existing DWC3 PR, you will need to reword the commit message
> to make this acceptable for barebox inclusion:
> 
> 1) All contributions must have a Signed-off-by certifying origin, see
>   https://developercertificate.org/
> 
> 2) Commit message needs to be useful on its own. The Github issue can
>   be referenced, but should not be the only documentation
> 
> 3) Commit title needs to be adapted to convention for the driver.
> 
> 
> Here's my suggestion:
> 
>  usb: dwc3: support snps,dis-u2-freeclk-exists-quirk for i.MX8MP
> 
>  The driver currently configures the Synopsis USB 2.0 PHY to
>  unconditionally provide a free-running PHY clock. This must
>  be disabled on the i.MX8MP as well as RK3328 and RK3399 SoC,
>  because the clock control input is inactive.
> 
>  The upstream Linux binding specifies a boolean DT property
>  snps,dis-u2-freeclk-exists-quirk to disable the free-running
>  PHY clock and the property is already used in the i.MX8MP
>  DT, that barebox imports from upstream. Thus implement the
>  binding for barebox. This fixes barebox USB host support on the
>  i.MX8MP, which previously triggered a BUG() when enumerating[1].
> 
>  Link: https://github.com/saschahauer/barebox/issues/13 [1]
>  Fixes: e213627bbe1d ("usb: dwc3: of-simple: add i.MX8MP compatible")
>  Signed-off-by: Author Name <author.email at example.com> 
> 
> 
> If you squash this commit message into your PR, feel free to
> append my
> 
> Reviewed-by: Ahmad Fatoum <a.fatoum at pengutronix.de>
> 
> Also Cc'ing Lucas, as I am wondering why you ran into the issue, but Lucas
> apparently didn't.
> 
> Cheers,
> Ahmad
> 
>> Hans Christian
>> 
>>> 16. sep. 2022 kl. 19:26 skrev Ahmad Fatoum <a.fatoum at pengutronix.de>:
>>> 
>>> Hello Hans,
>>> 
>>> On 16.09.22 15:36, Hans Christian Lønstad wrote:
>>>> Support fitimage configuration nodes without populated compatible fields
>>>> 
>>>> Yocto fit image recipe does not populate the compatible field
>>>> in the generated ITS file configuration nodes.
>>>> Barebox is thus only able to load the default configuration
>>>> preventing the use of variant based bootloader update bundles.
>>>> 
>>>> If the compatible match fails, fall through using a global
>>>> variable boot.fitnode allowing a match against the configuration
>>>> node name.
>>>> 
>>>> This allows variant boards to pick the correct configuration.
>>> 
>>> Thanks for your patch.
>>> 
>>>> 
>>>> ---
>>>> common/image-fit.c | 18 ++++++++++++++++++
>>>> 1 file changed, 18 insertions(+)
>>>> 
>>>> diff --git a/common/image-fit.c b/common/image-fit.c
>>>> index a410632d70..f92e813a8b 100644
>>>> --- a/common/image-fit.c
>>>> +++ b/common/image-fit.c
>>>> @@ -7,6 +7,7 @@
>>>> 
>>>> #define pr_fmt(fmt) "FIT: " fmt
>>>> #include <common.h>
>>>> +#include <environment.h>
>>>> #include <init.h>
>>>> #include <bootm.h>
>>>> #include <libfile.h>
>>>> @@ -663,6 +664,7 @@ static int fit_find_compatible_unit(struct device_node *conf_node,
>>>> struct device_node *barebox_root;
>>>> const char *machine;
>>>> int ret;
>>>> + const char *config_node;
>>>> 
>>>> barebox_root = of_get_root_node();
>>>> if (!barebox_root)
>>>> @@ -680,6 +682,22 @@ static int fit_find_compatible_unit(struct device_node *conf_node,
>>>> }
>>>> }
>>>> 
>>>> + /*
>>>> + * If the match against compatible in config node does not match
>>>> + * (or is missing as in Yocto fitimage recipe)
>>>> + * check for matching node name using global.boot.fitnode
>>>> + */
>>>> + config_node = getenv("global.boot.fitnode");
>>>> + if (config_node) {
>>>> + for_each_child_of_node (conf_node, child) {
>>>> + if (strcmp(child->name, config_node) == 0) {
>>>> + *unit = child->name;
>>>> + pr_info("matching node name unit '%s' found\n", *unit);
>>>> + return 0;
>>>> + }
>>>> + }
>>>> + }
>>> 
>>> Whitespace is broken (git send-email normally does the correct thing).
>>> 
>>> But are you aware that you can have your boot-target like:
>>> 
>>> bootm /dev/mmc0.fit at configuration1
>>> 
>>> and that configuration1 will be chosen? Does this already cover
>>> your use case? If it does, a documentation patch adding this
>>> information at the place where you didn't find it is surely
>>> a welcome alternate contribution. ;)
>>> 
>>> (also generally speaking magicvars need to be documented with
>>> BAREBOX_MAGICVAR).
>>> 
>>> Cheers,
>>> Ahmad
>>> 
>>> 
>>>> +
>>>> default_unit:
>>>> pr_info("No match found. Trying default.\n");
>>>> if (of_property_read_string(conf_node, "default", unit) == 0)
>>> 
>>> 
>>> -- 
>>> 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 |