Re: [PATCH] riscv: fix compitible with binutils 2.38

2022-08-30 Thread Leo Liang
On Fri, Aug 19, 2022 at 09:24:53AM -0600, Simon Glass wrote:
> Hi Leo,
> 
> On Fri, 19 Aug 2022 at 03:09, Leo Liang  wrote:
> >
> > On Thu, Aug 11, 2022 at 10:23:02PM +, Leo Liang wrote:
> > > On Mon, May 30, 2022 at 11:05:54AM -0400, Tom Rini wrote:
> > > > On Sat, May 28, 2022 at 09:02:09AM +, Leo Liang wrote:
> > > > > On Fri, May 27, 2022 at 09:30:49AM -0400, Tom Rini wrote:
> > > > > > On Fri, May 27, 2022 at 02:36:29AM +, Leo Liang wrote:
> > > > > >
> > > > > > > Hi Tom,
> > > > > > >
> > > > > > > The following changes since commit 
> > > > > > > 7e0edcadb09d55d5319fdc862041fd1b874476f5:
> > > > > > >
> > > > > > >   Merge branch 'master' of 
> > > > > > > https://source.denx.de/u-boot/custodians/u-boot-sunxi (2022-05-24 
> > > > > > > 23:29:00 -0400)
> > > > > > >
> > > > > > > are available in the Git repository at:
> > > > > > >
> > > > > > >   https://source.denx.de/u-boot/custodians/u-boot-riscv.git
> > > > > > >
> > > > > > > for you to fetch changes up to 
> > > > > > > c544b281cd3e549a4fcbf4ba9a05a5d72c9557dd:
> > > > > > >
> > > > > > >   riscv: qemu: Set kernel_comp_addr_r for compressed kernel 
> > > > > > > (2022-05-26 18:42:34 +0800)
> > > > > > >
> > > > > > > CI result shows no issue: 
> > > > > > > https://source.denx.de/u-boot/custodians/u-boot-riscv/-/pipelines/12131
> > > > > >
> > > > > > First, I've applied this to u-boot/master now.  Second, will
> > > > > > https://patchwork.ozlabs.org/project/uboot/patch/ph7pr14mb5594fd11d1be74284f554bebce...@ph7pr14mb5594.namprd14.prod.outlook.com/
> > > > > > be coming soon?  Thanks!
> > > > >
> > > > > Hi Tom,
> > > > >
> > > > > This patch you mentioned will not pass CI, and the reason for that
> > > > > is the toolchain used for RISC-V in CI does not have corresponding
> > > > > settings for zifencei and zicsr.
> > > > > (detailed disscussion: 
> > > > > https://patchwork.ozlabs.org/project/uboot/patch/20220128134713.2322800-1-alexandre.gh...@canonical.com/)
> > > > > (CI result: 
> > > > > https://source.denx.de/u-boot/custodians/u-boot-riscv/-/jobs/440735)
> > > > >
> > > > > The patch looks valid, but will fail CI on 32-bit configs.
> > > > > If we use 32-bit toolchain to test 32-bit configs, then
> > > > > problems solved.
> > > > >
> > > > > Do you have any comments?
> > > >
> > > > I guess I'm OK with saying we should use a 32bit toolchain for 32bit
> > > > riscv, if  that's how things should be handled moving forward for
> > > > everyone else.
> > > >
> > > > --
> > > > Tom
> > >
> > > Hi Tom,
> > >
> > > Sorry for taking such a long time to reply.
> > >
> > > Recap:
> > > All the "riscv: fix compitible with binutils 2.38" patches that
> > > try to support new RISC-V ISA extension will cause U-Boot CI to fail
> > > because the toolchain used in U-Boot CI do not support the new multilib 
> > > settings.
> > > (original discussion:
> > > https://patchwork.ozlabs.org/project/uboot/patch/20220128134713.2322800-1-alexandre.gh...@canonical.com/)
> > >
> > > We found that current RISC-V toolchains from kernel.org do not
> > > support zifencei and zicsr extensions' multilib settings,
> > > regardless of the toolchain version.
> > > (Both gcc 11.1.0, 12.1.0 do not support the needed settings.
> > > https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/11.1.0/
> > > https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/12.1.0/)
> > >
> > > But we also found that if we use recent upstream riscv-gnu-toolchain,
> > > we could build an gcc-12.1.0 toolchain that does support multilib
> > > settings and could fix this issue.
> > >
> > > We have provided a Dockerfile as a reference build script[1] and
> > > a prebuilt toolchain[2] for U-Boot CI to use.
> > >
> > > We have also verified the CI process could execute successfully
> > > with your base image and the provided riscv64-linux toolchain[3].
> > >
> > > I guess the coming update of the toolchain in kernel.org should
> > > contain the new multilib settings, so I was wondering if we could
> > > replace the riscv64-linux toolchain from kernel.org with this prebuilt
> > > toolchain we've provided on github[2] temporarily?
> > >
> > > After studying a bit of the buildman tool, the earlier idea that
> > > "use different toolchains for different board configs" would require
> > > an amount of modification, thus we think its best to replace the toolchain
> > > temporarily to fix this issue, then the patch could be applied without CI 
> > > failure.
> > >
> > > [1] 
> > > https://github.com/ycliang-andes/riscv-toolchain/blob/master/Dockerfile
> > > [2] 
> > > https://github.com/ycliang-andes/riscv-toolchain/releases/download/v1.0/x86_64-gcc-12.1.0-nolibc-riscv64-linux.tar.xz
> > > [3] 
> > > https://source.denx.de/u-boot/custodians/u-boot-riscv/-/pipelines/13129
> >
> > Hi Tom,
> >
> > A gentle ping.
> > Do you have any concern on changing the toolchain source for now?
> 
> It is easy enough for people to download the toolchain and update the
> .buildman fil

[PATCHv4] drivers: tee: i2c: support the NXP SE05x probe errata

2022-08-30 Thread Jorge Ramirez-Ortiz
Early instantiation of this I2C device would lock up when being
probed.

 https://www.nxp.com/docs/en/errata/SE050_Erratasheet.pdf
 3.2.2
   In scenarios of detecting I2C ICs on the bus using an empty
   I2C frame containing only the address the SE050 will block
   the I2C bus.

Tested on STM32MP1

Signed-off-by: Jorge Ramirez-Ortiz 
Acked-by: Oleksandr Suvorov 
---
 drivers/tee/optee/Kconfig | 14 +
 drivers/tee/optee/i2c.c   | 44 +++
 2 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/drivers/tee/optee/Kconfig b/drivers/tee/optee/Kconfig
index d03028070b..a80ddaed2c 100644
--- a/drivers/tee/optee/Kconfig
+++ b/drivers/tee/optee/Kconfig
@@ -37,6 +37,20 @@ config OPTEE_TA_SCP03
help
  Enables support for controlling (enabling, provisioning) the
  Secure Channel Protocol 03 operation in the OP-TEE SCP03 TA.
+
+config TEE_I2C_NXP_SE05X_ERRATA
+   bool "Enable NXP SE05X Errata"
+   select TEE_I2C_NXP_SE05X_ERRATA_IN_BUS
+   default n
+   help
+ This config prevents the I2C trampoline driver from probing
+ on every transfer.
+
+config TEE_I2C_NXP_SE05X_ERRATA_IN_BUS
+   int "I2C bus where to apply the NXP SE05X errata"
+   depends on TEE_I2C_NXP_SE05X_ERRATA
+   default 0
+
 endmenu
 
 endif
diff --git a/drivers/tee/optee/i2c.c b/drivers/tee/optee/i2c.c
index ef4e10f991..e1b2534909 100644
--- a/drivers/tee/optee/i2c.c
+++ b/drivers/tee/optee/i2c.c
@@ -3,13 +3,18 @@
  * Copyright (c) 2020 Foundries.io Ltd
  */
 
+#define LOG_CATEGORY UCLASS_I2C
+
 #include 
 #include 
 #include 
+#include 
 #include 
 #include "optee_msg.h"
 #include "optee_private.h"
 
+#define NXP_SE05X_ADDR 0x48
+
 static int check_xfer_flags(struct udevice *chip, uint tee_flags)
 {
uint flags;
@@ -30,6 +35,30 @@ static int check_xfer_flags(struct udevice *chip, uint 
tee_flags)
return 0;
 }
 
+static struct udevice *get_chip_dev(int bnum, int addr)
+{
+   struct udevice *chip;
+   struct udevice *bus;
+
+#if defined(CONFIG_TEE_I2C_NXP_SE05X_ERRATA)
+   if (bnum == CONFIG_TEE_I2C_NXP_SE05X_ERRATA_IN_BUS &&
+   addr == NXP_SE05X_ADDR) {
+   if (uclass_get_device_by_seq(UCLASS_I2C, bnum, &bus))
+   return NULL;
+
+   if (i2c_get_chip(bus, addr, 0, &chip))
+   return NULL;
+
+   return chip;
+   }
+#endif
+
+   if (i2c_get_chip_for_busnum(bnum, addr, 0, &chip))
+   return NULL;
+
+   return chip;
+}
+
 void optee_suppl_cmd_i2c_transfer(struct optee_msg_arg *arg)
 {
const u8 attr[] = {
@@ -38,7 +67,8 @@ void optee_suppl_cmd_i2c_transfer(struct optee_msg_arg *arg)
OPTEE_MSG_ATTR_TYPE_RMEM_INOUT,
OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT,
};
-   struct udevice *chip_dev;
+   struct udevice *chip_dev = NULL;
+
struct tee_shm *shm;
u8 *buf;
int ret;
@@ -56,9 +86,9 @@ void optee_suppl_cmd_i2c_transfer(struct optee_msg_arg *arg)
if (!buf)
goto bad;
 
-   if (i2c_get_chip_for_busnum((int)arg->params[0].u.value.b,
-   (int)arg->params[0].u.value.c,
-   0, &chip_dev))
+   chip_dev = get_chip_dev((int)arg->params[0].u.value.b,
+   (int)arg->params[0].u.value.c);
+   if (!chip_dev)
goto bad;
 
if (check_xfer_flags(chip_dev, arg->params[1].u.value.a))
@@ -66,10 +96,16 @@ void optee_suppl_cmd_i2c_transfer(struct optee_msg_arg *arg)
 
switch (arg->params[0].u.value.a) {
case OPTEE_MSG_RPC_CMD_I2C_TRANSFER_RD:
+   log_debug("OPTEE_MSG_RPC_CMD_I2C_TRANSFER_RD %d\n",
+ (size_t)arg->params[2].u.rmem.size);
+
ret = dm_i2c_read(chip_dev, 0, buf,
  (size_t)arg->params[2].u.rmem.size);
break;
case OPTEE_MSG_RPC_CMD_I2C_TRANSFER_WR:
+   log_debug("OPTEE_MSG_RPC_CMD_I2C_TRANSFER_WR %d\n",
+ (size_t)arg->params[2].u.rmem.size);
+
ret = dm_i2c_write(chip_dev, 0, buf,
   (size_t)arg->params[2].u.rmem.size);
break;
-- 
2.34.1



Re: [PATCHv2 1/2] i2c: stm32f7: fix clearing the control register

2022-08-30 Thread Patrick DELAUNAY

Hi,

On 8/15/22 16:52, Jorge Ramirez-Ortiz wrote:

Bits should be set to 0, not 1.

Signed-off-by: Jorge Ramirez-Ortiz 
---
  drivers/i2c/stm32f7_i2c.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/stm32f7_i2c.c b/drivers/i2c/stm32f7_i2c.c
index bf2a6c9b4b..3a727e68ac 100644
--- a/drivers/i2c/stm32f7_i2c.c
+++ b/drivers/i2c/stm32f7_i2c.c
@@ -413,7 +413,7 @@ static int stm32_i2c_check_end_of_message(struct 
stm32_i2c_priv *i2c_priv)
setbits_le32(®s->icr, STM32_I2C_ICR_STOPCF);
  
  		/* Clear control register 2 */

-   setbits_le32(®s->cr2, STM32_I2C_CR2_RESET_MASK);
+   clrbits_le32(®s->cr2, STM32_I2C_CR2_RESET_MASK);
}
  
  	return ret;



Reviewed-by: Patrick Delaunay 

Thanks
Patrick




Re: [PATCH] disk: part: remove dependency to ubifs for spl

2022-08-30 Thread Stefan Herbrechtsmeier

Hi,

Am 30.08.2022 um 04:29 schrieb Simon Glass:

Hi Stefan,

On Mon, 29 Aug 2022 at 09:53, Stefan Herbrechtsmeier
 wrote:


Hi Heinrich,

Am 29.08.2022 um 17:01 schrieb Heinrich Schuchardt:

On 8/8/22 16:45, Stefan Herbrechtsmeier wrote:

From: Stefan Herbrechtsmeier 

The spl doesn't support ubifs and thereby doesn't provide the
ubifs_is_mounted function. Remove the dependency to ubifs for the spl.

Signed-off-by: Stefan Herbrechtsmeier


---

   disk/part.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/disk/part.c b/disk/part.c
index 79955c7fb0..de1b917e84 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -479,7 +479,7 @@ int blk_get_device_part_str(const char *ifname,
const char *dev_part_str,
   }
   #endif

-#ifdef CONFIG_CMD_UBIFS
+#if IS_ENABLED(CONFIG_CMD_UBIFS) && !IS_ENABLED(CONFIG_SPL_BUILD)


This configuration seems strange. The support for a file system should
not depend on a command. I think a CONFIG_UBIFS is missing.


The support for the ubi file system depends on the command. The code use
the ubifs_is_mounted function from cmd/ubifs.c file and reference the
ubifsmount command from the same file.


Yes it does, but it should not (I think I made the point in another
thread but did not understand your response). The UBIFS Kconfig should
be separate from CMD_UBIFS.


I think we agree that the ubifs implementation isn't optimal. But this 
patch only fix a problem for SPL if UBIFS is enabled in u-boot proper. 
The wrong implementation was accept by u-boot 7 years ago.


The problem for me is that we no longer use UBI. I fear that a rework of 
UBI needs a lot of time to to fix a 'should not'. On the other hand I 
have patches with an unclear status that fix problems / incompatibilities.


Regards
  Stefan


Re: [RFC PATCH v2] clk: fix clk_get_rate() always return ulong

2022-08-30 Thread Julien Masson
Hi Simon,

On Tue 30 Aug 2022 at 10:32, Simon Glass  wrote:

> Hi Julien,
> 
> On Mon, 29 Aug 2022 at 06:06, Julien Masson  wrote:
>>
>> According to clk_ops struct definition, the callback `get_rate()`
>> return current clock rate value as ulong.
>> `clk_get_rate()` should handle the clock rate returned as ulong also.
>>
>> Otherwise we may have invalid/truncated clock rate value returned by
>> `clk_get_rate()`.
>>
>> `log_ret` has also been removed since it use an `int` in the macro
>> definition.
>>
>> Signed-off-by: Julien Masson 
>> ---
>> drivers/clk/clk-uclass.c | 8 ++--
>> 1 file changed, 2 insertions(+), 6 deletions(-)
> 
> Reviewed-by: Simon Glass 
Thanks for the review, I appreciate it.

> 
> I would prefer to create a new log_rete() to handle this, with a long
> argument and return value. But this is OK, I suppose.
Yes and I guess that could be used in other functions in clk-uclass.c

> 
>>
>> diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
>> index b89c77bf79..c351fa97d1 100644
>> --- a/drivers/clk/clk-uclass.c
>> +++ b/drivers/clk/clk-uclass.c
>> @@ -469,7 +469,7 @@ void clk_free(struct clk *clk)
>> ulong clk_get_rate(struct clk *clk)
>> {
>> const struct clk_ops *ops;
>> -   int ret;
>> +   ulong ret;
>>
>> debug("%s(clk=%p)\n", __func__, clk);
>> if (!clk_valid(clk))
>> @@ -479,11 +479,7 @@ ulong clk_get_rate(struct clk *clk)
>> if (!ops->get_rate)
>> return -ENOSYS;
>>
>> -   ret = ops->get_rate(clk);
>> -   if (ret)
>> -   return log_ret(ret);
>> -
>> -   return 0;
>> +   return ops->get_rate(clk);
>> }
>>
>> struct clk *clk_get_parent(struct clk *clk)
>> --
>> 2.37.2
>>
> 
> Regards,
> Simon

-- 
Julien Masson


Re: [PATCH] ARM: stm32: Switch DHSOM to FMC2 EBI driver

2022-08-30 Thread Patrick DELAUNAY

Hi,

On 8/23/22 19:27, Marek Vasut wrote:

Perform long overdue conversion of ad-hoc FMC2 EBI bus initialization
to upstream FMC2 EBI driver. No functional change.

Signed-off-by: Marek Vasut 
Cc: Patrice Chotard 
Cc: Patrick Delaunay 
---
  .../dts/stm32mp15xx-dhcom-picoitx-u-boot.dtsi |  8 ---
  arch/arm/dts/stm32mp15xx-dhcom-u-boot.dtsi| 14 -
  .../stm32mp15xx-dhcor-drc-compact-u-boot.dtsi | 16 --
  board/dhelectronics/dh_stm32mp1/board.c   | 52 ---
  configs/stm32mp15_dhcom_basic_defconfig   |  1 +
  configs/stm32mp15_dhcor_basic_defconfig   |  1 +
  6 files changed, 2 insertions(+), 90 deletions(-)



Reviewed-by: Patrick Delaunay 

Thanks
Patrick



Re: [PATCH] configs: stm32: Enable CONFIG_DM_REGULATOR for stm32f769-disco

2022-08-30 Thread Patrick DELAUNAY

Hi,

On 8/24/22 15:44, Patrice Chotard wrote:

Since commit 5bc6f8c2a97e("video: stm32: remove test on CONFIG_DM_REGULATOR")
backlight was broken with the following message at boot:
stm32-display-dsi dsi@40016c00: Warning: cannot get phy dsi supply
stm32_display display-controller@40016800: panel panel enable backlight error 
-38

DM_REGULATOR flag must be enabled to fix this issue

Signed-off-by: Patrice Chotard 
---

  configs/stm32f769-disco_defconfig | 1 +
  configs/stm32f769-disco_spl_defconfig | 1 +
  2 files changed, 2 insertions(+)



Reviewed-by: Patrick Delaunay 

Thanks
Patrick



Re: [PATCH] ARM: dts: stm32: Fix display-timings settings for stm32f746-disco

2022-08-30 Thread Patrick DELAUNAY

Hi,

On 8/24/22 15:42, Patrice Chotard wrote:

Since commit ef4ce6df3289 "video: stm32: stm32_ltdc: fix data enable polarity"
The panel display output wasn't functional anymore.
Device tree display-timings de-active property value must be updated
to 1.

Signed-off-by: Patrice Chotard 
---

  arch/arm/dts/stm32f746-disco-u-boot.dtsi | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)



Reviewed-by: Patrick Delaunay 

Thanks
Patrick




Re: [PATCH v2] pci: Do not enable PCIe GEN3 link retrain workaround by default

2022-08-30 Thread Maciej W. Rozycki
On Sat, 27 Aug 2022, Pali Rohár wrote:

> Moreover this workaround is enabled for all existing hardware and also all
> future PCIe hardware, which opens a hole that other PCIe vendors may
> introduce same HW issue as on systems where this workaround is required and
> nobody would notice it because U-Boot automatically apply workaround for it.

 Why is it a problem?  Is the intent to cause hassle to end users and 
force them to take action when they have a non-working piece of hardware?  

 I'd say in 99% of cases this will only cause frustration and they won't 
bother.  They will just conclude that either piece of hardware involved is 
broken and will throw it away.  Just as I almost did.  The seller has 
offered me a refund, which seems thought to be a universal solution 
nowadays (but I need to do what I meant to and getting money back doesn't 
solve it).

 And at least I know what U-boot (or indeed firmware) is and have a 
general understanding of how computers work.  Most people just want to 
plug stuff in and use it for whatever their need is.  Expecting them to 
take action to get things working is wasting their time (which BTW seems 
to have been a growing trend in last ~30 years: putting burden on the end 
user to get our problems solved, which saves our time and money at the 
expense of end user's).

 NB I'm slowly getting fed up with the amount of non-working stuff piling 
up around.  Every other piece of equipment I try doesn't work for one 
reason or another and I need to either chase bugs myself or to spend days 
and weeks to persuade someone at least to believe a problem is there to 
get that sorted.  All in my free time I'd rather spend on something else.  
I'd welcome things working automagically for a change so that I could 
focus on what I mean to be doing, and therefore I take breaking things 
deliberately as a major offence.

 FWIW,

  Maciej


Re: [PATCH v2] pci: Do not enable PCIe GEN3 link retrain workaround by default

2022-08-30 Thread Pali Rohár
On Tuesday 30 August 2022 10:04:51 Maciej W. Rozycki wrote:
> On Sat, 27 Aug 2022, Pali Rohár wrote:
> 
> > Moreover this workaround is enabled for all existing hardware and also all
> > future PCIe hardware, which opens a hole that other PCIe vendors may
> > introduce same HW issue as on systems where this workaround is required and
> > nobody would notice it because U-Boot automatically apply workaround for it.
> 
>  Why is it a problem?

I think I wrote it. One issue is that it is increasing size of SPL image
and we really should not include into SPL things which are not required
for all target platforms. Lot of boards have size constrained memory
requirements and unnecessary features should not be automatically
enabled.

> Is the intent to cause hassle to end users and 
> force them to take action when they have a non-working piece of hardware?  

Vendors should really test their hardware without any automatic
workaround. Otherwise we are going into the hell if some workarounds are
automatically enabled and nobody notice broken behavior. And vendors
really build software with default options and do not care about it if
default options are suitable.

There is already direction to make all workaround targeted and enabled
only for platforms / hardware which really need it.

So enabling some workaround for all platforms which are produced and
will be produced in the future is step backward.

>  I'd say in 99% of cases this will only cause frustration and they won't 
> bother.  They will just conclude that either piece of hardware involved is 
> broken and will throw it away.  Just as I almost did.  The seller has 
> offered me a refund, which seems thought to be a universal solution 
> nowadays (but I need to do what I meant to and getting money back doesn't 
> solve it).
> 
>  And at least I know what U-boot (or indeed firmware) is and have a 
> general understanding of how computers work.  Most people just want to 
> plug stuff in and use it for whatever their need is.  Expecting them to 
> take action to get things working is wasting their time (which BTW seems 
> to have been a growing trend in last ~30 years: putting burden on the end 
> user to get our problems solved, which saves our time and money at the 
> expense of end user's).

The another issue here is that it was not fully investigated where the
issue is. If it is processor specific, PCIe switch specific or endpoint
specific, or combination of these options. It was just observed that
proposed workaround fix this issue on one specific combination. And you
confirmed this in previous post, that you are unsure if it is not
specific to downstream ports of the switch.

We really should not include such "bloatware" code to be enabled for
everything, on every one board. You are the first who reported this
issue. Nobody else complained about it, so I really do not see reason
why all other users and developers must be forced to have it in their
U-Boot binaries. Moreover lot of boards on which is U-Boot running do
not have PCIe bus exported on the slot and have PCIe devices integrated
and soldered on the board. Why on the earth I have to need this
workaround also on these boards (which are moreover sized constrained)?

>  NB I'm slowly getting fed up with the amount of non-working stuff piling 
> up around.  Every other piece of equipment I try doesn't work for one 
> reason or another and I need to either chase bugs myself or to spend days 
> and weeks to persuade someone at least to believe a problem is there to 
> get that sorted.  All in my free time I'd rather spend on something else.  
> I'd welcome things working automagically for a change so that I could 
> focus on what I mean to be doing, and therefore I take breaking things 
> deliberately as a major offence.
> 
>  FWIW,
> 
>   Maciej

What other U-Boot developers think?


Re: [PATCH v5 1/8] binman: add support for skipping file concatenation for mkimage

2022-08-30 Thread Quentin Schulz

Hi Simon,

On 8/27/22 02:21, Simon Glass wrote:

Hi Quentin,

On Fri, 26 Aug 2022 at 09:37, Quentin Schulz  wrote:


From: Quentin Schulz 

Some image types handled by mkimage require the datafiles to be passed
independently (-d data1:data2) for specific handling of each. A
concatenation of datafiles prior to passing them to mkimage wouldn't
work.

That is the case for rkspi for example which requires page alignment
and only writing 2KB every 4KB.

This adds the ability to tell binman to pass the datafiles without
prior concatenation to mkimage, by adding the multiple-data-files
boolean property to the mkimage node.

Cc: Quentin Schulz 
Reviewed-by: Simon Glass 
Signed-off-by: Quentin Schulz 
---

v5:
  - changed to use full path from input dir with tools.get_input_filename
  to make it possible to run the unit tests,
  - added unit test,


  tools/binman/entries.rst  | 22 ++
  tools/binman/etype/mkimage.py | 41 +--
  tools/binman/ftest.py | 16 


Please put the new test at the end.


  .../test/241_mkimage_multiple_data_files.dts  | 21 ++
  4 files changed, 96 insertions(+), 4 deletions(-)
  create mode 100644 tools/binman/test/241_mkimage_multiple_data_files.dts


This is pretty close but it still missing a line of test coverage.
Please try 'binman test -T' to see it. I'd also prefer a shorter


This does not work on Fedora.
1) there's no python3-coverage binary available,
2) After replacing python3-coverage with just coverage, the tests are 
stuck and never finish, (I have seen the patches to use COVERAGE 
environment variable so I guess the required changes might be tackled 
soon in master),


Any tip on how to identify which test is stuck except going through them 
one by one?


python3-coverage is also not available in the container image built from 
tools/docker/Dockerfile.



filename for the 241 file.

I've pushed a tree containing a suggested fix (updating this patch). I
can update it when applying if you like, otherwise please send a new
version.



Where did you push the tree?

Cheers,
Quentin


Re: [PATCH 1/2] patman: do not hardcode coverage tool

2022-08-30 Thread Quentin Schulz

Hi Michal,

On 8/25/22 08:49, Michal Suchanek wrote:

The coverage tool name varies across distributions.

Add COVERAGE variable to specify the tool name.

Also there is one place where prefix is prepended to the tool path,
remove the prefix.

Signed-off-by: Michal Suchanek 
---
  doc/develop/testing.rst   |  3 +++
  tools/patman/test_util.py | 18 ++
  2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/doc/develop/testing.rst b/doc/develop/testing.rst
index 1abe4d7f0f..054fbfc814 100644
--- a/doc/develop/testing.rst
+++ b/doc/develop/testing.rst
@@ -17,6 +17,9 @@ To run most tests on sandbox, type this::
  in the U-Boot directory. Note that only the pytest suite is run using this
  command.
  
+Note: external tool `python3-coverage` is used by tests. The environment

+variable `COVERAGE` can be set to alternative name or location of this tool.
+
  Some tests take ages to run and are marked with @pytest.mark.slow. To run just
  the quick ones, type this::
  
diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py

index 0f6d1aa902..e11806b626 100644
--- a/tools/patman/test_util.py
+++ b/tools/patman/test_util.py
@@ -15,6 +15,8 @@ from patman import command
  
  from io import StringIO
  
+coverage = os.environ.get('COVERAGE', 'python3-coverage')

+
  buffer_outputs = True
  use_concurrent = True
  try:
@@ -58,11 +60,11 @@ def run_test_coverage(prog, filter_fname, exclude_list, 
build_dir, required=None
  prefix = ''
  if build_dir:
  prefix = 'PYTHONPATH=$PYTHONPATH:%s/sandbox_spl/tools ' % build_dir
-cmd = ('%spython3-coverage run '
-   '--omit "%s" %s %s %s -P1' % (prefix, ','.join(glob_list),
+cmd = ('%s run '
+   '--omit "%s" %s %s %s -P1' % (coverage, ','.join(glob_list),
   prog, extra_args or '', test_cmd))


What about using
python3 -m coverage run
instead?
This way we wouldn't rely on the binary name the host distribution 
chooses (python3-coverage for Ubuntu, coverage for Fedora).


I'm not sure there is a need to give the user the ability to override 
this value since I expect only coverage.py is supported at the moment?


Cheers,
Quentin


Re: [PATCH 1/2] patman: do not hardcode coverage tool

2022-08-30 Thread Michal Suchánek
On Tue, Aug 30, 2022 at 12:01:55PM +0200, Quentin Schulz wrote:
> Hi Michal,
> 
> On 8/25/22 08:49, Michal Suchanek wrote:
> > The coverage tool name varies across distributions.
> > 
> > Add COVERAGE variable to specify the tool name.
> > 
> > Also there is one place where prefix is prepended to the tool path,
> > remove the prefix.
> > 
> > Signed-off-by: Michal Suchanek 
> > ---
> >   doc/develop/testing.rst   |  3 +++
> >   tools/patman/test_util.py | 18 ++
> >   2 files changed, 13 insertions(+), 8 deletions(-)
> > 
> > diff --git a/doc/develop/testing.rst b/doc/develop/testing.rst
> > index 1abe4d7f0f..054fbfc814 100644
> > --- a/doc/develop/testing.rst
> > +++ b/doc/develop/testing.rst
> > @@ -17,6 +17,9 @@ To run most tests on sandbox, type this::
> >   in the U-Boot directory. Note that only the pytest suite is run using this
> >   command.
> > +Note: external tool `python3-coverage` is used by tests. The environment
> > +variable `COVERAGE` can be set to alternative name or location of this 
> > tool.
> > +
> >   Some tests take ages to run and are marked with @pytest.mark.slow. To run 
> > just
> >   the quick ones, type this::
> > diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py
> > index 0f6d1aa902..e11806b626 100644
> > --- a/tools/patman/test_util.py
> > +++ b/tools/patman/test_util.py
> > @@ -15,6 +15,8 @@ from patman import command
> >   from io import StringIO
> > +coverage = os.environ.get('COVERAGE', 'python3-coverage')
> > +
> >   buffer_outputs = True
> >   use_concurrent = True
> >   try:
> > @@ -58,11 +60,11 @@ def run_test_coverage(prog, filter_fname, exclude_list, 
> > build_dir, required=None
> >   prefix = ''
> >   if build_dir:
> >   prefix = 'PYTHONPATH=$PYTHONPATH:%s/sandbox_spl/tools ' % 
> > build_dir
> > -cmd = ('%spython3-coverage run '
> > -   '--omit "%s" %s %s %s -P1' % (prefix, ','.join(glob_list),
> > +cmd = ('%s run '
> > +   '--omit "%s" %s %s %s -P1' % (coverage, ','.join(glob_list),
> >prog, extra_args or '', 
> > test_cmd))
> 
> What about using
> python3 -m coverage run
> instead?
> This way we wouldn't rely on the binary name the host distribution chooses
> (python3-coverage for Ubuntu, coverage for Fedora).
> 
> I'm not sure there is a need to give the user the ability to override this
> value since I expect only coverage.py is supported at the moment?

Then you run into the problems that you do not have coverage on
python3.4 but only python3.6 or whatever is the relevant version for
your distribution ATM.

In other words the only general solution is to specify the tool AFAICT.

Thanks

Michal


Re: [PATCH 2/2] tests: Do not hardcode sudo tool

2022-08-30 Thread Michal Suchánek
On Thu, Aug 25, 2022 at 09:01:08AM -0600, Simon Glass wrote:
> Hi Michal,
> 
> On Wed, 24 Aug 2022 at 23:51, Michal Suchanek  wrote:
> >
> > In some situations it may be needed to pass parameters to sudo or to use
> > a different tool to gain root access. Add SUDO variable to specify the
> > sudo tool.
> >
> > Signed-off-by: Michal Suchanek 
> > ---
> >  doc/develop/testing.rst   |  5 +++--
> >  test/fs/fat-noncontig-test.sh |  9 +
> >  test/fs/fs-test.sh| 26 ++
> >  test/py/tests/test_fs/conftest.py |  8 +---
> >  test/py/tests/test_ut.py  | 14 --
> >  5 files changed, 35 insertions(+), 27 deletions(-)
> 
> This is missing a change log and should have 'Series-version: 2' if
> you are using patman.

Hello,

sorry for the confusion. This is v2 for

https://lists.denx.de/pipermail/u-boot/2022-August/492636.html
https://lists.denx.de/pipermail/u-boot/2022-August/492637.html

and adds the documentation changes. With that the patches are no longer
independent and need to be applied in order.

I do not use patman, the coverage results (or errors about missing
coverage tool) are displayed during test runs.

Thanks

Michal


Re: [PATCH v3] dm: core: Do not stop uclass iteration on error

2022-08-30 Thread Michal Suchánek
On Sat, Aug 27, 2022 at 07:52:27PM -0600, Simon Glass wrote:
> Hi Michal,
> 
> On Fri, 19 Aug 2022 at 14:23, Michal Suchanek  wrote:
> >
> > When probing a device fails NULL pointer is returned, and other devices
> > cannot be iterated. Skip to next device on error instead.
> >
> > Fixes: 6494d708bf ("dm: Add base driver model support")
> 
> I think you should drop this as you are doing a change of behaviour,
> not fixing a bug!

You can hardly fix a bug without a change in behavior.

These functions are used for iterating devices, and are not iterating
devices. That's clearly a bug.

> > Signed-off-by: Michal Suchanek 
> > ---
> > v2: - Fix up tests
> > v3: - Fix up API doc
> > - Correctly forward error from uclass_get
> > - Do not return an error when last device fails to probe
> > - Drop redundant initialization
> > - Wrap at 80 columns
> > ---
> >  drivers/core/uclass.c | 32 
> >  include/dm/uclass.h   | 13 -
> >  test/dm/test-fdt.c| 20 
> >  3 files changed, 48 insertions(+), 17 deletions(-)
> 
> Unfortunately this still fails one test. Try 'make qcheck' to see it -
> it is ethernet.

I will look at that.

> I actually think you should create new functions for this feature,
> e.g.uclass_first_device_ok(), since it makes it impossible to see what
> when wrong with a device in the middle.
> 
> I have long had all this in my mind. One idea for a future change is
> to return the error, but set dev, so that the caller knows there is a
> device, which failed. When we are at the end, dev is set to NULL.

We already have uclass_first_device_check() and
uclass_next_device_check() to iterate all devices, including broken
ones, and getting the errors as well.

That's for the case you want all the details, and these are for the case
you just want to get devices and don't care about the details.

That's AFAICT as much as this iteration interface can provide, and we
have both cases covered.

Thanks

Michal


[PATCH 0/2] Add HS configs for J7200 and J721S2 TI SOCs

2022-08-30 Thread Jayesh Choudhary
This series adds the r5 and a72 defconfigs for 2 platforms:
- J7200 HS
- J721S2 HS

Andrew Davis (1):
  configs: Add configs for J7200 High Security EVM

Jayesh Choudhary (1):
  configs: Add configs for j721s2 High Security EVM

 MAINTAINERS |   4 +
 configs/j7200_hs_evm_a72_defconfig  | 208 ++
 configs/j7200_hs_evm_r5_defconfig   | 172 ++
 configs/j721s2_hs_evm_a72_defconfig | 217 
 configs/j721s2_hs_evm_r5_defconfig  | 178 +++
 5 files changed, 779 insertions(+)
 create mode 100644 configs/j7200_hs_evm_a72_defconfig
 create mode 100644 configs/j7200_hs_evm_r5_defconfig
 create mode 100644 configs/j721s2_hs_evm_a72_defconfig
 create mode 100644 configs/j721s2_hs_evm_r5_defconfig

-- 
2.25.1



[PATCH 1/2] configs: Add configs for J7200 High Security EVM

2022-08-30 Thread Jayesh Choudhary
From: Andrew Davis 

Add J7200 High Security EVM defconfig.

These defconfigs are the same as for the non-secure part, except for:
CONFIG_TI_SECURE_DEVICE option set to 'y'
CONFIG_BOOTCOMMAND uses FIT images for booting

Signed-off-by: Andrew Davis 
[j-choudh...@ti.com: add few configs from GP variant which were missing]
Signed-off-by: Jayesh Choudhary 
---
 MAINTAINERS|   2 +
 configs/j7200_hs_evm_a72_defconfig | 208 +
 configs/j7200_hs_evm_r5_defconfig  | 172 
 3 files changed, 382 insertions(+)
 create mode 100644 configs/j7200_hs_evm_a72_defconfig
 create mode 100644 configs/j7200_hs_evm_r5_defconfig

diff --git a/MAINTAINERS b/MAINTAINERS
index 36a2b69fcb..e7c9a7359d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1377,6 +1377,8 @@ F:configs/am65x_hs_evm_r5_defconfig
 F: configs/am65x_hs_evm_a53_defconfig
 F: configs/j721e_hs_evm_r5_defconfig
 F: configs/j721e_hs_evm_a72_defconfig
+F: configs/j7200_hs_evm_r5_defconfig
+F: configs/j7200_hs_evm_a72_defconfig
 
 TPM DRIVERS
 M: Ilias Apalodimas 
diff --git a/configs/j7200_hs_evm_a72_defconfig 
b/configs/j7200_hs_evm_a72_defconfig
new file mode 100644
index 00..0583a2ab62
--- /dev/null
+++ b/configs/j7200_hs_evm_a72_defconfig
@@ -0,0 +1,208 @@
+CONFIG_ARM=y
+CONFIG_ARCH_K3=y
+CONFIG_TI_SECURE_DEVICE=y
+CONFIG_SYS_MALLOC_LEN=0x200
+CONFIG_SYS_MALLOC_F_LEN=0x8000
+CONFIG_SPL_GPIO=y
+CONFIG_SPL_LIBCOMMON_SUPPORT=y
+CONFIG_SPL_LIBGENERIC_SUPPORT=y
+CONFIG_NR_DRAM_BANKS=2
+CONFIG_SOC_K3_J721E=y
+CONFIG_TARGET_J7200_A72_EVM=y
+CONFIG_ENV_SIZE=0x2
+CONFIG_ENV_OFFSET=0x68
+CONFIG_DM_GPIO=y
+CONFIG_SPL_DM_SPI=y
+CONFIG_DEFAULT_DEVICE_TREE="k3-j7200-common-proc-board"
+CONFIG_SPL_TEXT_BASE=0x8008
+CONFIG_SPL_MMC=y
+CONFIG_SPL_SERIAL=y
+CONFIG_SPL_DRIVERS_MISC=y
+CONFIG_SPL_STACK_R_ADDR=0x8200
+CONFIG_ENV_OFFSET_REDUND=0x6A
+CONFIG_SPL_FS_FAT=y
+CONFIG_SPL_LIBDISK_SUPPORT=y
+CONFIG_SPL_SPI_FLASH_SUPPORT=y
+CONFIG_SPL_SPI=y
+# CONFIG_PSCI_RESET is not set
+CONFIG_DISTRO_DEFAULTS=y
+CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
+CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x8048
+# CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
+CONFIG_SPL_LOAD_FIT=y
+CONFIG_SPL_LOAD_FIT_ADDRESS=0x8100
+CONFIG_FIT_IMAGE_POST_PROCESS=y
+CONFIG_SPL_FIT_IMAGE_POST_PROCESS=y
+# CONFIG_USE_SPL_FIT_GENERATOR is not set
+CONFIG_OF_BOARD_SETUP=y
+CONFIG_BOOTCOMMAND="run findfdt; run envboot; run init_${boot}; run 
boot_rprocs; run get_fit_${boot}; run get_overlaystring; run run_fit"
+CONFIG_LOGLEVEL=7
+CONFIG_SPL_MAX_SIZE=0xc
+CONFIG_SPL_HAS_BSS_LINKER_SECTION=y
+CONFIG_SPL_BSS_START_ADDR=0x80a0
+CONFIG_SPL_BSS_MAX_SIZE=0x8
+CONFIG_SPL_BOARD_INIT=y
+CONFIG_SPL_SYS_MALLOC_SIMPLE=y
+CONFIG_SPL_STACK_R=y
+CONFIG_SYS_SPL_MALLOC=y
+CONFIG_SYS_SPL_MALLOC_SIZE=0x80
+CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y
+CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x1400
+CONFIG_SPL_DMA=y
+CONFIG_SPL_ENV_SUPPORT=y
+CONFIG_SPL_FS_LOAD_PAYLOAD_NAME="u-boot.img"
+CONFIG_SPL_I2C=y
+CONFIG_SPL_DM_MAILBOX=y
+CONFIG_SPL_MTD_SUPPORT=y
+CONFIG_SPL_DM_SPI_FLASH=y
+CONFIG_SPL_NOR_SUPPORT=y
+CONFIG_SPL_DM_RESET=y
+CONFIG_SPL_POWER_DOMAIN=y
+CONFIG_SPL_RAM_SUPPORT=y
+CONFIG_SPL_RAM_DEVICE=y
+# CONFIG_SPL_SPI_FLASH_TINY is not set
+CONFIG_SPL_SPI_FLASH_SFDP_SUPPORT=y
+CONFIG_SPL_SPI_LOAD=y
+CONFIG_SYS_SPI_U_BOOT_OFFS=0x28
+CONFIG_SPL_USB_GADGET=y
+CONFIG_SPL_DFU=y
+CONFIG_SPL_YMODEM_SUPPORT=y
+CONFIG_SYS_MAXARGS=64
+CONFIG_CMD_ASKENV=y
+CONFIG_CMD_DFU=y
+# CONFIG_CMD_FLASH is not set
+CONFIG_CMD_GPIO=y
+CONFIG_CMD_GPT=y
+CONFIG_CMD_I2C=y
+CONFIG_CMD_MMC=y
+CONFIG_CMD_MTD=y
+CONFIG_CMD_REMOTEPROC=y
+CONFIG_CMD_UFS=y
+CONFIG_CMD_USB=y
+CONFIG_CMD_USB_MASS_STORAGE=y
+# CONFIG_CMD_SETEXPR is not set
+CONFIG_CMD_TIME=y
+CONFIG_CMD_EXT4_WRITE=y
+CONFIG_MTDIDS_DEFAULT="nor0=4704.spi.0,nor0=47034000.hyperbus"
+CONFIG_MTDPARTS_DEFAULT="mtdparts=4704.spi.0:512k(ospi.tiboot3),2m(ospi.tispl),4m(ospi.u-boot),128k(ospi.env),128k(ospi.env.backup),1m(ospi.sysfw),-@8m(ospi.rootfs);47034000.hyperbus:512k(hbmc.tiboot3),2m(hbmc.tispl),4m(hbmc.u-boot),256k(hbmc.env),1m(hbmc.sysfw),-@8m(hbmc.rootfs)"
+CONFIG_CMD_UBI=y
+# CONFIG_ISO_PARTITION is not set
+# CONFIG_SPL_EFI_PARTITION is not set
+CONFIG_OF_CONTROL=y
+CONFIG_SPL_OF_CONTROL=y
+CONFIG_SPL_MULTI_DTB_FIT=y
+CONFIG_SPL_MULTI_DTB_FIT_NO_COMPRESSION=y
+CONFIG_ENV_OVERWRITE=y
+CONFIG_ENV_IS_IN_MMC=y
+CONFIG_SYS_REDUNDAND_ENVIRONMENT=y
+CONFIG_SYS_RELOC_GD_ENV_ADDR=y
+CONFIG_NET_RANDOM_ETHADDR=y
+CONFIG_DM=y
+CONFIG_SPL_DM=y
+CONFIG_SPL_DM_SEQ_ALIAS=y
+CONFIG_REGMAP=y
+CONFIG_SPL_REGMAP=y
+CONFIG_SYSCON=y
+CONFIG_SPL_SYSCON=y
+CONFIG_SPL_OF_TRANSLATE=y
+CONFIG_CLK=y
+CONFIG_SPL_CLK=y
+CONFIG_CLK_CCF=y
+CONFIG_CLK_TI_SCI=y
+CONFIG_DFU_MMC=y
+CONFIG_DFU_RAM=y
+CONFIG_DFU_SF=y
+CONFIG_SYS_DFU_DATA_BUF_SIZE=0x4
+CONFIG_SYS_DFU_MAX_FILE_SIZE=0x80
+CONFIG_DMA_CHANNELS=y
+CONFIG_TI_K3_NAVSS_UDMA=y
+CONFIG_USB_FUNCTION_FASTBOOT=y
+CONFIG_FAS

[PATCH 2/2] configs: Add configs for j721s2 High Security EVM

2022-08-30 Thread Jayesh Choudhary
Add j721s2 High Security EVM defconfig.

These configs are same as for the non-secure part, except for:
CONFIG_TI_SECURE_DEVICE option set to 'y'
CONFIG_FIT_IMAGE_POST_PROCESS option set to 'y'
CONFIG_SPL_FIT_IMAGE_POST_PROCESS option set to 'y'
CONFIG_BOOTCOMMAND uses FIT images for booting

Signed-off-by: Jayesh Choudhary 
---
 MAINTAINERS |   2 +
 configs/j721s2_hs_evm_a72_defconfig | 217 
 configs/j721s2_hs_evm_r5_defconfig  | 178 +++
 3 files changed, 397 insertions(+)
 create mode 100644 configs/j721s2_hs_evm_a72_defconfig
 create mode 100644 configs/j721s2_hs_evm_r5_defconfig

diff --git a/MAINTAINERS b/MAINTAINERS
index e7c9a7359d..763b6cd529 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1379,6 +1379,8 @@ F:configs/j721e_hs_evm_r5_defconfig
 F: configs/j721e_hs_evm_a72_defconfig
 F: configs/j7200_hs_evm_r5_defconfig
 F: configs/j7200_hs_evm_a72_defconfig
+F: configs/j721s2_hs_evm_r5_defconfig
+F: configs/j721s2_hs_evm_a72_defconfig
 
 TPM DRIVERS
 M: Ilias Apalodimas 
diff --git a/configs/j721s2_hs_evm_a72_defconfig 
b/configs/j721s2_hs_evm_a72_defconfig
new file mode 100644
index 00..851c98577d
--- /dev/null
+++ b/configs/j721s2_hs_evm_a72_defconfig
@@ -0,0 +1,217 @@
+CONFIG_ARM=y
+CONFIG_ARCH_K3=y
+CONFIG_TI_SECURE_DEVICE=y
+CONFIG_SYS_MALLOC_LEN=0x200
+CONFIG_SYS_MALLOC_F_LEN=0x8000
+CONFIG_SPL_GPIO=y
+CONFIG_SPL_LIBCOMMON_SUPPORT=y
+CONFIG_SPL_LIBGENERIC_SUPPORT=y
+CONFIG_NR_DRAM_BANKS=2
+CONFIG_SOC_K3_J721S2=y
+CONFIG_TARGET_J721S2_A72_EVM=y
+CONFIG_ENV_SIZE=0x2
+CONFIG_ENV_OFFSET=0x68
+CONFIG_DM_GPIO=y
+CONFIG_SPL_DM_SPI=y
+CONFIG_DEFAULT_DEVICE_TREE="k3-j721s2-common-proc-board"
+CONFIG_SPL_TEXT_BASE=0x8008
+CONFIG_SPL_MMC=y
+CONFIG_SPL_SERIAL=y
+CONFIG_SPL_DRIVERS_MISC=y
+CONFIG_SPL_STACK_R_ADDR=0x8200
+CONFIG_ENV_OFFSET_REDUND=0x6A
+CONFIG_SPL_FS_FAT=y
+CONFIG_SPL_LIBDISK_SUPPORT=y
+CONFIG_SPL_SPI_FLASH_SUPPORT=y
+CONFIG_SPL_SPI=y
+# CONFIG_PSCI_RESET is not set
+CONFIG_DISTRO_DEFAULTS=y
+CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
+CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x8048
+# CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
+CONFIG_SPL_LOAD_FIT=y
+CONFIG_SPL_LOAD_FIT_ADDRESS=0x8100
+CONFIG_FIT_IMAGE_POST_PROCESS=y
+CONFIG_SPL_FIT_IMAGE_POST_PROCESS=y
+# CONFIG_USE_SPL_FIT_GENERATOR is not set
+CONFIG_OF_BOARD_SETUP=y
+CONFIG_BOOTCOMMAND="run findfdt; run envboot; run init_${boot}; run 
boot_rprocs; run get_fit_${boot}; run get_overlaystring; run run_fit"
+CONFIG_LOGLEVEL=7
+CONFIG_SPL_MAX_SIZE=0xc
+CONFIG_SPL_HAS_BSS_LINKER_SECTION=y
+CONFIG_SPL_BSS_START_ADDR=0x80a0
+CONFIG_SPL_BSS_MAX_SIZE=0x8
+CONFIG_SPL_BOARD_INIT=y
+CONFIG_SPL_SYS_MALLOC_SIMPLE=y
+CONFIG_SPL_STACK_R=y
+CONFIG_SYS_SPL_MALLOC=y
+CONFIG_SYS_SPL_MALLOC_SIZE=0x80
+CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y
+CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x1400
+CONFIG_SPL_DMA=y
+CONFIG_SPL_ENV_SUPPORT=y
+CONFIG_SPL_FS_LOAD_PAYLOAD_NAME="u-boot.img"
+CONFIG_SPL_I2C=y
+CONFIG_SPL_DM_MAILBOX=y
+CONFIG_SPL_MTD_SUPPORT=y
+CONFIG_SPL_DM_SPI_FLASH=y
+CONFIG_SPL_NOR_SUPPORT=y
+CONFIG_SPL_DM_RESET=y
+CONFIG_SPL_POWER_DOMAIN=y
+CONFIG_SPL_RAM_SUPPORT=y
+CONFIG_SPL_RAM_DEVICE=y
+# CONFIG_SPL_SPI_FLASH_TINY is not set
+CONFIG_SPL_SPI_FLASH_SFDP_SUPPORT=y
+CONFIG_SPL_SPI_LOAD=y
+CONFIG_SYS_SPI_U_BOOT_OFFS=0x28
+CONFIG_SPL_THERMAL=y
+CONFIG_SPL_USB_GADGET=y
+CONFIG_SPL_DFU=y
+CONFIG_SPL_YMODEM_SUPPORT=y
+CONFIG_SYS_MAXARGS=64
+CONFIG_CMD_ASKENV=y
+CONFIG_CMD_DFU=y
+# CONFIG_CMD_FLASH is not set
+CONFIG_CMD_GPIO=y
+CONFIG_CMD_GPT=y
+CONFIG_CMD_I2C=y
+CONFIG_CMD_MMC=y
+CONFIG_CMD_MTD=y
+CONFIG_CMD_REMOTEPROC=y
+CONFIG_CMD_UFS=y
+CONFIG_CMD_USB=y
+CONFIG_CMD_USB_MASS_STORAGE=y
+# CONFIG_CMD_SETEXPR is not set
+CONFIG_CMD_TIME=y
+CONFIG_CMD_EXT4_WRITE=y
+CONFIG_MTDIDS_DEFAULT="nor0=4704.spi.0,nor0=47034000.hyperbus"
+CONFIG_MTDPARTS_DEFAULT="mtdparts=4704.spi.0:512k(ospi.tiboot3),2m(ospi.tispl),4m(ospi.u-boot),256k(ospi.env),256k(ospi.env.backup),57088k@8m(ospi.rootfs),256k(ospi.phypattern);47034000.hyperbus:512k(hbmc.tiboot3),2m(hbmc.tispl),4m(hbmc.u-boot),256k(hbmc.env),-@8m(hbmc.rootfs)"
+CONFIG_CMD_UBI=y
+# CONFIG_ISO_PARTITION is not set
+# CONFIG_SPL_EFI_PARTITION is not set
+CONFIG_OF_CONTROL=y
+CONFIG_SPL_OF_CONTROL=y
+CONFIG_SPL_MULTI_DTB_FIT=y
+CONFIG_SPL_MULTI_DTB_FIT_NO_COMPRESSION=y
+CONFIG_ENV_OVERWRITE=y
+CONFIG_ENV_IS_IN_MMC=y
+CONFIG_SYS_REDUNDAND_ENVIRONMENT=y
+CONFIG_SYS_RELOC_GD_ENV_ADDR=y
+CONFIG_NET_RANDOM_ETHADDR=y
+CONFIG_DM=y
+CONFIG_SPL_DM=y
+CONFIG_SPL_DM_SEQ_ALIAS=y
+CONFIG_REGMAP=y
+CONFIG_SPL_REGMAP=y
+CONFIG_SYSCON=y
+CONFIG_SPL_SYSCON=y
+CONFIG_SPL_OF_TRANSLATE=y
+CONFIG_CLK=y
+CONFIG_SPL_CLK=y
+CONFIG_CLK_CCF=y
+CONFIG_CLK_TI_SCI=y
+CONFIG_DFU_MMC=y
+CONFIG_DFU_RAM=y
+CONFIG_DFU_SF=y
+CONFIG_SYS_DFU_DATA_BUF_SIZE=0x4
+CONFIG_SYS_DFU_MAX_FILE_SIZE=0x80
+CONFIG_DMA_CHANNELS=y
+CONFIG_TI_K3_NAVSS_UDMA=y
+CONFIG_USB_FUNCTI

Re: [PATCH v2] pci: Do not enable PCIe GEN3 link retrain workaround by default

2022-08-30 Thread Stefan Roese

On 30.08.22 11:19, Pali Rohár wrote:

On Tuesday 30 August 2022 10:04:51 Maciej W. Rozycki wrote:

On Sat, 27 Aug 2022, Pali Rohár wrote:


Moreover this workaround is enabled for all existing hardware and also all
future PCIe hardware, which opens a hole that other PCIe vendors may
introduce same HW issue as on systems where this workaround is required and
nobody would notice it because U-Boot automatically apply workaround for it.


  Why is it a problem?


I think I wrote it. One issue is that it is increasing size of SPL image
and we really should not include into SPL things which are not required
for all target platforms. Lot of boards have size constrained memory
requirements and unnecessary features should not be automatically
enabled.


I have to agree with Pali here. We need to be careful with size increase
in the SPL images, as some of the build targets are very limited here.
So making this workaround configurable is definitely a good idea.

The question remains, at least for me, if the Kconfig option should be
enable per default or not. For SPL my suggestions is to disable is per
default because of the size remarks above. For U-Boot proper I'm not so
sure. Please see below...


Is the intent to cause hassle to end users and
force them to take action when they have a non-working piece of hardware?


Vendors should really test their hardware without any automatic
workaround. Otherwise we are going into the hell if some workarounds are
automatically enabled and nobody notice broken behavior. And vendors
really build software with default options and do not care about it if
default options are suitable.

There is already direction to make all workaround targeted and enabled
only for platforms / hardware which really need it.

So enabling some workaround for all platforms which are produced and
will be produced in the future is step backward.


All this makes sense to me. You both have good points AFAICT. Please
see below...


  I'd say in 99% of cases this will only cause frustration and they won't
bother.  They will just conclude that either piece of hardware involved is
broken and will throw it away.  Just as I almost did.  The seller has
offered me a refund, which seems thought to be a universal solution
nowadays (but I need to do what I meant to and getting money back doesn't
solve it).

  And at least I know what U-boot (or indeed firmware) is and have a
general understanding of how computers work.  Most people just want to
plug stuff in and use it for whatever their need is.  Expecting them to
take action to get things working is wasting their time (which BTW seems
to have been a growing trend in last ~30 years: putting burden on the end
user to get our problems solved, which saves our time and money at the
expense of end user's).


The another issue here is that it was not fully investigated where the
issue is. If it is processor specific, PCIe switch specific or endpoint
specific, or combination of these options. It was just observed that
proposed workaround fix this issue on one specific combination. And you
confirmed this in previous post, that you are unsure if it is not
specific to downstream ports of the switch.

We really should not include such "bloatware" code to be enabled for
everything, on every one board. You are the first who reported this
issue. Nobody else complained about it, so I really do not see reason
why all other users and developers must be forced to have it in their
U-Boot binaries. Moreover lot of boards on which is U-Boot running do
not have PCIe bus exported on the slot and have PCIe devices integrated
and soldered on the board. Why on the earth I have to need this
workaround also on these boards (which are moreover sized constrained)?


Agreed. But I also understand the reasoning from Maciej, at least in
parts. Thinking a bit more about this, my preference would be to still
include this workaround per default in U-Boot proper though. To not
make things too complicated here.

Just my 0.02$.


  NB I'm slowly getting fed up with the amount of non-working stuff piling
up around.  Every other piece of equipment I try doesn't work for one
reason or another and I need to either chase bugs myself or to spend days
and weeks to persuade someone at least to believe a problem is there to
get that sorted.  All in my free time I'd rather spend on something else.
I'd welcome things working automagically for a change so that I could
focus on what I mean to be doing, and therefore I take breaking things
deliberately as a major offence.

  FWIW,

   Maciej


What other U-Boot developers think?


Please see above.

Thanks,
Stefan


Re: [PATCH 4/4] arm: dts: chameleonv3: Add 270-2 variant

2022-08-30 Thread Alexandru M Stan
Hey Simon,

On Mon, Aug 29, 2022 at 7:29 PM Simon Glass  wrote:
>
> Hi Paweł,
>
> On Mon, 29 Aug 2022 at 02:23, Paweł Anikiel  wrote:
> >
> > On Sat, Aug 27, 2022 at 2:22 AM Simon Glass  wrote:
> > >
> > > Hi Paweł,
> > >
> > > On Fri, 26 Aug 2022 at 01:54, Paweł Anikiel  wrote:
> > > >
> > > > Add devicetree for chameleonv3 with the 270-2I2-D11E variant of the
> > > > Mercury+ AA1 module
> > > >
> > > > Signed-off-by: Paweł Anikiel 
> > > > ---
> > > >  arch/arm/dts/Makefile|  1 +
> > > >  .../socfpga_arria10_chameleonv3_270_2-u-boot.dtsi| 12 
> > > >  arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts   |  5 +
> > > >  3 files changed, 18 insertions(+)
> > > >  create mode 100644 
> > > > arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
> > > >  create mode 100644 arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
> > > >
>
>
> > > > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> > > > index 7330121dba..36d5d65595 100644
> > > > --- a/arch/arm/dts/Makefile
> > > > +++ b/arch/arm/dts/Makefile
> > > > @@ -425,6 +425,7 @@ dtb-$(CONFIG_ARCH_SOCFPGA) +=   
> > > > \
> > > > socfpga_agilex_socdk.dtb\
> > > > socfpga_arria5_secu1.dtb\
> > > > socfpga_arria5_socdk.dtb\
> > > > +   socfpga_arria10_chameleonv3_270_2.dtb   \
> > > > socfpga_arria10_chameleonv3_270_3.dtb   \
> > > > socfpga_arria10_chameleonv3_480_2.dtb   \
> > > > socfpga_arria10_socdk_sdmmc.dtb \
> > > > diff --git a/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi 
> > > > b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
> > > > new file mode 100644
> > > > index 00..05b4485cf3
> > > > --- /dev/null
> > > > +++ b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
> > > > @@ -0,0 +1,12 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * Copyright 2022 Google LLC
> > > > + */
> > > > +#include "socfpga_arria10_chameleonv3_480_2_handoff.h"
> > > > +#include "socfpga_arria10-handoff.dtsi"
> > > > +#include "socfpga_arria10_handoff_u-boot.dtsi"
> > > > +#include "socfpga_arria10_mercury_aa1-u-boot.dtsi"
> > > > +
> > > > +&fpga_mgr {
> > > > +   altr,bitstream = "fpga-270-2.itb";
> > > > +};
> > > > diff --git a/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts 
> > > > b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
> > > > new file mode 100644
> > > > index 00..5f40af6eb9
> > > > --- /dev/null
> > > > +++ b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
> > > > @@ -0,0 +1,5 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * Copyright 2022 Google LLC
> > > > + */
> > > > +#include "socfpga_arria10_chameleonv3.dts"
> > >
> > > Can you create a common .dtsi file instead? We should not be including
> > > a .dts file into another file.
> > >
> > Do you mean renaming chameleonv3.dts to .dtsi? In Linux it's a .dts,
> > because nothing includes it (no handoff headers are needed). Is it
> > fine to have the names differ across U-Boot and Linux?
>
> Ideally not, but we should not include a .dts file in another one and
> it is probably more important to follow that rule. But why is Linux
> not getting this variant?
>
> Regards,
> Simon

Linux (at least for the near future) does not care about which variant
it is. The big differences between 270, 480, -2, -3 are mostly about
the number of FPGA logic gates and speed grades. Such things affect
the FPGA bitstream greatly, and might even affect clock presets that
u-boot cares about, but by the time linux loads it doesn't matter
anymore.

Alexandru Stan (amstan)


RE: [PATCH v2 3/4] cmd: bdinfo: introduce bdinfo_print_size() helper

2022-08-30 Thread Jason Liu


> -Original Message-
> From: Ovidiu Panait 
> Sent: 2022年8月30日 1:02
> To: u-boot@lists.denx.de
> Cc: Ovidiu Panait ; Simon Glass ;
> Andy Shevchenko ; Dzmitry Sankouski
> ; Heinrich Schuchardt ; Jason
> Liu 
> Subject: [PATCH v2 3/4] cmd: bdinfo: introduce bdinfo_print_size() helper
> 
> Add bdinfo_print_size() helper to display size variables (such as cache
> sizes) in bdinfo format. The size is printed as "xxx Bytes", "xxx KiB",
"xxx MiB",
> "xxx GiB", etc as needed;
> 
> Reviewed-by: Simon Glass 
> Signed-off-by: Ovidiu Panait 
> ---
> 
> Changes in v2:
> Added "Reviewed-by" tag from Simon.
> 
>  cmd/bdinfo.c   |  7 +++
>  include/init.h | 13 +
>  2 files changed, 20 insertions(+)
> 
The patch looks good to me.

Reviewed-by: Jason Liu 

> diff --git a/cmd/bdinfo.c b/cmd/bdinfo.c index 37cd8a57eb..9e23c4dd8f
> 100644
> --- a/cmd/bdinfo.c
> +++ b/cmd/bdinfo.c
> @@ -16,9 +16,16 @@
>  #include 
>  #include 
>  #include 
> +#include 
> 
>  DECLARE_GLOBAL_DATA_PTR;
> 
> +void bdinfo_print_size(const char *name, uint64_t size) {
> + printf("%-12s= ", name);
> + print_size(size, "\n");
> +}
> +
>  void bdinfo_print_num_l(const char *name, ulong value)  {
>   printf("%-12s= 0x%0*lx\n", name, 2 * (int)sizeof(value), value);
diff --git
> a/include/init.h b/include/init.h index 7b8f62c121..02bb4ce13e 100644
> --- a/include/init.h
> +++ b/include/init.h
> @@ -343,6 +343,19 @@ void bdinfo_print_num_ll(const char *name,
> unsigned long long value);
>  /* Print a clock speed in MHz */
>  void bdinfo_print_mhz(const char *name, unsigned long hz);
> 
> +/**
> + * bdinfo_print_size - print size variables in bdinfo format
> + * @name:string to print before the size
> + * @size:size to print
> + *
> + * Helper function for displaying size variables as properly formatted
> +bdinfo
> + * entries. The size is printed as "xxx Bytes", "xxx KiB", "xxx MiB",
> + * "xxx GiB", etc. as needed;
> + *
> + * For use in arch_print_bdinfo().
> + */
> +void bdinfo_print_size(const char *name, uint64_t size);
> +
>  /* Show arch-specific information for the 'bd' command */  void
> arch_print_bdinfo(void);
> 
> --
> 2.25.1



smime.p7s
Description: S/MIME cryptographic signature


[PATCH 016/347] FogBugz #516535: Fix QSPI write issues

2022-08-30 Thread Jit Loon Lim
From: Chee Hong Ang 

QSPI driver perform chip select on every flash read/write
access. The driver need to disable/enable the QSPI controller
while performing chip select. This may cause some data lost
especially the QSPI controller is configured to run at slower
speed as it may take longer time to access the flash device.
This patch prevent the driver from disable/enable the QSPI
controller too soon and inadvertently halting any ongoing flash
read/write access by ensuring the QSPI controller is always in
idle mode after each read/write access.

Signed-off-by: Chee Hong Ang 
---
 drivers/spi/cadence_qspi_apb.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index 2cdf4c9c9f..5e03495f45 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -858,13 +858,9 @@ cadence_qspi_apb_indirect_read_execute(struct 
cadence_spi_plat *plat,
writel(CQSPI_REG_INDIRECTRD_DONE,
   plat->regbase + CQSPI_REG_INDIRECTRD);
 
-   /* Check indirect done status */
-   ret = wait_for_bit_le32(plat->regbase + CQSPI_REG_INDIRECTRD,
-   CQSPI_REG_INDIRECTRD_DONE, 0, 10, 0);
-   if (ret) {
-   printf("Indirect read clear completion error (%i)\n", ret);
-   goto failrd;
-   }
+   /* Wait til QSPI is idle */
+   if (!cadence_qspi_wait_idle(plat->regbase))
+   return -EIO;
 
return 0;
 
@@ -1031,6 +1027,11 @@ cadence_qspi_apb_indirect_write_execute(struct 
cadence_spi_plat *plat,
 
if (bounce_buf)
free(bounce_buf);
+
+   /* Wait til QSPI is idle */
+   if (!cadence_qspi_wait_idle(plat->regbase))
+   return -EIO;
+
return 0;
 
 failwr:
-- 
2.25.1



[PATCH] ddr: socfpga: Fix integer overflow

2022-08-30 Thread Jit Loon Lim
From: Tien Fong Chee 

dramaddrw data structure are defined as u32. Compiler assume 32-bit
arithmetic on expression and the shifting operation to be done on a
32-bit value. So, the expression would overflow if the shifting
is more than 32-bit.

Signed-off-by: Tien Fong Chee 
---
 drivers/ddr/altera/sdram_soc64.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ddr/altera/sdram_soc64.c b/drivers/ddr/altera/sdram_soc64.c
index 9b1710c135..81ad02118b 100644
--- a/drivers/ddr/altera/sdram_soc64.c
+++ b/drivers/ddr/altera/sdram_soc64.c
@@ -246,7 +246,7 @@ phys_size_t sdram_calculate_size(struct altera_sdram_plat 
*plat)
 DRAMADDRW_CFG_ROW_ADDR_WIDTH(dramaddrw) +
 DRAMADDRW_CFG_COL_ADDR_WIDTH(dramaddrw));
 
-   size *= (2 << (hmc_ecc_readl(plat, DDRIOCTRL) &
+   size *= ((phys_size_t)2 << (hmc_ecc_readl(plat, DDRIOCTRL) &
DDR_HMC_DDRIOCTRL_IOSIZE_MSK));
 
return size;
-- 
2.26.2



Re: [PATCH 4/4] arm: dts: chameleonv3: Add 270-2 variant

2022-08-30 Thread Paweł Anikiel
On Tue, Aug 30, 2022 at 5:13 AM Alexandru M Stan  wrote:
>
> Hey Simon,
>
> On Mon, Aug 29, 2022 at 7:29 PM Simon Glass  wrote:
> >
> > Hi Paweł,
> >
> > On Mon, 29 Aug 2022 at 02:23, Paweł Anikiel  wrote:
> > >
> > > On Sat, Aug 27, 2022 at 2:22 AM Simon Glass  wrote:
> > > >
> > > > Hi Paweł,
> > > >
> > > > On Fri, 26 Aug 2022 at 01:54, Paweł Anikiel  wrote:
> > > > >
> > > > > Add devicetree for chameleonv3 with the 270-2I2-D11E variant of the
> > > > > Mercury+ AA1 module
> > > > >
> > > > > Signed-off-by: Paweł Anikiel 
> > > > > ---
> > > > >  arch/arm/dts/Makefile|  1 +
> > > > >  .../socfpga_arria10_chameleonv3_270_2-u-boot.dtsi| 12 
> > > > > 
> > > > >  arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts   |  5 +
> > > > >  3 files changed, 18 insertions(+)
> > > > >  create mode 100644 
> > > > > arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
> > > > >  create mode 100644 arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
> > > > >
> >
> >
> > > > > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> > > > > index 7330121dba..36d5d65595 100644
> > > > > --- a/arch/arm/dts/Makefile
> > > > > +++ b/arch/arm/dts/Makefile
> > > > > @@ -425,6 +425,7 @@ dtb-$(CONFIG_ARCH_SOCFPGA) += 
> > > > >   \
> > > > > socfpga_agilex_socdk.dtb\
> > > > > socfpga_arria5_secu1.dtb\
> > > > > socfpga_arria5_socdk.dtb\
> > > > > +   socfpga_arria10_chameleonv3_270_2.dtb   \
> > > > > socfpga_arria10_chameleonv3_270_3.dtb   \
> > > > > socfpga_arria10_chameleonv3_480_2.dtb   \
> > > > > socfpga_arria10_socdk_sdmmc.dtb \
> > > > > diff --git 
> > > > > a/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi 
> > > > > b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
> > > > > new file mode 100644
> > > > > index 00..05b4485cf3
> > > > > --- /dev/null
> > > > > +++ b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
> > > > > @@ -0,0 +1,12 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > +/*
> > > > > + * Copyright 2022 Google LLC
> > > > > + */
> > > > > +#include "socfpga_arria10_chameleonv3_480_2_handoff.h"
> > > > > +#include "socfpga_arria10-handoff.dtsi"
> > > > > +#include "socfpga_arria10_handoff_u-boot.dtsi"
> > > > > +#include "socfpga_arria10_mercury_aa1-u-boot.dtsi"
> > > > > +
> > > > > +&fpga_mgr {
> > > > > +   altr,bitstream = "fpga-270-2.itb";
> > > > > +};
> > > > > diff --git a/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts 
> > > > > b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
> > > > > new file mode 100644
> > > > > index 00..5f40af6eb9
> > > > > --- /dev/null
> > > > > +++ b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
> > > > > @@ -0,0 +1,5 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > +/*
> > > > > + * Copyright 2022 Google LLC
> > > > > + */
> > > > > +#include "socfpga_arria10_chameleonv3.dts"
> > > >
> > > > Can you create a common .dtsi file instead? We should not be including
> > > > a .dts file into another file.
> > > >
> > > Do you mean renaming chameleonv3.dts to .dtsi? In Linux it's a .dts,
> > > because nothing includes it (no handoff headers are needed). Is it
> > > fine to have the names differ across U-Boot and Linux?
> >
> > Ideally not, but we should not include a .dts file in another one and
> > it is probably more important to follow that rule. But why is Linux
> > not getting this variant?
> >
> > Regards,
> > Simon
>
> Linux (at least for the near future) does not care about which variant
> it is. The big differences between 270, 480, -2, -3 are mostly about
> the number of FPGA logic gates and speed grades. Such things affect
> the FPGA bitstream greatly, and might even affect clock presets that
> u-boot cares about, but by the time linux loads it doesn't matter
> anymore.

Perhaps a more detailed explanation:

The Main and Peripheral PLLs (as well as some other clocks) are
configured by U-Boot. On the other hand, Linux expects them to be
configured when it boots, and does not touch them.

The clock configuration depends mainly on the speed grade of the Arria
10 SoC (marked by us as -2 and -3), but also on the fpga hardware
design (e.g. user-defined clocks for the fpga), and is included in the
u-boot devicetree:
> +#include "socfpga_arria10_chameleonv3_480_2_handoff.h"
> +#include "socfpga_arria10-handoff.dtsi"
> +#include "socfpga_arria10_handoff_u-boot.dtsi"

Linux, on the other hand, doesn't need such information, and there is
no distinction between the different chameleon variants.

Regards,
Paweł


[PATCH 0/6] Enable CONFIG_TIMER for all Kirwood / MVEBU boards

2022-08-30 Thread Stefan Roese
This patchset enhaces the recently added Orion Timer driver to support
all other Kirkwood & 32bit MVEBU Armada platforms. Additionally, this
timer support is then enabled per default for those platforms, so that
the board config files don't need to be changed. Also necessary is
some dts hacking, so that the timer DT node is available in early
U-Boot stages.

I've successfully tested this patchset on an Armada XP board. Additional
test on other boards and platforms are very welcome and necessary.

Thanks,
Stefan

Stefan Roese (6):
  timer: orion-timer: Add support for other Armada SoC's
  timer: orion-timer: Add timer_get_boot_us() for BOOTSTAGE support
  arm: mvebu: Use CONFIG_TIMER on all MVEBU & KIRKWOOD platforms
  arm: mvebu: dts: Makefile: Compile Armada 375 dtb in a separate step
  arm: mvebu: dts: armada-375.dtsi: Add timer0 & timer1
  arm: mvebu: dts: mvebu-u-boot.dtsi: Add "u-boot,dm-pre-reloc" to timer
DT node

 arch/arm/Kconfig  |  4 ++
 arch/arm/dts/Makefile |  6 ++-
 arch/arm/dts/armada-375.dtsi  |  4 +-
 arch/arm/dts/mvebu-u-boot.dtsi| 11 
 arch/arm/mach-mvebu/include/mach/config.h |  5 --
 drivers/timer/Kconfig |  5 +-
 drivers/timer/orion-timer.c   | 66 +--
 7 files changed, 89 insertions(+), 12 deletions(-)

-- 
2.37.2



[PATCH 1/6] timer: orion-timer: Add support for other Armada SoC's

2022-08-30 Thread Stefan Roese
This patch adds support for other Marvell Armada SoC's, supporting the
25MHz fixed clock operation, like the Armada XP etc.

Signed-off-by: Stefan Roese 
---
 drivers/timer/Kconfig   |  5 -
 drivers/timer/orion-timer.c | 44 ++---
 2 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig
index 404929014810..b0814acca3fb 100644
--- a/drivers/timer/Kconfig
+++ b/drivers/timer/Kconfig
@@ -197,8 +197,11 @@ config OMAP_TIMER
 config ORION_TIMER
bool "Orion timer support"
depends on TIMER
+   default y if ARCH_KIRKWOOD || (ARCH_MVEBU && ARMADA_32BIT)
+   select TIMER_EARLY if ARCH_MVEBU
help
- Select this to enable an timer for Orion devices.
+ Select this to enable an timer for Orion and Armada devices
+ like Armada XP etc.
 
 config RISCV_TIMER
bool "RISC-V timer support"
diff --git a/drivers/timer/orion-timer.c b/drivers/timer/orion-timer.c
index fd30e1bf036c..02ed138642b8 100644
--- a/drivers/timer/orion-timer.c
+++ b/drivers/timer/orion-timer.c
@@ -11,10 +11,36 @@
 #define TIMER0_RELOAD  0x10
 #define TIMER0_VAL 0x14
 
+enum input_clock_type {
+   INPUT_CLOCK_NON_FIXED,
+   INPUT_CLOCK_25MHZ,  /* input clock rate is fixed to 25MHz */
+};
+
 struct orion_timer_priv {
void *base;
 };
 
+#define MVEBU_TIMER_FIXED_RATE_25MHZ   2500
+
+#if defined(CONFIG_ARCH_MVEBU) && IS_ENABLED(CONFIG_TIMER_EARLY)
+/**
+ * timer_early_get_rate() - Get the timer rate before driver model
+ */
+unsigned long notrace timer_early_get_rate(void)
+{
+   return MVEBU_TIMER_FIXED_RATE_25MHZ;
+}
+
+/**
+ * timer_early_get_count() - Get the timer count before driver model
+ *
+ */
+u64 notrace timer_early_get_count(void)
+{
+   return ~readl(MVEBU_TIMER_BASE + TIMER0_VAL);
+}
+#endif
+
 static uint64_t orion_timer_get_count(struct udevice *dev)
 {
struct orion_timer_priv *priv = dev_get_priv(dev);
@@ -25,6 +51,7 @@ static uint64_t orion_timer_get_count(struct udevice *dev)
 static int orion_timer_probe(struct udevice *dev)
 {
struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);
+   enum input_clock_type type = dev_get_driver_data(dev);
struct orion_timer_priv *priv = dev_get_priv(dev);
 
priv->base = devfdt_remap_addr_index(dev, 0);
@@ -33,11 +60,20 @@ static int orion_timer_probe(struct udevice *dev)
return -ENOMEM;
}
 
-   uc_priv->clock_rate = CONFIG_SYS_TCLK;
-
writel(~0, priv->base + TIMER0_VAL);
writel(~0, priv->base + TIMER0_RELOAD);
 
+   if (type == INPUT_CLOCK_25MHZ) {
+   /*
+* On Armada XP / 38x ..., the 25MHz clock source needs to
+* be enabled
+*/
+   setbits_le32(priv->base + TIMER_CTRL, BIT(11));
+   uc_priv->clock_rate = MVEBU_TIMER_FIXED_RATE_25MHZ;
+   } else {
+   uc_priv->clock_rate = CONFIG_SYS_TCLK;
+   }
+
/* enable timer */
setbits_le32(priv->base + TIMER_CTRL, TIMER0_EN | TIMER0_RELOAD_EN);
 
@@ -49,7 +85,9 @@ static const struct timer_ops orion_timer_ops = {
 };
 
 static const struct udevice_id orion_timer_ids[] = {
-   { .compatible = "marvell,orion-timer" },
+   { .compatible = "marvell,orion-timer", .data = INPUT_CLOCK_NON_FIXED },
+   { .compatible = "marvell,armada-370-timer", .data = INPUT_CLOCK_25MHZ },
+   { .compatible = "marvell,armada-xp-timer", .data = INPUT_CLOCK_25MHZ },
{}
 };
 
-- 
2.37.2



[PATCH 4/6] arm: mvebu: dts: Makefile: Compile Armada 375 dtb in a separate step

2022-08-30 Thread Stefan Roese
This patch changes the compilation, so that the Armada 375 board(s) are
compiled in a separate step. This is necessary for the timer dts
conversion, as A375 has a different / timer description in the dts.

Signed-off-by: Stefan Roese 
---
 arch/arm/dts/Makefile | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index 7330121dbaba..03e79e14681f 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -233,8 +233,11 @@ dtb-$(CONFIG_ARCH_TEGRA) += tegra20-harmony.dtb \
tegra210-p3450-.dtb
 
 ifdef CONFIG_ARMADA_32BIT
+ifdef CONFIG_ARMADA_375
+dtb-$(CONFIG_ARCH_MVEBU) +=\
+   armada-375-db.dtb
+else
 dtb-$(CONFIG_ARCH_MVEBU) +=\
-   armada-375-db.dtb   \
armada-385-atl-x530.dtb \
armada-385-atl-x530DP.dtb   \
armada-385-db-88f6820-amc.dtb   \
@@ -254,6 +257,7 @@ dtb-$(CONFIG_ARCH_MVEBU) += \
armada-xp-maxbcm.dtb\
armada-xp-synology-ds414.dtb\
armada-xp-theadorable.dtb
+endif
 else
 dtb-$(CONFIG_ARCH_MVEBU) +=\
armada-3720-db.dtb  \
-- 
2.37.2



[PATCH 5/6] arm: mvebu: dts: armada-375.dtsi: Add timer0 & timer1

2022-08-30 Thread Stefan Roese
Add the DT bindings / descriptions for timer0 & timer1, exactly as done
in mainline Linux.

Signed-off-by: Stefan Roese 
---
 arch/arm/dts/armada-375.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/dts/armada-375.dtsi b/arch/arm/dts/armada-375.dtsi
index 20a8c352b2f1..a044b3fc994f 100644
--- a/arch/arm/dts/armada-375.dtsi
+++ b/arch/arm/dts/armada-375.dtsi
@@ -187,7 +187,7 @@
reg = <0xc000 0x58>;
};
 
-   timer@c600 {
+   timer0: timer@c600 {
compatible = "arm,cortex-a9-twd-timer";
reg = <0xc600 0x20>;
interrupts = ;
@@ -416,7 +416,7 @@
interrupts = ;
};
 
-   timer@20300 {
+   timer1: timer@20300 {
compatible = "marvell,armada-375-timer", 
"marvell,armada-370-timer";
reg = <0x20300 0x30>, <0x21040 0x30>;
interrupts-extended = <&gic  GIC_SPI  8 
IRQ_TYPE_LEVEL_HIGH>,
-- 
2.37.2



[PATCH 2/6] timer: orion-timer: Add timer_get_boot_us() for BOOTSTAGE support

2022-08-30 Thread Stefan Roese
Add timer_get_boot_us() to support boards, that have CONFIG_BOOTSTAGE
enabled, like pogo_v4.

Signed-off-by: Stefan Roese 
---
 drivers/timer/orion-timer.c | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/drivers/timer/orion-timer.c b/drivers/timer/orion-timer.c
index 02ed138642b8..7e920eaeaa40 100644
--- a/drivers/timer/orion-timer.c
+++ b/drivers/timer/orion-timer.c
@@ -41,6 +41,28 @@ u64 notrace timer_early_get_count(void)
 }
 #endif
 
+#if CONFIG_IS_ENABLED(BOOTSTAGE)
+ulong timer_get_boot_us(void)
+{
+   u64 ticks = 0;
+   u32 rate = 1;
+   u64 us;
+   int ret;
+
+   ret = dm_timer_init();
+   if (!ret) {
+   /* The timer is available */
+   rate = timer_get_rate(gd->timer);
+   timer_get_count(gd->timer, &ticks);
+   } else {
+   return 0;
+   }
+
+   us = (ticks * 1000) / rate;
+   return us;
+}
+#endif
+
 static uint64_t orion_timer_get_count(struct udevice *dev)
 {
struct orion_timer_priv *priv = dev_get_priv(dev);
-- 
2.37.2



[PATCH 3/6] arm: mvebu: Use CONFIG_TIMER on all MVEBU & KIRKWOOD platforms

2022-08-30 Thread Stefan Roese
Now that the new timer support is available for these platforms, let's
select this IF for all these platforms. This way it's not necessary
that each board changes it's config header.

Signed-off-by: Stefan Roese 
---
 arch/arm/Kconfig  | 4 
 arch/arm/mach-mvebu/include/mach/config.h | 5 -
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 0b72e4f6503e..60f524a2d118 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -618,6 +618,7 @@ config ARCH_KIRKWOOD
select BOARD_EARLY_INIT_F
select CPU_ARM926EJS
select GPIO_EXTRA_HEADER
+   select TIMER
 
 config ARCH_MVEBU
bool "Marvell MVEBU family (Armada XP/375/38x/3700/7K/8K)"
@@ -629,6 +630,8 @@ config ARCH_MVEBU
select GPIO_EXTRA_HEADER
select SPL_DM_SPI if SPL
select SPL_DM_SPI_FLASH if SPL
+   select SPL_TIMER if SPL
+   select TIMER
select OF_CONTROL
select OF_SEPARATE
select SPI
@@ -639,6 +642,7 @@ config ARCH_ORION5X
select CPU_ARM926EJS
select GPIO_EXTRA_HEADER
select SPL_SEPARATE_BSS if SPL
+   select TIMER
 
 config TARGET_STV0991
bool "Support stv0991"
diff --git a/arch/arm/mach-mvebu/include/mach/config.h 
b/arch/arm/mach-mvebu/include/mach/config.h
index 4add0d9e1030..9b5036c31dd3 100644
--- a/arch/arm/mach-mvebu/include/mach/config.h
+++ b/arch/arm/mach-mvebu/include/mach/config.h
@@ -41,9 +41,4 @@
 #endif
 #endif
 
-/* Use common timer */
-#define CONFIG_SYS_TIMER_COUNTS_DOWN
-#define CONFIG_SYS_TIMER_COUNTER   (MVEBU_TIMER_BASE + 0x14)
-#define CONFIG_SYS_TIMER_RATE  2500
-
 #endif /* __MVEBU_CONFIG_H */
-- 
2.37.2



[PATCH 6/6] arm: mvebu: dts: mvebu-u-boot.dtsi: Add "u-boot, dm-pre-reloc" to timer DT node

2022-08-30 Thread Stefan Roese
Adding the "u-boot,dm-pre-reloc" DT property to the timer node is
necesssary to support the timer in the early boot phases (e.g.
SPL & pre-reloc).

Signed-off-by: Stefan Roese 
---
 arch/arm/dts/mvebu-u-boot.dtsi | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/dts/mvebu-u-boot.dtsi b/arch/arm/dts/mvebu-u-boot.dtsi
index 5538f95148de..db4bf39920b1 100644
--- a/arch/arm/dts/mvebu-u-boot.dtsi
+++ b/arch/arm/dts/mvebu-u-boot.dtsi
@@ -15,6 +15,17 @@
u-boot,dm-pre-reloc;
 };
 
+#ifdef CONFIG_ARMADA_375
+/* Armada 375 has multiple timers, use timer1 here */
+&timer1 {
+   u-boot,dm-pre-reloc;
+};
+#else
+&timer {
+   u-boot,dm-pre-reloc;
+};
+#endif
+
 #ifdef CONFIG_SPL_SPI
 &spi0 {
u-boot,dm-pre-reloc;
-- 
2.37.2



Re: [PATCH v14 01/10] eficonfig: menu-driven addition of UEFI boot option

2022-08-30 Thread Ilias Apalodimas
Hi Kojima-san,

[...]
> +/**
> + * eficonfig_choice_entry() - user key input handler
> + *
> + * @data:pointer to the efimenu structure
> + * Return:   key string to identify the selected entry
> + */
> +static char *eficonfig_choice_entry(void *data)
> +{
> + int esc = 0;
> + struct list_head *pos, *n;
> + struct eficonfig_entry *entry;
> + enum bootmenu_key key = KEY_NONE;
> + struct efimenu *efi_menu = data;
> +
> + while (1) {
> + bootmenu_loop((struct bootmenu_data *)efi_menu, &key, &esc);
> +
> + switch (key) {
> + case KEY_UP:
> + if (efi_menu->active > 0)
> + --efi_menu->active;
> + /* no menu key selected, regenerate menu */
> + return NULL;
> + case KEY_DOWN:
> + if (efi_menu->active < efi_menu->count - 1)
> + ++efi_menu->active;
> + /* no menu key selected, regenerate menu */
> + return NULL;
> + case KEY_SELECT:
> + list_for_each_safe(pos, n, &efi_menu->list) {
> + entry = list_entry(pos, struct eficonfig_entry, 
> list);
> + if (entry->num == efi_menu->active)
> + return entry->key;
> + }
> + break;
> + case KEY_QUIT:
> + /* Quit by choosing the last entry */
> + entry = list_last_entry(&efi_menu->list, struct 
> eficonfig_entry, list);
> + return entry->key;
> + default:
> + /* Pressed key is not valid, no need to regenerate the 
> menu */

Even with this comment I am still not sure I am following. 
The function definition is a char *.  There are cases ('default' and
'KEY_SELECT') which have no return value at all.
Shouldn't there be a 'return NULL' in the end of the function, even if
that's not supposed to happen?

[...]

Thanks
/Ilias
 


Re: [PATCH v2] pci: Do not enable PCIe GEN3 link retrain workaround by default

2022-08-30 Thread Pali Rohár
On Tuesday 30 August 2022 13:15:26 Stefan Roese wrote:
> On 30.08.22 11:19, Pali Rohár wrote:
> > On Tuesday 30 August 2022 10:04:51 Maciej W. Rozycki wrote:
> > > On Sat, 27 Aug 2022, Pali Rohár wrote:
> > > 
> > > > Moreover this workaround is enabled for all existing hardware and also 
> > > > all
> > > > future PCIe hardware, which opens a hole that other PCIe vendors may
> > > > introduce same HW issue as on systems where this workaround is required 
> > > > and
> > > > nobody would notice it because U-Boot automatically apply workaround 
> > > > for it.
> > > 
> > >   Why is it a problem?
> > 
> > I think I wrote it. One issue is that it is increasing size of SPL image
> > and we really should not include into SPL things which are not required
> > for all target platforms. Lot of boards have size constrained memory
> > requirements and unnecessary features should not be automatically
> > enabled.
> 
> I have to agree with Pali here. We need to be careful with size increase
> in the SPL images, as some of the build targets are very limited here.
> So making this workaround configurable is definitely a good idea.
> 
> The question remains, at least for me, if the Kconfig option should be
> enable per default or not. For SPL my suggestions is to disable is per
> default because of the size remarks above. For U-Boot proper I'm not so
> sure. Please see below...
> 
> > > Is the intent to cause hassle to end users and
> > > force them to take action when they have a non-working piece of hardware?
> > 
> > Vendors should really test their hardware without any automatic
> > workaround. Otherwise we are going into the hell if some workarounds are
> > automatically enabled and nobody notice broken behavior. And vendors
> > really build software with default options and do not care about it if
> > default options are suitable.
> > 
> > There is already direction to make all workaround targeted and enabled
> > only for platforms / hardware which really need it.
> > 
> > So enabling some workaround for all platforms which are produced and
> > will be produced in the future is step backward.
> 
> All this makes sense to me. You both have good points AFAICT. Please
> see below...
> 
> > >   I'd say in 99% of cases this will only cause frustration and they won't
> > > bother.  They will just conclude that either piece of hardware involved is
> > > broken and will throw it away.  Just as I almost did.  The seller has
> > > offered me a refund, which seems thought to be a universal solution
> > > nowadays (but I need to do what I meant to and getting money back doesn't
> > > solve it).
> > > 
> > >   And at least I know what U-boot (or indeed firmware) is and have a
> > > general understanding of how computers work.  Most people just want to
> > > plug stuff in and use it for whatever their need is.  Expecting them to
> > > take action to get things working is wasting their time (which BTW seems
> > > to have been a growing trend in last ~30 years: putting burden on the end
> > > user to get our problems solved, which saves our time and money at the
> > > expense of end user's).
> > 
> > The another issue here is that it was not fully investigated where the
> > issue is. If it is processor specific, PCIe switch specific or endpoint
> > specific, or combination of these options. It was just observed that
> > proposed workaround fix this issue on one specific combination. And you
> > confirmed this in previous post, that you are unsure if it is not
> > specific to downstream ports of the switch.
> > 
> > We really should not include such "bloatware" code to be enabled for
> > everything, on every one board. You are the first who reported this
> > issue. Nobody else complained about it, so I really do not see reason
> > why all other users and developers must be forced to have it in their
> > U-Boot binaries. Moreover lot of boards on which is U-Boot running do
> > not have PCIe bus exported on the slot and have PCIe devices integrated
> > and soldered on the board. Why on the earth I have to need this
> > workaround also on these boards (which are moreover sized constrained)?
> 
> Agreed. But I also understand the reasoning from Maciej, at least in
> parts. Thinking a bit more about this, my preference would be to still
> include this workaround per default in U-Boot proper though. To not
> make things too complicated here.
> 
> Just my 0.02$.

I understand it.

Anyway, I would really to know where is the issue (in which part of PCIe
hierarchy) and what exactly is affected.

I think that deep understanding of the issue is important or at least
confirmation from the vendor (which we know that it would not come).

> > >   NB I'm slowly getting fed up with the amount of non-working stuff piling
> > > up around.  Every other piece of equipment I try doesn't work for one
> > > reason or another and I need to either chase bugs myself or to spend days
> > > and weeks to persuade someone at least to believe a problem is there t

Re: [PATCH 2/6] timer: orion-timer: Add timer_get_boot_us() for BOOTSTAGE support

2022-08-30 Thread Michael Walle

Am 2022-08-30 13:53, schrieb Stefan Roese:

Add timer_get_boot_us() to support boards, that have CONFIG_BOOTSTAGE
enabled, like pogo_v4.

Signed-off-by: Stefan Roese 
---
 drivers/timer/orion-timer.c | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/drivers/timer/orion-timer.c b/drivers/timer/orion-timer.c
index 02ed138642b8..7e920eaeaa40 100644
--- a/drivers/timer/orion-timer.c
+++ b/drivers/timer/orion-timer.c
@@ -41,6 +41,28 @@ u64 notrace timer_early_get_count(void)
 }
 #endif

+#if CONFIG_IS_ENABLED(BOOTSTAGE)
+ulong timer_get_boot_us(void)
+{
+   u64 ticks = 0;
+   u32 rate = 1;
+   u64 us;
+   int ret;
+
+   ret = dm_timer_init();
+   if (!ret) {
+   /* The timer is available */
+   rate = timer_get_rate(gd->timer);
+   timer_get_count(gd->timer, &ticks);
+   } else {
+   return 0;
+   }
+
+   us = (ticks * 1000) / rate;
+   return us;
+}
+#endif


This is duplicate code in almost all the timer drivers, shouldn't
this be a (weak) default implementation in timer-uclass.c? Also,
do you need to guard it with CONFIG_IS_ENABLED(BOOTSTAGE), aren't
unused functions discarded anyway?

-michael


Re: [PATCH 3/6] arm: mvebu: Use CONFIG_TIMER on all MVEBU & KIRKWOOD platforms

2022-08-30 Thread Michael Walle

Am 2022-08-30 13:53, schrieb Stefan Roese:

Now that the new timer support is available for these platforms, let's
select this IF for all these platforms. This way it's not necessary
that each board changes it's config header.

Signed-off-by: Stefan Roese 
---
 arch/arm/Kconfig  | 4 
 arch/arm/mach-mvebu/include/mach/config.h | 5 -
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 0b72e4f6503e..60f524a2d118 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -618,6 +618,7 @@ config ARCH_KIRKWOOD
select BOARD_EARLY_INIT_F
select CPU_ARM926EJS
select GPIO_EXTRA_HEADER
+   select TIMER


If selected by the arch now (and the timer driver defaulting to
y on this arch), could you clean the lschvl2 and lsxhl_defconfigs up
and remove the two symbols there?



 config ARCH_MVEBU
bool "Marvell MVEBU family (Armada XP/375/38x/3700/7K/8K)"
@@ -629,6 +630,8 @@ config ARCH_MVEBU
select GPIO_EXTRA_HEADER
select SPL_DM_SPI if SPL
select SPL_DM_SPI_FLASH if SPL
+   select SPL_TIMER if SPL
+   select TIMER
select OF_CONTROL
select OF_SEPARATE
select SPI
@@ -639,6 +642,7 @@ config ARCH_ORION5X
select CPU_ARM926EJS
select GPIO_EXTRA_HEADER
select SPL_SEPARATE_BSS if SPL
+   select TIMER

 config TARGET_STV0991
bool "Support stv0991"
diff --git a/arch/arm/mach-mvebu/include/mach/config.h
b/arch/arm/mach-mvebu/include/mach/config.h
index 4add0d9e1030..9b5036c31dd3 100644
--- a/arch/arm/mach-mvebu/include/mach/config.h
+++ b/arch/arm/mach-mvebu/include/mach/config.h
@@ -41,9 +41,4 @@
 #endif
 #endif

-/* Use common timer */
-#define CONFIG_SYS_TIMER_COUNTS_DOWN
-#define CONFIG_SYS_TIMER_COUNTER   (MVEBU_TIMER_BASE + 0x14)
-#define CONFIG_SYS_TIMER_RATE  2500
-
 #endif /* __MVEBU_CONFIG_H */


Re: [PATCH 2/6] timer: orion-timer: Add timer_get_boot_us() for BOOTSTAGE support

2022-08-30 Thread Stefan Roese

Adding Simon to Cc...

On 30.08.22 14:00, Michael Walle wrote:

Am 2022-08-30 13:53, schrieb Stefan Roese:

Add timer_get_boot_us() to support boards, that have CONFIG_BOOTSTAGE
enabled, like pogo_v4.

Signed-off-by: Stefan Roese 
---
 drivers/timer/orion-timer.c | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/drivers/timer/orion-timer.c b/drivers/timer/orion-timer.c
index 02ed138642b8..7e920eaeaa40 100644
--- a/drivers/timer/orion-timer.c
+++ b/drivers/timer/orion-timer.c
@@ -41,6 +41,28 @@ u64 notrace timer_early_get_count(void)
 }
 #endif

+#if CONFIG_IS_ENABLED(BOOTSTAGE)
+ulong timer_get_boot_us(void)
+{
+    u64 ticks = 0;
+    u32 rate = 1;
+    u64 us;
+    int ret;
+
+    ret = dm_timer_init();
+    if (!ret) {
+    /* The timer is available */
+    rate = timer_get_rate(gd->timer);
+    timer_get_count(gd->timer, &ticks);
+    } else {
+    return 0;
+    }
+
+    us = (ticks * 1000) / rate;
+    return us;
+}
+#endif


This is duplicate code in almost all the timer drivers, shouldn't
this be a (weak) default implementation in timer-uclass.c?


Yes. I was lazy and just copied this function and did not notice, that
even more timer drivers have this function. Of course it makes sense
to not duplicate code here, but have a common function for this. Frankly
I don't even know why exactly this function is needed, as I did not
look into BOOTSTAGE yet.

Simon, do we really need this function? Can't bootstage just use the
"normal" timer functionality instead?


Also,
do you need to guard it with CONFIG_IS_ENABLED(BOOTSTAGE), aren't
unused functions discarded anyway?


Yes, this should be the case. Also just a result of my lazy copy-and-
past.

Thanks,
Stefan



[PATCH v3 3/4] pinctrl: pinctrl_stm32: Populate uc_priv->name[] with pinmux node's name

2022-08-30 Thread Patrice Chotard
Populate uc_priv->name[] with pinmux node's name in order to indicate
the pinmuxing's name in case GPIO is configured in alternate.

For example, for STM32 SoC's based platform, "gpio status" command
output :

  before
Bank GPIOZ:
  GPIOZ0: unused : 0 [ ]
  GPIOZ1: unused : 0 [ ]
  GPIOZ2: unused : 0 [ ]
  GPIOZ3: unused : 0 [ ]
  GPIOZ4: func
  GPIOZ5: func
  GPIOZ6: unused : 0 [ ]
  GPIOZ7: unused : 0 [ ]
  GPIOZ8: unknown
  GPIOZ9: unknown
  GPIOZ10: unknown
  GPIOZ11: unknown
  GPIOZ12: unknown
  GPIOZ13: unknown
  GPIOZ14: unknown
  GPIOZ15: unknown

  After
Bank GPIOZ:
  GPIOZ0: unused : 0 [ ]
  GPIOZ1: unused : 0 [ ]
  GPIOZ2: unused : 0 [ ]
  GPIOZ3: unused : 0 [ ]
  GPIOZ4: func i2c4-0
  GPIOZ5: func i2c4-0
  GPIOZ6: unused : 0 [ ]
  GPIOZ7: unused : 0 [ ]
  GPIOZ8: unknown
  GPIOZ9: unknown
  GPIOZ10: unknown
  GPIOZ11: unknown
  GPIOZ12: unknown
  GPIOZ13: unknown
  GPIOZ14: unknown
  GPIOZ15: unknown

Signed-off-by: Patrice Chotard 
---

(no changes since v1)

 drivers/pinctrl/pinctrl_stm32.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/pinctrl_stm32.c b/drivers/pinctrl/pinctrl_stm32.c
index 990cd19286..b755fa42b4 100644
--- a/drivers/pinctrl/pinctrl_stm32.c
+++ b/drivers/pinctrl/pinctrl_stm32.c
@@ -257,10 +257,12 @@ static int stm32_pinctrl_probe(struct udevice *dev)
return 0;
 }
 
-static int stm32_gpio_config(struct gpio_desc *desc,
+static int stm32_gpio_config(ofnode node,
+struct gpio_desc *desc,
 const struct stm32_gpio_ctl *ctl)
 {
struct stm32_gpio_priv *priv = dev_get_priv(desc->dev);
+   struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(desc->dev);
struct stm32_gpio_regs *regs = priv->regs;
struct stm32_pinctrl_priv *ctrl_priv;
int ret;
@@ -291,6 +293,8 @@ static int stm32_gpio_config(struct gpio_desc *desc,
index = desc->offset;
clrsetbits_le32(®s->otyper, OTYPE_MSK << index, ctl->otype << index);
 
+   uc_priv->name[desc->offset] = strdup(ofnode_get_name(node));
+
hwspinlock_unlock(&ctrl_priv->hws);
 
return 0;
@@ -385,7 +389,7 @@ static int stm32_pinctrl_config(ofnode node)
if (rv)
return rv;
desc.offset = gpio_dsc.pin;
-   rv = stm32_gpio_config(&desc, &gpio_ctl);
+   rv = stm32_gpio_config(node, &desc, &gpio_ctl);
log_debug("rv = %d\n\n", rv);
if (rv)
return rv;
-- 
2.25.1



[PATCH v3 4/4] gpio: sandbox: Add GPIOD_IS_AF for gpio configured in alternate function

2022-08-30 Thread Patrice Chotard
This allows to test if a pin's label if displayed using gpio_get_status()
when this pin is configured in alternate function.

Signed-off-by: Patrice Chotard 
Reviewed-by: Simon Glass 

---

(no changes since v1)

 drivers/gpio/sandbox.c  |  5 +
 include/asm-generic/gpio.h  |  1 +
 include/dt-bindings/gpio/sandbox-gpio.h |  3 +++
 test/dm/gpio.c  | 30 +
 4 files changed, 39 insertions(+)

diff --git a/drivers/gpio/sandbox.c b/drivers/gpio/sandbox.c
index 106b2a7b27..305f9a6ff6 100644
--- a/drivers/gpio/sandbox.c
+++ b/drivers/gpio/sandbox.c
@@ -196,6 +196,8 @@ static int sb_gpio_get_function(struct udevice *dev, 
unsigned offset)
return GPIOF_OUTPUT;
if (get_gpio_flag(dev, offset, GPIOD_IS_IN))
return GPIOF_INPUT;
+   if (get_gpio_flag(dev, offset, GPIOD_IS_AF))
+   return GPIOF_FUNC;
 
return GPIOF_INPUT; /*GPIO is not configurated */
 }
@@ -219,6 +221,9 @@ static int sb_gpio_xlate(struct udevice *dev, struct 
gpio_desc *desc,
if (args->args[1] & GPIO_OUT_ACTIVE)
desc->flags |= GPIOD_IS_OUT_ACTIVE;
 
+   if (args->args[1] & GPIO_AF)
+   desc->flags |= GPIOD_IS_AF;
+
return 0;
 }
 
diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index 81f63f06f1..0fcf70983f 100644
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -127,6 +127,7 @@ struct gpio_desc {
 #define GPIOD_OPEN_SOURCE  BIT(6)  /* GPIO is open source type */
 #define GPIOD_PULL_UP  BIT(7)  /* GPIO has pull-up enabled */
 #define GPIOD_PULL_DOWNBIT(8)  /* GPIO has pull-down enabled */
+#define GPIOD_IS_AFBIT(9)  /* GPIO is an alternate function */
 
 /* Flags for updating the above */
 #define GPIOD_MASK_DIR (GPIOD_IS_OUT | GPIOD_IS_IN | \
diff --git a/include/dt-bindings/gpio/sandbox-gpio.h 
b/include/dt-bindings/gpio/sandbox-gpio.h
index e4bfdb3ce1..05f9836583 100644
--- a/include/dt-bindings/gpio/sandbox-gpio.h
+++ b/include/dt-bindings/gpio/sandbox-gpio.h
@@ -21,4 +21,7 @@
 /* Bit 18 express GPIO output is active */
 #define GPIO_OUT_ACTIVE0x4
 
+/* Bit 19 express GPIO set as alternate function */
+#define GPIO_AF0x8
+
 #endif
diff --git a/test/dm/gpio.c b/test/dm/gpio.c
index 33ae98701f..a8c35d4370 100644
--- a/test/dm/gpio.c
+++ b/test/dm/gpio.c
@@ -778,3 +778,33 @@ static int dm_test_gpio_get_values_as_int_base3(struct 
unit_test_state *uts)
 }
 DM_TEST(dm_test_gpio_get_values_as_int_base3,
UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
+
+/* Check that gpio_get_status return the label of a GPIO configured as 
GPIOD_AF */
+static int dm_test_gpio_function(struct unit_test_state *uts)
+{
+   struct gpio_desc desc;
+   struct udevice *dev;
+   ulong flags;
+   unsigned int offset, gpio;
+   char buf[80];
+
+   ut_assertok(uclass_get_device(UCLASS_TEST_FDT, 0, &dev));
+   ut_asserteq_str("a-test", dev->name);
+
+   /* request gpio_b 5 */
+   ut_assertok(gpio_request_by_name(dev, "test-gpios", 2, &desc, 0));
+   /* update gpio_b 5 function to GPIO_AF */
+   ut_assertok(dm_gpio_clrset_flags(&desc, GPIOD_IS_AF, GPIOD_IS_AF));
+   ut_assertok(dm_gpio_get_flags(&desc, &flags));
+   ut_asserteq(GPIOD_IS_AF, flags);
+   /* check using gpio_get_status that label is displayed for a pin with 
GPIO_AF function */
+   ut_assertok(gpio_lookup_name("b5", &dev, &offset, &gpio));
+   ut_assertok(gpio_get_status(dev, offset, buf, sizeof(buf)));
+   ut_asserteq_str("b5: func a-test.test-gpios2", buf);
+
+   ut_assertok(dm_gpio_free(dev, &desc));
+
+   return 0;
+}
+DM_TEST(dm_test_gpio_function,
+   UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
-- 
2.25.1



[PATCH v3 1/4] gpio: Allow to print pin's label even for pin with GPIOF_FUNC function

2022-08-30 Thread Patrice Chotard
Currently, if pin's function is GPIOF_FUNC, only "func" if displayed
without any other information. It would be interesting, if information is
available, to indicate which pinmuxing's name is used.

For example, for STM32 SoC's based platform, "gpio status" command
output :

   before
Bank GPIOZ:
  GPIOZ0: unused : 0 [ ]
  GPIOZ1: unused : 0 [ ]
  GPIOZ2: unused : 0 [ ]
  GPIOZ3: unused : 0 [ ]
  GPIOZ4: func
  GPIOZ5: func
  GPIOZ6: unused : 0 [ ]
  GPIOZ7: unused : 0 [ ]
  GPIOZ8: unknown
  GPIOZ9: unknown
  GPIOZ10: unknown
  GPIOZ11: unknown
  GPIOZ12: unknown
  GPIOZ13: unknown
  GPIOZ14: unknown
  GPIOZ15: unknown

   After
Bank GPIOZ:
  GPIOZ0: unused : 0 [ ]
  GPIOZ1: unused : 0 [ ]
  GPIOZ2: unused : 0 [ ]
  GPIOZ3: unused : 0 [ ]
  GPIOZ4: func i2c4-0
  GPIOZ5: func i2c4-0
  GPIOZ6: unused : 0 [ ]
  GPIOZ7: unused : 0 [ ]
  GPIOZ8: unknown
  GPIOZ9: unknown
  GPIOZ10: unknown
  GPIOZ11: unknown
  GPIOZ12: unknown
  GPIOZ13: unknown
  GPIOZ14: unknown
  GPIOZ15: unknown

Signed-off-by: Patrice Chotard 
Reviewed-by: Simon Glass 
---

Changes in v3:
   - add lost Simon Glass's reviewed-by during v1 review

 drivers/gpio/gpio-uclass.c | 16 +++-
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
index 0ed32b7217..d60e46159a 100644
--- a/drivers/gpio/gpio-uclass.c
+++ b/drivers/gpio/gpio-uclass.c
@@ -884,26 +884,31 @@ int gpio_get_status(struct udevice *dev, int offset, char 
*buf, int buffsize)
const struct dm_gpio_ops *ops = gpio_get_ops(dev);
struct gpio_dev_priv *priv;
char *str = buf;
+   const char *label;
int func;
int ret;
int len;
+   bool used;
 
BUILD_BUG_ON(GPIOF_COUNT != ARRAY_SIZE(gpio_function));
 
*buf = 0;
priv = dev_get_uclass_priv(dev);
-   ret = gpio_get_raw_function(dev, offset, NULL);
+   ret = gpio_get_raw_function(dev, offset, &label);
if (ret < 0)
return ret;
func = ret;
len = snprintf(str, buffsize, "%s%d: %s",
   priv->bank_name ? priv->bank_name : "",
   offset, gpio_function[func]);
-   if (func == GPIOF_INPUT || func == GPIOF_OUTPUT ||
-   func == GPIOF_UNUSED) {
-   const char *label;
-   bool used;
 
+   switch (func) {
+   case GPIOF_FUNC:
+   snprintf(str + len, buffsize - len, " %s", label ? label : "");
+   break;
+   case GPIOF_INPUT:
+   case GPIOF_OUTPUT:
+   case GPIOF_UNUSED:
ret = ops->get_value(dev, offset);
if (ret < 0)
return ret;
@@ -913,6 +918,7 @@ int gpio_get_status(struct udevice *dev, int offset, char 
*buf, int buffsize)
 used ? 'x' : ' ',
 used ? " " : "",
 label ? label : "");
+   break;
}
 
return 0;
-- 
2.25.1



[PATCH v3 2/4] gpio: Fix pin's status display for pin with GPIOF_UNUSED function

2022-08-30 Thread Patrice Chotard
Even pin with GPIOF_UNUSED function can have a label.
The criteria to add or not a space character is linked to label not to
the used/unused status.

Signed-off-by: Patrice Chotard 
Reviewed-by: Simon Glass 
---

Changes in v3:
   - add lost Simon Glass's reviewed-by during v1 review

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

diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
index d60e46159a..a00880e446 100644
--- a/drivers/gpio/gpio-uclass.c
+++ b/drivers/gpio/gpio-uclass.c
@@ -916,7 +916,7 @@ int gpio_get_status(struct udevice *dev, int offset, char 
*buf, int buffsize)
snprintf(str + len, buffsize - len, ": %d [%c]%s%s",
 ret,
 used ? 'x' : ' ',
-used ? " " : "",
+label ? " " : "",
 label ? label : "");
break;
}
-- 
2.25.1



[PATCH v3 0/4] gpio: update gpio_get_status()

2022-08-30 Thread Patrice Chotard


Currently, if pin's function is GPIOF_FUNC, only "func" if displayed
without any other information. It would be interesting, if information is
available, to indicate which pinmuxing's name is used.


Changes in v3:
   - add lost Simon Glass's reviewed-by during v1 review in patch 1 and 2

Patrice Chotard (4):
  gpio: Allow to print pin's label even for pin with GPIOF_FUNC function
  gpio: Fix pin's status display for pin with GPIOF_UNUSED function
  pinctrl: pinctrl_stm32: Populate uc_priv->name[] with pinmux node's
name
  gpio: sandbox: Add GPIOD_IS_AF for gpio configured in alternate
function

 drivers/gpio/gpio-uclass.c  | 18 ++-
 drivers/gpio/sandbox.c  |  5 +
 drivers/pinctrl/pinctrl_stm32.c |  8 +--
 include/asm-generic/gpio.h  |  1 +
 include/dt-bindings/gpio/sandbox-gpio.h |  3 +++
 test/dm/gpio.c  | 30 +
 6 files changed, 57 insertions(+), 8 deletions(-)

-- 
2.25.1



Re: [PATCH 3/6] arm: mvebu: Use CONFIG_TIMER on all MVEBU & KIRKWOOD platforms

2022-08-30 Thread Stefan Roese

On 30.08.22 14:04, Michael Walle wrote:

Am 2022-08-30 13:53, schrieb Stefan Roese:

Now that the new timer support is available for these platforms, let's
select this IF for all these platforms. This way it's not necessary
that each board changes it's config header.

Signed-off-by: Stefan Roese 
---
 arch/arm/Kconfig  | 4 
 arch/arm/mach-mvebu/include/mach/config.h | 5 -
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 0b72e4f6503e..60f524a2d118 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -618,6 +618,7 @@ config ARCH_KIRKWOOD
 select BOARD_EARLY_INIT_F
 select CPU_ARM926EJS
 select GPIO_EXTRA_HEADER
+    select TIMER


If selected by the arch now (and the timer driver defaulting to
y on this arch), could you clean the lschvl2 and lsxhl_defconfigs up
and remove the two symbols there?


Sure. I can add another patch for this in the next patchset version.
Please complain, if I forget to do this. ;)

Thanks,
Stefan



 config ARCH_MVEBU
 bool "Marvell MVEBU family (Armada XP/375/38x/3700/7K/8K)"
@@ -629,6 +630,8 @@ config ARCH_MVEBU
 select GPIO_EXTRA_HEADER
 select SPL_DM_SPI if SPL
 select SPL_DM_SPI_FLASH if SPL
+    select SPL_TIMER if SPL
+    select TIMER
 select OF_CONTROL
 select OF_SEPARATE
 select SPI
@@ -639,6 +642,7 @@ config ARCH_ORION5X
 select CPU_ARM926EJS
 select GPIO_EXTRA_HEADER
 select SPL_SEPARATE_BSS if SPL
+    select TIMER

 config TARGET_STV0991
 bool "Support stv0991"
diff --git a/arch/arm/mach-mvebu/include/mach/config.h
b/arch/arm/mach-mvebu/include/mach/config.h
index 4add0d9e1030..9b5036c31dd3 100644
--- a/arch/arm/mach-mvebu/include/mach/config.h
+++ b/arch/arm/mach-mvebu/include/mach/config.h
@@ -41,9 +41,4 @@
 #endif
 #endif

-/* Use common timer */
-#define CONFIG_SYS_TIMER_COUNTS_DOWN
-#define CONFIG_SYS_TIMER_COUNTER    (MVEBU_TIMER_BASE + 0x14)
-#define CONFIG_SYS_TIMER_RATE    2500
-
 #endif /* __MVEBU_CONFIG_H */


Viele Grüße,
Stefan Roese

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


Re: [PATCH v3 1/4] gpio: Allow to print pin's label even for pin with GPIOF_FUNC function

2022-08-30 Thread Patrick DELAUNAY

Hi Patrice

On 8/30/22 14:09, Patrice Chotard wrote:

Currently, if pin's function is GPIOF_FUNC, only "func" if displayed
without any other information. It would be interesting, if information is
available, to indicate which pinmuxing's name is used.

For example, for STM32 SoC's based platform, "gpio status" command
output :

before
 Bank GPIOZ:
   GPIOZ0: unused : 0 [ ]
   GPIOZ1: unused : 0 [ ]
   GPIOZ2: unused : 0 [ ]
   GPIOZ3: unused : 0 [ ]
   GPIOZ4: func
   GPIOZ5: func
   GPIOZ6: unused : 0 [ ]
   GPIOZ7: unused : 0 [ ]
   GPIOZ8: unknown
   GPIOZ9: unknown
   GPIOZ10: unknown
   GPIOZ11: unknown
   GPIOZ12: unknown
   GPIOZ13: unknown
   GPIOZ14: unknown
   GPIOZ15: unknown

After
 Bank GPIOZ:
   GPIOZ0: unused : 0 [ ]
   GPIOZ1: unused : 0 [ ]
   GPIOZ2: unused : 0 [ ]
   GPIOZ3: unused : 0 [ ]
   GPIOZ4: func i2c4-0
   GPIOZ5: func i2c4-0
   GPIOZ6: unused : 0 [ ]
   GPIOZ7: unused : 0 [ ]
   GPIOZ8: unknown
   GPIOZ9: unknown
   GPIOZ10: unknown
   GPIOZ11: unknown
   GPIOZ12: unknown
   GPIOZ13: unknown
   GPIOZ14: unknown
   GPIOZ15: unknown

Signed-off-by: Patrice Chotard 
Reviewed-by: Simon Glass 
---

Changes in v3:
- add lost Simon Glass's reviewed-by during v1 review

  drivers/gpio/gpio-uclass.c | 16 +++-
  1 file changed, 11 insertions(+), 5 deletions(-)



Reviewed-by: Patrick Delaunay 

Thanks
Patrick




Re: [PATCH v3 2/4] gpio: Fix pin's status display for pin with GPIOF_UNUSED function

2022-08-30 Thread Patrick DELAUNAY

Hi Patrice,


On 8/30/22 14:09, Patrice Chotard wrote:

Even pin with GPIOF_UNUSED function can have a label.
The criteria to add or not a space character is linked to label not to
the used/unused status.

Signed-off-by: Patrice Chotard 
Reviewed-by: Simon Glass 
---

Changes in v3:
- add lost Simon Glass's reviewed-by during v1 review

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




Reviewed-by: Patrick Delaunay 

Thanks
Patrick



Re: [PATCH v3 3/4] pinctrl: pinctrl_stm32: Populate uc_priv->name[] with pinmux node's name

2022-08-30 Thread Patrick DELAUNAY

Hi,

On 8/30/22 14:09, Patrice Chotard wrote:

Populate uc_priv->name[] with pinmux node's name in order to indicate
the pinmuxing's name in case GPIO is configured in alternate.

For example, for STM32 SoC's based platform, "gpio status" command
output :

   before
 Bank GPIOZ:
   GPIOZ0: unused : 0 [ ]
   GPIOZ1: unused : 0 [ ]
   GPIOZ2: unused : 0 [ ]
   GPIOZ3: unused : 0 [ ]
   GPIOZ4: func
   GPIOZ5: func
   GPIOZ6: unused : 0 [ ]
   GPIOZ7: unused : 0 [ ]
   GPIOZ8: unknown
   GPIOZ9: unknown
   GPIOZ10: unknown
   GPIOZ11: unknown
   GPIOZ12: unknown
   GPIOZ13: unknown
   GPIOZ14: unknown
   GPIOZ15: unknown

   After
 Bank GPIOZ:
   GPIOZ0: unused : 0 [ ]
   GPIOZ1: unused : 0 [ ]
   GPIOZ2: unused : 0 [ ]
   GPIOZ3: unused : 0 [ ]
   GPIOZ4: func i2c4-0
   GPIOZ5: func i2c4-0
   GPIOZ6: unused : 0 [ ]
   GPIOZ7: unused : 0 [ ]
   GPIOZ8: unknown
   GPIOZ9: unknown
   GPIOZ10: unknown
   GPIOZ11: unknown
   GPIOZ12: unknown
   GPIOZ13: unknown
   GPIOZ14: unknown
   GPIOZ15: unknown

Signed-off-by: Patrice Chotard 
---

(no changes since v1)

  drivers/pinctrl/pinctrl_stm32.c | 8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)




Reviewed-by: Patrick Delaunay 

Thanks
Patrick






Re: [PATCH v3 4/4] gpio: sandbox: Add GPIOD_IS_AF for gpio configured in alternate function

2022-08-30 Thread Patrick DELAUNAY

Hi Patrice,

On 8/30/22 14:09, Patrice Chotard wrote:

This allows to test if a pin's label if displayed using gpio_get_status()
when this pin is configured in alternate function.

Signed-off-by: Patrice Chotard 
Reviewed-by: Simon Glass 

---

(no changes since v1)

  drivers/gpio/sandbox.c  |  5 +
  include/asm-generic/gpio.h  |  1 +
  include/dt-bindings/gpio/sandbox-gpio.h |  3 +++
  test/dm/gpio.c  | 30 +
  4 files changed, 39 insertions(+)




Reviewed-by: Patrick Delaunay 

Thanks
Patrick





Re: [PATCH] arm: mvebu: Espressobin: When emmc is not present disable it also in OF_LIVE

2022-08-30 Thread Stefan Roese

On 27.08.22 14:00, Pali Rohár wrote:

Signed-off-by: Pali Rohár 


As mentioned by Simon, please add a short sentence in the commit text
as well.

Other than that:

Reviewed-by: Stefan Roese 

Thanks,
Stefan


---
  board/Marvell/mvebu_armada-37xx/board.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/board/Marvell/mvebu_armada-37xx/board.c 
b/board/Marvell/mvebu_armada-37xx/board.c
index 5ae133879f32..45779198b678 100644
--- a/board/Marvell/mvebu_armada-37xx/board.c
+++ b/board/Marvell/mvebu_armada-37xx/board.c
@@ -132,6 +132,8 @@ int board_late_init(void)
dev = mmc_dev->dev;
device_remove(dev, DM_REMOVE_NORMAL);
device_unbind(dev);
+   if (of_live_active())
+   ofnode_set_enabled(dev_ofnode(dev), false);
}
  
  	/* Ensure that 'env default -a' set correct value to $fdtfile */


Viele Grüße,
Stefan Roese

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


Re: [PATCH] board: turris: Initialize serial# env

2022-08-30 Thread Stefan Roese

On 27.08.22 20:06, Pali Rohár wrote:

Store serial number from atsha cryptochip into the serial# env variable.
U-Boot automatically puts content of this variable into the root device
tree property serial-number when booting Linux kernel. Refactor turris
atsha code and from turris_atsha_otp_get_serial_number() function returns
directly string suitable for printing or storing into device tree. Because
during different boot stages is env storage read-only, it is not possible
to always store serial number into env storage. So introduce a new function
turris_atsha_otp_init_serial_number() which is called at later stage and
which ensures that serial number is correctly stored into env.

Signed-off-by: Pali Rohár 


Reviewed-by: Stefan Roese 

Thanks,
Stefan


---
  board/CZ.NIC/turris_atsha_otp.c  | 33 +---
  board/CZ.NIC/turris_atsha_otp.h  |  3 ++-
  board/CZ.NIC/turris_omnia/turris_omnia.c | 11 +++-
  3 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/board/CZ.NIC/turris_atsha_otp.c b/board/CZ.NIC/turris_atsha_otp.c
index aa4e29b1560e..a29fe3623174 100644
--- a/board/CZ.NIC/turris_atsha_otp.c
+++ b/board/CZ.NIC/turris_atsha_otp.c
@@ -93,30 +93,57 @@ int turris_atsha_otp_init_mac_addresses(int first_idx)
return 0;
  }
  
-int turris_atsha_otp_get_serial_number(u32 *version_num, u32 *serial_num)

+int turris_atsha_otp_init_serial_number(void)
+{
+   char serial[17];
+   int ret;
+
+   ret = turris_atsha_otp_get_serial_number(serial);
+   if (ret)
+   return ret;
+
+   if (!env_get("serial#"))
+   return -1;
+
+   return 0;
+}
+
+int turris_atsha_otp_get_serial_number(char serial[17])
  {
struct udevice *dev = get_atsha204a_dev();
+   u32 version_num, serial_num;
+   const char *serial_env;
int ret;
  
  	if (!dev)

return -1;
  
+	serial_env = env_get("serial#");

+   if (serial_env && strlen(serial_env) == 16) {
+   memcpy(serial, serial_env, 17);
+   return 0;
+   }
+
ret = atsha204a_wakeup(dev);
if (ret)
return ret;
  
  	ret = atsha204a_read(dev, ATSHA204A_ZONE_OTP, false,

 TURRIS_ATSHA_OTP_VERSION,
-(u8 *)version_num);
+(u8 *)&version_num);
if (ret)
return ret;
  
  	ret = atsha204a_read(dev, ATSHA204A_ZONE_OTP, false,

 TURRIS_ATSHA_OTP_SERIAL,
-(u8 *)serial_num);
+(u8 *)&serial_num);
if (ret)
return ret;
  
  	atsha204a_sleep(dev);

+
+   sprintf(serial, "%08X%08X", be32_to_cpu(version_num), 
be32_to_cpu(serial_num));
+   env_set("serial#", serial);
+
return 0;
  }
diff --git a/board/CZ.NIC/turris_atsha_otp.h b/board/CZ.NIC/turris_atsha_otp.h
index bd4308fdc3ef..2cfe20bbc3a7 100644
--- a/board/CZ.NIC/turris_atsha_otp.h
+++ b/board/CZ.NIC/turris_atsha_otp.h
@@ -4,6 +4,7 @@
  #define TURRIS_ATSHA_OTP_H
  
  int turris_atsha_otp_init_mac_addresses(int first_idx);

-int turris_atsha_otp_get_serial_number(u32 *version_num, u32 *serial_num);
+int turris_atsha_otp_init_serial_number(void);
+int turris_atsha_otp_get_serial_number(char serial[17]);
  
  #endif

diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c 
b/board/CZ.NIC/turris_omnia/turris_omnia.c
index ab5061ef582b..cf8a6026702b 100644
--- a/board/CZ.NIC/turris_omnia/turris_omnia.c
+++ b/board/CZ.NIC/turris_omnia/turris_omnia.c
@@ -963,19 +963,15 @@ int board_late_init(void)
  
  int show_board_info(void)

  {
-   u32 version_num, serial_num;
+   char serial[17];
int err;
  
-	err = turris_atsha_otp_get_serial_number(&version_num, &serial_num);

+   err = turris_atsha_otp_get_serial_number(serial);
printf("Model: Turris Omnia\n");
printf("  MCU type: %s\n", omnia_get_mcu_type());
printf("  MCU version: %s\n", omnia_get_mcu_version());
printf("  RAM size: %i MiB\n", omnia_get_ram_size_gb() * 1024);
-   if (err)
-   printf("  Serial Number: unknown\n");
-   else
-   printf("  Serial Number: %08X%08X\n", be32_to_cpu(version_num),
-  be32_to_cpu(serial_num));
+   printf("  Serial Number: %s\n", !err ? serial : "unknown");
  
  	return 0;

  }
@@ -983,6 +979,7 @@ int show_board_info(void)
  int misc_init_r(void)
  {
turris_atsha_otp_init_mac_addresses(1);
+   turris_atsha_otp_init_serial_number();
return 0;
  }
  


Viele Grüße,
Stefan Roese

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


Re: [PATCH] arm: mvebu: turris_{omnia, mox}: Reset bootdelay env for rescue

2022-08-30 Thread Stefan Roese

On 27.08.22 20:49, Pali Rohár wrote:

When rescue mode was activated reset also bootdelay env variable to its
default value. This will ensure that reset button works and starts rescue
mode also in the case when user changed bootdelay env variable to -1 (which
has meaning to not start autoboot).

Signed-off-by: Pali Rohár 


Reviewed-by: Stefan Roese 

Thanks,
Stefan


---
  board/CZ.NIC/turris_mox/turris_mox.c | 5 +++--
  board/CZ.NIC/turris_omnia/turris_omnia.c | 5 +++--
  2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/board/CZ.NIC/turris_mox/turris_mox.c 
b/board/CZ.NIC/turris_mox/turris_mox.c
index 3dbd68e52366..5e1cf328ecc1 100644
--- a/board/CZ.NIC/turris_mox/turris_mox.c
+++ b/board/CZ.NIC/turris_mox/turris_mox.c
@@ -440,8 +440,9 @@ static void handle_reset_button(void)
env_set_default_vars(1, (char * const *)vars, 0);
  
  	if (read_reset_button()) {

-   const char * const vars[2] = {
+   const char * const vars[3] = {
"bootcmd",
+   "bootdelay",
"distro_bootcmd",
};
  
@@ -449,7 +450,7 @@ static void handle_reset_button(void)

 * Set the above envs to their default values, in case the user
 * managed to break them.
 */
-   env_set_default_vars(2, (char * const *)vars, 0);
+   env_set_default_vars(3, (char * const *)vars, 0);
  
  		/* Ensure bootcmd_rescue is used by distroboot */

env_set("boot_targets", "rescue");
diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c 
b/board/CZ.NIC/turris_omnia/turris_omnia.c
index cf8a6026702b..a7f96e5b7739 100644
--- a/board/CZ.NIC/turris_omnia/turris_omnia.c
+++ b/board/CZ.NIC/turris_omnia/turris_omnia.c
@@ -549,8 +549,9 @@ static void handle_reset_button(void)
env_set_ulong("omnia_reset", reset_status);
  
  	if (reset_status) {

-   const char * const vars[2] = {
+   const char * const vars[3] = {
"bootcmd",
+   "bootdelay",
"distro_bootcmd",
};
  
@@ -558,7 +559,7 @@ static void handle_reset_button(void)

 * Set the above envs to their default values, in case the user
 * managed to break them.
 */
-   env_set_default_vars(2, (char * const *)vars, 0);
+   env_set_default_vars(3, (char * const *)vars, 0);
  
  		/* Ensure bootcmd_rescue is used by distroboot */

env_set("boot_targets", "rescue");


Viele Grüße,
Stefan Roese

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


Re: [PATCH] arm: mvebu: turris_mox: Add support for distroboot $fdt_addr

2022-08-30 Thread Stefan Roese

On 29.08.22 15:44, Pali Rohár wrote:

$fdt_addr is mandatory for systems which provides DTB in HW (e.g. ROM) and
wishes to pass that DTB to Linux.

Turris Mox contains DTB binary in SPI NOR memory at "dtb" partition which
starts at offset 0x7f and is 0x1 bytes long.

Armada 3700 CPU does not allow mapping SPI NOR memory into physical address
space like on other architectures and therefore set $fdt_addr variable to
memory range in RAM and loads this DTB binary from SPI NOR in misc_init_r()
function.

Signed-off-by: Pali Rohár 


Reviewed-by: Stefan Roese 

Thanks,
Stefan


---
  board/CZ.NIC/turris_mox/turris_mox.c | 48 
  include/configs/turris_mox.h |  1 +
  2 files changed, 49 insertions(+)

diff --git a/board/CZ.NIC/turris_mox/turris_mox.c 
b/board/CZ.NIC/turris_mox/turris_mox.c
index 3dbd68e52366..3402515bbae2 100644
--- a/board/CZ.NIC/turris_mox/turris_mox.c
+++ b/board/CZ.NIC/turris_mox/turris_mox.c
@@ -23,6 +23,7 @@
  #include 
  #include 
  #include 
+#include 
  
  #include "mox_sp.h"
  
@@ -339,6 +340,51 @@ static int get_reset_gpio(struct gpio_desc *reset_gpio)

return 0;
  }
  
+/* Load default system DTB binary to $fdr_addr */

+static void load_spi_dtb(void)
+{
+   const char *const env_name[1] = { "fdt_addr" };
+   unsigned long size, offset;
+   struct udevice *spi_dev;
+   struct spi_flash *flash;
+   const char *addr_str;
+   unsigned long addr;
+   void *buf;
+
+   addr_str = env_get(env_name[0]);
+   if (!addr_str) {
+   env_set_default_vars(1, (char * const *)env_name, 0);
+   addr_str = env_get(env_name[0]);
+   }
+
+   if (!addr_str)
+   return;
+
+   addr = hextoul(addr_str, NULL);
+   if (!addr)
+   return;
+
+   spi_flash_probe_bus_cs(CONFIG_SF_DEFAULT_BUS, CONFIG_SF_DEFAULT_CS, 
&spi_dev);
+   flash = dev_get_uclass_priv(spi_dev);
+   if (!flash)
+   return;
+
+   /*
+* SPI NOR "dtb" partition offset & size hardcoded for now because the
+* mtd subsystem does not offer finding the partition yet and we do not
+* want to reimplement OF partition parser here.
+*/
+   offset = 0x7f;
+   size = 0x1;
+
+   buf = map_physmem(addr, size, MAP_WRBACK);
+   if (!buf)
+   return;
+
+   spi_flash_read(flash, offset, size, buf);
+   unmap_physmem(buf, size);
+}
+
  int misc_init_r(void)
  {
u8 mac[2][6];
@@ -358,6 +404,8 @@ int misc_init_r(void)
eth_env_set_enetaddr_by_index("eth", i, mac[i]);
}
  
+	load_spi_dtb();

+
return 0;
  }
  
diff --git a/include/configs/turris_mox.h b/include/configs/turris_mox.h

index b8ff705ac923..f549f9f7ad06 100644
--- a/include/configs/turris_mox.h
+++ b/include/configs/turris_mox.h
@@ -36,6 +36,7 @@
"bootm 0x580"
  
  #define CONFIG_EXTRA_ENV_SETTINGS\

+   "fdt_addr=0x4c0\0"\
"scriptaddr=0x4d0\0"  \
"pxefile_addr_r=0x4e0\0"  \
"fdt_addr_r=0x4f0\0"  \


Viele Grüße,
Stefan Roese

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


[PATCH] ARM: imx: Update Data Modul i.MX8M Mini eDM SBC DRAM timing

2022-08-30 Thread Marek Vasut
Adjust the DRAM timing settings for this board per ones provided
by hardware department. The change is applied to the LPDDR4 MR11
register CA ODT configuration, from RZQ/6 to RZQ/3, which fixes
stability issues on subset of boards. The DDR PHY PIE block has
been updated accordingly.

Signed-off-by: Marek Vasut 
Cc: Fabio Estevam 
Cc: Peng Fan 
Cc: Stefano Babic 
---
 .../imx8mm_edm_sbc/lpddr4_timing_2G_32.c  | 45 ++-
 .../imx8mm_edm_sbc/lpddr4_timing_4G_32.c  | 40 +
 2 files changed, 45 insertions(+), 40 deletions(-)

diff --git a/board/data_modul/imx8mm_edm_sbc/lpddr4_timing_2G_32.c 
b/board/data_modul/imx8mm_edm_sbc/lpddr4_timing_2G_32.c
index c2abcb54895..f40fd48fc49 100644
--- a/board/data_modul/imx8mm_edm_sbc/lpddr4_timing_2G_32.c
+++ b/board/data_modul/imx8mm_edm_sbc/lpddr4_timing_2G_32.c
@@ -22,7 +22,7 @@ static struct dram_cfg_param ddr_ddrc_cfg[] = {
{ 0x3d4000d4, 0x94 },
{ 0x3d4000dc, 0xd4002d },
{ 0x3d4000e0, 0x31 },
-   { 0x3d4000e8, 0x66004d },
+   { 0x3d4000e8, 0x36004d },
{ 0x3d4000ec, 0x16004d },
{ 0x3d400100, 0x191e1920 },
{ 0x3d400104, 0x60630 },
@@ -55,6 +55,7 @@ static struct dram_cfg_param ddr_ddrc_cfg[] = {
{ 0x3d400204, 0x80808 },
{ 0x3d400214, 0x7070707 },
{ 0x3d400218, 0x7070707 },
+   { 0x3d40021c, 0xf0f },
{ 0x3d400250, 0x29001701 },
{ 0x3d400254, 0x2c },
{ 0x3d40025c, 0x430 },
@@ -72,7 +73,7 @@ static struct dram_cfg_param ddr_ddrc_cfg[] = {
{ 0x3d402064, 0xc001c },
{ 0x3d4020dc, 0x84 },
{ 0x3d4020e0, 0x31 },
-   { 0x3d4020e8, 0x66004d },
+   { 0x3d4020e8, 0x36004d },
{ 0x3d4020ec, 0x16004d },
{ 0x3d402100, 0xa040305 },
{ 0x3d402104, 0x30407 },
@@ -97,7 +98,7 @@ static struct dram_cfg_param ddr_ddrc_cfg[] = {
{ 0x3d403064, 0x30007 },
{ 0x3d4030dc, 0x84 },
{ 0x3d4030e0, 0x31 },
-   { 0x3d4030e8, 0x66004d },
+   { 0x3d4030e8, 0x36004d },
{ 0x3d4030ec, 0x16004d },
{ 0x3d403100, 0xa010102 },
{ 0x3d403104, 0x30404 },
@@ -1059,25 +1060,25 @@ static struct dram_cfg_param ddr_fsp0_cfg[] = {
{ 0x54012, 0x110 },
{ 0x54019, 0x2dd4 },
{ 0x5401a, 0x31 },
-   { 0x5401b, 0x4d66 },
+   { 0x5401b, 0x4d36 },
{ 0x5401c, 0x4d00 },
{ 0x5401e, 0x16 },
{ 0x5401f, 0x2dd4 },
{ 0x54020, 0x31 },
-   { 0x54021, 0x4d66 },
+   { 0x54021, 0x4d36 },
{ 0x54022, 0x4d00 },
{ 0x54024, 0x16 },
{ 0x5402b, 0x1000 },
{ 0x5402c, 0x1 },
{ 0x54032, 0xd400 },
{ 0x54033, 0x312d },
-   { 0x54034, 0x6600 },
+   { 0x54034, 0x3600 },
{ 0x54035, 0x4d },
{ 0x54036, 0x4d },
{ 0x54037, 0x1600 },
{ 0x54038, 0xd400 },
{ 0x54039, 0x312d },
-   { 0x5403a, 0x6600 },
+   { 0x5403a, 0x3600 },
{ 0x5403b, 0x4d },
{ 0x5403c, 0x4d },
{ 0x5403d, 0x1600 },
@@ -1098,25 +1099,25 @@ static struct dram_cfg_param ddr_fsp1_cfg[] = {
{ 0x54012, 0x110 },
{ 0x54019, 0x84 },
{ 0x5401a, 0x31 },
-   { 0x5401b, 0x4d66 },
+   { 0x5401b, 0x4d36 },
{ 0x5401c, 0x4d00 },
{ 0x5401e, 0x16 },
{ 0x5401f, 0x84 },
{ 0x54020, 0x31 },
-   { 0x54021, 0x4d66 },
+   { 0x54021, 0x4d36 },
{ 0x54022, 0x4d00 },
{ 0x54024, 0x16 },
{ 0x5402b, 0x1000 },
{ 0x5402c, 0x1 },
{ 0x54032, 0x8400 },
{ 0x54033, 0x3100 },
-   { 0x54034, 0x6600 },
+   { 0x54034, 0x3600 },
{ 0x54035, 0x4d },
{ 0x54036, 0x4d },
{ 0x54037, 0x1600 },
{ 0x54038, 0x8400 },
{ 0x54039, 0x3100 },
-   { 0x5403a, 0x6600 },
+   { 0x5403a, 0x3600 },
{ 0x5403b, 0x4d },
{ 0x5403c, 0x4d },
{ 0x5403d, 0x1600 },
@@ -1137,25 +1138,25 @@ static struct dram_cfg_param ddr_fsp2_cfg[] = {
{ 0x54012, 0x110 },
{ 0x54019, 0x84 },
{ 0x5401a, 0x31 },
-   { 0x5401b, 0x4d66 },
+   { 0x5401b, 0x4d36 },
{ 0x5401c, 0x4d00 },
{ 0x5401e, 0x16 },
{ 0x5401f, 0x84 },
{ 0x54020, 0x31 },
-   { 0x54021, 0x4d66 },
+   { 0x54021, 0x4d36 },
{ 0x54022, 0x4d00 },
{ 0x54024, 0x16 },
{ 0x5402b, 0x1000 },
{ 0x5402c, 0x1 },
{ 0x54032, 0x8400 },
{ 0x54033, 0x3100 },
-   { 0x54034, 0x6600 },
+   { 0x54034, 0x3600 },
{ 0x54035, 0x4d },
{ 0x54036, 0x4d },
{ 0x54037, 0x1600 },
{ 0x54038, 0x8400 },
{ 0x54039, 0x3100 },
-   { 0x5403a, 0x6600 },
+   { 0x5403a, 0x3600 },
{ 0x5403b, 0x4d },
{ 0x5403c, 0x4d },
{ 0x5403d, 0x1600 },
@@ -1177,25 +1178,25 @@ static struct dram_cfg_param ddr_fsp0_2d_cfg[] = {
{ 0x54012, 0x110 },
{ 0x54019, 0x2dd4 },
{ 0x5401a, 0x31 },
-   {

Re: [PATCH] ARM: imx: Update Data Modul i.MX8M Mini eDM SBC DRAM timing

2022-08-30 Thread Fabio Estevam
On Tue, Aug 30, 2022 at 9:34 AM Marek Vasut  wrote:
>
> Adjust the DRAM timing settings for this board per ones provided
> by hardware department. The change is applied to the LPDDR4 MR11
> register CA ODT configuration, from RZQ/6 to RZQ/3, which fixes
> stability issues on subset of boards. The DDR PHY PIE block has
> been updated accordingly.
>
> Signed-off-by: Marek Vasut 
> Cc: Fabio Estevam 
> Cc: Peng Fan 
> Cc: Stefano Babic 

Looks like this one should be applied to 2022.10:

Reviewed-by: Fabio Estevam 


Re: [PATCH v14 01/10] eficonfig: menu-driven addition of UEFI boot option

2022-08-30 Thread Ilias Apalodimas
On Tue, 30 Aug 2022 at 14:53, Ilias Apalodimas
 wrote:
>
> Hi Kojima-san,
>
> [...]
> > +/**
> > + * eficonfig_choice_entry() - user key input handler
> > + *
> > + * @data:pointer to the efimenu structure
> > + * Return:   key string to identify the selected entry
> > + */
> > +static char *eficonfig_choice_entry(void *data)
> > +{
> > + int esc = 0;
> > + struct list_head *pos, *n;
> > + struct eficonfig_entry *entry;
> > + enum bootmenu_key key = KEY_NONE;
> > + struct efimenu *efi_menu = data;
> > +
> > + while (1) {
> > + bootmenu_loop((struct bootmenu_data *)efi_menu, &key, &esc);
> > +
> > + switch (key) {
> > + case KEY_UP:
> > + if (efi_menu->active > 0)
> > + --efi_menu->active;
> > + /* no menu key selected, regenerate menu */
> > + return NULL;
> > + case KEY_DOWN:
> > + if (efi_menu->active < efi_menu->count - 1)
> > + ++efi_menu->active;
> > + /* no menu key selected, regenerate menu */
> > + return NULL;
> > + case KEY_SELECT:
> > + list_for_each_safe(pos, n, &efi_menu->list) {
> > + entry = list_entry(pos, struct 
> > eficonfig_entry, list);
> > + if (entry->num == efi_menu->active)
> > + return entry->key;
> > + }
> > + break;
> > + case KEY_QUIT:
> > + /* Quit by choosing the last entry */
> > + entry = list_last_entry(&efi_menu->list, struct 
> > eficonfig_entry, list);
> > + return entry->key;
> > + default:
> > + /* Pressed key is not valid, no need to regenerate 
> > the menu */
>
> Even with this comment I am still not sure I am following.
> The function definition is a char *.  There are cases ('default' and
> 'KEY_SELECT') which have no return value at all.
> Shouldn't there be a 'return NULL' in the end of the function, even if
> that's not supposed to happen?

nvm, I missed the while(1) loop at the beginning.  This is fine

Cheers
/Ilias
>
> [...]
>
> Thanks
> /Ilias
>


[PATCH v2 00/17] IPv6 support

2022-08-30 Thread Viacheslav Mitrofanov
This patch set adds basic IPv6 support to U-boot.
It is based on Chris's Packham patches
(https://lists.denx.de/pipermail/u-boot/2017-January/279366.html)
Chris's patches were taken as base. There were efforts to launch it on HiFive
SiFive Unmatched board but the board didn't work well. The code was refactored,
fixed some bugs as CRC for little-endian, some parts were implemented in our own
way, something was taken from Linux. Finally we did manual tests and the board
worked well.

Testing was done on HiFive SiFive Unmatched board (RISC-V)

Signed-off-by: Viacheslav Mitrofanov 

---
Changes in v2:
 - Split big patches into smaller
 - If an address in tftpboot is IPv6 than use IPv6 to boot
 - Add tests

Viacheslav Mitrofanov (17):
  net: ipv6: Add IPv6 basic primitives
  net: ipv6: Add IPv6 build options
  net: ipv6: Add callbacks declarations to get access to IPv6 variables
  net: ipv6: Add Neighbor Discovery Protocol (NDP)
  net: ipv6: Add string_to_ip6 converter
  net: ipv6: Enable IPv6 typeconversion specifier
  net: ipv6: Add ip6addr, gatewayip6, serverip6 variables callbacks
  net: ipv6: Add implementation of main IPv6 functions
  net: ipv6: Incorporate IPv6 support into u-boot net subsystem
  net: tftp: Add IPv6 support for tftpboot
  net: ping6: Add ping6 command
  test: dm: eth: Add string_to_ip6 test
  test: dm: eth: Add csum_ipv6_magic test
  test: dm: eth: Add ip6_addr_in_subnet test
  test: dm: eth: Add ip6_make_snma test
  test: dm: eth: Add ip6_make_lladdr test
  test/py: add a ping6 test

 cmd/Kconfig   |   7 +
 cmd/net.c |  48 
 include/env_callback.h|  10 +
 include/env_flags.h   |  10 +
 include/ndisc.h   |  65 +
 include/net.h |   4 +-
 include/net6.h| 312 
 lib/net_utils.c   | 122 ++
 lib/vsprintf.c|  11 +-
 net/Kconfig   |   4 +
 net/Makefile  |   3 +
 net/ndisc.c   | 276 ++
 net/net.c |  70 +-
 net/net6.c| 484 ++
 net/ping6.c   | 117 +
 net/tftp.c|  62 -
 test/dm/eth.c | 140 +++
 test/py/tests/test_net.py |  15 ++
 18 files changed, 1741 insertions(+), 19 deletions(-)
 create mode 100644 include/ndisc.h
 create mode 100644 include/net6.h
 create mode 100644 net/ndisc.c
 create mode 100644 net/net6.c
 create mode 100644 net/ping6.c

-- 
2.25.1



[PATCH 01/17] net: ipv6: Add IPv6 basic primitives

2022-08-30 Thread Viacheslav Mitrofanov
This patch is a collection of basic primitives that are prerequisite for
further IPv6 implementation.

There are structures definition such as IPv6 header, UDP header
(for TFTP), ICMPv6 header. There are auxiliary defines such as protocol
codes, padding, struct size and etc. Also here are functions prototypes
and its empty implementation that will be used as API for further patches.
Here are variables declaration such as IPv6 address of our host,
gateway, ipv6 server.

Signed-off-by: Viacheslav Mitrofanov 
---
 include/net6.h | 256 +
 net/net6.c |  33 +++
 2 files changed, 289 insertions(+)
 create mode 100644 include/net6.h
 create mode 100644 net/net6.c

diff --git a/include/net6.h b/include/net6.h
new file mode 100644
index 00..71072d1416
--- /dev/null
+++ b/include/net6.h
@@ -0,0 +1,256 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (C) 2013 Allied Telesis Labs NZ
+ * Chris Packham, 
+ *
+ * Copyright (C) 2022 YADRO
+ * Viacheslav Mitrofanov 
+ */
+
+#ifndef __NET6_H__
+#define __NET6_H__
+
+#include 
+#include 
+
+struct in6_addr {
+   union {
+   u8  u6_addr8[16];
+   __be16  u6_addr16[8];
+   __be32  u6_addr32[4];
+   } in6_u;
+
+#define s6_addrin6_u.u6_addr8
+#define s6_addr16  in6_u.u6_addr16
+#define s6_addr32  in6_u.u6_addr32
+};
+
+#define PROT_IP6   0x86DD  /* IPv6 protocol */
+
+#define IN6ADDRSZ  sizeof(struct in6_addr)
+#define INETHADDRSZsizeof(net_ethaddr)
+
+#define IPV6_ADDRSCOPE_INTF0x01
+#define IPV6_ADDRSCOPE_LINK0x02
+#define IPV6_ADDRSCOPE_AMDIN   0x04
+#define IPV6_ADDRSCOPE_SITE0x05
+#define IPV6_ADDRSCOPE_ORG 0x08
+#define IPV6_ADDRSCOPE_GLOBAL  0x0E
+
+#define USE_IP6_CMD_PARAM  "-ipv6"
+
+/**
+ * struct ipv6hdr - Internet Protocol V6 (IPv6) header.
+ *
+ * IPv6 packet header as defined in RFC 2460.
+ */
+struct ip6_hdr {
+#if defined(__LITTLE_ENDIAN_BITFIELD)
+   u8  priority:4,
+   version:4;
+#elif defined(__BIG_ENDIAN_BITFIELD)
+   u8  version:4,
+   priority:4;
+#else
+#error  "Please fix "
+#endif
+   u8  flow_lbl[3];
+   __be16  payload_len;
+   u8  nexthdr;
+   u8  hop_limit;
+   struct in6_addr saddr;
+   struct in6_addr daddr;
+};
+
+struct udp_hdr {
+   u16 udp_src;/* UDP source port  */
+   u16 udp_dst;/* UDP destination port */
+   u16 udp_len;/* Length of UDP packet */
+   u16 udp_xsum;   /* Checksum */
+} __packed;
+#define IP6_HDR_SIZE (sizeof(struct ip6_hdr))
+
+/* Handy for static initialisations of struct in6_addr, atlhough the
+ * c99 '= { 0 }' idiom might work depending on you compiler.
+ */
+#define ZERO_IPV6_ADDR { { { 0x00, 0x00, 0x00, 0x00, \
+ 0x00, 0x00, 0x00, 0x00, \
+ 0x00, 0x00, 0x00, 0x00, \
+ 0x00, 0x00, 0x00, 0x00 } } }
+
+#define IPV6_LINK_LOCAL_PREFIX 0xfe80
+
+enum {
+   __ND_OPT_PREFIX_INFO_END= 0,
+   ND_OPT_SOURCE_LL_ADDR   = 1,
+   ND_OPT_TARGET_LL_ADDR   = 2,
+   ND_OPT_PREFIX_INFO  = 3,
+   ND_OPT_REDIRECT_HDR = 4,
+   ND_OPT_MTU  = 5,
+   __ND_OPT_MAX
+};
+
+/* ICMPv6 */
+#define IPPROTO_ICMPV6 58
+/* hop limit for neighbour discovery packets */
+#define IPV6_NDISC_HOPLIMIT 255
+#define NDISC_TIMEOUT  5000UL
+#define NDISC_TIMEOUT_COUNT 3
+
+struct icmp6hdr {
+   u8  icmp6_type;
+#define IPV6_ICMP_ECHO_REQUEST 128
+#define IPV6_ICMP_ECHO_REPLY   129
+#define IPV6_NDISC_ROUTER_SOLICITATION 133
+#define IPV6_NDISC_ROUTER_ADVERTISEMENT134
+#define IPV6_NDISC_NEIGHBOUR_SOLICITATION  135
+#define IPV6_NDISC_NEIGHBOUR_ADVERTISEMENT 136
+#define IPV6_NDISC_REDIRECT137
+   u8  icmp6_code;
+   __be16  icmp6_cksum;
+
+   union {
+   __be32  un_data32[1];
+   __be16  un_data16[2];
+   u8  un_data8[4];
+
+   struct icmpv6_echo {
+   __be16  identifier;
+   __be16  sequence;
+   } u_echo;
+
+   struct icmpv6_nd_advt {
+#if defined(__LITTLE_ENDIAN_BITFIELD)
+   __be32  reserved:5,
+   override:1,
+   solicited:1,
+   router:1,
+   reserved2:24;
+#elif defined(__BIG_ENDIAN_BITFIELD)
+   __be32  router:1,
+   solicited:1,
+   

[PATCH 03/17] net: ipv6: Add callbacks declarations to get access to IPv6 variables

2022-08-30 Thread Viacheslav Mitrofanov
Set up callbacks for main IPv6 variables ip6add, serverip6, gatewayip6
and set options to them in flag file. These variables are often set up by
users.

Signed-off-by: Viacheslav Mitrofanov 
---
 include/env_callback.h | 10 ++
 include/env_flags.h| 10 ++
 2 files changed, 20 insertions(+)

diff --git a/include/env_callback.h b/include/env_callback.h
index d5d2b2fcad..14c6ce0d24 100644
--- a/include/env_callback.h
+++ b/include/env_callback.h
@@ -57,6 +57,15 @@
 #define NET_CALLBACKS
 #endif
 
+#ifdef CONFIG_IPV6
+#define NET6_CALLBACKS \
+   "ip6addr:ip6addr," \
+   "serverip6:serverip6," \
+   "gatewayip6:gatewayip6,"
+#else
+#define NET6_CALLBACKS
+#endif
+
 #ifdef CONFIG_BOOTSTD
 #define BOOTSTD_CALLBACK   "bootmeths:bootmeths,"
 #else
@@ -71,6 +80,7 @@
ENV_DOT_ESCAPE ENV_FLAGS_VAR ":flags," \
"baudrate:baudrate," \
NET_CALLBACKS \
+   NET6_CALLBACKS \
BOOTSTD_CALLBACK \
"loadaddr:loadaddr," \
SILENT_CALLBACK \
diff --git a/include/env_flags.h b/include/env_flags.h
index 313cb8c49a..718d72773c 100644
--- a/include/env_flags.h
+++ b/include/env_flags.h
@@ -67,6 +67,15 @@ enum env_flags_varaccess {
 #define NET_FLAGS
 #endif
 
+#ifdef CONFIG_IPV6
+#define NET6_FLAGS \
+   "ip6addr:s," \
+   "serverip6:s," \
+   "gatewayip6:s"
+#else
+#define NET6_FLAGS
+#endif
+
 #ifndef CONFIG_ENV_OVERWRITE
 #define SERIAL_FLAGS "serial#:so,"
 #else
@@ -76,6 +85,7 @@ enum env_flags_varaccess {
 #define ENV_FLAGS_LIST_STATIC \
ETHADDR_FLAGS \
NET_FLAGS \
+   NET6_FLAGS \
SERIAL_FLAGS \
CONFIG_ENV_FLAGS_LIST_STATIC
 
-- 
2.25.1



[PATCH 02/17] net: ipv6: Add IPv6 build options

2022-08-30 Thread Viacheslav Mitrofanov
Add options to Makefile and Kconfig file to build IPv6

Signed-off-by: Viacheslav Mitrofanov 
---
 net/Kconfig  | 4 
 net/Makefile | 1 +
 2 files changed, 5 insertions(+)

diff --git a/net/Kconfig b/net/Kconfig
index 52e261884d..568e622de8 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -174,6 +174,10 @@ config BOOTP_MAX_ROOT_PATH_LEN
help
  Select maximal length of option 17 root path.
 
+config IPV6
+   bool "IPv6 support"
+   default n
+
 endif   # if NET
 
 config SYS_RX_ETH_BUFFER
diff --git a/net/Makefile b/net/Makefile
index 6c812502d3..4ea2a14f27 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -21,6 +21,7 @@ obj-$(CONFIG_DM_MDIO_MUX) += mdio-mux-uclass.o
 obj-$(CONFIG_NET)  += eth_common.o
 obj-$(CONFIG_CMD_LINK_LOCAL) += link_local.o
 obj-$(CONFIG_NET)  += net.o
+obj-$(CONFIG_IPV6) += net6.o
 obj-$(CONFIG_CMD_NFS)  += nfs.o
 obj-$(CONFIG_CMD_PING) += ping.o
 obj-$(CONFIG_CMD_PCAP) += pcap.o
-- 
2.25.1



[PATCH 04/17] net: ipv6: Add Neighbor Discovery Protocol (NDP)

2022-08-30 Thread Viacheslav Mitrofanov
Implement basic of NDP. It doesn't include such things as Router
Solicitation, Router Advertisement and Redirect. It just has Neighbor
Solicitation and Neighbor Advertisement. Only these two features are used
in u-boot IPv6. Implementation of some NDP functions uses API that was
exposed in "net: ipv6: Add IPv6 basic primitives".

Also this patch inlcudes update in Makefile to build NDP.

Signed-off-by: Viacheslav Mitrofanov 
---
 include/ndisc.h |  65 
 net/Makefile|   1 +
 net/ndisc.c | 276 
 3 files changed, 342 insertions(+)
 create mode 100644 include/ndisc.h
 create mode 100644 net/ndisc.c

diff --git a/include/ndisc.h b/include/ndisc.h
new file mode 100644
index 00..7debdd79d6
--- /dev/null
+++ b/include/ndisc.h
@@ -0,0 +1,65 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (C) 2013 Allied Telesis Labs NZ
+ * Chris Packham, 
+ *
+ * Copyright (C) 2022 YADRO
+ * Viacheslav Mitrofanov 
+ */
+
+#ifndef __NDISC_H__
+#define __NDISC_H__
+
+#include 
+
+struct nd_msg {
+   struct icmp6hdr icmph;
+   struct in6_addr target;
+   __u8opt[0];
+};
+
+struct echo_msg {
+   struct icmp6hdr icmph;
+   __u16   id;
+   __u16   sequence;
+};
+
+/* IPv6 destination address of packet waiting for ND */
+extern struct in6_addr net_nd_sol_packet_ip6;
+/* MAC destination address of packet waiting for ND */
+extern uchar *net_nd_packet_mac;
+/* pointer to packet waiting to be transmitted after ND is resolved */
+extern uchar *net_nd_tx_packet;
+/* size of packet waiting to be transmitted */
+extern int net_nd_tx_packet_size;
+/* the timer for ND resolution */
+extern ulong net_nd_timer_start;
+/* the number of requests we have sent so far */
+extern int net_nd_try;
+
+#ifdef CONFIG_IPV6
+void ndisc_init(void);
+void ndisc_receive(struct ethernet_hdr *et, struct ip6_hdr *ip6, int len);
+void ndisc_request(void);
+int ndisc_timeout_check(void);
+#else
+static inline void ndisc_init(void)
+{
+}
+
+static inline void
+ndisc_receive(struct ethernet_hdr *et, struct ip6_hdr *ip6, int len)
+{
+}
+
+static inline void ndisc_request(void)
+{
+}
+
+static inline int ndisc_timeout_check(void)
+{
+   return 0;
+}
+#endif
+
+#endif /* __NDISC_H__ */
diff --git a/net/Makefile b/net/Makefile
index 4ea2a14f27..766dd04135 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_DM_MDIO)  += mdio-uclass.o
 obj-$(CONFIG_DM_MDIO_MUX) += mdio-mux-uclass.o
 obj-$(CONFIG_NET)  += eth_common.o
 obj-$(CONFIG_CMD_LINK_LOCAL) += link_local.o
+obj-$(CONFIG_IPV6) += ndisc.o
 obj-$(CONFIG_NET)  += net.o
 obj-$(CONFIG_IPV6) += net6.o
 obj-$(CONFIG_CMD_NFS)  += nfs.o
diff --git a/net/ndisc.c b/net/ndisc.c
new file mode 100644
index 00..1bd06211f1
--- /dev/null
+++ b/net/ndisc.c
@@ -0,0 +1,276 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2013 Allied Telesis Labs NZ
+ * Chris Packham, 
+ *
+ * Copyright (C) 2022 YADRO
+ * Viacheslav Mitrofanov 
+ */
+
+/*
+ * Neighbour Discovery for IPv6
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+/* IPv6 destination address of packet waiting for ND */
+struct in6_addr net_nd_sol_packet_ip6 = ZERO_IPV6_ADDR;
+/* IPv6 address we are expecting ND advert from */
+static struct in6_addr net_nd_rep_packet_ip6 = ZERO_IPV6_ADDR;
+/* MAC destination address of packet waiting for ND */
+uchar *net_nd_packet_mac;
+/* pointer to packet waiting to be transmitted after ND is resolved */
+uchar *net_nd_tx_packet;
+static uchar net_nd_packet_buf[PKTSIZE_ALIGN + PKTALIGN];
+/* size of packet waiting to be transmitted */
+int net_nd_tx_packet_size;
+/* the timer for ND resolution */
+ulong net_nd_timer_start;
+/* the number of requests we have sent so far */
+int net_nd_try;
+
+#define IP6_NDISC_OPT_SPACE(len) (((len) + 2 + 7) & ~7)
+
+/**
+ * Insert an option into a neighbor discovery packet.
+ * Returns the number of bytes inserted (which may be >= len)
+ */
+static int
+ndisc_insert_option(struct nd_msg *ndisc, int type, u8 *data, int len)
+{
+   int space = IP6_NDISC_OPT_SPACE(len);
+
+   ndisc->opt[0] = type;
+   ndisc->opt[1] = space >> 3;
+   memcpy(&ndisc->opt[2], data, len);
+   len += 2;
+
+   /* fill the remainder with 0 */
+   if ((space - len) > 0)
+   memset(&ndisc->opt[len], 0, space - len);
+
+   return space;
+}
+
+/**
+ * Extract the Ethernet address from a neighbor discovery packet.
+ * Note that the link layer address could be anything but the only networking
+ * media that u-boot supports is Ethernet so we assume we're extracting a 6
+ * byte Ethernet MAC address.
+ */
+static void ndisc_extract_enetaddr(struct nd_msg *ndisc, uchar enetaddr[6])
+{
+   memcpy(enetaddr, &ndisc->opt[2], 6);
+}
+
+/**
+ * Check to see if the neighbor discovery packet has
+ * the specified option set.
+ */
+static int ndisc_has_option(struct ip6_hdr *ip6, __u8 type)
+{
+   struct

[PATCH 06/17] net: ipv6: Enable IPv6 typeconversion specifier

2022-08-30 Thread Viacheslav Mitrofanov
Add the possibility to recognize IPv6 address in print function.
To output IPv6 address use %pI6 specifier.

Signed-off-by: Viacheslav Mitrofanov 
---
 lib/vsprintf.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index fe06aa2d71..d5856b1cbe 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -346,6 +346,7 @@ static char *mac_address_string(char *buf, char *end, u8 
*addr, int field_width,
  flags & ~SPECIAL);
 }
 
+#if IS_ENABLED(CONFIG_IPV6)
 static char *ip6_addr_string(char *buf, char *end, u8 *addr, int field_width,
 int precision, int flags)
 {
@@ -365,6 +366,7 @@ static char *ip6_addr_string(char *buf, char *end, u8 
*addr, int field_width,
return string(buf, end, ip6_addr, field_width, precision,
  flags & ~SPECIAL);
 }
+#endif
 
 static char *ip4_addr_string(char *buf, char *end, u8 *addr, int field_width,
 int precision, int flags)
@@ -450,10 +452,6 @@ static char *uuid_string(char *buf, char *end, u8 *addr, 
int field_width,
  *   decimal for v4 and colon separated network-order 16 bit hex for v6)
  * - 'i' [46] for 'raw' IPv4/IPv6 addresses, IPv6 omits the colons, IPv4 is
  *   currently the same
- *
- * Note: IPv6 support is currently if(0)'ed out. If you ever need
- * %pI6, please add an IPV6 Kconfig knob, make your code select or
- * depend on that, and change the 0 below to CONFIG_IS_ENABLED(IPV6).
  */
 static char *pointer(const char *fmt, char *buf, char *end, void *ptr,
int field_width, int precision, int flags)
@@ -498,10 +496,11 @@ static char *pointer(const char *fmt, char *buf, char 
*end, void *ptr,
flags |= SPECIAL;
/* Fallthrough */
case 'I':
-   /* %pI6 currently unused */
-   if (0 && fmt[1] == '6')
+#if IS_ENABLED(CONFIG_IPV6)
+   if (fmt[1] == '6')
return ip6_addr_string(buf, end, ptr, field_width,
   precision, flags);
+#endif
if (fmt[1] == '4')
return ip4_addr_string(buf, end, ptr, field_width,
   precision, flags);
-- 
2.25.1



[PATCH 05/17] net: ipv6: Add string_to_ip6 converter

2022-08-30 Thread Viacheslav Mitrofanov
This functions is used as a converter from IPv6 address string notation
to struct ip6_addr that is used everywhere in IPv6 implementation. For
example it is used to parse and convert IPv6 address from tftpboot
command. Conversion algorithm uses two passes, first to verify syntax and
locate colons and second pass to read the address. In case of valid IPv6
address it returns 0.

Examples of valid strings:
2001:db8::0:1234:1
2001:0db8:::::1234:0001
::1
:::192.168.1.1

Examples of invalid strings
2001:db8::0::0  (:: can only appear once)
2001:db8:192.168.1.1::1 (v4 part can only appear at the end)
192.168.1.1 (we don't implicity map v4)

Signed-off-by: Viacheslav Mitrofanov 
---
 include/net6.h  |   5 ++
 lib/net_utils.c | 122 
 2 files changed, 127 insertions(+)

diff --git a/include/net6.h b/include/net6.h
index 71072d1416..68fa38adbb 100644
--- a/include/net6.h
+++ b/include/net6.h
@@ -177,10 +177,15 @@ extern u32 net_prefix_length; /* Our prefixlength (0 
= unknown) */
 extern struct in6_addr net_server_ip6; /* Server IPv6 addr (0 = unknown) */
 extern bool use_ip6;
 
+#if IS_ENABLED(CONFIG_IPV6)
+/* Convert a string to an ipv6 address */
+int string_to_ip6(const char *s, struct in6_addr *addr);
+#else
 static inline int string_to_ip6(const char *s, struct in6_addr *addr)
 {
return -1;
 }
+#endif
 
 static inline int ip6_is_unspecified_addr(struct in6_addr *addr)
 {
diff --git a/lib/net_utils.c b/lib/net_utils.c
index 72a3b098a7..09554c520b 100644
--- a/lib/net_utils.c
+++ b/lib/net_utils.c
@@ -11,6 +11,7 @@
 
 #include 
 #include 
+#include 
 
 struct in_addr string_to_ip(const char *s)
 {
@@ -43,6 +44,127 @@ struct in_addr string_to_ip(const char *s)
return addr;
 }
 
+/**
+ * Parses an struct in6_addr from the given string. IPv6 address parsing is a
+ * bit more complicated than v4 due to the flexible format and some of the
+ * special cases (e.g. v4 mapped).
+ *
+ * Examples of valid strings:
+ *   2001:db8::0:1234:1
+ *   2001:0db8:::::1234:0001
+ *   ::1
+ *   :::192.168.1.1
+ *
+ * Examples of invalid strings
+ *   2001:db8::0::0  (:: can only appear once)
+ *   2001:db8:192.168.1.1::1 (v4 part can only appear at the end)
+ *   192.168.1.1 (we don't implicity map v4)
+ */
+#if IS_ENABLED(CONFIG_IPV6)
+int string_to_ip6(const char *strpt, struct in6_addr *addrpt)
+{
+   int colon_count = 0;
+   int found_double_colon = 0;
+   int xstart = 0; /* first zero (double colon) */
+   int len = 7;/* num words the double colon represents */
+   int i;
+   const char *s = strpt;
+   struct in_addr zero_ip = {.s_addr = 0};
+
+   if (!strpt)
+   return -1;
+
+   /* First pass, verify the syntax and locate the double colon */
+   for (;;) {
+   while (isxdigit((int)*s))
+   s++;
+   if (*s == '\0')
+   break;
+   if (*s != ':') {
+   if (*s == '.' && len >= 2) {
+   struct in_addr v4;
+
+   while (s != strpt && *(s - 1) != ':')
+   --s;
+   v4 = string_to_ip(s);
+   if (memcmp(&zero_ip, &v4,
+  sizeof(struct in_addr) != 0)) {
+   len -= 2;
+   break;
+   }
+   }
+   /* This could be a valid address */
+   break;
+   }
+   if (s == strpt) {
+   /* The address begins with a colon */
+   if (*++s != ':')
+   /* Must start with a double colon or a number */
+   goto out_err;
+   } else {
+   s++;
+   if (found_double_colon)
+   len--;
+   else
+   xstart++;
+   }
+
+   if (*s == ':') {
+   if (found_double_colon)
+   /* Two double colons are not allowed */
+   goto out_err;
+   found_double_colon = 1;
+   len -= xstart;
+   s++;
+   }
+
+   if (++colon_count == 7)
+   /* Found all colons */
+   break;
+   }
+
+   if (colon_count == 0)
+   goto out_err;
+   if (*--s == ':')
+   len++;
+
+   /* Second pass, read the address */
+   s = strpt;
+   for (i = 0; i < 8; i++) {
+   int val = 0;
+  

[PATCH 07/17] net: ipv6: Add ip6addr, gatewayip6, serverip6 variables callbacks

2022-08-30 Thread Viacheslav Mitrofanov
Implement actions on ip6addr, gatewayip6, serverip6 varaibles.
on_ip6addr - convert IPv6 string addr to struct ip6_addr
on_gatewayip6 - convert IPv6 string addr to struct ip6_addr
on_serverip6 - convert IPv6 string addr to struct ip6_addr

Signed-off-by: Viacheslav Mitrofanov 
---
 net/net6.c | 67 ++
 1 file changed, 67 insertions(+)

diff --git a/net/net6.c b/net/net6.c
index c2569e0e68..a0410ea8ba 100644
--- a/net/net6.c
+++ b/net/net6.c
@@ -31,3 +31,70 @@ struct in6_addr net_server_ip6 = ZERO_IPV6_ADDR;
 u32 net_prefix_length;
 
 bool use_ip6;
+
+static int on_ip6addr(const char *name, const char *value, enum env_op op,
+ int flags)
+{
+   char *v, *s, *strcopy;
+   int i;
+
+   if (flags & H_PROGRAMMATIC)
+   return 0;
+
+   if (op == env_op_delete) {
+   net_prefix_length = 0;
+   net_copy_ip6(&net_ip6, &net_null_addr_ip6);
+   return 0;
+   }
+
+   strcopy = strdup(value);
+   if (!strcopy)
+   return -1;
+
+   net_prefix_length = 128;
+   i = 0;
+   s = strcopy;
+   while (s) {
+   v = strsep(&s, "/");
+   if (!v)
+   break;
+
+   switch (i++) {
+   case 0:
+   string_to_ip6(v, &net_ip6);
+   break;
+   case 1:
+   net_prefix_length = simple_strtoul(v, NULL, 10);
+   break;
+   default:
+   break;
+   }
+   }
+   free(strcopy);
+
+   return 0;
+}
+
+U_BOOT_ENV_CALLBACK(ip6addr, on_ip6addr);
+
+static int on_gatewayip6(const char *name, const char *value, enum env_op op,
+int flags)
+{
+   if (flags & H_PROGRAMMATIC)
+   return 0;
+
+   return string_to_ip6(value, &net_gateway6);
+}
+
+U_BOOT_ENV_CALLBACK(gatewayip6, on_gatewayip6);
+
+static int on_serverip6(const char *name, const char *value, enum env_op op,
+   int flags)
+{
+   if (flags & H_PROGRAMMATIC)
+   return 0;
+
+   return string_to_ip6(value, &net_server_ip6);
+}
+
+U_BOOT_ENV_CALLBACK(serverip6, on_serverip6);
-- 
2.25.1



[PATCH 08/17] net: ipv6: Add implementation of main IPv6 functions

2022-08-30 Thread Viacheslav Mitrofanov
Functions that were exposed in "net: ipv6: Add IPv6 basic primitives"
had only empty implementations and were exposed as API for futher
patches. This patch add implementation of these functions. Main
functions are: net_ip6_handler() - IPv6 packet handler for incoming
packets; net_send_udp_packet6() - make up and send an UDP packet;
csum_ipv6_magic() - compute checksum of IPv6 "psuedo-header" per RFC2460
section 8.1; ip6_addr_in_subnet() - check if an address is in our
subnet. Other functions are auxiliary.

Signed-off-by: Viacheslav Mitrofanov 
---
 include/net6.h |  36 -
 net/net6.c | 380 +
 2 files changed, 415 insertions(+), 1 deletion(-)

diff --git a/include/net6.h b/include/net6.h
index 68fa38adbb..a7be2496d9 100644
--- a/include/net6.h
+++ b/include/net6.h
@@ -180,12 +180,45 @@ extern bool use_ip6;
 #if IS_ENABLED(CONFIG_IPV6)
 /* Convert a string to an ipv6 address */
 int string_to_ip6(const char *s, struct in6_addr *addr);
+
+/* check that an IPv6 address is unspecified (zero) */
+int ip6_is_unspecified_addr(struct in6_addr *addr);
+/* check that an IPv6 address is ours */
+int ip6_is_our_addr(struct in6_addr *addr);
+/* check if neighbour is in the same subnet as us */
+int ip6_addr_in_subnet(struct in6_addr *our_addr, struct in6_addr *neigh_addr,
+  u32 prefix_length);
+
+void ip6_make_lladdr(struct in6_addr *lladr, unsigned char const enetaddr[6]);
+void ip6_make_snma(struct in6_addr *mcast_addr, struct in6_addr *ip6_addr);
+void ip6_make_mult_ethdstaddr(unsigned char enetaddr[6],
+ struct in6_addr *mcast_addr);
+
+unsigned int csum_partial(const unsigned char *buff, int len, unsigned int 
sum);
+unsigned short int csum_ipv6_magic(struct in6_addr *saddr,
+  struct in6_addr *daddr, u16 len,
+  unsigned short proto, unsigned int csum);
+
+int ip6_add_hdr(uchar *xip, struct in6_addr *src, struct in6_addr *dest,
+   int nextheader, int hoplimit, int payload_len);
+
+/* Transmit UDP packet using IPv6, performing neighbour discovery if needed */
+int net_send_udp_packet6(uchar *ether, struct in6_addr *dest,
+int dport, int sport, int len);
+
+/* handler for incoming IPv6 echo packet */
+void net_ip6_handler(struct ethernet_hdr *et, struct ip6_hdr *ip6,
+int len);
+
+static inline void net_copy_ip6(void *to, const void *from)
+{
+   memcpy((void *)to, from, sizeof(struct in6_addr));
+}
 #else
 static inline int string_to_ip6(const char *s, struct in6_addr *addr)
 {
return -1;
 }
-#endif
 
 static inline int ip6_is_unspecified_addr(struct in6_addr *addr)
 {
@@ -257,5 +290,6 @@ net_ip6_handler(struct ethernet_hdr *et, struct ip6_hdr 
*ip6,
 static inline void net_copy_ip6(void *to, const void *from)
 {
 }
+#endif
 
 #endif /* __NET6_H__ */
diff --git a/net/net6.c b/net/net6.c
index a0410ea8ba..0799d411b2 100644
--- a/net/net6.c
+++ b/net/net6.c
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /* NULL IPv6 address */
 struct in6_addr const net_null_addr_ip6 = ZERO_IPV6_ADDR;
@@ -98,3 +99,382 @@ static int on_serverip6(const char *name, const char 
*value, enum env_op op,
 }
 
 U_BOOT_ENV_CALLBACK(serverip6, on_serverip6);
+
+int ip6_is_unspecified_addr(struct in6_addr *addr)
+{
+   return (addr->s6_addr32[0] | addr->s6_addr32[1] |
+   addr->s6_addr32[2] | addr->s6_addr32[3]) == 0;
+}
+
+/**
+ * We have 2 addresses that we should respond to. A link
+ * local address and a global address. This returns true
+ * if the specified address matches either of these.
+ */
+int ip6_is_our_addr(struct in6_addr *addr)
+{
+   return !memcmp(addr, &net_link_local_ip6, sizeof(struct in6_addr)) ||
+  !memcmp(addr, &net_ip6, sizeof(struct in6_addr));
+}
+
+void ip6_make_eui(unsigned char eui[8], unsigned char const enetaddr[6])
+{
+   memcpy(eui, enetaddr, 3);
+   memcpy(&eui[5], &enetaddr[3], 3);
+   eui[3] = 0xFF;
+   eui[4] = 0xFE;
+   eui[0] ^= 2;/* "u" bit set to indicate global scope */
+}
+
+void ip6_make_lladdr(struct in6_addr *lladr, unsigned char const enetaddr[6])
+{
+   uchar eui[8];
+
+   memset(lladr, 0, sizeof(struct in6_addr));
+   lladr->s6_addr16[0] = htons(IPV6_LINK_LOCAL_PREFIX);
+   ip6_make_eui(eui, enetaddr);
+   memcpy(&lladr->s6_addr[8], eui, 8);
+}
+
+/*
+ * Given an IPv6 address generate an equivalent Solicited Node Multicast
+ * Address (SNMA) as described in RFC2461.
+ */
+void ip6_make_snma(struct in6_addr *mcast_addr, struct in6_addr *ip6_addr)
+{
+   memset(mcast_addr, 0, sizeof(struct in6_addr));
+   mcast_addr->s6_addr[0] = 0xff;
+   mcast_addr->s6_addr[1] = IPV6_ADDRSCOPE_LINK;
+   mcast_addr->s6_addr[11] = 0x01;
+   mcast_addr->s6_addr[12] = 0xff;
+   mcast_addr->s6_addr[13] = ip6_addr->s6_addr[13];
+   mcast_addr->s6_addr[14] =

[PATCH 09/17] net: ipv6: Incorporate IPv6 support into u-boot net subsystem

2022-08-30 Thread Viacheslav Mitrofanov
Add net_ip6_handler (an IPv6 packet handler) into net_loop. Add
neighbor discovery mechanism into network init process. That is the
main step to run IPv6 in u-boot. Now u-boot is capable to use NDP and
handle IPv6 packets.

Signed-off-by: Viacheslav Mitrofanov 
---
 net/net.c | 23 ++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/net/net.c b/net/net.c
index 81905f6315..f818170930 100644
--- a/net/net.c
+++ b/net/net.c
@@ -91,6 +91,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 #include 
 #if defined(CONFIG_CMD_PCAP)
@@ -340,8 +342,17 @@ void net_auto_load(void)
 
 static int net_init_loop(void)
 {
-   if (eth_get_dev())
+   if (eth_get_dev()) {
memcpy(net_ethaddr, eth_get_ethaddr(), 6);
+
+   if (IS_ENABLED(CONFIG_IPV6)) {
+   ip6_make_lladdr(&net_link_local_ip6, net_ethaddr);
+   if (!memcmp(&net_ip6, &net_null_addr_ip6,
+   sizeof(struct in6_addr)))
+   memcpy(&net_ip6, &net_link_local_ip6,
+  sizeof(struct in6_addr));
+   }
+   }
else
/*
 * Not ideal, but there's no way to get the actual error, and I
@@ -382,6 +393,7 @@ int net_init(void)
(i + 1) * PKTSIZE_ALIGN;
}
arp_init();
+   ndisc_init();
net_clear_handlers();
 
/* Only need to setup buffer pointers once. */
@@ -563,6 +575,10 @@ restart:
if (arp_timeout_check() > 0)
time_start = get_timer(0);
 
+   if (IS_ENABLED(CONFIG_IPV6)) {
+   if (use_ip6 && (ndisc_timeout_check() > 0))
+   time_start = get_timer(0);
+   }
/*
 *  Check the ethernet for a new packet.  The ethernet
 *  receive routine will process it.
@@ -1187,6 +1203,11 @@ void net_process_received_packet(uchar *in_packet, int 
len)
case PROT_RARP:
rarp_receive(ip, len);
break;
+#endif
+#if IS_ENABLED(CONFIG_IPV6)
+   case PROT_IP6:
+   net_ip6_handler(et, (struct ip6_hdr *)ip, len);
+   break;
 #endif
case PROT_IP:
debug_cond(DEBUG_NET_PKT, "Got IP\n");
-- 
2.25.1



[PATCH 10/17] net: tftp: Add IPv6 support for tftpboot

2022-08-30 Thread Viacheslav Mitrofanov
The command tftpboot uses IPv4 by default. Add the possibility to use IPv6
instead. If an address in the command is an IPv6 address it will use IPv6
to boot or if there is a suffix -ipv6 in the end of the command it also
force using IPv6. All other tftpboot features and parameters are left
the same.

Signed-off-by: Viacheslav Mitrofanov 
---
 cmd/net.c  | 22 +++
 net/net.c  | 34 --
 net/tftp.c | 62 +++---
 3 files changed, 108 insertions(+), 10 deletions(-)

diff --git a/cmd/net.c b/cmd/net.c
index 3619c843d8..0225f9ce3e 100644
--- a/cmd/net.c
+++ b/cmd/net.c
@@ -14,6 +14,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -44,12 +45,22 @@ int do_tftpb(struct cmd_tbl *cmdtp, int flag, int argc, 
char *const argv[])
return ret;
 }
 
+#if IS_ENABLED(CONFIG_IPV6)
+U_BOOT_CMD(
+   tftpboot,   4,  1,  do_tftpb,
+   "boot image via network using TFTP protocol\n"
+   "To use IPv6 add -ipv6 parameter or use IPv6 hostIPaddr framed "
+   "with [] brackets",
+   "[loadAddress] [[hostIPaddr:]bootfilename] [" USE_IP6_CMD_PARAM "]"
+);
+#else
 U_BOOT_CMD(
tftpboot,   3,  1,  do_tftpb,
"boot image via network using TFTP protocol",
"[loadAddress] [[hostIPaddr:]bootfilename]"
 );
 #endif
+#endif
 
 #ifdef CONFIG_CMD_TFTPPUT
 static int do_tftpput(struct cmd_tbl *cmdtp, int flag, int argc,
@@ -205,6 +216,17 @@ static int netboot_common(enum proto_t proto, struct 
cmd_tbl *cmdtp, int argc,
if (s != NULL)
image_load_addr = hextoul(s, NULL);
 
+   if (IS_ENABLED(CONFIG_IPV6)) {
+   use_ip6 = false;
+
+   /* IPv6 parameter has to be always *last* */
+   if (!strcmp(argv[argc - 1], USE_IP6_CMD_PARAM)) {
+   use_ip6 = true;
+   /* It is a hack not to break switch/case code */
+   --argc;
+   }
+   }
+
switch (argc) {
case 1:
/* refresh bootfile name from env */
diff --git a/net/net.c b/net/net.c
index f818170930..77f42d1d59 100644
--- a/net/net.c
+++ b/net/net.c
@@ -449,6 +449,23 @@ restart:
debug_cond(DEBUG_INT_STATE, "--- net_loop Init\n");
net_init_loop();
 
+   /* Find out what protocol to use */
+   if (IS_ENABLED(CONFIG_IPV6) && !use_ip6) {
+   char *s, *e;
+   char str_ip6[16];
+
+   s = strchr(net_boot_file_name, '[');
+   e = strchr(net_boot_file_name, ']');
+   if (s && e) {
+   size_t len = e - s;
+
+   memcpy(str_ip6, s + 1, len);
+   str_ip6[len - 1] = '\0';
+   if (!string_to_ip6(str_ip6, &net_server_ip6))
+   use_ip6 = true;
+   }
+   }
+
switch (net_check_prereq(protocol)) {
case 1:
/* network not configured */
@@ -1381,7 +1398,14 @@ static int net_check_prereq(enum proto_t protocol)
/* Fall through */
case TFTPGET:
case TFTPPUT:
-   if (net_server_ip.s_addr == 0 && !is_serverip_in_cmd()) {
+   if (IS_ENABLED(CONFIG_IPV6) && use_ip6) {
+   if (!memcmp(&net_server_ip6, &net_null_addr_ip6,
+   sizeof(struct in6_addr)) &&
+   !strchr(net_boot_file_name, '[')) {
+   puts("*** ERROR: `serverip6' not set\n");
+   return 1;
+   }
+   } else if (net_server_ip.s_addr == 0 && !is_serverip_in_cmd()) {
puts("*** ERROR: `serverip' not set\n");
return 1;
}
@@ -1394,7 +1418,13 @@ common:
case NETCONS:
case FASTBOOT:
case TFTPSRV:
-   if (net_ip.s_addr == 0) {
+   if (IS_ENABLED(CONFIG_IPV6) && use_ip6) {
+   if (!memcmp(&net_link_local_ip6, &net_null_addr_ip6,
+   sizeof(struct in6_addr))) {
+   puts("*** ERROR: `ip6addr` not set\n");
+   return 1;
+   }
+   } else if (net_ip.s_addr == 0) {
puts("*** ERROR: `ipaddr' not set\n");
return 1;
}
diff --git a/net/tftp.c b/net/tftp.c
index dea9c25ffd..52ff1a846d 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -15,6 +15,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include "bootp.h"
@@ -41,6 +42,7 @@ DECLARE_GLOBAL_DATA_PTR;
 static ulong timeout_ms = TIMEOUT;
 static int timeout_count_max = (CONFIG_NET_RETRY_COUNT * 2);
 static ulong time_start;   /* Record time we started tftp */
+static struct in6_addr tftp_remote_ip6;
 
 /*
  * These 

[PATCH 12/17] test: dm: eth: Add string_to_ip6 test

2022-08-30 Thread Viacheslav Mitrofanov
Add a test to check convertation from char* to struct in6_addr.
Use in sandbox

Signed-off-by: Viacheslav Mitrofanov 
---
 test/dm/eth.c | 48 
 1 file changed, 48 insertions(+)

diff --git a/test/dm/eth.c b/test/dm/eth.c
index 5437f9ea4a..4cc90cf514 100644
--- a/test/dm/eth.c
+++ b/test/dm/eth.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -22,6 +23,53 @@
 
 #define DM_TEST_ETH_NUM4
 
+static int dm_test_string_to_ip6(struct unit_test_state *uts)
+{
+   struct test_ip6_pair
+   {
+   char*string_addr;
+   struct in6_addr ip6_addr;
+   };
+
+   struct in6_addr ip6 = {0};
+
+   /* Correct statements */
+   struct test_ip6_pair test_suite[] = {
+   {"2001:db8::0:1234:1", {.s6_addr32[0] = 0xb80d0120,
+   .s6_addr32[1] = 0x,
+   .s6_addr32[2] = 0x,
+   .s6_addr32[3] = 0x01003412}},
+   {"2001:0db8:::::1234:0001",
+  {.s6_addr32[0] = 0xb80d0120,
+   .s6_addr32[1] = 0x,
+   .s6_addr32[2] = 0x,
+   .s6_addr32[3] = 0x01003412}},
+   {"::1",{.s6_addr32[0] = 0x,
+   .s6_addr32[1] = 0x,
+   .s6_addr32[2] = 0x,
+   .s6_addr32[3] = 0x0100}},
+   {":::192.168.1.1", {.s6_addr32[0] = 0x,
+   .s6_addr32[1] = 0x,
+   .s6_addr32[2] = 0x,
+   .s6_addr32[3] = 0x0101a8c0}},
+   };
+
+   for (int i = 0; i < ARRAY_SIZE(test_suite); ++i) {
+   ut_assertok(string_to_ip6(test_suite[i].string_addr, &ip6));
+   ut_asserteq_mem(&ip6, &test_suite[i].ip6_addr,
+   sizeof(struct in6_addr));
+   }
+
+   /* Incorrect statements */
+   ut_assertok(!string_to_ip6("hello:world", &ip6));
+   ut_assertok(!string_to_ip6("2001:db8::0::0", &ip6));
+   ut_assertok(!string_to_ip6("2001:db8:192.168.1.1::1", &ip6));
+   ut_assertok(!string_to_ip6("192.168.1.1", &ip6));
+
+   return 0;
+}
+DM_TEST(dm_test_string_to_ip6, 0);
+
 static int dm_test_eth(struct unit_test_state *uts)
 {
net_ping_ip = string_to_ip("1.1.2.2");
-- 
2.25.1



[PATCH 11/17] net: ping6: Add ping6 command

2022-08-30 Thread Viacheslav Mitrofanov
Implement ping6 command to ping hosts using IPv6. It works the same way as
an ordinary ping command. There is no ICMP request so it is not possible
to ping our host. This patch adds options in Kconfig and Makefile to
build ping6 command.

Signed-off-by: Viacheslav Mitrofanov 
---
 cmd/Kconfig|   7 +++
 cmd/net.c  |  26 +++
 include/net.h  |   4 +-
 include/net6.h |  17 +++
 net/Makefile   |   1 +
 net/net.c  |  13 ++
 net/net6.c |   4 ++
 net/ping6.c| 117 +
 8 files changed, 187 insertions(+), 2 deletions(-)
 create mode 100644 net/ping6.c

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 8ea064b8d2..4c95c6fd9b 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1801,6 +1801,13 @@ config CMD_PING
help
  Send ICMP ECHO_REQUEST to network host
 
+config CMD_PING6
+   bool "ping6"
+   depends on IPV6
+   default y if (CMD_PING && IPV6)
+   help
+ Send ICMPv6 ECHO_REQUEST to network host
+
 config CMD_CDP
bool "cdp"
help
diff --git a/cmd/net.c b/cmd/net.c
index 0225f9ce3e..cd24813bcd 100644
--- a/cmd/net.c
+++ b/cmd/net.c
@@ -334,6 +334,32 @@ U_BOOT_CMD(
 );
 #endif
 
+#if IS_ENABLED(CONFIG_CMD_PING6)
+int do_ping6(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[])
+{
+   if (string_to_ip6(argv[1], &net_ping_ip6) != 0)
+   return CMD_RET_USAGE;
+
+   use_ip6 = true;
+   if (net_loop(PING6) < 0) {
+   use_ip6 = false;
+   printf("ping6 failed; host %pI6c is not alive\n",
+  &net_ping_ip6);
+   return 1;
+   }
+
+   use_ip6 = false;
+   printf("host %pI6c is alive\n", &net_ping_ip6);
+   return 0;
+}
+
+U_BOOT_CMD(
+   ping6,  2,  1,  do_ping6,
+   "send ICMPv6 ECHO_REQUEST to network host",
+   "pingAddress"
+);
+#endif /* CONFIG_CMD_PING6 */
+
 #if defined(CONFIG_CMD_CDP)
 
 static void cdp_update_env(void)
diff --git a/include/net.h b/include/net.h
index c06b577808..72d32d358a 100644
--- a/include/net.h
+++ b/include/net.h
@@ -559,8 +559,8 @@ extern ushort   net_native_vlan;/* Our 
Native VLAN */
 extern int net_restart_wrap;   /* Tried all network devices */
 
 enum proto_t {
-   BOOTP, RARP, ARP, TFTPGET, DHCP, PING, DNS, NFS, CDP, NETCONS, SNTP,
-   TFTPSRV, TFTPPUT, LINKLOCAL, FASTBOOT, WOL, UDP
+   BOOTP, RARP, ARP, TFTPGET, DHCP, PING, PING6, DNS, NFS, CDP, NETCONS,
+   SNTP, TFTPSRV, TFTPPUT, LINKLOCAL, FASTBOOT, WOL, UDP
 };
 
 extern charnet_boot_file_name[1024];/* Boot File name */
diff --git a/include/net6.h b/include/net6.h
index a7be2496d9..08d31b8197 100644
--- a/include/net6.h
+++ b/include/net6.h
@@ -175,6 +175,7 @@ extern struct in6_addr net_ip6; /* Our IPv6 addr (0 = 
unknown) */
 extern struct in6_addr net_link_local_ip6; /* Our link local IPv6 addr */
 extern u32 net_prefix_length;  /* Our prefixlength (0 = unknown) */
 extern struct in6_addr net_server_ip6; /* Server IPv6 addr (0 = unknown) */
+extern struct in6_addr net_ping_ip6; /* the ipv6 address to ping */
 extern bool use_ip6;
 
 #if IS_ENABLED(CONFIG_IPV6)
@@ -292,4 +293,20 @@ static inline void net_copy_ip6(void *to, const void *from)
 }
 #endif
 
+#if IS_ENABLED(CONFIG_CMD_PING6)
+/* starts a Ping6 process */
+void ping6_start(void);
+/* handles reception of icmpv6 echo request/reply */
+void ping6_receive(struct ethernet_hdr *et, struct ip6_hdr *ip6, int len);
+#else
+static inline void ping6_start(void)
+{
+}
+
+static inline
+void ping6_receive(struct ethernet_hdr *et, struct ip6_hdr *ip6, int len)
+{
+}
+#endif /* CONFIG_CMD_PING6 */
+
 #endif /* __NET6_H__ */
diff --git a/net/Makefile b/net/Makefile
index 766dd04135..e32e913d68 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_NET)  += net.o
 obj-$(CONFIG_IPV6) += net6.o
 obj-$(CONFIG_CMD_NFS)  += nfs.o
 obj-$(CONFIG_CMD_PING) += ping.o
+obj-$(CONFIG_CMD_PING6) += ping6.o
 obj-$(CONFIG_CMD_PCAP) += pcap.o
 obj-$(CONFIG_CMD_RARP) += rarp.o
 obj-$(CONFIG_CMD_SNTP) += sntp.o
diff --git a/net/net.c b/net/net.c
index 77f42d1d59..babc4e2c17 100644
--- a/net/net.c
+++ b/net/net.c
@@ -526,6 +526,11 @@ restart:
ping_start();
break;
 #endif
+#if defined(CONFIG_CMD_PING6)
+   case PING6:
+   ping6_start();
+   break;
+#endif
 #if defined(CONFIG_CMD_NFS) && !defined(CONFIG_SPL_BUILD)
case NFS:
nfs_start();
@@ -1377,6 +1382,14 @@ static int net_check_prereq(enum proto_t protocol)
}
goto common;
 #endif
+#if defined(CONFIG_CMD_PING6)
+   case PING6:
+   if (ip6_is_unspecified_addr(&net_ping_ip6)) {
+   puts("*** ERROR: ping address not given\n");
+   return 1;
+   }
+   goto common;
+#endif

[PATCH 14/17] test: dm: eth: Add ip6_addr_in_subnet test

2022-08-30 Thread Viacheslav Mitrofanov
Add a test if two address are in the same subnet. Use in sandbox

Signed-off-by: Viacheslav Mitrofanov 
---
 test/dm/eth.c | 25 +
 1 file changed, 25 insertions(+)

diff --git a/test/dm/eth.c b/test/dm/eth.c
index bbfa14ef6d..d4a39e53d9 100644
--- a/test/dm/eth.c
+++ b/test/dm/eth.c
@@ -99,6 +99,31 @@ static int dm_test_csum_ipv6_magic(struct unit_test_state 
*uts)
 }
 DM_TEST(dm_test_csum_ipv6_magic, 0);
 
+static int dm_test_ip6_addr_in_subnet(struct unit_test_state *uts)
+{
+   struct in6_addr our = {.s6_addr32[0] = 0x80fe,
+.s6_addr32[1] = 0x,
+.s6_addr32[2] = 0xffe9f242,
+.s6_addr32[3] = 0xe8f66dfe};
+   struct in6_addr neigh1 = {.s6_addr32[0] = 0x80fe,
+.s6_addr32[1] = 0x,
+.s6_addr32[2] = 0xffd5b372,
+.s6_addr32[3] = 0x3ef692fe};
+   struct in6_addr neigh2 = {.s6_addr32[0] = 0x60480120,
+.s6_addr32[1] = 0x6048,
+.s6_addr32[2] = 0x,
+.s6_addr32[3] = 0x};
+
+   /* in */
+   ut_assert(ip6_addr_in_subnet(&our, &neigh1, 64));
+   /* outside */
+   ut_assert(!ip6_addr_in_subnet(&our, &neigh2, 64));
+   ut_assert(!ip6_addr_in_subnet(&our, &neigh1, 128));
+
+   return 0;
+}
+DM_TEST(dm_test_ip6_addr_in_subnet, 0);
+
 static int dm_test_eth(struct unit_test_state *uts)
 {
net_ping_ip = string_to_ip("1.1.2.2");
-- 
2.25.1



[PATCH 13/17] test: dm: eth: Add csum_ipv6_magic test

2022-08-30 Thread Viacheslav Mitrofanov
Test checksum computation. csum_ipv6_magic() uses in upper layer
protocols as TCP/UDP/ICMPv6/etc to calculate payload checksum.

Signed-off-by: Viacheslav Mitrofanov 
---
 test/dm/eth.c | 29 +
 1 file changed, 29 insertions(+)

diff --git a/test/dm/eth.c b/test/dm/eth.c
index 4cc90cf514..bbfa14ef6d 100644
--- a/test/dm/eth.c
+++ b/test/dm/eth.c
@@ -70,6 +70,35 @@ static int dm_test_string_to_ip6(struct unit_test_state *uts)
 }
 DM_TEST(dm_test_string_to_ip6, 0);
 
+static int dm_test_csum_ipv6_magic(struct unit_test_state *uts)
+{
+   unsigned short csum = 0xBEEF;
+   /* Predefined correct parameters */
+   unsigned short correct_csum = 0xD8AC;
+   struct in6_addr saddr = {.s6_addr32[0] = 0x80fe,
+.s6_addr32[1] = 0x,
+.s6_addr32[2] = 0xffe9f242,
+.s6_addr32[3] = 0xe8f66dfe};
+   struct in6_addr daddr = {.s6_addr32[0] = 0x80fe,
+.s6_addr32[1] = 0x,
+.s6_addr32[2] = 0xffd5b372,
+.s6_addr32[3] = 0x3ef692fe};
+   u16 len = 1460;
+   unsigned short proto = 17;
+   unsigned int head_csum = 0x91f0;
+
+   csum = csum_ipv6_magic(&saddr, &daddr, len, proto, head_csum);
+   ut_assert(csum == correct_csum);
+
+   /* Broke a parameter */
+   proto--;
+   csum = csum_ipv6_magic(&saddr, &daddr, len, proto, head_csum);
+   ut_assert(csum != correct_csum);
+
+   return 0;
+}
+DM_TEST(dm_test_csum_ipv6_magic, 0);
+
 static int dm_test_eth(struct unit_test_state *uts)
 {
net_ping_ip = string_to_ip("1.1.2.2");
-- 
2.25.1



[PATCH 16/17] test: dm: eth: Add ip6_make_lladdr test

2022-08-30 Thread Viacheslav Mitrofanov
Add a test that checks generated Link Local Address. Use in sandbox

Signed-off-by: Viacheslav Mitrofanov 
---
 test/dm/eth.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/test/dm/eth.c b/test/dm/eth.c
index c892c68523..f84714b735 100644
--- a/test/dm/eth.c
+++ b/test/dm/eth.c
@@ -144,6 +144,24 @@ static int dm_test_ip6_make_snma(struct unit_test_state 
*uts)
 }
 DM_TEST(dm_test_ip6_make_snma, 0);
 
+static int dm_test_ip6_make_lladdr(struct unit_test_state *uts)
+{
+   struct in6_addr generated_lladdr = {0};
+   struct in6_addr correct_lladdr = {
+.s6_addr32[0] = 0x80fe,
+.s6_addr32[1] = 0x,
+.s6_addr32[2] = 0xffabf33a,
+.s6_addr32[3] = 0xfbb352fe};
+   const unsigned char mac[6] = {0x38, 0xf3, 0xab, 0x52, 0xb3, 0xfb};
+
+   ip6_make_lladdr(&generated_lladdr, mac);
+   ut_asserteq_mem(&generated_lladdr, &correct_lladdr,
+   sizeof(struct in6_addr));
+
+   return 0;
+}
+DM_TEST(dm_test_ip6_make_lladdr, UT_TESTF_SCAN_FDT);
+
 static int dm_test_eth(struct unit_test_state *uts)
 {
net_ping_ip = string_to_ip("1.1.2.2");
-- 
2.25.1



[PATCH 15/17] test: dm: eth: Add ip6_make_snma test

2022-08-30 Thread Viacheslav Mitrofanov
Add a test that checks generated Solicited Node Multicast Address from our
ipv6 address. Use in sandbox

Signed-off-by: Viacheslav Mitrofanov 
---
 test/dm/eth.c | 20 
 1 file changed, 20 insertions(+)

diff --git a/test/dm/eth.c b/test/dm/eth.c
index d4a39e53d9..c892c68523 100644
--- a/test/dm/eth.c
+++ b/test/dm/eth.c
@@ -124,6 +124,26 @@ static int dm_test_ip6_addr_in_subnet(struct 
unit_test_state *uts)
 }
 DM_TEST(dm_test_ip6_addr_in_subnet, 0);
 
+static int dm_test_ip6_make_snma(struct unit_test_state *uts)
+{
+   struct in6_addr mult = {0};
+   struct in6_addr correct_addr = {
+.s6_addr32[0] = 0x02ff,
+.s6_addr32[1] = 0x,
+.s6_addr32[2] = 0x0100,
+.s6_addr32[3] = 0xe8f66dff};
+   struct in6_addr addr = { .s6_addr32[0] = 0x80fe,
+.s6_addr32[1] = 0x,
+.s6_addr32[2] = 0xffe9f242,
+.s6_addr32[3] = 0xe8f66dfe};
+
+   ip6_make_snma(&mult, &addr);
+   ut_asserteq_mem(&mult, &correct_addr, sizeof(struct in6_addr));
+
+   return 0;
+}
+DM_TEST(dm_test_ip6_make_snma, 0);
+
 static int dm_test_eth(struct unit_test_state *uts)
 {
net_ping_ip = string_to_ip("1.1.2.2");
-- 
2.25.1



[PATCH 17/17] test/py: add a ping6 test

2022-08-30 Thread Viacheslav Mitrofanov
Add a simple ICMPv6 echo request test

Signed-off-by: Viacheslav Mitrofanov 
---
 test/py/tests/test_net.py | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/test/py/tests/test_net.py b/test/py/tests/test_net.py
index 9ca6743afd..bd3d60774b 100644
--- a/test/py/tests/test_net.py
+++ b/test/py/tests/test_net.py
@@ -126,6 +126,21 @@ def test_net_ping(u_boot_console):
 output = u_boot_console.run_command('ping $serverip')
 assert 'is alive' in output
 
+@pytest.mark.buildconfigspec('cmd_ping6')
+def test_net_ping6(u_boot_console):
+"""Test the ping6 command.
+
+The $serverip6 (as set up by test_net_setup_static) is pinged.
+The test validates that the host is alive, as reported by the
+ping6 command's output.
+"""
+
+if not net_set_up:
+pytest.skip('Network not initialized')
+
+output = u_boot_console.run_command('ping6 $serverip6')
+assert 'is alive' in output
+
 @pytest.mark.buildconfigspec('cmd_net')
 def test_net_tftpboot(u_boot_console):
 """Test the tftpboot command.
-- 
2.25.1



[PATCH] doc: Add gpio status output fields description

2022-08-30 Thread Patrice Chotard
Add gpio status output fields description and one output example.

Signed-off-by: Patrice Chotard 
---

 doc/usage/cmd/gpio.rst | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/doc/usage/cmd/gpio.rst b/doc/usage/cmd/gpio.rst
index f6a5668388..b768b08a9b 100644
--- a/doc/usage/cmd/gpio.rst
+++ b/doc/usage/cmd/gpio.rst
@@ -45,6 +45,17 @@ gpio status
 
 Display the status of one or multiple GPIOs. By default only claimed GPIOs
 are displayed.
+gpio status command output fields are : [name]: [function]: [value] [x] [label]
+
+[function] can be:
+  - "output" : pin configured in gpio output.
+  - "input"  : pin configured in gpio input.
+  - "func"   : pin configured in alternate function, followed by [label]
+ which shows pinmuxing label.
+  - "unused" : pin not configured.
+
+For [function] "output" or "input", [value] field indicates pin's level,
+[x] or [ ] indicates respectively if gpio is used or not, [label] shows gpio 
label.
 
 -a
 Display GPIOs irrespective of being claimed.
@@ -77,6 +88,21 @@ Switch the status of a GPIO::
 => echo $myvar
 0
 
+=> gpio status
+Bank GPIOA:
+GPIOA1: func rgmii-0
+GPIOA2: func rgmii-0
+GPIOA7: func rgmii-0
+GPIOA10: output: 0 [x] hdmi-transmitter@39.reset-gpios
+GPIOA13: output: 1 [x] red.gpios
+
+Bank GPIOB:
+GPIOB0: func rgmii-0
+GPIOB1: func rgmii-0
+GPIOB2: func uart4-0
+GPIOB7: input: 0 [x] m...@58005000.cd-gpios
+GPIOB11: func rgmii-0
+
 Configuration
 -
 
-- 
2.25.1



Re: [PATCH 4/4] arm: dts: chameleonv3: Add 270-2 variant

2022-08-30 Thread Simon Glass
Hi Paweł,

On Tue, 30 Aug 2022 at 05:51, Paweł Anikiel  wrote:
>
> On Tue, Aug 30, 2022 at 5:13 AM Alexandru M Stan  wrote:
> >
> > Hey Simon,
> >
> > On Mon, Aug 29, 2022 at 7:29 PM Simon Glass  wrote:
> > >
> > > Hi Paweł,
> > >
> > > On Mon, 29 Aug 2022 at 02:23, Paweł Anikiel  wrote:
> > > >
> > > > On Sat, Aug 27, 2022 at 2:22 AM Simon Glass  wrote:
> > > > >
> > > > > Hi Paweł,
> > > > >
> > > > > On Fri, 26 Aug 2022 at 01:54, Paweł Anikiel  wrote:
> > > > > >
> > > > > > Add devicetree for chameleonv3 with the 270-2I2-D11E variant of the
> > > > > > Mercury+ AA1 module
> > > > > >
> > > > > > Signed-off-by: Paweł Anikiel 
> > > > > > ---
> > > > > >  arch/arm/dts/Makefile|  1 +
> > > > > >  .../socfpga_arria10_chameleonv3_270_2-u-boot.dtsi| 12 
> > > > > > 
> > > > > >  arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts   |  5 +
> > > > > >  3 files changed, 18 insertions(+)
> > > > > >  create mode 100644 
> > > > > > arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
> > > > > >  create mode 100644 
> > > > > > arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
> > > > > >
> > >
> > >
> > > > > > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> > > > > > index 7330121dba..36d5d65595 100644
> > > > > > --- a/arch/arm/dts/Makefile
> > > > > > +++ b/arch/arm/dts/Makefile
> > > > > > @@ -425,6 +425,7 @@ dtb-$(CONFIG_ARCH_SOCFPGA) +=   
> > > > > > \
> > > > > > socfpga_agilex_socdk.dtb\
> > > > > > socfpga_arria5_secu1.dtb\
> > > > > > socfpga_arria5_socdk.dtb\
> > > > > > +   socfpga_arria10_chameleonv3_270_2.dtb   \
> > > > > > socfpga_arria10_chameleonv3_270_3.dtb   \
> > > > > > socfpga_arria10_chameleonv3_480_2.dtb   \
> > > > > > socfpga_arria10_socdk_sdmmc.dtb \
> > > > > > diff --git 
> > > > > > a/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi 
> > > > > > b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
> > > > > > new file mode 100644
> > > > > > index 00..05b4485cf3
> > > > > > --- /dev/null
> > > > > > +++ b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2-u-boot.dtsi
> > > > > > @@ -0,0 +1,12 @@
> > > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > > +/*
> > > > > > + * Copyright 2022 Google LLC
> > > > > > + */
> > > > > > +#include "socfpga_arria10_chameleonv3_480_2_handoff.h"
> > > > > > +#include "socfpga_arria10-handoff.dtsi"
> > > > > > +#include "socfpga_arria10_handoff_u-boot.dtsi"
> > > > > > +#include "socfpga_arria10_mercury_aa1-u-boot.dtsi"
> > > > > > +
> > > > > > +&fpga_mgr {
> > > > > > +   altr,bitstream = "fpga-270-2.itb";
> > > > > > +};
> > > > > > diff --git a/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts 
> > > > > > b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
> > > > > > new file mode 100644
> > > > > > index 00..5f40af6eb9
> > > > > > --- /dev/null
> > > > > > +++ b/arch/arm/dts/socfpga_arria10_chameleonv3_270_2.dts
> > > > > > @@ -0,0 +1,5 @@
> > > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > > +/*
> > > > > > + * Copyright 2022 Google LLC
> > > > > > + */
> > > > > > +#include "socfpga_arria10_chameleonv3.dts"
> > > > >
> > > > > Can you create a common .dtsi file instead? We should not be including
> > > > > a .dts file into another file.
> > > > >
> > > > Do you mean renaming chameleonv3.dts to .dtsi? In Linux it's a .dts,
> > > > because nothing includes it (no handoff headers are needed). Is it
> > > > fine to have the names differ across U-Boot and Linux?
> > >
> > > Ideally not, but we should not include a .dts file in another one and
> > > it is probably more important to follow that rule. But why is Linux
> > > not getting this variant?
> > >
> > > Regards,
> > > Simon
> >
> > Linux (at least for the near future) does not care about which variant
> > it is. The big differences between 270, 480, -2, -3 are mostly about
> > the number of FPGA logic gates and speed grades. Such things affect
> > the FPGA bitstream greatly, and might even affect clock presets that
> > u-boot cares about, but by the time linux loads it doesn't matter
> > anymore.
>
> Perhaps a more detailed explanation:
>
> The Main and Peripheral PLLs (as well as some other clocks) are
> configured by U-Boot. On the other hand, Linux expects them to be
> configured when it boots, and does not touch them.
>
> The clock configuration depends mainly on the speed grade of the Arria
> 10 SoC (marked by us as -2 and -3), but also on the fpga hardware
> design (e.g. user-defined clocks for the fpga), and is included in the
> u-boot devicetree:
> > +#include "socfpga_arria10_chameleonv3_480_2_handoff.h"
> > +#include "socfpga_arria10-handoff.dtsi"
> > +#include "socfpga_arria10_handoff_u-boot.dtsi"
>
> Linux, on the other hand, doesn't need such information, and there is
>

Re: [PATCH v3] dm: core: Do not stop uclass iteration on error

2022-08-30 Thread Simon Glass
Hi Michal,

On Tue, 30 Aug 2022 at 04:23, Michal Suchánek  wrote:
>
> On Sat, Aug 27, 2022 at 07:52:27PM -0600, Simon Glass wrote:
> > Hi Michal,
> >
> > On Fri, 19 Aug 2022 at 14:23, Michal Suchanek  wrote:
> > >
> > > When probing a device fails NULL pointer is returned, and other devices
> > > cannot be iterated. Skip to next device on error instead.
> > >
> > > Fixes: 6494d708bf ("dm: Add base driver model support")
> >
> > I think you should drop this as you are doing a change of behaviour,
> > not fixing a bug!
>
> You can hardly fix a bug without a change in behavior.
>
> These functions are used for iterating devices, and are not iterating
> devices. That's clearly a bug.

If it were clear I would have changed this long ago. The new way you
have this function ignores errors, so they cannot be reported.

We should almost always report errors, which is why I think your
methods should be named differently.

>
> > > Signed-off-by: Michal Suchanek 
> > > ---
> > > v2: - Fix up tests
> > > v3: - Fix up API doc
> > > - Correctly forward error from uclass_get
> > > - Do not return an error when last device fails to probe
> > > - Drop redundant initialization
> > > - Wrap at 80 columns
> > > ---
> > >  drivers/core/uclass.c | 32 
> > >  include/dm/uclass.h   | 13 -
> > >  test/dm/test-fdt.c| 20 
> > >  3 files changed, 48 insertions(+), 17 deletions(-)
> >
> > Unfortunately this still fails one test. Try 'make qcheck' to see it -
> > it is ethernet.
>
> I will look at that.
>
> > I actually think you should create new functions for this feature,
> > e.g.uclass_first_device_ok(), since it makes it impossible to see what
> > when wrong with a device in the middle.
> >
> > I have long had all this in my mind. One idea for a future change is
> > to return the error, but set dev, so that the caller knows there is a
> > device, which failed. When we are at the end, dev is set to NULL.
>
> We already have uclass_first_device_check() and
> uclass_next_device_check() to iterate all devices, including broken
> ones, and getting the errors as well.
>
> That's for the case you want all the details, and these are for the case
> you just want to get devices and don't care about the details.
>
> That's AFAICT as much as this iteration interface can provide, and we
> have both cases covered.

I see three cases:
- want to see the next device, returning the error if it cannot be
probed - uclass_first_device()
- want to get the next device that can successfully probe - your new functions
- want to see each device, along with any errors - uclass_first_device_check()

Regards,
Simon


Re: [PATCH v5 1/8] binman: add support for skipping file concatenation for mkimage

2022-08-30 Thread Simon Glass
Hi Quentin,

On Tue, 30 Aug 2022 at 03:57, Quentin Schulz
 wrote:
>
> Hi Simon,
>
> On 8/27/22 02:21, Simon Glass wrote:
> > Hi Quentin,
> >
> > On Fri, 26 Aug 2022 at 09:37, Quentin Schulz  wrote:
> >>
> >> From: Quentin Schulz 
> >>
> >> Some image types handled by mkimage require the datafiles to be passed
> >> independently (-d data1:data2) for specific handling of each. A
> >> concatenation of datafiles prior to passing them to mkimage wouldn't
> >> work.
> >>
> >> That is the case for rkspi for example which requires page alignment
> >> and only writing 2KB every 4KB.
> >>
> >> This adds the ability to tell binman to pass the datafiles without
> >> prior concatenation to mkimage, by adding the multiple-data-files
> >> boolean property to the mkimage node.
> >>
> >> Cc: Quentin Schulz 
> >> Reviewed-by: Simon Glass 
> >> Signed-off-by: Quentin Schulz 
> >> ---
> >>
> >> v5:
> >>   - changed to use full path from input dir with tools.get_input_filename
> >>   to make it possible to run the unit tests,
> >>   - added unit test,
> >>
> >>
> >>   tools/binman/entries.rst  | 22 ++
> >>   tools/binman/etype/mkimage.py | 41 +--
> >>   tools/binman/ftest.py | 16 
> >
> > Please put the new test at the end.
> >
> >>   .../test/241_mkimage_multiple_data_files.dts  | 21 ++
> >>   4 files changed, 96 insertions(+), 4 deletions(-)
> >>   create mode 100644 tools/binman/test/241_mkimage_multiple_data_files.dts
> >
> > This is pretty close but it still missing a line of test coverage.
> > Please try 'binman test -T' to see it. I'd also prefer a shorter
>
> This does not work on Fedora.
> 1) there's no python3-coverage binary available,
> 2) After replacing python3-coverage with just coverage, the tests are
> stuck and never finish, (I have seen the patches to use COVERAGE
> environment variable so I guess the required changes might be tackled
> soon in master),
>
> Any tip on how to identify which test is stuck except going through them
> one by one?

One way is to add comment blocks '''...''' across the ftest.py file,
using a binary chop to identify the problem.

Or, since tests are run in series, you could hack test_util to pass
verbose parameters when it runs the tests - see 'cmd =' in
run_test_coverage().

>
> python3-coverage is also not available in the container image built from
> tools/docker/Dockerfile.

does 'python3 -m coverage' work?

or this:

https://coverage.readthedocs.io/en/6.3.2/install.html

>
> > filename for the 241 file.
> >
> > I've pushed a tree containing a suggested fix (updating this patch). I
> > can update it when applying if you like, otherwise please send a new
> > version.
> >
>
> Where did you push the tree?

Sorry I forgot to mention that:

https://github.com/sjg20/u-boot/tree/try-rk4

Regards,
SImon


Re: [PATCH] doc: Add gpio status output fields description

2022-08-30 Thread Simon Glass
On Tue, 30 Aug 2022 at 08:56, Patrice Chotard
 wrote:
>
> Add gpio status output fields description and one output example.
>
> Signed-off-by: Patrice Chotard 
> ---
>
>  doc/usage/cmd/gpio.rst | 26 ++
>  1 file changed, 26 insertions(+)

Reviewed-by: Simon Glass 


Re: [PATCH 2/6] timer: orion-timer: Add timer_get_boot_us() for BOOTSTAGE support

2022-08-30 Thread Simon Glass
Hi Stefan,

On Tue, 30 Aug 2022 at 06:08, Stefan Roese  wrote:
>
> Adding Simon to Cc...
>
> On 30.08.22 14:00, Michael Walle wrote:
> > Am 2022-08-30 13:53, schrieb Stefan Roese:
> >> Add timer_get_boot_us() to support boards, that have CONFIG_BOOTSTAGE
> >> enabled, like pogo_v4.
> >>
> >> Signed-off-by: Stefan Roese 
> >> ---
> >>  drivers/timer/orion-timer.c | 22 ++
> >>  1 file changed, 22 insertions(+)
> >>
> >> diff --git a/drivers/timer/orion-timer.c b/drivers/timer/orion-timer.c
> >> index 02ed138642b8..7e920eaeaa40 100644
> >> --- a/drivers/timer/orion-timer.c
> >> +++ b/drivers/timer/orion-timer.c
> >> @@ -41,6 +41,28 @@ u64 notrace timer_early_get_count(void)
> >>  }
> >>  #endif
> >>
> >> +#if CONFIG_IS_ENABLED(BOOTSTAGE)
> >> +ulong timer_get_boot_us(void)
> >> +{
> >> +u64 ticks = 0;
> >> +u32 rate = 1;
> >> +u64 us;
> >> +int ret;
> >> +
> >> +ret = dm_timer_init();
> >> +if (!ret) {
> >> +/* The timer is available */
> >> +rate = timer_get_rate(gd->timer);
> >> +timer_get_count(gd->timer, &ticks);
> >> +} else {
> >> +return 0;
> >> +}
> >> +
> >> +us = (ticks * 1000) / rate;
> >> +return us;
> >> +}
> >> +#endif
> >
> > This is duplicate code in almost all the timer drivers, shouldn't
> > this be a (weak) default implementation in timer-uclass.c?
>
> Yes. I was lazy and just copied this function and did not notice, that
> even more timer drivers have this function. Of course it makes sense
> to not duplicate code here, but have a common function for this. Frankly
> I don't even know why exactly this function is needed, as I did not
> look into BOOTSTAGE yet.
>
> Simon, do we really need this function? Can't bootstage just use the
> "normal" timer functionality instead?

It is needed because bootstage is called before driver model is ready.
In fact it can be used to time driver model things.

The function here isn't that useful, since we cannot call
dm_timer_init() before driver model is set up.

The timer_get_boot_us() function can by in a timer driver, but must
exist outside the driver infrastructure. It must init the hard and
then return the correct time. One driver model probes the timer
driver, it must then continue on in that way.

Actually it looks like all the timers do this wrong. See
arch/arm/mach-imx/syscounter.c or arch/arm/cpu/armv8/generic_timer.c
for some ideas.

>
> > Also,
> > do you need to guard it with CONFIG_IS_ENABLED(BOOTSTAGE), aren't
> > unused functions discarded anyway?
>
> Yes, this should be the case. Also just a result of my lazy copy-and-
> past.
>

Regards,
SImon


Re: [PATCH v4 1/2] bootm: fix typo imape_comp -> image_comp

2022-08-30 Thread Simon Glass
Hi Daniel,

On Mon, 29 Aug 2022 at 21:04, Daniel Golle  wrote:
>
> On Mon, Aug 29, 2022 at 08:30:14PM -0600, Simon Glass wrote:
> > On Fri, 26 Aug 2022 at 21:15, Daniel Golle  wrote:
> > >
> > > Chage variable name 'imape_comp' to the supposedly intended name
> >
> > Change
>
> Can you fix that on-the-fly while comitting or should I repost with
> that typo in the patch description fixed?

Yes I suspect Tom can do that.

Regards,
Simon


>
> >
> > > 'image_comp'.
> > >
> > > Signed-off-by: Daniel Golle 
> > > ---
> > > v4: add missing name replacement
> > >
> > > boot/bootm.c | 8 
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > >
> >
> > Reviewed-by: Simon Glass 


Re: [PATCH] ARM: imx: Update Data Modul i.MX8M Mini eDM SBC DRAM timing

2022-08-30 Thread Tim Harvey
On Tue, Aug 30, 2022 at 5:34 AM Marek Vasut  wrote:
>
> Adjust the DRAM timing settings for this board per ones provided
> by hardware department. The change is applied to the LPDDR4 MR11
> register CA ODT configuration, from RZQ/6 to RZQ/3, which fixes
> stability issues on subset of boards. The DDR PHY PIE block has
> been updated accordingly.
>
> Signed-off-by: Marek Vasut 
> Cc: Fabio Estevam 
> Cc: Peng Fan 
> Cc: Stefano Babic 
> ---
>  .../imx8mm_edm_sbc/lpddr4_timing_2G_32.c  | 45 ++-
>  .../imx8mm_edm_sbc/lpddr4_timing_4G_32.c  | 40 +
>  2 files changed, 45 insertions(+), 40 deletions(-)
>
> diff --git a/board/data_modul/imx8mm_edm_sbc/lpddr4_timing_2G_32.c 
> b/board/data_modul/imx8mm_edm_sbc/lpddr4_timing_2G_32.c
> index c2abcb54895..f40fd48fc49 100644
> --- a/board/data_modul/imx8mm_edm_sbc/lpddr4_timing_2G_32.c
> +++ b/board/data_modul/imx8mm_edm_sbc/lpddr4_timing_2G_32.c
> @@ -22,7 +22,7 @@ static struct dram_cfg_param ddr_ddrc_cfg[] = {
> { 0x3d4000d4, 0x94 },
> { 0x3d4000dc, 0xd4002d },
> { 0x3d4000e0, 0x31 },
> -   { 0x3d4000e8, 0x66004d },
> +   { 0x3d4000e8, 0x36004d },
> { 0x3d4000ec, 0x16004d },
> { 0x3d400100, 0x191e1920 },
> { 0x3d400104, 0x60630 },
> @@ -55,6 +55,7 @@ static struct dram_cfg_param ddr_ddrc_cfg[] = {
> { 0x3d400204, 0x80808 },
> { 0x3d400214, 0x7070707 },
> { 0x3d400218, 0x7070707 },
> +   { 0x3d40021c, 0xf0f },
> { 0x3d400250, 0x29001701 },
> { 0x3d400254, 0x2c },
> { 0x3d40025c, 0x430 },
> @@ -72,7 +73,7 @@ static struct dram_cfg_param ddr_ddrc_cfg[] = {
> { 0x3d402064, 0xc001c },
> { 0x3d4020dc, 0x84 },
> { 0x3d4020e0, 0x31 },
> -   { 0x3d4020e8, 0x66004d },
> +   { 0x3d4020e8, 0x36004d },
> { 0x3d4020ec, 0x16004d },
> { 0x3d402100, 0xa040305 },
> { 0x3d402104, 0x30407 },
> @@ -97,7 +98,7 @@ static struct dram_cfg_param ddr_ddrc_cfg[] = {
> { 0x3d403064, 0x30007 },
> { 0x3d4030dc, 0x84 },
> { 0x3d4030e0, 0x31 },
> -   { 0x3d4030e8, 0x66004d },
> +   { 0x3d4030e8, 0x36004d },
> { 0x3d4030ec, 0x16004d },
> { 0x3d403100, 0xa010102 },
> { 0x3d403104, 0x30404 },
> @@ -1059,25 +1060,25 @@ static struct dram_cfg_param ddr_fsp0_cfg[] = {
> { 0x54012, 0x110 },
> { 0x54019, 0x2dd4 },
> { 0x5401a, 0x31 },
> -   { 0x5401b, 0x4d66 },
> +   { 0x5401b, 0x4d36 },
> { 0x5401c, 0x4d00 },
> { 0x5401e, 0x16 },
> { 0x5401f, 0x2dd4 },
> { 0x54020, 0x31 },
> -   { 0x54021, 0x4d66 },
> +   { 0x54021, 0x4d36 },
> { 0x54022, 0x4d00 },
> { 0x54024, 0x16 },
> { 0x5402b, 0x1000 },
> { 0x5402c, 0x1 },
> { 0x54032, 0xd400 },
> { 0x54033, 0x312d },
> -   { 0x54034, 0x6600 },
> +   { 0x54034, 0x3600 },
> { 0x54035, 0x4d },
> { 0x54036, 0x4d },
> { 0x54037, 0x1600 },
> { 0x54038, 0xd400 },
> { 0x54039, 0x312d },
> -   { 0x5403a, 0x6600 },
> +   { 0x5403a, 0x3600 },
> { 0x5403b, 0x4d },
> { 0x5403c, 0x4d },
> { 0x5403d, 0x1600 },
> @@ -1098,25 +1099,25 @@ static struct dram_cfg_param ddr_fsp1_cfg[] = {
> { 0x54012, 0x110 },
> { 0x54019, 0x84 },
> { 0x5401a, 0x31 },
> -   { 0x5401b, 0x4d66 },
> +   { 0x5401b, 0x4d36 },
> { 0x5401c, 0x4d00 },
> { 0x5401e, 0x16 },
> { 0x5401f, 0x84 },
> { 0x54020, 0x31 },
> -   { 0x54021, 0x4d66 },
> +   { 0x54021, 0x4d36 },
> { 0x54022, 0x4d00 },
> { 0x54024, 0x16 },
> { 0x5402b, 0x1000 },
> { 0x5402c, 0x1 },
> { 0x54032, 0x8400 },
> { 0x54033, 0x3100 },
> -   { 0x54034, 0x6600 },
> +   { 0x54034, 0x3600 },
> { 0x54035, 0x4d },
> { 0x54036, 0x4d },
> { 0x54037, 0x1600 },
> { 0x54038, 0x8400 },
> { 0x54039, 0x3100 },
> -   { 0x5403a, 0x6600 },
> +   { 0x5403a, 0x3600 },
> { 0x5403b, 0x4d },
> { 0x5403c, 0x4d },
> { 0x5403d, 0x1600 },
> @@ -1137,25 +1138,25 @@ static struct dram_cfg_param ddr_fsp2_cfg[] = {
> { 0x54012, 0x110 },
> { 0x54019, 0x84 },
> { 0x5401a, 0x31 },
> -   { 0x5401b, 0x4d66 },
> +   { 0x5401b, 0x4d36 },
> { 0x5401c, 0x4d00 },
> { 0x5401e, 0x16 },
> { 0x5401f, 0x84 },
> { 0x54020, 0x31 },
> -   { 0x54021, 0x4d66 },
> +   { 0x54021, 0x4d36 },
> { 0x54022, 0x4d00 },
> { 0x54024, 0x16 },
> { 0x5402b, 0x1000 },
> { 0x5402c, 0x1 },
> { 0x54032, 0x8400 },
> { 0x54033, 0x3100 },
> -   { 0x54034, 0x6600 },
> +   { 0x54034, 0x3600 },
> { 0x54035, 0x4d },
> { 0x54036, 0x4d },
> { 0x54037, 0x1600 },
> { 0x54038, 0x8400 },
>   

Re: [PATCH 1/2] dt-bindings: clock: sifive: sync FU740 PRCI clock binding header

2022-08-30 Thread Icenowy Zheng
在 2022-08-30星期二的 03:26 +,Leo Liang写道:
> Hi Icenowy,
> On Thu, Aug 25, 2022 at 04:11:18PM +0800, Icenowy Zheng wrote:
> > This commit sychornizes the header file for FU740 PRCI clocks with
> > the
> > one from Linux 5.19.
> > 
> > The constant values are the same, but all constant names are
> > changed
> > (most are just prefixed with FU740_).
> > 
> > Signed-off-by: Icenowy Zheng 
> > ---
> >  arch/riscv/dts/fu740-c000-u-boot.dtsi | 16 +--
> >  arch/riscv/dts/fu740-c000.dtsi    | 28 +--
> > 
> >  drivers/clk/sifive/fu740-prci.c   | 18 ++--
> >  drivers/clk/sifive/sifive-prci.c  |  4 +--
> >  include/dt-bindings/clock/sifive-fu740-prci.h | 25 ---
> > --
> >  5 files changed, 45 insertions(+), 46 deletions(-)
> > 
> > diff --git a/arch/riscv/dts/fu740-c000-u-boot.dtsi
> > b/arch/riscv/dts/fu740-c000-u-boot.dtsi
> > index a6f7a0873e..917e9bf163 100644
> > --- a/arch/riscv/dts/fu740-c000-u-boot.dtsi
> > +++ b/arch/riscv/dts/fu740-c000-u-boot.dtsi
> > @@ -7,11 +7,11 @@
> >  
> >  / {
> > cpus {
> > -   assigned-clocks = <&prci PRCI_CLK_COREPLL>;
> > +   assigned-clocks = <&prci FU740_PRCI_CLK_COREPLL>;
> > assigned-clock-rates = <12>;
> > u-boot,dm-spl;
> > cpu0: cpu@0 {
> > -   clocks = <&prci PRCI_CLK_COREPLL>;
> > +   clocks = <&prci FU740_PRCI_CLK_COREPLL>;
> > u-boot,dm-spl;
> > status = "okay";
> > cpu0_intc: interrupt-controller {
> > @@ -19,28 +19,28 @@
> > };
> > };
> > cpu1: cpu@1 {
> > -   clocks = <&prci PRCI_CLK_COREPLL>;
> > +   clocks = <&prci FU740_PRCI_CLK_COREPLL>;
> > u-boot,dm-spl;
> > cpu1_intc: interrupt-controller {
> > u-boot,dm-spl;
> > };
> > };
> > cpu2: cpu@2 {
> > -   clocks = <&prci PRCI_CLK_COREPLL>;
> > +   clocks = <&prci FU740_PRCI_CLK_COREPLL>;
> > u-boot,dm-spl;
> > cpu2_intc: interrupt-controller {
> >  u-boot,dm-spl;
> > };
> > };
> > cpu3: cpu@3 {
> > -   clocks = <&prci PRCI_CLK_COREPLL>;
> > +   clocks = <&prci FU740_PRCI_CLK_COREPLL>;
> > u-boot,dm-spl;
> > cpu3_intc: interrupt-controller {
> > u-boot,dm-spl;
> > };
> > };
> > cpu4: cpu@4 {
> > -   clocks = <&prci PRCI_CLK_COREPLL>;
> > +   clocks = <&prci FU740_PRCI_CLK_COREPLL>;
> > u-boot,dm-spl;
> > cpu4_intc: interrupt-controller {
> > u-boot,dm-spl;
> > @@ -76,7 +76,7 @@
> > reg = <0x0 0x100b 0x0 0x0800
> >    0x0 0x100b2000 0x0 0x2000
> >    0x0 0x100b8000 0x0 0x1000>;
> > -   clocks = <&prci PRCI_CLK_DDRPLL>;
> > +   clocks = <&prci FU740_PRCI_CLK_DDRPLL>;
> > clock-frequency = <93324>;
> > u-boot,dm-spl;
> > };
> > @@ -100,7 +100,7 @@
> >  };
> >  
> >  ð0 {
> > -   assigned-clocks = <&prci PRCI_CLK_GEMGXLPLL>;
> > +   assigned-clocks = <&prci FU740_PRCI_CLK_GEMGXLPLL>;
> > assigned-clock-rates = <125125000>;
> >  };
> >  
> > diff --git a/arch/riscv/dts/fu740-c000.dtsi b/arch/riscv/dts/fu740-
> > c000.dtsi
> > index 649efe400a..0e14aa71e7 100644
> > --- a/arch/riscv/dts/fu740-c000.dtsi
> > +++ b/arch/riscv/dts/fu740-c000.dtsi
> > @@ -166,7 +166,7 @@
> > reg = <0x0 0x1001 0x0 0x1000>;
> > interrupt-parent = <&plic0>;
> > interrupts = <39>;
> > -   clocks = <&prci PRCI_CLK_PCLK>;
> > +   clocks = <&prci FU740_PRCI_CLK_PCLK>;
> > status = "disabled";
> > };
> > uart1: serial@10011000 {
> > @@ -174,7 +174,7 @@
> > reg = <0x0 0x10011000 0x0 0x1000>;
> > interrupt-parent = <&plic0>;
> > interrupts = <40>;
> > -   clocks = <&prci PRCI_CLK_PCLK>;
> > +   clocks = <&prci FU740_PRCI_CLK_PCLK>;
> > status = "disabled";
> > };
> > i2c0: i2c@1003 {
> > @@ -182,7 +182,7 @@
> > reg =

Re: [PATCH v3] dm: core: Do not stop uclass iteration on error

2022-08-30 Thread Michal Suchánek
On Tue, Aug 30, 2022 at 09:56:52AM -0600, Simon Glass wrote:
> Hi Michal,
> 
> On Tue, 30 Aug 2022 at 04:23, Michal Suchánek  wrote:
> >
> > On Sat, Aug 27, 2022 at 07:52:27PM -0600, Simon Glass wrote:
> > > Hi Michal,
> > >
> > > On Fri, 19 Aug 2022 at 14:23, Michal Suchanek  wrote:
> > > >
> > > > When probing a device fails NULL pointer is returned, and other devices
> > > > cannot be iterated. Skip to next device on error instead.
> > > >
> > > > Fixes: 6494d708bf ("dm: Add base driver model support")
> > >
> > > I think you should drop this as you are doing a change of behaviour,
> > > not fixing a bug!
> >
> > You can hardly fix a bug without a change in behavior.
> >
> > These functions are used for iterating devices, and are not iterating
> > devices. That's clearly a bug.
> 
> If it were clear I would have changed this long ago. The new way you
> have this function ignores errors, so they cannot be reported.
> 
> We should almost always report errors, which is why I think your
> methods should be named differently.
> 
> >
> > > > Signed-off-by: Michal Suchanek 
> > > > ---
> > > > v2: - Fix up tests
> > > > v3: - Fix up API doc
> > > > - Correctly forward error from uclass_get
> > > > - Do not return an error when last device fails to probe
> > > > - Drop redundant initialization
> > > > - Wrap at 80 columns
> > > > ---
> > > >  drivers/core/uclass.c | 32 
> > > >  include/dm/uclass.h   | 13 -
> > > >  test/dm/test-fdt.c| 20 
> > > >  3 files changed, 48 insertions(+), 17 deletions(-)
> > >
> > > Unfortunately this still fails one test. Try 'make qcheck' to see it -
> > > it is ethernet.
> >
> > I will look at that.
> >
> > > I actually think you should create new functions for this feature,
> > > e.g.uclass_first_device_ok(), since it makes it impossible to see what
> > > when wrong with a device in the middle.
> > >
> > > I have long had all this in my mind. One idea for a future change is
> > > to return the error, but set dev, so that the caller knows there is a
> > > device, which failed. When we are at the end, dev is set to NULL.
> >
> > We already have uclass_first_device_check() and
> > uclass_next_device_check() to iterate all devices, including broken
> > ones, and getting the errors as well.
> >
> > That's for the case you want all the details, and these are for the case
> > you just want to get devices and don't care about the details.
> >
> > That's AFAICT as much as this iteration interface can provide, and we
> > have both cases covered.
> 
> I see three cases:
> - want to see the next device, returning the error if it cannot be
> probed - uclass_first_device()

And the point of this is what exactly?

The device order in the uclass is not well defined - at any time a new
device which will become the first can be added, fail probe, and block
what was assumed a loop iterating the uclass from returning any devices
at all. That's exactly what happened with the new sysreset.

What is exactly the point of returning the error and not the pointer to
the next device?

The only point of these simplified iterators is that the caller can
check only one value (device pointer) and then not check the error
because they don't care. If they do cate uclass_first_device_check()
provides all the details available.

> - want to get the next device that can successfully probe - your new functions
> - want to see each device, along with any errors - uclass_first_device_check()

Thanks

Michal


Re: [PATCH v4 1/2] bootm: fix typo imape_comp -> image_comp

2022-08-30 Thread Tom Rini
On Tue, Aug 30, 2022 at 09:56:56AM -0600, Simon Glass wrote:
> Hi Daniel,
> 
> On Mon, 29 Aug 2022 at 21:04, Daniel Golle  wrote:
> >
> > On Mon, Aug 29, 2022 at 08:30:14PM -0600, Simon Glass wrote:
> > > On Fri, 26 Aug 2022 at 21:15, Daniel Golle  wrote:
> > > >
> > > > Chage variable name 'imape_comp' to the supposedly intended name
> > >
> > > Change
> >
> > Can you fix that on-the-fly while comitting or should I repost with
> > that typo in the patch description fixed?
> 
> Yes I suspect Tom can do that.

Yes, that's fine.

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH] ARM: imx: Update Data Modul i.MX8M Mini eDM SBC DRAM timing

2022-08-30 Thread Marek Vasut

On 8/30/22 18:01, Tim Harvey wrote:

Hi,


Marek,

Might I ask how you ran into the issue


There were a couple of boards which were unstable and kept crashing at 
runtime, often when using GPU. Either the machine locked up completely 
or there were rendering artifacts. Eventually also memtester triggered 
on errors during walking ones test.



and how you came up with the
new numbers?


The hardware experts ran simulation of the bus in I think hyperlynx and 
come up with new values for the RPA.



Were the new register settings obtained by simply using a
newver version of the NXP DDR tool or a new version of the RPA
Spreadsheet used with the tool or some other means?


Nope, the RPA spreadsheet version was the same, it was the simulation 
the hardware people ran which exposed the sub-optimal inputs into the RPA.


Re: [PATCH v5 1/8] binman: add support for skipping file concatenation for mkimage

2022-08-30 Thread Quentin Schulz

Hi Simon,

On 8/30/22 17:56, Simon Glass wrote:

Hi Quentin,

On Tue, 30 Aug 2022 at 03:57, Quentin Schulz
 wrote:


Hi Simon,

On 8/27/22 02:21, Simon Glass wrote:

Hi Quentin,

On Fri, 26 Aug 2022 at 09:37, Quentin Schulz  wrote:


From: Quentin Schulz 

Some image types handled by mkimage require the datafiles to be passed
independently (-d data1:data2) for specific handling of each. A
concatenation of datafiles prior to passing them to mkimage wouldn't
work.

That is the case for rkspi for example which requires page alignment
and only writing 2KB every 4KB.

This adds the ability to tell binman to pass the datafiles without
prior concatenation to mkimage, by adding the multiple-data-files
boolean property to the mkimage node.

Cc: Quentin Schulz 
Reviewed-by: Simon Glass 
Signed-off-by: Quentin Schulz 
---

v5:
   - changed to use full path from input dir with tools.get_input_filename
   to make it possible to run the unit tests,
   - added unit test,


   tools/binman/entries.rst  | 22 ++
   tools/binman/etype/mkimage.py | 41 +--
   tools/binman/ftest.py | 16 


Please put the new test at the end.


   .../test/241_mkimage_multiple_data_files.dts  | 21 ++
   4 files changed, 96 insertions(+), 4 deletions(-)
   create mode 100644 tools/binman/test/241_mkimage_multiple_data_files.dts


This is pretty close but it still missing a line of test coverage.
Please try 'binman test -T' to see it. I'd also prefer a shorter


This does not work on Fedora.
1) there's no python3-coverage binary available,
2) After replacing python3-coverage with just coverage, the tests are
stuck and never finish, (I have seen the patches to use COVERAGE
environment variable so I guess the required changes might be tackled
soon in master),

Any tip on how to identify which test is stuck except going through them
one by one?


One way is to add comment blocks '''...''' across the ftest.py file,
using a binary chop to identify the problem.

Or, since tests are run in series, you could hack test_util to pass
verbose parameters when it runs the tests - see 'cmd =' in
run_test_coverage().



I just commented out tests and found the following two are failing on my 
system:

testCompUtilVersions and testListBintools.

After digging a bit it seems that it is stuck here: 
https://source.denx.de/u-boot/u-boot/-/blob/master/tools/patman/command.py#L105 
for bzip2.


Furthermore:
bzip2 -V > /dev/null
bzip2 -V > /dev/null 2>&1
both get stuck which I assume is where the issue lies :)

bzip2 --help is just fine BTW.

I tested on a colleague's PC running Ubuntu 22.04.1, it works as 
intended. I guess I'll have to check if Fedora or Ubuntu has patches on 
top of bzip2 source code that triggers/patches this behavior.




python3-coverage is also not available in the container image built from
tools/docker/Dockerfile.


does 'python3 -m coverage' work?



diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py
index 0f6d1aa902..eaa769a564 100644
--- a/tools/patman/test_util.py
+++ b/tools/patman/test_util.py
@@ -58,11 +58,11 @@ def run_test_coverage(prog, filter_fname, 
exclude_list, build_dir, required=None

 prefix = ''
 if build_dir:
 prefix = 'PYTHONPATH=$PYTHONPATH:%s/sandbox_spl/tools ' % 
build_dir

-cmd = ('%spython3-coverage run '
+cmd = ('%spython3 -m coverage run '
'--omit "%s" %s %s %s -P1' % (prefix, ','.join(glob_list),
  prog, extra_args or '', 
test_cmd))

 os.system(cmd)
-stdout = command.output('python3-coverage', 'report')
+stdout = command.output('python3', '-m', 'coverage', 'report')
 lines = stdout.splitlines()
 if required:
 # Convert '/path/to/name.py' just the module name 'name'
@@ -81,7 +81,7 @@ def run_test_coverage(prog, filter_fname, 
exclude_list, build_dir, required=None

 print(coverage)
 if coverage != '100%':
 print(stdout)
-print("To get a report in 'htmlcov/index.html', type: 
python3-coverage html")
+print("To get a report in 'htmlcov/index.html', type: python3 
-m coverage html")

 print('Coverage error: %s, but should be 100%%' % coverage)
 ok = False
 if not ok:

works just fine for me.

Michal Suchánek seems to disagree with me on this one, see 
https://lore.kernel.org/u-boot/20220830101149.gm28...@kitsune.suse.cz/



or this:

https://urldefense.proofpoint.com/v2/url?u=https-3A__coverage.readthedocs.io_en_6.3.2_install.html&d=DwIBaQ&c=_sEr5x9kUWhuk4_nFwjJtA&r=LYjLexDn7rXIzVmkNPvw5ymA1XTSqHGq8yBP6m6qZZ4njZguQhZhkI_-172IIy1t&m=AH6ijvL2fp8TDrFjgeja0AVykFkzBjRPDOAfg8m_eKMHJW7RzTVA1wMpGn7qBwOe&s=BLW968ZKOcdPWg0s4-4AlA_rqiJCCCKPjP-Y-Fux6oI&e=




filename for the 241 file.

I've pushed a tree containing a suggested fix (updating this patch). I
can update it when applying if you like, otherwise please send a new
version.



Where did you push the tree

[RFC PATCH] mxs: Don't enable 4P2 reg if mx28 is powered only from DCDC_BATT without 5V

2022-08-30 Thread Alan Kay
mxs_power_enable_4p2() was added to mxs_batt_boot() in
'commit a0f97610757d ("ARM: mxs: Enable DCDC converter for battery boot")'
to enable DCDC converter when board is powered from 5V and has
detected sufficient battery voltage.
This involves enabling 4P2 regulator and there is a code
in mxs_power_enable_4p2() that disables VDDIO, VDDA, VDDD outputs of
the DCDC converter and enables BO for each power rail e.g.

setbits_le32(&power_regs->hw_power_vddioctrl,
POWER_VDDIOCTRL_DISABLE_FET | POWER_VDDIOCTRL_PWDN_BRNOUT);

In case the mx28 is powered by the 5V source and linear regulators are
supplying power to the VDDIO, VDDA, VDDD rails there is no issue.

However if the mx28 is powered by the DCDC_BATT source only without 5V,
disabling the DCDC converter outputs causes brownout power down.

The proposed solution is not to call mxs_power_enable_4p2() at all
if the mx28 is powered by the DCDC_BATT source only. There is no
reason to enable 4P2 regulator in this case and setup of all registers
is done in mxs_batt_boot().

Also there is no need to enable 5V brownout in
mxs_power_init() in this case.

Please consider if this is acceptable and I will submit a proper patch.

Signed-off-by: Alan Kay 
---

 arch/arm/cpu/arm926ejs/mxs/spl_power_init.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c 
b/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c
index c33170f06d..c8feadce54 100644
--- a/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c
+++ b/arch/arm/cpu/arm926ejs/mxs/spl_power_init.c
@@ -752,7 +752,9 @@ static void mxs_batt_boot(void)
POWER_5VCTRL_CHARGE_4P2_ILIMIT_MASK,
0x8 << POWER_5VCTRL_CHARGE_4P2_ILIMIT_OFFSET);
 
+#ifndef CONFIG_SPL_MXS_NO_VDD5V_SOURCE
mxs_power_enable_4p2();
+#endif
 }
 
 /**
@@ -1267,7 +1269,9 @@ void mxs_power_init(void)
POWER_CTRL_VBUS_VALID_IRQ | POWER_CTRL_BATT_BO_IRQ |
POWER_CTRL_DCDC4P2_BO_IRQ, &power_regs->hw_power_ctrl_clr);
 
+#ifndef CONFIG_SPL_MXS_NO_VDD5V_SOURCE
writel(POWER_5VCTRL_PWDN_5VBRNOUT, &power_regs->hw_power_5vctrl_set);
+#endif
 
early_delay(1000);
 }
-- 
2.25.1



[PATCH] arch: arm: mach-socfpga: Add mailbox command for HPS execution stage notification

2022-08-30 Thread Jit Loon Lim
From: Chin Liang See 

Add a new mailbox command "HPS_STAGE_NOTIFY" to notify Secure Device
Manager (SDM) on the stage of HPS code execution. In general, there
are three main code execution stages: First Stage Boot Loader (FSBL)
which is U-Boot SPL, Second Stage Boot Loader (SSBL) which is U-Boot,
and the Operating System which is Linux

Signed-off-by: Chin Liang See 
Signed-off-by: Jit Loon Lim 
---
 arch/arm/mach-socfpga/include/mach/mailbox_s10.h | 7 +++
 arch/arm/mach-socfpga/mailbox_s10.c  | 6 ++
 2 files changed, 13 insertions(+)

diff --git a/arch/arm/mach-socfpga/include/mach/mailbox_s10.h 
b/arch/arm/mach-socfpga/include/mach/mailbox_s10.h
index fbaf11597e..87af6f1cfe 100644
--- a/arch/arm/mach-socfpga/include/mach/mailbox_s10.h
+++ b/arch/arm/mach-socfpga/include/mach/mailbox_s10.h
@@ -123,6 +123,7 @@ enum ALT_SDM_MBOX_RESP_CODE {
 #define MBOX_QSPI_CLOSE51
 #define MBOX_QSPI_DIRECT   59
 #define MBOX_REBOOT_HPS71
+#define MBOX_HPS_STAGE_NOTIFY  93
 
 /* Mailbox registers */
 #define MBOX_CIN   0   /* command valid offset */
@@ -166,6 +167,11 @@ enum ALT_SDM_MBOX_RESP_CODE {
 #define RCF_SOFTFUNC_STATUS_SEU_ERROR  BIT(3)
 #define RCF_PIN_STATUS_NSTATUS BIT(31)
 
+/* Defines for HPS_STAGE_NOTIFY */
+#define HPS_EXECUTION_STATE_FSBL   0
+#define HPS_EXECUTION_STATE_SSBL   1
+#define HPS_EXECUTION_STATE_OS 2
+
 int mbox_send_cmd(u8 id, u32 cmd, u8 is_indirect, u32 len, u32 *arg, u8 urgent,
  u32 *resp_buf_len, u32 *resp_buf);
 int mbox_send_cmd_psci(u8 id, u32 cmd, u8 is_indirect, u32 len, u32 *arg,
@@ -182,6 +188,7 @@ int mbox_qspi_open(void);
 #endif
 
 int mbox_reset_cold(void);
+int mbox_hps_stage_notify(u32 execution_stage);
 int mbox_get_fpga_config_status(u32 cmd);
 int mbox_get_fpga_config_status_psci(u32 cmd);
 #endif /* _MAILBOX_S10_H_ */
diff --git a/arch/arm/mach-socfpga/mailbox_s10.c 
b/arch/arm/mach-socfpga/mailbox_s10.c
index 101af23855..e86d9bf214 100644
--- a/arch/arm/mach-socfpga/mailbox_s10.c
+++ b/arch/arm/mach-socfpga/mailbox_s10.c
@@ -479,6 +479,12 @@ int __secure mbox_send_cmd_psci(u8 id, u32 cmd, u8 
is_indirect, u32 len,
  urgent, resp_buf_len, resp_buf);
 }
 
+int mbox_hps_stage_notify(u32 execution_stage)
+{
+   return mbox_send_cmd(MBOX_ID_UBOOT, MBOX_HPS_STAGE_NOTIFY,
+MBOX_CMD_DIRECT, 1, &execution_stage, 0, 0, NULL);
+}
+
 int mbox_send_cmd_only(u8 id, u32 cmd, u8 is_indirect, u32 len, u32 *arg)
 {
return mbox_send_cmd_only_common(id, cmd, is_indirect, len, arg);
-- 
2.26.2



[PATCH] arch: arm : mach-socfpga: Add mailbox command for HPS execution stage notification

2022-08-30 Thread Jit Loon Lim
From: Chin Liang See 

Add a new mailbox command "HPS_STAGE_NOTIFY" to notify Secure Device
Manager (SDM) on the stage of HPS code execution. In general, there
are three main code execution stages: First Stage Boot Loader (FSBL)
which is U-Boot SPL, Second Stage Boot Loader (SSBL) which is U-Boot,
and the Operating System which is Linux

Signed-off-by: Chin Liang See 
Signed-off-by: Jit Loon Lim 
---
 arch/arm/mach-socfpga/include/mach/mailbox_s10.h | 7 +++
 arch/arm/mach-socfpga/mailbox_s10.c  | 6 ++
 2 files changed, 13 insertions(+)

diff --git a/arch/arm/mach-socfpga/include/mach/mailbox_s10.h 
b/arch/arm/mach-socfpga/include/mach/mailbox_s10.h
index fbaf11597e..87af6f1cfe 100644
--- a/arch/arm/mach-socfpga/include/mach/mailbox_s10.h
+++ b/arch/arm/mach-socfpga/include/mach/mailbox_s10.h
@@ -123,6 +123,7 @@ enum ALT_SDM_MBOX_RESP_CODE {
 #define MBOX_QSPI_CLOSE51
 #define MBOX_QSPI_DIRECT   59
 #define MBOX_REBOOT_HPS71
+#define MBOX_HPS_STAGE_NOTIFY  93
 
 /* Mailbox registers */
 #define MBOX_CIN   0   /* command valid offset */
@@ -166,6 +167,11 @@ enum ALT_SDM_MBOX_RESP_CODE {
 #define RCF_SOFTFUNC_STATUS_SEU_ERROR  BIT(3)
 #define RCF_PIN_STATUS_NSTATUS BIT(31)
 
+/* Defines for HPS_STAGE_NOTIFY */
+#define HPS_EXECUTION_STATE_FSBL   0
+#define HPS_EXECUTION_STATE_SSBL   1
+#define HPS_EXECUTION_STATE_OS 2
+
 int mbox_send_cmd(u8 id, u32 cmd, u8 is_indirect, u32 len, u32 *arg, u8 urgent,
  u32 *resp_buf_len, u32 *resp_buf);
 int mbox_send_cmd_psci(u8 id, u32 cmd, u8 is_indirect, u32 len, u32 *arg,
@@ -182,6 +188,7 @@ int mbox_qspi_open(void);
 #endif
 
 int mbox_reset_cold(void);
+int mbox_hps_stage_notify(u32 execution_stage);
 int mbox_get_fpga_config_status(u32 cmd);
 int mbox_get_fpga_config_status_psci(u32 cmd);
 #endif /* _MAILBOX_S10_H_ */
diff --git a/arch/arm/mach-socfpga/mailbox_s10.c 
b/arch/arm/mach-socfpga/mailbox_s10.c
index 101af23855..e86d9bf214 100644
--- a/arch/arm/mach-socfpga/mailbox_s10.c
+++ b/arch/arm/mach-socfpga/mailbox_s10.c
@@ -479,6 +479,12 @@ int __secure mbox_send_cmd_psci(u8 id, u32 cmd, u8 
is_indirect, u32 len,
  urgent, resp_buf_len, resp_buf);
 }
 
+int mbox_hps_stage_notify(u32 execution_stage)
+{
+   return mbox_send_cmd(MBOX_ID_UBOOT, MBOX_HPS_STAGE_NOTIFY,
+MBOX_CMD_DIRECT, 1, &execution_stage, 0, 0, NULL);
+}
+
 int mbox_send_cmd_only(u8 id, u32 cmd, u8 is_indirect, u32 len, u32 *arg)
 {
return mbox_send_cmd_only_common(id, cmd, is_indirect, len, arg);
-- 
2.26.2



[PATCH] arch: arm: mach-socfpga: To notify SDM when SPL pass control to U-Boot

2022-08-30 Thread Jit Loon Lim
From: Chin Liang See 

Prior SPL pass control to U-Boot, SPL will send a mailbox command
"HPS_STAGE_NOTIFY" to notify Secure Device Manager (SDM) on HPS SW
transition.

The purpose is for debug as user can query SDM on HPS error details
when HPS enters a warm reset due to error such as watchdog.

Signed-off-by: Chin Liang See 
Signed-off-by: Jit Loon Lim 
---
 arch/arm/mach-socfpga/spl_s10.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/mach-socfpga/spl_s10.c b/arch/arm/mach-socfpga/spl_s10.c
index c20e87cdbe..9bfd1a0f6a 100644
--- a/arch/arm/mach-socfpga/spl_s10.c
+++ b/arch/arm/mach-socfpga/spl_s10.c
@@ -92,3 +92,9 @@ void board_init_f(ulong dummy)
mbox_qspi_open();
 #endif
 }
+
+/* board specific function prior loading SSBL / U-Boot */
+void spl_board_prepare_for_boot(void)
+{
+   mbox_hps_stage_notify(HPS_EXECUTION_STATE_SSBL);
+}
-- 
2.19.0



[PATCH] arch: arm: mach-socfpga: To notify SDM when SPL pass control to U-Boot

2022-08-30 Thread Jit Loon Lim
From: Chin Liang See 

Prior SPL pass control to U-Boot, SPL will send a mailbox command
"HPS_STAGE_NOTIFY" to notify Secure Device Manager (SDM) on HPS SW
transition.

The purpose is for debug as user can query SDM on HPS error details
when HPS enters a warm reset due to error such as watchdog.

Signed-off-by: Jit Loon Lim 
---
 arch/arm/mach-socfpga/spl_s10.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/mach-socfpga/spl_s10.c b/arch/arm/mach-socfpga/spl_s10.c
index c20e87cdbe..9bfd1a0f6a 100644
--- a/arch/arm/mach-socfpga/spl_s10.c
+++ b/arch/arm/mach-socfpga/spl_s10.c
@@ -92,3 +92,9 @@ void board_init_f(ulong dummy)
mbox_qspi_open();
 #endif
 }
+
+/* board specific function prior loading SSBL / U-Boot */
+void spl_board_prepare_for_boot(void)
+{
+   mbox_hps_stage_notify(HPS_EXECUTION_STATE_SSBL);
+}
-- 
2.26.2



[PATCH] configs: stm32mp*: reset via CONFIG_RESET_SCMI

2022-08-30 Thread Jorge Ramirez-Ortiz
Enabling CONFIG_SYSRESET_PSCI prevents CONFIG_RESET_SCMI
from executing.

The side effect observed are I2C devices no longer being
accessible from U-boot after a soft reset.

Signed-off-by: Jorge Ramirez-Ortiz 
---
 configs/stm32mp13_defconfig | 1 -
 configs/stm32mp15_defconfig | 1 -
 configs/stm32mp15_trusted_defconfig | 1 -
 3 files changed, 3 deletions(-)

diff --git a/configs/stm32mp13_defconfig b/configs/stm32mp13_defconfig
index 673b468d31..44cee2e656 100644
--- a/configs/stm32mp13_defconfig
+++ b/configs/stm32mp13_defconfig
@@ -69,7 +69,6 @@ CONFIG_RNG_OPTEE=y
 CONFIG_DM_RTC=y
 CONFIG_RTC_STM32=y
 CONFIG_SERIAL_RX_BUFFER=y
-CONFIG_SYSRESET_PSCI=y
 CONFIG_TEE=y
 CONFIG_OPTEE=y
 # CONFIG_OPTEE_TA_AVB is not set
diff --git a/configs/stm32mp15_defconfig b/configs/stm32mp15_defconfig
index e5a2996c2c..2ad02f3652 100644
--- a/configs/stm32mp15_defconfig
+++ b/configs/stm32mp15_defconfig
@@ -133,7 +133,6 @@ CONFIG_SPI=y
 CONFIG_DM_SPI=y
 CONFIG_STM32_QSPI=y
 CONFIG_STM32_SPI=y
-CONFIG_SYSRESET_PSCI=y
 CONFIG_TEE=y
 CONFIG_OPTEE=y
 # CONFIG_OPTEE_TA_AVB is not set
diff --git a/configs/stm32mp15_trusted_defconfig 
b/configs/stm32mp15_trusted_defconfig
index e14668042f..9e24e82920 100644
--- a/configs/stm32mp15_trusted_defconfig
+++ b/configs/stm32mp15_trusted_defconfig
@@ -134,7 +134,6 @@ CONFIG_SPI=y
 CONFIG_DM_SPI=y
 CONFIG_STM32_QSPI=y
 CONFIG_STM32_SPI=y
-CONFIG_SYSRESET_PSCI=y
 CONFIG_TEE=y
 CONFIG_OPTEE=y
 # CONFIG_OPTEE_TA_AVB is not set
-- 
2.34.1



[PATCHv5] drivers: tee: i2c: support the NXP SE05x probe errata

2022-08-30 Thread Jorge Ramirez-Ortiz
Early instantiation of this I2C device would lock up when being
probed.

 https://www.nxp.com/docs/en/errata/SE050_Erratasheet.pdf
 3.2.2
   In scenarios of detecting I2C ICs on the bus using an empty
   I2C frame containing only the address the SE050 will block
   the I2C bus.

Tested on STM32MP1

Signed-off-by: Jorge Ramirez-Ortiz 
Acked-by: Oleksandr Suvorov 
---

 v5: address Kconfig issue with select (remove)
 v4: address build issue when feature not enabled
 v3,2,1: no changes (were part of a patchset)

 drivers/tee/optee/Kconfig | 13 
 drivers/tee/optee/i2c.c   | 44 +++
 2 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/drivers/tee/optee/Kconfig b/drivers/tee/optee/Kconfig
index d03028070b..8b02b71be2 100644
--- a/drivers/tee/optee/Kconfig
+++ b/drivers/tee/optee/Kconfig
@@ -37,6 +37,19 @@ config OPTEE_TA_SCP03
help
  Enables support for controlling (enabling, provisioning) the
  Secure Channel Protocol 03 operation in the OP-TEE SCP03 TA.
+
+config TEE_I2C_NXP_SE05X_ERRATA
+   bool "Enable NXP SE05X Errata"
+   default n
+   help
+This config prevents the I2C trampoline driver from probing
+on every transfer.
+
+config TEE_I2C_NXP_SE05X_ERRATA_IN_BUS
+   int "I2C bus where to apply the NXP SE05X errata"
+   depends on TEE_I2C_NXP_SE05X_ERRATA
+   default 0
+
 endmenu
 
 endif
diff --git a/drivers/tee/optee/i2c.c b/drivers/tee/optee/i2c.c
index ef4e10f991..e1b2534909 100644
--- a/drivers/tee/optee/i2c.c
+++ b/drivers/tee/optee/i2c.c
@@ -3,13 +3,18 @@
  * Copyright (c) 2020 Foundries.io Ltd
  */
 
+#define LOG_CATEGORY UCLASS_I2C
+
 #include 
 #include 
 #include 
+#include 
 #include 
 #include "optee_msg.h"
 #include "optee_private.h"
 
+#define NXP_SE05X_ADDR 0x48
+
 static int check_xfer_flags(struct udevice *chip, uint tee_flags)
 {
uint flags;
@@ -30,6 +35,30 @@ static int check_xfer_flags(struct udevice *chip, uint 
tee_flags)
return 0;
 }
 
+static struct udevice *get_chip_dev(int bnum, int addr)
+{
+   struct udevice *chip;
+   struct udevice *bus;
+
+#if defined(CONFIG_TEE_I2C_NXP_SE05X_ERRATA)
+   if (bnum == CONFIG_TEE_I2C_NXP_SE05X_ERRATA_IN_BUS &&
+   addr == NXP_SE05X_ADDR) {
+   if (uclass_get_device_by_seq(UCLASS_I2C, bnum, &bus))
+   return NULL;
+
+   if (i2c_get_chip(bus, addr, 0, &chip))
+   return NULL;
+
+   return chip;
+   }
+#endif
+
+   if (i2c_get_chip_for_busnum(bnum, addr, 0, &chip))
+   return NULL;
+
+   return chip;
+}
+
 void optee_suppl_cmd_i2c_transfer(struct optee_msg_arg *arg)
 {
const u8 attr[] = {
@@ -38,7 +67,8 @@ void optee_suppl_cmd_i2c_transfer(struct optee_msg_arg *arg)
OPTEE_MSG_ATTR_TYPE_RMEM_INOUT,
OPTEE_MSG_ATTR_TYPE_VALUE_OUTPUT,
};
-   struct udevice *chip_dev;
+   struct udevice *chip_dev = NULL;
+
struct tee_shm *shm;
u8 *buf;
int ret;
@@ -56,9 +86,9 @@ void optee_suppl_cmd_i2c_transfer(struct optee_msg_arg *arg)
if (!buf)
goto bad;
 
-   if (i2c_get_chip_for_busnum((int)arg->params[0].u.value.b,
-   (int)arg->params[0].u.value.c,
-   0, &chip_dev))
+   chip_dev = get_chip_dev((int)arg->params[0].u.value.b,
+   (int)arg->params[0].u.value.c);
+   if (!chip_dev)
goto bad;
 
if (check_xfer_flags(chip_dev, arg->params[1].u.value.a))
@@ -66,10 +96,16 @@ void optee_suppl_cmd_i2c_transfer(struct optee_msg_arg *arg)
 
switch (arg->params[0].u.value.a) {
case OPTEE_MSG_RPC_CMD_I2C_TRANSFER_RD:
+   log_debug("OPTEE_MSG_RPC_CMD_I2C_TRANSFER_RD %d\n",
+ (size_t)arg->params[2].u.rmem.size);
+
ret = dm_i2c_read(chip_dev, 0, buf,
  (size_t)arg->params[2].u.rmem.size);
break;
case OPTEE_MSG_RPC_CMD_I2C_TRANSFER_WR:
+   log_debug("OPTEE_MSG_RPC_CMD_I2C_TRANSFER_WR %d\n",
+ (size_t)arg->params[2].u.rmem.size);
+
ret = dm_i2c_write(chip_dev, 0, buf,
   (size_t)arg->params[2].u.rmem.size);
break;
-- 
2.34.1



[PATCH] arm: smh: Fix uninitialized parameters with newer GCCs

2022-08-30 Thread Sean Anderson
Newer versions of GCC won't initialize parts of structures which don't
appear to be used. This results in uninitialized semihosting parameters
passed via R1. Fix this by marking the inline assembly as clobbering
memory.

Signed-off-by: Sean Anderson 
---

 arch/arm/lib/semihosting.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/lib/semihosting.c b/arch/arm/lib/semihosting.c
index dbea2b06fb2..01d652a6b83 100644
--- a/arch/arm/lib/semihosting.c
+++ b/arch/arm/lib/semihosting.c
@@ -32,12 +32,12 @@ static noinline long smh_trap(unsigned int sysnum, void 
*addr)
 {
register long result asm("r0");
 #if defined(CONFIG_ARM64)
-   asm volatile ("hlt #0xf000" : "=r" (result) : "0"(sysnum), "r"(addr));
+   asm volatile ("hlt #0xf000" : "=r" (result) : "0"(sysnum), "r"(addr) : 
"memory");
 #elif defined(CONFIG_CPU_V7M)
-   asm volatile ("bkpt #0xAB" : "=r" (result) : "0"(sysnum), "r"(addr));
+   asm volatile ("bkpt #0xAB" : "=r" (result) : "0"(sysnum), "r"(addr) : 
"memory");
 #else
/* Note - untested placeholder */
-   asm volatile ("svc #0x123456" : "=r" (result) : "0"(sysnum), "r"(addr));
+   asm volatile ("svc #0x123456" : "=r" (result) : "0"(sysnum), "r"(addr) 
: "memory");
 #endif
return result;
 }
-- 
2.35.1.1320.gc452695387.dirty



[PATCH] cli: Reduce size of readline buffers in SPL

2022-08-30 Thread Sean Anderson
Normally, readline is not used int SPL. However, it may be useful to
enable the Freescale DDR interactive mode in SPL, while U-Boot is still
executing from SRAM. The default settings for readline result in a large
buffer being allocated. Reduce the size of the maximum input line, and
the number of lines of scrollback when building for SPL.

Signed-off-by: Sean Anderson 
---

 common/cli_readline.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/common/cli_readline.c b/common/cli_readline.c
index c7614a4c90f..2b81763b016 100644
--- a/common/cli_readline.c
+++ b/common/cli_readline.c
@@ -72,8 +72,13 @@ static char *delete_char (char *buffer, char *p, int *colp, 
int *np, int plen)
 #define getcmd_getch() getchar()
 #define getcmd_cbeep() getcmd_putch('\a')
 
+#ifdef CONFIG_SPL_BUILD
+#define HIST_MAX   3
+#define HIST_SIZE  32
+#else
 #define HIST_MAX   20
 #define HIST_SIZE  CONFIG_SYS_CBSIZE
+#endif
 
 static int hist_max;
 static int hist_add_idx;
-- 
2.35.1.1320.gc452695387.dirty



[PATCH] ddr: fsl: Reduce the size of interactive options

2022-08-30 Thread Sean Anderson
The interactive mode uses large several tables of options which can be
configured. However, much of the contents of these tables are
repetetive. For example, no struct is larger than half a kilobyte, so
the offset only takes up 9 bits. Similarly, the size is only ever 4 or
8, and printhex is a boolean. Reduce the size of these fields. This
reduces the size of the options tables by around 10 KiB. However, the
largest contributor to the size of the options tables is the use of a
pointer for the strings. A better approach would be to use a separate
array of strings, and store an integer index in the options tables.
However, this would require a large re-architecting of this file.

Signed-off-by: Sean Anderson 
---

 drivers/ddr/fsl/interactive.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/ddr/fsl/interactive.c b/drivers/ddr/fsl/interactive.c
index 2f76beb2dbe..eb2f06e8300 100644
--- a/drivers/ddr/fsl/interactive.c
+++ b/drivers/ddr/fsl/interactive.c
@@ -27,9 +27,9 @@
 /* Option parameter Structures */
 struct options_string {
const char *option_name;
-   size_t offset;
-   unsigned int size;
-   const char printhex;
+   u32 offset : 9;
+   u32 size : 4;
+   u32 printhex : 1;
 };
 
 static unsigned int picos_to_mhz(unsigned int picos)
-- 
2.35.1.1320.gc452695387.dirty



[PATCH] ddr: fsl: Make bank_addr_bits reflect actual bits

2022-08-30 Thread Sean Anderson
In both the Freescale DDR controller and the SPD spec, bank address bits
are stored as the number of bank address bits minus 2. For example, if a
chip had 8 banks (3 total bank address bits), the value of
bank_addr_bits would be 1. This is rather surprising for users
configuring their memory manually, since they can't set bank_addr_bits
to the actual number of bank address bits. Rectify this.

There is at least one example of this kind of mistake already, in
board/freescale/t102xrdb/ddr.c. The documented MT40A512M8HX has two bank
address bits, but bank_addr_bits was set to 2, implying 4 bank address
bits. Such a value is reserved in BA_BITS_CS, but I suspect the
controller simply ignores the top bit, making this kind of mistake
harmless, if misleading.

Fixes: e8a7f1c32b5 ("powerpc/t1023rdb: Add T1023 RDB board support")
Signed-off-by: Sean Anderson 
---

 board/freescale/ls1043ardb/ddr.c   | 2 +-
 drivers/ddr/fsl/ctrl_regs.c| 2 +-
 drivers/ddr/fsl/ddr4_dimm_params.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/board/freescale/ls1043ardb/ddr.c b/board/freescale/ls1043ardb/ddr.c
index 08b43ff5e4c..4d2fce38412 100644
--- a/board/freescale/ls1043ardb/ddr.c
+++ b/board/freescale/ls1043ardb/ddr.c
@@ -114,7 +114,7 @@ dimm_params_t ddr_raw_timing = {
.mirrored_dimm = 0,
.n_row_addr = 15,
.n_col_addr = 10,
-   .bank_addr_bits = 0,
+   .bank_addr_bits = 2,
.bank_group_bits = 2,
.edc_config = 0,
.burst_lengths_bitmask = 0x0c,
diff --git a/drivers/ddr/fsl/ctrl_regs.c b/drivers/ddr/fsl/ctrl_regs.c
index b5122d1a1c3..0b0b4e5cb7e 100644
--- a/drivers/ddr/fsl/ctrl_regs.c
+++ b/drivers/ddr/fsl/ctrl_regs.c
@@ -214,7 +214,7 @@ static void set_csn_config(int dimm_number, int i, 
fsl_ddr_cfg_regs_t *ddr,
odt_rd_cfg = popts->cs_local_opts[i].odt_rd_cfg;
odt_wr_cfg = popts->cs_local_opts[i].odt_wr_cfg;
 #ifdef CONFIG_SYS_FSL_DDR4
-   ba_bits_cs_n = dimm_params[dimm_number].bank_addr_bits;
+   ba_bits_cs_n = dimm_params[dimm_number].bank_addr_bits - 2;
bg_bits_cs_n = dimm_params[dimm_number].bank_group_bits;
 #else
n_banks_per_sdram_device
diff --git a/drivers/ddr/fsl/ddr4_dimm_params.c 
b/drivers/ddr/fsl/ddr4_dimm_params.c
index e2bdc12ef2c..ea791622628 100644
--- a/drivers/ddr/fsl/ddr4_dimm_params.c
+++ b/drivers/ddr/fsl/ddr4_dimm_params.c
@@ -246,7 +246,7 @@ unsigned int ddr_compute_dimm_parameters(const unsigned int 
ctrl_num,
/* SDRAM device parameters */
pdimm->n_row_addr = ((spd->addressing >> 3) & 0x7) + 12;
pdimm->n_col_addr = (spd->addressing & 0x7) + 9;
-   pdimm->bank_addr_bits = (spd->density_banks >> 4) & 0x3;
+   pdimm->bank_addr_bits = ((spd->density_banks >> 4) & 0x3) + 2;
pdimm->bank_group_bits = (spd->density_banks >> 6) & 0x3;
 
/*
-- 
2.35.1.1320.gc452695387.dirty



Re: [PATCH] dt-bindings: nvmem: u-boot,env: add basic NVMEM cells

2022-08-30 Thread Tom Rini
On Tue, Aug 16, 2022 at 11:26:37AM +0200, Rafał Miłecki wrote:
> On 3.07.2022 10:48, Rafał Miłecki wrote:
> > U-Boot doesn't have cells at hardcoded addresses. They are stored in
> > internal format. It's still important to define relevant cells in DT so
> > NVMEM consumers can reference them.
> > 
> > Update binding to allow including basic cells as NVMEM device subnodes.
> 
> Ping :)
> 
> 
> For a reference you can see Broadcom's NVRAM (identical feature):
> 
> 084973e944bec ("dt-bindings: nvmem: brcm,nvram: add basic NVMEM cells")
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=084973e944bec21804f8afb0515b25434438699a
> 
> c8442f0fb09ca ("ARM: dts: BCM5301X: Add Ethernet MAC address to Luxul 
> XWR-3150")
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c8442f0fb09ca3d842b9b23d1d0650f649fd10f8

Seems fine to me.

Reviewed-by: Tom Rini 

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH 0/6] Enable CONFIG_TIMER for all Kirwood / MVEBU boards

2022-08-30 Thread Tony Dinh
Hi Stefan,

On Tue, Aug 30, 2022 at 4:53 AM Stefan Roese  wrote:
>
> This patchset enhaces the recently added Orion Timer driver to support
> all other Kirkwood & 32bit MVEBU Armada platforms. Additionally, this
> timer support is then enabled per default for those platforms, so that
> the board config files don't need to be changed. Also necessary is
> some dts hacking, so that the timer DT node is available in early
> U-Boot stages.
>
> I've successfully tested this patchset on an Armada XP board. Additional
> test on other boards and platforms are very welcome and necessary.

I've run some tests with the following 2 Kirkwood boards: Cloud
Engines Pogo V4 88F6192 (with CONFIG_DM_RTC and CONFIG_RTC_EMULATION),
and Marvell Sheevaplug 88F6281 (with CONFIG_DM_RTC and CONFIG_RTC_MV).

It seems that it was either frozen or the timer did not expire at some
subsequent sleep commands. Sometime it happened at 2nd command, some
time at a later sleep command. For example,

=== Pogo V4 (the 1st sleep command works correctly at 10 seconds on my
stopwatch)

U-Boot 2022.10-rc3-00048-g66ccd87a9c-dirty (Aug 30 2022 - 13:38:24 -0700)
Pogoplug V4

Hit any key to stop autoboot:  0
Pogo_V4> sleep 10
Pogo_V4> sleep 31.5


=== Sheevaplug (RTC battery is old, so the date was not updated, but
the clock seems OK)

U-Boot 2022.10-rc3-00048-g66ccd87a9c-dirty (Aug 30 2022 - 14:14:24 -0700)
Marvell-Sheevaplug
Hit any key to stop autoboot:  0
=> date
Date: 2000-01-01 (Saturday)Time:  0:02:55
=> sleep 10
=> date
Date: 2000-01-01 (Saturday)Time:  0:03:18
=> sleep 10
=> sleep 20.1


Please let me know what I can do (i.e. perhaps running a debug patch).
Thanks,
Tony

> Thanks,
> Stefan
>
> Stefan Roese (6):
>   timer: orion-timer: Add support for other Armada SoC's
>   timer: orion-timer: Add timer_get_boot_us() for BOOTSTAGE support
>   arm: mvebu: Use CONFIG_TIMER on all MVEBU & KIRKWOOD platforms
>   arm: mvebu: dts: Makefile: Compile Armada 375 dtb in a separate step
>   arm: mvebu: dts: armada-375.dtsi: Add timer0 & timer1
>   arm: mvebu: dts: mvebu-u-boot.dtsi: Add "u-boot,dm-pre-reloc" to timer
> DT node
>
>  arch/arm/Kconfig  |  4 ++
>  arch/arm/dts/Makefile |  6 ++-
>  arch/arm/dts/armada-375.dtsi  |  4 +-
>  arch/arm/dts/mvebu-u-boot.dtsi| 11 
>  arch/arm/mach-mvebu/include/mach/config.h |  5 --
>  drivers/timer/Kconfig |  5 +-
>  drivers/timer/orion-timer.c   | 66 +--
>  7 files changed, 89 insertions(+), 12 deletions(-)
>
> --
> 2.37.2
>


Re: [PATCH] cli: Reduce size of readline buffers in SPL

2022-08-30 Thread Simon Glass
On Tue, 30 Aug 2022 at 14:40, Sean Anderson  wrote:
>
> Normally, readline is not used int SPL. However, it may be useful to
> enable the Freescale DDR interactive mode in SPL, while U-Boot is still
> executing from SRAM. The default settings for readline result in a large
> buffer being allocated. Reduce the size of the maximum input line, and
> the number of lines of scrollback when building for SPL.
>
> Signed-off-by: Sean Anderson 
> ---
>
>  common/cli_readline.c | 5 +
>  1 file changed, 5 insertions(+)

Reviewed-by: Simon Glass 


[PATCH v5 0/7] tpm: Various minor fixes and enhancements

2022-08-30 Thread Simon Glass
This series contains some minor enhancements for the TPM code to make it
work with Chromium OS verified boot.

Changes in v5:
- Reword mention of measured boot
- Use size_t for buffer size

Changes in v4:
- Rename function and add arguments for the command/subcmd
- Rename function and add arguments for the command/subcmd

Changes in v3:
- Drop limits on the TPM hash size
- Update commit message

Changes in v2:
- Use "cmd" for the digest type in the tpm command
- Update comment for tpm_pcr_extend() 'name' parameter
- Use constants instead of open-coded values

Simon Glass (7):
  tpm: Require a digest source when extending the PCR
  tpm: Correct the permissions command in TPMv1
  tpm: Correct the define-space command in TPMv2
  tpm: sandbox: Allow init of TPM in a different phase
  tpm: Allow reporting the internal state
  tpm: Implement state command for Cr50
  tpm: Allow committing non-volatile data

 cmd/tpm-common.c   |  20 
 cmd/tpm-user-utils.h   |   2 +
 cmd/tpm-v1.c   |   3 +-
 cmd/tpm-v2.c   |   3 +
 cmd/tpm_test.c |   5 +-
 drivers/tpm/cr50_i2c.c | 163 +
 drivers/tpm/tpm-uclass.c   |  10 ++
 drivers/tpm/tpm2_tis_sandbox.c |  17 +++-
 include/tpm-common.h   |  20 
 include/tpm-v2.h   |  30 ++
 include/tpm_api.h  |   8 +-
 lib/tpm-v1.c   |   5 +-
 lib/tpm-v2.c   |  70 --
 lib/tpm_api.c  |  10 +-
 test/dm/Makefile   |   1 +
 test/dm/tpm.c  |  34 +++
 16 files changed, 380 insertions(+), 21 deletions(-)
 create mode 100644 test/dm/tpm.c

-- 
2.37.2.672.g94769d06f0-goog



[PATCH v5 1/7] tpm: Require a digest source when extending the PCR

2022-08-30 Thread Simon Glass
This feature is used for measured boot, so we can add a log entry to the
TCPA with some information about where the digest comes from. It is not
currently supported in the TPM drivers, but add it to the API so that
code which expects it can signal its request.

Signed-off-by: Simon Glass 
Reviewed-by: Ilias Apalodimas 
---

Changes in v5:
- Reword mention of measured boot

Changes in v3:
- Drop limits on the TPM hash size
- Update commit message

Changes in v2:
- Use "cmd" for the digest type in the tpm command
- Update comment for tpm_pcr_extend() 'name' parameter

 cmd/tpm-v1.c  |  3 ++-
 cmd/tpm_test.c|  5 +++--
 include/tpm_api.h |  8 +---
 lib/tpm-v2.c  |  2 ++
 lib/tpm_api.c | 10 ++
 5 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/cmd/tpm-v1.c b/cmd/tpm-v1.c
index bf238a9f2e3..0efb079b0a9 100644
--- a/cmd/tpm-v1.c
+++ b/cmd/tpm-v1.c
@@ -131,7 +131,8 @@ static int do_tpm_extend(struct cmd_tbl *cmdtp, int flag, 
int argc,
return CMD_RET_FAILURE;
}
 
-   rc = tpm_pcr_extend(dev, index, in_digest, out_digest);
+   rc = tpm_pcr_extend(dev, index, in_digest, sizeof(in_digest),
+   out_digest, "cmd");
if (!rc) {
puts("PCR value after execution of the command:\n");
print_byte_string(out_digest, sizeof(out_digest));
diff --git a/cmd/tpm_test.c b/cmd/tpm_test.c
index a3ccb12f53a..b35eae81dc3 100644
--- a/cmd/tpm_test.c
+++ b/cmd/tpm_test.c
@@ -91,7 +91,8 @@ static int test_early_extend(struct udevice *dev)
tpm_init(dev);
TPM_CHECK(tpm_startup(dev, TPM_ST_CLEAR));
TPM_CHECK(tpm_continue_self_test(dev));
-   TPM_CHECK(tpm_pcr_extend(dev, 1, value_in, value_out));
+   TPM_CHECK(tpm_pcr_extend(dev, 1, value_in, sizeof(value_in), value_out,
+"test"));
printf("done\n");
return 0;
 }
@@ -438,7 +439,7 @@ static int test_timing(struct udevice *dev)
   100);
TTPM_CHECK(tpm_nv_read_value(dev, INDEX0, (uint8_t *)&x, sizeof(x)),
   100);
-   TTPM_CHECK(tpm_pcr_extend(dev, 0, in, out), 200);
+   TTPM_CHECK(tpm_pcr_extend(dev, 0, in, sizeof(in), out, "test"), 200);
TTPM_CHECK(tpm_set_global_lock(dev), 50);
TTPM_CHECK(tpm_tsc_physical_presence(dev, PHYS_PRESENCE), 100);
printf("done\n");
diff --git a/include/tpm_api.h b/include/tpm_api.h
index 11aa14eb793..8979d9d6df7 100644
--- a/include/tpm_api.h
+++ b/include/tpm_api.h
@@ -81,14 +81,16 @@ u32 tpm_nv_write_value(struct udevice *dev, u32 index, 
const void *data,
  *
  * @param dev  TPM device
  * @param indexindex of the PCR
- * @param in_digest160-bit value representing the event to be
+ * @param in_digest160/256-bit value representing the event to be
  * recorded
- * @param out_digest   160-bit PCR value after execution of the
+ * @param size size of digest in bytes
+ * @param out_digest   160/256-bit PCR value after execution of the
  * command
+ * @param name digest source, used for log output
  * Return: return code of the operation
  */
 u32 tpm_pcr_extend(struct udevice *dev, u32 index, const void *in_digest,
-  void *out_digest);
+  uint size, void *out_digest, const char *name);
 
 /**
  * Issue a TPM_PCRRead command.
diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c
index 1bf627853af..6058f2e1e4f 100644
--- a/lib/tpm-v2.c
+++ b/lib/tpm-v2.c
@@ -157,6 +157,8 @@ u32 tpm2_pcr_extend(struct udevice *dev, u32 index, u32 
algorithm,
};
int ret;
 
+   if (!digest)
+   return -EINVAL;
/*
 * Fill the command structure starting from the first buffer:
 * - the digest
diff --git a/lib/tpm_api.c b/lib/tpm_api.c
index 032f383ca04..7e8df8795ef 100644
--- a/lib/tpm_api.c
+++ b/lib/tpm_api.c
@@ -140,15 +140,17 @@ u32 tpm_write_lock(struct udevice *dev, u32 index)
 }
 
 u32 tpm_pcr_extend(struct udevice *dev, u32 index, const void *in_digest,
-  void *out_digest)
+  uint size, void *out_digest, const char *name)
 {
-   if (tpm_is_v1(dev))
+   if (tpm_is_v1(dev)) {
return tpm1_extend(dev, index, in_digest, out_digest);
-   else if (tpm_is_v2(dev))
+   } else if (tpm_is_v2(dev)) {
return tpm2_pcr_extend(dev, index, TPM2_ALG_SHA256, in_digest,
   TPM2_DIGEST_LEN);
-   else
+   /* @name is ignored as we do not support the TPM log here */
+   } else {
return -ENOSYS;
+   }
 }
 
 u32 tpm_pcr_read(struct udevice *dev, u32 index, void *data, size_t count)
-- 
2.37.2.672.g94769d06f0-goog



  1   2   >