Re: [PATCH 1/1] doc: board: qemu-mips.rst: fix code examples

[Heinrich Schuchardt]

On 1/8/20 3:15 PM, Daniel Schwierzeck wrote:



Am 01.01.20 um 08:34 schrieb Heinrich Schuchardt:

Code sections should be syntax highlighted as bash.
Comment lines in code should start with a hash sign ('#') but code lines
should not.
Most commands can be executed as normal users. Prepend those commands
requiring elevated authorization with 'sudo'.
dd does not have a parameter cout.
sfdisk does not have a -C parameter on Debian Buster.
Provide the necessary input to sfdisk.
Creating a partition of length zero makes no sense.

Signed-off-by: Heinrich Schuchardt 
---
  doc/board/emulation/qemu-mips.rst | 108 +-
  1 file changed, 60 insertions(+), 48 deletions(-)



applied to u-boot-mips, thanks.



Hello Daniel,

I can see the patch in mips/next. But unfortunately it never made it
into a pull request.

Best regards

Heinrich

[PATCH v2 1/1] net: tftp: use correct printf codes

[Heinrich Schuchardt]

When printing unsigned numbers use %u.

Signed-off-by: Heinrich Schuchardt 
Acked-by: Joe Hershberger 
---
v2:
rebased
---
 net/tftp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/tftp.c b/net/tftp.c
index 02401898c5..585eb6ef0c 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -223,7 +223,7 @@ static int load_block(unsigned block, uchar *dst, unsigned 
len)

tosend = min(net_boot_file_size - offset, tosend);
(void)memcpy(dst, (void *)(image_save_addr + offset), tosend);
-   debug("%s: block=%d, offset=%ld, len=%d, tosend=%ld\n", __func__,
+   debug("%s: block=%u, offset=%lu, len=%u, tosend=%lu\n", __func__,
  block, offset, len, tosend);
return tosend;
 }
--
2.24.1

Re: [PATCH v2 11/21] arm: socfpga: Secure register access for clock manager (SoC 64bits)

[Simon Goldschmidt]

Ang, Chee Hong  schrieb am Sa., 22. Feb. 2020,
06:30:

> > From: Chee Hong Ang 
> >
> > Allow clock manager driver to access the System Manager's Boot Scratch
> > Register 0 in non-secure mode (EL2) on SoC 64bits platform.
> >
> > Signed-off-by: Chee Hong Ang 
> > ---
> >  arch/arm/mach-socfpga/clock_manager_agilex.c | 5 +++--
> >  arch/arm/mach-socfpga/clock_manager_s10.c| 5 +++--
> >  2 files changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm/mach-socfpga/clock_manager_agilex.c
> b/arch/arm/mach-
> > socfpga/clock_manager_agilex.c
> > index 4ee2b7b..e5a0998 100644
> > --- a/arch/arm/mach-socfpga/clock_manager_agilex.c
> > +++ b/arch/arm/mach-socfpga/clock_manager_agilex.c
> > @@ -12,6 +12,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >
> >  DECLARE_GLOBAL_DATA_PTR;
> >
> > @@ -65,8 +66,8 @@ unsigned int cm_get_l4_sys_free_clk_hz(void)
> >
> >  u32 cm_get_qspi_controller_clk_hz(void)
> >  {
> > - return readl(socfpga_get_sysmgr_addr() +
> > -  SYSMGR_SOC64_BOOT_SCRATCH_COLD0);
> > + return socfpga_secure_reg_read32(socfpga_get_sysmgr_addr() +
> > +
> > SYSMGR_SOC64_BOOT_SCRATCH_COLD0);
> >  }
> >
> >  void cm_print_clock_quick_summary(void)
> > diff --git a/arch/arm/mach-socfpga/clock_manager_s10.c b/arch/arm/mach-
> > socfpga/clock_manager_s10.c
> > index 05e4212..02578cc 100644
> > --- a/arch/arm/mach-socfpga/clock_manager_s10.c
> > +++ b/arch/arm/mach-socfpga/clock_manager_s10.c
> > @@ -9,6 +9,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >
> >  DECLARE_GLOBAL_DATA_PTR;
> >
> > @@ -385,8 +386,8 @@ unsigned int cm_get_l4_sp_clk_hz(void)
> >
> >  unsigned int cm_get_qspi_controller_clk_hz(void)
> >  {
> > - return readl(socfpga_get_sysmgr_addr() +
> > -  SYSMGR_SOC64_BOOT_SCRATCH_COLD0);
> > + return socfpga_secure_reg_read32(socfpga_get_sysmgr_addr() +
> > +
> > SYSMGR_SOC64_BOOT_SCRATCH_COLD0);
> >  }
> >
> >  unsigned int cm_get_spi_controller_clk_hz(void)
> > --
> > 2.7.4
> SPL reads the clock info from handoff table (OCRAM) and write
> the clock info into the System Manager's boot scratch register.
> U-Boot proper will read from the System Manager's boot scratch
> register to get the clock info in case the handoff table (OCRAM)
> is no longer available.
> After some investigations, the handoff table in OCRAM should be preserved
> for warm boot. In other words, this handoff table should be left untouched.
> SPL and U-Boot should directly read the clock info from handoff table in
> OCRAM.
> Therefore, U-Boot proper no longer need to read the clock info from
> System Manager's boot scratch register (secure zone) from non-secure world
> (EL2).
>

I don't think that's a good idea: for security reasons, SPL memory should
not be accessible from EL2 if it is required/used for the next reboot.

Regards,
Simon

[PATCH 1/1] doc/efi: add load file 2 protocol to HTML documentation

[Heinrich Schuchardt]

The load file 2 protocol can be used by the Linux kernel to load the initial
RAM disk. U-Boot can be configured to provide an implementation.

Add a description to the UEFI overview and document the related functions
in the API section.

Signed-off-by: Heinrich Schuchardt 
---
 doc/api/efi.rst   |  9 +
 doc/uefi/uefi.rst | 12 
 2 files changed, 21 insertions(+)

diff --git a/doc/api/efi.rst b/doc/api/efi.rst
index bc59382608..631c0ceb1d 100644
--- a/doc/api/efi.rst
+++ b/doc/api/efi.rst
@@ -125,6 +125,15 @@ Graphical output protocol
 .. kernel-doc:: lib/efi_loader/efi_gop.c
:internal:

+Load file 2 protocol
+
+
+The load file 2 protocol can be used by the Linux kernel to load the initial
+RAM disk. U-Boot can be configured to provide an implementation.
+
+.. kernel-doc:: lib/efi_loader/efi_load_initrd.c
+   :internal:
+
 Network protocols
 ~

diff --git a/doc/uefi/uefi.rst b/doc/uefi/uefi.rst
index a8fd886d6b..cfe2d84a4c 100644
--- a/doc/uefi/uefi.rst
+++ b/doc/uefi/uefi.rst
@@ -356,6 +356,18 @@ This driver is only available if U-Boot is configured 
with::
 CONFIG_BLK=y
 CONFIG_PARTITIONS=y

+Miscellaneous
+-
+
+Load file 2 protocol
+
+
+The load file 2 protocol can be used by the Linux kernel to load the initial
+RAM disk. U-Boot can be configured to provide an implementation with::
+
+EFI_LOAD_FILE2_INITRD=y
+EFI_INITRD_FILESPEC=interface dev:part path_to_initrd
+
 Links
 -

--
2.25.0

[PATCH 1/1] MAINTAINERS: set maintainer for doc/api/efi.rst

[Heinrich Schuchardt]

doc/api/efi.rst belongs to the UEFI sub-system documentation.

Signed-off-by: Heinrich Schuchardt 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 82e4159bec..1842569f24 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -563,6 +563,7 @@ M:  Heinrich Schuchardt 
 R: Alexander Graf 
 S: Maintained
 T: git https://gitlab.denx.de/u-boot/custodians/u-boot-efi.git
+F: doc/api/efi.rst
 F: doc/uefi/*
 F: include/capitalization.h
 F: include/charset.h
--
2.25.0

RE: [PATCH v2 16/21] arm: socfpga: Secure register access in Reset Manager driver

[Ang, Chee Hong]

> From: Chee Hong Ang 
> 
> Allow socfpga_bridges_reset() function in Reset Manager driver to access
> System Manager's register in non-secure mode (EL2).
> 
> Signed-off-by: Chee Hong Ang 
> ---
>  arch/arm/mach-socfpga/reset_manager_s10.c | 31 ++--
> ---
>  1 file changed, 18 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/arm/mach-socfpga/reset_manager_s10.c b/arch/arm/mach-
> socfpga/reset_manager_s10.c
> index c743077..d03f121 100644
> --- a/arch/arm/mach-socfpga/reset_manager_s10.c
> +++ b/arch/arm/mach-socfpga/reset_manager_s10.c
> @@ -7,6 +7,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
> 
> @@ -56,34 +57,37 @@ void socfpga_bridges_reset(int enable)  {
>   if (enable) {
>   /* clear idle request to all bridges */
> - setbits_le32(socfpga_get_sysmgr_addr() +
> -  SYSMGR_SOC64_NOC_IDLEREQ_CLR, ~0);
> + socfpga_secure_reg_update32(socfpga_get_sysmgr_addr() +
> + SYSMGR_SOC64_NOC_IDLEREQ_CLR,
> + ~0, ~0);
> 
>   /* Release all bridges from reset state */
>   clrbits_le32(socfpga_get_rstmgr_addr() +
> RSTMGR_SOC64_BRGMODRST,
>~0);
> 
>   /* Poll until all idleack to 0 */
> - while (readl(socfpga_get_sysmgr_addr() +
> -  SYSMGR_SOC64_NOC_IDLEACK))
> + while (socfpga_secure_reg_read32(socfpga_get_sysmgr_addr()
> +
> +
> SYSMGR_SOC64_NOC_IDLEACK))
>   ;
>   } else {
>   /* set idle request to all bridges */
> - writel(~0,
> -socfpga_get_sysmgr_addr() +
> -SYSMGR_SOC64_NOC_IDLEREQ_SET);
> + socfpga_secure_reg_write32(~0, socfpga_get_sysmgr_addr() +
> +SYSMGR_SOC64_NOC_IDLEREQ_SET);
> 
>   /* Enable the NOC timeout */
> - writel(1, socfpga_get_sysmgr_addr() +
> SYSMGR_SOC64_NOC_TIMEOUT);
> + socfpga_secure_reg_write32(1, socfpga_get_sysmgr_addr() +
> +SYSMGR_SOC64_NOC_TIMEOUT);
> 
>   /* Poll until all idleack to 1 */
> - while ((readl(socfpga_get_sysmgr_addr() +
> SYSMGR_SOC64_NOC_IDLEACK) ^
> - (SYSMGR_NOC_H2F_MSK |
> SYSMGR_NOC_LWH2F_MSK)))
> + while ((socfpga_secure_reg_read32(socfpga_get_sysmgr_addr()
> +
> +SYSMGR_SOC64_NOC_IDLEACK) ^
> (SYSMGR_NOC_H2F_MSK |
> +SYSMGR_NOC_LWH2F_MSK)))
>   ;
> 
>   /* Poll until all idlestatus to 1 */
> - while ((readl(socfpga_get_sysmgr_addr() +
> SYSMGR_SOC64_NOC_IDLESTATUS) ^
> - (SYSMGR_NOC_H2F_MSK |
> SYSMGR_NOC_LWH2F_MSK)))
> + while ((socfpga_secure_reg_read32(socfpga_get_sysmgr_addr()
> +
> +SYSMGR_SOC64_NOC_IDLESTATUS) ^
> (SYSMGR_NOC_H2F_MSK |
> +SYSMGR_NOC_LWH2F_MSK)))
>   ;
> 
>   /* Reset all bridges (except NOR DDR scheduler & F2S) */ @@ -
> 92,7 +96,8 @@ void socfpga_bridges_reset(int enable)
>  RSTMGR_BRGMODRST_FPGA2SOC_MASK));
> 
>   /* Disable NOC timeout */
> - writel(0, socfpga_get_sysmgr_addr() +
> SYSMGR_SOC64_NOC_TIMEOUT);
> + socfpga_secure_reg_write32(0, socfpga_get_sysmgr_addr() +
> +SYSMGR_SOC64_NOC_TIMEOUT);
>   }
>  }
> 
> --
> 2.7.4
ATF already has similar function to enable/disable the socfpga bridge.
This function is needed in U-Boot proper (non-secure, EL2) for enabling
the bridge for soft IP access after FPGA is configured.
Will add a high-level API in ATF for non-secure world.
The API info will be documented in 'include/linux/intel-smc.h'

RE: [PATCH v2 15/21] net: designware: socfpga: Secure register access in MAC driver

[Ang, Chee Hong]

> From: Chee Hong Ang 
> 
> Allow MAC driver to access System Manager's EMAC control registers in non-
> secure mode.
> 
> Signed-off-by: Chee Hong Ang 
> ---
>  drivers/net/dwmac_socfpga.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/dwmac_socfpga.c b/drivers/net/dwmac_socfpga.c index
> e93561d..293c660 100644
> --- a/drivers/net/dwmac_socfpga.c
> +++ b/drivers/net/dwmac_socfpga.c
> @@ -17,6 +17,7 @@
>  #include 
>  #include 
> 
> +#include 
>  #include 
> 
>  struct dwmac_socfpga_platdata {
> @@ -98,8 +99,8 @@ static int dwmac_socfpga_probe(struct udevice *dev)
>   reset_assert_bulk(_bulk);
> 
>   modemask = SYSMGR_EMACGRP_CTRL_PHYSEL_MASK << pdata-
> >reg_shift;
> - clrsetbits_le32(pdata->phy_intf, modemask,
> - modereg << pdata->reg_shift);
> + socfpga_secure_reg_update32((phys_addr_t)pdata->phy_intf,
> modemask,
> + modereg << pdata->reg_shift);
> 
>   reset_release_bulk(_bulk);
> 
> --
> 2.7.4
This MAC driver need to access System Manager's EMAC control register
(secure zone) to configure the PHY interface (GMII/RGMII/RMII).
This should be part of this MAC driver as well.
 I will add/define a high-level API in ATF to be used by this MAC driver
from non-secure world (EL2).
This high-level API (SMC/PSCI calls) will be properly documented in
'include/linux/intel-smc.h'

RE: [PATCH v2 14/21] mmc: dwmmc: socfpga: Secure register access in MMC driver

[Ang, Chee Hong]

> From: Chee Hong Ang 
> 
> Allow MMC driver to access System Manager's SDMMC control register in non-
> secure mode (EL2).
> 
> Signed-off-by: Chee Hong Ang 
> ---
>  drivers/mmc/socfpga_dw_mmc.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/socfpga_dw_mmc.c
> b/drivers/mmc/socfpga_dw_mmc.c index 786cdc7..558f246 100644
> --- a/drivers/mmc/socfpga_dw_mmc.c
> +++ b/drivers/mmc/socfpga_dw_mmc.c
> @@ -5,6 +5,7 @@
> 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -57,10 +58,12 @@ static void socfpga_dwmci_clksel(struct dwmci_host
> *host)
> 
>   debug("%s: drvsel %d smplsel %d\n", __func__,
> priv->drvsel, priv->smplsel);
> - writel(sdmmc_mask, socfpga_get_sysmgr_addr() + SYSMGR_SDMMC);
> + socfpga_secure_reg_write32(sdmmc_mask, socfpga_get_sysmgr_addr()
> +
> +SYSMGR_SDMMC);
> 
>   debug("%s: SYSMGR_SDMMCGRP_CTRL_REG = 0x%x\n", __func__,
> - readl(socfpga_get_sysmgr_addr() + SYSMGR_SDMMC));
> +   socfpga_secure_reg_read32(socfpga_get_sysmgr_addr() +
> +   SYSMGR_SDMMC));
> 
>   /* Enable SDMMC clock */
>   setbits_le32(socfpga_get_clkmgr_addr() + CLKMGR_PERPLL_EN,
> --
> 2.7.4
This SDMMC driver need to access System Manager's SDMMC control register
(secure zone) to configure the clock's phase shift settings. I don't think this
could be separated from the driver. I will add/define a high-level API in ATF
to be used by this SDMMC driver from non-secure world (EL2).
This high-level API (SMC/PSCI calls) will be properly documented in
'include/linux/intel-smc.h'

RE: [PATCH v2 13/21] arm: socfpga: Secure register access for reading PLL frequency

[Ang, Chee Hong]

> From: Chee Hong Ang 
> 
> Allow reading external oscillator and FPGA clock's frequency from System
> Manager's Boot Scratch Register 1 and Boot Scratch Register 2 in non-secure
> mode (EL2) on SoC 64bits platform.
> 
> Signed-off-by: Chee Hong Ang 
> ---
>  arch/arm/mach-socfpga/wrap_pll_config_s10.c | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/mach-socfpga/wrap_pll_config_s10.c b/arch/arm/mach-
> socfpga/wrap_pll_config_s10.c
> index 3da8579..7bd92d0 100644
> --- a/arch/arm/mach-socfpga/wrap_pll_config_s10.c
> +++ b/arch/arm/mach-socfpga/wrap_pll_config_s10.c
> @@ -9,6 +9,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
> 
>  const struct cm_config * const cm_get_default_config(void)  { @@ -39,8 +40,8
> @@ const unsigned int cm_get_osc_clk_hz(void)
>   writel(clock,
>  socfpga_get_sysmgr_addr() +
> SYSMGR_SOC64_BOOT_SCRATCH_COLD1);  #endif
> - return readl(socfpga_get_sysmgr_addr() +
> -  SYSMGR_SOC64_BOOT_SCRATCH_COLD1);
> + return socfpga_secure_reg_read32(socfpga_get_sysmgr_addr() +
> +
> SYSMGR_SOC64_BOOT_SCRATCH_COLD1);
>  }
> 
>  const unsigned int cm_get_intosc_clk_hz(void) @@ -56,6 +57,6 @@ const
> unsigned int cm_get_fpga_clk_hz(void)
>   writel(clock,
>  socfpga_get_sysmgr_addr() +
> SYSMGR_SOC64_BOOT_SCRATCH_COLD2);  #endif
> - return readl(socfpga_get_sysmgr_addr() +
> -  SYSMGR_SOC64_BOOT_SCRATCH_COLD2);
> + return socfpga_secure_reg_read32(socfpga_get_sysmgr_addr() +
> +
> SYSMGR_SOC64_BOOT_SCRATCH_COLD2);
>  }
> --
> 2.7.4
This clock info could be directly read from the handoff table (OCRAM)
instead of the System Manager's boot scratch register (secure zone).
Please refer to my full explanation in my previous email reply:
[PATCH v2 11/21] arm: socfpga: Secure register access for clock manager (SoC 
64bits)

