* [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.