[PATCH 0/7] Arm Juno board OF_CONTROL upgrade

[PATCH 0/7] Arm Juno board OF_CONTROL upgrade

[Andre Przywara]

Hi,

The Juno port in U-Boot didn't see much love lately, so it has fallen
a bit behind. We already get a build warning for using an old network
driver, but there is more:
- The port is using hardcoded information, even though we have quite
  decent DTs available to find things at runtime.
- There is no support for USB or PCI, which pretty much limits the board
  to load a kernel from flash (yuck!) or TFTP (at least!).
- Probably because of this, newer features like UEFI support don't work
  properly.
- There are minor things like less-than-ideal default load addresses and
  missing reset support.

This series is the first part of fixing this. The main part is to switch
the board port to use OF_CONTROL, so U-Boot will use a DT to configure
itself at runtime. This requires some update to the PL011 driver first
(patch 2/7), and allows us to simply enable USB in the defconfig (patch
6/7). USB requires two "usb reset" calls after the initial "usb start" to
recognise any devices, not sure why this is.
But eventually I am able to load grub from a USB hard drive and do a full
featured Ubuntu UEFI boot from there (with a distro kernel).

Patches 1, 3, and 7 are mere fixes, patch 4/7 does the actual OF_CONTROL
conversion.

I also have some proper DM_PCI compliant driver in an advanced state,
which allows to load from a SATA hard disk. Unfortunately there is no
sky2 network driver in U-Boot, so the Gigabit Ethernet chip connected
to PCI will not work easily.
I will post this once this is cleaned up and tested.

Converting the smc network driver to DM_ETH is on my list as well, but
the code is shared with some U-Boot *application* code, also used by
some PowerPC boards, so that's not really a low hanging fruit.
But it would remove the deprecation warning.

Cheers,
Andre

P.S. In case you want to test this without flashing it, you can
chainload U-Boot from an existing U-Boot installation:
$ mkimage -A arm64 -O u-boot -T standalone -C none -a 0xe0000000 -e 0xe0000000
  -d u-boot.bin -n U-Boot /srv/tftp/u-boot-juno.img
VExpress64# tftpboot 0xe0000000 u-boot-juno.img
VExpress64# bootm 0xe0000000

Andre Przywara (7):
  arm: juno: Fix Juno address variables
  uart: pl011: Add proper DM clock support
  arm: juno: Fix UART clock rate
  arm: juno: Enable OF_CONTROL
  arm: juno: Use PSCI based reset
  arm: juno: enable USB
  arm: vexpress64: Remove unneeded CONFIG_ check

 arch/arm/Kconfig                       | 11 ++++++
 board/armltd/vexpress64/Kconfig        |  7 ++++
 board/armltd/vexpress64/vexpress64.c   | 61 ++++++++++++++++++++++++++++++++--
 configs/vexpress_aemv8a_juno_defconfig |  9 +++--
 drivers/serial/serial_pl01x.c          | 10 +++++-
 include/configs/vexpress_aemv8a.h      | 35 ++++++++++---------
 6 files changed, 108 insertions(+), 25 deletions(-)

-- 
2.14.5

[PATCH 1/7] arm: juno: Fix Juno address variables

[Andre Przywara]

The U-Boot documentation explains that variables ending with "_r" hold
addresses in DRAM, while those without that ending point to flash/ROM.
The default variables for the Juno board pointing to the kernel and DTB
load addresses were not complying with this scheme: they lack the
extension, but point to DRAM. This is particularly confusing since the
Juno board features parallel NOR flash, so there *is* a memory mapped
NOR address holding a DTB, for instance.

Fix the variables to use the proper names. On the way adjust the FDT
load address to be situated *before* the kernel, since users happened
to overwrite the DTB by the kernel clearing its .BSS section during
initialisation.

That fixes loading debug kernels, which happened to overwrite the DTB on
certain setups.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
---
 include/configs/vexpress_aemv8a.h | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/include/configs/vexpress_aemv8a.h b/include/configs/vexpress_aemv8a.h
index 9a9cec414c..edb08b0e68 100644
--- a/include/configs/vexpress_aemv8a.h
+++ b/include/configs/vexpress_aemv8a.h
@@ -138,35 +138,35 @@
 #define CONFIG_EXTRA_ENV_SETTINGS	\
 				"kernel_name=norkern\0"	\
 				"kernel_alt_name=Image\0"	\
-				"kernel_addr=0x80080000\0" \
+				"kernel_addr_r=0x80080000\0" \
 				"initrd_name=ramdisk.img\0"	\
-				"initrd_addr=0x84000000\0"	\
+				"initrd_addr_r=0x88000000\0"	\
 				"fdtfile=board.dtb\0" \
 				"fdt_alt_name=juno\0" \
-				"fdt_addr=0x83000000\0" \
+				"fdt_addr_r=0x80000000\0" \
 				"fdt_high=0xffffffffffffffff\0" \
 				"initrd_high=0xffffffffffffffff\0" \
 
 /* Copy the kernel and FDT to DRAM memory and boot */
-#define CONFIG_BOOTCOMMAND	"afs load ${kernel_name} ${kernel_addr} ; " \
+#define CONFIG_BOOTCOMMAND	"afs load ${kernel_name} ${kernel_addr_r} ;"\
 				"if test $? -eq 1; then "\
 				"  echo Loading ${kernel_alt_name} instead of "\
 				"${kernel_name}; "\