RE: [PATCH v2 12/21] arm: socfpga: Secure register access in PHY mode setup

[Ang, Chee Hong]

> From: Chee Hong Ang 
> 
> Allow access to System Manager's EMAC control register from non-secure mode
> during PHY mode setup.
> 
> Signed-off-by: Chee Hong Ang 
> ---
>  arch/arm/mach-socfpga/misc_s10.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/mach-socfpga/misc_s10.c b/arch/arm/mach-
> socfpga/misc_s10.c
> index 25c3ff6..6593308 100644
> --- a/arch/arm/mach-socfpga/misc_s10.c
> +++ b/arch/arm/mach-socfpga/misc_s10.c
> @@ -18,6 +18,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
> 
>  #include 
> 
> @@ -65,9 +66,9 @@ static u32 socfpga_phymode_setup(u32 gmac_index,
> const char *phymode)
>   else
>   return -EINVAL;
> 
> - clrsetbits_le32(socfpga_get_sysmgr_addr() + SYSMGR_SOC64_EMAC0 +
> - gmac_index,
> - SYSMGR_EMACGRP_CTRL_PHYSEL_MASK, modereg);
> + socfpga_secure_reg_update32(socfpga_get_sysmgr_addr() +
> + SYSMGR_SOC64_EMAC0 + gmac_index,
> + SYSMGR_EMACGRP_CTRL_PHYSEL_MASK,
> modereg);
> 
>   return 0;
>  }
> --
> 2.7.4
Looks like this PHY setup is redundant and  no longer needed because it is
already being taken care in 'drivers/net/dwmac_socfpga.c' which is written
by Marek.
Will check with Ley Foon whether this socfpga_phymode_setup() can be safely 
removed.

RE: [PATCH v2 11/21] arm: socfpga: Secure register access for clock manager (SoC 64bits)

[Ang, Chee Hong]

> From: Chee Hong Ang 
> 
> Allow clock manager driver to access the System Manager's Boot Scratch
> Register 0 in non-secure mode (EL2) on SoC 64bits platform.
> 
> Signed-off-by: Chee Hong Ang 
> ---
>  arch/arm/mach-socfpga/clock_manager_agilex.c | 5 +++--
>  arch/arm/mach-socfpga/clock_manager_s10.c| 5 +++--
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/mach-socfpga/clock_manager_agilex.c b/arch/arm/mach-
> socfpga/clock_manager_agilex.c
> index 4ee2b7b..e5a0998 100644
> --- a/arch/arm/mach-socfpga/clock_manager_agilex.c
> +++ b/arch/arm/mach-socfpga/clock_manager_agilex.c
> @@ -12,6 +12,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
> 
>  DECLARE_GLOBAL_DATA_PTR;
> 
> @@ -65,8 +66,8 @@ unsigned int cm_get_l4_sys_free_clk_hz(void)
> 
>  u32 cm_get_qspi_controller_clk_hz(void)
>  {
> - return readl(socfpga_get_sysmgr_addr() +
> -  SYSMGR_SOC64_BOOT_SCRATCH_COLD0);
> + return socfpga_secure_reg_read32(socfpga_get_sysmgr_addr() +
> +
> SYSMGR_SOC64_BOOT_SCRATCH_COLD0);
>  }
> 
>  void cm_print_clock_quick_summary(void)
> diff --git a/arch/arm/mach-socfpga/clock_manager_s10.c b/arch/arm/mach-
> socfpga/clock_manager_s10.c
> index 05e4212..02578cc 100644
> --- a/arch/arm/mach-socfpga/clock_manager_s10.c
> +++ b/arch/arm/mach-socfpga/clock_manager_s10.c
> @@ -9,6 +9,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
> 
>  DECLARE_GLOBAL_DATA_PTR;
> 
> @@ -385,8 +386,8 @@ unsigned int cm_get_l4_sp_clk_hz(void)
> 
>  unsigned int cm_get_qspi_controller_clk_hz(void)
>  {
> - return readl(socfpga_get_sysmgr_addr() +
> -  SYSMGR_SOC64_BOOT_SCRATCH_COLD0);
> + return socfpga_secure_reg_read32(socfpga_get_sysmgr_addr() +
> +
> SYSMGR_SOC64_BOOT_SCRATCH_COLD0);
>  }
> 
>  unsigned int cm_get_spi_controller_clk_hz(void)
> --
> 2.7.4
SPL reads the clock info from handoff table (OCRAM) and write
the clock info into the System Manager's boot scratch register.
U-Boot proper will read from the System Manager's boot scratch
register to get the clock info in case the handoff table (OCRAM)
is no longer available.
After some investigations, the handoff table in OCRAM should be preserved
for warm boot. In other words, this handoff table should be left untouched.
SPL and U-Boot should directly read the clock info from handoff table in OCRAM.
Therefore, U-Boot proper no longer need to read the clock info from
System Manager's boot scratch register (secure zone) from non-secure world 
(EL2).

Re: [PATCH v2 2/5] travis: build HTML docs

[Bin Meng]

On Sat, Feb 22, 2020 at 1:24 AM Heinrich Schuchardt  wrote:
>
> Several patches delivered incorrect restructured text as documentation.
> We should be able to discover this in Travis CI.
>
> Provide a build step for 'make htmldocs'.
>
> Add required package graphviz.
>
> Signed-off-by: Heinrich Schuchardt 
> ---
> v2:
> change sequence of targets: put htmldocs after cppcheck
> ---
>  .travis.yml | 5 +
>  1 file changed, 5 insertions(+)
>

Reviewed-by: Bin Meng 

Re: [PATCH v2 5/5] doc/Makefile: turn warnings into errors

[Bin Meng]

On Sat, Feb 22, 2020 at 1:25 AM Heinrich Schuchardt  wrote:
>
> Several patches delivered incorrect restructured text as documentation. We
> should be able to discover this in Travis CI, Gitlab CI, or Azure CI.
>
> So let us turn all build warnings into errors.
>
> Signed-off-by: Heinrich Schuchardt 
> ---
> v2:
> change commit message
> ---
>  doc/Makefile | 1 +
>  1 file changed, 1 insertion(+)
>

Reviewed-by: Bin Meng 

Re: [PATCH] RFC: nvedit: support doing one (extra) expansion of the value in "env set"

[Tom Rini]

On Fri, Feb 21, 2020 at 11:32:16PM +, Rasmus Villemoes wrote:
> On 18/02/2020 09.11, Rasmus Villemoes wrote:
> 
> > Thanks, Wolfgang. Consider both "env set -E" and the alternative "env
> > get" withdrawn.
> 
> So, if I wanted to change the status of such a patch in patchwork, what
> would be the appropriate status? There's no "Withdrawn" or "Retracted".
> So "Not applicable" or "Rejected"?

No strong preference, you can even just put it at / leave it at RFC.
Thanks!

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH] RFC: nvedit: support doing one (extra) expansion of the value in "env set"

[Rasmus Villemoes]

On 18/02/2020 09.11, Rasmus Villemoes wrote:

> Thanks, Wolfgang. Consider both "env set -E" and the alternative "env
> get" withdrawn.

So, if I wanted to change the status of such a patch in patchwork, what
would be the appropriate status? There's no "Withdrawn" or "Retracted".
So "Not applicable" or "Rejected"?

Rasmus

Re: [PATCH] efi_selftest: Ensure we include the object directory for generated headers

[Heinrich Schuchardt]

On 2/21/20 10:52 PM, Tom Rini wrote:

On Fri, Feb 21, 2020 at 10:34:01PM +0100, Heinrich Schuchardt wrote:

On 2/21/20 9:48 PM, Tom Rini wrote:

The current rules for generating -I lines for objects gives us both
-I/full/src/path/lib/efi_selftest and -Ilib/efi_selftest.  However, if
we re-sync the Kbuild logic we no longer get the latter implicitly.  We
will need to be explicit in such cases, so do so.

Cc: Heinrich Schuchardt 
Cc: Masahiro Yamada 
Signed-off-by: Tom Rini 


Hello Tom,

yes, in lib/efi_selftest we have generated includes, so
-Ilib/efi_selftest is relevant.

Do you have a git branch with the Kconfig changes for reference and testing?


Currently WIP/kbuild-resync-v1-v4.18 but please bear in mind I'm (a)
rebase it and (b) it doesn't even build sandbox_spl right now (more of
this type of error, but my non-CI world build will finish up shortly and
I'll grab all of the places I need to make a fix like this there).



I was able to reproduce that the proposed change for
lib/efi_selftest/Makefile is needed for out of tree building.

So the patch is fine with me.

Best regards

Heinrich

Re: [PATCH] efi_selftest: Ensure we include the object directory for generated headers

[Tom Rini]

On Fri, Feb 21, 2020 at 10:34:01PM +0100, Heinrich Schuchardt wrote:
> On 2/21/20 9:48 PM, Tom Rini wrote:
> > The current rules for generating -I lines for objects gives us both
> > -I/full/src/path/lib/efi_selftest and -Ilib/efi_selftest.  However, if
> > we re-sync the Kbuild logic we no longer get the latter implicitly.  We
> > will need to be explicit in such cases, so do so.
> > 
> > Cc: Heinrich Schuchardt 
> > Cc: Masahiro Yamada 
> > Signed-off-by: Tom Rini 
> 
> Hello Tom,
> 
> yes, in lib/efi_selftest we have generated includes, so
> -Ilib/efi_selftest is relevant.
> 
> Do you have a git branch with the Kconfig changes for reference and testing?

Currently WIP/kbuild-resync-v1-v4.18 but please bear in mind I'm (a)
rebase it and (b) it doesn't even build sandbox_spl right now (more of
this type of error, but my non-CI world build will finish up shortly and
I'll grab all of the places I need to make a fix like this there).

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH] efi_selftest: Ensure we include the object directory for generated headers

[Heinrich Schuchardt]

On 2/21/20 9:48 PM, Tom Rini wrote:

The current rules for generating -I lines for objects gives us both
-I/full/src/path/lib/efi_selftest and -Ilib/efi_selftest.  However, if
we re-sync the Kbuild logic we no longer get the latter implicitly.  We
will need to be explicit in such cases, so do so.

Cc: Heinrich Schuchardt 
Cc: Masahiro Yamada 
Signed-off-by: Tom Rini 


Hello Tom,

yes, in lib/efi_selftest we have generated includes, so
-Ilib/efi_selftest is relevant.

Do you have a git branch with the Kconfig changes for reference and testing?

Acked-by: Heinrich Schuchardt 


---
  lib/efi_selftest/Makefile | 4 
  1 file changed, 4 insertions(+)

diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile
index 3ad96e1cbf08..7d0199d634c6 100644
--- a/lib/efi_selftest/Makefile
+++ b/lib/efi_selftest/Makefile
@@ -12,6 +12,10 @@ CFLAGS_efi_selftest_miniapp_exit.o := $(CFLAGS_EFI) -Os 
-ffreestanding
  CFLAGS_REMOVE_efi_selftest_miniapp_exit.o := $(CFLAGS_NON_EFI)
  CFLAGS_efi_selftest_miniapp_return.o := $(CFLAGS_EFI) -Os -ffreestanding
  CFLAGS_REMOVE_efi_selftest_miniapp_return.o := $(CFLAGS_NON_EFI)
+CFLAGS_efi_selftest_exception.o += -I$(objtree)/$(obj)
+CFLAGS_efi_selftest_loadimage.o += -I$(objtree)/$(obj)
+CFLAGS_efi_selftest_startimage_exit.o += -I$(objtree)/$(obj)
+CFLAGS_efi_selftest_startimage_return.o += -I$(objtree)/$(obj)

  obj-y += \
  efi_selftest.o \

[PATCH] efi_selftest: Ensure we include the object directory for generated headers

[Tom Rini]

The current rules for generating -I lines for objects gives us both
-I/full/src/path/lib/efi_selftest and -Ilib/efi_selftest.  However, if
we re-sync the Kbuild logic we no longer get the latter implicitly.  We
will need to be explicit in such cases, so do so.

Cc: Heinrich Schuchardt 
Cc: Masahiro Yamada 
Signed-off-by: Tom Rini 
---
 lib/efi_selftest/Makefile | 4 
 1 file changed, 4 insertions(+)

diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile
index 3ad96e1cbf08..7d0199d634c6 100644
--- a/lib/efi_selftest/Makefile
+++ b/lib/efi_selftest/Makefile
@@ -12,6 +12,10 @@ CFLAGS_efi_selftest_miniapp_exit.o := $(CFLAGS_EFI) -Os 
-ffreestanding
 CFLAGS_REMOVE_efi_selftest_miniapp_exit.o := $(CFLAGS_NON_EFI)
 CFLAGS_efi_selftest_miniapp_return.o := $(CFLAGS_EFI) -Os -ffreestanding
 CFLAGS_REMOVE_efi_selftest_miniapp_return.o := $(CFLAGS_NON_EFI)
+CFLAGS_efi_selftest_exception.o += -I$(objtree)/$(obj)
+CFLAGS_efi_selftest_loadimage.o += -I$(objtree)/$(obj)
+CFLAGS_efi_selftest_startimage_exit.o += -I$(objtree)/$(obj)
+CFLAGS_efi_selftest_startimage_return.o += -I$(objtree)/$(obj)
 
 obj-y += \
 efi_selftest.o \
-- 
2.17.1

Re: [PATCH 1/1] travis-ci: provide env__efi_fit_tftp_file

[Stephen Warren]

On 2/21/20 12:35 PM, Heinrich Schuchardt wrote:

On 1/8/20 2:16 AM, Stephen Warren wrote:

On 1/7/20 10:06 AM, Heinrich Schuchardt wrote:

Provide dictionary env__efi_fit_tftp_file describing the file used for
the
UEFI FIT image test.


Acked-by: Stephen Warren 



Hello Stephen,

this patch for has been lingering for a while waiting to be merged into
https://github.com/swarren/uboot-test-hooks.git.

Could you, please, give it some love.


Sorry, I hadn't realized this was for that repo, since it was posted to 
the list which usually means the patch is for u-boot.git. If you repost 
it I can apply it.

Re: [PATCH 1/1] travis-ci: provide env__efi_fit_tftp_file

[Heinrich Schuchardt]

On 1/8/20 2:16 AM, Stephen Warren wrote:

On 1/7/20 10:06 AM, Heinrich Schuchardt wrote:

Provide dictionary env__efi_fit_tftp_file describing the file used for
the
UEFI FIT image test.


Acked-by: Stephen Warren 



Hello Stephen,

this patch for has been lingering for a while waiting to be merged into
https://github.com/swarren/uboot-test-hooks.git.

Could you, please, give it some love.

Best regards

Heinrich

Re: Pull request: u-boot-rockchip-20200220

[Tom Rini]

On Fri, Feb 21, 2020 at 09:17:41AM +0800, Kever Yang wrote:

