* [PATCH] cmd: fdt: skip board specific fixup using env variable
@ 2021-01-27 13:09 Wasim Khan
2021-01-27 14:10 ` Tom Rini
0 siblings, 1 reply; 6+ messages in thread
From: Wasim Khan @ 2021-01-27 13:09 UTC (permalink / raw)
To: u-boot
From: Wasim Khan <wasim.khan@nxp.com>
Sometimes it is useful to boot OS with already fixed-up
device tree. Check for env variable 'skip_board_fixup'
before calling ft_board_setup().
Current behaviour is unchanged, additionally user can
set skip_board_fixup to 1 to skip the fixup.
Signed-off-by: Wasim Khan <wasim.khan@nxp.com>
---
common/image-fdt.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/common/image-fdt.c b/common/image-fdt.c
index 327a8c4c39..0435176863 100644
--- a/common/image-fdt.c
+++ b/common/image-fdt.c
@@ -572,11 +572,18 @@ int image_setup_libfdt(bootm_headers_t *images, void *blob,
fdt_fixup_pstore(blob);
#endif
if (IMAGE_OF_BOARD_SETUP) {
- fdt_ret = ft_board_setup(blob, gd->bd);
- if (fdt_ret) {
- printf("ERROR: board-specific fdt fixup failed: %s\n",
- fdt_strerror(fdt_ret));
- goto err;
+ const char *skip_board_fixup;
+
+ skip_board_fixup = env_get("skip_board_fixup");
+ if (skip_board_fixup && ((int)simple_strtol(skip_board_fixup, NULL, 10) == 1)) {
+ printf("skip board fdt fixup\n");
+ } else {
+ fdt_ret = ft_board_setup(blob, gd->bd);
+ if (fdt_ret) {
+ printf("ERROR: board-specific fdt fixup failed: %s\n",
+ fdt_strerror(fdt_ret));
+ goto err;
+ }
}
}
if (IMAGE_OF_SYSTEM_SETUP) {
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] cmd: fdt: skip board specific fixup using env variable
2021-01-27 13:09 [PATCH] cmd: fdt: skip board specific fixup using env variable Wasim Khan
@ 2021-01-27 14:10 ` Tom Rini
2021-01-28 8:15 ` Wasim Khan
0 siblings, 1 reply; 6+ messages in thread
From: Tom Rini @ 2021-01-27 14:10 UTC (permalink / raw)
To: u-boot
On Wed, Jan 27, 2021 at 02:09:48PM +0100, Wasim Khan wrote:
> From: Wasim Khan <wasim.khan@nxp.com>
>
> Sometimes it is useful to boot OS with already fixed-up
> device tree. Check for env variable 'skip_board_fixup'
> before calling ft_board_setup().
> Current behaviour is unchanged, additionally user can
> set skip_board_fixup to 1 to skip the fixup.
>
> Signed-off-by: Wasim Khan <wasim.khan@nxp.com>
> ---
> common/image-fdt.c | 17 ++++++++++++-----
> 1 file changed, 12 insertions(+), 5 deletions(-)
Can you provide a specific example or two here? Thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210127/f60db30f/attachment.sig>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] cmd: fdt: skip board specific fixup using env variable
2021-01-27 14:10 ` Tom Rini
@ 2021-01-28 8:15 ` Wasim Khan
2021-01-29 19:23 ` Tom Rini
0 siblings, 1 reply; 6+ messages in thread
From: Wasim Khan @ 2021-01-28 8:15 UTC (permalink / raw)
To: u-boot
Hi Tom,
> -----Original Message-----
> From: Tom Rini <trini@konsulko.com>
> Sent: Wednesday, January 27, 2021 7:41 PM
> To: Wasim Khan (OSS) <wasim.khan@oss.nxp.com>
> Cc: sjg at chromium.org; t-kristo at ti.com; Varun Sethi <V.Sethi@nxp.com>; u-
> boot at lists.denx.de; Wasim Khan <wasim.khan@nxp.com>
> Subject: Re: [PATCH] cmd: fdt: skip board specific fixup using env variable
>
> On Wed, Jan 27, 2021 at 02:09:48PM +0100, Wasim Khan wrote:
>
> > From: Wasim Khan <wasim.khan@nxp.com>
> >
> > Sometimes it is useful to boot OS with already fixed-up device tree.
> > Check for env variable 'skip_board_fixup'
> > before calling ft_board_setup().
> > Current behaviour is unchanged, additionally user can set
> > skip_board_fixup to 1 to skip the fixup.
> >
> > Signed-off-by: Wasim Khan <wasim.khan@nxp.com>
> > ---
> > common/image-fdt.c | 17 ++++++++++++-----
> > 1 file changed, 12 insertions(+), 5 deletions(-)
>
> Can you provide a specific example or two here? Thanks!
Thank you for your comments.
Recently I faced issue where Linux crash in PCIe was observed in customer's env.
Whereas same Linux was booting fine in my environment. After debugging we found that U-boot was doing fixup based on SVR value. SVR check was failing in customer's env because of different SoC personality which was causing the crash in OS.
So I thought it is good idea to have an option to bypass U-boot fixup and use an already fixed-up device tree so that we can quickly check if the problem is in OS or it is caused because of missed/wrong fixup from Uboot.
Another use case I can think of is suppose if we want to introduce a new dtb fixup , we can directly add the change in a fixed-up device tree, do all experiments and once validated thoroughly , we can do the same change via Uboot fixup.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] cmd: fdt: skip board specific fixup using env variable
2021-01-28 8:15 ` Wasim Khan
@ 2021-01-29 19:23 ` Tom Rini
2021-02-04 7:19 ` Wasim Khan
0 siblings, 1 reply; 6+ messages in thread
From: Tom Rini @ 2021-01-29 19:23 UTC (permalink / raw)
To: u-boot
On Thu, Jan 28, 2021 at 08:15:33AM +0000, Wasim Khan (OSS) wrote:
> Hi Tom,
>
> > -----Original Message-----
> > From: Tom Rini <trini@konsulko.com>
> > Sent: Wednesday, January 27, 2021 7:41 PM
> > To: Wasim Khan (OSS) <wasim.khan@oss.nxp.com>
> > Cc: sjg at chromium.org; t-kristo at ti.com; Varun Sethi <V.Sethi@nxp.com>; u-
> > boot at lists.denx.de; Wasim Khan <wasim.khan@nxp.com>
> > Subject: Re: [PATCH] cmd: fdt: skip board specific fixup using env variable
> >
> > On Wed, Jan 27, 2021 at 02:09:48PM +0100, Wasim Khan wrote:
> >
> > > From: Wasim Khan <wasim.khan@nxp.com>
> > >
> > > Sometimes it is useful to boot OS with already fixed-up device tree.
> > > Check for env variable 'skip_board_fixup'
> > > before calling ft_board_setup().
> > > Current behaviour is unchanged, additionally user can set
> > > skip_board_fixup to 1 to skip the fixup.
> > >
> > > Signed-off-by: Wasim Khan <wasim.khan@nxp.com>
> > > ---
> > > common/image-fdt.c | 17 ++++++++++++-----
> > > 1 file changed, 12 insertions(+), 5 deletions(-)
> >
> > Can you provide a specific example or two here? Thanks!
>
> Thank you for your comments.
>
> Recently I faced issue where Linux crash in PCIe was observed in customer's env.
> Whereas same Linux was booting fine in my environment. After debugging we found that U-boot was doing fixup based on SVR value. SVR check was failing in customer's env because of different SoC personality which was causing the crash in OS.
> So I thought it is good idea to have an option to bypass U-boot fixup and use an already fixed-up device tree so that we can quickly check if the problem is in OS or it is caused because of missed/wrong fixup from Uboot.
>
> Another use case I can think of is suppose if we want to introduce a new dtb fixup , we can directly add the change in a fixed-up device tree, do all experiments and once validated thoroughly , we can do the same change via Uboot fixup.
OK, I can see how this can be useful in some circumstances, thanks.
What I'd like to see is updating checkpatch.pl to have a check that we
aren't setting this by default in environments (something like the check
that exists now for (fdt|initrd)_high=0xffffffff should be a good
reference) as my worry is that someone will decide that's how to
work-around some issue and we'll end up in another nightmare down the
road of uncorrected dtbs causing other problems. Thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210129/d2c37157/attachment.sig>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] cmd: fdt: skip board specific fixup using env variable
2021-01-29 19:23 ` Tom Rini
@ 2021-02-04 7:19 ` Wasim Khan
2021-02-04 13:20 ` Tom Rini
0 siblings, 1 reply; 6+ messages in thread
From: Wasim Khan @ 2021-02-04 7:19 UTC (permalink / raw)
To: u-boot
Hi Tom,
> -----Original Message-----
> From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Tom Rini
> Sent: Saturday, January 30, 2021 12:53 AM
> To: Wasim Khan (OSS) <wasim.khan@oss.nxp.com>
> Cc: sjg at chromium.org; t-kristo at ti.com; Varun Sethi <V.Sethi@nxp.com>; u-
> boot at lists.denx.de
> Subject: Re: [PATCH] cmd: fdt: skip board specific fixup using env variable
>
> On Thu, Jan 28, 2021 at 08:15:33AM +0000, Wasim Khan (OSS) wrote:
> > Hi Tom,
> >
> > > -----Original Message-----
> > > From: Tom Rini <trini@konsulko.com>
> > > Sent: Wednesday, January 27, 2021 7:41 PM
> > > To: Wasim Khan (OSS) <wasim.khan@oss.nxp.com>
> > > Cc: sjg at chromium.org; t-kristo at ti.com; Varun Sethi
> > > <V.Sethi@nxp.com>; u- boot at lists.denx.de; Wasim Khan
> > > <wasim.khan@nxp.com>
> > > Subject: Re: [PATCH] cmd: fdt: skip board specific fixup using env
> > > variable
> > >
> > > On Wed, Jan 27, 2021 at 02:09:48PM +0100, Wasim Khan wrote:
> > >
> > > > From: Wasim Khan <wasim.khan@nxp.com>
> > > >
> > > > Sometimes it is useful to boot OS with already fixed-up device tree.
> > > > Check for env variable 'skip_board_fixup'
> > > > before calling ft_board_setup().
> > > > Current behaviour is unchanged, additionally user can set
> > > > skip_board_fixup to 1 to skip the fixup.
> > > >
> > > > Signed-off-by: Wasim Khan <wasim.khan@nxp.com>
> > > > ---
> > > > common/image-fdt.c | 17 ++++++++++++-----
> > > > 1 file changed, 12 insertions(+), 5 deletions(-)
> > >
> > > Can you provide a specific example or two here? Thanks!
> >
> > Thank you for your comments.
> >
> > Recently I faced issue where Linux crash in PCIe was observed in customer's
> env.
> > Whereas same Linux was booting fine in my environment. After debugging
> we found that U-boot was doing fixup based on SVR value. SVR check was
> failing in customer's env because of different SoC personality which was
> causing the crash in OS.
> > So I thought it is good idea to have an option to bypass U-boot fixup and use
> an already fixed-up device tree so that we can quickly check if the problem is in
> OS or it is caused because of missed/wrong fixup from Uboot.
> >
> > Another use case I can think of is suppose if we want to introduce a new dtb
> fixup , we can directly add the change in a fixed-up device tree, do all
> experiments and once validated thoroughly , we can do the same change via
> Uboot fixup.
>
> OK, I can see how this can be useful in some circumstances, thanks.
> What I'd like to see is updating checkpatch.pl to have a check that we aren't
> setting this by default in environments (something like the check that exists now
> for (fdt|initrd)_high=0xffffffff should be a good
> reference) as my worry is that someone will decide that's how to work-around
> some issue and we'll end up in another nightmare down the road of
> uncorrected dtbs causing other problems. Thanks!
>
> --
> Tom
Thanks, If I understood your comment correctly, you suggesting to add 'skip_board_fixup' to CONFIG_EXTRA_ENV_SETTINGS for all of our platforms and check its default value in checkpatch.pl , which must be 0 ?
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] cmd: fdt: skip board specific fixup using env variable
2021-02-04 7:19 ` Wasim Khan
@ 2021-02-04 13:20 ` Tom Rini
0 siblings, 0 replies; 6+ messages in thread
From: Tom Rini @ 2021-02-04 13:20 UTC (permalink / raw)
To: u-boot
On Thu, Feb 04, 2021 at 07:19:21AM +0000, Wasim Khan (OSS) wrote:
> Hi Tom,
>
> > -----Original Message-----
> > From: U-Boot <u-boot-bounces@lists.denx.de> On Behalf Of Tom Rini
> > Sent: Saturday, January 30, 2021 12:53 AM
> > To: Wasim Khan (OSS) <wasim.khan@oss.nxp.com>
> > Cc: sjg at chromium.org; t-kristo at ti.com; Varun Sethi <V.Sethi@nxp.com>; u-
> > boot at lists.denx.de
> > Subject: Re: [PATCH] cmd: fdt: skip board specific fixup using env variable
> >
> > On Thu, Jan 28, 2021 at 08:15:33AM +0000, Wasim Khan (OSS) wrote:
> > > Hi Tom,
> > >
> > > > -----Original Message-----
> > > > From: Tom Rini <trini@konsulko.com>
> > > > Sent: Wednesday, January 27, 2021 7:41 PM
> > > > To: Wasim Khan (OSS) <wasim.khan@oss.nxp.com>
> > > > Cc: sjg at chromium.org; t-kristo at ti.com; Varun Sethi
> > > > <V.Sethi@nxp.com>; u- boot at lists.denx.de; Wasim Khan
> > > > <wasim.khan@nxp.com>
> > > > Subject: Re: [PATCH] cmd: fdt: skip board specific fixup using env
> > > > variable
> > > >
> > > > On Wed, Jan 27, 2021 at 02:09:48PM +0100, Wasim Khan wrote:
> > > >
> > > > > From: Wasim Khan <wasim.khan@nxp.com>
> > > > >
> > > > > Sometimes it is useful to boot OS with already fixed-up device tree.
> > > > > Check for env variable 'skip_board_fixup'
> > > > > before calling ft_board_setup().
> > > > > Current behaviour is unchanged, additionally user can set
> > > > > skip_board_fixup to 1 to skip the fixup.
> > > > >
> > > > > Signed-off-by: Wasim Khan <wasim.khan@nxp.com>
> > > > > ---
> > > > > common/image-fdt.c | 17 ++++++++++++-----
> > > > > 1 file changed, 12 insertions(+), 5 deletions(-)
> > > >
> > > > Can you provide a specific example or two here? Thanks!
> > >
> > > Thank you for your comments.
> > >
> > > Recently I faced issue where Linux crash in PCIe was observed in customer's
> > env.
> > > Whereas same Linux was booting fine in my environment. After debugging
> > we found that U-boot was doing fixup based on SVR value. SVR check was
> > failing in customer's env because of different SoC personality which was
> > causing the crash in OS.
> > > So I thought it is good idea to have an option to bypass U-boot fixup and use
> > an already fixed-up device tree so that we can quickly check if the problem is in
> > OS or it is caused because of missed/wrong fixup from Uboot.
> > >
> > > Another use case I can think of is suppose if we want to introduce a new dtb
> > fixup , we can directly add the change in a fixed-up device tree, do all
> > experiments and once validated thoroughly , we can do the same change via
> > Uboot fixup.
> >
> > OK, I can see how this can be useful in some circumstances, thanks.
> > What I'd like to see is updating checkpatch.pl to have a check that we aren't
> > setting this by default in environments (something like the check that exists now
> > for (fdt|initrd)_high=0xffffffff should be a good
> > reference) as my worry is that someone will decide that's how to work-around
> > some issue and we'll end up in another nightmare down the road of
> > uncorrected dtbs causing other problems. Thanks!
>
> Thanks, If I understood your comment correctly, you suggesting to add
> 'skip_board_fixup' to CONFIG_EXTRA_ENV_SETTINGS for all of our
> platforms and check its default value in checkpatch.pl , which must be
> 0 ?
I'm saying it can't be in CONFIG_EXTRA_ENV_SETTINGS by default, and
checkpatch.pl needs a test to complain if it's found.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210204/5edad9bc/attachment.sig>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-02-04 13:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-27 13:09 [PATCH] cmd: fdt: skip board specific fixup using env variable Wasim Khan
2021-01-27 14:10 ` Tom Rini
2021-01-28 8:15 ` Wasim Khan
2021-01-29 19:23 ` Tom Rini
2021-02-04 7:19 ` Wasim Khan
2021-02-04 13:20 ` Tom Rini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.