-				"  afs load ${kernel_alt_name} ${kernel_addr};"\
+				"  afs load ${kernel_alt_name} ${kernel_addr_r};"\
 				"fi ; "\
-				"afs load  ${fdtfile} ${fdt_addr} ; " \
+				"afs load ${fdtfile} ${fdt_addr_r} ;"\
 				"if test $? -eq 1; then "\
 				"  echo Loading ${fdt_alt_name} instead of "\
 				"${fdtfile}; "\
-				"  afs load ${fdt_alt_name} ${fdt_addr}; "\
+				"  afs load ${fdt_alt_name} ${fdt_addr_r}; "\
 				"fi ; "\
-				"fdt addr ${fdt_addr}; fdt resize; " \
-				"if afs load  ${initrd_name} ${initrd_addr} ; "\
+				"fdt addr ${fdt_addr_r}; fdt resize; " \
+				"if afs load  ${initrd_name} ${initrd_addr_r} ; "\
 				"then "\
-				"  setenv initrd_param ${initrd_addr}; "\
+				"  setenv initrd_param ${initrd_addr_r}; "\
 				"  else setenv initrd_param -; "\
 				"fi ; " \
-				"booti ${kernel_addr} ${initrd_param} ${fdt_addr}"
+				"booti ${kernel_addr_r} ${initrd_param} ${fdt_addr_r}"
 
 
 #elif CONFIG_TARGET_VEXPRESS64_BASE_FVP
-- 
2.14.5

[PATCH 1/7] arm: juno: Fix Juno address variables

[Tom Rini]

On Wed, Mar 25, 2020 at 02:46:56PM +0000, Andre Przywara wrote:

> The U-Boot documentation explains that variables ending with "_r" hold
> addresses in DRAM, while those without that ending point to flash/ROM.
> The default variables for the Juno board pointing to the kernel and DTB
> load addresses were not complying with this scheme: they lack the
> extension, but point to DRAM. This is particularly confusing since the
> Juno board features parallel NOR flash, so there *is* a memory mapped
> NOR address holding a DTB, for instance.
> 
> Fix the variables to use the proper names. On the way adjust the FDT
> load address to be situated *before* the kernel, since users happened
> to overwrite the DTB by the kernel clearing its .BSS section during
> initialisation.
> 
> That fixes loading debug kernels, which happened to overwrite the DTB on
> certain setups.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
[snip]
> -				"fdt_addr=0x83000000\0" \
> +				"fdt_addr_r=0x80000000\0" \
>  				"fdt_high=0xffffffffffffffff\0" \
>  				"initrd_high=0xffffffffffffffff\0" \

On a related note, using fdt_high=0xff... to disable relocation is a bad
idea and can lead to U-Boot knowing we have it at an invalid (unaligned)
location but doing nothing and causing problems down the chain.  Please
use bootm_size or similar (documented in the README, still) to limit
where the fdt can be.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200325/ecd7c65a/attachment.sig>

[PATCH 1/7] arm: juno: Fix Juno address variables

[André Przywara]

On 26/03/2020 02:38, Tom Rini wrote:

Hi,

> On Wed, Mar 25, 2020 at 02:46:56PM +0000, Andre Przywara wrote:
> 
>> The U-Boot documentation explains that variables ending with "_r" hold
>> addresses in DRAM, while those without that ending point to flash/ROM.
>> The default variables for the Juno board pointing to the kernel and DTB
>> load addresses were not complying with this scheme: they lack the
>> extension, but point to DRAM. This is particularly confusing since the
>> Juno board features parallel NOR flash, so there *is* a memory mapped
>> NOR address holding a DTB, for instance.
>>
>> Fix the variables to use the proper names. On the way adjust the FDT
>> load address to be situated *before* the kernel, since users happened
>> to overwrite the DTB by the kernel clearing its .BSS section during
>> initialisation.
>>
>> That fixes loading debug kernels, which happened to overwrite the DTB on
>> certain setups.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>> Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
> [snip]
>> -				"fdt_addr=0x83000000\0" \
>> +				"fdt_addr_r=0x80000000\0" \
>>  				"fdt_high=0xffffffffffffffff\0" \
>>  				"initrd_high=0xffffffffffffffff\0" \
> 
> On a related note, using fdt_high=0xff... to disable relocation is a bad
> idea and can lead to U-Boot knowing we have it at an invalid (unaligned)
> location but doing nothing and causing problems down the chain.  Please
> use bootm_size or similar (documented in the README, still) to limit
> where the fdt can be.  Thanks!

Thanks, looks like I will drop this then. arm64 kernels before 4.2 had a
limit of placing the DTB with 512MB of the kernel image, but this has
been lifted since then. I might address this later shall people complain.

On a related note: I just see we use initrd_* variables here, where
there are more users of ramdisk_addr*. Shall I fix this here as well?
Seems like only ramdisk_addr* is mentioned in README.

Cheers,
Andre

[PATCH 1/7] arm: juno: Fix Juno address variables

[Tom Rini]