> Hi Tom,
> 
> Please pull the rockchip updates/fixes:
> - rk3399: split roc-pc-rk3399 out of evb_rk3399
> - sync dts from upstream kernel for rk3399,rk3288,px30
> - boot_mode: find the saradc device name
> 
> Travis:
> https://travis-ci.org/keveryang/u-boot/builds/652785865
> 
> Thanks,
> - Kever
> 
> The following changes since commit f2a73d6867ef973fbb8471cc87058205999b5e5c:
> 
>   Merge tag 'u-boot-stm32-20200214' of 
> https://gitlab.denx.de/u-boot/custodians/u-boot-stm (2020-02-14 07:31:47 
> -0500)
> 
> are available in the Git repository at:
> 
>   https://gitlab.denx.de/u-boot/custodians/u-boot-rockchip.git 
> tags/u-boot-rockchip-20200220
> 
> for you to fetch changes up to f9561d8e3671415c7780df4b5e70f0f1e2d2bf57:
> 
>   arch: arm: rockchip: order the rk3399 entries alphabetically (2020-02-20 
> 09:18:14 +0800)
> 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature

RE: [PATCH v2 10/21] arm: socfpga: Add secure register access helper functions for SoC 64bits

[Ang, Chee Hong]

> On 2/21/20 7:01 PM, Ang, Chee Hong wrote:
> >> On 2/20/20 6:54 PM, Ang, Chee Hong wrote:
>  On 2/20/20 3:02 AM, Ang, Chee Hong wrote:
>  [...]
> >>> +#if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_ATF)
> >>> +u32 socfpga_secure_reg_read32(phys_addr_t reg_addr); void
> >>> +socfpga_secure_reg_write32(u32 val, phys_addr_t reg_addr); void
> >>> +socfpga_secure_reg_update32(phys_addr_t reg_addr, u32 mask, u32
> >>> +val); #else
> >>> +#define socfpga_secure_reg_read32readl
> >>> +#define socfpga_secure_reg_write32   writel
> >>> +#define socfpga_secure_reg_update32  clrsetbits_le32
> >>> +#endif
> >>
> >> I think I don't understand how this is supposed to work. Would
> >> every place in U- Boot have to be patched to call these functions now ?
> >
> > Not every register access need this. Only those accessing
> > registers in secure zone such as 'System Manager' registers need to call
> this.
> > It's basically determine whether the driver should issue SMC/PSCI
> > call if it's running in EL2 (non-secure) or access the registers
> > directly by simply using
>  readl/writel and etc if it's running in EL3 (secure).
> > Accessing those registers in secure zone in non-secure mode (EL2)
> > will cause
>  SError exception.
> > So we can determine this behaviour in compile time:
> > SPL always running in EL3. So it just simply fallback to use
>  readl/writel/clrsetbits_le32.
> >
> > For U-Boot proper (SSBL), there are 2 scenarios:
> > 1) If CONFIG_SPL_ATF is defined, it means ATF is supported. It
> > implies that U-Boot proper will be running in EL2 (non-secure),
> > then it will use
>  SMC/PSCI calls to access the secure registers.
> >
> > 2) CONFIG_SPL_ATF is not defined, no ATF support. U-Boot proper
> > will be running in EL3 which will fall back to simply using the
> > direct access functions
>  (readl/writel and etc).
> 
>  I would expect the standard IO accessors would or should handle this 
>  stuff ?
> >>> Standard IO accessors are just general memory read/write functions
> >>> designed to be compatible with general hardware platforms. Not all
> >>> platforms have secure/non-secure hardware zones. I don't think they
> >>> should
> >> handle this.
> >>>
> >>> If it's running in EL3 (secure mode) the standard I/O accessors will
> >>> work just fine because
> >>> EL3 can access to all secure/non-secure zones. In the header file,
> >>> you can see the secure I/O accessors will be replaced by the
> >>> standard I/O accessors if it's built for SPL and U-Boot proper
> >>> without ATF. Because both are
> >> running in EL3 (secure).
> >>>
> >>> If ATF is enabled, SPL will be still running in EL3 but U-Boot
> >>> proper will be running in
> >>> EL2 (non-secure). If any code accessing those secure zones without
> >>> going through ATF (making SMC/PSCI calls to EL3), it will trigger 'SError'
> >> exceptions and crash the U-Boot.
> >>
> >> Hmmm, if U-Boot is running in EL2 (non-secure), why would it ever
> >> access secure zones in the first place ?
> > SPL and U-Boot reuse the same drivers code. It runs without issue in
> > SPL (EL3) but it crashes if running the same driver code in EL2 which access
> some secure zone registers.
> > The System Manager (secure zone) contains some register which control
> > the behaviours of EMAC/SDMMC and etc. Clock manager driver rely on
> > those registers in System Manager as well for retrieving clock
> > information. These clock/EMAC/SDMMC drivers access the System Manager
> (secure zone).
> 
> Maybe those registers should only be configured by the secure OS, so maybe
> those drivers should be fixed ?
> 
> >> And if that's really necessary, should the ATF really provide
> >> secure-mode register access primitives or should it provide some more high-
> level interface instead ?
> > I see your point. I should have mentioned to you that these
> > secure-mode register access provided by ATF actually do more stuffs than 
> > just
> primitive accessors.
> 
> So socfpga_secure_reg_read32 does not just read a register ? Then the naming
> is misleading . And documentation is missing.
> 
> > ATF only allow certain secure registers required by the non-secure driver 
> > to be
> accessed.
> > It will check the secure register address and block access if the
> > register address is not allowed to be accessed by non-secure world (EL2).
> 
> Why don't you configure those secure registers in the secure mode then ?
> It seems like that's the purpose of those registers being secure only.
> 
> > So these secure register access provided by ATF is not just simple
> > accessor/delegate which simply allow access to any secure zone from non-
> secure world without any restrictions.
> > I would say the secure register access provided by ATF is a
> > 'middle-level' interface not just some primitive accessors.
> >
> > Currently, we have 

RE: [PATCH v2 05/21] arm: socfpga: Override 'lowlevel_init' to support ATF

[Ang, Chee Hong]

> > On 2/20/20 8:05 PM, Ang, Chee Hong wrote:
> > >> On 2/20/20 3:27 AM, Ang, Chee Hong wrote:
> >  On 2/19/20 1:25 PM, chee.hong@intel.com wrote:
> >  [...]
> > > diff --git a/arch/arm/mach-socfpga/lowlevel_init.S
> > > b/arch/arm/mach-socfpga/lowlevel_init.S
> > > new file mode 100644
> > > index 000..68053a0
> > > --- /dev/null
> > > +++ b/arch/arm/mach-socfpga/lowlevel_init.S
> > 
> >  This should be some lowlevel_init_64.S to make it clear it's only
> >  for
> >  arm64 platforms.
> > >>> OK. It makes sense. Thanks.
> > 
> > > @@ -0,0 +1,85 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +/*
> > > + * Copyright (C) 2019, Intel Corporation  */
> > > +
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +
> > > +ENTRY(lowlevel_init)
> > > + mov x29, lr /* Save LR */
> > > +
> > > +#if defined(CONFIG_GICV2) || defined(CONFIG_GICV3) #ifdef
> > > +CONFIG_SPL_ATF
> > > + branch_if_slave x0, 2f
> > > +#else
> > > + branch_if_slave x0, 1f
> > > +#endif
> > > +
> > > + ldr x0, =GICD_BASE
> > > + bl  gic_init_secure
> > > +#ifdef CONFIG_SPL_BUILD
> > > + b   2f
> > > +#else
> > > + b   3f
> > > +#endif
> > 
> >  Can't this be done in C code ? Can we reduce the ifdeffery ?
> > >>> This lowlevel_init function is shared by SPL and U-Boot and they
> > >>> run in slightly different flow.
> > >>
> > >> What does this 'different flow' mean ?
> > > This has something to with multi-cores CPU such as A53.
> > > For SPL, we need to make sure the slave CPUs (CPU1/2/3) trapped in a 
> > > 'place'
> > > Where they could be 'activated' by kernel for multi-processor environment.
> > > It means the kernel get to 'activate' the slave CPUs from master CPU
> > > (CPU0)  U-Boot proper only run on master CPU (CPU0). The rest of
> > > slave CPUs are trapped in the beginning of SPL waiting to be 'activated'
> > > by kernel.
> >
> > OK, so the secondary CPUs are spinning until the kernel releases them.
> >
> > > In U-Boot proper, only master CPU gets to run this code and it will
> > > just do the basic GIC setup and skip the 'trap'. The 'trap' is to
> > > prevent the slave CPUs from running the same SPL, ATF and U-Boot
> > > code as the master CPU in parallel. Only single core (maser CPU) is
> > > needed for
> > bootloaders and firmware.
> >
> > I would expect all the other SMP platforms solved this issue with
> > secondary CPUs already, so why is agilex special ?
> Nothing special. Just like other SMP platforms, I am just telling you the 
> master
> CPU always skips the 'spinning trap' in SPL and U-Boot proper.
ATF is now used to activate the secondary CPUs by Linux.
This lowlevel_init is to make sure secondary CPUs trap in ATF
instead of SPL after ATF is initialized. 

RE: [PATCH v2 05/21] arm: socfpga: Override 'lowlevel_init' to support ATF

[Ang, Chee Hong]

> On 2/20/20 8:05 PM, Ang, Chee Hong wrote:
> >> On 2/20/20 3:27 AM, Ang, Chee Hong wrote:
>  On 2/19/20 1:25 PM, chee.hong@intel.com wrote:
>  [...]
> > diff --git a/arch/arm/mach-socfpga/lowlevel_init.S
> > b/arch/arm/mach-socfpga/lowlevel_init.S
> > new file mode 100644
> > index 000..68053a0
> > --- /dev/null
> > +++ b/arch/arm/mach-socfpga/lowlevel_init.S
> 
>  This should be some lowlevel_init_64.S to make it clear it's only
>  for
>  arm64 platforms.
> >>> OK. It makes sense. Thanks.
> 
> > @@ -0,0 +1,85 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (C) 2019, Intel Corporation  */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +ENTRY(lowlevel_init)
> > +   mov x29, lr /* Save LR */
> > +
> > +#if defined(CONFIG_GICV2) || defined(CONFIG_GICV3) #ifdef
> > +CONFIG_SPL_ATF
> > +   branch_if_slave x0, 2f
> > +#else
> > +   branch_if_slave x0, 1f
> > +#endif
> > +
> > +   ldr x0, =GICD_BASE
> > +   bl  gic_init_secure
> > +#ifdef CONFIG_SPL_BUILD
> > +   b   2f
> > +#else
> > +   b   3f
> > +#endif
> 
>  Can't this be done in C code ? Can we reduce the ifdeffery ?
> >>> This lowlevel_init function is shared by SPL and U-Boot and they run
> >>> in slightly different flow.
> >>
> >> What does this 'different flow' mean ?
> > This has something to with multi-cores CPU such as A53.
> > For SPL, we need to make sure the slave CPUs (CPU1/2/3) trapped in a 'place'
> > Where they could be 'activated' by kernel for multi-processor environment.
> > It means the kernel get to 'activate' the slave CPUs from master CPU
> > (CPU0)  U-Boot proper only run on master CPU (CPU0). The rest of slave
> > CPUs are trapped in the beginning of SPL waiting to be 'activated'
> > by kernel.
> 
> OK, so the secondary CPUs are spinning until the kernel releases them.
> 
> > In U-Boot proper, only master CPU gets to run this code and it will
> > just do the basic GIC setup and skip the 'trap'. The 'trap' is to
> > prevent the slave CPUs from running the same SPL, ATF and U-Boot code
> > as the master CPU in parallel. Only single core (maser CPU) is needed for
> bootloaders and firmware.
> 
> I would expect all the other SMP platforms solved this issue with secondary
> CPUs already, so why is agilex special ?
Nothing special. Just like other SMP platforms, I am just telling you the 
master CPU
always skips the 'spinning trap' in SPL and U-Boot proper.

Aw: [PATCH 3/3] arm: mediatek: remove unuse binman config

[Frank Wunderlich]

Hi Sam

thank you for the Patch fixing the binman_fdt issue on bananapi r2/r64.

but i think this should be posted separately because it's not related to pwm. I 
hope Tom will pick up it soon (independ of pwm driver) to fix 2020-04.

regards Frank

RE: [PATCH v2 01/21] configs: agilex: Remove CONFIG_OF_EMBED

[Ang, Chee Hong]

> On 2/20/20 6:04 PM, Westergreen, Dalon wrote:
> 
> Please fix your mailer, it makes your reply completely unreadable.
> 
> > On Thu, 2020-02-20 at 17:44 +0100, Marek Vasut wrote:
> >
> > On 2/20/20 3:12 AM, Ang, Chee Hong wrote:
> >
> > On 2/19/20 1:25 PM,
> >
> > 
> >
> > chee.hong@intel.com
> >
> >  wrote:
> >
> > From: Chee Hong Ang <
> >
> > 
> >
> > chee.hong@intel.com
> >
> >>
> >
> >
> > CONFIG_OF_EMBED was primarily enabled to support the agilex spl hex
> >
> > file requirements.  Since this option now produces a warning during
> >
> > build, and the spl hex can be created using alternate methods,
> >
> > CONFIG_OF_EMBED is no longer needed.
> 
> OK, so this patch removes functionality.
> Can that functionality be retained ? Or what can be done here?
The functionality is no longer required.
> 
> [...]

Re: [PATCH v2 10/21] arm: socfpga: Add secure register access helper functions for SoC 64bits

[Marek Vasut]

On 2/21/20 7:01 PM, Ang, Chee Hong wrote:
>> On 2/20/20 6:54 PM, Ang, Chee Hong wrote:
 On 2/20/20 3:02 AM, Ang, Chee Hong wrote:
 [...]
>>> +#if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_ATF)
>>> +u32 socfpga_secure_reg_read32(phys_addr_t reg_addr); void
>>> +socfpga_secure_reg_write32(u32 val, phys_addr_t reg_addr); void
>>> +socfpga_secure_reg_update32(phys_addr_t reg_addr, u32 mask, u32
>>> +val); #else
>>> +#define socfpga_secure_reg_read32  readl
>>> +#define socfpga_secure_reg_write32 writel
>>> +#define socfpga_secure_reg_update32clrsetbits_le32
>>> +#endif
>>
>> I think I don't understand how this is supposed to work. Would
>> every place in U- Boot have to be patched to call these functions now ?
>
> Not every register access need this. Only those accessing registers
> in secure zone such as 'System Manager' registers need to call this.
> It's basically determine whether the driver should issue SMC/PSCI
> call if it's running in EL2 (non-secure) or access the registers
> directly by simply using
 readl/writel and etc if it's running in EL3 (secure).
> Accessing those registers in secure zone in non-secure mode (EL2)
> will cause
 SError exception.
> So we can determine this behaviour in compile time:
> SPL always running in EL3. So it just simply fallback to use
 readl/writel/clrsetbits_le32.
>
> For U-Boot proper (SSBL), there are 2 scenarios:
> 1) If CONFIG_SPL_ATF is defined, it means ATF is supported. It
> implies that U-Boot proper will be running in EL2 (non-secure), then
> it will use
 SMC/PSCI calls to access the secure registers.
>
> 2) CONFIG_SPL_ATF is not defined, no ATF support. U-Boot proper will
> be running in EL3 which will fall back to simply using the direct
> access functions
 (readl/writel and etc).

 I would expect the standard IO accessors would or should handle this stuff 
 ?
>>> Standard IO accessors are just general memory read/write functions
>>> designed to be compatible with general hardware platforms. Not all
>>> platforms have secure/non-secure hardware zones. I don't think they should
>> handle this.
>>>
>>> If it's running in EL3 (secure mode) the standard I/O accessors will
>>> work just fine because
>>> EL3 can access to all secure/non-secure zones. In the header file, you
>>> can see the secure I/O accessors will be replaced by the standard I/O
>>> accessors if it's built for SPL and U-Boot proper without ATF. Because both 
>>> are
>> running in EL3 (secure).
>>>
>>> If ATF is enabled, SPL will be still running in EL3 but U-Boot proper
>>> will be running in
>>> EL2 (non-secure). If any code accessing those secure zones without
>>> going through ATF (making SMC/PSCI calls to EL3), it will trigger 'SError'
>> exceptions and crash the U-Boot.
>>
>> Hmmm, if U-Boot is running in EL2 (non-secure), why would it ever access
>> secure zones in the first place ?
> SPL and U-Boot reuse the same drivers code. It runs without issue in SPL 
> (EL3) but
> it crashes if running the same driver code in EL2 which access some secure 
> zone registers.
> The System Manager (secure zone) contains some register which control the 
> behaviours of
> EMAC/SDMMC and etc. Clock manager driver rely on those registers in System 
> Manager as well
> for retrieving clock information. These clock/EMAC/SDMMC drivers access the 
> System Manager
> (secure zone).

