Re: Xilinx ZynqMP - ZCU102: SPL not able to use MMC/SD card

2021-11-29 Thread Michal Simek

Hi,

On 11/30/21 00:01, Daniel Cizinsky wrote:

On Tue, Nov 09, 2021 at 09:21:41PM +0100, Daniel Cizinsky wrote:

Hi!

I am trying to switch to as much current vanilla SW as possible on Xilinx
ZCU102 evaluation board.  But I am stuck at a very early stage.

I've got U-Boot + SPL + Linux kernel & userspace compilled, but even after
trying hard I had no success in running SPL to read ATF, not even speaking
about the main U-Boot.

I don't even know whether my e-mail went through to the conference. I just
wanted to know, if not being able to use SD card on this board using a
vanilla modern U-Boot SPL is a known bug, or if there are any people
outthere using it successfully. Probably not, or they are not interested in
sharing the simple fact.

With rc3 I'm getting exactly the same errors:

U-Boot SPL 2022.01-rc3 (Nov 29 2021 - 23:50:47 +0100)
PMUFW:  v1.1
Loading new PMUFW cfg obj (2024 bytes)
Silicon version:3
EL Level:   EL3
Multiboot:  0
Trying to boot from MMC2
spl: could not initialize mmc. error: -19
Trying to boot from MMC1
arasan_sdhci mmc@ff17: Using predefined clock phase for clk-phase-legacy = 
0 0
arasan_sdhci mmc@ff17: Using predefined clock phase for clk-phase-mmc-hs = 
63 72
arasan_sdhci mmc@ff17: Using predefined clock phase for clk-phase-sd-hs = 
63 60
arasan_sdhci mmc@ff17: Using predefined clock phase for clk-phase-uhs-sdr12 
= 0 0
arasan_sdhci mmc@ff17: Using predefined clock phase for clk-phase-uhs-sdr25 
= 63 60
arasan_sdhci mmc@ff17: Using predefined clock phase for clk-phase-uhs-sdr50 
= 0 72
arasan_sdhci mmc@ff17: Using predefined clock phase for 
clk-phase-uhs-sdr104 = 0 135
arasan_sdhci mmc@ff17: Using predefined clock phase for clk-phase-uhs-ddr50 
= 183 48
arasan_sdhci mmc@ff17: Using predefined clock phase for clk-phase-mmc-ddr52 
= 54 72
arasan_sdhci mmc@ff17: Using predefined clock phase for clk-phase-mmc-hs200 
= 0 135
arasan_sdhci mmc@ff17: Using predefined clock phase for clk-phase-mmc-hs400 
= 0 0
arasan_sdhci mmc@ff17: arasan_sdhci_set_tapdelay, host:mmc@ff17, mode:0
CMD_SEND:0
 ARG  0x
 MMC_RSP_NONE
CMD_SEND:8
 ARG  0x01aa
 RET  -110
CMD_SEND:55
 ARG  0x
 RET  -110
CMD_SEND:0
 ARG  0x
 MMC_RSP_NONE
CMD_SEND:1
 ARG  0x
 RET  -110
Card did not respond to voltage select! : -110
mmc_init: -95, time 23
spl: mmc init failed with error: -95
SPL: Unsupported Boot Device 0
SPL: failed to boot from all boot devices (err=-6)
### ERROR ### Please RESET the board ###



I did try latest yesterday and didn't see any issue. How do you build 
u-boot? And what board version do you have?


Log below.

Thanks,
Michal


U-Boot SPL 2022.01-rc2-00097-g657a869c04e9 (Nov 29 2021 - 14:15:08 +0100)
PMUFW:  v1.1
Loading new PMUFW cfg obj (2032 bytes)
Silicon version:3
EL Level:   EL3
Chip ID:zu9eg
Multiboot:  0
Trying to boot from MMC2
spl: could not initialize mmc. error: -19
Trying to boot from MMC1
spl_load_image_fat_os: error reading image u-boot.bin, err - -2
NOTICE:  BL31: Secure code at 0x7e00
NOTICE:  BL31: Non secure code at 0x800
NOTICE:  BL31: v2.4(release):v2.4-594-g82a773bd39b4
NOTICE:  BL31: Built : 07:54:47, Oct 20 2021


U-Boot 2022.01-rc2-00097-g657a869c04e9 (Nov 29 2021 - 14:15:08 +0100)

CPU:   ZynqMP
Silicon: v3
Model: ZynqMP ZCU102 Rev1.0
Board: Xilinx ZynqMP
DRAM:  4 GiB
PMUFW:  v1.1
Xilinx I2C Legacy format at nvmem0:
 Board name:zcu102
 Board rev: 1.0
 Board SN:  847316301727-67998
 Ethernet mac:  00:0a:35:03:70:f6
EL Level:   EL2
Chip ID:zu9eg
NAND:  0 MiB
MMC:   mmc@ff17: 0
Loading Environment from FAT... *** Error - No Valid Environment Area found
*** Warning - bad env area, using default environment

In:serial
Out:   serial
Err:   serial
Bootmode: LVL_SHFT_SD_MODE1
Reset reason:   SRST
Net:
ZYNQ GEM: ff0e, mdio bus ff0e, phyaddr 12, interface rgmii-id
eth0: ethernet@ff0e
scanning bus for devices...
SATA link 0 timeout.
SATA link 1 timeout.
AHCI 0001.0301 32 slots 2 ports 6 Gbps 0x3 impl SATA mode
flags: 64bit ncq pm clo only pmp fbss pio slum part ccc apst
starting USB...
Bus dwc3@fe20: Register 2000440 NbrPorts 2
Starting the controller
USB XHCI 1.00
scanning bus dwc3@fe20 for devices... 1 USB Device(s) found
   scanning usb for storage devices... 0 Storage Device(s) found
Hit any key to stop autoboot:  0
ZynqMP>



Re: [PATCH] board: ti: am43xx: pass boot device information from SPL to U-Boot proper