On Thu, Mar 26, 2020 at 04:14:03PM +0000, Andr? Przywara wrote:
> On 26/03/2020 02:38, Tom Rini wrote:
> 
> Hi,
> 
> > On Wed, Mar 25, 2020 at 02:46:56PM +0000, Andre Przywara wrote:
> > 
> >> The U-Boot documentation explains that variables ending with "_r" hold
> >> addresses in DRAM, while those without that ending point to flash/ROM.
> >> The default variables for the Juno board pointing to the kernel and DTB
> >> load addresses were not complying with this scheme: they lack the
> >> extension, but point to DRAM. This is particularly confusing since the
> >> Juno board features parallel NOR flash, so there *is* a memory mapped
> >> NOR address holding a DTB, for instance.
> >>
> >> Fix the variables to use the proper names. On the way adjust the FDT
> >> load address to be situated *before* the kernel, since users happened
> >> to overwrite the DTB by the kernel clearing its .BSS section during
> >> initialisation.
> >>
> >> That fixes loading debug kernels, which happened to overwrite the DTB on
> >> certain setups.
> >>
> >> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> >> Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
> > [snip]
> >> -				"fdt_addr=0x83000000\0" \
> >> +				"fdt_addr_r=0x80000000\0" \
> >>  				"fdt_high=0xffffffffffffffff\0" \
> >>  				"initrd_high=0xffffffffffffffff\0" \
> > 
> > On a related note, using fdt_high=0xff... to disable relocation is a bad
> > idea and can lead to U-Boot knowing we have it at an invalid (unaligned)
> > location but doing nothing and causing problems down the chain.  Please
> > use bootm_size or similar (documented in the README, still) to limit
> > where the fdt can be.  Thanks!
> 
> Thanks, looks like I will drop this then. arm64 kernels before 4.2 had a
> limit of placing the DTB with 512MB of the kernel image, but this has
> been lifted since then. I might address this later shall people complain.

Thanks.
 
> On a related note: I just see we use initrd_* variables here, where
> there are more users of ramdisk_addr*. Shall I fix this here as well?
> Seems like only ramdisk_addr* is mentioned in README.

initrd_high is the other "do not relocate" variable.  For anything else,
yes, matching other platforms so that distro boot can be used at some
point (I assume it's not on Juno today) would be good.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200326/d2eb1be5/attachment.sig>

[PATCH 1/7] arm: juno: Fix Juno address variables

[Linus Walleij]

On Wed, Mar 25, 2020 at 3:47 PM Andre Przywara <andre.przywara@arm.com> wrote:

> The U-Boot documentation explains that variables ending with "_r" hold
> addresses in DRAM, while those without that ending point to flash/ROM.
> The default variables for the Juno board pointing to the kernel and DTB
> load addresses were not complying with this scheme: they lack the
> extension, but point to DRAM. This is particularly confusing since the
> Juno board features parallel NOR flash, so there *is* a memory mapped
> NOR address holding a DTB, for instance.
>
> Fix the variables to use the proper names. On the way adjust the FDT
> load address to be situated *before* the kernel, since users happened
> to overwrite the DTB by the kernel clearing its .BSS section during
> initialisation.
>
> That fixes loading debug kernels, which happened to overwrite the DTB on
> certain setups.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>

Makes perfect sense.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 2/7] uart: pl011: Add proper DM clock support

[Andre Przywara]

Even though the PL011 UART driver claims to be DM compliant, it does not
really a good job with parsing DT nodes. U-Boot seems to adhere to a
non-standard binding, either requiring to have a "skip-init" property in
the node, or to have an extra "clock" property holding the base
*frequency* value for the baud rate generator.
DTs in the U-Boot tree seem to have been hacked to match this
requirement.

The official binding does not mention any of these properties, instead
recommends a standard "clocks" property to point to the baud base clock.

Some boards use simple "fixed-clock" providers, which U-Boot readily
supports, so let's add some simple DM clock code to the PL011 driver to
learn the rate of the first clock, as described by the official binding.

These clock nodes seem to be not ready very early in the boot process,
so provide a fallback value, by re-using the already existing
CONFIG_PL011_CLOCK variable.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/serial/serial_pl01x.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/serial/serial_pl01x.c b/drivers/serial/serial_pl01x.c
index 2a5f256184..1ab0ccadb2 100644
--- a/drivers/serial/serial_pl01x.c
+++ b/drivers/serial/serial_pl01x.c
@@ -12,6 +12,7 @@
 
 #include <common.h>
 #include <dm.h>
+#include <clk.h>
 #include <errno.h>
 #include <watchdog.h>
 #include <asm/io.h>
