Re: [PATCH] Revert "lib: Improve _parse_integer_fixup_radix base 16 detection"

2020-06-06 Thread Sean Anderson
On 6/7/20 2:40 AM, Heinrich Schuchardt wrote:
> On 6/7/20 7:36 AM, Sean Anderson wrote:
>> This reverts commit 0486497e2b5f4d36fa968a1a60fea358cbf70b65.
>>
>> The strtoul has well-defined semantics. It is defined by the C standard and
>> POSIX. To quote the relevant section of the man pages,
>>
>>> If base is zero or 16, the string may then include a "0x" prefix, and the
>>> number will be read in base 16; otherwise, a zero base is taken as 10
>>> (decimal) unless the next character is '0', in which case it is taken as
>>> 8 (octal).
>>
>> Keeping these semantics is important for several reasons. First, it is very
>> surprising for standard library functions to behave differently than usual.
>> Every other implementation of strtoul has different semantics than the
>> implementation in U-Boot at the moment. Second, it can result in very
>> surprising results from small changes. For example, changing the string
>> "1f" to "20" causes the parsed value to *decrease*. Forcing use of the "0x"
>> prefix to specify hexidecimal numbers is a feature, not a bug. Lastly, this
>> is slightly less performant, since the entire number is parsed twice.
>>
>> This fixes the str_simple_strtoul test failing with
>>
>> test/str_ut.c:29, run_strtoul(): expect_val == val: Expected 0x44b (1099), 
>> got 0x1099ab (1087915)
>> test/str_ut.c:46, str_simple_strtoul(): 0 == run_strtoul(uts, str2, 0, 1099, 
>> 4): Expected 0x0 (0), got 0x1 (1)
>>
> 
> I suggest to add a Fixes line here:
> 
> Fixes: 0486497e2b5f ("lib: Improve _parse_integer_fixup_radix base 16
> detection")

Thanks, I will add that.

> 
> Cf.
> https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes
> 
>> Signed-off-by: Sean Anderson 
>> CC: Michal Simek 
>> CC: Shiril Tichkule 
>> ---
>>
>>  lib/strto.c | 18 +-
>>  1 file changed, 1 insertion(+), 17 deletions(-)
>>
>> diff --git a/lib/strto.c b/lib/strto.c
>> index 3d77115d4d..c00bb5895d 100644
>> --- a/lib/strto.c
>> +++ b/lib/strto.c
>> @@ -22,25 +22,9 @@ static const char *_parse_integer_fixup_radix(const char 
>> *s, unsigned int *base)
>>  *base = 16;
>>  else
>>  *base = 8;
>> -} else {
>> -int i = 0;
>> -char var;
>> -
>> +} else
> 
> scripts/checkpatch.pl reports a problem:
> 
> CHECK: Unbalanced braces around else statement
> #50: FILE: lib/strto.c:25:
> +   } else

I chose this form since that is what is used in Linux, and that is
how it formatted before the commit this reverts. I do think that it is
better form to have braces around this else block, though.

--Sean


Re: [PATCH v1 00/15] add basic driver support for broadcom NS3 soc

2020-06-06 Thread dphadke
USB host node is missing from this patch series.

Driver was previously added here -
https://lists.denx.de/pipermail/u-boot/2020-April/405764.html

Linux arch/arm64/boot/dts/broadcom/stingray/stingray-usb.dtsi

Regards,
Dhananjay




--
Sent from: http://u-boot.10912.n7.nabble.com/


Re: [PATCH] Revert "lib: Improve _parse_integer_fixup_radix base 16 detection"

2020-06-06 Thread Heinrich Schuchardt
On 6/7/20 7:36 AM, Sean Anderson wrote:
> This reverts commit 0486497e2b5f4d36fa968a1a60fea358cbf70b65.
>
> The strtoul has well-defined semantics. It is defined by the C standard and
> POSIX. To quote the relevant section of the man pages,
>
>> If base is zero or 16, the string may then include a "0x" prefix, and the
>> number will be read in base 16; otherwise, a zero base is taken as 10
>> (decimal) unless the next character is '0', in which case it is taken as
>> 8 (octal).
>
> Keeping these semantics is important for several reasons. First, it is very
> surprising for standard library functions to behave differently than usual.
> Every other implementation of strtoul has different semantics than the
> implementation in U-Boot at the moment. Second, it can result in very
> surprising results from small changes. For example, changing the string
> "1f" to "20" causes the parsed value to *decrease*. Forcing use of the "0x"
> prefix to specify hexidecimal numbers is a feature, not a bug. Lastly, this
> is slightly less performant, since the entire number is parsed twice.
>
> This fixes the str_simple_strtoul test failing with
>
> test/str_ut.c:29, run_strtoul(): expect_val == val: Expected 0x44b (1099), 
> got 0x1099ab (1087915)
> test/str_ut.c:46, str_simple_strtoul(): 0 == run_strtoul(uts, str2, 0, 1099, 
> 4): Expected 0x0 (0), got 0x1 (1)
>

I suggest to add a Fixes line here:

Fixes: 0486497e2b5f ("lib: Improve _parse_integer_fixup_radix base 16
detection")

Cf.
https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes

> Signed-off-by: Sean Anderson 
> CC: Michal Simek 
> CC: Shiril Tichkule 
> ---
>
>  lib/strto.c | 18 +-
>  1 file changed, 1 insertion(+), 17 deletions(-)
>
> diff --git a/lib/strto.c b/lib/strto.c
> index 3d77115d4d..c00bb5895d 100644
> --- a/lib/strto.c
> +++ b/lib/strto.c
> @@ -22,25 +22,9 @@ static const char *_parse_integer_fixup_radix(const char 
> *s, unsigned int *base)
>   *base = 16;
>   else
>   *base = 8;
> - } else {
> - int i = 0;
> - char var;
> -
> + } else

scripts/checkpatch.pl reports a problem:

CHECK: Unbalanced braces around else statement
#50: FILE: lib/strto.c:25:
+   } else

Best regards

Heinrich

>   *base = 10;
> -
> - do {
> - var = tolower(s[i++]);
> - if (var >= 'a' && var <= 'f') {
> - *base = 16;
> - break;
> - }
> -
> - if (!(var >= '0' && var <= '9'))
> - break;
> - } while (var);
> - }
>   }
> -
>   if (*base == 16 && s[0] == '0' && tolower(s[1]) == 'x')
>   s += 2;
>   return s;
>



Re: [PATCH] test: add dependency for UT_LOG

2020-06-06 Thread Heinrich Schuchardt
Am June 7, 2020 3:52:37 AM UTC schrieb Kever Yang :
>The callback of do_ut_log() is defined in test/log/test-main.c
>which is depend on CONFIG_LOG.

Where do you see such a dependency?

With which configuration did you have a build problem?

>
>Signed-off-by: Kever Yang 
>---
>
> test/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/test/Kconfig b/test/Kconfig
>index 9b2f84b551..8c32fa5cbd 100644
>--- a/test/Kconfig
>+++ b/test/Kconfig
>@@ -42,6 +42,7 @@ endif
> 
> config UT_LOG
>   bool "Unit tests for logging functions"
>+  depends on LOG

This seems incorrect. We want to run the nolog test if CONFIG_LOG=n.

See https://github.com/trini/u-boot/blob/master/test/log/Makefile#L15

Best regards

Heinrich


>   depends on UNIT_TEST
>   default y
>   help



[PATCH] Revert "lib: Improve _parse_integer_fixup_radix base 16 detection"

2020-06-06 Thread Sean Anderson
This reverts commit 0486497e2b5f4d36fa968a1a60fea358cbf70b65.

The strtoul has well-defined semantics. It is defined by the C standard and
POSIX. To quote the relevant section of the man pages,

> If base is zero or 16, the string may then include a "0x" prefix, and the
> number will be read in base 16; otherwise, a zero base is taken as 10
> (decimal) unless the next character is '0', in which case it is taken as
> 8 (octal).

Keeping these semantics is important for several reasons. First, it is very
surprising for standard library functions to behave differently than usual.
Every other implementation of strtoul has different semantics than the
implementation in U-Boot at the moment. Second, it can result in very
surprising results from small changes. For example, changing the string
"1f" to "20" causes the parsed value to *decrease*. Forcing use of the "0x"
prefix to specify hexidecimal numbers is a feature, not a bug. Lastly, this
is slightly less performant, since the entire number is parsed twice.

This fixes the str_simple_strtoul test failing with

test/str_ut.c:29, run_strtoul(): expect_val == val: Expected 0x44b (1099), got 
0x1099ab (1087915)
test/str_ut.c:46, str_simple_strtoul(): 0 == run_strtoul(uts, str2, 0, 1099, 
4): Expected 0x0 (0), got 0x1 (1)

Signed-off-by: Sean Anderson 
CC: Michal Simek 
CC: Shiril Tichkule 
---

 lib/strto.c | 18 +-
 1 file changed, 1 insertion(+), 17 deletions(-)

diff --git a/lib/strto.c b/lib/strto.c
index 3d77115d4d..c00bb5895d 100644
--- a/lib/strto.c
+++ b/lib/strto.c
@@ -22,25 +22,9 @@ static const char *_parse_integer_fixup_radix(const char *s, 
unsigned int *base)
*base = 16;
else
*base = 8;
-   } else {
-   int i = 0;
-   char var;
-
+   } else
*base = 10;
-
-   do {
-   var = tolower(s[i++]);
-   if (var >= 'a' && var <= 'f') {
-   *base = 16;
-   break;
-   }
-
-   if (!(var >= '0' && var <= '9'))
-   break;
-   } while (var);
-   }
}
-
if (*base == 16 && s[0] == '0' && tolower(s[1]) == 'x')
s += 2;
return s;
-- 
2.26.2



Re: [PATCH v3 5/8] rockchip: puma: reorganize devicetrees to actually work and match upstream

2020-06-06 Thread Kever Yang



On 2020/6/5 下午6:06, Heiko Stuebner wrote:

From: Heiko Stuebner 

So far the puma dts files only just included the main puma dtsi without
handling the actual baseboard and rk3399-puma.dtsi was very much
detached from the variant in the mainline Linux kernel.

Recent changes resulted in a strange situation with nonworking puma boards.

Commit ab800e5a6f28 ("arm: dts: rockchip: puma: move U-Boot specific bits to 
u-boot.dtsi")
moved the sdram include from rk3399-puma-ddrX.dts to new files
rk3399-puma-ddrx-u-boot.dtsi which were never included anywhere though.

Commit 167efc2c7a46 ("arm64: dts: rk3399: Sync v5.7-rc1 from Linux")
replaced the rk3399-puma.dtsi nearly completely, but in the kernel
it definitly depends on a baseboard dts to actually enable peripherals
like sd-slot, uarts, etc.

So to untagle this and bring the whole thing more in line with mainline
Linux, bring the rk3399-puma-haikou.dts over as well, drop the separate
DDR-option devicetrees and instead replace them with a puma Kconfig option
to select and include the needed DDR variant.

Signed-off-by: Heiko Stuebner 
Reviewed-by: Philipp Tomsich 


Reviewed-by: Kever Yang 

Thanks,
- Kever

---
  arch/arm/dts/Makefile |   4 +-
  arch/arm/dts/rk3399-puma-ddr1333-u-boot.dtsi  |   4 -
  arch/arm/dts/rk3399-puma-ddr1333.dts  |   8 -
  arch/arm/dts/rk3399-puma-ddr1600-u-boot.dtsi  |   4 -
  arch/arm/dts/rk3399-puma-ddr1600.dts  |   9 -
  arch/arm/dts/rk3399-puma-ddr1866-u-boot.dtsi  |   4 -
  arch/arm/dts/rk3399-puma-ddr1866.dts  |   8 -
  ...ot.dtsi => rk3399-puma-haikou-u-boot.dtsi} |  11 +
  arch/arm/dts/rk3399-puma-haikou.dts   | 271 ++
  board/theobroma-systems/puma_rk3399/Kconfig   |  15 +
  configs/puma-rk3399_defconfig |   2 +-
  11 files changed, 299 insertions(+), 41 deletions(-)
  delete mode 100644 arch/arm/dts/rk3399-puma-ddr1333-u-boot.dtsi
  delete mode 100644 arch/arm/dts/rk3399-puma-ddr1333.dts
  delete mode 100644 arch/arm/dts/rk3399-puma-ddr1600-u-boot.dtsi
  delete mode 100644 arch/arm/dts/rk3399-puma-ddr1600.dts
  delete mode 100644 arch/arm/dts/rk3399-puma-ddr1866-u-boot.dtsi
  delete mode 100644 arch/arm/dts/rk3399-puma-ddr1866.dts
  rename arch/arm/dts/{rk3399-puma-u-boot.dtsi => 
rk3399-puma-haikou-u-boot.dtsi} (80%)
  create mode 100644 arch/arm/dts/rk3399-puma-haikou.dts

diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index c6af87cf5e..a66bdb439d 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -130,9 +130,7 @@ dtb-$(CONFIG_ROCKCHIP_RK3399) += \
rk3399-nanopi-neo4.dtb \
rk3399-orangepi.dtb \
rk3399-pinebook-pro.dtb \
-   rk3399-puma-ddr1333.dtb \
-   rk3399-puma-ddr1600.dtb \
-   rk3399-puma-ddr1866.dtb \
+   rk3399-puma-haikou.dtb \
rk3399-roc-pc.dtb \
rk3399-roc-pc-mezzanine.dtb \
rk3399-rock-pi-4.dtb \
diff --git a/arch/arm/dts/rk3399-puma-ddr1333-u-boot.dtsi 
b/arch/arm/dts/rk3399-puma-ddr1333-u-boot.dtsi
deleted file mode 100644
index 39d3927f49..00
--- a/arch/arm/dts/rk3399-puma-ddr1333-u-boot.dtsi
+++ /dev/null
@@ -1,4 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+
-
-#include "rk3399-puma-u-boot.dtsi"
-#include "rk3399-sdram-ddr3-1333.dtsi"
diff --git a/arch/arm/dts/rk3399-puma-ddr1333.dts 
b/arch/arm/dts/rk3399-puma-ddr1333.dts
deleted file mode 100644
index 80f27699f4..00
--- a/arch/arm/dts/rk3399-puma-ddr1333.dts
+++ /dev/null
@@ -1,8 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+ OR X11
-/*
- * (C) Copyright 2017 Theobroma Systems Design und Consulting GmbH
- */
-
-/dts-v1/;
-
-#include "rk3399-puma.dtsi"
diff --git a/arch/arm/dts/rk3399-puma-ddr1600-u-boot.dtsi 
b/arch/arm/dts/rk3399-puma-ddr1600-u-boot.dtsi
deleted file mode 100644
index be58311dc4..00
--- a/arch/arm/dts/rk3399-puma-ddr1600-u-boot.dtsi
+++ /dev/null
@@ -1,4 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+
-
-#include "rk3399-puma-u-boot.dtsi"
-#include "rk3399-sdram-ddr3-1600.dtsi"
diff --git a/arch/arm/dts/rk3399-puma-ddr1600.dts 
b/arch/arm/dts/rk3399-puma-ddr1600.dts
deleted file mode 100644
index cb76b0165c..00
--- a/arch/arm/dts/rk3399-puma-ddr1600.dts
+++ /dev/null
@@ -1,9 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+ OR X11
-/*
- * (C) Copyright 2017 Theobroma Systems Design und Consulting GmbH
- */
-
-/dts-v1/;
-
-#include "rk3399-puma.dtsi"
-#include "rk3399-u-boot.dtsi"
diff --git a/arch/arm/dts/rk3399-puma-ddr1866-u-boot.dtsi 
b/arch/arm/dts/rk3399-puma-ddr1866-u-boot.dtsi
deleted file mode 100644
index 48da076329..00
--- a/arch/arm/dts/rk3399-puma-ddr1866-u-boot.dtsi
+++ /dev/null
@@ -1,4 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+
-
-#include "rk3399-puma-u-boot.dtsi"
-#include "rk3399-sdram-ddr3-1866.dtsi"
diff --git a/arch/arm/dts/rk3399-puma-ddr1866.dts 
b/arch/arm/dts/rk3399-puma-ddr1866.dts
deleted file mode 100644
index 80f27699f4..00
--- a/arch/arm/dts/rk3399-puma-ddr1866.dts
+++ /dev/null
@@ -1,8 

