From: "Hans Christian Lønstad" <hcl@datarespons.no>
To: Ahmad Fatoum <a.fatoum@pengutronix.de>
Cc: "barebox@lists.infradead.org" <barebox@lists.infradead.org>,
Lucas Stach <lst@pengutronix.de>
Subject: Re: fitimage: Allow match against config node name
Date: Sat, 17 Sep 2022 10:59:31 +0000 [thread overview]
Message-ID: <BDDE3FBD-A6F2-46C2-9551-6A8626B192FA@datarespons.no> (raw)
In-Reply-To: <4bb50cb7-d7a0-9562-b990-24231358aa1c@pengutronix.de>
Great, re-issued PR with the proper wording …
Hans Christian
> 17. sep. 2022 kl. 12:13 skrev Ahmad Fatoum <a.fatoum@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@example.com>
>
>
> If you squash this commit message into your PR, feel free to
> append my
>
> Reviewed-by: Ahmad Fatoum <a.fatoum@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@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@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 |
prev parent reply other threads:[~2022-09-17 11:01 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-16 14:36 Hans Christian Lønstad
2022-09-16 17:26 ` Ahmad Fatoum
2022-09-17 6:51 ` Hans Christian Lønstad
2022-09-17 10:13 ` Ahmad Fatoum
2022-09-17 10:59 ` Hans Christian Lønstad [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=BDDE3FBD-A6F2-46C2-9551-6A8626B192FA@datarespons.no \
--to=hcl@datarespons.no \
--cc=a.fatoum@pengutronix.de \
--cc=barebox@lists.infradead.org \
--cc=lst@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