@@ -340,14 +341,21 @@ static const struct udevice_id pl01x_serial_id[] ={
 int pl01x_serial_ofdata_to_platdata(struct udevice *dev)
 {
 	struct pl01x_serial_platdata *plat = dev_get_platdata(dev);
+	struct clk clk;
 	fdt_addr_t addr;
+	int ret;
 
 	addr = devfdt_get_addr(dev);
 	if (addr == FDT_ADDR_T_NONE)
 		return -EINVAL;
 
 	plat->base = addr;
-	plat->clock = dev_read_u32_default(dev, "clock", 1);
+	plat->clock = dev_read_u32_default(dev, "clock", CONFIG_PL011_CLOCK);
+	ret = clk_get_by_index(dev, 0, &clk);
+	if (!ret) {
+		clk_enable(&clk);
+		plat->clock = clk_get_rate(&clk);
+	}
 	plat->type = dev_get_driver_data(dev);
 	plat->skip_init = dev_read_bool(dev, "skip-init");
 
-- 
2.14.5

[PATCH 2/7] uart: pl011: Add proper DM clock support

[Simon Glass]

Hi Andre,

On Wed, 25 Mar 2020 at 08:47, Andre Przywara <andre.przywara@arm.com> wrote:
>
> Even though the PL011 UART driver claims to be DM compliant, it does not
> really a good job with parsing DT nodes. U-Boot seems to adhere to a
> non-standard binding, either requiring to have a "skip-init" property in
> the node, or to have an extra "clock" property holding the base
> *frequency* value for the baud rate generator.
> DTs in the U-Boot tree seem to have been hacked to match this
> requirement.
>
> The official binding does not mention any of these properties, instead
> recommends a standard "clocks" property to point to the baud base clock.
>
> Some boards use simple "fixed-clock" providers, which U-Boot readily
> supports, so let's add some simple DM clock code to the PL011 driver to
> learn the rate of the first clock, as described by the official binding.
>
> These clock nodes seem to be not ready very early in the boot process,
> so provide a fallback value, by re-using the already existing
> CONFIG_PL011_CLOCK variable.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  drivers/serial/serial_pl01x.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/serial/serial_pl01x.c b/drivers/serial/serial_pl01x.c
> index 2a5f256184..1ab0ccadb2 100644
> --- a/drivers/serial/serial_pl01x.c
> +++ b/drivers/serial/serial_pl01x.c
> @@ -12,6 +12,7 @@
>
>  #include <common.h>
>  #include <dm.h>
> +#include <clk.h>
>  #include <errno.h>
>  #include <watchdog.h>
>  #include <asm/io.h>
> @@ -340,14 +341,21 @@ static const struct udevice_id pl01x_serial_id[] ={
>  int pl01x_serial_ofdata_to_platdata(struct udevice *dev)
>  {
>         struct pl01x_serial_platdata *plat = dev_get_platdata(dev);
> +       struct clk clk;
>         fdt_addr_t addr;
> +       int ret;
>
>         addr = devfdt_get_addr(dev);
>         if (addr == FDT_ADDR_T_NONE)
>                 return -EINVAL;
>
>         plat->base = addr;
> -       plat->clock = dev_read_u32_default(dev, "clock", 1);
> +       plat->clock = dev_read_u32_default(dev, "clock", CONFIG_PL011_CLOCK);

is this needed?

> +       ret = clk_get_by_index(dev, 0, &clk);
> +       if (!ret) {
> +               clk_enable(&clk);
> +               plat->clock = clk_get_rate(&clk);
> +       }
>         plat->type = dev_get_driver_data(dev);
>         plat->skip_init = dev_read_bool(dev, "skip-init");
>
> --
> 2.14.5
>

Regards,
Simon

[PATCH 2/7] uart: pl011: Add proper DM clock support

[André Przywara]

On 26/03/2020 16:20, Simon Glass wrote:

Hi Simon,

> On Wed, 25 Mar 2020 at 08:47, Andre Przywara <andre.przywara@arm.com> wrote:
>>
>> Even though the PL011 UART driver claims to be DM compliant, it does not
>> really a good job with parsing DT nodes. U-Boot seems to adhere to a
>> non-standard binding, either requiring to have a "skip-init" property in
>> the node, or to have an extra "clock" property holding the base
>> *frequency* value for the baud rate generator.
>> DTs in the U-Boot tree seem to have been hacked to match this
>> requirement.
>>
>> The official binding does not mention any of these properties, instead
>> recommends a standard "clocks" property to point to the baud base clock.
>>
>> Some boards use simple "fixed-clock" providers, which U-Boot readily
>> supports, so let's add some simple DM clock code to the PL011 driver to
>> learn the rate of the first clock, as described by the official binding.
>>
>> These clock nodes seem to be not ready very early in the boot process,
>> so provide a fallback value, by re-using the already existing
>> CONFIG_PL011_CLOCK variable.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>> ---
>>  drivers/serial/serial_pl01x.c | 10 +++++++++-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/serial/serial_pl01x.c b/drivers/serial/serial_pl01x.c
>> index 2a5f256184..1ab0ccadb2 100644
>> --- a/drivers/serial/serial_pl01x.c
>> +++ b/drivers/serial/serial_pl01x.c
>> @@ -12,6 +12,7 @@
>>
>>  #include <common.h>
>>  #include <dm.h>
>> +#include <clk.h>
>>  #include <errno.h>
>>  #include <watchdog.h>
>>  #include <asm/io.h>
>> @@ -340,14 +341,21 @@ static const struct udevice_id pl01x_serial_id[] ={
>>  int pl01x_serial_ofdata_to_platdata(struct udevice *dev)
>>  {
>>         struct pl01x_serial_platdata *plat = dev_get_platdata(dev);
>> +       struct clk clk;
>>         fdt_addr_t addr;
>> +       int ret;
>>
>>         addr = devfdt_get_addr(dev);
>>         if (addr == FDT_ADDR_T_NONE)
>>                 return -EINVAL;
>>
>>         plat->base = addr;
>> -       plat->clock = dev_read_u32_default(dev, "clock", 1);
>> +       plat->clock = dev_read_u32_default(dev, "clock", CONFIG_PL011_CLOCK);
> 
> is this needed?

This is to provide the existing behaviour as a fallback. Some SoCs have
a complex clock providing the baud rate clock (HiKey 960, FSL LS2080a),
which U-Boot doesn't suport. I'd rather not break them, but also don't
really want to provide a clock driver ;-)

Also this mimics the !DM_SERIAL behaviour, which sets this clock rate
based on Kconfig, again as a fallback. I needed that because I think the
clock driver wasn't ready that early. It's a bit hard to confirm without
serial output ;-)

So the order should be:
- If there is a clocks property and we support that clock provider
(fixed-clock), then use that value.
- If not, check for a "clock" property in the DT node and use that value.
- If there is no "clock property", use the Kconfig variable.

Just written the other way around in the code.

Does this make sense?

Cheers,
Andre.

>> +       ret = clk_get_by_index(dev, 0, &clk);
>> +       if (!ret) {
>> +               clk_enable(&clk);
>> +               plat->clock = clk_get_rate(&clk);
>> +       }
>>         plat->type = dev_get_driver_data(dev);
>>         plat->skip_init = dev_read_bool(dev, "skip-init");
>>
>> --
>> 2.14.5
>>
> 
> Regards,
> Simon
> 

[PATCH 2/7] uart: pl011: Add proper DM clock support

[Simon Glass]

Hi Andre,

On Thu, 26 Mar 2020 at 11:06, Andr? Przywara <andre.przywara@arm.com> wrote:
>
> On 26/03/2020 16:20, Simon Glass wrote:
>
> Hi Simon,
>
> > On Wed, 25 Mar 2020 at 08:47, Andre Przywara <andre.przywara@arm.com> wrote:
> >>
> >> Even though the PL011 UART driver claims to be DM compliant, it does not
> >> really a good job with parsing DT nodes. U-Boot seems to adhere to a
> >> non-standard binding, either requiring to have a "skip-init" property in
> >> the node, or to have an extra "clock" property holding the base
> >> *frequency* value for the baud rate generator.
> >> DTs in the U-Boot tree seem to have been hacked to match this
> >> requirement.
> >>
> >> The official binding does not mention any of these properties, instead
> >> recommends a standard "clocks" property to point to the baud base clock.
> >>
> >> Some boards use simple "fixed-clock" providers, which U-Boot readily
> >> supports, so let's add some simple DM clock code to the PL011 driver to
> >> learn the rate of the first clock, as described by the official binding.
> >>
> >> These clock nodes seem to be not ready very early in the boot process,
> >> so provide a fallback value, by re-using the already existing
> >> CONFIG_PL011_CLOCK variable.
> >>
> >> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> >> ---
> >>  drivers/serial/serial_pl01x.c | 10 +++++++++-
> >>  1 file changed, 9 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/serial/serial_pl01x.c b/drivers/serial/serial_pl01x.c
> >> index 2a5f256184..1ab0ccadb2 100644
> >> --- a/drivers/serial/serial_pl01x.c
> >> +++ b/drivers/serial/serial_pl01x.c
> >> @@ -12,6 +12,7 @@
> >>
> >>  #include <common.h>
> >>  #include <dm.h>
> >> +#include <clk.h>
> >>  #include <errno.h>
> >>  #include <watchdog.h>
> >>  #include <asm/io.h>
> >> @@ -340,14 +341,21 @@ static const struct udevice_id pl01x_serial_id[] ={
> >>  int pl01x_serial_ofdata_to_platdata(struct udevice *dev)
> >>  {
> >>         struct pl01x_serial_platdata *plat = dev_get_platdata(dev);
> >> +       struct clk clk;
> >>         fdt_addr_t addr;
> >> +       int ret;
> >>
> >>         addr = devfdt_get_addr(dev);
> >>         if (addr == FDT_ADDR_T_NONE)
> >>                 return -EINVAL;
> >>
> >>         plat->base = addr;
> >> -       plat->clock = dev_read_u32_default(dev, "clock", 1);
> >> +       plat->clock = dev_read_u32_default(dev, "clock", CONFIG_PL011_CLOCK);
> >
> > is this needed?
>
> This is to provide the existing behaviour as a fallback. Some SoCs have
> a complex clock providing the baud rate clock (HiKey 960, FSL LS2080a),
> which U-Boot doesn't suport. I'd rather not break them, but also don't
> really want to provide a clock driver ;-)
>
> Also this mimics the !DM_SERIAL behaviour, which sets this clock rate
> based on Kconfig, again as a fallback. I needed that because I think the
> clock driver wasn't ready that early. It's a bit hard to confirm without
> serial output ;-)
>
> So the order should be:
> - If there is a clocks property and we support that clock provider
> (fixed-clock), then use that value.
> - If not, check for a "clock" property in the DT node and use that value.
> - If there is no "clock property", use the Kconfig variable.
>
> Just written the other way around in the code.
>
> Does this make sense?

Hmm it might make more sense to migrate the boards?

Reviewed-by: Simon Glass <sjg@chromium.org>

Regards,
Simon

[PATCH 2/7] uart: pl011: Add proper DM clock support

[Linus Walleij]

On Wed, Mar 25, 2020 at 3:47 PM Andre Przywara <andre.przywara@arm.com> wrote:

> Even though the PL011 UART driver claims to be DM compliant, it does not
> really a good job with parsing DT nodes. U-Boot seems to adhere to a
> non-standard binding, either requiring to have a "skip-init" property in
> the node, or to have an extra "clock" property holding the base
> *frequency* value for the baud rate generator.
> DTs in the U-Boot tree seem to have been hacked to match this
> requirement.
>
> The official binding does not mention any of these properties, instead
> recommends a standard "clocks" property to point to the baud base clock.
>
> Some boards use simple "fixed-clock" providers, which U-Boot readily
> supports, so let's add some simple DM clock code to the PL011 driver to
> learn the rate of the first clock, as described by the official binding.
>
> These clock nodes seem to be not ready very early in the boot process,
> so provide a fallback value, by re-using the already existing
> CONFIG_PL011_CLOCK variable.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

A good start for other platforms to follow.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 3/7] arm: juno: Fix UART clock rate

[Andre Przywara]

The UART base clock rate was typo-ed in the header file, probably because
the reference (the Linux .dts) was also wrong[1].

Fix the number to make the baud rate more correct.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=39a1a8941b2
---
 include/configs/vexpress_aemv8a.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/configs/vexpress_aemv8a.h b/include/configs/vexpress_aemv8a.h
index edb08b0e68..d788d21acf 100644
--- a/include/configs/vexpress_aemv8a.h
+++ b/include/configs/vexpress_aemv8a.h
@@ -102,7 +102,7 @@
 
 /* PL011 Serial Configuration */
 #ifdef CONFIG_TARGET_VEXPRESS64_JUNO
-#define CONFIG_PL011_CLOCK		7273800
+#define CONFIG_PL011_CLOCK		7372800
 #else
 #define CONFIG_PL011_CLOCK		24000000
 #endif
-- 
2.14.5

[PATCH 3/7] arm: juno: Fix UART clock rate

[Linus Walleij]

On Wed, Mar 25, 2020 at 3:47 PM Andre Przywara <andre.przywara@arm.com> wrote:

> The UART base clock rate was typo-ed in the header file, probably because
> the reference (the Linux .dts) was also wrong[1].
>
> Fix the number to make the baud rate more correct.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 4/7] arm: juno: Enable OF_CONTROL

[Andre Przywara]

The Arm Juno board was still somewhat stuck in "hardcoded land", even
though there are stable DTs around, and one happens to actually be on
the memory mapped NOR flash.

Enable the configuration options to let the board use OF_CONTROL, and
add a routine to find the address of the DTB partition in NOR
flash, to use that for U-Boot's own purposes.
This can also passed on via $fdtcontroladdr to any kernel or EFI
application, removing the need to actually load a device tree.

Since the existing "afs" command and its flash routines require
flash_init() to be called before being usable, and this is done much
later in the boot process, we introduce a stripped-down partition finder
routine in vexpress64.c, to scan the NOR flash partitions for the
DT partition. This location is then used for U-Boot to find and probe
devices.

The name of the partition can be configured, if needed, but defaults
to "board.dtb", which is used by Linaro's firmware image provided.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 arch/arm/Kconfig                       |  5 +++
 board/armltd/vexpress64/Kconfig        |  7 +++++
 board/armltd/vexpress64/vexpress64.c   | 57 ++++++++++++++++++++++++++++++++++
 configs/vexpress_aemv8a_juno_defconfig |  4 +--
 4 files changed, 70 insertions(+), 3 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 5d367888d8..2d3e79bf52 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1127,6 +1127,11 @@ config TARGET_VEXPRESS64_JUNO
 	bool "Support Versatile Express Juno Development Platform"
 	select ARM64
 	select PL01X_SERIAL
+	select DM
+	select OF_CONTROL
+	select OF_BOARD
+	select CLK
+	select DM_SERIAL
 
 config TARGET_LS2080A_EMU
 	bool "Support ls2080a_emu"
diff --git a/board/armltd/vexpress64/Kconfig b/board/armltd/vexpress64/Kconfig
index 9014418433..1d13f542e6 100644
--- a/board/armltd/vexpress64/Kconfig
+++ b/board/armltd/vexpress64/Kconfig
@@ -9,4 +9,11 @@ config SYS_VENDOR
 config SYS_CONFIG_NAME
 	default "vexpress_aemv8a"
 
+config JUNO_DTB_PART
+	string "NOR flash partition holding DTB"
+	default "board.dtb"
+	help
+	  The ARM partition name in the NOR flash memory holding the
+	  device tree blob to configure U-Boot.
+
 endif
diff --git a/board/armltd/vexpress64/vexpress64.c b/board/armltd/vexpress64/vexpress64.c
index dd0ebdd303..ba49b32e58 100644
--- a/board/armltd/vexpress64/vexpress64.c
+++ b/board/armltd/vexpress64/vexpress64.c
@@ -82,6 +82,63 @@ int dram_init_banksize(void)
 	return 0;
 }
 
+#ifdef CONFIG_OF_BOARD
+#define JUNO_FLASH_SEC_SIZE	(256 * 1024)
+static phys_addr_t find_dtb_in_nor_flash(const char *partname)
+{
+	phys_addr_t sector = CONFIG_SYS_FLASH_BASE;
+	int i;
+
+	for (i = 0;
+	     i < CONFIG_SYS_MAX_FLASH_SECT;
+	     i++, sector += JUNO_FLASH_SEC_SIZE) {
+		int len = strlen(partname) + 1;
+		int offs;
+		phys_addr_t imginfo;
+		u32 reg;
+
+		reg = readl(sector + JUNO_FLASH_SEC_SIZE - 0x04);
+                /* This makes up the string "HSLFTOOF" flash footer */
+		if (reg != 0x464F4F54U)
+			continue;
+		reg = readl(sector + JUNO_FLASH_SEC_SIZE - 0x08);
+                if (reg != 0x464C5348U)
+			continue;
+
+		for (offs = 0; offs < 32; offs += 4, len -= 4) {
+			reg = readl(sector + JUNO_FLASH_SEC_SIZE - 0x30 + offs);
+			if (strncmp(partname + offs, (char *)&reg,
+			            len > 4 ? 4 : len))
+				break;
+
+			if (len > 4)
+				continue;
+
+			reg = readl(sector + JUNO_FLASH_SEC_SIZE - 0x10);
+			imginfo = sector + JUNO_FLASH_SEC_SIZE - 0x30 - reg;
+			reg = readl(imginfo + 0x54);
+
+			return CONFIG_SYS_FLASH_BASE +
+			       reg * JUNO_FLASH_SEC_SIZE;
+		}
+	}
+
+	printf("No DTB found\n");
+
+	return ~0;
+}
+
+void *board_fdt_blob_setup(void)
+{
+	phys_addr_t fdt_rom_addr = find_dtb_in_nor_flash(CONFIG_JUNO_DTB_PART);
+
+	if (fdt_rom_addr == ~0UL)
+		return NULL;
+
+	return (void *)fdt_rom_addr;
+}
+#endif
+
 /*
  * Board specific reset that is system reset.
  */
diff --git a/configs/vexpress_aemv8a_juno_defconfig b/configs/vexpress_aemv8a_juno_defconfig
index 8628d05e68..6cb21e7a1b 100644
--- a/configs/vexpress_aemv8a_juno_defconfig
+++ b/configs/vexpress_aemv8a_juno_defconfig
@@ -10,6 +10,7 @@ CONFIG_DISTRO_DEFAULTS=y
 CONFIG_BOOTDELAY=1
 CONFIG_USE_BOOTARGS=y
 CONFIG_BOOTARGS="console=ttyAMA0,115200n8 root=/dev/sda2 rw rootwait earlycon=pl011,0x7ff80000 debug user_debug=31 androidboot.hardware=juno loglevel=9"
+CONFIG_OF_BOARD=y
 # CONFIG_USE_BOOTCOMMAND is not set
 # CONFIG_DISPLAY_CPUINFO is not set
 # CONFIG_DISPLAY_BOARDINFO is not set
@@ -30,7 +31,6 @@ CONFIG_CMD_UBI=y
 # CONFIG_EFI_PARTITION is not set
 CONFIG_ENV_IS_IN_FLASH=y
 CONFIG_ENV_ADDR=0xBFC0000
-CONFIG_DM=y
 # CONFIG_MMC is not set
 CONFIG_MTD=y
 CONFIG_MTD_NOR_FLASH=y
@@ -41,5 +41,3 @@ CONFIG_SYS_FLASH_CFI=y
 CONFIG_SMC911X=y
 CONFIG_SMC911X_BASE=0x018000000
 CONFIG_SMC911X_32_BIT=y
-CONFIG_DM_SERIAL=y
-CONFIG_OF_LIBFDT=y
-- 
2.14.5

[PATCH 4/7] arm: juno: Enable OF_CONTROL

[Linus Walleij]

On Wed, Mar 25, 2020 at 3:47 PM Andre Przywara <andre.przywara@arm.com> wrote:

> The Arm Juno board was still somewhat stuck in "hardcoded land", even
> though there are stable DTs around, and one happens to actually be on
> the memory mapped NOR flash.
>
> Enable the configuration options to let the board use OF_CONTROL, and
> add a routine to find the address of the DTB partition in NOR
> flash, to use that for U-Boot's own purposes.
> This can also passed on via $fdtcontroladdr to any kernel or EFI
> application, removing the need to actually load a device tree.
>
> Since the existing "afs" command and its flash routines require
> flash_init() to be called before being usable, and this is done much
> later in the boot process, we introduce a stripped-down partition finder
> routine in vexpress64.c, to scan the NOR flash partitions for the
> DT partition. This location is then used for U-Boot to find and probe
> devices.
>
> The name of the partition can be configured, if needed, but defaults
> to "board.dtb", which is used by Linaro's firmware image provided.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

It's a bit of duplication but what can we do. We need to inspect the
flash to find the DTB that defines where the flash is. We can certainly
live with this bootstrapping as a compromise.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 5/7] arm: juno: Use PSCI based reset