Maybe those registers should only be configured by the secure OS, so
maybe those drivers should be fixed ?

>> And if that's really necessary, should the ATF really provide secure-mode 
>> register
>> access primitives or should it provide some more high-level interface 
>> instead ?
> I see your point. I should have mentioned to you that these secure-mode 
> register access provided by
> ATF actually do more stuffs than just primitive accessors.

So socfpga_secure_reg_read32 does not just read a register ? Then the
naming is misleading . And documentation is missing.

> ATF only allow certain secure registers required by the non-secure driver to 
> be accessed.
> It will check the secure register address and block access if the register 
> address is not allowed
> to be accessed by non-secure world (EL2).

Why don't you configure those secure registers in the secure mode then ?
It seems like that's the purpose of those registers being secure only.

> So these secure register access provided by ATF is not just simple 
> accessor/delegate which
> simply allow access to any secure zone from non-secure world without any 
> restrictions.
> I would say the secure register access provided by ATF is a 'middle-level' 
> interface not just
> some primitive accessors.
> 
> Currently, we have like 20+ secure registers allowed access by drivers 
> running in
> non-secure mode (U-Boot proper / Linux).
> I don't think we want to define and maintain those high level 

RE: [PATCH v2 10/21] arm: socfpga: Add secure register access helper functions for SoC 64bits

[Ang, Chee Hong]

> On 2/20/20 6:54 PM, Ang, Chee Hong wrote:
> >> On 2/20/20 3:02 AM, Ang, Chee Hong wrote:
> >> [...]
> > +#if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_ATF)
> > +u32 socfpga_secure_reg_read32(phys_addr_t reg_addr); void
> > +socfpga_secure_reg_write32(u32 val, phys_addr_t reg_addr); void
> > +socfpga_secure_reg_update32(phys_addr_t reg_addr, u32 mask, u32
> > +val); #else
> > +#define socfpga_secure_reg_read32  readl
> > +#define socfpga_secure_reg_write32 writel
> > +#define socfpga_secure_reg_update32clrsetbits_le32
> > +#endif
> 
>  I think I don't understand how this is supposed to work. Would
>  every place in U- Boot have to be patched to call these functions now ?
> >>>
> >>> Not every register access need this. Only those accessing registers
> >>> in secure zone such as 'System Manager' registers need to call this.
> >>> It's basically determine whether the driver should issue SMC/PSCI
> >>> call if it's running in EL2 (non-secure) or access the registers
> >>> directly by simply using
> >> readl/writel and etc if it's running in EL3 (secure).
> >>> Accessing those registers in secure zone in non-secure mode (EL2)
> >>> will cause
> >> SError exception.
> >>> So we can determine this behaviour in compile time:
> >>> SPL always running in EL3. So it just simply fallback to use
> >> readl/writel/clrsetbits_le32.
> >>>
> >>> For U-Boot proper (SSBL), there are 2 scenarios:
> >>> 1) If CONFIG_SPL_ATF is defined, it means ATF is supported. It
> >>> implies that U-Boot proper will be running in EL2 (non-secure), then
> >>> it will use
> >> SMC/PSCI calls to access the secure registers.
> >>>
> >>> 2) CONFIG_SPL_ATF is not defined, no ATF support. U-Boot proper will
> >>> be running in EL3 which will fall back to simply using the direct
> >>> access functions
> >> (readl/writel and etc).
> >>
> >> I would expect the standard IO accessors would or should handle this stuff 
> >> ?
> > Standard IO accessors are just general memory read/write functions
> > designed to be compatible with general hardware platforms. Not all
> > platforms have secure/non-secure hardware zones. I don't think they should
> handle this.
> >
> > If it's running in EL3 (secure mode) the standard I/O accessors will
> > work just fine because
> > EL3 can access to all secure/non-secure zones. In the header file, you
> > can see the secure I/O accessors will be replaced by the standard I/O
> > accessors if it's built for SPL and U-Boot proper without ATF. Because both 
> > are
> running in EL3 (secure).
> >
> > If ATF is enabled, SPL will be still running in EL3 but U-Boot proper
> > will be running in
> > EL2 (non-secure). If any code accessing those secure zones without
> > going through ATF (making SMC/PSCI calls to EL3), it will trigger 'SError'
> exceptions and crash the U-Boot.
> 
> Hmmm, if U-Boot is running in EL2 (non-secure), why would it ever access
> secure zones in the first place ?
SPL and U-Boot reuse the same drivers code. It runs without issue in SPL (EL3) 
but
it crashes if running the same driver code in EL2 which access some secure zone 
registers.
The System Manager (secure zone) contains some register which control the 
behaviours of
EMAC/SDMMC and etc. Clock manager driver rely on those registers in System 
Manager as well
for retrieving clock information. These clock/EMAC/SDMMC drivers access the 
System Manager
(secure zone).
> 
> And if that's really necessary, should the ATF really provide secure-mode 
> register
> access primitives or should it provide some more high-level interface instead 
> ?
I see your point. I should have mentioned to you that these secure-mode 
register access provided by
ATF actually do more stuffs than just primitive accessors.
ATF only allow certain secure registers required by the non-secure driver to be 
accessed.
It will check the secure register address and block access if the register 
address is not allowed
to be accessed by non-secure world (EL2).
So these secure register access provided by ATF is not just simple 
accessor/delegate which
simply allow access to any secure zone from non-secure world without any 
restrictions.
I would say the secure register access provided by ATF is a 'middle-level' 
interface not just
some primitive accessors.

Currently, we have like 20+ secure registers allowed access by drivers running 
in
non-secure mode (U-Boot proper / Linux).
I don't think we want to define and maintain those high level interfaces for 
each of those secure
register accesses in ATF and U-Boot.

[PATCH v2 1/5] doc: update doc/sphinx/kerneldoc.py

[Heinrich Schuchardt]

Update doc/sphinx/kerneldoc.py from Linux next-20200219 to avoid warnings
like:

doc/sphinx/kerneldoc.py:125: RemovedInSphinx20Warning:
AutodocReporter is now deprecated. Use
sphinx.util.docutils.switch_source_input() instead.
  self.state.memo.reporter =
AutodocReporter(result, self.state.memo.reporter)

Signed-off-by: Heinrich Schuchardt 
---
v2:
new patch
---
 doc/sphinx/kerneldoc.py | 51 +++--
 1 file changed, 39 insertions(+), 12 deletions(-)

diff --git a/doc/sphinx/kerneldoc.py b/doc/sphinx/kerneldoc.py
index e536360de1..4bcbd6ae01 100644
--- a/doc/sphinx/kerneldoc.py
+++ b/doc/sphinx/kerneldoc.py
@@ -37,7 +37,17 @@ import glob
 from docutils import nodes, statemachine
 from docutils.statemachine import ViewList
 from docutils.parsers.rst import directives, Directive
-from sphinx.ext.autodoc import AutodocReporter
+
+#
+# AutodocReporter is only good up to Sphinx 1.7
+#
+import sphinx
+
+Use_SSI = sphinx.__version__[:3] >= '1.7'
+if Use_SSI:
+from sphinx.util.docutils import switch_source_input
+else:
+from sphinx.ext.autodoc import AutodocReporter

 import kernellog

@@ -49,9 +59,10 @@ class KernelDocDirective(Directive):
 optional_arguments = 4
 option_spec = {
 'doc': directives.unchanged_required,
-'functions': directives.unchanged_required,
 'export': directives.unchanged,
 'internal': directives.unchanged,
+'identifiers': directives.unchanged,
+'functions': directives.unchanged,
 }
 has_content = False

@@ -67,6 +78,10 @@ class KernelDocDirective(Directive):

 tab_width = self.options.get('tab-width', 
self.state.document.settings.tab_width)

+# 'function' is an alias of 'identifiers'
+if 'functions' in self.options:
+self.options['identifiers'] = self.options.get('functions')
+
 # FIXME: make this nicer and more robust against errors
 if 'export' in self.options:
 cmd += ['-export']
@@ -76,9 +91,13 @@ class KernelDocDirective(Directive):
 export_file_patterns = str(self.options.get('internal')).split()
 elif 'doc' in self.options:
 cmd += ['-function', str(self.options.get('doc'))]
-elif 'functions' in self.options:
-for f in str(self.options.get('functions')).split():
-cmd += ['-function', f]
+elif 'identifiers' in self.options:
+identifiers = self.options.get('identifiers').split()
+if identifiers:
+for i in identifiers:
+cmd += ['-function', i]
+else:
+cmd += ['-no-doc-sections']

 for pattern in export_file_patterns:
 for f in glob.glob(env.config.kerneldoc_srctree + '/' + pattern):
@@ -121,13 +140,7 @@ class KernelDocDirective(Directive):
 lineoffset += 1

 node = nodes.section()
-buf = self.state.memo.title_styles, self.state.memo.section_level, 
self.state.memo.reporter
-self.state.memo.reporter = AutodocReporter(result, 
self.state.memo.reporter)
-self.state.memo.title_styles, self.state.memo.section_level = [], 0
-try:
-self.state.nested_parse(result, 0, node, match_titles=1)
-finally:
-self.state.memo.title_styles, self.state.memo.section_level, 
self.state.memo.reporter = buf
+self.do_parse(result, node)

 return node.children