2021-11-29 Thread Josef Luštický
>
>
> Hi Josef
>
> On Fri, Nov 26, 2021 at 10:56 AM Josef Lusticky  wrote:
> >
> > TI AM43xx SoC supports various boot devices (peripherals).
> > There is already handoff mechanism prepared to allow passing
> > the information which boot device was used to load the SPL.
> >
> > Use the handoff mechanism to pass this information to U-Boot proper
> > and set the "boot_device" environment variable in board_late_init.
> >
> > Signed-off-by: Josef Lusticky 
> > Cc: Tom Rini 
> > Cc: Lokesh Vutla 
> > Cc: Michael Trimarchi 
> > ---
> >
> > I use the boot_device variable later in U-Boot scripting - e.g. to avoid
> running
> > bootcmd when the SPL was loaded from UART but run it when loaded from
> MMC.
> > Only AM43xx is supported by this patch, but for other TI SoCs
> > the procedure should be the same:
> > - figure out supported boot devices from
> arch/arm/include/asm/arch-am33xx/spl.h
> > or arch/arm/include/asm/arch-omapX/spl.h
> > - implement setting the boot_device env variable in board_late_init()
> >
> > You'll need to enable the following in the config:
> > CONFIG_BLOBLIST=y (required by CONFIG_HANDOFF)
> > CONFIG_HANDOFF=y
> > CONFIG_BLOBLIST_ADDR=0x8700 (i set this based on other values
> defined by
> > the DEFAULT_LINUX_BOOT_ENV macro in include/configs/ti_armv7_common.h,
> you
> > may want to use a different address)
> >
> >  arch/arm/include/asm/handoff.h|  3 +++
> >  arch/arm/mach-omap2/boot-common.c |  9 
> >  board/ti/am43xx/board.c   | 38 +++
> >  3 files changed, 50 insertions(+)
> >
> > diff --git a/arch/arm/include/asm/handoff.h
> b/arch/arm/include/asm/handoff.h
> > index 0790d2ab1e..1b7aa432a2 100644
> > --- a/arch/arm/include/asm/handoff.h
> > +++ b/arch/arm/include/asm/handoff.h
> > @@ -16,6 +16,9 @@
> >   */
> >  struct arch_spl_handoff {
> > ulong usable_ram_top;
> > +#ifdef CONFIG_ARCH_OMAP2PLUS
> > +   u32 omap_boot_device;
> > +#endif
> >  };
>
> Simon is working on a more structured way to pass arguments in
> multi-stage boot. I forget to read all the patches.
> Anyway adding a specific handoff parameter for one architecture makes
> no such sense. I will remind you that this
> was already implemented in the past using dts injection (something
> that I don't  like)
>
> Hi Michael,
thank you for your response.

I agree that such support for a single architecture makes no such sense.
I tried to make the same thing work on SUNXI and Atmel platforms.
Atmel uses its at91bootstrap, so it's rather complicated and would require
modification of both U-Boot and at91bootstrap.
The SUNXI platform worked the exact same way, but only for SPI NOR and MMC
boot_device.
I've got Allwinner A13 to test and apart from SPI NOR and MMC, all SUNXI
SoCs also support USB "FEL" boot.
However I was not able to make my patch work for boot_device FEL detection,
because on this particular platform it works a bit different:
The SPL is transferred over USB and booted from FEL. After the clock setup,
pinmux setup, etc. the SPL detects that it should continue booting from FEL
and it returns back to FEL which loads U-Boot proper next.
Unfortunately the return back to FEL happens before the HANDOFF is set.

I patched the code to return to FEL after the HANDOFF, but I am almost
certain that it would break other architectures.
Here's the code change:

diff --git a/common/spl/spl.c b/common/spl/spl.c
index a0a608fd77..b1f872ad0b 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -708,12 +708,6 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
spl_image.boot_device = BOOT_DEVICE_NONE;
board_boot_order(spl_boot_list);

-   if (boot_from_devices(_image, spl_boot_list,
- ARRAY_SIZE(spl_boot_list))) {
-   puts(SPL_TPL_PROMPT "failed to boot from all boot
devices\n");
-   hang();
-   }
-
spl_perform_fixups(_image);
if (CONFIG_IS_ENABLED(HANDOFF)) {
ret = write_spl_handoff();
@@ -728,6 +722,12 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
   ret);
}

+   if (boot_from_devices(_image, spl_boot_list,
+ ARRAY_SIZE(spl_boot_list))) {
+   puts(SPL_TPL_PROMPT "failed to boot from all boot
devices\n");
+   hang();
+   }
+
 #ifdef CONFIG_CPU_V7M
spl_image.entry_point |= 0x1;
 #endif


Best regards
Josef


> Michael
>
> >
> >  #endif
> > diff --git a/arch/arm/mach-omap2/boot-common.c
> b/arch/arm/mach-omap2/boot-common.c
> > index 1268a32503..191bb2a42d 100644
> > --- a/arch/arm/mach-omap2/boot-common.c
> > +++ b/arch/arm/mach-omap2/boot-common.c
> > @@ -236,3 +236,12 @@ void arch_preboot_os(void)
> > ahci_reset((void __iomem *)DWC_AHSATA_BASE);
> >  }
> >  #endif
> > +
> > +#if CONFIG_IS_ENABLED(HANDOFF)
> > +int handoff_arch_save(struct spl_handoff *ho)
> > +{
> > +   ho->arch.omap_boot_device = spl_boot_device();
> > +
> > +   return 0;
> > 

Re: [PATCH] serial: a37xx: Reset whole UART when changing parent clock from TBG to XTAL

2021-11-29 Thread Stefan Roese

On 11/15/21 13:48, Pali Rohár wrote:

Sometimes UART stops transmitting characters after UART clock is changed
back to XTAL. In this state UART fifo is always full. Kernel during early
boot wants to print output on UART and is waiting for non-empty UART fifo.
Which leads to CPU hangup without any (debug) output on UART.

Marvell Armada 3700 Functional Specifications says that for programming
fractional divisor registers it is required to disable UART, enable
loopback mode, reset fifos, program registers, disable loopback mode,
release reset of fifos and enable UART.

But these steps do not fix above mentioned issue that UART hangup. Also
gating UART clock does not help. And even resetting UART state machines do
not help.

Experiments showed that UART fifo is unblocked after board is being reset
(during board reset UART HW transmit UART fifo even CPU is not executing
kernel/bootloader anymore).

And another experiments showed that same workaround can be achieved also
by external reset of UART HW (without need to reset board).

So do not implement any of "Marvell recommended" steps from Functional
Specifications as they do not work. And rather prior changing parent clock
back to XTAL, do external reset of UART HW. This operation also resets all
UART registers, so basically it also sets UART clock to default, which is
XTAL. It is unknown why UART hangups and enters such broken state.

Signed-off-by: Pali Rohár 
---
  drivers/serial/serial_mvebu_a3700.c | 13 -
  1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/serial/serial_mvebu_a3700.c 
b/drivers/serial/serial_mvebu_a3700.c
index 6bca8e4b7e2d..93e4d38d34c7 100644
--- a/drivers/serial/serial_mvebu_a3700.c
+++ b/drivers/serial/serial_mvebu_a3700.c
@@ -9,6 +9,7 @@
  #include 
  #include 
  #include 
+#include 
  
  struct mvebu_plat {

void __iomem *base;
@@ -213,6 +214,7 @@ static int mvebu_serial_remove(struct udevice *dev)
u32 new_oversampling;
u32 oversampling;
u32 d1, d2;
+   u32 nb_rst;
  
  	/*

 * Switch UART base clock back to XTAL because older Linux kernel
@@ -260,12 +262,22 @@ static int mvebu_serial_remove(struct udevice *dev)
return 0;
}
  
+	/* wait until TX empty */

while (!(readl(base + UART_STATUS_REG) & UART_STATUS_TX_EMPTY))
;
  
+	/* external reset of UART via North Bridge Peripheral */

+   nb_rst = readl(MVEBU_REGISTER(0x12400));
+   writel(nb_rst & ~BIT(3), MVEBU_REGISTER(0x12400));
+   writel(nb_rst | BIT(3), MVEBU_REGISTER(0x12400));


I'm not that fond of this type of access to some "external" register in
this driver. But adding some reset driver might be a bit too much here.
So:

Reviewed-by: Stefan Roese 

Thanks,
Stefan



+
+   /* set baudrate and oversampling */
writel(new_divider, base + UART_BAUD_REG);
writel(new_oversampling, base + UART_POSSR_REG);
  
+	/* No Parity, 1 Stop */

+   writel(0, base + UART_CTRL_REG);
+
return 0;
  }
  
@@ -305,7 +317,6 @@ U_BOOT_DRIVER(serial_mvebu) = {

  #ifdef CONFIG_DEBUG_MVEBU_A3700_UART
  
  #include 

-#include 
  
  static inline void _debug_uart_init(void)

  {



Viele Grüße,
Stefan Roese

--
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 u-boot-marvell v2 9/9] arm: mvebu: spl: Fix 100 column exceeds

2021-11-29 Thread Stefan Roese

On 11/26/21 15:37, Marek Behún wrote:

From: Marek Behún 

Fix 100 column exceeds in arch/arm/mach-mvebu/spl.c.

Signed-off-by: Marek Behún 


Reviewed-by: Stefan Roese 

Thanks,
Stefan


---
  arch/arm/mach-mvebu/spl.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c
index 7dbe8eeba3..7450e1e3da 100644
--- a/arch/arm/mach-mvebu/spl.c
+++ b/arch/arm/mach-mvebu/spl.c
@@ -47,7 +47,8 @@
  #if defined(CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR) && 
CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR != 0
  #error CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR must be set to 0
  #endif
-#if defined(CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET) && 
CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET != 0
+#if defined(CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET) && \
+CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET != 0
  #error CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET must be set to 0
  #endif
  #endif
@@ -58,7 +59,8 @@
   * set to 1. Otherwise U-Boot SPL would not be able to load U-Boot proper.
   */
  #ifdef CONFIG_SPL_SATA
-#if !defined(CONFIG_SPL_SATA_RAW_U_BOOT_USE_SECTOR) || 
!defined(CONFIG_SPL_SATA_RAW_U_BOOT_SECTOR) || 
CONFIG_SPL_SATA_RAW_U_BOOT_SECTOR != 1
+#if !defined(CONFIG_SPL_SATA_RAW_U_BOOT_USE_SECTOR) || \
+!defined(CONFIG_SPL_SATA_RAW_U_BOOT_SECTOR) || 
CONFIG_SPL_SATA_RAW_U_BOOT_SECTOR != 1
  #error CONFIG_SPL_SATA_RAW_U_BOOT_SECTOR must be set to 1
  #endif
  #endif



Viele Grüße,
Stefan Roese

--
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 u-boot-marvell v2 8/9] arm: mvebu: spl: Use IS_ENABLED() instead of #ifdef where possible

2021-11-29 Thread Stefan Roese

On 11/26/21 15:37, Marek Behún wrote:

From: Marek Behún 

Use the preferred
   if (IS_ENABLED(X))
instead of
   #ifdef X
where possible.

There are still places where this is not possible or is more complicated
to convert in this file. Leave those be for now.

Signed-off-by: Marek Behún 


Nice, thanks.

Reviewed-by: Stefan Roese 

Thanks,
Stefan


---
  arch/arm/mach-mvebu/spl.c | 43 ---
  1 file changed, 18 insertions(+), 25 deletions(-)

diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c
index 97d7aea179..7dbe8eeba3 100644
--- a/arch/arm/mach-mvebu/spl.c
+++ b/arch/arm/mach-mvebu/spl.c
@@ -150,26 +150,24 @@ int spl_parse_board_header(struct spl_image_info 
*spl_image,
return -EINVAL;
}
  
-#ifdef CONFIG_SPL_SPI_FLASH_SUPPORT

-   if (bootdev->boot_device == BOOT_DEVICE_SPI &&
+   if (IS_ENABLED(CONFIG_SPL_SPI_FLASH_SUPPORT) &&
+   bootdev->boot_device == BOOT_DEVICE_SPI &&
mhdr->blockid != IBR_HDR_SPI_ID) {
printf("ERROR: Wrong blockid (%u) in SPI kwbimage\n",
   mhdr->blockid);
return -EINVAL;
}
-#endif
  
-#ifdef CONFIG_SPL_SATA

-   if (bootdev->boot_device == BOOT_DEVICE_SATA &&
+   if (IS_ENABLED(CONFIG_SPL_SATA) &&
+   bootdev->boot_device == BOOT_DEVICE_SATA &&
mhdr->blockid != IBR_HDR_SATA_ID) {
printf("ERROR: Wrong blockid (%u) in SATA kwbimage\n",
   mhdr->blockid);
return -EINVAL;
}
-#endif
  
-#ifdef CONFIG_SPL_MMC

-   if ((bootdev->boot_device == BOOT_DEVICE_MMC1 ||
+   if (IS_ENABLED(CONFIG_SPL_MMC) &&
+   (bootdev->boot_device == BOOT_DEVICE_MMC1 ||
 bootdev->boot_device == BOOT_DEVICE_MMC2 ||
 bootdev->boot_device == BOOT_DEVICE_MMC2_2) &&
mhdr->blockid != IBR_HDR_SDIO_ID) {
@@ -177,18 +175,16 @@ int spl_parse_board_header(struct spl_image_info 
*spl_image,
   mhdr->blockid);
return -EINVAL;
}
-#endif
  
  	spl_image->offset = mhdr->srcaddr;
  
-#ifdef CONFIG_SPL_SATA

/*
 * For SATA srcaddr is specified in number of sectors.
 * The main header is must be stored at sector number 1.
 * This expects that sector size is 512 bytes and recalculates
 * data offset to bytes relative to the main header.
 */
-   if (mhdr->blockid == IBR_HDR_SATA_ID) {
+   if (IS_ENABLED(CONFIG_SPL_SATA) && mhdr->blockid == IBR_HDR_SATA_ID) {
if (spl_image->offset < 1) {
printf("ERROR: Wrong SATA srcaddr (%u) in kwbimage\n",
   spl_image->offset);
@@ -197,17 +193,14 @@ int spl_parse_board_header(struct spl_image_info 
*spl_image,
spl_image->offset -= 1;
spl_image->offset *= 512;
}
-#endif
  
-#ifdef CONFIG_SPL_MMC

/*
 * For SDIO (eMMC) srcaddr is specified in number of sectors.
 * This expects that sector size is 512 bytes and recalculates
 * data offset to bytes.
 */
-   if (mhdr->blockid == IBR_HDR_SDIO_ID)
+   if (IS_ENABLED(CONFIG_SPL_MMC) && mhdr->blockid == IBR_HDR_SDIO_ID)
spl_image->offset *= 512;
-#endif
  
  	if (spl_image->offset % 4 != 0) {

printf("ERROR: Wrong srcaddr (%u) in kwbimage\n",
@@ -340,17 +333,17 @@ void board_init_f(ulong dummy)
timer_init();
  
  	/* Armada 375 does not support SerDes and DDR3 init yet */

-#if !defined(CONFIG_ARMADA_375)
-   /* First init the serdes PHY's */
-   serdes_phy_config();
-
-   /* Setup DDR */
-   ret = ddr3_init();
-   if (ret) {
-   debug("ddr3_init() failed: %d\n", ret);
-   hang();
+   if (!IS_ENABLED(CONFIG_ARMADA_375)) {
+   /* First init the serdes PHY's */
+   serdes_phy_config();
+
+   /* Setup DDR */
+   ret = ddr3_init();
+   if (ret) {
+   debug("ddr3_init() failed: %d\n", ret);
+   hang();
+   }
}
-#endif
  
  	/* Initialize Auto Voltage Scaling */

mv_avs_init();



Viele Grüße,
Stefan Roese

--
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 u-boot-marvell v2 7/9] arm: mvebu: spl: Use preferred types u8/u16/u32 instead of uintN_t

2021-11-29 Thread Stefan Roese

On 11/26/21 15:37, Marek Behún wrote:

From: Marek Behún 

Checkpatch warns about using uint32/16/8_t instead of u32/16/8.

Use the preferred types.

Signed-off-by: Marek Behún 


Reviewed-by: Stefan Roese 

Thanks,
Stefan


---
  arch/arm/mach-mvebu/spl.c | 34 +-
  1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c
index 8c8cbc833f..97d7aea179 100644
--- a/arch/arm/mach-mvebu/spl.c
+++ b/arch/arm/mach-mvebu/spl.c
@@ -74,23 +74,23 @@
  
  /* Structure of the main header, version 1 (Armada 370/XP/375/38x/39x) */

  struct kwbimage_main_hdr_v1 {
-   uint8_t  blockid;   /* 0x0   */
-   uint8_t  flags; /* 0x1   */
-   uint16_t nandpagesize;  /* 0x2-0x3   */
-   uint32_t blocksize; /* 0x4-0x7   */
-   uint8_t  version;   /* 0x8   */
-   uint8_t  headersz_msb;  /* 0x9   */
-   uint16_t headersz_lsb;  /* 0xA-0xB   */
-   uint32_t srcaddr;   /* 0xC-0xF   */
-   uint32_t destaddr;  /* 0x10-0x13 */
-   uint32_t execaddr;  /* 0x14-0x17 */
-   uint8_t  options;   /* 0x18  */
-   uint8_t  nandblocksize; /* 0x19  */
-   uint8_t  nandbadblklocation;/* 0x1A  */
-   uint8_t  reserved4; /* 0x1B  */
-   uint16_t reserved5; /* 0x1C-0x1D */
-   uint8_t  ext;   /* 0x1E  */
-   uint8_t  checksum;  /* 0x1F  */
+   u8  blockid;   /* 0x0   */
+   u8  flags; /* 0x1   */
+   u16 nandpagesize;  /* 0x2-0x3   */
+   u32 blocksize; /* 0x4-0x7   */
+   u8  version;   /* 0x8   */
+   u8  headersz_msb;  /* 0x9   */
+   u16 headersz_lsb;  /* 0xA-0xB   */
+   u32 srcaddr;   /* 0xC-0xF   */
+   u32 destaddr;  /* 0x10-0x13 */
+   u32 execaddr;  /* 0x14-0x17 */
+   u8  options;   /* 0x18  */
+   u8  nandblocksize; /* 0x19  */
+   u8  nandbadblklocation;/* 0x1A  */
+   u8  reserved4; /* 0x1B  */
+   u16 reserved5; /* 0x1C-0x1D */
+   u8  ext;   /* 0x1E  */
+   u8  checksum;  /* 0x1F  */
  } __packed;
  
  #ifdef CONFIG_SPL_MMC




Viele Grüße,
Stefan Roese

--
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 u-boot-marvell v2 6/9] arm: mvebu: spl: Print srcaddr in error message

2021-11-29 Thread Stefan Roese

On 11/26/21 15:37, Marek Behún wrote:

From: Marek Behún 

Print the wrong srcaddr (spl_image->offset) in error message also for
SATA case.

Signed-off-by: Marek Behún 


Reviewed-by: Stefan Roese 

Thanks,
Stefan


---
  arch/arm/mach-mvebu/spl.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c
index af9e45ac7a..8c8cbc833f 100644
--- a/arch/arm/mach-mvebu/spl.c
+++ b/arch/arm/mach-mvebu/spl.c
@@ -190,7 +190,8 @@ int spl_parse_board_header(struct spl_image_info *spl_image,
 */
if (mhdr->blockid == IBR_HDR_SATA_ID) {
if (spl_image->offset < 1) {
-   printf("ERROR: Wrong SATA srcaddr in kwbimage\n");
+   printf("ERROR: Wrong SATA srcaddr (%u) in kwbimage\n",
+  spl_image->offset);
return -EINVAL;
}
spl_image->offset -= 1;



Viele Grüße,
Stefan Roese

--
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 u-boot-marvell RESEND 09/11] phy: marvell: a3700: Convert to official DT bindings in COMPHY driver

2021-11-29 Thread Stefan Roese

On 11/26/21 14:57, Marek Behún wrote:

From: Pali Rohár 

Convert A3720 common PHY driver to official DT bindings.

This puts us closer to be able to synchronize A3720 device-trees with
those from Linux.

Signed-off-by: Pali Rohár 
Signed-off-by: Marek Behún 
Cc: Konstantin Porotchkin 
Cc: Robert Marko 
Cc: Luka Perkov 
Cc: Marcin Wojtas 
Cc: Grzegorz Jaszczyk 


Reviewed-by: Stefan Roese 

Thanks,
Stefan


---
  arch/arm/dts/armada-3720-espressobin.dts |  21 +---
  arch/arm/dts/armada-3720-turris-mox.dts  |  25 ++---
  arch/arm/dts/armada-3720-uDPU.dts|  23 +---
  arch/arm/dts/armada-37xx.dtsi|  20 +++-
  drivers/phy/marvell/comphy_a3700.c   | 133 +++
  drivers/phy/marvell/comphy_core.c|  59 +-
  drivers/phy/marvell/comphy_core.h|  23 
  drivers/phy/marvell/comphy_cp110.c   |  58 ++
  8 files changed, 250 insertions(+), 112 deletions(-)

diff --git a/arch/arm/dts/armada-3720-espressobin.dts 
b/arch/arm/dts/armada-3720-espressobin.dts
index cba6139be6..360d521bba 100644
--- a/arch/arm/dts/armada-3720-espressobin.dts
+++ b/arch/arm/dts/armada-3720-espressobin.dts
@@ -80,24 +80,6 @@
};
  };
  
- {

-   max-lanes = <3>;
-   phy0 {
-   phy-type = ;
-   phy-speed = ;
-   };
-
-   phy1 {
-   phy-type = ;
-   phy-speed = ;
-   };
-
-   phy2 {
-   phy-type = ;
-   phy-speed = ;
-   };
-};
-
   {
status = "okay";
pinctrl-names = "default";
@@ -119,6 +101,7 @@
  /* CON3 */
   {
status = "okay";
+   phys = < 0>;
  };
  
   {

@@ -200,6 +183,7 @@
  /* CON31 */
   {
status = "okay";
+   phys = < 0>;
  };
  
   {

@@ -207,4 +191,5 @@
pinctrl-0 = <_pins>;
reset-gpios = < 3 GPIO_ACTIVE_LOW>;
status = "okay";
+   phys = < 0>;
  };
diff --git a/arch/arm/dts/armada-3720-turris-mox.dts 
b/arch/arm/dts/armada-3720-turris-mox.dts
index f47ced05c5..d01757062f 100644
--- a/arch/arm/dts/armada-3720-turris-mox.dts
+++ b/arch/arm/dts/armada-3720-turris-mox.dts
@@ -94,24 +94,6 @@
};
  };
  
- {

-   max-lanes = <3>;
-   phy0 {
-   phy-type = ;
-   phy-speed = ;
-   };
-
-   phy1 {
-   phy-type = ;
-   phy-speed = ;
-   };
-
-   phy2 {
-   phy-type = ;
-   phy-speed = ;
-   };
-};
-
   {
status = "okay";
pinctrl-names = "default";
@@ -120,6 +102,11 @@
phy = <_phy1>;
  };
  
+ {

+   phy-mode = "2500base-x";
+   phys = < 1>;
+};
+
   {
pinctrl-names = "default";
pinctrl-0 = <_pins>;
@@ -222,6 +209,7 @@
   {
vbus-supply = <_usb3_vbus>;
status = "okay";
+   phys = < 0>;
  };
  
   {

@@ -229,4 +217,5 @@
pinctrl-0 = <_pins>;
reset-gpios = < 3 GPIO_ACTIVE_LOW>;
status = "disabled";
+   phys = < 0>;
  };
diff --git a/arch/arm/dts/armada-3720-uDPU.dts 
b/arch/arm/dts/armada-3720-uDPU.dts
index 4bf6d2eac7..58557c680a 100644
--- a/arch/arm/dts/armada-3720-uDPU.dts
+++ b/arch/arm/dts/armada-3720-uDPU.dts
@@ -106,36 +106,21 @@
};
  };
  
- {

-   phy0 {
-   phy-type = ;
-   phy-speed = ;
-   };
-
-   phy1 {
-   phy-type = ;
-   phy-speed = ;
-   };
-
-   phy2 {
-   phy-type = ;
-   phy-speed = ;
-   };
-};
-
   {
pinctrl-0 = <_pins>;
status = "okay";
-   phy-mode = "2500base-x";
+   phy-mode = "sgmii";
managed = "in-band-status";
phy = <>;
+   phys = < 0>;
  };
  
   {

status = "okay";
-   phy-mode = "2500base-x";
+   phy-mode = "sgmii";
managed = "in-band-status";
phy = <>;
+   phys = < 1>;
  };
  
   {

diff --git a/arch/arm/dts/armada-37xx.dtsi b/arch/arm/dts/armada-37xx.dtsi
index fec34609cf..bef6ef03df 100644
--- a/arch/arm/dts/armada-37xx.dtsi
+++ b/arch/arm/dts/armada-37xx.dtsi
@@ -316,9 +316,23 @@
compatible = "marvell,mvebu-comphy", 
"marvell,comphy-armada-3700";
reg = <0x18300 0x28>,
  <0x1f300 0x3d000>;
-   mux-bitcount = <4>;
-   mux-lane-order = <1 0 2>;
-   max-lanes = <3>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   comphy0: phy@0 {
+   reg = <0>;
+   #phy-cells = <1>;
+   };
+
+   comphy1: phy@1 {
+   reg = <1>;
+   #phy-cells = <1>;
+   };
+
+   comphy2: phy@2 {
+   

Re: [PATCH] pci: When disabling pref MEM set all base bits

2021-11-29 Thread Stefan Roese

On 11/25/21 11:34, Pali Rohár wrote:

It is common to set all base address bits to one and all limit address bits
to zero for disabling address forwarding. Forwarding is disabled when base
address is higher than limit address, so this change should not have any
effect.

Signed-off-by: Pali Rohár 
---
  drivers/pci/pci_auto.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c
index 6e5ed194f247..c0acf331398d 100644
--- a/drivers/pci/pci_auto.c
+++ b/drivers/pci/pci_auto.c
@@ -243,7 +243,7 @@ void dm_pciauto_prescan_setup_bridge(struct udevice *dev, 
int sub_bus)
cmdstat |= PCI_COMMAND_MEMORY;
} else {
/* We don't support prefetchable memory for now, so disable */
-   dm_pci_write_config16(dev, PCI_PREF_MEMORY_BASE, 0x1000 |
+   dm_pci_write_config16(dev, PCI_PREF_MEMORY_BASE, 0xfff0 |
prefechable_64);


Again, does it make sense to add / use a macro for this 0xfff0 above?

Other than this:

Reviewed-by: Stefan Roese 

Thanks,
Stefan



dm_pci_write_config16(dev, PCI_PREF_MEMORY_LIMIT, 0x0 |
prefechable_64);



Viele Grüße,
Stefan Roese

--
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] pci: Disable I/O forwarding during autoconfiguration if unsupported

2021-11-29 Thread Stefan Roese

On 11/25/21 11:32, Pali Rohár wrote:

If U-Boot does not have any I/O resource for assignment then disable I/O
forwarding in PCI bridge autoconfiguration code. Default initial state of
PCI bridge IO registers is unspecified, therefore they can be in enabled if
U-Boot does not touch them.

Signed-off-by: Pali Rohár 

---
  drivers/pci/pci_auto.c | 8 
  1 file changed, 8 insertions(+)

diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c
index 7e6ee54be087..6e5ed194f247 100644
--- a/drivers/pci/pci_auto.c
+++ b/drivers/pci/pci_auto.c
@@ -265,6 +265,14 @@ void dm_pciauto_prescan_setup_bridge(struct udevice *dev, 
int sub_bus)
  (pci_io->bus_lower & 0x) >> 16);
  
  		cmdstat |= PCI_COMMAND_IO;

+   } else {
+   /* Disable I/O if unsupported */
+   dm_pci_write_config8(dev, PCI_IO_BASE, 0xf0 | io_32);
+   dm_pci_write_config8(dev, PCI_IO_LIMIT, 0x0 | io_32);


Does it perhaps make sense to add / use a macro for this 0xf0 above?

Other than this:

Reviewed-by: Stefan Roese 

Thanks,
Stefan



+   if (io_32 == PCI_IO_RANGE_TYPE_32) {
+   dm_pci_write_config16(dev, PCI_IO_BASE_UPPER16, 0x0);
+   dm_pci_write_config16(dev, PCI_IO_LIMIT_UPPER16, 0x0);
+   }
}
  
  	/* Enable memory and I/O accesses, enable bus master */




Viele Grüße,
Stefan Roese

--
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] pci: Fix register for determining type of IO base address

2021-11-29 Thread Stefan Roese

On 11/25/21 11:30, Pali Rohár wrote:

Function dm_pciauto_prescan_setup_bridge() configures base address
registers, therefore it should read type of IO from base address registers
(and not from limit address registers).

Note that base and limit address registers should have same type, so this
change is just usage correction and has no functional change on correctly
working hardware.

Fixes: 8e85f36a8fab ("pci: Fix configuring io/memory base and limit registers of PCI 
bridges")
Signed-off-by: Pali Rohár 


Reviewed-by: Stefan Roese 

Thanks,
Stefan


---
  drivers/pci/pci_auto.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c
index 5af4ee6e56df..7e6ee54be087 100644
--- a/drivers/pci/pci_auto.c
+++ b/drivers/pci/pci_auto.c
@@ -197,7 +197,7 @@ void dm_pciauto_prescan_setup_bridge(struct udevice *dev, 
int sub_bus)
dm_pci_read_config16(dev, PCI_COMMAND, );
dm_pci_read_config16(dev, PCI_PREF_MEMORY_BASE, _64);
prefechable_64 &= PCI_PREF_RANGE_TYPE_MASK;
-   dm_pci_read_config8(dev, PCI_IO_LIMIT, _32);
+   dm_pci_read_config8(dev, PCI_IO_BASE, _32);
io_32 &= PCI_IO_RANGE_TYPE_MASK;
  
  	/* Configure bus number registers */




Viele Grüße,
Stefan Roese

--
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 1/1] Fix wrong QSPI clock calculation for AM4372

2021-11-29 Thread Tom Rini
On Tue, Nov 30, 2021 at 01:06:56AM +0100, Stefan Mätje wrote:

> On AM4372 the SPI_GCLK input gets its clock from the PRCM module which
> divides the PER_CLKOUTM2 frequency (192MHz) by a fixed factor of 4.
> See AM437x Reference Manual in section 27 QSPI >> 27.2 Integration.
> 
> The QSPI_FCLK therefore needs to take this factor into account and
> becomes (19200 / 4).
> 
> Signed-off-by: Stefan Mätje 
> ---
>  drivers/spi/ti_qspi.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/ti_qspi.c b/drivers/spi/ti_qspi.c
> index 664b9cad79..bccdeeaf82 100644
> --- a/drivers/spi/ti_qspi.c
> +++ b/drivers/spi/ti_qspi.c
> @@ -25,7 +25,8 @@ DECLARE_GLOBAL_DATA_PTR;
>  
>  /* ti qpsi register bit masks */
>  #define QSPI_TIMEOUT200
> -#define QSPI_FCLK19200
> +/* AM4372: QSPI gets SPI_GCLK from PRCM unit as PER_CLKOUTM2 divided by 4. */
> +#define QSPI_FCLK   (19200 / 4)
>  #define QSPI_DRA7XX_FCLK7680
>  #define QSPI_WLEN_MAX_BITS   128
>  #define QSPI_WLEN_MAX_BYTES  (QSPI_WLEN_MAX_BITS >> 3)

How is this treated in the kernel?  Thanks.

-- 
Tom


signature.asc
Description: PGP signature


Re: [ANN] U-Boot v2022.01-rc3 released

2021-11-29 Thread Jesse Taube




On 11/29/21 11:28, Tom Rini wrote:

Hey all,

It's been two weeks since v2022.01-rc2, so here's -rc3.

To repeat what I said with -rc2, we've enabled issue tracking on our
gitlab instance.  You can sign up and then be able to file issues at:
https://source.denx.de/groups/u-boot/-/issues

This is intended for everyone to be able to use, both custodians for
their own needs (you can see for example Heinrich has filed something
for UEFI and LMB) as well as users to just report bugs so they don't
feel like they're lost in the mailing list.

As noted with the last release, the -next branch is open and I'll sync
in -rc3 shortly.  Please feel free to get a PR ready now if you're able.

In terms of a changelog,
git log --merges v2022.01-rc2..v2022.01-rc3
contains what I've pulled but as always, better PR messages and tags
will provide better results here.

So we're now looking at regular releases every other Monday, and with
final release on January 10th, 2022.  Thanks all!


Hey tom,

Thank you for the update as always!

I have a question about a recent commit, I hope its okay to ask here.
In commit cd82f199852d88218e1f17f5ec07cdd9112a89c4
In arch/arm/lib/relocate.S:81 on my SBC it returns an invalid value.
My soc is Thumb2 but the instruction `adr r3, relocate_code`
assembles to `subw r3, pc, #3` which is not 32bit aligned. If i change 
the instruction to `adr.w r3, relocate_code` it evaluates to `subw r3, 
pc, #4`, which is.


There is a slight problem as it seems to work fine on my laptop using 
Debian bullseye, but on my Desktop where I found this I'm running sid.

They are both gcc-10. I have yet to find a way to consistently replicate it.

What are your thoughts of this?

Thanks,
Jesse


Re: Xilinx ZynqMP - ZCU102: SPL not able to use MMC/SD card

2021-11-29 Thread Daniel Cizinsky
On Tue, Nov 09, 2021 at 09:21:41PM +0100, Daniel Cizinsky wrote:
> Hi!
> 
> I am trying to switch to as much current vanilla SW as possible on Xilinx
> ZCU102 evaluation board.  But I am stuck at a very early stage.
> 
> I've got U-Boot + SPL + Linux kernel & userspace compilled, but even after
> trying hard I had no success in running SPL to read ATF, not even speaking
> about the main U-Boot.
I don't even know whether my e-mail went through to the conference. I just
wanted to know, if not being able to use SD card on this board using a
vanilla modern U-Boot SPL is a known bug, or if there are any people
outthere using it successfully. Probably not, or they are not interested in
sharing the simple fact.

With rc3 I'm getting exactly the same errors:

U-Boot SPL 2022.01-rc3 (Nov 29 2021 - 23:50:47 +0100)
PMUFW:  v1.1
Loading new PMUFW cfg obj (2024 bytes)
Silicon version:3
EL Level:   EL3
Multiboot:  0
Trying to boot from MMC2
spl: could not initialize mmc. error: -19
Trying to boot from MMC1
arasan_sdhci mmc@ff17: Using predefined clock phase for clk-phase-legacy = 
0 0
arasan_sdhci mmc@ff17: Using predefined clock phase for clk-phase-mmc-hs = 
63 72
arasan_sdhci mmc@ff17: Using predefined clock phase for clk-phase-sd-hs = 
63 60
arasan_sdhci mmc@ff17: Using predefined clock phase for clk-phase-uhs-sdr12 
= 0 0
arasan_sdhci mmc@ff17: Using predefined clock phase for clk-phase-uhs-sdr25 
= 63 60
arasan_sdhci mmc@ff17: Using predefined clock phase for clk-phase-uhs-sdr50 
= 0 72
arasan_sdhci mmc@ff17: Using predefined clock phase for 
clk-phase-uhs-sdr104 = 0 135
arasan_sdhci mmc@ff17: Using predefined clock phase for clk-phase-uhs-ddr50 
= 183 48
arasan_sdhci mmc@ff17: Using predefined clock phase for clk-phase-mmc-ddr52 
= 54 72
arasan_sdhci mmc@ff17: Using predefined clock phase for clk-phase-mmc-hs200 
= 0 135
arasan_sdhci mmc@ff17: Using predefined clock phase for clk-phase-mmc-hs400 
= 0 0
arasan_sdhci mmc@ff17: arasan_sdhci_set_tapdelay, host:mmc@ff17, mode:0
CMD_SEND:0
ARG  0x
MMC_RSP_NONE
CMD_SEND:8
ARG  0x01aa
RET  -110
CMD_SEND:55
ARG  0x
RET  -110
CMD_SEND:0
ARG  0x
MMC_RSP_NONE
CMD_SEND:1
ARG  0x
RET  -110
Card did not respond to voltage select! : -110
mmc_init: -95, time 23
spl: mmc init failed with error: -95
SPL: Unsupported Boot Device 0
SPL: failed to boot from all boot devices (err=-6)
### ERROR ### Please RESET the board ###

-- 
Daniel Cizinsky at various lists


Re: a question about falcon mode

2021-11-29 Thread Alex G.




On 11/26/21 4:36 PM, Abder wrote:

Hi Alex,

Just a quick remarque that intrigued me:

Le jeu. 25 nov. 2021 à 15:57, Alex G.  a écrit :


On 11/25/21 1:07 AM, Chan Kim wrote:

Hello all,

I'm trying to implement falcon mode for our board. Then should I first
implement the normal mode(spl + proper)?

It looks like so while I'm reading doc/README.falcon. (It says, after
loading kernel, DT etc. I should give 'spl export' command).



Falcon mode is a bit board dependent.  There are a couple of ways you
could go about this.

The first is to have an "fdtargs" partition. This is where "spl export"
comes in. Once you run "spl export", it will give a modified dtb at
"$fdtargsaddr". It's that DTB that you need to write to your ftdargs
partition. For example:

  > spl export fdt $loadaddr - $fdt_addr_r
  > mmc write $fdtargsaddr 0x9800 0x8000

In this example the ftdargs partition starts at sector 0x9800, and is
0x800 sectors long.


The second option is to forget about "spl export" and "fdtargs", and
package your kernel, devicetree, and overlays in a FIT container. You'd
make sure to enable SPL_LOAD_FIT_APPLY_OVERLAY. There isn't much more to
this other than the usual gotcha's with FIT and overlays.



Do you mean by this that the SPL has the capability to generate the
"fdtargs" by it self (if we provide it with the dtb in the fitImage) ?

Form my last experience with the falcon mode, I had a - not sure -
conclusion that the only way to generate the "fdtargs" is by using the
"spl export" command from uboot cmdline !
because the reality of the fdtargs blob, as its name indicates, is not
just the fdt but it has also the bootargs (inside the chosen node )
that are required by the kernel. So if you give only the DTB to the
SPL it will not work - to my knowledge -, cuz the data that will be
passed to the kernel needs to contain also the bootargs !

Can you please confirm to me if this capability is implemented on the
SPL and that we can actually forget about the "spl export" command ?


It might not be obvious that an overlay can contain the "/chosen" node 
with the appropriate bootargs:


/dts-v1/;
/plugin/;
/ {
fragment@1 {
target-path = "/chosen";
__overlay__ {
bootargs = "root=blablabla console=ttyeS0";
};
};
};


Thanks
And apologies Chan for jumping on your thread,


Best regards,
--
Abder



Re: [PATCH v2] efi_loader: tcg2: Return success even when TPM device is not found

2021-11-29 Thread Ilias Apalodimas
On Mon, 29 Nov 2021 at 18:50, Ilias Apalodimas
 wrote:
>
> Heinrich,
>
> On Mon, 29 Nov 2021 at 18:41, Heinrich Schuchardt  wrote:
> >
> > On 11/29/21 15:55, Ilias Apalodimas wrote:
> > > On Mon, 29 Nov 2021 at 16:26, Michal Simek  
> > > wrote:
> > >>
> > >> For systems which have TPM support enabled but actual device is missing
> > >> there is no reason to show a message that measurement failed in
> > >> efi_load_pe(). To ensure that the patch is returning EFI_SUCCESS even for
> > >> cases where TPM device is not found.
> > >> The reason is that other parts of the code return also EFI_NOT_FOUND in
> > >> tcg2_measure_pe_image() (e.g efi_search_protocol) that's why this error
> > >> code can't be checked but still it needs to be reported.
> > >>
> > >> The same logic is also used in efi_tcg2_get_eventlog() added by
> > >> commit c8d0fd582576 ("efi_loader: Introduce eventlog support for
> > >> TCG2_PROTOCOL").
> > >>
> > >> Signed-off-by: Michal Simek 
> > >> ---
> > >>
> > >> Changes in v2:
> > >> - Change subject and description
> > >> - Change logic in different location
> > >> - Origin thread was 
> > >> https://lore.kernel.org/r/657a869c04e9b09e3bd2e6fd74ff94320b7fbe9b.1638191161.git.michal.si...@xilinx.com
> > >>
> > >>   lib/efi_loader/efi_tcg2.c | 3 ++-
> > >>   1 file changed, 2 insertions(+), 1 deletion(-)
> > >>
> > >> diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> > >> index 8c1f22e3377b..db785f4d8c27 100644
> > >> --- a/lib/efi_loader/efi_tcg2.c
> > >> +++ b/lib/efi_loader/efi_tcg2.c
> > >> @@ -888,7 +888,8 @@ efi_status_t tcg2_measure_pe_image(void *efi, u64 
> > >> efi_size,
> > >>
> > >>  ret = platform_get_tpm2_device();
> > >>  if (ret != EFI_SUCCESS)
> > >> -   return ret;
> > >> +   /* don't fail when TPM is not found */
> > >> +   return EFI_SUCCESS;
> > >>
> > >>  switch (handle->image_type) {
> > >>  case IMAGE_SUBSYSTEM_EFI_APPLICATION:
> > >> --
> > >> 2.33.1
> > >>
> > >
> > > Reviewed-by: Ilias Apalodimas 
> > >
> >
> > This patch means:
> >
> > You can run some command that initializes the TCG2 protocol (e.g.
> > debug_hd), then unbind the TPM, run a first EFI binary which diverts EFI
> > API addresses, bind the TPM again and run the normal binary and nobody
> > will see the first binary in boot measurement.
>
> Why?  What you describe is an issue with, or without this patch.  The
> code never stops if tcg2_measure_pe_image() fails.  The only thing
> this patch does is silence a print if a TPM device is not found.
>

But tbh we can sort out Heinrich's concern while not printing that
error message.  I'll come up with a patch shortly.


Cheers
/Ilias
> Regards
> /Ilias
>
> >
> > Best regards
> >
> > Heinrich


Re: [PATCH 1/2] fsl-layerscape: add dtb overlay feature

2021-11-29 Thread Michael Walle

Hi Sahil,

Am 2021-11-29 12:55, schrieb Sahil Malhotra (OSS):

Am 2021-11-17 19:11, schrieb Sahil Malhotra (OSS):

Could you please add some description what this is doing and for what
this is intended? To have a "DTB overlay feature", it is enough to
just enable CONFIG_OF_LIBFDT_OVERLAY.

I will add some description, and yes for DTB overlay feature, it is
enough to enable CONFIG_OF_LIBFDT_OVERLAY but we need to do this step
before booting the kernel that's why also have to enable
CONFIG_OF_SYSTEM_SETUP.



Ok. What will the overlay do? Could you give an example?

This overlay will be disabling the crypto nodes which will be used by
optee in secure world, so that linux should not use it.



Apparently you're adding an overlay passed by optee. Doesn't this
have to be applied to u-boot's control dtb too?

Yes, we will be applying the overlay passed by optee, yes it will be
applied to dtb which will be passed to uboot for kernel booting.


If I read this patch correctly, you're modifying the DT before you 
jump to linux. But I was asking whether you also have to modify the DT 
which is used by u-boot. Eg. if you disable some kind of crypto nodes 
(because optee will use them in secure world), this also have to 
communicated to u-boot, not only linux, no?

Yes, I got your point now, and is very valid, but as of now for u-boot
we are just using the first available node for communicating with CAAM
leaving other job rings as it is.
So we need not to apply overlay to DTB used by uboot.


But we should do the correct thing, so that u-boot and linux
doesn't see a different version of the device tree.

Also what do you mean with "the first available node"?
There is already a new CAAM driver for u-boot pending,
see
https://lore.kernel.org/u-boot/2025070014.17586-1-gaurav.j...@nxp.com/

-michael


Re: [PATCH u-boot-marvell 02/10] arm: mvebu: a38x: serdes: Move non-serdes PCIe code to pci_mvebu.c

2021-11-29 Thread Marek Behún
On Mon, 29 Nov 2021 17:07:54 +0100
Stefan Roese  wrote:

> > Just I'm not sure if this "enable port functionality" should be
> > implemented via Reset Controller API...  
> 
> How else should / could this be done then? Do you have alterative ideas?

syscon regmap


Re: [PATCH v2] efi_loader: tcg2: Return success even when TPM device is not found

2021-11-29 Thread Ilias Apalodimas
Heinrich,

On Mon, 29 Nov 2021 at 18:41, Heinrich Schuchardt  wrote:
>
> On 11/29/21 15:55, Ilias Apalodimas wrote:
> > On Mon, 29 Nov 2021 at 16:26, Michal Simek  wrote:
> >>
> >> For systems which have TPM support enabled but actual device is missing
> >> there is no reason to show a message that measurement failed in
> >> efi_load_pe(). To ensure that the patch is returning EFI_SUCCESS even for
> >> cases where TPM device is not found.
> >> The reason is that other parts of the code return also EFI_NOT_FOUND in
> >> tcg2_measure_pe_image() (e.g efi_search_protocol) that's why this error
> >> code can't be checked but still it needs to be reported.
> >>
> >> The same logic is also used in efi_tcg2_get_eventlog() added by
> >> commit c8d0fd582576 ("efi_loader: Introduce eventlog support for
> >> TCG2_PROTOCOL").
> >>
> >> Signed-off-by: Michal Simek 
> >> ---
> >>
> >> Changes in v2:
> >> - Change subject and description
> >> - Change logic in different location
> >> - Origin thread was 
> >> https://lore.kernel.org/r/657a869c04e9b09e3bd2e6fd74ff94320b7fbe9b.1638191161.git.michal.si...@xilinx.com
> >>
> >>   lib/efi_loader/efi_tcg2.c | 3 ++-
> >>   1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> >> index 8c1f22e3377b..db785f4d8c27 100644
> >> --- a/lib/efi_loader/efi_tcg2.c
> >> +++ b/lib/efi_loader/efi_tcg2.c
> >> @@ -888,7 +888,8 @@ efi_status_t tcg2_measure_pe_image(void *efi, u64 
> >> efi_size,
> >>
> >>  ret = platform_get_tpm2_device();
> >>  if (ret != EFI_SUCCESS)
> >> -   return ret;
> >> +   /* don't fail when TPM is not found */
> >> +   return EFI_SUCCESS;
> >>
> >>  switch (handle->image_type) {
> >>  case IMAGE_SUBSYSTEM_EFI_APPLICATION:
> >> --
> >> 2.33.1
> >>
> >
> > Reviewed-by: Ilias Apalodimas 
> >
>
> This patch means:
>
> You can run some command that initializes the TCG2 protocol (e.g.
> debug_hd), then unbind the TPM, run a first EFI binary which diverts EFI
> API addresses, bind the TPM again and run the normal binary and nobody
> will see the first binary in boot measurement.

Why?  What you describe is an issue with, or without this patch.  The
code never stops if tcg2_measure_pe_image() fails.  The only thing
this patch does is silence a print if a TPM device is not found.

Regards
/Ilias

>
> Best regards
>
> Heinrich


Re: [PATCH v2] efi_loader: tcg2: Return success even when TPM device is not found

2021-11-29 Thread Heinrich Schuchardt

On 11/29/21 15:55, Ilias Apalodimas wrote:

On Mon, 29 Nov 2021 at 16:26, Michal Simek  wrote:


For systems which have TPM support enabled but actual device is missing
there is no reason to show a message that measurement failed in
efi_load_pe(). To ensure that the patch is returning EFI_SUCCESS even for
cases where TPM device is not found.
The reason is that other parts of the code return also EFI_NOT_FOUND in
tcg2_measure_pe_image() (e.g efi_search_protocol) that's why this error
code can't be checked but still it needs to be reported.

The same logic is also used in efi_tcg2_get_eventlog() added by
commit c8d0fd582576 ("efi_loader: Introduce eventlog support for
TCG2_PROTOCOL").

Signed-off-by: Michal Simek 
---

Changes in v2:
- Change subject and description
- Change logic in different location
- Origin thread was 
https://lore.kernel.org/r/657a869c04e9b09e3bd2e6fd74ff94320b7fbe9b.1638191161.git.michal.si...@xilinx.com

  lib/efi_loader/efi_tcg2.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index 8c1f22e3377b..db785f4d8c27 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -888,7 +888,8 @@ efi_status_t tcg2_measure_pe_image(void *efi, u64 efi_size,

 ret = platform_get_tpm2_device();
 if (ret != EFI_SUCCESS)
-   return ret;
+   /* don't fail when TPM is not found */
+   return EFI_SUCCESS;

 switch (handle->image_type) {
 case IMAGE_SUBSYSTEM_EFI_APPLICATION:
--
2.33.1



Reviewed-by: Ilias Apalodimas 



This patch means:

You can run some command that initializes the TCG2 protocol (e.g.
debug_hd), then unbind the TPM, run a first EFI binary which diverts EFI
API addresses, bind the TPM again and run the normal binary and nobody
will see the first binary in boot measurement.

Best regards

Heinrich


[ANN] U-Boot v2022.01-rc3 released

2021-11-29 Thread Tom Rini
Hey all,

It's been two weeks since v2022.01-rc2, so here's -rc3.

To repeat what I said with -rc2, we've enabled issue tracking on our
gitlab instance.  You can sign up and then be able to file issues at:
https://source.denx.de/groups/u-boot/-/issues

This is intended for everyone to be able to use, both custodians for
their own needs (you can see for example Heinrich has filed something
for UEFI and LMB) as well as users to just report bugs so they don't
feel like they're lost in the mailing list.

As noted with the last release, the -next branch is open and I'll sync
in -rc3 shortly.  Please feel free to get a PR ready now if you're able.

In terms of a changelog, 
git log --merges v2022.01-rc2..v2022.01-rc3
contains what I've pulled but as always, better PR messages and tags
will provide better results here.

So we're now looking at regular releases every other Monday, and with
final release on January 10th, 2022.  Thanks all!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH] board: iot2050: update build documentation for OP-TEE

2021-11-29 Thread Tom Rini
On Sun, Nov 28, 2021 at 09:57:01PM +, Ivan Mikhaylov wrote:

> From: Ivan Mikhaylov 
> 
> Set ta-target explicitly to correspond with OP-TEE recipe in
> siemens/meta-iot2050.
> 
> Errors without explicit set of ta-target:
> aarch64-linux-gnu-gcc: error: unrecognized command-line option ‘-mthumb’
> aarch64-linux-gnu-gcc: error: unrecognized command-line option 
> ‘-mno-unaligned-access’
> aarch64-linux-gnu-gcc: error: unrecognized command-line option 
> ‘-mfloat-abi=hard’
> make: *** [mk/compile.mk:159: out/arm-plat-k3/ta_arm32-lib/libdl/dlfcn.o] 
> Error 1
> 
> Signed-off-by: Ivan Mikhaylov 
> Reviewed-by: Jan Kiszka 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH] qemu: common: Fix build with update capsule

2021-11-29 Thread Tom Rini
On Wed, Nov 24, 2021 at 03:54:20PM +0100, Vincent Stehlé wrote:

> The common emulation Makefile has a dependency on a non-existent
> qemu_capsule.o when building with support for capsule update enabled
> (CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y).
> The code which was in qemu_capsule.c has been completely moved to
> lib/efi_loader/efi_capsule.c by commit 7a6fb28c8e4b ("efi_loader: capsule:
> add back efi_get_public_key_data()").
> Remove the false dependency.
> 
> This fixes the following build error:
> 
>   make[1]: *** No rule to make target 
> 'board/emulation/common/qemu_capsule.o', needed by 
> 'board/emulation/common/built-in.o'.  Stop.
> 
> Fixes: commit 47a25e81d35c ("Revert "efi_capsule: Move signature from DTB to 
> .rodata"")
> Signed-off-by: Vincent Stehlé 
> Cc: Simon Glass 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH] fastboot: Add maintainers entry

2021-11-29 Thread Tom Rini
On Tue, Nov 23, 2021 at 11:33:11PM -0500, Sean Anderson wrote:

> Add an entry in maintainers for fastboot. It is starting off orphaned, but
> hopefully someone can pick it up.
> 
> Signed-off-by: Sean Anderson 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH] clk: Add myself as a maintainer for the clock subsystem

2021-11-29 Thread Tom Rini
On Tue, Nov 23, 2021 at 11:23:40PM -0500, Sean Anderson wrote:

> Lukasz has not been very responsive in reviewing clock patches. Add
> myself as a maintainer.
> 
> Signed-off-by: Sean Anderson 
> Acked-by: Lukasz Majewski 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH v4 3/6] ARM: dts: sync Actions Semi S700 DT from Linux 5.10-rc7

2021-11-29 Thread Tom Rini
On Sun, Nov 28, 2021 at 05:02:22PM +0530, Amit Singh Tomar wrote:

> From: Amit Singh Tomar 
> 
> This Synchronizes the Actions Semi S700 SoC DT changes from
> commit "0477e9288185" ("Linux 5.10-rc7").
> 
> Signed-off-by: Amit Singh Tomar 
> ---
> Changes since previous versions
>   * No change.
> ---
>  arch/arm/dts/s700.dtsi| 17 -
>  .../dt-bindings/power/owl-s700-powergate.h| 19 +++
>  2 files changed, 35 insertions(+), 1 deletion(-)
>  create mode 100644 include/dt-bindings/power/owl-s700-powergate.h

That's a fairly old tag to sync to, can we please pick something newer
(either latest release or latest rc)?  Thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH u-boot-marvell 02/10] arm: mvebu: a38x: serdes: Move non-serdes PCIe code to pci_mvebu.c

2021-11-29 Thread Stefan Roese

On 11/29/21 15:28, Pali Rohár wrote:

On Monday 29 November 2021 14:27:48 Pali Rohár wrote:

On Monday 29 November 2021 13:30:45 Stefan Roese wrote:

Hi Pali,

On 11/29/21 12:47, Pali Rohár wrote:

Hello!

On Monday 29 November 2021 10:22:58 Stefan Roese wrote:

On 11/29/21 10:06, Pali Rohár wrote:




After this DTS change, pci-mvebu.c will just replace value of current
number of lanes (which is set to 4 by serdes code) to value from DTS,
which is 4. Therefore there should be no change.

Could you test whole patch series with above DTS change if it works
properly on Theadorable board?


Yes, I don't see any issues with this patchset applied plus this DT
patch on theadorable. The PCIe links are up and with the correct width.

What I'm wondering is, when exactly does the PCIe RP start the link
establishment. In my case with AXP this is still in the AXP serdes code
of course. But in the A38x code, it should be in the PCIe controller
driver now AFAIU. I see that you configure the link width in the
controller and do some other configuration (address windows etc), but
at the end you "simply" wait for the link to come up via
mvebu_pcie_wait_for_link(). I would have expected here some special
command (config bit?) to the PCIe controller to start the link
establishment. So when exactly does the A38x start this action?


That is interesting question... While I'm reading it again, I really do
not know. Because you are right that mvebu_pcie_wait_for_link() is just
waiting for a link and it "magically" comes up. I have tested it on A385
and it is stable with different Compex Atheros cards which caused issues
in past also on A3720.


I would prefer, to fully understand when exactly the link establishment
is started. Since this is crucial for the setup of the controller that
needs to be done *before* the link starts to come up.


I try to dig as more information as possible and finally I find out that
important information is available also in now removed, but originally
public A38x documentation. Thankfully web archive has copy of it:

https://web.archive.org/web/20200420191927/https://www.marvell.com/content/dam/marvell/en/public-collateral/embedded-processors/marvell-embedded-processors-armada-38x-functional-specifications-2015-11.pdf

17.3 Link Initialization

In case the initialization fails and no link is established, the PHY
will keep on trying to initiate a link forever unless the port is
disabled. As long as the port is enabled, the PHY will continue trying
to establish a link; once the PHY identifies that a device is
connected to it, a link will be established.

PCIe port is enabled by bits in SoC Control 1 Register, which is done in
U-Boot SerDes initialization code. This is IIRC SoC specific, and reason
why every Armada SoC has own SerDes init code.

And looks like that due to "the PHY will keep on trying to initiate a
link forever", the PCIe link comes up when pci-mvebu.c sets all required
registers to correct values. E.g. set correct mode (RC vs endpoint),
link width (x1 vs x4), etc...


Could you perhaps try to remove some of the register configurations in
the MVEBU PCIe driver to see, if the link establishment relies on this
register to be written to (e.g. PCI_EXP_LNKCAP)?


First port on A385 is by default set to X4 width, other ports to X1
width. Without updating LNKCAP to correct width, card in first PCIe port
never initialize. And cards in other ports are initialized even before
pci-mvebu.c starts configuration.


So the PCIe ports are now trying to establish the links, even when the
correct configuration is not yet done. This might work but sound far
from perfect to me IMHO.


Yes, it looks like (based on behavior of the first port). And it is not
perfect, just another mess :-(


So seems that this matches above behavior. SerDes init code enabled all
PCIe ports. Ports which are using default configuration (second, third)
are immediately initialized and link is established. Port which requires
additional configuration (first port, for switching from X4 to X1) just
stay in that "keep on trying to initiate a link forever" state until
pci-mvebu starts and set PCI_EXP_LNKCAP register, after which PHY try X1
width and success. And seems that this is the reason why 100ms timeout
is needed... As at this stage when pci-mvebu.c switches X4 to X1, init
timeout as defined in PCIe spec (that 100ms) starts ticking. For other
ports it starts ticking when serdes init code enables ports.


I have looked into all PCIe registers which are present in functional
spec, but it looks like that there is no pci-mvebu register which can
turn of LTSSM and link training, like it is in other PCIe controllers.
It looks like that only SoC-specific port enable bits are there.

It is starting to be bigger mess as before... Any suggestion how to
continue with it?

We cannot (easily) move that code which flips PCIe bits in SoC Control 1
Register from SerDes init code to pci-mvebu.c as this is outside of

Re: [PATCH] sdhci: zynqmp: Setting up clock frequency based on DT

2021-11-29 Thread Michal Simek




On 11/22/21 16:31, Michal Simek wrote:

Hi,

On 11/19/21 18:16, Sean Anderson wrote:



On 11/18/21 7:03 AM, Michal Simek wrote:

Using clock-frequency property to define desired clock speed for
controllers.

Signed-off-by: Michal Simek 
---

  drivers/mmc/zynq_sdhci.c | 10 ++
  1 file changed, 10 insertions(+)

diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c
index 5cea4c695e8d..ee87907939fe 100644
--- a/drivers/mmc/zynq_sdhci.c
+++ b/drivers/mmc/zynq_sdhci.c
@@ -58,6 +58,7 @@ struct arasan_sdhci_plat {
  struct arasan_sdhci_priv {
  struct sdhci_host *host;
  struct arasan_sdhci_clk_data clk_data;
+    u32 frequency;
  u8 deviceid;
  u8 bank;
  u8 no_1p8;
@@ -721,6 +722,14 @@ static int arasan_sdhci_probe(struct udevice *dev)
  return ret;
  }
+    if (priv->frequency) {
+    ret = clk_set_rate(, priv->frequency);
+    if (IS_ERR_VALUE(ret)) {
+    dev_err(dev, "failed to set clock rate\n");
+    return ret;
+    }
+    }
+
  clock = clk_get_rate();
  if (IS_ERR_VALUE(clock)) {
  dev_err(dev, "failed to get rate\n");
@@ -804,6 +813,7 @@ static int arasan_sdhci_of_to_plat(struct udevice 
*dev)

  if (IS_ERR(priv->host->ioaddr))
  return PTR_ERR(priv->host->ioaddr);
+    priv->frequency = dev_read_u32_default(dev, "clock-frequency", 0);
  priv->deviceid = dev_read_u32_default(dev, "xlnx,device_id", -1);
  priv->bank = dev_read_u32_default(dev, "xlnx,mio-bank", 0);
  priv->no_1p8 = dev_read_bool(dev, "no-1-8-v");



Why not just assigned-clock-rates? Are there any existing users with 
just clock-frequency?


There is no user of it now in public domain. I was looking for the right 
properly and found only clock-frequency but assigned-clock-rates works 
for me. Will test and send v2.


Just for the record I have tried assigned-clock-rates and there is no 
need to do any change in the driver. That's why please ignore this patch.


Thanks,
Michal


Re: [BUG] emmc `env erase` erase unrelated data

2021-11-29 Thread Tom Rini
On Mon, Nov 29, 2021 at 04:38:34PM +0100, Francesco Dolcini wrote:
> Hello Tom,
> 
> On Mon, Nov 29, 2021 at 10:25:30AM -0500, Tom Rini wrote:
> > On Mon, Nov 29, 2021 at 02:21:23PM +0100, Francesco Dolcini wrote:
> > > I noticed an issue with env erase command when environment is stored in a
> > > emmc device, in case start/end are not aligned to the emmc erase groups
> > > size additional data is erased with just a warning.
> ...
> > > I do not think that this is the correct behavior, I think that in case
> > > the env is not aligned to the erase block size the erase should either
> > > fail or fall back to just writing 0xff. Not sure if just changing
> > > `mmc_berase()` is going to affect any other use case in which is valid
> > > to have a non-aligned start/size and just erase around it after a
> > > warning.
> > 
> > Largely intentional behavior.  Perhaps the help text at least should be
> > updated to note that environment size for MMC needs to be a multiple of
> > erase block size?  And further that when using redundant environment
> > it's strongly encouraged to make sure each environment is on its own
> > erase block.
> 
> I'm not really convinced, but let's move forward from this point.

Alright.  I'll also agree that it was designed when erase blocks were
probably smaller and that yes, it's a lot more wasteful these days.

> Are you aware of any other issues in case the emmc environment is not a
> multiple/aligned to the erase size? I believe that the rest of the code
> just read/write to it, so once it is aligned to the read/write block
> size (512 bytes, usually) there should be no problem.

I believe you're correct.

-- 
Tom


signature.asc
Description: PGP signature


Re: [BUG] emmc `env erase` erase unrelated data

2021-11-29 Thread Francesco Dolcini
Hello Tom,

On Mon, Nov 29, 2021 at 10:25:30AM -0500, Tom Rini wrote:
> On Mon, Nov 29, 2021 at 02:21:23PM +0100, Francesco Dolcini wrote:
> > I noticed an issue with env erase command when environment is stored in a
> > emmc device, in case start/end are not aligned to the emmc erase groups
> > size additional data is erased with just a warning.
...
> > I do not think that this is the correct behavior, I think that in case
> > the env is not aligned to the erase block size the erase should either
> > fail or fall back to just writing 0xff. Not sure if just changing
> > `mmc_berase()` is going to affect any other use case in which is valid
> > to have a non-aligned start/size and just erase around it after a
> > warning.
> 
> Largely intentional behavior.  Perhaps the help text at least should be
> updated to note that environment size for MMC needs to be a multiple of
> erase block size?  And further that when using redundant environment
> it's strongly encouraged to make sure each environment is on its own
> erase block.

I'm not really convinced, but let's move forward from this point.

Are you aware of any other issues in case the emmc environment is not a
multiple/aligned to the erase size? I believe that the rest of the code
just read/write to it, so once it is aligned to the read/write block
size (512 bytes, usually) there should be no problem.

Francesco



Re: [BUG] emmc `env erase` erase unrelated data

2021-11-29 Thread Tom Rini
On Mon, Nov 29, 2021 at 02:21:23PM +0100, Francesco Dolcini wrote:
> Hello,
> I noticed an issue with env erase command when environment is stored in a
> emmc device, in case start/end are not aligned to the emmc erase groups
> size additional data is erased with just a warning.
> 
> ```
> Erasing Environment on MMC...
> 
> Caution! Your devices Erase group is 0x400
> The erase range would be change to 0xf800~0xfbff
> ```
> 
> Output from `mmc info`:
> 
> ```
> Device: FSL_SDHC
> Manufacturer ID: 13
> OEM: 14e
> Name: S0J56
> Bus Speed: 2
> Mode: HS400 (200MHz)
> Rd Block Len: 512
> MMC version 5.1
> High Capacity: Yes
> Capacity: 14.8 GiB
> Bus Width: 8-bit DDR
> Erase Group Size: 512 KiB
> HC WP Group Size: 8 MiB
> User Capacity: 14.8 GiB WRREL
> Boot Capacity: 31.5 MiB ENH
> RPMB Capacity: 4 MiB ENH
> ```
> 
> I do not think that this is the correct behavior, I think that in case
> the env is not aligned to the erase block size the erase should either
> fail or fall back to just writing 0xff. Not sure if just changing
> `mmc_berase()` is going to affect any other use case in which is valid
> to have a non-aligned start/size and just erase around it after a
> warning.
> 
> Any comment?

Largely intentional behavior.  Perhaps the help text at least should be
updated to note that environment size for MMC needs to be a multiple of
erase block size?  And further that when using redundant environment
it's strongly encouraged to make sure each environment is on its own
erase block.

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH] phy: zynqmp: Add serdes/psgtr driver

2021-11-29 Thread Sean Anderson




On 11/24/21 9:52 AM, Michal Simek wrote:



On 11/22/21 22:53, Sean Anderson wrote:



On 11/18/21 7:30 AM, Michal Simek wrote:

Add PSGTR driver for Xilinx ZynqMP.
The most of configurations are taken from Linux kernel psgtr driver.

USB3.0 and SGMII configurations are tested on SOM. In SGMII case also
IOU_SLCR reg is updated to get proper clock setup and signal detection
configuration.


Are USB3 and SGMII all that's been tested? I noticed that the kernel
driver has DP- and SATA-specific stuff which has been left out.
Presumably they are not supported?


I have tested USB3 and SGMII. I didn't test DP/SATA and there is missing some 
code for it.
DP will be tested with u-boot driver which we will develop.



Perhaps also note that the termination fix is not implemented.


Terminanation fix was for v1 silicon which none is really using now that's why 
this code is not needed for newly written SW.




Signed-off-by: Michal Simek 
---

  MAINTAINERS  |   1 +
  drivers/phy/Kconfig  |   7 +
  drivers/phy/Makefile |   1 +
  drivers/phy/phy-zynqmp.c | 690 +++
  4 files changed, 699 insertions(+)
  create mode 100644 drivers/phy/phy-zynqmp.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 1eb71cbdad12..d1e9fbd4a279 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -611,6 +611,7 @@ F:drivers/mmc/zynq_sdhci.c
  F:drivers/mtd/nand/raw/zynq_nand.c
  F:drivers/net/phy/xilinx_phy.c
  F:drivers/net/zynq_gem.c
+F:drivers/phy/phy-zynqmp.c
  F:drivers/serial/serial_zynq.c
  F:drivers/reset/reset-zynqmp.c
  F:drivers/rtc/zynqmp_rtc.c
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 4767d215f337..d79798429b18 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -281,6 +281,13 @@ config PHY_IMX8MQ_USB
  help
Support the USB3.0 PHY in NXP i.MX8MQ SoC

+config PHY_XILINX_ZYNQMP
+tristate "Xilinx ZynqMP PHY driver"
+depends on PHY && ARCH_ZYNQMP
+help
+  Enable this to support ZynqMP High Speed Gigabit Transceiver
+  that is part of ZynqMP SoC.
+
  source "drivers/phy/rockchip/Kconfig"
  source "drivers/phy/cadence/Kconfig"
  source "drivers/phy/ti/Kconfig"
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 13a8ade8919f..bf9b40932fe3 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -38,5 +38,6 @@ obj-$(CONFIG_MT76X8_USB_PHY) += mt76x8-usb-phy.o
  obj-$(CONFIG_PHY_DA8XX_USB) += phy-da8xx-usb.o
  obj-$(CONFIG_PHY_MTK_TPHY) += phy-mtk-tphy.o
  obj-$(CONFIG_PHY_IMX8MQ_USB) += phy-imx8mq-usb.o
+obj-$(CONFIG_PHY_XILINX_ZYNQMP) += phy-zynqmp.o
  obj-y += cadence/
  obj-y += ti/
diff --git a/drivers/phy/phy-zynqmp.c b/drivers/phy/phy-zynqmp.c
new file mode 100644
index ..d6fe8dcef74e
--- /dev/null
+++ b/drivers/phy/phy-zynqmp.c
@@ -0,0 +1,690 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * phy-zynqmp.c - PHY driver for Xilinx ZynqMP GT.
+ *
+ * Copyright (C) 2018-2021 Xilinx Inc.
+ *
+ * Author: Anurag Kumar Vulisha 
+ * Author: Subbaraya Sundeep 
+ * Author: Laurent Pinchart 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#include 
+
+/*
+ * Lane Registers
+ */
+
+/* TX De-emphasis parameters */
+#define L0_TX_ANA_TM_180x0048
+#define L0_TX_ANA_TM_1180x01d8
+#define L0_TX_ANA_TM_118_FORCE_17_0BIT(0)
+
+/* DN Resistor calibration code parameters */
+#define L0_TXPMA_ST_30x0b0c
+#define L0_DN_CALIB_CODE0x3f
+
+/* PMA control parameters */
+#define L0_TXPMD_TM_450x0cb4
+#define L0_TXPMD_TM_480x0cc0
+#define L0_TXPMD_TM_45_OVER_DP_MAINBIT(0)
+#define L0_TXPMD_TM_45_ENABLE_DP_MAINBIT(1)
+#define L0_TXPMD_TM_45_OVER_DP_POST1BIT(2)
+#define L0_TXPMD_TM_45_ENABLE_DP_POST1BIT(3)
+#define L0_TXPMD_TM_45_OVER_DP_POST2BIT(4)
+#define L0_TXPMD_TM_45_ENABLE_DP_POST2BIT(5)
+
+/* PCS control parameters */
+#define L0_TM_DIG_60x106c
+#define L0_TM_DIS_DESCRAMBLE_DECODER0x0f
+#define L0_TX_DIG_610x00f4
+#define L0_TM_DISABLE_SCRAMBLE_ENCODER0x0f
+
+/* PLL Test Mode register parameters */
+#define L0_TM_PLL_DIG_370x2094
+#define L0_TM_COARSE_CODE_LIMIT0x10
+
+/* PLL SSC step size offsets */
+#define L0_PLL_SS_STEPS_0_LSB0x2368
+#define L0_PLL_SS_STEPS_1_MSB0x236c
+#define L0_PLL_SS_STEP_SIZE_0_LSB0x2370
+#define L0_PLL_SS_STEP_SIZE_10x2374
+#define L0_PLL_SS_STEP_SIZE_20x2378
+#define L0_PLL_SS_STEP_SIZE_3_MSB0x237c
+#define L0_PLL_STATUS_READ_10x23e4
+
+/* SSC step size parameters */
+#define STEP_SIZE_0_MASK0xff
+#define STEP_SIZE_1_MASK0xff
+#define STEP_SIZE_2_MASK0xff
+#define STEP_SIZE_3_MASK0x3
+#define STEP_SIZE_SHIFT8
+#define FORCE_STEP_SIZE0x10
+#define FORCE_STEPS  

Re: [PATCH v2] efi_loader: tcg2: Return success even when TPM device is not found

2021-11-29 Thread Ilias Apalodimas
On Mon, 29 Nov 2021 at 16:26, Michal Simek  wrote:
>
> For systems which have TPM support enabled but actual device is missing
> there is no reason to show a message that measurement failed in
> efi_load_pe(). To ensure that the patch is returning EFI_SUCCESS even for
> cases where TPM device is not found.
> The reason is that other parts of the code return also EFI_NOT_FOUND in
> tcg2_measure_pe_image() (e.g efi_search_protocol) that's why this error
> code can't be checked but still it needs to be reported.
>
> The same logic is also used in efi_tcg2_get_eventlog() added by
> commit c8d0fd582576 ("efi_loader: Introduce eventlog support for
> TCG2_PROTOCOL").
>
> Signed-off-by: Michal Simek 
> ---
>
> Changes in v2:
> - Change subject and description
> - Change logic in different location
> - Origin thread was 
> https://lore.kernel.org/r/657a869c04e9b09e3bd2e6fd74ff94320b7fbe9b.1638191161.git.michal.si...@xilinx.com
>
>  lib/efi_loader/efi_tcg2.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> index 8c1f22e3377b..db785f4d8c27 100644
> --- a/lib/efi_loader/efi_tcg2.c
> +++ b/lib/efi_loader/efi_tcg2.c
> @@ -888,7 +888,8 @@ efi_status_t tcg2_measure_pe_image(void *efi, u64 
> efi_size,
>
> ret = platform_get_tpm2_device();
> if (ret != EFI_SUCCESS)
> -   return ret;
> +   /* don't fail when TPM is not found */
> +   return EFI_SUCCESS;
>
> switch (handle->image_type) {
> case IMAGE_SUBSYSTEM_EFI_APPLICATION:
> --
> 2.33.1
>

Reviewed-by: Ilias Apalodimas 


Re: Please pull u-boot-dm/next

2021-11-29 Thread Tom Rini
On Sun, Nov 28, 2021 at 06:29:12PM -0700, Simon Glass wrote:

> Hi Tom,
> 
> This is for the next branch.
> 
> https://source.denx.de/u-boot/custodians/u-boot-dm/-/pipelines/10005
> 
> 
> he following changes since commit 1943f2a2a7c58b76812fcad2d3012036af7464ce:
> 
>   Merge branch '2021-11-23-scmi-and-tee-updates' into next (2021-11-23
> 16:24:24 -0500)
> 
> are available in the Git repository at:
> 
>   g...@source.denx.de:u-boot/custodians/u-boot-dm.git tags/dm-pull-28nov21
> 
> for you to fetch changes up to 452e8c9086a9f95739582da5ccc2130e4bf1ae8b:
> 
>   test/py: Raise a ValueError if a command fails (2021-11-28 16:51:51 -0700)
> 

Applied to u-boot/next, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH u-boot-marvell 02/10] arm: mvebu: a38x: serdes: Move non-serdes PCIe code to pci_mvebu.c

2021-11-29 Thread Pali Rohár
On Monday 29 November 2021 14:27:48 Pali Rohár wrote:
> On Monday 29 November 2021 13:30:45 Stefan Roese wrote:
> > Hi Pali,
> > 
> > On 11/29/21 12:47, Pali Rohár wrote:
> > > Hello!
> > > 
> > > On Monday 29 November 2021 10:22:58 Stefan Roese wrote:
> > > > On 11/29/21 10:06, Pali Rohár wrote:
> > > > 
> > > > 
> > > > 
> > > > > > > After this DTS change, pci-mvebu.c will just replace value of 
> > > > > > > current
> > > > > > > number of lanes (which is set to 4 by serdes code) to value from 
> > > > > > > DTS,
> > > > > > > which is 4. Therefore there should be no change.
> > > > > > > 
> > > > > > > Could you test whole patch series with above DTS change if it 
> > > > > > > works
> > > > > > > properly on Theadorable board?
> > > > > > 
> > > > > > Yes, I don't see any issues with this patchset applied plus this DT
> > > > > > patch on theadorable. The PCIe links are up and with the correct 
> > > > > > width.
> > > > > > 
> > > > > > What I'm wondering is, when exactly does the PCIe RP start the link
> > > > > > establishment. In my case with AXP this is still in the AXP serdes 
> > > > > > code
> > > > > > of course. But in the A38x code, it should be in the PCIe controller
> > > > > > driver now AFAIU. I see that you configure the link width in the
> > > > > > controller and do some other configuration (address windows etc), 
> > > > > > but
> > > > > > at the end you "simply" wait for the link to come up via
> > > > > > mvebu_pcie_wait_for_link(). I would have expected here some special
> > > > > > command (config bit?) to the PCIe controller to start the link
> > > > > > establishment. So when exactly does the A38x start this action?
> > > > > 
> > > > > That is interesting question... While I'm reading it again, I really 
> > > > > do
> > > > > not know. Because you are right that mvebu_pcie_wait_for_link() is 
> > > > > just
> > > > > waiting for a link and it "magically" comes up. I have tested it on 
> > > > > A385
> > > > > and it is stable with different Compex Atheros cards which caused 
> > > > > issues
> > > > > in past also on A3720.
> > > > 
> > > > I would prefer, to fully understand when exactly the link establishment
> > > > is started. Since this is crucial for the setup of the controller that
> > > > needs to be done *before* the link starts to come up.
> > > 
> > > I try to dig as more information as possible and finally I find out that
> > > important information is available also in now removed, but originally
> > > public A38x documentation. Thankfully web archive has copy of it:
> > > 
> > > https://web.archive.org/web/20200420191927/https://www.marvell.com/content/dam/marvell/en/public-collateral/embedded-processors/marvell-embedded-processors-armada-38x-functional-specifications-2015-11.pdf
> > > 
> > >17.3 Link Initialization
> > > 
> > >In case the initialization fails and no link is established, the PHY
> > >will keep on trying to initiate a link forever unless the port is
> > >disabled. As long as the port is enabled, the PHY will continue trying
> > >to establish a link; once the PHY identifies that a device is
> > >connected to it, a link will be established.
> > > 
> > > PCIe port is enabled by bits in SoC Control 1 Register, which is done in
> > > U-Boot SerDes initialization code. This is IIRC SoC specific, and reason
> > > why every Armada SoC has own SerDes init code.
> > > 
> > > And looks like that due to "the PHY will keep on trying to initiate a
> > > link forever", the PCIe link comes up when pci-mvebu.c sets all required
> > > registers to correct values. E.g. set correct mode (RC vs endpoint),
> > > link width (x1 vs x4), etc...
> > > 
> > > > Could you perhaps try to remove some of the register configurations in
> > > > the MVEBU PCIe driver to see, if the link establishment relies on this
> > > > register to be written to (e.g. PCI_EXP_LNKCAP)?
> > > 
> > > First port on A385 is by default set to X4 width, other ports to X1
> > > width. Without updating LNKCAP to correct width, card in first PCIe port
> > > never initialize. And cards in other ports are initialized even before
> > > pci-mvebu.c starts configuration.
> > 
> > So the PCIe ports are now trying to establish the links, even when the
> > correct configuration is not yet done. This might work but sound far
> > from perfect to me IMHO.
> 
> Yes, it looks like (based on behavior of the first port). And it is not
> perfect, just another mess :-(
> 
> > > So seems that this matches above behavior. SerDes init code enabled all
> > > PCIe ports. Ports which are using default configuration (second, third)
> > > are immediately initialized and link is established. Port which requires
> > > additional configuration (first port, for switching from X4 to X1) just
> > > stay in that "keep on trying to initiate a link forever" state until
> > > pci-mvebu starts and set PCI_EXP_LNKCAP register, after which PHY try X1
> > > width and success. And seems that this is the reason why 

[PATCH v2] efi_loader: tcg2: Return success even when TPM device is not found

2021-11-29 Thread Michal Simek
For systems which have TPM support enabled but actual device is missing
there is no reason to show a message that measurement failed in
efi_load_pe(). To ensure that the patch is returning EFI_SUCCESS even for
cases where TPM device is not found.
The reason is that other parts of the code return also EFI_NOT_FOUND in
tcg2_measure_pe_image() (e.g efi_search_protocol) that's why this error
code can't be checked but still it needs to be reported.

The same logic is also used in efi_tcg2_get_eventlog() added by
commit c8d0fd582576 ("efi_loader: Introduce eventlog support for
TCG2_PROTOCOL").

Signed-off-by: Michal Simek 
---

Changes in v2:
- Change subject and description
- Change logic in different location
- Origin thread was 
https://lore.kernel.org/r/657a869c04e9b09e3bd2e6fd74ff94320b7fbe9b.1638191161.git.michal.si...@xilinx.com

 lib/efi_loader/efi_tcg2.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index 8c1f22e3377b..db785f4d8c27 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -888,7 +888,8 @@ efi_status_t tcg2_measure_pe_image(void *efi, u64 efi_size,
 
ret = platform_get_tpm2_device();
if (ret != EFI_SUCCESS)
-   return ret;
+   /* don't fail when TPM is not found */
+   return EFI_SUCCESS;
 
switch (handle->image_type) {
case IMAGE_SUBSYSTEM_EFI_APPLICATION:
-- 
2.33.1



Re: [PATCH 08/10] microblaze: add Kconfig symbol for the vector base address

2021-11-29 Thread Michal Simek
st 17. 11. 2021 v 13:41 odesílatel Ovidiu Panait
 napsal:
>
> MicroBlaze vector base address is configurable (hdl C_BASE_VECTORS
> configuration parameter). Current code assumes that the reset vector
> location is always 0x0.
>
> Add the XILINX_MICROBLAZE0_VECTOR_BASE_ADDR Kconfig option so the user
> can adjust the reset vector address.
>
> Signed-off-by: Ovidiu Panait 
> ---
>
>  board/xilinx/microblaze-generic/Kconfig | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/board/xilinx/microblaze-generic/Kconfig 
> b/board/xilinx/microblaze-generic/Kconfig
> index e3243297fe..0aa88feb55 100644
> --- a/board/xilinx/microblaze-generic/Kconfig
> +++ b/board/xilinx/microblaze-generic/Kconfig
> @@ -42,4 +42,8 @@ config XILINX_MICROBLAZE0_USR_EXCEP
> bool "MicroBlaze user exception support"
> default y
>
> +config XILINX_MICROBLAZE0_VECTOR_BASE_ADDR
> +   hex "Location of MicroBlaze vectors"
> +   default 0x0
> +

Here also having a small paragraph for everybody to understand what's
going on would be useful.

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs


Re: [PATCH 07/10] microblaze: migrate CONFIG_SYS_USR_EXCEP to Kconfig

2021-11-29 Thread Michal Simek
st 17. 11. 2021 v 13:41 odesílatel Ovidiu Panait
 napsal:
>
> Migrate CONFIG_SYS_USR_EXCEP to Kconfig. Also, rename it to
> XILINX_MICROBLAZE0_USR_EXCEP in order to match the naming convention of
> microblaze-generic Kconfig options.
>
> Signed-off-by: Ovidiu Panait 
> ---
>
>  arch/microblaze/cpu/exception.c | 2 +-
>  arch/microblaze/cpu/start.S | 2 +-
>  board/xilinx/microblaze-generic/Kconfig | 4 
>  include/configs/microblaze-generic.h| 2 --
>  scripts/config_whitelist.txt| 1 -
>  5 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/arch/microblaze/cpu/exception.c b/arch/microblaze/cpu/exception.c
> index b8dedc4e19..e9476abedb 100644
> --- a/arch/microblaze/cpu/exception.c
> +++ b/arch/microblaze/cpu/exception.c
> @@ -55,7 +55,7 @@ void _hw_exception_handler (void)
> hang();
>  }
>
> -#ifdef CONFIG_SYS_USR_EXCEP
> +#if CONFIG_IS_ENABLED(XILINX_MICROBLAZE0_USR_EXCEP)
>  void _exception_handler (void)
>  {
> puts("User vector_exception\n");
> diff --git a/arch/microblaze/cpu/start.S b/arch/microblaze/cpu/start.S
> index 74ed998c55..68f97f426c 100644
> --- a/arch/microblaze/cpu/start.S
> +++ b/arch/microblaze/cpu/start.S
> @@ -144,7 +144,7 @@ __setup_exceptions:
> rsubi   r8, r10, 0x6
> sh  r6, r0, r8
>
> -#ifdef CONFIG_SYS_USR_EXCEP
> +#if CONFIG_IS_ENABLED(XILINX_MICROBLAZE0_USR_EXCEP)
> /* user_vector_exception */
> swi r2, r0, 0x8 /* user vector exception - imm opcode */
> swi r3, r0, 0xC /* user vector exception - brai opcode */
> diff --git a/board/xilinx/microblaze-generic/Kconfig 
> b/board/xilinx/microblaze-generic/Kconfig
> index f2fa0f72b1..e3243297fe 100644
> --- a/board/xilinx/microblaze-generic/Kconfig
> +++ b/board/xilinx/microblaze-generic/Kconfig
> @@ -38,4 +38,8 @@ config XILINX_MICROBLAZE0_HW_VER
> string "Core version number"
> default "7.10.d"
>
> +config XILINX_MICROBLAZE0_USR_EXCEP
> +   bool "MicroBlaze user exception support"
> +   default y

Can you please write a small paragraph here?

Thanks,
Michal


Re: [PATCH] efi_loader: Do not show error message if TPM is not present

2021-11-29 Thread Ilias Apalodimas
Hi Michal

On Mon, 29 Nov 2021 at 15:44, Michal Simek  wrote:
>
>
>
> On 11/29/21 14:30, Ilias Apalodimas wrote:
> > Hi Michal,
> >
> > On Mon, 29 Nov 2021 at 15:06, Michal Simek  wrote:
> >>
> >> For systems which have TPM support enabled but actual device is missing
> >> there is no reason to show a message that measurement failed.
> >> That's why properly check error code which is returned.
> >>
> >> Signed-off-by: Michal Simek 
> >> ---
> >>
> >>   lib/efi_loader/efi_image_loader.c | 5 +++--
> >>   1 file changed, 3 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/lib/efi_loader/efi_image_loader.c 
> >> b/lib/efi_loader/efi_image_loader.c
> >> index eb95580538cc..c6a254dc25dd 100644
> >> --- a/lib/efi_loader/efi_image_loader.c
> >> +++ b/lib/efi_loader/efi_image_loader.c
> >> @@ -934,8 +934,9 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj 
> >> *handle,
> >>
> >>   #if CONFIG_IS_ENABLED(EFI_TCG2_PROTOCOL)
> >>  /* Measure an PE/COFF image */
> >> -   if (tcg2_measure_pe_image(efi, efi_size, handle,
> >> - loaded_image_info))
> >> +   ret = tcg2_measure_pe_image(efi, efi_size, handle,
> >> +   loaded_image_info);
> >> +   if (ret && ret != EFI_NOT_FOUND)
> >>  log_err("PE image measurement failed\n");
> >>   #endif
> >
> > Indeed that's needed.  Looking at it again though maybe it's better to
> > add an identical check in tcg2_measure_pe_image() and return
> > EFI_SUCCESS if platform_get_tpm2_device() returned EFI_NOT_FOUND.  The
> > reason is that other parts of the code return EFI_NOT_FOUND in that
> > function (e.g efi_search_protocol).  So we need to make sure we report
> > the error in that case.
>
> just like this?
>
> diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> index 8c1f22e3377b..db6e7488b7fb 100644
> --- a/lib/efi_loader/efi_tcg2.c
> +++ b/lib/efi_loader/efi_tcg2.c
> @@ -887,8 +887,10 @@ efi_status_t tcg2_measure_pe_image(void *efi, u64
> efi_size,
>  struct efi_handler *handler;
>
>  ret = platform_get_tpm2_device();
> -   if (ret != EFI_SUCCESS)
> +   if (ret != EFI_SUCCESS) {
> +   ret = EFI_SUCCESS;
>  return ret;
> +   }
>

Yea I don't expect platform_get_tpm2_device() to change.  Can you also
add a comment on why we do that for  future readers?

Cheers
/Ilias
>  switch (handle->image_type) {
>  case IMAGE_SUBSYSTEM_EFI_APPLICATION:
>
>
> M


Re: [PATCH] efi_loader: Do not show error message if TPM is not present

2021-11-29 Thread Michal Simek




On 11/29/21 14:30, Ilias Apalodimas wrote:

Hi Michal,

On Mon, 29 Nov 2021 at 15:06, Michal Simek  wrote:


For systems which have TPM support enabled but actual device is missing
there is no reason to show a message that measurement failed.
That's why properly check error code which is returned.

Signed-off-by: Michal Simek 
---

  lib/efi_loader/efi_image_loader.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/efi_loader/efi_image_loader.c 
b/lib/efi_loader/efi_image_loader.c
index eb95580538cc..c6a254dc25dd 100644
--- a/lib/efi_loader/efi_image_loader.c
+++ b/lib/efi_loader/efi_image_loader.c
@@ -934,8 +934,9 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj 
*handle,

  #if CONFIG_IS_ENABLED(EFI_TCG2_PROTOCOL)
 /* Measure an PE/COFF image */
-   if (tcg2_measure_pe_image(efi, efi_size, handle,
- loaded_image_info))
+   ret = tcg2_measure_pe_image(efi, efi_size, handle,
+   loaded_image_info);
+   if (ret && ret != EFI_NOT_FOUND)
 log_err("PE image measurement failed\n");
  #endif


Indeed that's needed.  Looking at it again though maybe it's better to
add an identical check in tcg2_measure_pe_image() and return
EFI_SUCCESS if platform_get_tpm2_device() returned EFI_NOT_FOUND.  The
reason is that other parts of the code return EFI_NOT_FOUND in that
function (e.g efi_search_protocol).  So we need to make sure we report
the error in that case.


just like this?

diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index 8c1f22e3377b..db6e7488b7fb 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -887,8 +887,10 @@ efi_status_t tcg2_measure_pe_image(void *efi, u64 
efi_size,

struct efi_handler *handler;

ret = platform_get_tpm2_device();
-   if (ret != EFI_SUCCESS)
+   if (ret != EFI_SUCCESS) {
+   ret = EFI_SUCCESS;
return ret;
+   }

switch (handle->image_type) {
case IMAGE_SUBSYSTEM_EFI_APPLICATION:


M


Re: [PATCH] efi_loader: Do not show error message if TPM is not present

2021-11-29 Thread Ilias Apalodimas
Hi Michal,

On Mon, 29 Nov 2021 at 15:06, Michal Simek  wrote:
>
> For systems which have TPM support enabled but actual device is missing
> there is no reason to show a message that measurement failed.
> That's why properly check error code which is returned.
>
> Signed-off-by: Michal Simek 
> ---
>
>  lib/efi_loader/efi_image_loader.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/lib/efi_loader/efi_image_loader.c 
> b/lib/efi_loader/efi_image_loader.c
> index eb95580538cc..c6a254dc25dd 100644
> --- a/lib/efi_loader/efi_image_loader.c
> +++ b/lib/efi_loader/efi_image_loader.c
> @@ -934,8 +934,9 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj 
> *handle,
>
>  #if CONFIG_IS_ENABLED(EFI_TCG2_PROTOCOL)
> /* Measure an PE/COFF image */
> -   if (tcg2_measure_pe_image(efi, efi_size, handle,
> - loaded_image_info))
> +   ret = tcg2_measure_pe_image(efi, efi_size, handle,
> +   loaded_image_info);
> +   if (ret && ret != EFI_NOT_FOUND)
> log_err("PE image measurement failed\n");
>  #endif

Indeed that's needed.  Looking at it again though maybe it's better to
add an identical check in tcg2_measure_pe_image() and return
EFI_SUCCESS if platform_get_tpm2_device() returned EFI_NOT_FOUND.  The
reason is that other parts of the code return EFI_NOT_FOUND in that
function (e.g efi_search_protocol).  So we need to make sure we report
the error in that case.


>
> --
> 2.33.1
>

Thanks
/Ilias


Re: [PATCH u-boot-marvell 02/10] arm: mvebu: a38x: serdes: Move non-serdes PCIe code to pci_mvebu.c

2021-11-29 Thread Pali Rohár
On Monday 29 November 2021 13:30:45 Stefan Roese wrote:
> Hi Pali,
> 
> On 11/29/21 12:47, Pali Rohár wrote:
> > Hello!
> > 
> > On Monday 29 November 2021 10:22:58 Stefan Roese wrote:
> > > On 11/29/21 10:06, Pali Rohár wrote:
> > > 
> > > 
> > > 
> > > > > > After this DTS change, pci-mvebu.c will just replace value of 
> > > > > > current
> > > > > > number of lanes (which is set to 4 by serdes code) to value from 
> > > > > > DTS,
> > > > > > which is 4. Therefore there should be no change.
> > > > > > 
> > > > > > Could you test whole patch series with above DTS change if it works
> > > > > > properly on Theadorable board?
> > > > > 
> > > > > Yes, I don't see any issues with this patchset applied plus this DT
> > > > > patch on theadorable. The PCIe links are up and with the correct 
> > > > > width.
> > > > > 
> > > > > What I'm wondering is, when exactly does the PCIe RP start the link
> > > > > establishment. In my case with AXP this is still in the AXP serdes 
> > > > > code
> > > > > of course. But in the A38x code, it should be in the PCIe controller
> > > > > driver now AFAIU. I see that you configure the link width in the
> > > > > controller and do some other configuration (address windows etc), but
> > > > > at the end you "simply" wait for the link to come up via
> > > > > mvebu_pcie_wait_for_link(). I would have expected here some special
> > > > > command (config bit?) to the PCIe controller to start the link
> > > > > establishment. So when exactly does the A38x start this action?
> > > > 
> > > > That is interesting question... While I'm reading it again, I really do
> > > > not know. Because you are right that mvebu_pcie_wait_for_link() is just
> > > > waiting for a link and it "magically" comes up. I have tested it on A385
> > > > and it is stable with different Compex Atheros cards which caused issues
> > > > in past also on A3720.
> > > 
> > > I would prefer, to fully understand when exactly the link establishment
> > > is started. Since this is crucial for the setup of the controller that
> > > needs to be done *before* the link starts to come up.
> > 
> > I try to dig as more information as possible and finally I find out that
> > important information is available also in now removed, but originally
> > public A38x documentation. Thankfully web archive has copy of it:
> > 
> > https://web.archive.org/web/20200420191927/https://www.marvell.com/content/dam/marvell/en/public-collateral/embedded-processors/marvell-embedded-processors-armada-38x-functional-specifications-2015-11.pdf
> > 
> >17.3 Link Initialization
> > 
> >In case the initialization fails and no link is established, the PHY
> >will keep on trying to initiate a link forever unless the port is
> >disabled. As long as the port is enabled, the PHY will continue trying
> >to establish a link; once the PHY identifies that a device is
> >connected to it, a link will be established.
> > 
> > PCIe port is enabled by bits in SoC Control 1 Register, which is done in
> > U-Boot SerDes initialization code. This is IIRC SoC specific, and reason
> > why every Armada SoC has own SerDes init code.
> > 
> > And looks like that due to "the PHY will keep on trying to initiate a
> > link forever", the PCIe link comes up when pci-mvebu.c sets all required
> > registers to correct values. E.g. set correct mode (RC vs endpoint),
> > link width (x1 vs x4), etc...
> > 
> > > Could you perhaps try to remove some of the register configurations in
> > > the MVEBU PCIe driver to see, if the link establishment relies on this
> > > register to be written to (e.g. PCI_EXP_LNKCAP)?
> > 
> > First port on A385 is by default set to X4 width, other ports to X1
> > width. Without updating LNKCAP to correct width, card in first PCIe port
> > never initialize. And cards in other ports are initialized even before
> > pci-mvebu.c starts configuration.
> 
> So the PCIe ports are now trying to establish the links, even when the
> correct configuration is not yet done. This might work but sound far
> from perfect to me IMHO.

Yes, it looks like (based on behavior of the first port). And it is not
perfect, just another mess :-(

> > So seems that this matches above behavior. SerDes init code enabled all
> > PCIe ports. Ports which are using default configuration (second, third)
> > are immediately initialized and link is established. Port which requires
> > additional configuration (first port, for switching from X4 to X1) just
> > stay in that "keep on trying to initiate a link forever" state until
> > pci-mvebu starts and set PCI_EXP_LNKCAP register, after which PHY try X1
> > width and success. And seems that this is the reason why 100ms timeout
> > is needed... As at this stage when pci-mvebu.c switches X4 to X1, init
> > timeout as defined in PCIe spec (that 100ms) starts ticking. For other
> > ports it starts ticking when serdes init code enables ports.
> > 
> > 
> > I have looked into all PCIe registers which are present in 

[BUG] emmc `env erase` erase unrelated data

2021-11-29 Thread Francesco Dolcini
Hello,
I noticed an issue with env erase command when environment is stored in a
emmc device, in case start/end are not aligned to the emmc erase groups
size additional data is erased with just a warning.

```
Erasing Environment on MMC...

Caution! Your devices Erase group is 0x400
The erase range would be change to 0xf800~0xfbff
```

Output from `mmc info`:

```
Device: FSL_SDHC
Manufacturer ID: 13
OEM: 14e
Name: S0J56
Bus Speed: 2
Mode: HS400 (200MHz)
Rd Block Len: 512
MMC version 5.1
High Capacity: Yes
Capacity: 14.8 GiB
Bus Width: 8-bit DDR
Erase Group Size: 512 KiB
HC WP Group Size: 8 MiB
User Capacity: 14.8 GiB WRREL
Boot Capacity: 31.5 MiB ENH
RPMB Capacity: 4 MiB ENH
```

I do not think that this is the correct behavior, I think that in case
the env is not aligned to the erase block size the erase should either
fail or fall back to just writing 0xff. Not sure if just changing
`mmc_berase()` is going to affect any other use case in which is valid
to have a non-aligned start/size and just erase around it after a
warning.

Any comment?

Francesco



[PATCH] efi_loader: Do not show error message if TPM is not present

2021-11-29 Thread Michal Simek
For systems which have TPM support enabled but actual device is missing
there is no reason to show a message that measurement failed.
That's why properly check error code which is returned.

Signed-off-by: Michal Simek 
---

 lib/efi_loader/efi_image_loader.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/efi_loader/efi_image_loader.c 
b/lib/efi_loader/efi_image_loader.c
index eb95580538cc..c6a254dc25dd 100644
--- a/lib/efi_loader/efi_image_loader.c
+++ b/lib/efi_loader/efi_image_loader.c
@@ -934,8 +934,9 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj 
*handle,
 
 #if CONFIG_IS_ENABLED(EFI_TCG2_PROTOCOL)
/* Measure an PE/COFF image */
-   if (tcg2_measure_pe_image(efi, efi_size, handle,
- loaded_image_info))
+   ret = tcg2_measure_pe_image(efi, efi_size, handle,
+   loaded_image_info);
+   if (ret && ret != EFI_NOT_FOUND)
log_err("PE image measurement failed\n");
 #endif
 
-- 
2.33.1



Re: [PATCH] xilinx: firmware: Move dcache handling directly to pmufw load config

2021-11-29 Thread Michal Simek
čt 18. 11. 2021 v 13:00 odesílatel Michal Simek
 napsal:
>
> Core function should make sure that data is stored properly that's why move
> cache operations directly to zynqmp_pmufw_load_config_object() to be able
> to call it from other functions.
>
> Signed-off-by: Michal Simek 
> ---
>
>  board/xilinx/zynqmp/cmds.c | 1 -
>  drivers/firmware/firmware-zynqmp.c | 3 +++
>  2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/board/xilinx/zynqmp/cmds.c b/board/xilinx/zynqmp/cmds.c
> index b15c0f599bda..5a277c712f60 100644
> --- a/board/xilinx/zynqmp/cmds.c
> +++ b/board/xilinx/zynqmp/cmds.c
> @@ -211,7 +211,6 @@ static int do_zynqmp_pmufw(struct cmd_tbl *cmdtp, int 
> flag, int argc,
>
> addr = hextoul(argv[2], NULL);
> size = hextoul(argv[3], NULL);
> -   flush_dcache_range((ulong)addr, (ulong)(addr + size));
>
> zynqmp_pmufw_load_config_object((const void *)(uintptr_t)addr,
> (size_t)size);
> diff --git a/drivers/firmware/firmware-zynqmp.c 
> b/drivers/firmware/firmware-zynqmp.c
> index b44fede30799..aa20e33b4046 100644
> --- a/drivers/firmware/firmware-zynqmp.c
> +++ b/drivers/firmware/firmware-zynqmp.c
> @@ -6,6 +6,7 @@
>   */
>
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -99,6 +100,8 @@ void zynqmp_pmufw_load_config_object(const void *cfg_obj, 
> size_t size)
>
> printf("Loading new PMUFW cfg obj (%ld bytes)\n", size);
>
> +   flush_dcache_range((ulong)cfg_obj, (ulong)(cfg_obj + size));
> +
> err = xilinx_pm_request(PM_SET_CONFIGURATION, (u32)(u64)cfg_obj, 0, 0,
> 0, ret_payload);
> if (err == XST_PM_NO_ACCESS) {
> --
> 2.33.1
>

Applied.
M

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs


Re: [PATCH] arm64: zynqmp: Switch SOM to shared psu configuration

2021-11-29 Thread Michal Simek
čt 18. 11. 2021 v 12:58 odesílatel Michal Simek
 napsal:
>
> Previous psu init was targeting SOM + KV260 carrier card and also contain
> configurations for other devices on carrier card. This config is removing
> all expected configurations for CC and let U-Boot to handle all of it self.
> This configuration is designed for SOM itself (and I would bet without
> eMMC).
>
> Signed-off-by: Michal Simek 
> ---
>
>  .../zynqmp/zynqmp-sm-k26-revA/psu_init_gpl.c  | 200 ++
>  1 file changed, 70 insertions(+), 130 deletions(-)
>
> diff --git a/board/xilinx/zynqmp/zynqmp-sm-k26-revA/psu_init_gpl.c 
> b/board/xilinx/zynqmp/zynqmp-sm-k26-revA/psu_init_gpl.c
> index c448f2abb1a5..ed025790bc3a 100644
> --- a/board/xilinx/zynqmp/zynqmp-sm-k26-revA/psu_init_gpl.c
> +++ b/board/xilinx/zynqmp/zynqmp-sm-k26-revA/psu_init_gpl.c
> @@ -9,33 +9,31 @@
>  static unsigned long psu_pll_init_data(void)
>  {
> psu_mask_write(0xFF5E0034, 0xFE7FEDEFU, 0x7E4B0C62U);
> -   psu_mask_write(0xFF5E0030, 0x00717F00U, 0x00014600U);
> +   psu_mask_write(0xFF5E0030, 0x00717F00U, 0x00014000U);
> psu_mask_write(0xFF5E0030, 0x0008U, 0x0008U);
> psu_mask_write(0xFF5E0030, 0x0001U, 0x0001U);
> psu_mask_write(0xFF5E0030, 0x0001U, 0xU);
> mask_poll(0xFF5E0040, 0x0002U);
> psu_mask_write(0xFF5E0030, 0x0008U, 0xU);
> -   psu_mask_write(0xFF5E0048, 0x3F00U, 0x0300U);
> -   psu_mask_write(0xFF5E0038, 0x8000U, 0xU);
> +   psu_mask_write(0xFF5E0048, 0x3F00U, 0x0200U);
> psu_mask_write(0xFF5E0108, 0x013F3F07U, 0x01012300U);
> -   psu_mask_write(0xFF5E0024, 0xFE7FEDEFU, 0x7E672C6CU);
> -   psu_mask_write(0xFF5E0020, 0x00717F00U, 0x2D00U);
> +   psu_mask_write(0xFF5E0024, 0xFE7FEDEFU, 0x7E4E2C62U);
> +   psu_mask_write(0xFF5E0020, 0x00717F00U, 0x00013C00U);
> psu_mask_write(0xFF5E0020, 0x0008U, 0x0008U);
> psu_mask_write(0xFF5E0020, 0x0001U, 0x0001U);
> psu_mask_write(0xFF5E0020, 0x0001U, 0xU);
> mask_poll(0xFF5E0040, 0x0001U);
> psu_mask_write(0xFF5E0020, 0x0008U, 0xU);
> -   psu_mask_write(0xFF5E0044, 0x3F00U, 0x0300U);
> -   psu_mask_write(0xFF5E0028, 0x8000U, 0xU);
> +   psu_mask_write(0xFF5E0044, 0x3F00U, 0x0200U);
> psu_mask_write(0xFD1A0024, 0xFE7FEDEFU, 0x7E4B0C62U);
> -   psu_mask_write(0xFD1A0020, 0x00717F00U, 0x00014800U);
> +   psu_mask_write(0xFD1A0020, 0x00717F00U, 0x00015000U);
> psu_mask_write(0xFD1A0020, 0x0008U, 0x0008U);
> psu_mask_write(0xFD1A0020, 0x0001U, 0x0001U);
> psu_mask_write(0xFD1A0020, 0x0001U, 0xU);
> mask_poll(0xFD1A0044, 0x0001U);
> psu_mask_write(0xFD1A0020, 0x0008U, 0xU);
> psu_mask_write(0xFD1A0048, 0x3F00U, 0x0300U);
> -   psu_mask_write(0xFD1A0028, 0x8000U, 0xU);
> +   psu_mask_write(0xFD1A0028, 0x8000U, 0x8033U);
> psu_mask_write(0xFD1A0030, 0xFE7FEDEFU, 0x7E4B0C62U);
> psu_mask_write(0xFD1A002C, 0x00717F00U, 0x00014000U);
> psu_mask_write(0xFD1A002C, 0x0008U, 0x0008U);
> @@ -43,58 +41,43 @@ static unsigned long psu_pll_init_data(void)
> psu_mask_write(0xFD1A002C, 0x0001U, 0xU);
> mask_poll(0xFD1A0044, 0x0002U);
> psu_mask_write(0xFD1A002C, 0x0008U, 0xU);
> -   psu_mask_write(0xFD1A004C, 0x3F00U, 0x0300U);
> -   psu_mask_write(0xFD1A0034, 0x8000U, 0xU);
> -   psu_mask_write(0xFD1A003C, 0xFE7FEDEFU, 0x7E4B0C62U);
> -   psu_mask_write(0xFD1A0038, 0x00717F00U, 0x00014700U);
> +   psu_mask_write(0xFD1A004C, 0x3F00U, 0x0200U);
> +   psu_mask_write(0xFD1A003C, 0xFE7FEDEFU, 0x7E4B0C82U);
> +   psu_mask_write(0xFD1A0038, 0x00717F00U, 0x00015A00U);
> psu_mask_write(0xFD1A0038, 0x0008U, 0x0008U);
> psu_mask_write(0xFD1A0038, 0x0001U, 0x0001U);
> psu_mask_write(0xFD1A0038, 0x0001U, 0xU);
> mask_poll(0xFD1A0044, 0x0004U);
> psu_mask_write(0xFD1A0038, 0x0008U, 0xU);
> psu_mask_write(0xFD1A0050, 0x3F00U, 0x0300U);
> -   psu_mask_write(0xFD1A0040, 0x8000U, 0xU);
>
> return 1;
>  }
>
>  static unsigned long psu_clock_init_data(void)
>  {
> -   psu_mask_write(0xFF5E005C, 0x063F3F07U, 0x06010C00U);
> -   psu_mask_write(0xFF5E0100, 0x013F3F07U, 0x01010600U);
> -   psu_mask_write(0xFF5E0060, 0x023F3F07U, 0x02010600U);
> -   psu_mask_write(0xFF5E004C, 0x023F3F07U, 0x020F0500U);
> -   psu_mask_write(0xFF5E0068, 0x013F3F07U, 0x01010C00U);
> -   psu_mask_write(0xFF5E006C, 0x013F3F07U, 0x01010800U);
> -   psu_mask_write(0xFF5E0070, 0x013F3F07U, 0x01010800U);
> -   psu_mask_write(0xFF18030C, 0x00020003U, 

Re: [PATCH] xilinx: versal: Fix sdhci node name as per DT

2021-11-29 Thread Michal Simek
čt 18. 11. 2021 v 12:57 odesílatel Michal Simek
 napsal:
>
> From: T Karthik Reddy 
>
> Fix the sdhci node name in versal board file as per the name in
> device tree and also check for sdhci node as part of backward
> compatibility.
>
> Signed-off-by: T Karthik Reddy 
> Signed-off-by: Michal Simek 
> ---
>
>  board/xilinx/versal/board.c | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/board/xilinx/versal/board.c b/board/xilinx/versal/board.c
> index 6045eb2baa84..03604d730a0b 100644
> --- a/board/xilinx/versal/board.c
> +++ b/board/xilinx/versal/board.c
> @@ -151,6 +151,8 @@ int board_late_init(void)
> case EMMC_MODE:
> puts("EMMC_MODE\n");
> if (uclass_get_device_by_name(UCLASS_MMC,
> + "mmc@f105", ) &&
> +   uclass_get_device_by_name(UCLASS_MMC,
>   "sdhci@f105", )) {
> puts("Boot from EMMC but without SD1 enabled!\n");
> return -1;
> @@ -162,6 +164,8 @@ int board_late_init(void)
> case SD_MODE:
> puts("SD_MODE\n");
> if (uclass_get_device_by_name(UCLASS_MMC,
> + "mmc@f104", ) &&
> +   uclass_get_device_by_name(UCLASS_MMC,
>   "sdhci@f104", )) {
> puts("Boot from SD0 but without SD0 enabled!\n");
> return -1;
> @@ -177,6 +181,8 @@ int board_late_init(void)
> case SD_MODE1:
> puts("SD_MODE1\n");
> if (uclass_get_device_by_name(UCLASS_MMC,
> + "mmc@f105", ) &&
> +   uclass_get_device_by_name(UCLASS_MMC,
>   "sdhci@f105", )) {
> puts("Boot from SD1 but without SD1 enabled!\n");
> return -1;
> --
> 2.33.1
>

Applied.
M

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs


Re: [PATCH 1/2] arm64: zynqmp: Add resets to all GEMs

2021-11-29 Thread Michal Simek
čt 18. 11. 2021 v 13:42 odesílatel Michal Simek
 napsal:
>
> There is a need to get IP out of reset to operate properly.
>
> Signed-off-by: Michal Simek 
> ---
>
>  arch/arm/dts/zynqmp.dtsi | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/arch/arm/dts/zynqmp.dtsi b/arch/arm/dts/zynqmp.dtsi
> index 07d4d4b91201..2264a80e3312 100644
> --- a/arch/arm/dts/zynqmp.dtsi
> +++ b/arch/arm/dts/zynqmp.dtsi
> @@ -533,6 +533,7 @@
> #stream-id-cells = <1>;
> iommus = < 0x874>;
> power-domains = <_firmware PD_ETH_0>;
> +   resets = <_reset ZYNQMP_RESET_GEM0>;
> };
>
> gem1: ethernet@ff0c {
> @@ -547,6 +548,7 @@
> #stream-id-cells = <1>;
> iommus = < 0x875>;
> power-domains = <_firmware PD_ETH_1>;
> +   resets = <_reset ZYNQMP_RESET_GEM1>;
> };
>
> gem2: ethernet@ff0d {
> @@ -561,6 +563,7 @@
> #stream-id-cells = <1>;
> iommus = < 0x876>;
> power-domains = <_firmware PD_ETH_2>;
> +   resets = <_reset ZYNQMP_RESET_GEM2>;
> };
>
> gem3: ethernet@ff0e {
> @@ -575,6 +578,7 @@
> #stream-id-cells = <1>;
> iommus = < 0x877>;
> power-domains = <_firmware PD_ETH_3>;
> +   resets = <_reset ZYNQMP_RESET_GEM3>;
> };
>
> gpio: gpio@ff0a {
> --
> 2.33.1
>

Applied.
M

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs


Re: [PATCH 2/2] arm64: zynqmp: Remove clock-names from GEM in zynqmp-clk-ccf.dtsi

2021-11-29 Thread Michal Simek
čt 18. 11. 2021 v 13:42 odesílatel Michal Simek
 napsal:
>
> Remove clock-names from GEM nodes from clk-ccf because they should be only
> present in zynqmp.dtsi. And as is visible both clock-names defined didn't
> really match.
>
> Signed-off-by: Michal Simek 
> ---
>
>  arch/arm/dts/zynqmp-clk-ccf.dtsi | 4 
>  arch/arm/dts/zynqmp.dtsi | 8 
>  2 files changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm/dts/zynqmp-clk-ccf.dtsi 
> b/arch/arm/dts/zynqmp-clk-ccf.dtsi
> index b27b0aaf7c9b..664e65896d7e 100644
> --- a/arch/arm/dts/zynqmp-clk-ccf.dtsi
> +++ b/arch/arm/dts/zynqmp-clk-ccf.dtsi
> @@ -169,28 +169,24 @@
> clocks = <_clk LPD_LSBUS>, <_clk GEM0_REF>,
>  <_clk GEM0_TX>, <_clk GEM0_RX>,
>  <_clk GEM_TSU>;
> -   clock-names = "pclk", "hclk", "tx_clk", "rx_clk", "tsu_clk";
>  };
>
>   {
> clocks = <_clk LPD_LSBUS>, <_clk GEM1_REF>,
>  <_clk GEM1_TX>, <_clk GEM1_RX>,
>  <_clk GEM_TSU>;
> -   clock-names = "pclk", "hclk", "tx_clk", "rx_clk", "tsu_clk";
>  };
>
>   {
> clocks = <_clk LPD_LSBUS>, <_clk GEM2_REF>,
>  <_clk GEM2_TX>, <_clk GEM2_RX>,
>  <_clk GEM_TSU>;
> -   clock-names = "pclk", "hclk", "tx_clk", "rx_clk", "tsu_clk";
>  };
>
>   {
> clocks = <_clk LPD_LSBUS>, <_clk GEM3_REF>,
>  <_clk GEM3_TX>, <_clk GEM3_RX>,
>  <_clk GEM_TSU>;
> -   clock-names = "pclk", "hclk", "tx_clk", "rx_clk", "tsu_clk";
>  };
>
>   {
> diff --git a/arch/arm/dts/zynqmp.dtsi b/arch/arm/dts/zynqmp.dtsi
> index 2264a80e3312..015a582d7a79 100644
> --- a/arch/arm/dts/zynqmp.dtsi
> +++ b/arch/arm/dts/zynqmp.dtsi
> @@ -527,7 +527,7 @@
> interrupt-parent = <>;
> interrupts = <0 57 4>, <0 57 4>;
> reg = <0x0 0xff0b 0x0 0x1000>;
> -   clock-names = "pclk", "hclk", "tx_clk";
> +   clock-names = "pclk", "hclk", "tx_clk", "rx_clk", 
> "tsu_clk";
> #address-cells = <1>;
> #size-cells = <0>;
> #stream-id-cells = <1>;
> @@ -542,7 +542,7 @@
> interrupt-parent = <>;
> interrupts = <0 59 4>, <0 59 4>;
> reg = <0x0 0xff0c 0x0 0x1000>;
> -   clock-names = "pclk", "hclk", "tx_clk";
> +   clock-names = "pclk", "hclk", "tx_clk", "rx_clk", 
> "tsu_clk";
> #address-cells = <1>;
> #size-cells = <0>;
> #stream-id-cells = <1>;
> @@ -557,7 +557,7 @@
> interrupt-parent = <>;
> interrupts = <0 61 4>, <0 61 4>;
> reg = <0x0 0xff0d 0x0 0x1000>;
> -   clock-names = "pclk", "hclk", "tx_clk";
> +   clock-names = "pclk", "hclk", "tx_clk", "rx_clk", 
> "tsu_clk";
> #address-cells = <1>;
> #size-cells = <0>;
> #stream-id-cells = <1>;
> @@ -572,7 +572,7 @@
> interrupt-parent = <>;
> interrupts = <0 63 4>, <0 63 4>;
> reg = <0x0 0xff0e 0x0 0x1000>;
> -   clock-names = "pclk", "hclk", "tx_clk";
> +   clock-names = "pclk", "hclk", "tx_clk", "rx_clk", 
> "tsu_clk";
> #address-cells = <1>;
> #size-cells = <0>;
> #stream-id-cells = <1>;
> --
> 2.33.1
>

Applied.
M

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs


Re: [PATCH] arm64: zynqmp: Add u-boot,dm-pre-reloc to dpsub node

2021-11-29 Thread Michal Simek
čt 18. 11. 2021 v 13:40 odesílatel Michal Simek
 napsal:
>
> u-boot,dm-pre-reloc is necessary for DP driver to allocate enough space for
> framebuffer before relocation.
> Power domain driver is called when video console is used for example by
> loading BMP image.
>
> Signed-off-by: Michal Simek 
> ---
>
>  arch/arm/dts/zynqmp.dtsi | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/dts/zynqmp.dtsi b/arch/arm/dts/zynqmp.dtsi
> index 3f3689ffab0b..2264a80e3312 100644
> --- a/arch/arm/dts/zynqmp.dtsi
> +++ b/arch/arm/dts/zynqmp.dtsi
> @@ -965,6 +965,7 @@
> };
>
> zynqmp_dpsub: display@fd4a {
> +   u-boot,dm-pre-reloc;
> compatible = "xlnx,zynqmp-dpsub-1.7";
> status = "disabled";
> reg = <0x0 0xfd4a 0x0 0x1000>,
> --
> 2.33.1
>

Applied.
M

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs


Re: [PATCH u-boot-marvell 02/10] arm: mvebu: a38x: serdes: Move non-serdes PCIe code to pci_mvebu.c

2021-11-29 Thread Stefan Roese

Hi Pali,

On 11/29/21 12:47, Pali Rohár wrote:

Hello!

On Monday 29 November 2021 10:22:58 Stefan Roese wrote:

On 11/29/21 10:06, Pali Rohár wrote:




After this DTS change, pci-mvebu.c will just replace value of current
number of lanes (which is set to 4 by serdes code) to value from DTS,
which is 4. Therefore there should be no change.

Could you test whole patch series with above DTS change if it works
properly on Theadorable board?


Yes, I don't see any issues with this patchset applied plus this DT
patch on theadorable. The PCIe links are up and with the correct width.

What I'm wondering is, when exactly does the PCIe RP start the link
establishment. In my case with AXP this is still in the AXP serdes code
of course. But in the A38x code, it should be in the PCIe controller
driver now AFAIU. I see that you configure the link width in the
controller and do some other configuration (address windows etc), but
at the end you "simply" wait for the link to come up via
mvebu_pcie_wait_for_link(). I would have expected here some special
command (config bit?) to the PCIe controller to start the link
establishment. So when exactly does the A38x start this action?


That is interesting question... While I'm reading it again, I really do
not know. Because you are right that mvebu_pcie_wait_for_link() is just
waiting for a link and it "magically" comes up. I have tested it on A385
and it is stable with different Compex Atheros cards which caused issues
in past also on A3720.


I would prefer, to fully understand when exactly the link establishment
is started. Since this is crucial for the setup of the controller that
needs to be done *before* the link starts to come up.


I try to dig as more information as possible and finally I find out that
important information is available also in now removed, but originally
public A38x documentation. Thankfully web archive has copy of it:

https://web.archive.org/web/20200420191927/https://www.marvell.com/content/dam/marvell/en/public-collateral/embedded-processors/marvell-embedded-processors-armada-38x-functional-specifications-2015-11.pdf

   17.3 Link Initialization

   In case the initialization fails and no link is established, the PHY
   will keep on trying to initiate a link forever unless the port is
   disabled. As long as the port is enabled, the PHY will continue trying
   to establish a link; once the PHY identifies that a device is
   connected to it, a link will be established.

PCIe port is enabled by bits in SoC Control 1 Register, which is done in
U-Boot SerDes initialization code. This is IIRC SoC specific, and reason
why every Armada SoC has own SerDes init code.

And looks like that due to "the PHY will keep on trying to initiate a
link forever", the PCIe link comes up when pci-mvebu.c sets all required
registers to correct values. E.g. set correct mode (RC vs endpoint),
link width (x1 vs x4), etc...


Could you perhaps try to remove some of the register configurations in
the MVEBU PCIe driver to see, if the link establishment relies on this
register to be written to (e.g. PCI_EXP_LNKCAP)?


First port on A385 is by default set to X4 width, other ports to X1
width. Without updating LNKCAP to correct width, card in first PCIe port
never initialize. And cards in other ports are initialized even before
pci-mvebu.c starts configuration.


So the PCIe ports are now trying to establish the links, even when the
correct configuration is not yet done. This might work but sound far
from perfect to me IMHO.


So seems that this matches above behavior. SerDes init code enabled all
PCIe ports. Ports which are using default configuration (second, third)
are immediately initialized and link is established. Port which requires
additional configuration (first port, for switching from X4 to X1) just
stay in that "keep on trying to initiate a link forever" state until
pci-mvebu starts and set PCI_EXP_LNKCAP register, after which PHY try X1
width and success. And seems that this is the reason why 100ms timeout
is needed... As at this stage when pci-mvebu.c switches X4 to X1, init
timeout as defined in PCIe spec (that 100ms) starts ticking. For other
ports it starts ticking when serdes init code enables ports.


I have looked into all PCIe registers which are present in functional
spec, but it looks like that there is no pci-mvebu register which can
turn of LTSSM and link training, like it is in other PCIe controllers.
It looks like that only SoC-specific port enable bits are there.

It is starting to be bigger mess as before... Any suggestion how to
continue with it?

We cannot (easily) move that code which flips PCIe bits in SoC Control 1
Register from SerDes init code to pci-mvebu.c as this is outside of
pci-mvebu.c address space and also it is different on every SoC.
pci-mvebu.c registers are same on all Marvell SoCs, starting from Orion
up to the A39x.


One idea would be, to use a "reset-controller" driver on the Armada
platforms, that is capable to at least 

Re: [RFC PATCH 3/3] imx: imx8mp_evk: override env_get_location

2021-11-29 Thread Marek Behún
On Sun, 28 Nov 2021 18:47:11 +0100
Tommaso Merciai  wrote:

> On Fri, Nov 26, 2021 at 07:00:21PM +0100, Marek Behún wrote:
> > On Fri, 26 Nov 2021 18:43:31 +0100
> > Tommaso Merciai  wrote:
> >   
> > > Override env_get_location function at board level, previously dropped
> > > down from soc.c
> > > 
> > > References:
> > >  - commit f1575f23df1ef704051f218d5bc4aeeb20c2c542
> > > 
> > > Signed-off-by: Tommaso Merciai 
> > > ---
> > > Changes since v1:
> > >  - Remove wrong env_get_offset function from commit
> > > 
> > >  board/freescale/imx8mp_evk/imx8mp_evk.c | 41 +
> > >  1 file changed, 41 insertions(+)
> > > 
> > > diff --git a/board/freescale/imx8mp_evk/imx8mp_evk.c 
> > > b/board/freescale/imx8mp_evk/imx8mp_evk.c
> > > index 62096c24fb..6b2eeaf152 100644
> > > --- a/board/freescale/imx8mp_evk/imx8mp_evk.c
> > > +++ b/board/freescale/imx8mp_evk/imx8mp_evk.c
> > > @@ -5,6 +5,7 @@
> > >  
> > >  #include 
> > >  #include 
> > > +#include 
> > >  #include 
> > >  #include 
> > >  #include 
> > > @@ -17,6 +18,7 @@
> > >  #include 
> > >  #include 
> > >  #include 
> > > +#include 
> > >  
> > >  DECLARE_GLOBAL_DATA_PTR;
> > >  
> > > @@ -32,6 +34,45 @@ static iomux_v3_cfg_t const wdog_pads[] = {
> > >   MX8MP_PAD_GPIO1_IO02__WDOG1_WDOG_B  | MUX_PAD_CTRL(WDOG_PAD_CTRL),
> > >  };
> > >  
> > > +enum env_location env_get_location(enum env_operation op, int prio)
> > > +{
> > > + enum boot_device dev = get_boot_device();
> > > + enum env_location env_loc = ENVL_UNKNOWN;
> > > +
> > > + if (prio)
> > > + return env_loc;
> > > +
> > > + switch (dev) {
> > > +#ifdef CONFIG_ENV_IS_IN_SPI_FLASH
> > > + case QSPI_BOOT:
> > > + env_loc = ENVL_SPI_FLASH;
> > > + break;
> > > +#endif
> > > +#ifdef CONFIG_ENV_IS_IN_NAND
> > > + case NAND_BOOT:
> > > + env_loc = ENVL_NAND;
> > > + break;
> > > +#endif
> > > +#ifdef CONFIG_ENV_IS_IN_MMC
> > > + case SD1_BOOT:
> > > + case SD2_BOOT:
> > > + case SD3_BOOT:
> > > + case MMC1_BOOT:
> > > + case MMC2_BOOT:
> > > + case MMC3_BOOT:
> > > + env_loc =  ENVL_MMC;
> > > + break;
> > > +#endif
> > > + default:
> > > +#if defined(CONFIG_ENV_IS_NOWHERE)
> > > + env_loc = ENVL_NOWHERE;
> > > +#endif  
> > 
> > Using
> >   if (IS_ENABLED(CONFIG_ENV_IS_NOWHERE))
> > instead of #ifdefs is preferred because the compiler then warns about
> > bugs even in the disabled codepaths (that's why checkpatch complains
> > about #ifdefs).
> > 
> > I know that this cannot be combined with switch() in a simple way,
> > though.
> > 
> > Do you need to use switch? Can't you rewrite it to use if()s and use
> > IS_ENABLED()?
> > 
> > Marek  
> 
> Hi Marek,
> Thanks for your review. What do you think about this solution?
> 
> enum env_location env_get_location(enum env_operation op, int prio)
> {
>   enum boot_device dev = get_boot_device();
>   enum env_location env_loc = ENVL_UNKNOWN;
> 
>   if (prio)
>   return env_loc;
> 
>   if (IS_ENABLED(CONFIG_ENV_IS_IN_SPI_FLASH) && dev == QSPI_BOOT) {
>   env_loc = ENVL_SPI_FLASH;
>   }
>   else if (IS_ENABLED(CONFIG_ENV_IS_IN_NAND) && dev == NAND_BOOT) {
>   env_loc = ENVL_NAND;
>   }
>   else if (IS_ENABLED(CONFIG_ENV_IS_IN_MMC)) {
>   switch (dev) {
>   case SD1_BOOT:
>   case SD2_BOOT:
>   case SD3_BOOT:
>   case MMC1_BOOT:
>   case MMC2_BOOT:
>   case MMC3_BOOT:
>   env_loc = ENVL_MMC;
>   break;
>   default:
>   break;
>   }
>   }
>   else if (IS_ENABLED(CONFIG_ENV_IS_NOWHERE)) {
>   env_loc = ENVL_MMC;
>   }
> 
>   return env_loc;
> }

Thanks, this looks ok, just run it through checkpatch to correct the
indentation of 'case' statements and put 'else if' on the same line as
'}':

  if () {
  } else if () {
  } ...

Marek


[PATCH] arm:dts:k3-am64-sk: EMIF tool update to 0.8.0 with 1333MTs for lpddr4

2021-11-29 Thread Sinthu Raja
From: Sinthu Raja 

EMIF tool for AM64 SK is now updated to 0.8.0 that includes
* disabled Write DQ training
* improve CA ODT to 60 ohms

The lpddr4 enabled with periodic WDQ training is causing periodic 26us
stall. This makes the SoC stall without doing anything which leads to
R5 interrupt latency in TCM memory. Due to this periodic training there
are some outstanding CPU transactions waiting for the lpddr4 to complete.

Hence, disable the periodic write DQ training during the
non-initialization stage of lpddr4 which results in an approximate 1us
stall. Also, update the lpddr4 config to improve CA ODT by 60 ohms

The rationales are as follows:
- PI_WDQLVL_EN: 2 Bits register field to support write DQ leveling,
  disable bit 1 that supports Write DQ during non-initialization to
  avoid ~26us stall during code execution.

- MR11_DATA_F1/F2_x register fields value changed to 0x66 that changes
  the CA ODT from 48ohm to 60ohm to improve the eye margin on CA bus by
  increasing the signal swing.

Signed-off-by: James Doublesin 
Signed-off-by: Sinthu Raja 
---
 arch/arm/dts/k3-am64-sk-lp4-1333MTs.dtsi | 28 
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/arch/arm/dts/k3-am64-sk-lp4-1333MTs.dtsi 
b/arch/arm/dts/k3-am64-sk-lp4-1333MTs.dtsi
index 64a159f6d0..dde5ab150d 100644
--- a/arch/arm/dts/k3-am64-sk-lp4-1333MTs.dtsi
+++ b/arch/arm/dts/k3-am64-sk-lp4-1333MTs.dtsi
@@ -2,8 +2,8 @@
 /*
  * Copyright (C) 2021 Texas Instruments Incorporated - http://www.ti.com/
  * This file was generated with the
- * AM64x SysConfig DDR Subsystem Register Configuration Tool v0.06.00
- * Mon Apr 26 2021 20:47:47 GMT-0500 (Central Daylight Time)
+ * AM64x SysConfig DDR Subsystem Register Configuration Tool v0.08.00
+ * Wed Oct 13 2021 10:08:29 GMT-0500 (Central Daylight Time)
  * DDR Type: LPDDR4
  * F0 = 50MHzF1 = 666.7MHzF2 = 666.7MHz
  * Density (per channel): 16Gb
@@ -268,8 +268,8 @@
 #define DDRSS_CTL_251_DATA 0x
 #define DDRSS_CTL_252_DATA 0x
 #define DDRSS_CTL_253_DATA 0x
-#define DDRSS_CTL_254_DATA 0x6600
-#define DDRSS_CTL_255_DATA 0x2766
+#define DDRSS_CTL_254_DATA 0x46004646
+#define DDRSS_CTL_255_DATA 0x2746
 #define DDRSS_CTL_256_DATA 0x0027
 #define DDRSS_CTL_257_DATA 0x0027
 #define DDRSS_CTL_258_DATA 0x0027
@@ -660,13 +660,13 @@
 #define DDRSS_PI_220_DATA 0x00A7
 #define DDRSS_PI_221_DATA 0x1900
 #define DDRSS_PI_222_DATA 0x3256
-#define DDRSS_PI_223_DATA 0x06000301
+#define DDRSS_PI_223_DATA 0x06000101
 #define DDRSS_PI_224_DATA 0x001D0204
 #define DDRSS_PI_225_DATA 0x32120059
-#define DDRSS_PI_226_DATA 0x05000301
+#define DDRSS_PI_226_DATA 0x05000101
 #define DDRSS_PI_227_DATA 0x001D0409
 #define DDRSS_PI_228_DATA 0x32120059
-#define DDRSS_PI_229_DATA 0x05000301
+#define DDRSS_PI_229_DATA 0x05000101
 #define DDRSS_PI_230_DATA 0x0409
 #define DDRSS_PI_231_DATA 0x05030900
 #define DDRSS_PI_232_DATA 0x00040900
@@ -748,7 +748,7 @@
 #define DDRSS_PI_308_DATA 0x0031
 #define DDRSS_PI_309_DATA 0x
 #define DDRSS_PI_310_DATA 0x
-#define DDRSS_PI_311_DATA 0x6600
+#define DDRSS_PI_311_DATA 0x4600
 #define DDRSS_PI_312_DATA 0x00150F27
 #define DDRSS_PI_313_DATA 0x
 #define DDRSS_PI_314_DATA 0x0024
@@ -756,7 +756,7 @@
 #define DDRSS_PI_316_DATA 0x0031
 #define DDRSS_PI_317_DATA 0x
 #define DDRSS_PI_318_DATA 0x
-#define DDRSS_PI_319_DATA 0x6600
+#define DDRSS_PI_319_DATA 0x4600
 #define DDRSS_PI_320_DATA 0x00150F27
 #define DDRSS_PI_321_DATA 0x
 #define DDRSS_PI_322_DATA 0x0004
@@ -772,7 +772,7 @@
 #define DDRSS_PI_332_DATA 0x0031
 #define DDRSS_PI_333_DATA 0x
 #define DDRSS_PI_334_DATA 0x
-#define DDRSS_PI_335_DATA 0x6600
+#define DDRSS_PI_335_DATA 0x4600
 #define DDRSS_PI_336_DATA 0x00150F27
 #define DDRSS_PI_337_DATA 0x
 #define DDRSS_PI_338_DATA 0x0024
@@ -780,7 +780,7 @@
 #define DDRSS_PI_340_DATA 0x0031
 #define DDRSS_PI_341_DATA 0x
 #define DDRSS_PI_342_DATA 0x
-#define DDRSS_PI_343_DATA 0x6600
+#define DDRSS_PI_343_DATA 0x4600
 #define DDRSS_PI_344_DATA 0x00150F27
 #define DDRSS_PHY_0_DATA 0x04F0
 #define DDRSS_PHY_1_DATA 0x
@@ -873,7 +873,7 @@
 #define DDRSS_PHY_88_DATA 0x51516041
 #define DDRSS_PHY_89_DATA 0x31C06000
 #define DDRSS_PHY_90_DATA 0x07AB0340
-#define DDRSS_PHY_91_DATA 0x0100C0C0
+#define DDRSS_PHY_91_DATA 0xC0C0
 #define DDRSS_PHY_92_DATA 0x0304
 #define DDRSS_PHY_93_DATA 0x0403
 #define DDRSS_PHY_94_DATA 0x42100010
@@ -1129,7 +1129,7 @@
 #define DDRSS_PHY_344_DATA 0x51516041
 #define DDRSS_PHY_345_DATA 0x31C06000
 #define DDRSS_PHY_346_DATA 0x07AB0340
-#define DDRSS_PHY_347_DATA 0x0100C0C0
+#define DDRSS_PHY_347_DATA 0xC0C0
 #define DDRSS_PHY_348_DATA 0x0304
 #define DDRSS_PHY_349_DATA 0x0403
 #define DDRSS_PHY_350_DATA 0x42100010
@@ -2157,7 +2157,7 @@
 #define DDRSS_PHY_1372_DATA 0x0002
 #define 

RE: [PATCH 1/2] fsl-layerscape: add dtb overlay feature

2021-11-29 Thread Sahil Malhotra (OSS)
Hi Michael,


-Original Message-
From: Michael Walle  
Sent: Wednesday, November 17, 2021 11:51 PM
To: Sahil Malhotra (OSS) 
Cc: Clément Faure ; Gaurav Jain ; 
Pankaj Gupta ; Priyanka Jain ; 
u-boot@lists.denx.de; Varun Sethi ; Ye Li ; 
ZHIZHIKIN Andrey 
Subject: Re: [PATCH 1/2] fsl-layerscape: add dtb overlay feature

Hi Sahil,

Am 2021-11-17 19:11, schrieb Sahil Malhotra (OSS):
>> Could you please add some description what this is doing and for what 
>> this is intended? To have a "DTB overlay feature", it is enough to 
>> just enable CONFIG_OF_LIBFDT_OVERLAY.
> I will add some description, and yes for DTB overlay feature, it is 
> enough to enable CONFIG_OF_LIBFDT_OVERLAY but we need to do this step 
> before booting the kernel that's why also have to enable 
> CONFIG_OF_SYSTEM_SETUP.

> Ok. What will the overlay do? Could you give an example?
This overlay will be disabling the crypto nodes which will be used by optee in 
secure world, so that linux should not use it.


>> Apparently you're adding an overlay passed by optee. Doesn't this 
>> have to be applied to u-boot's control dtb too?
> Yes, we will be applying the overlay passed by optee, yes it will be 
> applied to dtb which will be passed to uboot for kernel booting.

> If I read this patch correctly, you're modifying the DT before you jump to 
> linux. But I was asking whether you also have to modify the DT which is used 
> by u-boot. Eg. if you disable some kind of crypto nodes (because optee will 
> use them in secure world), this also have to communicated to u-boot, not only 
> linux, no?
Yes, I got your point now, and is very valid, but as of now for u-boot we are 
just using the first available node for communicating with CAAM leaving other 
job rings as it is. 
So we need not to apply overlay to DTB used by uboot.

Regards,
Sahil Malhotra


Re: [PATCH u-boot-marvell 02/10] arm: mvebu: a38x: serdes: Move non-serdes PCIe code to pci_mvebu.c

2021-11-29 Thread Pali Rohár
Hello!

On Monday 29 November 2021 10:22:58 Stefan Roese wrote:
> On 11/29/21 10:06, Pali Rohár wrote:
> 
> 
> 
> > > > After this DTS change, pci-mvebu.c will just replace value of current
> > > > number of lanes (which is set to 4 by serdes code) to value from DTS,
> > > > which is 4. Therefore there should be no change.
> > > > 
> > > > Could you test whole patch series with above DTS change if it works
> > > > properly on Theadorable board?
> > > 
> > > Yes, I don't see any issues with this patchset applied plus this DT
> > > patch on theadorable. The PCIe links are up and with the correct width.
> > > 
> > > What I'm wondering is, when exactly does the PCIe RP start the link
> > > establishment. In my case with AXP this is still in the AXP serdes code
> > > of course. But in the A38x code, it should be in the PCIe controller
> > > driver now AFAIU. I see that you configure the link width in the
> > > controller and do some other configuration (address windows etc), but
> > > at the end you "simply" wait for the link to come up via
> > > mvebu_pcie_wait_for_link(). I would have expected here some special
> > > command (config bit?) to the PCIe controller to start the link
> > > establishment. So when exactly does the A38x start this action?
> > 
> > That is interesting question... While I'm reading it again, I really do
> > not know. Because you are right that mvebu_pcie_wait_for_link() is just
> > waiting for a link and it "magically" comes up. I have tested it on A385
> > and it is stable with different Compex Atheros cards which caused issues
> > in past also on A3720.
> 
> I would prefer, to fully understand when exactly the link establishment
> is started. Since this is crucial for the setup of the controller that
> needs to be done *before* the link starts to come up.

I try to dig as more information as possible and finally I find out that
important information is available also in now removed, but originally
public A38x documentation. Thankfully web archive has copy of it:

https://web.archive.org/web/20200420191927/https://www.marvell.com/content/dam/marvell/en/public-collateral/embedded-processors/marvell-embedded-processors-armada-38x-functional-specifications-2015-11.pdf

  17.3 Link Initialization

  In case the initialization fails and no link is established, the PHY
  will keep on trying to initiate a link forever unless the port is
  disabled. As long as the port is enabled, the PHY will continue trying
  to establish a link; once the PHY identifies that a device is
  connected to it, a link will be established.

PCIe port is enabled by bits in SoC Control 1 Register, which is done in
U-Boot SerDes initialization code. This is IIRC SoC specific, and reason
why every Armada SoC has own SerDes init code.

And looks like that due to "the PHY will keep on trying to initiate a
link forever", the PCIe link comes up when pci-mvebu.c sets all required
registers to correct values. E.g. set correct mode (RC vs endpoint),
link width (x1 vs x4), etc...

> Could you perhaps try to remove some of the register configurations in
> the MVEBU PCIe driver to see, if the link establishment relies on this
> register to be written to (e.g. PCI_EXP_LNKCAP)?

First port on A385 is by default set to X4 width, other ports to X1
width. Without updating LNKCAP to correct width, card in first PCIe port
never initialize. And cards in other ports are initialized even before
pci-mvebu.c starts configuration.

So seems that this matches above behavior. SerDes init code enabled all
PCIe ports. Ports which are using default configuration (second, third)
are immediately initialized and link is established. Port which requires
additional configuration (first port, for switching from X4 to X1) just
stay in that "keep on trying to initiate a link forever" state until
pci-mvebu starts and set PCI_EXP_LNKCAP register, after which PHY try X1
width and success. And seems that this is the reason why 100ms timeout
is needed... As at this stage when pci-mvebu.c switches X4 to X1, init
timeout as defined in PCIe spec (that 100ms) starts ticking. For other
ports it starts ticking when serdes init code enables ports.


I have looked into all PCIe registers which are present in functional
spec, but it looks like that there is no pci-mvebu register which can
turn of LTSSM and link training, like it is in other PCIe controllers.
It looks like that only SoC-specific port enable bits are there.

It is starting to be bigger mess as before... Any suggestion how to
continue with it?

We cannot (easily) move that code which flips PCIe bits in SoC Control 1
Register from SerDes init code to pci-mvebu.c as this is outside of
pci-mvebu.c address space and also it is different on every SoC.
pci-mvebu.c registers are same on all Marvell SoCs, starting from Orion
up to the A39x.

> Thanks,
> Stefan


Re: [RESEND RFC PATCH 09/10] FWU: Add support for FWU Multi Bank Update feature

2021-11-29 Thread Sughosh Ganu
On Fri, 26 Nov 2021 at 18:25, Heinrich Schuchardt 
wrote:

> On 11/25/21 08:13, Sughosh Ganu wrote:
> > The FWU Multi Bank Update feature supports updation of firmware images
> > to one of multiple sets(also called banks) of images. The firmware
> > images are clubbed together in banks, with the system booting images
> > from the active bank. Information on the images such as which bank
> > they belong to is stored as part of the metadata structure, which is
> > stored on the same storage media as the firmware images on a dedicated
> > partition.
> >
> > At the time of update, the metadata is read to identify the bank to
> > which the images need to be flashed(update bank). On a successful
> > update, the metadata is modified to set the updated bank as active
> > bank to subsequently boot from.
> >
> > Signed-off-by: Sughosh Ganu 
> > ---
> >   include/fwu_metadata.h   |  10 ++
> >   lib/Kconfig  |  32 ++
> >   lib/Makefile |   1 +
> >   lib/efi_loader/efi_capsule.c | 190 ++-
> >   lib/fwu_updates/Makefile |  11 ++
> >   lib/fwu_updates/fwu.c|  29 +-
> >   6 files changed, 269 insertions(+), 4 deletions(-)
> >   create mode 100644 lib/fwu_updates/Makefile
> >
> > diff --git a/include/fwu_metadata.h b/include/fwu_metadata.h
> > index 02897f33a8..2d276a019a 100644
> > --- a/include/fwu_metadata.h
> > +++ b/include/fwu_metadata.h
> > @@ -106,7 +106,16 @@ struct fwu_metadata_ops {
> >   EFI_GUID(0x8a7a84a0, 0x8387, 0x40f6, 0xab, 0x41, \
> >0xa8, 0xb9, 0xa5, 0xa6, 0x0d, 0x23)
> >
> > +#define FWU_OS_REQUEST_FW_REVERT_GUID \
> > + EFI_GUID(0xacd58b4b, 0xc0e8, 0x475f, 0x99, 0xb5, \
> > +  0x6b, 0x3f, 0x7e, 0x07, 0xaa, 0xf0)
> > +
> > +#define FWU_OS_REQUEST_FW_ACCEPT_GUID \
> > + EFI_GUID(0x0c996046, 0xbcc0, 0x4d04, 0x85, 0xec, \
> > +  0xe1, 0xfc, 0xed, 0xf1, 0xc6, 0xf8)
> > +
> >   #define FWU_METADATA_VERSION0x1
> > +#define FWU_IMAGE_ACCEPTED   0x1
> >
> >   extern struct fwu_metadata_ops fwu_gpt_blk_ops;
> >
> > @@ -126,5 +135,6 @@ int fwu_plat_get_update_index(u32 *update_idx);
> >   int fwu_plat_get_blk_desc(struct blk_desc **desc);
> >   void fwu_plat_get_bootidx(void *boot_idx);
> >   int fwu_boottime_checks(void);
> > +int fwu_trial_state_ctr_start(void);
> >
> >   #endif /* _FWU_METADATA_H_ */
> > diff --git a/lib/Kconfig b/lib/Kconfig
> > index 807a4c6ade..7cb306317c 100644
> > --- a/lib/Kconfig
> > +++ b/lib/Kconfig
> > @@ -835,3 +835,35 @@ config PHANDLE_CHECK_SEQ
> > When there are multiple device tree nodes with same name,
> > enable this config option to distinguish them using
> > phandles in fdtdec_get_alias_seq() function.
> > +
> > +config FWU_MULTI_BANK_UPDATE
> > + bool "Enable FWU Multi Bank Update Feature"
>
> Why do we need a configuration variable? Having 1 bank is just a special
> case which should not use separate code.
>

This is needed to distinguish between using the update along with
maintaining the metadata, as described by the specification. With the case
of a single bank, I don't see any reason for having the metadata -- the
firmware update can be carried out using the capsule driver.


> > + depends on EFI_HAVE_CAPSULE_SUPPORT
> > + select PARTITION_TYPE_GUID
> > + select EFI_SETUP_EARLY
> > + help
> > +   Feature for updating firmware images on platforms having
> > +   multiple banks(copies) of the firmware images. One of the
> > +   bank is selected for updating all the firmware components
> > +
> > +config FWU_NUM_BANKS
> > + int "Number of Banks defined by the platform"
> > + depends on FWU_MULTI_BANK_UPDATE
>
> This should default to 1.
>

Like we discussed, I will check if this value and the #images_per_bank can
be incorporated in the metadata structure. If it gets added to the
metadata, the corresponding configs can be removed.


> > + help
> > +   Define the number of banks of firmware images on a platform
> > +
> > +config FWU_NUM_IMAGES_PER_BANK
> > + int "Number of firmware images per bank"
> > + depends on FWU_MULTI_BANK_UPDATE
>
> This dependency makes no sense. Even if I have one bank I can have
> multiple images.
>
> Why should this configuration variable be needed?
> The dfu variables and the capsule define how many images are updated.
>
> > + help
> > +   Define the number of firmware images per bank. This value
> > +   should be the same for all the banks.
> > +
> > +config FWU_TRIAL_STATE_CNT
> > + int "Number of times system boots in Trial State"
> > + depends on FWU_MULTI_BANK_UPDATE
> > + default 3
> > + help
> > +   With FWU Multi Bank Update feature enabled, number of times
> > +   the platform is allowed to boot in Trial State after an
> > +   update.
>
> Do you mean:
>
> "This is the number of boots in trial state before falling back to the
> last successfully used bank."
>

Yes, this 

Re: [RESEND RFC PATCH 07/10] EFI: FMP: Add provision to update image's ImageTypeId in image descriptor

2021-11-29 Thread Sughosh Ganu
hi Heinrich,
Thanks for taking up the review.

On Fri, 26 Nov 2021 at 18:18, Heinrich Schuchardt 
wrote:

> On 11/25/21 08:12, Sughosh Ganu wrote:
> > The FWU Multi Banks Update feature allows updating different types of
> > updatable firmware images on the platform. These image types are
> > identified using the ImageTypeId GUID value. Add support in the
> > GetImageInfo function of the FMP protocol to get the GUID values for
> > the individual images and populate these in the image descriptor for
> > the corresponding images.
> >
> > Signed-off-by: Sughosh Ganu 
> > ---
> >   lib/efi_loader/efi_firmware.c | 76 ---
> >   1 file changed, 71 insertions(+), 5 deletions(-)
> >
> > diff --git a/lib/efi_loader/efi_firmware.c
> b/lib/efi_loader/efi_firmware.c
> > index a1b88dbfc2..a2b639b448 100644
> > --- a/lib/efi_loader/efi_firmware.c
> > +++ b/lib/efi_loader/efi_firmware.c
> > @@ -10,6 +10,7 @@
> >   #include 
> >   #include 
> >   #include 
> > +#include 
> >   #include 
> >   #include 
> >
> > @@ -106,7 +107,8 @@ efi_status_t EFIAPI
> efi_firmware_set_package_info_unsupported(
> >* @descriptor_size:Pointer to descriptor size
> >* package_version: Package version
> >* package_version_name:Package version's name
> > - * image_type:   Image type GUID
> > + * guid_array:   Image type GUID array
> > + * nparts:   Number of partions on the storage device
> >*
> >* Return information bout the current firmware image in @image_info.
> >* @image_info will consist of a number of descriptors.
> > @@ -122,7 +124,7 @@ static efi_status_t efi_get_dfu_info(
> >   efi_uintn_t *descriptor_size,
> >   u32 *package_version,
> >   u16 **package_version_name,
> > - const efi_guid_t *image_type)
> > + const efi_guid_t *guid_array, u32 nparts)
> >   {
> >   struct dfu_entity *dfu;
> >   size_t names_len, total_size;
> > @@ -145,6 +147,19 @@ static efi_status_t efi_get_dfu_info(
> >   return EFI_SUCCESS;
> >   }
> >
> > + if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE)) {
>
> The concept of multiple banks is not related to the concept of multiple
> GUIDs. So this if statement should be removed.


Okay. Will make the change.


>


> > + /*
> > +  * For FWU multi bank updates, the number of partitions
> > +  * should at least be same as dfu partitions or less
> > +  */
> > + if (nparts > dfu_num) {
> > + log_err("Number of dfu alt no's less than
> partitions\n");
> > + dfu_free_entities();
> > +
> > + return EFI_INVALID_PARAMETER;
> > + }
> > + }
> > +
> >   total_size = sizeof(*image_info) * dfu_num + names_len;
> >   /*
> >* we will assume that sizeof(*image_info) * dfu_name
> > @@ -172,7 +187,11 @@ static efi_status_t efi_get_dfu_info(
> >   next = name;
> >   list_for_each_entry(dfu, _list, list) {
> >   image_info[i].image_index = dfu->alt + 1;
> > - image_info[i].image_type_id = *image_type;
> > + if (IS_ENABLED(CONFIG_FWU_MULTI_BANK_UPDATE))
>
> The number of GUIDs is not related to the number of banks.
> Please, remove this test.
>

Okay, will make it common, as you suggest.


>
> > + image_info[i].image_type_id = guid_array[i];
>
> The sequence of GUIDs in a capsule should not influence the update. The
> design lacks a configuration defining which GUID maps to which DFU part.
>
> Please, get rid of all DFU environment variables and describe the update
> in a configuration file that you add to the capsule.
>

Like we discussed, the information on the update, like the device,
partition number etc cannot be stored as part of the capsule header
currently. So, for now, we have to make do with using the dfu variables for
this.


> > + else
> > + image_info[i].image_type_id = *guid_array;
>
> Do you want to write the same image to multiple places? Why?
>

I was keeping the logic consistent with the way it is currently. This will
go away as the explicit check for CONFIG_FWU_MULTI_BANK_UPDATE is removed
based on your comment above.

-sughosh


> Best regards
>
> Heirnich
>
> > +
> >   image_info[i].image_id = dfu->alt;
> >
> >   /* copy the DFU entity name */
> > @@ -249,7 +268,9 @@ efi_status_t EFIAPI efi_firmware_fit_get_image_info(
> >   u32 *package_version,
> >   u16 **package_version_name)
> >   {
> > + u32 nparts;
> >   efi_status_t ret;
> > + efi_guid_t *part_guid_arr;
> >
> >   EFI_ENTRY("%p %p %p %p %p %p %p %p\n", this,
> > image_info_size, image_info,
> > @@ -264,12 +285,24 @@ efi_status_t EFIAPI
> efi_firmware_fit_get_image_info(
> >!descriptor_size || !package_version ||
> !package_version_name))
> >   

Re: [PATCH] board: iot2050: update build documentation for OP-TEE

2021-11-29 Thread Jan Kiszka
On 28.11.21 22:57, Ivan Mikhaylov wrote:
> From: Ivan Mikhaylov 
> 
> Set ta-target explicitly to correspond with OP-TEE recipe in
> siemens/meta-iot2050.
> 
> Errors without explicit set of ta-target:
> aarch64-linux-gnu-gcc: error: unrecognized command-line option ‘-mthumb’
> aarch64-linux-gnu-gcc: error: unrecognized command-line option 
> ‘-mno-unaligned-access’
> aarch64-linux-gnu-gcc: error: unrecognized command-line option 
> ‘-mfloat-abi=hard’
> make: *** [mk/compile.mk:159: out/arm-plat-k3/ta_arm32-lib/libdl/dlfcn.o] 
> Error 1
> 
> Signed-off-by: Ivan Mikhaylov 
> ---
>  doc/board/siemens/iot2050.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/doc/board/siemens/iot2050.rst b/doc/board/siemens/iot2050.rst
> index 592c59be03..7e97f817ce 100644
> --- a/doc/board/siemens/iot2050.rst
> +++ b/doc/board/siemens/iot2050.rst
> @@ -49,7 +49,7 @@ OP-TEE:
>  
>  .. code-block:: text
>  
> - $ make PLATFORM=k3-am65x CFG_ARM64_core=y CFG_TEE_CORE_LOG_LEVEL=2 
> CFG_CONSOLE_UART=1
> + $ make PLATFORM=k3-am65x CFG_ARM64_core=y CFG_TEE_CORE_LOG_LEVEL=2 
> CFG_CONSOLE_UART=1 CFG_USER_TA_TARGETS="ta_arm64"
>  
>  U-Boot:
>  
> 

Reviewed-by: Jan Kiszka 

Thanks,
Jan

PS: We should align our own build recipe to this pattern
(CFG_USER_TA_TARGETS, rather than ta-targets).

-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux


[RFC Patch v2] binman: add support for creating dummy files for external blobs

2021-11-29 Thread Heiko Thiery
While converting to binman for an imx8mq board, it has been found that
building in the u-boot CI fails. This is because an imx8mq requires an
external binary (signed_hdmi_imx8m.bin). If this file cannot be found
mkimage fails.
To be able to build this board in the u-boot CI a binman option
(--fake-ext-blobs) is introduced that can be switched on via the u-boot
makefile option BINMAN_FAKE_EXT_BLOBS. With that the needed dummy files are
created.

Signed-off-by: Heiko Thiery 
---
v2:
 - pass allow_fake_blobs to ProcessImage()
 - set AllowAllowFakeBlob() to images/entries
 - create fake blob in Entry_blot.ObtainContents() when file is missing and
   creation is allowed

 still missing:
  - unittest
  - option to set BINMAN_FAKE_EXT_BLOBS in Makefile via environment
variable. With that we could simply set this env variable in the CI
(gitlab-ci.yml) with adding support to buildman.

 Makefile   |  1 +
 tools/binman/cmdline.py|  2 ++
 tools/binman/control.py|  9 +++--
 tools/binman/entry.py  | 11 +++
 tools/binman/etype/blob.py |  7 +++
 tools/binman/etype/blob_ext.py |  8 
 tools/binman/etype/mkimage.py  |  9 +
 tools/binman/etype/section.py  |  9 +
 8 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index f911f70344..1a833a1637 100644
--- a/Makefile
+++ b/Makefile
@@ -1307,6 +1307,7 @@ cmd_binman = $(srctree)/tools/binman/binman $(if 
$(BINMAN_DEBUG),-D) \
-a tpl-bss-pad=$(if $(CONFIG_TPL_SEPARATE_BSS),,1) \
-a spl-dtb=$(CONFIG_SPL_OF_REAL) \
-a tpl-dtb=$(CONFIG_SPL_OF_REAL) \
+   $(if $(BINMAN_FAKE_EXT_BLOBS),--fake-ext-blobs) \
$(BINMAN_$(@F))
 
 OBJCOPYFLAGS_u-boot.ldr.hex := -I binary -O ihex
diff --git a/tools/binman/cmdline.py b/tools/binman/cmdline.py
index d6156df408..2b29981cb4 100644
--- a/tools/binman/cmdline.py
+++ b/tools/binman/cmdline.py
@@ -52,6 +52,8 @@ controlled by a description in the board device tree.'''
 help='Configuration file (.dtb) to use')
 build_parser.add_argument('--fake-dtb', action='store_true',
 help='Use fake device tree contents (for testing only)')
+build_parser.add_argument('--fake-ext-blobs', action='store_true',
+help='Create fake ext blobs with dummy content (for testing only)')
 build_parser.add_argument('-i', '--image', type=str, action='append',
 help='Image filename to build (if not specified, build all)')
 build_parser.add_argument('-I', '--indir', action='append',
diff --git a/tools/binman/control.py b/tools/binman/control.py
index 0dbcbc28e9..fa034bddef 100644
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -479,7 +479,8 @@ def PrepareImagesAndDtbs(dtb_fname, select_images, 
update_fdt, use_expanded):
 
 
 def ProcessImage(image, update_fdt, write_map, get_contents=True,
- allow_resize=True, allow_missing=False):
+ allow_resize=True, allow_missing=False,
+ allow_fake_blobs=False):
 """Perform all steps for this image, including checking and # writing it.
 
 This means that errors found with a later image will be reported after
@@ -495,12 +496,14 @@ def ProcessImage(image, update_fdt, write_map, 
get_contents=True,
 allow_resize: True to allow entries to change size (this does a re-pack
 of the entries), False to raise an exception
 allow_missing: Allow blob_ext objects to be missing
+allow_fake_blobs: Allow blob_ext objects to be faked with dummy files
 
 Returns:
 True if one or more external blobs are missing, False if all are 
present
 """
 if get_contents:
 image.SetAllowMissing(allow_missing)
+image.SetAllowFakeBlob(allow_fake_blobs)
 image.GetEntryContents()
 image.GetEntryOffsets()
 
@@ -629,13 +632,15 @@ def Binman(args):
 
 images = PrepareImagesAndDtbs(dtb_fname, args.image,
   args.update_fdt, use_expanded)
+
 if args.test_section_timeout:
 # Set the first image to timeout, used in testThreadTimeout()
 images[list(images.keys())[0]].test_section_timeout = True
 missing = False
 for image in images.values():
 missing |= ProcessImage(image, args.update_fdt, args.map,
-allow_missing=args.allow_missing)
+allow_missing=args.allow_missing,
+allow_fake_blobs=args.fake_ext_blobs)
 
 # Write the updated FDTs to our output files
 for dtb_item in state.GetAllFdts():
diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index 70222718ea..9aa4359be1 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -70,6 +70,8 @@ class Entry(object):
 

Re: [PATCH u-boot-marvell 02/10] arm: mvebu: a38x: serdes: Move non-serdes PCIe code to pci_mvebu.c

2021-11-29 Thread Stefan Roese

On 11/29/21 10:06, Pali Rohár wrote:




After this DTS change, pci-mvebu.c will just replace value of current
number of lanes (which is set to 4 by serdes code) to value from DTS,
which is 4. Therefore there should be no change.

Could you test whole patch series with above DTS change if it works
properly on Theadorable board?


Yes, I don't see any issues with this patchset applied plus this DT
patch on theadorable. The PCIe links are up and with the correct width.

What I'm wondering is, when exactly does the PCIe RP start the link
establishment. In my case with AXP this is still in the AXP serdes code
of course. But in the A38x code, it should be in the PCIe controller
driver now AFAIU. I see that you configure the link width in the
controller and do some other configuration (address windows etc), but
at the end you "simply" wait for the link to come up via
mvebu_pcie_wait_for_link(). I would have expected here some special
command (config bit?) to the PCIe controller to start the link
establishment. So when exactly does the A38x start this action?


That is interesting question... While I'm reading it again, I really do
not know. Because you are right that mvebu_pcie_wait_for_link() is just
waiting for a link and it "magically" comes up. I have tested it on A385
and it is stable with different Compex Atheros cards which caused issues
in past also on A3720.


I would prefer, to fully understand when exactly the link establishment
is started. Since this is crucial for the setup of the controller that
needs to be done *before* the link starts to come up.

Could you perhaps try to remove some of the register configurations in
the MVEBU PCIe driver to see, if the link establishment relies on this
register to be written to (e.g. PCI_EXP_LNKCAP)?

Thanks,
Stefan


Re: [PATCH u-boot-marvell 02/10] arm: mvebu: a38x: serdes: Move non-serdes PCIe code to pci_mvebu.c

2021-11-29 Thread Pali Rohár
On Monday 29 November 2021 08:46:47 Stefan Roese wrote:
> Hi Pali,
> 
> On 11/23/21 16:59, Pali Rohár wrote:
> > On Friday 19 November 2021 07:55:00 Stefan Roese wrote:
> > > On 11/18/21 19:01, Pali Rohár wrote:
> > > > On Friday 12 November 2021 15:01:57 Stefan Roese wrote:
> > > > > On 11/11/21 16:35, Marek Behún wrote:
> > > > > > From: Pali Rohár 
> > > > > > 
> > > > > > As explained in commit 3bedbcc3aa18 ("arm: mvebu: a38x: serdes: 
> > > > > > Don't
> > > > > > overwrite read-only SAR PCIe registers") it is required to set 
> > > > > > Maximum Link
> > > > > > Width bits of PCIe Root Port Link Capabilities Register depending 
> > > > > > of number
> > > > > > of used serdes lanes. As this register is part of PCIe address 
> > > > > > space and
> > > > > > not serdes address space, move it into pci_mvebu.c driver.
> > > > > > 
> > > > > > Read number of PCIe lanes from DT propery "num-lanes" which is used 
> > > > > > also by
> > > > > > other PCIe controller drivers in Linux kernel. If this property is 
> > > > > > absent.
> > > > > > default to 1. This property needs to be set to 4 for every mvebu 
> > > > > > board
> > > > > > which use PEX_ROOT_COMPLEX_X4. Currently in U-Boot there is no such 
> > > > > > board.
> > > > > > 
> > > > > > Signed-off-by: Pali Rohár 
> > > > > > Signed-off-by: Marek Behún 
> > > > > > ---
> > > > > > arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.h |  4 
> > > > > > .../serdes/a38x/high_speed_env_spec.c  | 15 
> > > > > > ---
> > > > > > drivers/pci/pci_mvebu.c| 18 
> > > > > > ++
> > > > > > 3 files changed, 18 insertions(+), 19 deletions(-)
> > > > > 
> > > > > I'm wondering now, if and how this works on Armada XP, which uses the
> > > > > same PCIe driver but a different serdes/axp/*. Did you take this into
> > > > > account?
> > > > 
> > > > It looks like that axp serdes code also touches register of PCIe Root
> > > > Ports, e.g. in section /* 6.2 PCI Express Link Capabilities */. So it is
> > > > something which could be improved and cleaned. But it should not cause
> > > > any issue if registers are configures two times, once in serdes code and
> > > > once in pci-mvebu.c code. Moreover registers in pci-mvebu space,
> > > > including config space of PCIe Root Ports are reconfigured by kernel, so
> > > > I think that it should not cause any issue.
> > > 
> > > I assume that the AXP serdes code is very similar to the A38x code that
> > > you recently overhauled very thoroughly. For the AXP serdes code, I know
> > > that the PCIe link is already established in the serdes code right now.
> > > And since we had some link-up issues on the theadorable AXP board, we
> > > have been trying to optimize / tune this in this ugly serdes code at few
> > > years ago. From what I understand, you've removed all this PCIe specific
> > > code in the A38x serdes part, so that the link establishment now happens
> > > in the PCIe driver itself (pci_mvebu.c). Which makes perfect sense IMHO.
> > > 
> > > Since A38x and AXP share the same PCIe driver in U-Boot, I would very
> > > much like to see some similar changes being made to the AXP serdes code
> > > as well, so that the link establishment solely will happen for all
> > > these platforms in the PCIe driver.
> > 
> > I have looked into AXP serdes code and seems to be similar mess like it
> > was in A38x serdes code. Unfortunately I do not want to touch this code
> > and do movement without heavy hardware testing as it can be easy to
> > break. And I do not have Theadorable board for testing.
> 
> I fully agree, that such a rework / cleanup, as you have done for a38x,
> can only be done with intensive testing. I might try to find some time
> in the future to work on this, as theadorable is still actively used and
> PCIe is always a potential basket of trouble here.
> 
> > Anyway, all changes done in pci_mvebu.c driver just configures registers
> > to correct or expected values, so they should have low probability of
> > breaking existing hardware...
> 
> Okay. Please see below...
> 
> > > > But what could cause issues is X1 vs X4 configuration in pci-mvebu.c if
> > > > "num-lanes" property is not properly set. As is mentioned in commit
> > > > message there is no A38x board in U-Boot which uses X4.
> > > > 
> > > > Now with your comment for Armada XP I checked that serdes code and find
> > > > out that AXP uses macro 'PEX_BUS_MODE_X4' for X4 mode (A38x serdes uses
> > > > PEX_ROOT_COMPLEX_X4). And in U-Boot there are two boards which sets this
> > > > macro: board/Synology/ds414/ds414.c and board/theadorable/theadorable.c
> > > > 
> > > > So it would be needed to adjust this patch for these two boards. Please
> > > > hold this one patch for now. I will try to prepare new fixed version.
> > > > Other patches should be OK and independent of this one.
> > > 
> > > Thanks. And yes, theadorable has one x4 and one x1.
> > 
> > I think that this change should 

Re: [RFC PATCH 0/3] imx8m: move env_get_location for imx8mn and imx8mp at board level

2021-11-29 Thread Michael Nazzareno Trimarchi
Hi

On Mon, Nov 29, 2021 at 9:46 AM ZHIZHIKIN Andrey
 wrote:
>
> Hello Michael,
>
> > -Original Message-
> > From: Michael Nazzareno Trimarchi 
> > Sent: Monday, November 29, 2021 9:40 AM
> > To: ZHIZHIKIN Andrey 
> > Cc: Tommaso Merciai ; Stefano Babic 
> > ;
> > Fabio Estevam ; NXP i.MX U-Boot Team 
> > ;
> > Peng Fan ; Ye Li ; Marek Vasut 
> > ;
> > Simon Glass ; Frieder Schrempf 
> > ;
> > Marek Behún ; Ying-Chun Liu (PaulLiu) 
> > ;
> > u-boot@lists.denx.de
> > Subject: Re: [RFC PATCH 0/3] imx8m: move env_get_location for imx8mn and 
> > imx8mp
> > at board level
> >
> >
> > HI
> >
> > On Mon, Nov 29, 2021 at 9:38 AM ZHIZHIKIN Andrey
> >  wrote:
> > >
> > > Hello Tommaso,
> > >
> > > > -Original Message-
> > > > From: Tommaso Merciai 
> > > > Sent: Friday, November 26, 2021 6:43 PM
> > > > Cc: mich...@amarulasolutions.com; ZHIZHIKIN Andrey 
> > > >  > > > geosystems.com>; Tommaso Merciai ; Stefano Babic
> > > > ; Fabio Estevam ; NXP i.MX U-Boot 
> > > > Team
> > > > ; Peng Fan ; Ye Li ;
> > Marek
> > > > Vasut ; Simon Glass ; Frieder Schrempf
> > > > ; Marek Behún ; 
> > > > Ying-Chun
> > Liu
> > > > (PaulLiu) ; u-boot@lists.denx.de
> > > > Subject: [RFC PATCH 0/3] imx8m: move env_get_location for imx8mn and 
> > > > imx8mp
> > at
> > > > board level
> > > >
> > > >
> > > > This series move env_get_location from soc to board level. As suggested
> > > > by Michael  make no sense to define an
> > > > unique way for multiple board. One board can boot from emmc and having
> > > > env on spi flash etc.. Anyways, this function is kept in both imx8mn
> > > > and imx8mp evk boards instead of being completely dropped.
> > > > (as suggested by Andrey )
> > >
> > > I believe there has been another suggestion from my side regarding this 
> > > patch:
> > > Since it look like that Michael Trimarchi submitted another part to drop
> > > env_get_offset() in [1], combined with the first patch in this series - 
> > > it is
> > > a complete revert of 2707faf01f ("imx8mn/imx8mp: override env_get_offset 
> > > and
> > > env_get_location").
> > >
> > > I suggest you to submit a revert instead of your first patch and 
> > > deprecate the
> > > patch from Michael, instead of having 2 separate patches for this.
> > >
> >
> > I think they are totally different problems. One is code not used and
> > the other moves that implementation
> > in specific parts.
>
> They might be logically different, but 2 commits combined together - is a full
> revert to me.
>
> I'd leave this up to maintainer to decide, but for me it would be logical to 
> see
> a revert instead of 2 separate commits - this makes tracking more transparent.
>

The first one (mine) is not a logical change. It means that nothing
get wrong. The other
is anywway some change so if you needs to revert then you can pick
specific part.

Michael

> >
> > Michael
> >
> > > >
> > > > Tommaso Merciai (3):
> > > >   imx8m: drop env_get_location for imx8mn and imx8mp
> > > >   imx: imx8mn_evk: override env_get_location
> > > >   imx: imx8mp_evk: override env_get_location
> > > >
> > > >  arch/arm/mach-imx/imx8m/soc.c   | 39 ---
> > > >  board/freescale/imx8mn_evk/imx8mn_evk.c | 42 +
> > > >  board/freescale/imx8mp_evk/imx8mp_evk.c | 41 
> > > >  3 files changed, 83 insertions(+), 39 deletions(-)
> > > >
> > > > --
> > > > 2.25.1
> > >
> > > Link: [1]: 
> > > http://patchwork.ozlabs.org/project/uboot/patch/2027143456.34441-1-mich...@amarulasolutions.com/
> > >
> > > -- andrey
> >
> >
> >
> > --
> > Michael Nazzareno Trimarchi
> > Co-Founder & Chief Executive Officer
> > M. +39 347 913 2170
> > mich...@amarulasolutions.com
> > __
> >
> > Amarula Solutions BV
> > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> > T. +31 (0)85 111 9172
> > i...@amarulasolutions.com
> > https://eur02.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.amarulasolut
> > ions.com%2Fdata=04%7C01%7C%7C666e97d4bb5f425c9a0408d9b313dd84%7C1b16ab3eb8f6
> > 4fe39f3e2db7fe549f6a%7C0%7C0%7C637737720180120994%7CUnknown%7CTWFpbGZsb3d8eyJWIjo
> > iMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000sdata=NJQ7
> > qjpMWu%2BhGoIcqwmD%2BLc4Ekq1oHjqPSCqkiCr4DA%3Dreserved=0
>
> -- andrey



-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


RE: [RFC PATCH 0/3] imx8m: move env_get_location for imx8mn and imx8mp at board level

2021-11-29 Thread ZHIZHIKIN Andrey
Hello Michael,

> -Original Message-
> From: Michael Nazzareno Trimarchi 
> Sent: Monday, November 29, 2021 9:40 AM
> To: ZHIZHIKIN Andrey 
> Cc: Tommaso Merciai ; Stefano Babic ;
> Fabio Estevam ; NXP i.MX U-Boot Team ;
> Peng Fan ; Ye Li ; Marek Vasut 
> ;
> Simon Glass ; Frieder Schrempf 
> ;
> Marek Behún ; Ying-Chun Liu (PaulLiu) 
> ;
> u-boot@lists.denx.de
> Subject: Re: [RFC PATCH 0/3] imx8m: move env_get_location for imx8mn and 
> imx8mp
> at board level
> 
> 
> HI
> 
> On Mon, Nov 29, 2021 at 9:38 AM ZHIZHIKIN Andrey
>  wrote:
> >
> > Hello Tommaso,
> >
> > > -Original Message-
> > > From: Tommaso Merciai 
> > > Sent: Friday, November 26, 2021 6:43 PM
> > > Cc: mich...@amarulasolutions.com; ZHIZHIKIN Andrey 
> > >  > > geosystems.com>; Tommaso Merciai ; Stefano Babic
> > > ; Fabio Estevam ; NXP i.MX U-Boot Team
> > > ; Peng Fan ; Ye Li ;
> Marek
> > > Vasut ; Simon Glass ; Frieder Schrempf
> > > ; Marek Behún ; Ying-Chun
> Liu
> > > (PaulLiu) ; u-boot@lists.denx.de
> > > Subject: [RFC PATCH 0/3] imx8m: move env_get_location for imx8mn and 
> > > imx8mp
> at
> > > board level
> > >
> > >
> > > This series move env_get_location from soc to board level. As suggested
> > > by Michael  make no sense to define an
> > > unique way for multiple board. One board can boot from emmc and having
> > > env on spi flash etc.. Anyways, this function is kept in both imx8mn
> > > and imx8mp evk boards instead of being completely dropped.
> > > (as suggested by Andrey )
> >
> > I believe there has been another suggestion from my side regarding this 
> > patch:
> > Since it look like that Michael Trimarchi submitted another part to drop
> > env_get_offset() in [1], combined with the first patch in this series - it 
> > is
> > a complete revert of 2707faf01f ("imx8mn/imx8mp: override env_get_offset and
> > env_get_location").
> >
> > I suggest you to submit a revert instead of your first patch and deprecate 
> > the
> > patch from Michael, instead of having 2 separate patches for this.
> >
> 
> I think they are totally different problems. One is code not used and
> the other moves that implementation
> in specific parts.

They might be logically different, but 2 commits combined together - is a full
revert to me.

I'd leave this up to maintainer to decide, but for me it would be logical to see
a revert instead of 2 separate commits - this makes tracking more transparent.

> 
> Michael
> 
> > >
> > > Tommaso Merciai (3):
> > >   imx8m: drop env_get_location for imx8mn and imx8mp
> > >   imx: imx8mn_evk: override env_get_location
> > >   imx: imx8mp_evk: override env_get_location
> > >
> > >  arch/arm/mach-imx/imx8m/soc.c   | 39 ---
> > >  board/freescale/imx8mn_evk/imx8mn_evk.c | 42 +
> > >  board/freescale/imx8mp_evk/imx8mp_evk.c | 41 
> > >  3 files changed, 83 insertions(+), 39 deletions(-)
> > >
> > > --
> > > 2.25.1
> >
> > Link: [1]: 
> > http://patchwork.ozlabs.org/project/uboot/patch/2027143456.34441-1-mich...@amarulasolutions.com/
> >
> > -- andrey
> 
> 
> 
> --
> Michael Nazzareno Trimarchi
> Co-Founder & Chief Executive Officer
> M. +39 347 913 2170
> mich...@amarulasolutions.com
> __
> 
> Amarula Solutions BV
> Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> T. +31 (0)85 111 9172
> i...@amarulasolutions.com
> https://eur02.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.amarulasolut
> ions.com%2Fdata=04%7C01%7C%7C666e97d4bb5f425c9a0408d9b313dd84%7C1b16ab3eb8f6
> 4fe39f3e2db7fe549f6a%7C0%7C0%7C637737720180120994%7CUnknown%7CTWFpbGZsb3d8eyJWIjo
> iMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000sdata=NJQ7
> qjpMWu%2BhGoIcqwmD%2BLc4Ekq1oHjqPSCqkiCr4DA%3Dreserved=0

-- andrey


Re: [RFC PATCH 0/3] imx8m: move env_get_location for imx8mn and imx8mp at board level

2021-11-29 Thread Michael Nazzareno Trimarchi
HI

On Mon, Nov 29, 2021 at 9:38 AM ZHIZHIKIN Andrey
 wrote:
>
> Hello Tommaso,
>
> > -Original Message-
> > From: Tommaso Merciai 
> > Sent: Friday, November 26, 2021 6:43 PM
> > Cc: mich...@amarulasolutions.com; ZHIZHIKIN Andrey  > geosystems.com>; Tommaso Merciai ; Stefano Babic
> > ; Fabio Estevam ; NXP i.MX U-Boot Team
> > ; Peng Fan ; Ye Li ; 
> > Marek
> > Vasut ; Simon Glass ; Frieder Schrempf
> > ; Marek Behún ; Ying-Chun 
> > Liu
> > (PaulLiu) ; u-boot@lists.denx.de
> > Subject: [RFC PATCH 0/3] imx8m: move env_get_location for imx8mn and imx8mp 
> > at
> > board level
> >
> >
> > This series move env_get_location from soc to board level. As suggested
> > by Michael  make no sense to define an
> > unique way for multiple board. One board can boot from emmc and having
> > env on spi flash etc.. Anyways, this function is kept in both imx8mn
> > and imx8mp evk boards instead of being completely dropped.
> > (as suggested by Andrey )
>
> I believe there has been another suggestion from my side regarding this patch:
> Since it look like that Michael Trimarchi submitted another part to drop
> env_get_offset() in [1], combined with the first patch in this series - it is
> a complete revert of 2707faf01f ("imx8mn/imx8mp: override env_get_offset and
> env_get_location").
>
> I suggest you to submit a revert instead of your first patch and deprecate the
> patch from Michael, instead of having 2 separate patches for this.
>

I think they are totally different problems. One is code not used and
the other moves that implementation
in specific parts.

Michael

> >
> > Tommaso Merciai (3):
> >   imx8m: drop env_get_location for imx8mn and imx8mp
> >   imx: imx8mn_evk: override env_get_location
> >   imx: imx8mp_evk: override env_get_location
> >
> >  arch/arm/mach-imx/imx8m/soc.c   | 39 ---
> >  board/freescale/imx8mn_evk/imx8mn_evk.c | 42 +
> >  board/freescale/imx8mp_evk/imx8mp_evk.c | 41 
> >  3 files changed, 83 insertions(+), 39 deletions(-)
> >
> > --
> > 2.25.1
>
> Link: [1]: 
> http://patchwork.ozlabs.org/project/uboot/patch/2027143456.34441-1-mich...@amarulasolutions.com/
>
> -- andrey



-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


RE: [RFC PATCH 0/3] imx8m: move env_get_location for imx8mn and imx8mp at board level

2021-11-29 Thread ZHIZHIKIN Andrey
Hello Tommaso,

> -Original Message-
> From: Tommaso Merciai 
> Sent: Friday, November 26, 2021 6:43 PM
> Cc: mich...@amarulasolutions.com; ZHIZHIKIN Andrey  geosystems.com>; Tommaso Merciai ; Stefano Babic
> ; Fabio Estevam ; NXP i.MX U-Boot Team
> ; Peng Fan ; Ye Li ; Marek
> Vasut ; Simon Glass ; Frieder Schrempf
> ; Marek Behún ; Ying-Chun Liu
> (PaulLiu) ; u-boot@lists.denx.de
> Subject: [RFC PATCH 0/3] imx8m: move env_get_location for imx8mn and imx8mp at
> board level
> 
> 
> This series move env_get_location from soc to board level. As suggested
> by Michael  make no sense to define an
> unique way for multiple board. One board can boot from emmc and having
> env on spi flash etc.. Anyways, this function is kept in both imx8mn
> and imx8mp evk boards instead of being completely dropped.
> (as suggested by Andrey )

I believe there has been another suggestion from my side regarding this patch:
Since it look like that Michael Trimarchi submitted another part to drop
env_get_offset() in [1], combined with the first patch in this series - it is
a complete revert of 2707faf01f ("imx8mn/imx8mp: override env_get_offset and
env_get_location").

I suggest you to submit a revert instead of your first patch and deprecate the
patch from Michael, instead of having 2 separate patches for this.

> 
> Tommaso Merciai (3):
>   imx8m: drop env_get_location for imx8mn and imx8mp
>   imx: imx8mn_evk: override env_get_location
>   imx: imx8mp_evk: override env_get_location
> 
>  arch/arm/mach-imx/imx8m/soc.c   | 39 ---
>  board/freescale/imx8mn_evk/imx8mn_evk.c | 42 +
>  board/freescale/imx8mp_evk/imx8mp_evk.c | 41 
>  3 files changed, 83 insertions(+), 39 deletions(-)
> 
> --
> 2.25.1

Link: [1]: 
http://patchwork.ozlabs.org/project/uboot/patch/2027143456.34441-1-mich...@amarulasolutions.com/

-- andrey


[PATCH v2] doc: qemu-arm peripherials

2021-11-29 Thread Heinrich Schuchardt
* add description how to add RNG device
* for a disk specify format=raw to avoid a warning
* fix a typo

Signed-off-by: Heinrich Schuchardt 
---
v2:
remove unrelated change
---
 doc/board/emulation/qemu-arm.rst | 9 +++--
 drivers/tpm/tpm2_tis_mmio.c  | 5 -
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/doc/board/emulation/qemu-arm.rst b/doc/board/emulation/qemu-arm.rst
index 584ef0a7e1..7c24e29410 100644
--- a/doc/board/emulation/qemu-arm.rst
+++ b/doc/board/emulation/qemu-arm.rst
@@ -65,7 +65,8 @@ can be enabled with the following command line parameters:
 
 - To add a Serial ATA disk via an Intel ICH9 AHCI controller, pass e.g.::
 
--drive if=none,file=disk.img,id=mydisk -device ich9-ahci,id=ahci -device 
ide-drive,drive=mydisk,bus=ahci.0
+-drive if=none,file=disk.img,format=raw,id=mydisk \
+-device ich9-ahci,id=ahci -device ide-drive,drive=mydisk,bus=ahci.0
 
 - To add an Intel E1000 network adapter, pass e.g.::
 
@@ -75,10 +76,14 @@ can be enabled with the following command line parameters:
 
 -device usb-ehci,id=ehci
 
-- To add a NVMe disk, pass e.g.::
+- To add an NVMe disk, pass e.g.::
 
 -drive if=none,file=disk.img,id=mydisk -device nvme,drive=mydisk,serial=foo
 
+- To add a random number generator, pass e.g.::
+
+-device virtio-rng-pci
+
 These have been tested in QEMU 2.9.0 but should work in at least 2.5.0 as well.
 
 Enabling TPMv2 support
-- 
2.32.0