[Andre Przywara]

So far the Juno board wasn't implementing reset. Let's just use the
already existing PSCI_RESET based method to avoid any extra code.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Acked-by: Liviu Dudau <liviu.dudau@arm.com>
---
 arch/arm/Kconfig                     | 2 ++
 board/armltd/vexpress64/vexpress64.c | 4 +---
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 2d3e79bf52..f5c8ae1b8d 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1132,6 +1132,8 @@ config TARGET_VEXPRESS64_JUNO
 	select OF_BOARD
 	select CLK
 	select DM_SERIAL
+	select ARM_PSCI_FW
+	select PSCI_RESET
 
 config TARGET_LS2080A_EMU
 	bool "Support ls2080a_emu"
diff --git a/board/armltd/vexpress64/vexpress64.c b/board/armltd/vexpress64/vexpress64.c
index ba49b32e58..5c7a8f55f0 100644
--- a/board/armltd/vexpress64/vexpress64.c
+++ b/board/armltd/vexpress64/vexpress64.c
@@ -139,9 +139,7 @@ void *board_fdt_blob_setup(void)
 }
 #endif
 
-/*
- * Board specific reset that is system reset.
- */
+/* Actual reset is done via PSCI. */
 void reset_cpu(ulong addr)
 {
 }
-- 
2.14.5