@@ -136,6 +149,20 @@ class KernelDocDirective(Directive):
(" ".join(cmd), str(e)))
 return [nodes.error(None, nodes.paragraph(text = "kernel-doc 
missing"))]

+def do_parse(self, result, node):
+if Use_SSI:
+with switch_source_input(self.state, result):
+self.state.nested_parse(result, 0, node, match_titles=1)
+else:
+save = self.state.memo.title_styles, 
self.state.memo.section_level, self.state.memo.reporter
+self.state.memo.reporter = AutodocReporter(result, 
self.state.memo.reporter)
+self.state.memo.title_styles, self.state.memo.section_level = [], 0
+try:
+self.state.nested_parse(result, 0, node, match_titles=1)
+finally:
+self.state.memo.title_styles, self.state.memo.section_level, 
self.state.memo.reporter = save
+
+
 def setup(app):
 app.add_config_value('kerneldoc_bin', None, 'env')
 app.add_config_value('kerneldoc_srctree', None, 'env')
--
2.25.0

[PATCH v2 0/5] make htmldocs in CI

[Heinrich Schuchardt]

This patch series provides a htmldocs target for Travis, Gitlab and Azure
CI.

doc/sphinx/kerneldoc.py is updated to avoid a build warning with current
Sphinx.

The last patch turns build warnings of 'make htmldocs' into errors.

While first four patches can be immediately merged the last patch requires
an updated Docker image, cf.

[PATCH 1/1] Dockerfile: add imagemagick
https://lists.denx.de/pipermail/u-boot/2020-February/400983.html

v2:
update update doc/sphinx/kerneldoc.py
change sequence of targets in Travis Ci
change a commit message

Heinrich Schuchardt (5):
  doc: update doc/sphinx/kerneldoc.py
  travis: build HTML docs
  gitlab: build HTML documentation
  azure: build HTML documentation
  doc/Makefile: turn warnings into errors

 .azure-pipelines.yml| 10 
 .gitlab-ci.yml  |  7 ++
 .travis.yml |  5 
 doc/Makefile|  1 +
 doc/sphinx/kerneldoc.py | 51 +++--
 5 files changed, 62 insertions(+), 12 deletions(-)

--
2.25.0

[PATCH v2 5/5] doc/Makefile: turn warnings into errors

[Heinrich Schuchardt]

Several patches delivered incorrect restructured text as documentation. We
should be able to discover this in Travis CI, Gitlab CI, or Azure CI.

So let us turn all build warnings into errors.

Signed-off-by: Heinrich Schuchardt 
---
v2:
change commit message
---
 doc/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/doc/Makefile b/doc/Makefile
index 5135a96e88..0e0da5666f 100644
--- a/doc/Makefile
+++ b/doc/Makefile
@@ -56,6 +56,7 @@ quiet_cmd_sphinx = SPHINX  $@ --> file://$(abspath 
$(BUILDDIR)/$3/$4)
PYTHONDONTWRITEBYTECODE=1 \
BUILDDIR=$(abspath $(BUILDDIR)) SPHINX_CONF=$(abspath 
$(srctree)/$(src)/$5/$(SPHINX_CONF)) \
$(SPHINXBUILD) \
+   -W \
-b $2 \
-c $(abspath $(srctree)/$(src)) \
-d $(abspath $(BUILDDIR)/.doctrees/$3) \
--
2.25.0

[PATCH v2 4/5] azure: build HTML documentation

[Heinrich Schuchardt]

Several patches delivered incorrect restructured text as documentation. We
should be able to discover this in Azure CI.

Provide a build step for 'make htmldocs'.

Signed-off-by: Heinrich Schuchardt 
Reviewed-by: Bin Meng 
---
v2:
no change
---
 .azure-pipelines.yml | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml
index c22095830c..129035a1a5 100644
--- a/.azure-pipelines.yml
+++ b/.azure-pipelines.yml
@@ -54,6 +54,16 @@ jobs:
 steps:
   - script: cppcheck --force --quiet --inline-suppr .

+  - job: htmldocs
+displayName: 'Build HTML documentation'
+pool:
+  vmImage: $(ubuntu_vm)
+container:
+  image: $(ci_runner_image)
+  options: $(container_option)
+steps:
+  - script: make htmldocs
+
   - job: todo
 displayName: 'Search for TODO within source tree'
 pool:
--
2.25.0

[PATCH v2 2/5] travis: build HTML docs

[Heinrich Schuchardt]

Several patches delivered incorrect restructured text as documentation.
We should be able to discover this in Travis CI.

Provide a build step for 'make htmldocs'.

Add required package graphviz.

Signed-off-by: Heinrich Schuchardt 
---
v2:
change sequence of targets: put htmldocs after cppcheck
---
 .travis.yml | 5 +
 1 file changed, 5 insertions(+)

diff --git a/.travis.yml b/.travis.yml
index e6db9d6a72..911c85e130 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -38,6 +38,7 @@ addons:
 - libisl15
 - clang-7
 - srecord
+- graphviz

 install:
  # Clone uboot-test-hooks
@@ -352,6 +353,10 @@ matrix:
 - name: "cppcheck"
   script:
 - cppcheck --force --quiet --inline-suppr .
+# build HTML documentation
+- name: "htmldocs"
+  script:
+- make htmldocs
 # search for TODO within source tree
 - name: "grep TODO"
   script:
--
2.25.0

[PATCH v2 3/5] gitlab: build HTML documentation

[Heinrich Schuchardt]

Several patches delivered incorrect restructured text as documentation. We
should be able to discover this in Gitlab CI.

Provide a build step for 'make htmldocs'.

Signed-off-by: Heinrich Schuchardt 
Reviewed-by: Bin Meng 
---
v2:
no change
---
 .gitlab-ci.yml | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index d486e72042..652ed3b501 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -122,6 +122,13 @@ grep TODO/FIXME/HACK:
 # search for HACK within source tree and ignore HACKKIT board
 - grep -r HACK . | grep -v HACKKIT

+# build HTML documentation
+htmldocs:
+  tags: [ 'all' ]
+  stage: testsuites
+  script:
+- make htmldocs
+
 # some statistics about the code base
 sloccount:
   tags: [ 'all' ]
--
2.25.0

Re: [PATCH v7 0/7] rsa: extend rsa_verify() for UEFI secure boot

[Tom Rini]

On Fri, Feb 21, 2020 at 03:12:54PM +0900, AKASHI Takahiro wrote:

> # This patch set is a prerequisite for UEFI secure boot.
> 
> The current rsa_verify() requires five parameters for a RSA public key
> for efficiency while RSA, in theory, requires only two. In addition,
> those parameters are expected to come from FIT image.
> 
> So this function won't fit very well when we want to use it for the purpose
> of implementing UEFI secure boot, in particular, image authentication
> as well as variable authentication, where the essential two parameters
> are set to be retrieved from one of X509 certificates in signature
> database.
> 
> So, in this patch, additional three parameters will be calculated
> on the fly when rsa_verify() is called without fdt which should contain
> parameters above.
> 
> This calculation heavily relies on "big-number (or multi-precision)
> library." Therefore some routines from BearSSL[1] under MIT license are
> imported in this implementation. See Patch#4.
> # Please let me know if this is not appropriate.
> 
> Prerequisite:
> * public key parser in my "import x509/pkcs7 parser" patch[2]

This has been applied a long while back.

And for the record, without http://patchwork.ozlabs.org/patch/1239098/
applied sandbox fails to build.  I had said I would take care of that
specific issue, so I'm just noting it here.  I'm kicking off a larger
test now.

-- 
Tom


signature.asc
Description: PGP signature

Re: [GIT] Pull request: u-boot-dfu (21.02.2020)

[Tom Rini]

On Fri, Feb 21, 2020 at 06:04:51PM +0100, Marek Vasut wrote:
> On 2/21/20 6:01 PM, Tom Rini wrote:
> > On Fri, Feb 21, 2020 at 05:18:09PM +0100, Marek Vasut wrote:
> >> On 2/21/20 3:54 PM, Lukasz Majewski wrote:
> >>> Dear Marek,
> >>
> >> Hi,
> >>
> >>> The following changes since commit
> >>> f2a73d6867ef973fbb8471cc87058205999b5e5c:
> >>>
> >>>   Merge tag 'u-boot-stm32-20200214' of
> >>>   https://gitlab.denx.de/u-boot/custodians/u-boot-stm (2020-02-14
> >>>   07:31:47 -0500)
> >>>
> >>> are available in the Git repository at:
> >>>
> >>>   g...@gitlab.denx.de:u-boot/custodians/u-boot-dfu.git 
> >>
> >> This should be git:// or https:// URI here.
> >>
> >>> for you to fetch changes up to eb38f3703c5d0bc8a84db4d513ad220ce4f78f01:
> >>>
> >>>   gadget: f_thor: add missing line breaks for pr_err() (2020-02-21
> >>>   10:22:38 +0100)
> >>>
> >>> 
> >>> Andy Shevchenko (1):
> >>>   dfu: Reset timeout in case of DFU request
> >>>
> >>> Seung-Woo Kim (1):
> >>>   gadget: f_thor: add missing line breaks for pr_err()
> >>>
> >>>  drivers/usb/gadget/f_dfu.c  |  5 +
> >>>  drivers/usb/gadget/f_thor.c | 24 
> >>>  2 files changed, 17 insertions(+), 12 deletions(-)
> >>>
> >>>
> >>> BRANCH: master
> >>
> >> Can you please add this information -- which branch to pull -- to
> >> $subject ? It took me a long time to notice this important piece of
> >> information, because it was buried somewhere in a wall of text down below.
> >>
> >>> https://gitlab.denx.de/u-boot/custodians/u-boot-dfu/commits/master
> >>>
> >>> Merge tag info:
> >>> - Thor gadget pr_err() formatting change (adding trailing '\n' char)
> >>> - Reset timeout when new DFU request is received. This prevents from
> >>>   rebooting the device when in the middle of long update process.
> >>>
> >>> Marek, please regard those two patches as FIXES.
> > 
> > The general format is to say the URI and tag to pull (after the "are
> > available in the Git repository at:" line), and have that tag contain
> > the above info in it.
> 
> ... assuming the repo uses tags. But branch is fine by me and it works
> much faster for me if I see which branch I should pull right away.

And tags are strongly encouraged.  If it's not a tag but a branch the
branch should still be right there after the URI.  But please use tags
and provide a summary.  We've long had demand for "what changed in this
release?" and tags with summaries means that "git log --merges
TAG1..TAG2" produces useful information for that.  Thanks!

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH 0/3] Remove CONFIG_MMC_BROKEN_CD

[Tom Rini]

On Fri, Feb 21, 2020 at 04:21:52PM +0900, Jaehoon Chung wrote:
> On 2/21/20 10:07 AM, Jaehoon Chung wrote:
> > Hi Tom,
> > 
> > On 2/21/20 4:57 AM, Tom Rini wrote:
> >> On Thu, Feb 20, 2020 at 01:45:31PM +0900, Jaehoon Chung wrote:
> >>
> >>> CONFIG_MMC_BROKEN_CD needs not to define to Kconfig.
> >>> broken-cd is already provide to dt-property.
> >>> If want to poll card-detect, set to broken-cd instead of enabling 
> >>> CONFIG_MMC_BROKEN_CD.
> >>>
> >>> When checked the boards that is eabled CONFIG_MMC_BROKEN_CD,
> >>> it also used the value of dt as broken-cd.
> >>>
> >>> Jaehoon Chung (3):
> >>>   mmc: jz_mmc; add MMC_CAP_NEEDS_POLL by default
> >>>   mmc: check the flags of host_caps about broken-cd
> >>>   mmc: Kconfig: remove MMC_BROKEN_CD configuration
> >>>
> >>>  configs/brppt2_defconfig|  1 -
> >>>  configs/ci20_mmc_defconfig  |  1 -
> >>>  configs/meerkat96_defconfig |  1 -
> >>>  drivers/mmc/Kconfig |  5 -
> >>>  drivers/mmc/jz_mmc.c|  6 --
> >>>  drivers/mmc/mmc.c   | 10 +-
> >>>  6 files changed, 9 insertions(+), 15 deletions(-)
> >>
> >> Did you size-test this change?  ci20 is extremely tight on space.
> > 
> > I didn't check size-test about this. Is there any check-tool?
> > If there is check-tool, let me know, plz. Then i will check it. (In future, 
> > I will check before sending patch.)
> > I just checked this patch with CI.
> 
> u-boot.img  : 327573 -> 327625
> u-boot-spl.bin : 10336 -> 10464
> 
> What is ci20's limitation size?


The ci20 limit is  ((14 * 1024) - 0xa00) for SPL.  So we grow by 100
bytes.  And with that small of an SPL we're not using DT, so, is the
driver still acting correctly?  Can we restructure the change such that
growth is absolutely minimal?  Thanks!

-- 
Tom


signature.asc
Description: PGP signature

Re: [GIT] Pull request: u-boot-dfu (21.02.2020)

[Marek Vasut]

On 2/21/20 6:01 PM, Tom Rini wrote:
> On Fri, Feb 21, 2020 at 05:18:09PM +0100, Marek Vasut wrote:
>> On 2/21/20 3:54 PM, Lukasz Majewski wrote:
>>> Dear Marek,
>>
>> Hi,
>>
>>> The following changes since commit
>>> f2a73d6867ef973fbb8471cc87058205999b5e5c:
>>>
>>>   Merge tag 'u-boot-stm32-20200214' of
>>>   https://gitlab.denx.de/u-boot/custodians/u-boot-stm (2020-02-14
>>>   07:31:47 -0500)
>>>
>>> are available in the Git repository at:
>>>
>>>   g...@gitlab.denx.de:u-boot/custodians/u-boot-dfu.git 
>>
>> This should be git:// or https:// URI here.
>>
>>> for you to fetch changes up to eb38f3703c5d0bc8a84db4d513ad220ce4f78f01:
>>>
>>>   gadget: f_thor: add missing line breaks for pr_err() (2020-02-21
>>>   10:22:38 +0100)
>>>
>>> 
>>> Andy Shevchenko (1):
>>>   dfu: Reset timeout in case of DFU request
>>>
>>> Seung-Woo Kim (1):
>>>   gadget: f_thor: add missing line breaks for pr_err()
>>>
>>>  drivers/usb/gadget/f_dfu.c  |  5 +
>>>  drivers/usb/gadget/f_thor.c | 24 
>>>  2 files changed, 17 insertions(+), 12 deletions(-)
>>>
>>>
>>> BRANCH: master
>>
>> Can you please add this information -- which branch to pull -- to
>> $subject ? It took me a long time to notice this important piece of
>> information, because it was buried somewhere in a wall of text down below.
>>
>>> https://gitlab.denx.de/u-boot/custodians/u-boot-dfu/commits/master
>>>
>>> Merge tag info:
>>> - Thor gadget pr_err() formatting change (adding trailing '\n' char)
>>> - Reset timeout when new DFU request is received. This prevents from
>>>   rebooting the device when in the middle of long update process.
>>>
>>> Marek, please regard those two patches as FIXES.
> 
> The general format is to say the URI and tag to pull (after the "are
> available in the Git repository at:" line), and have that tag contain
> the above info in it.

... assuming the repo uses tags. But branch is fine by me and it works
much faster for me if I see which branch I should pull right away.

Re: [GIT] Pull request: u-boot-dfu (21.02.2020)

[Tom Rini]

On Fri, Feb 21, 2020 at 05:18:09PM +0100, Marek Vasut wrote:
> On 2/21/20 3:54 PM, Lukasz Majewski wrote:
> > Dear Marek,
> 
> Hi,
> 
> > The following changes since commit
> > f2a73d6867ef973fbb8471cc87058205999b5e5c:
> > 
> >   Merge tag 'u-boot-stm32-20200214' of
> >   https://gitlab.denx.de/u-boot/custodians/u-boot-stm (2020-02-14
> >   07:31:47 -0500)
> > 
> > are available in the Git repository at:
> > 
> >   g...@gitlab.denx.de:u-boot/custodians/u-boot-dfu.git 
> 
> This should be git:// or https:// URI here.
> 
> > for you to fetch changes up to eb38f3703c5d0bc8a84db4d513ad220ce4f78f01:
> > 
> >   gadget: f_thor: add missing line breaks for pr_err() (2020-02-21
> >   10:22:38 +0100)
> > 
> > 
> > Andy Shevchenko (1):
> >   dfu: Reset timeout in case of DFU request
> > 
> > Seung-Woo Kim (1):
> >   gadget: f_thor: add missing line breaks for pr_err()
> > 
> >  drivers/usb/gadget/f_dfu.c  |  5 +
> >  drivers/usb/gadget/f_thor.c | 24 
> >  2 files changed, 17 insertions(+), 12 deletions(-)
> > 
> > 
> > BRANCH: master
> 
> Can you please add this information -- which branch to pull -- to
> $subject ? It took me a long time to notice this important piece of
> information, because it was buried somewhere in a wall of text down below.
> 
> > https://gitlab.denx.de/u-boot/custodians/u-boot-dfu/commits/master
> > 
> > Merge tag info:
> > - Thor gadget pr_err() formatting change (adding trailing '\n' char)
> > - Reset timeout when new DFU request is received. This prevents from
> >   rebooting the device when in the middle of long update process.
> > 
> > Marek, please regard those two patches as FIXES.

The general format is to say the URI and tag to pull (after the "are
available in the Git repository at:" line), and have that tag contain
the above info in it.

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v2 10/21] arm: socfpga: Add secure register access helper functions for SoC 64bits

[Marek Vasut]

On 2/20/20 6:54 PM, Ang, Chee Hong wrote:
>> On 2/20/20 3:02 AM, Ang, Chee Hong wrote:
>> [...]
> +#if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_SPL_ATF)
> +u32 socfpga_secure_reg_read32(phys_addr_t reg_addr); void
> +socfpga_secure_reg_write32(u32 val, phys_addr_t reg_addr); void
> +socfpga_secure_reg_update32(phys_addr_t reg_addr, u32 mask, u32
> +val); #else
> +#define socfpga_secure_reg_read32readl
> +#define socfpga_secure_reg_write32   writel
> +#define socfpga_secure_reg_update32  clrsetbits_le32
> +#endif

 I think I don't understand how this is supposed to work. Would every
 place in U- Boot have to be patched to call these functions now ?
>>>
>>> Not every register access need this. Only those accessing registers in
>>> secure zone such as 'System Manager' registers need to call this. It's
>>> basically determine whether the driver should issue SMC/PSCI call if
>>> it's running in EL2 (non-secure) or access the registers directly by simply 
>>> using
>> readl/writel and etc if it's running in EL3 (secure).
>>> Accessing those registers in secure zone in non-secure mode (EL2) will cause
>> SError exception.
>>> So we can determine this behaviour in compile time:
>>> SPL always running in EL3. So it just simply fallback to use
>> readl/writel/clrsetbits_le32.
>>>
>>> For U-Boot proper (SSBL), there are 2 scenarios:
>>> 1) If CONFIG_SPL_ATF is defined, it means ATF is supported. It implies
>>> that U-Boot proper will be running in EL2 (non-secure), then it will use
>> SMC/PSCI calls to access the secure registers.
>>>
>>> 2) CONFIG_SPL_ATF is not defined, no ATF support. U-Boot proper will
>>> be running in EL3 which will fall back to simply using the direct access 
>>> functions
>> (readl/writel and etc).
>>
>> I would expect the standard IO accessors would or should handle this stuff ?
> Standard IO accessors are just general memory read/write functions designed 
> to be
> compatible with general hardware platforms. Not all platforms have 
> secure/non-secure
> hardware zones. I don't think they should handle this.
> 
> If it's running in EL3 (secure mode) the standard I/O accessors will work 
> just fine because
> EL3 can access to all secure/non-secure zones. In the header file, you can 
> see the secure
> I/O accessors will be replaced by the standard I/O accessors if it's built 
> for SPL and U-Boot
> proper without ATF. Because both are running in EL3 (secure).
> 
> If ATF is enabled, SPL will be still running in EL3 but U-Boot proper will be 
> running in
> EL2 (non-secure). If any code accessing those secure zones without going 
> through ATF
> (making SMC/PSCI calls to EL3), it will trigger 'SError' exceptions and crash 
> the U-Boot.

Hmmm, if U-Boot is running in EL2 (non-secure), why would it ever access
secure zones in the first place ?

And if that's really necessary, should the ATF really provide
secure-mode register access primitives or should it provide some more
high-level interface instead ?

Re: [GIT] Pull request: u-boot-dfu (21.02.2020)

[Marek Vasut]

On 2/21/20 3:54 PM, Lukasz Majewski wrote:
> Dear Marek,

Hi,

> The following changes since commit
> f2a73d6867ef973fbb8471cc87058205999b5e5c:
> 
>   Merge tag 'u-boot-stm32-20200214' of
>   https://gitlab.denx.de/u-boot/custodians/u-boot-stm (2020-02-14
>   07:31:47 -0500)
> 
> are available in the Git repository at:
> 
>   g...@gitlab.denx.de:u-boot/custodians/u-boot-dfu.git 

This should be git:// or https:// URI here.

> for you to fetch changes up to eb38f3703c5d0bc8a84db4d513ad220ce4f78f01:
> 
>   gadget: f_thor: add missing line breaks for pr_err() (2020-02-21
>   10:22:38 +0100)
> 
> 
> Andy Shevchenko (1):
>   dfu: Reset timeout in case of DFU request
> 
> Seung-Woo Kim (1):
>   gadget: f_thor: add missing line breaks for pr_err()
> 
>  drivers/usb/gadget/f_dfu.c  |  5 +
>  drivers/usb/gadget/f_thor.c | 24 
>  2 files changed, 17 insertions(+), 12 deletions(-)
> 
> 
> BRANCH: master

Can you please add this information -- which branch to pull -- to
$subject ? It took me a long time to notice this important piece of
information, because it was buried somewhere in a wall of text down below.

> https://gitlab.denx.de/u-boot/custodians/u-boot-dfu/commits/master
> 
> Merge tag info:
> - Thor gadget pr_err() formatting change (adding trailing '\n' char)
> - Reset timeout when new DFU request is received. This prevents from
>   rebooting the device when in the middle of long update process.
> 
> Marek, please regard those two patches as FIXES.
> 
> 
> Travis-CI:
> https://travis-ci.org/lmajewski/u-boot-dfu/builds/653370363

Applied, thanks.

-- 
Best regards,
Marek Vasut

Re: [PATCH v2 01/21] configs: agilex: Remove CONFIG_OF_EMBED

[Marek Vasut]

On 2/20/20 6:04 PM, Westergreen, Dalon wrote:

Please fix your mailer, it makes your reply completely unreadable.

> On Thu, 2020-02-20 at 17:44 +0100, Marek Vasut wrote:
> 
> On 2/20/20 3:12 AM, Ang, Chee Hong wrote:
> 
> On 2/19/20 1:25 PM,
> 
> 
> 
> chee.hong@intel.com
> 
>  wrote:
> 
> From: Chee Hong Ang <
> 
> 
> 
> chee.hong@intel.com
> 
>>
> 
> 
> CONFIG_OF_EMBED was primarily enabled to support the agilex spl hex
> 
> file requirements.  Since this option now produces a warning during
> 
> build, and the spl hex can be created using alternate methods,
> 
> CONFIG_OF_EMBED is no longer needed.

OK, so this patch removes functionality.
Can that functionality be retained ? Or what can be done here?

[...]

Re: [PATCH v2 1/9] usb: dwc3-of-simple: Drop redundant inclding header file

[Marek Vasut]

On 2/20/20 3:32 AM, Kever Yang wrote:
> The fdtdec.h is no use in this file, remove the include code.
> 
> Signed-off-by: Kever Yang 
> ---
> 
> Changes in v2: None
> 
>  drivers/usb/host/dwc3-of-simple.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/usb/host/dwc3-of-simple.c 
> b/drivers/usb/host/dwc3-of-simple.c
> index 45df614b09..e4abc6f3b9 100644
> --- a/drivers/usb/host/dwc3-of-simple.c
> +++ b/drivers/usb/host/dwc3-of-simple.c
> @@ -12,7 +12,6 @@
>  
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  

Nit: you have a typo in 'inclding' in the subject.

Re: [PATCH v2 05/21] arm: socfpga: Override 'lowlevel_init' to support ATF

[Marek Vasut]

