date:20210409

[PATCH v2] power: regulator: Add support for regulator-force-boot-off

2021-04-09 Thread Stefan Roese

From: Konstantin Porotchkin 

Add support for regulator-force-boot-off DT property.
This property can be used by the board/device drivers for
turning off regulators on early init stages as pre-requisite
for the other components initialization.

Signed-off-by: Konstantin Porotchkin 
Signed-off-by: Stefan Roese 
Cc: Jaehoon Chung 
Cc: Simon Glass 
---
v2:
- Add check for uc_pdata in regulator_unset()

 drivers/power/regulator/regulator-uclass.c | 38 ++
 include/power/regulator.h  | 23 +
 2 files changed, 61 insertions(+)

diff --git a/drivers/power/regulator/regulator-uclass.c 
b/drivers/power/regulator/regulator-uclass.c
index 4d2e730271f9..fac960682331 100644
--- a/drivers/power/regulator/regulator-uclass.c
+++ b/drivers/power/regulator/regulator-uclass.c
@@ -311,6 +311,17 @@ int regulator_autoset(struct udevice *dev)
return ret;
 }
 
+int regulator_unset(struct udevice *dev)
+{
+   struct dm_regulator_uclass_plat *uc_pdata;
+
+   uc_pdata = dev_get_uclass_plat(dev);
+   if (uc_pdata && uc_pdata->force_off)
+   return regulator_set_enable(dev, false);
+
+   return -EMEDIUMTYPE;
+}
+
 static void regulator_show(struct udevice *dev, int ret)
 {
struct dm_regulator_uclass_plat *uc_pdata;
@@ -443,6 +454,7 @@ static int regulator_pre_probe(struct udevice *dev)
uc_pdata->boot_on = dev_read_bool(dev, "regulator-boot-on");
uc_pdata->ramp_delay = dev_read_u32_default(dev, "regulator-ramp-delay",
0);
+   uc_pdata->force_off = dev_read_bool(dev, "regulator-force-boot-off");
 
node = dev_read_subnode(dev, "regulator-state-mem");
if (ofnode_valid(node)) {
@@ -495,6 +507,32 @@ int regulators_enable_boot_on(bool verbose)
return ret;
 }
 
+int regulators_enable_boot_off(bool verbose)
+{
+   struct udevice *dev;
+   struct uclass *uc;
+   int ret;
+
+   ret = uclass_get(UCLASS_REGULATOR, &uc);
+   if (ret)
+   return ret;
+   for (uclass_first_device(UCLASS_REGULATOR, &dev);
+dev;
+uclass_next_device(&dev)) {
+   ret = regulator_unset(dev);
+   if (ret == -EMEDIUMTYPE) {
+   ret = 0;
+   continue;
+   }
+   if (verbose)
+   regulator_show(dev, ret);
+   if (ret == -ENOSYS)
+   ret = 0;
+   }
+
+   return ret;
+}
+
 UCLASS_DRIVER(regulator) = {
.id = UCLASS_REGULATOR,
.name   = "regulator",
diff --git a/include/power/regulator.h b/include/power/regulator.h
index da9a065bdde0..fad87c99e5db 100644
--- a/include/power/regulator.h
+++ b/include/power/regulator.h
@@ -151,6 +151,7 @@ enum regulator_flag {
  * @max_uA*- maximum amperage (micro Amps)
  * @always_on* - bool type, true or false
  * @boot_on*   - bool type, true or false
+ * @force_off* - bool type, true or false
  * TODO(s...@chromium.org): Consider putting the above two into @flags
  * @ramp_delay - Time to settle down after voltage change (unit: uV/us)
  * @flags: - flags value (see REGULATOR_FLAG_...)
@@ -176,6 +177,7 @@ struct dm_regulator_uclass_plat {
unsigned int ramp_delay;
bool always_on;
bool boot_on;
+   bool force_off;
const char *name;
int flags;
u8 ctrl_reg;
@@ -420,6 +422,15 @@ int regulator_set_mode(struct udevice *dev, int mode_id);
  */
 int regulators_enable_boot_on(bool verbose);
 
+/**
+ * regulators_enable_boot_off() - disable regulators needed for boot
+ *
+ * This disables all regulators which are marked to be off at boot time.
+ *
+ * This effectively calls regulator_unset() for every regulator.
+ */
+int regulators_enable_boot_off(bool verbose);
+
 /**
  * regulator_autoset: setup the voltage/current on a regulator
  *
@@ -439,6 +450,18 @@ int regulators_enable_boot_on(bool verbose);
  */
 int regulator_autoset(struct udevice *dev);
 
+/**
+ * regulator_unset: turn off a regulator
+ *
+ * The setup depends on constraints found in device's uclass's platform data
+ * (struct dm_regulator_uclass_platdata):
+ *
+ * - Disable - will set - if  'force_off' is set to true,
+ *
+ * The function returns on the first-encountered error.
+ */
+int regulator_unset(struct udevice *dev);
+
 /**
  * regulator_autoset_by_name: setup the regulator given by its uclass's
  * platform data name field. The setup depends on constraints found in device's
-- 
2.31.1

Re: [PATCH] power: regulator: Add support for regulator-force-boot-off

2021-04-09 Thread Jaehoon Chung

Hi Stefan,

On 4/9/21 2:20 PM, Stefan Roese wrote:
> Hi Jaehoon,
> 
> On 09.04.21 02:37, Jaehoon Chung wrote:
>> On 4/9/21 7:52 AM, Jaehoon Chung wrote:
>>> Hi Stefan,
>>>
>>> On 4/8/21 6:20 PM, Stefan Roese wrote:
 From: Konstantin Porotchkin 

 Add support for regulator-force-boot-off DT property.
 This property can be used by the board/device drivers for
 turning off regulators on early init stages as pre-requisite
 for the other components initialization.

 Signed-off-by: Konstantin Porotchkin 
 Signed-off-by: Stefan Roese 
 Cc: Jaehoon Chung 
 Cc: Simon Glass 
 ---
   drivers/power/regulator/regulator-uclass.c | 38 ++
   include/power/regulator.h  | 23 +
   2 files changed, 61 insertions(+)

 diff --git a/drivers/power/regulator/regulator-uclass.c 
 b/drivers/power/regulator/regulator-uclass.c
 index 4d2e730271f9..423867edced8 100644
 --- a/drivers/power/regulator/regulator-uclass.c
 +++ b/drivers/power/regulator/regulator-uclass.c
 @@ -311,6 +311,17 @@ int regulator_autoset(struct udevice *dev)
   return ret;
   }
   +int regulator_unset(struct udevice *dev)
 +{
 +    struct dm_regulator_uclass_plat *uc_pdata;
 +
 +    uc_pdata = dev_get_uclass_plat(dev);
 +    if (uc_pdata->force_off)
>>>
>>> Could you check whether uc_pdata is NULL or not?
>>> It can be returned to NULL.
>>>
 +    return regulator_set_enable(dev, false);
 +
 +    return -EMEDIUMTYPE;
 +}
 +
   static void regulator_show(struct udevice *dev, int ret)
   {
   struct dm_regulator_uclass_plat *uc_pdata;
 @@ -443,6 +454,7 @@ static int regulator_pre_probe(struct udevice *dev)
   uc_pdata->boot_on = dev_read_bool(dev, "regulator-boot-on");
   uc_pdata->ramp_delay = dev_read_u32_default(dev, 
 "regulator-ramp-delay",
   0);
 +    uc_pdata->force_off = dev_read_bool(dev, "regulator-force-boot-off");
     node = dev_read_subnode(dev, "regulator-state-mem");
   if (ofnode_valid(node)) {
 @@ -495,6 +507,32 @@ int regulators_enable_boot_on(bool verbose)
   return ret;
   }
   +int regulators_enable_boot_off(bool verbose)
 +{
 +    struct udevice *dev;
 +    struct uclass *uc;
 +    int ret;
 +
 +    ret = uclass_get(UCLASS_REGULATOR, &uc);
 +    if (ret)
 +    return ret;
 +    for (uclass_first_device(UCLASS_REGULATOR, &dev);
 + dev;
>>>
>>> typo?
>>
>> I have checked that it was not typo. :)
>> how about making one line?
> 
> I could do this. But this would lead to a quite long line:
> 
> for (uclass_first_device(UCLASS_REGULATOR, &dev); dev; 
> uclass_next_device(&dev)) {
> 
> Only putting "dev;" into the same line is not appealing, at least not
> to me. Please note that this style of uclass looping is pretty
> common. It's also used a few lines above in regulators_enable_boot_on():
> 
> int regulators_enable_boot_on(bool verbose)
> {
> struct udevice *dev;
> struct uclass *uc;
> int ret;
> 
> ret = uclass_get(UCLASS_REGULATOR, &uc);
> if (ret)
>     return ret;
> for (uclass_first_device(UCLASS_REGULATOR, &dev);
>  dev;
>  uclass_next_device(&dev)) {
>     ret = regulator_autoset(dev);
> ...
> 
> So I would like to keep it as in v1.

Yes, If you prefer to it, keep it as in v1. 
I think that can be changed to below.

for (uclass_first_device(UCLASS_REGULATOR, &dev);
dev; uclass_next_device(&dev)) {
ret = reuglator_autoset(dev);
...
}

But it's not critical thing. Thanks!

Best Regards,
Jaehoon Chung

> 
> Thanks,
> Stefan
> 
>>
>> Best Regards,
>> Jaehoo Chung
>>
>>>
>>> Best Regards,
>>> Jaehoon Chung
>>>
 + uclass_next_device(&dev)) {
 +    ret = regulator_unset(dev);
 +    if (ret == -EMEDIUMTYPE) {
 +    ret = 0;
 +    continue;
 +    }
 +    if (verbose)
 +    regulator_show(dev, ret);
 +    if (ret == -ENOSYS)
 +    ret = 0;
 +    }
 +
 +    return ret;
 +}
 +
   UCLASS_DRIVER(regulator) = {
   .id    = UCLASS_REGULATOR,
   .name    = "regulator",
 diff --git a/include/power/regulator.h b/include/power/regulator.h
 index da9a065bdde0..fad87c99e5db 100644
 --- a/include/power/regulator.h
 +++ b/include/power/regulator.h
 @@ -151,6 +151,7 @@ enum regulator_flag {
    * @max_uA*    - maximum amperage (micro Amps)
    * @always_on* - bool type, true or false
    * @boot_on*   - bool type, true or false
 + * @force_off* - bool type, true or false
    * TODO(s...@chromium.org): Consider putting the above two into @flags
    * @ramp_delay - Time to settle down after voltage change (unit: uV/us)
    * @flags: - flags v

Re: [PATCH v9] Add support for stack-protector

2021-04-09 Thread Joel Peshkin

Hi Heinrich,

Has there been any progress in getting the EFI erors fixed so that this can
be committed?  There seems to be little point in my refreshing this patch
until that is done.

Thanks,

-Joel


On Mon, Mar 22, 2021 at 10:37 AM Heinrich Schuchardt 
wrote:

> On 09.02.21 04:36, Joel Peshkin wrote:
> > Add support for stack protector for UBOOT, SPL, and TPL
> > as well as new pytest for stackprotector
> >
> > Signed-off-by: Joel Peshkin 
> > ---
> > Cc: Simon Glass 
> > Cc: Heinrich Schuchardt 
> >
> > Changes for v9:
> >- Fix pytest script post-test reboot
> > Changes for v8:
> >- Fix commit message
> >- Force canary to UL type
> > Changes for v7:
> >- Fix commit message
> >- add __builtin_extract_return_addr() calls
> > Changes for v6:
> >- Fix commit message
> > Changes for v5:
> >- Rebase
> > Changes for v4:
> >- Exclude EFI from stackprotector
> >- Cleanups of extra includes and declaration
> > Changes for v3:
> >- Move test command to cmd/
> >- Update Kconfig names and depends
> >- clean up default canary initialization
> > Changes for v2:
> >- Add test command and corresponding config
> >- Fixed incorrect description in Kconfig
> >- Add unit test
> > ---
> >  MAINTAINERS  |  7 +++
> >  Makefile |  5 +
> >  cmd/Kconfig  | 10 ++
> >  cmd/Makefile |  1 +
> >  cmd/stackprot_test.c | 21 +
> >  common/Kconfig   | 17 +
> >  common/Makefile  |  2 ++
> >  common/stackprot.c   | 19 +++
> >  configs/sandbox_defconfig| 14 +++---
> >  scripts/Makefile.spl |  6 ++
> >  test/py/tests/test_stackprotector.py | 15 +++
> >  11 files changed, 110 insertions(+), 7 deletions(-)
> >  create mode 100644 cmd/stackprot_test.c
> >  create mode 100644 common/stackprot.c
> >  create mode 100644 test/py/tests/test_stackprotector.py
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 26dd254..d3971e8 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -1024,6 +1024,13 @@ F: include/sqfs.h
> >  F:   cmd/sqfs.c
> >  F:   test/py/tests/test_fs/test_squashfs/
> >
> > +STACKPROTECTOR
> > +M:   Joel Peshkin 
> > +S:   Maintained
> > +F:   common/stackprot.c
> > +F:   cmd/stackprot_test.c
> > +F:   test/py/tests/test_stackprotector.py
> > +
> >  TARGET_BCMNS3
> >  M:   Bharat Gooty 
> >  M:   Rayagonda Kokatanur 
> > diff --git a/Makefile b/Makefile
> > index 902a976..65c5cb8 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -677,7 +677,12 @@ else
> >  KBUILD_CFLAGS+= -O2
> >  endif
> >
> > +ifeq ($(CONFIG_STACKPROTECTOR),y)
> > +KBUILD_CFLAGS += $(call cc-option,-fstack-protector-strong)
> > +CFLAGS_EFI += $(call cc-option,-fno-stack-protector)
> > +else
> >  KBUILD_CFLAGS += $(call cc-option,-fno-stack-protector)
> > +endif
> >  KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks)
> >
> >  # disable stringop warnings in gcc 8+
> > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > index da86a94..054b2f3 100644
> > --- a/cmd/Kconfig
> > +++ b/cmd/Kconfig
> > @@ -2280,6 +2280,16 @@ config CMD_AVB
> >   avb read_part_hex - read data from partition and output to
> stdout
> >   avb write_part - write data to partition
> >   avb verify - run full verification chain
> > +
> > +config CMD_STACKPROTECTOR_TEST
> > + bool "Test command for stack protector"
> > + depends on STACKPROTECTOR
> > + default n
> > + help
> > +   Enable stackprot_test command
> > +   The stackprot_test command will force a stack overrun to test
> > +   the stack smashing detection mechanisms.
> > +
> >  endmenu
> >
> >  config CMD_UBI
> > diff --git a/cmd/Makefile b/cmd/Makefile
> > index 5b3400a..1d7afea 100644
> > --- a/cmd/Makefile
> > +++ b/cmd/Makefile
> > @@ -142,6 +142,7 @@ obj-$(CONFIG_CMD_SPI) += spi.o
> >  obj-$(CONFIG_CMD_STRINGS) += strings.o
> >  obj-$(CONFIG_CMD_SMC) += smccc.o
> >  obj-$(CONFIG_CMD_SYSBOOT) += sysboot.o pxe_utils.o
> > +obj-$(CONFIG_CMD_STACKPROTECTOR_TEST) += stackprot_test.o
> >  obj-$(CONFIG_CMD_TERMINAL) += terminal.o
> >  obj-$(CONFIG_CMD_TIME) += time.o
> >  obj-$(CONFIG_CMD_TIMER) += timer.o
> > diff --git a/cmd/stackprot_test.c b/cmd/stackprot_test.c
> > new file mode 100644
> > index 000..6ad287e
> > --- /dev/null
> > +++ b/cmd/stackprot_test.c
> > @@ -0,0 +1,21 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + *  Copyright 2021 Broadcom
> > + */
> > +
> > +#include 
> > +#include 
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
>
> Hello Joël,
>
> This line is not needed.
>
> > +
> > +static int do_test_stackprot_fail(struct cmd_tbl *cmdtp, int flag, int
> argc,
> > +   char *const argv[])
> > +{
> > + char a[128];
> > +
> > + memset(a, 0xa5, 512);
> > + re

Re: [PATCH 19/19] ARM: imx8m: verdin-imx8mm: Enable USB Host support

2021-04-09 Thread Marek Vasut


On 4/8/21 9:08 AM, Marcel Ziswiler wrote:

Hi Marek


Hi,


Upon compiling I noticed the following but I do realize that it is not this 
patch set which introduced this
issue.

drivers/usb/host/ehci-mx6.c: In function ‘ehci_usb_probe’:
drivers/usb/host/ehci-mx6.c:686:30: warning: cast from pointer to integer of 
different size [-Wpointer-to-int-
cast]
   686 |  hccr = (struct ehci_hccr *)((uint32_t)&ehci->caplength);


That's easy enough to fix.

[...]


Then upon booting I noticed the following two issues. Did you also see that?

write error to device: 930f90 register: x!write error to device: 930f90 
register: x!write error to device:
930f90 register: x!write error to device: 930f90 register: x!Normal Boot

imx_wdt watchdog@3028: pinctrl_select_state_full: 
uclass_get_device_by_phandle_id: err=-19


I suspect this has nothing to do with USB, but rather PMIC and then WDT?


Anyway, the USB host stuff this patch is about works perfectly. Thanks!


Great. Except V2 as soon as I sort out a few remaining CI issues.

Re: [PATCH 06/11] imx: ventana: convert U-Boot to OF_CONTROL using FIT image

2021-04-09 Thread Tim Harvey

On Fri, Apr 9, 2021 at 1:14 PM Tim Harvey  wrote:
>
> On Fri, Apr 9, 2021 at 1:08 PM Tim Harvey  wrote:
> >
> > On Fri, Apr 9, 2021 at 1:00 PM Stefano Babic  wrote:
> > >
> > > Hi Tim,
> > >
> > > On 09.04.21 21:52, Tim Harvey wrote:
> > > > On Thu, Apr 8, 2021 at 1:57 PM  wrote:
> > > >>
> > > >>> In preparation for dm conversion convert to OF_CONTROL by adding FIT 
> > > >>> image
> > > >>> support and multi dtb.
> > > >>> Add a board_fit_config_name_match to match the dtb based off of EEPROM
> > > >>> model.
> > > >>> Signed-off-by: Tim Harvey 
> > > >> Applied to u-boot-imx, master, thanks !
> > > >>
> > > >> Best regards,
> > > >> Stefano Babic
> > > >>
> > > >
> > > > Stefano,
> > > >
> > > > Something broke apparently between the branch I had my patches on top
> > > > of and u-boot-imx/master that these got merged in. My board is hanging
> > > > at dram_init_banksize - did something change recently that you are
> > > > aware of that may require some changes on my end?
> > > >
> > >
> > > There are a lot of changes, but they are related to i.MX8. I have not
> > > merged something for i.MX6 that can justify this behavior. So I do not
> > > know - have you compared the two branches, at least for Ventana boards ?
> > >
> >
> > It's hanging at arch/arm/mach-imx/spl.c:dram_init_banksize on
> > 'gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE' so I suppose
> > something is wrong with gd.
>
> specifically gd->bd is NULL. I'll have to track down how that gets setup.
>

Stefano,

commit 38d6b7ebdaee ("spl: Drop bd_info in the data section") is the
culprit which breaks IMX boards using SPL that don't define
SPL_ALLOC_BD. There is another thread regarding if that commit should
be reverted or if instead it can be fixed (ie by defining
SPL_ALLOC_BD) so probably not worth discussing in this thread anymore.

Best regards,

Tim

Re: [PATCH] Revert "spl: Drop bd_info in the data section"

2021-04-09 Thread Tim Harvey

On Fri, Apr 9, 2021 at 1:53 PM Tom Rini  wrote:
>
> On Fri, Apr 09, 2021 at 03:24:41PM -0500, Adam Ford wrote:
> > On Fri, Apr 9, 2021 at 2:20 PM Alex G.  wrote:
> > >
> > > Hi Simon
> > >
> > > On 4/8/21 6:55 PM, Simon Glass wrote:
> > > > Hi Alexandru,
> > > >
> > > > On Fri, 9 Apr 2021 at 04:56, Alexandru Gagniuc  
> > > > wrote:
> > > >>
> > > >> This reverts commit 38d6b7ebdaee3e0e8426ef1b9df88bdce8ae2e75.
> > > >>
> > > >> struct global_data contains a pointer to the bd_info structure. This
> > > >> pointer was populated spl_set_bd() to a pre-allocated bd_info in the
> > > >> ".data" section. The referenced commit replaced this mechanism to one
> > > >> that uses malloc(). That new mechanism is only used if SPL_ALLOC_BD=y.
> > > >> which very few boards do.
> > > >>
> > > >> The result is that (struct global_data)->bd is NULL in SPL on most
> > > >> platforms. This breaks falcon mode, since arch_fixup_fdt() tries to
> > > >> access (struct global_data)->bd and set the "/memory" node in the
> > > >> devicetree. The result is that the "/memory" node contains garbage
> > > >> values, causing linux to panic() as it sets up the page table.
> > > >>
> > > >> Instead of trying to fix the mess, potentially causing other issues,
> > > >> revert to the code that worked, while this change is reworked.
> > > >
> > > > The goal here is to drop a feature that few boards use and reduce
> > > > memory usage in SPL. It has been in place for two releases now.
> > > >
> > > > If Falcon mode needs it, perhaps you could add an 'imply' in the
> > > > Kconfig for that feature? Is there one? Or perhaps
> > > > CONFIG_ARCH_FIXUP_FDT_MEMORY ?
> > > >
> > > > One option would be to return an error in arch_fixup_fdt(). In
> > > > general, fixups make things tricky because there is no way to
> > > > determine when they are used but at least this one has a CONFIG.
> > > >
> > >
> > > The argument that this has been in place for two releases is incorrect.
> > > Commit 38d6b7ebdaee3e0e8426ef1b9df88bdce8ae2e75 was only introduced with
> > > the v2021.04 release. It definitely was not in 2021.01. It's only in the
> > > last release, which is four days old t the time of this writing.
>
> Yes.  But another way of saying that is that we're 4 days in to the
> merge window.  That's a bit early to say we must revert the change.  If
> this was just before the release, yes, revert would be the right answer.
> It's also the case the original commit fixes some cases while also
> saving size, if I read it right.  So a strict revert isn't right, we'd
> need to also probably also default y SPL_ALLOC_BD in some cases.
>
> > > Although I was able to find one example, the reality is that we don't
> > > know the full extent of the breakage. The prudent thing at this point is
> > > to revert.
> > >
> > > The knowledge of how to init the platform is in the devicetree and code.
> > > Why should kconfig also be involved in storing this knowledge? By this
> > > model, as the number of boards increases without bounds, every "if"
> > > predicate tends to be Kconfig driven. That is not maintainable, and why
> > > I think the original change --and the proposed fixes-- are broken by 
> > > design.
> > >
> > > Furthermore, I'm happy to discuss what to do about Falcon mode, and if
> > > we should kill it entirely (I have an alternative proposal).  But we
> > > shouldn't have that discussion in a manner holding my platform hostage.
> > > To be fair to you, I don't think reverting a 64-byte savings in .data
> > > will hold your platform hostage either.
> >
> > That original patch broke a bunch of OMAP boards, but enabling
> > SPL_ALLOC_BD was the solution to fix the issue.
> > Can you try enabling SPL_ALLOC_BD and see if that fixes it?  Maybe we
> > can make falcon mode imply it.
>
> It would be "select" since it needs it rather than imply.
>

I just ran into this as well finding that commit 38d6b7ebdaee ("spl:
Drop bd_info in the data section") breaks Gateworks Ventana and
defining SPL_ALLOC_BD does resolve it. In this case, Falcon mode is
not being used and the breakage is because arch/arm/mach-imx/spl.c
dram_init_banksize() accesses gd->bd which is NULL.

So I would guess that this probably broke a whole lot of IMX based
boards that use SPL.

I don't quite know what the best solution is... we now have a v2021.04
that is broken for several or many boards and I dont' know if its
clear what cases break.

Best regards,

Tim

Re: [PATCH v2] cmd: net: Add the "arp" command

2021-04-09 Thread Heinrich Schuchardt


On 4/9/21 11:46 PM, lg...@hotmail.com wrote:

From: Joe Xue 

The command is to query and show mac address of a specific ipAddress.

Signed-off-by: Joe Xue 
---

  cmd/Kconfig   |  6 ++
  cmd/net.c | 36 
  doc/usage/arp.rst | 31 +++
  include/net.h |  5 +
  net/arp.c | 24 
  net/arp.h |  4 
  net/net.c |  8 
  7 files changed, 114 insertions(+)
  create mode 100644 doc/usage/arp.rst

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 9bf5e863e4..1da4cb67f6 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1587,6 +1587,12 @@ config CMD_PING
help
  Send ICMP ECHO_REQUEST to network host

+config CMD_ARP
+   bool "arp"
+   help
+ Sends ARP_REQUEST to network host and shows the result if there is arp
+ response.
+
  config CMD_CDP
bool "cdp"
help
diff --git a/cmd/net.c b/cmd/net.c
index beb2877dfd..369d15474c 100644
--- a/cmd/net.c
+++ b/cmd/net.c
@@ -479,4 +479,40 @@ U_BOOT_CMD(
""
  );



Have a look at the #ifdef above in the code. I assume that the
dependency on CONFIG_CMD_LINK_LOCAL was not your intention.


+#ifdef CONFIG_CMD_ARP
+static int do_arp(struct cmd_tbl *cmdtp, int flag, int argc, char *const 
argv[])
+{
+   u8 *ethaddr = arp_query_ethaddr;
+
+   if (argc < 2)
+   return CMD_RET_USAGE;
+
+   arp_query_ip = string_to_ip(argv[1]);
+   if (arp_query_ip.s_addr == 0)
+   return CMD_RET_USAGE;
+
+   if ((arp_query_ip.s_addr & net_netmask.s_addr) !=
+   (net_ip.s_addr & net_netmask.s_addr)) {
+   printf("The host %s is not in the same network\n", argv[1]);
+   return CMD_RET_SUCCESS;
+   }
+
+   if (net_loop(ARP) < 0) {
+   printf("arp failed; host %s is not alive\n", argv[1]);
+   return CMD_RET_FAILURE;
+   }
+
+   printf("%s\t%02x:%02x:%02x:%02x:%02x:%02x\n", argv[1], ethaddr[0],
+  ethaddr[1], ethaddr[2], ethaddr[3], ethaddr[4], ethaddr[5]);
+
+   return CMD_RET_SUCCESS;
+}
+
+U_BOOT_CMD(
+   arp,2,  1,  do_arp,
+   "send ARP ARP_REQUEST to network host",
+   "ipAddress"
+);
+#endif
+
  #endif  /* CONFIG_CMD_LINK_LOCAL */
diff --git a/doc/usage/arp.rst b/doc/usage/arp.rst
new file mode 100644
index 00..b1f08a2ae9
--- /dev/null
+++ b/doc/usage/arp.rst
@@ -0,0 +1,31 @@
+arp command
+===
+
+Synopis
+---
+
+::
+
+arp ipAddress
+
+Description
+---
+
+The arp command is used to send ARP_REQUEST to network host and show the 
result.
+
+ipAddress
+the host ip address


%s/ip/IP/

Why should the IP address be a required parameter? The most interesting
use case for the arp command is discovering neighbors.

On Linux 'arp -n' gives me the list of *all* devices in the same network
segment with both MAC address and IP address.

Can we do the same in U-Boot?

Best regards

Heinrich


+
+Example
+---
+
+::
+
+=> arp 192.168.0.1
+Using host_ens33 device
+192.168.0.1 84:94:8c:5f:e1:62
+
+Return value
+
+
+The return value $? is 0 if the comand running was successful else 1
diff --git a/include/net.h b/include/net.h
index b95d6a6f60..60b4bf610e 100644
--- a/include/net.h
+++ b/include/net.h
@@ -580,6 +580,11 @@ extern char *net_dns_env_var;  /* the env var 
to put the ip into */
  extern struct in_addr net_ping_ip;/* the ip address to ping */
  #endif

+#if defined(CONFIG_CMD_ARP)
+extern u8 arp_query_ethaddr[6];/* the arp query result */
+extern struct in_addr arp_query_ip; /* the ip address to arp query */
+#endif
+
  #if defined(CONFIG_CMD_CDP)
  /* when CDP completes these hold the return values */
  extern ushort cdp_native_vlan;/* CDP returned native VLAN */
diff --git a/net/arp.c b/net/arp.c
index 1d06ed2572..83c24072d7 100644
--- a/net/arp.c
+++ b/net/arp.c
@@ -220,11 +220,20 @@ void arp_receive(struct ethernet_hdr *et, struct 
ip_udp_hdr *ip, int len)
net_get_arp_handler()((uchar *)arp, 0, reply_ip_addr,
  0, len);

+#ifdef CONFIG_CMD_ARP
+   if (arp_query_ip.s_addr != 0) {
+   arp_query_ip.s_addr = 0;
+   net_set_state(NETLOOP_SUCCESS);
+   } else {
+#endif
/* set the mac address in the waiting packet's header
   and transmit it */
memcpy(((struct ethernet_hdr *)net_tx_packet)->et_dest,
   &arp->ar_sha, ARP_HLEN);
net_send_packet(net_tx_packet, arp_wait_tx_packet_size);
+#ifdef CONFIG_CMD_ARP
+   }
+#endif

/* no arp request pending now */
net_arp_wait_packet_ip.s_addr = 0;
@@ -243,3 +252,18 @

Re: [PATCH 0/5] Add support for embedding public key in platform's dtb

2021-04-09 Thread Simon Glass

Hi Sughosh,

On Fri, 9 Apr 2021 at 23:27, Sughosh Ganu  wrote:
>
> hi Simon,
>
> On Fri, 9 Apr 2021 at 05:26, Simon Glass  wrote:
>>
>> Hi Sughosh,
>>
>> On Thu, 8 Apr 2021 at 18:53, Sughosh Ganu  wrote:
>> >
>> > hi Simon,
>> >
>> > On Wed, 7 Apr 2021 at 21:44, Simon Glass  wrote:
>> >>
>> >> Hi,
>> >>
>> >> On Wed, 7 Apr 2021 at 23:54, Sughosh Ganu  wrote:
>> >> >
>> >> > Patch 1 fixes an issue of selection of IMAGE_SIGN_INFO config option
>> >> > when capsule authentication is enabled.
>> >> >
>> >> > Patch 2 add two config symbols, EFI_PKEY_DTB_EMBED and EFI_PKEY_FILE
>> >> > which are used for enabling embedding of the public key in the dtb,
>> >> > and specifying the esl file name.
>> >> >
>> >> > Patch 3 moves efi_capsule_auth_enabled as a weak function, which can
>> >> > be used as a default mechanism for checking if capsule authentication
>> >> > has been enabled.
>> >> >
>> >> > Patch 4 adds a default weak function for retrieving the public key
>> >> > from the platform's dtb.
>> >> >
>> >> > Patch 5 adds the functionality to embed the esl file into the
>> >> > platform's dtb during the platform build.
>> >> >
>> >> > I have tested this functionality on the STM32MP157C DK2 board.
>> >> >
>> >> > [1] - https://lists.denx.de/pipermail/u-boot/2021-March/442867.html
>> >> >
>> >> > Sughosh Ganu (5):
>> >> >   efi_loader: Kconfig: Select IMAGE_SIGN_INFO when capsule
>> >> > authentication is enabled
>> >> >   efi_loader: Kconfig: Add symbols for embedding the public key into the
>> >> > platform's dtb
>> >> >   efi_capsule: Add a weak function to check whether capsule
>> >> > authentication is enabled
>> >> >   efi_capsule: Add a weak function to get the public key needed for
>> >> > capsule authentication
>> >> >   Makefile: Add provision for embedding public key in platform's dtb
>> >> >
>> >> >  Makefile  | 10 ++
>> >> >  board/emulation/common/qemu_capsule.c |  6 
>> >> >  lib/efi_loader/Kconfig| 16 ++
>> >> >  lib/efi_loader/efi_capsule.c  | 44 ---
>> >> >  4 files changed, 66 insertions(+), 10 deletions(-)
>> >> >
>> >> > --
>> >> > 2.17.1
>> >> >
>> >>
>> >> We need to rethink the use of weak functions for this sort of thing,
>> >> or we will end up with an unnavigable mess at some point. If we need
>> >> to adjust the flow of boot, let's adjust the flow rather than adding
>> >> hooks everywhere.
>> >
>> >
>> > There are two weak functions being added. One is for retrieving the public 
>> > key to be used for the capsule authentication, and the other is for 
>> > checking for whether capsule authentication has been enabled. The reason 
>> > why a weak function is needed is because platforms can have other 
>> > mechanisms for retrieval of the public key or for testing if capsule 
>> > authentication has been enabled.
>> >
>> > If we consider the case of public key retrieval, the majority of platforms 
>> > would be built with the device tree concatenated with the u-boot binary. 
>> > The weak function would cater to all of those platforms -- having a weak 
>> > function would mean that we are not required to repeat the same 
>> > functionality for every platform that uses the same mechanism for 
>> > extracting the public key. However, there would be platforms where the 
>> > public key is obtained through a different mechanism which is platform 
>> > specific. Those platforms would have to define their own function to get 
>> > the public key. Same for checking whether capsule authentication feature 
>> > has been enabled or not.
>> >
>> > -sughosh
>>
>> Yes, I get it, but if this is to be a critical feature and part of the
>> grand new design for verified boot using UEFI, surely we should be
>> building a new front door, not digging a tunnel in the bathroom :-)
>>
>> We can either use drivers with driver model, or perhaps have a Kconfig
>> that enables calling the function (so we get a link error if not
>> provided). Or if there will be more than one handler, a linker_list.
>
>
> I will use the Kconfig symbol for selecting the function to use for 
> retrieving the public key. So, in the current scenario, the function would be 
> under the CONFIG_EFI_PKEY_DTB_EMBED symbol. This should cater to the majority 
> of the cases. If some platform wants to use a different method to get the 
> public key, it would need to define it's own function.

I wonder why another method would be needed, but if it, then someone
can turn your thing into a choice, I suppose.

Regards,
Simon

Re: [PATCH] board_f: cosmetic: change the debug trace to KB in reserve_video

2021-04-09 Thread Simon Glass

On Sat, 10 Apr 2021 at 04:02, Patrick Delaunay
 wrote:
>
> Update the debug trace for the reserved video memory to KB as indicated
> in the message with "%luk"; before the patch the computed size
> gd->relocaddr - addr is in bytes.
>
> This patch aligns the debug trace in reserve_video() with others
> functions, for example on stm32mp157c-dk2:
>
>   - Reserving 3080192k for video at: dfd0
>   + Reserving 3008k for video at: dfd0
> Reserving 873k for U-Boot at: dfc25000
> Reserving 32776k for malloc() at: ddc23000
> Reserving 72 Bytes for Board Info at: ddc22fb0
> Reserving 280 Bytes for Global Data at: ddc22e90
> Reserving 119072 Bytes for FDT at: ddc05d70
> Reserving 0x278 Bytes for bootstage at: ddc05af0
>
> Fixes: 5630d2fbc50f3035 ("board: Show memory for frame buffers")
> Signed-off-by: Patrick Delaunay 
> ---
>
>  common/board_f.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Simon Glass

Re: [PATCH] Revert "spl: Drop bd_info in the data section"

2021-04-09 Thread Tom Rini

On Fri, Apr 09, 2021 at 03:24:41PM -0500, Adam Ford wrote:
> On Fri, Apr 9, 2021 at 2:20 PM Alex G.  wrote:
> >
> > Hi Simon
> >
> > On 4/8/21 6:55 PM, Simon Glass wrote:
> > > Hi Alexandru,
> > >
> > > On Fri, 9 Apr 2021 at 04:56, Alexandru Gagniuc  
> > > wrote:
> > >>
> > >> This reverts commit 38d6b7ebdaee3e0e8426ef1b9df88bdce8ae2e75.
> > >>
> > >> struct global_data contains a pointer to the bd_info structure. This
> > >> pointer was populated spl_set_bd() to a pre-allocated bd_info in the
> > >> ".data" section. The referenced commit replaced this mechanism to one
> > >> that uses malloc(). That new mechanism is only used if SPL_ALLOC_BD=y.
> > >> which very few boards do.
> > >>
> > >> The result is that (struct global_data)->bd is NULL in SPL on most
> > >> platforms. This breaks falcon mode, since arch_fixup_fdt() tries to
> > >> access (struct global_data)->bd and set the "/memory" node in the
> > >> devicetree. The result is that the "/memory" node contains garbage
> > >> values, causing linux to panic() as it sets up the page table.
> > >>
> > >> Instead of trying to fix the mess, potentially causing other issues,
> > >> revert to the code that worked, while this change is reworked.
> > >
> > > The goal here is to drop a feature that few boards use and reduce
> > > memory usage in SPL. It has been in place for two releases now.
> > >
> > > If Falcon mode needs it, perhaps you could add an 'imply' in the
> > > Kconfig for that feature? Is there one? Or perhaps
> > > CONFIG_ARCH_FIXUP_FDT_MEMORY ?
> > >
> > > One option would be to return an error in arch_fixup_fdt(). In
> > > general, fixups make things tricky because there is no way to
> > > determine when they are used but at least this one has a CONFIG.
> > >
> >
> > The argument that this has been in place for two releases is incorrect.
> > Commit 38d6b7ebdaee3e0e8426ef1b9df88bdce8ae2e75 was only introduced with
> > the v2021.04 release. It definitely was not in 2021.01. It's only in the
> > last release, which is four days old t the time of this writing.

Yes.  But another way of saying that is that we're 4 days in to the
merge window.  That's a bit early to say we must revert the change.  If
this was just before the release, yes, revert would be the right answer.
It's also the case the original commit fixes some cases while also
saving size, if I read it right.  So a strict revert isn't right, we'd
need to also probably also default y SPL_ALLOC_BD in some cases.

> > Although I was able to find one example, the reality is that we don't
> > know the full extent of the breakage. The prudent thing at this point is
> > to revert.
> >
> > The knowledge of how to init the platform is in the devicetree and code.
> > Why should kconfig also be involved in storing this knowledge? By this
> > model, as the number of boards increases without bounds, every "if"
> > predicate tends to be Kconfig driven. That is not maintainable, and why
> > I think the original change --and the proposed fixes-- are broken by design.
> >
> > Furthermore, I'm happy to discuss what to do about Falcon mode, and if
> > we should kill it entirely (I have an alternative proposal).  But we
> > shouldn't have that discussion in a manner holding my platform hostage.
> > To be fair to you, I don't think reverting a 64-byte savings in .data
> > will hold your platform hostage either.
> 
> That original patch broke a bunch of OMAP boards, but enabling
> SPL_ALLOC_BD was the solution to fix the issue.
> Can you try enabling SPL_ALLOC_BD and see if that fixes it?  Maybe we
> can make falcon mode imply it.

It would be "select" since it needs it rather than imply.

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH] cmd: net: Add the "arp" command

2021-04-09 Thread Heinrich Schuchardt

Am 9. April 2021 20:45:36 MESZ schrieb lg...@hotmail.com:
>From: Joe Xue 
>
>The command is to query and show mac address of a specific ipAddress.

Dear Joe,

please, add a man-page for the command in /doc/usage/.

>
>Signed-off-by: Joe Xue 
>---
>
> cmd/Kconfig   |  6 ++
> cmd/net.c | 36 
> include/net.h |  5 +
> net/arp.c | 24 
> net/arp.h |  4 
> net/net.c |  8 
> 6 files changed, 83 insertions(+)
>
>diff --git a/cmd/Kconfig b/cmd/Kconfig
>index 9bf5e863e4..1da4cb67f6 100644
>--- a/cmd/Kconfig
>+++ b/cmd/Kconfig
>@@ -1587,6 +1587,12 @@ config CMD_PING
>   help
> Send ICMP ECHO_REQUEST to network host
> 
>+config CMD_ARP
>+  bool "arp"
>+  help
>+Sends ARP_REQUEST to network host and shows the result if there is
>arp

an arp response

Best regards

Heinrich

>+response.
>+
> config CMD_CDP
>   bool "cdp"
>   help
>diff --git a/cmd/net.c b/cmd/net.c
>index beb2877dfd..369d15474c 100644
>--- a/cmd/net.c
>+++ b/cmd/net.c
>@@ -479,4 +479,40 @@ U_BOOT_CMD(
>   ""
> );
> 
>+#ifdef CONFIG_CMD_ARP
>+static int do_arp(struct cmd_tbl *cmdtp, int flag, int argc, char
>*const argv[])
>+{
>+  u8 *ethaddr = arp_query_ethaddr;
>+
>+  if (argc < 2)
>+  return CMD_RET_USAGE;
>+
>+  arp_query_ip = string_to_ip(argv[1]);
>+  if (arp_query_ip.s_addr == 0)
>+  return CMD_RET_USAGE;
>+
>+  if ((arp_query_ip.s_addr & net_netmask.s_addr) !=
>+  (net_ip.s_addr & net_netmask.s_addr)) {
>+  printf("The host %s is not in the same network\n", argv[1]);
>+  return CMD_RET_SUCCESS;
>+  }
>+
>+  if (net_loop(ARP) < 0) {
>+  printf("arp failed; host %s is not alive\n", argv[1]);
>+  return CMD_RET_FAILURE;
>+  }
>+
>+  printf("%s\t%02x:%02x:%02x:%02x:%02x:%02x\n", argv[1], ethaddr[0],
>+ ethaddr[1], ethaddr[2], ethaddr[3], ethaddr[4], ethaddr[5]);
>+
>+  return CMD_RET_SUCCESS;
>+}
>+
>+U_BOOT_CMD(
>+  arp,2,  1,  do_arp,
>+  "send ARP ARP_REQUEST to network host",
>+  "ipAddress"
>+);
>+#endif
>+
> #endif  /* CONFIG_CMD_LINK_LOCAL */
>diff --git a/include/net.h b/include/net.h
>index b95d6a6f60..60b4bf610e 100644
>--- a/include/net.h
>+++ b/include/net.h
>@@ -580,6 +580,11 @@ extern char *net_dns_env_var; /* the env var 
>to
>put the ip into */
> extern struct in_addr net_ping_ip;/* the ip address to ping */
> #endif
> 
>+#if defined(CONFIG_CMD_ARP)
>+extern u8 arp_query_ethaddr[6];   /* the arp query result */
>+extern struct in_addr arp_query_ip; /* the ip address to arp query */
>+#endif
>+
> #if defined(CONFIG_CMD_CDP)
> /* when CDP completes these hold the return values */
> extern ushort cdp_native_vlan;/* CDP returned native VLAN */
>diff --git a/net/arp.c b/net/arp.c
>index 1d06ed2572..83c24072d7 100644
>--- a/net/arp.c
>+++ b/net/arp.c
>@@ -220,11 +220,20 @@ void arp_receive(struct ethernet_hdr *et, struct
>ip_udp_hdr *ip, int len)
>   net_get_arp_handler()((uchar *)arp, 0, reply_ip_addr,
> 0, len);
> 
>+#ifdef CONFIG_CMD_ARP
>+  if (arp_query_ip.s_addr != 0) {
>+  arp_query_ip.s_addr = 0;
>+  net_set_state(NETLOOP_SUCCESS);
>+  } else {
>+#endif
>   /* set the mac address in the waiting packet's header
>  and transmit it */
>   memcpy(((struct ethernet_hdr *)net_tx_packet)->et_dest,
>  &arp->ar_sha, ARP_HLEN);
>   net_send_packet(net_tx_packet, arp_wait_tx_packet_size);
>+#ifdef CONFIG_CMD_ARP
>+  }
>+#endif
> 
>   /* no arp request pending now */
>   net_arp_wait_packet_ip.s_addr = 0;
>@@ -243,3 +252,18 @@ bool arp_is_waiting(void)
> {
>   return !!net_arp_wait_packet_ip.s_addr;
> }
>+
>+#ifdef CONFIG_CMD_ARP
>+u8 arp_query_ethaddr[6];
>+struct in_addr arp_query_ip;
>+
>+void arp_query(void)
>+{
>+  arp_wait_packet_ethaddr = arp_query_ethaddr;
>+  net_arp_wait_packet_ip = arp_query_ip;
>+
>+  arp_wait_timer_start = get_timer(0);
>+  printf("Using %s device\n", eth_get_name());
>+  arp_request();
>+}
>+#endif
>diff --git a/net/arp.h b/net/arp.h
>index 25b3c00d5c..f3e5cb8504 100644
>--- a/net/arp.h
>+++ b/net/arp.h
>@@ -29,4 +29,8 @@ void arp_raw_request(struct in_addr source_ip, const
>uchar *targetEther,
> int arp_timeout_check(void);
>void arp_receive(struct ethernet_hdr *et, struct ip_udp_hdr *ip, int
>len);
> 
>+#ifdef CONFIG_CMD_ARP
>+void arp_query(void);
>+#endif
>+
> #endif /* __ARP_H__ */
>diff --git a/net/net.c b/net/net.c
>index b58f3062b2..21623562e8 100644
>--- a/net/net.c
>+++ b/net/net.c
>@@ -492,6 +492,11 @@ restart:
>

Re: [PATCH] Revert "spl: Drop bd_info in the data section"

2021-04-09 Thread Adam Ford

On Fri, Apr 9, 2021 at 2:20 PM Alex G.  wrote:
>
> Hi Simon
>
> On 4/8/21 6:55 PM, Simon Glass wrote:
> > Hi Alexandru,
> >
> > On Fri, 9 Apr 2021 at 04:56, Alexandru Gagniuc  wrote:
> >>
> >> This reverts commit 38d6b7ebdaee3e0e8426ef1b9df88bdce8ae2e75.
> >>
> >> struct global_data contains a pointer to the bd_info structure. This
> >> pointer was populated spl_set_bd() to a pre-allocated bd_info in the
> >> ".data" section. The referenced commit replaced this mechanism to one
> >> that uses malloc(). That new mechanism is only used if SPL_ALLOC_BD=y.
> >> which very few boards do.
> >>
> >> The result is that (struct global_data)->bd is NULL in SPL on most
> >> platforms. This breaks falcon mode, since arch_fixup_fdt() tries to
> >> access (struct global_data)->bd and set the "/memory" node in the
> >> devicetree. The result is that the "/memory" node contains garbage
> >> values, causing linux to panic() as it sets up the page table.
> >>
> >> Instead of trying to fix the mess, potentially causing other issues,
> >> revert to the code that worked, while this change is reworked.
> >
> > The goal here is to drop a feature that few boards use and reduce
> > memory usage in SPL. It has been in place for two releases now.
> >
> > If Falcon mode needs it, perhaps you could add an 'imply' in the
> > Kconfig for that feature? Is there one? Or perhaps
> > CONFIG_ARCH_FIXUP_FDT_MEMORY ?
> >
> > One option would be to return an error in arch_fixup_fdt(). In
> > general, fixups make things tricky because there is no way to
> > determine when they are used but at least this one has a CONFIG.
> >
>
> The argument that this has been in place for two releases is incorrect.
> Commit 38d6b7ebdaee3e0e8426ef1b9df88bdce8ae2e75 was only introduced with
> the v2021.04 release. It definitely was not in 2021.01. It's only in the
> last release, which is four days old t the time of this writing.
>
> Although I was able to find one example, the reality is that we don't
> know the full extent of the breakage. The prudent thing at this point is
> to revert.
>
> The knowledge of how to init the platform is in the devicetree and code.
> Why should kconfig also be involved in storing this knowledge? By this
> model, as the number of boards increases without bounds, every "if"
> predicate tends to be Kconfig driven. That is not maintainable, and why
> I think the original change --and the proposed fixes-- are broken by design.
>
> Furthermore, I'm happy to discuss what to do about Falcon mode, and if
> we should kill it entirely (I have an alternative proposal).  But we
> shouldn't have that discussion in a manner holding my platform hostage.
> To be fair to you, I don't think reverting a 64-byte savings in .data
> will hold your platform hostage either.

That original patch broke a bunch of OMAP boards, but enabling
SPL_ALLOC_BD was the solution to fix the issue.
Can you try enabling SPL_ALLOC_BD and see if that fixes it?  Maybe we
can make falcon mode imply it.

adam
>
> Alex
>

Re: [PATCH 06/11] imx: ventana: convert U-Boot to OF_CONTROL using FIT image

2021-04-09 Thread Tim Harvey

On Fri, Apr 9, 2021 at 1:08 PM Tim Harvey  wrote:
>
> On Fri, Apr 9, 2021 at 1:00 PM Stefano Babic  wrote:
> >
> > Hi Tim,
> >
> > On 09.04.21 21:52, Tim Harvey wrote:
> > > On Thu, Apr 8, 2021 at 1:57 PM  wrote:
> > >>
> > >>> In preparation for dm conversion convert to OF_CONTROL by adding FIT 
> > >>> image
> > >>> support and multi dtb.
> > >>> Add a board_fit_config_name_match to match the dtb based off of EEPROM
> > >>> model.
> > >>> Signed-off-by: Tim Harvey 
> > >> Applied to u-boot-imx, master, thanks !
> > >>
> > >> Best regards,
> > >> Stefano Babic
> > >>
> > >
> > > Stefano,
> > >
> > > Something broke apparently between the branch I had my patches on top
> > > of and u-boot-imx/master that these got merged in. My board is hanging
> > > at dram_init_banksize - did something change recently that you are
> > > aware of that may require some changes on my end?
> > >
> >
> > There are a lot of changes, but they are related to i.MX8. I have not
> > merged something for i.MX6 that can justify this behavior. So I do not
> > know - have you compared the two branches, at least for Ventana boards ?
> >
>
> It's hanging at arch/arm/mach-imx/spl.c:dram_init_banksize on
> 'gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE' so I suppose
> something is wrong with gd.

specifically gd->bd is NULL. I'll have to track down how that gets setup.

Tim

Re: [PATCH 06/11] imx: ventana: convert U-Boot to OF_CONTROL using FIT image

2021-04-09 Thread Tim Harvey

On Fri, Apr 9, 2021 at 1:00 PM Stefano Babic  wrote:
>
> Hi Tim,
>
> On 09.04.21 21:52, Tim Harvey wrote:
> > On Thu, Apr 8, 2021 at 1:57 PM  wrote:
> >>
> >>> In preparation for dm conversion convert to OF_CONTROL by adding FIT image
> >>> support and multi dtb.
> >>> Add a board_fit_config_name_match to match the dtb based off of EEPROM
> >>> model.
> >>> Signed-off-by: Tim Harvey 
> >> Applied to u-boot-imx, master, thanks !
> >>
> >> Best regards,
> >> Stefano Babic
> >>
> >
> > Stefano,
> >
> > Something broke apparently between the branch I had my patches on top
> > of and u-boot-imx/master that these got merged in. My board is hanging
> > at dram_init_banksize - did something change recently that you are
> > aware of that may require some changes on my end?
> >
>
> There are a lot of changes, but they are related to i.MX8. I have not
> merged something for i.MX6 that can justify this behavior. So I do not
> know - have you compared the two branches, at least for Ventana boards ?
>

It's hanging at arch/arm/mach-imx/spl.c:dram_init_banksize on
'gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE' so I suppose
something is wrong with gd.

I'll have to compare branches as you suggest.

Tim

Re: [PATCH 06/11] imx: ventana: convert U-Boot to OF_CONTROL using FIT image

2021-04-09 Thread Stefano Babic


Hi Tim,

On 09.04.21 21:52, Tim Harvey wrote:

On Thu, Apr 8, 2021 at 1:57 PM  wrote:



In preparation for dm conversion convert to OF_CONTROL by adding FIT image
support and multi dtb.
Add a board_fit_config_name_match to match the dtb based off of EEPROM
model.
Signed-off-by: Tim Harvey 

Applied to u-boot-imx, master, thanks !

Best regards,
Stefano Babic



Stefano,

Something broke apparently between the branch I had my patches on top
of and u-boot-imx/master that these got merged in. My board is hanging
at dram_init_banksize - did something change recently that you are
aware of that may require some changes on my end?



There are a lot of changes, but they are related to i.MX8. I have not 
merged something for i.MX6 that can justify this behavior. So I do not 
know - have you compared the two branches, at least for Ventana boards ?


Stefano


Thanks,

Tim



--
=
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=

Re: [PATCH 06/11] imx: ventana: convert U-Boot to OF_CONTROL using FIT image

2021-04-09 Thread Tim Harvey

On Thu, Apr 8, 2021 at 1:57 PM  wrote:
>
> > In preparation for dm conversion convert to OF_CONTROL by adding FIT image
> > support and multi dtb.
> > Add a board_fit_config_name_match to match the dtb based off of EEPROM
> > model.
> > Signed-off-by: Tim Harvey 
> Applied to u-boot-imx, master, thanks !
>
> Best regards,
> Stefano Babic
>

Stefano,

Something broke apparently between the branch I had my patches on top
of and u-boot-imx/master that these got merged in. My board is hanging
at dram_init_banksize - did something change recently that you are
aware of that may require some changes on my end?

Thanks,

Tim

Re: [PATCH] Revert "spl: Drop bd_info in the data section"

2021-04-09 Thread Alex G.


Hi Simon

On 4/8/21 6:55 PM, Simon Glass wrote:

Hi Alexandru,

On Fri, 9 Apr 2021 at 04:56, Alexandru Gagniuc  wrote:


This reverts commit 38d6b7ebdaee3e0e8426ef1b9df88bdce8ae2e75.

struct global_data contains a pointer to the bd_info structure. This
pointer was populated spl_set_bd() to a pre-allocated bd_info in the
".data" section. The referenced commit replaced this mechanism to one
that uses malloc(). That new mechanism is only used if SPL_ALLOC_BD=y.
which very few boards do.

The result is that (struct global_data)->bd is NULL in SPL on most
platforms. This breaks falcon mode, since arch_fixup_fdt() tries to
access (struct global_data)->bd and set the "/memory" node in the
devicetree. The result is that the "/memory" node contains garbage
values, causing linux to panic() as it sets up the page table.

Instead of trying to fix the mess, potentially causing other issues,
revert to the code that worked, while this change is reworked.


The goal here is to drop a feature that few boards use and reduce
memory usage in SPL. It has been in place for two releases now.

If Falcon mode needs it, perhaps you could add an 'imply' in the
Kconfig for that feature? Is there one? Or perhaps
CONFIG_ARCH_FIXUP_FDT_MEMORY ?

One option would be to return an error in arch_fixup_fdt(). In
general, fixups make things tricky because there is no way to
determine when they are used but at least this one has a CONFIG.



The argument that this has been in place for two releases is incorrect. 
Commit 38d6b7ebdaee3e0e8426ef1b9df88bdce8ae2e75 was only introduced with 
the v2021.04 release. It definitely was not in 2021.01. It's only in the 
last release, which is four days old t the time of this writing.


Although I was able to find one example, the reality is that we don't 
know the full extent of the breakage. The prudent thing at this point is 
to revert.


The knowledge of how to init the platform is in the devicetree and code. 
Why should kconfig also be involved in storing this knowledge? By this 
model, as the number of boards increases without bounds, every "if" 
predicate tends to be Kconfig driven. That is not maintainable, and why 
I think the original change --and the proposed fixes-- are broken by design.


Furthermore, I'm happy to discuss what to do about Falcon mode, and if 
we should kill it entirely (I have an alternative proposal).  But we 
shouldn't have that discussion in a manner holding my platform hostage. 
To be fair to you, I don't think reverting a 64-byte savings in .data 
will hold your platform hostage either.


Alex

Re: Running u-boot 2021.04 on Raspberry Pi 4

2021-04-09 Thread Roman Shaposhnik

On Fri, Apr 9, 2021 at 3:15 AM Matthias Brugger  wrote:

>
>
> On 09/04/2021 10:14, Nicolas Saenz Julienne wrote:
> > [ Adding Matthias for the SMBIOS part ]
> >
> > On Fri, 2021-04-09 at 00:00 -0700, Roman Shaposhnik wrote:
> >> On Thu, Apr 8, 2021 at 8:59 PM Sean Anderson  wrote:
> >>> On 4/8/21 8:18 PM, Roman Shaposhnik wrote:
>  Hi!
> 
>  first time poster, long time lurker here. Over at Project EVE
>  https://github.com/lf-edge/eve I've been trying to migrate
>  from our current u-boot v2020.07 + patches:
> 
> 
> https://github.com/lf-edge/eve/tree/master/pkg/u-boot/patches/patches-v2020.07
>  to the latest u-boot 2021.04.
> 
>  Great news is that most of the patches we dependent
>  on seem to have been pulled upstream. However, this
>  single *chunk* of one patchset wasn't:
> 
> 
> https://github.com/lf-edge/eve/blob/master/pkg/u-boot/patches/patches-v2020.07/0001-usb-xhci-Load-Raspberry-Pi-4-VL805-s-firmware.patch#L293
> 
>  I'm wondering what was the reason for leaving it behind,
> >>>
> >>> +CC Nicolas
> >>>
>    - Get rid of PCI core patch as not needed with correct DT PCI
> topology
> >>>
> >>> also from the cover letter
> >>>
>  This also depends on a DT/bindings patch available on the
> linux-mailing lists:
>  https://www.mail-archive.com/linux-kernel@.../msg2205783.html
> >>>
> >>> The merged version of this series is
> >>>
> >>>
> https://patchwork.kernel.org/project/linux-usb/list/?series=310015&state=%2A&archive=both
> >>>
>  Here is the relevant bit for reference/discussion:
> 
>   &pcie0 {
>  pci@1,0 {
>  #address-cells = <3>;
>  #size-cells = <2>;
>  ranges;
> 
>  reg = <0 0 0 0 0>;
> 
>  usb@1,0 {
>  reg = <0x1 0 0 0 0>;
>  resets = <&reset
> RASPBERRYPI_FIRMWARE_RESET_ID_USB>;
>  };
>  };
>   };
> >>>
> >
> > Yes, instead of using a PCI quirk we settled on a reset controller. All
> in all
> > it is less hacky. But needs changes in DT.
> >
> >> Aha! Thank you so much -- this is super helpful!
> >>
>  since without it I don't seem to have functioning USB
>  devices on my  Raspberry Pi 4. In fact, adding it back:
> 
> 
> https://github.com/rvs/eve/tree/u-boot/pkg/u-boot/patches/patches-v2021.04
>  (just that one chunk -- 'cuz the reset got upstreamed)
>  seems to solve the issue for me.
> 
>  Another question I have is that the new u-boot seems to have
>  some kind of a regression that I can't quite debug. The SMBIOS
>  tables that it constructs during EFI boot sequence seem to be
>  broken (see the dmidecode output below). Again, this seems
>  to be a regression compared to  v2020.07. Any ideas on what
>  could be wrong or how can I start debugging it would be
> >>>
>
> Yes, that's not working right now. I'm working on a fix for the tables:
>
> https://patchwork.ozlabs.org/project/uboot/patch/20210406090435.19357-1-matthias@kernel.org/
>
> This will fix the error en dmidecode but the tables will be basically
> empty.
> Before that there was some information that helped you to identify that
> you are
> running on a RaspberryPi.
>
> A quick fix would be to add that information to the DTS. Like for example
> done here:
>
> https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/dts/rk3328-rock64-u-boot.dtsi#L13


Thanks! Works like a charm:

https://github.com/lf-edge/eve/blob/master/pkg/u-boot/rpi/overlays/raspberrypi-rpi.dts

But yes -- it would be nice to fix the default behaviour. Speaking of
tables being empty
(once your fix above makes it in) it may also make sense to document it
someplace,
but I honestly don't know what a good place for that would be ;-)


> On the long run we want to add a sysinfo driver to read the information
> for the
> mailbox driver and use that. But my understanding is that for that we
> would need
> to create a SPL for the mailbox driver to provide that info in a shared
> data
> structure. It's still on my list for investigation.
>

That sounds pretty useful too -- although my usecase is much more limited
-- I just
need to be able to provide quick DT overlays to reliably identify various
HATs on RPi
at the SMBIOS level.

Where it gets interesting, of course, are the HATs that provide their OWN
DTs via
EEPROM I2C.

Thanks,
Roman.

Re: mkimage regression when building ARCH=mips defconfig Linux kernel

2021-04-09 Thread Tom Rini

On Fri, Apr 09, 2021 at 11:55:52AM +1200, Simon Glass wrote:
> +Tom Rini
> 
> Hi Nathan,
> 
> On Fri, 9 Apr 2021 at 06:23, Nathan Chancellor  wrote:
> >
> > Hi Simon,
> >
> > Apologies if this is not the proper way to report a regression, this is my 
> > first
> > time interacting with the U-Boot community.
> >
> > My distribution updated the uboot-tools package to 2021.04, which broke my
> > Linux kernel builds for ARCH=mips:
> >
> > $ make -skj"$(nproc)" ARCH=mips CROSS_COMPILE=mips-linux- defconfig all
> > ...
> > /usr/bin/mkimage: verify_header failed for FIT Image support with exit code 
> > 1
> > make[2]: *** [arch/mips/boot/Makefile:173: arch/mips/boot/vmlinux.gz.itb] 
> > Error 1
> > ...
> >
> > I bisected this down to your commit:
> >
> > 3f04db891a353f4b127ed57279279f851c6b4917 is the first bad commit
> > commit 3f04db891a353f4b127ed57279279f851c6b4917
> > Author: Simon Glass 
> > Date:   Mon Feb 15 17:08:12 2021 -0700
> >
> > image: Check for unit addresses in FITs
> >
> > Using unit addresses in a FIT is a security risk. Add a check for this
> > and disallow it.
> >
> > CVE-2021-27138
> >
> > Signed-off-by: Simon Glass 
> > Reported-by: Bruce Monroe 
> > Reported-by: Arie Haenel 
> > Reported-by: Julien Lenoir 
> >
> >  common/image-fit.c  | 56 
> > +
> >  test/py/tests/test_vboot.py |  9 
> >  2 files changed, 57 insertions(+), 8 deletions(-)
> > bisect run success
> >
> > $ git bisect log
> > # bad: [e9c99db7787e3b5c2ef05701177c43ed1c023c27] Merge branch 
> > '2021-04-07-CI-improvements'
> > # good: [c4fddedc48f336eabc4ce3f74940e6aa372de18c] Prepare v2021.01
> > git bisect start 'e9c99db7787e3b5c2ef05701177c43ed1c023c27' 'v2021.01'
> > # good: [b2c86f596cfb1ea9f7f5138f72f1c5c49e3ae3f1] arm: dts: r8a774a1: 
> > Import DTS queued for Linux 5.12-rc1
> > git bisect good b2c86f596cfb1ea9f7f5138f72f1c5c49e3ae3f1
> > # bad: [74f4929c2c73beb595faf7d5d9bb6a78d710c2fd] ddr: marvell: axp: fix 
> > array types have different bounds warning
> > git bisect bad 74f4929c2c73beb595faf7d5d9bb6a78d710c2fd
> > # bad: [cbe607b920bc0827d8fe379ed4f5ae4e2058513e] Merge tag 
> > 'xilinx-for-v2021.04-rc3' of 
> > https://gitlab.denx.de/u-boot/custodians/u-boot-microblaze
> > git bisect bad cbe607b920bc0827d8fe379ed4f5ae4e2058513e
> > # good: [d5f3aadacbc63df3b690d6fd9f0aa3f575b43356] test: Add tests for the 
> > 'evil' vboot attacks
> > git bisect good d5f3aadacbc63df3b690d6fd9f0aa3f575b43356
> > # bad: [a1a652e8016426e2d67148cab225cd5ec45189fb] Merge tag 'mmc-2021-2-19' 
> > of https://gitlab.denx.de/u-boot/custodians/u-boot-mmc
> > git bisect bad a1a652e8016426e2d67148cab225cd5ec45189fb
> > # bad: [aeedeae40733131467de72c68e639cf9d795e059] spl: fit: Replace #ifdef 
> > blocks with more readable constructs
> > git bisect bad aeedeae40733131467de72c68e639cf9d795e059
> > # bad: [eb5fd9e46c11ea41430d9c5bcc81d4583424216e] usb: kbd: destroy device 
> > after console is stopped
> > git bisect bad eb5fd9e46c11ea41430d9c5bcc81d4583424216e
> > # bad: [99cb2b996bd649d98069a95941beaaade0a4447a] stdio: Split out 
> > nulldev_register() and move it under #if
> > git bisect bad 99cb2b996bd649d98069a95941beaaade0a4447a
> > # bad: [3f04db891a353f4b127ed57279279f851c6b4917] image: Check for unit 
> > addresses in FITs
> > git bisect bad 3f04db891a353f4b127ed57279279f851c6b4917
> > # good: [6f3c2d8aa5e6cbd80b5e869bbbddecb66c329d01] image: Add an option to 
> > do a full check of the FIT
> > git bisect good 6f3c2d8aa5e6cbd80b5e869bbbddecb66c329d01
> > # good: [124c255731c76a2b09587378b2bcce561bcd3f2d] libfdt: Check for 
> > multiple/invalid root nodes
> > git bisect good 124c255731c76a2b09587378b2bcce561bcd3f2d
> > # first bad commit: [3f04db891a353f4b127ed57279279f851c6b4917] image: Check 
> > for unit addresses in FITs
> >
> > Is this an actual regression or is this now the expected behavior? I have 
> > added
> > Thomas and the linux-mips mailing list to take a look and see if the Linux
> > kernel needs to have its sources updated.
> 
> It is expected. See the code in that commit:
> 
>   /*
>* U-Boot stopped using unit addressed in 2017. Since libfdt
>* can match nodes ignoring any unit address, signature
>* verification can see the wrong node if one is inserted with
>* the same name as a valid node but with a unit address
>* attached. Protect against this by disallowing unit addresses.
>*/
>   if (!ret && CONFIG_IS_ENABLED(FIT_SIGNATURE)) {
>  ret = fdt_check_no_at(fit, 0);
> 
>  if (ret) {
> log_debug("FIT check error %d\n", ret);
> return ret;
>  }
>   }
> 
> Possibly you are using @ nodes in your FIT files in the kernel. Is it
> possible to use a hyphen instead?

Yeah, it looks like arch/mips/generic/*.its.S in the kernel will need to
be updated.

-- 
Tom


signature.asc
Description: PGP signature

Re: Pull request: u-boot-imx u-boot-imx-20210409

2021-04-09 Thread Tom Rini

On Fri, Apr 09, 2021 at 01:40:08PM +0200, Stefano Babic wrote:

> Hi Tom,
> 
> please pull from u-boot-imx, thanks !
> 
> The following changes since commit e9c99db7787e3b5c2ef05701177c43ed1c023c27:
> 
>   Merge branch '2021-04-07-CI-improvements' (2021-04-07 15:54:07 -0400)
> 
> are available in the Git repository at:
> 
>   https://gitlab.denx.de/u-boot/custodians/u-boot-imx.git
> tags/u-boot-imx-20210409
> 
> for you to fetch changes up to 2fc93e5bafdae7cf6373479e054e9f3943fde23c:
> 
>   imx: bootaux fix elf loading (2021-04-08 23:59:50 +0200)
> 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature

Re: [RFC PATCH v4 1/2] arch: riscv: cpu: Add callback to init each core

2021-04-09 Thread Green Wan

Hi folks,

Correct me if I'm wrong, like Rick mentioned, i/dcache
enable/disable() is only called on the main hart. Right now the dummy
i/dcache enable/disable are empty and shared among all riscv CPU. The
ax25 is the only one that has its own implementation for now.

FU540/FU740 also leverages the dummy i/dcache enable/disable()
functions (only main hart calls them). L2 cache on FU540/FU740 is
enabled as SRAM purpose. And according to the HW design behavior, once
L2 is enabled, it can't be disabled unless doing a reset.[1] The Linux
L2$ driver will handle that according to the configuration of L2
registers.

[1] https://static.dev.sifive.com/FU540-C000-v1.0.pdf

Thanks,

On Fri, Apr 9, 2021 at 9:18 PM Sean Anderson  wrote:
>
> On 4/9/21 4:16 AM, Rick Chen wrote:
> > Hi Sean ,Bin
> >
> >> From: Bin Meng [mailto:bmeng...@gmail.com]
> >> Sent: Tuesday, April 06, 2021 5:16 PM
> >> To: Sean Anderson
> >> Cc: Green Wan; Rick Jian-Zhi Chen(陳建志); Paul Walmsley; Pragnesh Patel; Bin 
> >> Meng; Simon Glass; Atish Patra; Leo Yu-Chi Liang(梁育齊); Brad Kim; U-Boot 
> >> Mailing List
> >> Subject: Re: [RFC PATCH v4 1/2] arch: riscv: cpu: Add callback to init 
> >> each core
> >>
> >> On Sat, Apr 3, 2021 at 6:53 AM Sean Anderson  wrote:
> >>>
> >>> On 3/30/21 1:26 AM, Green Wan wrote:
>  Add a callback riscv_hart_early_init() to ./arch/riscv/cpu/start.S to
>  allow different riscv hart perform setup code for each hart as early
>  as possible. Since all the harts enter the callback, they must be able
>  to run the same setup.
> 
>  Signed-off-by: Green Wan 
>  ---
> arch/riscv/cpu/cpu.c   | 15 +++
> arch/riscv/cpu/start.S | 14 ++
> 2 files changed, 29 insertions(+)
> 
>  diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c
>  index 85592f5bee..1652e51137 100644
>  --- a/arch/riscv/cpu/cpu.c
>  +++ b/arch/riscv/cpu/cpu.c
>  @@ -140,3 +140,18 @@ int arch_early_init_r(void)
> {
> return riscv_cpu_probe();
> }
>  +
>  +/**
>  + * riscv_hart_early_init() - A dummy function called by
>  + * ./arch/riscv/cpu/start.S to allow to disable/enable features of each 
>  core.
>  + * For example, turn on or off the functional block of CPU harts.
>  + *
>  + * In a multi-core system, this function must not access shared 
>  resources.
>  + *
>  + * Any access to such resources would probably be better done with
>  + * available_harts_lock held. However, I doubt that any such access 
>  will be
>  + * necessary.
>  + */
>  +__weak void riscv_hart_early_init(void)
>  +{
>  +}
>  diff --git a/arch/riscv/cpu/start.S b/arch/riscv/cpu/start.S
>  index 8589509e01..ab73008f23 100644
>  --- a/arch/riscv/cpu/start.S
>  +++ b/arch/riscv/cpu/start.S
>  @@ -117,6 +117,20 @@ call_board_init_f_0:
> mv  sp, a0
> #endif
> 
>  +#if CONFIG_IS_ENABLED(RISCV_MMODE)
>  + /*
>  +  * Jump to riscv_hart_early_init() to perform init for each core. 
>  Not
>  +  * expect to access gd since gd is not initialized. All operations 
>  in the
>  +  * function should affect core itself only. In multi-core system, 
>  any access
>  +  * to common resource or registers outside core should be avoided 
>  or need a
>  +  * protection for multicore.
>  +  *
>  +  * A dummy implementation is provided in ./arch/riscv/cpu/cpu.c.
>  +  */
>  +call_riscv_hart_early_init:
>  + jal riscv_hart_early_init
>  +#endif
>  +
> >>>
> >>> I wonder if we could move the calls to icache_enable and dcache_enable
> >>> into this function. Though this would have the consequence of enabling
> >>> caches on all harts for CPUs which previously only enabled them for the
> >>> boot hart. I think ax25 is the only CPU which currently does this. Bin,
> >>> would this be an issue?
> >
> > No, they are functions shall be called in different stage about lottery.
> > riscv_hart_early_init() is called before lottery for all harts.
> > It shall not move icache_enable() and dcache_enable() into
> > riscv_hart_early_init(), they are belong to the stage after lottery
> > only for main hart.
> >
> >>
> >> Good catch. I believe AX25 cache support is currently somehow broken?
> >
> > No, AX25 cache support is currently work well.
> >
> >>
> >> I think Rick has to comment on this as he added this via commit:
> >>
> >> commit 52923c6db7f00e0197ec894c8c1bb8a7681974bb
> >> Author: Rick Chen 
> >> Date:   Wed Nov 7 09:34:06 2018 +0800
> >>
> >>  riscv: cache: Implement i/dcache [status, enable, disable]
> >>
> >>  AndeStar RISC-V(V5) provide mcache_ctl register which
> >>  can configure I/D cache as enabled or disabled.
> >>
> >>  This CSR will be encapsulated by CONFIG_RISCV_NDS.
> >>  If you want to configure cache on AndeStar V5
> >>

[PATCH] board_f: cosmetic: change the debug trace to KB in reserve_video

2021-04-09 Thread Patrick Delaunay

Update the debug trace for the reserved video memory to KB as indicated
in the message with "%luk"; before the patch the computed size
gd->relocaddr - addr is in bytes.

This patch aligns the debug trace in reserve_video() with others
functions, for example on stm32mp157c-dk2:

  - Reserving 3080192k for video at: dfd0
  + Reserving 3008k for video at: dfd0
Reserving 873k for U-Boot at: dfc25000
Reserving 32776k for malloc() at: ddc23000
Reserving 72 Bytes for Board Info at: ddc22fb0
Reserving 280 Bytes for Global Data at: ddc22e90
Reserving 119072 Bytes for FDT at: ddc05d70
Reserving 0x278 Bytes for bootstage at: ddc05af0

Fixes: 5630d2fbc50f3035 ("board: Show memory for frame buffers")
Signed-off-by: Patrick Delaunay 
---

 common/board_f.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/board_f.c b/common/board_f.c
index 0cddf0359d..203e965799 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -394,7 +394,7 @@ static int reserve_video(void)
if (ret)
return ret;
debug("Reserving %luk for video at: %08lx\n",
- (unsigned long)gd->relocaddr - addr, addr);
+ ((unsigned long)gd->relocaddr - addr) >> 10, addr);
gd->relocaddr = addr;
 #elif defined(CONFIG_LCD)
 #  ifdef CONFIG_FB_ADDR
-- 
2.17.1

Re: [PATCH] powerpc, wdt: disable ratelimiting when disabling interrupts

2021-04-09 Thread Christophe Leroy

Le 09/04/2021 à 16:12, Rasmus Villemoes a écrit :

On powerpc, time as measured by get_timer() ceases to pass when
interrupts are disabled (since on powerpc get_timer() returns the
value of a volatile variable that gets updated via a timer
interrupt). That in turn means the watchdog_reset() function provided
by CONFIG_WDT ceases to work due to the ratelimiting it imposes.

Normally, interrupts are just disabled very briefly. However, during
bootm, they are disabled for good prior to decompressing the kernel
image, which can be a somewhat time-consuming operation. Even when we
manage to decompress the kernel and do the other preparation steps and
hand over control to the kernel, the kernel also takes some time
before it is ready to assume responsibility for handling the
watchdog. The end result is that the board gets reset prematurely.

The ratelimiting isn't really strictly needed (prior to DM WDT, no
such thing existed), so just disable it when we know that time no
longer passes and have watchdog_reset() (e.g. called from
decompression loop) unconditionally reset the watchdog timer.

Do we need to make it that complicated ? I think before the generic implementation, powerpc didn't
have a rate limitation at all for pinging the watchdog, why not go back this direction, all the time ?

I mean we could simply set reset_period to 0 at all time for powerpc ( and change the test to
time_after_eq() instead of time_after() ).

Signed-off-by: Rasmus Villemoes
---

I previously sent a patch to change the ratelimiting to be based on
get_ticks() instead of get_timer(), but that has gone nowhere
[1]. This is an alternative which only affects powerpc (and only
boards that have enabled CONFIG_WDT). I hope the watchdog maintainers
will accept at least one of these, or suggest a third alternative, so
I don't have to keep some out-of-tree patch applied without knowing if
that's the direction upstream will take.

[1]
https://patchwork.ozlabs.org/project/uboot/patch/20200605111657.28773-1-rasmus.villem...@prevas.dk/

1 2 >

1 - 100 of 114 matches

Mail list logo