Re: [PATCH] watchdog: use time_after_eq() in watchdog_reset()

2021-04-14 Thread Stefan Roese

On 13.04.21 16:43, Rasmus Villemoes wrote:

Some boards don't work with the rate-limiting done in the generic
watchdog_reset() provided by wdt-uclass.

For example, on powerpc, get_timer() ceases working during bootm since
interrupts are disabled before the kernel image gets decompressed, and
when the decompression takes longer than the watchdog device
allows (or enough of the budget that the kernel doesn't get far enough
to assume responsibility for petting the watchdog), the result is a
non-booting board.

As a somewhat hacky workaround (because DT is supposed to describe
hardware), allow specifying hw_margin_ms=0 in device tree to
effectively disable the ratelimiting and actually ping the watchdog
every time watchdog_reset() is called. For that to work, the "has
enough time passed" check just needs to be tweaked a little to allow
the now==next_reset case as well.

Suggested-by: Christophe Leroy 
Signed-off-by: Rasmus Villemoes 
---

It's the option I dislike the most (because of the DT abuse), but I
also do accept that it's the one with the minimal code impact, and
apparently the path of least resistance. So here it is.


Right. An alternative way would have been to add a new Kconfig symbol
to define the default value of "reset_period" so that it can be
configured to different values via Kconfig as well.


  drivers/watchdog/wdt-uclass.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
index 0603ffbd36..2687135296 100644
--- a/drivers/watchdog/wdt-uclass.c
+++ b/drivers/watchdog/wdt-uclass.c
@@ -148,7 +148,7 @@ void watchdog_reset(void)
  
  	/* Do not reset the watchdog too often */