[PATCH 5/7] arm: juno: Use PSCI based reset

[Linus Walleij]

On Wed, Mar 25, 2020 at 3:47 PM Andre Przywara <andre.przywara@arm.com> wrote:

> So far the Juno board wasn't implementing reset. Let's just use the
> already existing PSCI_RESET based method to avoid any extra code.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> Acked-by: Liviu Dudau <liviu.dudau@arm.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 6/7] arm: juno: enable USB

[Andre Przywara]

The Juno board features a standard compliant EHCI/OHCI USB host
controller pair, which we can just enable.
The platform data is taken from the device tree.

This allows to use USB mass storage (the only storage on a Juno r0)
for loading.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Acked-by: Liviu Dudau <liviu.dudau@arm.com>
---
 arch/arm/Kconfig                       | 4 ++++
 configs/vexpress_aemv8a_juno_defconfig | 5 +++++
 include/configs/vexpress_aemv8a.h      | 5 +++++
 3 files changed, 14 insertions(+)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index f5c8ae1b8d..dad7596683 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1134,6 +1134,10 @@ config TARGET_VEXPRESS64_JUNO
 	select DM_SERIAL
 	select ARM_PSCI_FW
 	select PSCI_RESET
+	select DM
+	select BLK
+	select USB
+	select DM_USB
 
 config TARGET_LS2080A_EMU
 	bool "Support ls2080a_emu"