On 2/20/20 8:05 PM, Ang, Chee Hong wrote:
>> On 2/20/20 3:27 AM, Ang, Chee Hong wrote:
 On 2/19/20 1:25 PM, chee.hong@intel.com wrote:
 [...]
> diff --git a/arch/arm/mach-socfpga/lowlevel_init.S
> b/arch/arm/mach-socfpga/lowlevel_init.S
> new file mode 100644
> index 000..68053a0
> --- /dev/null
> +++ b/arch/arm/mach-socfpga/lowlevel_init.S

 This should be some lowlevel_init_64.S to make it clear it's only for
 arm64 platforms.
>>> OK. It makes sense. Thanks.

> @@ -0,0 +1,85 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2019, Intel Corporation  */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +ENTRY(lowlevel_init)
> + mov x29, lr /* Save LR */
> +
> +#if defined(CONFIG_GICV2) || defined(CONFIG_GICV3) #ifdef
> +CONFIG_SPL_ATF
> + branch_if_slave x0, 2f
> +#else
> + branch_if_slave x0, 1f
> +#endif
> +
> + ldr x0, =GICD_BASE
> + bl  gic_init_secure
> +#ifdef CONFIG_SPL_BUILD
> + b   2f
> +#else
> + b   3f
> +#endif

 Can't this be done in C code ? Can we reduce the ifdeffery ?
>>> This lowlevel_init function is shared by SPL and U-Boot and they run
>>> in slightly different flow.
>>
>> What does this 'different flow' mean ?
> This has something to with multi-cores CPU such as A53.
> For SPL, we need to make sure the slave CPUs (CPU1/2/3) trapped in a 'place'
> Where they could be 'activated' by kernel for multi-processor environment.
> It means the kernel get to 'activate' the slave CPUs from master CPU (CPU0)
>  U-Boot proper only run on master CPU (CPU0). The rest of slave CPUs
> are trapped in the beginning of SPL waiting to be 'activated'
> by kernel.

OK, so the secondary CPUs are spinning until the kernel releases them.

> In U-Boot proper, only master CPU gets to run this code and it will just
> do the basic GIC setup and skip the 'trap'. The 'trap' is to prevent the slave
> CPUs from running the same SPL, ATF and U-Boot code as the master CPU in
> parallel. Only single core (maser CPU) is needed for bootloaders and firmware.

I would expect all the other SMP platforms solved this issue with
secondary CPUs already, so why is agilex special ?

Re: [PATCH v2 5/9] usb: dwc3-generic: Use dev_read_addr() instead of devfdt_get_addr()

[Marek Vasut]