now = get_timer(0);
-   if (time_after(now, next_reset)) {
+   if (time_after_eq(now, next_reset)) {
next_reset = now + reset_period;
wdt_reset(gd->watchdog_dev);
}



Reviewed-by: Stefan Roese 

Thanks,
Stefan


Re: [PATCH v2 2/2] powerpc: introduce CONFIG_CACHE_FLUSH_WATCHDOG_THRESHOLD

2021-04-14 Thread Stefan Roese

On 13.04.21 21:38, Rasmus Villemoes wrote:

When flush_cache() is called during boot on our ~7M kernel image, the
hundreds of thousands of WATCHDOG_RESET calls end up adding
significantly to boottime. Flushing a single cache line doesn't take
many microseconds, so doing these calls for every cache line is
complete overkill.

The generic watchdog_reset() provided by wdt-uclass.c actually
contains some rate-limiting logic that should in theory mitigate this,
but alas, that rate-limiting must be disabled on powerpc because of
its get_timer() implementation - get_timer() works just fine until
interrupts are disabled, but it just so happens that the "big"
flush_cache() call happens in the part of bootm where interrupts are
indeed disabled. [1] [2] [3]

I have checked with objdump that the generated code doesn't change
when this option is left at its default value of 0: gcc is smart
enough to see that wd_limit is constant 0, the ">=" comparisons are
tautologically true, hence all assignments to "flushed" are eliminated


Nitpicking:
s/tautologically/autologically. BTW, I've never heard or read this word
before.


as dead stores.

On our board, setting the option to something like 65536 ends up
reducing total boottime by about 0.8 seconds.

[1] 
https://patchwork.ozlabs.org/project/uboot/patch/20200605111657.28773-1-rasmus.villem...@prevas.dk/
[2] https://lists.denx.de/pipermail/u-boot/2021-April/446906.html
[3] https://lists.denx.de/pipermail/u-boot/2021-April/447280.html

Signed-off-by: Rasmus Villemoes 
---
  arch/powerpc/Kconfig |  1 +
  arch/powerpc/lib/Kconfig |  9 +
  arch/powerpc/lib/cache.c | 14 --
  3 files changed, 22 insertions(+), 2 deletions(-)
  create mode 100644 arch/powerpc/lib/Kconfig

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 6a2e88fed2..133447648c 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -49,5 +49,6 @@ source "arch/powerpc/cpu/mpc83xx/Kconfig"
  source "arch/powerpc/cpu/mpc85xx/Kconfig"
  source "arch/powerpc/cpu/mpc86xx/Kconfig"
  source "arch/powerpc/cpu/mpc8xx/Kconfig"
+source "arch/powerpc/lib/Kconfig"
  
  endmenu

diff --git a/arch/powerpc/lib/Kconfig b/arch/powerpc/lib/Kconfig
new file mode 100644
index 00..b30b5edf7c
--- /dev/null
+++ b/arch/powerpc/lib/Kconfig
@@ -0,0 +1,9 @@
+config CACHE_FLUSH_WATCHDOG_THRESHOLD
+   int "Bytes to flush between WATCHDOG_RESET calls"
+   default 0
+   help
+ The flush_cache() function periodically, and by default for
+ every cache line, calls WATCHDOG_RESET(). When flushing a
+ large area, that may add a significant amount of
+ overhead. This option allows you to set a threshold for how
+ many bytes to flush between each WATCHDOG_RESET call.
diff --git a/arch/powerpc/lib/cache.c b/arch/powerpc/lib/cache.c
index 3e487f50fe..115ae8e160 100644
--- a/arch/powerpc/lib/cache.c
+++ b/arch/powerpc/lib/cache.c
@@ -12,6 +12,8 @@
  void flush_cache(ulong start_addr, ulong size)
  {
ulong addr, start, end;
+   ulong wd_limit = CONFIG_CACHE_FLUSH_WATCHDOG_THRESHOLD;
+   ulong flushed = 0;
  
  	start = start_addr & ~(CONFIG_SYS_CACHELINE_SIZE - 1);

end = start_addr + size - 1;
@@ -19,7 +21,11 @@ void flush_cache(ulong start_addr, ulong size)
for (addr = start; (addr <= end) && (addr >= start);
addr += CONFIG_SYS_CACHELINE_SIZE) {
asm volatile("dcbst 0,%0" : : "r" (addr) : "memory");
-   WATCHDOG_RESET();
+   flushed += CONFIG_SYS_CACHELINE_SIZE;
+   if (flushed >= wd_limit) {
+   WATCHDOG_RESET();
+   flushed = 0;
+   }
}
/* wait for all dcbst to complete on bus */
asm volatile("sync" : : : "memory");
@@ -27,7 +33,11 @@ void flush_cache(ulong start_addr, ulong size)
for (addr = start; (addr <= end) && (addr >= start);
addr += CONFIG_SYS_CACHELINE_SIZE) {
asm volatile("icbi 0,%0" : : "r" (addr) : "memory");
-   WATCHDOG_RESET();
+   flushed += CONFIG_SYS_CACHELINE_SIZE;
+   if (flushed >= wd_limit) {
+   WATCHDOG_RESET();
+   flushed = 0;
+   }


It might make sense to pull these 2 identical code snippets into a
function to not duplicate this code.

Other than this:

Reviewed-by: Stefan Roese 

Thanks,
Stefan


Re: [PATCH v2 1/2] powerpc: lib: remove leftover CONFIG_5xx

2021-04-14 Thread Stefan Roese

On 13.04.21 21:38, Rasmus Villemoes wrote:

CONFIG_5xx hasn't existed since commit 502589777416 (powerpc, 5xx:
remove support for 5xx). Remove this last mention of it.

Signed-off-by: Rasmus Villemoes 
---
  arch/powerpc/lib/cache.c | 2 --
  1 file changed, 2 deletions(-)

diff --git a/arch/powerpc/lib/cache.c b/arch/powerpc/lib/cache.c
index 3c3c470bbb..3e487f50fe 100644
--- a/arch/powerpc/lib/cache.c
+++ b/arch/powerpc/lib/cache.c
@@ -11,7 +11,6 @@
  
  void flush_cache(ulong start_addr, ulong size)

  {
-#ifndef CONFIG_5xx
ulong addr, start, end;
  
  	start = start_addr & ~(CONFIG_SYS_CACHELINE_SIZE - 1);

@@ -33,5 +32,4 @@ void flush_cache(ulong start_addr, ulong size)
asm volatile("sync" : : : "memory");
/* flush prefetch queue */
asm volatile("isync" : : : "memory");
-#endif
  }



Reviewed-by: Stefan Roese 

Thanks,
Stefan


Re: [PATCH v6 1/7] riscv: dts: add fu740 support

2021-04-14 Thread Rick Chen
Hi Green,

> From: Green Wan [mailto:green@sifive.com]
> Sent: Thursday, April 08, 2021 9:40 PM
> Cc: bmeng...@gmail.com; Green Wan; Greentime Hu; Rick Jian-Zhi Chen(陳建志); 
> Paul Walmsley; Palmer Dabbelt; Anup Patel; Atish Patra; Pragnesh Patel; 
> Lukasz Majewski; Joe Hershberger; Ramon Fried; u-boot@lists.denx.de
> Subject: [PATCH v6 1/7] riscv: dts: add fu740 support
>
> Add dts support for fu740. The HiFive Unmatched support is based on
> fu740 cpu and drivers in following patch set.
>
> Signed-off-by: Green Wan 
> [greentime.hu: set fu740 speed to 1.2GHz]
> Signed-off-by: Greentime Hu 
> Reviewed-by: Bin Meng 
> ---
>  arch/riscv/dts/Makefile   |   1 +
>  arch/riscv/dts/fu740-c000-u-boot.dtsi | 105 ++
>  arch/riscv/dts/fu740-c000.dtsi| 329 ++
>  include/dt-bindings/clock/sifive-fu740-prci.h |  25 ++
>  include/dt-bindings/reset/sifive-fu740-prci.h |  19 +
>  5 files changed, 479 insertions(+)
>  create mode 100644 arch/riscv/dts/fu740-c000-u-boot.dtsi
>  create mode 100644 arch/riscv/dts/fu740-c000.dtsi
>  create mode 100644 include/dt-bindings/clock/sifive-fu740-prci.h
>  create mode 100644 include/dt-bindings/reset/sifive-fu740-prci.h
>
> diff --git a/arch/riscv/dts/Makefile b/arch/riscv/dts/Makefile
> index 8138d89d84..ed32f56c9e 100644
> --- a/arch/riscv/dts/Makefile
> +++ b/arch/riscv/dts/Makefile
> @@ -2,6 +2,7 @@
>
>  dtb-$(CONFIG_TARGET_AX25_AE350) += ae350_32.dtb ae350_64.dtb
>  dtb-$(CONFIG_TARGET_SIFIVE_UNLEASHED) += hifive-unleashed-a00.dtb
> +dtb-$(CONFIG_TARGET_SIFIVE_UNMATCHED) += hifive-unmatched-a00.dtb

Makefile need the dts file, but it is not exist in this patch. It
doesn't make sense.

Maybe you can combine with the dts relative files in [PATCH v6 6/7]
into one patch and name as :
riscv: dts: ...

LGTM.
Other than that,

Reviewed-by: Rick Chen 

>  dtb-$(CONFIG_TARGET_SIPEED_MAIX) += k210-maix-bit.dtb
>  dtb-$(CONFIG_TARGET_MICROCHIP_ICICLE) += microchip-mpfs-icicle-kit.dtb
>
> diff --git a/arch/riscv/dts/fu740-c000-u-boot.dtsi 
> b/arch/riscv/dts/fu740-c000-u-boot.dtsi
> new file mode 100644
> index 00..a5d0688b06
> --- /dev/null
> +++ b/arch/riscv/dts/fu740-c000-u-boot.dtsi
> @@ -0,0 +1,105 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/*
> + * (C) Copyright 2020-2021 SiFive, Inc
> + */
> +
> +#include 
> +
> +/ {
> +   cpus {
> +   assigned-clocks = < PRCI_CLK_COREPLL>;
> +   assigned-clock-rates = <12>;
> +   u-boot,dm-spl;
> +   cpu0: cpu@0 {
> +   clocks = < PRCI_CLK_COREPLL>;
> +   u-boot,dm-spl;
> +   status = "okay";
> +   cpu0_intc: interrupt-controller {
> +   u-boot,dm-spl;
> +   };
> +   };
> +   cpu1: cpu@1 {
> +   clocks = < PRCI_CLK_COREPLL>;
> +   u-boot,dm-spl;
> +   cpu1_intc: interrupt-controller {
> +   u-boot,dm-spl;
> +   };
> +   };
> +   cpu2: cpu@2 {
> +   clocks = < PRCI_CLK_COREPLL>;
> +   u-boot,dm-spl;
> +   cpu2_intc: interrupt-controller {
> +u-boot,dm-spl;
> +   };
> +   };
> +   cpu3: cpu@3 {
> +   clocks = < PRCI_CLK_COREPLL>;
> +   u-boot,dm-spl;
> +   cpu3_intc: interrupt-controller {
> +   u-boot,dm-spl;
> +   };
> +   };
> +   cpu4: cpu@4 {
> +   clocks = < PRCI_CLK_COREPLL>;
> +   u-boot,dm-spl;
> +   cpu4_intc: interrupt-controller {
> +   u-boot,dm-spl;
> +   };
> +   };
> +   };
> +
> +   soc {
> +   u-boot,dm-spl;
> +   clint: clint@200 {
> +   compatible = "riscv,clint0";
> +   interrupts-extended = <_intc 3 _intc 7
> +  _intc 3 _intc 7
> +  _intc 3 _intc 7
> +  _intc 3 _intc 7
> +  _intc 3 _intc 7>;
> +   reg = <0x0 0x200 0x0 0x1>;
> +   u-boot,dm-spl;
> +   };
> +   prci: clock-controller@1000 {
> +   #reset-cells = <1>;
> +   resets = < PRCI_RST_DDR_CTRL_N>,
> +< PRCI_RST_DDR_AXI_N>,
> +< PRCI_RST_DDR_AHB_N>,
> +< PRCI_RST_DDR_PHY_N>,
> +< 

Re: [PATCH] mtd: cfi: Fix PPB lock status readout

2021-04-14 Thread Stefan Roese

On 11.04.21 20:47, Marek Vasut wrote:

According to S26KL512S datasheet [1] and S29GL01GS datasheet [2],
the procedure to read out PPB lock bits is to send the PPB Entry,
PPB Read, Reset/ASO Exit. Currently, the code does send incorrect
PPB Entry, PPB Read and Reset/ASO Exit is completely missing.

The PPB Entry sent is implemented by sending flash_unlock_seq()
and flash_write_cmd(..., FLASH_CMD_READ_ID). This translates to
sequence 0x555:0xaa, 0x2aa:0x55, 0x555:0x90=FLASH_CMD_READ_ID.
However, both [1] and [2] specify the last byte of PPB Entry as
0xc0=AMD_CMD_SET_PPB_ENTRY instead of 0x90=FLASH_CMD_READ_ID,
that is  0x555:0xaa, 0x2aa:0x55, 0x555:0xc0=AMD_CMD_SET_PPB_ENTRY.
Since this does make sense, this patch fixes it and thus also
aligns the code in flash_get_size() with flash_real_protect().

The PPB Read returns 00h in case of Protected state and 01h in case
of Unprotected state, according to [1] Note 83 and [2] Note 17, so
invert the result. Moreover, align the arguments with similar code
in flash_real_protect().

Finally, Reset/ASO Exit command should be executed to exit the PPB
mode, so add the missing reset.

[1] https://www.cypress.com/file/213346/download
 Document Number: 001-99198 Rev. *M
 Table 40. Command Definitions, Nonvolatile Sector Protection
 Command Set Definitions
[2] https://www.cypress.com/file/177976/download
 Document Number: 001-98285 Rev. *R
 Table 7.1 Command Definitions, Nonvolatile Sector Protection
 Command Set Definitions

Fixes: 03deff433e ("cfi_flash: Read PPB sector protection from device for 
AMD/Spansion chips")
Signed-off-by: Marek Vasut 
Cc: Stefan Roese 


Reviewed-by: Stefan Roese 

Thanks,
Stefan


---
  drivers/mtd/cfi_flash.c | 10 +-
  1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
index 9642d7c7dc..9c27fea5d8 100644
--- a/drivers/mtd/cfi_flash.c
+++ b/drivers/mtd/cfi_flash.c
@@ -2276,12 +2276,12 @@ ulong flash_get_size(phys_addr_t base, int banknum)
flash_unlock_seq(info, 0);
flash_write_cmd(info, 0,
info->addr_unlock1,
-   FLASH_CMD_READ_ID);
+   AMD_CMD_SET_PPB_ENTRY);
info->protect[sect_cnt] =
-   flash_isset(
-   info, sect_cnt,
-   FLASH_OFFSET_PROTECT,
-   FLASH_STATUS_PROTECT);
+   !flash_isset(info, sect_cnt,
+0, 0x01);
+   flash_write_cmd(info, 0, 0,
+   info->cmd_reset);
break;
default:
/* default: not protected */




Viele Grüße,
Stefan

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


Re: [PATCH] arm: at91: gardena-smart-gateway-at91sam: Adjust to production values

2021-04-14 Thread Stefan Roese

Hi Reto,

On 14.04.21 16:49, Reto Schneider wrote:

From: Reto Schneider 

This commit updates the default config with the values that are actually
used "in the wild" and which are close to what is used on the MediaTek
MT7688 based, 2nd generation of the GARDENA smart gateway:
  - Reduce startup time by setting bootdelay to 0 (still allows accessing
the shell, one just has to send a key press quicker)
  - Adjusting U-Boot environment volume names and MTD partitions to
the actual layout

Signed-off-by: Reto Schneider 
---

  configs/gardena-smart-gateway-at91sam_defconfig | 9 +
  1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/configs/gardena-smart-gateway-at91sam_defconfig 
b/configs/gardena-smart-gateway-at91sam_defconfig
index e3d5bc47d6..76a1c42ce0 100644
--- a/configs/gardena-smart-gateway-at91sam_defconfig
+++ b/configs/gardena-smart-gateway-at91sam_defconfig
@@ -24,6 +24,7 @@ CONFIG_NAND_BOOT=y
  CONFIG_BOOTDELAY=3


Please see below...


  CONFIG_USE_BOOTARGS=y
  CONFIG_BOOTARGS="console=ttyS0,115200 earlyprintk 
mtdparts=atmel_nand:256k(bootstrap)ro,768k(uboot)ro,256k(env_redundant),256k(env),512k(dtb),6M(kernel)ro,-(rootfs)
 rootfstype=ubifs ubi.mtd=6 root=ubi0:rootfs rw"
+CONFIG_BOOTDELAY=0


I'm wondering if this defconfig was generated via make savedefconfig? As
there are multiple configurations for BOOTDELAY. Could you please
double-check again?

Other than that:

Reviewed-by: Stefan Roese 

Thanks,
Stefan


  CONFIG_SYS_CONSOLE_IS_IN_ENV=y
  CONFIG_SYS_CONSOLE_INFO_QUIET=y
  CONFIG_SPL_SYS_MALLOC_SIMPLE=y
@@ -46,8 +47,8 @@ CONFIG_CMD_CACHE=y
  CONFIG_CMD_TIME=y
  CONFIG_CMD_FAT=y
  CONFIG_CMD_MTDPARTS=y
-CONFIG_MTDIDS_DEFAULT="nand0=nand0"
-CONFIG_MTDPARTS_DEFAULT="nand0:1536k(uboot),1024k(unused),512k(dtb_old),4608k(kernel_old),86528k(ubi),-(rootfs_old)"
+CONFIG_MTDIDS_DEFAULT="nand0=atmel_nand"
+CONFIG_MTDPARTS_DEFAULT="atmel_nand:1536k(uboot),10752k(unused),-(ubi)"
  CONFIG_CMD_UBI=y
  CONFIG_OF_CONTROL=y
  CONFIG_SPL_OF_CONTROL=y
@@ -55,8 +56,8 @@ CONFIG_OF_SPL_REMOVE_PROPS="pinctrl-0 pinctrl-names clocks 
clock-names interrupt
  CONFIG_ENV_IS_IN_UBI=y
  CONFIG_SYS_REDUNDAND_ENVIRONMENT=y
  CONFIG_ENV_UBI_PART="ubi"
-CONFIG_ENV_UBI_VOLUME="env"
-CONFIG_ENV_UBI_VOLUME_REDUND="env_r"
+CONFIG_ENV_UBI_VOLUME="uboot_env0"
+CONFIG_ENV_UBI_VOLUME_REDUND="uboot_env1"
  CONFIG_SYS_RELOC_GD_ENV_ADDR=y
  CONFIG_NET_RANDOM_ETHADDR=y
  CONFIG_DM=y




Viele Grüße,
Stefan

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


Please pull u-boot-x86

2021-04-14 Thread Bin Meng
Hi Tom,

This PR includes the following changes for v2021.07:

- Minor fix to Apollo Lake devicetree bindings for FSP
- Refactor Designware PCIe drivers to core and SoC parts
- Add Amlogic Meson Designware PCIe controller driver

Azure results: PASS
https://dev.azure.com/bmeng/GitHub/_build/results?buildId=346=results

The following changes since commit a94ab561e2f49a80d8579930e840b810ab1a1330:

  Merge branch '2021-04-13-assorted-improvements' (2021-04-13 09:50:45 -0400)

are available in the Git repository at:

  https://source.denx.de/u-boot/custodians/u-boot-x86

for you to fetch changes up to 2c32c701ea6cc04881589c09cc229ee77acae723:

  pci: add Amlogic Meson Designware PCIe controller (2021-04-15 10:43:17 +0800)


Neil Armstrong (4):
  pci: add common Designware PCIe functions
  pci: pcie_dw_ti: migrate to common Designware PCIe functions
  pci: pcie_dw_rockchip: migrate to common Designware PCIe functions
  pci: add Amlogic Meson Designware PCIe controller

Wolfgang Wallner (2):
  dt-bindings: fsp: Fix Apollo Lake FSP-S devicetree bindings
  x86: mtrr: Fix function descriptions

 arch/x86/include/asm/mtrr.h|   4 +-
 doc/device-tree-bindings/fsp/fsp2/apollolake/fsp-s.txt |  19 ++---
 drivers/pci/Kconfig|  16 +++-
 drivers/pci/Makefile   |   2 +
 drivers/pci/pcie_dw_common.c   | 365
+++
 drivers/pci/pcie_dw_common.h   | 155

 drivers/pci/pcie_dw_meson.c| 459
+
 drivers/pci/pcie_dw_rockchip.c | 472
+++-
 drivers/pci/pcie_dw_ti.c   | 444
+
 9 files changed, 1073 insertions(+), 863 deletions(-)
 create mode 100644 drivers/pci/pcie_dw_common.c
 create mode 100644 drivers/pci/pcie_dw_common.h
 create mode 100644 drivers/pci/pcie_dw_meson.c

Regards,
Bin


Re: [PATCH RFT v2 0/4] pci: add common Designware PCIe functions and support Amlogic Meson PCIe controller

2021-04-14 Thread Bin Meng
On Thu, Apr 15, 2021 at 6:24 AM Bin Meng  wrote:
>
> Hi,
>
> On Thu, Apr 8, 2021 at 3:41 PM Neil Armstrong  wrote:
> >
> > On 08/04/2021 07:29, Bin Meng wrote:
> > > Hi Neil,
> > >
> > > On Tue, Apr 6, 2021 at 8:22 PM Neil Armstrong  
> > > wrote:
> > >>
> > >> Hi Bin,
> > >>
> > >> On 25/03/2021 15:49, Neil Armstrong wrote:
> > >>> With the introduction of pcie_dw_rockchip, and need to support the DW 
> > >>> PCIe in the
> > >>> Amlogic AXG & G12 SoCs, most of the DW PCIe helpers would be duplicated.
> > >>>
> > >>> This introduce a "common" DW PCIe helpers file with common code merged 
> > >>> from the
> > >>> dw_ti and dw_rockchip drivers and adapted to fit with the upcoming 
> > >>> dw_meson.
> > >>>
> > >>> The following changes will switch the dw_ti and dw_rockchip, and 
> > >>> introduce a new
> > >>> driver to support the Amlogic AXG & G12 SoCs using these new common 
> > >>> helpers.
> > >>>
> > >>> The dw_meson has been validated, but the dw_ti and dw_rockchip would 
> > >>> need testing
> > >>> on hardware to validate nothing has been broken.
> > >>
> > >> How should be proceed ? I can't possibly test patches 2 & 3, but Green 
> > >> has tested it
> > >> on Sifive, should patch 1 & 4 be merged then we should wait feedback 
> > >> from the Ti & Rockchip
> > >> owners ?
> > >
> > > This series is not assigned to me in patchwork so I cannot decide 
> > > anything :)
> >
> > I can switch them to you, but I never know who to assign for these 
> > multi-vendor series...
> >
> > >
> > > But my opinion was that let's wait for some more time, and if nobody
> > > jumps out let's merge this series.
> >
> > Fine fore me !
>
> I am going to apply this series via the x86 tree if there are no
> further comments.

series applied to u-boot-x86, thanks!


Re: [PATCH 03/13] dm: pci: Skip setting VGA bridge bits if parent device is the host bus

2021-04-14 Thread Masami Hiramatsu
Hi Bin,

On Thu, 15 Apr 2021 06:30:27 +0800
Bin Meng  wrote:

> Hi,
> 
> On Thu, Apr 15, 2021 at 3:39 AM Simon Glass  wrote:
> >
> > On Tue, 13 Apr 2021 at 16:23, Masami Hiramatsu
> >  wrote:
> > >
> > > Commit bbbcb5262839 ("dm: pci: Enable VGA address forwarding on bridges")
> > > sets the VGA bridge bits by checking pplat->class, but if the parent
> > > device is the pci host bus device, it can be skipped. Moreover, it
> > > shouldn't access the pplat because the parent has different plat data.
> > >
> > > Without this fix, "pci enum" command cause a synchronous abort.
> > >
> > > pci_auto_config_devices: start
> > > PCI Autoconfig: Bus Memory region: [7800-7fff],
> > > Physical Memory [7800-7fffx]
> > > PCI Autoconfig: Bus I/O region: [0-],
> > > Physical Memory [77f0-77f0x]
> > > pci_auto_config_devices: device pci_6:0.0
> > > PCI Autoconfig: BAR 0, Mem, size=0x100, address=0x7800 
> > > bus_lower=0x7900
> > >
> > > PCI Autoconfig: BAR 1, Mem, size=0x800, No room in resource, avail 
> > > start=7900 / size=800, need=800
> > > PCI: Failed autoconfig bar 14
> > >
> > > PCI Autoconfig: BAR 2, I/O, size=0x4, address=0x1000 bus_lower=0x1004
> > >
> > > PCI Autoconfig: BAR 3, Mem, size=0x200, address=0x7a00 
> > > bus_lower=0x7c00
> > >
> > > PCI Autoconfig: BAR 4, I/O, size=0x80, address=0x1080 bus_lower=0x1100
> > >
> > > PCI Autoconfig: ROM, size=0x8, address=0x7c00 bus_lower=0x7c08
> > >
> > > "Synchronous Abort" handler, esr 0x9606
> > > elr: e002bd28 lr : e002bce8 (reloc)
> > > elr: fff6fd28 lr : fff6fce8
> > > x0 : 1041 x1 : 003e
> > > x2 : ffb0f8c8 x3 : 0001
> > > x4 : 0080 x5 : 
> > > x6 : fff718fc x7 : 000f
> > > x8 : ffb0f238 x9 : 0008
> > > x10:  x11: 0010
> > > x12: 0006 x13: 0001869f
> > > x14: ffb0fcd0 x15: 0020
> > > x16: fff71cc4 x17: 
> > > x18: ffb13d90 x19: ffb14320
> > > x20:  x21: ffb14090
> > > x22: ffb0f8c8 x23: 0001
> > > x24: ffb14c10 x25: 
> > > x26:  x27: 
> > > x28: ffb14c70 x29: ffb0f830
> > >
> > > Code: 52800843 52800061 52800e00 97ffcf65 (b9400280)
> > > Resetting CPU ...
> > >
> > > Signed-off-by: Masami Hiramatsu 
> > > ---
> > >  drivers/pci/pci-uclass.c |3 +++
> > >  1 file changed, 3 insertions(+)
> >
> > Reviewed-by: Simon Glass 
> 
> I can't find this patch in my inbox, nor in the patchwork.

Hmm, it is strange... I set up gitconfig but it seems not working well.
Let me send patch via my MUA next time.

> Could you please resend?

OK, I attached it to this mail. (maybe it is safer in this case)

Thank you,


-- 
Linaro 


dm-pci-skip-setting-vga-bridge
Description: Binary data


Re: [PATCH 1/8] ARM: dragonboard410c: Enable DM_ETH

2021-04-14 Thread Ramon Fried
On Sat, Apr 3, 2021 at 12:48 AM Peter Robinson  wrote:
>
> The dragonboard410c only uses USB ethernet interfaces so just
> enable DM_ETH.
>
> Signed-off-by: Peter Robinson 
> Cc: Ramon Fried 
> ---
>  configs/dragonboard410c_defconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/configs/dragonboard410c_defconfig 
> b/configs/dragonboard410c_defconfig
> index f5db4720b7..9c4311aa45 100644
> --- a/configs/dragonboard410c_defconfig
> +++ b/configs/dragonboard410c_defconfig
> @@ -33,6 +33,7 @@ CONFIG_FASTBOOT_FLASH=y
>  CONFIG_FASTBOOT_FLASH_MMC_DEV=0
>  CONFIG_MSM_GPIO=y
>  CONFIG_PM8916_GPIO=y
> +CONFIG_DM_ETH=y
>  CONFIG_LED=y
>  CONFIG_LED_GPIO=y
>  CONFIG_DM_MMC=y
> --
> 2.31.1
>
Acked-by: Ramon Fried 


[PATCH v2] checkpatch: Ignore ENOSYS warnings

2021-04-14 Thread Sean Anderson
There are no system calls in U-Boot, but ENOSYS is still allowed (and
preferred since 42a2668743 ("dm: core: Document the common error codes")).
Silence this warning.

Signed-off-by: Sean Anderson 
Reviewed-by: Simon Glass 
---

Changes in v2:
- Removed accidentally-included tag

 .checkpatch.conf | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/.checkpatch.conf b/.checkpatch.conf
index ed0c2150ba..9e40ea060b 100644
--- a/.checkpatch.conf
+++ b/.checkpatch.conf
@@ -26,6 +26,9 @@
 # addresses are __aligned(2)".
 --ignore PREFER_ETHER_ADDR_COPY
 
+# ENOSYS is a conventionally used error, even though U-Boot lacks system calls.
+--ignore ENOSYS
+
 # A bit shorter of a description is OK with us.
 --min-conf-desc-length=2
 
-- 
2.31.0



Re: [PATCH 3/6] net: calxedagmac: Convert to DM_ETH

2021-04-14 Thread Ramon Fried
On Mon, Apr 12, 2021 at 3:05 AM Andre Przywara  wrote:
>
> To squash that nasty warning message and make better use of the newly
> gained OF_CONTROL feature, let's convert the calxedagmac driver to the
> "new" driver model.
> The conversion is pretty straight forward, mostly just adjusting the
> use of the involved data structures.
> The only actual change is the required split of the receive routine into
> a receive and free_pkt part.
> Also this allows us to get rid of the hardcoded platform information and
> explicit init calls.
>
> This also uses the opportunity to wrap the code decoding the MMIO
> register base address, to make it safe for using PHYS_64BIT later.
>
> Signed-off-by: Andre Przywara 
> ---
>  arch/arm/Kconfig |   1 +
>  board/highbank/highbank.c|  13 ---
>  configs/highbank_defconfig   |   1 +
>  drivers/net/Kconfig  |   7 ++
>  drivers/net/calxedaxgmac.c   | 192 +++
>  include/configs/highbank.h   |   2 -
>  include/netdev.h |   1 -
>  scripts/config_whitelist.txt |   1 -
>  8 files changed, 137 insertions(+), 81 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index bd6064923fe..0082d06182a 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -756,6 +756,7 @@ config ARCH_HIGHBANK
> select CLK
> select CLK_CCF
> select AHCI
> +   select DM_ETH
>
>  config ARCH_INTEGRATOR
> bool "ARM Ltd. Integrator family"
> diff --git a/board/highbank/highbank.c b/board/highbank/highbank.c
> index 2e2300a307f..0667a48965c 100644
> --- a/board/highbank/highbank.c
> +++ b/board/highbank/highbank.c
> @@ -10,7 +10,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>
> @@ -52,18 +51,6 @@ int board_init(void)
> return 0;
>  }
>
> -/* We know all the init functions have been run now */
> -int board_eth_init(struct bd_info *bis)
> -{
> -   int rc = 0;
> -
> -#ifdef CONFIG_CALXEDA_XGMAC
> -   rc += calxedaxgmac_initialize(0, 0xfff5);
> -   rc += calxedaxgmac_initialize(1, 0xfff51000);
> -#endif
> -   return rc;
> -}
> -
>  #ifdef CONFIG_SCSI_AHCI_PLAT
>  void scsi_init(void)
>  {
> diff --git a/configs/highbank_defconfig b/configs/highbank_defconfig
> index 773ed7a00bf..c3352b827d7 100644
> --- a/configs/highbank_defconfig
> +++ b/configs/highbank_defconfig
> @@ -27,3 +27,4 @@ CONFIG_SCSI=y
>  CONFIG_CONS_INDEX=0
>  CONFIG_OF_LIBFDT=y
>  CONFIG_OF_BOARD=y
> +CONFIG_CALXEDA_XGMAC=y
> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> index cf062fad4da..cebd84035c8 100644
> --- a/drivers/net/Kconfig
> +++ b/drivers/net/Kconfig
> @@ -171,6 +171,13 @@ config CORTINA_NI_ENET
>   This driver supports the Cortina-Access Ethernet MAC for
>   all supported CA SoCs.
>
> +config CALXEDA_XGMAC
> +   bool "Calxeda XGMAC support"
> +   depends on DM_ETH
> +   help
> + This driver supports the XGMAC in Calxeda Highbank and Midway
> + machines.
> +
>  config DWC_ETH_QOS
> bool "Synopsys DWC Ethernet QOS device support"
> depends on DM_ETH
> diff --git a/drivers/net/calxedaxgmac.c b/drivers/net/calxedaxgmac.c
> index 8b2ee49b441..b98d709117a 100644
> --- a/drivers/net/calxedaxgmac.c
> +++ b/drivers/net/calxedaxgmac.c
> @@ -10,6 +10,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include /* for dev_set_priv() */
>
>  #define TX_NUM_DESC1
>  #define RX_NUM_DESC32
> @@ -212,6 +214,18 @@ struct xgmac_dma_desc {
> __le32 res[3];
>  };
>
> +static struct xgmac_regs *xgmac_get_regs(struct eth_pdata *pdata)
> +{
> +   /*
> +* We use PHYS_64BIT on Highbank, so phys_addr_t is bigger than
> +* a pointer. U-Boot doesn't use LPAE (not even the MMU on highbank),
> +* so we can't access anything above 4GB.
> +* We have a check in the probe function below the ensure this,
> +* so casting to a 32-bit pointer type is fine here.
> +*/
> +   return (struct xgmac_regs *)(uintptr_t)pdata->iobase;
> +}
> +
>  /* XGMAC Descriptor Access Helpers */
>  static inline void desc_set_buf_len(struct xgmac_dma_desc *p, u32 buf_sz)
>  {
> @@ -304,8 +318,6 @@ struct calxeda_eth_dev {
>
> u32 tx_currdesc;
> u32 rx_currdesc;
> -
> -   struct eth_device *dev;
>  } __aligned(32);
>
>  /*
> @@ -313,10 +325,10 @@ struct calxeda_eth_dev {
>   * advanced descriptors.
>   */
>
> -static void init_rx_desc(struct calxeda_eth_dev *priv)
> +static void init_rx_desc(struct eth_pdata *pdata, struct calxeda_eth_dev 
> *priv)
>  {
> struct xgmac_dma_desc *rxdesc = priv->rx_chain;
> -   struct xgmac_regs *regs = (struct xgmac_regs *)priv->dev->iobase;
> +   struct xgmac_regs *regs = xgmac_get_regs(pdata);
> void *rxbuffer = priv->rxbuffer;
> int i;
>
> @@ -330,17 +342,16 @@ static void init_rx_desc(struct calxeda_eth_dev *priv)
> }
>  }
>

Re: [PATCH 0/2] net: jr2: Fix for jr2 switch

2021-04-14 Thread Ramon Fried
On Mon, Apr 12, 2021 at 12:26 AM Horatiu Vultur
 wrote:
>
> Hi,
>
> A gentle ping. Thanks.
>
> The 03/10/2021 09:31, Horatiu Vultur wrote:
> > This patch series contains two patches. The first patch resets the
> > switch at probe time while the second one fixes an issue with the
> > serdes6g configuration which is used on jr2_pcb111 board
> >
> > Horatiu Vultur (2):
> >   net: jr2: Reset switch
> >   net: jr2: Fix Serdes6G configuration
> >
> >  arch/mips/dts/mscc,jr2.dtsi   |  6 ++--
> >  drivers/net/mscc_eswitch/jr2_switch.c | 43 +++
> >  2 files changed, 42 insertions(+), 7 deletions(-)
> >
> > --
> > 2.30.1
> >
>
> --
> /Horatiu
It was reviewed. it will be picked up by Tom eventually.


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

2021-04-14 Thread Ramon Fried
On Sat, Apr 10, 2021 at 5:17 PM  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 | 37 +
>  doc/usage/arp.rst | 31 +++
>  include/net.h |  5 +
>  net/arp.c | 24 
>  net/arp.h |  4 
>  net/net.c |  8 
>  7 files changed, 115 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..56703e9641 100644
> --- a/cmd/net.c
> +++ b/cmd/net.c
> @@ -480,3 +480,40 @@ U_BOOT_CMD(
>  );
>
>  #endif  /* CONFIG_CMD_LINK_LOCAL */
> +
> +#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;
> +   }
Why do we care about that ?
> +
> +   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 /* CONFIG_CMD_ARP */
> 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
Typo, Synopsis
> +---
> +
> +::
> +
> +arp ipAddress
> +
> +Description
> +---
> +
> +The arp command is used to send ARP_REQUEST to network host and show the 
> result.
> +
> +ipAddress
> +the host ip address
> +
> +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
Typo, command
> 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,
>>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;

Re: [PATCH 13/37] net: fec_mxc: support i.MX8ULP

2021-04-14 Thread Ramon Fried
On Mon, Apr 12, 2021 at 2:53 PM Peng Fan (OSS)  wrote:
>
> From: Peng Fan 
>
> Support i.MX8ULP in fec_mxc
>
> Signed-off-by: Peng Fan 
> ---
>  drivers/net/Kconfig   | 2 +-
>  drivers/net/fec_mxc.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> index cf062fad4d..a443b499ba 100644
> --- a/drivers/net/Kconfig
> +++ b/drivers/net/Kconfig
> @@ -314,7 +314,7 @@ config FEC_MXC_MDIO_BASE
>
>  config FEC_MXC
> bool "FEC Ethernet controller"
> -   depends on MX28 || MX5 || MX6 || MX7 || IMX8 || IMX8M || VF610
> +   depends on MX28 || MX5 || MX6 || MX7 || IMX8 || IMX8M || IMX8ULP || 
> VF610
> help
>   This driver supports the 10/100 Fast Ethernet controller for
>   NXP i.MX processors.
> diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
> index ec21157d71..57ba856915 100644
> --- a/drivers/net/fec_mxc.c
> +++ b/drivers/net/fec_mxc.c
> @@ -628,7 +628,7 @@ static int fec_init(struct eth_device *dev, struct 
> bd_info *bd)
> writel(0x, >eth->gaddr2);
>
> /* Do not access reserved register */
> -   if (!is_mx6ul() && !is_mx6ull() && !is_imx8() && !is_imx8m()) {
> +   if (!is_mx6ul() && !is_mx6ull() && !is_imx8() && !is_imx8m() && 
> !is_imx8ulp()) {
> /* clear MIB RAM */
> for (i = mib_ptr; i <= mib_ptr + 0xfc; i += 4)
> writel(0, i);
> --
> 2.30.0
>
Reviewed-by: Ramon Fried 


Re: [PATCH] net: dwc_eth_qos: add support of device tree configuration for reset delay

2021-04-14 Thread Ramon Fried
On Wed, Apr 14, 2021 at 5:36 PM Marek Vasut  wrote:
>
> On 4/14/21 4:07 PM, Patrick DELAUNAY wrote:
> > Hi,
>
> Hi,
>
> > On 4/9/21 2:22 PM, Marek Vasut wrote:
> >> On 4/9/21 10:00 AM, Patrick Delaunay wrote:
> >>> The gpio reset assert/deassert delay are today harcoded in U-Boot driver
> >>> but the value should be read from DT.
> >>>
> >>> STM32 use the generic binding defined in linux:
> >>> Documentation/devicetree/bindings/net/ethernet-phy.yaml
> >>>
> >>>reset-gpios:
> >>>  maxItems: 1
> >>>  description:
> >>>The GPIO phandle and specifier for the PHY reset signal.
> >>>
> >>>reset-assert-us:
> >>>  description:
> >>>Delay after the reset was asserted in microseconds. If this
> >>>property is missing the delay will be skipped.
> >>>
> >>>reset-deassert-us:
> >>>  description:
> >>>Delay after the reset was deasserted in microseconds. If
> >>>this property is missing the delay will be skipped.
> >>>
> >>> See also U-Boot: doc/device-tree-bindings/net/phy.txt
> >>
> >> Since this is a PHY property, shouldn't that be handled in
> >> drivers/net/phy/ instead of MAC driver ?
> >
> >
> > I was my first idea but I don't found found the correct location in phy
> > (driver or uclass)
> >
> > to manage these generic property and the generic property "reset-gpios"
> > was already
> >
> > managed in eth driver, so I continue to patch the driver.
> >
> >
> > But I come back to this idea after your remark
> >
> > => in linux these property are managed in
> >
> >  drivers/net/mdio/of_mdio.c::of_mdiobus_phy_device_register
> >
> >  parse DT node and add info in mdio
> >
> >  drivers/net/phy/mdio_device.c::mdio_device_reset
> >
> >  called in  mdio_probe / mdio_remove
> >
> >
> > In my first search, I don't found the same level in the U-Boot drivers
> > in drivers/net/phy/
>
> Note that this is MDIO-wide PHY reset (e.g. you can have single reset
> line connected to multiple PHYs on single MDIO bus), this is not
> PHY-specific reset.
>
> > but I miss the uclass defined in drivers/net/eth-phy-uclass.c
> >
> >
> > Finally I think I need to manage the generic binding property
> >
> > (reset-gpios, reset-assert-us, reset-deassert-us) directly  in
> >
> > => drivers/net/mdio-uclass
> >
> >
> > The GPIO RESET will be managed in mdio  ops: pre_probe/ post_remove
> >
> > as it is done in linux
> >
> > warning: today post_remove ops don't exit in u-class.
> >
> >
> > Do you think it is the correct location ?
>
> For single-PHY reset, the correct location is in drivers/net/phy/ somewhere.
>
> > Do you think it should be a new serie (migrate the eqos property in mdio)
> >
> > after this eqos is accepted
> >
> > or I shoudl sent a new serie to replace this serie.
>
> I'll leave that decision to Ramon/Joe.
Joe, I'll leave this to you.


Re: [PATCH] ls1012a: net: pfe: remove pfe stop from bootcmd

2021-04-14 Thread Ramon Fried
On Wed, Apr 14, 2021 at 1:34 PM Mian Yousaf Kaukab  wrote:
>
> When using bootefi to boot a EFI binary, u-boot is supposed to
> provide networking service for EFI application. Currently, 'pfe stop'
> command is called from bootcmd before running bootefi. As a result
> network stops working for EFI applications and console is flooded with
> "Rx pkt not on expected port" messages.
>
> Implement board_quiesce_devices() for ls1012a boards and call
> pfe_command_stop() from it instead of calling 'pfe stop' from
> *_bootcmd and bootcmd.
>
> Tested-by: Anji Jagarlmudi 
> Signed-off-by: Mian Yousaf Kaukab 
> ---
>  board/freescale/ls1012afrdm/ls1012afrdm.c | 8 
>  board/freescale/ls1012aqds/ls1012aqds.c   | 8 
>  board/freescale/ls1012ardb/ls1012ardb.c   | 8 
>  drivers/net/pfe_eth/pfe_cmd.c | 2 +-
>  include/configs/ls1012a2g5rdb.h   | 6 +++---
>  include/configs/ls1012a_common.h  | 4 ++--
>  include/configs/ls1012afrdm.h | 6 +++---
>  include/configs/ls1012afrwy.h | 6 +++---
>  include/configs/ls1012aqds.h  | 6 +++---
>  include/configs/ls1012ardb.h  | 6 +++---
>  include/net/pfe_eth/pfe/pfe_hw.h  | 6 ++
>  11 files changed, 48 insertions(+), 18 deletions(-)
>
> diff --git a/board/freescale/ls1012afrdm/ls1012afrdm.c 
> b/board/freescale/ls1012afrdm/ls1012afrdm.c
> index 2cd651b943fb..7e87e5a9846f 100644
> --- a/board/freescale/ls1012afrdm/ls1012afrdm.c
> +++ b/board/freescale/ls1012afrdm/ls1012afrdm.c
> @@ -23,6 +23,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> @@ -185,6 +186,13 @@ int board_init(void)
> return 0;
>  }
>
> +#ifdef CONFIG_FSL_PFE
> +void board_quiesce_devices(void)
> +{
> +pfe_command_stop(0, NULL);
> +}
> +#endif
> +
>  int ft_board_setup(void *blob, struct bd_info *bd)
>  {
> arch_fixup_fdt(blob);
> diff --git a/board/freescale/ls1012aqds/ls1012aqds.c 
> b/board/freescale/ls1012aqds/ls1012aqds.c
> index cfe3f3360cd9..4f98a8652f21 100644
> --- a/board/freescale/ls1012aqds/ls1012aqds.c
> +++ b/board/freescale/ls1012aqds/ls1012aqds.c
> @@ -32,6 +32,7 @@
>  #include "../common/qixis.h"
>  #include "ls1012aqds_qixis.h"
>  #include "ls1012aqds_pfe.h"
> +#include 
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> @@ -163,6 +164,13 @@ int board_init(void)
> return 0;
>  }
>
> +#ifdef CONFIG_FSL_PFE
> +void board_quiesce_devices(void)
> +{
> +pfe_command_stop(0, NULL);
> +}
> +#endif
> +
>  int esdhc_status_fixup(void *blob, const char *compat)
>  {
> char esdhc0_path[] = "/soc/esdhc@156";
> diff --git a/board/freescale/ls1012ardb/ls1012ardb.c 
> b/board/freescale/ls1012ardb/ls1012ardb.c
> index 41bcf6f935e9..62e8af48cf14 100644
> --- a/board/freescale/ls1012ardb/ls1012ardb.c
> +++ b/board/freescale/ls1012ardb/ls1012ardb.c
> @@ -28,6 +28,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> @@ -186,6 +187,13 @@ int board_init(void)
> return 0;
>  }
>
> +#ifdef CONFIG_FSL_PFE
> +void board_quiesce_devices(void)
> +{
> +   pfe_command_stop(0, NULL);
> +}
> +#endif
> +
>  #ifdef CONFIG_TARGET_LS1012ARDB
>  int esdhc_status_fixup(void *blob, const char *compat)
>  {
> diff --git a/drivers/net/pfe_eth/pfe_cmd.c b/drivers/net/pfe_eth/pfe_cmd.c
> index 1e69525cb71f..364750f65c75 100644
> --- a/drivers/net/pfe_eth/pfe_cmd.c
> +++ b/drivers/net/pfe_eth/pfe_cmd.c
> @@ -418,7 +418,7 @@ static void send_dummy_pkt_to_hif(void)
> writel(buf, TMU_PHY_INQ_PKTINFO);
>  }
>
> -static void pfe_command_stop(int argc, char *const argv[])
> +void pfe_command_stop(int argc, char *const argv[])
>  {
> int pfe_pe_id, hif_stop_loop = 10;
> u32 rx_status;
> diff --git a/include/configs/ls1012a2g5rdb.h b/include/configs/ls1012a2g5rdb.h
> index bbc3ffd7f0d3..c55fc6487756 100644
> --- a/include/configs/ls1012a2g5rdb.h
> +++ b/include/configs/ls1012a2g5rdb.h
> @@ -79,7 +79,7 @@
> "installer=load mmc 0:2 $load_addr "\
>"/flex_installer_arm64.itb; "\
>"bootm $load_addr#$board\0"  \
> -   "qspi_bootcmd=pfe stop; echo Trying load from qspi..;"  \
> +   "qspi_bootcmd=echo Trying load from qspi..;"\
> "sf probe && sf read $load_addr "   \
> "$kernel_addr $kernel_size; env exists secureboot " \
> "&& sf read $kernelheader_addr_r $kernelheader_addr "   \
> @@ -89,11 +89,11 @@
>  #undef CONFIG_BOOTCOMMAND
>  #ifdef CONFIG_TFABOOT
>  #undef QSPI_NOR_BOOTCOMMAND
> -#define QSPI_NOR_BOOTCOMMAND "pfe stop;run distro_bootcmd; run qspi_bootcmd; 
> " \
> +#define QSPI_NOR_BOOTCOMMAND "run distro_bootcmd; run qspi_bootcmd; " \
>  "env exists secureboot && esbc_halt;"
>  #else
>  #if defined(CONFIG_QSPI_BOOT) || defined(CONFIG_SD_BOOT_QSPI)
> -#define CONFIG_BOOTCOMMAND "pfe stop;run distro_bootcmd; run qspi_bootcmd; " 
> \
> +#define 

Re: [PATCH v3 13/28] pci: Update to use new sequence numbers

2021-04-14 Thread Simon Glass
Hi Tim,

On Thu, 15 Apr 2021, 12:29 Tim Harvey,  wrote:

> On Wed, Apr 14, 2021 at 12:37 PM Simon Glass  wrote:
> >
> > Hi Tim,
> >
> > On Wed, 14 Apr 2021 at 06:32, Tim Harvey  wrote:
> > >
> > > On Sat, Dec 19, 2020 at 8:43 AM Simon Glass  wrote:
> > > >
> > > > Now that we know the sequence number at bind time, there is no need
> for
> > > > special-case code in dm_pci_hose_probe_bus().
> > > >
> > > > Note: the PCI_CAP_ID_EA code may need a look, but there are no test
> > > > failures so I have left it as is.
> > > >
> > > > Signed-off-by: Simon Glass 
> > > > ---
> > > >
> > > > Changes in v3:
> > > > - Update PCI to use manual sequence numbering
> > > >
> > > > Changes in v2:
> > > > - Use the sequence number directly instead of max bus
> > > >
> > > >  drivers/pci/pci-uclass.c | 45
> 
> > > >  drivers/pci/pci_auto.c   | 10 -
> > > >  2 files changed, 32 insertions(+), 23 deletions(-)
> > > >
> > > > Applied to u-boot-dm/next, thanks!
> > >
> > > Hi Simon,
> > >
> > > I have a (not yet submitted) pending patch to migrate the gwventana
> > > board to DM_PCI / DM_ETH that this particular patch broke and I'm
> > > trying to understand why.
> > >
> > > The Gateworks Ventana boards have a PCIe switch where the downstream
> > > PERST#'s are GPIO's off the switch and a e1000 PCIe GBe device is on
> > > one of those downstream ports. For non-dm PCI I have a
> > > 'board_pci_fixup_dev' function that allows board support to configure
> > > the switch's GPIO and toggle downstream PERST# during enumeration.
> > > When I add this function to dm_pci I end up getting a data abort when
> > > the e1000 driver tries to initialize (after PCI enumeration).
> >
> > Firstly, I think I did the PCI conversion about 6 years ago so it is
> > not fresh in my memory.
> >
> > >
> > > Any idea what is causing this and what I need to do to work around it?
> > > Nothing in my patch deals with device sequence numbers.
> >
> > An abort presumably indicates that the memory is not there, so perhaps
> > the e1000 is still in reset, or has been set up to appear at a
> > different address?
> >
>
> Simon,
>
> It does appear that everything downstream from the switch is
> inaccessible 'after' enumeration and I've verified that PERST# both
> upstream and downstream is in the correct state and never gets
> asserted again.
>
> The issue appears to be related to the PCI_CAP_ID_EA code and per your
> commit log it sounds like you were not sure if that was correct.
> Adding back in the following resolves the issue for me:
>
> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
> index 9abdbc54b0..fcfe520e5e 100644
> --- a/drivers/pci/pci-uclass.c
> +++ b/drivers/pci/pci-uclass.c
> @@ -646,6 +646,15 @@ int dm_pci_hose_probe_bus(struct udevice *bus)
> return log_msg_ret("probe", ret);
> }
>
> +   if (!ea_pos) {
> +   if (sub_bus != dev_seq(bus)) {
> +   debug("%s: Internal error, bus '%s' got seq
> %d, expected %d\n",
> +   __func__, bus->name,
> dev_seq(bus), sub_bus);
> +   return -EPIPE;
> +   }
> +   sub_bus = pci_get_bus_max();
> +   }
> +
> dm_pciauto_postscan_setup_bridge(bus, sub_bus);
>
> return sub_bus;
>
> Adding some debugging it appears that the devices I have downstream
> from the bridge have 'ea_os = 0'.
>
> Does the above let you know what's going on?
>

Maybe,but I think someone just sent a patch for it.can you check the
mailing list?

Regards,
Simon


> Tim
>
>
>
>
>
> > You should be abe to use 'pci hdr 0.4.0' or whatever to look at the
> > BARs and make sure they are value with 'md', etc.
> >
> > > Additionally I feel like the best way to add support for the custom
> > > downstream PCI switch reset requirements is to add a UCLASS_PCI driver
> > > for the switch in my board support but when I attempted that solution
> > > I run into an issue where pci read/write's cause a prefetch abort
> > > because I need to use imx_pcie_dm_{read,write}_config instead of the
> > > default ops. I'm not quite sure how to get hold of the ops for the imx
> >
> > If you write a PCI driver then you can provide the access operations
> > yourselfsee drivers/pci for some examples. I suggest you add
> > subnodes to the devicetree and specify the compatible string of your
> > PCI switch so it picks up the right driver.
> >
> > > controller to set this up properly. Furthermore if I do end up with
> > > that solution I later need to be able to walk the PCI bus to perform
> > > various dt fixups on pci device nodes - do you have an example of how
> > > I could walk the bus using DM_PCI?
> >
> > Do you mean a breadth-first search? It is something like:
> >
> > struct udevice *dev;
> >
> > static void do_something(struct udevice *parent)
> > {
> >device_foreach_child_probe(dev, parent) {
> >do_something((dev);
> >}
> >
> > 

Re: [PATCH v3 13/28] pci: Update to use new sequence numbers

2021-04-14 Thread Tim Harvey
On Wed, Apr 14, 2021 at 12:37 PM Simon Glass  wrote:
>
> Hi Tim,
>
> On Wed, 14 Apr 2021 at 06:32, Tim Harvey  wrote:
> >
> > On Sat, Dec 19, 2020 at 8:43 AM Simon Glass  wrote:
> > >
> > > Now that we know the sequence number at bind time, there is no need for
> > > special-case code in dm_pci_hose_probe_bus().
> > >
> > > Note: the PCI_CAP_ID_EA code may need a look, but there are no test
> > > failures so I have left it as is.
> > >
> > > Signed-off-by: Simon Glass 
> > > ---
> > >
> > > Changes in v3:
> > > - Update PCI to use manual sequence numbering
> > >
> > > Changes in v2:
> > > - Use the sequence number directly instead of max bus
> > >
> > >  drivers/pci/pci-uclass.c | 45 
> > >  drivers/pci/pci_auto.c   | 10 -
> > >  2 files changed, 32 insertions(+), 23 deletions(-)
> > >
> > > Applied to u-boot-dm/next, thanks!
> >
> > Hi Simon,
> >
> > I have a (not yet submitted) pending patch to migrate the gwventana
> > board to DM_PCI / DM_ETH that this particular patch broke and I'm
> > trying to understand why.
> >
> > The Gateworks Ventana boards have a PCIe switch where the downstream
> > PERST#'s are GPIO's off the switch and a e1000 PCIe GBe device is on
> > one of those downstream ports. For non-dm PCI I have a
> > 'board_pci_fixup_dev' function that allows board support to configure
> > the switch's GPIO and toggle downstream PERST# during enumeration.
> > When I add this function to dm_pci I end up getting a data abort when
> > the e1000 driver tries to initialize (after PCI enumeration).
>
> Firstly, I think I did the PCI conversion about 6 years ago so it is
> not fresh in my memory.
>
> >
> > Any idea what is causing this and what I need to do to work around it?
> > Nothing in my patch deals with device sequence numbers.
>
> An abort presumably indicates that the memory is not there, so perhaps
> the e1000 is still in reset, or has been set up to appear at a
> different address?
>

Simon,

It does appear that everything downstream from the switch is
inaccessible 'after' enumeration and I've verified that PERST# both
upstream and downstream is in the correct state and never gets
asserted again.

The issue appears to be related to the PCI_CAP_ID_EA code and per your
commit log it sounds like you were not sure if that was correct.
Adding back in the following resolves the issue for me:

diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
index 9abdbc54b0..fcfe520e5e 100644
--- a/drivers/pci/pci-uclass.c
+++ b/drivers/pci/pci-uclass.c
@@ -646,6 +646,15 @@ int dm_pci_hose_probe_bus(struct udevice *bus)
return log_msg_ret("probe", ret);
}

+   if (!ea_pos) {
+   if (sub_bus != dev_seq(bus)) {
+   debug("%s: Internal error, bus '%s' got seq
%d, expected %d\n",
+   __func__, bus->name,
dev_seq(bus), sub_bus);
+   return -EPIPE;
+   }
+   sub_bus = pci_get_bus_max();
+   }
+
dm_pciauto_postscan_setup_bridge(bus, sub_bus);

return sub_bus;

Adding some debugging it appears that the devices I have downstream
from the bridge have 'ea_os = 0'.

Does the above let you know what's going on?

Tim





> You should be abe to use 'pci hdr 0.4.0' or whatever to look at the
> BARs and make sure they are value with 'md', etc.
>
> > Additionally I feel like the best way to add support for the custom
> > downstream PCI switch reset requirements is to add a UCLASS_PCI driver
> > for the switch in my board support but when I attempted that solution
> > I run into an issue where pci read/write's cause a prefetch abort
> > because I need to use imx_pcie_dm_{read,write}_config instead of the
> > default ops. I'm not quite sure how to get hold of the ops for the imx
>
> If you write a PCI driver then you can provide the access operations
> yourselfsee drivers/pci for some examples. I suggest you add
> subnodes to the devicetree and specify the compatible string of your
> PCI switch so it picks up the right driver.
>
> > controller to set this up properly. Furthermore if I do end up with
> > that solution I later need to be able to walk the PCI bus to perform
> > various dt fixups on pci device nodes - do you have an example of how
> > I could walk the bus using DM_PCI?
>
> Do you mean a breadth-first search? It is something like:
>
> struct udevice *dev;
>
> static void do_something(struct udevice *parent)
> {
>device_foreach_child_probe(dev, parent) {
>do_something((dev);
>}
>
> ...
>
> struct udevice *bus;
> int  ret;
>
> ret = uclass_first_device_err(UCLASS_PCI, );
> if (ret)
>return log_msg_ret("pci", ret);
> do_something(bus);
>
> >
> > Best regards,
> >
> > Tim
>
> Regards,
> Simon


[PATCH] arm: at91: gardena-smart-gateway-at91sam: Adjust to production values

2021-04-14 Thread Reto Schneider
From: Reto Schneider 

This commit updates the default config with the values that are actually
used "in the wild" and which are close to what is used on the MediaTek
MT7688 based, 2nd generation of the GARDENA smart gateway:
 - Reduce startup time by setting bootdelay to 0 (still allows accessing
   the shell, one just has to send a key press quicker)
 - Adjusting U-Boot environment volume names and MTD partitions to
   the actual layout

Signed-off-by: Reto Schneider 
---

 configs/gardena-smart-gateway-at91sam_defconfig | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/configs/gardena-smart-gateway-at91sam_defconfig 
b/configs/gardena-smart-gateway-at91sam_defconfig
index e3d5bc47d6..76a1c42ce0 100644
--- a/configs/gardena-smart-gateway-at91sam_defconfig
+++ b/configs/gardena-smart-gateway-at91sam_defconfig
@@ -24,6 +24,7 @@ CONFIG_NAND_BOOT=y
 CONFIG_BOOTDELAY=3
 CONFIG_USE_BOOTARGS=y
 CONFIG_BOOTARGS="console=ttyS0,115200 earlyprintk 
mtdparts=atmel_nand:256k(bootstrap)ro,768k(uboot)ro,256k(env_redundant),256k(env),512k(dtb),6M(kernel)ro,-(rootfs)
 rootfstype=ubifs ubi.mtd=6 root=ubi0:rootfs rw"
+CONFIG_BOOTDELAY=0
 CONFIG_SYS_CONSOLE_IS_IN_ENV=y
 CONFIG_SYS_CONSOLE_INFO_QUIET=y
 CONFIG_SPL_SYS_MALLOC_SIMPLE=y
@@ -46,8 +47,8 @@ CONFIG_CMD_CACHE=y
 CONFIG_CMD_TIME=y
 CONFIG_CMD_FAT=y
 CONFIG_CMD_MTDPARTS=y
-CONFIG_MTDIDS_DEFAULT="nand0=nand0"
-CONFIG_MTDPARTS_DEFAULT="nand0:1536k(uboot),1024k(unused),512k(dtb_old),4608k(kernel_old),86528k(ubi),-(rootfs_old)"
+CONFIG_MTDIDS_DEFAULT="nand0=atmel_nand"
+CONFIG_MTDPARTS_DEFAULT="atmel_nand:1536k(uboot),10752k(unused),-(ubi)"
 CONFIG_CMD_UBI=y
 CONFIG_OF_CONTROL=y
 CONFIG_SPL_OF_CONTROL=y
@@ -55,8 +56,8 @@ CONFIG_OF_SPL_REMOVE_PROPS="pinctrl-0 pinctrl-names clocks 
clock-names interrupt
 CONFIG_ENV_IS_IN_UBI=y
 CONFIG_SYS_REDUNDAND_ENVIRONMENT=y
 CONFIG_ENV_UBI_PART="ubi"
-CONFIG_ENV_UBI_VOLUME="env"
-CONFIG_ENV_UBI_VOLUME_REDUND="env_r"
+CONFIG_ENV_UBI_VOLUME="uboot_env0"
+CONFIG_ENV_UBI_VOLUME_REDUND="uboot_env1"
 CONFIG_SYS_RELOC_GD_ENV_ADDR=y
 CONFIG_NET_RANDOM_ETHADDR=y
 CONFIG_DM=y
-- 
2.29.2



Re: [PATCH 03/13] dm: pci: Skip setting VGA bridge bits if parent device is the host bus

2021-04-14 Thread Bin Meng
Hi,

On Thu, Apr 15, 2021 at 3:39 AM Simon Glass  wrote:
>
> On Tue, 13 Apr 2021 at 16:23, Masami Hiramatsu
>  wrote:
> >
> > Commit bbbcb5262839 ("dm: pci: Enable VGA address forwarding on bridges")
> > sets the VGA bridge bits by checking pplat->class, but if the parent
> > device is the pci host bus device, it can be skipped. Moreover, it
> > shouldn't access the pplat because the parent has different plat data.
> >
> > Without this fix, "pci enum" command cause a synchronous abort.
> >
> > pci_auto_config_devices: start
> > PCI Autoconfig: Bus Memory region: [7800-7fff],
> > Physical Memory [7800-7fffx]
> > PCI Autoconfig: Bus I/O region: [0-],
> > Physical Memory [77f0-77f0x]
> > pci_auto_config_devices: device pci_6:0.0
> > PCI Autoconfig: BAR 0, Mem, size=0x100, address=0x7800 
> > bus_lower=0x7900
> >
> > PCI Autoconfig: BAR 1, Mem, size=0x800, No room in resource, avail 
> > start=7900 / size=800, need=800
> > PCI: Failed autoconfig bar 14
> >
> > PCI Autoconfig: BAR 2, I/O, size=0x4, address=0x1000 bus_lower=0x1004
> >
> > PCI Autoconfig: BAR 3, Mem, size=0x200, address=0x7a00 
> > bus_lower=0x7c00
> >
> > PCI Autoconfig: BAR 4, I/O, size=0x80, address=0x1080 bus_lower=0x1100
> >
> > PCI Autoconfig: ROM, size=0x8, address=0x7c00 bus_lower=0x7c08
> >
> > "Synchronous Abort" handler, esr 0x9606
> > elr: e002bd28 lr : e002bce8 (reloc)
> > elr: fff6fd28 lr : fff6fce8
> > x0 : 1041 x1 : 003e
> > x2 : ffb0f8c8 x3 : 0001
> > x4 : 0080 x5 : 
> > x6 : fff718fc x7 : 000f
> > x8 : ffb0f238 x9 : 0008
> > x10:  x11: 0010
> > x12: 0006 x13: 0001869f
> > x14: ffb0fcd0 x15: 0020
> > x16: fff71cc4 x17: 
> > x18: ffb13d90 x19: ffb14320
> > x20:  x21: ffb14090
> > x22: ffb0f8c8 x23: 0001
> > x24: ffb14c10 x25: 
> > x26:  x27: 
> > x28: ffb14c70 x29: ffb0f830
> >
> > Code: 52800843 52800061 52800e00 97ffcf65 (b9400280)
> > Resetting CPU ...
> >
> > Signed-off-by: Masami Hiramatsu 
> > ---
> >  drivers/pci/pci-uclass.c |3 +++
> >  1 file changed, 3 insertions(+)
>
> Reviewed-by: Simon Glass 

I can't find this patch in my inbox, nor in the patchwork.

Could you please resend?

Regards,
Bin


Re: [PATCH RFT v2 0/4] pci: add common Designware PCIe functions and support Amlogic Meson PCIe controller

2021-04-14 Thread Bin Meng
Hi,

On Thu, Apr 8, 2021 at 3:41 PM Neil Armstrong  wrote:
>
> On 08/04/2021 07:29, Bin Meng wrote:
> > Hi Neil,
> >
> > On Tue, Apr 6, 2021 at 8:22 PM Neil Armstrong  
> > wrote:
> >>
> >> Hi Bin,
> >>
> >> On 25/03/2021 15:49, Neil Armstrong wrote:
> >>> With the introduction of pcie_dw_rockchip, and need to support the DW 
> >>> PCIe in the
> >>> Amlogic AXG & G12 SoCs, most of the DW PCIe helpers would be duplicated.
> >>>
> >>> This introduce a "common" DW PCIe helpers file with common code merged 
> >>> from the
> >>> dw_ti and dw_rockchip drivers and adapted to fit with the upcoming 
> >>> dw_meson.
> >>>
> >>> The following changes will switch the dw_ti and dw_rockchip, and 
> >>> introduce a new
> >>> driver to support the Amlogic AXG & G12 SoCs using these new common 
> >>> helpers.
> >>>
> >>> The dw_meson has been validated, but the dw_ti and dw_rockchip would need 
> >>> testing
> >>> on hardware to validate nothing has been broken.
> >>
> >> How should be proceed ? I can't possibly test patches 2 & 3, but Green has 
> >> tested it
> >> on Sifive, should patch 1 & 4 be merged then we should wait feedback from 
> >> the Ti & Rockchip
> >> owners ?
> >
> > This series is not assigned to me in patchwork so I cannot decide anything 
> > :)
>
> I can switch them to you, but I never know who to assign for these 
> multi-vendor series...
>
> >
> > But my opinion was that let's wait for some more time, and if nobody
> > jumps out let's merge this series.
>
> Fine fore me !

I am going to apply this series via the x86 tree if there are no
further comments.

Regards,
Bin


Re: Getting rid of falcon mode

2021-04-14 Thread Tom Rini
On Wed, Apr 14, 2021 at 08:37:07PM +0100, Simon Glass wrote:
> Hi,
> 
> On Tue, 13 Apr 2021 at 09:32, Alex G.  wrote:
> >
> > ## Introduction
> >
> > Today we use "falcon mode" to mean "boot linux straight from SPL". This
> > designation makes sense, since falcons "fly at high speed and change
> > direction rapidly" according to Wikipedia.
> >
> > The way we implement falcon mode is to reserve two areas of storage:
> >* kernel area/partition
> >* dtb area/partition
> > By using some "special cases", and "spl export", SPL can more or less
> > figure out how to skip u-boot.
> >
> >
> > ## The plot twist
> >
> > People familiar with FIT, will have recognized that the above is
> > achievable with a very basic FIT image. With some advantages:
> >
> >  - No "special cases" in SPL code
> >  - Signed kernel images
> >  - Signed kernel devicetree
> >  - Devicetree overlays
> >  - Automatic selection of correct devicetree
> >
> >
> > ## The problems
> >
> > The advantages of FIT are not obvious by looking at SPL code. A
> > noticeable amount of SPL code is hidden under #ifdef CONFIG_SPL_OS_BOOT,
> > leading one to believe that SPL_OS_BOOT is very important. It must be
> > since it takes up so much code.
> >
> > Enabling falcon mode is not well documented, and requires a lot of trial
> > and error. I've had to define 7 macros, and one new function to get it
> > working on my board -- and vividly remember the grief. This is an
> > antiquated way of doing things, and completely ignores the u-boot
> > devicetree -- we could just as well have defined those seven values in
> > the dtb.
> >
> > SPL assumes that it must load u-boot, unless in instances under
> > CONFIG_SPL_OS_BOOT. This has cause me a huge amount of grief and
> > confusion over the past several months. I have no less than three patch
> > series trying to address shortfalls there. It's awful.
> >
> >
> > ## The proposal
> >
> > I propose we drop falcon mode support for legacy images.
> >
> >- Drop CONFIG_SPL_OS_BOOT. Support for this is implied by SPL_FIT
> >- Drop the "dtb area/partition". The dtb is derived from the FIT
> >- Drop "spl export"
> >
> >
> > How do we deal with devicetrees in the FIT then? The options are to use
> > a modified devicetree which has the desired "/chosen" node, or use DTB
> > overlays.
> >
> > What are the reasons why we shouldn't go this route?
> 
> I think this makes sense, on the basis that it is a legacy image and
> people can always use the U-Boot path if needed.
> 
> I believe Falcon boot made a lot more sense when the cache was off.
> But at least in my experience, we were able to get through two SPLs
> and two U-Boots and boot a kernel about 800ms on a Cortex-A15, so
> Falcon mode might not be so relevant anymore, and supporting a legacy
> image seems like unnecessary complexity.

I'm curious where the end of that 800ms is, and I think we might need to
post grabserial logs so everyone is on the same page about where / when
we're at in booting.

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH 3/5] common: Rename macro appropriately

2021-04-14 Thread Simon Glass
On Mon, 12 Apr 2021 at 23:16, Steffen Jaeckel
 wrote:
>
> While doing code-review internally this got nitpicked by 2 reviewers, so
> I decided to include this here.
>
> Signed-off-by: Steffen Jaeckel 
> ---
>
>  common/autoboot.c | 18 +-
>  1 file changed, 9 insertions(+), 9 deletions(-)

Reviewed-by: Simon Glass 


Re: [PATCH 11/11] test: Add K210 PLL tests to sandbox defconfigs

2021-04-14 Thread Simon Glass
On Mon, 12 Apr 2021 at 04:58, Sean Anderson  wrote:
>
> This adds the unit test for the K210 PLL to the sandbox defconfigs.
>
> Signed-off-by: Sean Anderson 
> ---
>
>  configs/sandbox64_defconfig| 2 ++
>  configs/sandbox_defconfig  | 2 ++
>  configs/sandbox_flattree_defconfig | 2 ++
>  3 files changed, 6 insertions(+)

Reviewed-by: Simon Glass 


Re: [PATCH] dm: core: Fix uninitialized return value from dm_scan_fdt_node

2021-04-14 Thread Simon Glass
On Thu, 8 Apr 2021 at 22:15, Sean Anderson  wrote:
>
> If there are no nodes or if all nodes are disabled, this function would
> return err without setting it first. Fix this by initializing err to
> zero.
>
> Fixes: 94f7afdf7e ("dm: core: Ignore disabled devices when binding")
>
> Signed-off-by: Sean Anderson 
> ---
>
>  drivers/core/root.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Simon Glass 


Re: [PATCH v2 03/17] drivers: reset: Handle gracefully NULL pointers

2021-04-14 Thread Simon Glass
Hi Kishon,

On Mon, 5 Apr 2021 at 11:28, Kishon Vijay Abraham I  wrote:
>
> From: Jean-Jacques Hiblot 
>
> The reset framework provides devm_reset_control_get_optional()
> which can return NULL (not an error case). So all the other reset_ops
> should handle NULL gracefully. Prepare the way for a managed reset
> API by handling NULL pointers without crashing nor failing.
>
> Signed-off-by: Jean-Jacques Hiblot 
> Signed-off-by: Vignesh Raghavendra 
> Signed-off-by: Kishon Vijay Abraham I 
> ---
>  drivers/reset/reset-uclass.c | 30 +-
>  1 file changed, 17 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/reset/reset-uclass.c b/drivers/reset/reset-uclass.c
> index 071c389ca0..98304bc0ee 100644
> --- a/drivers/reset/reset-uclass.c
> +++ b/drivers/reset/reset-uclass.c
> @@ -13,9 +13,12 @@
>  #include 
>  #include 
>
> -static inline struct reset_ops *reset_dev_ops(struct udevice *dev)
> +struct reset_ops nop_reset_ops = {
> +};
> +
> +static inline struct reset_ops *reset_dev_ops(struct reset_ctl *r)
>  {
> -   return (struct reset_ops *)dev->driver->ops;
> +   return r ? (struct reset_ops *)r->dev->driver->ops : _reset_ops;

This behaviour still seems odd to me. Why do you have a reset driver
with no ops? That is not allowed.

>  }
>
>  static int reset_of_xlate_default(struct reset_ctl *reset_ctl,
> @@ -54,9 +57,10 @@ static int reset_get_by_index_tail(int ret, ofnode node,
> debug("%s %d\n", ofnode_get_name(args->node), args->args[0]);
> return ret;
> }
> -   ops = reset_dev_ops(dev_reset);
>
> reset_ctl->dev = dev_reset;
> +   ops = reset_dev_ops(reset_ctl);
> +
> if (ops->of_xlate)
> ret = ops->of_xlate(reset_ctl, args);
> else
> @@ -162,29 +166,29 @@ int reset_get_by_name(struct udevice *dev, const char 
> *name,
>
>  int reset_request(struct reset_ctl *reset_ctl)
>  {
> -   struct reset_ops *ops = reset_dev_ops(reset_ctl->dev);
> +   struct reset_ops *ops = reset_dev_ops(reset_ctl);
>
> debug("%s(reset_ctl=%p)\n", __func__, reset_ctl);
>
> -   return ops->request(reset_ctl);
> +   return ops->request ? ops->request(reset_ctl) : 0;

Can you check this first and return -ENOSYS ? E.g.

if (!ops->request)
   return -ENOSYS;

return ops->request(reset_ctl);

Same below

>  }
>
>  int reset_free(struct reset_ctl *reset_ctl)
>  {
> -   struct reset_ops *ops = reset_dev_ops(reset_ctl->dev);
> +   struct reset_ops *ops = reset_dev_ops(reset_ctl);
>
> debug("%s(reset_ctl=%p)\n", __func__, reset_ctl);
>
> -   return ops->rfree(reset_ctl);
> +   return ops->rfree ? ops->rfree(reset_ctl) : 0;
>  }
>
>  int reset_assert(struct reset_ctl *reset_ctl)
>  {
> -   struct reset_ops *ops = reset_dev_ops(reset_ctl->dev);
> +   struct reset_ops *ops = reset_dev_ops(reset_ctl);
>
> debug("%s(reset_ctl=%p)\n", __func__, reset_ctl);
>
> -   return ops->rst_assert(reset_ctl);
> +   return ops->rst_assert ? ops->rst_assert(reset_ctl) : 0;
>  }
>
>  int reset_assert_bulk(struct reset_ctl_bulk *bulk)
> @@ -202,11 +206,11 @@ int reset_assert_bulk(struct reset_ctl_bulk *bulk)
>
>  int reset_deassert(struct reset_ctl *reset_ctl)
>  {
> -   struct reset_ops *ops = reset_dev_ops(reset_ctl->dev);
> +   struct reset_ops *ops = reset_dev_ops(reset_ctl);
>
> debug("%s(reset_ctl=%p)\n", __func__, reset_ctl);
>
> -   return ops->rst_deassert(reset_ctl);
> +   return ops->rst_deassert ? ops->rst_deassert(reset_ctl) : 0;
>  }
>
>  int reset_deassert_bulk(struct reset_ctl_bulk *bulk)
> @@ -224,11 +228,11 @@ int reset_deassert_bulk(struct reset_ctl_bulk *bulk)
>
>  int reset_status(struct reset_ctl *reset_ctl)
>  {
> -   struct reset_ops *ops = reset_dev_ops(reset_ctl->dev);
> +   struct reset_ops *ops = reset_dev_ops(reset_ctl);
>
> debug("%s(reset_ctl=%p)\n", __func__, reset_ctl);
>
> -   return ops->rst_status(reset_ctl);
> +   return ops->rst_status ? ops->rst_status(reset_ctl) : 0;
>  }
>
>  int reset_release_all(struct reset_ctl *reset_ctl, int count)
> --
> 2.17.1
>

Regards,
Simon


Re: [PATCH v10 2/3] x86: correct usage of CFLAGS_NON_EFI

2021-04-14 Thread Simon Glass
Hi Heinrich,

On Sun, 11 Apr 2021 at 10:23, Heinrich Schuchardt  wrote:
>
> The current usage of the variable CFLAGS_NON_EFI on the x86 architecture
> deviates from other architectures.
>
> Variable CFLAGS_NON_EFI is the list of compiler flags to be removed when
> building UEFI applications. It is not a list of flags to be added anywhere.
>
> Signed-off-by: Heinrich Schuchardt 
> ---
> v10:
> new patch
> ---
>  arch/x86/config.mk | 10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)

Well it used to be the flags to add when not building EFI apps... I
suppose it doesn't matter so long as it works.

Regards,
Simon


Re: [PATCH v10 1/3] test: fix test/dm/regmap.c

2021-04-14 Thread Simon Glass
On Sun, 11 Apr 2021 at 10:23, Heinrich Schuchardt  wrote:
>
> regmap_read() only fills the first two bytes of val. The last two bytes are
> random data from the stack. This means the test will fail randomly.
>
> For low endian systems we could simply initialize val to 0 and get correct
> results. But tests should not depend on endianness. So let's use a pointer
> conversion instead.
>
> Signed-off-by: Heinrich Schuchardt 
> ---
> v10:
> new patch
> ---
>  test/dm/regmap.c | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)

Reviewed-by: Simon Glass 


Re: [PATCH 2/6] spl: LOAD_FIT_FULL: Do not hard-code os to IH_OS_U_BOOT

2021-04-14 Thread Simon Glass
On Tue, 30 Mar 2021 at 18:54, Alex G.  wrote:
>
>
>
> On 3/29/21 2:43 AM, Simon Glass wrote:
> > On Fri, 12 Mar 2021 at 10:32, Alexandru Gagniuc  
> > wrote:
> >>
> >> The information on the OS should be contained in the FIT, as the
> >> self-explanatory "os" property of a node under /images. Hard-coding
> >> this to U_BOOT might send us down the wrong path later in the boot
> >> process.
> >>
> >> Signed-off-by: Alexandru Gagniuc 
> >> ---
> >>   common/spl/spl.c | 5 +++--
> >>   1 file changed, 3 insertions(+), 2 deletions(-)
> >>

Reviewed-by: Simon Glass 


Re: [PATCH v1 1/2] cmd: bind: Fix driver binding on a device

2021-04-14 Thread Simon Glass
On Fri, 9 Apr 2021 at 08:36, Patrice Chotard
 wrote:
>
> Fix a regression brings by commit 84f8e36f03fa ("cmd: bind: allow to
> bind driver with driver data")
>
> As example, the following bind command doesn't work:
>
>bind /soc/usb-otg@4900 usb_ether
>
> As usb_ether driver has no compatible string, it can't be find by
> lists_bind_fdt(). In bind_by_node_path(), which called lists_bind_fdt(),
> the driver entry is known, pass it to lists_bind_fdt() to force the driver
> entry selection.
>
> For this, add a new parameter struct *driver to lists_bind_fdt().
> Fix also all lists_bind_fdt() callers.
>
> Fixes: 84f8e36f03fa ("cmd: bind: allow to bind driver with driver data")
>
> Signed-off-by: Patrice Chotard 
> Reported-by: Herbert Poetzl 
> Cc: Marek Vasut 
> Cc: Herbert Poetzl 
> ---
>
>  cmd/bind.c |  2 +-
>  drivers/core/device.c  |  2 +-
>  drivers/core/lists.c   | 11 ---
>  drivers/core/root.c|  2 +-
>  drivers/misc/imx8/scu.c|  2 +-
>  drivers/serial/serial-uclass.c |  2 +-
>  drivers/timer/timer-uclass.c   |  2 +-
>  include/dm/lists.h |  3 ++-
>  test/dm/nop.c  |  2 +-
>  test/dm/test-fdt.c |  2 +-
>  10 files changed, 18 insertions(+), 12 deletions(-)
>

Reviewed-by: Simon Glass 

Really this command needs a test.

Regards,
Simon


Re: MXC I2C recover/idle_bus does not work if CONFIG_DM_I2C is configured

2021-04-14 Thread Simon Glass
Hi Kees,

On Fri, 9 Apr 2021 at 08:00, Trommel, Kees (Contractor)
 wrote:
>
> Simon,
>
> I found this issue in 2020.10. I just checked the master version and found 
> that the "flags |= desc->flags" statement in dm_gpio_set_dir_flags has been 
> removed. So it looks like that this issue is already solved.

Yes I believe so.


- Simon

>
> Kees.
>
> -Original Message-
> From: Heiko Schocher 
> Sent: 09 April 2021 06:40
> To: Trommel, Kees (Contractor) 
> Cc: u-boot@lists.denx.de; Simon Glass 
> Subject: Re: MXC I2C recover/idle_bus does not work if CONFIG_DM_I2C is 
> configured
>
> Hello Kees,
>
> added Simon to cc...
>
> On 08.04.21 14:20, Trommel, Kees (Contractor) wrote:
> > Heiko,
> >
> > When an I2C transaction fails because a previous transaction (by the
> > kernel) was aborted halfway the MXC I2C driver tries to recover from
> > this by calling i2c_idle_bus (if CONFIG_DM_I2C is configured).
> > i2c_idle_bus is defined in drivers/i2c/mxc_i2c.c
>
> Ok, so imx based hardware.
>
> > As part of the recover procedure i2c_idle_bus (tries) to change the 
> > direction of I2C GPIO pins from output to input using the function 
> > dm_gpio_set_dir_flags. However dm_gpio_set_dir_flags only sets flags 
> > without clearing any previously set flags, see statement "flags |= 
> > desc->flags" in dm_gpio_set_dir_flags. Because it is not allowed to set 
> > both GPIOD_IS_IN and GPIOD_IS_OUT (see function check_dir_flags) only the 
> > dm_gpio_set_dir_flags(GPIO_D_IS_OUT) calls are successful,  the 
> > dm_gpio_set_dir_flags(GPIO_D_IS_IN) calls fail and the GPIO pin is never 
> > set as an input. This causes that i2c_idle_bus finds that clearing the bus 
> > is unsuccessful and returns an error.
>
> Ok, sounds like a bug to me.
>
> > Although i2c_idle_bus returns an error the i2c recover procedure itself is 
> > successful and the next I2C transfer will be successful. This requires that 
> > the I2C application to do a retry. This will work but is not the intention 
> > of the I2C driver.
> >
> > I want to fix this problem. However it looks like that no dm_gpio API 
> > exists to change the direction of an GPIO pin while this is required to 
> > successful recover the i2c bus and detect that the recovery is successful. 
> > To fix this issue I see 2 possibilities:
> >
> > 1. Use the old fashioned APIs gpio_direction_input and
> > gpio_direction_output 2. Add a new dm_gpio API
> > dm_gpio_clear_and_set_dir_flags that allows to clear all existing set
> > flags before setting the new flag(s)
>
> Hmm.. I see, we only "or" the flags in dm_gpio_set_dir_flags() ... and than 
> check if there are conflicts ... May we rework dm_gpio_set_dir_flags() so we 
> prevent the conflicts we check in check_dir_flags() ?
>
> Assumption is, the caller of dm_gpio_set_dir_flags() knows what he do...
>
> @Simon: What do you think?
>
> > I prefer option 2 but I like to hear the opinion of the developer that 
> > designed the dm_gpio interface.
>
> I did not designed the gpio interface!
>
> bye,
> Heiko
> >
> > Kind regards,
> >
> > Kees Trommel.
> >
> > ---
> > This communication contains confidential information. If you are not the 
> > intended recipient please return this email to the sender and delete it 
> > from your records.
> >
> > Diese Nachricht enthaelt vertrauliche Informationen. Sollten Sie nicht der 
> > beabsichtigte Empfaenger dieser E-mail sein, senden Sie bitte diese an den 
> > Absender zurueck und loeschen Sie die E-mail aus Ihrem System.
> >
>
> --
> DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: h...@denx.de


Re: [PATCH 1/4] command: Use a constant pointer for the help

2021-04-14 Thread Simon Glass
Hi Sean,

On Fri, 9 Apr 2021 at 04:32, Sean Anderson  wrote:
>
> On 4/6/21 12:30 AM, Simon Glass wrote:
> > This text should never change during execution, so it makes sense to
> > use a const char * so that it can be declared as const in the code.
> > Update struct cmd_tbl with a const char * pointer for 'help'.
> >
> > Signed-off-by: Simon Glass 
> > ---
> >
> >   include/command.h | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/command.h b/include/command.h
> > index 747f8f80958..675c16a1249 100644
> > --- a/include/command.h
> > +++ b/include/command.h
> > @@ -45,7 +45,7 @@ struct cmd_tbl {
> >  char *const argv[]);
> >   char*usage; /* Usage message(short) */
>
> Should usage also be const?

I think so, but would need to check it builds, etc.

Regards,
Simon


Re: [PATCH 01/11] clk: Allow force setting clock defaults before relocation

2021-04-14 Thread Simon Glass
Hi Sean,

On Mon, 12 Apr 2021 at 04:58, Sean Anderson  wrote:
>
> Since 291da96b8e ("clk: Allow clock defaults to be set during re-reloc
> state for SPL only") it has been impossible to set clock defaults before
> relocation. This is annoying on boards without SPL, since there is no way
> to set clock defaults before U-Boot proper. In particular, the aisram rate
> must be changed before relocation on the K210, since U-Boot will hang if we
> try and change the rate while we are using aisram.
>
> To get around this, (ab)use the stage parameter to force setting defaults,
> even if they would be otherwise posponed for later. A device tree property
> was decided against because of the concerns in the original commit thread
> about the overhead of repeatedly parsing the device tree.
>
> Signed-off-by: Sean Anderson 
> ---
>
>  drivers/clk/clk-uclass.c | 9 +++--
>  include/clk.h| 4 +++-
>  2 files changed, 10 insertions(+), 3 deletions(-)
>

Reviewed-by: Simon Glass 

But I think this should be an enum.


Re: [PATCH v2 3/4] efi_capsule: Add a function to get the public key needed for capsule authentication

2021-04-14 Thread Simon Glass
On Mon, 12 Apr 2021 at 16:06, Sughosh Ganu  wrote:
>
> Define a function which would be used in the scenario where the
> public key is stored on the platform's dtb. This dtb is concatenated
> with the u-boot binary during the build process. Platforms which have
> a different mechanism for getting the public key would define their
> own platform specific function under a different Kconfig symbol.
>
> Signed-off-by: Sughosh Ganu 
> ---
>
> Changes since V1:
> * Remove the weak function, and add the functionality to retrieve the
>   public key under the config symbol CONFIG_EFI_PKEY_DTB_EMBED.
>
>  lib/efi_loader/efi_capsule.c | 43 +++-
>  1 file changed, 38 insertions(+), 5 deletions(-)

Is this defined in a header file somewhere?

>
> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> index 2cc8f2dee0..d95e9377fe 100644
> --- a/lib/efi_loader/efi_capsule.c
> +++ b/lib/efi_loader/efi_capsule.c
> @@ -14,10 +14,13 @@
>  #include 
>  #include 
>
> +#include 
>  #include 
>  #include 
>  #include 
>
> +DECLARE_GLOBAL_DATA_PTR;
> +
>  const efi_guid_t efi_guid_capsule_report = EFI_CAPSULE_REPORT_GUID;
>  static const efi_guid_t efi_guid_firmware_management_capsule_id =
> EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
> @@ -208,15 +211,45 @@ skip:
>  const efi_guid_t efi_guid_capsule_root_cert_guid =
> EFI_FIRMWARE_MANAGEMENT_CAPSULE_ID_GUID;
>
> -__weak int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len)
> +#if defined(CONFIG_EFI_PKEY_DTB_EMBED)

Can you drop the #ifdef ?

> +int efi_get_public_key_data(void **pkey, efi_uintn_t *pkey_len)

pkey should really have a time...what is it?

>  {
> -   /* The platform is supposed to provide
> -* a method for getting the public key
> -* stored in the form of efi signature
> -* list
> +   /*
> +* This is a function for retrieving the public key from the
> +* platform's device tree. The platform's device tree has been
> +* concatenated with the u-boot binary.
> +* If a platform has a different mechanism to get the public
> +* key, it can define it's own kconfig symbol and define a
> +* function to retrieve the public key
>  */
> +   const void *fdt_blob = gd->fdt_blob;
> +   const void *blob;

prop? val? It is not a DT blob

> +   const char *cnode_name = "capsule-key";
> +   const char *snode_name = "signature";

I believe these FIT things are defined in image.h

> +   int sig_node;
> +   int len;
> +
> +   sig_node = fdt_subnode_offset(fdt_blob, 0, snode_name);
> +   if (sig_node < 0) {
> +   EFI_PRINT("Unable to get signature node offset\n");
> +   return -FDT_ERR_NOTFOUND;
> +   }

Can you use the livetree API?

> +
> +   blob = fdt_getprop(fdt_blob, sig_node, cnode_name, );
> +
> +   if (!blob || len < 0) {
> +   EFI_PRINT("Unable to get capsule-key value\n");
> +   *pkey = NULL;
> +   *pkey_len = 0;
> +   return -FDT_ERR_NOTFOUND;
> +   }
> +
> +   *pkey = (void *)blob;
> +   *pkey_len = len;
> +
> return 0;
>  }
> +#endif /* CONFIG_EFI_PKEY_DTB_EMBED */
>
>  efi_status_t efi_capsule_authenticate(const void *capsule, efi_uintn_t 
> capsule_size,
>   void **image, efi_uintn_t *image_size)
> --
> 2.17.1
>

Regards,
Simon


Re: [PATCH v2 3/3] test: Add test for partitions

2021-04-14 Thread Simon Glass
Hi Sean,

On Mon, 12 Apr 2021 at 23:53, Sean Anderson  wrote:
>
> This is technically a library function, but we use MMCs for testing, so
> it is easier to do it with DM. At the moment, the only block devices in
> sandbox are MMCs (AFAIK) so we just test with those.
>
> Signed-off-by: Sean Anderson 
> ---
>
> Changes in v2:
> - New
>
>  test/dm/Makefile |  1 +
>  test/dm/part.c   | 76 
>  2 files changed, 77 insertions(+)
>  create mode 100644 test/dm/part.c
>
> diff --git a/test/dm/Makefile b/test/dm/Makefile
> index f5cc5540e8..7d017f8750 100644
> --- a/test/dm/Makefile
> +++ b/test/dm/Makefile
> @@ -98,5 +98,6 @@ endif
>  ifneq ($(CONFIG_EFI_PARTITION),)
>  obj-$(CONFIG_FASTBOOT_FLASH_MMC) += fastboot.o
>  endif
> +obj-$(CONFIG_EFI_PARTITION) += part.o
>  endif
>  endif # !SPL
> diff --git a/test/dm/part.c b/test/dm/part.c
> new file mode 100644
> index 00..051e9010b6
> --- /dev/null
> +++ b/test/dm/part.c
> @@ -0,0 +1,76 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2020 Sean Anderson 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +static int dm_test_part(struct unit_test_state *uts)
> +{
> +   char str_disk_guid[UUID_STR_LEN + 1];
> +   struct blk_desc *mmc_dev_desc;
> +   struct disk_partition part_info;
> +   struct disk_partition parts[2] = {
> +   {
> +   .start = 48, /* GPT data takes up the first 34 blocks 
> or so */
> +   .size = 1,
> +   .name = "test1",
> +   },
> +   {
> +   .start = 49,
> +   .size = 1,
> +   .name = "test2",
> +   },
> +   };
> +
> +   ut_asserteq(1, blk_get_device_by_str("mmc", "1", _dev_desc));
> +   if (CONFIG_IS_ENABLED(RANDOM_UUID)) {
> +   gen_rand_uuid_str(parts[0].uuid, UUID_STR_FORMAT_STD);
> +   gen_rand_uuid_str(parts[1].uuid, UUID_STR_FORMAT_STD);
> +   gen_rand_uuid_str(str_disk_guid, UUID_STR_FORMAT_STD);
> +   }
> +   ut_assertok(gpt_restore(mmc_dev_desc, str_disk_guid, parts,
> +   ARRAY_SIZE(parts)));
> +
> +#define test(expected, part_str, whole) \

Can this be a function instead of a macro?

> +   ut_asserteq(expected, \
> +   part_get_info_by_dev_and_name_or_num("mmc", part_str, \
> +_dev_desc, \
> +_info, whole))
> +
> +   test(-ENODEV, "", true);
> +   env_set("bootdevice", "0");
> +   test(0, "", true);
> +   env_set("bootdevice", "1");
> +   test(1, "", false);
> +   test(1, "-", false);
> +   env_set("bootdevice", "");
> +   test(-EPROTONOSUPPORT, "0", false);
> +   test(0, "0", true);
> +   test(0, ":0", true);
> +   test(0, ".0", true);
> +   test(0, ".0:0", true);
> +   test(-EINVAL, "#test1", true);
> +   test(1, "1", false);
> +   test(1, "1", true);
> +   test(-ENOENT, "1:0", false);
> +   test(0, "1:0", true);
> +   test(1, "1:1", false);
> +   test(2, "1:2", false);
> +   test(1, "1.0", false);
> +   test(0, "1.0:0", true);
> +   test(1, "1.0:1", false);
> +   test(2, "1.0:2", false);
> +   test(-EINVAL, "1#bogus", false);
> +   test(1, "1#test1", false);
> +   test(2, "1#test2", false);
> +
> +   return 0;
> +}
> +DM_TEST(dm_test_part, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
> --
> 2.25.1
>

Regards,
Simon


Re: [PATCH v2 2/3] part: Fix bogus return from part_get_info_by_dev_and_name

2021-04-14 Thread Simon Glass
On Mon, 12 Apr 2021 at 23:53, Sean Anderson  wrote:
>
> blk_get_device_by_str returns the device number on success. So we must
> check if the return was negative to determine an error.
>
> Signed-off-by: Sean Anderson 
> ---
>
> Changes in v2:
> - New
>
>  disk/part.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Reviewed-by: Simon Glass 


Re: [PATCH 03/13] dm: pci: Skip setting VGA bridge bits if parent device is the host bus

2021-04-14 Thread Simon Glass
On Tue, 13 Apr 2021 at 16:23, Masami Hiramatsu
 wrote:
>
> Commit bbbcb5262839 ("dm: pci: Enable VGA address forwarding on bridges")
> sets the VGA bridge bits by checking pplat->class, but if the parent
> device is the pci host bus device, it can be skipped. Moreover, it
> shouldn't access the pplat because the parent has different plat data.
>
> Without this fix, "pci enum" command cause a synchronous abort.
>
> pci_auto_config_devices: start
> PCI Autoconfig: Bus Memory region: [7800-7fff],
> Physical Memory [7800-7fffx]
> PCI Autoconfig: Bus I/O region: [0-],
> Physical Memory [77f0-77f0x]
> pci_auto_config_devices: device pci_6:0.0
> PCI Autoconfig: BAR 0, Mem, size=0x100, address=0x7800 
> bus_lower=0x7900
>
> PCI Autoconfig: BAR 1, Mem, size=0x800, No room in resource, avail 
> start=7900 / size=800, need=800
> PCI: Failed autoconfig bar 14
>
> PCI Autoconfig: BAR 2, I/O, size=0x4, address=0x1000 bus_lower=0x1004
>
> PCI Autoconfig: BAR 3, Mem, size=0x200, address=0x7a00 
> bus_lower=0x7c00
>
> PCI Autoconfig: BAR 4, I/O, size=0x80, address=0x1080 bus_lower=0x1100
>
> PCI Autoconfig: ROM, size=0x8, address=0x7c00 bus_lower=0x7c08
>
> "Synchronous Abort" handler, esr 0x9606
> elr: e002bd28 lr : e002bce8 (reloc)
> elr: fff6fd28 lr : fff6fce8
> x0 : 1041 x1 : 003e
> x2 : ffb0f8c8 x3 : 0001
> x4 : 0080 x5 : 
> x6 : fff718fc x7 : 000f
> x8 : ffb0f238 x9 : 0008
> x10:  x11: 0010
> x12: 0006 x13: 0001869f
> x14: ffb0fcd0 x15: 0020
> x16: fff71cc4 x17: 
> x18: ffb13d90 x19: ffb14320
> x20:  x21: ffb14090
> x22: ffb0f8c8 x23: 0001
> x24: ffb14c10 x25: 
> x26:  x27: 
> x28: ffb14c70 x29: ffb0f830
>
> Code: 52800843 52800061 52800e00 97ffcf65 (b9400280)
> Resetting CPU ...
>
> Signed-off-by: Masami Hiramatsu 
> ---
>  drivers/pci/pci-uclass.c |3 +++
>  1 file changed, 3 insertions(+)

Reviewed-by: Simon Glass 


Re: [PATCH 02/13] ata: ahci-pci: Use scsi_ops to initialize ops

2021-04-14 Thread Simon Glass
On Tue, 13 Apr 2021 at 16:22, Masami Hiramatsu
 wrote:
>
> Without this fix, scsi-scan will cause a synchronous abort
> when accessing ops->scan.
>
> Signed-off-by: Masami Hiramatsu 
> ---
>  drivers/ata/ahci-pci.c |2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Simon Glass 


Re: [PATCH 01/13] pci: Update the highest subordinate bus number for bridge setup

2021-04-14 Thread Simon Glass
On Tue, 13 Apr 2021 at 16:21, Masami Hiramatsu
 wrote:
>
> Update the highest subordinate bus number after probing the devices
> under the bus for setting up the bridge correctly.
> The commit 42f3663a3f67 ("pci: Update to use new sequence numbers")
> removed this but it is required if a PCIe bridge is under the bus.
>
> Fixes: 42f3663a3f67 ("pci: Update to use new sequence numbers")
> Signed-off-by: Masami Hiramatsu 
> ---
>  drivers/pci/pci-uclass.c |3 +++
>  1 file changed, 3 insertions(+)

Reviewed-by: Simon Glass 

Can we add a test for this?


Re: [PATCH] checkpatch: Ignore ENOSYS warnings

2021-04-14 Thread Simon Glass
Hi Sean,

On Tue, 13 Apr 2021 at 00:31, Sean Anderson  wrote:
>
> On 4/12/21 2:04 PM, Simon Glass wrote:
> > Hi Sean,
> >
> > On Mon, 12 Apr 2021 at 16:22, Sean Anderson  wrote:
> >>
> >> There are no system calls in U-Boot, but ENOSYS is still allowed (and
> >> preferred since 42a2668743 ("dm: core: Document the common error codes")).
> >> Silence this warning.
> >>
> >> Signed-off-by: Sean Anderson 
> >> Seriies-to: sjg
> >
> > This looks OK, except for that tag.
>
> Should I resend the patch?

Yes I think that would be easiest.

Reviewed-by: Simon Glass 


Re: [PATCH v2 01/17] dm: core: Add helper to compare node names

2021-04-14 Thread Simon Glass
On Mon, 5 Apr 2021 at 22:28, Kishon Vijay Abraham I  wrote:
>
> Add helper to compare node names.
>
> Signed-off-by: Kishon Vijay Abraham I 
> ---
>  drivers/core/ofnode.c | 13 +
>  include/dm/ofnode.h   | 10 ++
>  2 files changed, 23 insertions(+)
>
> diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
> index fa0bd2a9c4..adc96fcaf3 100644
> --- a/drivers/core/ofnode.c
> +++ b/drivers/core/ofnode.c
> @@ -18,6 +18,19 @@
>  #include 
>  #include 
>
> +bool ofnode_name_eq(ofnode node, const char *name)
> +{
> +   const char *node_name;
> +   size_t len;
> +
> +   assert(ofnode_valid(node));
> +
> +   node_name = ofnode_get_name(node);
> +   len = strchrnul(node_name, '@') - node_name;
> +
> +   return (strlen(name) == len) && (!strncmp(node_name, name, len));

drop extra brackets

> +}
> +
>  int ofnode_read_u32(ofnode node, const char *propname, u32 *outp)
>  {
> return ofnode_read_u32_index(node, propname, 0, outp);
> diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
> index 2c0597c407..a866b9b9b8 100644
> --- a/include/dm/ofnode.h
> +++ b/include/dm/ofnode.h
> @@ -231,6 +231,16 @@ static inline ofnode ofnode_root(void)
> return node;
>  }
>
> +/**
> + * ofnode_name_eq() - Check if the node name is equivalent to a given name
> + *ignoring the unit address
> + *
> + * @node:  valid node reference that has to be compared
> + * @name:  name that has to be compared with the node name
> + * @return 1 if matches, 0 if it doesn't match

true / false



> + */
> +bool ofnode_name_eq(ofnode node, const char *name);
> +
>  /**
>   * ofnode_read_u32() - Read a 32-bit integer from a property
>   *
> --
> 2.17.1
>


Re: [PATCH v2 02/17] dm: test: Add test case to check node name ignoring unit address

2021-04-14 Thread Simon Glass
On Mon, 5 Apr 2021 at 11:28, Kishon Vijay Abraham I  wrote:
>
> Add test to check node name ignoring unit address.
>
> Signed-off-by: Kishon Vijay Abraham I 
> ---
>  test/dm/core.c | 14 ++
>  1 file changed, 14 insertions(+)

Reviewed-by: Simon Glass 


>
> diff --git a/test/dm/core.c b/test/dm/core.c
> index 35ca689d64..b88b1966fd 100644
> --- a/test/dm/core.c
> +++ b/test/dm/core.c
> @@ -178,6 +178,20 @@ static int dm_test_autobind_uclass_pdata_alloc(struct 
> unit_test_state *uts)
>  }
>  DM_TEST(dm_test_autobind_uclass_pdata_alloc, UT_TESTF_SCAN_PDATA);
>
> +/* compare node names ignoring the unit address */
> +static int dm_test_compare_node_name(struct unit_test_state *uts)
> +{
> +   ofnode node;
> +
> +   node = ofnode_path("/mmio-bus@0");
> +   ut_assert(ofnode_valid(node));
> +   ut_assert(ofnode_name_eq(node, "mmio-bus"));
> +
> +   return 0;
> +}
> +
> +DM_TEST(dm_test_compare_node_name, UT_TESTF_SCAN_PDATA);
> +
>  /* Test that binding with uclass plat setting occurs correctly */
>  static int dm_test_autobind_uclass_pdata_valid(struct unit_test_state *uts)
>  {
> --
> 2.17.1
>


Re: Getting rid of falcon mode

2021-04-14 Thread Simon Glass
Hi,

On Tue, 13 Apr 2021 at 09:32, Alex G.  wrote:
>
> ## Introduction
>
> Today we use "falcon mode" to mean "boot linux straight from SPL". This
> designation makes sense, since falcons "fly at high speed and change
> direction rapidly" according to Wikipedia.
>
> The way we implement falcon mode is to reserve two areas of storage:
>* kernel area/partition
>* dtb area/partition
> By using some "special cases", and "spl export", SPL can more or less
> figure out how to skip u-boot.
>
>
> ## The plot twist
>
> People familiar with FIT, will have recognized that the above is
> achievable with a very basic FIT image. With some advantages:
>
>  - No "special cases" in SPL code
>  - Signed kernel images
>  - Signed kernel devicetree
>  - Devicetree overlays
>  - Automatic selection of correct devicetree
>
>
> ## The problems
>
> The advantages of FIT are not obvious by looking at SPL code. A
> noticeable amount of SPL code is hidden under #ifdef CONFIG_SPL_OS_BOOT,
> leading one to believe that SPL_OS_BOOT is very important. It must be
> since it takes up so much code.
>
> Enabling falcon mode is not well documented, and requires a lot of trial
> and error. I've had to define 7 macros, and one new function to get it
> working on my board -- and vividly remember the grief. This is an
> antiquated way of doing things, and completely ignores the u-boot
> devicetree -- we could just as well have defined those seven values in
> the dtb.
>
> SPL assumes that it must load u-boot, unless in instances under
> CONFIG_SPL_OS_BOOT. This has cause me a huge amount of grief and
> confusion over the past several months. I have no less than three patch
> series trying to address shortfalls there. It's awful.
>
>
> ## The proposal
>
> I propose we drop falcon mode support for legacy images.
>
>- Drop CONFIG_SPL_OS_BOOT. Support for this is implied by SPL_FIT
>- Drop the "dtb area/partition". The dtb is derived from the FIT
>- Drop "spl export"
>
>
> How do we deal with devicetrees in the FIT then? The options are to use
> a modified devicetree which has the desired "/chosen" node, or use DTB
> overlays.
>
> What are the reasons why we shouldn't go this route?

I think this makes sense, on the basis that it is a legacy image and
people can always use the U-Boot path if needed.

I believe Falcon boot made a lot more sense when the cache was off.
But at least in my experience, we were able to get through two SPLs
and two U-Boots and boot a kernel about 800ms on a Cortex-A15, so
Falcon mode might not be so relevant anymore, and supporting a legacy
image seems like unnecessary complexity.

Regards,
Simon


Re: [PATCH v3 13/28] pci: Update to use new sequence numbers

2021-04-14 Thread Simon Glass
Hi Tim,

On Wed, 14 Apr 2021 at 06:32, Tim Harvey  wrote:
>
> On Sat, Dec 19, 2020 at 8:43 AM Simon Glass  wrote:
> >
> > Now that we know the sequence number at bind time, there is no need for
> > special-case code in dm_pci_hose_probe_bus().
> >
> > Note: the PCI_CAP_ID_EA code may need a look, but there are no test
> > failures so I have left it as is.
> >
> > Signed-off-by: Simon Glass 
> > ---
> >
> > Changes in v3:
> > - Update PCI to use manual sequence numbering
> >
> > Changes in v2:
> > - Use the sequence number directly instead of max bus
> >
> >  drivers/pci/pci-uclass.c | 45 
> >  drivers/pci/pci_auto.c   | 10 -
> >  2 files changed, 32 insertions(+), 23 deletions(-)
> >
> > Applied to u-boot-dm/next, thanks!
>
> Hi Simon,
>
> I have a (not yet submitted) pending patch to migrate the gwventana
> board to DM_PCI / DM_ETH that this particular patch broke and I'm
> trying to understand why.
>
> The Gateworks Ventana boards have a PCIe switch where the downstream
> PERST#'s are GPIO's off the switch and a e1000 PCIe GBe device is on
> one of those downstream ports. For non-dm PCI I have a
> 'board_pci_fixup_dev' function that allows board support to configure
> the switch's GPIO and toggle downstream PERST# during enumeration.
> When I add this function to dm_pci I end up getting a data abort when
> the e1000 driver tries to initialize (after PCI enumeration).

Firstly, I think I did the PCI conversion about 6 years ago so it is
not fresh in my memory.

>
> Any idea what is causing this and what I need to do to work around it?
> Nothing in my patch deals with device sequence numbers.

An abort presumably indicates that the memory is not there, so perhaps
the e1000 is still in reset, or has been set up to appear at a
different address?

You should be abe to use 'pci hdr 0.4.0' or whatever to look at the
BARs and make sure they are value with 'md', etc.

> Additionally I feel like the best way to add support for the custom
> downstream PCI switch reset requirements is to add a UCLASS_PCI driver
> for the switch in my board support but when I attempted that solution
> I run into an issue where pci read/write's cause a prefetch abort
> because I need to use imx_pcie_dm_{read,write}_config instead of the
> default ops. I'm not quite sure how to get hold of the ops for the imx

If you write a PCI driver then you can provide the access operations
yourselfsee drivers/pci for some examples. I suggest you add
subnodes to the devicetree and specify the compatible string of your
PCI switch so it picks up the right driver.

> controller to set this up properly. Furthermore if I do end up with
> that solution I later need to be able to walk the PCI bus to perform
> various dt fixups on pci device nodes - do you have an example of how
> I could walk the bus using DM_PCI?

Do you mean a breadth-first search? It is something like:

struct udevice *dev;

static void do_something(struct udevice *parent)
{
   device_foreach_child_probe(dev, parent) {
   do_something((dev);
   }

...

struct udevice *bus;
int  ret;

ret = uclass_first_device_err(UCLASS_PCI, );
if (ret)
   return log_msg_ret("pci", ret);
do_something(bus);

>
> Best regards,
>
> Tim

Regards,
Simon


[PATCH resend 2/2] env: add CONFIG_ENV_SECT_SIZE_AUTO

2021-04-14 Thread Rasmus Villemoes
This is roughly the U-Boot side equivalent to commit
e282c422e0 (tools: fw_env: use erasesize from MEMGETINFO ioctl). The
motivation is the case where one has a board with several revisions,
where the SPI flashes have different erase sizes.

In our case, we have an 8K environment, and the flashes have erase
sizes of 4K (newer boards) and 64K (older boards). Currently, we must
set CONFIG_ENV_SECT_SIZE to 64K to make the code work on the older
boards, but for the newer ones, that ends up wasting quite a bit of
time reading/erasing/restoring the last 56K.

At first, I wanted to allow setting CONFIG_ENV_SECT_SIZE to 0 to mean
"use the erase size the chip reports", but that config
option is used in a number of preprocessor conditionals, and shared
between ENV_IS_IN_FLASH and ENV_IS_IN_SPI_FLASH.

So instead, introduce a new boolean config option, which for now can
only be used with ENV_IS_IN_SPI_FLASH. If left off, there's no change
in behaviour.

The only slightly annoying detail is that, when selected, the compiler
is apparently not smart enough to see that the the saved_size and
saved_offset variables are only used under the same "if (sect_size >
CONFIG_ENV_SIZE)" condition as where they are computed, so we need to
initialize them to 0 to avoid "may be used uninitialized" warnings.

On our newer boards with the 4K erase size, saving the environment now
takes 0.080 seconds instead of 0.53 seconds, which directly translates
to that much faster boot time since our logic always causes the
environment to be written during boot.

Signed-off-by: Rasmus Villemoes 
---
 env/Kconfig | 14 ++
 env/sf.c| 10 --
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/env/Kconfig b/env/Kconfig
index b473d7cfe1..844c312870 100644
--- a/env/Kconfig
+++ b/env/Kconfig
@@ -324,6 +324,20 @@ config ENV_IS_IN_SPI_FLASH
  during a "saveenv" operation. CONFIG_ENV_OFFSET_REDUND must be
  aligned to an erase sector boundary.
 
+config ENV_SECT_SIZE_AUTO
+   bool "Use automatically detected sector size"
+   depends on ENV_IS_IN_SPI_FLASH
+   help
+ Some boards exist in multiple variants, with different
+ flashes having different sector sizes. In such cases, you
+ can select this option to make U-Boot use the actual sector
+ size when figuring out how much to erase, which can thus be
+ more efficient on the flashes with smaller erase size. Since
+ the environment must always be aligned on a sector boundary,
+ CONFIG_ENV_OFFSET must be aligned to the largest of the
+ different sector sizes, and CONFIG_ENV_SECT_SIZE should be
+ set to that value.
+
 config USE_ENV_SPI_BUS
bool "SPI flash bus for environment"
depends on ENV_IS_IN_SPI_FLASH
diff --git a/env/sf.c b/env/sf.c
index d9ed08a78e..06cc62e005 100644
--- a/env/sf.c
+++ b/env/sf.c
@@ -72,7 +72,7 @@ static int env_sf_save(void)
 {
env_t   env_new;
char*saved_buffer = NULL, flag = ENV_REDUND_OBSOLETE;
-   u32 saved_size, saved_offset, sector;
+   u32 saved_size = 0, saved_offset = 0, sector;
u32 sect_size = CONFIG_ENV_SECT_SIZE;
int ret;
 
@@ -80,6 +80,9 @@ static int env_sf_save(void)
if (ret)
return ret;
 
+   if (IS_ENABLED(CONFIG_ENV_SECT_SIZE_AUTO))
+   sect_size = env_flash->mtd.erasesize;
+
ret = env_export(_new);
if (ret)
return -EIO;
@@ -187,7 +190,7 @@ out:
 #else
 static int env_sf_save(void)
 {
-   u32 saved_size, saved_offset, sector;
+   u32 saved_size = 0, saved_offset = 0, sector;
u32 sect_size = CONFIG_ENV_SECT_SIZE;
char*saved_buffer = NULL;
int ret = 1;
@@ -197,6 +200,9 @@ static int env_sf_save(void)
if (ret)
return ret;
 
+   if (IS_ENABLED(CONFIG_ENV_SECT_SIZE_AUTO))
+   sect_size = env_flash->mtd.erasesize;
+
/* Is the sector larger than the env (i.e. embedded) */
if (sect_size > CONFIG_ENV_SIZE) {
saved_size = sect_size - CONFIG_ENV_SIZE;
-- 
2.29.2



[PATCH resend 0/2] add CONFIG_ENV_SECT_SIZE_AUTO

2021-04-14 Thread Rasmus Villemoes
This is a resend, just rebased to current master, of patches I sent
way back in May.

This is roughly the U-Boot side equivalent to commit e282c422e0
(tools: fw_env: use erasesize from MEMGETINFO ioctl), at least for
SPI_FLASH backend.

When CONFIG_ENV_SECT_SIZE_AUTO is not selected (and it is of course
default n), there's no functional change, and the compiler even seems
to generate identical binary code.

The motivation is to cut about half a second off boottime on our newer
revisions, while still having the same U-Boot binary work on both.

Rasmus Villemoes (2):
  env/sf.c: use a variable to hold the sector size
  env: add CONFIG_ENV_SECT_SIZE_AUTO

 env/Kconfig | 14 ++
 env/sf.c| 32 
 2 files changed, 34 insertions(+), 12 deletions(-)

-- 
2.29.2



[PATCH resend 1/2] env/sf.c: use a variable to hold the sector size

2021-04-14 Thread Rasmus Villemoes
As preparation for the next patch, use a local variable to represent
the sector size. No functional change.

Signed-off-by: Rasmus Villemoes 
---
 env/sf.c | 22 --
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/env/sf.c b/env/sf.c
index 88ec1108b6..d9ed08a78e 100644
--- a/env/sf.c
+++ b/env/sf.c
@@ -73,6 +73,7 @@ static int env_sf_save(void)
env_t   env_new;
char*saved_buffer = NULL, flag = ENV_REDUND_OBSOLETE;
u32 saved_size, saved_offset, sector;
+   u32 sect_size = CONFIG_ENV_SECT_SIZE;
int ret;
 
ret = setup_flash_device();
@@ -93,8 +94,8 @@ static int env_sf_save(void)
}
 
/* Is the sector larger than the env (i.e. embedded) */
-   if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
-   saved_size = CONFIG_ENV_SECT_SIZE - CONFIG_ENV_SIZE;
+   if (sect_size > CONFIG_ENV_SIZE) {
+   saved_size = sect_size - CONFIG_ENV_SIZE;
saved_offset = env_new_offset + CONFIG_ENV_SIZE;
saved_buffer = memalign(ARCH_DMA_MINALIGN, saved_size);
if (!saved_buffer) {
@@ -107,11 +108,11 @@ static int env_sf_save(void)
goto done;
}
 
-   sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, CONFIG_ENV_SECT_SIZE);
+   sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, sect_size);
 
puts("Erasing SPI flash...");
ret = spi_flash_erase(env_flash, env_new_offset,
-   sector * CONFIG_ENV_SECT_SIZE);
+   sector * sect_size);
if (ret)
goto done;
 
@@ -122,7 +123,7 @@ static int env_sf_save(void)
if (ret)
goto done;
 
-   if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
+   if (sect_size > CONFIG_ENV_SIZE) {
ret = spi_flash_write(env_flash, saved_offset,
saved_size, saved_buffer);
if (ret)
@@ -187,6 +188,7 @@ out:
 static int env_sf_save(void)
 {
u32 saved_size, saved_offset, sector;
+   u32 sect_size = CONFIG_ENV_SECT_SIZE;
char*saved_buffer = NULL;
int ret = 1;
env_t   env_new;
@@ -196,8 +198,8 @@ static int env_sf_save(void)
return ret;
 
/* Is the sector larger than the env (i.e. embedded) */
-   if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
-   saved_size = CONFIG_ENV_SECT_SIZE - CONFIG_ENV_SIZE;
+   if (sect_size > CONFIG_ENV_SIZE) {
+   saved_size = sect_size - CONFIG_ENV_SIZE;
saved_offset = CONFIG_ENV_OFFSET + CONFIG_ENV_SIZE;
saved_buffer = malloc(saved_size);
if (!saved_buffer)
@@ -213,11 +215,11 @@ static int env_sf_save(void)
if (ret)
goto done;
 
-   sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, CONFIG_ENV_SECT_SIZE);
+   sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, sect_size);
 
puts("Erasing SPI flash...");
ret = spi_flash_erase(env_flash, CONFIG_ENV_OFFSET,
-   sector * CONFIG_ENV_SECT_SIZE);
+   sector * sect_size);
if (ret)
goto done;
 
@@ -227,7 +229,7 @@ static int env_sf_save(void)
if (ret)
goto done;
 
-   if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
+   if (sect_size > CONFIG_ENV_SIZE) {
ret = spi_flash_write(env_flash, saved_offset,
saved_size, saved_buffer);
if (ret)
-- 
2.29.2



Re: [PATCH 11/13] board: synquacer: Add DeveloperBox 96boards EE support

2021-04-14 Thread Tom Rini
On Wed, Apr 14, 2021 at 10:12:42AM +0900, Masami Hiramatsu wrote:
> Hello Tom,
> 
> 2021年4月14日(水) 2:47 Tom Rini :
> >
> > On Wed, Apr 14, 2021 at 12:31:21AM +0900, Masami Hiramatsu wrote:
> >
> > > Add the DeveloperBox 96boards EE support. This board is also
> > > known as Socionext SynQuacer E-Series. It contians one "SC2A11"
> > > SoC, which has 24-cores of arm Cortex-A53, and 4 DDR3 slots,
> > > 3 PCIe slots (1 4x port and 2 1x ports which are expanded via
> > > PCIe bridge chip), 2 USB 3.0 ports and 2 USB 2.0 ports, 2 SATA
> > > ports and 1 GbE, 64MB NOR flash and 8GB eMMC on standard
> > > MicroATX Form Factor.
> > >
> > > For more information, see this page;
> > >   https://www.96boards.org/product/developerbox/
> > >
> > > Signed-off-by: Masami Hiramatsu 
> > [snip]
> > > diff --git a/arch/arm/include/asm/arch-sc2a11/gpio.h 
> > > b/arch/arm/include/asm/arch-sc2a11/gpio.h
> > > new file mode 100644
> > > index 00..6779803080
> > > --- /dev/null
> > > +++ b/arch/arm/include/asm/arch-sc2a11/gpio.h
> > > @@ -0,0 +1,9 @@
> > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > +/*
> > > + * Copyright 2021 (C) Linaro Ltd.
> > > + */
> > > +
> > > +#ifndef __ASM_ARCH_SC2A11_GPIO_H
> > > +#define __ASM_ARCH_SC2A11_GPIO_H
> > > +
> > > +#endif
> >
> > Please update the list in arch/arm/include/asm/gpio.h to not look for
> > asm/arch/gpio.h on this SoC, thanks.
> 
> Ah, I missed that. OK, I'll change arch/arm/include/asm/gpio.h.
> 
> BTW, isn't it better to introduce CONFIG_ARCH_GENERIC_GPIO
> instead of updating the header?

Such a clean-up would be appreciated.

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH 10/13] ARM: dts: synquacer: Add device trees for DeveloperBox

2021-04-14 Thread Tom Rini
On Wed, Apr 14, 2021 at 10:06:04AM +0900, Masami Hiramatsu wrote:
> Hello Tom,
> 
> Thank you for your comment!
> 
> 2021年4月14日(水) 2:47 Tom Rini :
> >
> > On Wed, Apr 14, 2021 at 12:30:15AM +0900, Masami Hiramatsu wrote:
> >
> > > Add device trees for 96boards EE DeveloperBox and basement SynQuacer
> > > SoC dtsi. These files are imported from EDK2 with cosmetic change
> > > and modified for accessing SPI NOR flash.
> > >
> > > Signed-off-by: Masami Hiramatsu 
> > > ---
> > >  arch/arm/dts/DeveloperBox.dts |  146 +
> > >  arch/arm/dts/Makefile |2
> > >  arch/arm/dts/SynQuacer.dtsi   |  590 
> > > +
> > >  arch/arm/dts/SynQuacerCaches.dtsi |   72 +
> >
> > This poses a bit of a naming challenge.  I assume, but please correct me
> > if I'm wrong, that you don't intend to push these dts files to the Linux
> > kernel.  So that means EDK2 is the primary source of the files, yes?
> 
> Yes, those are originally written for the EDK2 and I ported it.

OK.

> > We
> > want to keep them as-is from the upstream project, and note that
> > relevant git hash/tag/etc that matches where we pulled from, to make
> > future syncs easier, in the commit message.
> 
> Let me confirm what you mean, is the git hash/tag/etc what the commit
> I copied from? and note it in commit message or in the devicetree file?

Correct.

> BTW, I made some changes on it for U-Boot drivers.
> - Enabled OP-TEE node by default (EDK2 checks OP-TEE existance
>   and enables it)
> - Add SPI node information for accessing SPI-NOR from U-Boot (EDK2
>  embedded such information inside)
> Thus the DeveloperBox.dts may not be kept as-is.

These kinds of changes can be done in something such as
DeveloperBox-u-boot.dtsi.  Check out the logic in scripts/Makefile.lib
around automatic inclusion of a "-u-boot.dtsi" file.

> > I assume this is not part of the uniphier family, so we should start by
> > naming these as arch/arm/dts/synquacer-... to fit with the general
> > scheme.  Perhaps -developerbox, -core and -caches ?
> 
> OK. BTW, I'm not sure what is the best way to use SYS_CPU and SYS_SOC.
> This SoC family name is SynQuacer and the SoC itself is SC2A11.
> In this case,
> CONFIG_SYS_CPU=synquacer
> CONFIG_SYS_SOC=sc2a11
> or
> CONFIG_SYS_SOC=synquacer-sc2a11

I think the first example you list is likely the best.

> > And for any changes
> > we do need, they go in a -u-boot.dtsi file and then potentially
> > automatically included (if it fits in the logic we have today for that)
> > or specifically #included otherwise.
> 
> It seems that (CONFIG_SYS_SOC)-u-boot.dtsi is automatically included,
> am I correct?

For the full list and how it works, see scripts/Makefile.lib

-- 
Tom


signature.asc
Description: PGP signature


[PATCH] imx: Add support for Ronetix's iMX8MM-CM board

2021-04-14 Thread Ilko Iliev
Supported peripherals: ETH, SD, eMMC, PMIC, QSPI NOR Flash

U-Boot SPL 2021.04-00759-g59701858a1-dirty (Apr 14 2021 - 17:23:43 +0200)
PMIC:  BD71847 ID=0xa1
Normal Boot
WDT:   Not starting
Trying to boot from MMC1

U-Boot 2021.04-00759-g59701858a1-dirty (Apr 14 2021 - 17:23:43 +0200)

CPU:   Freescale i.MX8MMQL rev1.0 at 1200 MHz
Reset cause: POR
Model: Ronetix i.MX8MM-CM board
DRAM:  1 GiB
WDT:   Started with servicing (60s timeout)
MMC:   FSL_SDHC: 1, FSL_SDHC: 2
Loading Environment from MMC... OK
In:serial
Out:   serial
Err:   serial
Net:
Warning: ethernet@30be (eth0) using random MAC address - 4a:f6:bf:8d:d8:a9
eth0: ethernet@30be
Hit any key to stop autoboot:  0

Signed-off-by: Ilko Iliev 
---
 arch/arm/dts/Makefile |1 +
 arch/arm/dts/imx8mm-cm-u-boot.dtsi|  253 +++
 arch/arm/dts/imx8mm-cm.dts|  558 +
 arch/arm/mach-imx/imx8m/Kconfig   |8 +
 board/ronetix/common  |1 +
 board/ronetix/imx8mm-cm/Kconfig   |   14 +
 board/ronetix/imx8mm-cm/MAINTAINERS   |6 +
 board/ronetix/imx8mm-cm/Makefile  |   12 +
 board/ronetix/imx8mm-cm/README|   43 +
 board/ronetix/imx8mm-cm/imx8mm_cm.c   |   68 +
 .../ronetix/imx8mm-cm/imximage-8mm-lpddr4.cfg |9 +
 board/ronetix/imx8mm-cm/lpddr4_timing.c   | 1862 +
 board/ronetix/imx8mm-cm/spl.c |  168 ++
 configs/imx8mm_cm_defconfig   |  110 +
 include/configs/imx8mm_cm.h   |  107 +
 15 files changed, 3220 insertions(+)
 create mode 100644 arch/arm/dts/imx8mm-cm-u-boot.dtsi
 create mode 100644 arch/arm/dts/imx8mm-cm.dts
 create mode 12 board/ronetix/common
 create mode 100644 board/ronetix/imx8mm-cm/Kconfig
 create mode 100644 board/ronetix/imx8mm-cm/MAINTAINERS
 create mode 100644 board/ronetix/imx8mm-cm/Makefile
 create mode 100644 board/ronetix/imx8mm-cm/README
 create mode 100644 board/ronetix/imx8mm-cm/imx8mm_cm.c
 create mode 100644 board/ronetix/imx8mm-cm/imximage-8mm-lpddr4.cfg
 create mode 100644 board/ronetix/imx8mm-cm/lpddr4_timing.c
 create mode 100644 board/ronetix/imx8mm-cm/spl.c
 create mode 100644 configs/imx8mm_cm_defconfig
 create mode 100644 include/configs/imx8mm_cm.h

diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index b58f841472..b52c65fd09 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -827,6 +827,7 @@ dtb-$(CONFIG_ARCH_IMX8) += \
imx8-giedi.dtb
 
 dtb-$(CONFIG_ARCH_IMX8M) += \
+   imx8mm-cm.dtb \
imx8mm-evk.dtb \
imx8mm-venice.dtb \
imx8mm-venice-gw71xx-0x.dtb \
diff --git a/arch/arm/dts/imx8mm-cm-u-boot.dtsi 
b/arch/arm/dts/imx8mm-cm-u-boot.dtsi
new file mode 100644
index 00..c22b14a6e2
--- /dev/null
+++ b/arch/arm/dts/imx8mm-cm-u-boot.dtsi
@@ -0,0 +1,253 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2019 NXP
+ */
+
+/ {
+   binman: binman {
+   multiple-images;
+   };
+
+   wdt-reboot {
+   compatible = "wdt-reboot";
+   wdt = <>;
+   u-boot,dm-spl;
+   };
+
+   firmware {
+   optee {
+   compatible = "linaro,optee-tz";
+   method = "smc";
+   };
+   };
+};
+
+&{/soc@0} {
+   u-boot,dm-pre-reloc;
+   u-boot,dm-spl;
+};
+
+ {
+   u-boot,dm-spl;
+   u-boot,dm-pre-reloc;
+   /delete-property/ assigned-clocks;
+   /delete-property/ assigned-clock-parents;
+   /delete-property/ assigned-clock-rates;
+};
+
+_24m {
+   u-boot,dm-spl;
+   u-boot,dm-pre-reloc;
+};
+
+ {
+   u-boot,dm-spl;
+   u-boot,dm-pre-reloc;
+};
+
+ {
+   u-boot,dm-spl;
+};
+
+ {
+   u-boot,dm-spl;
+};
+
+ {
+   u-boot,dm-spl;
+};
+
+_usdhc2_vmmc {
+   u-boot,off-on-delay-us = <2>;
+};
+
+_reg_usdhc2_vmmc {
+   u-boot,dm-spl;
+};
+
+_uart2 {
+   u-boot,dm-spl;
+};
+
+_usdhc2_gpio {
+   u-boot,dm-spl;
+};
+
+_usdhc2 {
+   u-boot,dm-spl;
+};
+
+_usdhc3 {
+   u-boot,dm-spl;
+};
+
+ {
+   u-boot,dm-spl;
+};
+
+ {
+   u-boot,dm-spl;
+};
+
+ {
+   u-boot,dm-spl;
+};
+
+ {
+   u-boot,dm-spl;
+};
+
+ {
+   u-boot,dm-spl;
+};
+
+ {
+   u-boot,dm-spl;
+};
+
+ {
+   u-boot,dm-spl;
+};
+
+ {
+   u-boot,dm-spl;
+   sd-uhs-sdr104;
+   sd-uhs-ddr50;
+   fsl,signal-voltage-switch-extra-delay-ms = <8>;
+};
+
+ {
+   u-boot,dm-spl;
+   mmc-hs400-1_8v;
+   mmc-hs400-enhanced-strobe;
+};
+
+ {
+   u-boot,dm-spl;
+};
+
+&{/soc@0/bus@3080/i2c@30a3/pmic@4b} {
+   u-boot,dm-spl;
+};
+
+&{/soc@0/bus@3080/i2c@30a3/pmic@4b/regulators} {
+   u-boot,dm-spl;
+};
+
+_i2c2 {
+   u-boot,dm-spl;
+};
+
+_pmic {
+   u-boot,dm-spl;
+};
+
+ {
+   phy-reset-gpios = < 29 GPIO_ACTIVE_LOW>;
+};
+
+ {
+   u-boot,dm-spl;
+};
+
+ {
+u-boot-spl-ddr {
+   filename 

Re: [PATCH v2 7/7] arm: cache: cp15: don't map the reserved region with no-map property

2021-04-14 Thread Patrick DELAUNAY

Hi,

On 2/8/21 2:26 PM, Patrick Delaunay wrote:

No more map the reserved region with "no-map" property by marking
the corresponding TLB entries with invalid entry (=0) to avoid
speculative access.

This patch fixes potential issue when predictive access is done by ARM
core.

Signed-off-by: Patrick Delaunay 
Signed-off-by: Patrick Delaunay 
---

(no changes since v1)

  arch/arm/include/asm/system.h |  3 +++
  arch/arm/lib/cache-cp15.c | 19 +--
  2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
index 11fceec4d2..c63ed07f2c 100644
--- a/arch/arm/include/asm/system.h
+++ b/arch/arm/include/asm/system.h
@@ -444,6 +444,7 @@ static inline void set_cr(unsigned int val)
  
  /* options available for data cache on each page */

  enum dcache_option {
+   INVALID_ENTRY = 0,
DCACHE_OFF = TTB_SECT | TTB_SECT_MAIR(0) | TTB_SECT_XN_MASK,
DCACHE_WRITETHROUGH = TTB_SECT | TTB_SECT_MAIR(1),
DCACHE_WRITEBACK = TTB_SECT | TTB_SECT_MAIR(2),
@@ -474,6 +475,7 @@ enum dcache_option {
   *   11  1   Outer/Inner Write-Back, Read-Allocate Write-Allocate
   */
  enum dcache_option {
+   INVALID_ENTRY = 0,
DCACHE_OFF = TTB_SECT_DOMAIN(0) | TTB_SECT_XN_MASK | TTB_SECT,
DCACHE_WRITETHROUGH = TTB_SECT_DOMAIN(0) | TTB_SECT | TTB_SECT_C_MASK,
DCACHE_WRITEBACK = DCACHE_WRITETHROUGH | TTB_SECT_B_MASK,
@@ -483,6 +485,7 @@ enum dcache_option {
  #define TTB_SECT_AP   (3 << 10)
  /* options available for data cache on each page */
  enum dcache_option {
+   INVALID_ENTRY = 0,
DCACHE_OFF = 0x12,
DCACHE_WRITETHROUGH = 0x1a,
DCACHE_WRITEBACK = 0x1e,
diff --git a/arch/arm/lib/cache-cp15.c b/arch/arm/lib/cache-cp15.c
index 8a49e5217c..8a354d364d 100644
--- a/arch/arm/lib/cache-cp15.c
+++ b/arch/arm/lib/cache-cp15.c
@@ -6,6 +6,7 @@
  
  #include 

  #include 
+#include 
  #include 
  #include 
  #include 
@@ -101,18 +102,32 @@ void mmu_set_region_dcache_behaviour(phys_addr_t start, 
size_t size,
  __weak void dram_bank_mmu_setup(int bank)
  {
struct bd_info *bd = gd->bd;
+   struct lmb lmb;
int i;
  
  	/* bd->bi_dram is available only after relocation */

if ((gd->flags & GD_FLG_RELOC) == 0)
return;
  
+	/*

+* don't allow cache on reserved memory tagged 'no-map' in DT
+* => avoid speculative access to "secure" data
+*/
+   if (IS_ENABLED(CONFIG_LMB))
+   lmb_init_and_reserve(, bd, (void *)gd->fdt_blob);
+
debug("%s: bank: %d\n", __func__, bank);
for (i = bd->bi_dram[bank].start >> MMU_SECTION_SHIFT;
 i < (bd->bi_dram[bank].start >> MMU_SECTION_SHIFT) +
 (bd->bi_dram[bank].size >> MMU_SECTION_SHIFT);
-i++)
-   set_section_dcache(i, DCACHE_DEFAULT_OPTION);
+i++) {
+   if (IS_ENABLED(CONFIG_LMB) &&
+   lmb_is_reserved_flags(, i << MMU_SECTION_SHIFT,
+ LMB_NOMAP))
+   set_section_dcache(i, INVALID_ENTRY);
+   else
+   set_section_dcache(i, DCACHE_DEFAULT_OPTION);
+   }
  }
  
  /* to activate the MMU we need to set up virtual memory: use 1M areas */



Hi,


After more test on stm32mp15x platform, the patch [6/7] introduced

performance issue for the boot time.


The device tree parsing done in lmb_init_and_reserve() to found the

the reserved memory region with "no-map" tag is done with data

cache deactivated (as it is called before MMU and data cache activation).


This parsing increase the device boot time (several second lost in 
stm32mp15).


So I need to sent a update version for this patch [6/7]


But I will also drop this patch  patch [7/7] to avoid to increase the 
boot time


on other arm platform using cp15.


Regards


Patrick




Re: [PATCH] net: dwc_eth_qos: add support of device tree configuration for reset delay

2021-04-14 Thread Marek Vasut

On 4/14/21 4:07 PM, Patrick DELAUNAY wrote:

Hi,


Hi,


On 4/9/21 2:22 PM, Marek Vasut wrote:

On 4/9/21 10:00 AM, Patrick Delaunay wrote:

The gpio reset assert/deassert delay are today harcoded in U-Boot driver
but the value should be read from DT.

STM32 use the generic binding defined in linux:
Documentation/devicetree/bindings/net/ethernet-phy.yaml

   reset-gpios:
 maxItems: 1
 description:
   The GPIO phandle and specifier for the PHY reset signal.

   reset-assert-us:
 description:
   Delay after the reset was asserted in microseconds. If this
   property is missing the delay will be skipped.

   reset-deassert-us:
 description:
   Delay after the reset was deasserted in microseconds. If
   this property is missing the delay will be skipped.

See also U-Boot: doc/device-tree-bindings/net/phy.txt


Since this is a PHY property, shouldn't that be handled in 
drivers/net/phy/ instead of MAC driver ?



I was my first idea but I don't found found the correct location in phy 
(driver or uclass)


to manage these generic property and the generic property "reset-gpios" 
was already


managed in eth driver, so I continue to patch the driver.


But I come back to this idea after your remark

=> in linux these property are managed in

     drivers/net/mdio/of_mdio.c::of_mdiobus_phy_device_register

         parse DT node and add info in mdio

     drivers/net/phy/mdio_device.c::mdio_device_reset

         called in  mdio_probe / mdio_remove


In my first search, I don't found the same level in the U-Boot drivers 
in drivers/net/phy/


Note that this is MDIO-wide PHY reset (e.g. you can have single reset 
line connected to multiple PHYs on single MDIO bus), this is not 
PHY-specific reset.



but I miss the uclass defined in drivers/net/eth-phy-uclass.c


Finally I think I need to manage the generic binding property

(reset-gpios, reset-assert-us, reset-deassert-us) directly  in

=> drivers/net/mdio-uclass


The GPIO RESET will be managed in mdio  ops: pre_probe/ post_remove

as it is done in linux

warning: today post_remove ops don't exit in u-class.


Do you think it is the correct location ?


For single-PHY reset, the correct location is in drivers/net/phy/ somewhere.


Do you think it should be a new serie (migrate the eqos property in mdio)

after this eqos is accepted

or I shoudl sent a new serie to replace this serie.


I'll leave that decision to Ramon/Joe.


Re: [PATCH] net: dwc_eth_qos: add support of device tree configuration for reset delay

2021-04-14 Thread Patrick DELAUNAY

Hi,

On 4/9/21 2:22 PM, Marek Vasut wrote:

On 4/9/21 10:00 AM, Patrick Delaunay wrote:

The gpio reset assert/deassert delay are today harcoded in U-Boot driver
but the value should be read from DT.

STM32 use the generic binding defined in linux:
Documentation/devicetree/bindings/net/ethernet-phy.yaml

   reset-gpios:
 maxItems: 1
 description:
   The GPIO phandle and specifier for the PHY reset signal.

   reset-assert-us:
 description:
   Delay after the reset was asserted in microseconds. If this
   property is missing the delay will be skipped.

   reset-deassert-us:
 description:
   Delay after the reset was deasserted in microseconds. If
   this property is missing the delay will be skipped.

See also U-Boot: doc/device-tree-bindings/net/phy.txt


Since this is a PHY property, shouldn't that be handled in 
drivers/net/phy/ instead of MAC driver ?



I was my first idea but I don't found found the correct location in phy 
(driver or uclass)


to manage these generic property and the generic property "reset-gpios" 
was already


managed in eth driver, so I continue to patch the driver.


But I come back to this idea after your remark

=> in linux these property are managed in

    drivers/net/mdio/of_mdio.c::of_mdiobus_phy_device_register

        parse DT node and add info in mdio

    drivers/net/phy/mdio_device.c::mdio_device_reset

        called in  mdio_probe / mdio_remove


In my first search, I don't found the same level in the U-Boot drivers 
in drivers/net/phy/


but I miss the uclass defined in drivers/net/eth-phy-uclass.c


Finally I think I need to manage the generic binding property

(reset-gpios, reset-assert-us, reset-deassert-us) directly  in

=> drivers/net/mdio-uclass


The GPIO RESET will be managed in mdio  ops: pre_probe/ post_remove

as it is done in linux

warning: today post_remove ops don't exit in u-class.


Do you think it is the correct location ?


Do you think it should be a new serie (migrate the eqos property in mdio)

after this eqos is accepted

or I shoudl sent a new serie to replace this serie.


Regards


Patrick





Re: [PATCH v4 21/22] ppc: qemu: Enable eTSEC support

2021-04-14 Thread Bin Meng
Hi Priyanka,

On Wed, Apr 14, 2021 at 7:54 PM Priyanka Jain  wrote:
>
>
>
> >-Original Message-
> >From: Bin Meng 
> >Sent: Sunday, March 14, 2021 5:45 PM
> >To: Priyanka Jain ; Ramon Fried
> >; Simon Glass ; u-
> >b...@lists.denx.de
> >Cc: Tom Rini ; Vladimir Oltean ;
> >Bin Meng 
> >Subject: [PATCH v4 21/22] ppc: qemu: Enable eTSEC support
> >
> >QEMU ppce500 target can dynamically instantiate an eTSEC device if "-device
> >eTSEC" is given to QEMU. This commit enables eTSEC driver and the required
> >fixed PHY driver to create a usable network configuration using eTSEC.
> >
> >Unlike a real world 85xx board that usually stores the eTSEC MAC address in 
> >an
> >EEPROM, CONFIG_NET_RANDOM_ETHADDR is required for QEMU otherwise U-
> >Boot ethernet initialization complains no valid ethernet address is set.
> >
> >Signed-off-by: Bin Meng 
> >Reviewed-by: Vladimir Oltean 
> >---
> >
> >(no changes since v1)
> >
> > configs/qemu-ppce500_defconfig | 4 
> > 1 file changed, 4 insertions(+)
> >
> >diff --git a/configs/qemu-ppce500_defconfig b/configs/qemu-ppce500_defconfig
> >index 151834b4cf..a1b9ea56ca 100644
> >--- a/configs/qemu-ppce500_defconfig
> >+++ b/configs/qemu-ppce500_defconfig
> >@@ -27,6 +27,7 @@ CONFIG_OF_CONTROL=y
> > CONFIG_OF_BOARD=y
> > CONFIG_ENV_OVERWRITE=y
> > CONFIG_SYS_RELOC_GD_ENV_ADDR=y
> >+CONFIG_NET_RANDOM_ETHADDR=y
> > CONFIG_DM=y
> > CONFIG_SIMPLE_BUS_CORRECT_RANGE=y
> > CONFIG_BLK=y
> >@@ -35,8 +36,11 @@ CONFIG_MPC8XXX_GPIO=y  CONFIG_DM_I2C=y
> >CONFIG_SYS_I2C_FSL=y  # CONFIG_MMC is not set
> >+CONFIG_PHY_FIXED=y
> > CONFIG_DM_ETH=y
> >+CONFIG_DM_MDIO=y
> > CONFIG_E1000=y
> >+CONFIG_TSEC_ENET=y
> > CONFIG_DM_PCI=y
> > CONFIG_PCI_MPC85XX=y
> > CONFIG_DM_RTC=y
> >--
> >2.25.1
>
> I tried integrating the series and was getting below error:
> 2021-04-12T09:39:56.7536565Z FAILED 
> test/py/tests/test_efi_selftest.py::test_efi_selftest - u_boot_spawn.T...
> 2021-04-12T09:39:56.7537048Z = 1 failed, 108 passed, 227 skipped, 1 
> deselected, 3 warnings in 65.61s (0:01:05) =
>
> Details at 
> https://dev.azure.com/u-boot/a1096300-2999-4ec4-a21a-4c22075e3771/_apis/build/builds/2112/logs/251
>
> I reverted this patch and it then build fine .
> https://github.com/u-boot/u-boot/pull/65

As I mentioned in this series cover letter, Azure results were all PASS.

Please see:
https://dev.azure.com/bmeng/GitHub/_build/results?buildId=343=results

Regards,
Bin


Re: [PATCH v4 21/22] ppc: qemu: Enable eTSEC support

2021-04-14 Thread Tom Rini
On Wed, Apr 14, 2021 at 11:54:39AM +, Priyanka Jain wrote:
> 
> 
> >-Original Message-
> >From: Bin Meng 
> >Sent: Sunday, March 14, 2021 5:45 PM
> >To: Priyanka Jain ; Ramon Fried
> >; Simon Glass ; u-
> >b...@lists.denx.de
> >Cc: Tom Rini ; Vladimir Oltean ;
> >Bin Meng 
> >Subject: [PATCH v4 21/22] ppc: qemu: Enable eTSEC support
> >
> >QEMU ppce500 target can dynamically instantiate an eTSEC device if "-device
> >eTSEC" is given to QEMU. This commit enables eTSEC driver and the required
> >fixed PHY driver to create a usable network configuration using eTSEC.
> >
> >Unlike a real world 85xx board that usually stores the eTSEC MAC address in 
> >an
> >EEPROM, CONFIG_NET_RANDOM_ETHADDR is required for QEMU otherwise U-
> >Boot ethernet initialization complains no valid ethernet address is set.
> >
> >Signed-off-by: Bin Meng 
> >Reviewed-by: Vladimir Oltean 
> >---
> >
> >(no changes since v1)
> >
> > configs/qemu-ppce500_defconfig | 4 
> > 1 file changed, 4 insertions(+)
> >
> >diff --git a/configs/qemu-ppce500_defconfig b/configs/qemu-ppce500_defconfig
> >index 151834b4cf..a1b9ea56ca 100644
> >--- a/configs/qemu-ppce500_defconfig
> >+++ b/configs/qemu-ppce500_defconfig
> >@@ -27,6 +27,7 @@ CONFIG_OF_CONTROL=y
> > CONFIG_OF_BOARD=y
> > CONFIG_ENV_OVERWRITE=y
> > CONFIG_SYS_RELOC_GD_ENV_ADDR=y
> >+CONFIG_NET_RANDOM_ETHADDR=y
> > CONFIG_DM=y
> > CONFIG_SIMPLE_BUS_CORRECT_RANGE=y
> > CONFIG_BLK=y
> >@@ -35,8 +36,11 @@ CONFIG_MPC8XXX_GPIO=y  CONFIG_DM_I2C=y
> >CONFIG_SYS_I2C_FSL=y  # CONFIG_MMC is not set
> >+CONFIG_PHY_FIXED=y
> > CONFIG_DM_ETH=y
> >+CONFIG_DM_MDIO=y
> > CONFIG_E1000=y
> >+CONFIG_TSEC_ENET=y
> > CONFIG_DM_PCI=y
> > CONFIG_PCI_MPC85XX=y
> > CONFIG_DM_RTC=y
> >--
> >2.25.1
> 
> I tried integrating the series and was getting below error: 
> 2021-04-12T09:39:56.7536565Z FAILED 
> test/py/tests/test_efi_selftest.py::test_efi_selftest - u_boot_spawn.T...
> 2021-04-12T09:39:56.7537048Z = 1 failed, 108 passed, 227 skipped, 1 
> deselected, 3 warnings in 65.61s (0:01:05) =
> 
> Details at 
> https://dev.azure.com/u-boot/a1096300-2999-4ec4-a21a-4c22075e3771/_apis/build/builds/2112/logs/251
> 
> I reverted this patch and it then build fine .
> https://github.com/u-boot/u-boot/pull/65

Did this fail more than once in that job?  Sometimes due to I assume
some race/etc, that test will fail from time to time.

-- 
Tom


signature.asc
Description: PGP signature


RE: [PATCH v4 21/22] ppc: qemu: Enable eTSEC support

2021-04-14 Thread Priyanka Jain



>-Original Message-
>From: Bin Meng 
>Sent: Sunday, March 14, 2021 5:45 PM
>To: Priyanka Jain ; Ramon Fried
>; Simon Glass ; u-
>b...@lists.denx.de
>Cc: Tom Rini ; Vladimir Oltean ;
>Bin Meng 
>Subject: [PATCH v4 21/22] ppc: qemu: Enable eTSEC support
>
>QEMU ppce500 target can dynamically instantiate an eTSEC device if "-device
>eTSEC" is given to QEMU. This commit enables eTSEC driver and the required
>fixed PHY driver to create a usable network configuration using eTSEC.
>
>Unlike a real world 85xx board that usually stores the eTSEC MAC address in an
>EEPROM, CONFIG_NET_RANDOM_ETHADDR is required for QEMU otherwise U-
>Boot ethernet initialization complains no valid ethernet address is set.
>
>Signed-off-by: Bin Meng 
>Reviewed-by: Vladimir Oltean 
>---
>
>(no changes since v1)
>
> configs/qemu-ppce500_defconfig | 4 
> 1 file changed, 4 insertions(+)
>
>diff --git a/configs/qemu-ppce500_defconfig b/configs/qemu-ppce500_defconfig
>index 151834b4cf..a1b9ea56ca 100644
>--- a/configs/qemu-ppce500_defconfig
>+++ b/configs/qemu-ppce500_defconfig
>@@ -27,6 +27,7 @@ CONFIG_OF_CONTROL=y
> CONFIG_OF_BOARD=y
> CONFIG_ENV_OVERWRITE=y
> CONFIG_SYS_RELOC_GD_ENV_ADDR=y
>+CONFIG_NET_RANDOM_ETHADDR=y
> CONFIG_DM=y
> CONFIG_SIMPLE_BUS_CORRECT_RANGE=y
> CONFIG_BLK=y
>@@ -35,8 +36,11 @@ CONFIG_MPC8XXX_GPIO=y  CONFIG_DM_I2C=y
>CONFIG_SYS_I2C_FSL=y  # CONFIG_MMC is not set
>+CONFIG_PHY_FIXED=y
> CONFIG_DM_ETH=y
>+CONFIG_DM_MDIO=y
> CONFIG_E1000=y
>+CONFIG_TSEC_ENET=y
> CONFIG_DM_PCI=y
> CONFIG_PCI_MPC85XX=y
> CONFIG_DM_RTC=y
>--
>2.25.1

I tried integrating the series and was getting below error: 
2021-04-12T09:39:56.7536565Z FAILED 
test/py/tests/test_efi_selftest.py::test_efi_selftest - u_boot_spawn.T...
2021-04-12T09:39:56.7537048Z = 1 failed, 108 passed, 227 skipped, 1 deselected, 
3 warnings in 65.61s (0:01:05) =

Details at 
https://dev.azure.com/u-boot/a1096300-2999-4ec4-a21a-4c22075e3771/_apis/build/builds/2112/logs/251

I reverted this patch and it then build fine .
https://github.com/u-boot/u-boot/pull/65

Kindly check.

Regards
Priyanka


[PATCH v4 1/2] timer: mpc83xx_timer: fix build with CONFIG_{HW_, }WATCHDOG

2021-04-14 Thread Rasmus Villemoes
The code, which is likely copied from arch/powerpc/lib/interrupts.c,
lacks a fallback definition of CONFIG_SYS_WATCHDOG_FREQ and refers to
a non-existing timestamp variable - obviously priv->timestamp is
meant.

Signed-off-by: Rasmus Villemoes 
---
 drivers/timer/mpc83xx_timer.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/timer/mpc83xx_timer.c b/drivers/timer/mpc83xx_timer.c
index f4f6e90387..9adb4b4cab 100644
--- a/drivers/timer/mpc83xx_timer.c
+++ b/drivers/timer/mpc83xx_timer.c
@@ -20,6 +20,10 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
+#ifndef CONFIG_SYS_WATCHDOG_FREQ
+#define CONFIG_SYS_WATCHDOG_FREQ (CONFIG_SYS_HZ / 2)
+#endif
+
 /**
  * struct mpc83xx_timer_priv - Private data structure for MPC83xx timer driver
  * @decrementer_count: Value to which the decrementer register should be re-set
@@ -171,7 +175,7 @@ void timer_interrupt(struct pt_regs *regs)
priv->timestamp++;
 
 #if defined(CONFIG_WATCHDOG) || defined(CONFIG_HW_WATCHDOG)
-   if ((timestamp % (CONFIG_SYS_WATCHDOG_FREQ)) == 0)
+   if ((priv->timestamp % (CONFIG_SYS_WATCHDOG_FREQ)) == 0)
WATCHDOG_RESET();
 #endif/* CONFIG_WATCHDOG || CONFIG_HW_WATCHDOG */
 
-- 
2.29.2



Re: [PATCH] efi_loader: fix possible buffer overflow

2021-04-14 Thread Heinrich Schuchardt

On 4/14/21 7:43 AM, Ilias Apalodimas wrote:

On Wed, Apr 14, 2021 at 11:55:49AM +0900, Masahisa Kojima wrote:

Variable "final" will have SHA512 digest, but currently
the array size is not sufficient. Let's fix it.

Signed-off-by: Masahisa Kojima 
---
  lib/efi_loader/efi_tcg2.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index ed86a220fb..d5eca68769 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -515,7 +515,7 @@ static efi_status_t tcg2_create_digest(const u8 *input, u32 
length,
sha1_context ctx;
sha256_context ctx_256;
sha512_context ctx_512;
-   u8 final[TPM2_ALG_SHA512];
+   u8 final[TPM2_SHA512_DIGEST_SIZE];
efi_status_t ret;
u32 active;
int i;
--
2.17.1



Thanks!

Reviewed-by: Ilias Apalodimas 



I have queued the patch for my next pull request.

Reviewed-by: Heinrich Schuchardt 


Re: [PATCH] efi_loader: esrt: Remove incorrect invocations of EFI_CALL macro

2021-04-14 Thread Heinrich Schuchardt

On 4/14/21 9:08 AM, Sughosh Ganu wrote:

Remove function invocations using the EFI_CALL macro for those
functions that do not have an EFI_ENTRY call in their definition. Such
functions can use u-boot api's which rely on u-boot global data(gd)
pointer. The Arm and RiscV architectures maintain a separate gd
pointer, one for u-boot, and a separate gd for the efi application.

Calling a function through the EFI_CALL macro changes the gd pointer
to that used for the efi application, with u-boot gd being
unavailable. Any function then trying to dereference u-boot's gd will
result in an abort.

Fix this issue by removing the EFI_CALL macro for all of such
functions which do not begin by an EFI_ENTRY function call.

Signed-off-by: Sughosh Ganu 


Thanks for reviewing the ESRT code.

Reviewed-by: Heinrich Schuchardt 


[PATCH] ls1012a: net: pfe: remove pfe stop from bootcmd

2021-04-14 Thread Mian Yousaf Kaukab
When using bootefi to boot a EFI binary, u-boot is supposed to
provide networking service for EFI application. Currently, 'pfe stop'
command is called from bootcmd before running bootefi. As a result
network stops working for EFI applications and console is flooded with
"Rx pkt not on expected port" messages.

Implement board_quiesce_devices() for ls1012a boards and call
pfe_command_stop() from it instead of calling 'pfe stop' from
*_bootcmd and bootcmd.

Tested-by: Anji Jagarlmudi 
Signed-off-by: Mian Yousaf Kaukab 
---
 board/freescale/ls1012afrdm/ls1012afrdm.c | 8 
 board/freescale/ls1012aqds/ls1012aqds.c   | 8 
 board/freescale/ls1012ardb/ls1012ardb.c   | 8 
 drivers/net/pfe_eth/pfe_cmd.c | 2 +-
 include/configs/ls1012a2g5rdb.h   | 6 +++---
 include/configs/ls1012a_common.h  | 4 ++--
 include/configs/ls1012afrdm.h | 6 +++---
 include/configs/ls1012afrwy.h | 6 +++---
 include/configs/ls1012aqds.h  | 6 +++---
 include/configs/ls1012ardb.h  | 6 +++---
 include/net/pfe_eth/pfe/pfe_hw.h  | 6 ++
 11 files changed, 48 insertions(+), 18 deletions(-)

diff --git a/board/freescale/ls1012afrdm/ls1012afrdm.c 
b/board/freescale/ls1012afrdm/ls1012afrdm.c
index 2cd651b943fb..7e87e5a9846f 100644
--- a/board/freescale/ls1012afrdm/ls1012afrdm.c
+++ b/board/freescale/ls1012afrdm/ls1012afrdm.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -185,6 +186,13 @@ int board_init(void)
return 0;
 }
 
+#ifdef CONFIG_FSL_PFE
+void board_quiesce_devices(void)
+{
+pfe_command_stop(0, NULL);
+}
+#endif
+
 int ft_board_setup(void *blob, struct bd_info *bd)
 {
arch_fixup_fdt(blob);
diff --git a/board/freescale/ls1012aqds/ls1012aqds.c 
b/board/freescale/ls1012aqds/ls1012aqds.c
index cfe3f3360cd9..4f98a8652f21 100644
--- a/board/freescale/ls1012aqds/ls1012aqds.c
+++ b/board/freescale/ls1012aqds/ls1012aqds.c
@@ -32,6 +32,7 @@
 #include "../common/qixis.h"
 #include "ls1012aqds_qixis.h"
 #include "ls1012aqds_pfe.h"
+#include 
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -163,6 +164,13 @@ int board_init(void)
return 0;
 }
 
+#ifdef CONFIG_FSL_PFE
+void board_quiesce_devices(void)
+{
+pfe_command_stop(0, NULL);
+}
+#endif
+
 int esdhc_status_fixup(void *blob, const char *compat)
 {
char esdhc0_path[] = "/soc/esdhc@156";
diff --git a/board/freescale/ls1012ardb/ls1012ardb.c 
b/board/freescale/ls1012ardb/ls1012ardb.c
index 41bcf6f935e9..62e8af48cf14 100644
--- a/board/freescale/ls1012ardb/ls1012ardb.c
+++ b/board/freescale/ls1012ardb/ls1012ardb.c
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -186,6 +187,13 @@ int board_init(void)
return 0;
 }
 
+#ifdef CONFIG_FSL_PFE
+void board_quiesce_devices(void)
+{
+   pfe_command_stop(0, NULL);
+}
+#endif
+
 #ifdef CONFIG_TARGET_LS1012ARDB
 int esdhc_status_fixup(void *blob, const char *compat)
 {
diff --git a/drivers/net/pfe_eth/pfe_cmd.c b/drivers/net/pfe_eth/pfe_cmd.c
index 1e69525cb71f..364750f65c75 100644
--- a/drivers/net/pfe_eth/pfe_cmd.c
+++ b/drivers/net/pfe_eth/pfe_cmd.c
@@ -418,7 +418,7 @@ static void send_dummy_pkt_to_hif(void)
writel(buf, TMU_PHY_INQ_PKTINFO);
 }
 
-static void pfe_command_stop(int argc, char *const argv[])
+void pfe_command_stop(int argc, char *const argv[])
 {
int pfe_pe_id, hif_stop_loop = 10;
u32 rx_status;
diff --git a/include/configs/ls1012a2g5rdb.h b/include/configs/ls1012a2g5rdb.h
index bbc3ffd7f0d3..c55fc6487756 100644
--- a/include/configs/ls1012a2g5rdb.h
+++ b/include/configs/ls1012a2g5rdb.h
@@ -79,7 +79,7 @@
"installer=load mmc 0:2 $load_addr "\
   "/flex_installer_arm64.itb; "\
   "bootm $load_addr#$board\0"  \
-   "qspi_bootcmd=pfe stop; echo Trying load from qspi..;"  \
+   "qspi_bootcmd=echo Trying load from qspi..;"\
"sf probe && sf read $load_addr "   \
"$kernel_addr $kernel_size; env exists secureboot " \
"&& sf read $kernelheader_addr_r $kernelheader_addr "   \
@@ -89,11 +89,11 @@
 #undef CONFIG_BOOTCOMMAND
 #ifdef CONFIG_TFABOOT
 #undef QSPI_NOR_BOOTCOMMAND
-#define QSPI_NOR_BOOTCOMMAND "pfe stop;run distro_bootcmd; run qspi_bootcmd; " 
\
+#define QSPI_NOR_BOOTCOMMAND "run distro_bootcmd; run qspi_bootcmd; " \
 "env exists secureboot && esbc_halt;"
 #else
 #if defined(CONFIG_QSPI_BOOT) || defined(CONFIG_SD_BOOT_QSPI)
-#define CONFIG_BOOTCOMMAND "pfe stop;run distro_bootcmd; run qspi_bootcmd; " \
+#define CONFIG_BOOTCOMMAND "run distro_bootcmd; run qspi_bootcmd; " \
   "env exists secureboot && esbc_halt;"
 #endif
 #endif
diff --git a/include/configs/ls1012a_common.h b/include/configs/ls1012a_common.h
index a908b0acb098..6f55acc7db11 100644
--- a/include/configs/ls1012a_common.h
+++ 

RE: [v1 12/17] ddr: altera: Add SDRAM driver for Intel N5X device

2021-04-14 Thread Tan, Ley Foon



> -Original Message-
> From: Lim, Elly Siew Chin 
> Sent: Wednesday, March 31, 2021 10:39 PM
> To: u-boot@lists.denx.de
> Cc: Marek Vasut ; Tan, Ley Foon
> ; See, Chin Liang ;
> Simon Goldschmidt ; Chee, Tien Fong
> ; Westergreen, Dalon
> ; Simon Glass ; Gan,
> Yau Wai ; Lim, Elly Siew Chin
> 
> Subject: [v1 12/17] ddr: altera: Add SDRAM driver for Intel N5X device
> 
> The DDR subsystem in Diamond Mesa is consisted of controller, PHY,
> memory reset manager and memory clock manager.
> 
> Configuration settings of controller, PHY and  memory reset manager
> is come from DDR handoff data in bitstream, which contain the register
> base addresses and user settings from Quartus.
> 
> Configuration settings of memory clock manager is come from the HPS
> handoff data in bitstream, however the register base address is defined
> in device tree.
> 
> The calibration is fully done in HPS, which requires IMEM and DMEM
> binaries loading to PHY SRAM for running this calibration, both
> IMEM and DMEM binaries are also part of bitstream, this bitstream
> would be loaded to OCRAM by SDM, and configured by DDR driver.
> 
> Signed-off-by: Siew Chin Lim 
> Signed-off-by: Tien Fong Chee 
> ---
>  arch/arm/mach-socfpga/include/mach/firewall.h  |6 +
>  .../include/mach/system_manager_soc64.h|   10 +-
>  drivers/ddr/altera/Makefile|3 +-
>  drivers/ddr/altera/sdram_n5x.c | 2316 
> 
>  drivers/ddr/altera/sdram_soc64.c   |   10 +-
>  5 files changed, 2342 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/ddr/altera/sdram_n5x.c

[...]

> --- /dev/null
> +++ b/drivers/ddr/altera/sdram_n5x.c
> @@ -0,0 +1,2316 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020-2021 Intel Corporation 
> + *
> + */
> +
> +#include 
Sorting this.
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include "sdram_soc64.h"
> +#include 
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +/* MPFE NOC registers */
> +#define FPGA2SDRAM_MGR_MAIN_SIDEBANDMGR_FLAGOUTSET0
>   0xF8024050
> +
> +/* Memory reset manager */
> +#define MEM_RST_MGR_STATUS   0x8
> +
> +/* Register and bit in memory reset manager */
> +#define MEM_RST_MGR_STATUS_RESET_COMPLETEBIT(0)
> +#define MEM_RST_MGR_STATUS_PWROKIN_STATUSBIT(1)
> +#define MEM_RST_MGR_STATUS_CONTROLLER_RSTBIT(2)
> +#define MEM_RST_MGR_STATUS_AXI_RST   BIT(3)
> +
> +#define TIMEOUT_200MS 200
> +#define TIMEOUT_5000MS5000
> +
> +/* DDR4 umctl2 */
> +#define DDR4_MSTR_OFFSET 0x0
> +#define DDR4_FREQ_RATIO  BIT(22)
> +
> +#define DDR4_STAT_OFFSET 0x4
> +#define DDR4_STAT_SELFREF_TYPE   (BIT(5) | BIT(4))
> +#define DDR4_STAT_SELFREF_TYPE_SHIFT 4
> +#define DDR4_STAT_OPERATING_MODE (BIT(2) | BIT(1) | BIT(0))
> +
> +#define DDR4_MRCTRL0_OFFSET  0x10
> +#define DDR4_MRCTRL0_MR_TYPE BIT(0)
> +#define DDR4_MRCTRL0_MPR_EN  BIT(1)
> +#define DDR4_MRCTRL0_MR_RANK (BIT(5) | BIT(4))
> +#define DDR4_MRCTRL0_MR_RANK_SHIFT   4
> +#define DDR4_MRCTRL0_MR_ADDR (BIT(15) | BIT(14) | BIT(13) |
> BIT(12))
This is mask value? If yes, can use GENMASK() macro.
Same for the defines below.

> +#define DDR4_MRCTRL0_MR_ADDR_SHIFT   12
> +#define DDR4_MRCTRL0_MR_WR   BIT(31)
> +
> +#define DDR4_MRCTRL1_OFFSET  0x14
> +#define DDR4_MRCTRL1_MR_DATA 0x3
> +
> +#define DDR4_MRSTAT_OFFSET   0x18
> +#define DDR4_MRSTAT_MR_WR_BUSY   BIT(0)
> +
> +#define DDR4_MRCTRL2_OFFSET  0x1C
> +
> +#define DDR4_PWRCTL_OFFSET   0x30
> +#define DDR4_PWRCTL_SELFREF_EN   BIT(0)
> +#define DDR4_PWRCTL_POWERDOWN_EN BIT(1)
> +#define DDR4_PWRCTL_EN_DFI_DRAM_CLK_DISABLE  BIT(3)
> +#define DDR4_PWRCTL_SELFREF_SW   BIT(5)
> +
> +#define DDR4_PWRTMG_OFFSET   0x34
> +#define DDR4_HWLPCTL_OFFSET  0x38
> +#define DDR4_RFSHCTL0_OFFSET 0x50
> +#define DDR4_RFSHCTL1_OFFSET 0x54
> +
> +#define DDR4_RFSHCTL3_OFFSET 0x60
> +#define DDR4_RFSHCTL3_DIS_AUTO_REFRESH   BIT(0)
> +#define DDR4_RFSHCTL3_REFRESH_MODE   (BIT(6) | BIT(5) |
> BIT(4))
> +#define DDR4_RFSHCTL3_REFRESH_MODE_SHIFT 4
> +
> +#define DDR4_ECCCFG0_OFFSET  0x70
> +#define DDR4_ECC_MODE(BIT(2) | BIT(1) | BIT(0))
> +#define DDR4_DIS_SCRUB   BIT(4)
> +#define LPDDR4_ECCCFG0_ECC_REGION_MAP_GRANU_SHIFT30
> +#define LPDDR4_ECCCFG0_ECC_REGION_MAP_SHIFT  8
> +
> +#define DDR4_ECCCFG1_OFFSET  0x74
> +#define LPDDR4_ECCCFG1_ECC_REGIONS_PARITY_LOCK   BIT(4)
> +
> +#define DDR4_CRCPARCTL0_OFFSET   0xC0
> +#define 

Re: [RFC PATCH v5 2/2] board: sifive: unmatched: clear feature disable CSR

2021-04-14 Thread Green Wan
On Wed, Apr 14, 2021 at 11:22 AM Rick Chen  wrote:
>
> Hi Green,
>
> > From: Green Wan [mailto:green@sifive.com]
> > Sent: Tuesday, April 13, 2021 5:32 PM
> > Cc: Green Wan; Sean Anderson; Bin Meng; Rick Jian-Zhi Chen(陳建志); Paul 
> > Walmsley; Pragnesh Patel; Bin Meng; Simon Glass; Atish Patra; Leo Yu-Chi 
> > Liang(梁育齊); Brad Kim; open list
> > Subject: [RFC PATCH v5 2/2] board: sifive: unmatched: clear feature disable 
> > CSR
> >
> > Clear feature disable CSR to turn on all features of hart. The detail
> > is specified at section, 'SiFive Feature Disable CSR', in user manual
> >
> > https://sifive.cdn.prismic.io/sifive/aee0dd4c-d156-496e-a6c4-db0cf54bbe68_sifive_U74MC_rtl_full_20G1.03.00_manual.pdf
> >
> > Signed-off-by: Green Wan 
> > Reviewed-by: Sean Anderson 
> > Reviewed-by: Bin Meng 
> > ---
> >  board/sifive/unmatched/spl.c | 15 +++
>
> Is this CSR depends on unmatched board ?
> I just wonder why not add in /arch/riscv/cpu/ and create fu74, just
> like /arch/riscv/cpu/fu540/spl.c ?

You're right. This CSR doesn't depend on the Unmatched. Moving to
arch/riscv/cpu/fu740/spl.c makes more sense. will do it.


>
> Thanks,
> Rick
>
> >  1 file changed, 15 insertions(+)
> >
> > diff --git a/board/sifive/unmatched/spl.c b/board/sifive/unmatched/spl.c
> > index 5e1333b09a..ed48dac511 100644
> > --- a/board/sifive/unmatched/spl.c
> > +++ b/board/sifive/unmatched/spl.c
> > @@ -12,6 +12,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -22,6 +23,20 @@
> >  #define MODE_SELECT_SD 0xb
> >  #define MODE_SELECT_MASK   GENMASK(3, 0)
> >
> > +#define CSR_U74_FEATURE_DISABLE0x7c1
> > +
> > +void harts_early_init(void)
> > +{
> > +   /*
> > +* Feature Disable CSR
> > +*
> > +* Clear feature disable CSR to '0' to turn on all features for
> > +* each core. This operation must be in M-mode.
> > +*/
> > +   if (CONFIG_IS_ENABLED(RISCV_MMODE))
> > +   csr_write(CSR_U74_FEATURE_DISABLE, 0);
> > +}
> > +
> >  int spl_board_init_f(void)
> >  {
> > int ret;
> > --
> > 2.31.0


[PATCH v4 2/2] allow opting out of WATCHDOG_RESET() from timer interrupt

2021-04-14 Thread Rasmus Villemoes
Having WATCHDOG_RESET() called automatically from the timer interrupt
runs counter to the idea of a watchdog device - if the board runs into
an infinite loops with interrupts still enabled, the watchdog will
never fire.

When using CONFIG_(SPL_)WDT, the watchdog_reset function is a lot more
complicated than just poking a few SOC-specific registers - it
involves accessing all kinds of global data, and if the interrupt
happens at the wrong time (say, in the middle of an WATCHDOG_RESET()
call from ordinary code), that can end up corrupting said global data.

Allow the board to opt out of calling WATCHDOG_RESET() from the timer
interrupt handler by setting CONFIG_SYS_WATCHDOG_FREQ to 0 - as that
setting is currently nonsensical (it would be compile-time
divide-by-zero), it cannot affect any existing boards.

Add documentation for both the existing and extended meaning of
CONFIG_SYS_WATCHDOG_FREQ.

Signed-off-by: Rasmus Villemoes 
---
 README| 9 +
 arch/m68k/lib/time.c  | 2 +-
 arch/powerpc/lib/interrupts.c | 2 +-
 drivers/timer/mpc83xx_timer.c | 2 +-
 4 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/README b/README
index a565748e43..ad13092bbb 100644
--- a/README
+++ b/README
@@ -747,6 +747,15 @@ The following options need to be configured:
SoC, then define this variable and provide board
specific code for the "hw_watchdog_reset" function.
 
+   CONFIG_SYS_WATCHDOG_FREQ
+   Some platforms automatically call WATCHDOG_RESET()
+   from the timer interrupt handler every
+   CONFIG_SYS_WATCHDOG_FREQ interrupts. If not set by the
+   board configuration file, a default of CONFIG_SYS_HZ/2
+   (i.e. 500) is used. Setting CONFIG_SYS_WATCHDOG_FREQ
+   to 0 disables calling WATCHDOG_RESET() from the timer
+   interrupt.
+
 - Real-Time Clock:
 
When CONFIG_CMD_DATE is selected, the type of the RTC
diff --git a/arch/m68k/lib/time.c b/arch/m68k/lib/time.c
index cbe29e72a8..ebb2ac54db 100644
--- a/arch/m68k/lib/time.c
+++ b/arch/m68k/lib/time.c
@@ -71,7 +71,7 @@ void dtimer_interrupt(void *not_used)
timestamp++;
 
#if defined(CONFIG_WATCHDOG) || defined (CONFIG_HW_WATCHDOG)
-   if ((timestamp % (CONFIG_SYS_WATCHDOG_FREQ)) == 0) {
+   if (CONFIG_SYS_WATCHDOG_FREQ && (timestamp % 
(CONFIG_SYS_WATCHDOG_FREQ)) == 0) {
WATCHDOG_RESET ();
}
#endif/* CONFIG_WATCHDOG || CONFIG_HW_WATCHDOG */
diff --git a/arch/powerpc/lib/interrupts.c b/arch/powerpc/lib/interrupts.c
index 73f270002c..5ba4cd0c13 100644
--- a/arch/powerpc/lib/interrupts.c
+++ b/arch/powerpc/lib/interrupts.c
@@ -80,7 +80,7 @@ void timer_interrupt(struct pt_regs *regs)
timestamp++;
 
 #if defined(CONFIG_WATCHDOG) || defined (CONFIG_HW_WATCHDOG)
-   if ((timestamp % (CONFIG_SYS_WATCHDOG_FREQ)) == 0)
+   if (CONFIG_SYS_WATCHDOG_FREQ && (timestamp % 
(CONFIG_SYS_WATCHDOG_FREQ)) == 0)
WATCHDOG_RESET ();
 #endif/* CONFIG_WATCHDOG || CONFIG_HW_WATCHDOG */
 
diff --git a/drivers/timer/mpc83xx_timer.c b/drivers/timer/mpc83xx_timer.c
index 9adb4b4cab..952293195f 100644
--- a/drivers/timer/mpc83xx_timer.c
+++ b/drivers/timer/mpc83xx_timer.c
@@ -175,7 +175,7 @@ void timer_interrupt(struct pt_regs *regs)
priv->timestamp++;
 
 #if defined(CONFIG_WATCHDOG) || defined(CONFIG_HW_WATCHDOG)
-   if ((priv->timestamp % (CONFIG_SYS_WATCHDOG_FREQ)) == 0)
+   if (CONFIG_SYS_WATCHDOG_FREQ && (priv->timestamp % 
(CONFIG_SYS_WATCHDOG_FREQ)) == 0)
WATCHDOG_RESET();
 #endif/* CONFIG_WATCHDOG || CONFIG_HW_WATCHDOG */
 
-- 
2.29.2



[PATCH v4 0/2] allow opting out of WATCHDOG_RESET() from timer interrupt

2021-04-14 Thread Rasmus Villemoes
This is a resend of v3 from a year ago. Please consider applying.

v4: rebase to current master.

v3: add fixup patch for mpc83xx_timer, add documentation hunk to
README, also update m68k instead of only ppc.

v2: add documentation comment

Rasmus Villemoes (2):
  timer: mpc83xx_timer: fix build with CONFIG_{HW_,}WATCHDOG
  allow opting out of WATCHDOG_RESET() from timer interrupt

 README| 9 +
 arch/m68k/lib/time.c  | 2 +-
 arch/powerpc/lib/interrupts.c | 2 +-
 drivers/timer/mpc83xx_timer.c | 6 +-
 4 files changed, 16 insertions(+), 3 deletions(-)

-- 
2.29.2



Re: [PATCH 11/13] board: synquacer: Add DeveloperBox 96boards EE support

2021-04-14 Thread Takahiro Akashi
On Wed, Apr 14, 2021 at 03:29:23PM +0900, Masami Hiramatsu wrote:
> Hi Takahiro,
> 
> 2021年4月14日(水) 13:48 Takahiro Akashi :
> >
> > > > So why not define UEFI load options (BOOT) and use UEFI boot manager
> > > > ("bootefi bootmgr")?
> > > > That is the way how UEFI (at least boot manager) boots the kernel.
> > >
> > > Good point! Actually, I'm not sure how to define the BOOT in
> > > config.h (I only
> > > know how to include efivars file when build). Could you tell me how to do 
> > > it?
> > > I would like to rewrite the default boot commands.
> >
> > For example,
> > => efidebug boot add 1 USBBOOT usb 0:1 /EFI/BOOT/bootaa64.efi 
> > => efidebug boot add 2 MMCBOOT mmc 0:1 /EFI/BOOT/bootaa64.efi 
> > => efidebug boot order 1 2
> > => bootefi bootmgr
> 
> Hmm, but this can not be embedded in the build process, can this?

Probably there are a couple of ways;
You may put them in "pre_boot_environment_command" if you don't mind :)

But it would be best to run them as part of OS installation process.
Or you may want to provide a default efivars file(?).

> >
> > Since "BOOTxxx" are non-volatile variables, we don't have to
> > set them again once those commands are run.
> 
> What is the default behavior of "bootefi bootmgr" if there is no
> BOOT is set?

Nothing will happen.

> If it just do nothing and exit, I think I can add it to the top of
> CONFIG_BOOTCOMMAND
> so that U-Boot can try it first.
> (BOOT will be set by user after boot)
> 
> > But distro_bootcmd can also detect and try to boot "bootaa64.efi" anyway.
> > (I'm not sure about the order of devices to detect though.)
> 
> Hmm, interesting. OK, I'll try to enable distro_bootcmd.

I'm pretty sure it will work.

-Takahiro Akashi

> Thank you,
> 
> >
> > -Takahiro Akashi
> >
> > > Thank you,
> > >
> > > --
> > > Masami Hiramatsu
> 
> 
> 
> -- 
> Masami Hiramatsu


[PATCH] efi_loader: esrt: Remove incorrect invocations of EFI_CALL macro

2021-04-14 Thread Sughosh Ganu
Remove function invocations using the EFI_CALL macro for those
functions that do not have an EFI_ENTRY call in their definition. Such
functions can use u-boot api's which rely on u-boot global data(gd)
pointer. The Arm and RiscV architectures maintain a separate gd
pointer, one for u-boot, and a separate gd for the efi application.

Calling a function through the EFI_CALL macro changes the gd pointer
to that used for the efi application, with u-boot gd being
unavailable. Any function then trying to dereference u-boot's gd will
result in an abort.

Fix this issue by removing the EFI_CALL macro for all of such
functions which do not begin by an EFI_ENTRY function call.

Signed-off-by: Sughosh Ganu 
---

I have squashed the earlier patch[1] into this one. This patch should
supersede the earlier patch.

[1] - 
https://patchwork.ozlabs.org/project/uboot/patch/20210410150948.24240-1-sughosh.g...@linaro.org/

 lib/efi_loader/efi_esrt.c | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/lib/efi_loader/efi_esrt.c b/lib/efi_loader/efi_esrt.c
index 40f53260e4..3ca55ce23a 100644
--- a/lib/efi_loader/efi_esrt.c
+++ b/lib/efi_loader/efi_esrt.c
@@ -139,7 +139,7 @@ efi_status_t efi_esrt_allocate_install(u32 num_entries)
 
/* If there was a previous ESRT, deallocate its memory now. */
if (esrt)
-   ret = EFI_CALL(efi_free_pool(esrt));
+   ret = efi_free_pool(esrt);
 
esrt = new_esrt;
 
@@ -253,8 +253,8 @@ efi_status_t efi_esrt_add_from_fmp(struct 
efi_firmware_management_protocol *fmp)
return EFI_INVALID_PARAMETER;
}
 
-   ret = EFI_CALL(efi_allocate_pool(EFI_BOOT_SERVICES_DATA, info_size,
-(void **)_info));
+   ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, info_size,
+   (void **)_info);
if (ret != EFI_SUCCESS) {
EFI_PRINT("ESRT failed to allocate memory for image info.\n");
return ret;
@@ -298,7 +298,7 @@ efi_status_t efi_esrt_add_from_fmp(struct 
efi_firmware_management_protocol *fmp)
}
 
 out:
-   EFI_CALL(efi_free_pool(img_info));
+   efi_free_pool(img_info);
return EFI_SUCCESS;
 }
 
@@ -384,8 +384,8 @@ efi_status_t efi_esrt_populate(void)
goto out;
}
 
-   ret = EFI_CALL(efi_allocate_pool(EFI_BOOT_SERVICES_DATA, 
info_size,
-(void **)_info));
+   ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, info_size,
+   (void **)_info);
if (ret != EFI_SUCCESS) {
EFI_PRINT("ESRT failed to allocate memory for image 
info\n");
goto out;
@@ -405,13 +405,13 @@ efi_status_t efi_esrt_populate(void)
 
if (ret != EFI_SUCCESS) {
EFI_PRINT("ESRT failed to obtain image info from 
FMP\n");
-   EFI_CALL(efi_free_pool(img_info));
+   efi_free_pool(img_info);
goto out;
}
 
num_entries += desc_count;
 
-   EFI_CALL(efi_free_pool(img_info));
+   efi_free_pool(img_info);
}
 
EFI_PRINT("ESRT create table with %u entries\n", num_entries);
@@ -430,9 +430,9 @@ efi_status_t efi_esrt_populate(void)
 */
it_handle = base_handle;
for (u32 idx = 0; idx < no_handles; idx++, it_handle++) {
-   ret = EFI_CALL(efi_search_protocol(*it_handle,
-  
_guid_firmware_management_protocol,
-  ));
+   ret = efi_search_protocol(*it_handle,
+ 
_guid_firmware_management_protocol,
+ );
 
if (ret != EFI_SUCCESS) {
EFI_PRINT("ESRT unable to find FMP handle (%u)\n",
@@ -448,7 +448,7 @@ efi_status_t efi_esrt_populate(void)
 
 out:
 
-   EFI_CALL(efi_free_pool(base_handle));
+   efi_free_pool(base_handle);
 
return ret;
 }
@@ -490,8 +490,8 @@ efi_status_t efi_esrt_register(void)
return ret;
}
 
-   ret = EFI_CALL(efi_create_event(EVT_NOTIFY_SIGNAL, TPL_CALLBACK,
-   efi_esrt_new_fmp_notify, NULL, NULL, 
));
+   ret = efi_create_event(EVT_NOTIFY_SIGNAL, TPL_CALLBACK,
+  efi_esrt_new_fmp_notify, NULL, NULL, );
if (ret != EFI_SUCCESS) {
EFI_PRINT("ESRT failed to create event\n");
return ret;
-- 
2.17.1



Re: [PATCH 11/13] board: synquacer: Add DeveloperBox 96boards EE support

2021-04-14 Thread Masami Hiramatsu
Hi Takahiro,

2021年4月14日(水) 13:48 Takahiro Akashi :
>
> > > So why not define UEFI load options (BOOT) and use UEFI boot manager
> > > ("bootefi bootmgr")?
> > > That is the way how UEFI (at least boot manager) boots the kernel.
> >
> > Good point! Actually, I'm not sure how to define the BOOT in
> > config.h (I only
> > know how to include efivars file when build). Could you tell me how to do 
> > it?
> > I would like to rewrite the default boot commands.
>
> For example,
> => efidebug boot add 1 USBBOOT usb 0:1 /EFI/BOOT/bootaa64.efi 
> => efidebug boot add 2 MMCBOOT mmc 0:1 /EFI/BOOT/bootaa64.efi 
> => efidebug boot order 1 2
> => bootefi bootmgr

Hmm, but this can not be embedded in the build process, can this?

>
> Since "BOOTxxx" are non-volatile variables, we don't have to
> set them again once those commands are run.

What is the default behavior of "bootefi bootmgr" if there is no
BOOT is set?
If it just do nothing and exit, I think I can add it to the top of
CONFIG_BOOTCOMMAND
so that U-Boot can try it first.
(BOOT will be set by user after boot)

> But distro_bootcmd can also detect and try to boot "bootaa64.efi" anyway.
> (I'm not sure about the order of devices to detect though.)

Hmm, interesting. OK, I'll try to enable distro_bootcmd.

Thank you,

>
> -Takahiro Akashi
>
> > Thank you,
> >
> > --
> > Masami Hiramatsu



-- 
Masami Hiramatsu