[PATCH] rockchip: rk3399-evb: add stdout-path for the board

2020-06-06 Thread Kever Yang
The 'stdout-path' is missing after dts sync.
Fixes: 167efc2c7a ("arm64: dts: rk3399: Sync v5.7-rc1 from Linux")

Signed-off-by: Kever Yang 
---

 arch/arm/dts/rk3399-evb-u-boot.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/dts/rk3399-evb-u-boot.dtsi 
b/arch/arm/dts/rk3399-evb-u-boot.dtsi
index c42bd2856f..1be54feacc 100644
--- a/arch/arm/dts/rk3399-evb-u-boot.dtsi
+++ b/arch/arm/dts/rk3399-evb-u-boot.dtsi
@@ -8,6 +8,7 @@
 
 / {
chosen {
+   stdout-path = "serial2:150n8";
u-boot,spl-boot-order = "same-as-spl", &sdhci, &sdmmc;
};
 };
-- 
2.17.1





[PATCH] test: add dependency for UT_LOG

2020-06-06 Thread Kever Yang
The callback of do_ut_log() is defined in test/log/test-main.c
which is depend on CONFIG_LOG.

Signed-off-by: Kever Yang 
---

 test/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/test/Kconfig b/test/Kconfig
index 9b2f84b551..8c32fa5cbd 100644
--- a/test/Kconfig
+++ b/test/Kconfig
@@ -42,6 +42,7 @@ endif
 
 config UT_LOG
bool "Unit tests for logging functions"
+   depends on LOG
depends on UNIT_TEST
default y
help
-- 
2.17.1





Re: [PATCH] patman: use new --u-boot option with checkpath.pl

2020-06-06 Thread Simon Glass
Hi Daniel,

On Sat, 6 Jun 2020 at 20:02, Simon Glass  wrote:
>
> Hi Daniel,
>
> On Sat, 6 Jun 2020 at 15:31, Daniel Schwierzeck
>  wrote:
> >
> > checkpatch.pl now supports a --u-boot option for U-Boot specific
> > checks. Use that in patman to check the patch series.
> >
> > Signed-off-by: Daniel Schwierzeck 
> >
> > ---
> >
> >  tools/patman/checkpatch.py | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/patman/checkpatch.py b/tools/patman/checkpatch.py
> > index 795b519314..7f507154b8 100644
> > --- a/tools/patman/checkpatch.py
> > +++ b/tools/patman/checkpatch.py
> > @@ -64,7 +64,7 @@ def CheckPatch(fname, verbose=False):
> >  result.problems = []
> >  chk = FindCheckPatch()
> >  item = {}
> > -result.stdout = command.Output(chk, '--no-tree', fname,
> > +result.stdout = command.Output(chk, '--no-tree', '--u-boot', fname,
> > raise_on_error=False)
>
> Can we make this conditional on something? Maybe detecting a U-Boot
> tree? Patman is used for linux and Zephyr, for example.

Actually I just remembered that I was hoping we could just adjust the
.checkpatch.conf file in the U-Boot directory to add the option. Would
that work?

>
> >  #pipe = subprocess.Popen(cmd, stdout=subprocess.PIPE)
> >  #stdout, stderr = pipe.communicate()
> > --
> > 2.27.0
> >

Regards,
Simon


Re: [PATCH] patman: use new --u-boot option with checkpath.pl

2020-06-06 Thread Simon Glass
Hi Daniel,

On Sat, 6 Jun 2020 at 15:31, Daniel Schwierzeck
 wrote:
>
> checkpatch.pl now supports a --u-boot option for U-Boot specific
> checks. Use that in patman to check the patch series.
>
> Signed-off-by: Daniel Schwierzeck 
>
> ---
>
>  tools/patman/checkpatch.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/patman/checkpatch.py b/tools/patman/checkpatch.py
> index 795b519314..7f507154b8 100644
> --- a/tools/patman/checkpatch.py
> +++ b/tools/patman/checkpatch.py
> @@ -64,7 +64,7 @@ def CheckPatch(fname, verbose=False):
>  result.problems = []
>  chk = FindCheckPatch()
>  item = {}
> -result.stdout = command.Output(chk, '--no-tree', fname,
> +result.stdout = command.Output(chk, '--no-tree', '--u-boot', fname,
> raise_on_error=False)

Can we make this conditional on something? Maybe detecting a U-Boot
tree? Patman is used for linux and Zephyr, for example.

>  #pipe = subprocess.Popen(cmd, stdout=subprocess.PIPE)
>  #stdout, stderr = pipe.communicate()
> --
> 2.27.0
>

Regards,
Simon


Re: [PATCH 1/5] arm: enable allocate-on-read for LPAE's DCACHE_WRITEBACK

2020-06-06 Thread Heinrich Schuchardt
On 6/7/20 1:17 AM, Ard Biesheuvel wrote:
> On Sat, 6 Jun 2020 at 22:14, Heinrich Schuchardt  wrote:
>>
>> On 6/6/20 7:15 PM, Ard Biesheuvel wrote:
>>> The LPAE version of DCACHE_WRITEBACK is currently defined as no-allocate
>>> for both reads and writes, which deviates from the non-LPAE definition,
>>> and mostly defeats the purpose of enabling the caches in the first place.
>>>
>>> So align LPAE with !LPAE, and enable allocate-on-read.
>>
>> Hello Ard,
>>
>> thanks for analyzing why booting Linux on QEMU fails in some scenarios.
>>
>> Do you know where in U-Boot is the value for !LPAE is defined?
>>
>
> Non-LPAE ARMV7A has (in arch/arm/include/asm/system.h)
>
> DCACHE_WRITETHROUGH = DCACHE_OFF | TTB_SECT_C_MASK,
> DCACHE_WRITEBACK = DCACHE_WRITETHROUGH | TTB_SECT_B_MASK,
> DCACHE_WRITEALLOC = DCACHE_WRITEBACK | TTB_SECT_TEX(1),
>
> and so DCACHE_WRITEBACK has the C and B bits set in the block
> descriptor, and the TEX field set to 0x0
>
> G5.7.2 in the ARM ARM (DDI0487E.a) describes this as
>
> Outer and Inner Write-Back, Read-Allocate
> No Write-Allocate
>
> DCACHE_WRITEALLOC has the C and B bits set in the block descriptor,
> and the TEX field set to 0x1, which is described as
>
> Outer and Inner Write-Back, Read-Allocate Write-Allocate
>
>>>
>>> Signed-off-by: Ard Biesheuvel 
>>> ---
>>>  arch/arm/include/asm/system.h | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
>>> index 7a40b56acdca..21b26557d28b 100644
>>> --- a/arch/arm/include/asm/system.h
>>> +++ b/arch/arm/include/asm/system.h
>>> @@ -445,7 +445,7 @@ static inline void set_dacr(unsigned int val)
>>>   * Memory types
>>>   */
>>
>> To me the lines below look like black magic.
>>
>> In the comment above the definition, please, add a reference explaining
>> where the values are defined and a comment explaining why the actual
>> values are chosen.
>>
>> Maybe this could be a starting point for the description:
>>
>> "This constant is used define memory attribute encodings in a
>> Long-descriptor format translation table entry for stage 1 translations.
>> It is used to set the Memory Attribute Indirection Registers MAIR and
>> HMAIR. For details see [1,2].
>>
>> [1] MAIR0, Memory Attribute Indirection Register 0
>>
>> https://developer.arm.com/docs/ddi0595/b/aarch32-system-registers/mair0/a/DDI0568A_b_armv8_r_supplement.pdf
>> [2] HMAIR0, Hyp Memory Attribute Indirection Register 0
>>
>> https://developer.arm.com/docs/ddi0595/b/aarch32-system-registers/hmair0 "
>>
>
> Better refer to the ARM ARM for the A profile here (not R). [DDI0487E.a]
>
> So the memory types are indexed: four fields of MAIR are populated
> with the four chosen memory types:
>
> [0] Device-nGnrnE
> [1] Outer and Inner Write-Through, Read-Allocate No Write-Allocate
> [2] Outer and Inner Write-Back, Read-Allocate No Write-Allocate
> [3] Outer and Inner Write-Back, Read-Allocate Write-Allocate
>
> and the enum just selects one of these fields:
>
> DCACHE_OFF = TTB_SECT | TTB_SECT_MAIR(0) | TTB_SECT_XN_MASK,
> DCACHE_WRITETHROUGH = TTB_SECT | TTB_SECT_MAIR(1),
> DCACHE_WRITEBACK = TTB_SECT | TTB_SECT_MAIR(2),
> DCACHE_WRITEALLOC = TTB_SECT | TTB_SECT_MAIR(3),
>
> BTW it seems DCACHE_WRITETHROUGH is also incorrect: this should be
> 0xaa for read-allocate as well.

This description would give a good starting point for other developers
when supplied as a comment for MEMORY_ATTRIBUTES.

Best regards

Heinrich

>
>
>>>  #define MEMORY_ATTRIBUTES((0x00 << (0 * 8)) | (0x88 << (1 * 8)) | \
>>> -  (0xcc << (2 * 8)) | (0xff << (3 * 8)))
>>> +  (0xee << (2 * 8)) | (0xff << (3 * 8)))
>>>
>>>  /* options available for data cache on each page */
>>>  enum dcache_option {
>>>
>>



Re: [PATCH 1/5] arm: enable allocate-on-read for LPAE's DCACHE_WRITEBACK

2020-06-06 Thread Ard Biesheuvel
On Sat, 6 Jun 2020 at 22:14, Heinrich Schuchardt  wrote:
>
> On 6/6/20 7:15 PM, Ard Biesheuvel wrote:
> > The LPAE version of DCACHE_WRITEBACK is currently defined as no-allocate
> > for both reads and writes, which deviates from the non-LPAE definition,
> > and mostly defeats the purpose of enabling the caches in the first place.
> >
> > So align LPAE with !LPAE, and enable allocate-on-read.
>
> Hello Ard,
>
> thanks for analyzing why booting Linux on QEMU fails in some scenarios.
>
> Do you know where in U-Boot is the value for !LPAE is defined?
>

Non-LPAE ARMV7A has (in arch/arm/include/asm/system.h)

DCACHE_WRITETHROUGH = DCACHE_OFF | TTB_SECT_C_MASK,
DCACHE_WRITEBACK = DCACHE_WRITETHROUGH | TTB_SECT_B_MASK,
DCACHE_WRITEALLOC = DCACHE_WRITEBACK | TTB_SECT_TEX(1),

and so DCACHE_WRITEBACK has the C and B bits set in the block
descriptor, and the TEX field set to 0x0

G5.7.2 in the ARM ARM (DDI0487E.a) describes this as

Outer and Inner Write-Back, Read-Allocate
No Write-Allocate

DCACHE_WRITEALLOC has the C and B bits set in the block descriptor,
and the TEX field set to 0x1, which is described as

Outer and Inner Write-Back, Read-Allocate Write-Allocate

> >
> > Signed-off-by: Ard Biesheuvel 
> > ---
> >  arch/arm/include/asm/system.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
> > index 7a40b56acdca..21b26557d28b 100644
> > --- a/arch/arm/include/asm/system.h
> > +++ b/arch/arm/include/asm/system.h
> > @@ -445,7 +445,7 @@ static inline void set_dacr(unsigned int val)
> >   * Memory types
> >   */
>
> To me the lines below look like black magic.
>
> In the comment above the definition, please, add a reference explaining
> where the values are defined and a comment explaining why the actual
> values are chosen.
>
> Maybe this could be a starting point for the description:
>
> "This constant is used define memory attribute encodings in a
> Long-descriptor format translation table entry for stage 1 translations.
> It is used to set the Memory Attribute Indirection Registers MAIR and
> HMAIR. For details see [1,2].
>
> [1] MAIR0, Memory Attribute Indirection Register 0
>
> https://developer.arm.com/docs/ddi0595/b/aarch32-system-registers/mair0/a/DDI0568A_b_armv8_r_supplement.pdf
> [2] HMAIR0, Hyp Memory Attribute Indirection Register 0
>
> https://developer.arm.com/docs/ddi0595/b/aarch32-system-registers/hmair0 "
>

Better refer to the ARM ARM for the A profile here (not R). [DDI0487E.a]

So the memory types are indexed: four fields of MAIR are populated
with the four chosen memory types:

[0] Device-nGnrnE
[1] Outer and Inner Write-Through, Read-Allocate No Write-Allocate
[2] Outer and Inner Write-Back, Read-Allocate No Write-Allocate
[3] Outer and Inner Write-Back, Read-Allocate Write-Allocate

and the enum just selects one of these fields:

DCACHE_OFF = TTB_SECT | TTB_SECT_MAIR(0) | TTB_SECT_XN_MASK,
DCACHE_WRITETHROUGH = TTB_SECT | TTB_SECT_MAIR(1),
DCACHE_WRITEBACK = TTB_SECT | TTB_SECT_MAIR(2),
DCACHE_WRITEALLOC = TTB_SECT | TTB_SECT_MAIR(3),

BTW it seems DCACHE_WRITETHROUGH is also incorrect: this should be
0xaa for read-allocate as well.


> >  #define MEMORY_ATTRIBUTES((0x00 << (0 * 8)) | (0x88 << (1 * 8)) | \
> > -  (0xcc << (2 * 8)) | (0xff << (3 * 8)))
> > +  (0xee << (2 * 8)) | (0xff << (3 * 8)))
> >
> >  /* options available for data cache on each page */
> >  enum dcache_option {
> >
>


Re: [U-Boot] [PATCH v2 04/16] arm: socfpga: stratix10: Add pinmux support for Stratix10 SoC

2020-06-06 Thread Heinrich Schuchardt
On 5/18/18 4:05 PM, Ley Foon Tan wrote:
> Add pinmux driver support for Stratix SoC
>
> Signed-off-by: Chin Liang See 
> Signed-off-by: Ley Foon Tan 
> ---
>  arch/arm/mach-socfpga/Makefile |2 +
>  .../arm/mach-socfpga/include/mach/system_manager.h |5 +-
>  .../mach-socfpga/include/mach/system_manager_s10.h |  176 
> 
>  arch/arm/mach-socfpga/system_manager_s10.c |   91 ++
>  arch/arm/mach-socfpga/wrap_pinmux_config_s10.c |   56 ++
>  5 files changed, 329 insertions(+), 1 deletions(-)
>  create mode 100644 arch/arm/mach-socfpga/include/mach/system_manager_s10.h
>  create mode 100644 arch/arm/mach-socfpga/system_manager_s10.c
>  create mode 100644 arch/arm/mach-socfpga/wrap_pinmux_config_s10.c
>
> diff --git a/arch/arm/mach-socfpga/Makefile b/arch/arm/mach-socfpga/Makefile
> index 69bdb84..61f5778 100644
> --- a/arch/arm/mach-socfpga/Makefile
> +++ b/arch/arm/mach-socfpga/Makefile
> @@ -31,6 +31,8 @@ endif
>  ifdef CONFIG_TARGET_SOCFPGA_STRATIX10
>  obj-y+= clock_manager_s10.o
>  obj-y+= reset_manager_s10.o
> +obj-y+= system_manager_s10.o
> +obj-y+= wrap_pinmux_config_s10.o
>  obj-y+= wrap_pll_config_s10.o
>  endif
>  ifdef CONFIG_SPL_BUILD
> diff --git a/arch/arm/mach-socfpga/include/mach/system_manager.h 
> b/arch/arm/mach-socfpga/include/mach/system_manager.h
> index fbe2a8b..7e76df7 100644
> --- a/arch/arm/mach-socfpga/include/mach/system_manager.h
> +++ b/arch/arm/mach-socfpga/include/mach/system_manager.h
> @@ -6,6 +6,9 @@
>  #ifndef _SYSTEM_MANAGER_H_
>  #define _SYSTEM_MANAGER_H_
>
> +#if defined(CONFIG_TARGET_SOCFPGA_STRATIX10)
> +#include 
> +#else
>  #define SYSMGR_ROMCODEGRP_CTRL_WARMRSTCFGPINMUX  BIT(0)
>  #define SYSMGR_ROMCODEGRP_CTRL_WARMRSTCFGIO  BIT(1)
>  #define SYSMGR_ECC_OCRAM_EN  BIT(0)
> @@ -88,5 +91,5 @@
>
>  #define SYSMGR_GET_BOOTINFO_BSEL(bsel)   \
>   (((bsel) >> SYSMGR_BOOTINFO_BSEL_SHIFT) & 7)
> -
> +#endif
>  #endif /* _SYSTEM_MANAGER_H_ */
> diff --git a/arch/arm/mach-socfpga/include/mach/system_manager_s10.h 
> b/arch/arm/mach-socfpga/include/mach/system_manager_s10.h
> new file mode 100644
> index 000..813dff2
> --- /dev/null
> +++ b/arch/arm/mach-socfpga/include/mach/system_manager_s10.h
> @@ -0,0 +1,176 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + *
> + * Copyright (C) 2016-2018 Intel Corporation 
> + *
> + */
> +
> +#ifndef  _SYSTEM_MANAGER_S10_
> +#define  _SYSTEM_MANAGER_S10_
> +
> +void sysmgr_pinmux_init(void);
> +void populate_sysmgr_fpgaintf_module(void);
> +void populate_sysmgr_pinmux(void);
> +void sysmgr_pinmux_table_sel(const u32 **table, unsigned int *table_len);
> +void sysmgr_pinmux_table_ctrl(const u32 **table, unsigned int *table_len);
> +void sysmgr_pinmux_table_fpga(const u32 **table, unsigned int *table_len);
> +void sysmgr_pinmux_table_delay(const u32 **table, unsigned int *table_len);
> +
> +struct socfpga_system_manager {
> + /* System Manager Module */
> + u32 siliconid1; /* 0x00 */
> + u32 siliconid2;
> + u32 wddbg;
> + u32 _pad_0xc;
> + u32 mpu_status; /* 0x10 */
> + u32 mpu_ace;
> + u32 _pad_0x18_0x1c[2];
> + u32 dma;/* 0x20 */
> + u32 dma_periph;
> + /* SDMMC Controller Group */
> + u32 sdmmcgrp_ctrl;
> + u32 sdmmcgrp_l3master;
> + /* NAND Flash Controller Register Group */
> + u32 nandgrp_bootstrap;  /* 0x30 */
> + u32 nandgrp_l3master;
> + /* USB Controller Group */
> + u32 usb0_l3master;
> + u32 usb1_l3master;
> + /* EMAC Group */
> + u32 emac_gbl;   /* 0x40 */
> + u32 emac0;
> + u32 emac1;
> + u32 emac2;
> + u32 emac0_ace;  /* 0x50 */
> + u32 emac1_ace;
> + u32 emac2_ace;
> + u32 nand_axuser;
> + u32 _pad_0x60_0x64[2];  /* 0x60 */
> + /* FPGA interface Group */
> + u32 fpgaintf_en_1;
> + u32 fpgaintf_en_2;
> + u32 fpgaintf_en_3;  /* 0x70 */
> + u32 dma_l3master;
> + u32 etr_l3master;
> + u32 _pad_0x7c;
> + u32 sec_ctrl_slt;   /* 0x80 */
> + u32 osc_trim;
> + u32 _pad_0x88_0x8c[2];
> + /* ECC Group */
> + u32 ecc_intmask_value;  /* 0x90 */
> + u32 ecc_intmask_set;
> + u32 ecc_intmask_clr;
> + u32 ecc_intstatus_serr;
> + u32 ecc_intstatus_derr; /* 0xa0 */
> + u32 _pad_0xa4_0xac[3];
> + u32 noc_addr_remap; /* 0xb0 */
> + u32 hmc_clk;
> + u32 io_pa_ctrl;
> + u32 _pad_0xbc;
> + /* NOC Group */
> + u32 noc_timeout;/* 0xc0 */
> + u32 noc_idlereq_set;
> + u32 noc_idlereq_clr;
> + u32

Re: [PATCH 5/5] arm: qemu: override flash accessors to use virtualizable instructions

2020-06-06 Thread Ard Biesheuvel
On Sat, 6 Jun 2020 at 23:08, Heinrich Schuchardt  wrote:
>
> On 6/6/20 7:15 PM, Ard Biesheuvel wrote:
> > Some instructions in the ARM ISA have multiple output registers, such
> > as ldrd/ldp (load pair), where two registers are loaded from memory,
> > but also ldr with indexing, where the memory base register is incremented
> > as well when the value is loaded to the destination register.
> >
> > MMIO emulation under KVM is based on using the architecturally defined
> > syndrome information that is provided when an exception is taken to the
> > hypervisor. This syndrome information describes whether the instruction
> > that triggered the exception is a load or a store, what the faulting
> > address was, and which register was the destination register.
> >
> > This syndrome information can only describe one destination register, and
> > when the trapping instruction is one with multiple outputs, KVM throws an
> > error like
> >
> >   kvm [615929]: Data abort outside memslots with no valid syndrome info
> >
> > on the host and kills the QEMU process with the following error:
> >
> >   U-Boot 2020.07-rc3-00208-g88bd5b179360-dirty (Jun 06 2020 - 11:59:22 
> > +0200)
> >
> >   DRAM:  1 GiB
> >   Flash: error: kvm run failed Function not implemented
> >   R00=0001 R01=0040 R02=7ee0ce20 R03=
> >   R04=7ffd9eec R05=0004 R06=7ffda3f8 R07=0055
> >   R08=7ffd9eec R09=7ef0ded0 R10=7ee0ce20 R11=
> >   R12=0004 R13=7ee0cdf8 R14= R15=7ff72d08
> >   PSR=21d3 --C- A svc32
> >   QEMU: Terminated
> >
> > This means that, in order to run U-Boot in QEMU under KVM, we need to
> > avoid such instructions when accessing emulated devices. For the flash
> > in particular, which is a hybrid between a ROM (backed by a memslot)
> > when in array mode, and an emulated MMIO device (when in write mode),
> > we need to take care to only use instructions that KVM can deal with
> > when they trap.
> >
> > So override the flash accessors that are used when running on QEMU.
> > Note that the write accessors are included for completeness, but the
> > read accessors are the ones that need this special care.
> >
> > Signed-off-by: Ard Biesheuvel 
> > ---
> >  board/emulation/qemu-arm/qemu-arm.c | 55 
> >  include/configs/qemu-arm.h  |  1 +
> >  2 files changed, 56 insertions(+)
> >
> > diff --git a/board/emulation/qemu-arm/qemu-arm.c 
> > b/board/emulation/qemu-arm/qemu-arm.c
> > index 1b0d543b93c1..32e18fd8b985 100644
> > --- a/board/emulation/qemu-arm/qemu-arm.c
> > +++ b/board/emulation/qemu-arm/qemu-arm.c
> > @@ -142,3 +142,58 @@ efi_status_t platform_get_rng_device(struct udevice 
> > **dev)
> >   return EFI_SUCCESS;
> >  }
> >  #endif /* CONFIG_EFI_RNG_PROTOCOL */
> > +
> > +#ifdef CONFIG_ARM64
> > +#define __W  "w"
> > +#else
> > +#define __W
> > +#endif
> > +
> > +void flash_write8(u8 value, void *addr)
> > +{
> > + asm("strb %" __W "1, %0" : "=m"(*(u8 *)addr) : "r"(value));
> > +}
> > +
> > +void flash_write16(u16 value, void *addr)
> > +{
> > + asm("strh %" __W "1, %0" : "=m"(*(u16 *)addr) : "r"(value));
> > +}
> > +
> > +void flash_write32(u32 value, void *addr)
> > +{
> > + asm("str %" __W "1, %0" : "=m"(*(u32 *)addr) : "r"(value));
> > +}
> > +
> > +void flash_write64(u64 value, void *addr)
> > +{
> > + BUG(); /* FLASH_CFI_64BIT is not implemented by QEMU */
>
> Why should this BUG() on aarch64?

QEMU's CFI emulation does not implement 8 byte width, so there is no
point in implementing this accessor, as it will never be called
anyway.

The BUG() is there to ensure that *if* QEMU ever does get support for
8 byte wide CFI flash, we notice immediately, rather than having to
debug weird failures.

> Why is panicking in U-Boot better than crashing in QEMU?

Because U-boot crashes in the guest, while QEMU crashes in the host.

> Why can't this be realized as two 32bit writes?
>

It could be but there is no point: QEMU will never exercise this code
path anyway.

> This seems to be wrong:
> drivers/mtd/cfi_flash.c:179:
> /* No architectures currently implement __raw_readq() */
>
> I find definitions for all architectures.
>
> So shouldn't you correct cfi_flash.c and override __arch_getq() and
> __arch_putq() instead?
>

If anyone wants to fix 8 byte CFI, they are welcome to do so, but it
is a separate issue.

> > +}
> > +
> > +u8 flash_read8(void *addr)
> > +{
> > + u8 ret;
> > +
> > + asm("ldrb %" __W "0, %1" : "=r"(ret) : "m"(*(u8 *)addr));
> > + return ret;
> > +}
> > +
> > +u16 flash_read16(void *addr)
> > +{
> > + u16 ret;
> > +
> > + asm("ldrh %" __W "0, %1" : "=r"(ret) : "m"(*(u16 *)addr));
> > + return ret;
> > +}
> > +
> > +u32 flash_read32(void *addr)
> > +{
> > + u32 ret;
> > +
> > + asm("ldr %" __W "0, %1" : "=r"(ret) : "m"(*(u32 *)addr));
> > + return ret;
> > +}
> > +
> > +u64 flash_read64(void *addr)
> > +{
> > + BUG(); /* FLASH_CFI_64BIT is not implemented by QEMU */
>
> Same here.
>
> Best

Out of bounds access in arch/arm/cpu/armv7/iproc-common/armpll.c

2020-06-06 Thread Heinrich Schuchardt
Hello Scott,

CppCheck gives an error:

[arch/arm/cpu/armv7/iproc-common/armpll.c:137]: (error) Array
'armpll_clk_tab[13]' accessed at index 17, which is out of bounds.

In the loop at the start of armpll_config() i is determined. The maximum
value of i after checking status in line 53 is 12.

In line 137 your code accesses

armpll_clk_tab[i+4].freqid

The patch where you introduced the code dates from 2014. But hopefully
you still remember what you intended to do in that line.

Best regards

Heinrich


[PATCH] patman: use new --u-boot option with checkpath.pl

2020-06-06 Thread Daniel Schwierzeck
checkpatch.pl now supports a --u-boot option for U-Boot specific
checks. Use that in patman to check the patch series.

Signed-off-by: Daniel Schwierzeck 

---

 tools/patman/checkpatch.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/patman/checkpatch.py b/tools/patman/checkpatch.py
index 795b519314..7f507154b8 100644
--- a/tools/patman/checkpatch.py
+++ b/tools/patman/checkpatch.py
@@ -64,7 +64,7 @@ def CheckPatch(fname, verbose=False):
 result.problems = []
 chk = FindCheckPatch()
 item = {}
-result.stdout = command.Output(chk, '--no-tree', fname,
+result.stdout = command.Output(chk, '--no-tree', '--u-boot', fname,
raise_on_error=False)
 #pipe = subprocess.Popen(cmd, stdout=subprocess.PIPE)
 #stdout, stderr = pipe.communicate()
-- 
2.27.0



[PATCH 1/1] mtd: cfi_flash: use __raw_writeq(), __raw_readq()

2020-06-06 Thread Heinrich Schuchardt
Functions __raw_writeq(), __raw_readq() are available for all
architectures. So let's use them.

Signed-off-by: Heinrich Schuchardt 
---
 drivers/mtd/cfi_flash.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
index b7289ba539..e31e07ca80 100644
--- a/drivers/mtd/cfi_flash.c
+++ b/drivers/mtd/cfi_flash.c
@@ -155,8 +155,7 @@ __maybe_weak void flash_write32(u32 value, void *addr)

 __maybe_weak void flash_write64(u64 value, void *addr)
 {
-   /* No architectures currently implement __raw_writeq() */
-   *(volatile u64 *)addr = value;
+   __raw_writeq(value, addr);
 }

 __maybe_weak u8 flash_read8(void *addr)
@@ -176,8 +175,7 @@ __maybe_weak u32 flash_read32(void *addr)

 __maybe_weak u64 flash_read64(void *addr)
 {
-   /* No architectures currently implement __raw_readq() */
-   return *(volatile u64 *)addr;
+   return __raw_readq(addr);
 }

 /*---
--
2.26.2



Re: [PATCH 5/5] arm: qemu: override flash accessors to use virtualizable instructions

2020-06-06 Thread Heinrich Schuchardt
On 6/6/20 7:15 PM, Ard Biesheuvel wrote:
> Some instructions in the ARM ISA have multiple output registers, such
> as ldrd/ldp (load pair), where two registers are loaded from memory,
> but also ldr with indexing, where the memory base register is incremented
> as well when the value is loaded to the destination register.
>
> MMIO emulation under KVM is based on using the architecturally defined
> syndrome information that is provided when an exception is taken to the
> hypervisor. This syndrome information describes whether the instruction
> that triggered the exception is a load or a store, what the faulting
> address was, and which register was the destination register.
>
> This syndrome information can only describe one destination register, and
> when the trapping instruction is one with multiple outputs, KVM throws an
> error like
>
>   kvm [615929]: Data abort outside memslots with no valid syndrome info
>
> on the host and kills the QEMU process with the following error:
>
>   U-Boot 2020.07-rc3-00208-g88bd5b179360-dirty (Jun 06 2020 - 11:59:22 +0200)
>
>   DRAM:  1 GiB
>   Flash: error: kvm run failed Function not implemented
>   R00=0001 R01=0040 R02=7ee0ce20 R03=
>   R04=7ffd9eec R05=0004 R06=7ffda3f8 R07=0055
>   R08=7ffd9eec R09=7ef0ded0 R10=7ee0ce20 R11=
>   R12=0004 R13=7ee0cdf8 R14= R15=7ff72d08
>   PSR=21d3 --C- A svc32
>   QEMU: Terminated
>
> This means that, in order to run U-Boot in QEMU under KVM, we need to
> avoid such instructions when accessing emulated devices. For the flash
> in particular, which is a hybrid between a ROM (backed by a memslot)
> when in array mode, and an emulated MMIO device (when in write mode),
> we need to take care to only use instructions that KVM can deal with
> when they trap.
>
> So override the flash accessors that are used when running on QEMU.
> Note that the write accessors are included for completeness, but the
> read accessors are the ones that need this special care.
>
> Signed-off-by: Ard Biesheuvel 
> ---
>  board/emulation/qemu-arm/qemu-arm.c | 55 
>  include/configs/qemu-arm.h  |  1 +
>  2 files changed, 56 insertions(+)
>
> diff --git a/board/emulation/qemu-arm/qemu-arm.c 
> b/board/emulation/qemu-arm/qemu-arm.c
> index 1b0d543b93c1..32e18fd8b985 100644
> --- a/board/emulation/qemu-arm/qemu-arm.c
> +++ b/board/emulation/qemu-arm/qemu-arm.c
> @@ -142,3 +142,58 @@ efi_status_t platform_get_rng_device(struct udevice 
> **dev)
>   return EFI_SUCCESS;
>  }
>  #endif /* CONFIG_EFI_RNG_PROTOCOL */
> +
> +#ifdef CONFIG_ARM64
> +#define __W  "w"
> +#else
> +#define __W
> +#endif
> +
> +void flash_write8(u8 value, void *addr)
> +{
> + asm("strb %" __W "1, %0" : "=m"(*(u8 *)addr) : "r"(value));
> +}
> +
> +void flash_write16(u16 value, void *addr)
> +{
> + asm("strh %" __W "1, %0" : "=m"(*(u16 *)addr) : "r"(value));
> +}
> +
> +void flash_write32(u32 value, void *addr)
> +{
> + asm("str %" __W "1, %0" : "=m"(*(u32 *)addr) : "r"(value));
> +}
> +
> +void flash_write64(u64 value, void *addr)
> +{
> + BUG(); /* FLASH_CFI_64BIT is not implemented by QEMU */

Why should this BUG() on aarch64?
Why is panicking in U-Boot better than crashing in QEMU?
Why can't this be realized as two 32bit writes?

This seems to be wrong:
drivers/mtd/cfi_flash.c:179:
/* No architectures currently implement __raw_readq() */

I find definitions for all architectures.

So shouldn't you correct cfi_flash.c and override __arch_getq() and
__arch_putq() instead?

> +}
> +
> +u8 flash_read8(void *addr)
> +{
> + u8 ret;
> +
> + asm("ldrb %" __W "0, %1" : "=r"(ret) : "m"(*(u8 *)addr));
> + return ret;
> +}
> +
> +u16 flash_read16(void *addr)
> +{
> + u16 ret;
> +
> + asm("ldrh %" __W "0, %1" : "=r"(ret) : "m"(*(u16 *)addr));
> + return ret;
> +}
> +
> +u32 flash_read32(void *addr)
> +{
> + u32 ret;
> +
> + asm("ldr %" __W "0, %1" : "=r"(ret) : "m"(*(u32 *)addr));
> + return ret;
> +}
> +
> +u64 flash_read64(void *addr)
> +{
> + BUG(); /* FLASH_CFI_64BIT is not implemented by QEMU */

Same here.

Best regards

Heinrich

> +}
> diff --git a/include/configs/qemu-arm.h b/include/configs/qemu-arm.h
> index 1ef75a87836b..bc8b7c5c1238 100644
> --- a/include/configs/qemu-arm.h
> +++ b/include/configs/qemu-arm.h
> @@ -53,5 +53,6 @@
>  #define CONFIG_SYS_MAX_FLASH_BANKS   2
>  #endif
>  #define CONFIG_SYS_MAX_FLASH_SECT256 /* Sector: 256K, Bank: 64M */
> +#define CONFIG_CFI_FLASH_USE_WEAK_ACCESSORS
>
>  #endif /* __CONFIG_H */
>



[PATCH 5/6] .travis.yml: add Qemu tests for MIPS Malta board

2020-06-06 Thread Daniel Schwierzeck
Add Qemu tests for the MIPS Malta machine as a replacement for
the deprecated generic MIPS machine.

Signed-off-by: Daniel Schwierzeck 
---

 .travis.yml | 28 
 1 file changed, 28 insertions(+)

diff --git a/.travis.yml b/.travis.yml
index bb02b6d816..a042aa2c7d 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -573,6 +573,34 @@ matrix:
   TEST_PY_TEST_SPEC="not sleep"
   QEMU_TARGET="mips64el-softmmu"
   TOOLCHAIN="mips"
+- name: "test/py qemu-malta"
+  env:
+- TEST_PY_BD="malta"
+  TEST_PY_TEST_SPEC="not sleep and not efi"
+  TEST_PY_ID="--id qemu"
+  QEMU_TARGET="mips-softmmu"
+  TOOLCHAIN="mips"
+- name: "test/py qemu-maltael"
+  env:
+- TEST_PY_BD="maltael"
+  TEST_PY_TEST_SPEC="not sleep and not efi"
+  TEST_PY_ID="--id qemu"
+  QEMU_TARGET="mipsel-softmmu"
+  TOOLCHAIN="mips"
+- name: "test/py qemu-malta64"
+  env:
+- TEST_PY_BD="malta64"
+  TEST_PY_TEST_SPEC="not sleep and not efi"
+  TEST_PY_ID="--id qemu"
+  QEMU_TARGET="mips64-softmmu"
+  TOOLCHAIN="mips"
+- name: "test/py qemu-malta64el"
+  env:
+- TEST_PY_BD="malta64el"
+  TEST_PY_TEST_SPEC="not sleep and not efi"
+  TEST_PY_ID="--id qemu"
+  QEMU_TARGET="mips64el-softmmu"
+  TOOLCHAIN="mips"
 - name: "test/py qemu-ppce500"
   env:
 - TEST_PY_BD="qemu-ppce500"
-- 
2.27.0



[PATCH 6/6] .azure-pipelines.yml: add Qemu tests for MIPS Malta board

2020-06-06 Thread Daniel Schwierzeck
Add Qemu tests for the MIPS Malta machine as a replacement for
the deprecated generic MIPS machine.

Signed-off-by: Daniel Schwierzeck 

---

 .azure-pipelines.yml | 16 
 1 file changed, 16 insertions(+)

diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml
index 636500d6ce..718f458fc0 100644
--- a/.azure-pipelines.yml
+++ b/.azure-pipelines.yml
@@ -226,6 +226,22 @@ jobs:
 qemu_mips64el:
   TEST_PY_BD: "qemu_mips64el"
   TEST_PY_TEST_SPEC: "not sleep"
+qemu_malta:
+  TEST_PY_BD: "malta"
+  TEST_PY_ID: "--id qemu"
+  TEST_PY_TEST_SPEC: "not sleep and not efi"
+qemu_maltael:
+  TEST_PY_BD: "maltael"
+  TEST_PY_ID: "--id qemu"
+  TEST_PY_TEST_SPEC: "not sleep and not efi"
+qemu_malta64:
+  TEST_PY_BD: "malta64"
+  TEST_PY_ID: "--id qemu"
+  TEST_PY_TEST_SPEC: "not sleep and not efi"
+qemu_malta64el:
+  TEST_PY_BD: "malta64el"
+  TEST_PY_ID: "--id qemu"
+  TEST_PY_TEST_SPEC: "not sleep and not efi"
 qemu_ppce500:
   TEST_PY_BD: "qemu-ppce500"
   TEST_PY_TEST_SPEC: "not sleep"
-- 
2.27.0



[PATCH 4/6] .gitlab-ci.yml: add Qemu tests for MIPS Malta board

2020-06-06 Thread Daniel Schwierzeck
Add Qemu tests for the MIPS Malta machine as a replacement for
the deprecated generic MIPS machine.

Signed-off-by: Daniel Schwierzeck 
---

 .gitlab-ci.yml | 32 
 1 file changed, 32 insertions(+)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index badfcb4254..d1a92951fb 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -276,6 +276,38 @@ qemu_mips64el test.py:
 TEST_PY_TEST_SPEC: "not sleep"
   <<: *buildman_and_testpy_dfn
 
+qemu_malta test.py:
+  tags: [ 'all' ]
+  variables:
+TEST_PY_BD: "malta"
+TEST_PY_TEST_SPEC: "not sleep and not efi"
+TEST_PY_ID: "--id qemu"
+  <<: *buildman_and_testpy_dfn
+
+qemu_maltael test.py:
+  tags: [ 'all' ]
+  variables:
+TEST_PY_BD: "maltael"
+TEST_PY_TEST_SPEC: "not sleep and not efi"
+TEST_PY_ID: "--id qemu"
+  <<: *buildman_and_testpy_dfn
+
+qemu_malta64 test.py:
+  tags: [ 'all' ]
+  variables:
+TEST_PY_BD: "malta64"
+TEST_PY_TEST_SPEC: "not sleep and not efi"
+TEST_PY_ID: "--id qemu"
+  <<: *buildman_and_testpy_dfn
+
+qemu_malta64el test.py:
+  tags: [ 'all' ]
+  variables:
+TEST_PY_BD: "malta64el"
+TEST_PY_TEST_SPEC: "not sleep and not efi"
+TEST_PY_ID: "--id qemu"
+  <<: *buildman_and_testpy_dfn
+
 qemu-ppce500 test.py:
   tags: [ 'all' ]
   variables:
-- 
2.27.0



[PATCH 3/6] mips: malta: build u-boot-swap.bin

2020-06-06 Thread Daniel Schwierzeck
The Qemu Malta machine expects the firmware in Big-Endian byte order.
Therefore the Little-Endian variants of the Malta board needs to
be byte swapped.

Signed-off-by: Daniel Schwierzeck 
---

 configs/malta64el_defconfig | 1 +
 configs/maltael_defconfig   | 1 +
 2 files changed, 2 insertions(+)

diff --git a/configs/malta64el_defconfig b/configs/malta64el_defconfig
index a9efe7736e..a35ae86c55 100644
--- a/configs/malta64el_defconfig
+++ b/configs/malta64el_defconfig
@@ -3,6 +3,7 @@ CONFIG_SYS_TEXT_BASE=0xBE00
 CONFIG_ENV_SIZE=0x2
 CONFIG_ENV_SECT_SIZE=0x2
 CONFIG_TARGET_MALTA=y
+CONFIG_BUILD_TARGET="u-boot-swap.bin"
 CONFIG_SYS_LITTLE_ENDIAN=y
 CONFIG_CPU_MIPS64_R2=y
 CONFIG_MISC_INIT_R=y
diff --git a/configs/maltael_defconfig b/configs/maltael_defconfig
index 31c9ff6a77..b3f046c993 100644
--- a/configs/maltael_defconfig
+++ b/configs/maltael_defconfig
@@ -3,6 +3,7 @@ CONFIG_SYS_TEXT_BASE=0xBE00
 CONFIG_ENV_SIZE=0x2
 CONFIG_ENV_SECT_SIZE=0x2
 CONFIG_TARGET_MALTA=y
+CONFIG_BUILD_TARGET="u-boot-swap.bin"
 CONFIG_SYS_LITTLE_ENDIAN=y
 CONFIG_MISC_INIT_R=y
 CONFIG_BOARD_EARLY_INIT_F=y
-- 
2.27.0



[PATCH 2/6] Makefile: add rule to generate u-boot-swap.bin

2020-06-06 Thread Daniel Schwierzeck
This rule generates an u-boot binary file where the byte endianness
is swapped. This will be used by the MIPS Malta Little-Endian variants
to be able to boot with Qemu. The Qemu Malta Machine expects the
firmware in Big-Endian order.

Signed-off-by: Daniel Schwierzeck 
---

 Makefile | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/Makefile b/Makefile
index 3851dd9fa0..3ce511fcf0 100644
--- a/Makefile
+++ b/Makefile
@@ -1740,6 +1740,12 @@ u-boot-mtk.bin: u-boot.bin FORCE
$(call if_changed,mkimage)
 endif
 
+quiet_cmd_endian_swap = SWAP$@
+  cmd_endian_swap = $(srctree)/tools/endian-swap.py $< $@
+
+u-boot-swap.bin: u-boot.bin FORCE
+   $(call if_changed,endian_swap)
+
 ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(ARCH)/Makefile.postlink)
 
 # Rule to link u-boot
-- 
2.27.0



[PATCH 0/6] Add Qemu tests for MIPS Malta machine

2020-06-06 Thread Daniel Schwierzeck


The currently used qemu_mips target respectively the Qemu
generic MIPS machine is marked as deprecated by Qemu. One should
use the MIPS Malta machine. Also the qemu_mips target only exists
in U-Boot, the Linux support was removed centuries ago. Thus we
should deprecated and remove the qemu_mips target eventually.
The first step would be to move the MIPS specific Qemu CI tests to
the Malta targets which also covers all 32/64 bit and Little/Big
Endian variants.

Successful CI runs on Gitlab, Travis CI and Azure:

https://gitlab.denx.de/u-boot/custodians/u-boot-mips/pipelines/3557
https://travis-ci.org/github/danielschwierzeck/u-boot/builds/695247372
https://dev.azure.com/danielschwierzeck/u-boot/_build/results?buildId=4&view=results

Complementary pull request for uboot-test-hooks:

https://github.com/swarren/uboot-test-hooks/pull/30



Daniel Schwierzeck (6):
  tools: add script for byte endianness swapping
  Makefile: add rule to generate u-boot-swap.bin
  mips: malta: build u-boot-swap.bin
  .gitlab-ci.yml: add Qemu tests for MIPS Malta board
  .travis.yml: add Qemu tests for MIPS Malta board
  .azure-pipelines.yml: add Qemu tests for MIPS Malta board

 .azure-pipelines.yml| 16 +++
 .gitlab-ci.yml  | 32 +
 .travis.yml | 28 +++
 Makefile|  6 
 configs/malta64el_defconfig |  1 +
 configs/maltael_defconfig   |  1 +
 tools/endian-swap.py| 55 +
 7 files changed, 139 insertions(+)
 create mode 100755 tools/endian-swap.py

-- 
2.27.0



[PATCH 1/6] tools: add script for byte endianness swapping

2020-06-06 Thread Daniel Schwierzeck
This can be used to swap the byte endianness of a binary file
from Little-Endian to Big-Endian or vice-versa.

Signed-off-by: Daniel Schwierzeck 
---

 tools/endian-swap.py | 55 
 1 file changed, 55 insertions(+)
 create mode 100755 tools/endian-swap.py

diff --git a/tools/endian-swap.py b/tools/endian-swap.py
new file mode 100755
index 00..5990efa313
--- /dev/null
+++ b/tools/endian-swap.py
@@ -0,0 +1,55 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0+
+
+"""
+Simple tool to swap the byte endianness of a binary file.
+"""
+
+import argparse
+import io
+
+def parse_args():
+"""Parse command line arguments."""
+description = "Swap endianness of given input binary and write to output 
binary."
+
+parser = argparse.ArgumentParser(description=description)
+parser.add_argument("input_bin", type=str, help="input binary")
+parser.add_argument("output_bin", type=str, help="output binary")
+parser.add_argument("-c", action="store", dest="chunk_size", type=int,
+default=io.DEFAULT_BUFFER_SIZE, help="chunk size for reading")
+
+return parser.parse_args()
+
+def swap_chunk(chunk_orig):
+"""Swap byte endianness of the given chunk.
+
+Returns:
+swapped chunk
+"""
+chunk = bytearray(chunk_orig)
+
+# align to 4 bytes and pad with 0x0
+chunk_len = len(chunk)
+pad_len = chunk_len % 4
+if pad_len > 0:
+chunk += b'\x00' * (4 - pad_len)
+
+chunk[0::4], chunk[1::4], chunk[2::4], chunk[3::4] =\
+chunk[3::4], chunk[2::4], chunk[1::4], chunk[0::4]
+
+return chunk
+
+def main():
+args = parse_args()
+
+with open(args.input_bin, "rb") as input_bin:
+with open(args.output_bin, "wb") as output_bin:
+while True:
+chunk = bytearray(input_bin.read(args.chunk_size))
+if not chunk:
+break
+
+output_bin.write(swap_chunk(chunk))
+
+if __name__ == '__main__':
+main()
-- 
2.27.0



Re: [PATCH 4/5] arm: qemu: disable the EFI workaround for older GRUB

2020-06-06 Thread Heinrich Schuchardt
On 6/6/20 7:15 PM, Ard Biesheuvel wrote:
> The QEMU/mach-virt targeted port of u-boot currently only runs on
> QEMU under TCG emulation, which does not model the caches at all,
> and so no users can exist that are relying on the GRUB hack for
> EFI boot.
>
> We will shortly enable support for running under KVM, but the GRUB
> hack (which disables all caches without doing cache cleaning by VA
> during ExitBootServices()) is likely to cause more problems than it
> solves, given that KVM hosts require correct maintenance if they
> incorporate non-architected system caches.
>
> So let's disable the GRUB hack by default on the QEMU/mach-virt
> port.
>
> Signed-off-by: Ard Biesheuvel 

This patch could be merged with 2/5. You are changing the same defconfig.

Reviewed-by: Heinrich Schuchardt 

> ---
>  configs/qemu_arm_defconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/configs/qemu_arm_defconfig b/configs/qemu_arm_defconfig
> index 75bdce7708c7..1d2b4437cb07 100644
> --- a/configs/qemu_arm_defconfig
> +++ b/configs/qemu_arm_defconfig
> @@ -47,3 +47,4 @@ CONFIG_USB=y
>  CONFIG_DM_USB=y
>  CONFIG_USB_EHCI_HCD=y
>  CONFIG_USB_EHCI_PCI=y
> +# CONFIG_EFI_GRUB_ARM32_WORKAROUND is not set
>



Re: [PATCH 3/5] arm: qemu: implement enable_caches()

2020-06-06 Thread Heinrich Schuchardt
On 6/6/20 7:15 PM, Ard Biesheuvel wrote:
> Add an override for enable_caches to enable the I and D caches, along
> with the cached 1:1 mapping of all of DRAM. This is needed for running
> U-Boot under virtualization with QEMU/kvm.
>
> Signed-off-by: Ard Biesheuvel 
> ---
>  board/emulation/qemu-arm/qemu-arm.c | 7 +++
>  1 file changed, 7 insertions(+)
>
> diff --git a/board/emulation/qemu-arm/qemu-arm.c 
> b/board/emulation/qemu-arm/qemu-arm.c
> index 69e8ef46f1f5..1b0d543b93c1 100644
> --- a/board/emulation/qemu-arm/qemu-arm.c
> +++ b/board/emulation/qemu-arm/qemu-arm.c
> @@ -4,6 +4,7 @@
>   */
>
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -94,6 +95,12 @@ void *board_fdt_blob_setup(void)
>   return (void *)CONFIG_SYS_SDRAM_BASE;
>  }
>
> +void enable_caches(void)
> +{
> +  icache_enable();
> +  dcache_enable();
> +}
> +

For other ARM architectures I have seen:

int arch_cpu_init(void)
{
icache_enable();
    return 0;
}

void enable_caches(void)
{
    dcache_enable();
}

Some boards have

if (!icache_status())
icache_enable();

others

#if !CONFIG_IS_ENABLED(SYS_ICACHE_OFF)
icache_enable();
#endif

Tom could you, please, advice what is the correct way to do it.

Best regards

Heinrich

>  #if defined(CONFIG_EFI_RNG_PROTOCOL)
>  #include 
>  #include 
>



Re: [PATCH 2/5] arm: qemu: enable LPAE on 32-bit

2020-06-06 Thread Heinrich Schuchardt
On 6/6/20 10:32 PM, Heinrich Schuchardt wrote:
> On 6/6/20 7:15 PM, Ard Biesheuvel wrote:
>> QEMU's mach-virt machine only supports selecting CPU models that
>> implement the virtualization extensions, and are therefore guaranteed
>> to support LPAE as well.
>
> I wonder why
> qemu-system-arm -machine virt -cpu help
> lists cortex-a9 (which is not LPAE enabled).
>
> But when I try to use it I get
> qemu-system-arm: mach-virt: CPU type cortex-a9-arm-cpu not supported
> This looks like a missing feature in QEMU.
>
> The default CPU for machine=virt is arm,cortex-a15.
>
> Acked-by: Heinrich Schuchardt 
>>
>> Initially, QEMU would not allow emulating these CPUs running in HYP
>> mode (or EL2, for AArch64), but today, it also contains a complete
>> implementation of the virtualization extensions themselves.
>>
>> This means we could be running U-Boot in HYP mode, in which case the
>> LPAE long descriptor page table format is the only format that is
>> supported. If we are not running in HYP mode, we can use either.
>>
>> So let's enable CONFIG_ARMV7_LPAE for qemu_arm_defconfig so that we
>> get the best support for running with the MMU and caches enabled at
>> any privilege level.
>>
>> Signed-off-by: Ard Biesheuvel 

You missed to CC the maintainer of QEMU ARM 'VIRT' BOARD. - We have
scripts/get_maintainer.pl to find the maintainers.

Cc: Tuomas Tynkkynen 

Best regards

Heinrich

>> ---
>>  configs/qemu_arm_defconfig | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/configs/qemu_arm_defconfig b/configs/qemu_arm_defconfig
>> index a8473988bd76..75bdce7708c7 100644
>> --- a/configs/qemu_arm_defconfig
>> +++ b/configs/qemu_arm_defconfig
>> @@ -1,5 +1,6 @@
>>  CONFIG_ARM=y
>>  CONFIG_ARM_SMCCC=y
>> +CONFIG_ARMV7_LPAE=y
>>  CONFIG_ARCH_QEMU=y
>>  CONFIG_ENV_SIZE=0x4
>>  CONFIG_ENV_SECT_SIZE=0x4
>>



Re: [PATCH 2/5] arm: qemu: enable LPAE on 32-bit

2020-06-06 Thread Heinrich Schuchardt
On 6/6/20 7:15 PM, Ard Biesheuvel wrote:
> QEMU's mach-virt machine only supports selecting CPU models that
> implement the virtualization extensions, and are therefore guaranteed
> to support LPAE as well.

I wonder why
qemu-system-arm -machine virt -cpu help
lists cortex-a9 (which is not LPAE enabled).

But when I try to use it I get
qemu-system-arm: mach-virt: CPU type cortex-a9-arm-cpu not supported
This looks like a missing feature in QEMU.

The default CPU for machine=virt is arm,cortex-a15.

Acked-by: Heinrich Schuchardt 
> Initially, QEMU would not allow emulating these CPUs running in HYP
> mode (or EL2, for AArch64), but today, it also contains a complete
> implementation of the virtualization extensions themselves.
>
> This means we could be running U-Boot in HYP mode, in which case the
> LPAE long descriptor page table format is the only format that is
> supported. If we are not running in HYP mode, we can use either.
>
> So let's enable CONFIG_ARMV7_LPAE for qemu_arm_defconfig so that we
> get the best support for running with the MMU and caches enabled at
> any privilege level.
>
> Signed-off-by: Ard Biesheuvel 
> ---
>  configs/qemu_arm_defconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/configs/qemu_arm_defconfig b/configs/qemu_arm_defconfig
> index a8473988bd76..75bdce7708c7 100644
> --- a/configs/qemu_arm_defconfig
> +++ b/configs/qemu_arm_defconfig
> @@ -1,5 +1,6 @@
>  CONFIG_ARM=y
>  CONFIG_ARM_SMCCC=y
> +CONFIG_ARMV7_LPAE=y
>  CONFIG_ARCH_QEMU=y
>  CONFIG_ENV_SIZE=0x4
>  CONFIG_ENV_SECT_SIZE=0x4
>


Re: [PATCH 1/5] arm: enable allocate-on-read for LPAE's DCACHE_WRITEBACK

2020-06-06 Thread Heinrich Schuchardt
On 6/6/20 7:15 PM, Ard Biesheuvel wrote:
> The LPAE version of DCACHE_WRITEBACK is currently defined as no-allocate
> for both reads and writes, which deviates from the non-LPAE definition,
> and mostly defeats the purpose of enabling the caches in the first place.
>
> So align LPAE with !LPAE, and enable allocate-on-read.

Hello Ard,

thanks for analyzing why booting Linux on QEMU fails in some scenarios.

Do you know where in U-Boot is the value for !LPAE is defined?

>
> Signed-off-by: Ard Biesheuvel 
> ---
>  arch/arm/include/asm/system.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
> index 7a40b56acdca..21b26557d28b 100644
> --- a/arch/arm/include/asm/system.h
> +++ b/arch/arm/include/asm/system.h
> @@ -445,7 +445,7 @@ static inline void set_dacr(unsigned int val)
>   * Memory types
>   */

To me the lines below look like black magic.

In the comment above the definition, please, add a reference explaining
where the values are defined and a comment explaining why the actual
values are chosen.

Maybe this could be a starting point for the description:

"This constant is used define memory attribute encodings in a
Long-descriptor format translation table entry for stage 1 translations.
It is used to set the Memory Attribute Indirection Registers MAIR and
HMAIR. For details see [1,2].

[1] MAIR0, Memory Attribute Indirection Register 0

https://developer.arm.com/docs/ddi0595/b/aarch32-system-registers/mair0/a/DDI0568A_b_armv8_r_supplement.pdf
[2] HMAIR0, Hyp Memory Attribute Indirection Register 0

https://developer.arm.com/docs/ddi0595/b/aarch32-system-registers/hmair0 "

Best regards

Heinrich

>  #define MEMORY_ATTRIBUTES((0x00 << (0 * 8)) | (0x88 << (1 * 8)) | \
> -  (0xcc << (2 * 8)) | (0xff << (3 * 8)))
> +  (0xee << (2 * 8)) | (0xff << (3 * 8)))
>
>  /* options available for data cache on each page */
>  enum dcache_option {
>



[PATCH] riscv: sbi: Add newline to error message

2020-06-06 Thread Sean Anderson
Signed-off-by: Sean Anderson 

---

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

diff --git a/common/spl/spl_opensbi.c b/common/spl/spl_opensbi.c
index e88136e6f3..14f335f75f 100644
--- a/common/spl/spl_opensbi.c
+++ b/common/spl/spl_opensbi.c
@@ -56,7 +56,7 @@ void spl_invoke_opensbi(struct spl_image_info *spl_image)
/* Find U-Boot image in /fit-images */
ret = spl_opensbi_find_uboot_node(spl_image->fdt_addr, &uboot_node);
if (ret) {
-   pr_err("Can't find U-Boot node, %d", ret);
+   pr_err("Can't find U-Boot node, %d\n", ret);
hang();
}
 
-- 
2.26.2



Re: [PATCH v3 3/4] spl: fit: nand: fix fit loading in case of bad blocks

2020-06-06 Thread Michael Nazzareno Trimarchi
Hi Dario

I know that is an important bug to be addressed and I would like to
add even Tom on this

On Wed, May 27, 2020 at 1:56 PM Dario Binacchi  wrote:
>
> The offset at which the image to be loaded from NAND is located is
> retrieved from the itb header. The presence of bad blocks in the area
> of the NAND where the itb image is located could invalidate the offset
> which must therefore be adjusted taking into account the state of the
> sectors concerned.
>
> cc: Michael Trimarchi 
> Signed-off-by: Dario Binacchi 
>
> ---
>
> Changes in v3:
> The previous versions were buggy for devices others than NAND. This
> because the 'adjust_offset' helper was properly set only for the NAND
> case but called even for devices like MMC, RAM, and so on, crashing the
> boot up by that devices. Continuing to use the adjust_offset helper
> would mean checking the helper before calling it and patching too many
> files to set it properly before calling the spl_load_simple_fit routine.
> For this reason, the adjust_offset helper has been removed from the
> spl_image_info structure and the offset fixed inside the read helper for
> the NAND device only. This solution fixes the problem for the NAND device
> without side effects for other types of devices.
>
> Changes in v2: None
>
>  common/spl/spl_nand.c   |  5 -
>  drivers/mtd/nand/raw/nand_spl_loaders.c | 28 +
>  include/nand.h  |  1 +
>  3 files changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/common/spl/spl_nand.c b/common/spl/spl_nand.c
> index 48c97549eb..1e6f2dce56 100644
> --- a/common/spl/spl_nand.c
> +++ b/common/spl/spl_nand.c
> @@ -42,8 +42,11 @@ static int spl_nand_load_image(struct spl_image_info 
> *spl_image,
>  static ulong spl_nand_fit_read(struct spl_load_info *load, ulong offs,
>ulong size, void *dst)
>  {
> +   ulong sector;
> int ret;
>
> +   sector = *(int *)load->priv;
> +   offs = sector + nand_spl_adjust_offset(sector, offs - sector);

Check if is needed to call adjust offset in case offs and sector are
the same. Maybe a comment is needed
on top of the function to describe a bit more why is called

> ret = nand_spl_load_image(offs, size, dst);
> if (!ret)
> return size;
> @@ -66,7 +69,7 @@ static int spl_nand_load_element(struct spl_image_info 
> *spl_image,
>
> debug("Found FIT\n");
> load.dev = NULL;
> -   load.priv = NULL;
> +   load.priv = &offset;

It's a bit an abuse here but I don't have a better plan on my side

Reviewed-by: Michael Trimarchi 

Michael

> load.filename = NULL;
> load.bl_len = 1;
> load.read = spl_nand_fit_read;
> diff --git a/drivers/mtd/nand/raw/nand_spl_loaders.c 
> b/drivers/mtd/nand/raw/nand_spl_loaders.c
> index 177c12b581..4befc75c04 100644
> --- a/drivers/mtd/nand/raw/nand_spl_loaders.c
> +++ b/drivers/mtd/nand/raw/nand_spl_loaders.c
> @@ -41,6 +41,34 @@ int nand_spl_load_image(uint32_t offs, unsigned int size, 
> void *dst)
> return 0;
>  }
>
> +/**
> + * nand_spl_adjust_offset - Adjust offset from a starting sector
> + * @sector:Address of the sector
> + * @offs:  Offset starting from @sector
> + *
> + * If one or more bad blocks are in the address space between @sector
> + * and @sector + @offs, @offs is increased by the NAND block size for
> + * each bad block found.
> + */
> +u32 nand_spl_adjust_offset(u32 sector, u32 offs)
> +{
> +   unsigned int block, lastblock;
> +
> +   block = sector / CONFIG_SYS_NAND_BLOCK_SIZE;
> +   lastblock = (sector + offs) / CONFIG_SYS_NAND_BLOCK_SIZE;
> +
> +   while (block <= lastblock) {
> +   if (nand_is_bad_block(block)) {
> +   offs += CONFIG_SYS_NAND_BLOCK_SIZE;
> +   lastblock++;
> +   }
> +
> +   block++;
> +   }
> +
> +   return offs;
> +}
> +
>  #ifdef CONFIG_SPL_UBI
>  /*
>   * Temporary storage for non NAND page aligned and non NAND page sized
> diff --git a/include/nand.h b/include/nand.h
> index 93cbe1e25d..80dd6469bc 100644
> --- a/include/nand.h
> +++ b/include/nand.h
> @@ -120,6 +120,7 @@ int nand_unlock(struct mtd_info *mtd, loff_t start, 
> size_t length,
> int allexcept);
>  int nand_get_lock_status(struct mtd_info *mtd, loff_t offset);
>
> +u32 nand_spl_adjust_offset(u32 sector, u32 offs);
>  int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst);
>  int nand_spl_read_block(int block, int offset, int len, void *dst);
>  void nand_deselect(void);
> --
> 2.17.1
>


-- 
| Michael Nazzareno Trimarchi Amarula Solutions BV |
| COO  -  Founder  Cruquiuskade 47 |
| +31(0)851119172 Amsterdam 1018 AM NL |
|  [`as] http://www.amarulasolutions.com   |


Re: [PATCH 7/8] regmap: Add support for regmap fields

2020-06-06 Thread Simon Glass
Hi Pratyush,

On Fri, 5 Jun 2020 at 13:03, Pratyush Yadav  wrote:
>
> Hi Simon,
>
> On 31/05/20 08:08AM, Simon Glass wrote:
> > Hi Pratyush,
> >
> > On Wed, 27 May 2020 at 06:52, Pratyush Yadav  wrote:
> > >
> > > From: Jean-Jacques Hiblot 
> > >
> > > A regmap field is an abstraction available in Linux. It provides to access
> > > bitfields in a regmap without having to worry about shifts and masks.
> > >
> > > Signed-off-by: Jean-Jacques Hiblot 
> > > Reviewed-by: Simon Glass 
> > > Signed-off-by: Pratyush Yadav 
> > > ---
> > >  drivers/core/regmap.c |  78 ++
> > >  include/regmap.h  | 108 ++
> > >  2 files changed, 186 insertions(+)
> > >
> >
> > Reviewed-by: Simon Glass 
> >
> > Please see below
> >
> > > diff --git a/drivers/core/regmap.c b/drivers/core/regmap.c
> > > index 4792067f24..cbc01b689a 100644
> > > --- a/drivers/core/regmap.c
> > > +++ b/drivers/core/regmap.c
> > > @@ -18,6 +18,15 @@
> > >  #include 
> > >  #include 
> > >  #include 
> > > +#include 
> > > +
> > > +struct regmap_field {
> >
> > Needs comments as I'm not sure what this is for
>
> Heh, I'm that sure either. I went digging in the Linux source, and the
> only commit that touches this is the one that added regmap fields and it
> doesn't mention why we have both "regmap_field" and "reg_field".
>
> Looking at the patch, I gather that regmap_field and reg_field look at
> the field in two different ways. reg_field is how a driver would look at
> a regmap field: which register this field belongs to, and what are its
> LSB and MSB? Datasheets usually tell you this information directly.
>
> regmap_field looks at it from a lower level view: what register this
> feild belongs to, and how do I need to shift and mask the data I read
> from the bus? This is closer to how to actually read/write the field.
> The fact that regmap_field's definition is private to regmap.c also
> points in this direction.
>
> The conversion from reg_field to regmap_field is done in
> regmap_field_init().
>
> As to why have these two representations of the same thing, I can't say
> with certainty. My guess is that it makes the internal regmap code
> cleaner and slightly faster because you don't have to calculate the
> shift and mask every time.

That is good insight - please put those comments in the code.

Regards,
Simon


Re: UCLASS_BLK driver binding without devtree and U_BOOT_DEVICE

2020-06-06 Thread Simon Glass
Hi Anastasiia,

On Fri, 5 Jun 2020 at 08:21, Anastasiia Lukianenko
 wrote:
>
> Hi Simon,
>
> Thank you for detailed reply.
> We will bind UCLASS_BLK driver according to your advice. Block device
> will have it's own parent which handles the actual device access.
> Most likely I'll have more questions during realization.
>
> > As above, you likely need to add a new interface type. See
> > IF_TYPE_VIRTIO, for example.
>
> Earlier I mentioned that we have already created [IF_TYPE_PVBLOCK] =
> "pvblock" (UCLASS_BLK) typename and UCLASS. Can we use it?

Yes I don't see why not.

Regards,
Simon


>
> Regards,
> Anastasiia Lukianenko
>
>
>
> From: Simon Glass 
> Sent: 04 June 2020 16:00
> To: Anastasiia Lukianenko 
> Cc: u-boot@lists.denx.de ; Oleksandr Andrushchenko 
> ; Volodymyr Babchuk 
> 
> Subject: Re: UCLASS_BLK driver binding without devtree and U_BOOT_DEVICE
>
> Hi Anastasiia,
>
> On Thu, 4 Jun 2020 at 03:06, Anastasiia Lukianenko
>  wrote:
> >
> > Hello Simon,
> >
>
> Thanks for sending as plain text.
>
> > We are adding support for Xen [1] para-virtualized block device as a u-boot
> > driver and have some questions with respect to driver model and
> > instantiation of new devices.
> > Because of the nature of the device driver we cannot use device tree or
> > define statically the device instances beforehand and those get known
> > during some device enumeration.
> > So, this makes us bind the new devices to the driver at run-time. For that 
> > we
> > use:
> > (init)
> > device_bind_by_name(gd->dm_root, false, &info, &udev);
> > ...
> > ret = uclass_get(UCLASS_BLK, &uc);
> > if (ret) {
> > printk("UCLASS_BLK class has no registered entities\n");
> > return;
> > }
> > uclass_foreach_dev(udev, uc) {
> > ret = device_probe(udev);
> > if (ret < 0)
> > printk("Failed to probe " DRV_NAME " device\n");
> > }
> > ...
> > (probe)
> > struct blkfront_platdata *platdata = dev_get_platdata(udev);
> > struct blkfront_dev *blk_dev = dev_get_priv(udev);
> > struct blk_desc *desc = dev_get_uclass_platdata(udev);
> >
> > We create a device with [IF_TYPE_PVBLOCK] = "pvblock" (UCLASS_BLK)
> > typename and UCLASS. Everything goes smooth, driver's probe is
> > called, but the problem we are seeing is that when we call
> > "part list pvblock 0" command with the device description and udev's
> > parent not set or set to gd->dm_root then we see
> > <** Bad device pvblock 0 **> from blk_get_device_by_str function.
> >
> > Digging into the code shows that there is a check (equality of block
> > device's interface type and the uclass of the block device's parent) in
> > blk_get_devnum_by_typename function in drivers/blk-uclass.c file,
> > which makes us think that we should somehow set our parent to
> > UCLASS_BLK or something. At the same time setting udev to
> > udev->parent makes things work, but it doesn't seem to be correct.
> > (For example, "pvblock part 0" command works properly, because it
> > doesn't use blk_get_devnum_by_typename.)
>
> At present BLK devices have a parent device which handles the actual
> device access. For example, look at mmc-uclass.c where you can see
> mmc_blk which is always a child of the MMC device. The block device
> does not know how to access the hardware.
>
> >
> > 1. What is the right procedure to bind a device to a driver in such
> > cases as ours? Can we still use what we have? (Is it okay to use
> > device_bind_by_name function in init stage?)
>
> I suspect you should create a new uclass for a xen device, perhaps one
> specific to a xen device that provides block-level access.
>
> > 2. How can we become a child of UCLASS_BLK or any other entity, so
> > we are recognized by the framework properly? (Do we need to create
> > child from our udev device in probe stage by using blk_create_devicef
> > function?)
>
> You should bind your BLK device in your XEN device bind() routine.
>
> Probing should be automatic. You might find sunxi_mmc.c useful as an example.
>
> > 3. Why parent's and device's U_CLASS must be the same? For what
> > cases this check has been implemented?
>
> U-Boot has a variety of block devices. The existing mechanism
> (pre-dating driver model) uses an interface type to distinguish them
> (IF_TYPE_...). You can see this in commands like:
>
> fs ls usb 0:2
> ext2ls usb 0:2
>
> Here 'usb' is the interface type.
>
> As above, you likely need to add a new interface type. See
> IF_TYPE_VIRTIO, for eaxmple.
>
> > 4. According to blk uclass implementation is it okay to have parent with
> > UCLASS_ROOT for block device? If no, then what parent should we assign?
>
> I think we might do that in tests in strange cases, but I think you
> should have your own parent, as above.
>
>
> >
> > [1] - 
> > https://urldefense.com/v3/__https://xenproject.org/__;!!GF_29dbcQIUBPA!g5WmW2skbXIkHInOdxw6_N3awQzZJOOUWP_Mvz1dEY9tvY6TYlYAvxfsIwMlqyo9Lon-Xik$
>
> Regards,
> Simon


[PATCH 4/5] arm: qemu: disable the EFI workaround for older GRUB

2020-06-06 Thread Ard Biesheuvel
The QEMU/mach-virt targeted port of u-boot currently only runs on
QEMU under TCG emulation, which does not model the caches at all,
and so no users can exist that are relying on the GRUB hack for
EFI boot.

We will shortly enable support for running under KVM, but the GRUB
hack (which disables all caches without doing cache cleaning by VA
during ExitBootServices()) is likely to cause more problems than it
solves, given that KVM hosts require correct maintenance if they
incorporate non-architected system caches.

So let's disable the GRUB hack by default on the QEMU/mach-virt
port.

Signed-off-by: Ard Biesheuvel 
---
 configs/qemu_arm_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/qemu_arm_defconfig b/configs/qemu_arm_defconfig
index 75bdce7708c7..1d2b4437cb07 100644
--- a/configs/qemu_arm_defconfig
+++ b/configs/qemu_arm_defconfig
@@ -47,3 +47,4 @@ CONFIG_USB=y
 CONFIG_DM_USB=y
 CONFIG_USB_EHCI_HCD=y
 CONFIG_USB_EHCI_PCI=y
+# CONFIG_EFI_GRUB_ARM32_WORKAROUND is not set
-- 
2.26.2



[PATCH 5/5] arm: qemu: override flash accessors to use virtualizable instructions

2020-06-06 Thread Ard Biesheuvel
Some instructions in the ARM ISA have multiple output registers, such
as ldrd/ldp (load pair), where two registers are loaded from memory,
but also ldr with indexing, where the memory base register is incremented
as well when the value is loaded to the destination register.

MMIO emulation under KVM is based on using the architecturally defined
syndrome information that is provided when an exception is taken to the
hypervisor. This syndrome information describes whether the instruction
that triggered the exception is a load or a store, what the faulting
address was, and which register was the destination register.

This syndrome information can only describe one destination register, and
when the trapping instruction is one with multiple outputs, KVM throws an
error like

  kvm [615929]: Data abort outside memslots with no valid syndrome info

on the host and kills the QEMU process with the following error:

  U-Boot 2020.07-rc3-00208-g88bd5b179360-dirty (Jun 06 2020 - 11:59:22 +0200)

  DRAM:  1 GiB
  Flash: error: kvm run failed Function not implemented
  R00=0001 R01=0040 R02=7ee0ce20 R03=
  R04=7ffd9eec R05=0004 R06=7ffda3f8 R07=0055
  R08=7ffd9eec R09=7ef0ded0 R10=7ee0ce20 R11=
  R12=0004 R13=7ee0cdf8 R14= R15=7ff72d08
  PSR=21d3 --C- A svc32
  QEMU: Terminated

This means that, in order to run U-Boot in QEMU under KVM, we need to
avoid such instructions when accessing emulated devices. For the flash
in particular, which is a hybrid between a ROM (backed by a memslot)
when in array mode, and an emulated MMIO device (when in write mode),
we need to take care to only use instructions that KVM can deal with
when they trap.

So override the flash accessors that are used when running on QEMU.
Note that the write accessors are included for completeness, but the
read accessors are the ones that need this special care.

Signed-off-by: Ard Biesheuvel 
---
 board/emulation/qemu-arm/qemu-arm.c | 55 
 include/configs/qemu-arm.h  |  1 +
 2 files changed, 56 insertions(+)

diff --git a/board/emulation/qemu-arm/qemu-arm.c 
b/board/emulation/qemu-arm/qemu-arm.c
index 1b0d543b93c1..32e18fd8b985 100644
--- a/board/emulation/qemu-arm/qemu-arm.c
+++ b/board/emulation/qemu-arm/qemu-arm.c
@@ -142,3 +142,58 @@ efi_status_t platform_get_rng_device(struct udevice **dev)
return EFI_SUCCESS;
 }
 #endif /* CONFIG_EFI_RNG_PROTOCOL */
+
+#ifdef CONFIG_ARM64
+#define __W"w"
+#else
+#define __W
+#endif
+
+void flash_write8(u8 value, void *addr)
+{
+   asm("strb %" __W "1, %0" : "=m"(*(u8 *)addr) : "r"(value));
+}
+
+void flash_write16(u16 value, void *addr)
+{
+   asm("strh %" __W "1, %0" : "=m"(*(u16 *)addr) : "r"(value));
+}
+
+void flash_write32(u32 value, void *addr)
+{
+   asm("str %" __W "1, %0" : "=m"(*(u32 *)addr) : "r"(value));
+}
+
+void flash_write64(u64 value, void *addr)
+{
+   BUG(); /* FLASH_CFI_64BIT is not implemented by QEMU */
+}
+
+u8 flash_read8(void *addr)
+{
+   u8 ret;
+
+   asm("ldrb %" __W "0, %1" : "=r"(ret) : "m"(*(u8 *)addr));
+   return ret;
+}
+
+u16 flash_read16(void *addr)
+{
+   u16 ret;
+
+   asm("ldrh %" __W "0, %1" : "=r"(ret) : "m"(*(u16 *)addr));
+   return ret;
+}
+
+u32 flash_read32(void *addr)
+{
+   u32 ret;
+
+   asm("ldr %" __W "0, %1" : "=r"(ret) : "m"(*(u32 *)addr));
+   return ret;
+}
+
+u64 flash_read64(void *addr)
+{
+   BUG(); /* FLASH_CFI_64BIT is not implemented by QEMU */
+}
diff --git a/include/configs/qemu-arm.h b/include/configs/qemu-arm.h
index 1ef75a87836b..bc8b7c5c1238 100644
--- a/include/configs/qemu-arm.h
+++ b/include/configs/qemu-arm.h
@@ -53,5 +53,6 @@
 #define CONFIG_SYS_MAX_FLASH_BANKS 2
 #endif
 #define CONFIG_SYS_MAX_FLASH_SECT  256 /* Sector: 256K, Bank: 64M */
+#define CONFIG_CFI_FLASH_USE_WEAK_ACCESSORS
 
 #endif /* __CONFIG_H */
-- 
2.26.2



[PATCH 3/5] arm: qemu: implement enable_caches()

2020-06-06 Thread Ard Biesheuvel
Add an override for enable_caches to enable the I and D caches, along
with the cached 1:1 mapping of all of DRAM. This is needed for running
U-Boot under virtualization with QEMU/kvm.

Signed-off-by: Ard Biesheuvel 
---
 board/emulation/qemu-arm/qemu-arm.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/board/emulation/qemu-arm/qemu-arm.c 
b/board/emulation/qemu-arm/qemu-arm.c
index 69e8ef46f1f5..1b0d543b93c1 100644
--- a/board/emulation/qemu-arm/qemu-arm.c
+++ b/board/emulation/qemu-arm/qemu-arm.c
@@ -4,6 +4,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -94,6 +95,12 @@ void *board_fdt_blob_setup(void)
return (void *)CONFIG_SYS_SDRAM_BASE;
 }
 
+void enable_caches(void)
+{
+icache_enable();
+dcache_enable();
+}
+
 #if defined(CONFIG_EFI_RNG_PROTOCOL)
 #include 
 #include 
-- 
2.26.2



[PATCH 1/5] arm: enable allocate-on-read for LPAE's DCACHE_WRITEBACK

2020-06-06 Thread Ard Biesheuvel
The LPAE version of DCACHE_WRITEBACK is currently defined as no-allocate
for both reads and writes, which deviates from the non-LPAE definition,
and mostly defeats the purpose of enabling the caches in the first place.

So align LPAE with !LPAE, and enable allocate-on-read.

Signed-off-by: Ard Biesheuvel 
---
 arch/arm/include/asm/system.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
index 7a40b56acdca..21b26557d28b 100644
--- a/arch/arm/include/asm/system.h
+++ b/arch/arm/include/asm/system.h
@@ -445,7 +445,7 @@ static inline void set_dacr(unsigned int val)
  * Memory types
  */
 #define MEMORY_ATTRIBUTES  ((0x00 << (0 * 8)) | (0x88 << (1 * 8)) | \
-(0xcc << (2 * 8)) | (0xff << (3 * 8)))
+(0xee << (2 * 8)) | (0xff << (3 * 8)))
 
 /* options available for data cache on each page */
 enum dcache_option {
-- 
2.26.2



[PATCH 2/5] arm: qemu: enable LPAE on 32-bit

2020-06-06 Thread Ard Biesheuvel
QEMU's mach-virt machine only supports selecting CPU models that
implement the virtualization extensions, and are therefore guaranteed
to support LPAE as well.

Initially, QEMU would not allow emulating these CPUs running in HYP
mode (or EL2, for AArch64), but today, it also contains a complete
implementation of the virtualization extensions themselves.

This means we could be running U-Boot in HYP mode, in which case the
LPAE long descriptor page table format is the only format that is
supported. If we are not running in HYP mode, we can use either.

So let's enable CONFIG_ARMV7_LPAE for qemu_arm_defconfig so that we
get the best support for running with the MMU and caches enabled at
any privilege level.

Signed-off-by: Ard Biesheuvel 
---
 configs/qemu_arm_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/qemu_arm_defconfig b/configs/qemu_arm_defconfig
index a8473988bd76..75bdce7708c7 100644
--- a/configs/qemu_arm_defconfig
+++ b/configs/qemu_arm_defconfig
@@ -1,5 +1,6 @@
 CONFIG_ARM=y
 CONFIG_ARM_SMCCC=y
+CONFIG_ARMV7_LPAE=y
 CONFIG_ARCH_QEMU=y
 CONFIG_ENV_SIZE=0x4
 CONFIG_ENV_SECT_SIZE=0x4
-- 
2.26.2



[PATCH 0/5] Fixes for running U-boot under QEMU/KVM

2020-06-06 Thread Ard Biesheuvel
This series fixes a number of issues that exist in the QEMU/mach-virt
port of u-boot, and that prevent it from executing correctly under
virtualization (as opposed to TCG emulation)

As the Linux EFI subsystem maintainer, I am looking to increase test
coverage for the EFI related changes that are under development for
Linux, and one of the things I plan to do is start using U-boot as
test firmware for boot testing. This can be done under TCG emulation,
but given how loosely TCG implements the architecture, it is better
to test under virtualization as well.

With these changes applied, u-boot can boot Linux in EFI mode under
KVM.

Cc: Tom Rini 
Cc: Sughosh Ganu 
Cc: Heinrich Schuchardt 

Ard Biesheuvel (5):
  arm: enable allocate-on-read for LPAE's DCACHE_WRITEBACK
  arm: qemu: enable LPAE on 32-bit
  arm: qemu: implement enable_caches()
  arm: qemu: disable the EFI workaround for older GRUB
  arm: qemu: override flash accessors to use virtualizable instructions

 arch/arm/include/asm/system.h   |  2 +-
 board/emulation/qemu-arm/qemu-arm.c | 62 
 configs/qemu_arm_defconfig  |  2 +
 include/configs/qemu-arm.h  |  1 +
 4 files changed, 66 insertions(+), 1 deletion(-)

-- 
2.26.2



Re: [PATCH 3/6] env: Fix invalid env handling in env_init()

2020-06-06 Thread Tom Rini
On Sat, Jun 06, 2020 at 04:54:52PM +0200, Marek Vasut wrote:
> On 6/5/20 11:11 PM, Tom Rini wrote:
> > On Fri, Jun 05, 2020 at 10:47:24PM +0200, Marek Vasut wrote:
> >> On 6/5/20 9:07 PM, Tom Rini wrote:
> >>> On Wed, Jun 03, 2020 at 02:01:08AM +0200, Marek Vasut wrote:
> >>>
>  In case the env storage driver marks environment as ENV_INVALID, we must
>  reset the $ret return value to -ENOENT to let the env init code reset the
>  environment to the default one a bit further down.
> 
>  Signed-off-by: Marek Vasut 
>  ---
>   env/env.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
>  diff --git a/env/env.c b/env/env.c
>  index dcc25c030b..024d36fdbe 100644
>  --- a/env/env.c
>  +++ b/env/env.c
>  @@ -300,6 +300,9 @@ int env_init(void)
>   
>   debug("%s: Environment %s init done (ret=%d)\n", 
>  __func__,
> drv->name, ret);
>  +
>  +if (gd->env_valid == ENV_INVALID)
>  +ret = -ENOENT;
>   }
>   
>   if (!prio)
> >>>
> >>> Is the storage driver marking the environment as invalid but not
> >>> returning ENOENT valid?
> >>
> >> Yes, some are doing that.
> > 
> > Why?  Is that correct or incorrect?  It doesn't seem like this is
> > something that should be inconsistent from storage driver to storage
> > driver and needs fixing.
> 
> The default env driver is doing it, whether it's a workaround or correct
> behavior, I really don't know. Maybe Joe does ?
> 
> >>> How does all of this work in the case of multiple configured storage
> >>> drivers?
> >>
> >> If the env is invalid, then we report it as invalid.
> > 
> > Right.  And what change, if any, does your proposed change have in this
> > case?  Thanks!
> 
> Before this patch, that check was missing and the result was random,
> depending on which env order you had.