diff --git a/configs/vexpress_aemv8a_juno_defconfig b/configs/vexpress_aemv8a_juno_defconfig
index 6cb21e7a1b..ca7aa5ab02 100644
--- a/configs/vexpress_aemv8a_juno_defconfig
+++ b/configs/vexpress_aemv8a_juno_defconfig
@@ -27,6 +27,7 @@ CONFIG_CMD_ARMFLASH=y
 CONFIG_CMD_CACHE=y
 # CONFIG_CMD_MISC is not set
 CONFIG_CMD_UBI=y
+CONFIG_CMD_USB=y
 # CONFIG_ISO_PARTITION is not set
 # CONFIG_EFI_PARTITION is not set
 CONFIG_ENV_IS_IN_FLASH=y
@@ -41,3 +42,7 @@ CONFIG_SYS_FLASH_CFI=y
 CONFIG_SMC911X=y
 CONFIG_SMC911X_BASE=0x018000000
 CONFIG_SMC911X_32_BIT=y
+CONFIG_USB_EHCI_HCD=y
+CONFIG_USB_EHCI_GENERIC=y
+CONFIG_USB_OHCI_HCD=y
+CONFIG_USB_OHCI_GENERIC=y
diff --git a/include/configs/vexpress_aemv8a.h b/include/configs/vexpress_aemv8a.h
index d788d21acf..b02caae96c 100644
--- a/include/configs/vexpress_aemv8a.h
+++ b/include/configs/vexpress_aemv8a.h
@@ -211,6 +211,11 @@
 #define CONFIG_SYS_FLASH_CFI_WIDTH	FLASH_CFI_32BIT
 #define CONFIG_SYS_MAX_FLASH_BANKS	1
 