On 2/20/20 3:32 AM, Kever Yang wrote:
> Use more common API so that we can support live DT.
> 
> Signed-off-by: Kever Yang 
> ---
> 
> Changes in v2: None
> 
>  drivers/usb/dwc3/dwc3-generic.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c
> index 820cc7075c..ced0f53bfd 100644
> --- a/drivers/usb/dwc3/dwc3-generic.c
> +++ b/drivers/usb/dwc3/dwc3-generic.c
> @@ -88,17 +88,17 @@ static int dwc3_generic_remove(struct udevice *dev,
>  static int dwc3_generic_ofdata_to_platdata(struct udevice *dev)
>  {
>   struct dwc3_generic_plat *plat = dev_get_platdata(dev);
> - int node = dev_of_offset(dev);
> + ofnode node = dev->node;
>  
> - plat->base = devfdt_get_addr(dev);
> + plat->base = dev_read_addr(dev);
>  
> - plat->maximum_speed = usb_get_maximum_speed(node);
> + plat->maximum_speed = usb_get_maximum_speed(ofnode_to_offset(node));
>   if (plat->maximum_speed == USB_SPEED_UNKNOWN) {
>   pr_info("No USB maximum speed specified. Using super speed\n");
>   plat->maximum_speed = USB_SPEED_SUPER;
>   }
>  
> - plat->dr_mode = usb_get_dr_mode(node);
> + plat->dr_mode = usb_get_dr_mode(ofnode_to_offset(node));

I'm really not fond of this duplication, just add a variable and remove
it in a subsequent patch if needed.

Re: [PATCH] net: phy: Fix overlong PHY timeout

[Matthias Brugger]

Hi Joe,

On 30/01/2020 12:00, Matthias Brugger wrote:
> 
> 
> On 03/01/2020 23:08, Andre Przywara wrote:
>> Commit 27c3f70f3b50 ("net: phy: Increase link up delay in
>> genphy_update_link()") increased the per-iteration waiting time from
>> 1ms to 50ms, without adjusting the timeout counter. This lead to the
>> timeout increasing from the typical 4 seconds to over three minutes.
>>
>> Adjust the timeout counter evaluation by that factor of 50 to bring the
>> timeout back to the intended value.
>>
>> Signed-off-by: Andre Przywara 
> 
> I tested this on RPi4 with the genet patches on top. Now the timeout is
> reasonable :)
> 
> Tested-by: Matthias Brugger 
> 

Friedly reminder, are you planning to take this fix for v2020.04?

Regards,
Matthias

>> ---
>>  drivers/net/phy/phy.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
>> index 80a7664e49..5cf9c165b6 100644
>> --- a/drivers/net/phy/phy.c
>> +++ b/drivers/net/phy/phy.c
>> @@ -244,7 +244,7 @@ int genphy_update_link(struct phy_device *phydev)
>>  /*
>>   * Timeout reached ?
>>   */
>> -if (i > PHY_ANEG_TIMEOUT) {
>> +if (i > (PHY_ANEG_TIMEOUT / 50)) {
>>  printf(" TIMEOUT !\n");
>>  phydev->link = 0;
>>  return -ETIMEDOUT;
>>

Re: [PATCH 3/5] env/fat.c: remove private CMD_SAVEENV logic

[Tom Rini]

On Fri, Feb 21, 2020 at 05:14:14PM +0100, Wolfgang Denk wrote:
> Dear Rasmus,
> 
> In message <5265fdd5-3992-4e5f-3235-5586b3b77...@prevas.dk> you wrote:
> >
> > So without the fat.c patch, CONFIG_SPL_SAVEENV is effectively ignored.
> 
> OK, but what about bords that don't store the envionment in a file
> system, but instead for example in (parallel or SPI) NOR flash or in
> a UBI volume?

I think the intent is that there is no change today but the door is now
open for someone that can test / confirm changes there to do so.

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH 3/5] env/fat.c: remove private CMD_SAVEENV logic

[Wolfgang Denk]

Dear Rasmus,

In message <5265fdd5-3992-4e5f-3235-5586b3b77...@prevas.dk> you wrote:
>
> So without the fat.c patch, CONFIG_SPL_SAVEENV is effectively ignored.

OK, but what about bords that don't store the envionment in a file
system, but instead for example in (parallel or SPI) NOR flash or in
a UBI volume?


> Actually, the situation for ext4 is even worse than indicated.
...
> Anyway, a side-effect of my ext4 patch is that the above combination
> actually builds, because env_ext4_save is not linked in, so hexport_r
> isn't needed. And turning on SPL_SAVEENV also works as expected.

OK. Thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
When you die, the first thing you lose is your life. The  next  thing
is the illusions.   - Terry Pratchett, _Pyramids_

[GIT] Pull request: u-boot-dfu (21.02.2020)

[Lukasz Majewski]

Dear Marek,

The following changes since commit
f2a73d6867ef973fbb8471cc87058205999b5e5c:

  Merge tag 'u-boot-stm32-20200214' of
  https://gitlab.denx.de/u-boot/custodians/u-boot-stm (2020-02-14
  07:31:47 -0500)

are available in the Git repository at:

  g...@gitlab.denx.de:u-boot/custodians/u-boot-dfu.git 

for you to fetch changes up to eb38f3703c5d0bc8a84db4d513ad220ce4f78f01:

  gadget: f_thor: add missing line breaks for pr_err() (2020-02-21
  10:22:38 +0100)


Andy Shevchenko (1):
  dfu: Reset timeout in case of DFU request

Seung-Woo Kim (1):
  gadget: f_thor: add missing line breaks for pr_err()

 drivers/usb/gadget/f_dfu.c  |  5 +
 drivers/usb/gadget/f_thor.c | 24 
 2 files changed, 17 insertions(+), 12 deletions(-)


BRANCH: master
https://gitlab.denx.de/u-boot/custodians/u-boot-dfu/commits/master

Merge tag info:
- Thor gadget pr_err() formatting change (adding trailing '\n' char)
- Reset timeout when new DFU request is received. This prevents from
  rebooting the device when in the middle of long update process.

Marek, please regard those two patches as FIXES.


Travis-CI:
https://travis-ci.org/lmajewski/u-boot-dfu/builds/653370363


Best regards,

Lukasz Majewski

--

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


pgpRmUrhB8aQ4.pgp
Description: OpenPGP digital signature

Re: [PATCH 1/1] Dockerfile: add imagemagick

[Tom Rini]

On Fri, Feb 21, 2020 at 06:14:44AM +0100, Heinrich Schuchardt wrote:

> In reStructured text documentation we sometimes find formatting problems.
> So we should add 'make htmldocs' to our Gitlab Ci steps and let all
> warning result in a build failure.
> 
> For building the htmldocs target without warnings we need the ImageMagick
> package.
> 
> Reviewed-by: Bin Meng 
> Signed-off-by: Heinrich Schuchardt 

Applied to gitlab-ci-runner/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature

RE: [PATCH v2 1/8] phy: generic: add error trace to detect PHY issue in uclass

[Patrick DELAUNAY]

Hi Marek,

> From: Marek Vasut 
> Sent: mardi 18 février 2020 18:40
> 
> On 2/18/20 9:38 AM, Patrick Delaunay wrote:
> [...]
> >  static inline struct phy_ops *phy_dev_ops(struct udevice *dev)  { @@
> > -109,56 +110,86 @@ int generic_phy_get_by_name(struct udevice *dev,
> > const char *phy_name,  int generic_phy_init(struct phy *phy)  {
> > struct phy_ops const *ops;
> > +   int ret;
> >
> > if (!phy)
> > return 0;
> > ops = phy_dev_ops(phy->dev);
> >
> > -   return ops->init ? ops->init(phy) : 0;
> > +   ret = ops->init ? ops->init(phy) : 0;
> 
> if (!ops->init)
>return 0;
> ret = ops->init();
> if (ret)
>dev_err...
> 
> return ret;
> 
> Please fix globally.

Yes it is more clear, I am the v3 serie
 
> > +   if (ret)
> > +   dev_err(phy->dev, "PHY: Failed to init %s: %d.\n",
> > +   phy->dev->name, ret);
> > +
> > +   return ret;
> [...]

Regards,

Patrick

[PATCH v2] env: ti: boot: Fix Android boot on AM57x EVM

[Sam Protsenko]

When applying DTBO on top of DTB (with "fdt apply" command) on AM57x EVM
board, there is not enough memory reserved in RAM for DTB blob. Hence,
DTBO can't be merged in DTB. It leads to inability to boot Android with
next error message:

failed on fdt_overlay_apply(): FDT_ERR_NOSPACE

To overcome that issue let's provide 512 KiB of space to keep DTB and
all merged DTBO blobs. To do so, "length" parameter should be specified
for "fdt addr" command:

=> fdt addr $fdtaddr 0x8

512 KiB is the maximum size we can use for this, because next address
after $fdtaddr is 512 KiB ahead of it:

fdtaddr=0x8800
rdaddr=0x8808

Also add size variables to 'adtimg' command invocations, to avoid
cluttering the console with DTBO blob sizes.

Signed-off-by: Sam Protsenko 
---
Changes in v2:
  - Reserve 512 KiB for DTB area, not 1 MiB

 include/environment/ti/boot.h | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/environment/ti/boot.h b/include/environment/ti/boot.h
index 523c8fc4fe..11a57af0a4 100644
--- a/include/environment/ti/boot.h
+++ b/include/environment/ti/boot.h
@@ -103,18 +103,18 @@
"echo \"  Reading DTB for AM57x EVM RevA3...\"; " \
"abootimg get dtb --index=0 dtb_start dtb_size; " \
"cp.b $dtb_start $fdtaddr $dtb_size; " \
-   "fdt addr $fdtaddr; " \
+   "fdt addr $fdtaddr 0x8; " \
"echo \"  Applying DTBOs for AM57x EVM RevA3...\"; " \
"adtimg addr $dtboaddr; " \
-   "adtimg get dt --index=0 dtbo0_addr; " \
+   "adtimg get dt --index=0 dtbo0_addr dtbo0_size; " \
"fdt apply $dtbo0_addr; " \
-   "adtimg get dt --index=1 dtbo1_addr; " \
+   "adtimg get dt --index=1 dtbo1_addr dtbo1_size; " \
"fdt apply $dtbo1_addr; " \
"elif test $board_name = beagle_x15_revc; then " \
"echo \"  Reading DTB for Beagle X15 RevC...\"; " \
"abootimg get dtb --index=0 dtb_start dtb_size; " \
"cp.b $dtb_start $fdtaddr $dtb_size; " \
-   "fdt addr $fdtaddr; " \
+   "fdt addr $fdtaddr 0x8; " \
"else " \
"echo Error: Android boot is not supported for $board_name; " \
"exit; " \
-- 
2.24.1

Re: [PATCH 1/1] azure: build HTML documentation

[Bin Meng]

On Fri, Feb 21, 2020 at 1:33 PM Heinrich Schuchardt  wrote:
>
> Several patches delivered incorrect restructured text as documentation. We
> should be able to discover this in Azure CI.
>
> Provide a build step for 'make htmldocs'.
>
> Signed-off-by: Heinrich Schuchardt 
> ---
> Hello Tom,
>
> I have no Azure account to check if this actually works.
>
> The patch depends on:
>
> [PATCH 1/1] Dockerfile: add imagemagick
> https://lists.denx.de/pipermail/u-boot/2020-February/400983.html
>
> Best regards
>
> Heinrich
> ---
>  .azure-pipelines.yml | 10 ++
>  1 file changed, 10 insertions(+)
>

Reviewed-by: Bin Meng 

I think you can group all CI changes in one series.

Regards,
Bin

Re: [PATCH 2/2] travis: build HTML docs

[Bin Meng]

On Fri, Feb 21, 2020 at 11:27 AM Heinrich Schuchardt  wrote:
>
> Several patches delivered incorrect restructured text as documentation.
> We should be able to discover this in Travis CI.
>
> Provide a build step for 'make htmldocs'.
>
> Add required package graphviz.
>
> Signed-off-by: Heinrich Schuchardt 
> ---
>  .travis.yml | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/.travis.yml b/.travis.yml
> index e6db9d6a72..a5918dd5d9 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -38,6 +38,7 @@ addons:
>  - libisl15
>  - clang-7
>  - srecord
> +- graphviz
>
>  install:
>   # Clone uboot-test-hooks
> @@ -360,6 +361,10 @@ matrix:
>  - name: "grep FIXME HACK"
>script:
>  - grep -r FIXME .
> +# build HTML documentation
> +- name: "htmldocs"
> +  script:
> +- make htmldocs

Could you insert this after cppcheck, to align with what you did in
gitlab and azure?

>  # search for HACK within source tree and ignore HACKKIT board
>script:
>  - grep -r HACK . | grep -v HACKKIT
> --


Regards,
Bin

Re: [PATCH 1/1] gitlab: build HTML documentation

[Bin Meng]

On Fri, Feb 21, 2020 at 1:20 PM Heinrich Schuchardt  wrote:
>
> Several patches delivered incorrect restructured text as documentation. We
> should be able to discover this in Gitlab CI.
>
> Provide a build step for 'make htmldocs'.
>
> Signed-off-by: Heinrich Schuchardt 
> ---
> This patch requires a new Docker image. Cf.
>
> [PATCH 1/1] Dockerfile: add imagemagick
> https://lists.denx.de/pipermail/u-boot/2020-February/400983.html
>
> If have tested successfully against Docker image
> xypron/u-boot-gitlab-ci-runner:2020-02-20:
> https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/jobs/60271
> ---
>  .gitlab-ci.yml | 7 +++
>  1 file changed, 7 insertions(+)
>

Reviewed-by: Bin Meng 

Re: [PATCH 1/1] Dockerfile: add imagemagick

[Bin Meng]

On Fri, Feb 21, 2020 at 1:15 PM Heinrich Schuchardt  wrote:
>
> In reStructured text documentation we sometimes find formatting problems.
> So we should add 'make htmldocs' to our Gitlab Ci steps and let all
> warning result in a build failure.
>
> For building the htmldocs target without warnings we need the ImageMagick
> package.
>
> Signed-off-by: Heinrich Schuchardt 
> ---
>  Dockerfile | 1 +
>  1 file changed, 1 insertion(+)
>

Reviewed-by: Bin Meng 

Re: [PATCH 1/1] .dockerignore: remove .git/ from build context

[Bin Meng]

On Fri, Feb 21, 2020 at 12:14 PM Heinrich Schuchardt  wrote:
>
> We do not need .git in the build context.
>
> Signed-off-by: Heinrich Schuchardt 
> ---
>  .dockerignore | 2 ++
>  1 file changed, 2 insertions(+)
>  create mode 100644 .dockerignore
>

Reviewed-by: Bin Meng 

Re: [PATCH 2/2] travis: build HTML docs

[Bin Meng]

On Fri, Feb 21, 2020 at 11:27 AM Heinrich Schuchardt  wrote:
>
> Several patches delivered incorrect restructured text as documentation.
> We should be able to discover this in Travis CI.
>
> Provide a build step for 'make htmldocs'.
>
> Add required package graphviz.
>
> Signed-off-by: Heinrich Schuchardt 
> ---
>  .travis.yml | 5 +
>  1 file changed, 5 insertions(+)
>

Reviewed-by: Bin Meng 

Re: [PATCH 1/2] doc/Makefile: turn warnings into errors

[Bin Meng]

On Fri, Feb 21, 2020 at 11:27 AM Heinrich Schuchardt  wrote:
>
> Several patches delivered incorrect restructured text as documentation. We
> should be able to discover this in Travis CI or Gitlab CI.

should mention Azure as well.

>
> So let us turn all build warnings into errors.
>
> We cannot add --keep-going as Gitlab does not use sphinx-build version 1.8
> or later yet.
>
> Signed-off-by: Heinrich Schuchardt 
> ---
>  doc/Makefile | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/doc/Makefile b/doc/Makefile
> index 5135a96e88..0e0da5666f 100644
> --- a/doc/Makefile
> +++ b/doc/Makefile
> @@ -56,6 +56,7 @@ quiet_cmd_sphinx = SPHINX  $@ --> file://$(abspath 
> $(BUILDDIR)/$3/$4)
> PYTHONDONTWRITEBYTECODE=1 \
> BUILDDIR=$(abspath $(BUILDDIR)) SPHINX_CONF=$(abspath 
> $(srctree)/$(src)/$5/$(SPHINX_CONF)) \
> $(SPHINXBUILD) \
> +   -W \
> -b $2 \
> -c $(abspath $(srctree)/$(src)) \
> -d $(abspath $(BUILDDIR)/.doctrees/$3) \
> --


Regards,
Bin

Re: [PATCH 0/3] Remove CONFIG_MMC_BROKEN_CD

[Tom Rini]

On Fri, Feb 21, 2020 at 10:07:47AM +0900, Jaehoon Chung wrote:
> Hi Tom,
> 
> On 2/21/20 4:57 AM, Tom Rini wrote:
> > On Thu, Feb 20, 2020 at 01:45:31PM +0900, Jaehoon Chung wrote:
> > 
> >> CONFIG_MMC_BROKEN_CD needs not to define to Kconfig.
> >> broken-cd is already provide to dt-property.
> >> If want to poll card-detect, set to broken-cd instead of enabling 
> >> CONFIG_MMC_BROKEN_CD.
> >>
> >> When checked the boards that is eabled CONFIG_MMC_BROKEN_CD,
> >> it also used the value of dt as broken-cd.
> >>
> >> Jaehoon Chung (3):
> >>   mmc: jz_mmc; add MMC_CAP_NEEDS_POLL by default
> >>   mmc: check the flags of host_caps about broken-cd
> >>   mmc: Kconfig: remove MMC_BROKEN_CD configuration
> >>
> >>  configs/brppt2_defconfig|  1 -
> >>  configs/ci20_mmc_defconfig  |  1 -
> >>  configs/meerkat96_defconfig |  1 -
> >>  drivers/mmc/Kconfig |  5 -
> >>  drivers/mmc/jz_mmc.c|  6 --
> >>  drivers/mmc/mmc.c   | 10 +-
> >>  6 files changed, 9 insertions(+), 15 deletions(-)
> > 
> > Did you size-test this change?  ci20 is extremely tight on space.
> 
> I didn't check size-test about this. Is there any check-tool?

Yes, buildman has a few different size related options.  I use a wrapper
like this:

#!/bin/bash

# Initial and constant buildman args
ARGS="-devl"
ALL=0
KEEP=0

# Find our arguments
while test $# -ne 0; do
if [ "$1" == "--all" ]; then
ALL=1
shift 1
elif [ "$1" == "--branch" ]; then
BRANCH=$2
shift 2
elif [ "$1" == "--keep" ]; then
KEEP=1
ARGS="$ARGS -k"
shift 1
else
MACHINE=$1
shift
fi
done

if [ -z $MACHINE ]; then
echo Usage: $0 MACHINE [--all] [--keep] [--branch BRANCH]
exit 1
fi

# If not all, then only first/last
if [ $ALL -ne 1 ]; then
ARGS="$ARGS --step 0"
fi

if [ ! -z $BRANCH ]; then
ARGS="$ARGS -b $BRANCH"
else
ARGS="$ARGS -b `git rev-parse --abbrev-ref HEAD`"
fi

mkdir -p /tmp/$MACHINE

export SOURCE_DATE_EPOCH=`date +%s`
./tools/buildman/buildman -o /tmp/$MACHINE $ARGS -SBC $MACHINE
./tools/buildman/buildman -o /tmp/$MACHINE $ARGS -SsB $MACHINE

[ $KEEP -eq 0 ] && rm -rf /tmp/$MACHINE

This will either build the first/last commit in a series (do things
change at all?) or every commit (What commit introduced the growth I
want to know more about).  I can also tell it to keep the resulting
output directory if I want to dig around the map files more by hand.

-- 
Tom


signature.asc
Description: PGP signature

[PATCH 0/3] mediatek: pwm: add pwm driver for MediaTek SoCs

[Sam Shih]

Add pwm support for MediaTek SoCs: mt7622, mt7623 and mt7629,
also fix mt7622 and mt7623 boot problem caused by BINMAN_FDT 

Sam Shih (3):
  mediatek: pwm: add pwm driver for MediaTek SoCs
  arm: dts: add pwm support for MediaTek SoCs
  arm: mediatek: remove unuse binman config

 arch/arm/Kconfig   |   1 -
 arch/arm/dts/mt7622.dtsi   |  19 
 arch/arm/dts/mt7623.dtsi   |  17 +++
 arch/arm/dts/mt7629.dtsi   |  16 +++
 arch/arm/mach-mediatek/Kconfig |   1 +
 drivers/pwm/Kconfig|   7 ++
 drivers/pwm/Makefile   |   1 +
 drivers/pwm/pwm-mtk.c  | 188 +
 8 files changed, 249 insertions(+), 1 deletion(-)
 create mode 100644 drivers/pwm/pwm-mtk.c

-- 
2.17.1

[PATCH 1/3] mediatek: pwm: add pwm driver for MediaTek SoCs

[Sam Shih]

This driver support the standard PWM API for
MediaTek MT7623, MT7622 and MT7629 SoCs

Signed-off-by: Sam Shih 
---
 drivers/pwm/Kconfig   |   7 ++
 drivers/pwm/Makefile  |   1 +
 drivers/pwm/pwm-mtk.c | 188 ++
 3 files changed, 196 insertions(+)
 create mode 100644 drivers/pwm/pwm-mtk.c

diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 1f36fc78fa..edb3f0f538 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -23,6 +23,13 @@ config PWM_IMX
help
  This PWM is found i.MX27 and later i.MX SoCs.
 
+config PWM_MTK
+   bool "Enable support for MediaTek PWM"
+   depends on DM_PWM
+   help
+ This PWM is found on MT7622, MT7623, and MT7629. It supports a
+ programmable period and duty cycle.
+
 config PWM_ROCKCHIP
bool "Enable support for the Rockchip PWM"
depends on DM_PWM
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index a837c35ed2..2c3a069006 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_DM_PWM)  += pwm-uclass.o
 
 obj-$(CONFIG_PWM_EXYNOS)   += exynos_pwm.o
 obj-$(CONFIG_PWM_IMX)  += pwm-imx.o pwm-imx-util.o
+obj-$(CONFIG_PWM_MTK)  += pwm-mtk.o
 obj-$(CONFIG_PWM_ROCKCHIP) += rk_pwm.o
 obj-$(CONFIG_PWM_SANDBOX)  += sandbox_pwm.o
 obj-$(CONFIG_PWM_TEGRA)+= tegra_pwm.o
diff --git a/drivers/pwm/pwm-mtk.c b/drivers/pwm/pwm-mtk.c
new file mode 100644
index 00..97ed477025
--- /dev/null
+++ b/drivers/pwm/pwm-mtk.c
@@ -0,0 +1,188 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2020 MediaTek Inc. All Rights Reserved.
+ *
+ * Author: Sam Shih 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* PWM registers and bits definitions */
+#define PWMCON 0x00
+#define PWMHDUR0x04
+#define PWMLDUR0x08
+#define PWMGDUR0x0c
+#define PWMWAVENUM 0x28
+#define PWMDWIDTH  0x2c
+#define PWM45DWIDTH_FIXUP  0x30
+#define PWMTHRES   0x30
+#define PWM45THRES_FIXUP   0x34
+
+#define PWM_CLK_DIV_MAX7
+#define MAX_PWM_NUM8
+
+#define NSEC_PER_SEC 10L
+
+static const unsigned int mtk_pwm_reg_offset[] = {
+   0x0010, 0x0050, 0x0090, 0x00d0, 0x0110, 0x0150, 0x0190, 0x0220
+};
+
+struct mtk_pwm_soc {
+   unsigned int num_pwms;
+   bool pwm45_fixup;
+};
+
+struct mtk_pwm_priv {
+   void __iomem *base;
+   struct clk top_clk;
+   struct clk main_clk;
+   struct clk pwm_clks[MAX_PWM_NUM];
+   const struct mtk_pwm_soc *soc;
+};
+
+static void mtk_pwm_w32(struct udevice *dev, uint channel, uint reg, uint val)
+{
+   struct mtk_pwm_priv *priv = dev_get_priv(dev);
+   u32 offset = mtk_pwm_reg_offset[channel];
+
+   writel(val, priv->base + offset + reg);
+}
+
+static int mtk_pwm_set_config(struct udevice *dev, uint channel,
+ uint period_ns, uint duty_ns)
+{
+   struct mtk_pwm_priv *priv = dev_get_priv(dev);
+   u32 clkdiv = 0, clksel = 0, cnt_period, cnt_duty,
+   reg_width = PWMDWIDTH, reg_thres = PWMTHRES;
+   u64 resolution;
+   int ret = 0;
+
+   clk_enable(>top_clk);
+   clk_enable(>main_clk);
+   /* Using resolution in picosecond gets accuracy higher */
+   resolution = (u64)NSEC_PER_SEC * 1000;
+   do_div(resolution, clk_get_rate(>pwm_clks[channel]));
+   cnt_period = DIV_ROUND_CLOSEST_ULL((u64)period_ns * 1000, resolution);
+   while (cnt_period > 8191) {
+   resolution *= 2;
+   clkdiv++;
+   cnt_period = DIV_ROUND_CLOSEST_ULL((u64)period_ns * 1000,
+  resolution);
+   if (clkdiv > PWM_CLK_DIV_MAX && clksel == 0) {
+   clksel = 1;
+   clkdiv = 0;
+   resolution = (u64)NSEC_PER_SEC * 1000 * 1625;
+   do_div(resolution,
+  clk_get_rate(>pwm_clks[channel]));
+   cnt_period = DIV_ROUND_CLOSEST_ULL(
+   (u64)period_ns * 1000, resolution);
+   clk_enable(>pwm_clks[channel]);
+   }
+   }
+   if (clkdiv > PWM_CLK_DIV_MAX && clksel == 1) {
+   printf("pwm period %u not supported\n", period_ns);
+   return -EINVAL;
+   }
+   if (priv->soc->pwm45_fixup && channel > 2) {
+   /*
+* PWM[4,5] has distinct offset for PWMDWIDTH and PWMTHRES
+* from the other PWMs on MT7623.
+*/
+   reg_width = PWM45DWIDTH_FIXUP;
+   reg_thres = PWM45THRES_FIXUP;
+   }
+   cnt_duty = DIV_ROUND_CLOSEST_ULL((u64)duty_ns * 1000, resolution);
+   if (clksel == 1)
+   

[PATCH 2/3] arm: dts: add pwm support for MediaTek SoCs

[Sam Shih]

This patch add pwm support for mt7622, mt7623 and mt7629 SoCs

Signed-off-by: Sam Shih 
---
 arch/arm/dts/mt7622.dtsi | 19 +++
 arch/arm/dts/mt7623.dtsi | 17 +
 arch/arm/dts/mt7629.dtsi | 16 
 3 files changed, 52 insertions(+)

diff --git a/arch/arm/dts/mt7622.dtsi b/arch/arm/dts/mt7622.dtsi
index 1e8ec9b48b..f9ce0c6c3e 100644
--- a/arch/arm/dts/mt7622.dtsi
+++ b/arch/arm/dts/mt7622.dtsi
@@ -227,4 +227,23 @@
#clock-cells = <1>;
};
 
+   pwm: pwm@11006000 {
+   compatible = "mediatek,mt7622-pwm";
+   reg = <0x11006000 0x1000>;
+   #clock-cells = <1>;
+   #pwm-cells = <2>;
+   interrupts = ;
+   clocks = < CLK_TOP_PWM_SEL>,
+< CLK_PERI_PWM_PD>,
+< CLK_PERI_PWM1_PD>,
+< CLK_PERI_PWM2_PD>,
+< CLK_PERI_PWM3_PD>,
+< CLK_PERI_PWM4_PD>,
+< CLK_PERI_PWM5_PD>,
+< CLK_PERI_PWM6_PD>;
+   clock-names = "top", "main", "pwm1", "pwm2", "pwm3", "pwm4",
+ "pwm5", "pwm6";
+   status = "disabled";
+   };
+
 };
diff --git a/arch/arm/dts/mt7623.dtsi b/arch/arm/dts/mt7623.dtsi
index 1f45dea575..0452889ef8 100644
--- a/arch/arm/dts/mt7623.dtsi
+++ b/arch/arm/dts/mt7623.dtsi
@@ -400,4 +400,21 @@
mediatek,ethsys = <>;
status = "disabled";
};
+
+   pwm: pwm@11006000 {
+   compatible = "mediatek,mt7623-pwm";
+   reg = <0x11006000 0x1000>;
+   #clock-cells = <1>;
+   #pwm-cells = <2>;
+   clocks = < CLK_TOP_PWM_SEL>,
+< CLK_PERI_PWM>,
+< CLK_PERI_PWM1>,
+< CLK_PERI_PWM2>,
+< CLK_PERI_PWM3>,
+< CLK_PERI_PWM4>,
+< CLK_PERI_PWM5>;
+   clock-names = "top", "main", "pwm1", "pwm2", "pwm3", "pwm4",
+ "pwm5";
+   status = "disabled";
+   };
 };
diff --git a/arch/arm/dts/mt7629.dtsi b/arch/arm/dts/mt7629.dtsi
index a33a74a556..644d2da4a8 100644
--- a/arch/arm/dts/mt7629.dtsi
+++ b/arch/arm/dts/mt7629.dtsi
@@ -281,4 +281,20 @@
reg = <0x1b13 0x1000>;
#clock-cells = <1>;
};
+
+   pwm: pwm@11006000 {
+   compatible = "mediatek,mt7629-pwm";
+   reg = <0x11006000 0x1000>;
+   #clock-cells = <1>;
+   #pwm-cells = <2>;
+   interrupts = ;
+   clocks = < CLK_TOP_PWM_SEL>,
+< CLK_PERI_PWM_PD>,
+< CLK_PERI_PWM1_PD>;
+   clock-names = "top", "main", "pwm1";
+   assigned-clocks = < CLK_TOP_PWM_SEL>;
+   assigned-clock-parents = < CLK_TOP_UNIVPLL2_D4>;
+   status = "disabled";
+   };
+
 };
-- 
2.17.1

[PATCH 3/3] arm: mediatek: remove unuse binman config

[Sam Shih]

The binman-option BINMAN_FDT is introduced by this commit:
commit 3c10dc95bdd0 ("binman: Add a library to access binman entries")
BINMAN_FDT being selected when BINMAN=y that resulting in mt7623
and mt7622 are unable to boot. The root cause of this issue is commit:
commit cbd2fba1eca1 ("arm: MediaTek: add basic support for MT7629 boards")
select BINMAN=y in all mediatek SoCs, and others mediatek SoCs not
expect to use BINMAN_FDT.
This patch remove BINMAN=y option when ARCH_MEDIATEK=y and
move this to the specify SoCs part config.

Signed-off-by: Sam Shih 
---
 arch/arm/Kconfig   | 1 -
 arch/arm/mach-mediatek/Kconfig | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 8d9f7fcce7..9ca3d90aa2 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -778,7 +778,6 @@ config ARCH_MESON
 
 config ARCH_MEDIATEK
bool "MediaTek SoCs"
-   select BINMAN
select DM
select OF_CONTROL
select SPL_DM if SPL
diff --git a/arch/arm/mach-mediatek/Kconfig b/arch/arm/mach-mediatek/Kconfig
index 17b84db5a8..0042e57017 100644
--- a/arch/arm/mach-mediatek/Kconfig
+++ b/arch/arm/mach-mediatek/Kconfig
@@ -36,6 +36,7 @@ config TARGET_MT7629
bool "MediaTek MT7629 SoC"
select CPU_V7A
select SPL
+   select BINMAN
help
  The MediaTek MT7629 is a ARM-based SoC with a dual-core Cortex-A7
  including DDR3, crypto engine, 3x3 11n/ac Wi-Fi, Gigabit Ethernet,
-- 
2.17.1

Re: [PATCH] net: bcmgenet: Don't set ID_MODE_DIS when not using RGMII

[Nicolas Saenz Julienne]

Florian,

On Fri, 2020-02-21 at 11:54 +0100, Nicolas Saenz Julienne wrote:
> Hi Matthias,
> 
> On Thu, 2020-02-20 at 19:58 +0100, Matthias Brugger wrote:
> > On 20/02/2020 17:36, Nicolas Saenz Julienne wrote:
> > > As per Linux's driver, ID_MODE_DIS is only set when the PHY interface is
> > > RGMII. Don't enable it for the rest of setups.
> > > 
> > > This has been seen to misconfigure RPi4's PHY when booting Linux.
> > > 
> > > Signed-off-by: Nicolas Saenz Julienne 
> > > ---
> > >  drivers/net/bcmgenet.c | 5 -
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/net/bcmgenet.c b/drivers/net/bcmgenet.c
> > > index 8f4848aec6..e971b556ac 100644
> > > --- a/drivers/net/bcmgenet.c
> > > +++ b/drivers/net/bcmgenet.c
> > > @@ -448,7 +448,10 @@ static int bcmgenet_adjust_link(struct
> > > bcmgenet_eth_priv *priv)
> > >   }
> > >  
> > >   clrsetbits_32(priv->mac_reg + EXT_RGMII_OOB_CTRL, OOB_DISABLE,
> > > - RGMII_LINK | RGMII_MODE_EN | ID_MODE_DIS);
> > > + RGMII_LINK | RGMII_MODE_EN);
> > > +
> > > + if (phy_dev->interface == PHY_INTERFACE_MODE_RGMII)
> > > + setbits_32(priv->mac_reg + EXT_RGMII_OOB_CTRL, ID_MODE_DIS);
> > 
> > Is this given because by different DTS? Shouldn't that be uniform on the
> > RPi4?
> 
> The interface type is read from DT, the 'phy-mode' property. In the case of
> the
> RPi4 it's 'rgmii-rxid'.
> 
> The downstream DT used to be configured differently ('rgmii' and using
> 'ethernet-phy-ieee802.3-c22'), that's why you might have seen the board
> working
> at some point with this driver. But as we updated the DT to match upstream's
> we
> switched to 'rgmii-rxid' which is being misconfigured as 'rgmii' in u-boot. So
> you have u-boot configuring 'rgmii' while Linux configures 'rgmii-rxid', which
> fails to clear the ID_MODE_DIS bit. This, I imagine, blocks the delay
> configuration process from the PHY (I don't have any documentation).

With this in mind, would it make sens to do this in the Linux driver?

diff --git a/drivers/net/ethernet/broadcom/genet/bcmmii.c
b/drivers/net/ethernet/broadcom/genet/bcmmii.c
index 6392a2530183..10244941a7a6 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmmii.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmmii.c
@@ -294,6 +294,7 @@ int bcmgenet_mii_config(struct net_device *dev, bool init)
*/
if (priv->ext_phy) {
reg = bcmgenet_ext_readl(priv, EXT_RGMII_OOB_CTRL);
+   reg &= ~ID_MODE_DIS;
reg |= id_mode_dis;
if (GENET_IS_V1(priv) || GENET_IS_V2(priv) || GENET_IS_V3(priv))
reg |= RGMII_MODE_EN_V123;

It all comes down to wheter we trust bootloader's config or not.

Regards,
Nicolas



signature.asc
Description: This is a digitally signed message part

Re: [PATCH] net: bcmgenet: Don't set ID_MODE_DIS when not using RGMII

[Nicolas Saenz Julienne]

Hi Florian,

On Thu, 2020-02-20 at 11:05 -0800, Florian Fainelli wrote:
> On 2/20/20 8:36 AM, Nicolas Saenz Julienne wrote:
> > As per Linux's driver, ID_MODE_DIS is only set when the PHY interface is
> > RGMII. Don't enable it for the rest of setups.
> > 
> > This has been seen to misconfigure RPi4's PHY when booting Linux.
> > 
> > Signed-off-by: Nicolas Saenz Julienne 
> 
> Does the failure look like the following: you have a driver for the
> Broadcom PHY used on the Pi4 in u-boot, and the phy_dev->interface value
> is being used to configure the Ethernet PHY chip in a certain way.
> 
> Later when you boot Linux, you do not have CONFIG_BROADCOM_PHY enabled
> so the Generic PHY driver gets used instead, and there is a disagreement
> between the AMAC and PHY as to whom should be adding the delays?

I added an explanation to Matthias' response. I think it fits yours, modulo my
limited knowledge in the area :)

> At any rate:
> 
> Reviewed-by: Florian Fainelli 

Thanks!

Regards,
Nicolas



signature.asc
Description: This is a digitally signed message part

Re: [PATCH] net: bcmgenet: Don't set ID_MODE_DIS when not using RGMII

[Nicolas Saenz Julienne]

Hi Matthias,

On Thu, 2020-02-20 at 19:58 +0100, Matthias Brugger wrote:
> 
> On 20/02/2020 17:36, Nicolas Saenz Julienne wrote:
> > As per Linux's driver, ID_MODE_DIS is only set when the PHY interface is
> > RGMII. Don't enable it for the rest of setups.
> > 
> > This has been seen to misconfigure RPi4's PHY when booting Linux.
> > 
> > Signed-off-by: Nicolas Saenz Julienne 
> > ---
> >  drivers/net/bcmgenet.c | 5 -
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/bcmgenet.c b/drivers/net/bcmgenet.c
> > index 8f4848aec6..e971b556ac 100644
> > --- a/drivers/net/bcmgenet.c
> > +++ b/drivers/net/bcmgenet.c
> > @@ -448,7 +448,10 @@ static int bcmgenet_adjust_link(struct
> > bcmgenet_eth_priv *priv)
> > }
> >  
> > clrsetbits_32(priv->mac_reg + EXT_RGMII_OOB_CTRL, OOB_DISABLE,
> > -   RGMII_LINK | RGMII_MODE_EN | ID_MODE_DIS);
> > +   RGMII_LINK | RGMII_MODE_EN);
> > +
> > +   if (phy_dev->interface == PHY_INTERFACE_MODE_RGMII)
> > +   setbits_32(priv->mac_reg + EXT_RGMII_OOB_CTRL, ID_MODE_DIS);
> 
> Is this given because by different DTS? Shouldn't that be uniform on the RPi4?

The interface type is read from DT, the 'phy-mode' property. In the case of the
RPi4 it's 'rgmii-rxid'.

The downstream DT used to be configured differently ('rgmii' and using
'ethernet-phy-ieee802.3-c22'), that's why you might have seen the board working
at some point with this driver. But as we updated the DT to match upstream's we
switched to 'rgmii-rxid' which is being misconfigured as 'rgmii' in u-boot. So
you have u-boot configuring 'rgmii' while Linux configures 'rgmii-rxid', which
fails to clear the ID_MODE_DIS bit. This, I imagine, blocks the delay
configuration process from the PHY (I don't have any documentation).

Regards,
Nicolas



signature.asc
Description: This is a digitally signed message part

Re: [PATCH 1/2] doc/Makefile: turn warnings into errors

[Tom Rini]

On Fri, Feb 21, 2020 at 05:43:40AM +0100, Heinrich Schuchardt wrote:
> On 2/21/20 5:31 AM, Heinrich Schuchardt wrote:
> > On 2/21/20 5:06 AM, Tom Rini wrote:
> > > On Fri, Feb 21, 2020 at 04:26:36AM +0100, Heinrich Schuchardt wrote:
> > > 
> > > > Several patches delivered incorrect restructured text as
> > > > documentation. We
> > > > should be able to discover this in Travis CI or Gitlab CI.
> > > > 
> > > > So let us turn all build warnings into errors.
> > > > 
> > > > We cannot add --keep-going as Gitlab does not use sphinx-build
> > > > version 1.8
> > > > or later yet.
> > > 
> > > Why can't we upgrade sphinx-build so that we then run these builds also
> > > in Gitlab / Azure?  Thanks!
> 
> Maybe this was unclear to you: My choice was not to use --keep-going
> because I want run the test in the current Gitlab Docker image. With the
> patch as it is we can run on Gitlab.

Ah, yes, OK.  The commit message isn't clear.  We should just drop that
line about --keep-going.  Thanks!

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH 1/3] clk: imx: imx8mn: add enet clk

[Lukasz Majewski]

Hi Alifer,

> Hello Lukasz,
> 
> > Why have you sent those patches twice?
> >  
> 
> I'm sorry for that, 

I was just curious if you made a v2 :-)

> the first time I sent this patch I forgot to send
> to u-boot lists too, so I sent it again.
> 
> Regards,
> Alifer




Best regards,

Lukasz Majewski

--

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


pgpA5obruqnYU.pgp
Description: OpenPGP digital signature

[PATCH] doc: Chromebook Coral: Fix typo for "Top of CAR region"

[Wolfgang Wallner]

The value for "Top of CAR region" should be fefc, not
fefc000. This matches the Kconfig default values, as
SYS_CAR_ADDR and SYS_CAR_SIZE are 0xfef0 and 0xc
respectively.

Signed-off-by: Wolfgang Wallner 

---

 doc/board/google/chromebook_coral.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/board/google/chromebook_coral.rst 
b/doc/board/google/chromebook_coral.rst
index d10e0c4954..40bd9397d4 100644
--- a/doc/board/google/chromebook_coral.rst
+++ b/doc/board/google/chromebook_coral.rst
@@ -179,7 +179,7 @@ Partial memory map
    Top of ROM (and last byte of 32-bit address space)
 8000   TPL loaded here (from IFWI)
 ff00   Bottom of ROM
- fefc000   Top of CAR region
+fefc   Top of CAR region
 fef96000   Stack for FSP-M
 fef4 59000 FSP-M
 fef11000   SPL loaded here
-- 
2.25.0

Re: [PATCH 1/3] clk: imx: imx8mn: add enet clk

[Alifer Moraes]

Hello Lukasz,

> Why have you sent those patches twice?
>

I'm sorry for that, the first time I sent this patch I forgot to send
to u-boot lists too, so I sent it again.

Regards,
Alifer

Re: [PATCH] usb: gadget: Handle SPL_* configs

[Lukasz Majewski]

Hi Nathan,

> On Tue, 28 Jan 2020 at 20:26, Lukasz Majewski  wrote:
> >
> > On Tue, 28 Jan 2020 17:50:03 +1000
> > Nathan Rossi  wrote:
> >  
> > > On Mon, 27 Jan 2020 at 22:51, Lukasz Majewski 
> > > wrote:  
> > > >
> > > > Hi Nathan,
> > > >  
> > > > > Handle selection of objects based on $(SPL_) to allow for
> > > > > normal and SPL builds to have differing object compilation.  
> > > >
> > > > Could you share the exact use case? I do guess that you want to
> > > > add some gadget(s) to SPL?  
> > >
> > > I am primarily trying to disable SPL_ENV_SUPPORT for beaglebone
> > > (am335x_evm). SPL_USB_ETHER has a dependency on SPL_ENV_SUPPORT,
> > > thus the desire to disable it (whilst leaving SPL_USB_GADGET
> > > enabled).  
> >
> > Ok.
> >  
> > >  
> > > >
> > > > (Such changes may cause issues on boards already using this
> > > > feature
> > > > - could you run:
> > > >
> > > > ./tools/buildman/buildman.py --branch=HEAD siemens samsung bbb
> > > > --detail --verbose --show_errors --force-build --count=1
> > > > --output-dir=./BUILD/  
> > >
> > > Running this showed no regressions. Also I noticed "bbb" does not
> > > refer to any boards?  
> >
> > I see. Then please try am335x instead.  
> 
> There are no regressions for those boards either.

I did some tests before pulling:

-master branch u-boot
SHA1: f2a73d6867ef973fbb8471cc87058205999b5e5c

./tools/buildman/buildman.py --branch=HEAD am43xx_evm_usbhost_boot
am335x --show_errors --force-build --verbose --count=1
--output-dir=../BUILD/


   arm:  +   am43xx_evm_usbhost_boot
+../drivers/usb/gadget/ether.c: In function ‘usb_eth_probe’:
+../drivers/usb/gadget/ether.c:2654:17: error: ‘CONFIG_USBNET_DEVADDR’ 
undeclared (first use in this function)
+  get_ether_addr(CONFIG_USBNET_DEVADDR, pdata->enetaddr);
+ ^

Could you fix it and repost the patch?

> 
> Regards,
> Nathan
> 
> >  
> > >
> > > Thanks,
> > > Nathan
> > >  
> > > >
> > > >
> > > >  
> > > > >
> > > > > Signed-off-by: Nathan Rossi 
> > > > > ---
> > > > >  drivers/usb/gadget/Makefile | 8 
> > > > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/drivers/usb/gadget/Makefile
> > > > > b/drivers/usb/gadget/Makefile index 70f3bf43e7..8967745513
> > > > > 100644 --- a/drivers/usb/gadget/Makefile
> > > > > +++ b/drivers/usb/gadget/Makefile
> > > > > @@ -3,8 +3,8 @@
> > > > >  # (C) Copyright 2000-2007
> > > > >  # Wolfgang Denk, DENX Software Engineering, w...@denx.de.
> > > > >
> > > > > -obj-$(CONFIG_USB_GADGET) += epautoconf.o config.o usbstring.o
> > > > > -obj-$(CONFIG_USB_ETHER) += epautoconf.o config.o usbstring.o
> > > > > +obj-$(CONFIG_$(SPL_)USB_GADGET) += epautoconf.o config.o
> > > > > usbstring.o +obj-$(CONFIG_$(SPL_)USB_ETHER) += epautoconf.o
> > > > > config.o usbstring.o
> > > > >
> > > > >  ifdef CONFIG_SPL_BUILD
> > > > >  obj-$(CONFIG_SPL_USB_GADGET) += g_dnl.o
> > > > > @@ -13,7 +13,7 @@ obj-$(CONFIG_SPL_USB_SDP_SUPPORT) += f_sdp.o
> > > > >  endif
> > > > >
> > > > >  # new USB gadget layer dependencies
> > > > > -ifdef CONFIG_USB_GADGET
> > > > > +ifdef CONFIG_$(SPL_)USB_GADGET
> > > > >  obj-$(CONFIG_USB_GADGET_AT91) += at91_udc.o
> > > > >  obj-$(CONFIG_USB_GADGET_ATMEL_USBA) += atmel_usba_udc.o
> > > > >  obj-$(CONFIG_USB_GADGET_BCM_UDC_OTG_PHY) += bcm_udc_otg_phy.o
> > > > > @@ -31,7 +31,7 @@ obj-$(CONFIG_USB_FUNCTION_SDP) += f_sdp.o
> > > > >  obj-$(CONFIG_USB_FUNCTION_ROCKUSB) += f_rockusb.o
> > > > >  endif
> > > > >  endif
> > > > > -ifdef CONFIG_USB_ETHER
> > > > > +ifdef CONFIG_$(SPL_)USB_ETHER
> > > > >  obj-y += ether.o
> > > > >  obj-$(CONFIG_USB_ETH_RNDIS) += rndis.o
> > > > >  obj-$(CONFIG_CI_UDC) += ci_udc.o
> > > > > ---
> > > > > 2.24.1  
> > > >
> > > >
> > > >
> > > >
> > > > Best regards,
> > > >
> > > > Lukasz Majewski
> > > >
> > > > --
> > > >
> > > > DENX Software Engineering GmbH,  Managing Director: Wolfgang
> > > > Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194
> > > > Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax:
> > > > (+49)-8142-66989-80 Email: lu...@denx.de  
> >
> >
> >
> >
> > Best regards,
> >
> > Lukasz Majewski
> >
> > --
> >
> > DENX Software Engineering GmbH,  Managing Director: Wolfgang
> > Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell,
> > Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> > lu...@denx.de  




Best regards,

Lukasz Majewski

--

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


pgpz28e3zWDTB.pgp
Description: OpenPGP digital signature