So have you changed the behavior of multiple environments then?  Today
it's indeed link order based, which is not optimal but used.

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH 3/6] env: Fix invalid env handling in env_init()

2020-06-06 Thread Marek Vasut
On 6/5/20 11:11 PM, Tom Rini wrote:
> On Fri, Jun 05, 2020 at 10:47:24PM +0200, Marek Vasut wrote:
>> On 6/5/20 9:07 PM, Tom Rini wrote:
>>> On Wed, Jun 03, 2020 at 02:01:08AM +0200, Marek Vasut wrote:
>>>
 In case the env storage driver marks environment as ENV_INVALID, we must
 reset the $ret return value to -ENOENT to let the env init code reset the
 environment to the default one a bit further down.

 Signed-off-by: Marek Vasut 
 ---
  env/env.c | 3 +++
  1 file changed, 3 insertions(+)

 diff --git a/env/env.c b/env/env.c
 index dcc25c030b..024d36fdbe 100644
 --- a/env/env.c
 +++ b/env/env.c
 @@ -300,6 +300,9 @@ int env_init(void)
  
debug("%s: Environment %s init done (ret=%d)\n", __func__,
  drv->name, ret);
 +
 +  if (gd->env_valid == ENV_INVALID)
 +  ret = -ENOENT;
}
  
if (!prio)
>>>
>>> Is the storage driver marking the environment as invalid but not
>>> returning ENOENT valid?
>>
>> Yes, some are doing that.
> 
> Why?  Is that correct or incorrect?  It doesn't seem like this is
> something that should be inconsistent from storage driver to storage
> driver and needs fixing.

The default env driver is doing it, whether it's a workaround or correct
behavior, I really don't know. Maybe Joe does ?

>>> How does all of this work in the case of multiple configured storage
>>> drivers?
>>
>> If the env is invalid, then we report it as invalid.
> 
> Right.  And what change, if any, does your proposed change have in this
> case?  Thanks!

Before this patch, that check was missing and the result was random,
depending on which env order you had.


[PATCH] clk: renesas: Synchronize Gen2 MSTP teardown tables

2020-06-06 Thread Marek Vasut
Synchronize Gen2 MSTP teardown tables with datasheet Rev.2.00
Feb 01, 2016. This corrects the following bits:
  - added H2 MSTP3[10] SCIF2
  - added H2/M2/E2 MSTP7[29] TCON
  - removed E2 MSTP5[22] Thermal Sensor
  - removed E2 MSTP10[31,24:22] SRC0, SRC7:9

Signed-off-by: Marek Vasut 
Cc: Nobuhiro Iwamatsu 
---
 drivers/clk/renesas/r8a7790-cpg-mssr.c | 4 ++--
 drivers/clk/renesas/r8a7791-cpg-mssr.c | 2 +-
 drivers/clk/renesas/r8a7794-cpg-mssr.c | 6 +++---
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/renesas/r8a7790-cpg-mssr.c 
b/drivers/clk/renesas/r8a7790-cpg-mssr.c
index 7451f53ba3..6f4aa6f0ca 100644
--- a/drivers/clk/renesas/r8a7790-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a7790-cpg-mssr.c
@@ -239,11 +239,11 @@ static const struct mstp_stop_table r8a7790_mstp_table[] 
= {
{ 0x00640801, 0x40, 0x00640801, 0x0 },
{ 0xDB6E9BDF, 0x0, 0xDB6E9BDF, 0x0 },
{ 0x300DA1FC, 0x2010, 0x300DA1FC, 0x0 },
-   { 0xF08CF831, 0x0, 0xF08CF831, 0x0 },
+   { 0xF08CFC31, 0x0, 0xF08CFC31, 0x0 },
{ 0x8184, 0x180, 0x8184, 0x0 },
{ 0x44C00046, 0x0, 0x44C00046, 0x0 },
{ 0x0, 0x0, 0x0, 0x0 }, /* SMSTP6 is not present on Gen2 */
-   { 0x07F30718, 0x20, 0x07F30718, 0x0 },
+   { 0x27F30718, 0x20, 0x27F30718, 0x0 },
{ 0x01F0FF84, 0x0, 0x01F0FF84, 0x0 },
{ 0xF5979FCF, 0x0, 0xF5979FCF, 0x0 },
{ 0xFFFEFFE0, 0x0, 0xFFFEFFE0, 0x0 },
diff --git a/drivers/clk/renesas/r8a7791-cpg-mssr.c 
b/drivers/clk/renesas/r8a7791-cpg-mssr.c
index 25fd489609..3f02f419a4 100644
--- a/drivers/clk/renesas/r8a7791-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a7791-cpg-mssr.c
@@ -245,7 +245,7 @@ static const struct mstp_stop_table r8a7791_mstp_table[] = {
{ 0x81C4, 0x180, 0x81C4, 0x0 },
{ 0x44C00046, 0x0, 0x44C00046, 0x0 },
{ 0x0, 0x0, 0x0, 0x0 }, /* SMSTP6 is not present on Gen2 */
-   { 0x05BFE618, 0x20, 0x05BFE618, 0x0 },
+   { 0x25BFE618, 0x20, 0x25BFE618, 0x0 },
{ 0x40C0FE85, 0x0, 0x40C0FE85, 0x0 },
{ 0xFF979FFF, 0x0, 0xFF979FFF, 0x0 },
{ 0xFFFEFFE0, 0x0, 0xFFFEFFE0, 0x0 },
diff --git a/drivers/clk/renesas/r8a7794-cpg-mssr.c 
b/drivers/clk/renesas/r8a7794-cpg-mssr.c
index 7093e0d42c..53b55b679d 100644
--- a/drivers/clk/renesas/r8a7794-cpg-mssr.c
+++ b/drivers/clk/renesas/r8a7794-cpg-mssr.c
@@ -218,12 +218,12 @@ static const struct mstp_stop_table r8a7794_mstp_table[] 
= {
{ 0x100D21FC, 0x2000, 0x100D21FC, 0x0 },
{ 0xE084D810, 0x0, 0xE084D810, 0x0 },
{ 0x81C4, 0x180, 0x81C4, 0x0 },
-   { 0x40C00044, 0x0, 0x40C00044, 0x0 },
+   { 0x40800044, 0x0, 0x40800044, 0x0 },
{ 0x0, 0x0, 0x0, 0x0 }, /* SMSTP6 is not present on Gen2 */
-   { 0x013FE618, 0x8, 0x013FE618, 0x0 },
+   { 0x21BFE618, 0x8, 0x21BFE618, 0x0 },
{ 0x40803C05, 0x0, 0x40803C05, 0x0 },
{ 0xFB879FEE, 0x0, 0xFB879FEE, 0x0 },
-   { 0xFFFEFFE0, 0x0, 0xFFFEFFE0, 0x0 },
+   { 0x7E3EFFE0, 0x0, 0x7E3EFFE0, 0x0 },
{ 0x01C0, 0x0, 0x01C0, 0x0 },
 };
 
-- 
2.25.1



Re: [PATCH 01/15] net: pcnet: Drop typedef struct pcnet_priv_t

2020-06-06 Thread Marek Vasut
On 6/6/20 1:56 AM, Daniel Schwierzeck wrote:
> 
> 
> Am 05.06.20 um 19:50 schrieb Marek Vasut:
>> On 6/5/20 6:38 PM, Daniel Schwierzeck wrote:
>>>
>>>
>>> Am 05.06.20 um 18:22 schrieb Marek Vasut:
 On 6/5/20 5:28 PM, Daniel Schwierzeck wrote:
>
>
> Am 17.05.20 um 18:24 schrieb Marek Vasut:
>> Use struct pcnet_priv all over the place instead.
>>
>> Signed-off-by: Marek Vasut 
>> Cc: Daniel Schwierzeck 
>> Cc: Joe Hershberger 
>> ---
>>  drivers/net/pcnet.c | 10 +-
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>
>
> series applied to u-boot-mips/next, thanks.
>
> Marek, the Qemu Malta tests in Travis CI are basically working. But only
> with your patch series applied, the 64 Bit variants are running stable.

 Can you share the patch you added so I can also add it to u-boot-sh/net
 for my CI tests there ?
>>>
>>> https://github.com/danielschwierzeck/uboot-test-hooks/commit/6f46bcf59476f5005ac19336a22140c717a9ab6b
>>>
>>> https://gitlab.denx.de/u-boot/custodians/u-boot-mips/-/commit/09320af5578d46ae4d7bb557b79ca4567a6aaa6c
>>
>> Patch for this series I mean.
>>
> 
> ah ok, I don't have a separate patch, I just updated your last patch
> with the missing defconfigs.
> 
> https://gitlab.denx.de/u-boot/custodians/u-boot-mips/-/commit/9e3097ff7939799b1bd2276085ca359a5d5e2aef

Nice, thanks!


[PATCH 1/1] efi_loader: printf code in efi_image_parse()

2020-06-06 Thread Heinrich Schuchardt
For size_t we have to use %zu for printing not %lu.

Fixes: 4540dabdcaca ("efi_loader: image_loader: support image
authentication")
Signed-off-by: Heinrich Schuchardt 
---
 lib/efi_loader/efi_image_loader.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_image_loader.c 
b/lib/efi_loader/efi_image_loader.c
index 478aaf50d3..230d41ae5e 100644
--- a/lib/efi_loader/efi_image_loader.c
+++ b/lib/efi_loader/efi_image_loader.c
@@ -369,7 +369,7 @@ bool efi_image_parse(void *efi, size_t len, struct 
efi_image_regions **regp,

/* 3. Extra data excluding Certificates Table */
if (bytes_hashed + authsz < len) {
-   debug("extra data for hash: %lu\n",
+   debug("extra data for hash: %zu\n",
  len - (bytes_hashed + authsz));
efi_image_region_add(regs, efi + bytes_hashed,
 efi + len - authsz, 0);
--
2.26.2



[PATCH 1/1] arm: sunxi: increase SYS_MALLOC_F_LEN

2020-06-06 Thread Heinrich Schuchardt
The current default of 0x400 for SYS_MALLOC_F_LEN is too small if any
additional drivers marked as DM_FLAG_PRE_RELOC are loaded before
relocation.

CONFIG_RSA=y which is needed for UEFI secure boot or for FIT image
verification loads the driver mod_exp_sw which has DM_FLAG_PRE_RELOC.

CONFIG_LOG=Y is another setting requiring additional early malloc
area, cf. log_init().

When running pine64-lts_defconfig with CONFIG_RSA=y and debug UART enabled
we see as output in main U-Boot

alloc_simple() alloc space exhausted

With this patch the default values of SYS_MALLOC_F_LEN and
SPL_SYS_MALLOC_F_LEN on ARCH_SUNXI are raised to 0x2000.

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

diff --git a/Kconfig b/Kconfig
index 0e7ccc0b07..13204e2384 100644
--- a/Kconfig
+++ b/Kconfig
@@ -146,7 +146,7 @@ config SYS_MALLOC_F_LEN
default 0x2000 if (ARCH_IMX8 || ARCH_IMX8M || ARCH_MX7 || \
   ARCH_MX7ULP || ARCH_MX6 || ARCH_MX5 || \
   ARCH_LS1012A || ARCH_LS1021A || ARCH_LS1043A || \
-  ARCH_LS1046A)
+  ARCH_LS1046A || ARCH_SUNXI)
default 0x400
help
  Before relocation, memory is very limited on many platforms. Still,
--
2.20.1



Re: [PATCH 1/1] sunxi: CONFIG_INIT_SP_RELATIVE=y for Pine64 LTS

2020-06-06 Thread Heinrich Schuchardt
On 6/3/20 7:07 PM, Heinrich Schuchardt wrote:
> On 6/3/20 6:31 PM, André Przywara wrote:
>> On 03/06/2020 16:47, Heinrich Schuchardt wrote:
>>
>> Hi Heinrich,
>>
>>> On 03.06.20 01:46, André Przywara wrote:
 On 02/06/2020 20:55, Tom Rini wrote:

 Hi,

> On Tue, Jun 02, 2020 at 09:45:25PM +0200, Heinrich Schuchardt wrote:
>> On 6/2/20 5:51 PM, Tom Rini wrote:
>>> On Sun, May 31, 2020 at 10:43:00AM +, Heinrich Schuchardt wrote:
>>>
 Booting pine64-lts_defconfig with either of CONFIG_RSA=y or 
 CONFIG_LOG=y
 fails if CONFIG_INIT_SP_RELATIVE is not set.

 Signed-off-by: Heinrich Schuchardt 
 ---
  configs/pine64-lts_defconfig | 3 +++
  1 file changed, 3 insertions(+)

 diff --git a/configs/pine64-lts_defconfig 
 b/configs/pine64-lts_defconfig
 index ef108a1a31..b03bef01b1 100644
 --- a/configs/pine64-lts_defconfig
 +++ b/configs/pine64-lts_defconfig
 @@ -1,5 +1,8 @@
  CONFIG_ARM=y
 +CONFIG_INIT_SP_RELATIVE=y
  CONFIG_ARCH_SUNXI=y
 +CONFIG_SYS_MALLOC_F_LEN=0x8000
 +CONFIG_SPL_SYS_MALLOC_F_LEN=0x400
  CONFIG_SPL=y
  CONFIG_MACH_SUN50I=y
  CONFIG_SUNXI_DRAM_LPDDR3_STOCK=y
>>>
>>> Look at the option however.  This is something that all sunxi should be
>>> selecting.
>>
>> What indicates that all sunxi should select CONFIG_INIT_SP_RELATIVE?
>
> config INIT_SP_RELATIVE
> bool "Specify the early stack pointer relative to the .bss 
> section"
> help
>   U-Boot typically uses a hard-coded value for the stack pointer
>   before relocation. Enable this option to instead calculate the
>   initial SP at run-time. This is useful to avoid hard-coding 
> addresses
>   into U-Boot, so that it can be loaded and executed at arbitrary
>   addresses and thus avoid using arbitrary addresses at runtime.
>
>   If this option is enabled, the early stack pointer is set to
>   &_bss_start with a offset value added. The offset is specified 
> by
>   SYS_INIT_SP_BSS_OFFSET.
>
> And that in turn tells me this is something that's done at the SoC
> level, especially for sunxi where things have been abstracted such that
> everyone (very roughly) shares the same board file, linker file, etc,
> and it's just defconfig+dts* files that change from board to board.

 I agree on that, this particular defconfig change is generic to all
 sunxi boards and we should fix this properly for the whole platform.

> So, to put it another way, what about the pine64-lts config is unique
> and causing this to be needed, but another arm64 sunxi platform would
> not?

 I think were Heinrich came from is selecting CONFIG_RSA for his
 particular build. I guess this requires a bigger malloc area. Now this
 somehow collides which how we calculate the stack pointer, but it is too
 late here to really track this down ;-)

 Heinrich, can you please give an explanation what problem
 CONFIG_INIT_SP_RELATIVE fixed and why? It seems like to cause some side
 effect, but does not sound like the proper solution (because this is
 more mean for the position independent build option).

 Cheers,
 Andre

>>>
>>> Hello André,
>>>
>>> the problem is reproducible on the FriendlyARM NanoPi NEO Plus2. So Tom
>>> is right in that we should solve it for all Allwinner ARMv8 devices.
>>
>> Thanks for testing this, and yeah, I am not surprised. As Tom mentioned,
>> all (arm64) Allwinner boards are the same in those basic aspects.
>>
>>> I simply use a nanopi_neo_plus2_defconfig or pine64-lts_defconfig and
>>> add CONFIG_RSA=y.
>>>
>>> The result is that the whenever the BL31 loads main U-Boot the board
>>> falls back to SPL. This loops forever. See below.
>>>
>>> CONFIG_RSA=y is needed for signed UEFI variables and other use cases.
>>>
>>> But the problem is not specific to RSA. You get into the same kind of
>>> problems when selecting CONFIG_LOG=y.
>>>
>>> With CONFIG_INIT_SP_RELATIVE=y the problem disappears.
>>>
>>> As this all happens before we have a U-Boot console I was not able to
>>> debug further than what I wrote in the thread
>>> https://lists.denx.de/pipermail/u-boot/2020-May/414346.html. A
>>> DEBUG_UART configuration for sunxi would have been very helpful here.
>>
>> Many thanks for the info, that looks very helpful! I wasn't sure if the
>> problem is in the SPL (because you just increased the malloc area for
>> U-Boot proper), but it looks very much like it. I will dig out some
>> debug techniques I used in the past to see what's actually going on. The
>> code size for Allwinner arm64 SPL is very limited, so we had this issue
>> before, where the stack ran into the code (if it didn't already violate

Re: [PATCH 0/2] omap4: panda: convert to device model

2020-06-06 Thread Jonathan Gray
On Tue, Jun 02, 2020 at 02:19:07PM +0300, Tero Kristo wrote:
> Hi,
> 
> As there is looming death to OMAP4 Panda board u-boot support, I decided
> to take a shot and convert it to device model myself. With these patches
> it boots up fine, and there are no DM_SPL conversion complaints during
> compile time anymore. I think USB ethernet does not work anymore with
> this, but its better than dropping the support for the board completely.
> USB itself appears working, so it should be relatively easy for someone
> to fix the networking support if they need it.
> 
> -Tero

Thanks, OpenBSD still boots on PandaBoard ES with these changes.

USB Ethernet does not seem to work before your patches on 2020.04
"Net:   No ethernet found."

-U-Boot SPL 2020.04 (May 26 2020 - 09:37:17 -0600)
+U-Boot SPL 2020.07-rc3-00212-g83e9328144 (Jun 06 2020 - 17:18:39 +1000)
 OMAP4460-GP ES1.1
 Trying to boot from MMC1
-SPL: Please implement spl_start_uboot() for your board
-SPL: Direct Linux boot not active!
+spl_load_image_fat_os: error reading image args, err - -2
 
 
-U-Boot 2020.04 (May 26 2020 - 09:37:17 -0600)
+U-Boot 2020.07-rc3-00212-g83e9328144 (Jun 06 2020 - 17:18:39 +1000)
 
 CPU  : OMAP4460-GP ES1.1
+Model: TI OMAP4 PandaBoard
 Board: OMAP4 Panda
 I2C:   ready
 DRAM:  1 GiB
@@ -23,67 +23,58 @@ SD/MMC found on device 0
 switch to partitions #0, OK
 mmc0 is current device
 Scanning mmc 0:1...
-83524 bytes read in 6 ms (13.3 MiB/s)
+83524 bytes read in 32 ms (2.5 MiB/s)
 Found EFI removable media binary efi/boot/bootarm.efi
-Scanning disks on usb...
-Disk usb0 not ready
-Disk usb1 not ready
-Disk usb2 not ready
-Disk usb3 not ready
-Scanning disks on mmc...
+Scanning disk m...@0.blk...
 ** Unrecognized filesystem type **
-MMC Device 1 not found
-MMC Device 2 not found
-MMC Device 3 not found
-Found 3 disks
 BootOrder not defined
 EFI boot manager: Cannot load any image
-105448 bytes read in 11 ms (9.1 MiB/s)
+105448 bytes read in 45 ms (2.2 MiB/s)
 disks: sd0*
 >> OpenBSD/armv7 BOOTARM 1.15
 boot>
-booting sd0a:/bsd: 4689044+688008+252268+562848-[254389+120+324848+288747]=0x0
+booting sd0a:/bsd: 4695860+688940+255424+565064\[254202+120+324848+288747]=0x0
 
 OpenBSD/armv7 booting ...


U-Boot SPL 2020.07-rc3-00212-g83e9328144 (Jun 06 2020 - 17:18:39 +1000)
OMAP4460-GP ES1.1
Trying to boot from MMC1
spl_load_image_fat_os: error reading image args, err - -2


U-Boot 2020.07-rc3-00212-g83e9328144 (Jun 06 2020 - 17:18:39 +1000)

CPU  : OMAP4460-GP ES1.1
Model: TI OMAP4 PandaBoard
Board: OMAP4 Panda
I2C:   ready
DRAM:  1 GiB
MMC:   OMAP SD/MMC: 0
Loading Environment from FAT... *** Warning - bad CRC, using default environment

Net:   No ethernet found.
Hit any key to stop autoboot:  0
## Error: "init_console" not defined
switch to partitions #0, OK
mmc0 is current device
SD/MMC found on device 0
switch to partitions #0, OK
mmc0 is current device
Scanning mmc 0:1...
83524 bytes read in 32 ms (2.5 MiB/s)
Found EFI removable media binary efi/boot/bootarm.efi
Scanning disk m...@0.blk...
** Unrecognized filesystem type **
BootOrder not defined
EFI boot manager: Cannot load any image
105448 bytes read in 45 ms (2.2 MiB/s)
disks: sd0*
>> OpenBSD/armv7 BOOTARM 1.15
boot>
booting sd0a:/bsd: 4695860+688940+255424+565064\[254202+120+324848+288747]=0x0

OpenBSD/armv7 booting ...