+#ifdef CONFIG_USB_EHCI_HCD
+#define CONFIG_USB_OHCI_NEW
+#define CONFIG_SYS_USB_OHCI_MAX_ROOT_PORTS 1
+#endif
+
 #define CONFIG_SYS_FLASH_EMPTY_INFO	/* flinfo indicates empty blocks */
 #define FLASH_MAX_SECTOR_SIZE		0x00040000
 
-- 
2.14.5

[PATCH 6/7] arm: juno: enable USB

[Linus Walleij]

On Wed, Mar 25, 2020 at 3:47 PM Andre Przywara <andre.przywara@arm.com> wrote:

> The Juno board features a standard compliant EHCI/OHCI USB host
> controller pair, which we can just enable.
> The platform data is taken from the device tree.
>
> This allows to use USB mass storage (the only storage on a Juno r0)
> for loading.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> Acked-by: Liviu Dudau <liviu.dudau@arm.com>

Excellent.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 7/7] arm: vexpress64: Remove unneeded CONFIG_ check

[Andre Przywara]

CONFIG_SEMIHOSTING is selected for the VFP target by the means of
Kconfig already, there is no need to check this in the header file.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 include/configs/vexpress_aemv8a.h | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/include/configs/vexpress_aemv8a.h b/include/configs/vexpress_aemv8a.h
index b02caae96c..adbe886ae5 100644
--- a/include/configs/vexpress_aemv8a.h
+++ b/include/configs/vexpress_aemv8a.h
@@ -7,12 +7,6 @@
 #ifndef __VEXPRESS_AEMV8A_H
 #define __VEXPRESS_AEMV8A_H
 
-#ifdef CONFIG_TARGET_VEXPRESS64_BASE_FVP
-#ifndef CONFIG_SEMIHOSTING
-#error CONFIG_TARGET_VEXPRESS64_BASE_FVP requires CONFIG_SEMIHOSTING
-#endif
-#endif
-
 #define CONFIG_REMAKE_ELF
 
 /* Link Definitions */
-- 
2.14.5

[PATCH 7/7] arm: vexpress64: Remove unneeded CONFIG_ check

[Linus Walleij]

On Wed, Mar 25, 2020 at 3:47 PM Andre Przywara <andre.przywara@arm.com> wrote:

> CONFIG_SEMIHOSTING is selected for the VFP target by the means of
> Kconfig already, there is no need to check this in the header file.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij