[PATCH 2/2] board: dh_stm32mp1: Only print board code with CONFIG_SPL_DISPLAY_PRINT
Ensure that the SoM and board code information is only printed when
CONFIG_SPL_DISPLAY_PRINT is set.
Signed-off-by: Harald Seiler
---
board/dhelectronics/dh_stm32mp1/board.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/board/dhelectronics/dh_stm32mp1/board.c
b/board/dhelectronics/dh_stm32mp1/board.c
index f9cfabe2420..b933761d0de 100644
--- a/board/dhelectronics/dh_stm32mp1/board.c
+++ b/board/dhelectronics/dh_stm32mp1/board.c
@@ -229,8 +229,9 @@ static void board_get_coding_straps(void)
gpio_free_list_nodev(gpio, ret);
- printf("Code: SoM:rev=%d,ddr3=%d Board:rev=%d\n",
- somcode, ddr3code, brdcode);
+ if (CONFIG_IS_ENABLED(DISPLAY_PRINT))
+ printf("Code: SoM:rev=%d,ddr3=%d Board:rev=%d\n",
+ somcode, ddr3code, brdcode);
}
int board_stm32mp1_ddr_config_name_match(struct udevice *dev,
--
2.41.0
[PATCH 1/2] ram: stm32mp1: Only print RAM config with CONFIG_SPL_DISPLAY_PRINT
Ensure that the RAM configuration line is only printed when
CONFIG_SPL_DISPLAY_PRINT is set.
Signed-off-by: Harald Seiler
---
drivers/ram/stm32mp1/stm32mp1_ram.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/ram/stm32mp1/stm32mp1_ram.c
b/drivers/ram/stm32mp1/stm32mp1_ram.c
index a6c19af9722..2808d07c3ae 100644
--- a/drivers/ram/stm32mp1/stm32mp1_ram.c
+++ b/drivers/ram/stm32mp1/stm32mp1_ram.c
@@ -126,7 +126,8 @@ static int stm32mp1_ddr_setup(struct udevice *dev)
dev_dbg(dev, "no st,mem-name\n");
return -EINVAL;
}
- printf("RAM: %s\n", config.info.name);
+ if (CONFIG_IS_ENABLED(DISPLAY_PRINT))
+ printf("RAM: %s\n", config.info.name);
for (idx = 0; idx < ARRAY_SIZE(param); idx++) {
ret = ofnode_read_u32_array(node, param[idx].name,
--
2.41.0
Re: [PATCH] CI: gitlab: Collect pytest artifacts
Hi Marek, On Tue, 2023-02-28 at 03:25 +0100, Marek Vasut wrote: > On 2/28/23 00:36, Tom Rini wrote: > > [...] > > > > @@ -454,9 +457,4 @@ coreboot test.py: > > > TEST_PY_BD: "coreboot" > > > TEST_PY_TEST_SPEC: "not sleep" > > > TEST_PY_ID: "--id qemu" > > > - artifacts: > > > -paths: > > > - - "*.html" > > > - - "*.css" > > > -expire_in: 1 week > > > <<: *buildman_and_testpy_dfn > > > > So, what looks like a debugging artifact was included in the proper > > patch and merged, and yes, > > https://u-boot.source-pages.denx.de/-/u-boot/-/jobs/585388/artifacts/test-log.html > > is quite handy in the case of test failure. > > Do you also get SSL_ERROR_BAD_CERT_DOMAIN warning ? That might be > something to fix. +CC Harald. ACK, the certificate does not match. I'm not sure why exactly, we need to look into it. -- Harald
Re: [PATCH] usb: gadget: dwc2_udc_otg: implement pullup()
Hi,
On Tue, 2023-01-10 at 10:11 +0100, Mattijs Korpershoek wrote:
> Pullup is used by the usb framework in order to do software-controlled
> usb_gadget_connect() and usb_gadget_disconnect().
>
> Implement pullup() for dwc2 using the SOFT_DISCONNECT bit in the dctl
> register.
>
> This is especially useful when a gadget disconnection is initiated but
> no board_usb_cleanup() is called.
>
> Signed-off-by: Mattijs Korpershoek
> ---
> On some boards using the dwc2 controller, like the Khadas VIM3L, whenever
> usb_gadget_release() is called, the D+ and D- lines are in an unknown state.
>
> Because of that, the host can't detect usb disconnection.
>
> It was attempted to be be fixed with [1] but ended up doing the gadget
> disconnection
> too early, creating issues on NXP-based boards which use uuu [2].
>
> By implementing pullup() in the controller driver, we ensure that the
> disconnection will
> only be done when the framework calls usb_gadget_disconnect().
>
> [1]
> https://lore.kernel.org/all/20220728-reset-usb-controller-v2-1-ef7657ce7...@baylibre.com/
> [2]
> https://lore.kernel.org/all/20230107164807.3597020-1-dario.binac...@amarulasolutions.com/
> ---
> drivers/usb/gadget/dwc2_udc_otg.c | 16
> 1 file changed, 16 insertions(+)
>
> diff --git a/drivers/usb/gadget/dwc2_udc_otg.c
> b/drivers/usb/gadget/dwc2_udc_otg.c
> index 77988f78ab30..93ed9712d18a 100644
> --- a/drivers/usb/gadget/dwc2_udc_otg.c
> +++ b/drivers/usb/gadget/dwc2_udc_otg.c
> @@ -236,6 +236,21 @@ static int udc_enable(struct dwc2_udc *dev)
> return 0;
> }
>
> +static int dwc2_gadget_pullup(struct usb_gadget *g, int is_on)
> +{
> + unsigned int uTemp;
Just some minor points about style...
I think the `u32` type should be used for 32-bit registers instead.
More explicit and no room for accidentally choosing the wrong size.
Also, U-Boot and Linux don't use hungarian notation for variable names,
just call it `tmp` or even better `dctl` to be explicit.
> +
> + is_on = !!is_on;
This is superfluous, the if condition works either way.
> + uTemp = readl(>dctl);
> + if (is_on)
> + uTemp = uTemp & ~SOFT_DISCONNECT;
> + else
> + uTemp |= SOFT_DISCONNECT;
> + writel(uTemp, >dctl);
> +
> + return 0;
> +}
> +
> #if !CONFIG_IS_ENABLED(DM_USB_GADGET)
> /*
>Register entry point for the peripheral controller driver.
> @@ -805,6 +820,7 @@ static void dwc2_fifo_flush(struct usb_ep *_ep)
> }
>
> static const struct usb_gadget_ops dwc2_udc_ops = {
> + .pullup = dwc2_gadget_pullup,
> /* current versions must always be self-powered */
> #if CONFIG_IS_ENABLED(DM_USB_GADGET)
> .udc_start = dwc2_gadget_start,
>
> ---
> base-commit: 7b84c973b96775576dcff228d865e8570be26c82
> change-id: 20230110-dwc2-pullup-5b0f5a073d6b
>
> Best regards,
Regards,
Harald
[PATCH] Revert "time: add weak annotation to timer_read_counter declaration"
This reverts commit 65ba7add0d609bbd035b8d42fafdaf428ac24751.
A weak extern is a nasty sight to behold: If the symbol is never
defined, on ARM, the linker will replace the function call with a NOP.
This behavior isn't well documented but there are at least some hints
to it [1].
When timer_read_counter() is not defined, this obviously does the wrong
thing here and it does so silently. The consequence is that a board
without timer_read_counter() will sleep for random amounts and generally
have erratic get_ticks() values.
Drop the __weak annotation of the extern so a linker error is raised
when timer_read_counter() is not defined. This is okay, the original
reason for the reverted change - breaking the sandbox build - no longer
applies.
Final sidenote: This was the only weak extern in the entire tree at
this time as far as I can tell. I guess we should avoid introduction of
them again as they are obviously a very big footgun.
[1]:
https://stackoverflow.com/questions/31203402/gcc-behavior-for-unresolved-weak-functions
Fixes: 65ba7add0d60 ("time: add weak annotation to timer_read_counter
declaration")
Reported-by: Serge Bazanski
Signed-off-by: Harald Seiler
---
lib/time.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/time.c b/lib/time.c
index f3aaf472d1..1e24b1b03c 100644
--- a/lib/time.c
+++ b/lib/time.c
@@ -63,7 +63,7 @@ ulong timer_get_boot_us(void)
}
#else
-extern unsigned long __weak timer_read_counter(void);
+extern unsigned long timer_read_counter(void);
#endif
#if CONFIG_IS_ENABLED(TIMER)
--
2.39.0
Re: TFTP boot using DNS
Hi Valerio,
On Thu, 2022-12-15 at 10:27 +0100, Valerio Nappi wrote:
> Hello everyone,
>
> Context:
> I'm new to the u-boot project. I am trying to boot a Zynq SoC from an
> internal TFTP server, but i have no guarantee that the IP of the
> server is going to stay the same. A DNS service is offered, and kept
> updated.
>
> When i try to update the serverip variable using the dns
> serverip command, the variable seems updated (in the sense that I can
> print the new value) but it is not. Apparently this is ignored on
> purpose at this line of the source [1]. Commit fd3056337 that
> introduced this change, mentions "Don't update the variables when the
> source was programmatic, since the variables were the source of the
> new value.", that is not true in this case, in fact the source of the
> new value is the DNS service.
>
> Is there a particular reason why this usecase was not forseen?
Unrelated to your point about proper support for this usecase, but maybe
you can use the following as a workaround in the meantime?
tftp ${loadaddr} ${serverip}:path/to/file
Also adding Joe on CC as he authored the commit you mentioned.
--
Harald
Re: [PATCH] console: Add option to keep it silent until env is loaded
Hi Simon, On Wed, 2022-07-13 at 09:28 -0600, Simon Glass wrote: > Hi Harald, > > On Tue, 12 Jul 2022 at 05:58, Harald Seiler wrote: > > > > Hi Simon, > > > > On Tue, 2022-07-12 at 04:58 -0600, Simon Glass wrote: > > > Hi Harald, > > > > > > On Wed, 6 Jul 2022 at 05:19, Harald Seiler wrote: > > > > > > > > Add a config-option which forces the console to stay silent until the > > > > proper environment is loaded from flash. > > > > > > > > This is important when the default environment does not silence the > > > > console but no output must be printed when 'silent' is set in the flash > > > > environment. > > > > > > > > After the environment from flash is loaded, the console will be > > > > silenced/unsilenced depending on it. If PRE_CONSOLE_BUFFER is also > > > > used, the buffer will now be flushed if the console should not be > > > > silenced. > > > > > > > > Signed-off-by: Harald Seiler > > > > --- > > > > common/Kconfig | 10 ++ > > > > common/console.c | 5 + > > > > 2 files changed, 15 insertions(+) > > > > > > This seems OK to me. You might want to implement the silent-console > > > device tree property in console_update_silent() too, which was dropped > > > in the conversion to driver model. > > > > This looks interesting, I'll have to look into it. > > > > Do you know if there's any effort towards supporting such a flag in the > > kernel as well? I had to remove the serial console property from my DT > > and instead pass console info via cmdline to make the silent console > > work. A "pure DT" solution of any sort would have been nicer of > > course... > > The console's 'silent' flag is propagated to Linux by dropping the > 'console=' text from the bootargs. See fixup_silent_linux(). Right, this is what I am relying on right now. The problem is that this does not have any effect when `console=` is not used and the console is instead passed using the `/chosen/stdout-path` DT property. I was wondering whether U-Boot should maybe delete this property from the DT passed to Linux when going silent... (But of course this is unrelated to the original patch here) -- Harald
Re: [PATCH] console: Add option to keep it silent until env is loaded
Hi Simon, On Tue, 2022-07-12 at 04:58 -0600, Simon Glass wrote: > Hi Harald, > > On Wed, 6 Jul 2022 at 05:19, Harald Seiler wrote: > > > > Add a config-option which forces the console to stay silent until the > > proper environment is loaded from flash. > > > > This is important when the default environment does not silence the > > console but no output must be printed when 'silent' is set in the flash > > environment. > > > > After the environment from flash is loaded, the console will be > > silenced/unsilenced depending on it. If PRE_CONSOLE_BUFFER is also > > used, the buffer will now be flushed if the console should not be > > silenced. > > > > Signed-off-by: Harald Seiler > > --- > > common/Kconfig | 10 ++ > > common/console.c | 5 + > > 2 files changed, 15 insertions(+) > > This seems OK to me. You might want to implement the silent-console > device tree property in console_update_silent() too, which was dropped > in the conversion to driver model. This looks interesting, I'll have to look into it. Do you know if there's any effort towards supporting such a flag in the kernel as well? I had to remove the serial console property from my DT and instead pass console info via cmdline to make the silent console work. A "pure DT" solution of any sort would have been nicer of course... -- Harald
[PATCH v2] spl: mmc: Use correct MMC device when loading image
When attempting to load images from multiple MMC devices in sequence,
spl_mmc_load() chooses the wrong device from the second attempt onwards.
The reason is that MMC initialization is only done on its first call and
spl_mmc_load() will then continue using this same device for all future
calls.
Fix this by checking the devnum of the "cached" device struct against
the one which is requested. If they match, use the cached one but if
they do not match, initialize the new device.
This fixes specifying multiple MMC devices in the SPL's boot order to
fall back when U-Boot Proper is corrupted or missing on the first
attempted MMC device.
Fixes: e1eb6ada4e38 ("spl: Make image loader infrastructure more universal")
Signed-off-by: Harald Seiler
---
Notes:
Changes in v2:
- Make it work with CONFIG_SPL_BLK as well
common/spl/spl_mmc.c | 17 +++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
index e1a7d25bd0..1c12f69d96 100644
--- a/common/spl/spl_mmc.c
+++ b/common/spl/spl_mmc.c
@@ -361,6 +361,17 @@ int __weak spl_mmc_emmc_boot_partition(struct mmc *mmc)
return default_spl_mmc_emmc_boot_partition(mmc);
}
+static int spl_mmc_get_mmc_devnum(struct mmc *mmc)
+{
+ struct blk_desc *block_dev;
+#if !CONFIG_IS_ENABLED(BLK)
+ block_dev = >block_dev;
+#else
+ block_dev = dev_get_uclass_plat(mmc->dev);
+#endif
+ return block_dev->devnum;
+}
+
int spl_mmc_load(struct spl_image_info *spl_image,
struct spl_boot_device *bootdev,
const char *filename,
@@ -371,9 +382,11 @@ int spl_mmc_load(struct spl_image_info *spl_image,
u32 boot_mode;
int err = 0;
__maybe_unused int part = 0;
+ int mmc_dev;
- /* Perform peripheral init only once */
- if (!mmc) {
+ /* Perform peripheral init only once for an mmc device */
+ mmc_dev = spl_mmc_get_device_index(bootdev->boot_device);
+ if (!mmc || spl_mmc_get_mmc_devnum(mmc) != mmc_dev) {
err = spl_mmc_find_device(, bootdev->boot_device);
if (err)
return err;
--
2.36.1
Re: [PATCH] spl: mmc: Use correct MMC device when loading image
Hi,
On Mon, 2022-07-11 at 12:18 +0200, Quentin Schulz wrote:
> Hi Harald,
>
> On 7/6/22 12:58, Harald Seiler wrote:
> > When attempting to load images from multiple MMC devices in sequence,
> > spl_mmc_load() chooses the wrong device from the second attempt onwards.
> >
> > The reason is that MMC initialization is only done on its first call and
> > spl_mmc_load() will then continue using this same device for all future
> > calls.
> >
> > Fix this by checking the devnum of the "cached" device struct against
> > the one which is requested. If they match, use the cached one but if
> > they do not match, initialize the new device.
> >
> > This fixes specifying multiple MMC devices in the SPL's boot order to
> > fall back when U-Boot Proper is corrupted or missing on the first
> > attempted MMC device.
> >
> > Fixes: e1eb6ada4e38 ("spl: Make image loader infrastructure more universal")
> > Signed-off-by: Harald Seiler
> > ---
> > common/spl/spl_mmc.c | 6 --
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
> > index e1a7d25bd0..22c8a8097c 100644
> > --- a/common/spl/spl_mmc.c
> > +++ b/common/spl/spl_mmc.c
> > @@ -371,9 +371,11 @@ int spl_mmc_load(struct spl_image_info *spl_image,
> > u32 boot_mode;
> > int err = 0;
> > __maybe_unused int part = 0;
> > + int mmc_dev;
> >
> > - /* Perform peripheral init only once */
> > - if (!mmc) {
> > + /* Perform peripheral init only once for an mmc device */
> > + mmc_dev = spl_mmc_get_device_index(bootdev->boot_device);
> > + if (!mmc || mmc->block_dev.devnum != mmc_dev) {
>
> block_dev field only exists when CONFIG_BLK is not defined, c.f.
> https://elixir.bootlin.com/u-boot/latest/source/include/mmc.h#L705
>
> Considering the commit introducing this, c.f.
> https://source.denx.de/u-boot/u-boot/-/commit/33fb211dd2706e666db4008801dc0d5903fd82f6,
>
> I can suggest the following diff (which makes it compile with
> rk3399-puma_defconfig):
>
> diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
> index e0e1a80536..c40b436a11 100644
> --- a/common/spl/spl_mmc.c
> +++ b/common/spl/spl_mmc.c
> @@ -409,10 +409,17 @@ int spl_mmc_load(struct spl_image_info *spl_image,
> int err = 0;
> __maybe_unused int part = 0;
> int mmc_dev;
> + struct blk_desc *block_dev;
>
> /* Perform peripheral init only once for an mmc device */
> mmc_dev = spl_mmc_get_device_index(bootdev->boot_device);
> - if (!mmc || mmc->block_dev.devnum != mmc_dev) {
> +#if !CONFIG_IS_ENABLED(BLK)
> + block_dev = >block_dev;
> +#else
> + block_dev = dev_get_uclass_plat(mmc->dev);
> +#endif
> +
> + if (!mmc || block_dev->devnum != mmc_dev) {
> err = spl_mmc_find_device(, bootdev->boot_device);
> if (err)
> return err;
>
> Note: Only build tested.
Thanks for reporting. It's not quite that simple because `mmc` is NULL
on first call to spl_mmc_load(). I'll send a v2 to fix this.
--
Harald
[PATCH] console: Add option to keep it silent until env is loaded
Add a config-option which forces the console to stay silent until the
proper environment is loaded from flash.
This is important when the default environment does not silence the
console but no output must be printed when 'silent' is set in the flash
environment.
After the environment from flash is loaded, the console will be
silenced/unsilenced depending on it. If PRE_CONSOLE_BUFFER is also
used, the buffer will now be flushed if the console should not be
silenced.
Signed-off-by: Harald Seiler
---
common/Kconfig | 10 ++
common/console.c | 5 +
2 files changed, 15 insertions(+)
diff --git a/common/Kconfig b/common/Kconfig
index fdcf4536d0..506a4d6245 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -120,6 +120,16 @@ config SILENT_CONSOLE_UPDATE_ON_RELOC
(e.g. NAND). This option makes the value of the 'silent'
environment variable take effect at relocation.
+config SILENT_CONSOLE_UNTIL_ENV
+ bool "Keep console silent until environment is loaded"
+ depends on SILENT_CONSOLE
+ help
+ This option makes sure U-Boot will never use the console unless the
+ environment from flash does not contain the 'silent' variable. If
+ set, the console is kept silent until after the environment was
+ loaded. Use this in combination with PRE_CONSOLE_BUFFER to print out
+ earlier messages after loading the environment when allowed.
+
config PRE_CONSOLE_BUFFER
bool "Buffer characters before the console is available"
help
diff --git a/common/console.c b/common/console.c
index 0013d183ae..f0f5a9cf80 100644
--- a/common/console.c
+++ b/common/console.c
@@ -893,6 +893,11 @@ static bool console_update_silent(void)
if (!IS_ENABLED(CONFIG_SILENT_CONSOLE))
return false;
+ if (IS_ENABLED(CONFIG_SILENT_CONSOLE_UNTIL_ENV) && !(gd->flags &
GD_FLG_ENV_READY)) {
+ gd->flags |= GD_FLG_SILENT;
+ return false;
+ }
+
if (env_get("silent")) {
gd->flags |= GD_FLG_SILENT;
return false;
--
2.36.1
[PATCH] spl: mmc: Use correct MMC device when loading image
When attempting to load images from multiple MMC devices in sequence,
spl_mmc_load() chooses the wrong device from the second attempt onwards.
The reason is that MMC initialization is only done on its first call and
spl_mmc_load() will then continue using this same device for all future
calls.
Fix this by checking the devnum of the "cached" device struct against
the one which is requested. If they match, use the cached one but if
they do not match, initialize the new device.
This fixes specifying multiple MMC devices in the SPL's boot order to
fall back when U-Boot Proper is corrupted or missing on the first
attempted MMC device.
Fixes: e1eb6ada4e38 ("spl: Make image loader infrastructure more universal")
Signed-off-by: Harald Seiler
---
common/spl/spl_mmc.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
index e1a7d25bd0..22c8a8097c 100644
--- a/common/spl/spl_mmc.c
+++ b/common/spl/spl_mmc.c
@@ -371,9 +371,11 @@ int spl_mmc_load(struct spl_image_info *spl_image,
u32 boot_mode;
int err = 0;
__maybe_unused int part = 0;
+ int mmc_dev;
- /* Perform peripheral init only once */
- if (!mmc) {
+ /* Perform peripheral init only once for an mmc device */
+ mmc_dev = spl_mmc_get_device_index(bootdev->boot_device);
+ if (!mmc || mmc->block_dev.devnum != mmc_dev) {
err = spl_mmc_find_device(, bootdev->boot_device);
if (err)
return err;
--
2.36.1
Re: [PATCH 2/5] imx8mm-cl-iot-gate: Retrieve the DDR type from EEPROM
On Sat, 2022-03-19 at 09:22 -0300, Fabio Estevam wrote:
> From: Fabio Estevam
>
> Currently, the DDR type is retrieved by iteracting inside an array
> of possible DDR types.
>
> This may take saveral attempts, which slows the overall U-Boot process
> and does not provide a good user experience:
>
> U-Boot SPL 2021.07 (Feb 28 2022 - 06:39:32 +)
> DDRINFO: Cfg attempt: [ 1/6 ]
> DDRINFO(M): mr5-8 [ 0xff10 ]
> DDRINFO(T): mr5-8 [ 0x510 ]
> resetting ...
>
> U-Boot SPL 2021.07 (Feb 28 2022 - 06:39:32 +)
> DDRINFO: Cfg attempt: [ 2/6 ]
> DDRINFO(M): mr5-8 [ 0xff10 ]
> DDRINFO(T): mr5-8 [ 0x1061010 ]
> resetting ...
>
> U-Boot SPL 2021.07 (Feb 28 2022 - 06:39:32 +)
> DDRINFO: Cfg attempt: [ 3/6 ]
> DDRINFO(M): mr5-8 [ 0xff10 ]
> DDRINFO(T): mr5-8 [ 0xff10 ]
> Normal Boot
> WDT: Not starting
> Trying to boot from MMC2
> NOTICE: BL31: v2.5(release):v2.5
> NOTICE: BL31: Built : 07:12:44, Jan 24 2022
>
> Improve the boot time by retrieving the correct DDR information from
> the EEPROM:
>
> U-Boot SPL 2022.04-rc4-00045-g6d02bc40d58c (Mar 19 2022 - 08:22:29 -0300)
> DDRINFO(D): Kingston 4096G
> DDRINFO(M): mr5-8 [ 0xff10 ]
> DDRINFO(E): mr5-8 [ 0xff10 ]
> Normal Boot
> WDT: Started watchdog@3028 with servicing (60s timeout)
> Trying to boot from MMC2
> NOTICE: BL31: v2.5(release):v2.5
> NOTICE: BL31: Built : 22:28:11, Mar 15 2022
>
> Based on the original code from Compulab's U-Boot.
>
> Tested on a imx8mm-cl-iot-gate board populated with 4GB of RAM.
>
> Signed-off-by: Fabio Estevam
Tested-by: Harald Seiler
> ---
> board/compulab/imx8mm-cl-iot-gate/ddr/ddr.c | 24 ++---
> board/compulab/imx8mm-cl-iot-gate/ddr/ddr.h | 5 +
> 2 files changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/board/compulab/imx8mm-cl-iot-gate/ddr/ddr.c
> b/board/compulab/imx8mm-cl-iot-gate/ddr/ddr.c
> index 42dd0dbf18fa..5b93491923e9 100644
> --- a/board/compulab/imx8mm-cl-iot-gate/ddr/ddr.c
> +++ b/board/compulab/imx8mm-cl-iot-gate/ddr/ddr.c
> @@ -22,6 +22,8 @@
> #include
> #include "ddr.h"
>
> +#include
> +
> static unsigned int lpddr4_mr_read(unsigned int mr_rank, unsigned int
> mr_addr)
> {
> unsigned int tmp;
> @@ -137,10 +139,11 @@ void spl_dram_init_compulab(void)
> (struct lpddr4_tcm_desc *)SPL_TCM_DATA;
>
> if (lpddr4_tcm_desc->sign != DEFAULT) {
> - /* if not in tcm scan mode */
> + /* get ddr type from the eeprom if not in tcm scan mode */
> + ddr_info = cl_eeprom_get_ddrinfo();
> for (i = 0; i < ARRAY_SIZE(lpddr4_array); i++) {
> if (lpddr4_array[i].id == ddr_info &&
> - lpddr4_array[i].subind == 0xff) {
> + lpddr4_array[i].subind == cl_eeprom_get_subind()) {
> ddr_found = 1;
> break;
> }
> @@ -198,10 +201,25 @@ void spl_dram_init_compulab(void)
>
> SPL_TCM_FINI;
>
> + if (ddr_found == 0) {
> + /* Update eeprom */
> + cl_eeprom_set_ddrinfo(ddr_info_mrr);
> + mdelay(10);
> + ddr_info = cl_eeprom_get_ddrinfo();
> + mdelay(10);
> + cl_eeprom_set_subind(lpddr4_array[i].subind);
> + /* make sure that the ddr_info has reached the eeprom */
> + printf("DDRINFO(E): mr5-8 [ 0x%x ], read back\n", ddr_info);
> + if (ddr_info_mrr != ddr_info || cl_eeprom_get_subind() !=
> lpddr4_array[i].subind) {
> + printf("DDRINFO(EEPROM): make sure that the eeprom is
> accessible\n");
> + printf("DDRINFO(EEPROM): i2c dev 1; i2c md 0x51 0x40
> 0x50\n");
> + }
> + }
> +
> /* Pass the dram size to th U-Boot through the tcm memory */
> { /* To figure out what to store into the TCM buffer */
> /* For debug purpouse only. To override the real memsize */
> - unsigned int ddr_tcm_size = 0;
> + unsigned int ddr_tcm_size = cl_eeprom_get_osize();
>
> if (ddr_tcm_size == 0 || ddr_tcm_size == -1)
> ddr_tcm_size = lpddr4_array[i].size;
> diff --git a/board/compulab/imx8mm-cl-iot-gate/ddr/ddr.h
> b/board/compulab/imx8mm-cl-iot-gate/ddr/ddr.h
> index 59c18911592e..f7d4fdc1016a 100644
> --- a/board/compulab/imx8mm-cl-iot-gate/ddr/ddr.h
> +++ b/board/compulab/imx8mm-cl-iot-gate/ddr/ddr.h
> @@ -23,4 +23,9 @@ struct lpddr4_tcm_desc {
> unsigned int count;
> };
>
> +u32 cl_eeprom_get_ddrinfo(void);
> +u32 cl_eeprom_set_ddrinfo(u32 ddrinfo);
> +u32 cl_eeprom_get_subind(void);
> +u32 cl_eeprom_set_subind(u32 subind);
> +u32 cl_eeprom_get_osize(void);
> #endif
--
Harald
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-62 Fax: +49-8142-66989-80 Email: h...@denx.de
Re: Question on running uboot_testpy with tbot
Hi Simon, On Thu, 2022-01-27 at 09:05 -0700, Simon Glass wrote: > Hi Harald, > [...] > > Thanks for all the info. I tried master and it mostly works, although > Iget the same error: > [...] > │ File > "/home/sglass/.local/lib/python3.8/site-packages/tbot-0.9.4-py3.8.egg/tbot/tc/uboot/testpy.py", > line 332, in testpy > │ os.write(chan_console.fileno(), data) > │ OSError: [Errno 9] Bad file descriptor > ├─ > └─FAILURE (43.835s) > > That happens after the tests finish, and what seems to be an exact 30 > second delay from 'Exiting poweroff' to 'Fail'. I am not really any > the wiser about what is happening there. The config is: Did you try changing the channel as I mentioned? The channel is probably actually selected in the lab config where the connection to the machine labelled "sglass" is made. If that's still a ParamikoConnector, please try switching to the SSHConnector as a test (and the SSHConnector is generally preferred these days...). > # Generated by labman from dut rpi3 > [...] > > Also I saw a failure for self tests: > [...] > │ │ │ File > "/home/sglass/.local/lib/python3.8/site-packages/tbot-0.9.4-py3.8.egg/tbot/machine/linux/bash.py", > line 204, in cmd_context > │ │ │ retcode = int(proxy_ch.read_until_prompt()) > │ │ │ ValueError: invalid literal for int() with base 10: 'o $?\n' > │ │ │ > │ │ └─Fail. (1.050s) > │ └─Fail. (1.091s) > ├─Exception: > Hmm, that's the old selftests... Can you try running the new selftests in the tbot repository: cd /path/to/tbot-sources python3 -m pytest selftest/ > > Finally, I have about 10 local patches which I'd like to send... Sure, please go ahead! Either on the tbot list or on GitHub, whatever you prefer. Regards, -- Harald DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-62 Fax: +49-8142-66989-80 Email: h...@denx.de
Re: Question on running uboot_testpy with tbot
Hi Simon,
first of all, sorry for the super late response to this :/ I had it on
my list for a long time but had trouble finding enough time to revisit
the test/py integration properly. You see, the existing test/py
integration has bitrotten quite a lot and I couldn't even get the
existing code working in my environment anymore...
I've now tried reworking all this code for a more robust solution [1].
It has reached a state where it works well for me and if you are still
interested, I'd be grateful for some feedback on how it fares for your
needs. That said, if your lab and board config haven't changed, the old
integration might still work for you.
The new one is not yet in a tbot release but will be shortly. In the
meantime you could check it out from the master branch and documentation
can be found here:
https://tbot.tools/contrib/uboot.html#tbot_contrib.uboot.testpy
More regarding your actual problem below.
[1]: https://github.com/Rahix/tbot/pull/64
On Sat, 2021-10-30 at 13:34 -0600, Simon Glass wrote:
> Hi,
>
> I am trying to run the pytests on my unit. The documentation doesn't really
> explain how it works.
>
> This is what I am trying:
>
> $ tbot -vv -l kea.py -b rpi3.py -T tbot/contrib -p 'testpy_args=["-k",
> "help", "-vv"]' uboot_testpy
> tbot starting ...
> ├─Parameters:
> │ testpy_args = ['-k', 'help', '-vv']
> ├─Calling uboot_testpy ...
> │ ├─Logging in on sglass@kea:22 ...
> │ ├─[sglass] bash --norc --noprofile
> │ ├─Calling uboot_setup_testhooks ...
> │ │ ├─[sglass] echo " ${HOME}"
> │ │ │ ## /home/sglass
> │ │ ├─[sglass] mkdir -p /home/sglass/tbot-workdir
> │ │ ├─[sglass] test -d /home/sglass/tbot-workdir/uboot-testpy-tbot
> │ │ ├─Creating FIFOs ...
> │ │ ├─[sglass] rm -rf
> /home/sglass/tbot-workdir/uboot-testpy-tbot/fifo_console_send
> │ │ ├─[sglass] mkfifo
> /home/sglass/tbot-workdir/uboot-testpy-tbot/fifo_console_send
> │ │ ├─[sglass] rm -rf
> /home/sglass/tbot-workdir/uboot-testpy-tbot/fifo_console_recv
> │ │ ├─[sglass] mkfifo
> /home/sglass/tbot-workdir/uboot-testpy-tbot/fifo_console_recv
> │ │ ├─[sglass] rm -rf
> /home/sglass/tbot-workdir/uboot-testpy-tbot/fifo_commands
> │ │ ├─[sglass] mkfifo
> /home/sglass/tbot-workdir/uboot-testpy-tbot/fifo_commands
> │ │ ├─[sglass] cat
> /home/sglass/tbot-workdir/uboot-testpy-tbot/tbot-scripts.sha256
> │ │ │ ##
> 2d30892b61eb713ce9413e06c4f2a0cd00d2a74b6b8c2ac6624e1e49909b1897
> │ │ ├─Hooks are up to date, skipping deployment ...
> │ │ ├─Adding hooks to $PATH ...
> │ │ ├─[sglass] echo " ${PATH}"
> │ │ │ ##
>
> /usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/snap/bin:/cosarm/depot_tools:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/l
> ocal/games:/snap/bin:/cosarm/depot_tools:/home/sglass/.local/bin:/vid/software/devel/ubtest/u-boot-test-hooks/bin:/vid/software/devel/ubtest/standalone-hdctools
> │ │ ├─[sglass] export PATH=/home/sglass/tbot-workdir/uboot-testpy-
> tbot:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/snap/bin:/cosarm/depot_tools:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/u
> sr/local/games:/snap/bin:/cosarm/depot_tools:/home/sglass/.local/bin:/vid/software/devel/ubtest/u-boot-test-hooks/bin:/vid/software/devel/ubtest/standalone-hdctools
> │ │ ├─Open console & command channels ...
> │ │ ├─[sglass] /home/sglass/tbot-workdir/uboot-testpy-tbot/tbot-console
> │ │ ├─[sglass] /home/sglass/tbot-workdir/uboot-testpy-tbot/tbot-commands
> │ │ └─Done. (0.028s)
> │ ├─Calling uboot_checkout ...
> │ │ ├─Builder: rpi3
> │ │ ├─[sglass] test -d /home/sglass/tbot-workdir/uboot-rpi3/.git
> │ │ ├─[sglass] git -C /home/sglass/tbot-workdir/uboot-rpi3 fetch
> │ │ └─Done. (0.121s)
> │ ├─[sglass] test -e /home/sglass/tbot-workdir/uboot-rpi3/.config
> │ ├─[sglass] test -e
> /home/sglass/tbot-workdir/uboot-rpi3/include/autoconf.mk
> │ ├─[sglass] picocom -q -b 115200 /dev/ttyusb_port1
> │ ├─POWERON (rpi3)
> │ ├─[sglass] sd-mux-ctrl --device-serial sdwire-18 --dut
> │ ├─[sglass] ykushcmd -s YK17698 -g 1
> │ │ ##
> │ │ ##
> │ │ ## Downstream port 1 is OFF
> │ │ ##
> │ ├─[sglass] ykushcmd -s YK17698 -u 1
> │ ├─UBOOT (rpi3-u-boot)
> │ │ <>
> │ │ <>
> │ │ <> U-Boot 2020.10-rc2-00140-g392aa09f310 (Oct 30 2021 - 12:38:07
> -0600)
> │ │ <>
> │ │ <> DRAM: 992 MiB
> │ │ <> RPI 3 Model B (0xa22082)
> │ │ <> MMC: mmc@7e202000: 0, sdhci@7e30: 1
> │ │ <> Loading Environment from FAT... *** Warning - bad CRC, using
> default environment
> │ │ <>
> │ │ <> In: serial
> │ │ <> Out: vidconsole
> │ │ <> Err: vidconsole
> │ │ <> Net: No ethernet found.
> │ │ <> Hit any key to stop autoboot: 0
> │ │ <> U-Boot>
> │ ├─[sglass] cd
Re: [PATCH] MAINTAINERS: Add watchdog maintainers entry
Hi Stefan, On Thu, 2022-01-13 at 16:57 +0100, Stefan Roese wrote: > I've been handling "inofficially" the watchdog related patches for a few > years now. Let's make this official and add a tree for it and also add > myself here in the MAINTAINERS file. Your tree is online and you should have maintainer access to it. Let me know if you need anything else. -- Harald > Signed-off-by: Stefan Roese > Cc: Tom Rini > Cc: Harald Seiler > --- > MAINTAINERS | 8 > 1 file changed, 8 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 6ae81c565943..b44651673d17 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -1297,6 +1297,14 @@ F: include/virtio*.h > F: test/dm/virtio.c > F: doc/develop/driver-model/virtio.rst > > +WATCHDOG > +M: Stefan Roese > +S: Maintained > +T: git https://source.denx.de/u-boot/custodians/u-boot-watchdog.git > +F: cmd/wdt.c > +F: drivers/watchdog/ > +F: include/watchdog*.h > + > X86 > M: Simon Glass > M: Bin Meng
[PATCH] mx6: Use imx6_src_get_boot_mode() to check boot device
Use imx6_src_get_boot_mode() instead of manually reading SBMR1. The
existing function has proper handling for software overrides of the
bootdevice which can happen, for example, when booting from an alternate
source using `bmode`.
Signed-off-by: Harald Seiler
---
arch/arm/mach-imx/mx6/soc.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/arm/mach-imx/mx6/soc.c b/arch/arm/mach-imx/mx6/soc.c
index aacfc854a2f8..bc56ef2b0b20 100644
--- a/arch/arm/mach-imx/mx6/soc.c
+++ b/arch/arm/mach-imx/mx6/soc.c
@@ -498,8 +498,7 @@ __weak int board_mmc_get_env_dev(int devno)
static int mmc_get_boot_dev(void)
{
- struct src *src_regs = (struct src *)SRC_BASE_ADDR;
- u32 soc_sbmr = readl(_regs->sbmr1);
+ u32 soc_sbmr = imx6_src_get_boot_mode();
u32 bootsel;
int devno;
--
2.31.1
[PATCH] imx: spl: Fix typo BMODE_EMI -> BMODE_EIM
The interface for NOR/OneNAND is called "EIM" not "EMI". Fix this.
Signed-off-by: Harald Seiler
---
arch/arm/include/asm/mach-imx/sys_proto.h | 10 +-
arch/arm/mach-imx/spl.c | 4 ++--
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/arm/include/asm/mach-imx/sys_proto.h
b/arch/arm/include/asm/mach-imx/sys_proto.h
index 444834995ed0..804a9b953642 100644
--- a/arch/arm/include/asm/mach-imx/sys_proto.h
+++ b/arch/arm/include/asm/mach-imx/sys_proto.h
@@ -88,9 +88,9 @@ struct bd_info;
#define IMX6_SRC_GPR10_PERSIST_SECONDARY_BOOT BIT(30)
#define IMX6_BMODE_MASKGENMASK(7, 0)
-#defineIMX6_BMODE_SHIFT4
-#define IMX6_BMODE_EMI_MASKBIT(3)
-#define IMX6_BMODE_EMI_SHIFT 3
+#define IMX6_BMODE_SHIFT 4
+#define IMX6_BMODE_EIM_MASKBIT(3)
+#define IMX6_BMODE_EIM_SHIFT 3
#define IMX6_BMODE_SERIAL_ROM_MASK GENMASK(26, 24)
#define IMX6_BMODE_SERIAL_ROM_SHIFT24
@@ -105,13 +105,13 @@ enum imx6_bmode_serial_rom {
IMX6_BMODE_I2C3,
};
-enum imx6_bmode_emi {
+enum imx6_bmode_eim {
IMX6_BMODE_NOR,
IMX6_BMODE_ONENAND,
};
enum imx6_bmode {
- IMX6_BMODE_EMI,
+ IMX6_BMODE_EIM,
#if defined(CONFIG_MX6UL) || defined(CONFIG_MX6ULL)
IMX6_BMODE_QSPI,
IMX6_BMODE_RESERVED,
diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
index 427b7f785990..2832b7350964 100644
--- a/arch/arm/mach-imx/spl.c
+++ b/arch/arm/mach-imx/spl.c
@@ -57,9 +57,9 @@ u32 spl_boot_device(void)
/* BOOT_CFG1[7:4] - see IMX6DQRM Table 8-8 */
switch ((reg & IMX6_BMODE_MASK) >> IMX6_BMODE_SHIFT) {
/* EIM: See 8.5.1, Table 8-9 */
- case IMX6_BMODE_EMI:
+ case IMX6_BMODE_EIM:
/* BOOT_CFG1[3]: NOR/OneNAND Selection */
- switch ((reg & IMX6_BMODE_EMI_MASK) >> IMX6_BMODE_EMI_SHIFT) {
+ switch ((reg & IMX6_BMODE_EIM_MASK) >> IMX6_BMODE_EIM_SHIFT) {
case IMX6_BMODE_ONENAND:
return BOOT_DEVICE_ONENAND;
case IMX6_BMODE_NOR:
--
2.31.1
Re: Two jobs at once on denx-vulcan?
Hi Simon, On Mon, 2021-09-20 at 08:06 -0600, Simon Glass wrote: > Hi Harald, > > On Mon, 20 Sept 2021 at 02:12, Harald Seiler wrote: > > > > Hi, > > > > On Sat, 2021-09-18 at 10:37 -0600, Simon Glass wrote: > > > Hi, > > > > > > Is there something screwy with this? It seems that denx-vulcan does > > > two builds at once? > > > > > > https://source.denx.de/u-boot/custodians/u-boot-dm/-/jobs/323540 > > > > Hm, I did some changes to the vulcan runner which might have caused > > this... But still, even if it is running multiple jobs in parallel, they > > should still be isolated, so how does this lead to a build failure? > > I'm not sure that it does, but I do see this at the above link: > > Error: Unable to create > '/builds/u-boot/custodians/u-boot-dm/.git/logs/HEAD.lock': File > exists. This is super strange... Each build should be running in its own container so there should never be a way for such a race to occur. No clue what is going on here... > Re doing multiple builds, have you set it up so it doesn't take on the > very large builds? I would love to enable multiple builds for the qemu > steps since they mostly use a single CPU, but am not sure how to do > it. Actually, this was more a mistake than an intentional change. I updated the runner on vulcan to also take jobs for some other repos and wanted those jobs to run in parallel. It looks like I just forgot setting the `limit = 1` option for the U-Boot runner. Now, I think doing what you suggest is possible. We need to tag build and "test" jobs differently and then define multiple runners with different limits. E.g. in `.gitlab-ci.yml`: build all 32bit ARM platforms: stage: world build tags: - build cppcheck: stage: testsuites tags: - test And then define two runners in `/etc/gitlab-runner/config.toml`: concurrent = 4 [[runners]] name = "u-boot builder on vulcan" limit = 1 ... [[runners]] name = "u-boot tester on vulcan" limit = 4 ... and during registration they get the `build` and `test` tags respectively. This would allow running (in this example) up to 4 test jobs concurrently, but only ever one large build job at once. -- Harald DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-62 Fax: +49-8142-66989-80 Email: h...@denx.de
Re: Two jobs at once on denx-vulcan?
Hi, On Sat, 2021-09-18 at 10:37 -0600, Simon Glass wrote: > Hi, > > Is there something screwy with this? It seems that denx-vulcan does > two builds at once? > > https://source.denx.de/u-boot/custodians/u-boot-dm/-/jobs/323540 Hm, I did some changes to the vulcan runner which might have caused this... But still, even if it is running multiple jobs in parallel, they should still be isolated, so how does this lead to a build failure? -- Harald DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-62 Fax: +49-8142-66989-80 Email: h...@denx.de
Re: [PATCH] am33xx: Fix USB for am335x boards
Hi,
On Sat, 2021-08-07 at 14:17 +0300, Matwey V. Kornilov wrote:
> USB nodes were mistakenly disabled in
>
> commit 942853dd96df ("arm: dts: Resync BeagleBone device trees")
To be precise, the problem is that only half of the device tree files
were synced. am33xx.dtsi (and seemingly some more) were skipped,
leading to the symptoms you found. I think it is likely that the
upstream changes in am33xx.dtsi which we are missing right now will lead
to more regressions of similar nature.
So I'd say we should dig deeper into the problems you encountered while
attempting to just sync the entirety of am33xx.dtsi. The end goal is
that all device-tree files not ending in `-uboot` match what is in
Linux, so it is inevitable that someone needs to look into this anyway.
--
Harald
> This commit is to fix the following issue:
>
> starting USB...
> No working controllers found
> USB is stopped. Please issue 'usb start' first.
> starting USB...
> No working controllers found
>
> Reference:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0782e8572ce43f521ed6ff15e4a7ab9aa5acdc85
> Fixes: 942853dd96df ("arm: dts: Resync BeagleBone device trees")
> Signed-off-by: Matwey V. Kornilov
> ---
> arch/arm/dts/am33xx.dtsi | 6 --
> 1 file changed, 6 deletions(-)
>
> diff --git a/arch/arm/dts/am33xx.dtsi b/arch/arm/dts/am33xx.dtsi
> index ce07cec846..b5093020ee 100644
> --- a/arch/arm/dts/am33xx.dtsi
> +++ b/arch/arm/dts/am33xx.dtsi
> @@ -380,28 +380,24 @@
> #address-cells = <1>;
> #size-cells = <1>;
> ti,hwmods = "usb_otg_hs";
> - status = "disabled";
>
> usb_ctrl_mod: control@44e10620 {
> compatible = "ti,am335x-usb-ctrl-module";
> reg = <0x44e10620 0x10
> 0x44e10648 0x4>;
> reg-names = "phy_ctrl", "wakeup";
> - status = "disabled";
> };
>
> usb0_phy: usb-phy@47401300 {
> compatible = "ti,am335x-usb-phy";
> reg = <0x47401300 0x100>;
> reg-names = "phy";
> - status = "disabled";
> ti,ctrl_mod = <_ctrl_mod>;
> #phy-cells = <0>;
> };
>
> usb0: usb@47401000 {
> compatible = "ti,musb-am33xx";
> - status = "disabled";
> reg = <0x47401400 0x400
> 0x47401000 0x200>;
> reg-names = "mc", "control";
> @@ -443,14 +439,12 @@
> compatible = "ti,am335x-usb-phy";
> reg = <0x47401b00 0x100>;
> reg-names = "phy";
> - status = "disabled";
> ti,ctrl_mod = <_ctrl_mod>;
> #phy-cells = <0>;
> };
>
> usb1: usb@47401800 {
> compatible = "ti,musb-am33xx";
> - status = "disabled";
> reg = <0x47401c00 0x400
> 0x47401800 0x200>;
> reg-names = "mc", "control";
--
Harald
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-62 Fax: +49-8142-66989-80 Email: h...@denx.de
Re: [BISECTED] arm: dts: Resync BeagleBone device trees
On Fri, 2021-08-06 at 12:44 +0100, Peter Robinson wrote:
> On Fri, Aug 6, 2021 at 12:39 PM Harald Seiler wrote:
> >
> > Hi,
> >
> > On Fri, 2021-08-06 at 13:54 +0300, Matwey V. Kornilov wrote:
> > > Hello,
> > >
> > > I've found that the following commit breaks USB on BeagleBone Black
> > > board (am335x based):
> > >
> > > commit 942853dd96df5de1c0a2a61c877c1cf1c24f1e91
> > > Author: Paul Barker
> > > Date: Mon Jul 12 21:14:09 2021 +0100
> > >
> > > arm: dts: Resync BeagleBone device trees
> > >
> > >
> > > When commit is applied then I see the following:
> > >
> > > starting USB...
> > > No working controllers found
> > > USB is stopped. Please issue 'usb start' first.
> > > starting USB...
> > > No working controllers found
> > >
> > > Before the commit USB was working as expected:
> > >
> > > starting USB...
> > > Bus usb@47401800: scanning bus usb@47401800 for devices... 1 USB
> > > Device(s) found
> > >scanning usb for storage devices... 1 Storage Device(s) found
> > >
> > > Device 0: Vendor: Rev: PMAP Prod: USB DISK Pro
> > > Type: Removable Hard Disk
> > > Capacity: 7381.2 MB = 7.2 GB (15116736 x 512)
> > > ... is now current device
> >
> > It looks the the arch/arm/dts/am33xx.dtsi file is out of sync with
> > Linux, leading to this regression. In the current U-Boot version, it
> > has
> >
> > status = "disabled";
> >
> > lines for e.g. usb0 while in Linux mainline, those are missing. As the
> > commit you found by bisecting drops the
> >
> > status = "okay";
> >
> > lines from arch/arm/dts/am335x-bone-common.dtsi, the device now are no
> > longer enabled. Maybe try syncing am33xx.dtsi as well to check if it
> > helps?
>
> It's usual for peripheral devices to be disabled in the SoC .dtsi
> file, those that are actively used by devices are supposed to enable
> them in the device .dts so I would expect to see the appropriate bit
> enabling it in the device .dts.
True, but apparently this was changed for am33xx.dtsi, see Linux commit
0782e8572ce4 ("ARM: dts: Probe am335x musb with ti-sysc") [1].
[1]:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0782e8572ce43f521ed6ff15e4a7ab9aa5acdc85
--
Harald
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-62 Fax: +49-8142-66989-80 Email: h...@denx.de
Re: [BISECTED] arm: dts: Resync BeagleBone device trees
Hi, On Fri, 2021-08-06 at 13:54 +0300, Matwey V. Kornilov wrote: > Hello, > > I've found that the following commit breaks USB on BeagleBone Black > board (am335x based): > > commit 942853dd96df5de1c0a2a61c877c1cf1c24f1e91 > Author: Paul Barker > Date: Mon Jul 12 21:14:09 2021 +0100 > > arm: dts: Resync BeagleBone device trees > > > When commit is applied then I see the following: > > starting USB... > No working controllers found > USB is stopped. Please issue 'usb start' first. > starting USB... > No working controllers found > > Before the commit USB was working as expected: > > starting USB... > Bus usb@47401800: scanning bus usb@47401800 for devices... 1 USB Device(s) > found >scanning usb for storage devices... 1 Storage Device(s) found > > Device 0: Vendor: Rev: PMAP Prod: USB DISK Pro > Type: Removable Hard Disk > Capacity: 7381.2 MB = 7.2 GB (15116736 x 512) > ... is now current device It looks the the arch/arm/dts/am33xx.dtsi file is out of sync with Linux, leading to this regression. In the current U-Boot version, it has status = "disabled"; lines for e.g. usb0 while in Linux mainline, those are missing. As the commit you found by bisecting drops the status = "okay"; lines from arch/arm/dts/am335x-bone-common.dtsi, the device now are no longer enabled. Maybe try syncing am33xx.dtsi as well to check if it helps? -- Harald DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-62 Fax: +49-8142-66989-80 Email: h...@denx.de
Re: Setting serverip from DHCP server
Hi,
On Thu, 2021-07-01 at 15:56 +, Gregory Anders wrote:
> (Re-sending now that I'm subscribed to the list)
>
> Hi all,
>
> I am running U-Boot on an embedded device that is connected via Ethernet
> to my workstation. The workstation is running dhcpd and U-Boot is able
> to successfully obtain an IP address via DHCP from the server. However,
> the `serverip` environment variable is not being set which prevents
> U-Boot from continuing to boot over the network. I have to manually
> enter `setenv serverip 10.0.10.1` each time.
>
> How do I get the DHCP server to set the serverip variable? My dhcpd.conf
> file is quite simple:
>
> subnet 10.0.10.0 netmask 255.255.255.0 {
> option routers 10.0.10.1;
> range 10.0.10.2;
> }
>
> I would have thought the 'option routers' line would do the trick, but
> apparently not. I've done a bit of searching online but haven't yet
> found anything helpful.
This is just a guess because I don't have a dhcpd server running, but with
dnsmasq we have this working without problems. From a quick search, maybe
adding
next-server 10.0.10.1;
might do the trick?
--
Harald
Re: [PATCH 2/2] ARM: imx: Pick correct eMMC boot partition from ROM log
Hi,
On Thu, 2021-07-01 at 01:08 +0200, Marek Vasut wrote:
> In case the iMX8M boot from eMMC boot partition and the primary image
> is corrupted, the BootROM is capable of starting a secondary image in
> the other eMMC boot partition as a fallback.
>
> However, the BootROM leaves the eMMC BOOT_PARTITION_ENABLE setting as
> it was, i.e. pointing to the boot partition containing the corrupted
> image, and the BootROM does not provide any indication that this sort
> of fallback occured.
>
> According to AN12853 i.MX ROMs Log Events, Rev. 0, May 2020, it is
> possible to determine whether fallback event occurred by parsing the
> ROM event log. In case ROM event ID 0x51 is present, fallback event
> did occur.
>
> This patch implements ROM event log parsing and search for event ID
> 0x51 for all iMX8M SoCs, and based on that corrects the eMMC boot
> partition selection. This way, the SPL loads the remaining boot
> components from the same eMMC boot partition from which it was
> started, even in case of the fallback.
>
> Signed-off-by: Marek Vasut
> Cc: Faiz Abbas
> Cc: Harald Seiler
> Cc: Lokesh Vutla
> Cc: Simon Glass
> Cc: Fabio Estevam
> Cc: Peng Fan
> Cc: Stefano Babic
> Cc: Ye Li
> ---
> arch/arm/mach-imx/imx8m/soc.c | 61 +++
> 1 file changed, 61 insertions(+)
>
> diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c
> index 0c44022a6dc..92a71b6ba29 100644
> --- a/arch/arm/mach-imx/imx8m/soc.c
> +++ b/arch/arm/mach-imx/imx8m/soc.c
> @@ -571,6 +571,67 @@ enum boot_device get_boot_device(void)
> }
> #endif
>
>
> +#if defined(CONFIG_IMX8M)
> +#include
> +int spl_mmc_emmc_boot_partition(struct mmc *mmc)
> +{
> + u32 *rom_log_addr = (u32 *)0x9e0;
> + u32 *rom_log;
> + u8 event_id;
> + int i, part;
> +
> + part = default_spl_mmc_emmc_boot_partition(mmc);
> +
> + /* If the ROM event log pointer is not valid. */
> + if (*rom_log_addr < 0x90 || *rom_log_addr >= 0xb0 ||
> + *rom_log_addr & 0x3)
> + return part;
> +
> + /* Parse the ROM event ID version 2 log */
> + rom_log = (u32 *)(uintptr_t)(*rom_log_addr);
> + for (i = 0; i < 128; i++) {
> + event_id = rom_log[i] >> 24;
> + switch (event_id) {
> + case 0x00: /* End of list */
> + break;
I assume your intention here is to break from the for loop? This `break`
will only exit the switch statement, so the loop will continue running on
the data following the "End of list". Or is this behavior intentional?
In that case I'd find the use of `continue` in the other branches a bit
odd, as `continue` and `break` do the same thing in this situation.
--
Harald
> + /* Log entries with 1 parameter, skip 1 */
> + case 0x80: /* Start to perform the device initialization */
> + case 0x81: /* The boot device initialization completes */
> + case 0x8f: /* The boot device initialization fails */
> + case 0x90: /* Start to read data from boot device */
> + case 0x91: /* Reading data from boot device completes */
> + case 0x9f: /* Reading data from boot device fails */
> + i += 1;
> + continue;
> + /* Log entries with 2 parameters, skip 2 */
> + case 0xa0: /* Image authentication result */
> + case 0xc0: /* Jump to the boot image soon */
> + i += 2;
> + continue;
> + /* Boot from the secondary boot image */
> + case 0x51:
> + /*
> + * Swap the eMMC boot partitions in case there was a
> + * fallback event (i.e. primary image was corrupted
> + * and that corruption was recognized by the BootROM),
> + * so the SPL loads the rest of the U-Boot from the
> + * correct eMMC boot partition, since the BootROM
> + * leaves the boot partition set to the corrupted one.
> + */
> + if (part == 1)
> + part = 2;
> + else if (part == 2)
> + part = 1;
> + continue;
> + default:
> + continue;
> + }
> + }
> +
> + return part;
> +}
> +#endif
> +
> bool is_usb_boot(void)
> {
> return get_boot_device() == USB_BOOT;
Re: [PATCH u-boot v2 00/38] U-Boot LTO (Sandbox + Some ARM boards)
On Fri, 2021-03-12 at 16:17 +0100, Pali Rohár wrote: > On Friday 12 March 2021 16:07:22 Harald Seiler wrote: > > On Fri, 2021-03-12 at 15:26 +0100, Marek Behun wrote: > > > On Fri, 12 Mar 2021 15:21:05 +0100 > > > Harald Seiler wrote: > > > > > > > Hi Marek, > > > > > > > > On Fri, 2021-03-12 at 11:33 +0100, Marek Behún wrote: > > > > > Hello, > > > > > > > > > > I am sending version 2 of patches adding support for LTO to U-Boot. > > > > > > > > > > This series was tested by Github/Azure CI at > > > > > https://github.com/u-boot/u-boot/pull/57 > > > > > > > > > > Code reduction is on average 4.23% for u-boot.bin and 13.58% for > > > > > u-boot-spl.bin. > > > > > > > > > > I am currently running a build test for all 1077 ARM defconfigs. > > > > > Of the first 232 defconfigs, 2 are failing when LTO is enabled > > > > > (chromebook_jerry and chromebook_speedy). Note that this series > > > > > only enables LTO for tested boards. > > > > > > > > > > Changes since v1: > > > > > - remove patches applied into u-boot-marvell > > > > > - added Reviewed-by tags > > > > > - addressed some issues discovered by Bin Meng, Marek Vasut, > > > > > Heinrich Schuchardt > > > > > - added more ARM boards (thanks to Adam Ford, Tim Harvey and Bin Meng) > > > > > - removed --gc-sections for ARM if internal libgcc is used > > > > > - remove -fwhole-program in final LTO LDFLAGS > > > > > - declared all 4 functions (memcpy, memset, memcmp, memmove) __used, > > > > > (these are mentioned in GCC man page for option -nodefaultlibs that > > > > > the compiler may generate; this seems to be a bug in GCC that > > > > > linking > > > > > fails with LTO even if these functions are present, because the > > > > > symbols can be renamed on some targets by optimization) > > > > > > > > I'm hitting a compiler error when building with imx6q_logic_defconfig: > > > > > > > > real-ld: error: no memory region specified for loadable section > > > > `.note.gnu.build-id' > > > > > > > > It seems this is caused by calling the linker through a gcc invocation > > > > which adds a `--build-id` commandline flag. I think the linker script > > > > which is used for SPL in this case (arch/arm/mach-omap2/u-boot-spl.lds) > > > > isn't properly set up to deal with a build-id. > > > > > > > > I'm not sure how to deal with this. One could either add > > > > `--build-id=none` to the GCC commandline to suppress generation of this > > > > section entirely (it is not emitted in non-LTO builds right now anyway) > > > > or > > > > include it in .text in said linker script so it is visible on the > > > > target. > > > > What do you think? > > > > > > > > I should note that I am using a Yocto-generated toolchain. I suppose > > > > most > > > > standard toolchains' behavior regarding the `--build-id` flag probably > > > > differs. > > > > > > > > Regards, > > > > > > I encountered this with Debian's cross toolchain, but since this did > > > not happen on my station with Gentoo crossdev toolchain, nor on Azure > > > CI, I ignored it. > > > > > > What is the purpose of --build-id? Why do people use it? > > > > I'm not entirely sure but I think it acts as a unique identifier for > > a certain binary. So you can match up a core-dump with its debug info for > > example. > > > > But I am unsure if anyone in the firmware space is actively using this > > feature... At least U-Boot does not actually include the build-id on the > > target - it is not generated for SPL at all and U-Boot proper only > > contains it in the ELF file, it is not exported into the raw binary. > > IIRC Debian is using build id to split out debug symbols to external ELF > binary. So based on build id it can correctly identify which external > ELF file contains correct debug symbols for stripped ELF binary. IIRC > gdb in Debian can locale external ELF file with debug symbols from > well-known location, so you just need to install appropriate -dbg > package and gdb automatically loads debug symbols. > > But if you want to use build id fo
Re: [PATCH u-boot v2 00/38] U-Boot LTO (Sandbox + Some ARM boards)
On Fri, 2021-03-12 at 16:07 +0100, Harald Seiler wrote: > On Fri, 2021-03-12 at 15:26 +0100, Marek Behun wrote: > > On Fri, 12 Mar 2021 15:21:05 +0100 > > Harald Seiler wrote: > > > > > Hi Marek, > > > > > > On Fri, 2021-03-12 at 11:33 +0100, Marek Behún wrote: > > > > Hello, > > > > > > > > I am sending version 2 of patches adding support for LTO to U-Boot. > > > > > > > > This series was tested by Github/Azure CI at > > > > https://github.com/u-boot/u-boot/pull/57 > > > > > > > > Code reduction is on average 4.23% for u-boot.bin and 13.58% for > > > > u-boot-spl.bin. > > > > > > > > I am currently running a build test for all 1077 ARM defconfigs. > > > > Of the first 232 defconfigs, 2 are failing when LTO is enabled > > > > (chromebook_jerry and chromebook_speedy). Note that this series > > > > only enables LTO for tested boards. > > > > > > > > Changes since v1: > > > > - remove patches applied into u-boot-marvell > > > > - added Reviewed-by tags > > > > - addressed some issues discovered by Bin Meng, Marek Vasut, > > > > Heinrich Schuchardt > > > > - added more ARM boards (thanks to Adam Ford, Tim Harvey and Bin Meng) > > > > - removed --gc-sections for ARM if internal libgcc is used > > > > - remove -fwhole-program in final LTO LDFLAGS > > > > - declared all 4 functions (memcpy, memset, memcmp, memmove) __used, > > > > (these are mentioned in GCC man page for option -nodefaultlibs that > > > > the compiler may generate; this seems to be a bug in GCC that linking > > > > fails with LTO even if these functions are present, because the > > > > symbols can be renamed on some targets by optimization) > > > > > > I'm hitting a compiler error when building with imx6q_logic_defconfig: > > > > > > real-ld: error: no memory region specified for loadable section > > > `.note.gnu.build-id' > > > > > > It seems this is caused by calling the linker through a gcc invocation > > > which adds a `--build-id` commandline flag. I think the linker script > > > which is used for SPL in this case (arch/arm/mach-omap2/u-boot-spl.lds) > > > isn't properly set up to deal with a build-id. > > > > > > I'm not sure how to deal with this. One could either add > > > `--build-id=none` to the GCC commandline to suppress generation of this > > > section entirely (it is not emitted in non-LTO builds right now anyway) or > > > include it in .text in said linker script so it is visible on the target. > > > What do you think? > > > > > > I should note that I am using a Yocto-generated toolchain. I suppose most > > > standard toolchains' behavior regarding the `--build-id` flag probably > > > differs. > > > > > > Regards, > > > > I encountered this with Debian's cross toolchain, but since this did > > not happen on my station with Gentoo crossdev toolchain, nor on Azure > > CI, I ignored it. > > > > What is the purpose of --build-id? Why do people use it? > > I'm not entirely sure but I think it acts as a unique identifier for > a certain binary. So you can match up a core-dump with its debug info for > example. > > But I am unsure if anyone in the firmware space is actively using this > feature... At least U-Boot does not actually include the build-id on the > target - it is not generated for SPL at all and U-Boot proper only > contains it in the ELF file, it is not exported into the raw binary. This is the origin of --build-id: https://fedoraproject.org/wiki/Releases/FeatureBuildId -- Harald
Re: [PATCH u-boot v2 00/38] U-Boot LTO (Sandbox + Some ARM boards)
On Fri, 2021-03-12 at 15:26 +0100, Marek Behun wrote: > On Fri, 12 Mar 2021 15:21:05 +0100 > Harald Seiler wrote: > > > Hi Marek, > > > > On Fri, 2021-03-12 at 11:33 +0100, Marek Behún wrote: > > > Hello, > > > > > > I am sending version 2 of patches adding support for LTO to U-Boot. > > > > > > This series was tested by Github/Azure CI at > > > https://github.com/u-boot/u-boot/pull/57 > > > > > > Code reduction is on average 4.23% for u-boot.bin and 13.58% for > > > u-boot-spl.bin. > > > > > > I am currently running a build test for all 1077 ARM defconfigs. > > > Of the first 232 defconfigs, 2 are failing when LTO is enabled > > > (chromebook_jerry and chromebook_speedy). Note that this series > > > only enables LTO for tested boards. > > > > > > Changes since v1: > > > - remove patches applied into u-boot-marvell > > > - added Reviewed-by tags > > > - addressed some issues discovered by Bin Meng, Marek Vasut, > > > Heinrich Schuchardt > > > - added more ARM boards (thanks to Adam Ford, Tim Harvey and Bin Meng) > > > - removed --gc-sections for ARM if internal libgcc is used > > > - remove -fwhole-program in final LTO LDFLAGS > > > - declared all 4 functions (memcpy, memset, memcmp, memmove) __used, > > > (these are mentioned in GCC man page for option -nodefaultlibs that > > > the compiler may generate; this seems to be a bug in GCC that linking > > > fails with LTO even if these functions are present, because the > > > symbols can be renamed on some targets by optimization) > > > > I'm hitting a compiler error when building with imx6q_logic_defconfig: > > > > real-ld: error: no memory region specified for loadable section > > `.note.gnu.build-id' > > > > It seems this is caused by calling the linker through a gcc invocation > > which adds a `--build-id` commandline flag. I think the linker script > > which is used for SPL in this case (arch/arm/mach-omap2/u-boot-spl.lds) > > isn't properly set up to deal with a build-id. > > > > I'm not sure how to deal with this. One could either add > > `--build-id=none` to the GCC commandline to suppress generation of this > > section entirely (it is not emitted in non-LTO builds right now anyway) or > > include it in .text in said linker script so it is visible on the target. > > What do you think? > > > > I should note that I am using a Yocto-generated toolchain. I suppose most > > standard toolchains' behavior regarding the `--build-id` flag probably > > differs. > > > > Regards, > > I encountered this with Debian's cross toolchain, but since this did > not happen on my station with Gentoo crossdev toolchain, nor on Azure > CI, I ignored it. > > What is the purpose of --build-id? Why do people use it? I'm not entirely sure but I think it acts as a unique identifier for a certain binary. So you can match up a core-dump with its debug info for example. But I am unsure if anyone in the firmware space is actively using this feature... At least U-Boot does not actually include the build-id on the target - it is not generated for SPL at all and U-Boot proper only contains it in the ELF file, it is not exported into the raw binary. -- Harald
Re: [PATCH u-boot v2 00/38] U-Boot LTO (Sandbox + Some ARM boards)
Hi Marek, On Fri, 2021-03-12 at 11:33 +0100, Marek Behún wrote: > Hello, > > I am sending version 2 of patches adding support for LTO to U-Boot. > > This series was tested by Github/Azure CI at > https://github.com/u-boot/u-boot/pull/57 > > Code reduction is on average 4.23% for u-boot.bin and 13.58% for > u-boot-spl.bin. > > I am currently running a build test for all 1077 ARM defconfigs. > Of the first 232 defconfigs, 2 are failing when LTO is enabled > (chromebook_jerry and chromebook_speedy). Note that this series > only enables LTO for tested boards. > > Changes since v1: > - remove patches applied into u-boot-marvell > - added Reviewed-by tags > - addressed some issues discovered by Bin Meng, Marek Vasut, > Heinrich Schuchardt > - added more ARM boards (thanks to Adam Ford, Tim Harvey and Bin Meng) > - removed --gc-sections for ARM if internal libgcc is used > - remove -fwhole-program in final LTO LDFLAGS > - declared all 4 functions (memcpy, memset, memcmp, memmove) __used, > (these are mentioned in GCC man page for option -nodefaultlibs that > the compiler may generate; this seems to be a bug in GCC that linking > fails with LTO even if these functions are present, because the > symbols can be renamed on some targets by optimization) I'm hitting a compiler error when building with imx6q_logic_defconfig: real-ld: error: no memory region specified for loadable section `.note.gnu.build-id' It seems this is caused by calling the linker through a gcc invocation which adds a `--build-id` commandline flag. I think the linker script which is used for SPL in this case (arch/arm/mach-omap2/u-boot-spl.lds) isn't properly set up to deal with a build-id. I'm not sure how to deal with this. One could either add `--build-id=none` to the GCC commandline to suppress generation of this section entirely (it is not emitted in non-LTO builds right now anyway) or include it in .text in said linker script so it is visible on the target. What do you think? I should note that I am using a Yocto-generated toolchain. I suppose most standard toolchains' behavior regarding the `--build-id` flag probably differs. Regards, -- Harald
Re: [PATCH 0/4] Remove addr parameter from reset_cpu()
Hi,
On Tue, 2020-12-15 at 16:47 +0100, Harald Seiler wrote:
> Hi,
>
> this is something I had on my mind for a longer time but never got
> around to actually do until now ... A while back, while working on the
> patchset that led to commit c5635a032a4b ("ARM: imx8m: Don't use the
> addr parameter of reset_cpu()"), I noticed that the `addr` parameter of
> reset_cpu() seems to not actually hold any meaningful value. All
> call-sites in the current tree just pass 0 and the vast majority of
> reset_cpu() implementations actually ignore the parameter.
>
> I dug a bit deeper to find out why this `addr` parameter exists in the
> first place and found out that it's mostly a legacy artifact:
>
> Historically, the reset_cpu() function had this `addr` parameter to
> pass an address of a reset vector location, where the CPU should
> reset to.
>
> The times where this was used are long gone and the only trace it left
> is some (dead) code for the NDS32 arch. The `addr` parameter lived on
> and it looks like it was sometimes used as a way to indicate different
> types of resets (e.g. COLD vs WARM).
>
> Today, however, reset_cpu() is only ever called with `addr` 0 in the
> mainline tree and as such, any code that gives a meaning to the `addr`
> value will only ever follow the `addr == 0` branch. This is probably
> not what the authors intended and as it seems quite unobvious to me,
> I think the best way forward is to remove the `addr` parameter entirely.
>
> This removes any ambiguity in the "contract" of reset_cpu() and thus
> hopefully prevents more code being added which wrongly assumes that the
> parameter can be used for any meaningful purpose. Instead, code which
> wants to properly support multiple reset types needs to be implemented
> as a sysreset driver.
>
>
> I did this API change via a coccinelle patch, see "reset: Remove addr
> parameter from reset_cpu()" for details. I also ran buildman for all
> boards I could, to verify that everything still compiles. One notable
> exception is NDS32 because I couldn't get the compiler to work there ...
I wanted to ask what the current state regarding this patchset is. Is
there anything I should still take care of?
I am a bit worried about it going stale if it stays lying around for too
long and new call-sites get introduced. So far everything is still fine
though, I just applied the patchset to v2021.04-rc3 and ran a worldbuild
- I do not see any builds regressing.
--
Harald
> Regards,
> Harald
>
> Harald Seiler (4):
> nds32: Remove dead reset_cpu() implementation
> board: ns3: Remove superfluous reset logic
> Revert "lpc32xx: cpu: add support for soft reset"
> reset: Remove addr parameter from reset_cpu()
>
> arch/arc/lib/reset.c | 4 ++--
> arch/arm/cpu/arm920t/ep93xx/cpu.c | 2 +-
> arch/arm/cpu/arm920t/imx/timer.c | 2 +-
> arch/arm/cpu/arm926ejs/armada100/timer.c | 2 +-
> arch/arm/cpu/arm926ejs/mx25/reset.c | 2 +-
> arch/arm/cpu/arm926ejs/mx27/reset.c | 2 +-
> arch/arm/cpu/arm926ejs/mxs/mxs.c | 4 ++--
> arch/arm/cpu/arm926ejs/spear/reset.c | 2 +-
> arch/arm/cpu/arm946es/cpu.c | 2 +-
> arch/arm/cpu/armv7/bcm281xx/reset.c | 2 +-
> arch/arm/cpu/armv7/bcmcygnus/reset.c | 2 +-
> arch/arm/cpu/armv7/bcmnsp/reset.c | 2 +-
> arch/arm/cpu/armv7/ls102xa/cpu.c | 2 +-
> arch/arm/cpu/armv7/s5p4418/cpu.c | 2 +-
> arch/arm/cpu/armv7/stv0991/reset.c| 2 +-
> arch/arm/cpu/armv7m/cpu.c | 2 +-
> arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 4 ++--
> arch/arm/cpu/armv8/s32v234/generic.c | 2 +-
> arch/arm/cpu/pxa/pxa2xx.c | 4 ++--
> arch/arm/cpu/sa1100/cpu.c | 2 +-
> arch/arm/lib/interrupts.c | 2 +-
> arch/arm/lib/interrupts_m.c | 2 +-
> arch/arm/lib/reset.c | 2 +-
> arch/arm/mach-at91/arm920t/reset.c| 2 +-
> arch/arm/mach-at91/arm926ejs/reset.c | 2 +-
> arch/arm/mach-at91/armv7/reset.c | 2 +-
> arch/arm/mach-bcm283x/reset.c | 2 +-
> arch/arm/mach-davinci/reset.c | 2 +-
> arch/arm/mach-exynos/soc.c| 2 +-
> arch/arm/mach-imx/imx8m/soc.c | 2 +-
> arch/arm/mach-imx/mx7ulp/soc.c| 2 +-
> arch/arm/mach-k3/common.c | 2 +-
> arch/arm/mach-keystone/ddr3.c | 4 ++--
> arch/arm/mach-keystone/init.c | 2 +-
> arch/arm/mach-ki
Re: [PATCH 3/4] Revert "lpc32xx: cpu: add support for soft reset"
Hello Sylvain,
On Tue, 2020-12-15 at 17:29 +, Sylvain Lemieux wrote:
> Hi,
>
> This functionality (soft vs hard reset) is used in multiple LPC32xx
> products with our custom hardware.
>
> If this support is remove from upstream, we will have to maintain this
> patch locally (out of tree).
My intention with this series is of course not to break existing and
useful functionality. Can you elaborate how you are making use of this
differentiation in its current form? Does some board code call
reset_cpu() directly, with different parameter values?
I would argue that at this time, the proper way to support both soft-reset
and hard-reset is via a sysreset driver, see
`drivers/sysreset/sysreset_psci.c` for a simple example. As I laid out in
the cover letter of this series, the `addr` parameter to reset_cpu() does
not really fit this purpose.
Regards,
--
Harald
> Sylvain Lemieux
>
> -Original Message-
> From: Harald Seiler
> Sent: Tuesday, December 15, 2020 10:48 AM
> To: u-boot@lists.denx.de
> Cc: Harald Seiler ; Tom Rini ; Simon Glass
> ; Sylvain Lemieux
> Subject: [PATCH 3/4] Revert "lpc32xx: cpu: add support for soft reset"
>
> This reverts commit 576007aec9a4a5f4f3dd1f690fb26a8c05ceb75f.
>
> The paramter passed to reset_cpu() no longer holds a meaning as all
> call-sites now pass the value 0. Thus, branching on it is essentially
> dead code and will just confuse future readers.
>
> Revert soft-reset support and just always perform a hard-reset for now.
> This is a preparation for removal of the reset_cpu() parameter across
> the entire tree in a later patch.
>
> Fixes: 576007aec9a4 ("lpc32xx: cpu: add support for soft reset")
> Cc: Sylvain Lemieux
> Signed-off-by: Harald Seiler
> ---
> arch/arm/mach-lpc32xx/cpu.c | 21 +
> 1 file changed, 5 insertions(+), 16 deletions(-)
>
> diff --git a/arch/arm/mach-lpc32xx/cpu.c b/arch/arm/mach-lpc32xx/cpu.c index
> 32af6206056b..7378192a33c2 100644
> --- a/arch/arm/mach-lpc32xx/cpu.c
> +++ b/arch/arm/mach-lpc32xx/cpu.c
> @@ -22,23 +22,12 @@ void reset_cpu(ulong addr)
> /* Enable watchdog clock */
> setbits_le32(>timclk_ctrl, CLK_TIMCLK_WATCHDOG);
>
> - /* To be compatible with the original U-Boot code:
> - * addr: - 0: perform hard reset.
> - * - !=0: perform a soft reset; i.e. "RESOUT_N" not asserted).
> */
> - if (addr == 0) {
> - /* Reset pulse length is 13005 peripheral clock frames */
> - writel(13000, >pulse);
> + /* Reset pulse length is 13005 peripheral clock frames */
> + writel(13000, >pulse);
>
> - /* Force WDOG_RESET2 and RESOUT_N signal active */
> - writel(WDTIM_MCTRL_RESFRC2 | WDTIM_MCTRL_RESFRC1
> - | WDTIM_MCTRL_M_RES2, >mctrl);
> - } else {
> - /* Force match output active */
> - writel(0x01, >emr);
> -
> - /* Internal reset on match output (no pulse on "RESOUT_N") */
> - writel(WDTIM_MCTRL_M_RES1, >mctrl);
> - }
> + /* Force WDOG_RESET2 and RESOUT_N signal active */
> + writel(WDTIM_MCTRL_RESFRC2 | WDTIM_MCTRL_RESFRC1 | WDTIM_MCTRL_M_RES2,
> + >mctrl);
>
> while (1)
> /* NOP */;
> --
> 2.26.2
>
--
Harald
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-62 Fax: +49-8142-66989-80 Email: h...@denx.de
[PATCH 4/4] reset: Remove addr parameter from reset_cpu()
Historically, the reset_cpu() function had an `addr` parameter which was
meant to pass in an address of the reset vector location, where the CPU
should reset to. This feature is no longer used anywhere in U-Boot as
all reset_cpu() implementations now ignore the passed value. Generic
code has been added which always calls reset_cpu() with `0` which means
this feature can no longer be used easily anyway.
Over time, many implementations seem to have "misunderstood" the
existence of this parameter as a way to customize/parameterize the reset
(e.g. COLD vs WARM resets). As this is not properly supported, the
code will almost always not do what it is intended to (because all
call-sites just call reset_cpu() with 0).
To avoid confusion and to clean up the codebase from unused left-overs
of the past, remove the `addr` parameter entirely. Code which intends
to support different kinds of resets should be rewritten as a sysreset
driver instead.
This transformation was done with the following coccinelle patch:
@@
expression argvalue;
@@
- reset_cpu(argvalue)
+ reset_cpu()
@@
identifier argname;
type argtype;
@@
- reset_cpu(argtype argname)
+ reset_cpu(void)
{ ... }
Signed-off-by: Harald Seiler
---
arch/arc/lib/reset.c| 4 ++--
arch/arm/cpu/arm920t/ep93xx/cpu.c | 2 +-
arch/arm/cpu/arm920t/imx/timer.c| 2 +-
arch/arm/cpu/arm926ejs/armada100/timer.c| 2 +-
arch/arm/cpu/arm926ejs/mx25/reset.c | 2 +-
arch/arm/cpu/arm926ejs/mx27/reset.c | 2 +-
arch/arm/cpu/arm926ejs/mxs/mxs.c| 4 ++--
arch/arm/cpu/arm926ejs/spear/reset.c| 2 +-
arch/arm/cpu/arm946es/cpu.c | 2 +-
arch/arm/cpu/armv7/bcm281xx/reset.c | 2 +-
arch/arm/cpu/armv7/bcmcygnus/reset.c| 2 +-
arch/arm/cpu/armv7/bcmnsp/reset.c | 2 +-
arch/arm/cpu/armv7/ls102xa/cpu.c| 2 +-
arch/arm/cpu/armv7/s5p4418/cpu.c| 2 +-
arch/arm/cpu/armv7/stv0991/reset.c | 2 +-
arch/arm/cpu/armv7m/cpu.c | 2 +-
arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 4 ++--
arch/arm/cpu/armv8/s32v234/generic.c| 2 +-
arch/arm/cpu/pxa/pxa2xx.c | 4 ++--
arch/arm/cpu/sa1100/cpu.c | 2 +-
arch/arm/lib/interrupts.c | 2 +-
arch/arm/lib/interrupts_m.c | 2 +-
arch/arm/lib/reset.c| 2 +-
arch/arm/mach-at91/arm920t/reset.c | 2 +-
arch/arm/mach-at91/arm926ejs/reset.c| 2 +-
arch/arm/mach-at91/armv7/reset.c| 2 +-
arch/arm/mach-bcm283x/reset.c | 2 +-
arch/arm/mach-davinci/reset.c | 2 +-
arch/arm/mach-exynos/soc.c | 2 +-
arch/arm/mach-imx/imx8m/soc.c | 2 +-
arch/arm/mach-imx/mx7ulp/soc.c | 2 +-
arch/arm/mach-k3/common.c | 2 +-
arch/arm/mach-keystone/ddr3.c | 4 ++--
arch/arm/mach-keystone/init.c | 2 +-
arch/arm/mach-kirkwood/cpu.c| 2 +-
arch/arm/mach-lpc32xx/cpu.c | 2 +-
arch/arm/mach-mediatek/mt7622/init.c| 2 +-
arch/arm/mach-mediatek/mt8512/init.c| 2 +-
arch/arm/mach-mediatek/mt8516/init.c| 2 +-
arch/arm/mach-mediatek/mt8518/init.c| 2 +-
arch/arm/mach-meson/board-common.c | 4 ++--
arch/arm/mach-mvebu/armada3700/cpu.c| 2 +-
arch/arm/mach-mvebu/armada8k/cpu.c | 2 +-
arch/arm/mach-mvebu/cpu.c | 2 +-
arch/arm/mach-octeontx/cpu.c| 2 +-
arch/arm/mach-octeontx2/cpu.c | 2 +-
arch/arm/mach-omap2/omap5/hwinit.c | 2 +-
arch/arm/mach-omap2/reset.c | 2 +-
arch/arm/mach-orion5x/cpu.c | 2 +-
arch/arm/mach-owl/soc.c | 2 +-
arch/arm/mach-socfpga/include/mach/reset_manager.h | 2 +-
arch/arm/mach-sunxi/board.c | 2 +-
arch/arm/mach-tegra/cmd_enterrcm.c | 2 +-
arch/arm/mach-tegra/pmc.c | 2 +-
arch/arm/mach-uniphier/arm32/psci.c | 2 +-
arch/arm/mach-uniphier/reset.c | 2 +-
arch/arm/mach-zynq/cpu.c| 2 +-
arch/arm/mach-zynqmp-r5/cpu.c |
[PATCH 2/4] board: ns3: Remove superfluous reset logic
The current implementation of reset_cpu() in the ns3 board code does not
archieve what it is supposed to (according to the comments), due to
a number of reasons:
1. The argument to reset_cpu() is _not_ actually passed from the
`reset` command, but is set to 0 in all call-sites (in this
specific case, see arch/arm/lib/reset.c). Thus, performing
different kinds of resets based on its value will not work as
expected.
2. Contrary to its documentation, the passed argument is not
interpreted, but a static `L3_RESET` define is used. The other
comment properly notes that this will always perform a L3 reset,
though.
3. The "parsing" of the static `L3_RESET` value is not even using the
upper and lower nibble as stated in the comment, but uses the last
two decimal digits of the value.
This is currently one of the only implementations left in U-Boot, which
make "use" of the value passed to reset_cpu(). As this is done under
false assumption (the value does not have any meaning anymore), it makes
sense to bring it into line with the rest and start ignoring the
parameter.
This is a preparation for removal of the reset_cpu() parameter across
the entire tree in a later patch.
Fixes: b5a152e7ca0b ("board: ns3: default reset type to L3")
Cc: Bharat Gooty
Cc: Rayagonda Kokatanur
Signed-off-by: Harald Seiler
---
board/broadcom/bcmns3/ns3.c | 22 ++
1 file changed, 2 insertions(+), 20 deletions(-)
diff --git a/board/broadcom/bcmns3/ns3.c b/board/broadcom/bcmns3/ns3.c
index 10ae344a06df..13dbbb826b2f 100644
--- a/board/broadcom/bcmns3/ns3.c
+++ b/board/broadcom/bcmns3/ns3.c
@@ -14,9 +14,6 @@
#include
#include
-/* Default reset-level = 3 and strap-val = 0 */
-#define L3_RESET 30
-
#define BANK_OFFSET(bank) ((u64)BCM_NS3_DDR_INFO_BASE + 8 + ((bank) * 16))
/*
@@ -189,23 +186,8 @@ ulong board_get_usable_ram_top(ulong total_size)
void reset_cpu(ulong level)
{
- u32 reset_level, strap_val;
-
- /* Default reset type is L3 reset */
- if (!level) {
- /*
-* Encoding: U-Boot reset command expects decimal argument,
-* Boot strap val: Bits[3:0]
-* reset level: Bits[7:4]
-*/
- strap_val = L3_RESET % 10;
- level = L3_RESET / 10;
- reset_level = level % 10;
- psci_system_reset2(reset_level, strap_val);
- } else {
- /* U-Boot cmd "reset" with any arg will trigger L1 reset */
- psci_system_reset();
- }
+ /* Perform a level 3 reset */
+ psci_system_reset2(3, 0);
}
#ifdef CONFIG_OF_BOARD_SETUP
--
2.26.2
[PATCH 3/4] Revert "lpc32xx: cpu: add support for soft reset"
This reverts commit 576007aec9a4a5f4f3dd1f690fb26a8c05ceb75f.
The paramter passed to reset_cpu() no longer holds a meaning as all
call-sites now pass the value 0. Thus, branching on it is essentially
dead code and will just confuse future readers.
Revert soft-reset support and just always perform a hard-reset for now.
This is a preparation for removal of the reset_cpu() parameter across
the entire tree in a later patch.
Fixes: 576007aec9a4 ("lpc32xx: cpu: add support for soft reset")
Cc: Sylvain Lemieux
Signed-off-by: Harald Seiler
---
arch/arm/mach-lpc32xx/cpu.c | 21 +
1 file changed, 5 insertions(+), 16 deletions(-)
diff --git a/arch/arm/mach-lpc32xx/cpu.c b/arch/arm/mach-lpc32xx/cpu.c
index 32af6206056b..7378192a33c2 100644
--- a/arch/arm/mach-lpc32xx/cpu.c
+++ b/arch/arm/mach-lpc32xx/cpu.c
@@ -22,23 +22,12 @@ void reset_cpu(ulong addr)
/* Enable watchdog clock */
setbits_le32(>timclk_ctrl, CLK_TIMCLK_WATCHDOG);
- /* To be compatible with the original U-Boot code:
-* addr: - 0: perform hard reset.
-* - !=0: perform a soft reset; i.e. "RESOUT_N" not asserted). */
- if (addr == 0) {
- /* Reset pulse length is 13005 peripheral clock frames */
- writel(13000, >pulse);
+ /* Reset pulse length is 13005 peripheral clock frames */
+ writel(13000, >pulse);
- /* Force WDOG_RESET2 and RESOUT_N signal active */
- writel(WDTIM_MCTRL_RESFRC2 | WDTIM_MCTRL_RESFRC1
- | WDTIM_MCTRL_M_RES2, >mctrl);
- } else {
- /* Force match output active */
- writel(0x01, >emr);
-
- /* Internal reset on match output (no pulse on "RESOUT_N") */
- writel(WDTIM_MCTRL_M_RES1, >mctrl);
- }
+ /* Force WDOG_RESET2 and RESOUT_N signal active */
+ writel(WDTIM_MCTRL_RESFRC2 | WDTIM_MCTRL_RESFRC1 | WDTIM_MCTRL_M_RES2,
+ >mctrl);
while (1)
/* NOP */;
--
2.26.2
[PATCH 1/4] nds32: Remove dead reset_cpu() implementation
nds32 is one of the only architectures which still have a reset_cpu() implementation that makes use of the `addr` parameter. The rest of U-Boot now ignores it and passes 0 everywhere. It turns out that even here, reset_cpu() is no longer referenced anywhere; reset is either not implemented (e.g. ae3xx) or realized using a WDT (e.g. ag101). Remove this left-over implementation in preparation for the removal of the `addr` parameter in the entire tree. Cc: Rick Chen Signed-off-by: Harald Seiler --- arch/nds32/cpu/n1213/start.S | 22 -- 1 file changed, 22 deletions(-) diff --git a/arch/nds32/cpu/n1213/start.S b/arch/nds32/cpu/n1213/start.S index 386c1998dcef..3395721552a3 100644 --- a/arch/nds32/cpu/n1213/start.S +++ b/arch/nds32/cpu/n1213/start.S @@ -500,25 +500,3 @@ software_interrupt: bal do_interruption .align 5 - -/* - * void reset_cpu(ulong addr); - * $r0: input address to jump to - */ -.globl reset_cpu -reset_cpu: -/* No need to disable MMU because we never enable it */ - - bal invalidate_icac - bal invalidate_dcac - mfsr$p0, $MMU_CFG - andi$p0, $p0, 0x3 ! MMPS - li $p1, 0x2! TLB MMU - bne $p0, $p1, 1f - tlbop flushall! Flush TLB -1: - mfsr$p0, MR_CAC_CTL ! Get the $CACHE_CTL reg - li $p1, DIS_DCAC - and $p0, $p0, $p1 ! Clear the DC_EN bit - mtsr$p0, MR_CAC_CTL ! Write back the $CACHE_CTL reg - br $r0 ! Jump to the input address -- 2.26.2
[PATCH 0/4] Remove addr parameter from reset_cpu()
Hi,
this is something I had on my mind for a longer time but never got
around to actually do until now ... A while back, while working on the
patchset that led to commit c5635a032a4b ("ARM: imx8m: Don't use the
addr parameter of reset_cpu()"), I noticed that the `addr` parameter of
reset_cpu() seems to not actually hold any meaningful value. All
call-sites in the current tree just pass 0 and the vast majority of
reset_cpu() implementations actually ignore the parameter.
I dug a bit deeper to find out why this `addr` parameter exists in the
first place and found out that it's mostly a legacy artifact:
Historically, the reset_cpu() function had this `addr` parameter to
pass an address of a reset vector location, where the CPU should
reset to.
The times where this was used are long gone and the only trace it left
is some (dead) code for the NDS32 arch. The `addr` parameter lived on
and it looks like it was sometimes used as a way to indicate different
types of resets (e.g. COLD vs WARM).
Today, however, reset_cpu() is only ever called with `addr` 0 in the
mainline tree and as such, any code that gives a meaning to the `addr`
value will only ever follow the `addr == 0` branch. This is probably
not what the authors intended and as it seems quite unobvious to me,
I think the best way forward is to remove the `addr` parameter entirely.
This removes any ambiguity in the "contract" of reset_cpu() and thus
hopefully prevents more code being added which wrongly assumes that the
parameter can be used for any meaningful purpose. Instead, code which
wants to properly support multiple reset types needs to be implemented
as a sysreset driver.
I did this API change via a coccinelle patch, see "reset: Remove addr
parameter from reset_cpu()" for details. I also ran buildman for all
boards I could, to verify that everything still compiles. One notable
exception is NDS32 because I couldn't get the compiler to work there ...
Regards,
Harald
Harald Seiler (4):
nds32: Remove dead reset_cpu() implementation
board: ns3: Remove superfluous reset logic
Revert "lpc32xx: cpu: add support for soft reset"
reset: Remove addr parameter from reset_cpu()
arch/arc/lib/reset.c | 4 ++--
arch/arm/cpu/arm920t/ep93xx/cpu.c | 2 +-
arch/arm/cpu/arm920t/imx/timer.c | 2 +-
arch/arm/cpu/arm926ejs/armada100/timer.c | 2 +-
arch/arm/cpu/arm926ejs/mx25/reset.c | 2 +-
arch/arm/cpu/arm926ejs/mx27/reset.c | 2 +-
arch/arm/cpu/arm926ejs/mxs/mxs.c | 4 ++--
arch/arm/cpu/arm926ejs/spear/reset.c | 2 +-
arch/arm/cpu/arm946es/cpu.c | 2 +-
arch/arm/cpu/armv7/bcm281xx/reset.c | 2 +-
arch/arm/cpu/armv7/bcmcygnus/reset.c | 2 +-
arch/arm/cpu/armv7/bcmnsp/reset.c | 2 +-
arch/arm/cpu/armv7/ls102xa/cpu.c | 2 +-
arch/arm/cpu/armv7/s5p4418/cpu.c | 2 +-
arch/arm/cpu/armv7/stv0991/reset.c| 2 +-
arch/arm/cpu/armv7m/cpu.c | 2 +-
arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 4 ++--
arch/arm/cpu/armv8/s32v234/generic.c | 2 +-
arch/arm/cpu/pxa/pxa2xx.c | 4 ++--
arch/arm/cpu/sa1100/cpu.c | 2 +-
arch/arm/lib/interrupts.c | 2 +-
arch/arm/lib/interrupts_m.c | 2 +-
arch/arm/lib/reset.c | 2 +-
arch/arm/mach-at91/arm920t/reset.c| 2 +-
arch/arm/mach-at91/arm926ejs/reset.c | 2 +-
arch/arm/mach-at91/armv7/reset.c | 2 +-
arch/arm/mach-bcm283x/reset.c | 2 +-
arch/arm/mach-davinci/reset.c | 2 +-
arch/arm/mach-exynos/soc.c| 2 +-
arch/arm/mach-imx/imx8m/soc.c | 2 +-
arch/arm/mach-imx/mx7ulp/soc.c| 2 +-
arch/arm/mach-k3/common.c | 2 +-
arch/arm/mach-keystone/ddr3.c | 4 ++--
arch/arm/mach-keystone/init.c | 2 +-
arch/arm/mach-kirkwood/cpu.c | 2 +-
arch/arm/mach-lpc32xx/cpu.c | 23 +-
arch/arm/mach-mediatek/mt7622/init.c | 2 +-
arch/arm/mach-mediatek/mt8512/init.c | 2 +-
arch/arm/mach-mediatek/mt8516/init.c | 2 +-
arch/arm/mach-mediatek/mt8518/init.c | 2 +-
arch/arm/mach-meson/board-common.c| 4 ++--
arch/arm/mach-mvebu/armada3700/cpu.c | 2 +-
arch/arm/mach-mvebu/armada8k/cpu.c| 2 +-
arch/arm/mach-mvebu/cpu.c | 2 +-
arch/arm/mach-octeontx/cpu.c | 2 +-
arch/arm/mach-octeontx2/cpu.c | 2 +-
arch/arm/mach-omap2/omap5/hwinit.c| 2 +-
arch/arm/mach-omap2/reset.c | 2 +-
arch/arm/mach-orion5x/cpu.c | 2 +-
arch/arm/mach-owl/soc.c
Re: [PATCH 6/6] env: Add support for explicit write access list
Hi Marek,
On Wed, 2020-06-03 at 02:01 +0200, Marek Vasut wrote:
> This option marks any U-Boot variable which does not have explicit 'w'
> writeable flag set as read-only. This way the environment can be locked
> down and only variables explicitly configured to be writeable can ever
> be changed by either 'env import', 'env set' or loading user environment
> from environment storage.
I haven't yet been able to get to the bottom of it but this patch seems to
regress the `envtools` build for me. Here is the rather weird error message:
HOSTCC tools/env/fw_env_main.o
In file included from tools/env/fw_env.c:15:
include/env_flags.h:27:22: error: missing binary operator before token "("
#if CONFIG_IS_ENABLED(ENV_WRITEABLE_LIST)
^
Any idea what could be the cause for this?
--
Harald
> Signed-off-by: Marek Vasut
> ---
> env/Kconfig | 8
> env/flags.c | 92 +++--
> include/env_flags.h | 6 ++-
> lib/hashtable.c | 5 ++-
> 4 files changed, 98 insertions(+), 13 deletions(-)
>
> diff --git a/env/Kconfig b/env/Kconfig
> index 8166e5df91..f53a1457fb 100644
> --- a/env/Kconfig
> +++ b/env/Kconfig
> @@ -613,6 +613,14 @@ config ENV_APPEND
> with newly imported data. This may be used in combination with static
> flags to e.g. to protect variables which must not be modified.
>
> +config ENV_WRITEABLE_LIST
> + bool "Permit write access only to listed variables"
> + default n
> + help
> + If defined, only environment variables which explicitly set the 'w'
> + writeable flag can be written and modified at runtime. No variables
> + can be otherwise created, written or imported into the environment.
> +
> config ENV_ACCESS_IGNORE_FORCE
> bool "Block forced environment operations"
> default n
> diff --git a/env/flags.c b/env/flags.c
> index f7a53775c4..a2f6c1a3ec 100644
> --- a/env/flags.c
> +++ b/env/flags.c
> @@ -28,8 +28,15 @@
> #define ENV_FLAGS_NET_VARTYPE_REPS ""
> #endif
>
> +#if CONFIG_IS_ENABLED(ENV_WRITEABLE_LIST)
> +#define ENV_FLAGS_WRITEABLE_VARACCESS_REPS "w"
> +#else
> +#define ENV_FLAGS_WRITEABLE_VARACCESS_REPS ""
> +#endif
> +
> static const char env_flags_vartype_rep[] = "sdxb"
> ENV_FLAGS_NET_VARTYPE_REPS;
> -static const char env_flags_varaccess_rep[] = "aroc";
> +static const char env_flags_varaccess_rep[] =
> + "aroc" ENV_FLAGS_WRITEABLE_VARACCESS_REPS;
> static const int env_flags_varaccess_mask[] = {
> 0,
> ENV_FLAGS_VARACCESS_PREVENT_DELETE |
> @@ -38,7 +45,11 @@ static const int env_flags_varaccess_mask[] = {
> ENV_FLAGS_VARACCESS_PREVENT_DELETE |
> ENV_FLAGS_VARACCESS_PREVENT_OVERWR,
> ENV_FLAGS_VARACCESS_PREVENT_DELETE |
> - ENV_FLAGS_VARACCESS_PREVENT_NONDEF_OVERWR};
> + ENV_FLAGS_VARACCESS_PREVENT_NONDEF_OVERWR,
> +#if CONFIG_IS_ENABLED(ENV_WRITEABLE_LIST)
> + ENV_FLAGS_VARACCESS_WRITEABLE,
> +#endif
> + };
>
> #ifdef CONFIG_CMD_ENV_FLAGS
> static const char * const env_flags_vartype_names[] = {
> @@ -56,6 +67,9 @@ static const char * const env_flags_varaccess_names[] = {
> "read-only",
> "write-once",
> "change-default",
> +#if CONFIG_IS_ENABLED(ENV_WRITEABLE_LIST)
> + "writeable",
> +#endif
> };
>
> /*
> @@ -130,21 +144,33 @@ enum env_flags_vartype env_flags_parse_vartype(const
> char *flags)
> */
> enum env_flags_varaccess env_flags_parse_varaccess(const char *flags)
> {
> +#if CONFIG_IS_ENABLED(ENV_WRITEABLE_LIST)
> + enum env_flags_varaccess va_default = env_flags_varaccess_readonly;
> +#else
> + enum env_flags_varaccess va_default = env_flags_varaccess_any;
> +#endif
> + enum env_flags_varaccess va;
> char *access;
>
> if (strlen(flags) <= ENV_FLAGS_VARACCESS_LOC)
> - return env_flags_varaccess_any;
> + return va_default;
>
> access = strchr(env_flags_varaccess_rep,
> flags[ENV_FLAGS_VARACCESS_LOC]);
>
> - if (access != NULL)
> - return (enum env_flags_varaccess)
> + if (access != NULL) {
> + va = (enum env_flags_varaccess)
> (access - _flags_varaccess_rep[0]);
> +#if CONFIG_IS_ENABLED(ENV_WRITEABLE_LIST)
> + if (va != env_flags_varaccess_writeable)
> + return env_flags_varaccess_readonly;
> +#endif
> + return va;
> + }
>
> printf("## Warning: Unknown environment variable access method '%c'\n",
> flags[ENV_FLAGS_VARACCESS_LOC]);
> - return env_flags_varaccess_any;
> + return va_default;
> }
>
> /*
> @@ -152,17 +178,29 @@ enum env_flags_varaccess
> env_flags_parse_varaccess(const char *flags)
> */
> enum env_flags_varaccess env_flags_parse_varaccess_from_binflags(int
> binflags)
> {
> +#if CONFIG_IS_ENABLED(ENV_WRITEABLE_LIST)
> + enum env_flags_varaccess va_default =
[PATCH] common: hash: Remove a debug printf statement
Remove a left-over debug printf that was introduced with SHA512 support.
Fixes: d16b38f42704 ("Add support for SHA384 and SHA512")
Signed-off-by: Harald Seiler
---
Notes:
Just noticed the SHA512 patch seems to still contain
a left-over debug printf.
common/hash.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/common/hash.c b/common/hash.c
index 5c75848b7d92..05238a8ba91e 100644
--- a/common/hash.c
+++ b/common/hash.c
@@ -146,8 +146,6 @@ static int hash_finish_sha512(struct hash_algo *algo, void
*ctx, void
if (size < algo->digest_size)
return -1;
- printf("hello world\n");
-
sha512_finish((sha512_context *)ctx, dest_buf);
free(ctx);
return 0;
--
2.25.4
[PATCH] tools: fw_env: Fix warning when reading too little
When using CONFIG_ENV_IS_IN_FAT and the config-file specifies a size
larger than what U-Boot wrote into the env-file, a confusing error
message is shown:
$ fw_printenv
Read error on /boot/uboot.env: Success
Fix this by showing a different error message when read returns too
little data.
Signed-off-by: Harald Seiler
---
tools/env/fw_env.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index 8734663cd4c7..c6378ecf34f6 100644
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -946,11 +946,17 @@ static int flash_read_buf(int dev, int fd, void *buf,
size_t count,
lseek(fd, blockstart + block_seek, SEEK_SET);
rc = read(fd, buf + processed, readlen);
- if (rc != readlen) {
+ if (rc == -1) {
fprintf(stderr, "Read error on %s: %s\n",
DEVNAME(dev), strerror(errno));
return -1;
}
+ if (rc != readlen) {
+ fprintf(stderr, "Read error on %s: "
+ "Attempted to read %d bytes but got %d\n",
+ DEVNAME(dev), readlen, rc);
+ return -1;
+ }
#ifdef DEBUG
fprintf(stderr, "Read 0x%x bytes at 0x%llx on %s\n",
rc, (unsigned long long)blockstart + block_seek,
--
2.25.4
Re: [PATCH] imx8mp_evk: Make SPL binary size smaller
Hello Fabio,
On Thu, 2020-05-07 at 09:00 -0300, Fabio Estevam wrote:
> Commit f24dea4e1b52 ("ARM: imx8m: Fix reset in SPL on NXP iMX8MP EVK") caused
> the u-boot-spl.bin binary size to grow by around 2000 bytes, which makes the
> board to fail to boot.
Just to check, the addition of the clock driver probably lead to the same
issue? So the clock driver is potentially working, it is just too big for
SPL?
> Reduce the SPL size by the same amount so that it can boot again.
>
> Further SPL reduction work is needed, such as removing driver model support
> in SPL.
>
> Just to provide a comparison: NXP U-Boot tree has a SPL binary size of 64kB
> versus 96KB in U-Boot mainline.
>
> Signed-off-by: Fabio Estevam
> ---
> Hi,
>
> I plan to reduce SPL size even further by removing SPL_DM=y, but this
> needs more time to accomplish, so I prefer to give a small SPL reduction
> at this time, just to allow the board to boot again.
>
> Also, will try to come up with a SPL size detection in build time, as it
> is hard to debug such issues in run-time.
The SPL linker script already has checks for SPL size. See
arch/arm/cpu/u-boot-spl.lds line 81 and following. Maybe you just need to
properly set those config-options for your board?
> configs/imx8mp_evk_defconfig | 4
> 1 file changed, 4 deletions(-)
>
> diff --git a/configs/imx8mp_evk_defconfig b/configs/imx8mp_evk_defconfig
> index 44b2935f69..00f4ccbe0d 100644
> --- a/configs/imx8mp_evk_defconfig
> +++ b/configs/imx8mp_evk_defconfig
> @@ -33,7 +33,6 @@ CONFIG_SPL_BOOTROM_SUPPORT=y
> CONFIG_SPL_SEPARATE_BSS=y
> CONFIG_SPL_I2C_SUPPORT=y
> CONFIG_SPL_POWER_SUPPORT=y
> -CONFIG_SPL_WATCHDOG_SUPPORT=y
> CONFIG_HUSH_PARSER=y
> CONFIG_SYS_PROMPT="u-boot=> "
> # CONFIG_CMD_EXPORTENV is not set
> @@ -79,8 +78,5 @@ CONFIG_DM_REGULATOR_FIXED=y
> CONFIG_DM_REGULATOR_GPIO=y
> CONFIG_MXC_UART=y
> CONFIG_SYSRESET=y
> -CONFIG_SPL_SYSRESET=y
> CONFIG_SYSRESET_PSCI=y
> -CONFIG_SYSRESET_WATCHDOG=y
> # CONFIG_WATCHDOG is not set
> -CONFIG_IMX_WATCHDOG=y
--
Harald
Re: [PATCH 6/8] ARM: imx8m: Fix reset in SPL on NXP iMX8MP EVK
Hello Fabio, On Mon, 2020-05-04 at 12:28 -0300, Fabio Estevam wrote: > On Mon, May 4, 2020 at 12:21 PM Harald Seiler wrote: > > > With or without the revert? > > When I change the defconfig like: > > --- a/configs/imx8mp_evk_defconfig > +++ b/configs/imx8mp_evk_defconfig > @@ -57,7 +57,9 @@ CONFIG_ENV_IS_IN_MMC=y > CONFIG_SYS_RELOC_GD_ENV_ADDR=y > CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG=y > CONFIG_SPL_DM=y > +CONFIG_SPL_CLK_COMPOSITE_CCF=y > CONFIG_CLK_COMPOSITE_CCF=y > +CONFIG_SPL_CLK_IMX8MP=y > CONFIG_CLK_IMX8MP=y > CONFIG_MXC_GPIO=y > CONFIG_DM_PCA953X=y > > It prints a single SPL line with the revert and also without the revert. Ok, I guess this means the imx8mp clock driver in SPL is broken and we can't easily fix the DM_WATCHDOG issue without it. I don't really know much about imx8 nor do I have any hardware which I could use to debug with so I can't help much with this. Maybe Peng, who wrote the clock driver, can comment? Otherwise, the series which contains this patch also fixed the non-DM reset in SPL so we could revert back to that if all else fails ... -- Harald
Re: [PATCH 6/8] ARM: imx8m: Fix reset in SPL on NXP iMX8MP EVK
On Mon, 2020-05-04 at 12:18 -0300, Fabio Estevam wrote:
> On Mon, May 4, 2020 at 12:05 PM Harald Seiler wrote:
>
> > "Failed to find clock node. Check device tree" comes from spl_board_init()
> > in board/freescale/imx8mp_evk/spl.c; line 56:
> >
> > ret = uclass_get_device_by_name(UCLASS_CLK,
> > "clock-controller@3038",
> > );
> >
> > I see that wdog1 references the same clock here:
> >
> > arch/arm/dts/imx8mp.dtsi; line 222:
> >
> > wdog1: watchdog@3028 {
> > compatible = "fsl,imx8mp-wdt", "fsl,imx21-wdt";
> > reg = <0x3028 0x1>;
> > interrupts = ;
> > clocks = < IMX8MP_CLK_WDOG1_ROOT>;
> > status = "disabled";
> > };
> >
> > So the two issues are very likely related. The relevant clock's node is
> > also enabled for SPL so I think the driver might be missing here. Maybe
> > you need to add
> >
> > CONFIG_SPL_CLK_IMX8MP=y
> >
> > to your defconfig?
>
> I tried like this:
>
> --- a/configs/imx8mp_evk_defconfig
> +++ b/configs/imx8mp_evk_defconfig
> @@ -57,7 +57,9 @@ CONFIG_ENV_IS_IN_MMC=y
> CONFIG_SYS_RELOC_GD_ENV_ADDR=y
> CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG=y
> CONFIG_SPL_DM=y
> +CONFIG_SPL_CLK_COMPOSITE_CCF=y
> CONFIG_CLK_COMPOSITE_CCF=y
> +CONFIG_SPL_CLK_IMX8MP=y
> CONFIG_CLK_IMX8MP=y
> CONFIG_MXC_GPIO=y
> CONFIG_DM_PCA953X=y
>
> but still only prints:
>
> U-Boot SPL 2020.07-rc1-00014-g8142a97d54-dirty (May 04 2020 - 12:16:25 -0300)
With or without the revert?
--
Harald
Re: [PATCH 6/8] ARM: imx8m: Fix reset in SPL on NXP iMX8MP EVK
Hello,
On Mon, 2020-05-04 at 16:32 +0200, Marek Vasut wrote:
> On 5/4/20 4:27 PM, Fabio Estevam wrote:
> > Hi,
> >
> > On Wed, Apr 29, 2020 at 10:05 AM Harald Seiler wrote:
> > > From: Marek Vasut
> > >
> > > Board files should not re-implement do_reset() to work around this
> > > function not being defined in for specific configurations. Rather,
> > > the fix is to compile in drivers which implement this properly.
> > > This patch enables sysreset and watchdog drivers in SPL and ties
> > > them together to implement the same as the do_reset() hack in the
> > > board file, except correctly in the DM/DT framework.
> > >
> > > Signed-off-by: Marek Vasut
> > > Cc: Fabio Estevam
> > > Cc: Flavio Suligoi
> > > Cc: Harald Seiler
> > > Cc: Igor Opaniuk
> > > Cc: Marcel Ziswiler
> > > Cc: Oleksandr Suvorov
> > > Cc: Peng Fan
> > > Cc: Stefano Babic
> >
> > I noticed that this patch breaks the boot on i.MX8MP EVK. Only the
> > following line is printed on boot:
> >
> > U-Boot SPL 2020.07-rc1-00014-g8142a97d54 (May 04 2020 - 11:15:50 -0300)
> >
> > If I revert this patch I can boot it again:
> >
> > U-Boot SPL 2020.07-rc1-00015-g02cd8db94f (May 04 2020 - 11:17:25 -0300)
> > Normal Boot
> > Failed to find clock node. Check device tree
> > WDT: Not found!
> > Trying to boot from BOOTROM
> > image offset 0x8000, pagesize 0x200, ivt offset 0x0
> >
> >
> > U-Boot 2020.07-rc1-00015-g02cd8db94f (May 04 2020 - 11:17:25 -0300)
> >
> > CPU: Freescale i.MX8MP rev1.0 at 1000 MHz
> > Reset cause: POR
> > Model: NXP i.MX8MPlus EVK board
> > DRAM: 6 GiB
> > MMC: FSL_SDHC: 1, FSL_SDHC: 2
> > Loading Environment from MMC... OK
> > In:serial
> > Out: serial
> > Err: serial
> > Net: No ethernet found.
> > Hit any key to stop autoboot: 0
> > u-boot=>
> >
> > The "Failed to find clock node. Check device tree" looks suspicious.
>
> The "WDT: not found!" is probably the root cause of your problem.
> Maybe the WDT driver fails to probe because it can't resolve it's clock
> phandle ("Failed to find clock node")? So maybe you need to fix your
> clock in SPL.
"Failed to find clock node. Check device tree" comes from spl_board_init()
in board/freescale/imx8mp_evk/spl.c; line 56:
ret = uclass_get_device_by_name(UCLASS_CLK,
"clock-controller@3038",
);
I see that wdog1 references the same clock here:
arch/arm/dts/imx8mp.dtsi; line 222:
wdog1: watchdog@3028 {
compatible = "fsl,imx8mp-wdt", "fsl,imx21-wdt";
reg = <0x3028 0x1>;
interrupts = ;
clocks = < IMX8MP_CLK_WDOG1_ROOT>;
status = "disabled";
};
So the two issues are very likely related. The relevant clock's node is
also enabled for SPL so I think the driver might be missing here. Maybe
you need to add
CONFIG_SPL_CLK_IMX8MP=y
to your defconfig?
--
Harald
Re: [PATCH] imx: spl: Fix build-time test for CONFIG_SPL_FS_FAT
Hello Jan, On Wed, 2020-04-29 at 19:23 +0200, Jan Kiszka wrote: > From: Jan Kiszka > > This was already changed in 0c3a9ed409a5 but apparently missed when > adding 9d86dbd9cf9d. I sent a very similar patch [1] as part of my "Fix spl_mmc_boot_mode() implementation for IMX" series [2] which tries to solve a few more issues in this particular function. [1]: http://patchwork.ozlabs.org/project/uboot/patch/20200423110753.51231-6-...@denx.de/ [2]: http://patchwork.ozlabs.org/project/uboot/list/?series=172241 > Signed-off-by: Jan Kiszka > --- > > Found by chance while working on other code. Not tested. > > arch/arm/mach-imx/spl.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c > index 49bb3b928d..bfdb3f3ada 100644 > --- a/arch/arm/mach-imx/spl.c > +++ b/arch/arm/mach-imx/spl.c > @@ -197,7 +197,7 @@ u32 spl_mmc_boot_mode(const u32 boot_device) > case SD1_BOOT: > case SD2_BOOT: > case SD3_BOOT: > -#if defined(CONFIG_SPL_FAT_SUPPORT) > +#if defined(CONFIG_SPL_FS_FAT) > return MMCSD_MODE_FS; > #else > return MMCSD_MODE_RAW; > @@ -206,7 +206,7 @@ u32 spl_mmc_boot_mode(const u32 boot_device) > case MMC1_BOOT: > case MMC2_BOOT: > case MMC3_BOOT: > -#if defined(CONFIG_SPL_FAT_SUPPORT) > +#if defined(CONFIG_SPL_FS_FAT) > return MMCSD_MODE_FS; > #elif defined(CONFIG_SUPPORT_EMMC_BOOT) > return MMCSD_MODE_EMMCBOOT; > -- > 2.16.4 Regards, -- Harald
Re: [PATCH 3/8] ARM: imx8m: Don't use the addr parameter of reset_cpu()
Hello Fabio, On Wed, 2020-04-29 at 10:14 -0300, Fabio Estevam wrote: > Hi Harald, > > On Wed, Apr 29, 2020 at 10:04 AM Harald Seiler wrote: > > From: Claudius Heine > > > > imx8m has the only implementation of reset_cpu() which does not ignore > > the addr parameter and instead gives it some meaning as the base address > > Unrelated to this patch, but maybe the reset_cpu() function should > change its parameter from addr to void instead? Yes! I have already prepared a patchset for that but as the parameter is still used here I could not send that yet. The parameter was introduces back in the days because some weird architecture needed the reset-vector address to perform a soft reset. This is a long-gone platform and this parameter is thus no longer necessary. Regards, -- Harald
[PATCH 6/8] ARM: imx8m: Fix reset in SPL on NXP iMX8MP EVK
From: Marek Vasut
Board files should not re-implement do_reset() to work around this
function not being defined in for specific configurations. Rather,
the fix is to compile in drivers which implement this properly.
This patch enables sysreset and watchdog drivers in SPL and ties
them together to implement the same as the do_reset() hack in the
board file, except correctly in the DM/DT framework.
Signed-off-by: Marek Vasut
Cc: Fabio Estevam
Cc: Flavio Suligoi
Cc: Harald Seiler
Cc: Igor Opaniuk
Cc: Marcel Ziswiler
Cc: Oleksandr Suvorov
Cc: Peng Fan
Cc: Stefano Babic
---
arch/arm/dts/imx8mp-evk-u-boot.dtsi | 12
board/freescale/imx8mp_evk/spl.c| 9 -
configs/imx8mp_evk_defconfig| 4
3 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/arch/arm/dts/imx8mp-evk-u-boot.dtsi
b/arch/arm/dts/imx8mp-evk-u-boot.dtsi
index 4675ada0a0a9..24a93ac2d690 100644
--- a/arch/arm/dts/imx8mp-evk-u-boot.dtsi
+++ b/arch/arm/dts/imx8mp-evk-u-boot.dtsi
@@ -3,6 +3,14 @@
* Copyright 2019 NXP
*/
+/ {
+ wdt-reboot {
+ compatible = "wdt-reboot";
+ wdt = <>;
+ u-boot,dm-spl;
+ };
+};
+
&{/soc@0} {
u-boot,dm-pre-reloc;
u-boot,dm-spl;
@@ -119,3 +127,7 @@
{
u-boot,dm-spl;
};
+
+ {
+ u-boot,dm-spl;
+};
diff --git a/board/freescale/imx8mp_evk/spl.c b/board/freescale/imx8mp_evk/spl.c
index 0b20668e2b30..39c1dae684ac 100644
--- a/board/freescale/imx8mp_evk/spl.c
+++ b/board/freescale/imx8mp_evk/spl.c
@@ -149,12 +149,3 @@ void board_init_f(ulong dummy)
board_init_r(NULL, 0);
}
-
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
-{
- puts("resetting ...\n");
-
- reset_cpu(WDOG1_BASE_ADDR);
-
- return 0;
-}
diff --git a/configs/imx8mp_evk_defconfig b/configs/imx8mp_evk_defconfig
index ce6b342c3672..09ed7a89c9aa 100644
--- a/configs/imx8mp_evk_defconfig
+++ b/configs/imx8mp_evk_defconfig
@@ -81,4 +81,8 @@ CONFIG_DM_REGULATOR_FIXED=y
CONFIG_DM_REGULATOR_GPIO=y
CONFIG_MXC_UART=y
CONFIG_SYSRESET=y
+CONFIG_SPL_SYSRESET=y
CONFIG_SYSRESET_PSCI=y
+CONFIG_SYSRESET_WATCHDOG=y
+# CONFIG_WATCHDOG is not set
+CONFIG_IMX_WATCHDOG=y
--
2.26.2
[PATCH 5/8] ARM: imx8m: Fix reset in SPL on NXP iMX8MN EVK
From: Marek Vasut
Board files should not re-implement do_reset() to work around this
function not being defined in for specific configurations. Rather,
the fix is to compile in drivers which implement this properly.
This patch enables sysreset and watchdog drivers in SPL and ties
them together to implement the same as the do_reset() hack in the
board file, except correctly in the DM/DT framework.
Signed-off-by: Marek Vasut
Cc: Fabio Estevam
Cc: Flavio Suligoi
Cc: Harald Seiler
Cc: Igor Opaniuk
Cc: Marcel Ziswiler
Cc: Oleksandr Suvorov
Cc: Peng Fan
Cc: Stefano Babic
---
arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi | 12
board/freescale/imx8mn_evk/spl.c | 9 -
configs/imx8mn_ddr4_evk_defconfig| 5 +
3 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi
b/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi
index 8d61597e0ce0..4419679d4c66 100644
--- a/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi
+++ b/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi
@@ -3,6 +3,14 @@
* Copyright 2019 NXP
*/
+/ {
+ wdt-reboot {
+ compatible = "wdt-reboot";
+ wdt = <>;
+ u-boot,dm-spl;
+ };
+};
+
&{/soc@0} {
u-boot,dm-pre-reloc;
u-boot,dm-spl;
@@ -90,3 +98,7 @@
{
u-boot,dm-spl;
};
+
+ {
+ u-boot,dm-spl;
+};
diff --git a/board/freescale/imx8mn_evk/spl.c b/board/freescale/imx8mn_evk/spl.c
index 7aed14c52b68..45417b24464d 100644
--- a/board/freescale/imx8mn_evk/spl.c
+++ b/board/freescale/imx8mn_evk/spl.c
@@ -114,12 +114,3 @@ void board_init_f(ulong dummy)
board_init_r(NULL, 0);
}
-
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
-{
- puts("resetting ...\n");
-
- reset_cpu(WDOG1_BASE_ADDR);
-
- return 0;
-}
diff --git a/configs/imx8mn_ddr4_evk_defconfig
b/configs/imx8mn_ddr4_evk_defconfig
index f7485ab27270..224aeb6f0dfa 100644
--- a/configs/imx8mn_ddr4_evk_defconfig
+++ b/configs/imx8mn_ddr4_evk_defconfig
@@ -32,6 +32,7 @@ CONFIG_SPL_BOARD_INIT=y
CONFIG_SPL_BOOTROM_SUPPORT=y
CONFIG_SPL_SEPARATE_BSS=y
CONFIG_SPL_I2C_SUPPORT=y
+CONFIG_SPL_WATCHDOG_SUPPORT=y
CONFIG_HUSH_PARSER=y
CONFIG_SYS_PROMPT="u-boot=> "
# CONFIG_CMD_EXPORTENV is not set
@@ -76,5 +77,9 @@ CONFIG_DM_REGULATOR_FIXED=y
CONFIG_DM_REGULATOR_GPIO=y
CONFIG_MXC_UART=y
CONFIG_SYSRESET=y
+CONFIG_SPL_SYSRESET=y
CONFIG_SYSRESET_PSCI=y
+CONFIG_SYSRESET_WATCHDOG=y
CONFIG_DM_THERMAL=y
+# CONFIG_WATCHDOG is not set
+CONFIG_IMX_WATCHDOG=y
--
2.26.2
[PATCH 8/8] ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for them
From: Claudius Heine In case CONFIG_SYSRESET is set, do_reset() from reset.c will not be available anywere, even if SYSRESET is disabled for SPL/TPL. do_reset() is called from SPL for instance from the panic handler and PANIC_HANG is not set Signed-off-by: Claudius Heine --- arch/arm/lib/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile index 8482f5446c5c..b839aa7a5096 100644 --- a/arch/arm/lib/Makefile +++ b/arch/arm/lib/Makefile @@ -57,7 +57,7 @@ obj-y += interrupts_64.o else obj-y += interrupts.o endif -ifndef CONFIG_SYSRESET +ifndef CONFIG_$(SPL_TPL_)SYSRESET obj-y += reset.o endif -- 2.26.2
[PATCH 4/8] ARM: imx8m: Fix reset in SPL on NXP iMX8MM EVK
From: Marek Vasut
Board files should not re-implement do_reset() to work around this
function not being defined in for specific configurations. Rather,
the fix is to compile in drivers which implement this properly.
This patch enables sysreset and watchdog drivers in SPL and ties
them together to implement the same as the do_reset() hack in the
board file, except correctly in the DM/DT framework.
Signed-off-by: Marek Vasut
Cc: Fabio Estevam
Cc: Flavio Suligoi
Cc: Harald Seiler
Cc: Igor Opaniuk
Cc: Marcel Ziswiler
Cc: Oleksandr Suvorov
Cc: Peng Fan
Cc: Stefano Babic
---
arch/arm/dts/imx8mm-evk-u-boot.dtsi | 12
board/freescale/imx8mm_evk/spl.c| 9 -
configs/imx8mm_evk_defconfig| 5 +
3 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
index 3502602fbb86..b5c12105a9d1 100644
--- a/arch/arm/dts/imx8mm-evk-u-boot.dtsi
+++ b/arch/arm/dts/imx8mm-evk-u-boot.dtsi
@@ -3,6 +3,14 @@
* Copyright 2019 NXP
*/
+/ {
+ wdt-reboot {
+ compatible = "wdt-reboot";
+ wdt = <>;
+ u-boot,dm-spl;
+ };
+};
+
&{/soc@0} {
u-boot,dm-pre-reloc;
u-boot,dm-spl;
@@ -117,3 +125,7 @@
{
phy-reset-gpios = < 22 GPIO_ACTIVE_LOW>;
};
+
+ {
+ u-boot,dm-spl;
+};
diff --git a/board/freescale/imx8mm_evk/spl.c b/board/freescale/imx8mm_evk/spl.c
index 5d17f397cb68..4d34622465b3 100644
--- a/board/freescale/imx8mm_evk/spl.c
+++ b/board/freescale/imx8mm_evk/spl.c
@@ -161,12 +161,3 @@ void board_init_f(ulong dummy)
board_init_r(NULL, 0);
}
-
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
-{
- puts ("resetting ...\n");
-
- reset_cpu(WDOG1_BASE_ADDR);
-
- return 0;
-}
diff --git a/configs/imx8mm_evk_defconfig b/configs/imx8mm_evk_defconfig
index d988507bc330..120c76633066 100644
--- a/configs/imx8mm_evk_defconfig
+++ b/configs/imx8mm_evk_defconfig
@@ -30,6 +30,7 @@ CONFIG_SPL_BOARD_INIT=y
CONFIG_SPL_SEPARATE_BSS=y
CONFIG_SPL_I2C_SUPPORT=y
CONFIG_SPL_POWER_SUPPORT=y
+CONFIG_SPL_WATCHDOG_SUPPORT=y
CONFIG_HUSH_PARSER=y
CONFIG_SYS_PROMPT="u-boot=> "
# CONFIG_CMD_EXPORTENV is not set
@@ -82,5 +83,9 @@ CONFIG_DM_REGULATOR_FIXED=y
CONFIG_DM_REGULATOR_GPIO=y
CONFIG_MXC_UART=y
CONFIG_SYSRESET=y
+CONFIG_SPL_SYSRESET=y
CONFIG_SYSRESET_PSCI=y
+CONFIG_SYSRESET_WATCHDOG=y
CONFIG_DM_THERMAL=y
+# CONFIG_WATCHDOG is not set
+CONFIG_IMX_WATCHDOG=y
--
2.26.2
[PATCH 2/8] ARM: imx8m: Fix indentation of reset_cpu() function
Use proper code-style, tabs instead of spaces for indentation.
Signed-off-by: Harald Seiler
---
arch/arm/mach-imx/imx8m/soc.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c
index 0f17252e80b7..5b3fbe712c6b 100644
--- a/arch/arm/mach-imx/imx8m/soc.c
+++ b/arch/arm/mach-imx/imx8m/soc.c
@@ -385,18 +385,18 @@ int ft_system_setup(void *blob, bd_t *bd)
#if !CONFIG_IS_ENABLED(SYSRESET)
void reset_cpu(ulong addr)
{
- struct watchdog_regs *wdog = (struct watchdog_regs *)addr;
+ struct watchdog_regs *wdog = (struct watchdog_regs *)addr;
- if (!addr)
- wdog = (struct watchdog_regs *)WDOG1_BASE_ADDR;
+ if (!addr)
+ wdog = (struct watchdog_regs *)WDOG1_BASE_ADDR;
- /* Clear WDA to trigger WDOG_B immediately */
- writew((WCR_WDE | WCR_SRS), >wcr);
+ /* Clear WDA to trigger WDOG_B immediately */
+ writew((WCR_WDE | WCR_SRS), >wcr);
- while (1) {
- /*
-* spin for .5 seconds before reset
-*/
- }
+ while (1) {
+ /*
+* spin for .5 seconds before reset
+*/
+ }
}
#endif
--
2.26.2
[PATCH 7/8] ARM: imx8m: Fix reset in SPL on Toradex iMX8MM Verdin
From: Marek Vasut
Board files should not re-implement do_reset() to work around this
function not being defined in for specific configurations. Rather,
the fix is to compile in drivers which implement this properly.
This patch enables sysreset and watchdog drivers in SPL and ties
them together to implement the same as the do_reset() hack in the
board file, except correctly in the DM/DT framework.
Signed-off-by: Marek Vasut
Cc: Fabio Estevam
Cc: Flavio Suligoi
Cc: Harald Seiler
Cc: Igor Opaniuk
Cc: Marcel Ziswiler
Cc: Oleksandr Suvorov
Cc: Peng Fan
Cc: Stefano Babic
---
arch/arm/dts/imx8mm-verdin-u-boot.dtsi | 12
board/toradex/verdin-imx8mm/spl.c | 9 -
configs/verdin-imx8mm_defconfig| 5 +
3 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/arch/arm/dts/imx8mm-verdin-u-boot.dtsi
b/arch/arm/dts/imx8mm-verdin-u-boot.dtsi
index e60b9faee442..fe6bb9bf03cf 100644
--- a/arch/arm/dts/imx8mm-verdin-u-boot.dtsi
+++ b/arch/arm/dts/imx8mm-verdin-u-boot.dtsi
@@ -3,6 +3,14 @@
* Copyright 2020 Toradex
*/
+/ {
+ wdt-reboot {
+ compatible = "wdt-reboot";
+ wdt = <>;
+ u-boot,dm-spl;
+ };
+};
+
{
u-boot,dm-spl;
u-boot,dm-pre-reloc;
@@ -105,3 +113,7 @@
{
u-boot,dm-spl;
};
+
+ {
+ u-boot,dm-spl;
+};
diff --git a/board/toradex/verdin-imx8mm/spl.c
b/board/toradex/verdin-imx8mm/spl.c
index a5dc54082054..dc5bd84f332e 100644
--- a/board/toradex/verdin-imx8mm/spl.c
+++ b/board/toradex/verdin-imx8mm/spl.c
@@ -169,12 +169,3 @@ void board_init_f(ulong dummy)
board_init_r(NULL, 0);
}
-
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
-{
- puts("resetting ...\n");
-
- reset_cpu(WDOG1_BASE_ADDR);
-
- return 0;
-}
diff --git a/configs/verdin-imx8mm_defconfig b/configs/verdin-imx8mm_defconfig
index 590750e9b2d1..21f6aa308208 100644
--- a/configs/verdin-imx8mm_defconfig
+++ b/configs/verdin-imx8mm_defconfig
@@ -38,6 +38,7 @@ CONFIG_SPL_SEPARATE_BSS=y
CONFIG_SPL_I2C_SUPPORT=y
CONFIG_SPL_POWER_SUPPORT=y
CONFIG_SPL_USB_HOST_SUPPORT=y
+CONFIG_SPL_WATCHDOG_SUPPORT=y
CONFIG_SYS_PROMPT="Verdin iMX8MM # "
# CONFIG_BOOTM_NETBSD is not set
CONFIG_CMD_ASKENV=y
@@ -94,5 +95,9 @@ CONFIG_DM_REGULATOR_FIXED=y
CONFIG_DM_REGULATOR_GPIO=y
CONFIG_MXC_UART=y
CONFIG_SYSRESET=y
+CONFIG_SPL_SYSRESET=y
CONFIG_SYSRESET_PSCI=y
+CONFIG_SYSRESET_WATCHDOG=y
CONFIG_DM_THERMAL=y
+# CONFIG_WATCHDOG is not set
+CONFIG_IMX_WATCHDOG=y
--
2.26.2
[PATCH 3/8] ARM: imx8m: Don't use the addr parameter of reset_cpu()
From: Claudius Heine
imx8m has the only implementation of reset_cpu() which does not ignore
the addr parameter and instead gives it some meaning as the base address
of watchdog registers. This breaks convention with the rest of U-Boot
where the parameter is ignored and callers are passing in 0.
Fixes: d2041725e84b ("imx8m: restrict reset_cpu")
Co-developed-by: Harald Seiler
Signed-off-by: Harald Seiler
Signed-off-by: Claudius Heine
---
arch/arm/mach-imx/imx8m/soc.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c
index 5b3fbe712c6b..2fe1fa75fb05 100644
--- a/arch/arm/mach-imx/imx8m/soc.c
+++ b/arch/arm/mach-imx/imx8m/soc.c
@@ -385,10 +385,7 @@ int ft_system_setup(void *blob, bd_t *bd)
#if !CONFIG_IS_ENABLED(SYSRESET)
void reset_cpu(ulong addr)
{
- struct watchdog_regs *wdog = (struct watchdog_regs *)addr;
-
- if (!addr)
- wdog = (struct watchdog_regs *)WDOG1_BASE_ADDR;
+ struct watchdog_regs *wdog = (struct watchdog_regs *)WDOG1_BASE_ADDR;
/* Clear WDA to trigger WDOG_B immediately */
writew((WCR_WDE | WCR_SRS), >wcr);
--
2.26.2
[PATCH 1/8] ARM: imx8m: Do not define do_reset() if sysreset is enabled
From: Marek Vasut
The SPL can also be compiled with sysreset drivers just fine, so
update the condition to cater for that option.
Signed-off-by: Marek Vasut
Cc: Fabio Estevam
Cc: Flavio Suligoi
Cc: Harald Seiler
Cc: Igor Opaniuk
Cc: Marcel Ziswiler
Cc: Oleksandr Suvorov
Cc: Peng Fan
Cc: Stefano Babic
---
arch/arm/mach-imx/imx8m/soc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c
index 7fcbd53f3020..0f17252e80b7 100644
--- a/arch/arm/mach-imx/imx8m/soc.c
+++ b/arch/arm/mach-imx/imx8m/soc.c
@@ -382,7 +382,7 @@ int ft_system_setup(void *blob, bd_t *bd)
}
#endif
-#if defined(CONFIG_SPL_BUILD) || !defined(CONFIG_SYSRESET)
+#if !CONFIG_IS_ENABLED(SYSRESET)
void reset_cpu(ulong addr)
{
struct watchdog_regs *wdog = (struct watchdog_regs *)addr;
--
2.26.2
[PATCH 0/8] ARM: imx: Fix reset in SPL
This series is a merge of "ARM: Fix reset in SPL if SYSRESET is not used" [1] and "ARM: imx: Do not define do_reset() if sysreset is enabled" [2] as the two solve the same problem. The first of the two was sent to originally to fix a problem not related to imx8m (see the last patch of this new series, "ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for them"). However, this broke imx8m boards as they define do_reset() in board code which was the reason for adding the imx8m patches. Now, Marek sent the latter series which solves the specific issues of the imx8m boards properly by converting them to use DM_SYSRESET in SPL. This approach is better than what Claudius and I did originally so I've dropped our (non-DM) fixes for the imx8m boards and added Marek's patches instead. I have, however, kept the fixes for the generic code so if anyone would go back to using the non-DM version, it would also work (and not be part of the tree while silently being broken). [1]: https://patchwork.ozlabs.org/project/uboot/list/?series=162379 [2]: https://patchwork.ozlabs.org/project/uboot/list/?series=173249 Claudius Heine (2): ARM: imx8m: Don't use the addr parameter of reset_cpu() ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for them Harald Seiler (1): ARM: imx8m: Fix indentation of reset_cpu() function Marek Vasut (5): ARM: imx8m: Do not define do_reset() if sysreset is enabled ARM: imx8m: Fix reset in SPL on NXP iMX8MM EVK ARM: imx8m: Fix reset in SPL on NXP iMX8MN EVK ARM: imx8m: Fix reset in SPL on NXP iMX8MP EVK ARM: imx8m: Fix reset in SPL on Toradex iMX8MM Verdin arch/arm/dts/imx8mm-evk-u-boot.dtsi | 12 arch/arm/dts/imx8mm-verdin-u-boot.dtsi | 12 arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi | 12 arch/arm/dts/imx8mp-evk-u-boot.dtsi | 12 arch/arm/lib/Makefile| 2 +- arch/arm/mach-imx/imx8m/soc.c| 21 + board/freescale/imx8mm_evk/spl.c | 9 - board/freescale/imx8mn_evk/spl.c | 9 - board/freescale/imx8mp_evk/spl.c | 9 - board/toradex/verdin-imx8mm/spl.c| 9 - configs/imx8mm_evk_defconfig | 5 + configs/imx8mn_ddr4_evk_defconfig| 5 + configs/imx8mp_evk_defconfig | 4 configs/verdin-imx8mm_defconfig | 5 + 14 files changed, 77 insertions(+), 49 deletions(-) -- 2.26.2
Re: [PATCH 1/5] ARM: imx: Do not define do_reset() if sysreset is enabled
Hello Marek,
On Wed, 2020-04-29 at 00:26 +0200, Marek Vasut wrote:
> On 4/28/20 11:23 PM, Harald Seiler wrote:
> > Hello Marek,
>
> Hi,
>
> > On Tue, 2020-04-28 at 16:22 +0200, Marek Vasut wrote:
> > > The SPL can also be compiled with sysreset drivers just fine, so
> > > update the condition to cater for that option.
> >
> > Me and Claudius solved the same problem in a different way a while back
> > (see [1] and [2]).
> >
> > The two approaches overlap but both contain some unique code that is
> > useful. I'll merge the two patchsets and send them anew if that is ok
> > with you.
>
> Can you be more specific about what is your plan ?
>
> Since the MX8M already has DM enabled in SPL for quite a few things and
> has enough SRAM space left, I would be inclined to just go with DM-based
> implementation of the reset over a non-DM one.
For me personally, the patch I care most about is the first of Claudius
and my series ("ARM: reset: use do_reset in SPL/TPL if SYSRESET was not
enabled for them") [1]. It is necessary to make SPL_USB_SDP_SUPPORT work
on the DH imx6 board (which does not use SYSRESET in SPL at the moment).
As this patch breaks the imx8m* boards in their present form, we tried
fixing them in their current non-DM state in "imx: imx8m*: Remove do_reset
from board files" [2] and "imx: imx8m: Don't use the addr parameter of
reset_cpu" [3]. These patches are theoretically superseeded by your
series which solves the problem much better and more future-proof.
But I think it would not hurt to pull in Claudius and my changes as well
so the non-DM version isn't lying around, silently broken.
Additionally, "imx: imx8m: Don't use the addr parameter of reset_cpu" [3]
is groundwork for removing this addr parameter altogether from the whole
tree which I want to do after this imx8 story is over.
So to summarize, the final state I'd want is this:
- "ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for
them" is applied for my own needs.
- No board defines do_reset().
- The imx8m boards use DM_SYSRESET in SPL (from your patches).
- The non-DM reset code for imx8 is also fixed, even though it is not
used in any mainline board anymore.
[1]:
https://patchwork.ozlabs.org/project/uboot/patch/62c163018998fcf476f0ad2edf83d1787d69445d.1583328917.git@denx.de/
[2]:
https://patchwork.ozlabs.org/project/uboot/patch/25277ba3658920ff3be7464020438070844f05da.1583328917.git@denx.de/
[3]:
https://patchwork.ozlabs.org/project/uboot/patch/b31a2cbb66950bff2e09ab07b710d53d9d46cf09.1583328917.git@denx.de/
--
Harald
Re: [PATCH 1/5] ARM: imx: Do not define do_reset() if sysreset is enabled
Hello Marek,
On Tue, 2020-04-28 at 16:22 +0200, Marek Vasut wrote:
> The SPL can also be compiled with sysreset drivers just fine, so
> update the condition to cater for that option.
Me and Claudius solved the same problem in a different way a while back
(see [1] and [2]).
The two approaches overlap but both contain some unique code that is
useful. I'll merge the two patchsets and send them anew if that is ok
with you.
[1]: https://lists.denx.de/pipermail/u-boot/2020-March/401935.html
[2]: https://patchwork.ozlabs.org/project/uboot/list/?series=162379
> Signed-off-by: Marek Vasut
> Cc: Fabio Estevam
> Cc: Flavio Suligoi
> Cc: Harald Seiler
> Cc: Igor Opaniuk
> Cc: Marcel Ziswiler
> Cc: Oleksandr Suvorov
> Cc: Peng Fan
> Cc: Stefano Babic
> ---
> arch/arm/mach-imx/imx8m/soc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c
> index 7fcbd53f30..0f17252e80 100644
> --- a/arch/arm/mach-imx/imx8m/soc.c
> +++ b/arch/arm/mach-imx/imx8m/soc.c
> @@ -382,7 +382,7 @@ int ft_system_setup(void *blob, bd_t *bd)
> }
> #endif
>
> -#if defined(CONFIG_SPL_BUILD) || !defined(CONFIG_SYSRESET)
> +#if !CONFIG_IS_ENABLED(SYSRESET)
> void reset_cpu(ulong addr)
> {
> struct watchdog_regs *wdog = (struct watchdog_regs *)addr;
--
Harald
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-62 Fax: +49-8142-66989-80 Email: h...@denx.de
Re: [PATCH 0/3] ARM: Fix reset in SPL if SYSRESET is not used
Hello, On Wed, 2020-03-04 at 15:23 +0100, Harald Seiler wrote: > Hello, > > continuing on the discussion around Claudius' patch for fixing reset in SPL > [1] > we have taken a closer look at the issue. To quickly summarize the situation: > > The original patch was to enable the generic ARM implementation of > `do_reset` if CONFIG_SYSRESET is not enabled in SPL. This would break > compilation for some boards which define their own `do_reset` in > `board/*/spl.c`. > > To be more specific, the following 4 boards have this custom `do_reset`: > > - toradex/verdin-imx8mm > - freescale/imx8mn_evk > - freescale/imx8mm_evk > - freescale/imx8mp_evk > > I hope we can all agree that `do_reset` is not at all meant to be implemented > in board files. From looking at the related code for imx8m, it feels like > this > was just a workaround hack to archieve the same thing which Claudius has > fixed. > So this patch series reverts the addition of `do_reset` implementations in > imx8m board files and instead switches to using the proper fix provided by > Claudius. > > > Additionally, the custom do_reset implementations were passing an address > (WDOG1_BASE_ADDR) to `reset_cpu()` instead of 0. This is the only place in > the > entire U-Boot tree where this happens. Instead, all other implementations > simply ignore the parameter and 0 is passed by callers. It looks a lot like > this is a legacy left-over which makes me think that using it for a > (hard-coded) > watchdog address is not a good idea as it breaks convention with the rest of > U-Boot. > > [1]: https://patchwork.ozlabs.org/patch/1201959 > > Claudius Heine (3): > ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for > them > imx: imx8m*: Remove do_reset from board files > imx: imx8m: Don't use the addr parameter of reset_cpu > > arch/arm/lib/Makefile | 2 +- > arch/arm/mach-imx/imx8m/soc.c | 5 + > board/freescale/imx8mm_evk/spl.c | 9 - > board/freescale/imx8mn_evk/spl.c | 9 - > board/freescale/imx8mp_evk/spl.c | 9 - > board/toradex/verdin-imx8mm/spl.c | 9 - > 6 files changed, 2 insertions(+), 41 deletions(-) > Quick question, what is the situation with this series? Is there anything which needs to be addressed? Without it, CONFIG_SPL_USB_SDP_SUPPORT is broken on the imx6q hardware I am working with and I guess the same issue might exist on many other boards as well (The USB stack needs a do_reset() implementation in SPL). Regards, -- Harald
[PATCH 5/5] imx: spl: Fix use of removed SPL_FAT_SUPPORT config
CONFIG_SPL_FAT_SUPPORT was removed in commit 0c3a9ed409a5
("spl: Kconfig: Replace CONFIG_SPL_FAT_SUPPORT with CONFIG_SPL_FS_FAT").
Fixup a leftover use of the symbol.
Fixes: 9d86dbd9cf9d ("imx: spl: implement spl_boot_mode for i.MX7/8/8M")
Signed-off-by: Harald Seiler
---
arch/arm/mach-imx/spl.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
index 32d78b799c36..fd3fa046002a 100644
--- a/arch/arm/mach-imx/spl.c
+++ b/arch/arm/mach-imx/spl.c
@@ -197,14 +197,14 @@ u32 spl_mmc_boot_mode(const u32 boot_device)
case SD1_BOOT:
case SD2_BOOT:
case SD3_BOOT:
- if (IS_ENABLED(CONFIG_SPL_FAT_SUPPORT))
+ if (IS_ENABLED(CONFIG_SPL_FS_FAT))
return MMCSD_MODE_FS;
else
return MMCSD_MODE_RAW;
case MMC1_BOOT:
case MMC2_BOOT:
case MMC3_BOOT:
- if (IS_ENABLED(CONFIG_SPL_FAT_SUPPORT))
+ if (IS_ENABLED(CONFIG_SPL_FS_FAT))
return MMCSD_MODE_FS;
else if (IS_ENABLED(CONFIG_SUPPORT_EMMC_BOOT))
return MMCSD_MODE_EMMCBOOT;
--
2.26.1
[PATCH 0/5] Fix spl_mmc_boot_mode() implementation for IMX
The spl_mmc_boot_mode() (formerly spl_boot_mode()) implementation for
IMX has grown quite big over time [1]. It has also started to steer
away from what it is meant to do in its current form and breaks some
use-cases of the SPL. Namely, the issue is that nowadays SPL can
attempt loading U-Boot from multiple MMC devices and this function needs
to return the correct mode for each one which it currently does not.
There have been multiple attempts to fix this in the past, starting with
a (rejected) patch from Anatolij [2]. Most recently, a patch from
Lukasz [3] was merged which introduces a config option
CONFIG_SPL_FORCE_MMC_BOOT.
I think this is going in the wrong direction when the real solution lies
in cutting the function back down as much as possible. IMX currently
has the biggest implementation in the tree and I don't really see
a reason why all that complexity is necessary. I have spend some time
to find out the full story here and have summarized that in a previous
mail [4].
To cut it as short as possible: I believe CONFIG_SPL_FORCE_MMC_BOOT is
superfluous as enabling it is the only correct behavior and
CONFIG_SPL_FORCE_MMC_BOOT=n is faulty and will lead to problems when
customizing the SPL boot sequence e.g. using board_boot_order().
Thus, this series contains Anatolij's original patch, reverts the
introduction of CONFIG_SPL_FORCE_MMC_BOOT and cleans up the
implementation a bit. One thing which is left out, which still needs to
happen in the future is getting rid of the big #if for IMX{7,8,8M}.
I don't have hardware to test IMX7 or 8 right now and haven't looked too
deep into what needs to be done here.
I'm pretty certain this series should not break any use-case and if it
does anyway, a breakage caused by it indicates the need to fix SPL
configuration for that board. I'm happy to help with that if the need
arises.
[1]:
https://gitlab.denx.de/u-boot/u-boot/-/blob/v2020.04/arch/arm/mach-imx/spl.c#L192-251
[2]: https://patchwork.ozlabs.org/patch/796237/
[3]:
https://gitlab.denx.de/u-boot/u-boot/-/commit/772b55723bcbe8ebe84f579d9cdc831d8e18579d
[4]: https://lists.denx.de/pipermail/u-boot/2020-April/405979.html
Anatolij Gustschin (1):
imx: spl: return boot mode for asked MMC device in spl_mmc_boot_mode()
Harald Seiler (4):
Revert "imx: Introduce CONFIG_SPL_FORCE_MMC_BOOT to force MMC boot on
falcon mode"
Revert "imx: defconfig: Enable CONFIG_SPL_FORCE_MMC_BOOT"
imx: spl: Remove ifdefs in spl_mmc_boot_mode()
imx: spl: Fix use of removed SPL_FAT_SUPPORT config
arch/arm/mach-imx/spl.c | 49 -
common/spl/Kconfig | 9 ---
configs/display5_defconfig | 1 -
configs/imx28_xea_defconfig | 1 -
4 files changed, 16 insertions(+), 44 deletions(-)
--
2.26.1
[PATCH 1/5] imx: spl: return boot mode for asked MMC device in spl_mmc_boot_mode()
From: Anatolij Gustschin
Boards may extend or re-define the boot list in their board_boot_order()
function by modifying spl_boot_list. E.g. a board might boot SPL from a
slow SPI NOR flash and then load the U-Boot from an eMMC or SD-card.
Or it might use additional MMC boot device in spl_boot_list for cases
when the image in SPI NOR flash is not found, so it could fall back to
eMMC, SD-card or another boot device.
Getting the MMC boot mode in spl_mmc will fail when we are trying to
boot from an MMC device in the spl_boot_list and the original board
boot mode (as returned by spl_boot_device()) is not an MMC boot mode.
Fix it by checking the asked MMC boot device from the spl_mmc_boot_mode()
argument.
Signed-off-by: Anatolij Gustschin
---
arch/arm/mach-imx/spl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
index 49bb3b928da1..e5835150a06d 100644
--- a/arch/arm/mach-imx/spl.c
+++ b/arch/arm/mach-imx/spl.c
@@ -229,7 +229,7 @@ u32 spl_mmc_boot_mode(const u32 boot_device)
#ifdef CONFIG_SPL_FORCE_MMC_BOOT
switch (boot_device) {
#else
- switch (spl_boot_device()) {
+ switch (boot_device) {
#endif
/* for MMC return either RAW or FAT mode */
case BOOT_DEVICE_MMC1:
--
2.26.1
[PATCH 3/5] Revert "imx: defconfig: Enable CONFIG_SPL_FORCE_MMC_BOOT"
CONFIG_SPL_FORCE_MMC_BOOT was removed in a previous patch as its behavior is the correct one in all cases. Remove all uses of it from defconfigs. This reverts commit 3201e5b444ae3a13aa31e8b5101ad38d7ff0640d and removes CONFIG_SPL_FORCE_MMC_BOOT from the imx28_xea defconfig. Signed-off-by: Harald Seiler --- configs/display5_defconfig | 1 - configs/imx28_xea_defconfig | 1 - 2 files changed, 2 deletions(-) diff --git a/configs/display5_defconfig b/configs/display5_defconfig index 9026c17f3fb5..f982191f7f53 100644 --- a/configs/display5_defconfig +++ b/configs/display5_defconfig @@ -38,7 +38,6 @@ CONFIG_SPL_DMA=y CONFIG_SPL_ENV_SUPPORT=y CONFIG_SPL_SAVEENV=y CONFIG_SPL_I2C_SUPPORT=y -CONFIG_SPL_FORCE_MMC_BOOT=y CONFIG_SPL_OS_BOOT=y CONFIG_SPL_SPI_LOAD=y CONFIG_SYS_SPI_U_BOOT_OFFS=0x2 diff --git a/configs/imx28_xea_defconfig b/configs/imx28_xea_defconfig index 2d49b664deae..79fa08bbab2b 100644 --- a/configs/imx28_xea_defconfig +++ b/configs/imx28_xea_defconfig @@ -28,7 +28,6 @@ CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0 CONFIG_SUPPORT_EMMC_BOOT_OVERRIDE_PART_CONFIG=y CONFIG_SPL_DMA=y -CONFIG_SPL_FORCE_MMC_BOOT=y CONFIG_SPL_MMC_TINY=y CONFIG_SPL_OS_BOOT=y CONFIG_SPL_PAYLOAD="u-boot.img" -- 2.26.1
[PATCH 4/5] imx: spl: Remove ifdefs in spl_mmc_boot_mode()
It is hard to read code which contains nested ifdef blocks. Replace
them with normal if-blocks and the IS_ENABLED() macro. This is not only
more readable but also helps as both arms are validated by the compiler
in all cases.
Signed-off-by: Harald Seiler
---
arch/arm/mach-imx/spl.c | 38 --
1 file changed, 16 insertions(+), 22 deletions(-)
diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
index e3f51e45edf2..32d78b799c36 100644
--- a/arch/arm/mach-imx/spl.c
+++ b/arch/arm/mach-imx/spl.c
@@ -197,23 +197,19 @@ u32 spl_mmc_boot_mode(const u32 boot_device)
case SD1_BOOT:
case SD2_BOOT:
case SD3_BOOT:
-#if defined(CONFIG_SPL_FAT_SUPPORT)
- return MMCSD_MODE_FS;
-#else
- return MMCSD_MODE_RAW;
-#endif
- break;
+ if (IS_ENABLED(CONFIG_SPL_FAT_SUPPORT))
+ return MMCSD_MODE_FS;
+ else
+ return MMCSD_MODE_RAW;
case MMC1_BOOT:
case MMC2_BOOT:
case MMC3_BOOT:
-#if defined(CONFIG_SPL_FAT_SUPPORT)
- return MMCSD_MODE_FS;
-#elif defined(CONFIG_SUPPORT_EMMC_BOOT)
- return MMCSD_MODE_EMMCBOOT;
-#else
- return MMCSD_MODE_RAW;
-#endif
- break;
+ if (IS_ENABLED(CONFIG_SPL_FAT_SUPPORT))
+ return MMCSD_MODE_FS;
+ else if (IS_ENABLED(CONFIG_SUPPORT_EMMC_BOOT))
+ return MMCSD_MODE_EMMCBOOT;
+ else
+ return MMCSD_MODE_RAW;
default:
puts("spl: ERROR: unsupported device\n");
hang();
@@ -224,14 +220,12 @@ u32 spl_mmc_boot_mode(const u32 boot_device)
case BOOT_DEVICE_MMC1:
case BOOT_DEVICE_MMC2:
case BOOT_DEVICE_MMC2_2:
-#if defined(CONFIG_SPL_FS_FAT)
- return MMCSD_MODE_FS;
-#elif defined(CONFIG_SUPPORT_EMMC_BOOT)
- return MMCSD_MODE_EMMCBOOT;
-#else
- return MMCSD_MODE_RAW;
-#endif
- break;
+ if (IS_ENABLED(CONFIG_SPL_FS_FAT))
+ return MMCSD_MODE_FS;
+ else if (IS_ENABLED(CONFIG_SUPPORT_EMMC_BOOT))
+ return MMCSD_MODE_EMMCBOOT;
+ else
+ return MMCSD_MODE_RAW;
default:
puts("spl: ERROR: unsupported device\n");
hang();
--
2.26.1
[PATCH 2/5] Revert "imx: Introduce CONFIG_SPL_FORCE_MMC_BOOT to force MMC boot on falcon mode"
The CONFIG_SPL_FORCE_MMC_BOOT config flag is not needed as its behavior
is the correct one in all cases; using spl_boot_device() instead of the
boot_device parameter will lead to inconsistency issues, for example,
when a board_boot_order() is defined. In fact, this is the reason the
parameter was introduced in the first place, in commit 2b1cdafa9fdd
("common: Pass the boot device into spl_boot_mode()").
This reverts commit 772b55723bcbe8ebe84f579d9cdc831d8e18579d.
Link: https://lists.denx.de/pipermail/u-boot/2020-April/405979.html
Signed-off-by: Harald Seiler
---
arch/arm/mach-imx/spl.c | 11 ---
common/spl/Kconfig | 9 -
2 files changed, 20 deletions(-)
diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
index e5835150a06d..e3f51e45edf2 100644
--- a/arch/arm/mach-imx/spl.c
+++ b/arch/arm/mach-imx/spl.c
@@ -219,18 +219,7 @@ u32 spl_mmc_boot_mode(const u32 boot_device)
hang();
}
#else
-/*
- * When CONFIG_SPL_FORCE_MMC_BOOT is defined the 'boot_device' is used
- * unconditionally to decide about device to use for booting.
- * This is crucial for falcon boot mode, when board boots up (i.e. ROM
- * loads SPL) from slow SPI-NOR memory and afterwards the SPL's 'falcon' boot
- * mode is used to load Linux OS from eMMC partition.
- */
-#ifdef CONFIG_SPL_FORCE_MMC_BOOT
switch (boot_device) {
-#else
- switch (boot_device) {
-#endif
/* for MMC return either RAW or FAT mode */
case BOOT_DEVICE_MMC1:
case BOOT_DEVICE_MMC2:
diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index 9d52b75cb434..344fb8003f63 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -669,15 +669,6 @@ config SPL_MMC_SUPPORT
this option to build the drivers in drivers/mmc as part of an SPL
build.
-config SPL_FORCE_MMC_BOOT
- bool "Force SPL booting from MMC"
- depends on SPL_MMC_SUPPORT
- default n
- help
- Force SPL to use MMC device for Linux kernel booting even when the
- SoC ROM recognized boot medium is not eMMC/SD. This is crucial for
- factory or 'falcon mode' booting.
-
config SPL_MMC_TINY
bool "Tiny MMC framework in SPL"
depends on SPL_MMC_SUPPORT
--
2.26.1
Re: Your message to U-Boot awaits moderator approval
Hello, On Mon, 2020-04-20 at 11:59 +1200, Reuben Dowle wrote: > Hi, > > I have attempted to submit patch to this mailing list twice, and each time > the patch appears to get lost in moderation. If the patch you are talking about is "Add support for SHA384 and SHA512", then I don't think it got lost; I can see the patch on the list twice. > Is there something extra I need to do to enable me to submit to this > mailing list (other than signing up to it)? > > Thanks, > > Reuben > > On Fri, 17 Apr 2020 at 08:19, wrote: > > > Your mail to 'U-Boot' with the subject > > > > [PATCH] Add support for SHA384 and SHA512 > > > > Is being held until the list moderator can review it for approval. > > > > The reason it is being held: > > > > Post to moderated list > > > > Either the message will get posted to the list, or you will receive > > notification of the moderator's decision. If you would like to cancel > > this posting, please visit the following URL: > > > > > > https://lists.denx.de/confirm/u-boot/2bed39af1bfa69a28767dbe2e76a6e811ebe2d53 > > > > -- Harald
Re: Network functionality in v2020.04 on Raspberry Pi 4
Hello Sasha, adding Matthias Brugger on Cc as he is the maintainer for Raspberry Pi. On Thu, 2020-04-16 at 20:25 +, Jevtic, Sasha wrote: > Hello all, > > I am encountering considerable difficulty with the networking > functionality on Raspberry Pi 4 on the latest release (2020.04). In > particular, I have observed: > > * any network operation that fails renders networking functionality > inoperable until reboot. > * ping always fails. > * TFTP download attempt of non-existent file fails (resulting in > subsequent network operations failing). > * TFTP download of an existing file usually works, but occasionally > experiences timeouts during transfers; some are timeouts are > intermittent (the transfer ultimately resumes), and some are > permanent. > * TFTP download of a script usually works (as above). The script > will run, but any additional TFTP downloads performed by a script > all fail. > > It is not entirely clear right now whether this is a problem in U-Boot > (i.e., instead of in the execution environment set up by the Raspberry > Pi firmware), but the TFTP infrastructure I am using is known to work > well with other hardware running an older version of U-Boot. Also, > there are also no network issues in the Linux environment that > I ultimately boot, so hardware also seems unlikely. Thus, suggestions > for things to check would be appreciated; I am of course also happy to > provide any supporting data required to investigate. If you have know which older version still worked, you can use git-bisect [1] to find out where the regression was introduced. Ideally, if you can automate installation of a newly built U-Boot version, this can run entirely unattended to find out where the bug was introduced. [1]: https://git-scm.com/docs/git-bisect > Please also be aware that I initially posted a very similar inquiry to > the Raspberry Pi Forum: > > https://www.raspberrypi.org/forums/viewtopic.php?f=29=271068 > > Thanks. > > Sasha -- Harald
[PATCH] ARM: imx6: DHCOM i.MX6 PDK: Fix usb-otg VBUS regulator
During the conversion of this board to DM_REGULATOR, usb-mass-storage
was broken and started failing with the following error:
=> ums 0 mmc 2
UMS: LUN 0, dev 2, hwpart 0, sector 0x0, count 0xe9
Error enabling VBUS supply
g_dnl_register: failed!, error: -38
g_dnl_register failed
Fix this by adding the relevant GPIO to the regulator node.
Fixes: 4ca99fe81aea ("ARM: imx: dh-imx6: Enable DM regulator")
Signed-off-by: Harald Seiler
---
Notes:
This patch currently depends on "ARM: imx6: DHCOM i.MX6 PDK: Convert to
DM_ETH" although this could be changed.
This is a working fix but I am not 100% sure if it is the correct thing
to do. The device-tree contains a second regulator which already
defines this exact GPIO:
reg_usb_h1_vbus: regulator-usb-h1-vbus {
compatible = "regulator-fixed";
regulator-name = "usb_h1_vbus";
regulator-min-microvolt = <500>;
regulator-max-microvolt = <500>;
gpio = < 31 GPIO_ACTIVE_HIGH>;
enable-active-high;
};
Maybe _usb_otg_vbus is superfluous and the two should be merged into
a single regulator used for both and ?
arch/arm/dts/imx6qdl-dhcom-pdk2-u-boot.dtsi | 2 ++
arch/arm/dts/imx6qdl-dhcom-u-boot.dtsi | 9 +
2 files changed, 11 insertions(+)
create mode 100644 arch/arm/dts/imx6qdl-dhcom-u-boot.dtsi
diff --git a/arch/arm/dts/imx6qdl-dhcom-pdk2-u-boot.dtsi
b/arch/arm/dts/imx6qdl-dhcom-pdk2-u-boot.dtsi
index a54e421de3e4..32128d4d2ab4 100644
--- a/arch/arm/dts/imx6qdl-dhcom-pdk2-u-boot.dtsi
+++ b/arch/arm/dts/imx6qdl-dhcom-pdk2-u-boot.dtsi
@@ -3,6 +3,8 @@
* Copyright (C) 2020 Harald Seiler
*/
+#include "imx6qdl-dhcom-u-boot.dtsi"
+
/ {
fec_vio: regulator-fec {
compatible = "regulator-fixed";
diff --git a/arch/arm/dts/imx6qdl-dhcom-u-boot.dtsi
b/arch/arm/dts/imx6qdl-dhcom-u-boot.dtsi
new file mode 100644
index ..4c3b5e82d61b
--- /dev/null
+++ b/arch/arm/dts/imx6qdl-dhcom-u-boot.dtsi
@@ -0,0 +1,9 @@
+// SPDX-License-Identifier: (GPL-2.0+)
+/*
+ * Copyright (C) 2020 Harald Seiler
+ */
+
+_usb_otg_vbus {
+ gpio = < 31 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+};
--
2.26.1
[PATCH v4] ARM: imx6: DHCOM i.MX6 PDK: Convert to DM_ETH
Use DM_ETH instead of legacy networking. Add VIO as a fixed regulator
to the relevant device-trees and augment the FEC node with properties
for the reset GPIO.
It should be noted that the relevant properties for the reset GPIO
already exist in the PHY node (reset-gpios, reset-delay-us,
reset-post-delay-us) but U-Boot currently ignores those and only
supports the bus-level reset properties in the FEC node
(phy-reset-gpios, phy-reset-duration, phy-reset-post-delay).
Signed-off-by: Harald Seiler
---
Notes:
Changes in v2:
- Move reset and VIO to device-tree.
- Always enable the clock, not just if CONFIG_FEC_MXC=y.
Changes in v3:
- Rename the dt file to imx6qdl-dhcom-pdk2-u-boot.dtsi because the PHY is
pdk2 specific.
- More verbose commit message.
Changes in v4:
- Remove regulator-always-on.
- Change ACTIVE_HIGH to ACTIVE_LOW to properly reflect the non-DM code.
arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi | 6 +++
arch/arm/dts/imx6q-dhcom-pdk2-u-boot.dtsi | 2 +
arch/arm/dts/imx6qdl-dhcom-pdk2-u-boot.dtsi | 21 +
board/dhelectronics/dh_imx6/dh_imx6.c | 51 +
configs/dh_imx6_defconfig | 2 +
5 files changed, 33 insertions(+), 49 deletions(-)
create mode 100644 arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi
create mode 100644 arch/arm/dts/imx6qdl-dhcom-pdk2-u-boot.dtsi
diff --git a/arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi
b/arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi
new file mode 100644
index ..fc7dffea2a69
--- /dev/null
+++ b/arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi
@@ -0,0 +1,6 @@
+// SPDX-License-Identifier: (GPL-2.0+)
+/*
+ * Copyright (C) 2020 Harald Seiler
+ */
+
+#include "imx6qdl-dhcom-pdk2-u-boot.dtsi"
diff --git a/arch/arm/dts/imx6q-dhcom-pdk2-u-boot.dtsi
b/arch/arm/dts/imx6q-dhcom-pdk2-u-boot.dtsi
index b94231edb3fb..026342df5a4a 100644
--- a/arch/arm/dts/imx6q-dhcom-pdk2-u-boot.dtsi
+++ b/arch/arm/dts/imx6q-dhcom-pdk2-u-boot.dtsi
@@ -3,6 +3,8 @@
* Copyright (C) 2019 Claudius Heine
*/
+#include "imx6qdl-dhcom-pdk2-u-boot.dtsi"
+
/ {
wdt-reboot {
compatible = "wdt-reboot";
diff --git a/arch/arm/dts/imx6qdl-dhcom-pdk2-u-boot.dtsi
b/arch/arm/dts/imx6qdl-dhcom-pdk2-u-boot.dtsi
new file mode 100644
index ..a54e421de3e4
--- /dev/null
+++ b/arch/arm/dts/imx6qdl-dhcom-pdk2-u-boot.dtsi
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: (GPL-2.0+)
+/*
+ * Copyright (C) 2020 Harald Seiler
+ */
+
+/ {
+ fec_vio: regulator-fec {
+ compatible = "regulator-fixed";
+
+ regulator-name = "fec-vio";
+ gpio = < 7 GPIO_ACTIVE_LOW>;
+ };
+};
+
+ {
+ phy-reset-gpios = < 0 GPIO_ACTIVE_LOW>;
+ phy-reset-duration = <1>;
+ phy-reset-post-delay = <10>;
+
+ phy-supply = <_vio>;
+};
diff --git a/board/dhelectronics/dh_imx6/dh_imx6.c
b/board/dhelectronics/dh_imx6/dh_imx6.c
index 33ce7e8ff115..b6f8b11a10cc 100644
--- a/board/dhelectronics/dh_imx6/dh_imx6.c
+++ b/board/dhelectronics/dh_imx6/dh_imx6.c
@@ -28,10 +28,7 @@
#include
#include
#include
-#include
#include
-#include
-#include
#include
#include
@@ -52,24 +49,6 @@ int overwrite_console(void)
return 1;
}
-#ifdef CONFIG_FEC_MXC
-static void eth_phy_reset(void)
-{
- /* Reset PHY */
- gpio_direction_output(IMX_GPIO_NR(5, 0) , 0);
- udelay(500);
- gpio_set_value(IMX_GPIO_NR(5, 0), 1);
-
- /* Enable VIO */
- gpio_direction_output(IMX_GPIO_NR(1, 7) , 0);
-
- /*
-* KSZ9021 PHY needs at least 10 mSec after PHY reset
-* is released to stabilize
-*/
- mdelay(10);
-}
-
static int setup_fec_clock(void)
{
struct iomuxc *iomuxc_regs = (struct iomuxc *)IOMUXC_BASE_ADDR;
@@ -80,34 +59,6 @@ static int setup_fec_clock(void)
return enable_fec_anatop_clock(0, ENET_50MHZ);
}
-int board_eth_init(bd_t *bis)
-{
- uint32_t base = IMX_FEC_BASE;
- struct mii_dev *bus = NULL;
- struct phy_device *phydev = NULL;
-
- gpio_request(IMX_GPIO_NR(5, 0), "PHY-reset");
- gpio_request(IMX_GPIO_NR(1, 7), "VIO");
-
- setup_fec_clock();
-
- eth_phy_reset();
-
- bus = fec_get_miibus(base, -1);
- if (!bus)
- return -EINVAL;
-
- /* Scan PHY 0 */
- phydev = phy_find_by_mask(bus, 0xf, PHY_INTERFACE_MODE_RGMII);
- if (!phydev) {
- printf("Ethernet PHY not found!\n");
- return -EINVAL;
- }
-
- return fec_probe(bis, -1, base, bus, phydev);
-}
-#endif
-
#ifdef CONFIG_USB_EHCI_MX6
static void setup_usb(void)
{
@@ -190,6 +141,8 @@ int board_init(void)
setup_dhcom_mac_from_fuse();
+ setup_fec_clock();
+
return 0;
}
diff --git a/configs/dh_imx6_defconfig b/configs/dh_imx6_defconfig
in
Re: [PATCH v3] ARM: imx6: DHCOM i.MX6 PDK: Convert to DM_ETH
Hello Marek,
On Wed, 2020-04-15 at 19:02 +0200, Marek Vasut wrote:
> On 4/15/20 5:54 PM, Harald Seiler wrote:
> > Use DM_ETH instead of legacy networking. Add VIO as a fixed regulator
> > to the relevant device-trees and augment the FEC node with properties
> > for the reset GPIO.
> >
> > It should be noted that the relevant properties for the reset GPIO
> > already exist in the PHY node but U-Boot currently ignores those and
> > only supports the bus-level reset properties in the FEC node.
> >
> > Signed-off-by: Harald Seiler
> > ---
> >
> > Notes:
> > Changes in v2:
> > - Move reset and VIO to device-tree.
> > - Always enable the clock, not just if CONFIG_FEC_MXC=y.
> >
> > Changes in v3:
> > - Rename the dt file to imx6qdl-dhcom-pdk2-u-boot.dtsi because the
> > PHY is
> > pdk2 specific.
> > - More verbose commit message.
> >
> > arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi | 6 +++
> > arch/arm/dts/imx6q-dhcom-pdk2-u-boot.dtsi | 2 +
> > arch/arm/dts/imx6qdl-dhcom-pdk2-u-boot.dtsi | 22 +
> > board/dhelectronics/dh_imx6/dh_imx6.c | 51 +
> > configs/dh_imx6_defconfig | 2 +
> > 5 files changed, 34 insertions(+), 49 deletions(-)
> > create mode 100644 arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi
> > create mode 100644 arch/arm/dts/imx6qdl-dhcom-pdk2-u-boot.dtsi
> >
> > diff --git a/arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi
> > b/arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi
> > new file mode 100644
> > index ..fc7dffea2a69
> > --- /dev/null
> > +++ b/arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi
>
> Do we really need a separate DT for DL ? If so, this should be a
> separate patch.
I can't really comment on the reason for the two separate device-trees but
it looks like they were introduced for commit 8039211a8a9c ("ARM: imx6:
DHCOM i.MX6 PDK: config SPL to load U-Boot fitImage with mulitple DTs").
I'm not sure I understand why you want two separate patches here.
Wouldn't it make more sense to fix both device-trees in one go so we don't
have a broken U-Boot for the DL version from just the first patch (which
would e.g. hurt bisecting)?
--
Harald
Re: [PATCH v3] ARM: imx6: DHCOM i.MX6 PDK: Convert to DM_ETH
Hello Fabio,
On Wed, 2020-04-15 at 13:15 -0300, Fabio Estevam wrote:
> Hi Harald,
>
> On Wed, Apr 15, 2020 at 12:54 PM Harald Seiler wrote:
>
> > +/ {
> > + fec_vio: regulator-fec {
> > + compatible = "regulator-fixed";
> > +
> > + regulator-name = "fec-vio";
> > + gpio = < 7 GPIO_ACTIVE_HIGH>;
>
> By looking at your board code, this should be GPIO_ACTIVE_LOW instead.
Yes, you are right, I will change this in v4. Interestingly, it works
with both ACTIVE_LOW and ACTIVE_HIGH but removing the regulator entirely
breaks it. Seems a bit weird to me ...
> > + regulator-always-on;
>
> This one could be removed since it has the FEC as a consumer.
I see, thanks!
--
Harald
[PATCH v3] ARM: imx6: DHCOM i.MX6 PDK: Convert to DM_ETH
Use DM_ETH instead of legacy networking. Add VIO as a fixed regulator
to the relevant device-trees and augment the FEC node with properties
for the reset GPIO.
It should be noted that the relevant properties for the reset GPIO
already exist in the PHY node but U-Boot currently ignores those and
only supports the bus-level reset properties in the FEC node.
Signed-off-by: Harald Seiler
---
Notes:
Changes in v2:
- Move reset and VIO to device-tree.
- Always enable the clock, not just if CONFIG_FEC_MXC=y.
Changes in v3:
- Rename the dt file to imx6qdl-dhcom-pdk2-u-boot.dtsi because the PHY is
pdk2 specific.
- More verbose commit message.
arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi | 6 +++
arch/arm/dts/imx6q-dhcom-pdk2-u-boot.dtsi | 2 +
arch/arm/dts/imx6qdl-dhcom-pdk2-u-boot.dtsi | 22 +
board/dhelectronics/dh_imx6/dh_imx6.c | 51 +
configs/dh_imx6_defconfig | 2 +
5 files changed, 34 insertions(+), 49 deletions(-)
create mode 100644 arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi
create mode 100644 arch/arm/dts/imx6qdl-dhcom-pdk2-u-boot.dtsi
diff --git a/arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi
b/arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi
new file mode 100644
index ..fc7dffea2a69
--- /dev/null
+++ b/arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi
@@ -0,0 +1,6 @@
+// SPDX-License-Identifier: (GPL-2.0+)
+/*
+ * Copyright (C) 2020 Harald Seiler
+ */
+
+#include "imx6qdl-dhcom-pdk2-u-boot.dtsi"
diff --git a/arch/arm/dts/imx6q-dhcom-pdk2-u-boot.dtsi
b/arch/arm/dts/imx6q-dhcom-pdk2-u-boot.dtsi
index b94231edb3fb..026342df5a4a 100644
--- a/arch/arm/dts/imx6q-dhcom-pdk2-u-boot.dtsi
+++ b/arch/arm/dts/imx6q-dhcom-pdk2-u-boot.dtsi
@@ -3,6 +3,8 @@
* Copyright (C) 2019 Claudius Heine
*/
+#include "imx6qdl-dhcom-pdk2-u-boot.dtsi"
+
/ {
wdt-reboot {
compatible = "wdt-reboot";
diff --git a/arch/arm/dts/imx6qdl-dhcom-pdk2-u-boot.dtsi
b/arch/arm/dts/imx6qdl-dhcom-pdk2-u-boot.dtsi
new file mode 100644
index ..88840bb45920
--- /dev/null
+++ b/arch/arm/dts/imx6qdl-dhcom-pdk2-u-boot.dtsi
@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: (GPL-2.0+)
+/*
+ * Copyright (C) 2020 Harald Seiler
+ */
+
+/ {
+ fec_vio: regulator-fec {
+ compatible = "regulator-fixed";
+
+ regulator-name = "fec-vio";
+ gpio = < 7 GPIO_ACTIVE_HIGH>;
+ regulator-always-on;
+ };
+};
+
+ {
+ phy-reset-gpios = < 0 GPIO_ACTIVE_LOW>;
+ phy-reset-duration = <1>;
+ phy-reset-post-delay = <10>;
+
+ phy-supply = <_vio>;
+};
diff --git a/board/dhelectronics/dh_imx6/dh_imx6.c
b/board/dhelectronics/dh_imx6/dh_imx6.c
index 33ce7e8ff115..b6f8b11a10cc 100644
--- a/board/dhelectronics/dh_imx6/dh_imx6.c
+++ b/board/dhelectronics/dh_imx6/dh_imx6.c
@@ -28,10 +28,7 @@
#include
#include
#include
-#include
#include
-#include
-#include
#include
#include
@@ -52,24 +49,6 @@ int overwrite_console(void)
return 1;
}
-#ifdef CONFIG_FEC_MXC
-static void eth_phy_reset(void)
-{
- /* Reset PHY */
- gpio_direction_output(IMX_GPIO_NR(5, 0) , 0);
- udelay(500);
- gpio_set_value(IMX_GPIO_NR(5, 0), 1);
-
- /* Enable VIO */
- gpio_direction_output(IMX_GPIO_NR(1, 7) , 0);
-
- /*
-* KSZ9021 PHY needs at least 10 mSec after PHY reset
-* is released to stabilize
-*/
- mdelay(10);
-}
-
static int setup_fec_clock(void)
{
struct iomuxc *iomuxc_regs = (struct iomuxc *)IOMUXC_BASE_ADDR;
@@ -80,34 +59,6 @@ static int setup_fec_clock(void)
return enable_fec_anatop_clock(0, ENET_50MHZ);
}
-int board_eth_init(bd_t *bis)
-{
- uint32_t base = IMX_FEC_BASE;
- struct mii_dev *bus = NULL;
- struct phy_device *phydev = NULL;
-
- gpio_request(IMX_GPIO_NR(5, 0), "PHY-reset");
- gpio_request(IMX_GPIO_NR(1, 7), "VIO");
-
- setup_fec_clock();
-
- eth_phy_reset();
-
- bus = fec_get_miibus(base, -1);
- if (!bus)
- return -EINVAL;
-
- /* Scan PHY 0 */
- phydev = phy_find_by_mask(bus, 0xf, PHY_INTERFACE_MODE_RGMII);
- if (!phydev) {
- printf("Ethernet PHY not found!\n");
- return -EINVAL;
- }
-
- return fec_probe(bis, -1, base, bus, phydev);
-}
-#endif
-
#ifdef CONFIG_USB_EHCI_MX6
static void setup_usb(void)
{
@@ -190,6 +141,8 @@ int board_init(void)
setup_dhcom_mac_from_fuse();
+ setup_fec_clock();
+
return 0;
}
diff --git a/configs/dh_imx6_defconfig b/configs/dh_imx6_defconfig
index cbfc3c394e7d..40de1d82031b 100644
--- a/configs/dh_imx6_defconfig
+++ b/configs/dh_imx6_defconfig
@@ -74,6 +74,8 @@ CONFIG_SPI_FLASH_MTD=y
CONFIG_PHYLIB=y
CONFIG_PHY_MICREL=y
CONFIG_PHY_MICREL_KSZ90X1=y
+CONFIG_DM_ETH=y
+CONFIG_DM_MDIO=y
CONFIG_FEC_MXC=y
CONFIG_MII=y
CONFIG_PINCTRL=y
--
2.26.0
Re: [PATCH v2] ARM: imx6: DHCOM i.MX6 PDK: Convert to DM_ETH
Hello Marek,
On Wed, 2020-04-15 at 16:53 +0200, Marek Vasut wrote:
> On 4/15/20 4:48 PM, Harald Seiler wrote:
> > Use DM_ETH instead of legacy networking.
>
> Some more descriptive commit message would help.
>
> [...]
>
> > diff --git a/arch/arm/dts/imx6qdl-dhcom-u-boot.dtsi
> > b/arch/arm/dts/imx6qdl-dhcom-u-boot.dtsi
> > new file mode 100644
> > index ..88840bb45920
> > --- /dev/null
> > +++ b/arch/arm/dts/imx6qdl-dhcom-u-boot.dtsi
> > @@ -0,0 +1,22 @@
> > +// SPDX-License-Identifier: (GPL-2.0+)
> > +/*
> > + * Copyright (C) 2020 Harald Seiler
> > + */
> > +
> > +/ {
> > + fec_vio: regulator-fec {
> > + compatible = "regulator-fixed";
> > +
> > + regulator-name = "fec-vio";
> > + gpio = < 7 GPIO_ACTIVE_HIGH>;
> > + regulator-always-on;
> > + };
> > +};
>
> The VIO regulator is on the pdk2, so it should be in the PDK2 U-Boot extras.
>
> > + {
> > + phy-reset-gpios = < 0 GPIO_ACTIVE_LOW>;
> > + phy-reset-duration = <1>;
> > + phy-reset-post-delay = <10>;
>
> So is the PHY, so this should also be in the PDK2 extras.
>
> (and it should be fixed in Linux too eventually, if it's not done yet)
I think Linux handles this a bit different: The node for the PHY contains
almost the same properties already so I believe that is what's used in the
kernel:
ethphy0: ethernet-phy@0 {
reg = <0>;
max-speed = <100>;
reset-gpios = < 0 GPIO_ACTIVE_LOW>;
reset-delay-us = <1000>;
reset-post-delay-us = <1000>;
};
Not sure why U-Boot uses a different set of properties, maybe it makes
sense at some point to start using those instead.
Also, this was the reason why I put it into the general dhcom dtsi. I was
thinking that, if the existing properties are this general, mine should
probably be, too.
> [...]
--
Harald
[PATCH v2] ARM: imx6: DHCOM i.MX6 PDK: Convert to DM_ETH
Use DM_ETH instead of legacy networking.
Signed-off-by: Harald Seiler
---
Notes:
Changes in v2:
- Move reset and VIO to device-tree
- Always enable the clock, not just if CONFIG_FEC_MXC=y
arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi | 6 +++
arch/arm/dts/imx6q-dhcom-pdk2-u-boot.dtsi | 2 +
arch/arm/dts/imx6qdl-dhcom-u-boot.dtsi | 22 ++
board/dhelectronics/dh_imx6/dh_imx6.c | 51 +-
configs/dh_imx6_defconfig | 2 +
5 files changed, 34 insertions(+), 49 deletions(-)
create mode 100644 arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi
create mode 100644 arch/arm/dts/imx6qdl-dhcom-u-boot.dtsi
diff --git a/arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi
b/arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi
new file mode 100644
index ..50bcb0419c9c
--- /dev/null
+++ b/arch/arm/dts/imx6dl-dhcom-pdk2-u-boot.dtsi
@@ -0,0 +1,6 @@
+// SPDX-License-Identifier: (GPL-2.0+)
+/*
+ * Copyright (C) 2020 Harald Seiler
+ */
+
+#include "imx6qdl-dhcom-u-boot.dtsi"
diff --git a/arch/arm/dts/imx6q-dhcom-pdk2-u-boot.dtsi
b/arch/arm/dts/imx6q-dhcom-pdk2-u-boot.dtsi
index b94231edb3fb..3060ee84f463 100644
--- a/arch/arm/dts/imx6q-dhcom-pdk2-u-boot.dtsi
+++ b/arch/arm/dts/imx6q-dhcom-pdk2-u-boot.dtsi
@@ -3,6 +3,8 @@
* Copyright (C) 2019 Claudius Heine
*/
+#include "imx6qdl-dhcom-u-boot.dtsi"
+
/ {
wdt-reboot {
compatible = "wdt-reboot";
diff --git a/arch/arm/dts/imx6qdl-dhcom-u-boot.dtsi
b/arch/arm/dts/imx6qdl-dhcom-u-boot.dtsi
new file mode 100644
index ..88840bb45920
--- /dev/null
+++ b/arch/arm/dts/imx6qdl-dhcom-u-boot.dtsi
@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: (GPL-2.0+)
+/*
+ * Copyright (C) 2020 Harald Seiler
+ */
+
+/ {
+ fec_vio: regulator-fec {
+ compatible = "regulator-fixed";
+
+ regulator-name = "fec-vio";
+ gpio = < 7 GPIO_ACTIVE_HIGH>;
+ regulator-always-on;
+ };
+};
+
+ {
+ phy-reset-gpios = < 0 GPIO_ACTIVE_LOW>;
+ phy-reset-duration = <1>;
+ phy-reset-post-delay = <10>;
+
+ phy-supply = <_vio>;
+};
diff --git a/board/dhelectronics/dh_imx6/dh_imx6.c
b/board/dhelectronics/dh_imx6/dh_imx6.c
index 33ce7e8ff115..b6f8b11a10cc 100644
--- a/board/dhelectronics/dh_imx6/dh_imx6.c
+++ b/board/dhelectronics/dh_imx6/dh_imx6.c
@@ -28,10 +28,7 @@
#include
#include
#include
-#include
#include
-#include
-#include
#include
#include
@@ -52,24 +49,6 @@ int overwrite_console(void)
return 1;
}
-#ifdef CONFIG_FEC_MXC
-static void eth_phy_reset(void)
-{
- /* Reset PHY */
- gpio_direction_output(IMX_GPIO_NR(5, 0) , 0);
- udelay(500);
- gpio_set_value(IMX_GPIO_NR(5, 0), 1);
-
- /* Enable VIO */
- gpio_direction_output(IMX_GPIO_NR(1, 7) , 0);
-
- /*
-* KSZ9021 PHY needs at least 10 mSec after PHY reset
-* is released to stabilize
-*/
- mdelay(10);
-}
-
static int setup_fec_clock(void)
{
struct iomuxc *iomuxc_regs = (struct iomuxc *)IOMUXC_BASE_ADDR;
@@ -80,34 +59,6 @@ static int setup_fec_clock(void)
return enable_fec_anatop_clock(0, ENET_50MHZ);
}
-int board_eth_init(bd_t *bis)
-{
- uint32_t base = IMX_FEC_BASE;
- struct mii_dev *bus = NULL;
- struct phy_device *phydev = NULL;
-
- gpio_request(IMX_GPIO_NR(5, 0), "PHY-reset");
- gpio_request(IMX_GPIO_NR(1, 7), "VIO");
-
- setup_fec_clock();
-
- eth_phy_reset();
-
- bus = fec_get_miibus(base, -1);
- if (!bus)
- return -EINVAL;
-
- /* Scan PHY 0 */
- phydev = phy_find_by_mask(bus, 0xf, PHY_INTERFACE_MODE_RGMII);
- if (!phydev) {
- printf("Ethernet PHY not found!\n");
- return -EINVAL;
- }
-
- return fec_probe(bis, -1, base, bus, phydev);
-}
-#endif
-
#ifdef CONFIG_USB_EHCI_MX6
static void setup_usb(void)
{
@@ -190,6 +141,8 @@ int board_init(void)
setup_dhcom_mac_from_fuse();
+ setup_fec_clock();
+
return 0;
}
diff --git a/configs/dh_imx6_defconfig b/configs/dh_imx6_defconfig
index cbfc3c394e7d..40de1d82031b 100644
--- a/configs/dh_imx6_defconfig
+++ b/configs/dh_imx6_defconfig
@@ -74,6 +74,8 @@ CONFIG_SPI_FLASH_MTD=y
CONFIG_PHYLIB=y
CONFIG_PHY_MICREL=y
CONFIG_PHY_MICREL_KSZ90X1=y
+CONFIG_DM_ETH=y
+CONFIG_DM_MDIO=y
CONFIG_FEC_MXC=y
CONFIG_MII=y
CONFIG_PINCTRL=y
--
2.26.0
[PATCH] ARM: imx6: DHCOM i.MX6 PDK: Convert to DM_ETH
Use DM_ETH instead of legacy networking.
Signed-off-by: Harald Seiler
---
Notes:
I am unsure whether I converted 'enough' of the board_eth_init()
function to DM. I think the reset GPIO in particular could be handled
by the DM driver if I add the following to the device-tree (did not try
it yet):
phy-reset-gpios = < 0 GPIO_ACTIVE_LOW>;
phy-reset-duration = <1>;
phy-reset-post-delay = <10>;
Is it preferred to change the device-tree here or should I keep the reset
from board_init() like I have it now? If I should use the dt, is there
a similar way for dealing with VIO?
board/dhelectronics/dh_imx6/dh_imx6.c | 26 +-
configs/dh_imx6_defconfig | 2 ++
2 files changed, 7 insertions(+), 21 deletions(-)
diff --git a/board/dhelectronics/dh_imx6/dh_imx6.c
b/board/dhelectronics/dh_imx6/dh_imx6.c
index 33ce7e8ff115..aafb9f1b341f 100644
--- a/board/dhelectronics/dh_imx6/dh_imx6.c
+++ b/board/dhelectronics/dh_imx6/dh_imx6.c
@@ -28,10 +28,7 @@
#include
#include
#include
-#include
#include
-#include
-#include
#include
#include
@@ -80,31 +77,14 @@ static int setup_fec_clock(void)
return enable_fec_anatop_clock(0, ENET_50MHZ);
}
-int board_eth_init(bd_t *bis)
+static void setup_fec(void)
{
- uint32_t base = IMX_FEC_BASE;
- struct mii_dev *bus = NULL;
- struct phy_device *phydev = NULL;
-
gpio_request(IMX_GPIO_NR(5, 0), "PHY-reset");
gpio_request(IMX_GPIO_NR(1, 7), "VIO");
setup_fec_clock();
eth_phy_reset();
-
- bus = fec_get_miibus(base, -1);
- if (!bus)
- return -EINVAL;
-
- /* Scan PHY 0 */
- phydev = phy_find_by_mask(bus, 0xf, PHY_INTERFACE_MODE_RGMII);
- if (!phydev) {
- printf("Ethernet PHY not found!\n");
- return -EINVAL;
- }
-
- return fec_probe(bis, -1, base, bus, phydev);
}
#endif
@@ -190,6 +170,10 @@ int board_init(void)
setup_dhcom_mac_from_fuse();
+#ifdef CONFIG_FEC_MXC
+ setup_fec();
+#endif
+
return 0;
}
diff --git a/configs/dh_imx6_defconfig b/configs/dh_imx6_defconfig
index cbfc3c394e7d..40de1d82031b 100644
--- a/configs/dh_imx6_defconfig
+++ b/configs/dh_imx6_defconfig
@@ -74,6 +74,8 @@ CONFIG_SPI_FLASH_MTD=y
CONFIG_PHYLIB=y
CONFIG_PHY_MICREL=y
CONFIG_PHY_MICREL_KSZ90X1=y
+CONFIG_DM_ETH=y
+CONFIG_DM_MDIO=y
CONFIG_FEC_MXC=y
CONFIG_MII=y
CONFIG_PINCTRL=y
--
2.26.0
[PATCH 0/2] Rename spl_boot_mode() and spl_boot_partition()
TL;DR: The two functions are only used in the SPL MMC driver so I think
their names should reflect that.
spl_boot_mode() has caused a bit of trouble to me and others because
some of its implementations miss what it is meant to do in its current
form. This is in part due to history and I suspect in part due to its
name being a bit misleading.
As a quick summary: spl_boot_mode() is solely used by the SPL MMC driver
to check how it should load U-Boot from the currently attempted
boot-source. There are three possibilities:
1. MMCSD_MODE_FS: The MMC device contains a filesystem (FAT or EXT4).
2. MMCSD_MODE_EMMCBOOT: U-Boot should be loaded from an eMMC
boot-partition.
3. MMCSD_MODE_RAW: U-Boot should be loaded from a raw offset or
a partition on the MMC device.
spl_boot_mode() should, for each MMC boot device, return one of those
3 modes. The boot_device parameter tells which device is queried.
Historically, the boot_device parameter did not exist. Because of this,
a lot of implementations relied on checking where SPL was loaded from
(spl_boot_device()) as it is common to load U-Boot from the same device.
However, nowadays it is possible to attempt loading U-Boot from multiple
MMC devices (as a fallback), for example using board_boot_order().
This means, spl_boot_mode() needs to properly tell the MMC driver the
mode of each device. This was the reason for introducing the
boot_device parameter in commit 2b1cdafa9fdd ("common: Pass the boot
device into spl_boot_mode()")).
I am bringing all this up because some existing implementations don't
handle this properly and new code has been merged which also doesn't.
To make it more clear what the function and its cousin,
spl_boot_partition(), are supposed to do, I suggest renaming them to
spl_mmc_boot_mode() and spl_mmc_boot_partition() respectively.
I will also send a second series to fix the implementation which is
affecting me (arch/arm/mach-imx/spl.c).
Harald Seiler (2):
spl: mmc: Rename spl_boot_mode() to spl_mmc_boot_mode()
spl: mmc: Rename spl_boot_partition() to spl_mmc_boot_partition()
include/spl.h | 32 --
common/spl/spl_mmc.c | 9
arch/arm/mach-imx/spl.c| 2 +-
arch/arm/mach-k3/am6_init.c| 2 +-
arch/arm/mach-k3/j721e_init.c | 2 +-
arch/arm/mach-omap2/boot-common.c | 2 +-
arch/arm/mach-rockchip/spl.c | 2 +-
arch/arm/mach-socfpga/spl_a10.c| 2 +-
arch/arm/mach-socfpga/spl_agilex.c | 2 +-
arch/arm/mach-socfpga/spl_gen5.c | 2 +-
arch/arm/mach-socfpga/spl_s10.c| 2 +-
arch/arm/mach-stm32mp/spl.c| 4 ++--
arch/arm/mach-uniphier/mmc-boot-mode.c | 2 +-
13 files changed, 46 insertions(+), 19 deletions(-)
--
2.26.0
[PATCH 2/2] spl: mmc: Rename spl_boot_partition() to spl_mmc_boot_partition()
This function is only relevant to the MMC driver so calling it
spl_boot_partition() might be confusing. Rename it to
spl_mmc_boot_partition() to make its purpose more clear (and bring
it in line with spl_mmc_boot_mode()).
Signed-off-by: Harald Seiler
---
include/spl.h | 14 +-
common/spl/spl_mmc.c| 5 ++---
arch/arm/mach-stm32mp/spl.c | 2 +-
3 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/include/spl.h b/include/spl.h
index fffcc610bb2b..8b15cd4914ce 100644
--- a/include/spl.h
+++ b/include/spl.h
@@ -255,7 +255,19 @@ u32 spl_boot_device(void);
* spl_boot_device() as U-Boot is not always loaded from the same device as
SPL.
*/
u32 spl_mmc_boot_mode(const u32 boot_device);
-int spl_boot_partition(const u32 boot_device);
+
+/**
+ * spl_mmc_boot_partition() - MMC partition to load U-Boot from.
+ * @boot_device: ID of the device which the MMC driver wants to load
+ * U-Boot from.
+ *
+ * This function should return the partition number which the SPL
+ * should load U-Boot from (on the given boot_device) when
+ * CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION is set.
+ *
+ * If not overridden, it is weakly defined in common/spl/spl_mmc.c.
+ */
+int spl_mmc_boot_partition(const u32 boot_device);
void spl_set_bd(void);
/**
diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
index fb8ad5d54006..a68cdec8dc8f 100644
--- a/common/spl/spl_mmc.c
+++ b/common/spl/spl_mmc.c
@@ -310,8 +310,7 @@ u32 __weak spl_mmc_boot_mode(const u32 boot_device)
}
#ifdef CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION
-__weak
-int spl_boot_partition(const u32 boot_device)
+int __weak spl_mmc_boot_partition(const u32 boot_device)
{
return CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION;
}
@@ -431,7 +430,7 @@ int spl_mmc_load_image(struct spl_image_info *spl_image,
NULL,
#endif
#ifdef CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION
- spl_boot_partition(bootdev->boot_device),
+ spl_mmc_boot_partition(bootdev->boot_device),
#else
0,
#endif
diff --git a/arch/arm/mach-stm32mp/spl.c b/arch/arm/mach-stm32mp/spl.c
index 55ff97de2794..f85391c6af2f 100644
--- a/arch/arm/mach-stm32mp/spl.c
+++ b/arch/arm/mach-stm32mp/spl.c
@@ -49,7 +49,7 @@ u32 spl_mmc_boot_mode(const u32 boot_device)
return MMCSD_MODE_RAW;
}
-int spl_boot_partition(const u32 boot_device)
+int spl_mmc_boot_partition(const u32 boot_device)
{
switch (boot_device) {
case BOOT_DEVICE_MMC1:
--
2.26.0
[PATCH 1/2] spl: mmc: Rename spl_boot_mode() to spl_mmc_boot_mode()
The function's name is misleading as one might think it is used
generally to select the boot-mode when in reality it is only used by the
MMC driver to find out in what way it should try reading U-Boot Proper
from a device (either using a filesystem, a raw sector/partition, or an
eMMC boot partition).
Rename it to spl_mmc_boot_mode() to make it more obvious what this
function is about.
Link: https://lists.denx.de/pipermail/u-boot/2020-April/405979.html
Signed-off-by: Harald Seiler
---
include/spl.h | 18 +-
common/spl/spl_mmc.c | 4 ++--
arch/arm/mach-imx/spl.c| 2 +-
arch/arm/mach-k3/am6_init.c| 2 +-
arch/arm/mach-k3/j721e_init.c | 2 +-
arch/arm/mach-omap2/boot-common.c | 2 +-
arch/arm/mach-rockchip/spl.c | 2 +-
arch/arm/mach-socfpga/spl_a10.c| 2 +-
arch/arm/mach-socfpga/spl_agilex.c | 2 +-
arch/arm/mach-socfpga/spl_gen5.c | 2 +-
arch/arm/mach-socfpga/spl_s10.c| 2 +-
arch/arm/mach-stm32mp/spl.c| 2 +-
arch/arm/mach-uniphier/mmc-boot-mode.c | 2 +-
13 files changed, 30 insertions(+), 14 deletions(-)
diff --git a/include/spl.h b/include/spl.h
index 5d8d14dbf5cd..fffcc610bb2b 100644
--- a/include/spl.h
+++ b/include/spl.h
@@ -238,7 +238,23 @@ int spl_load_imx_container(struct spl_image_info
*spl_image,
/* SPL common functions */
void preloader_console_init(void);
u32 spl_boot_device(void);
-u32 spl_boot_mode(const u32 boot_device);
+
+/**
+ * spl_mmc_boot_mode() - Lookup function for the mode of an MMC boot source.
+ * @boot_device: ID of the device which the MMC driver wants to read
+ * from. Common values are e.g. BOOT_DEVICE_MMC1,
+ * BOOT_DEVICE_MMC2, BOOT_DEVICE_MMC2_2.
+ *
+ * This function should return one of MMCSD_MODE_FS, MMCSD_MODE_EMMCBOOT, or
+ * MMCSD_MODE_RAW for each MMC boot source which is defined for the target.
The
+ * boot_device parameter tells which device the MMC driver is interested in.
+ *
+ * If not overridden, it is weakly defined in common/spl/spl_mmc.c.
+ *
+ * Note: It is important to use the boot_device parameter instead of e.g.
+ * spl_boot_device() as U-Boot is not always loaded from the same device as
SPL.
+ */
+u32 spl_mmc_boot_mode(const u32 boot_device);
int spl_boot_partition(const u32 boot_device);
void spl_set_bd(void);
diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
index a2ea363e96a9..fb8ad5d54006 100644
--- a/common/spl/spl_mmc.c
+++ b/common/spl/spl_mmc.c
@@ -298,7 +298,7 @@ static int spl_mmc_do_fs_boot(struct spl_image_info
*spl_image, struct mmc *mmc,
}
#endif
-u32 __weak spl_boot_mode(const u32 boot_device)
+u32 __weak spl_mmc_boot_mode(const u32 boot_device)
{
#if defined(CONFIG_SPL_FS_FAT) || defined(CONFIG_SPL_FS_EXT4)
return MMCSD_MODE_FS;
@@ -350,7 +350,7 @@ int spl_mmc_load(struct spl_image_info *spl_image,
}
}
- boot_mode = spl_boot_mode(bootdev->boot_device);
+ boot_mode = spl_mmc_boot_mode(bootdev->boot_device);
err = -EINVAL;
switch (boot_mode) {
case MMCSD_MODE_EMMCBOOT:
diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
index 87dbdf3011ee..49bb3b928da1 100644
--- a/arch/arm/mach-imx/spl.c
+++ b/arch/arm/mach-imx/spl.c
@@ -189,7 +189,7 @@ int g_dnl_bind_fixup(struct usb_device_descriptor *dev,
const char *name)
#if defined(CONFIG_SPL_MMC_SUPPORT)
/* called from spl_mmc to see type of boot mode for storage (RAW or FAT) */
-u32 spl_boot_mode(const u32 boot_device)
+u32 spl_mmc_boot_mode(const u32 boot_device)
{
#if defined(CONFIG_MX7) || defined(CONFIG_IMX8M) || defined(CONFIG_IMX8)
switch (get_boot_device()) {
diff --git a/arch/arm/mach-k3/am6_init.c b/arch/arm/mach-k3/am6_init.c
index 3768bccafa63..b692806352c8 100644
--- a/arch/arm/mach-k3/am6_init.c
+++ b/arch/arm/mach-k3/am6_init.c
@@ -199,7 +199,7 @@ void board_init_f(ulong dummy)
#endif
}
-u32 spl_boot_mode(const u32 boot_device)
+u32 spl_mmc_boot_mode(const u32 boot_device)
{
#if defined(CONFIG_SUPPORT_EMMC_BOOT)
u32 devstat = readl(CTRLMMR_MAIN_DEVSTAT);
diff --git a/arch/arm/mach-k3/j721e_init.c b/arch/arm/mach-k3/j721e_init.c
index f34090f9cc99..71fc20c30bfc 100644
--- a/arch/arm/mach-k3/j721e_init.c
+++ b/arch/arm/mach-k3/j721e_init.c
@@ -223,7 +223,7 @@ void board_init_f(ulong dummy)
#endif
}
-u32 spl_boot_mode(const u32 boot_device)
+u32 spl_mmc_boot_mode(const u32 boot_device)
{
switch (boot_device) {
case BOOT_DEVICE_MMC1:
diff --git a/arch/arm/mach-omap2/boot-common.c
b/arch/arm/mach-omap2/boot-common.c
index 734fa9d9e6f7..753852372431 100644
--- a/arch/arm/mach-omap2/boot-common.c
+++ b/arch/arm/mach-omap2/boot-common.c
@@ -187,7 +187,7 @@ u32 spl_boot_device(void)
return gd->arch.omap_boot_device;
}
-u32 spl_boot_mode(const u32 boot_device)
+u32 spl_mmc_boot_mode(const u32 bo
Re: [PATCH v2] imx: Introduce CONFIG_SPL_FORCE_MMC_BOOT to force MMC boot on falcon mode
Hello Marek,
On Wed, 2020-04-08 at 15:45 +0200, Marek Vasut wrote:
> On 4/8/20 2:42 PM, Harald Seiler wrote:
> > Hello,
>
> Hi,
>
> > On Mon, 2019-09-09 at 15:32 +0200, Lukasz Majewski wrote:
> > > This change tries to fix the following problem:
> > >
> > > - The board boots (to be more precise - ROM loads SPL) from a slow SPI-NOR
> > > memory.
> > > As a result the spl_boot_device() will return SPI-NOR as a boot device
> > > (which is correct).
> > >
> > > - The problem is that in 'falcon boot' the eMMC is used as a boot medium
> > > to
> > > load kernel from its partition.
> > > Calling spl_boot_device() will break things as it returns SPI-NOR
> > > device.
> > >
> > > To fix this issue the new CONFIG_SPL_FORCE_MMC_BOOT Kconfig flag is
> > > introduced to handle this special use case. By default it is not defined,
> > > so there is no change in the legacy code flow.
> >
> > I want to pick up this discussion (and the previous discussion about
> > Anatolij's rejected patch [1]) again, because this
>
> Can you define "this" ? What is not correct, that the patch was rejected
> or this patch ?
Right, sorry. I'm talking about the use of spl_boot_device() in the
switch-statement of spl_boot_mode(). That means, I think rejecting
Anatolij's original patch was wrong and this patch should not have been
necessary as what now would be CONFIG_SPL_FORCE_MMC_BOOT=y is the only
correct behavior (but it is not the default).
> > does not seem correct
> > to me. Also, through the addition of imx8 support, the state has worsened
> > further and I'd like to have this become more consistent again.
> >
> > Digging deep into the history, the `boot_device` parameter to
> > `spl_boot_mode` was introduced by Marek in commit 2b1cdafa9fdd ("common:
> > Pass the boot device into spl_boot_mode()"). The intention was to fix
> > exactly the problem which Anatolij encountered. For reference:
> >
> > common: Pass the boot device into spl_boot_mode()
> >
> > The SPL code already knows which boot device it calls the
> > spl_boot_mode()
> > on, so pass that information into the function. This allows the code of
> > spl_boot_mode() avoid invoking spl_boot_device() again, but it also lets
> > board_boot_order() correctly alter the behavior of the boot process.
> >
> > The later one is important, since in certain cases, it is desired that
> > spl_boot_device() return value be overriden using board_boot_order().
>
> Note that the entire madness above was needed for 8997de292a8b to work.
>
> ARM: at91: ma5d4: Boot from MMC2 when using SAM-BA
>
> Continue loading U-Boot from MMC2 when the SPL was loaded using SAM-BA
> loader. This allows the board to boot system from the removable media
> instead of the eMMC, which is useful for commissioning purposes. When
> booting from the eMMC, always boot from it as it is not possible to
> boot from the SD interface directly.
I see. Well, and trying to do the same thing on an IMX would not work at
the moment, because of the issue I am trying to describe.
> > It seems to me that using spl_boot_device() instead of the `boot_device`
> > parameter cannot be correct here (If I am wrong about the following,
> > please correct me!):
> >
> > spl_boot_mode() is essentially a lookup function which is used by the SPL
> > MMC driver (here [2]) to find out the 'mode' of the currently attempted
> > MMC device. That is, for each MMC device, it should tell the driver
> > whether this device has a FAT/ext4 filesystem (MMCSD_MODE_FS), is using an
> > eMMC boot-partition (MMCSD_MODE_EMMCBOOT), or should be accessed directly
> > (MMCSD_MODE_RAW).
>
> Yes
>
> > spl_boot_device() returns the device which SPL was booted from.
>
> Yes
>
> > Now because in most cases U-Boot Proper is loaded from the same MMC device
> > which the SPL was originally loaded from, the current code often
> > mistakenly does the right thing. But when this is not the case (e.g.
> > because a board_boot_order() was defined), it is obviously not correct to
> > return the mode of the MMC device which SPL was loaded from instead of the
> > mode of the device which the MMC driver is currently attempting to access.
> >
> > So, I think the function should in all circumstances use its `boot_device`
> > parameter to behave correctly (and all other implementations do this, from
> > what I can tell). It might make sense to rename it, though. It is not
> > really about the 'spl boot mode', but much more about 'mmc device mode'.
> >
> > I'd send a patch-series but first I'd like some input whether I am correct
> > about this ...
> >
> > [1]: https://patchwork.ozlabs.org/patch/796237/
> > [2]:
> > https://gitlab.denx.de/u-boot/u-boot/-/blob/v2020.01/common/spl/spl_mmc.c#L346
>
> I think you are correct about this.
--
Harald
Re: Basic syntax question
Hello Damien, On Wed, 2020-04-08 at 11:27 +0300, Damien LEFEVRE wrote: > Hi, > > I'm new to u-boot. I'm trying to store the return value of gpio command to > a variable. > > I have the following, (switch released): > Tegra186 (P2771--500) # gpio input FF1 > gpio: pin FF1 (gpio 241) value is 1 > Tegra186 (P2771--500) # setexpr X 'gpio input FF1' > Tegra186 (P2771--500) # echo $X > 0 > > But I'm expecting X to be 1, the return value of gpio. What you want to do is look at the return value of the gpio command. The return value in U-Boot is stored in a variable $?, similar to POSIX shells. As this variable is always the return value of the last command, you have to save its value immediately after running `gpio input`: => gpio input FF1 => setenv X $? Afterwards, you can continue however you like, e.g. => echo $X > If I press the switch I see the value change to 0, so I know the gpio state > changes > Tegra186 (P2771--500) # gpio input FF1 > gpio: pin FF1 (gpio 241) value is 0 > > Thanks, > -Damien Hope this helps! -- Harald
Re: [PATCH v2] imx: Introduce CONFIG_SPL_FORCE_MMC_BOOT to force MMC boot on falcon mode
Hello,
On Mon, 2019-09-09 at 15:32 +0200, Lukasz Majewski wrote:
> This change tries to fix the following problem:
>
> - The board boots (to be more precise - ROM loads SPL) from a slow SPI-NOR
> memory.
> As a result the spl_boot_device() will return SPI-NOR as a boot device
> (which is correct).
>
> - The problem is that in 'falcon boot' the eMMC is used as a boot medium to
> load kernel from its partition.
> Calling spl_boot_device() will break things as it returns SPI-NOR device.
>
> To fix this issue the new CONFIG_SPL_FORCE_MMC_BOOT Kconfig flag is
> introduced to handle this special use case. By default it is not defined,
> so there is no change in the legacy code flow.
I want to pick up this discussion (and the previous discussion about
Anatolij's rejected patch [1]) again, because this does not seem correct
to me. Also, through the addition of imx8 support, the state has worsened
further and I'd like to have this become more consistent again.
Digging deep into the history, the `boot_device` parameter to
`spl_boot_mode` was introduced by Marek in commit 2b1cdafa9fdd ("common:
Pass the boot device into spl_boot_mode()"). The intention was to fix
exactly the problem which Anatolij encountered. For reference:
common: Pass the boot device into spl_boot_mode()
The SPL code already knows which boot device it calls the spl_boot_mode()
on, so pass that information into the function. This allows the code of
spl_boot_mode() avoid invoking spl_boot_device() again, but it also lets
board_boot_order() correctly alter the behavior of the boot process.
The later one is important, since in certain cases, it is desired that
spl_boot_device() return value be overriden using board_boot_order().
It seems to me that using spl_boot_device() instead of the `boot_device`
parameter cannot be correct here (If I am wrong about the following,
please correct me!):
spl_boot_mode() is essentially a lookup function which is used by the SPL
MMC driver (here [2]) to find out the 'mode' of the currently attempted
MMC device. That is, for each MMC device, it should tell the driver
whether this device has a FAT/ext4 filesystem (MMCSD_MODE_FS), is using an
eMMC boot-partition (MMCSD_MODE_EMMCBOOT), or should be accessed directly
(MMCSD_MODE_RAW).
spl_boot_device() returns the device which SPL was booted from.
Now because in most cases U-Boot Proper is loaded from the same MMC device
which the SPL was originally loaded from, the current code often
mistakenly does the right thing. But when this is not the case (e.g.
because a board_boot_order() was defined), it is obviously not correct to
return the mode of the MMC device which SPL was loaded from instead of the
mode of the device which the MMC driver is currently attempting to access.
So, I think the function should in all circumstances use its `boot_device`
parameter to behave correctly (and all other implementations do this, from
what I can tell). It might make sense to rename it, though. It is not
really about the 'spl boot mode', but much more about 'mmc device mode'.
I'd send a patch-series but first I'd like some input whether I am correct
about this ...
[1]: https://patchwork.ozlabs.org/patch/796237/
[2]:
https://gitlab.denx.de/u-boot/u-boot/-/blob/v2020.01/common/spl/spl_mmc.c#L346
> Signed-off-by: Lukasz Majewski
> ---
>
> Changes in v2:
> - Update/enhance the Kconfig description of the SPL_FORCE_MMC_BOOT.
>
> This patch is a follow up of previous discussion/fix:
> https://patchwork.ozlabs.org/patch/796237/
>
> Travis-CI:
> https://travis-ci.org/lmajewski/u-boot-dfu/builds/580272414
> ---
> arch/arm/mach-imx/spl.c | 11 +++
> common/spl/Kconfig | 9 +
> 2 files changed, 20 insertions(+)
>
> diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
> index 1f230aca3397..daa3d8f7ed94 100644
> --- a/arch/arm/mach-imx/spl.c
> +++ b/arch/arm/mach-imx/spl.c
> @@ -178,7 +178,18 @@ int g_dnl_bind_fixup(struct usb_device_descriptor *dev,
> const char *name)
> /* called from spl_mmc to see type of boot mode for storage (RAW or FAT) */
> u32 spl_boot_mode(const u32 boot_device)
> {
> +/*
> + * When CONFIG_SPL_FORCE_MMC_BOOT is defined the 'boot_device' is used
> + * unconditionally to decide about device to use for booting.
> + * This is crucial for falcon boot mode, when board boots up (i.e. ROM
> + * loads SPL) from slow SPI-NOR memory and afterwards the SPL's 'falcon' boot
> + * mode is used to load Linux OS from eMMC partition.
> + */
> +#ifdef CONFIG_SPL_FORCE_MMC_BOOT
> + switch (boot_device) {
> +#else
> switch (spl_boot_device()) {
> +#endif
> /* for MMC return either RAW or FAT mode */
> case BOOT_DEVICE_MMC1:
> case BOOT_DEVICE_MMC2:
> diff --git a/common/spl/Kconfig b/common/spl/Kconfig
> index f467eca2be72..fe800840beb6 100644
> --- a/common/spl/Kconfig
> +++ b/common/spl/Kconfig
> @@ -607,6 +607,15 @@ config SPL_MMC_SUPPORT
> this
[PATCH] test/py: mmc: Fix 'mmc info' testcase
Commit 41e30dcf8796 ("cmd: mmc: Make Mode: printout consistent") fixed
the layout of `mmc info` output. Reflect this change in the respective
testcase.
Also fix a typo in the documentation.
Fixes: 41e30dcf8796 ("cmd: mmc: Make Mode: printout consistent")
Signed-off-by: Harald Seiler
---
test/py/tests/test_mmc_rd.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/test/py/tests/test_mmc_rd.py b/test/py/tests/test_mmc_rd.py
index a25aa5f6f78e..ea652f913618 100644
--- a/test/py/tests/test_mmc_rd.py
+++ b/test/py/tests/test_mmc_rd.py
@@ -56,7 +56,7 @@ env__mmc_dev_configs = (
'info_mode': ???,
'info_buswidth': ???.
},
-}
+)
# Configuration data for test_mmc_rd; defines regions of the MMC (entire
# devices, or ranges of sectors) which can be read:
@@ -210,7 +210,7 @@ def test_mmc_info(u_boot_console, env__mmc_dev_config):
assert good_response in response
good_response = "Bus Speed: %s" % info_speed
assert good_response in response
-good_response = "Mode : %s" % info_mode
+good_response = "Mode: %s" % info_mode
assert good_response in response
good_response = "Bus Width: %s" % info_buswidth
assert good_response in response
--
2.25.2
Re: [PATCH V3 10/13] net: smc911x: Pass around driver private data
Hi Marek,
On Wed, 2020-03-25 at 16:36 +0100, Marek Vasut wrote:
> Introduce a private data structure for this driver with embedded
> struct eth_device and pass it around. This prepares the driver to
> work with both DM and non-DM systems.
>
> Signed-off-by: Marek Vasut
> Cc: Joe Hershberger
> Cc: Masahiro Yamada
> ---
> V2: No change
> V3: No change
> ---
> drivers/net/smc911x.c | 230 ++
> 1 file changed, 123 insertions(+), 107 deletions(-)
>
> diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c
> index 364f8c5da8..786ab220b2 100644
> --- a/drivers/net/smc911x.c
> +++ b/drivers/net/smc911x.c
> @@ -20,6 +20,13 @@ struct chip_id {
> char *name;
> };
>
> +struct smc911x_priv {
> + struct eth_device dev;
> + phys_addr_t iobase;
> + const struct chip_id*chipid;
> + unsigned char enetaddr[6];
> +};
> +
> static const struct chip_id chip_ids[] = {
> { CHIP_89218, "LAN89218" },
> { CHIP_9115, "LAN9115" },
> @@ -45,57 +52,57 @@ static const struct chip_id chip_ids[] = {
> #endif
>
> #if defined (CONFIG_SMC911X_32_BIT)
> -static u32 smc911x_reg_read(struct eth_device *dev, u32 offset)
> +static u32 smc911x_reg_read(struct smc911x_priv *priv, u32 offset)
> {
> - return readl(dev->iobase + offset);
> + return readl(priv->iobase + offset);
> }
>
> -static void smc911x_reg_write(struct eth_device *dev, u32 offset, u32 val)
> +static void smc911x_reg_write(struct smc911x_priv *priv, u32 offset, u32 val)
> {
> - writel(val, dev->iobase + offset);
> + writel(val, priv->iobase + offset);
> }
> #elif defined (CONFIG_SMC911X_16_BIT)
> -static u32 smc911x_reg_read(struct eth_device *dev, u32 offset)
> +static u32 smc911x_reg_read(struct smc911x_priv *priv, u32 offset)
> {
> - return (readw(dev->iobase + offset) & 0x) |
> -(readw(dev->iobase + offset + 2) << 16);
> + return (readw(priv->iobase + offset) & 0x) |
> +(readw(priv->iobase + offset + 2) << 16);
> }
> -static void smc911x_reg_write(struct eth_device *dev, u32 offset, u32 val)
> +static void smc911x_reg_write(struct smc911x_priv *priv, u32 offset, u32 val)
> {
> - writew(val & 0x, dev->iobase + offset);
> - writew(val >> 16, dev->iobase + offset + 2);
> + writew(val & 0x, priv->iobase + offset);
> + writew(val >> 16, priv->iobase + offset + 2);
> }
> #else
> #error "SMC911X: undefined bus width"
> #endif /* CONFIG_SMC911X_16_BIT */
>
> -static u32 smc911x_get_mac_csr(struct eth_device *dev, u8 reg)
> +static u32 smc911x_get_mac_csr(struct smc911x_priv *priv, u8 reg)
> {
> - while (smc911x_reg_read(dev, MAC_CSR_CMD) & MAC_CSR_CMD_CSR_BUSY)
> + while (smc911x_reg_read(priv, MAC_CSR_CMD) & MAC_CSR_CMD_CSR_BUSY)
> ;
> - smc911x_reg_write(dev, MAC_CSR_CMD,
> + smc911x_reg_write(priv, MAC_CSR_CMD,
> MAC_CSR_CMD_CSR_BUSY | MAC_CSR_CMD_R_NOT_W | reg);
> - while (smc911x_reg_read(dev, MAC_CSR_CMD) & MAC_CSR_CMD_CSR_BUSY)
> + while (smc911x_reg_read(priv, MAC_CSR_CMD) & MAC_CSR_CMD_CSR_BUSY)
> ;
>
> - return smc911x_reg_read(dev, MAC_CSR_DATA);
> + return smc911x_reg_read(priv, MAC_CSR_DATA);
> }
>
> -static void smc911x_set_mac_csr(struct eth_device *dev, u8 reg, u32 data)
> +static void smc911x_set_mac_csr(struct smc911x_priv *priv, u8 reg, u32 data)
> {
> - while (smc911x_reg_read(dev, MAC_CSR_CMD) & MAC_CSR_CMD_CSR_BUSY)
> + while (smc911x_reg_read(priv, MAC_CSR_CMD) & MAC_CSR_CMD_CSR_BUSY)
> ;
> - smc911x_reg_write(dev, MAC_CSR_DATA, data);
> - smc911x_reg_write(dev, MAC_CSR_CMD, MAC_CSR_CMD_CSR_BUSY | reg);
> - while (smc911x_reg_read(dev, MAC_CSR_CMD) & MAC_CSR_CMD_CSR_BUSY)
> + smc911x_reg_write(priv, MAC_CSR_DATA, data);
> + smc911x_reg_write(priv, MAC_CSR_CMD, MAC_CSR_CMD_CSR_BUSY | reg);
> + while (smc911x_reg_read(priv, MAC_CSR_CMD) & MAC_CSR_CMD_CSR_BUSY)
> ;
> }
>
> -static int smc911x_detect_chip(struct eth_device *dev)
> +static int smc911x_detect_chip(struct smc911x_priv *priv)
> {
> unsigned long val, i;
>
> - val = smc911x_reg_read(dev, BYTE_TEST);
> + val = smc911x_reg_read(priv, BYTE_TEST);
> if (val == 0x) {
> /* Special case -- no chip present */
> return -1;
> @@ -104,7 +111,7 @@ static int smc911x_detect_chip(struct eth_device *dev)
> return -1;
> }
>
> - val = smc911x_reg_read(dev, ID_REV) >> 16;
> + val = smc911x_reg_read(priv, ID_REV) >> 16;
> for (i = 0; chip_ids[i].id != 0; i++) {
> if (chip_ids[i].id == val) break;
> }
> @@ -113,12 +120,12 @@ static int smc911x_detect_chip(struct eth_device *dev)
> return -1;
> }
>
> - dev->priv = (void *)_ids[i];
> + priv->chipid = _ids[i];
>
> return 0;
> }
>
> -static void
Re: [U-Boot] Sharing a hardware lab
Hello Simon,
On Sun, 2020-03-22 at 12:42 -0600, Simon Glass wrote:
> Hi Harald,
>
> On Sun, 22 Mar 2020 at 03:56, Harald Seiler wrote:
> > Hi Simon,
> >
> > On Sat, 2020-03-21 at 13:07 -0600, Simon Glass wrote:
[...]
> >
> > Huh, actually I messed up in the patch I sent you. Please apply the
> > following
> > diff:
> >
> > diff --git a/kea.py b/kea.py
> > index a3ca968dc41e..f65d91b67f1d 100644
> > --- a/kea.py
> > +++ b/kea.py
> > @@ -39,7 +39,7 @@ class KeaLab(
> > # toolchain is identified by a (unique) string. For pcduino3 in
> > this
> > # example, a toolchain named `armv7-a` is defined.
> > return {
> > -"armv7-a": ArmV7Toolchain,
> > +"armv7-a": ArmV7Toolchain(),
> > }
> >
> > def build(self):
> >
> > This will make everything work as intended and the patch you needed in tbot
> > is
> > also no longer necessary (Passing a class instead of an instance will not
> > bind the method's `self` argument so you got an error from a weird place.
> > Maybe I should add a check against this kind of mistake ...).
>
> OK that fixes it, thanks.
>
> Actually I can call 'buildman -A ' to get the toolchain prefix
> for a board, so perhaps I should figure out how to work that in
> somehow, and just have a generic toolchain problem,
That's not too difficult, actually. I suggest creating a generic UBootBuilder
class from which your individual board's builders inherit:
import abc
import contextlib
class BuildmanUBootBuilder(uboot.UBootBuilder):
@abc.abstractmethod
@property
def buildman_board(self) -> str:
raise Exception("abstract method")
# Overload `do_toolchain` to customize toolchain behavior
@contextlib.contextmanager
def do_toolchain(self, bh):
prefix = bh.exec0("buildman", "-A", self.buildman_board)
with bh.subshell():
# Setup toolchain here
bh.env("ARCH", "arm")
yield None
# and for each board
class MyBoardUBootBuilder(BuildmanUBootBuilder):
buildman_board = "tegra"
I have never used buildman myself so I am not really sure about the
details of what needs to be done here ... But if this would be a useful
feature I could look into supporting it directly in the UBootBuilder
class.
> > > So my next question is how to load U-Boot onto the board. I can see
> > > methods for poweron/poweroff but not for actually writing to the
> > > board. Is there a built-in structure for this? I cannot find it.
> > >
> > > I am hoping that the Pcduino3 class can define a method like
> > > 'load_uboot()' to write U-Boot to the board?
> >
> > I added something similar to this in our DENX internal tbot configurations
> > but
> > did not yet publish it anywhere (maybe I should add it to tbot_contrib?).
> > Just
> > for you, here is what I did:
>
> To me this should be a core feature, not contrib.
I guess the split of what belongs where is not yet clearly defined and I still
need to work on separating the different components better. For me, core is
just the code for interacting with machines; anything that looks like
a testcase should be separate from that. Right now I've got tbot_contrib for
this but I guess the naming might not be optimal ...
By the way, are you subscribed to t...@lists.denx.de? I'll be posting updates
regarding this (and other things) over there.
> I wonder if there is an upstream for tbot labs? I think it would be
> useful to have a directory containing people's labs like we do for
> pytest. Then we can end up with common functions for doing things.
Maybe it makes sense to add a place in upstream tbot for these as well.
Thanks for the idea!
> > 1. In the board configs, each U-Boot class defines a `flash()` method:
> >
> > from tbot.tc import shell, git
> >
> > class P2020RdbUBoot(board.Connector, board.UBootShell):
> > name = "p2020rdb-uboot"
> > prompt = "=> "
> > build = P2020RdbUBootBuilder()
> >
> > def flash(self, repo: git.GitRepository) -> None:
> > self.env("autoload", "no")
> > self.exec0("dhcp")
> > self.env("serverip", "192.168.1.1")
> >
> > tftpdir = self.host.fsroot / "tftpboot" / "p2020rdb" / "tbot2"
> > if not tftpdir.is_dir():
> > self.hos
Re: [U-Boot] Sharing a hardware lab
Hi Simon,
On Sat, 2020-03-21 at 13:07 -0600, Simon Glass wrote:
> Hi Harald,
>
> On Mon, 24 Feb 2020 at 06:27, Harald Seiler wrote:
> > Hello Simon,
> >
> > On Sun, 2020-02-23 at 19:34 -0700, Simon Glass wrote:
> > > Hi Heiko,
> > >
> > > Thanks for the hints! I pushed your things here:
> > >
> > > https://github.com/sglass68/tbot/tree/simon
> > >
> > > Then I try:
> > > tbot -l kea.py -b pcduino3.py uboot_build
> > >
> > > and get an error:
> > >
> > > tbot starting ...
> > > type
> > > ├─Calling uboot_build ...
> > > │ └─Fail. (0.000s)
> > > ├─Exception:
> > > │ Traceback (most recent call last):
> > > │ File
> > > "/home/sglass/.local/lib/python3.6/site-packages/tbot-0.8.0-py3.6.egg/tbot/main.py",
> > > line 318, in main
> > > │ func(**params)
> > > │ File
> > > "/home/sglass/.local/lib/python3.6/site-packages/tbot-0.8.0-py3.6.egg/tbot/decorators.py",
> > > line 103, in wrapped
> > > │ result = tc(*args, **kwargs)
> > > │ File
> > > "/home/sglass/.local/lib/python3.6/site-packages/tbot-0.8.0-py3.6.egg/tbot/tc/uboot/build.py",
> > > line 271, in _build
> > > │ builder = UBootBuilder._get_selected_builder()
> > > │ File
> > > "/home/sglass/.local/lib/python3.6/site-packages/tbot-0.8.0-py3.6.egg/tbot/tc/uboot/build.py",
> > > line 160, in _get_selected_builder
> > > │ builder = getattr(tbot.selectable.UBootMachine, "build")
> > > │ AttributeError: type object 'UBootMachine' has no attribute 'build'
> > >
> > > I'm a bit lost in all the classes and functions. A working example or
> > > a patch would be great!
> > >
> > > I've pushed all my scripts here:
> > >
> > > https://github.com/sglass68/ubtest
> > >
> > > The top commit is your patches.
> >
> > I think you mixed a few things up while adding Heiko's stuff. Instead
> > of your last commit, attempt the attached patch. It is untested of
> > course but should point you in the right direction; here is a short
> > summary of what it adds:
> >
> > 1. Your `kea` lab needs to be marked as a build-host. This means it
> >needs the `toolchains` property which returns what toolchains are
> >available. I added a dummy armv7-a toolchain which might need
> >a bit more adjustment to work for you.
> >
> > 2. I created a UBootBuilder for pcduino3. This builder just
> >specifies what defconfig to build and what toolchain to use (need
> >to be defined in the selected lab).
> >
> >Heiko's builder config is a bit more elaborate and also does some
> >patching after applying the defconfig. This is of course also
> >possible if you want to.
> >
> > 3. I added a U-Boot config for your board which is needed because
> >its `build` property specifies which U-Boot builder to use. This
> >will make the `uboot_build` testcase work properly. Later you
> >might want to adjust this U-Boot config to actually work so you
> >can make use of it for flashing the new U-Boot binary.
> >
> > Some more links to documentation:
> >
> > - Build-host config:
> > https://tbot.tools/modules/machine_linux.html#builder
> > - UBootBuilder class:
> > https://tbot.tools/modules/tc.html#tbot.tc.uboot.UBootBuilder
> >
> > Hope this helps!
>
> Yes it helps a lot, thank you. I now have it building U-Boot:
>
>tbot -l kea.py -b pcduino3.py -p clean=False uboot_build interactive_uboot
>
> I sent a tiny patch to tbot to fix an error I had, and I made a few
> minor changes to what you sent.
>
> https://github.com/sglass68/ubtest/commit/7da7a3794d505e970e0e21a9b6ed3a7e5f2f2190
Huh, actually I messed up in the patch I sent you. Please apply the following
diff:
diff --git a/kea.py b/kea.py
index a3ca968dc41e..f65d91b67f1d 100644
--- a/kea.py
+++ b/kea.py
@@ -39,7 +39,7 @@ class KeaLab(
# toolchain is identified by a (unique) string. For pcduino3 in this
# example, a toolchain named `armv7-a` is defined.
return {
-"armv7-a": ArmV7Toolchain,
+"armv7-a": ArmV7Toolchain(),
}
def build(self):
This will make everything work as intended and the patch you needed in tbot is
also no longer necessary (Passing a class instead of an instance will not
bind
Re: [RFC PATCH 3/9] mksunxi_fit_atf.sh: produce working binaries by default
Hi, On Wed, 2020-03-18 at 10:57 +0100, Petr Štetiar wrote: > At this moment unusable binaries are produced if bl31.bin file is > missing in order to allow passing of various CI tests. This intention of > broken binaries has to be now explicitly confirmed via new > BUILDBOT_BROKEN_BINARIES config option, so usable binaries are produced > by default from now on. > > Signed-off-by: Petr Štetiar > --- > board/sunxi/mksunxi_fit_atf.sh | 10 +++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/board/sunxi/mksunxi_fit_atf.sh b/board/sunxi/mksunxi_fit_atf.sh > index 88ad71974706..708b4248549d 100755 > --- a/board/sunxi/mksunxi_fit_atf.sh > +++ b/board/sunxi/mksunxi_fit_atf.sh > @@ -8,9 +8,13 @@ > [ -z "$BL31" ] && BL31="bl31.bin" > > if [ ! -f $BL31 ]; then > - echo "WARNING: BL31 file $BL31 NOT found, resulting binary is > non-functional" >&2 > - echo "Please read the section on ARM Trusted Firmware (ATF) in > board/sunxi/README.sunxi64" >&2 > - BL31=/dev/null > + if [ "$BUILDBOT_BROKEN_BINARIES" = "y" ]; then > + BL31=/dev/null > + else > + echo "ERROR: BL31 file $BL31 NOT found, resulting binary is > non-functional" >&2 This error message is not quite correct anymore because no binary is built now. > + echo "Please read the section on ARM Trusted Firmware (ATF) in > board/sunxi/README.sunxi64" >&2 Not only relevant to this patch, but to the following ones as well: Maybe it makes sense to mention CONFIG_BUILDBOT_BROKEN_BINARIES in the error message so people who are not aware of this change and want to continue building broken binaries don't have to search the list archive to find out what to do? For example: + echo "ERROR: BL31 file $BL31 NOT found." >&2 + echo "Please read the section on ARM Trusted Firmware (ATF) in board/sunxi/README.sunxi64" >&2 + echo "" >&2 + echo "If you want to build without a BL31 file (creating a NON-FUNCTIONAL binary), please" >&2 + echo "enable CONFIG_BUILDBOT_BROKEN_BINARIES." >&2 > + exit 1 > + fi > fi > > if grep -q "^CONFIG_MACH_SUN50I_H6=y" .config; then -- Harald
Re: [PATCH 3/3] imx: imx8m: Don't use the addr parameter of reset_cpu
Hello Marek,
On Wed, 2020-03-04 at 15:30 +0100, Marek Vasut wrote:
> On 3/4/20 3:23 PM, Harald Seiler wrote:
> > From: Claudius Heine
> >
> > imx8m has the only implementation of `reset_cpu` which does not ignore
> > the addr parameter and instead gives it some meaning as the base address
> > of watchdog registers. This breaks convention with the rest of U-Boot
> > where the parameter is ignored and callers are passing in 0.
> >
> > Fixes: d2041725e84b ("imx8m: restrict reset_cpu")
> > Co-Authored-by: Harald Seiler
> > Signed-off-by: Claudius Heine
> > Signed-off-by: Harald Seiler
> > ---
> > arch/arm/mach-imx/imx8m/soc.c | 5 +
> > 1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c
> > index 7fcbd53f3020..2d3afc61a452 100644
> > --- a/arch/arm/mach-imx/imx8m/soc.c
> > +++ b/arch/arm/mach-imx/imx8m/soc.c
> > @@ -385,10 +385,7 @@ int ft_system_setup(void *blob, bd_t *bd)
> > #if defined(CONFIG_SPL_BUILD) || !defined(CONFIG_SYSRESET)
> > void reset_cpu(ulong addr)
> > {
> > - struct watchdog_regs *wdog = (struct watchdog_regs *)addr;
> > -
> > - if (!addr)
> > - wdog = (struct watchdog_regs *)WDOG1_BASE_ADDR;
> > + struct watchdog_regs *wdog = (struct watchdog_regs
> > *)WDOG1_BASE_ADDR;
> >
> > /* Clear WDA to trigger WDOG_B immediately */
> > writew((WCR_WDE | WCR_SRS), >wcr);
>
> Can't we remove the param altogether ?
Yes, that is what I would suggest as well. Although I'd do that as
a separate follow up because it is not ARM nor i.MX specific.
> Otherwise
> Reviewed-by: Marek Vasut
--
Harald
[PATCH 2/3] imx: imx8m*: Remove do_reset from board files
From: Claudius Heine
Use the `do_reset` implementation of `arch/arm/lib/reset.c` in SPL
instead. It is very close to what is done here, anyway, and plays
more nicely with the rest of U-Boot than adding a custom `do_reset`
implementation into board files.
`do_reset` from `arch/arm/lib/reset.c` calls `reset_cpu` with 0 as the
addr parameter while the boards are passing WDOG1_BASE_ADDR. This is
ok because the `reset_cpu` implementation uses WDOG1_BASE_ADDR by
default if 0 is passed in.
Co-Authored-by: Harald Seiler
Signed-off-by: Claudius Heine
Signed-off-by: Harald Seiler
---
board/freescale/imx8mm_evk/spl.c | 9 -
board/freescale/imx8mn_evk/spl.c | 9 -
board/freescale/imx8mp_evk/spl.c | 9 -
board/toradex/verdin-imx8mm/spl.c | 9 -
4 files changed, 36 deletions(-)
diff --git a/board/freescale/imx8mm_evk/spl.c b/board/freescale/imx8mm_evk/spl.c
index 5d17f397cb68..4d34622465b3 100644
--- a/board/freescale/imx8mm_evk/spl.c
+++ b/board/freescale/imx8mm_evk/spl.c
@@ -161,12 +161,3 @@ void board_init_f(ulong dummy)
board_init_r(NULL, 0);
}
-
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
-{
- puts ("resetting ...\n");
-
- reset_cpu(WDOG1_BASE_ADDR);
-
- return 0;
-}
diff --git a/board/freescale/imx8mn_evk/spl.c b/board/freescale/imx8mn_evk/spl.c
index 7aed14c52b68..45417b24464d 100644
--- a/board/freescale/imx8mn_evk/spl.c
+++ b/board/freescale/imx8mn_evk/spl.c
@@ -114,12 +114,3 @@ void board_init_f(ulong dummy)
board_init_r(NULL, 0);
}
-
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
-{
- puts("resetting ...\n");
-
- reset_cpu(WDOG1_BASE_ADDR);
-
- return 0;
-}
diff --git a/board/freescale/imx8mp_evk/spl.c b/board/freescale/imx8mp_evk/spl.c
index 0b20668e2b30..39c1dae684ac 100644
--- a/board/freescale/imx8mp_evk/spl.c
+++ b/board/freescale/imx8mp_evk/spl.c
@@ -149,12 +149,3 @@ void board_init_f(ulong dummy)
board_init_r(NULL, 0);
}
-
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
-{
- puts("resetting ...\n");
-
- reset_cpu(WDOG1_BASE_ADDR);
-
- return 0;
-}
diff --git a/board/toradex/verdin-imx8mm/spl.c
b/board/toradex/verdin-imx8mm/spl.c
index a5dc54082054..dc5bd84f332e 100644
--- a/board/toradex/verdin-imx8mm/spl.c
+++ b/board/toradex/verdin-imx8mm/spl.c
@@ -169,12 +169,3 @@ void board_init_f(ulong dummy)
board_init_r(NULL, 0);
}
-
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
-{
- puts("resetting ...\n");
-
- reset_cpu(WDOG1_BASE_ADDR);
-
- return 0;
-}
--
2.24.1
[PATCH 0/3] ARM: Fix reset in SPL if SYSRESET is not used
Hello, continuing on the discussion around Claudius' patch for fixing reset in SPL [1] we have taken a closer look at the issue. To quickly summarize the situation: The original patch was to enable the generic ARM implementation of `do_reset` if CONFIG_SYSRESET is not enabled in SPL. This would break compilation for some boards which define their own `do_reset` in `board/*/spl.c`. To be more specific, the following 4 boards have this custom `do_reset`: - toradex/verdin-imx8mm - freescale/imx8mn_evk - freescale/imx8mm_evk - freescale/imx8mp_evk I hope we can all agree that `do_reset` is not at all meant to be implemented in board files. From looking at the related code for imx8m, it feels like this was just a workaround hack to archieve the same thing which Claudius has fixed. So this patch series reverts the addition of `do_reset` implementations in imx8m board files and instead switches to using the proper fix provided by Claudius. Additionally, the custom do_reset implementations were passing an address (WDOG1_BASE_ADDR) to `reset_cpu()` instead of 0. This is the only place in the entire U-Boot tree where this happens. Instead, all other implementations simply ignore the parameter and 0 is passed by callers. It looks a lot like this is a legacy left-over which makes me think that using it for a (hard-coded) watchdog address is not a good idea as it breaks convention with the rest of U-Boot. [1]: https://patchwork.ozlabs.org/patch/1201959 Claudius Heine (3): ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for them imx: imx8m*: Remove do_reset from board files imx: imx8m: Don't use the addr parameter of reset_cpu arch/arm/lib/Makefile | 2 +- arch/arm/mach-imx/imx8m/soc.c | 5 + board/freescale/imx8mm_evk/spl.c | 9 - board/freescale/imx8mn_evk/spl.c | 9 - board/freescale/imx8mp_evk/spl.c | 9 - board/toradex/verdin-imx8mm/spl.c | 9 - 6 files changed, 2 insertions(+), 41 deletions(-) -- Harald DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-62 Fax: +49-8142-66989-80 Email: h...@denx.de
[PATCH 3/3] imx: imx8m: Don't use the addr parameter of reset_cpu
From: Claudius Heine
imx8m has the only implementation of `reset_cpu` which does not ignore
the addr parameter and instead gives it some meaning as the base address
of watchdog registers. This breaks convention with the rest of U-Boot
where the parameter is ignored and callers are passing in 0.
Fixes: d2041725e84b ("imx8m: restrict reset_cpu")
Co-Authored-by: Harald Seiler
Signed-off-by: Claudius Heine
Signed-off-by: Harald Seiler
---
arch/arm/mach-imx/imx8m/soc.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c
index 7fcbd53f3020..2d3afc61a452 100644
--- a/arch/arm/mach-imx/imx8m/soc.c
+++ b/arch/arm/mach-imx/imx8m/soc.c
@@ -385,10 +385,7 @@ int ft_system_setup(void *blob, bd_t *bd)
#if defined(CONFIG_SPL_BUILD) || !defined(CONFIG_SYSRESET)
void reset_cpu(ulong addr)
{
- struct watchdog_regs *wdog = (struct watchdog_regs *)addr;
-
- if (!addr)
- wdog = (struct watchdog_regs *)WDOG1_BASE_ADDR;
+ struct watchdog_regs *wdog = (struct watchdog_regs *)WDOG1_BASE_ADDR;
/* Clear WDA to trigger WDOG_B immediately */
writew((WCR_WDE | WCR_SRS), >wcr);
--
2.24.1
[PATCH 1/3] ARM: reset: use do_reset in SPL/TPL if SYSRESET was not enabled for them
From: Claudius Heine In case CONFIG_SYSRESET is set, do_reset from reset.c will not be available anywere, even if SYSRESET is disabled for SPL/TPL. 'do_reset' is called from SPL for instance from the panic handler and PANIC_HANG is not set Signed-off-by: Claudius Heine --- arch/arm/lib/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile index 8482f5446c5c..b839aa7a5096 100644 --- a/arch/arm/lib/Makefile +++ b/arch/arm/lib/Makefile @@ -57,7 +57,7 @@ obj-y += interrupts_64.o else obj-y += interrupts.o endif -ifndef CONFIG_SYSRESET +ifndef CONFIG_$(SPL_TPL_)SYSRESET obj-y += reset.o endif -- 2.24.1
Re: [U-Boot] Sharing a hardware lab
'help' test. It seems to hang with > > > > > > > serial console problems if I try to do more. It is not 100% > > > > > > > reliable > > > > > > > yet. I based it on Stephen's test hooks: > > > > > > > > > > > > > > https://github.com/sglass68/uboot-test-hooks > > > > > > > > > > > > > > Is it possible to share this so that others can use the lab when > > > > > > > they > > > > > > > push trees? Is it as simple as adding to the .gitlab-ci.yml file > > > > > > > as I > > > > > > > have done here? > > > > > > > > > > > > > > https://gitlab.denx.de/u-boot/custodians/u-boot-dm/blob/gitlab-working/.gitlab-ci.yml > > > > > > > > > > > > > > I also got tbot going in a similar way, to test booting into > > > > > > > Linux. > > > > > > > Should we look at integrating that at the same time? It should be > > > > > > > fairly easy to do. > > > > > > > > > > > > > > I have quite a lot of random boards and in principle it should > > > > > > > not be > > > > > > > too hard to hook up some more of them, with sufficient SDwires, > > > > > > > hubs > > > > > > > and patience. > > > > > > > > > > Bumping this thread as I have now hooked up about about 8 mostly ARM > > > > > and x86 boards and have tbot and pytest automation mostly working for > > > > > them. > > > > > > > > Great news! > > > > > > > > added Harald Seiler to cc, as he did the tbot port to python3.6. > > > > > > > > Do you have somewhere your tbot integration online? > > > > > > https://github.com/sglass68/ubtest > > > > > > But it is tbot only. It would be good if there were a way to upstream > > > this stuff. > > > > > > For pytest I am sending upstream to: > > > > > > https://github.com/swarren/uboot-test-hooks > > > > > > BTW I have not yet got tbot to build U-Boot and write it onto the > > > board. Do you have examples for that? > > > > We have them on our gitlab server, but only private as I know. > > > > @Harald: Do you have some good examples somewhere online? > > > > May the doc help here too: > > > > http://tbot.tools/modules/tc.html#u-boot > > > > and see [1] > > > > > > I ask because on our ToDo list is to integrate pytest into tbot and may > > > > we can share work? > > > > > > > > > > There's two parts of this. The first part I think is that we need > > > > > > some > > > > > > good examples of how to have one private CI job poll / monitor other > > > > > > public jobs and run. I believe some labs do this today. This > > > > > > would be > > > > > > helpful as at least personally I'm kicking my hardware tests > > > > > > manually. > > > > > > This is because as best I can tell there isn't a way to include an > > > > > > optional stage/portion of a CI job. > > > > > > > > > > So the model here is that people with a lab 'watch' various repos? I > > > > > think that would be useful. Stephen Warren does this I think, but I'm > > > > > not sure how the builds are kicked off. > > > > > > > > > > But what about a full public lab? E.g. is it possible to add some of > > > > > the boards I have here to every build that people do? > > > > > > > > Here begins the hard game I think, because what happens if 2 builds > > > > triggered in parallel and want to test a board in the same lab > > > > at the same time? > > > > > > The gitlab-runner thing seems to handle that. > > > > Ah, so all gitlabs need to use the same gitlab runner, ok. > > > > > > If you trigger the test with tbot, it should be easy to add a locking > > > > mechanism into tbots lab specific function power_check() [1] > > > > > > > > May in this power_check() function tbot waits until it get the board... > > > > The locking mechanism itself is lab specific. > > > > > > > > > > The
Re: Boot from different partitions based on some condition
Hello Mats, On Thu, 2019-12-12 at 16:49 +0100, Mats Jansson wrote: > Hi, > > I want to set up for an upgrade of our embedded system, using a > special kernel for the upgrade. The standard will be to load the > system from mtd3 (kernel), mtd4 (devicetree) and mtd5 (rootfs). > > When an update is to take place I want to run the upgrade system from > mtd0. > > So basically I need to have some sort of telling U-boot that it should > launch the upgrade system instead of the standard system. > > Any ideas how I set this up? > How to tell u-boot? > How to get u-boot to boot from another partition based on a condition > like that? The easiest way to instruct U-Boot to boot a different system (in your case from a different partition) is to set a U-Boot environment variable from within Linux. Your boot-script in U-Boot should then check this environment variable and boot into your upgrade system. To do this, there are the envtools [1] found in the U-Boot sources in `tools/env/`. Alternatively, you can use libubootenv [2] which is a project separate from U-Boot but which archieves the same thing. Both those packages contain an `fw_setenv` tool which you can use just like `setenv` on the U-Boot command-line. The only difference is that `fw_setenv` automatically saves the environment so you don't need any `saveenv` like command afterwards. To make those tools work, you'll need to create a configuration file `/etc/fw_env.config` which tells the tool where the U-Boot environment is located. The example config [3] contains everything you need to know. > Regards > Mats mailto:matsman...@tele2.se > Hope this helps! [1]: https://gitlab.denx.de/u-boot/u-boot/tree/master/tools/env [2]: https://github.com/sbabic/libubootenv [3]: https://gitlab.denx.de/u-boot/u-boot/blob/master/tools/env/fw_env.config -- Harald
Re: gtlab slow?
Hi Simon, On Tue, 2019-12-10 at 05:48 -0700, Simon Glass wrote: > Hi, > > I sometimes find that gitlab.denx.de is very slow. Just now is is > worse than usual - it has taken almost 10 minutes to push a branch and > still not finished. I am getting a 503 error on the web site. > > Any ideas? Yes, we noticed that as well but so far have failed to find out the exact reason. Server-side we see horrible load spikes while the CPU is essentially in idle and no notable IO is happening. I personally believe it might have something to do with the virtualization technology but we don't have a certain answer yet. I added Claudius in CC, he might know more ... > Regards, > Simon -- Harald
Re: [U-Boot] [PATCH 1/4] ARM: dts: dh-imx6: add wdt-reboot node for sysreset driver
Hello Claudius, Fabio,
On Thu, 2019-11-28 at 09:49 -0300, Fabio Estevam wrote:
> Hi Claudius,
>
> On Thu, Nov 28, 2019 at 9:07 AM Claudius Heine wrote:
> > Signed-off-by: Claudius Heine
> > ---
> > arch/arm/dts/imx6qdl-dhcom-pdk2.dtsi | 5 +
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/arch/arm/dts/imx6qdl-dhcom-pdk2.dtsi
> > b/arch/arm/dts/imx6qdl-dhcom-pdk2.dtsi
> > index af4719aaeb..572bcbf8f0 100644
> > --- a/arch/arm/dts/imx6qdl-dhcom-pdk2.dtsi
> > +++ b/arch/arm/dts/imx6qdl-dhcom-pdk2.dtsi
> > @@ -30,6 +30,11 @@
> > mux-int-port = <1>;
> > mux-ext-port = <3>;
> > };
> > +
> > + wdt-reboot {
> > + compatible = "wdt-reboot";
> > + wdt = <>;
> > + };
> > };
>
> Could you use the the same way that Linux handles the imx2_wdt?
>
> Please see the commit below:
>
> commit ceea0c145d0c38badfcfc5443138e94ab094dc4a
> Author: Robert Hancock
> Date: Tue Aug 6 11:05:29 2019 -0600
>
> watchdog: imx: Add DT ext-reset handling
>
> The Linux imx2_wdt driver uses a fsl,ext-reset-output boolean in the
> device tree to specify whether the board design should use the external
> reset instead of the internal reset. Use this boolean to determine which
> mode to use rather than using external reset unconditionally.
>
> For the legacy non-DM mode, the external reset is always used in order
> to maintain the previous behavior.
I think the source of the problem lies within this: The old behavior
(before commit f2929d11a639 ("watchdog: imx: Use immediate reset bits
for expire_now")) was asserting *both* external and internal reset. The
datasheet mentions the following for the bit to enable external reset:
| There is no effect on [internal reset] upon writing on this bit.
| [External reset] gets asserted along with [internal reset] if this bit
| is set.
If the intention was to keep the old behavior, the
imx_watchdog_expire_now() function needs to be changed to either
- (ext_reset == false) write WCR_WDE | WCR_WDA (ie. assert only internal), or
- (ext_reset == true) write WCR_WDE (ie. assert both internal and external).
--
Harald
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-62 Fax: +49-8142-66989-80 Email: h...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 0/4] ARM: imx6: DHCOM i.MX6 PDK: Fixing reset
Hello Claudius,
On Thu, 2019-11-28 at 13:06 +0100, Claudius Heine wrote:
> Hi,
>
> currently the reset on the DHCOM i.MX6 board is brocken in u-boot.
>
> This patchset fixes that by integrating the sysreset and watchdog dm driver.
I think you should clarify that reset was broken by commit f2929d11a639
("watchdog: imx: Use immediate reset bits for expire_now") which changed
reset to, by default, only assert the external reset [1]. DHCOM i.MX6
needs the internal reset though, which previously was asserted as as
well. Maybe you can add a `Fixes:` line to one of your commits?
Additionally, I am still unsure whether the current default behavior is
correct. I'd rather assert both external and internal reset, which is
what the i.MX watchdog does on timeout as well (as long as WDT bit is
set, which we do by default [2]). There is also an inconsistency
between the non-DM implementation (external by default) and DM
implementation (internal by default).
> regards,
> Claudius
>
> Claudius Heine (4):
> ARM: dts: dh-imx6: add wdt-reboot node for sysreset driver
> ARM: imx6: DHCOM i.MX6 PDK: Enable sysreset driver
> ARM: imx6: DHCOM i.MX6 PDK: Enable wdt command
> ARM: imx6: DHCOM i.MX6 PDK: Use HW_WATCHDOG in SPL
>
> arch/arm/dts/imx6qdl-dhcom-pdk2.dtsi | 5 +
> configs/dh_imx6_defconfig| 3 +++
> include/configs/dh_imx6.h| 5 +
> 3 files changed, 13 insertions(+)
>
[1]:
https://gitlab.denx.de/u-boot/u-boot/blob/master/drivers/watchdog/imx_watchdog.c#L45
[2]:
https://gitlab.denx.de/u-boot/u-boot/blob/master/drivers/watchdog/imx_watchdog.c#L96
--
Harald
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v2] spl: imx: Use correct boot-device in spl_boot_mode
If a board defines a custom boot order using board_boot_order(), the
boot-device which is currently attempted might differ from the return
value of spl_boot_device(). Use the given boot_device parameter instead
of calling spl_boot_device() to prevent an incorrect "unsupported
device" error.
Supporting this use-case was the original intention for the boot_device
parameter, which was introduced in commit 2b1cdafa9fdd ("common: Pass the
boot device into spl_boot_mode()").
Fixes: 2b1cdafa9fdd ("common: Pass the boot device into spl_boot_mode()")
Signed-off-by: Harald Seiler
---
Changes for v2:
- Rebase ontop of master to fix a merge-conflict (no functional change)
arch/arm/mach-imx/spl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
index 5cc74b6f9b0f..26d07d1bf283 100644
--- a/arch/arm/mach-imx/spl.c
+++ b/arch/arm/mach-imx/spl.c
@@ -190,7 +190,7 @@ int g_dnl_bind_fixup(struct usb_device_descriptor *dev,
const char *name)
u32 spl_boot_mode(const u32 boot_device)
{
#if defined(CONFIG_MX7) || defined(CONFIG_IMX8M) || defined(CONFIG_IMX8)
- switch (get_boot_device()) {
+ switch (boot_device) {
/* for MMC return either RAW or FAT mode */
case SD1_BOOT:
case SD2_BOOT:
--
2.20.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH] spl: imx: Use correct boot-device in spl_boot_mode
If a board defines a custom boot order using board_boot_order(), the
boot-device which is currently attempted might differ from the return
value of spl_boot_device(). Use the given boot_device parameter instead
of calling spl_boot_device() to prevent an incorrect "unsupported
device" error.
Supporting this use-case was the original intention for the boot_device
parameter, which was introduced in commit 2b1cdafa9fdd ("common: Pass the
boot device into spl_boot_mode()").
Signed-off-by: Harald Seiler
---
arch/arm/mach-imx/spl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
index 1f230aca3397..d50d8e08a61c 100644
--- a/arch/arm/mach-imx/spl.c
+++ b/arch/arm/mach-imx/spl.c
@@ -178,7 +178,7 @@ int g_dnl_bind_fixup(struct usb_device_descriptor *dev,
const char *name)
/* called from spl_mmc to see type of boot mode for storage (RAW or FAT) */
u32 spl_boot_mode(const u32 boot_device)
{
- switch (spl_boot_device()) {
+ switch (boot_device) {
/* for MMC return either RAW or FAT mode */
case BOOT_DEVICE_MMC1:
case BOOT_DEVICE_MMC2:
--
2.20.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 1/3] ARM: imx: Rename imx6_is_bmode_from_gpr9 to imx6_is_bmode_from_gpr10
Hi Claudius,
On Wed, 2019-10-16 at 15:27 +0200, Claudius Heine wrote:
> The only register used in that function is gpr10, which is used to store
> the flag. So naming it after this makes sense.
I think the old name had a reason: The _value_ for bmode is stored in
GPR9 if this function returns true. But I agree that the name is a bit
confusing.
Maybe it would make more sense to call it imx6_is_bmode_from_gpr,
without any specific register name and add a comment documenting that if
bit (GPR10 & IMX6_SRC_GPR10_BMODE) is set, the bmode value is stored in
GPR9?
> Signed-off-by: Claudius Heine
> ---
> arch/arm/include/asm/mach-imx/sys_proto.h | 2 +-
> arch/arm/mach-imx/init.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/include/asm/mach-imx/sys_proto.h
> b/arch/arm/include/asm/mach-imx/sys_proto.h
> index 4925dd7894..96530e563e 100644
> --- a/arch/arm/include/asm/mach-imx/sys_proto.h
> +++ b/arch/arm/include/asm/mach-imx/sys_proto.h
> @@ -89,7 +89,7 @@ enum imx6_bmode {
> IMX6_BMODE_NAND_MAX = 0xf,
> };
>
> -static inline u8 imx6_is_bmode_from_gpr9(void)
> +static inline u8 imx6_is_bmode_from_gpr10(void)
> {
> return readl(_base->gpr10) & IMX6_SRC_GPR10_BMODE;
> }
> diff --git a/arch/arm/mach-imx/init.c b/arch/arm/mach-imx/init.c
> index b8d8d12372..b2e72d0dbd 100644
> --- a/arch/arm/mach-imx/init.c
> +++ b/arch/arm/mach-imx/init.c
> @@ -118,7 +118,7 @@ void boot_mode_apply(unsigned cfg_val)
> #if defined(CONFIG_MX6)
> u32 imx6_src_get_boot_mode(void)
> {
> - if (imx6_is_bmode_from_gpr9())
> + if (imx6_is_bmode_from_gpr10())
> return readl(_base->gpr9);
> else
> return readl(_base->sbmr1);
--
Harald
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-62 Fax: +49-8142-66989-80 Email: h...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] GitLab: make pipeline status public
Hi Heinrich, On Wed, 2019-08-07 at 21:26 +0200, Heinrich Schuchardt wrote: > Hello Harald, > > Tom suggested you could help on this issue. > > I suggest that the pipeline status should be public on all custodian > gits, e.g. page > https://gitlab.denx.de/u-boot/custodians/u-boot-efi/pipelines. > > I set the flag "Public pipelines" in the CI settings but this seems > not be sufficient. I still get a 404 error code if I am not logged on. There is another setting you need to enable, IIRC: Under Settings -> General -> Visibility, Permissions -> Pipelines , you need to change the dropdown from "Only Project Members" to "Everyone With Access". Can you check whether that works? > Best regards > > Heinrich -- Harald DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-62 Fax: +49-8142-66989-80 Email: h...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [ANNOUNCEMENT] Switching to gitlab.denx.de
Hello Peng, On Thu, 2019-06-20 at 06:42 +, Peng Fan wrote: > Hi Wolfgang, > [...] > > > > The switch will take place as follows: > > > > 1. We will create user accounts for all custodians at gitlab.denx.de > >You will receive an e-mail notification with your login > >credentials. We will start with this this afternoon (CEST), > >so if you have not received such an e-mail by tomorrow morning, > >please let us know. > > I did not receive email, could you please check? You are not listed as custodian for any of the trees in either the wiki or MAINTAINERS. Can you please verify to me which trees you should have access to? > Thanks, > Peng. -- Harald DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-62 Fax: +49-8142-66989-80 Email: h...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] MAINTAINERS: Update git repo links
On Wed, 2019-06-19 at 11:37 +0900, Masahiro Yamada wrote: > On Wed, Jun 19, 2019 at 1:49 AM Wolfgang Denk wrote: > > Dear Bin, > > > > In message <1560872253-6077-1-git-send-email-bmeng...@gmail.com> you wrote: > > > Update all git repo links with the new gitlab ones. > > > > > > Signed-off-by: Bin Meng > > > --- > > > > > > MAINTAINERS | 106 > > > ++-- > > > 1 file changed, 53 insertions(+), 53 deletions(-) > > > > Thanks a lot! > > > > Acked-by: Wolfgang Denk > > > > Best regards, > > > > Wolfgang Denk > > This involves two changes > [1] git.denx.de/* -> gitlab.denx.de/u-boot/* > [2] git:// -> https:// > > > Wolfgang said: > " c) git:// access is not available yet; this depends on the > URL rewrite rules and DNS changes and will be added later > - ASAP, see below. > " > > > Which is efficient and better to advertise, git:// or https:// ? > No difference in the clone speed? Nothing is better than taking actual measurements. This is what I observe: Benchmark #1: git clone git://gitlab.denx.de:60010/u-boot.git Time (mean ± σ): 35.970 s ± 3.251 s[User: 23.771 s, System: 3.932 s] Range (min … max): 32.600 s … 41.274 s10 runs Benchmark #2: git clone https://gitlab.denx.de/u-boot/u-boot.git Time (mean ± σ): 35.377 s ± 2.769 s[User: 24.838 s, System: 5.205 s] Range (min … max): 31.744 s … 39.598 s10 runs Summary 'git clone https://gitlab.denx.de/u-boot/u-boot.git' ran 1.02 ± 0.12 times faster than 'git clone git://gitlab.denx.de:60010/u-boot.git' (Ignore the git:// URL, it was just temporary for testing) As you can see, speed-wise there is next to no difference. BUT https is encrypted while git is not and https works better with proxy madness. So I would definitely choose https. -- Harald DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-62 Fax: +49-8142-66989-80 Email: h...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
