Re: [U-Boot] [PATCH] spl: fix debug print in spl_common_init()

[Dr. Philipp Tomsich]

> On 13 Aug 2018, at 11:24, Simon Goldschmidt  
> wrote:
> 
> spl_common_init() debug-prints "spl_early_init()\n" but it is
> called both from spl_early_init() and spl_init().
> 
> Fix this by moving the debug() statement to the calling functions
> which now print their name.
> 
> Signed-off-by: Simon Goldschmidt 

Reviewed-by: Philipp Tomsich 

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] kconfig: fix typo 'parition'

[Dr. Philipp Tomsich]

> On 13 Aug 2018, at 11:34, Simon Goldschmidt  
> wrote:
> 
> %s/parition/partition

Make this a whole sentence to make it easier to read.

> 
> Signed-off-by: Simon Goldschmidt 

Reviewed-by: Philipp Tomsich 

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 3/3] bootcount: add DM-based backing store for bootcount

[Dr. Philipp Tomsich]

Lukasz,

> On 14 Aug 2018, at 13:10, Lukasz Majewski  wrote:
> 
> Hi Philipp,
> 
>> The original bootcount methods do not provide an interface to DM and
>> rely on a static configuration for I2C devices (e.g. bus, chip-addr,
>> etc. are configured through defines statically).  On a modern system
>> that exposes multiple devices in a DTS-configurable way, this is less
>> than optimal and a interface to DM-based devices will be desirable.
>> 
>> This adds a simple driver that is DM-aware and configurable via DTS:
>> the /chosen/u-boot,bootcount-device property is used to detect the DM
>> device storing the bootcount and deviceclass-specific commands are
>> used to read/write the bootcount.
>> 
>> Initially, this provides support for the following DM devices:
>> * RTC devices implementing the read8/write8 ops
>> 
>> Signed-off-by: Philipp Tomsich 
>> Tested-by: Klaus Goger 
>> ---
>> 
>> doc/device-tree-bindings/chosen.txt | 27 +++
>> drivers/bootcount/Kconfig   | 12 +
>> drivers/bootcount/Makefile  |  1 +
>> drivers/bootcount/bootcount_dm.c| 93
>> + 4 files changed, 133
>> insertions(+) create mode 100644 drivers/bootcount/bootcount_dm.c
>> 
>> diff --git a/doc/device-tree-bindings/chosen.txt
>> b/doc/device-tree-bindings/chosen.txt index da7b4e6..734fd15 100644
>> --- a/doc/device-tree-bindings/chosen.txt
>> +++ b/doc/device-tree-bindings/chosen.txt
>> @@ -42,6 +42,33 @@ Example
>>  };
>> };
>> 
>> +u-boot,bootcount-device property
>> +
>> +In a DM-based system, the bootcount may be stored in a device known
>> to +the DM framework (e.g. in a battery-backed SRAM area within a RTC
>> +device).  To identify the device to be used for the bootcount, the
>> +u-boot,bootcount-device property needs to point to the target device.
>> +
>> +Further configuration in the target device's node may be required
>> +(e.g. an offset into an I2C RTC's address space), depending on the
>> +UCLASS of the target device.
>> +
>> +Example
>> +---
>> +/ {
>> +chosen {
>> +u-boot,bootcount-device = &rv3029;
>> +};
>> +
>> +i2c2 {
>> +rv3029: rtc@56 {
>> +compatible = "mc,rv3029";
>> +reg = <0x56>;
>> +u-boot,bootcount-offset = <0x38>;
>> +};
>> +};
>> +};
>> +
>> u-boot,spl-boot-order property
>> --
>> 
>> diff --git a/drivers/bootcount/Kconfig b/drivers/bootcount/Kconfig
>> index d335ed1..cdde7b5 100644
>> --- a/drivers/bootcount/Kconfig
>> +++ b/drivers/bootcount/Kconfig
>> @@ -70,6 +70,18 @@ config BOOTCOUNT_AT91
>>  bool "Boot counter for Atmel AT91SAM9XE"
>>  depends on AT91SAM9XE
>> 
>> +config BOOTCOUNT_DM
>> +bool "Boot counter in a device-model device"
>> +help
>> +  Enables reading/writing the bootcount in a device-model
>> +  device referenced from the /chosen node.  The type of the
>> +  device is detected from DM and any additional configuration
>> +  information (e.g. the offset into a RTC device that
>> supports
>> +  read32/write32) is read from the device's node.
>> +
>> +  At this time the following DM device classes are supported:
>> +   * RTC (using read32/write32)
>> +
>> endchoice
>> 
>> config BOOTCOUNT_ALEN
>> diff --git a/drivers/bootcount/Makefile b/drivers/bootcount/Makefile
>> index 68bc006..e8ed729 100644
>> --- a/drivers/bootcount/Makefile
>> +++ b/drivers/bootcount/Makefile
>> @@ -7,3 +7,4 @@ obj-$(CONFIG_BOOTCOUNT_RAM)  += bootcount_ram.o
>> obj-$(CONFIG_BOOTCOUNT_ENV)  += bootcount_env.o
>> obj-$(CONFIG_BOOTCOUNT_I2C)  += bootcount_i2c.o
>> obj-$(CONFIG_BOOTCOUNT_EXT)  += bootcount_ext.o
>> +obj-$(CONFIG_BOOTCOUNT_DM)  += bootcount_dm.o
>> diff --git a/drivers/bootcount/bootcount_dm.c
>> b/drivers/bootcount/bootcount_dm.c new file mode 100644
>> index 000..99bdb88
>> --- /dev/null
>> +++ b/drivers/bootcount/bootcount_dm.c
>> @@ -0,0 +1,93 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * (C) Copyright 2018 Theobroma Systems Design und Consulting GmbH
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +const u8 bootcount_magic = 0xbc;
>> +
>> +static void bootcount_store_rtc(struct udevice *dev, ulong a)
>> +{
>> +#if CONFIG_IS_ENABLED(DM_RTC)
>> +u32 offset;
>> +const char *offset_propname = "u-boot,bootcount-offset";
>> +const u16 val = bootcount_magic << 8 | (a & 0xff);
>> +
>> +if (dev_read_u32(dev, offset_propname, &offset) < 0) {
>> +debug("%s: requires %s\n", __func__,
>> offset_propname);
>> +return;
>> +}
>> +
>> +if (rtc_write16(dev, offset, val) < 0) {
>> +debug("%s: rtc_write16 failed\n", __func__);
>> +return;
>> +}
>> +#endif
>> +}
>> +
>> +static u32 bootcount_load_rtc(struct udevice *dev)
>> +{
>> +#if CONFIG_IS_ENABLED(DM_RTC)
>> +u32 offset;
>> +co

Re: [U-Boot] [PATCH 1/3] rtc: rv3029: add to Kconfig

[Dr. Philipp Tomsich]

> On 14 Aug 2018, at 18:46, Heinrich Schuchardt  wrote:
> 
> On 08/14/2018 11:51 AM, Philipp Tomsich wrote:
>> The MicroCrystal RV3029 driver didn't have a Kconfig entry and was not used
>> anywhere. Add it to Kconfig to make it selectable.
>> 
>> Signed-off-by: Philipp Tomsich 
>> Tested-by: Klaus Goger 
>> ---
>> 
>> drivers/rtc/Kconfig | 10 ++
>> 1 file changed, 10 insertions(+)
>> 
>> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
>> index 5436509..8a6e796 100644
>> --- a/drivers/rtc/Kconfig
>> +++ b/drivers/rtc/Kconfig
>> @@ -42,6 +42,16 @@ config RTC_ISL1208
>>This driver supports reading and writing the RTC/calendar and detects
>>total power failures.
>> 
>> +config RTC_RV3029
>> +bool "Enable RV3029 driver"
>> +depends on DM_RTC
>> +help
>> +  The MicroCrystal RV3029 is a I2C Real Time Clock (RTC) with 8-byte
> 
> %s/a I2C/an I2C/
> 
>> +  battery-backed SRAM.
>> +
>> +  This driver supports reading and writing the RTC/calendar and the
> 
> What do you mean by calendar? If you just mean the RTC date register,
> just drop the misleading word.
> 
> %s/RTC\/calendar/RTC/

The RV3029 datasheet also describes the devices as a RTC/calendar:
"The RV-3029 is a CMOS low power, real-time clock/calendar module…”

In the same Kconfig, there’s the same duality (i.e. RTC _and_ calendar) in the
help text for the
RTC_PCF2127
RTC_ISL1208

Happy to drop this here, if that’s the consensus...

> 
>> +  battery-baced SRAM section.
> 
> %s/baced/backed/
> 
> Best regards
> 
> Heinrich
> 
>> +
>> config RTC_RX8010SJ
>>  bool "Enable RX8010SJ driver"
>>  depends on DM_RTC
>> 
> 

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v2] kconfig: fix typo 'parition'

[Dr. Philipp Tomsich]

> On 16 Aug 2018, at 09:44, Simon Goldschmidt  
> wrote:
> 
> Replaced misspelled words "parition"/"paritioning" (missing 't') in two
> Kconfig files by correct words "partition"/"partitioning"
> 
> Signed-off-by: Simon Goldschmidt 

Reviewed-by: Philipp Tomsich 

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 3/3] bootcount: add DM-based backing store for bootcount

[Dr. Philipp Tomsich]

Simon,

> On 17 Aug 2018, at 14:49, Simon Glass  wrote:
> 
> Hi Philipp,
> 
> On 14 August 2018 at 05:39, Dr. Philipp Tomsich
>  <mailto:philipp.toms...@theobroma-systems.com>> wrote:
>> Lukasz,
>> 
>>> On 14 Aug 2018, at 13:10, Lukasz Majewski  wrote:
>>> 
>>> Hi Philipp,
>>> 
>>>> The original bootcount methods do not provide an interface to DM and
>>>> rely on a static configuration for I2C devices (e.g. bus, chip-addr,
>>>> etc. are configured through defines statically).  On a modern system
>>>> that exposes multiple devices in a DTS-configurable way, this is less
>>>> than optimal and a interface to DM-based devices will be desirable.
>>>> 
>>>> This adds a simple driver that is DM-aware and configurable via DTS:
>>>> the /chosen/u-boot,bootcount-device property is used to detect the DM
>>>> device storing the bootcount and deviceclass-specific commands are
>>>> used to read/write the bootcount.
>>>> 
>>>> Initially, this provides support for the following DM devices:
>>>> * RTC devices implementing the read8/write8 ops
>>>> 
>>>> Signed-off-by: Philipp Tomsich 
>>>> Tested-by: Klaus Goger 
>>>> ---
>>>> 
>>>> doc/device-tree-bindings/chosen.txt | 27 +++
>>>> drivers/bootcount/Kconfig   | 12 +
>>>> drivers/bootcount/Makefile  |  1 +
>>>> drivers/bootcount/bootcount_dm.c| 93
>>>> + 4 files changed, 133
>>>> insertions(+) create mode 100644 drivers/bootcount/bootcount_dm.c
>>>> 
>>>> diff --git a/doc/device-tree-bindings/chosen.txt
>>>> b/doc/device-tree-bindings/chosen.txt index da7b4e6..734fd15 100644
>>>> --- a/doc/device-tree-bindings/chosen.txt
>>>> +++ b/doc/device-tree-bindings/chosen.txt
>>>> @@ -42,6 +42,33 @@ Example
>>>> };
>>>> };
>>>> 
>>>> +u-boot,bootcount-device property
>>>> +
>>>> +In a DM-based system, the bootcount may be stored in a device known
>>>> to +the DM framework (e.g. in a battery-backed SRAM area within a RTC
>>>> +device).  To identify the device to be used for the bootcount, the
>>>> +u-boot,bootcount-device property needs to point to the target device.
>>>> +
>>>> +Further configuration in the target device's node may be required
>>>> +(e.g. an offset into an I2C RTC's address space), depending on the
>>>> +UCLASS of the target device.
>>>> +
>>>> +Example
>>>> +---
>>>> +/ {
>>>> +chosen {
>>>> +u-boot,bootcount-device = &rv3029;
>>>> +};
>>>> +
>>>> +i2c2 {
>>>> +rv3029: rtc@56 {
>>>> +compatible = "mc,rv3029";
>>>> +reg = <0x56>;
>>>> +u-boot,bootcount-offset = <0x38>;
>>>> +};
>>>> +};
>>>> +};
>>>> +
>>>> u-boot,spl-boot-order property
>>>> --
>>>> 
>>>> diff --git a/drivers/bootcount/Kconfig b/drivers/bootcount/Kconfig
>>>> index d335ed1..cdde7b5 100644
>>>> --- a/drivers/bootcount/Kconfig
>>>> +++ b/drivers/bootcount/Kconfig
>>>> @@ -70,6 +70,18 @@ config BOOTCOUNT_AT91
>>>> bool "Boot counter for Atmel AT91SAM9XE"
>>>> depends on AT91SAM9XE
>>>> 
>>>> +config BOOTCOUNT_DM
>>>> +bool "Boot counter in a device-model device"
>>>> +help
>>>> +  Enables reading/writing the bootcount in a device-model
>>>> +  device referenced from the /chosen node.  The type of the
>>>> +  device is detected from DM and any additional configuration
>>>> +  information (e.g. the offset into a RTC device that
>>>> supports
>>>> +  read32/write32) is read from the device's node.
>>>> +
>>>> +  At this time the following DM device classes are supported:
>>>> +   * RTC (using read32/write32)
>>>> +
>>>> endchoice
>>>> 
>>>> config BOOTCOUNT_ALEN
>>>> diff --git a/drivers/bootcount/Makefile b/drivers/b

Re: [U-Boot] [PATCH 3/3] bootcount: add DM-based backing store for bootcount

[Dr. Philipp Tomsich]

Simon,

> On 23 Aug 2018, at 12:45, Simon Glass  wrote:
> 
> Hi Philipp,
> 
> On 17 August 2018 at 06:56, Dr. Philipp Tomsich
>  wrote:
>> Simon,
>> 
>> On 17 Aug 2018, at 14:49, Simon Glass  wrote:
>> 
>> Hi Philipp,
>> 
>> On 14 August 2018 at 05:39, Dr. Philipp Tomsich
>>  wrote:
>> 
>> Lukasz,
>> 
>> On 14 Aug 2018, at 13:10, Lukasz Majewski  wrote:
>> 
>> Hi Philipp,
>> 
>> The original bootcount methods do not provide an interface to DM and
>> rely on a static configuration for I2C devices (e.g. bus, chip-addr,
>> etc. are configured through defines statically).  On a modern system
>> that exposes multiple devices in a DTS-configurable way, this is less
>> than optimal and a interface to DM-based devices will be desirable.
>> 
>> This adds a simple driver that is DM-aware and configurable via DTS:
>> the /chosen/u-boot,bootcount-device property is used to detect the DM
>> device storing the bootcount and deviceclass-specific commands are
>> used to read/write the bootcount.
>> 
>> Initially, this provides support for the following DM devices:
>> * RTC devices implementing the read8/write8 ops
>> 
>> Signed-off-by: Philipp Tomsich 
>> Tested-by: Klaus Goger 
>> ---
>> 
>> doc/device-tree-bindings/chosen.txt | 27 +++
>> drivers/bootcount/Kconfig   | 12 +
>> drivers/bootcount/Makefile  |  1 +
>> drivers/bootcount/bootcount_dm.c| 93
>> + 4 files changed, 133
>> insertions(+) create mode 100644 drivers/bootcount/bootcount_dm.c
>> 
>> diff --git a/doc/device-tree-bindings/chosen.txt
>> b/doc/device-tree-bindings/chosen.txt index da7b4e6..734fd15 100644
>> --- a/doc/device-tree-bindings/chosen.txt
>> +++ b/doc/device-tree-bindings/chosen.txt
>> @@ -42,6 +42,33 @@ Example
>>};
>> };
>> 
>> +u-boot,bootcount-device property
>> +
>> +In a DM-based system, the bootcount may be stored in a device known
>> to +the DM framework (e.g. in a battery-backed SRAM area within a RTC
>> +device).  To identify the device to be used for the bootcount, the
>> +u-boot,bootcount-device property needs to point to the target device.
>> +
>> +Further configuration in the target device's node may be required
>> +(e.g. an offset into an I2C RTC's address space), depending on the
>> +UCLASS of the target device.
>> +
>> +Example
>> +---
>> +/ {
>> +chosen {
>> +u-boot,bootcount-device = &rv3029;
>> +};
>> +
>> +i2c2 {
>> +rv3029: rtc@56 {
>> +compatible = "mc,rv3029";
>> +reg = <0x56>;
>> +u-boot,bootcount-offset = <0x38>;
>> +};
>> +};
>> +};
>> +
>> u-boot,spl-boot-order property
>> --
>> 
>> diff --git a/drivers/bootcount/Kconfig b/drivers/bootcount/Kconfig
>> index d335ed1..cdde7b5 100644
>> --- a/drivers/bootcount/Kconfig
>> +++ b/drivers/bootcount/Kconfig
>> @@ -70,6 +70,18 @@ config BOOTCOUNT_AT91
>>bool "Boot counter for Atmel AT91SAM9XE"
>>depends on AT91SAM9XE
>> 
>> +config BOOTCOUNT_DM
>> +bool "Boot counter in a device-model device"
>> +help
>> +  Enables reading/writing the bootcount in a device-model
>> +  device referenced from the /chosen node.  The type of the
>> +  device is detected from DM and any additional configuration
>> +  information (e.g. the offset into a RTC device that
>> supports
>> +  read32/write32) is read from the device's node.
>> +
>> +  At this time the following DM device classes are supported:
>> +   * RTC (using read32/write32)
>> +
>> endchoice
>> 
>> config BOOTCOUNT_ALEN
>> diff --git a/drivers/bootcount/Makefile b/drivers/bootcount/Makefile
>> index 68bc006..e8ed729 100644
>> --- a/drivers/bootcount/Makefile
>> +++ b/drivers/bootcount/Makefile
>> @@ -7,3 +7,4 @@ obj-$(CONFIG_BOOTCOUNT_RAM)  += bootcount_ram.o
>> obj-$(CONFIG_BOOTCOUNT_ENV)  += bootcount_env.o
>> obj-$(CONFIG_BOOTCOUNT_I2C)  += bootcount_i2c.o
>> obj-$(CONFIG_BOOTCOUNT_EXT)  += bootcount_ext.o
>> +obj-$(CONFIG_BOOTCOUNT_DM)  += bootcount_dm.o
>> diff --git a/drivers/bootcount/bootcount_dm.c
>> b/drivers/bootcount/bootcount_dm.c new file mode 1006

Re: [U-Boot] [PATCH 11/25] clk: Allow clock defaults to be set also during re-reloc state

[Dr. Philipp Tomsich]

+Kever

> On 24 Aug 2018, at 16:12, Tom Rini  wrote:
> 
> On Tue, Aug 21, 2018 at 08:01:49PM +0530, Lokesh Vutla wrote:
> 
>> From: Andreas Dannenberg 
>> 
>> The earlier commit f4fcba5c5ba ("clk: implement clk_set_defaults()")
>> which introduced the functionality for setting clock defaults such as
>> rates and parents will skip the processing when executing in a re-reloc
>> state. This for example can prevent the assigning of clock parents
>> when running in SPL code. Go ahead and remove this limitation.
>> 
>> Signed-off-by: Andreas Dannenberg 
>> Signed-off-by: Lokesh Vutla 
>> ---
>> drivers/clk/clk-uclass.c | 4 
>> 1 file changed, 4 deletions(-)
>> 
>> diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
>> index 2b15978e14..04b369aa5a 100644
>> --- a/drivers/clk/clk-uclass.c
>> +++ b/drivers/clk/clk-uclass.c
>> @@ -243,10 +243,6 @@ int clk_set_defaults(struct udevice *dev)
>> {
>>  int ret;
>> 
>> -/* If this is running pre-reloc state, don't take any action. */
>> -if (!(gd->flags & GD_FLG_RELOC))
>> -return 0;
>> -
>>  debug("%s(%s)\n", __func__, dev_read_name(dev));
>> 
>>  ret = clk_set_default_parents(dev);
> 
> Philipp? David?  Comments?  Thanks!

If I remember correctly, David had a concern regarding an increase in
boottime if we ran this twice… adding Kever, as he was also involved
in the discussion.

I settled on skipping it for pre-reloc, but it’s an imperfect solution: the
boottime increase comes from the fact that some devices have a large
number of assigned-clocks, that the device-tree processing has a cost,
and that we don’t have a way of synchronising between SPL and full
U-Boot to avoid redoing the complete init-flow.

Maybe we should have a SPL-specific property for the assigned-clocks
to be set pre-reloc?

> 
>> -- 
>> 2.18.0
>> 
>> ___
>> U-Boot mailing list
>> U-Boot@lists.denx.de
>> https://lists.denx.de/listinfo/u-boot
> 
> -- 
> Tom

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 11/25] clk: Allow clock defaults to be set also during re-reloc state

[Dr. Philipp Tomsich]

> On 24 Aug 2018, at 17:54, Andreas Dannenberg  wrote:
> 
> Philipp,
> 
> On Fri, Aug 24, 2018 at 04:42:15PM +0200, Dr. Philipp Tomsich wrote:
>> +Kever
>> 
>>> On 24 Aug 2018, at 16:12, Tom Rini  wrote:
>>> 
>>> On Tue, Aug 21, 2018 at 08:01:49PM +0530, Lokesh Vutla wrote:
>>> 
>>>> From: Andreas Dannenberg 
>>>> 
>>>> The earlier commit f4fcba5c5ba ("clk: implement clk_set_defaults()")
>>>> which introduced the functionality for setting clock defaults such as
>>>> rates and parents will skip the processing when executing in a re-reloc
>>>> state. This for example can prevent the assigning of clock parents
>>>> when running in SPL code. Go ahead and remove this limitation.
>>>> 
>>>> Signed-off-by: Andreas Dannenberg 
>>>> Signed-off-by: Lokesh Vutla 
>>>> ---
>>>> drivers/clk/clk-uclass.c | 4 
>>>> 1 file changed, 4 deletions(-)
>>>> 
>>>> diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
>>>> index 2b15978e14..04b369aa5a 100644
>>>> --- a/drivers/clk/clk-uclass.c
>>>> +++ b/drivers/clk/clk-uclass.c
>>>> @@ -243,10 +243,6 @@ int clk_set_defaults(struct udevice *dev)
>>>> {
>>>>int ret;
>>>> 
>>>> -  /* If this is running pre-reloc state, don't take any action. */
>>>> -  if (!(gd->flags & GD_FLG_RELOC))
>>>> -  return 0;
>>>> -
>>>>debug("%s(%s)\n", __func__, dev_read_name(dev));
>>>> 
>>>>ret = clk_set_default_parents(dev);
>>> 
>>> Philipp? David?  Comments?  Thanks!
>> 
>> If I remember correctly, David had a concern regarding an increase in
>> boottime if we ran this twice… adding Kever, as he was also involved
>> in the discussion.
>> 
>> I settled on skipping it for pre-reloc, but it’s an imperfect solution: the
>> boottime increase comes from the fact that some devices have a large
>> number of assigned-clocks, that the device-tree processing has a cost,
>> and that we don’t have a way of synchronising between SPL and full
>> U-Boot to avoid redoing the complete init-flow.
> 
> Good to know some of the background; when I did this patch initially it
> was not really clear why this was removed and it obviously was an issue
> for what I was doing that I had to overcome and re-adding this was the
> simpliest thing to do at that time.
> 
>> Maybe we should have a SPL-specific property for the assigned-clocks
>> to be set pre-reloc?
> 
> Need to think about this some more. Generally we probably want to do as
> little as possible before relocation. Unfortunately for the K3 family of
> SoCs much is dependent on loading/installing the system firmware (SYSFW)
> image including to get DDR operational which itself requires us to use a
> lot of DT/DM stuff pre-reloc. So a little bit of a chicken and egg
> problem…

If you need someone to throw your thoughts at, feel free to share them with
me.  With the newer Rockchip devices that I am focused on, we also do a lot
DT/DM stuff pre-reloc (I have a 100+KB SPL stage on the RK3399 at the 
moment) and this trend is likely to increase … e.g. I may split the RK3399
to use TPL for DRAM-init and will then have the SPL stage running from
DRAM which will remove the last remaining size constraints.

--Philipp.

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] rockchip: dts: fix unnecessary '-cells' warning

[Dr. Philipp Tomsich]

> On 28 Aug 2018, at 11:57, Kever Yang  wrote:
> 
> Fix warning below:
> unnecessary #address-cells/#size-cells without "ranges" or child "reg"
> property
> 
> Signed-off-by: Kever Yang 

Reviewed-by: Philipp Tomsich 
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [U-Boot, 01/10] rockchip: enable DEBUG_UART_BOARD_INIT by default

[Dr. Philipp Tomsich]

Kever,

Could you rebase this series to u-boot-rockchip/next and resubmit?
I’d like to avoid any issues from manually fixing up git-am issues (as I 
happened to introduce on last set of UART changes).

Thanks,
Philipp.

> On 29 Aug 2018, at 20:33, Philipp Tomsich 
>  wrote:
> 
>> All Rockchip SoCs use DEBUG_UART_BOARD_INIT to init per board
>> UART IOMUX, enable it by default.
>> 
>> Signed-off-by: Kever Yang 
>> ---
>> 
>> arch/arm/Kconfig   | 2 ++
>> arch/arm/mach-rockchip/Kconfig | 3 ---
>> 2 files changed, 2 insertions(+), 3 deletions(-)
>> 
> 
> Reviewed-by: Philipp Tomsich 
> ___
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] rockchip: add TPL_TINY_FRAMEWORK support

[Dr. Philipp Tomsich]

> On 28 Aug 2018, at 11:23, Kever Yang  wrote:
> 
> Rockchip BootRom support load firmware from storage media twice,
> one for ddr sdram init code into sram and another time to load
> firmware into DDR. For ddr sdram init code(which is TPL in U-Boot
> project), it's OK to re-use the stack and vector table. In order
> to get more available sram space, we need to skip all the init code
> from U-Boot project including start.S, reset.S and framework.
> 
> Signed-off-by: Kever Yang 

Reviewed-by: Philipp Tomsich 

See below for a few recommended changes, a question and one required change.

> ---
> 
> arch/arm/cpu/armv7/start.S |   2 +
> arch/arm/cpu/armv8/start.S |   2 +
> arch/arm/include/asm/arch-rockchip/boot0.h |  10 ++
> arch/arm/mach-rockchip/Kconfig |  12 +++
> arch/arm/mach-rockchip/Makefile|   2 +
> arch/arm/mach-rockchip/tiny_tpl.c  | 106 +
> 6 files changed, 134 insertions(+)
> create mode 100644 arch/arm/mach-rockchip/tiny_tpl.c
> 
> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
> index 81edec01bf..548d9ff23a 100644
> --- a/arch/arm/cpu/armv7/start.S
> +++ b/arch/arm/cpu/armv7/start.S
> @@ -38,7 +38,9 @@
> reset:
>   /* Allow the board to save important registers */
>   b   save_boot_params
> +#if !CONFIG_IS_ENABLED(TINY_FRAMEWORK)
> save_boot_params_ret:
> +#endif

Having the label here will not increase code-size, so there’s no point in
making this conditional.

> #ifdef CONFIG_ARMV7_LPAE
> /*
>  * check for Hypervisor support
> diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S
> index d4db4d044f..e0f8fad10c 100644
> --- a/arch/arm/cpu/armv8/start.S
> +++ b/arch/arm/cpu/armv8/start.S
> @@ -31,6 +31,7 @@ _start:
>   b   reset
> #endif
> 
> +#if !CONFIG_IS_ENABLED(TINY_FRAMEWORK)
>   .align 3
> 
> .globl_TEXT_BASE
> @@ -361,3 +362,4 @@ ENDPROC(c_runtime_cpu_setup)
> WEAK(save_boot_params)
>   b   save_boot_params_ret/* back to my caller */
> ENDPROC(save_boot_params)
> +#endif
> diff --git a/arch/arm/include/asm/arch-rockchip/boot0.h 
> b/arch/arm/include/asm/arch-rockchip/boot0.h
> index 9ea4708ada..7f00494513 100644
> --- a/arch/arm/include/asm/arch-rockchip/boot0.h
> +++ b/arch/arm/include/asm/arch-rockchip/boot0.h
> @@ -41,8 +41,18 @@ entry_counter:
> 
> #if (defined(CONFIG_SPL_BUILD) || defined(CONFIG_ARM64))
>   /* U-Boot proper of armv7 do not need this */
> +#if CONFIG_IS_ENABLED(TINY_FRAMEWORK)
> + /* Allow the board to save important registers */
> + b save_boot_params
> +.globl   save_boot_params_ret
> +.type   save_boot_params_ret, %function
> +
> +save_boot_params_ret:
> + b board_init_f
> +#else
>   b reset
> #endif
> +#endif

I don’t like these nested #if primitives.
Is there a way to do this without nesting?
E.g. 

#if CONFIG_IS_ENABLED(TINY_FRAMEWORK)
b reset
#elif …
original code
#endif

> 
> #if !defined(CONFIG_ARM64)
>   /*
> diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
> index 187aa14184..be23bcc37e 100644
> --- a/arch/arm/mach-rockchip/Kconfig
> +++ b/arch/arm/mach-rockchip/Kconfig
> @@ -306,6 +306,18 @@ config TPL_ROCKCHIP_EARLYRETURN_TO_BROM
> config SPL_MMC_SUPPORT
>   default y if !SPL_ROCKCHIP_BACK_TO_BROM
> 
> +config TPL_TINY_FRAMEWORK
> + bool "Tiny TPL for DDR init"
> + depends on TPL && !TPL_FRAMEWORK
> + default y
> + help
> +   Rockchip BootRom support load firmware from storage media twice,
> +   one for ddr sdram init code into sram and another time to load
> +   firmware into DDR. For ddr sdram init code(which is TPL in U-Boot
> +   project), it's OK to re-use the stack and vector table. In order
> +   to get more available sram space, we need to skip all the init code
> +   from U-Boot project including start.S, reset.S and framework.
> +
> source "arch/arm/mach-rockchip/rk3036/Kconfig"
> source "arch/arm/mach-rockchip/rk3128/Kconfig"
> source "arch/arm/mach-rockchip/rk3188/Kconfig"
> diff --git a/arch/arm/mach-rockchip/Makefile b/arch/arm/mach-rockchip/Makefile
> index 83afdac99d..c88700f10d 100644
> --- a/arch/arm/mach-rockchip/Makefile
> +++ b/arch/arm/mach-rockchip/Makefile
> @@ -9,6 +9,8 @@
> obj-spl-$(CONFIG_ROCKCHIP_BROM_HELPER) += bootrom.o
> obj-tpl-$(CONFIG_ROCKCHIP_BROM_HELPER) += bootrom.o
> 
> +obj-tpl-$(CONFIG_TPL_TINY_FRAMEWORK) += tiny_tpl.o
> +
> obj-tpl-$(CONFIG_ROCKCHIP_RK3288) += rk3288-board-tpl.o
> obj-tpl-$(CONFIG_ROCKCHIP_RK3368) += rk3368-board-tpl.o
> obj-tpl-$(CONFIG_ROCKCHIP_RK322X) += rk322x-board-tpl.o
> diff --git a/arch/arm/mach-rockchip/tiny_tpl.c 
> b/arch/arm/mach-rockchip/tiny_tpl.c
> new file mode 100644
> index 00..47f15c0af1
> --- /dev/null
> +++ b/arch/arm/mach-rockchip/tiny_tpl.c
> @@ -0,0 +1,106 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * (C) Copyright 2018 Rockchip Electronics Co., Ltd
> + */
> +
> +#include 
> +#i

Re: [U-Boot] [U-Boot, 1/8] rockchip: add STIMER_BASE for Rockchip SoCs

[Dr . Philipp Tomsich]

Kever,

[Sorry for the delay, I am switching laptops and this got stuck in my Drafts 
folder
on the old machine — which I noticed only today]

> On 3 Sep 2018, at 05:21, Kever Yang  wrote:
> 
> Hi Philipp,
> 
> 
> On 08/30/2018 05:11 PM, Philipp Tomsich wrote:
>> 
>> 
>> On Wed, 18 Apr 2018, Kever Yang wrote:
>> 
>>> Most of Rockchip SoCs have ARM arch/generic timer whose clock source
>>> is from one of secure timer(if the soc supports Trust environment).
>>> 
>>> STIMER can only access in secure mode, so it should be init before
>>> the proper U-Boot(usually in non-secure mode).
>>> Add a MACRO for timer base addr so that we can init with a common
>>> function in SPL/TPL.
>>> 
>>> Signed-off-by: Kever Yang 
>> 
>> See below for required changes.
>> 
>>> ---
>>> 
>>> arch/arm/mach-rockchip/Kconfig | 16 
>>> 1 file changed, 16 insertions(+)
>>> 
>>> diff --git a/arch/arm/mach-rockchip/Kconfig
>>> b/arch/arm/mach-rockchip/Kconfig
>>> index 0adaed4..55d3d5c 100644
>>> --- a/arch/arm/mach-rockchip/Kconfig
>>> +++ b/arch/arm/mach-rockchip/Kconfig
>>> @@ -191,6 +191,22 @@ config ROCKCHIP_BOOT_MODE_REG
>>>   The Soc will enter to different boot mode(defined in
>>> asm/arch/boot_mode.h)
>>>   according to the value from this register.
>>> 
>>> +config ROCKCHIP_STIMER_BASE
>>> +hex "Rockchip Secure timer base address"
>>> +default 0x200440a0 if ROCKCHIP_RK3036
>>> +default 0x20018020 if ROCKCHIP_RK3126
>>> +default 0x200440a0 if ROCKCHIP_RK3128
>>> +default 0x110d0020 if ROCKCHIP_RK322X
>>> +default 0xff810020 if ROCKCHIP_RK3288
>>> +default 0xff1d0020 if ROCKCHIP_RK3328
>>> +default 0xff830020 if ROCKCHIP_RK3368
>>> +default 0xff8680a0 if ROCKCHIP_RK3399
>>> +default 0x10350020 if ROCKCHIP_RV1108
>>> +default 0
>>> +help
>>> +  The secure timer inited in SPL/TPL in secure word, ARM generic
>>> timer
>>> +  works after this timer work.
>> 
>> NAK.
>> This is not a user-configurable/selectable option, but rather a
>> function of the chip used.
>> This belongs into a header file in arch/arm/include/asm/arch-rockchip.
> 
> Yes, you are correct in one way, but I think if this goes to header
> file, it will separate in different
> header file for different SoCs, or with a lot if MACRO like "#if
> defined(CONFIG_ROCKCHIP_RK3288) #elif"
> in one header file.

I don’t care whether we make this different files (my preferred choice in
the long run … i.e. after we clean up the header-file directory) or put it
into single file: at the moment both variants are common in our 
asm/arch-rockchip directory …

> Make ROCKCHIP_STIMER_BASE in Kconfig and use like ROCKCHIP_BOOT_MODE_REG
> seems like a better idea to make things simple.
> 
> Actually there are many other configs like this:
> - DEBUG_UART_BASE

This is an excellent example and really needs to go into Kconfig, as the
UART selection is done via this (e.g. we use a different DEBUG_UART
on the RK3399-Q7 than on Rockchip’s reference design) and it’s not
something the chip designs but rather a selection from a list encoded as
an address.

> - IRAM_START
> - DRAM_START
> 
> One more consideration is, I don't think make SoC configs into too much
> place is a good idea,
> now we have 3 places for configs:
> - Kconfig default value
> - configs/soc_defconfig
> - include/configs/soc_header.h (this is moving to Kconfig one by one now)
> 
> so I would not like to move it into arch/arm/include/asm/arch-rockchip,
> if you insist this should go to header file instead of Kconfig, I would
> prefer
> to use header file in 'include/configs/' folder, how do you think?

All of this should eventually go into asm/arch-rockchip, unless it’s a
an board-specific overrride/configuration item (include/configs was historically
meant for boards and not for SOCs). I believe that include/configs/ will
eventually be almost completely replaced by Kconfig entries and that
any SOC-related things will need to go into asm/arch-rockchip.

If you could put it into asm/arch-rockchip, I’d prefer this.
However, if you want to have it in include/configs, I won’t delay the series
because of it and we’ll deal with the clean-up when the time comes.

Thanks,
Philipp.

> 
> Thanks,
> - Kever
>> 
>>> +
>>> config ROCKCHIP_SPL_RESERVE_IRAM
>>> hex "Size of IRAM reserved in SPL"
>>> default 0

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] cmd: part: use MAX_SEARCH_PARTITIONS for part search

[Dr. Philipp Tomsich]

> On 7 Sep 2018, at 11:37, Kever Yang  wrote:
> 
> Use Macro instead of hard code.

Nitpick: mentioning where the macro is defined (and if it was recently 
introduced,
what commit this references) would be helpful in a cursory review.  I just ran a
'git grep’ instead ;-)

> Signed-off-by: Kever Yang 

Reviewed-by: Philipp Tomsich 

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] rtc: Add read8 and write8 support to isl1208 driver

[Dr. Philipp Tomsich]

> On 31 May 2018, at 20:14, Trent Piepho  wrote:
> 
> This can be used for device register access from board code.
> 
> This allows access to capabilities in the RTC chip not abstracted in
> U-Boot's RTC class.  E.g., device NVRAM or a tamper detection circuit.
> 
> Cc: Klaus Goger 
> Cc: Philipp Tomsich 
> Cc: Simon Glass 
> Signed-off-by: Trent Piepho 

Reviewed-by: Philipp Tomsich 

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] Exception Level switching seems broken on RK3399

[Dr. Philipp Tomsich]

Vincente,

> On 1 Jun 2018, at 00:28, Vicente Bergas  wrote:
> 
> On Thu, May 24, 2018 at 7:05 PM, Dr. Philipp Tomsich
>  wrote:
>> Vincente,
>> 
>> On 24 May 2018, at 18:48, Vicente Bergas  wrote:
>> 
>> Hello Philipp,
>> your answer is much appreciated.
>> 
>> On Thu, May 24, 2018 at 1:07 PM, Dr. Philipp Tomsich
>>  wrote:
>> 
>> Vincente,
>> 
>> On 19 May 2018, at 16:58, Vicente Bergas  wrote:
>> 
>> Hello,
>> I am writing this from a standalone Sapphire board [1],
>> that is, without the Excavator base board.
>> The CPU is the Rockchip RK3399, which implements ARMv8.0-A.
>> 
>> Currently the boot process is:
>> 1.- Boot ROM
>> 2.- SPL, provided as closed source binary blob [2]
>> 
>> 
>> SPL-support is available in mainline U-Boot.  We developed this for
>> the RK3399-Q7 and it has been successfully used on other RK3399
>> boards (e.g. I know that some Firefly-users are using this).
>> 
>> 
>> Thank you!
>> 
>> 
>> 3.- ATF, closed source binary blob [3]
>>   (not using the one from [2] because of stability issues)
>> 
>> 
>> Why use the closed-source blob, if the RK3399 is supported in the ATF
>> mainline and an ATF can be compiled from source?
>> 
>> 
>> Currently I am using both binary blobs (SPL and ATF) because I could
>> not make it work another way. I'll give it another try.
>> 
>> 
>> 4.- Mainline u-boot, master branch
>> 5.- Mainline linux, master branch
>> 
>> I would like to use an opensource boot process.
>> As a first approach I try to completely remove the ATF and
>> replace the SPL with the one from u-boot.
>> The modified boot process looks like:
>> 1.- Boot ROM
>> 2.- SPL, from mainline u-boot, master branch
>> 3.- Mainline u-boot, master branch
>> 4.- Mainline linux, master branch
>> But it is not working.
>> 
>> The replaced SPL works fine and loads u-boot.
>> U-boot also works fine, loads linux and jumps into it.
>> 
>> 
>> Yes, we’ve done some work to enable us to run U-Boot in EL3 on
>> the RK3399 (as we use it for programming the secure e-fuses on
>> the RK3399-Q7 in our factory programming setup).
>> 
>> 
>> I can indeed confirm that U-Boot runs fine in EL3.
>> 
>> 
>> But then, linux never gets executed.
>> 
>> I have traced the issue to: arch/arm/include/asm/macro.h
>> 202: msr  spsr_el3, \tmp
>> 203: msr  elr_el3, \ep
>> 204: eret // This is the last instruction executed
>> 
>> For testing, I have also set CONFIG_ARMV8_SWITCH_TO_EL1 and
>> checked that switch_to_el1 from arch/arm/lib/bootm.c is not reached.
>> 
>> At this point I have a few questions:
>> 1.- Is my first approach feasible? That is, is it possible to boot
>>   this CPU without ATF?
>> 
>> 
>> It is feasible (i.e.: requires implementation work) but not recommended:
>> Linux will use PSCI to bring up the secondary CPUs.  We have run Linux
>> (limited to a single CPU) in EL3 on this CPU during our own board bringup,
>> but I would strongly discourage this as it will entail unnecessary effort.
>> 
>> 
>> There is a misunderstanding here. My intention was to run U-Boot in EL3,
>> then switch to EL2 or EL1 from within U-Boot and afterwards run Linux
>> in the lower EL.
>> 
>> 
>> 2.- If so, what should I do to make it work? Probably it is just
>>   a configuration issue, but I do not know what to check. [4]
>> 3.- Else, why do I need ATF?
>> 
>> 
>> ATF is the secure monitor on ARMv8 and provides services such as PSCI
>> to start up secondary CPUs.  It will usually also be part of
>> power-management
>> on most SoCs (after all: power configuration needs to be done in the secure
>> envelope).
>> 
>> 
>> Do you mean that without ATF I can only run a single CPU core?
>> 
>> 
>> Linux (on the RK3399) uses PSCI to start the secondary CPUs and uses SMC
>> (secure monitor call) to call into PSCI.  The PSCI handlers thus live within
>> ATF.
>> 
>> While it is technically possible to start up the other cores using other
>> mechanisms,
>> it is the ‘road less travelled’.
>> 
>> Cheers,
>> Philipp.
>> 
>> 
>> Regards,
>> Vicenç.
>> 
>> 
>> Regards,
>> Philipp.
>> 
>> 
>> Regards,
>> Vicenç.
> 
> Hello Philipp,
> I have managed to make it work. It turns out that support for
> this platform was added to 

Re: [U-Boot] [PATCH 1/1] rockchip: evb-rk3399: correct README for board bring up

[Dr. Philipp Tomsich]

> On 3 Jun 2018, at 16:50, Heinrich Schuchardt  wrote:
> 
> %s/rkflashtool/rkdeveloptool/
> 
> We are using rkdeveloptool not rkflashtool.
> 
> Signed-off-by: Heinrich Schuchardt 

Reviewed-by: Philipp Tomsich 
Acked-by: Philipp Tomsich 

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 1/1] rockchip: rk3399: spl: add missing \n to output

[Dr. Philipp Tomsich]

> On 3 Jun 2018, at 21:10, Heinrich Schuchardt  wrote:
> 
> Without the patch SPL (in case of an error) creates an output like:
> 
>   U-Boot SPL board initMissing DTB
> 
> The patch adds the missing line feed. So now we get:
> 
>   U-Boot SPL board init
>   Missing DTB
> 
> Signed-off-by: Heinrich Schuchardt 

Reviewed-by: Philipp Tomsich 
Acked-by: Philipp Tomsich 

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH RESEND 1/2] rockchip: make_fit_atf: use elf entry point

[Dr. Philipp Tomsich]

Mian,

Did you change anything or is this just a resend with the same content?
I didn’t have a chance to review this yet, so wanted to make sure I work off
the latest version...

Thanks,
Philipp.

> On 8 Jun 2018, at 10:47, Mian Yousaf Kaukab  wrote:
> 
> make_fit_atf.py uses physical address of first segment as the
> entry point to bl31. It is incorrect and causes following abort
> when bl31_entry() is called:
> 
> U-Boot SPL board initTrying to boot from MMC1
> "Synchronous Abort" handler, esr 0x0200
> elr:  lr : ff8c7e8c
> x 0: ff8e x 1: 
> x 2:  x 3: ff8e0180
> x 4:  x 5: 
> x 6: 0030 x 7: ff8e0188
> x 8: 01e0 x 9: 
> x10: 0007fcdc x11: 002881b8
> x12: 01a2 x13: 0198
> x14: 0007fdcc x15: 002881b8
> x16: 003c0724 x17: 003c0718
> x18: 0007fe80 x19: ff8e
> x20: 0020 x21: ff8e
> x22:  x23: 0007fe30
> x24: ff8d1c3c x25: ff8d5000
> x26: deadbeef x27: 04a0
> x28: 009c x29: 0007fd90
> 
> Fix it by using the entry point from the elf header.
> 
> Signed-off-by: Mian Yousaf Kaukab 
> ---
> arch/arm/mach-rockchip/make_fit_atf.py | 7 ---
> 1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/mach-rockchip/make_fit_atf.py 
> b/arch/arm/mach-rockchip/make_fit_atf.py
> index 6b3d9201c9..b88a5e1f16 100755
> --- a/arch/arm/mach-rockchip/make_fit_atf.py
> +++ b/arch/arm/mach-rockchip/make_fit_atf.py
> @@ -53,7 +53,7 @@ DT_END="""
> };
> """
> 
> -def append_atf_node(file, atf_index, phy_addr):
> +def append_atf_node(file, atf_index, phy_addr, elf_entry):
> """
> Append ATF DT node to input FIT dts file.
> """
> @@ -67,7 +67,7 @@ def append_atf_node(file, atf_index, phy_addr):
> print >> file, '\t\t\tcompression = "none";'
> print >> file, '\t\t\tload = <0x%08x>;' % phy_addr
> if atf_index == 1:
> -print >> file, '\t\t\tentry = <0x%08x>;' % phy_addr
> +print >> file, '\t\t\tentry = <0x%08x>;' % elf_entry
> print >> file, '\t\t};'
> print >> file, ''
> 
> @@ -141,12 +141,13 @@ def generate_atf_fit_dts(fit_file_name, bl31_file_name, 
> uboot_file_name, dtbs_fi
> 
> with open(bl31_file_name) as bl31_file:
> bl31 = ELFFile(bl31_file)
> +elf_entry = bl31.header['e_entry']
> for i in range(bl31.num_segments()):
> seg = bl31.get_segment(i)
> if ('PT_LOAD' == seg.__getitem__(ELF_SEG_P_TYPE)):
> paddr = seg.__getitem__(ELF_SEG_P_PADDR)
> p= seg.__getitem__(ELF_SEG_P_PADDR)
> -append_atf_node(fit_file, i+1, paddr)
> +append_atf_node(fit_file, i+1, paddr, elf_entry)
> atf_cnt = i+1
> append_fdt_node(fit_file, dtbs_file_name)
> print >> fit_file, '%s' % DT_IMAGES_NODE_END
> --
> 2.11.0
> 



signature.asc
Description: Message signed with OpenPGP
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v2 3/3] rockchip: veyron: Set vcc33_sd regulator value

[Dr. Philipp Tomsich]

> On 11 Jun 2018, at 10:08, Carlo Caione  wrote:
> 
> From: Carlo Caione 
> 
> On the veyron board the vcc33_sd regulator is used as vmmc-supply for
> the SD card. This regulator is powered in the MMC core during power on
> but its value is never actually set.
> 
> In the veyron platform the reset value for the LDO output is 1.8V while
> the standard (min and max) value for this regulator defined in the DTS
> is 3.3V. When the MMC core enable the regulator without setting its
> value, the output is automatically set to 1.8V instead of 3.3V.
> 
> With this patch we preemptively set the value to 3.3V.
> 
> Signed-off-by: Carlo Caione 
> Reviewed-by: Simon Glass 

Reviewed-by: Philipp Tomsich 
Acked-by: Philipp Tomsich 

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v2 1/3] rk3288: veyron: Init boot-on regulators

[Dr. Philipp Tomsich]

> On 11 Jun 2018, at 10:08, Carlo Caione  wrote:
> 
> From: Carlo Caione 
> 
> Use regulators_enable_boot_on() to init all the regulators with
> regulator-boot-on property.
> 
> Signed-off-by: Carlo Caione 
> Reviewed-by: Simon Glass 

Reviewed-by: Philipp Tomsich 
Acked-by: Philipp Tomsich 

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v2 2/3] rk3288: Disable JTAG function from sdmmc0 IO

[Dr. Philipp Tomsich]

> On 11 Jun 2018, at 10:08, Carlo Caione  wrote:
> 
> From: Carlo Caione 
> 
> The GRF_SOC_CON0.grf_force_jtag bit is automatically set at boot and it
> is preventing the SDMMC to work correctly. Disable the JTAG function on
> the assumption that a working SD has higher priority over JTAG.
> 
> Signed-off-by: Carlo Caione 
> Reviewed-by: Simon Glass 

Reviewed-by: Philipp Tomsich 
Acked-by: Philipp Tomsich 

See below for a nitpick ...

> ---
> arch/arm/mach-rockchip/rk3288-board.c | 7 +++
> 1 file changed, 7 insertions(+)
> 
> diff --git a/arch/arm/mach-rockchip/rk3288-board.c 
> b/arch/arm/mach-rockchip/rk3288-board.c
> index 0365793009..bf24d8e074 100644
> --- a/arch/arm/mach-rockchip/rk3288-board.c
> +++ b/arch/arm/mach-rockchip/rk3288-board.c
> @@ -307,6 +307,7 @@ U_BOOT_CMD(
>   ""
> );
> 
> +#define GRF_SOC_CON0 0xff770244
> #define GRF_SOC_CON2 0xff77024c

Could you convert these to ‘const uintptr_t GRF_SOC_CON0 = …’ ?
The compiler will generate the same code for a const as if it’s a define, but 
we’ll
have full type-safety.

> 
> int board_early_init_f(void)
> @@ -339,5 +340,11 @@ int board_early_init_f(void)
>   }
>   rk_setreg(GRF_SOC_CON2, 1 << 0);
> 
> + /*
> +  * Disable JTAG on sdmmc0 IO. The SDMMC won't work until this bit is
> +  * cleared
> +  */
> + rk_clrreg(GRF_SOC_CON0, 1 << 12);
> +
>   return 0;
> }
> -- 
> 2.17.1
> 

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v2 2/3] rk3288: Disable JTAG function from sdmmc0 IO

[Dr. Philipp Tomsich]

On 11 Jun 2018, at 19:08, Carlo Caione  wrote:
> 
> On Mon, 2018-06-11 at 10:41 +0200, Dr. Philipp Tomsich wrote:
>>> On 11 Jun 2018, at 10:08, Carlo Caione  wrote:
> 
> /cut
>>> +#define GRF_SOC_CON0 0xff770244
>>> #define GRF_SOC_CON2 0xff77024c
>> 
>> Could you convert these to ‘const uintptr_t GRF_SOC_CON0 = …’ ?
>> The compiler will generate the same code for a const as if it’s a
>> define, but we’ll
>> have full type-safety.
> 
> Yeah, no problem. But if this is ok with you in v3 I'm going to leave
> this patch as is and adding one more patch to convert both the lines at
> the same time.

Sure. Add another patch and I’ll squash it when applying.

—Philipp.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 03/12] net: gmac_rockchip: Fix a register write in rk3328_gmac_set_to_rgmii

[Dr. Philipp Tomsich]

> On 14 Jun 2018, at 19:39, Joe Hershberger  wrote:
> 
> On Thu, Jun 14, 2018 at 4:48 AM, Janine Hagemann  wrote:
>> We have to use RK3328_RXCLK_DLY_ENA_GMAC_ENABLE instead of
>> RK3328_RXCLK_DLY_ENA_GMAC_MASK in rk3328_gmac_set_to_rgmii()
>> to enable the RX delay.
>> The MASK was used in a wrong way.
>> 
>> Signed-off-by: Janine Hagemann 
> 
> Acked-by: Joe Hershberger 

Reviewed-by: Philipp Tomsich 
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 0/4] rockchip: Reduce prerequisites from the host

[Dr. Philipp Tomsich]

Vicente,

> On 18 Jun 2018, at 22:07, Vicente Bergas  wrote:
> 
> On Wed, 13 Jun 2018 11:51:13 +0800, kever.yang at rock-chips.com wrote:
>> (snip)
>> I just not understand why remove the dependency on python is so important,
>> there already many modules depend on python.
> 
> On Wed, 13 Jun 2018 11:57:48 +0800, kever.yang at rock-chips.com wrote:
>> (snip)
>> Could you share why you don't want to use python, can convert the
>> script to C?
> 
> Hi Kever,
> there are several reasons, here are the ones I can think of:
> 1.- Python is a ~100MB weight dependency.
> 2.- Because of (1) I don not have it installed by default.
> 3.- My Linux distribution of choice defaults to python3 but
> u-boot requires 'python' to be python2.
> 4.- "Shall use C language whenever possible."
> http://www.denx.de/wiki/U-Boot/DesignRequirements
> 5.- It is not just a dependency on what needs to be installed
> on the host, is also a dependency on which programming
> languages u-boot's contributors need to know about.
> So, it can be considered an entry barrier.

Given that most of the recent FDT infrastructure (e.g. tools/dtoc) and
binman are Python, the overall opinion on Python seems to have
changed (and no-one bothered to update the Wiki).

That said: the original python-implementation (i.e. the .py tool in
the rockchip subdirectory) is a bit unfortunate as well, as any processing
of binaries to build a final bootable image should nowadays be included
in binman.

> I don not have strong feelings on this getting merged and certainly
> will not get upset if it does not. It is entirely your decision.
> 
> I have submitted this patch series mostly "for your information" and
> because Philipp thought that it "looks worthwhile”:

Just for the record: I want to have this patch in the mailing-list archive,
so it is archived and can be found by anyone working on this aspect
of our bootup in the future.

I still stand by my initial assessment that it’s worthwhile to not lose this
info and make sure it’s archived ;-)

Thanks,
Philipp.

> On Fri, 1 Jun 2018 00:37:17 +0200, philipp.tomsich at
> theobroma-systems.com wrote:
>> (snip)
>>> On 1 Jun 2018, at 00:28, Vicente Bergas  wrote:
>> (snip)
>>> SPL_FIT_GENERATOR and SPL_OF_PLATDATA require python.
>>> In order to remove this dependency:
>>> 1.- I have written a C version for SPL_FIT_GENERATOR.
>>> 2.- Disabled SPL_OF_PLATDATA, it just works.
>>> 
>>> MKIMAGE_DTC_PATH requires dtc in the PATH.
>>> In order to remove this dependency, I have changed it to use the built-in 
>>> one.
>>> 
>>> If there is interest in those changes, I can post the full patch.
>> 
>> Please submit a patch--it certainly looks worthwhile.
> 
> Regards,
>  Vicenç.

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v2 1/6] dm: SMEM (Shared memory) uclass

[Dr. Philipp Tomsich]

> On 22 Jun 2018, at 04:11, Ramon Fried  wrote:
> 
> This is a uclass for Shared memory manager drivers.
> 
> A Shared Memory Manager driver implements an interface for allocating
> and accessing items in the memory area shared among all of the
> processors.
> 
> Signed-off-by: Ramon Fried 

Reviewed-by: Philipp Tomsich 

See below for a requested correction.

> 
> ---
> 
> Changes in v2:
> (As suggested by Simon Glass)
> - Introduced a new dm class (CLASS_SMEM) instead of CLASS_SOC.
> - Added sandbox driver
> - Added testing for DM class.
> 
> drivers/Makefile   |  1 +
> drivers/smem/Kconfig   |  2 +
> drivers/smem/Makefile  |  5 +++
> drivers/smem/smem-uclass.c | 53 
> include/dm/uclass-id.h |  1 +
> include/smem.h | 84 ++
> 6 files changed, 146 insertions(+)
> create mode 100644 drivers/smem/Kconfig
> create mode 100644 drivers/smem/Makefile
> create mode 100644 drivers/smem/smem-uclass.c
> create mode 100644 include/smem.h
> 
> diff --git a/drivers/Makefile b/drivers/Makefile
> index a213ea9671..ba4a561358 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -98,6 +98,7 @@ obj-y += pwm/
> obj-y += reset/
> obj-y += input/
> # SOC specific infrastructure drivers.
> +obj-y += smem/
> obj-y += soc/
> obj-$(CONFIG_REMOTEPROC) += remoteproc/
> obj-y += thermal/
> diff --git a/drivers/smem/Kconfig b/drivers/smem/Kconfig
> new file mode 100644
> index 00..64337a8b8e
> --- /dev/null
> +++ b/drivers/smem/Kconfig
> @@ -0,0 +1,2 @@
> +menuconfig SMEM
> + bool  "SMEM (Shared Memory mamanger) support"
> diff --git a/drivers/smem/Makefile b/drivers/smem/Makefile
> new file mode 100644
> index 00..ca55c4512d
> --- /dev/null
> +++ b/drivers/smem/Makefile
> @@ -0,0 +1,5 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +#
> +# Makefile for the U-Boot SMEM interface drivers
> +
> +obj-$(CONFIG_SMEM) += smem-uclass.o
> diff --git a/drivers/smem/smem-uclass.c b/drivers/smem/smem-uclass.c
> new file mode 100644
> index 00..07ed1a92d8
> --- /dev/null
> +++ b/drivers/smem/smem-uclass.c
> @@ -0,0 +1,53 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2018 Ramon Fried 
> + */
> +
> +#include 
> +#include 
> +#include 
> +
> +int smem_alloc(struct udevice *dev, unsigned int host,
> + unsigned int item, size_t size)
> +{
> + struct smem_ops *ops = smem_get_ops(dev);
> +
> + if (!ops->alloc)
> + return -ENOSYS;
> +
> + return ops->alloc(host, item, size);
> +}
> +
> +void *smem_get(struct udevice *dev, unsigned int host,
> + unsigned int item, size_t *size)
> +{
> + struct smem_ops *ops = smem_get_ops(dev);
> +
> + if (!ops->get)
> + return NULL;
> +
> + return ops->get(host, item, size);
> +}
> +
> +int smem_get_free_space(struct udevice *dev, unsigned int host, size_t *free)
> +{
> + struct smem_ops *ops = smem_get_ops(dev);
> +
> + int ret;
> +
> + if (!ops->get_free_space)
> + return -ENOSYS;
> +
> + ret = ops->get_free_space(host);
> + if (ret > 0)
> + *free = ret;
> + else
> + return ret;
> +
> + return 0;
> +}
> +
> +UCLASS_DRIVER(smem) = {
> + .id = UCLASS_SMEM,
> + .name   = "smem",
> +};
> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
> index d7f9df3583..a39643ec5e 100644
> --- a/include/dm/uclass-id.h
> +++ b/include/dm/uclass-id.h
> @@ -74,6 +74,7 @@ enum uclass_id {
>   UCLASS_RTC, /* Real time clock device */
>   UCLASS_SCSI,/* SCSI device */
>   UCLASS_SERIAL,  /* Serial UART */
> + UCLASS_SMEM,/* Shared memory interface */
>   UCLASS_SPI, /* SPI bus */
>   UCLASS_SPMI,/* System Power Management Interface bus */
>   UCLASS_SPI_FLASH,   /* SPI flash */
> diff --git a/include/smem.h b/include/smem.h
> new file mode 100644
> index 00..201960232c
> --- /dev/null
> +++ b/include/smem.h
> @@ -0,0 +1,84 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * header file for smem driver.
> + *
> + * Copyright (c) 2018 Ramon Fried 
> + */
> +
> +#ifndef _smemh_
> +#define _smemh_
> +
> +/* struct smem_ops: Operations for the SMEM uclass */
> +struct smem_ops {
> + /**
> +  * alloc() - allocate space for a smem item
> +  *
> +  * @host:   remote processor id, or -1
> +  * @item:   smem item handle
> +  * @size:   number of bytes to be allocated
> +  * @return 0 if OK, -ve on error
> +  */
> + int (*alloc)(unsigned int host,
> + unsigned int item, size_t size);
> +
> + /**
> +  * get() - Resolve ptr of size of a smem item
> +  *
> +  * @host:   the remote processor, of -1
> +  * @item:   smem item handle
> +  * @size:   pointer to be filled out with the size of the item
> +  * @return  pointer on success, NULL 

Re: [U-Boot] [PATCH 2/2] arm: timer: sunxi: add Allwinner timer erratum workaround

[Dr. Philipp Tomsich]

> On 27 Jun 2018, at 02:42, Andre Przywara  wrote:
> 
> The Allwinner A64 SoCs suffers from an arch timer implementation erratum,
> where sometimes the lower 11 bits of the counter value erroneously
> become all 0's or all 1's [1]. This leads to sudden jumps, both forwards and
> backwards, with the latter one often showing weird behaviour.

Feels like a throwback a discussions between us from about 2 years back. ;-)
Too bad that there’s still no Errata-document for the A64...

> Port the workaround proposed for Linux to U-Boot and activate it for all
> A64 boards.
> This fixes crashes when accessing MMC devices (SD cards), caused by a
> recent change to actually use the counter value for timeout checks.
> 
> Fixes: 5ff8e54888e4d26a352453564f7f599d29696dc9 ("sunxi: improve throughput
> in the sunxi_mmc driver")
> 
> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2018-May/576886.html
> 
> Signed-off-by: Andre Przywara 

Reviewed-by: Philipp Tomsich 

> ---
> arch/arm/cpu/armv8/generic_timer.c | 24 
> arch/arm/mach-sunxi/Kconfig|  4 
> 2 files changed, 28 insertions(+)
> 
> diff --git a/arch/arm/cpu/armv8/generic_timer.c 
> b/arch/arm/cpu/armv8/generic_timer.c
> index 3d04fde650..c1706dcec1 100644
> --- a/arch/arm/cpu/armv8/generic_timer.c
> +++ b/arch/arm/cpu/armv8/generic_timer.c
> @@ -46,6 +46,30 @@ unsigned long timer_read_counter(void)
> 
>   return cntpct;
> }
> +#elif CONFIG_SUNXI_A64_TIMER_ERRATUM
> +/*
> + * This erratum sometimes flips the lower 11 bits of the counter value
> + * to all 0's or all 1's, leading to jumps forwards or backwards.
> + * Backwards jumps might be interpreted all roll-overs and be treated as
> + * huge jumps forward.
> + * The workaround is to check whether the lower 11 bits of the counter are
> + * all 0 or all 1, then discard this value and read again.
> + * This occasionally discards valid values, but will catch all erroneous
> + * reads and fixes the problem reliably. Also this mostly requires only a
> + * single read, so does not have any significant overhead.
> + * The algorithm was conceived by Samuel Holland.
> + */
> +unsigned long timer_read_counter(void)
> +{
> + unsigned long cntpct;
> +
> + isb();
> + do {
> + asm volatile("mrs %0, cntpct_el0" : "=r" (cntpct));
> + } while (((cntpct + 1) & GENMASK(10, 0)) <= 1);
> +
> + return cntpct;
> +}
> #else
> /*
>  * timer_read_counter() using the Arm Generic Timer (aka arch timer).
> diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> index a3f7723028..3624a03947 100644
> --- a/arch/arm/mach-sunxi/Kconfig
> +++ b/arch/arm/mach-sunxi/Kconfig
> @@ -84,6 +84,9 @@ config SUNXI_HIGH_SRAM
>   Chips using the latter setup are supposed to select this option to
>   adjust the addresses accordingly.
> 
> +config SUNXI_A64_TIMER_ERRATUM
> + bool
> +
> # Note only one of these may be selected at a time! But hidden choices are
> # not supported by Kconfig
> config SUNXI_GEN_SUN4I
> @@ -270,6 +273,7 @@ config MACH_SUN50I
>   select SUNXI_DRAM_DW_32BIT
>   select FIT
>   select SPL_LOAD_FIT
> + select SUNXI_A64_TIMER_ERRATUM
> 
> config MACH_SUN50I_H5
>   bool "sun50i (Allwinner H5)"
> -- 
> 2.14.4
> 

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] build fail due to dependence error

[Dr. Philipp Tomsich]

Kever,

is the link for ‘asm’ set up correctly?

Thanks,
Philipp.

> On 28 Jun 2018, at 10:38, Kever Yang  wrote:
> 
> Hi Simon,
> 
> Do you have any idea about this error?
> 
> CC  lib/asm-offsets.s
>   CC  arch/arm/lib/asm-offsets.s
> fixdep: error opening config file: arch/arm/include/asm/arch/hardware.h: No 
> such file or directory
> Kbuild:64: recipe for target 'arch/arm/lib/asm-offsets.s' failed
> make[1]: *** [arch/arm/lib/asm-offsets.s] Error 2
> make[1]: *** 正在等待未完成的任务
>   CHK include/config.h
> Makefile:1340: recipe for target 'prepare0' failed
> 
> Thanks,
> - Kever
> On 06/01/2018 05:59 PM, Kever Yang wrote:
>> Hi Guys,
>> 
>>  I met below error from time to time when build U-Boot project.
>> 
>> fixdep: error opening config file: arch/arm/include/asm/arch/hardware.h: No 
>> such file or directory
>> 
>> The folder 'arch/arm/include/asm/arch' is a link for
>> 'arch/arm/include/asm/arch-rockchip',
>> 
>> which is dynamic generate by build system, I thinks there should be some
>> dependency issue?
>> 
>> How to fix this kind of issue?
>> 
>> 
>> Thanks,
>> 
>> - Kever
>> 
>> ___
>> U-Boot mailing list
>> U-Boot@lists.denx.de 
>> https://lists.denx.de/listinfo/u-boot 
> 

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] build fail due to dependence error

[Dr. Philipp Tomsich]

Kever,

Could you check the build artifacts in Jenkins to see whether after
the ‘make [configname]_defconfig’, the link for asm/arch is correctly
set up?

Thanks,
Philipp.

> On 2 Jul 2018, at 09:54, Kever Yang  wrote:
> 
> Hi Simon,
> 
> 
> On 06/30/2018 12:19 PM, Simon Glass wrote:
>> Hi,
>> 
>> On 28 June 2018 at 01:41, Dr. Philipp Tomsich
>>  wrote:
>>> Kever,
>>> 
>>> is the link for ‘asm’ set up correctly?
>>> 
>>> Thanks,
>>> Philipp.
>>> 
>>> On 28 Jun 2018, at 10:38, Kever Yang  wrote:
>>> 
>>> Hi Simon,
>>> 
>>>Do you have any idea about this error?
>>> 
>>> 
>>> CC  lib/asm-offsets.s
>>>  CC  arch/arm/lib/asm-offsets.s
>>> fixdep: error opening config file: arch/arm/include/asm/arch/hardware.h: No
>>> such file or directory
>>> Kbuild:64: recipe for target 'arch/arm/lib/asm-offsets.s' failed
>>> make[1]: *** [arch/arm/lib/asm-offsets.s] Error 2
>>> make[1]: *** 正在等待未完成的任务
>>>  CHK include/config.h
>>> Makefile:1340: recipe for target 'prepare0' failed
>>> 
>>> Thanks,
>>> - Kever
>>> 
>>> On 06/01/2018 05:59 PM, Kever Yang wrote:
>>> 
>>> Hi Guys,
>>> 
>>> I met below error from time to time when build U-Boot project.
>>> 
>>> fixdep: error opening config file: arch/arm/include/asm/arch/hardware.h: No
>>> such file or directory
>>> 
>>>The folder 'arch/arm/include/asm/arch' is a link for
>>> 'arch/arm/include/asm/arch-rockchip',
>>> 
>>> which is dynamic generate by build system, I thinks there should be some
>>> dependency issue?
>>> 
>>>How to fix this kind of issue?
>> I am not sure what is going on there. I don't see this.
>> 
>> Are you using 'make distclean' or 'make mrproper' on your object tree
>> every now and then?
> 
> This is happen sometimes in our Jenkins verify, It will gone when I
> rebase the patch(re-build without any change),
> we do 'make distclean' every time before a new build.
> 
> Thanks,
> - Kever
>> Regards,
>> Simon

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] build fail due to dependence error

[Dr. Philipp Tomsich]

> On 9 Jul 2018, at 09:18, Patrick DELAUNAY  wrote:
> 
>> From: U-Boot  On Behalf Of Masahiro Yamada
>> Sent: lundi 9 juillet 2018 07:27
>> Subject: Re: [U-Boot] build fail due to dependence error
>> 
>> Hi.
>> 
>> 
>> 2018-07-09 11:39 GMT+09:00 Simon Glass :
>>> +Masahiro who might know
>>> 
>>> On 2 July 2018 at 01:23, Dr. Philipp Tomsich
>>>  wrote:
>>>> Kever,
>>>> 
>>>> Could you check the build artifacts in Jenkins to see whether after
>>>> the ‘make [configname]_defconfig’, the link for asm/arch is correctly
>>>> set up?
>>>> 
>>>> Thanks,
>>>> Philipp.
>>>> 
>>>> On 2 Jul 2018, at 09:54, Kever Yang  wrote:
>>>> 
>>>> Hi Simon,
>>>> 
>>>> 
>>>> On 06/30/2018 12:19 PM, Simon Glass wrote:
>>>> 
>>>> Hi,
>>>> 
>>>> On 28 June 2018 at 01:41, Dr. Philipp Tomsich
>>>>  wrote:
>>>> 
>>>> Kever,
>>>> 
>>>> is the link for ‘asm’ set up correctly?
>>>> 
>>>> Thanks,
>>>> Philipp.
>>>> 
>>>> On 28 Jun 2018, at 10:38, Kever Yang  wrote:
>>>> 
>>>> Hi Simon,
>>>> 
>>>>   Do you have any idea about this error?
>>>> 
>>>> 
>>>> CC  lib/asm-offsets.s
>>>> CC  arch/arm/lib/asm-offsets.s
>>>> fixdep: error opening config file:
>>>> arch/arm/include/asm/arch/hardware.h: No such file or directory
>>>> Kbuild:64: recipe for target 'arch/arm/lib/asm-offsets.s' failed
>>>> make[1]: *** [arch/arm/lib/asm-offsets.s] Error 2
>>>> make[1]: *** 正在等待未完成的任务
>>>> CHK include/config.h
>>>> Makefile:1340: recipe for target 'prepare0' failed
>>>> 
>>>> Thanks,
>>>> - Kever
>>>> 
>>>> On 06/01/2018 05:59 PM, Kever Yang wrote:
>>>> 
>>>> Hi Guys,
>>>> 
>>>>I met below error from time to time when build U-Boot project.
>>>> 
>>>> fixdep: error opening config file:
>>>> arch/arm/include/asm/arch/hardware.h: No such file or directory
>>>> 
>>>>   The folder 'arch/arm/include/asm/arch' is a link for
>>>> 'arch/arm/include/asm/arch-rockchip',
>>>> 
>>>> which is dynamic generate by build system, I thinks there should be
>>>> some dependency issue?
>>>> 
>>>>   How to fix this kind of issue?
>>>> 
>>>> I am not sure what is going on there. I don't see this.
>>>> 
>>>> Are you using 'make distclean' or 'make mrproper' on your object tree
>>>> every now and then?
>>>> 
>>>> 
>>>> This is happen sometimes in our Jenkins verify, It will gone when I
>>>> rebase the patch(re-build without any change), we do 'make distclean'
>>>> every time before a new build.
>> 
>> 
>> Hmm, I cannot reproduce this error.
>> 
>> 
>> I have no clue about this.
> 
> Just for information, I have also the same issue sometime in the same context 
> (Jenkins build) for stm32mp1 board.
> 
> The error is : "Missing stm32.h file in the directory include/asm/arch/"
> 
> I try to understood the dependency issue, but I don't found any issue: 
> the needed symbolic link is done just before in the trace of the artifact...
> 
> 2018-03-03-17-00-02] | if [ -d 
> /opt/STM/workspace/workdir/rdk/build-stm32mp1/tmp-glibc/work/stm32mp1-stm32mpvalid-linux-gnueabi/u-boot-basic-stm32mp/2018.01-git-AUTOINC+d741b91b3c.r0/git/arch/arm/mach-stm32mp/include/mach
>  ]; then   \
> [2018-03-03-17-00-02] |dest=arch/arm/mach-stm32mp/include/mach;   
>   \
> [2018-03-03-17-00-02] | else  
>\
> [2018-03-03-17-00-02] |dest=arch/arm/include/asm/arch-stm32mp;   \
> [2018-03-03-17-00-02] | fi;   
>\
> [2018-03-03-17-00-02] | ln -fsn 
> /opt/STM/workspace/workdir/rdk/build-stm32mp1/tmp-glibc/work/stm32mp1-stm32mpvalid-linux-gnueabi/u-boot-basic-stm32mp/2018.01-git-AUTOINC+d741b91b3c.r0/git/$dest
>  include/asm/arch
> [2018-03-03-17-00-02] | fixdep: error opening config file: 
> include/asm/arch/stm32.h: No such file or directory
> 
> I don't understood why

Re: [U-Boot] [PATCH 3/3] rockchip: fix incorrect detection of ram size

[Dr. Philipp Tomsich]

> On 10 Jul 2018, at 16:41, Simon Glass  wrote:
> 
> Hi,
> 
> On 8 May 2018 at 04:21, Dr. Philipp Tomsich
>  <mailto:philipp.toms...@theobroma-systems.com>> wrote:
>> Marty,
>> 
>>> On 8 May 2018, at 02:52, Marty E. Plummer  wrote:
>>> 
>>> On Mon, May 07, 2018 at 11:16:28AM +0200, Dr. Philipp Tomsich wrote:
>>>> 
>>>>> On 7 May 2018, at 04:34, Marty E. Plummer  wrote:
>>>>> 
>>>>> On Mon, May 07, 2018 at 10:20:55AM +0800, Kever Yang wrote:
>>>>>> Hi Marty,
>>>>>> 
>>>>>> 
>>>>>> On 05/06/2018 10:25 PM, Marty E. Plummer wrote:
>>>>>>> Taken from coreboot's src/soc/rockchip/rk3288/sdram.c
>>>>>>> 
>>>>>>> Without this change, my u-boot build for the asus c201 chromebook (4GiB)
>>>>>>> is incorrectly detected as 0 Bytes of ram.
>>>>>> 
>>>>>> I know the root cause for this issue, and I have a local patch for it.
>>>>>> The rk3288 is 32bit, and 4GB size is just out of range, so we need to 
>>>>>> before
>>>>>> the max size before return with '<<20'. Sorry for forgot to send it out.
>>>>>> 
>>>>>>> 
>>>>>>> Signed-off-by: Marty E. Plummer 
>>>>>>> ---
>>>>>>> arch/arm/mach-rockchip/sdram_common.c | 62 ---
>>>>>>> 1 file changed, 37 insertions(+), 25 deletions(-)
>>>>>>> 
>>>>>>> diff --git a/arch/arm/mach-rockchip/sdram_common.c 
>>>>>>> b/arch/arm/mach-rockchip/sdram_common.c
>>>>>>> index 76dbdc8715..a9c9f970a4 100644
>>>>>>> --- a/arch/arm/mach-rockchip/sdram_common.c
>>>>>>> +++ b/arch/arm/mach-rockchip/sdram_common.c
>>>>>>> @@ -10,6 +10,8 @@
>>>>>>> #include 
>>>>>>> #include 
>>>>>>> #include 
>>>>>>> +#include 
>>>>>>> +#include 
>>>>>>> 
>>>>>>> DECLARE_GLOBAL_DATA_PTR;
>>>>>>> size_t rockchip_sdram_size(phys_addr_t reg)
>>>>>>> @@ -19,34 +21,44 @@ size_t rockchip_sdram_size(phys_addr_t reg)
>>>>>>>  size_t size_mb = 0;
>>>>>>>  u32 ch;
>>>>>>> 
>>>>>>> - u32 sys_reg = readl(reg);
>>>>>>> - u32 ch_num = 1 + ((sys_reg >> SYS_REG_NUM_CH_SHIFT)
>>>>>>> -& SYS_REG_NUM_CH_MASK);
>>>>>>> + if (!size_mb) {
>>>>>> 
>>>>>> I don't understand this and follow up changes, we don't really need it,
>>>>>> isn't it?
>>>>>> I think don't need the changes before here.
>>>>> Yeah, that was just another level of indentation for the if (!size_mb)
>>>>> guard, but I've reworked the patch to not do that as it was pointed out
>>>>> that since size_mb is initialized to 0 prior.
>>>>>>> + /*
>>>>>>> +  * we use the 0x~0xfeff space
>>>>>>> +  * since 0xff00~0x is soc register space
>>>>>>> +  * so we reserve it
>>>>>>> +  */
>>>>>>> + size_mb = min(size_mb, 0xff00/SZ_1M);
>>>>>> 
>>>>>> This is what we really need, as Klaus point out, we need to use
>>>>>> SDRAM_MAX_SIZE
>>>>>> instead of hard code.
>>>>> Yeah, I've got a rework on that which uses SDRAM_MAX_SIZE as instructed,
>>>>> build and boot tested on my hardware.
>>>> 
>>>> In that case you just masked the problem but didn???t solve it: assuming 
>>>> size_mb
>>>> is size_t (I???ll assume this is 64bit, but did not check), then your 4GB 
>>>> is 0x1__ )
>>>> which overflows to 0x0 when converted to a u32.
>>>> 
>>>> In other words: we need to figure out where the truncation occurs (image 
>>>> what
>>>> happens if a new 32bit processor with LPAE comes out???).
>>>> 
>>> A very valid point. With the following patch to sdram_common.c and
>>> sdram_rk3288.c applied I get the debug output that follows it:
>>> diff --git a/arch/arm/mach-rock

Re: [U-Boot] [PATCH 1/1] rockchip: rk3399: spl: add missing \n to output

[Dr. Philipp Tomsich]

Ezequiel,

I started to apply my backlog yesterday and this is currently in one of my trees
running through travis.  You should see the ‘applied to master’ message at the
latest tomorrow.

Thanks,
Philipp.

> On 11 Jul 2018, at 15:06, Ezequiel Garcia  
> wrote:
> 
> On 3 June 2018 at 19:28, Dr. Philipp Tomsich
>  wrote:
>> 
>>> On 3 Jun 2018, at 21:10, Heinrich Schuchardt  wrote:
>>> 
>>> Without the patch SPL (in case of an error) creates an output like:
>>> 
>>>  U-Boot SPL board initMissing DTB
>>> 
>>> The patch adds the missing line feed. So now we get:
>>> 
>>>  U-Boot SPL board init
>>>  Missing DTB
>>> 
>>> Signed-off-by: Heinrich Schuchardt 
>> 
>> Reviewed-by: Philipp Tomsich 
>> Acked-by: Philipp Tomsich 
>> 
> 
> Guys,
> 
> Any chance we pick this one?
> 
> Thanks,
> -- 
> Ezequiel García, VanguardiaSur
> www.vanguardiasur.com.ar

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v3 4/4] rk3288: Convert register defines to const uintptr_t

[Dr. Philipp Tomsich]

> On 9 Jul 2018, at 05:05, Simon Glass  wrote:
> 
> On 11 June 2018 at 13:00, Carlo Caione  wrote:
>> From: Carlo Caione 
>> 
>> No functional change but at least we can now guarantee type safety.
>> 
>> Signed-off-by: Carlo Caione 
>> ---
>> arch/arm/mach-rockchip/rk3288-board.c | 5 ++---
>> 1 file changed, 2 insertions(+), 3 deletions(-)
> 
> Reviewed-by: Simon Glass 
> 
> I'm not sure this affects type safety. The value in the #define has
> the same value (and type) as your variables.

I prefer this as locally defined const variables, so I’ve squashed this onto 
2/4.

Philipp.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 2/3] rockchip: add support for veyron-speedy (ASUS Chromebook C201)

[Dr. Philipp Tomsich]

> On 7 May 2018, at 02:20, Marty E. Plummer  wrote:
> 
> On Mon, May 07, 2018 at 12:12:54AM +0200, klaus.go...@theobroma-systems.com 
>  wrote:
>> 
>>> On 06.05.2018, at 16:25, Marty E. Plummer  wrote:
>>> 
>>> This adds support for the ASUS C201, a RK3288-based clamshell
>>> device. The device tree comes from linus's linux tree at
>>> 87ef12027b9b1dd0e0b12cf311fbcb19f9d92539. The SDRAM parameters
>>> are for 4GB Samsung LPDDR3, decoded from coreboot's
>>> src/mainboard/google/veyron/sdram_inf/sdram-lpddr3-samsung-4GB.inc
>>> 
>>> Signed-off-by: Marty E. Plummer 
>>> ---
>>> arch/arm/dts/Makefile |   1 +
>>> arch/arm/dts/rk3288-veyron-speedy.dts | 189 ++
>>> arch/arm/mach-rockchip/rk3288-board-spl.c |   3 +-
>>> arch/arm/mach-rockchip/rk3288/Kconfig |  11 ++
>>> board/google/veyron/Kconfig   |  16 ++
>>> configs/chromebook_speedy_defconfig   |  96 +++
>>> 6 files changed, 315 insertions(+), 1 deletion(-)
>>> create mode 100644 arch/arm/dts/rk3288-veyron-speedy.dts
>>> create mode 100644 configs/chromebook_speedy_defconfig
>>> 
>>> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
>>> index ac7667b1e8..ee04d9bedd 100644
>>> --- a/arch/arm/dts/Makefile
>>> +++ b/arch/arm/dts/Makefile
>>> @@ -42,6 +42,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += \
>>> rk3288-veyron-jerry.dtb \
>>> rk3288-veyron-mickey.dtb \
>>> rk3288-veyron-minnie.dtb \
>>> +   rk3288-veyron-speedy.dtb \
>>> rk3288-vyasa.dtb \
>>> rk3328-evb.dtb \
>>> rk3368-lion.dtb \
>>> diff --git a/arch/arm/dts/rk3288-veyron-speedy.dts 
>>> b/arch/arm/dts/rk3288-veyron-speedy.dts
>>> new file mode 100644
>>> index 00..d5383cef0d
>>> --- /dev/null
>>> +++ b/arch/arm/dts/rk3288-veyron-speedy.dts
>> 
>> This file looks quite different then the one I see on kernel.org with that 
>> revision id. So you are sure you
>> imported that one?
> Dafuq... it seems I borked something up in doing the patch juggling to
> turn my single-commit mess of a patch (you know, the 'get the thing to
> work branch') into a good patch series I messed up on this one.
>> 
>>> @@ -0,0 +1,189 @@
>>> +/*
>>> + * Google Veyron Speedy Rev 1+ board device tree source
>>> + *
>>> + * Copyright 2015 Google, Inc
>>> + *
>>> + * SPDX-License-Identifier:GPL-2.0
>> 
>> This file is dual licensed upstream, keep it that way.
>> The comment will claim it's a X11 license but the license text below
>> is actually MIT so you may want to use the MIT SPDX tag for that.
>> 
> Yeah, I was listening in on the convo on irc. So, even though it 'says'
> its GPL/X11, the actual license text is MIT, so I should use that tag?
> Its not my code, obviously, so I have no dog in that race anyways.
>>> + */
>>> +
>>> +/dts-v1/;
>>> +#include "rk3288-veyron-chromebook.dtsi"
>>> +#include "cros-ec-sbs.dtsi"
>>> +
>>> +/ {
>>> +   model = "Google Speedy";
>>> +   compatible = "google,veyron-speedy-rev9", "google,veyron-speedy-rev8",
>>> +"google,veyron-speedy-rev7", "google,veyron-speedy-rev6",
>>> +"google,veyron-speedy-rev5", "google,veyron-speedy-rev4",
>>> +"google,veyron-speedy-rev3", "google,veyron-speedy-rev2",
>>> +"google,veyron-speedy", "google,veyron", "rockchip,rk3288";
>>> +
>>> +   chosen {
>>> +   stdout-path = &uart2;
>>> +   };
>>> +
>>> +   panel_regulator: panel-regulator {
>>> +   compatible = "regulator-fixed";
>>> +   enable-active-high;
>>> +   gpio = <&gpio7 RK_PB6 GPIO_ACTIVE_HIGH>;
>>> +   pinctrl-names = "default";
>>> +   pinctrl-0 = <&lcd_enable_h>;
>>> +   regulator-name = "panel_regulator";
>>> +   startup-delay-us = <10>;
>>> +   vin-supply = <&vcc33_sys>;
>>> +   };
>>> +
>>> +   vcc18_lcd: vcc18-lcd {
>>> +   compatible = "regulator-fixed";
>>> +   enable-active-high;
>>> +   gpio = <&gpio2 RK_PB5 GPIO_ACTIVE_HIGH>;
>>> +   pinctrl-names = "default";
>>> +   pinctrl-0 = <&avdd_1v8_disp_en>;
>>> +   regulator-name = "vcc18_lcd";
>>> +   regulator-always-on;
>>> +   regulator-boot-on;
>>> +   vin-supply = <&vcc18_wl>;
>>> +   };
>>> +
>>> +   backlight_regulator: backlight-regulator {
>>> +   compatible = "regulator-fixed";
>>> +   enable-active-high;
>>> +   gpio = <&gpio2 RK_PB4 GPIO_ACTIVE_HIGH>;
>>> +   pinctrl-names = "default";
>>> +   pinctrl-0 = <&bl_pwr_en>;
>>> +   regulator-name = "backlight_regulator";
>>> +   vin-supply = <&vcc33_sys>;
>>> +   startup-delay-us = <15000>;
>>> +   };
>>> +};
>>> +
>>> +&dmc {
>>> +   rockchip,pctl-timing = <0x215 0xc8 0x0 0x35 0x26 0x2 0x70 0x2000d
>>> +   0x6 0x0 0x8 0x4 0x17 0x24 0xd 0x6
>>> +   0x4 0x8 0x4 0x76 0x4 0x0 0x30 0x0
>>> +   0x1 0x2 0x2 0x4 0x0 0x0 0xc0 0x4
>>> +   0x8 0x1f4>

Re: [U-Boot] [PATCH 03/12] net: gmac_rockchip: Fix a register write in rk3328_gmac_set_to_rgmii

[Dr. Philipp Tomsich]

Joe,

> On 14 Jun 2018, at 20:26, Joe Hershberger  wrote:
> 
> On Thu, Jun 14, 2018 at 1:12 PM, Dr. Philipp Tomsich
>  wrote:
>> 
>>> On 14 Jun 2018, at 19:39, Joe Hershberger  wrote:
>>> 
>>> On Thu, Jun 14, 2018 at 4:48 AM, Janine Hagemann  
>>> wrote:
>>>> We have to use RK3328_RXCLK_DLY_ENA_GMAC_ENABLE instead of
>>>> RK3328_RXCLK_DLY_ENA_GMAC_MASK in rk3328_gmac_set_to_rgmii()
>>>> to enable the RX delay.
>>>> The MASK was used in a wrong way.
>>>> 
>>>> Signed-off-by: Janine Hagemann 
>>> 
>>> Acked-by: Joe Hershberger 
>> 
>> Reviewed-by: Philipp Tomsich 
> 
> I assume I should take this series? Or would you prefer to?


I’ll take these as part of the larger series.

Thanks,
Philipp.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] sysreset: Add support for gpio-restart

[Dr. Philipp Tomsich]

> On 13 Jul 2018, at 11:15, Michal Simek  wrote:
> 
> The Linux kernel has binding for gpio-restart node.
> This patch is adding basic support without supporting any optional
> properties.

Nice. This may also give us an opportunity to streamline some of the reset
logic (and the FDT-interface to the ATF) on our RK3399-Q7 board.

> This driver was tested on Microblaze system where gpio is connected to
> SoC reset logic.
> Output value is handled via gpios cells values.
> 
> In gpio_reboot_request() set_value is writing 1 because
> dm_gpio_set_value() is capable to changing it when it is ACTIVE_LOW.
> ...
>   if (desc->flags & GPIOD_ACTIVE_LOW)
>   value = !value;
> ...
> 
> Signed-off-by: Michal Simek 

Reviewed-by: Philipp Tomsich 

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] CONFIG_SPL_TINY_MEMSET for configs/smartweb_defconfig?

[Dr. Philipp Tomsich]

Heiko,

I have a SPL change assigned in my queue that breaks the smartweb config due to 
increasing SPL by a few bytes of extra code.  I’d like to enable 
CONFIG_SPL_TINY_MEMSET in configs/smartweb_defconfig to fix this.

If this is ok with you, I’ll submit a patch against the defconfing and then 
take it through the rockchip-tree (so I have a clean Travis run), once you give 
it a Reviewed or Acked tag.

Let me know if you have any concerns,
Philipp.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v2 01/11] arch: arm: mach-rockchip: rk3288: Enable regulators in board_init

[Dr. Philipp Tomsich]

Janine,

> On 18 Jul 2018, at 10:46, Janine Hagemann  wrote:
> 
> At start-up, the regulators have to be enabled. Let's use
> regulators_enable_boot_on() to enable the regulators needed
> for boot.
> 
> Signed-off-by: Wadim Egorov 
> Signed-off-by: Janine Hagemann 

An equivalent change from Carlo has already been applied to U-Boot master.
Please review whether there’s additional changes needed, otherwise I’ll just
skip this one when processing.

Thanks,
Philipp.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [U-Boot, 3/3] rockchip: rk3399: Add more instructions to the README

[Dr. Philipp Tomsich]

Thanks for the clarification, I’ll take this as-is.

> On 20 Jul 2018, at 19:09, Ezequiel Garcia  wrote:
> 
> On Fri, 2018-07-20 at 18:28 +0200, Philipp Tomsich wrote:
>> 
>> On Mon, 16 Jul 2018, Ezequiel Garcia wrote:
>> 
>>> This commit adds a content section and also instructions
>>> on how to create a bootable SD/MMC device for RK3399 boards.
>> 
>> Are any of these instructions Ficus-specific?  We have our own README for 
>> our own boards, as these have different instructions from the EVB boards.
>> 
> 
> Nope, it applies to any rk3399 board. And the rest of the file as well,
> as long as the board has eMMC.
> 
>> Just wondering, as I'd have expected this to come in as part of the ficus 
>> board-directory...
>> 
> 
> Yeah, but all the instructions on this file applied to the Ficus
> (and to many others) so I decided to just extend it for now.
> 
> If it needs moving to some generic doc, I think we can do that as a follow up 
> patch.
> 
>>> Signed-off-by: Ezequiel Garcia 
>>> Reviewed-by: Simon Glass 
>> 
>> Reviewed-by: Philipp Tomsich 
>> 
> 
> Thanks for the review!
> Eze

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [U-Boot, 1/3] arm: dts: rockchip: add some common pin-settings to rk3399

[Dr. Philipp Tomsich]

> On 20 Jul 2018, at 19:14, Ezequiel Garcia  wrote:
> 
> On Fri, 2018-07-20 at 18:26 +0200, Philipp Tomsich wrote:
>> 
>> On Mon, 16 Jul 2018, Ezequiel Garcia wrote:
>> 
>>> From: Randy Li 
>>> 
>>> Those pins would be used by many boards.
>> 
>> The Rockchip pinctrl driver in U-Boot does not (yet) read the DTS for pin 
>> configuration information.
>> 
>> Is this a sync of the Linux DTS (which seems unlikely, as I'd expect more 
>> changes) or why are we doing this as part of this series?
>> 
> 
> As a matter of fact, it is a sync. It is literally this commit:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git/commit/?h=v4.19-armsoc/dts64&id=b41023282d07b61a53e2c9b9508912b1e7ce7b4f
> 
> It's just needed so that at least the ficus.dts file compiles.
> 
> Thanks,
> Eze

Reviewed-by: Philipp Tomsich 
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PULL] Please pull u-boot-rockchip/master

[Dr. Philipp Tomsich]

Tom,

The first batch of changes for u-boot-rockchip/master in this iteration is 
ready for you to pull.
The associated Travis report (prior to the rebase) is at
https://travis-ci.org/ptomsich/u-boot-rockchip/builds/406299101

Thanks,
Philipp.


The following changes since commit 0dd1fc09bb16869fd8adaaad082cd554c60b2c1a:

  board/imgtec/boston: Add new defconfigs to the MAINTAINERS list (2018-07-20 
15:55:10 -0400)

are available in the git repository at:

  git://git.denx.de/u-boot-rockchip.git master

for you to fetch changes up to a2a5053a15e4059c7445737d60f7b8425ca863f8:

  rockchip: utilize CONFIG_DEFAULT_FDT_FILE (2018-07-21 01:56:59 +0200)


Alexander Kochetkov (2):
  rockchip: i2c: enable i2c controller for rk3066 and rk3188
  rockchip: rk3188: add rk_board_late_init() hook

Carlo Caione (3):
  rk3288: veyron: Init boot-on regulators
  rk3288: Disable JTAG function from sdmmc0 IO
  rockchip: veyron: Set vcc33_sd regulator value

Heinrich Schuchardt (3):
  rockchip: evb-rk3399: correct README for board bring up
  rockchip: doc: clarify usage of CONFIG_SPL_ROCKCHIP_BACK_TO_BROM
  rockchip: rk3399: spl: add missing \n to output

Jakob Unterwurzacher (1):
  rockchip: board: lion-rk3368: increase phy autonegotiation timeout

Klaus Goger (2):
  rockchip: rk3399: change boot_target based on u-boot, spl-boot-device
  rockchip: utilize CONFIG_DEFAULT_FDT_FILE

Philipp Tomsich (4):
  smartweb: use SPL_TINY_MEMSET
  spl: record boot_device into spl_image and call spl_perform_fixups
  spl: document 'chosen/u-boot, spl-boot-device'
  rockchip: rk3399: inject 'u-boot, spl-boot-device' for next-stage

 arch/arm/mach-rockchip/rk3188-board.c |  7 +-
 arch/arm/mach-rockchip/rk3288-board.c | 26 ++-
 arch/arm/mach-rockchip/rk3399-board-spl.c | 50 +++-
 board/rockchip/evb_rk3399/README  |  2 +-
 board/theobroma-systems/puma_rk3399/puma-rk3399.c | 74 ++
 common/spl/spl.c  | 12 ++-
 configs/chromebit_mickey_defconfig|  1 +
 configs/chromebook_jerry_defconfig|  1 +
 configs/chromebook_minnie_defconfig   |  1 +
 configs/evb-px5_defconfig |  1 +
 configs/evb-rk3036_defconfig  |  1 +
 configs/evb-rk3128_defconfig  |  1 +
 configs/evb-rk3229_defconfig  |  1 +
 configs/evb-rk3288_defconfig  |  1 +
 configs/evb-rk3328_defconfig  |  1 +
 configs/evb-rk3399_defconfig  |  1 +
 configs/evb-rv1108_defconfig  |  1 +
 configs/fennec-rk3288_defconfig   |  1 +
 configs/firefly-rk3288_defconfig  |  1 +
 configs/firefly-rk3399_defconfig  |  1 +
 configs/geekbox_defconfig |  1 +
 configs/kylin-rk3036_defconfig|  1 +
 configs/lion-rk3368_defconfig |  1 +
 configs/miqi-rk3288_defconfig |  1 +
 configs/phycore-rk3288_defconfig  |  1 +
 configs/popmetal-rk3288_defconfig |  1 +
 configs/puma-rk3399_defconfig |  1 +
 configs/rock2_defconfig   |  1 +
 configs/rock_defconfig|  1 +
 configs/sheep-rk3368_defconfig|  1 +
 configs/smartweb_defconfig|  1 +
 configs/tinker-rk3288_defconfig   |  1 +
 configs/vyasa-rk3288_defconfig|  1 +
 doc/README.rockchip   |  8 +-
 doc/device-tree-bindings/chosen.txt   | 10 +++
 drivers/i2c/rk_i2c.c  | 94 +--
 include/configs/lion_rk3368.h |  2 +
 include/configs/rk3036_common.h   |  1 +
 include/configs/rk3128_common.h   |  1 +
 include/configs/rk3188_common.h   |  1 +
 include/configs/rk322x_common.h   |  1 +
 include/configs/rk3288_common.h   |  2 +-
 include/configs/rk3328_common.h   |  1 +
 include/configs/rk3368_common.h   |  1 +
 include/configs/rk3399_common.h   |  2 +-
 include/spl.h |  7 ++
 46 files changed, 312 insertions(+), 17 deletions(-)

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [U-Boot,2/3] ARM: add RK3399 Ficus board

[Dr. Philipp Tomsich]

Ezequiel,

This series breaks the build (see 
https://travis-ci.org/ptomsich/u-boot-rockchip/builds/406351695).
Did you test with Travis prior to submitting?

When you revise, I’d also prefer a ‘rockchip:’ and a ‘board:’ tag over the ARM 
tag …

Thanks,
Philipp.


> On 20 Jul 2018, at 19:30, Philipp Tomsich 
>  wrote:
> 
>> This commit adds support for RK3399 Ficus board,
>> aka ROCK960 Enterprise Edition.
>> 
>> Following peripherals are tested and known to work:
>> * Gigabit Ethernet
>> * USB 2.0
>> * MMC
>> 
>> Signed-off-by: Ezequiel Garcia 
>> Reviewed-by: Simon Glass 
>> Reviewed-by: Philipp Tomsich 
>> ---
>> arch/arm/dts/Makefile|   1 +
>> arch/arm/dts/rk3399-ficus.dts| 564 +++
>> board/rockchip/evb_rk3399/README |   2 +
>> configs/ficus-rk3399_defconfig   |  71 
>> 4 files changed, 638 insertions(+)
>> create mode 100644 arch/arm/dts/rk3399-ficus.dts
>> create mode 100644 configs/ficus-rk3399_defconfig
>> 
> 
> Acked-by: Philipp Tomsich 
> ___
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 2/3] rockchip: add support for veyron-speedy (ASUS Chromebook C201)

[Dr. Philipp Tomsich]

> On 24 Jul 2018, at 07:12, Marty E. Plummer  wrote:
> 
> On Fri, Jul 13, 2018 at 12:31:49PM +0200, Dr. Philipp Tomsich wrote:
>> 
>>> On 7 May 2018, at 02:20, Marty E. Plummer  wrote:
>>> 
>>> On Mon, May 07, 2018 at 12:12:54AM +0200, klaus.go...@theobroma-systems.com 
>>> <mailto:klaus.go...@theobroma-systems.com> wrote:
>>>> 
>>>>> On 06.05.2018, at 16:25, Marty E. Plummer  wrote:
>>>>> 
>>>>> This adds support for the ASUS C201, a RK3288-based clamshell
>>>>> device. The device tree comes from linus's linux tree at
>>>>> 87ef12027b9b1dd0e0b12cf311fbcb19f9d92539. The SDRAM parameters
>>>>> are for 4GB Samsung LPDDR3, decoded from coreboot's
>>>>> src/mainboard/google/veyron/sdram_inf/sdram-lpddr3-samsung-4GB.inc
>>>>> 
>>>>> Signed-off-by: Marty E. Plummer 
>>>>> ---
>>>>> arch/arm/dts/Makefile |   1 +
>>>>> arch/arm/dts/rk3288-veyron-speedy.dts | 189 ++
>>>>> arch/arm/mach-rockchip/rk3288-board-spl.c |   3 +-
>>>>> arch/arm/mach-rockchip/rk3288/Kconfig |  11 ++
>>>>> board/google/veyron/Kconfig   |  16 ++
>>>>> configs/chromebook_speedy_defconfig   |  96 +++
>>>>> 6 files changed, 315 insertions(+), 1 deletion(-)
>>>>> create mode 100644 arch/arm/dts/rk3288-veyron-speedy.dts
>>>>> create mode 100644 configs/chromebook_speedy_defconfig
>>>>> 
>>>>> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
>>>>> index ac7667b1e8..ee04d9bedd 100644
>>>>> --- a/arch/arm/dts/Makefile
>>>>> +++ b/arch/arm/dts/Makefile
>>>>> @@ -42,6 +42,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += \
>>>>>   rk3288-veyron-jerry.dtb \
>>>>>   rk3288-veyron-mickey.dtb \
>>>>>   rk3288-veyron-minnie.dtb \
>>>>> + rk3288-veyron-speedy.dtb \
>>>>>   rk3288-vyasa.dtb \
>>>>>   rk3328-evb.dtb \
>>>>>   rk3368-lion.dtb \
>>>>> diff --git a/arch/arm/dts/rk3288-veyron-speedy.dts 
>>>>> b/arch/arm/dts/rk3288-veyron-speedy.dts
>>>>> new file mode 100644
>>>>> index 00..d5383cef0d
>>>>> --- /dev/null
>>>>> +++ b/arch/arm/dts/rk3288-veyron-speedy.dts
>>>> 
>>>> This file looks quite different then the one I see on kernel.org with that 
>>>> revision id. So you are sure you
>>>> imported that one?
>>> Dafuq... it seems I borked something up in doing the patch juggling to
>>> turn my single-commit mess of a patch (you know, the 'get the thing to
>>> work branch') into a good patch series I messed up on this one.
>>>> 
>>>>> @@ -0,0 +1,189 @@
>>>>> +/*
>>>>> + * Google Veyron Speedy Rev 1+ board device tree source
>>>>> + *
>>>>> + * Copyright 2015 Google, Inc
>>>>> + *
>>>>> + * SPDX-License-Identifier:  GPL-2.0
>>>> 
>>>> This file is dual licensed upstream, keep it that way.
>>>> The comment will claim it's a X11 license but the license text below
>>>> is actually MIT so you may want to use the MIT SPDX tag for that.
>>>> 
>>> Yeah, I was listening in on the convo on irc. So, even though it 'says'
>>> its GPL/X11, the actual license text is MIT, so I should use that tag?
>>> Its not my code, obviously, so I have no dog in that race anyways.
>>>>> + */
>>>>> +
>>>>> +/dts-v1/;
>>>>> +#include "rk3288-veyron-chromebook.dtsi"
>>>>> +#include "cros-ec-sbs.dtsi"
>>>>> +
>>>>> +/ {
>>>>> + model = "Google Speedy";
>>>>> + compatible = "google,veyron-speedy-rev9", "google,veyron-speedy-rev8",
>>>>> +  "google,veyron-speedy-rev7", "google,veyron-speedy-rev6",
>>>>> +  "google,veyron-speedy-rev5", "google,veyron-speedy-rev4",
>>>>> +  "google,veyron-speedy-rev3", "google,veyron-speedy-rev2",
>>>>> +  "google,veyron-speedy", "google,veyron", "rockchip,rk3288";
>>>>> +
>>>>> + chosen {
>>>>> +

Re: [U-Boot] [PATCH 3/3] rockchip: fix incorrect detection of ram size

[Dr. Philipp Tomsich]

Marty,

> On 6 Jul 2018, at 05:11, Marty E. Plummer  wrote:
> 
> On Sat, May 19, 2018 at 02:08:53PM +0200, Dr. Philipp Tomsich wrote:
>> Marty,
>> 
>>> On 19 May 2018, at 12:40, Marty E. Plummer  wrote:
>>> 
>>> So explain to me what you'd like me to do here, if you would. What I
>>> gather from this is you want me to flip CONFIG_PHYS_64BIT and see if it
>>> works or what? I can flash/reflash u-boot and coreboot pretty easily on
>>> the device, so I'm down for any sort of hardware testing needed to get
>>> this into a usable state.
>> 
>> Yes, just enable PHYS_64BIT and report on how far it goes (activating some
>> debug may be helpful to understand what goes wrong, if it fails).
>> 
>> My gut feeling is that it could work, but there’s a number of pitfalls and 
>> we may
>> not be lucky.
>> 
> Testing flipping CONFIG_PHYS_64BIT, both with and without my 'clamping'
> patch to sdram_common.c, has the same results, in that all that is
> output on the servo console is that wierd  output. Where to from
> here, then?

I have a patchset for changing the relevant fields in U-Boot to allow for 
33bits (i.e. using u64) for the RAM size and it finally passes Travis cleanly.
It will be another round or two of cleanup before I can submit the series — 
once this happens, I’ll copy you so you can give your Tested-by if it works…

Thanks,
Phil.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [U-Boot,2/3] ARM: add RK3399 Ficus board

[Dr. Philipp Tomsich]

Ezequiel,

> On 25 Jul 2018, at 22:31, Ezequiel Garcia  wrote:
> 
> On Sat, 2018-07-21 at 16:23 +0200, Dr. Philipp Tomsich wrote:
>> Ezequiel,
>> 
>> This series breaks the build (see 
>> https://travis-ci.org/ptomsich/u-boot-rockchip/builds/406351695).
>> Did you test with Travis prior to submitting?
>> 
> 
> No, I haven't. It's quite odd, as the README patch just adds some 
> documentation.

I responded to this mail, as there’s no cover letter.
It’s the series breaking the build, not the README change.

> I am having a hard time figuring out how could it break the build,
> and I do not see any logs in the travis-ci link either.

Does the direct link to the line with the error work for you:
https://travis-ci.org/ptomsich/u-boot-rockchip/jobs/406351815#L1330

If not, I’ll have to copy this into a mail manually...

> 
> Any ideas?
> 
>> When you revise, I’d also prefer a ‘rockchip:’ and a ‘board:’ tag over the 
>> ARM tag …
>> 
> 
> OK, I will.
> 
>> Thanks,
>> Philipp.
>> 
>> 
>>> On 20 Jul 2018, at 19:30, Philipp Tomsich 
>>>  wrote:
>>> 
>>>> This commit adds support for RK3399 Ficus board,
>>>> aka ROCK960 Enterprise Edition.
>>>> 
>>>> Following peripherals are tested and known to work:
>>>> * Gigabit Ethernet
>>>> * USB 2.0
>>>> * MMC
>>>> 
>>>> Signed-off-by: Ezequiel Garcia 
>>>> Reviewed-by: Simon Glass 
>>>> Reviewed-by: Philipp Tomsich 
>>>> ---
>>>> arch/arm/dts/Makefile|   1 +
>>>> arch/arm/dts/rk3399-ficus.dts| 564 +++
>>>> board/rockchip/evb_rk3399/README |   2 +
>>>> configs/ficus-rk3399_defconfig   |  71 
>>>> 4 files changed, 638 insertions(+)
>>>> create mode 100644 arch/arm/dts/rk3399-ficus.dts
>>>> create mode 100644 configs/ficus-rk3399_defconfig
>>>> 
>>> 
>>> Acked-by: Philipp Tomsich 
>>> ___
>>> U-Boot mailing list
>>> U-Boot@lists.denx.de
>>> https://lists.denx.de/listinfo/u-boot
>> 
>> 
> 

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 2/9] rockchip: support 4GB DRAM on 32bit systems

[Dr. Philipp Tomsich]

> On 26 Jul 2018, at 22:05, Carlo Caione  wrote:
> 
> On Thu, 2018-07-26 at 15:59 +0200, Philipp Tomsich wrote:
>> The calculation in `rockchip_sdram_size` would overflow for 4GB on
>> 32bit systems (i.e. when PHYS_64BIT is not defined).
>> 
>> This makes the internal variables and the signature prototype use a
>> u64 to ensure that we can represent the 33rd bit (as in '4GB').
>> 
> 
> Hi Philipp,
> just to let you know that this is still not working on the veyron jerry
> chromebook with 4GB I have here (RK3288). The boot stops at:
> 
> U-Boot SPL 2018.07-00403-gdbe6ef8276 (Jul 26 2018 - 17:21:11 +0100)
> Trying to boot from SPI
> 
> U-Boot 2018.07-00403-gdbe6ef8276 (Jul 26 2018 - 17:21:11 +0100)
> 
> Model: Google Jerry
> DRAM:  0 Bytes
> 
> I'm still investigating why but for sure there are several points to
> fix to have a proper debug, see [0].
> 
> Also I was wondering if we should also fix get_effective_memsize() and
> gd->bd->bi_dram[].size

Yes, we should. I missed that one…
I only have RK3368 and RK3399 targets to test here, so your testing is very
much appreciated.

> 
> [0]https://gist.github.com/carlocaione/b93cfd9ee71e07fcf68d5c02256ff0fa
> 
> Cheers,
> 
> --
> Carlo Caione 



signature.asc
Description: Message signed with OpenPGP
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 2/9] rockchip: support 4GB DRAM on 32bit systems

[Dr. Philipp Tomsich]

> On 26 Jul 2018, at 22:05, Carlo Caione  wrote:
> 
> On Thu, 2018-07-26 at 15:59 +0200, Philipp Tomsich wrote:
>> The calculation in `rockchip_sdram_size` would overflow for 4GB on
>> 32bit systems (i.e. when PHYS_64BIT is not defined).
>> 
>> This makes the internal variables and the signature prototype use a
>> u64 to ensure that we can represent the 33rd bit (as in '4GB').
>> 
> 
> Hi Philipp,
> just to let you know that this is still not working on the veyron jerry
> chromebook with 4GB I have here (RK3288). The boot stops at:
> 
> U-Boot SPL 2018.07-00403-gdbe6ef8276 (Jul 26 2018 - 17:21:11 +0100)
> Trying to boot from SPI
> 
> U-Boot 2018.07-00403-gdbe6ef8276 (Jul 26 2018 - 17:21:11 +0100)
> 
> Model: Google Jerry
> DRAM:  0 Bytes
> 
> I'm still investigating why but for sure there are several points to
> fix to have a proper debug, see [0].
> 
> Also I was wondering if we should also fix get_effective_memsize() and
> gd->bd->bi_dram[].size
> 
> [0]https://gist.github.com/carlocaione/b93cfd9ee71e07fcf68d5c02256ff0fa

Thanks for identifying these two… I’ll pick them up and include in the next
series.

> 
> Cheers,
> 
> -- 
> Carlo Caione 

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 2/9] rockchip: support 4GB DRAM on 32bit systems

[Dr. Philipp Tomsich]

> On 26 Jul 2018, at 22:05, Carlo Caione  wrote:
> 
> On Thu, 2018-07-26 at 15:59 +0200, Philipp Tomsich wrote:
>> The calculation in `rockchip_sdram_size` would overflow for 4GB on
>> 32bit systems (i.e. when PHYS_64BIT is not defined).
>> 
>> This makes the internal variables and the signature prototype use a
>> u64 to ensure that we can represent the 33rd bit (as in '4GB').
>> 
> 
> Hi Philipp,
> just to let you know that this is still not working on the veyron jerry
> chromebook with 4GB I have here (RK3288). The boot stops at:
> 
> U-Boot SPL 2018.07-00403-gdbe6ef8276 (Jul 26 2018 - 17:21:11 +0100)
> Trying to boot from SPI
> 
> U-Boot 2018.07-00403-gdbe6ef8276 (Jul 26 2018 - 17:21:11 +0100)
> 
> Model: Google Jerry
> DRAM:  0 Bytes
> 
> I'm still investigating why but for sure there are several points to
> fix to have a proper debug, see [0].

I reread ‘rockchip_sdram_size’ and it looks as if I forgot to mark the value
shifted to create chipsize_mb as ULL.  When looking at this code I had an
uneasy feeling that this might run into the C type rules (i.e. 1 will be a 32bit
integer and shifting it by 32 would result in 0), but I didn’t think that this
would ever be the case and that any 4GB DRAM setup would be made
up of multiple channels or of multiple ranks.

> 
> Also I was wondering if we should also fix get_effective_memsize() and
> gd->bd->bi_dram[].size
> 
> [0]https://gist.github.com/carlocaione/b93cfd9ee71e07fcf68d5c02256ff0fa
> 
> Cheers,
> 
> -- 
> Carlo Caione 

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 2/9] rockchip: support 4GB DRAM on 32bit systems

[Dr. Philipp Tomsich]

> On 27 Jul 2018, at 09:50, Carlo Caione  wrote:
> 
> On Fri, 2018-07-27 at 00:54 +0200, Dr. Philipp Tomsich wrote:
>>> On 26 Jul 2018, at 22:05, Carlo Caione  wrote:
>>> 
>>> On Thu, 2018-07-26 at 15:59 +0200, Philipp Tomsich wrote:
>>>> The calculation in `rockchip_sdram_size` would overflow for 4GB
>>>> on
>>>> 32bit systems (i.e. when PHYS_64BIT is not defined).
>>>> 
>>>> This makes the internal variables and the signature prototype use
>>>> a
>>>> u64 to ensure that we can represent the 33rd bit (as in '4GB').
>>>> 
>>> 
>>> Hi Philipp,
>>> just to let you know that this is still not working on the veyron
>>> jerry
>>> chromebook with 4GB I have here (RK3288). The boot stops at:
>>> 
>>> U-Boot SPL 2018.07-00403-gdbe6ef8276 (Jul 26 2018 - 17:21:11 +0100)
>>> Trying to boot from SPI
>>> 
>>> U-Boot 2018.07-00403-gdbe6ef8276 (Jul 26 2018 - 17:21:11 +0100)
>>> 
>>> Model: Google Jerry
>>> DRAM:  0 Bytes
>>> 
>>> I'm still investigating why but for sure there are several points
>>> to
>>> fix to have a proper debug, see [0].
>> 
>> I reread ‘rockchip_sdram_size’ and it looks as if I forgot to mark
>> the value
>> shifted to create chipsize_mb as ULL.  When looking at this code I
>> had an
>> uneasy feeling that this might run into the C type rules (i.e. 1 will
>> be a 32bit
>> integer and shifting it by 32 would result in 0), but I didn’t think
>> that this
>> would ever be the case and that any 4GB DRAM setup would be made
>> up of multiple channels or of multiple ranks.
> 
> It doesn't hurt but rockchip_sdram_size() is returning the correct
> value of 0x1 in my tests.

The 'gd->bd->bi_dram[i].size’ path will also be involved and truncate this
later on, but I am not convinced that it’s worth fixing the dram banks info
for the general case.

Generally, typing for these bi_dram structures seems a mess: they are
often cast to 32bit and sometimes to 64bit in common code (i.e. fdtdec.c
uses unsigned long long).
I hope I can avoid touching this code.

Btw, here’s a gem from board_f.c (these two lines are after each other):
> gd->ram_top = gd->ram_base + get_effective_memsize();
> gd->ram_top = board_get_usable_ram_top(gd->mon_len);

As the first line is clearly deal (except the fact that the compiler can’t tell
that get_effective_memsize() should be side-effect free), we can drop it.
I’ll send a separate patch for this to be picked up by Tom…

—Phil.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] board_f: drop 'dead code' in ram_top computation

[Dr. Philipp Tomsich]

> On 27 Jul 2018, at 11:16, Philipp Tomsich 
>  wrote:
> 
> gd->ram_top is assigned to twice on consecutive lines and the compiler
> won't be able to tell that the first assignment is dead (including its
> r-value) due to the r-value containing a (side-effect free) function
> call.
> 
> This drops the first assignment.
> 
> Signed-off-by: Philipp Tomsich 
> ---
> 
> common/board_f.c | 1 -
> 1 file changed, 1 deletion(-)
> 
> diff --git a/common/board_f.c b/common/board_f.c
> index 88d7700..1b8a003 100644
> --- a/common/board_f.c
> +++ b/common/board_f.c
> @@ -283,7 +283,6 @@ static int setup_dest_addr(void)
> #ifdef CONFIG_SYS_SDRAM_BASE
>   gd->ram_base = CONFIG_SYS_SDRAM_BASE;
> #endif
> - gd->ram_top = gd->ram_base + get_effective_memsize();
>   gd->ram_top = board_get_usable_ram_top(gd->mon_len);
>   gd->relocaddr = gd->ram_top;
>   debug("Ram top: %08lX\n", (ulong)gd->ram_top);
> -- 
> 2.1.4
> 

Oh my, I’ll need to revise, as board_get_usable_ram_top is implemented as 
follows:
> /* Get the top of usable RAM */
> __weak ulong board_get_usable_ram_top(ulong total_size)
> {
> #ifdef CONFIG_SYS_SDRAM_BASE
>   /*  
>  * Detect whether we have so much RAM that it goes past the end of 
> our  
>  * 32-bit address space. If so, clip the usable RAM so it doesn't.
>   
>  */
>   if (gd->ram_top < CONFIG_SYS_SDRAM_BASE)
>   /*  
>  * Will wrap back to top of 32-bit space when reservations
>   
>  * are made.  
>   
>  */
> return 0;
> #endif
>   return gd->ram_top;
> }

I.e. it consumes the previous value of gd->ram_top and ignores its argument.
I’ll clean this up before someone else falls into the same pit.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 1/9] dm: allow 4GB of DRAM on 32bit systems

[Dr. Philipp Tomsich]

> On 2 Aug 2018, at 22:36, Simon Glass  wrote:
> 
> On 26 July 2018 at 07:59, Philipp Tomsich
>  > wrote:
>> Even on 32bit systems a full 4GB of DRAM may be installed and reported
>> by the DRAM controller.  Whether these 4GB are larger available
>> depends on the size/configuration of address decoding windows and
>> architectural features (e.g. consider a hypothetical architecture that
>> uses dedicated instructions to access the 'memory-mapped' peripheral
>> IO ranges).  In U-Boot, the available DRAM, as reported by the
>> device-model is independent of the accessible DRAM (i.e. adjusted top
>> and effective size).
>> 
>> This was first reported against the RK3288, which may have 4GB DRAM
>> attached, but will have a small (e.g. 128MB) window not accessible due
>> to address decoding limitations.
>> 
>> The solution is to increase the types used for storing the ram_size to
>> have at least 33 bits even on 32bit systems... i.e. we need to use a
>> u64 for these always (this was previously only the case for
>> PHYS_64BIT, which will have unwanted side-effects for 32bit systems
>> and would require more intrusive changes).
>> 
>> This commit changes the size-field in 'struct ram' and the ram_size in
>> 'gd' to be a u64.
>> 
>> Reported-by: Marty E. Plummer 
>> Signed-off-by: Philipp Tomsich 
>> ---
>> 
>> include/asm-generic/global_data.h | 2 +-
>> include/ram.h | 9 -
>> 2 files changed, 9 insertions(+), 2 deletions(-)
>> 
> 
> Reviewed-by: Simon Glass mailto:s...@chromium.org>>
> 
>> diff --git a/include/asm-generic/global_data.h 
>> b/include/asm-generic/global_data.h
>> index 0fd4900..f3d687c 100644
>> --- a/include/asm-generic/global_data.h
>> +++ b/include/asm-generic/global_data.h
>> @@ -55,7 +55,7 @@ typedef struct global_data {
>>unsigned long ram_base; /* Base address of RAM used by U-Boot 
>> */
>>unsigned long ram_top;  /* Top address of RAM used by U-Boot 
>> */
>>unsigned long relocaddr;/* Start address of U-Boot in RAM */
>> -   phys_size_t ram_size;   /* RAM size */
>> +   u64 ram_size;   /* RAM size */
>>unsigned long mon_len;  /* monitor len */
>>unsigned long irq_sp;   /* irq stack pointer */
>>unsigned long start_addr_sp;/* start_addr_stackpointer */
>> diff --git a/include/ram.h b/include/ram.h
>> index 67e22d7..1fe024f 100644
>> --- a/include/ram.h
>> +++ b/include/ram.h
>> @@ -9,7 +9,14 @@
>> 
>> struct ram_info {
>>phys_addr_t base;
>> -   size_t size;
>> +   /*
>> +* We use a 64bit type for the size to correctly handle 32bit
>> +* systems with 4GB of DRAM (which would overflow a 32bit type
>> +* and read back as 0).  For 64bit systems, we assume (for now)
> 
> forever :-)
> 
>> +* that they will always have less than 2^65 byte of DRAM
>> +* installed. -- prt
> 
> Is the prt your signature? I suggest dropping it.

In fact it is. But I’ll need to rewrite the entire comment anyway for the next
version of this series as there’s even more functions and places that the
memory size is stored in...

> 
>> +*/
>> +   u64 size;
>> };
>> 
>> struct ram_ops {
>> --
>> 2.1.4
>> 
> 
> Regards,
> Simon

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v2 2/6] dm: core: Allow multiple drivers to bind for a single node

[Dr. Philipp Tomsich]

Hi Simon,

> On 03 Mar 2017, at 05:52, Simon Glass  wrote:
> 
> Hi Philipp,
> 
> On 22 February 2017 at 13:47, Philipp Tomsich
>  > wrote:
>> Currently, driver binding stops once it encounters the first
>> compatible driver that doesn't refuse to bind. However, there are
>> cases where a single node will need to be handled by multiple driver
>> classes. For those cases we provide a configurable option to continue
>> to bind after the first driver has been found.
>> 
>> The first use cases for this are from the DM conversion of the sunxi
>> (Allwinner) architecture:
>> * pinctrl (UCLASS_PINCTRL) and gpio (UCLASS_GPIO) drivers need to
>>   bind against a single node
>> * clock (UCLASS_CLK) and reset (UCLASS_RESET) drivers also need to
>>   bind against a single node
> 
> Does linux work this way? Another approach would be to have a separate
> MISC driver with two children, one pinctrl, one clk.

The linux CLK driver creates and registers a reset-controller; the PINCTRL 
driver
does the same with the gpio-controller. Similar code to do this is easily 
possible in
U-Boot … see sunxi_pctrl_bind_gpio(…) in [PATCH v2 1/6] of this series.

However, binding multiple times makes for much simpler code and allows to keep
driver data in separate drivers.

Regards,
Philipp.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v3 0/9] sunxi: DM-based CLK, RESET and PINCTRL

[Dr. Philipp Tomsich]

Andre,

> On 06 Mar 2017, at 03:08, André Przywara  wrote:
> 
> On 01/03/17 21:19, Philipp Tomsich wrote:
>> Hi everyone,
>> 
>> here's the the new version of CLK, RESET and PINCTRL drivers to
>> configure sunxi from the device-tree.  This adds support for the
>> upstream CCU node (for reset and pinctrl) and tries to address the
>> various concerns people had.
>> 
>> Note that (to stay in sync with the Linux files), some of the
>> patches may not fully adhere to the style (e.g. some of the code
>> reused verbatim from Linux and in the config tables).
>> 
>> This has been tested with Ethernet, I2C, SPI and MMC on the A64-uQ7.
>> See my separate patchsets for the conversion of these drivers over
>> to support DM-based CLK, RESET and PINCTRL configuration.
>> 
>> Changes is v3:
>> * add support for the 'new-style' clock subsystem (CCU) by porting
>>   from Linux and adding the necessary glue implementation
> 
> Gah, is this really necessary? Do we really need to pull in the whole of
> the complex Linux clock driver?

It’s not necessary and I’d be more than happy to reduce the list of the
clocks in the table, but reusing the Linux driver (and it’s infrastructure)
allows us to add new SoCs by reusing the table from Linux.  However,
reducing the number of table entries will not reduce the “bulk” much,
as these are only config entries (and the code to support the various
types of clocks should be present nonetheless).

Given that these tables are fully tested on the Linux side, I believe this
to be of more value than to keep this small for U-Boot… after all, the
full CCU-support will only be used for the final U-Boot stage (which runs
from DRAM) and never considered for SPL/TPL.

> In the end all that U-Boot needs to do is to program a few simple
> clocks, a task which it easily did so far with just some register
> writes. I don't think it's appropriate for U-Boot to get the whole
> complex clock driver from Linux for just that purpose.

I beg to differ on the “a few simple clocks” aspect of this.
A full-featured U-Boot should require (including reparenting) clocks for
SPI, I2C, MMC, EMAC, USB (incl. PHYs) and video (on the SoCs where
simplefb is supported).

It’s best to keep all of this in sync with Linux, as this will simplify the
long-term maintenance.

> One indication of this being too much is that the mailing list seemed to
> have blocked patch 8/9, probably because it's too big ;-)
> I am especially scared when it comes to adding all of the other SoC's
> clock drivers to the code base as well.

It was just a couple bytes over the limit to require moderation ;-)

The benefit of this approach is that all the ccu_*.c clock implementations
and the configuration table can be reused almost verbatim from Linux.
The only change necessary will be in the list of headers included and
the change of the _ops datatype.

> So can't we do a much simpler (because limited) implementation?
> If a peripheral driver gets the clock index from the DT and asks the
> clock driver to program (or just enable) clock "75", can't we just have
> a simple function which redirects this clock number to our already
> existing code?
> Something like (rough sketch):
> int sun50i_clk_set_freq(int nr, int freq)
> {
>   int idx;
> 
>   switch (nr) {
>   case CLK_MMC0 ... CLK_MMC2:
>   idx = nr - CLK_MMC0;
>   return mmc_set_mod_clk(idx,
>  clkreg_addr[MMC_CLK + idx],
>  freq);
>   }
> }
> ... with mmc_set_mod_clk() being a slightly changed version of the
> existing implementation?

This will not help with many of the use-cases, such as reparenting clocks
dynamically as we change clocks on SPI.

It seems to be a good alternative implementation for the SPL use-case,
though and I’d be happy to add this in as the SPL-alternative.

> I think that would seriously reduce the bloat and be a better fit for
> U-Boot. And it might even allow the SPL to reuse this by just calling
> into the dispatch function with hardcoded parameters.

The total bulk added is not trivial, but appears to be be reasonable (for the
final-stage U-Boot):
1. code to support the various clocks: +33KB
2. worst-case config-table (i.e. what we have on the sun50iw1p1): +15KB

And the config-table can easily be reduced to a more manageable size
to removing clocks that we’ll never need (e.g. GPU, audio, …).

Cheers,
Philipp.

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 00/17] SPL: extend FIT loading support

[Dr. Philipp Tomsich]

Reviewed-by: Philipp Tomsich 

> On 01 Mar 2017, at 03:25, Andre Przywara  wrote:
> 
> This is an updated and slightly extended version of the SPL FIT loading
> series I posted as an RFC some weeks ago.
> I tried to fix all bugs that have been pointed out by the diligent
> reviewers, also added patches to automatically build the FIT images.
> 
> The first patch is a bug fix for a regression introduced with -rc1.
> I put this in there to allow people testing the series and to provide
> an actual patch for this fix, which should make it still into 2017.03.
> The next four patches introduce the core of the extened SPL FIT loading
> support, see below for a description.
> Patches 6-9 make some room in the sunxi 64-bit SPL to allow
> compiling in the FIT loading bits. Patch 10 and 11 let the SPL choose
> the proper DT from the FIT image.
> The next two patches add the infrastructure and an actual generator script,
> so the FIT image is automatically created at build time.
> Patches 14 and 15 enable the SPL FIT support in the Pine64 and the
> OrangePi PC 2 defconfigs.
> The last two patches are new and eventually store a DT file name in the
> SPL header, so U-Boot can easily pick the proper DT when scanning the
> FIT image. The idea is that this DT name should stay with the board,
> ideally on eMMC or SPI flash. So both U-Boot and a firmware update tool
> could identify a board, updating with compatible firmware while keeping
> the DT name in place. Ideally a board vendor would once seed this name
> onto on-board storage like SPI flash.
> 
> Let me know what you think!
> 
> Cheers,
> Andre.
> 
> Based on top of sunxi/master (35affe7698e9).
> 
> ---
> Currently the FIT format is not used to its full potential in the SPL:
> It only loads the first image from the /images node and appends the
> proper FDT.
> Some boards and platforms would benefit from loading more images before
> starting U-Boot proper, notably Allwinner A64 and ARMv8 Rockchip boards,
> which use an ARM Trusted Firmware (ATF) image to be executed before U-Boot.
> 
> This series tries to solve this in a board agnostic and generic way:
> We extend the SPL FIT loading scheme to allow loading multiple images.
> So apart from loading the image which is referenced by the "firmware"
> property in the respective configuration node and placing the DTB right
> behind it, we iterate over all strings in the "loadable" property.
> Each image referenced there will be loaded to its specified load address.
> The entry point U-Boot eventually branches to will be taken from the
> first image to explicitly provide the "entry" property, or, if none
> of them does so, from the load address of the "firmware" image.
> This keeps the scheme compatible with the FIT images our Makefile creates
> automatically at the moment.
> Apart from the already mentioned ATF scenario this opens up more usage
> scenarios, of which the commit message of patch 04/11 lists some.
> The remaining patches prepare ane finally enable this scheme for the 64-bit
> Allwinner boards.
> 
> Andre Przywara (15):
>  SPL: FIT: refactor FDT loading
>  SPL: FIT: rework U-Boot image loading
>  SPL: FIT: factor out spl_load_fit_image()
>  SPL: FIT: allow loading multiple images
>  tools: mksunxiboot: allow larger SPL binaries
>  armv8: SPL: only compile GIC code if needed
>  armv8: fsl: move ccn504 code into FSL Makefile
>  sunxi: A64: move SPL stack to end of SRAM A2
>  sunxi: SPL: store RAM size in gd
>  sunxi: SPL: add FIT config selector for Pine64 boards
>  Makefile: add rules to generate SPL FIT images
>  sunxi: A64: Pine64: introduce FIT generator script
>  sunxi: Pine64: defconfig: enable SPL FIT support
>  sunxi: OrangePi-PC2: defconfig: enable SPL FIT support
>  sunxi: use SPL header DT name for FIT board matching
> 
> Philipp Tomsich (1):
>  armv8: spl: Call spl_relocate_stack_gd for ARMv8
> 
> Siarhei Siamashka (1):
>  sunxi: Store the device tree name in the SPL header
> 
> Kconfig|  17 ++
> Makefile   |  20 +++
> arch/arm/cpu/armv8/fsl-layerscape/Makefile |   1 +
> arch/arm/include/asm/arch-sunxi/spl.h  |  19 ++-
> arch/arm/lib/Makefile  |   3 +-
> arch/arm/lib/crt0_64.S |  14 +-
> board/sunxi/board.c|  36 -
> board/sunxi/mksunxi_fit_atf.sh |  73 +
> common/spl/spl_fit.c   | 246 +
> configs/orangepi_pc2_defconfig |   6 +
> configs/pine64_plus_defconfig  |   6 +
> include/configs/sunxi-common.h |  17 +-
> scripts/Makefile.spl   |   3 +-
> tools/mksunxiboot.c|  51 +-
> 14 files changed, 387 insertions(+), 125 deletions(-)
> create mode 100755 board/sunxi/mksunxi_fit_atf.sh
> 
> --
> 2.8.2
> 



signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: [U-Boot] [PATCH 5/8] sunxi: Add selective DRAM type and timing

[Dr. Philipp Tomsich]

> On 15 Mar 2017, at 01:42, André Przywara  wrote:
> 
> On 11/03/17 16:19, Icenowy Zheng wrote:
>> DRAM chip varies, and one code cannot satisfy all DRAMs.
>> 
>> Add options to select a timing set.
>> 
>> Currently only DDR3-1333 (the original set) is added into it.
> 
> Yes, separating the timings sounds like a good idea. Eventually we
> should move these parameters into a data structure, but let's start easy
> here and do one step after another (unless you feel bored and want to
> mimic how Philipp did it [1]).

Just a heads-up: this is already cleaned up on one of my outbound trees with
a shared data structure for sun6i, sun9i and sun50i.
I am just holding off on submitting this until I have the device-model changes
out of my queue.

Cheers,
Philipp.


___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 6/6] dts: rk3399-puma: add gmac for the RK3399-Q7

[Dr. Philipp Tomsich]

Simon,

> On 26 Mar 2017, at 05:48, Simon Glass  wrote:
> 
> Hi Philipp,
> 
> On 24 March 2017 at 12:24, Philipp Tomsich
>  wrote:
>> This change enables the Gigabit Ethernet support on the RK3399-Q7.
>> 
>> X-AffectedPlatforms: RK3399-Q7
>> Signed-off-by: Philipp Tomsich 
>> ---
>> 
>> arch/arm/dts/rk3399-puma.dts | 30 ++
>> 1 file changed, 30 insertions(+)
> 
> Acked-by: Simon Glass 
> 
> But I don't see this board in mainline. What tree are you basing this patch 
> on?

This is still our prerelease development tree (I can publish via 
git.theobroma-systems.com ,
if that makes your and everyone’s life easier) and I only included this one to 
provide context
for the underlying change-set.

I’ll send the patches for device-tree and defconfig in one of the next patch 
submissions, but
have been holding back as we depend on Andre’s FIT image support.

Regards,
Philipp.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v2 1/2] rockchip: rk3399: spl: add UART0 support for SPL

[Dr. Philipp Tomsich]

Simon,

you’ll need
CONFIG_DEBUG_UART_BOARD_INIT=y
as include/debug_uart.h checks this macro and either defines this function 
inline (with
an empty body) or allows the function definition.

Seems like everyone just adds this to their defconfig (as did we), but I am 
open towards
an automatic selection of this for ROCKCHIP_RK3399 via Kconfig.

Regards,
Philipp.

> On 26 Mar 2017, at 04:38, Simon Glass  wrote:
> 
> Hi,
> 
> On 23 March 2017 at 20:12, Kever Yang  > wrote:
>> 
>> Hi Philipp,
>> 
>> 
>> On 03/24/2017 06:24 AM, Philipp Tomsich wrote:
>>> 
>>> The RK3399-Q7 ("Puma") SoM exposes UART0 as the Qseven UART (i.e. the
>>> serial line available via standardised pins on the edge connector and
>>> available on a RS232 connector).
>>> 
>>> To support boards (such as the RK3399-Q7) that require UART0 as a
>>> debug console, we match CONFIG_DEBUG_UART_BASE and add the appropriate
>>> iomux setup to the rk3399 SPL code.
>>> 
>>> As we are already touching this code, we also move the board-specific
>>> UART setup (i.e. iomux setup) into board_debug_uart_init(). This will
>>> be called from the debug UART init when CONFIG_DEBUG_UART_BOARD_INIT
>>> is set.
>>> 
>>> Signed-off-by: Philipp Tomsich 
>>> ---
>>> 
>>> Changes in v2:
>>> - Changed hex constant to lowercase
>>> 
>>>  arch/arm/include/asm/arch-rockchip/grf_rk3399.h |  8 +++
>>>  arch/arm/mach-rockchip/rk3399-board-spl.c   | 29 
>>> ++---
>>>  2 files changed, 29 insertions(+), 8 deletions(-)
> 
> This patch causes a build error for me:
> 
>   aarch64:  +   evb-rk3399
> +arch/arm/mach-rockchip/rk3399-board-spl.c:60:6: error: redefinition
> of 'board_debug_uart_init'
> + void board_debug_uart_init(void)
> +  ^
> +In file included from arch/arm/mach-rockchip/rk3399-board-spl.c:8:0:
> +include/debug_uart.h:68:20: note: previous definition of
> 'board_debug_uart_init' was here
> + static inline void board_debug_uart_init(void)
> +^
> +make[3]: *** [spl/arch/arm/mach-rockchip/rk3399-board-spl.o] Error 1
> +make[2]: *** [spl/arch/arm/mach-rockchip] Error 2
> +make[1]: *** [spl/u-boot-spl] Error 2
> +make: *** [sub-make] Error 2
> 
> 
> Regards,
> Simon

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v2] rockchip: mmc: rk3399: work around DMA issue in SPL

[Dr. Philipp Tomsich]

Kever,

we didn’t have time to track this down yet, so we’ve put this work-around
in place to be reverted once we isolate this issue.

The problem goes away once ATF is running in EL3 and U-Boot executes
in its normal privilege level… so our guess is that it’s either an issue with
running in EL3 or a configuration issue of the various protection registers.

Regards,
Philipp.

> On 29 Mar 2017, at 04:31, Kever Yang  wrote:
> 
> Hi Philipp,
> 
>So you got hang in SPL if the DWMMC is no in fifo mode, do you have
> 
> any clue for what's the root cause?
> 
> + Ziyuan,
> 
> Hi Ziyuan,
> 
>Could you double check this issue? Does it also happen at rk3288 dwmmc?
> 
> Thanks,
> - Kever
> On 03/29/2017 01:14 AM, Philipp Tomsich wrote:
>> The RK3399 hangs during DMA of the Designware MMC controller, when
>> performing DMA-based transactions in SPL.  To work around this issue,
>> we disable DMA-based access modes in the SPL stage.
>> 
>> With this fix in place, we can now drop 'fifo-mode' in the DTS for the
>> RK3399-Q7 (Puma).
>> 
>> Signed-off-by: Philipp Tomsich 
>> 
>> ---
>> 
>> Changes in v2:
>> - Fixes switching to fifo_mode (should have been 1) in SPL. I broke
>>   this at the 11th hour due to sloppy preparation of the patch.
>> 
>>  arch/arm/dts/rk3399-puma.dts  |  1 -
>>  drivers/mmc/rockchip_dw_mmc.c | 11 +++
>>  2 files changed, 11 insertions(+), 1 deletion(-)
>> 
>> diff --git a/arch/arm/dts/rk3399-puma.dts b/arch/arm/dts/rk3399-puma.dts
>> index 917df1e..71eb72d 100644
>> --- a/arch/arm/dts/rk3399-puma.dts
>> +++ b/arch/arm/dts/rk3399-puma.dts
>> @@ -91,7 +91,6 @@
>>  &sdmmc {
>>  u-boot,dm-pre-reloc;
>>  bus-width = <4>;
>> -fifo-mode; /* until we fix DMA in SPL */
>>  status = "okay";
>>  };
>>  diff --git a/drivers/mmc/rockchip_dw_mmc.c b/drivers/mmc/rockchip_dw_mmc.c
>> index c36eda0..5b4ed7a 100644
>> --- a/drivers/mmc/rockchip_dw_mmc.c
>> +++ b/drivers/mmc/rockchip_dw_mmc.c
>> @@ -76,6 +76,17 @@ static int rockchip_dwmmc_ofdata_to_platdata(struct 
>> udevice *dev)
>>  return -EINVAL;
>>  priv->fifo_mode = fdtdec_get_bool(gd->fdt_blob, dev_of_offset(dev),
>>"fifo-mode");
>> +
>> +#if defined(CONFIG_ROCKCHIP_RK3399) && defined(CONFIG_SPL_BUILD)
>> +/*
>> + * For a RK3399 SPL build, force fifo_mode to on (i.e. disable
>> + * DMA) as the MMC transaction will otherwise hang. This issue
>> + * reproduces only for SPL (i.e. BL2 in the ATF terminology),
>> + * but doesn't occur with the full U-Boot stage.
>> + */
>> +priv->fifo_mode = 1;
>> +#endif
>> +
>>  if (fdtdec_get_int_array(gd->fdt_blob, dev_of_offset(dev),
>>   "clock-freq-min-max", priv->minmax, 2))
>>  return -EINVAL;
> 
> 

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v2] rockchip: mmc: rk3399: work around DMA issue in SPL

[Dr. Philipp Tomsich]

Kever,

I did a bit of quick experiment by altering the DMA target addresses for
the DMA and can confirm that the root cause must be one of the security
registers.

The DMA target addresses are located on the SPL stack, which by default
grows down from 0x0008_ (so the addresses will be 0x0007_fxxx).
If I put the stack below 0x0100_ (DMA at 0x00ff_fxxx), I can revert
this patch and still have working DMA.

If you want to try yourself, you can use the CONFIG_SPL_STACK_R_ADDR
configuration to quickly change the address range for the DMA.

Looks like we need to add additional initialisation for some of the security
registers into arch/arm/mach-rockchip/rk3399-board-spl.c …

Regards,
Philipp.

> On 29 Mar 2017, at 09:51, Dr. Philipp Tomsich 
>  wrote:
> 
> Kever,
> 
> we didn’t have time to track this down yet, so we’ve put this work-around
> in place to be reverted once we isolate this issue.
> 
> The problem goes away once ATF is running in EL3 and U-Boot executes
> in its normal privilege level… so our guess is that it’s either an issue with
> running in EL3 or a configuration issue of the various protection registers.
> 
> Regards,
> Philipp.
> 
>> On 29 Mar 2017, at 04:31, Kever Yang  wrote:
>> 
>> Hi Philipp,
>> 
>>   So you got hang in SPL if the DWMMC is no in fifo mode, do you have
>> 
>> any clue for what's the root cause?
>> 
>> + Ziyuan,
>> 
>> Hi Ziyuan,
>> 
>>   Could you double check this issue? Does it also happen at rk3288 dwmmc?
>> 
>> Thanks,
>> - Kever
>> On 03/29/2017 01:14 AM, Philipp Tomsich wrote:
>>> The RK3399 hangs during DMA of the Designware MMC controller, when
>>> performing DMA-based transactions in SPL.  To work around this issue,
>>> we disable DMA-based access modes in the SPL stage.
>>> 
>>> With this fix in place, we can now drop 'fifo-mode' in the DTS for the
>>> RK3399-Q7 (Puma).
>>> 
>>> Signed-off-by: Philipp Tomsich 
>>> 
>>> ---
>>> 
>>> Changes in v2:
>>> - Fixes switching to fifo_mode (should have been 1) in SPL. I broke
>>>  this at the 11th hour due to sloppy preparation of the patch.
>>> 
>>> arch/arm/dts/rk3399-puma.dts  |  1 -
>>> drivers/mmc/rockchip_dw_mmc.c | 11 +++
>>> 2 files changed, 11 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/arch/arm/dts/rk3399-puma.dts b/arch/arm/dts/rk3399-puma.dts
>>> index 917df1e..71eb72d 100644
>>> --- a/arch/arm/dts/rk3399-puma.dts
>>> +++ b/arch/arm/dts/rk3399-puma.dts
>>> @@ -91,7 +91,6 @@
>>> &sdmmc {
>>> u-boot,dm-pre-reloc;
>>> bus-width = <4>;
>>> -   fifo-mode; /* until we fix DMA in SPL */
>>> status = "okay";
>>> };
>>> diff --git a/drivers/mmc/rockchip_dw_mmc.c b/drivers/mmc/rockchip_dw_mmc.c
>>> index c36eda0..5b4ed7a 100644
>>> --- a/drivers/mmc/rockchip_dw_mmc.c
>>> +++ b/drivers/mmc/rockchip_dw_mmc.c
>>> @@ -76,6 +76,17 @@ static int rockchip_dwmmc_ofdata_to_platdata(struct 
>>> udevice *dev)
>>> return -EINVAL;
>>> priv->fifo_mode = fdtdec_get_bool(gd->fdt_blob, dev_of_offset(dev),
>>>   "fifo-mode");
>>> +
>>> +#if defined(CONFIG_ROCKCHIP_RK3399) && defined(CONFIG_SPL_BUILD)
>>> +   /*
>>> +* For a RK3399 SPL build, force fifo_mode to on (i.e. disable
>>> +* DMA) as the MMC transaction will otherwise hang. This issue
>>> +* reproduces only for SPL (i.e. BL2 in the ATF terminology),
>>> +* but doesn't occur with the full U-Boot stage.
>>> +*/
>>> +   priv->fifo_mode = 1;
>>> +#endif
>>> +
>>> if (fdtdec_get_int_array(gd->fdt_blob, dev_of_offset(dev),
>>>  "clock-freq-min-max", priv->minmax, 2))
>>> return -EINVAL;
>> 
>> 
> 

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v3 05/19] SPL: FIT: allow loading multiple images

[Dr. Philipp Tomsich]

> On 03 Apr 2017, at 12:05, Andre Przywara  wrote:
> 
> On 03/04/17 10:40, Lukasz Majewski wrote:
>> Hi Andre ,
>> 
>>> So far we were not using the FIT image format to its full potential:
>>> The SPL FIT loader was just loading the first image from the /images
>>> node plus one of the listed DTBs.
>>> Now with the refactored loader code it's easy to load an arbitrary
>>> number of images in addition to the two mentioned above.
>>> As described in the FIT image source file format description, iterate
>>> over all images listed at the "loadables" property in the
>>> configuration node and load every image at its desired location.
>>> This allows to load any kind of images:
>>> - firmware images to execute before U-Boot proper (for instance
>>>  ARM Trusted Firmware (ATF))
>>> - firmware images for management processors (SCP, arisc, ...)
>>> - firmware images for devices like WiFi controllers
>>> - bit files for FPGAs
>>> - additional configuration data
>>> - kernels and/or ramdisks
>> 
>> Would it be possible to adopt this code to also load SDRAM controller
>> configuration data?
> 
> As it stands at the moment, this code is run when the SPL is finished
> with initialising the platform and is about to load and hand over to
> U-Boot proper.
> It would need to be investigated if the device driver required to
> actually load the FIT image works without DRAM being available (as in:
> do we have enough heap and stack, for instance).
> Also I am not sure the FIT (fdt) parsing code is really tuned for low
> memory, since normally it runs with DRAM already available.
> 
> So while this doesn't sound impossible, it's probably some work ahead,
> depending on the resource limitation you have in SPL on your platform.

The more general solution would be to move towards having a TPL in
addition to the SPL…

Regards,
Philipp.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] usb: dwc3: gadget: make cache-maintenance on event buffers more robust

[Dr. Philipp Tomsich]

> On 04 Apr 2017, at 18:15, Marek Vasut  wrote:
> 
> On 04/03/2017 07:49 PM, Philipp Tomsich wrote:
>> Merely using dma_alloc_coherent does not ensure that there is no stale
>> data left in the caches for the allocated DMA buffer (i.e. that the
>> affected cacheline may still be dirty).
>> 
>> The original code was doing the following (on AArch64, which
>> translates a 'flush' into a 'clean + invalidate'):
>>  # during initialisation:
>>  1. allocate buffers via memalign
>>   => buffers may still be modified (cached, dirty)
>>  # during interrupt processing
>>  2. clean + invalidate buffers
>>   => may commit stale data from a modified cacheline
>>  3. read from buffers
>> 
>> This could lead to garbage info being written to buffers before
>> reading them during even-processing.
>> 
>> To make the event processing more robust, we use the following sequence
>> for the cache-maintenance:
>>  # during initialisation:
>>  1. allocate buffers via memalign
>>  2. clean + invalidate buffers
>>   (we only need the 'invalidate' part, but dwc3_flush_cache()
>>always performs a 'clean + invalidate')
>>  # during interrupt processing
>>  3. read the buffers
>>   (we know these lines are not cached, due to the previous
>>invalidation and no other code touching them in-between)
>>  4. clean + invalidate buffers
>>   => writes back any modification we may have made during event
>>  processing and ensures that the lines are not in the cache
>>  the next time we enter interrupt processing
>> 
>> Note that with the original sequence, we observe reproducible
>> (depending on the cache state: i.e. running dhcp/usb start before will
>> upset caches to get us around this) issues in the event processing (a
>> fatal synchronous abort in dwc3_gadget_uboot_handle_interrupt on the
>> first time interrupt handling is invoked) when running USB mass
>> storage emulation on our RK3399-Q7 with data-caches on.
>> 
>> Signed-off-by: Philipp Tomsich 
>> 
>> ---
>> 
>> drivers/usb/dwc3/core.c   | 2 ++
>> drivers/usb/dwc3/gadget.c | 5 +++--
>> 2 files changed, 5 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index b2c7eb1..f58c7ba 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -125,6 +125,8 @@ static struct dwc3_event_buffer 
>> *dwc3_alloc_one_event_buffer(struct dwc3 *dwc,
>>  if (!evt->buf)
>>  return ERR_PTR(-ENOMEM);
>> 
>> +dwc3_flush_cache((long)evt->buf, evt->length);
>> +
> 
> Is the length aligned ? If not, you will get cache alignment warning.
> Also, address should be uintptr_t to avoid 32/64 bit issues .

The length is a well-known value and aligned (it expands to PAGE_SIZE in the 
end).
Good point on the “long”, especially as I just copied this from other 
occurences and it’s consistently wrong throughout DWC3 in U-Boot:
drivers/usb/dwc3/core.c:dwc3_flush_cache((long)evt->buf, 
evt->length);
drivers/usb/dwc3/ep0.c: dwc3_flush_cache((long)buf_dma, len);
drivers/usb/dwc3/ep0.c: dwc3_flush_cache((long)trb, sizeof(*trb));
drivers/usb/dwc3/ep0.c: dwc3_flush_cache((long)trb, sizeof(*trb));
drivers/usb/dwc3/ep0.c: dwc3_flush_cache((long)trb, 
sizeof(*trb));
drivers/usb/dwc3/ep0.c: dwc3_flush_cache((long)dwc->ep0_bounce, 
DWC3_EP0_BOUNCE_SIZE);
drivers/usb/dwc3/gadget.c:  
dwc3_flush_cache((long)req->request.dma, req->request.length);
drivers/usb/dwc3/gadget.c:  dwc3_flush_cache((long)dma, length);
drivers/usb/dwc3/gadget.c:  dwc3_flush_cache((long)trb, 
sizeof(*trb));
drivers/usb/dwc3/gadget.c:  dwc3_flush_cache((long)trb, 
sizeof(*trb));
drivers/usb/dwc3/gadget.c:  
dwc3_flush_cache((long)evt->buf, evt->length);
drivers/usb/dwc3/io.h:static inline void dwc3_flush_cache(int addr, int 
length)

Worst of all: the definition of dwc3_flush_cache in io.h has “int” as a type, 
which will eat us alive if the DWC3’s physical address is beyond 32-bit.

I’ll revise all of these and make a patch-series out of this.

>>  return evt;
>> }
>> 
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index 1156662..61af71b 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -2668,11 +2668,12 @@ void dwc3_gadget_uboot_handle_interrupt(struct dwc3 
>> *dwc)
>>  int i;
>>  struct dwc3_event_buffer *evt;
>> 
>> +dwc3_thread_interrupt(0, dwc);
>> +
>> +/* Clean + Invalidate the buffers after touching them */
>>  for (i = 0; i < dwc->num_event_buffers; i++) {
>>  evt = dwc->ev_buffs[i];
>>  dwc3_flush_cache((long)evt->buf, evt->length);
>>  }
>> -
> 
> This makes me wonder, don't you need to invalidate the even

Re: [U-Boot] [PATCH] usb: dwc3: gadget: make cache-maintenance on event buffers more robust

[Dr. Philipp Tomsich]

> On 04 Apr 2017, at 21:01, Marek Vasut  wrote:
> 
>> Good point on the “long”, especially as I just copied this from other 
>> occurrences and it’s consistently wrong throughout DWC3 in U-Boot:
> 
> Hrm, I thought the driver was ported over from Linux, so is this broken
> in Linux too ?

Apparently, the dwc3_flush_cache calls (and the function itself) have been
introduced during the porting. There’s no explicit cache-maintenance in DWC3
for Linux. 

>> I’ll revise all of these and make a patch-series out of this.
> 
> Maybe you should check the Linux first and see if there are some fixes
> already.
> 
> Thanks

Given that this seems to have been introduced with the port to U-Boot, there’s
no applicable fixes there.

return evt;
 }
 
 diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
 index 1156662..61af71b 100644
 --- a/drivers/usb/dwc3/gadget.c
 +++ b/drivers/usb/dwc3/gadget.c
 @@ -2668,11 +2668,12 @@ void dwc3_gadget_uboot_handle_interrupt(struct 
 dwc3 *dwc)
int i;
struct dwc3_event_buffer *evt;
 
 +  dwc3_thread_interrupt(0, dwc);
 +
 +  /* Clean + Invalidate the buffers after touching them */
for (i = 0; i < dwc->num_event_buffers; i++) {
evt = dwc->ev_buffs[i];
dwc3_flush_cache((long)evt->buf, evt->length);
}
 -
>>> 
>>> This makes me wonder, don't you need to invalidate the event buffer
>>> somewhere so that the new data would be fetched from RAM ?
>> 
>> We flush the event buffer before leaving the function.
>> So the cache line will not be present in the cache, when we enter this 
>> function again.
> 
> Then shouldn't we invalidate it instead ? flush and invalidate are two
> different things …

The DWC3 flush expands to a clean+invalidate. It is not wrong, as long as
it is used as in my patch:
a. before the first time data is expected to be written by the peripheral (i.e.
before the peripheral is started)—to ensure that the cache line is not cached
any longer…
b. after the driver modifies any buffers (i.e. anything modified will be written
back) and before it next reads the buffers expecting possibly changed data
(i.e. invalidating).

Regards,
Philipp.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] usb: dwc3: gadget: make cache-maintenance on event buffers more robust

[Dr. Philipp Tomsich]

> On 04 Apr 2017, at 22:09, Marek Vasut  wrote:
> 
>> The DWC3 flush expands to a clean+invalidate. It is not wrong, as long as
>> it is used as in my patch:
>> a. before the first time data is expected to be written by the peripheral 
>> (i.e.
>> before the peripheral is started)—to ensure that the cache line is not cached
>> any longer…
> 
> So invalidate() is enough ?

If I had to write this from scratch, I’d got with the paranoid sequence of:

handler():
{
invalidate
do my stuff
clean
}

However, some architectures in U-Boot (e.g. ARMv8) don’t implement the
invalidate verb. Given this, I’d rather stay as close to what’s already there.

Note that using flush (i.e. clean+invalidate) aligns with how caches are
managed throughout various other drivers in U-Boot.

> 
>> b. after the driver modifies any buffers (i.e. anything modified will be 
>> written
>> back) and before it next reads the buffers expecting possibly changed data
>> (i.e. invalidating).
> 
> So flush+invalidate ? Keep in mind this driver may not be used on
> ARMv7/v8 only …

Yes, a clean+invalidate.
The flush_dcache_range(…, …) function in U-Boot implements C+I semantics
at least on arm, arm64, avr32, powerpc, xtensa …

Regards,
Philipp.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] usb: dwc3: gadget: make cache-maintenance on event buffers more robust

[Dr. Philipp Tomsich]

Felipe,

> On 05 Apr 2017, at 10:18, Felipe Balbi  wrote:
> 
>>> Good point on the “long”, especially as I just copied this from other 
>>> occurences and it’s consistently wrong throughout DWC3 in U-Boot:
>> 
>> Hrm, I thought the driver was ported over from Linux, so is this broken
>> in Linux too ?
> 
> haven't seen a problem in almost 6 years dealing with this IP.

The integer-sizes on the flushing really aren’t a big issue, as everyone runs 
from the lower 32bits as of today.
And it could easily be another 6 years, before we hit the first 64bit address 
for any of the buffers being flushed.
Even as the integer types on the dwc3_flush_range are consistently mismatches, 
that is just a sideshow and
doesn’t cause any issues for anyone.

The big one for us is really the patch submitted to reorder the flushes (i.e. 
clean+invalidate operations),
as we sometimes (depends both on what happened before that in U-Boot — e.g. 
using the network
stack will always hide this — and on what configuration we compile into U-Boot) 
have cachelines
matching the allocation via dma_alloc_coherent either as cached (or possibly 
even as modified) in our
cache.

Any opinion on changing the sequencing of cache-maintenance relative to the 
payload?

Regards,
Philipp.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] usb: dwc3: gadget: make cache-maintenance on event buffers more robust

[Dr. Philipp Tomsich]

> On 05 Apr 2017, at 12:25, Marek Vasut  wrote:
> 
> On 04/04/2017 10:26 PM, Dr. Philipp Tomsich wrote:
>> 
>>> On 04 Apr 2017, at 22:09, Marek Vasut  wrote:
>>> 
>>>> The DWC3 flush expands to a clean+invalidate. It is not wrong, as long as
>>>> it is used as in my patch:
>>>> a. before the first time data is expected to be written by the peripheral 
>>>> (i.e.
>>>> before the peripheral is started)—to ensure that the cache line is not 
>>>> cached
>>>> any longer…
>>> 
>>> So invalidate() is enough ?
>> 
>> If I had to write this from scratch, I’d got with the paranoid sequence of:
>> 
>>  handler():
>>  {
>>  invalidate
>>  do my stuff
>>  clean
>>  }
>> 
>> However, some architectures in U-Boot (e.g. ARMv8) don’t implement the
>> invalidate verb. Given this, I’d rather stay as close to what’s already 
>> there.
> 
> invalidate_dcache_range() must be implemented if flush_dcache_range()
> is, otherwise it's a bug.

The ARMv8 implementation for invalidate currently maps back to a 
clean+invalidate
(see arch/arm/cpu/armv8/cache_v8.c):

/*
 * Invalidates range in all levels of D-cache/unified cache
 */
void invalidate_dcache_range(unsigned long start, unsigned long stop)
{
__asm_flush_dcache_range(start, stop);
}

/*
 * Flush range(clean & invalidate) from all levels of D-cache/unified 
cache
 */
void flush_dcache_range(unsigned long start, unsigned long stop)
{
__asm_flush_dcache_range(start, stop);
}

I am a bit scared of either using (as this clearly is mislabeled) or changing 
(as
other code might depend on things being as they are) the invalidate-function
for ARMv8 at this point.

>> Note that using flush (i.e. clean+invalidate) aligns with how caches are
>> managed throughout various other drivers in U-Boot.
>> 
>>> 
>>>> b. after the driver modifies any buffers (i.e. anything modified will be 
>>>> written
>>>> back) and before it next reads the buffers expecting possibly changed data
>>>> (i.e. invalidating).
>>> 
>>> So flush+invalidate ? Keep in mind this driver may not be used on
>>> ARMv7/v8 only …
>> 
>> Yes, a clean+invalidate.
>> The flush_dcache_range(…, …) function in U-Boot implements C+I semantics
>> at least on arm, arm64, avr32, powerpc, xtensa …
> 
> flush on arm926 does not invalidate the cacheline iirc .

I dug up an ARMv5 architecture manual (I didn’t think I’d ever need this again) 
to
look at what the ARM926 does.

Here’s the code for reference:
void flush_dcache_range(unsigned long start, unsigned long stop)
{
if (!check_cache_range(start, stop))
return;

while (start < stop) {
asm volatile("mcr p15, 0, %0, c7, c14, 1\n" : : 
"r"(start));
start += CONFIG_SYS_CACHELINE_SIZE;
}

asm volatile("mcr p15, 0, %0, c7, c10, 4\n" : : "r"(0));
}

c7 are the cache-management functions, with the following opcodes:
—   “c7, c14, 1” is “Clean and invalidate data cache line” on the modified 
virtual-address (MVA)
—   “c7, c10, 4” is "Data Synchronization Barrier (formerly Drain Write 
Buffer)"

This discussion shows that (at some future point… and no, I am not volunteering
for this) the naming of the cache-maintenance functions and the documentation 
for
them should be reworked to avoid any confusion for the casual driver developer.

I’ll just go ahead and put together a v2 that also addresses the pointer-size 
concerns,
as I don’t want to have the fix for our DWC3 issue held up by this.


Regards,
Philipp.


___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v2 14/18] sunxi: Pine64: defconfig: enable SPL FIT support

[Dr. Philipp Tomsich]

> On 29/03/17 07:57, Maxime Ripard wrote:
>> On Tue, Mar 28, 2017 at 01:45:22AM +0100, Andre Przywara wrote:
>>> The Pine64 (and all other 64-bit Allwinner boards) need to load an
>>> ARM Trusted Firmware image beside the actual U-Boot proper.
>>> This can now be easily achieved by using the just extended SPL FIT
>>> loading support, so enable it in the Pine64 defconfig.
>>> Also add the FIT image as a build target to 64-bit sunxi board to
>>> trigger the respective Makefile rules.
>>> 
>>> Signed-off-by: Andre Przywara 
>>> ---
>>> configs/pine64_plus_defconfig  | 6 ++
>>> include/configs/sunxi-common.h | 4 
>>> 2 files changed, 10 insertions(+)
>>> 
>>> diff --git a/configs/pine64_plus_defconfig b/configs/pine64_plus_defconfig
>>> index 92bda60..8a90579 100644
>>> --- a/configs/pine64_plus_defconfig
>>> +++ b/configs/pine64_plus_defconfig
>>> @@ -3,9 +3,14 @@ CONFIG_ARCH_SUNXI=y
>>> CONFIG_MACH_SUN50I=y
>>> CONFIG_RESERVE_ALLWINNER_BOOT0_HEADER=y
>>> CONFIG_DEFAULT_DEVICE_TREE="sun50i-a64-pine64-plus"
>>> +CONFIG_OF_LIST="sun50i-a64-pine64 sun50i-a64-pine64-plus"
>>> # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
>>> CONFIG_CONSOLE_MUX=y
>>> CONFIG_SPL=y
>>> +CONFIG_FIT=y
>>> +CONFIG_SPL_FIT=y
>>> +CONFIG_SPL_LOAD_FIT=y
>>> +CONFIG_SPL_OF_LIBFDT=y
>> 
>> Again, this doesn't make any sense to enable it in *all* our
>> defconfigs. If this is something that should be enabled by default for
>> the A64 support, then do so in Kconfig directly.
> 
> Oh sorry, I think I completely misunderstood you last time.
> Indeed this makes sense, especially with more defconfigs for A64/H5
> boards coming up.
> 
> I will think about a solution that makes Icenowy happy as well,
> something like BOARD_NEEDS_SPL_FIT or so - though a simple "select"
> sounds tempting ;-)

In fact we combine this with
> # CONFIG_SPL_RAW_IMAGE_SUPPORT is not set
> # CONFIG_SPL_LEGACY_IMAGE_SUPPORT is not set

for our boards, so we can iterate through a list of potential boot sources than 
could contain a FIT image.

Cheers,
Phil.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v1 1/2] sunxi (sun50i): support i2c on A64 (pin-config, clocking)

[Dr. Philipp Tomsich]

> On 18 Feb 2017, at 02:15, André Przywara  wrote:
> 
>> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
>> index bba9b7c..a47b113 100644
>> --- a/board/sunxi/board.c
>> +++ b/board/sunxi/board.c
>> @@ -485,90 +485,108 @@ int board_mmc_init(bd_t *bis)
>> void i2c_init_board(void)
>> {
>> #ifdef CONFIG_I2C0_ENABLE
>> #if defined(CONFIG_MACH_SUN4I) || defined(CONFIG_MACH_SUN5I) || 
>> defined(CONFIG_MACH_SUN7I)
>>  sunxi_gpio_set_cfgpin(SUNXI_GPB(0), SUN4I_GPB_TWI0);
>>  sunxi_gpio_set_cfgpin(SUNXI_GPB(1), SUN4I_GPB_TWI0);
>>  clock_twi_onoff(0, 1);
>> #elif defined(CONFIG_MACH_SUN6I)
>>  sunxi_gpio_set_cfgpin(SUNXI_GPH(14), SUN6I_GPH_TWI0);
>>  sunxi_gpio_set_cfgpin(SUNXI_GPH(15), SUN6I_GPH_TWI0);
>>  clock_twi_onoff(0, 1);
>> #elif defined(CONFIG_MACH_SUN8I)
>>  sunxi_gpio_set_cfgpin(SUNXI_GPH(2), SUN8I_GPH_TWI0);
>>  sunxi_gpio_set_cfgpin(SUNXI_GPH(3), SUN8I_GPH_TWI0);
>>  clock_twi_onoff(0, 1);
>> +#elif defined(CONFIG_MACH_SUN50I)
>> +sunxi_gpio_set_cfgpin(SUNXI_GPH(0), SUN50I_GPH_TWI0);
>> +sunxi_gpio_set_cfgpin(SUNXI_GPH(1), SUN50I_GPH_TWI0);
>> +/* The A64-uQ7 doesn't have external pull-ups for I2C[01]. */
>> +sunxi_gpio_set_pull(SUNXI_GPH(0), SUNXI_GPIO_PULL_UP);
>> +sunxi_gpio_set_pull(SUNXI_GPH(1), SUNXI_GPIO_PULL_UP);
> 
> Is this specific to your board?

No. This is board-specific for any board.
Every I2C bus will need pull-ups, but it’s not defined where on the bus these
should/will be located.  There’s even a few I2C peripherals that have the
pull-ups integrated.

In other words: this was one of many things that triggered me going after
support DM (and it was the one change that caused me to look at the size
of the SPL and whether there’s still space available for DM drivers when
using OF_PLATDATA … but we both know the conclusion).

> Which isn't officially supported yet?

I can send patches with the board-specific DTS and defconfig, if that helps ;-)

Just wanted until we get dual-IO SPI reads validated (and the DM changes
merged), so it can be a single changeset instead of relying on multiple updates
to the same files.

If you think it’s worthwhile to send out a defconfig that doesn’t depend on DM
together with the DTS (without the gpiobank-entries), let me know.

> And having a DM conversion would make this easily fixable via DT, I guess?

Yes. But that will still leave the SPL/TPL discussion for another day, as the
SPL still depends on these programmatic settings.

Our board’s early-stage normally shouldn't need to do anything on this I2C
bus, but customer boards may come up with a use for it (image this going
towards battery/power managing circuitry on a baseboard).

Regards,
Philipp.

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v1] env_mmc: configure environment offsets via device tree

[Dr. Philipp Tomsich]

Igor,

> On 19 Feb 2017, at 12:52, Igor Grinberg  wrote:
> 
> On 02/17/17 19:28, Philipp Tomsich wrote:
>> This introduces the ability to override the environment offets from the
>> device tree by setting the following nodes in '/config':
>>  'u-boot,mmc-env-offset' - overrides CONFIG_ENV_OFFSET
>>  'u-boot,mmc-env-offset-redundant'
>>  - overrides CONFIG_ENV_OFFSET_REDUND
>> 
>> To keep with the previous logic, the CONFIG_* defines still need to
>> be available and the statically defined values become the defaults,
>> when the corresponding properties are not set in the device-tree.
> 
> That sounds too odd...
> DT's purpose is to describe the h/w... and that does not look so...
> We also, have a dt file name in the environment, so this creates will create
> a chicken and an egg problem…

I don’t really follow… as far as I knew the DT name would have to come
from some other source anyway, as the device containing the env might
only be described through the device tree (e.g. mmc0).

U-Boot usually locates the FDT to use at a preconfigured address (or one
configured in the early environment). This will not be the environment we’d
load from a MMC device configured via DM_MMC.

So there shouldn’t be a chicken & egg here.

> I really don't think we should go that direction. DT is not meant to provide
> a solution to all your problems...

I don’t see how this is different from other entries in chosen and config as
of today:
common/autoboot.c allows an override through /config/bootdelay
common/board_r.c uses /config/load-environment
common/cli.c can pull in /config/bootcmd
drivers/serial/serial-uclass.c uses /chosen/stdout-path

In fact, it is the absence of this mechanism that is causing problems today:
CONFIG_ENV_OFFSET is not configurable through Kconfig, so we would
need board-specific defines (e.g. CONFIG_SUNXI_BOARD_LYNX) and
matching #ifdef primitives in a shared header (sunxi-common.h in our case).

So putting this in the DT is the best (and least intrusive) option available.

Regards,
Philipp.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v1] sunxi: improve throughput in the sunxi_mmc driver

[Dr. Philipp Tomsich]

> On 20 Feb 2017, at 07:16, Chen-Yu Tsai  wrote:
> 
> On Mon, Feb 20, 2017 at 6:08 AM, Jaehoon Chung  > wrote:
>> Hi,
>> 
>> On 02/18/2017 02:22 AM, Philipp Tomsich wrote:
>>> Throughput tests have shown the sunxi_mmc driver to take over 10s to
>>> read 10MB from a fast eMMC device due to excessive delays in polling
>>> loops.
>>> 
>>> This commit restructures the main polling loops to use get_timer(...)
>>> to determine whether a (millisecond) timeout has expired.  We choose
>>> not to use the wait_bit function, as we don't need interruptability
>>> with ctrl-c and have at least one case where two bits (one for an
>>> error condition and another one for completion) need to be read and
>>> using wait_bit would have not added to the clarity.
>>> 
>>> The observed speedup in testing on a A31 is greater than 10x (e.g. a
>>> 10MB write decreases from 9.302s to 0.884s).
>> 
>> Your patch's format looks strange.
>> Some unchanging codes are included in patches.
>> Except them, Looks good to me.
>> If you will resend the patch, you can resend with my acked-by tag.
> 
> They were probably produced the patches with "git format-patch
> --function-context", which includes the whole body of any changed
> functions.

Yes, that’s my default setting for easier review (having the context helps
spotting logic-issues, in my experience).

I’ll just reroll with less context for a v2, as the consensus prefers the
smaller context.

Regards,
Phil.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v1] env_mmc: configure environment offsets via device tree

[Dr. Philipp Tomsich]

> On 20 Feb 2017, at 08:22, Igor Grinberg  wrote:
> 
>>> That sounds too odd...
>>> DT's purpose is to describe the h/w... and that does not look so...
>>> We also, have a dt file name in the environment, so this creates will create
>>> a chicken and an egg problem…
>> 
>> I don’t really follow… as far as I knew the DT name would have to come
>> from some other source anyway, as the device containing the env might
>> only be described through the device tree (e.g. mmc0).
> 
> Why? U-Boot can live pretty well w/o DT.

If U-Boot runs without DT, then nothing will/should change about how the setting
is retrieved from CONFIG_ENV_OFFSET.

The platform that motivated this change is ARCH_SUNXI, which does not use
per-board defines but aims to have one generic bootloader per-SoC.

>>> I really don't think we should go that direction. DT is not meant to provide
>>> a solution to all your problems...
>> 
>> I don’t see how this is different from other entries in chosen and config as
>> of today:
>>  common/autoboot.c allows an override through /config/bootdelay
>>  common/board_r.c uses /config/load-environment
>>  common/cli.c can pull in /config/bootcmd
>>  drivers/serial/serial-uclass.c uses /chosen/stdout-path
>> 
>> In fact, it is the absence of this mechanism that is causing problems today:
>> CONFIG_ENV_OFFSET is not configurable through Kconfig, so we would
>> need board-specific defines (e.g. CONFIG_SUNXI_BOARD_LYNX) and
>> matching #ifdef primitives in a shared header (sunxi-common.h in our case).
> 
> Right. Exactly, I think we should move the CONFIG_ENV_OFFSET to Kconfig.
> And that will solve the problem.

Doing this would still get into the way of architectures that want to build a 
single
‘universal’ bootloader for their SoC: the ENV_OFFSET may not be the same
across all board and vendor configurations. This can easily be handled with the
(optional) prop in the DT, but not with the compile-time ENV_OFFSET.

If we decide to this, I’d at least like to introduce the function call to (the 
weak
function) mmc_get_env_addr(…), so we can override this in the board code.

>> So putting this in the DT is the best (and least intrusive) option available.
> 
> Ok. I see your point now. Yet I think we should keep the DT to its purpose - 
> which
> is to describe the h/w (and not the s/w placement layout).
> 
> 
> -- 
> Regards,
> Igor.

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v1 1/2] sunxi (sun50i): support i2c on A64 (pin-config, clocking)

[Dr. Philipp Tomsich]

> On 20 Feb 2017, at 11:18, Andre Przywara  wrote:
> 
>> Our board’s early-stage normally shouldn't need to do anything on this I2C
>> bus, but customer boards may come up with a use for it (image this going
>> towards battery/power managing circuitry on a baseboard).
> 
> In this case we should definitely remove these three lines.

I should have said “we don’t do anything with this I2C bus, but it’s available
for customer use via the edge connector and a header on our eval-board”.

As there’s no way to enable pull-ups via a command in U-Boot, I’d prefer
to keep these pull ups configured.

Regards,
Philipp.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v1 02/11] sunxi: add pinctrl (UCLASS_PINCTRL) support for sunxi and tie back into GPIO

[Dr. Philipp Tomsich]

Chen,

> On 21 Feb 2017, at 04:48, Chen-Yu Tsai  wrote:
> 
> On Sat, Feb 18, 2017 at 1:52 AM, Philipp Tomsich
>  > wrote:
>> This change adds a full device-model pinctrl driver for sunxi (tested with
>> sun50iw1p1) based on the support available in Linux.
>> 
>> Details are:
>> * implements a driver for pinctrl devices and assigns sun50i-a64-pinctrl
>>   and sun50i-a64-r-pinctrl to it
>> * defines and implements a binding for sunxi-style GPIO banks (to make it
>>   easier to describe controllers like the A64 which may not start at 'A')
>>   and provide the necessary translation mechanisms:
>>   - As all our gpio-reference point back to either <&pio ..> or <&r_pio ..>
>> the device framework will try to access a UCLASS_GPIO device bound to
>> the same node id as the pinctrl device to perform it's of_xlate lookup.
>> For this, we provide a 'gpiobridge' driver (which needs to access the
>> platdata of the pinctrl device) and which can then map accesses to an
>> actual GPIO bank device.
>>   - For the individual GPIO banks, we use a new driver (which shares most
>> of its ops with the existing sunxi_gpio driver, except probe and bind)
>> and provides configuration without any platdata structure.
> 
> Why is the new binding and driver necessary? The existing driver in U-boot is
> already DM enabled and properly xlates GPIO phandles to its child GPIO banks.
> I specifically fixed this in commit 4694dc56e974 ("sunxi: gpio: Add .xlate
> function for gpio phandle resolution”).

The problem is that all GPIO is referenced to the pinctrl-node.  As we need to
provide a pinctrl driver (so the configuration of pins can go through that) for
the pinconfig to work, the GPIOs are then referenced to that node.

We had considered multiple solutions
(a) the one we’ve chosen, which avoids having to provide additional data
outside the device-tree to track the number of banks and bank-size
(b) binding the existing gpio driver to the same node (but as the driverdata
needs to be matched as well, we’d need to select this based on the
compatible string and have a tighter coupling between the drivers than
strictly necessary)
(c) dynamically creating gpio subnodes (just as done by the top-level
sunxi_gpio instances) for each of the banks from within the pinctrl 
driver

Another reason why we felt the gpiobank to be helpful is that it allows us to
easily describe where the register sets (i.e. PIO and IRQ) are for each bank.

> You are also not using the new gpiobank nodes anywhere in your dts changes.

The DTS change is in the same patch as the the pinctrl driver:
http://lists.denx.de/pipermail/u-boot/2017-February/281637.html 


I had generated this with full function context, which makes the diff in the DTS
hard to spot.

>> +#if defined(CONFIG_SUNXI_PINCTRL)
>> +
>> +/* When we use the sunxi pinctrl infrastructure, we have two issues that
>> + * are resolved by the gpiobank and gpiobrige drivers:
>> + *
>> + *  - We need to have a UCLASS_GPIO device bound to the pinctrl-nodes for
>> + *translating between gpio entries (e.g. <&pio 3 24 GPIO_ACTIVE_LOW>;)
>> + *in the DT and actual gpio banks. This is done using the gpiobridge
>> + *driver, which provides only a lookup/translation mechanism.
>> + *This mechanism lives in pinctrl-sunxi.c
>> + *
>> + *  - We introduce a generic gpiobank device, which resolves the need to
>> + *have distinct soc_data structure for each device and avoids having
>> + *to share data structures and config data between this file and the
>> + *sunxi pinctrl (the other option would be to have the soc_data info
>> + *visible in pinctrl-sunxi.c (or merge it into this file) and bind a
>> + *gpio_sunxi device that is set up with the appropriate soc_data) to
>> + *the same node as the pinctrl device.
> 
> Pushing hardware internals into the DT is not preferred.

That’s exactly the point: the current DT format encapsulates the internal
grouping of the hardware blocks into larger address-decoding groups. For
the case of pinctrl and GPIO, the actual structure looks as follows:
+ “bunch of registers”
+ N x [pinctrl functionality for a pin-group]
+ N x [irq-control for a pin-group]
+ N x [GPIO functionality for a pin-group]

So if I were asked to model this from scratch, I’d use a regmap-device for
the PIO and R_PIO address spaces and then reference the pinctrl and gpio
functionality back to it (hiding the internals of how each bank assigns bits
and where).

> Since the pinctrl and gpio drivers actually driver the same hardware block,
> just in different ways, and there should be some locking between the two,
> I think it would make sense to merge the two.
> 
> You can also get away with not having soc_data by

Re: [U-Boot] [PATCH v1 0/2] disk: efi: allow gap before partition entries

[Dr. Philipp Tomsich]

Maxime,

> On 21 Feb 2017, at 18:45, Maxime Ripard  
> wrote:
> 
> Hi Philipp,
> 
> On Fri, Feb 17, 2017 at 06:31:29PM +0100, Philipp Tomsich wrote:
>> Motivated by the the SPL layout for SD/MMC devices on Allwinner SoCs
>> (the SPL code needs to reside an 8K offset into the device), we add
>> support for leaving a gap between the MBR (LBA#0), GPT header (LBA#1)
>> and GPT partition entries (linked from field in the GPT header).
>> 
>> Note that this affects the creation of partition from U-Boot only and
>> has no effect on reading existing partition tables.
>> 
>> If defined, EFI_PARTITION_ENTRIES_OFF specifies a byte-offset into
>> a device and the parititon entries will be located starting at the
>> next LBA folling this offset.
>> 
>> In the latest incarnation of this change, we also allow the byte
>> offset to be read from the 'u-boot,efi-partition-entries-offset'
>> property of the '/config' node in the device-tree.
>> 
>> There's a similar change for gparted floating around the internet:
>>  
>> http://lists.alioth.debian.org/pipermail/parted-devel/2016-March/004826.html
>> 
>> This change has been in production use on our modules for a while
>> (over a year) both with a Linux and Android userspace.
> 
> Nice patch. We came up with the same need, but had to restrict the
> number of partitions in order to achieve the same goel. This is
> obviously a much better solution.
> 
> However, I'm a bit skeptical on the /config node. First, this node
> doesn't exist at all, and needs to be documented and acked by the DT
> maintainers. And why would one need to change that per device?

We are currently using the #define for production deployments, but
we have gotten the feedback that it would be nice to change this setting
easily without recompiling.

We also use different settings for the sun6i/sun9i and sun50i modules,
as code-size increases with AArch64 code...

> Also, and this is something that affects all your patches, you
> generated them with way too many context, which makes review more
> difficults, and make them more conflicts prone. git format-patch
> should just do the right thing.

Yes, I noticed that too, but apparently had my setup misconfigured on
Friday—you can probably imagine that pushing all these out on one
day was a bit hectic.

> Threading is also kind of broken (even though that's not really a big
> deal). git send-email with --no-chain-reply-to will just work too.
> 
> Thanks!
> Maxime
> 
> -- 
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com 
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v1] sunxi: improve throughput in the sunxi_mmc driver

[Dr. Philipp Tomsich]

> On 21 Feb 2017, at 19:00, Maxime Ripard  
> wrote:
> 
> On Fri, Feb 17, 2017 at 06:22:45PM +0100, Philipp Tomsich wrote:
>> Throughput tests have shown the sunxi_mmc driver to take over 10s to
>> read 10MB from a fast eMMC device due to excessive delays in polling
>> loops.
>> 
>> This commit restructures the main polling loops to use get_timer(...)
>> to determine whether a (millisecond) timeout has expired.  We choose
>> not to use the wait_bit function, as we don't need interruptability
>> with ctrl-c and have at least one case where two bits (one for an
>> error condition and another one for completion) need to be read and
>> using wait_bit would have not added to the clarity.
>> 
>> The observed speedup in testing on a A31 is greater than 10x (e.g. a
>> 10MB write decreases from 9.302s to 0.884s).
>> 
>> X-Affected-platforms: A31-uQ7, A80-Q7, A64-uQ7
>> Signed-off-by: Philipp Tomsich 
> 
> Awesome, I was about to start looking into the poor performances we
> had, but you beat me to it.
> 
> Do you know if the same changes can apply to the Linux MMC driver?

We’ve never seen performance problems on Linux and benchmarked at
up to ~50MByte/s on the A31-uQ7.

You might want to follow up off-list with Klaus on this, though...

Cheers,
Philipp.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v1] sun8i_emac: configure PHY reset GPIO via DM

[Dr. Philipp Tomsich]

> On 21 Feb 2017, at 18:57, Maxime Ripard  
> wrote:
> 
> On Fri, Feb 17, 2017 at 06:42:42PM +0100, Philipp Tomsich wrote:
>> This ports the support for configuring a GPIO for resetting the
>> Ethernet PHY (incl. such details as the reset polarity and
>> pulse-length) from the Designware driver.
> 
> Isn't that a property of the PHY itself, and not the MAC ?

It’s the MAC trying to signal “power-up the PHY”.
So we modelled it like a “wake-up output” from the MAC...

>> For easier migration for the Designware driver, the prefix of DTB
>> entries has simply been changed from 'snps,' to 'allwinner,':
>> i.e. 'snps,reset-gpio' became 'allwinner,reset-gpio'.
> 
> The upstrmea driver will use the snps prefix.

In fact I have a patch floating around here, which merges the sun8i_emac
back into designware.c (there really isn’t any difference) and then uses the
snps,reset-gpio.

Given that I found it a bit less appropriate to have that patch out first, this
is the consensus change to fall back onto in case the bigger change gets
shot down.

Cheers,
Philipp.

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v1 0/2] disk: efi: allow gap before partition entries

[Dr. Philipp Tomsich]

> On 21 Feb 2017, at 18:45, Maxime Ripard  
> wrote:
> 
> However, I'm a bit skeptical on the /config node. First, this node
> doesn't exist at all, and needs to be documented and acked by the DT
> maintainers. And why would one need to change that per device?

What’s the consensus on this: remove the /config-node dependency
(or split off into a separate patch), make the CONFIG-option a first
class citizen and add to Kconfig… and reroll as v2?

—Phil.



___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v1] sunxi: improve throughput in the sunxi_mmc driver

[Dr. Philipp Tomsich]

> On 21 Feb 2017, at 19:11, Dr. Philipp Tomsich 
>  wrote:
> 
> We’ve never seen performance problems on Linux and benchmarked at
> up to ~50MByte/s on the A31-uQ7.

I dug into our disti training slides from early 2015—looks like the throughput
number (sustained) for the eMMC was 12MB/s on write and 40MB/s on read
when using our standard part (at that time), which was a SDIN7DP2-4G.

Cheers,
Philipp.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v1 2/2] sun8i: enable support for the Micrel KSZ9031 with SUN8I_EMAC

[Dr. Philipp Tomsich]

Maxime,

> On 21 Feb 2017, at 20:56, Maxime Ripard  
> wrote:
> 
> On Fri, Feb 17, 2017 at 06:47:55PM +0100, Philipp Tomsich wrote:
>> #ifdef CONFIG_SUN8I_EMAC
>> #define CONFIG_PHY_GIGE  /* GMAC can use gigabit PHY 
>> */
>> +#define CONFIG_PHY_MICREL
>> +#define CONFIG_PHY_MICREL_KSZ9031   /* used on A64-uQ7  */
>> #endif
> 
> Same thing here, and that way you could enable it only on the board
> defconfig.

CONFIG_PHY_MICREL_KSZ9031 is not exported via Kconfig today.
I’m happy to see this moved to Kconfig & defconfig, but just checking
if that info changes anything…

Regards,
Phil.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v1 1/2] sun8i: define CONFIG_PHY_GIGE for EMAC

[Dr. Philipp Tomsich]

On 21 Feb 2017, at 20:55, Maxime Ripard  
wrote:
> 
> On Fri, Feb 17, 2017 at 06:47:54PM +0100, Philipp Tomsich wrote:
>> +#ifdef CONFIG_SUN8I_EMAC
>> +#define CONFIG_PHY_GIGE /* GMAC can use gigabit PHY 
>> */
>> +#endif
> 
> It would make more sense to move that option to Kconfig, and selecting
> it from SUN8I_EMAC.

CONFIG_PHY_GIGE is not a Kconfig option yet and only enabled via
board-specific config files.

Let me know if this changes anything, otherwise a change and reroll
will be coming…

Phil.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v1 02/11] sunxi: add pinctrl (UCLASS_PINCTRL) support for sunxi and tie back into GPIO

[Dr. Philipp Tomsich]

ChenYu,

> On 22 Feb 2017, at 07:07, Chen-Yu Tsai  wrote:
> 
>> (b) binding the existing gpio driver to the same node (but as the driverdata
>> needs to be matched as well, we’d need to select this based on the
>> compatible string and have a tighter coupling between the drivers than
>> strictly necessary)
> 
> They really should be the same driver. They control the same piece of 
> hardware,
> just exporting different functions to the 2 subsystems. There should be proper
> locking between the 2, as Allwinner's hardware cannot simultaneously mux a pin
> to some function and provide GPIO functionality. The user should not be able
> to request a GPIO on a pin that is already claimed by a UART or MMC card.

This is already being revised, although (with the user being able to change the
function assignment via mw.l — which we use a lot during bringup ourselves)
the data used by the driver might get modified hardware (as we fetch the latest
info directly from the hardware at all times).

Stay tuned for v2...

>> (c) dynamically creating gpio subnodes (just as done by the top-level
>> sunxi_gpio instances) for each of the banks from within the pinctrl driver
>> 
>> Another reason why we felt the gpiobank to be helpful is that it allows us
>> to
>> easily describe where the register sets (i.e. PIO and IRQ) are for each
>> bank.
> 
> These are always at fixed multiple offsets in Allwinner's design though.
> The driver can always calculate which registers it needs to access from
> the pin name/number. IRQ bank relations are also encoded in the pinctrl
> tables you imported. Everything needed is available in code.

I have to disagree on the ext-interrupt aspect of the pinctrl.
E.g. on the A31, we had ext-interrupt for PA at 0x200 and PB at 0x200, while
it’s now

Doesn’t really matter at this point though, as a new version with the changes
requested by you and Maxime will be coming shortly.

>> This will then require sharing the (internal) driver data structure from the
>> gpio-driver with pinctrl and populating it from the pinctrl probe… i.e. we
>> had
>> this in an earlier version of the changes, but it looked messy.
> 
> The Linux kernel driver also has pinctrl and gpio in the same driver. However
> the difference is that the kernel probes device tree nodes as platform 
> devices,
> and the device driver then registers pinctrl and gpio functions.
> 
> The U-boot model is a simplified one assuming that one device only has one
> function, which is not always true. You're going to run into the same issue
> with the CCU, which provides clocks and reset controls.

I fully agree, but didn’t want to open a question regarding whether the device
tree parsing and driver instantiation in U-Boot should be extended in a similar
way (i.e. continue looking for drivers that match a node beyond the first match)
at this stage.

We might in fact want to propose both solutions now, as we may need to have a
discussion about this...

Regards,
Philipp.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v1 0/2] disk: efi: allow gap before partition entries

[Dr. Philipp Tomsich]

> On 22 Feb 2017, at 07:11, Rask Ingemann Lambertsen  wrote:
> 
> On Fri, Feb 17, 2017 at 06:31:29PM +0100, Philipp Tomsich wrote:
>> Motivated by the the SPL layout for SD/MMC devices on Allwinner SoCs
>> (the SPL code needs to reside an 8K offset into the device), we add
>> support for leaving a gap between the MBR (LBA#0), GPT header (LBA#1)
>> and GPT partition entries (linked from field in the GPT header).
>> 
>> Note that this affects the creation of partition from U-Boot only and
>> has no effect on reading existing partition tables.
>> 
>> If defined, EFI_PARTITION_ENTRIES_OFF specifies a byte-offset into
>> a device and the parititon entries will be located starting at the
>> next LBA folling this offset.
> 
> In principle, this isn't really EFI specific, because all partition table
> formats will need to avoid the SPL area, so the configuration string
> shouldn't include the word EFI. EFI just happens to be the only(?) partition
> table format which is bloated enough to not easily fit into the space before
> the SPL area.
> 
> Also, it seems to me that there should also be a way of specifying how much
> room is available before the SPL area. So you could have
> CONFIG_SPL_AREA_START and CONFIG_SPL_AREA_SIZE, which added together gives
> the same value as your EFI_PARTITION_ENTRIES_OFF.

I would rather keep it simple for now, as it’s not just the SPL that we worry
about. In our default configuration, we punch a hole (of blocks not covered
by the partition tables, as they reside between the GPT header and the
partition entries) of at least 1MB (in some applications more) into the device
and then keep all of the following there:
1/  SPL
2/  FIT boot payload
3/  U-Boot environment

Regards,
Philipp.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v1 2/2] sun8i: enable support for the Micrel KSZ9031 with SUN8I_EMAC

[Dr. Philipp Tomsich]

Maxime,

> On 21 Feb 2017, at 21:35, Dr. Philipp Tomsich 
>  wrote:
> 
> Maxime,
> 
>> On 21 Feb 2017, at 20:56, Maxime Ripard  
>> wrote:
>> 
>> On Fri, Feb 17, 2017 at 06:47:55PM +0100, Philipp Tomsich wrote:
>>> #ifdef CONFIG_SUN8I_EMAC
>>> #define CONFIG_PHY_GIGE /* GMAC can use gigabit PHY 
>>> */
>>> +#define CONFIG_PHY_MICREL
>>> +#define CONFIG_PHY_MICREL_KSZ9031   /* used on A64-uQ7  */
>>> #endif
>> 
>> Same thing here, and that way you could enable it only on the board
>> defconfig.

I’d really like to keep this for all SUN8I_EMAC configurations, as we
need the PHY support to initialise the pad timing parameters from the
DTS. If these are not initialised, then RGMII (i.e. Gigabit) will not work.

As we are moving towards a ‘universal’ U-Boot binary for sun50i and
try to put all board specific info into the DTS, I’d like to keep this in
sunxi-common.h to ensure that nobody rebuild without it and then
has issues on our boards.

Regards,
Philipp.

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v1 1/2] sun8i: define CONFIG_PHY_GIGE for EMAC

[Dr. Philipp Tomsich]

> On 22 Feb 2017, at 18:18, Maxime Ripard  
> wrote:
> 
> On Tue, Feb 21, 2017 at 09:37:14PM +0100, Dr. Philipp Tomsich wrote:
>> On 21 Feb 2017, at 20:55, Maxime Ripard  
>> wrote:
>>> 
>>> On Fri, Feb 17, 2017 at 06:47:54PM +0100, Philipp Tomsich wrote:
>>>> +#ifdef CONFIG_SUN8I_EMAC
>>>> +#define CONFIG_PHY_GIGE   /* GMAC can use gigabit PHY 
>>>> */
>>>> +#endif
>>> 
>>> It would make more sense to move that option to Kconfig, and selecting
>>> it from SUN8I_EMAC.
>> 
>> CONFIG_PHY_GIGE is not a Kconfig option yet and only enabled via
>> board-specific config files.
> 
> Which is exactly what I was suggesting to do, sorry if it wasn't clear
> :)
> 
> We're still in the phase of getting options moved to kconfig, so this
> seems like the perfect occasion to do so.

Looks like this will be one of the many v2 patches for today.
All I have to do now is to get my patch-format right ;-)

Cheers,
Philipp.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v1] sunxi: improve throughput in the sunxi_mmc driver

[Dr. Philipp Tomsich]

> On 22 Feb 2017, at 18:22, Maxime Ripard  
> wrote:
> 
> On Tue, Feb 21, 2017 at 07:34:40PM +0100, Dr. Philipp Tomsich wrote:
>> 
>>> On 21 Feb 2017, at 19:11, Dr. Philipp Tomsich 
>>>  wrote:
>>> 
>>> We’ve never seen performance problems on Linux and benchmarked at
>>> up to ~50MByte/s on the A31-uQ7.
>> 
>> I dug into our disti training slides from early 2015—looks like the 
>> throughput
>> number (sustained) for the eMMC was 12MB/s on write and 40MB/s on read
>> when using our standard part (at that time), which was a SDIN7DP2-4G.
> 
> I was getting around 60MB/s on an A64 with an HS200 eMMC, which is
> quite good but left me the feeling that it could be better. But it was
> just a feeling :)

What does your eMMC datasheet list as expected throughput at HS200?

We haven’t even bothered benchmarking the eMMC on our boards lately,
as it’s “fast enough” for all practical purposes.

Regards,
Philipp.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v1 2/2] sun8i: enable support for the Micrel KSZ9031 with SUN8I_EMAC

[Dr. Philipp Tomsich]

Maxime,

> On 22 Feb 2017, at 20:00, Maxime Ripard  
> wrote:
> 
> On Wed, Feb 22, 2017 at 03:45:41PM +0100, Dr. Philipp Tomsich wrote:
>> Maxime,
>> 
>>> On 21 Feb 2017, at 21:35, Dr. Philipp Tomsich 
>>>  wrote:
>>> 
>>> Maxime,
>>> 
>>>> On 21 Feb 2017, at 20:56, Maxime Ripard  
>>>> wrote:
>>>> 
>>>> On Fri, Feb 17, 2017 at 06:47:55PM +0100, Philipp Tomsich wrote:
>>>>> #ifdef CONFIG_SUN8I_EMAC
>>>>> #define CONFIG_PHY_GIGE   /* GMAC can use gigabit PHY 
>>>>> */
>>>>> +#define CONFIG_PHY_MICREL
>>>>> +#define CONFIG_PHY_MICREL_KSZ9031   /* used on A64-uQ7  
>>>>> */
>>>>> #endif
>>>> 
>>>> Same thing here, and that way you could enable it only on the board
>>>> defconfig.
>> 
>> I’d really like to keep this for all SUN8I_EMAC configurations, as we
>> need the PHY support to initialise the pad timing parameters from the
>> DTS. If these are not initialised, then RGMII (i.e. Gigabit) will not work.
> 
> I'm not really discussing whether it is needed or not.

The discussion is moot anyway, as I had this change already committed
for v2 this afternoon.

>> As we are moving towards a ‘universal’ U-Boot binary for sun50i and
>> try to put all board specific info into the DTS, I’d like to keep this in
>> sunxi-common.h to ensure that nobody rebuild without it and then
>> has issues on our boards.
> 
> I'm not buying the whole universal U-Boot binary thing, but that's
> really not related to the discussion. sunxi-common.h should be
> reduced, not expanded to cover new stuff, in favor of Kconfig.

I’ve just put v2 onto the list and only touch the SUN8I_EMAC in v2.
The same should be done for SUNXI_GMAC and we’ll cycle back to it,
once the DM stuff is done (so we can update our A31 support to it and
release any pending changes left over from the A31-uQ7).

Regards,
Philipp.


___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v2 6/6] sun50i: dts: update DTS to avoid warnings

[Dr. Philipp Tomsich]

> On 23 Feb 2017, at 00:20, Maxime Ripard  
> wrote:
> 
> On Wed, Feb 22, 2017 at 09:47:32PM +0100, Philipp Tomsich wrote:
>> +<<< HEAD
>>  allwinner,drive = ;
>>  allwinner,pull = ;
>> +===
>> +drive-strength = < 30 >;
>> +bias-pull-up;
>>  };
>> 
>> -i2c0_pins: i2c0_pins {
>> +mmc2_8bit_pins: mmc2-8bit {
>> +allwinner,pins = "PC5", "PC6", "PC8",
>> + "PC9", "PC10", "PC11",
>> + "PC12", "PC13", "PC14",
>> + "PC15", "PC16";
>> +allwinner,function = "mmc2";
>> +drive-strength = < 30 >;
>> +bias-pull-up;
>> +>>> 20221b3... [f] dts warnings
> 
> Hmmm, are you sure about those ? :)

I can’t believe this slipped through. I think I need more sleep.
At least the patch-format wasn’t such a mess this time around…

Cheers,
Philipp.


___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v3] armv8: spl: Call spl_relocate_stack_gd for ARMv8

[Dr. Philipp Tomsich]

Simon,

> On 23 Feb 2017, at 03:23, Simon Glass  wrote:
> 
> On 22 February 2017 at 11:01, Philipp Tomsich
>  > wrote:
>> As part of the startup process for boards using the SPL, we need to
>> call spl_relocate_stack_gd. This is needed to set up malloc with its
>> DRAM buffer.
>> 
>> Signed-off-by: Philipp Tomsich 
>> Reviewed-by: Andre Przywara 
>> Reviewed-by: Simon Glass 
>> ---
>> arch/arm/lib/crt0_64.S | 13 +++--
>> 1 file changed, 11 insertions(+), 2 deletions(-)
>> 
>> diff --git a/arch/arm/lib/crt0_64.S b/arch/arm/lib/crt0_64.S
>> index 19c6a98..a7cead5 100644
>> --- a/arch/arm/lib/crt0_64.S
>> +++ b/arch/arm/lib/crt0_64.S
>> @@ -109,8 +109,17 @@ relocation_return:
>>  */
>>bl  c_runtime_cpu_setup /* still call old routine */
>> #endif /* !CONFIG_SPL_BUILD */
>> -
>> -/* TODO: For SPL, call spl_relocate_stack_gd() to alloc stack relocation */
>> +#if defined(CONFIG_SPL_BUILD)
>> +   bl  spl_relocate_stack_gd   /* may return NULL */
>> +   /* Perform 'sp = (x0 != NULL) ? x0 : sp' while working
>> +* around the constraint that conditional moves can not
>> +* have 'sp' as an operand
>> +*/
> 
> nit: Comment style again

I thought is was the missing asterisks at the beginning of the line…
What am I missing? Is it the indentation of the comment block?

>> +   mov x1, sp
>> +   cmp x0, #0
>> +   cselx0, x0, x1, ne
>> +   mov sp, x0
>> +#endif
>> 
>> /*
>>  * Clear BSS section
>> --
>> 1.9.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v1] spi: sunxi_spi: Add DM SPI driver for A31/A80/A64

[Dr. Philipp Tomsich]

Jagan,

> On 25 Feb 2017, at 09:43, Jagan Teki  wrote:
> 
>> +/* The SPI flash opcode for a FAST READ DUAL OUTPUT operation. */
>> +#define CMD_READ_DUAL_OUTPUT_FAST 0x3b
> 
> Flash(slave) specific opcodes shouldn't use it on spi driver, try to
> implement the spi in generic way instead of making to handle only
> specific slave.

I do agree, but there’s unfortunately a reason why this needs to be
in a SPI driver in U-Boot at this time: the SPI-NOR flash layer does
not correctly understand how a “FAST READ DUAL OUTPUT” cmd
is to be sent:
— the dual-IO fast-read needs to transmit the (a) cmd and
(b) 4-byte address as single-IO, then switch to Hi-Z while
sending a dummy-byte and then receive in dual-IO
— the SPI-NOR flash layer only sets the SPI_RX_DUAL mode
flag for the all exchanges and issues two separate spi_xfer
calls for the write and read phases (with the same mode).

I had a prototype implementation that added an additional spi_xfer
interface that allowed to specify a complete transaction (i.e. single-io,
dummy and dual-io phases), but switching the SPI-NOR layer to this
caused significant disruption there (it started to look like a rewrite, as
the probed READ_CMD couldn’t be described as a single-byte any
longer, but needed info on the transmission modes for CMD, ADDR,
dummy and data-in)… so it looks like a longer-term project.

Please note that
drivers/spi/ich.c
drivers/spi/fsl_qspi.c
use similar mechanisms to detect flash transactions and handle them
in a special mode.

Note that with the current implementation, this only code-path only 
has an effect if a dual-IO flash is configured as a slave (we even
check that the slave device is a flash-device according tot he DM).

Regards,
Philipp.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v8 4/8] rockchip: configs: rk3328: enable dwc2 driver and config fastboot

[Dr. Philipp Tomsich]

> On 28 Jun 2017, at 13:22, Meng Dongyang  wrote:
> 
> Config dwc2 driver support host and gadget function. Add support
> of fastboot function.
> 
> Signed-off-by: Meng Dongyang 
> Acked-by: Philipp Tomsich 

Reviewed-by: Philipp Tomsich 

I’ll migrate the remaining uses of CONFIG_USB_DWC2, when I apply.

Regards,
Philipp.

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 0/5] fix the boot issue of Rockchip RK3036

[Dr. Philipp Tomsich]

For what it’s worth: GCC 7.1 (in an aarch64-unknown-elf configuration) is
now our default build-environment for our RK3399 and RK3368 boards and
works as expected.

> On 04 Jul 2017, at 15:32, Tom Rini  wrote:
> 
> On Mon, Jul 03, 2017 at 04:02:59PM +0800, Andy Yan wrote:
>> Hi Philipp:
>> 
>> 
>> On 2017年06月30日 16:14, Dr. Philipp Tomsich wrote:
>>> Andy,
>>> 
>>>> On 30 Jun 2017, at 09:47, Andy Yan  wrote:
>>>> 
>>>> 
>>>> As Kever mentioned in [0], the RK3036 based boards could't
>>>> bootup for a long time.
>>>> After a git bisect, I found the RK3036 SPL code size has
>>>> increased from patch [1] [2]. Before Tom's patch [1], the
>>>> SPL size is 3160 bytes, but it becomes 4080 bytes after [1]
>>>> applied. After a look at this patch, I realised I should
>>>> disable SPL_USE_ARCH_MEMCPY/MEMSET, and the code size indeed
>>>> come down after I disabled them. But I got a LD error after
>>>> apply patch[2]: "undefined reference to memset", RK3036 SPL
>>>> didn't use lib/string because of the sram space imitation.
>>>> The compile succeed after CONFIG_SPL_LIBGENERIC_SUPPORT enabled,
>>>> but the spl code size become 3248 bytes.
>>>> 
>>>> Additionally, Simon post patch [3] call printf to print a
>>>> message before back to bootrom from spl, which make the spl
>>>> code size increased to nearly 3.7 kb.
>>>> 
>>>> RK3036 SPL only has 4kb sram to use, the spl code will use
>>>> 3.4 ~ 3.5 kb, the last 0.5kb are used for SP and GD, so there
>>>> is no space for malloc.
>>> gcc-6-arm-linux-gnueabi
>>> What version of GCC are you using?
>>> If your problem can also be solved by moving to GCC 6.3 (or newer) and
>>> the code-size improvements there, I’d rather just require a more recent
>>> GCC version.
>> 
>>I default use arm-linux-gnueabe-gcc v5.4.
>>The current upstream kylin-rk3036_defconfig compiled by gcc-5.4
>> is 4384 bytes, the size comes down to 3936 bytes if I use
>> arm-linux-gnueabihf-gcc v6.3 from linaro. But this is still too
>> large for rk3036.
> 
> Please note that (and U-Boot should be complaining at you) that with
> v2018.01 we'll be moving to gcc-6.x or later for ARM.
> 
>>Disable SPL_USE_ARCH_MEMCPY/MEMSET will make the spl size comes
>> down to 3042 bytes by gcc v6.3. But I still need some hack: enable
>> CONFIG_SPL_LIBGENERIC to get support for memset, masks Simon's print
>> in bootrom.c, or the code size will become very large. Event though
>> this hack make things work, we still lost a few hundreds bytes by
>> function board_init_f_alloc_reserve, because platforms with very
>> limit sram like rk3036 will return to bootrom after the dram
>> initialized, they never use the malloc space. This few hundreds
>> bytes is a large space for 4kb sarm,  it's better to letf them for
>> code or SP.
> 
> Since we're really size constrained here maybe it makes sense to move
> that print to a debug() ?
> 
> -- 
> Tom

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 1/2] armv8: apply -mgeneral-regs-only

[Dr. Philipp Tomsich]

Tom,

Please note that some GCC versions had a code-generation bug
when -mgeneral-regs-only was used (we hit this for a customer in
a vendor GCC we maintain):
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64304

Until everyone upgrades to a recently recent GCC (or until UBoot
enforces this next January), some people may hit hard-to-reproduce
issues.

A better way to suppress SIMD generation would be to use:
-march=armv8-a+nosimd

Regards,
Philipp.

> On 04 Jul 2017, at 15:32, Tom Rini  wrote:
> 
> On Mon, Jul 03, 2017 at 09:14:07PM +0800, Peng Fan wrote:
> 
>> Pass -mgeneral-regs-only to GCC, if not compiler may generate
>> instructions that use SIMD registers.
>> 
>> Signed-off-by: Peng Fan 
>> Cc: Albert Aribaud 
>> Cc: Tom Rini 
> 
> Reviewed-by: Tom Rini 
> 
> -- 
> Tom
> ___
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 1/2] armv8: apply -mgeneral-regs-only

[Dr. Philipp Tomsich]

> On 05 Jul 2017, at 03:23, Peng Fan  wrote:
> 
> 
> 
>> -Original Message-
>> From: Tom Rini [mailto:tr...@konsulko.com]
>> Sent: Wednesday, July 05, 2017 3:38 AM
>> To: Dr. Philipp Tomsich 
>> Cc: Peng Fan ; u-boot@lists.denx.de
>> Subject: Re: [U-Boot] [PATCH 1/2] armv8: apply -mgeneral-regs-only
>> 
>> On Tue, Jul 04, 2017 at 06:38:10PM +0200, Dr. Philipp Tomsich wrote:
>>> Tom,
>>> 
>>> Please note that some GCC versions had a code-generation bug when
>>> -mgeneral-regs-only was used (we hit this for a customer in a vendor
>>> GCC we maintain):
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64304
>>> 
>>> Until everyone upgrades to a recently recent GCC (or until UBoot
>>> enforces this next January), some people may hit hard-to-reproduce
>>> issues.
>>> 
>>> A better way to suppress SIMD generation would be to use:
>>> -march=armv8-a+nosimd
>> 
>> Ah, good to know.  Peng, please re-spin.
> 
> According to https://gcc.gnu.org/onlinedocs/gcc/ARM-Options.html,
> There is no +nosimd for armv8-a. I also tried +nosimd,

This is odd. When you look at
gcc/config/aarch64/aarch64-option-extensions.def
the “simd” option was there since the beginning of time for AArch64 (e.g. on the
gcc_4.8-branch — see 
https://github.com/gcc-mirror/gcc/blob/gcc-4_8-branch/gcc/config/aarch64/aarch64-option-extensions.def)
and is still there today (see 
https://github.com/gcc-mirror/gcc/blob/master/gcc/config/aarch64/aarch64-option-extensions.def)

I just tested the “armv8-a+nosimd” specification on (configured as 
aarch64-*-linux-gnueabi)
4.9.1, 4.9.3, 5.4.0, 6.3.0 and (configured as aarch64-*-elf) 6.3.0 and 7.1.0…
…unfortunately this was all that I could find installed on our target systems.

> There is still "str q0, [x29,#112] ".  If I use armv8-a+nofp, there is no
> such instructions. Then, do you agree using " armv8-a+nofp “?

I don’t mind if we go with +nofp, but I would like to understand how it comes
that your GCC differs from the FSF-released GCC source tree… this worries
me a bit, as users might encounter similar differences in the field.

> Thanks,
> Peng.
> 
>> 
>>> 
>>> Regards,
>>> Philipp.
>>> 
>>>> On 04 Jul 2017, at 15:32, Tom Rini  wrote:
>>>> 
>>>> On Mon, Jul 03, 2017 at 09:14:07PM +0800, Peng Fan wrote:
>>>> 
>>>>> Pass -mgeneral-regs-only to GCC, if not compiler may generate
>>>>> instructions that use SIMD registers.
>>>>> 
>>>>> Signed-off-by: Peng Fan 
>>>>> Cc: Albert Aribaud 
>>>>> Cc: Tom Rini 
>>>> 
>>>> Reviewed-by: Tom Rini 
>>>> 
>>>> --
>>>> Tom
>>>> ___
>>>> U-Boot mailing list
>>>> U-Boot@lists.denx.de
>>>> https://lists.denx.de/listinfo/u-boot
>>> 
>> 
>> --
>> Tom

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 1/2] armv8: apply -mgeneral-regs-only

[Dr. Philipp Tomsich]

> On 05 Jul 2017, at 10:59, Peng Fan  wrote:
> 
>> I just tested the “armv8-a+nosimd” specification on (configured as aarch64-*-
>> linux-gnueabi) 4.9.1, 4.9.3, 5.4.0, 6.3.0 and (configured as aarch64-*-elf) 
>> 6.3.0
>> and 7.1.0… …unfortunately this was all that I could find installed on our 
>> target
>> systems.
>> 
> 
> Then did you see simd registers are used?

I used some of our compiler tests for vectorisable code and verified the 
assembly
output: the compiler didn’t try to vectorise and also emitted an assembler 
directive
that would trigger an assembler error on AdvSIMD instructions.

To check with the U-Boot source, I’ll have to dig a bit deeper and get the full 
set
of toolchain versions set up in my U-Boot development environment.  However,
I plan to do so in the near future to make sure that we don’t encounter any
surprises when moving to GCC-6.3 for next year’s releases.

>>> There is still "str q0, [x29,#112] ".  If I use armv8-a+nofp, there is 
>>> no
>>> such instructions. Then, do you agree using " armv8-a+nofp “?
>> 
>> I don’t mind if we go with +nofp, but I would like to understand how it comes
>> that your GCC differs from the FSF-released GCC source tree… this worries me
>> a bit, as users might encounter similar differences in the field.
> 
> I tried yocto 6.2.0 and android 4.9.x toolchain, both could generate 
> instructions that use
> simd registers, not simd instructions.

Thanks, I’ll take a closer look.

Phil.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] net: gmac_rockchip: Add support for pinctrl

[Dr. Philipp Tomsich]

> On 10 Jul 2017, at 08:43, Romain Perier  wrote:
> 
> Le 07/07/2017 à 05:58, Simon Glass a écrit :
>> +Philipp
>> 
>> Hi Romain,
>> 
>> On 3 July 2017 at 01:13, Romain Perier  wrote:
>>> Currently, selecting state simple is done by the device driver model,
>>> prior probing the driver. The problem is that it's done and called on
>>> the pinctrl node with "gmac" as the "periph" struct udevice *. So
>>> pinctrl-rk3288 is looking for an interrupt property that is not found,
>>> and then gmac_config is never called.
>>> 
>>> This commits toggles the pinctrl on the right node from the probe
>>> function of the driver.
>> Is it possible to fix this while still using driver-model automatic pinctrl?
> 
> This is what I have tried to do, without success. The purpose of this
> patch is also to discuss about the possible solutions we have.

From looking at pinctrl_rk3288.c, I wonder if there’s a problem with
pinctrl on the RK3288 outside of SPL: I see that the (full) set_state
is also implemented outside of SPL (#ifndef CONFIG_SPL_BUILD)
and not guarded by a CONFIG_IS_ENABLED(PINCTRL_FULL). So
something might go down the wrong path.

Could start narrowing this down by enabling DEBUG in pinctrl_rk3288
to see whether this is even the simple pinctrl that gets called?

> Regards,
> Romain
> 
>> 
>>> Signed-off-by: Romain Perier 
>>> ---
>>> drivers/net/gmac_rockchip.c | 4 
>>> 1 file changed, 4 insertions(+)
>>> 
>>> diff --git a/drivers/net/gmac_rockchip.c b/drivers/net/gmac_rockchip.c
>>> index 5e2ca76302..8581026b4a 100644
>>> --- a/drivers/net/gmac_rockchip.c
>>> +++ b/drivers/net/gmac_rockchip.c
>>> @@ -160,6 +160,10 @@ static int gmac_rockchip_probe(struct udevice *dev)
>>>struct clk clk;
>>>int ret;
>>> 
>>> +   ret = pinctrl_select_state(dev, "simple");
>>> +   if (ret)
>>> +   return ret;
>>> +
>>>ret = clk_get_by_index(dev, 0, &clk);
>>>if (ret)
>>>return ret;
>>> --
>>> 2.11.0
>>> 
>> Regards,
>> Simon
> 

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] net: gmac_rockchip: Add support for pinctrl

[Dr. Philipp Tomsich]

> On 07 Jul 2017, at 05:58, Simon Glass  wrote:
> 
> +Philipp
> 
> Hi Romain,
> 
> On 3 July 2017 at 01:13, Romain Perier  wrote:
>> Currently, selecting state simple is done by the device driver model,
>> prior probing the driver. The problem is that it's done and called on
>> the pinctrl node with "gmac" as the "periph" struct udevice *. So
>> pinctrl-rk3288 is looking for an interrupt property that is not found,
>> and then gmac_config is never called.
>> 
>> This commits toggles the pinctrl on the right node from the probe
>> function of the driver.
> 
> Is it possible to fix this while still using driver-model automatic pinctrl?

I agree that we need to get a handle on the underlying issue and
solve that instead of putting a band-aid on.

>> Signed-off-by: Romain Perier 
>> ---
>> drivers/net/gmac_rockchip.c | 4 
>> 1 file changed, 4 insertions(+)
>> 
>> diff --git a/drivers/net/gmac_rockchip.c b/drivers/net/gmac_rockchip.c
>> index 5e2ca76302..8581026b4a 100644
>> --- a/drivers/net/gmac_rockchip.c
>> +++ b/drivers/net/gmac_rockchip.c
>> @@ -160,6 +160,10 @@ static int gmac_rockchip_probe(struct udevice *dev)
>>struct clk clk;
>>int ret;
>> 
>> +   ret = pinctrl_select_state(dev, "simple");
>> +   if (ret)
>> +   return ret;
>> +
>>ret = clk_get_by_index(dev, 0, &clk);
>>if (ret)
>>return ret;
>> --
>> 2.11.0
>> 
> 
> Regards,
> Simon

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] Data Abort with gcc 7.1

[Dr. Philipp Tomsich]

Maxime,

> On 11 Jul 2017, at 18:59, Tom Rini  wrote:
> 
> On Tue, Jul 11, 2017 at 06:54:55PM +0200, Maxime Ripard wrote:
>> Hi,
>> 
>> I recently got a gcc 7.1 based toolchain, and it seems like it
>> generates unaligned code, specifically in the net_set_ip_header
>> function in my case.
>> 
>> Whenever some packet is sent, this data abort is triggered:
>> 
>> => setenv ipaddr 10.42.0.1; ping 10.42.0.254
>> using musb-hdrc, OUT ep1out IN ep1in STATUS ep2in
>> MAC de:ad:be:ef:00:01
>> HOST MAC de:ad:be:af:00:00
>> RNDIS ready
>> musb-hdrc: peripheral reset irq lost!
>> high speed config #2: 2 mA, Ethernet Gadget, using RNDIS
>> USB RNDIS network up!
>> Using usb_ether device
>> data abort
>> pc : [<7ff9db10>]   lr : [<7ff9f00c>]
>> reloc pc : [<4a043b10>] lr : [<4a04500c>]
>> sp : 7bf37cc8  ip :   fp : 7ff6236c
>> r10: 7ffed2b8  r9 : 7bf39ee8  r8 : 7ffed2b8
>> r7 : 0001  r6 :   r5 : 002a  r4 : 7ffed30e
>> r3 : 1445  r2 : 01002a0a  r1 : fe002a0a  r0 : 7ffed30e
>> Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
>> Resetting CPU ...
>> 
>> 
>> Running objdump on it gives us this:
>> 
>> 4a043b04 :
>> 
>>  /*
>>   *  Construct an IP header.
>>   */
>>  /* IP_HDR_SIZE / 4 (not including UDP) */
>>  ip->ip_hl_v  = 0x45;
>> 4a043b04:e59f3074ldr r3, [pc, #116]  ; 4a043b80 
>> 
>> {
>> 4a043b08:e92d4013push{r0, r1, r4, lr}
>> 4a043b0c:e1a04000mov r4, r0
>>  ip->ip_hl_v  = 0x45;
>> 4a043b10:e5803000str r3, [r0] < Abort
>>  ip->ip_tos   = 0;
>>  ip->ip_len   = htons(IP_HDR_SIZE);
>>  ip->ip_id= htons(net_ip_id++);
>> 4a043b14:e59f3068ldr r3, [pc, #104]  ; 4a043b84 
>> 
>> 
>> It seems like r0 is indeed set to an unaligned address (0x7ffed30e)
>> for some reason.
>> 
>> Using a Linaro 6.3 toolchain works on the same commit with the same
>> config, so it really seems to be a compiler-related issue.
>> 
>> It generates this code:
>> 
>> 4a043ec4 :
>> 
>>  /*
>>   *  Construct an IP header.
>>   */
>>  /* IP_HDR_SIZE / 4 (not including UDP) */
>>  ip->ip_hl_v  = 0x45;
>> 4a043ec4:e3a03045mov r3, #69 ; 0x45
>> {
>> 4a043ec8:e92d4013push{r0, r1, r4, lr}
>> 4a043ecc:e1a04000mov r4, r0
>>  ip->ip_hl_v  = 0x45;
>> 4a043ed0:e5c03000strbr3, [r0]
>>  ip->ip_tos   = 0;
>>  ip->ip_len   = htons(IP_HDR_SIZE);
>> 4a043ed4:e3a03b05mov r3, #5120   ; 0x1400
>>  ip->ip_tos   = 0;
>> 4a043ed8:e3a0mov r0, #0
>>  ip->ip_len   = htons(IP_HDR_SIZE);
>> 4a043edc:e1c430b2strhr3, [r4, #2]
>>  ip->ip_id= htons(net_ip_id++);
>> 4a043ee0:e59f3064ldr r3, [pc, #100]  ; 4a043f4c 
>> 
>> 
>> And it seems like it's using an strb instruction to avoid the
>> unaligned access.
>> 
>> As far as I know, we are passing --wno-unaligned-access, so the broken
>> situation should not arise, and yet it does, so I'm a bit confused,
>> and not really sure what to do from there.
> 
> Can you reduce the code into a testcase?  I think the first step is
> filing a bug with gcc and seeing where it goes from there as yes, we
> should be passing -mno-unaligned-access.

I don’t think that this is a GCC bug, as “-mno-unaligned-access” will
change the behaviour for packed data-structures only. Here’s from
the GCC docs:
> -munaligned-access
> -mno-unaligned-access
> Enables (or disables) reading and writing of 16- and 32- bit values from 
> addresses that are not 16- or 32- bit aligned. By default unaligned access is 
> disabled for all pre-ARMv6, all ARMv6-M and for ARMv8-M Baseline 
> architectures, and enabled for all other architectures. If unaligned access 
> is not enabled then words in packed data structures are accessed a byte at a 
> time.

The key word seems to be “in packed data structures”.
However, I don’t see an attribute “packed” for the 'struct ip_udp_hdr’
(in include/net.h).

Could you try to verify that the error reproduces with a packed variant
of the ‘struct ip_udp_hdr’?

Regards,
Phil.

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] Pull request: u-boot-rockchip/master

[Dr. Philipp Tomsich]

Tom,

With the new merge window open, I’d like to sync some of changes accumulated 
for Rockchip.
Please pull into u-boot/master. Thanks!

Thanks,
Phil.

The following changes since commit d85ca029f257b53a96da6c2fb421e78a003a9943:

  Prepare v2017.07 (2017-07-10 13:07:38 -0400)

are available in the git repository at:

  git://git.denx.de/u-boot-rockchip.git master

for you to fetch changes up to 2454b719fb874120e06e4aa64bfb9450d091e56c:

  rockchip: rk3288: Add pinctrl support for the gmac ethernet interface 
(2017-07-11 15:23:38 +0200)


Kever Yang (28):
  rockchip: rk3328: correct mem_region
  rockchip: add sdram_common for common functions
  rockchip: use common sdram function
  rockchip: rk3328: add sdram driver in U-Boot
  rockchip: rk3368: add sdram driver for U-Boot
  rockchip: dts: rk3328: add dmc node
  rockchip: dts: rk3368: add dmc node
  rockchip: correct the bank0 ram size
  rockchip: rv1108: disable CONFIG_RAM before we have driver
  rockchip: mkimage: add support for rk322x soc
  rockchip: rk322x: add clock driver
  rockchip: rk322x: add pinctrl driver
  rockchip: rk322x: add dts file
  rockchip: rk322x: add basic soc support
  rockchip: rk322x: add sysreset driver
  rockchip: add evb_rk3229 board
  rockchip: rk3036: dtsi use max-frequency for mmc node
  rockchip: rk3288: dtsi use max-frequency for mmc node
  rockchip: rk3328: dtsi use max-frequency for mmc node
  rockchip: dwmmc: use max-frequency when OF_PLATDATA enabled
  rockchip: firefly-rk3399: dts: enable sdmmc device
  rockchip: firefly-rk3399: enable dwmmc driver for the board
  Revert "mmc: dw_mmc: rockchip: select proper card clock"
  rockchip: rk3036 remove CONFIG_RAM from defconfig
  rockchip: rk3036: sync os_reg2 define with other soc
  rockchip: pinctrl: rk3328: use gpio instead of sdmmc-pwren
  rockchip: dts: rk3328-evb: add sdmmc-pwren regulator
  rockchip: evb-rk3328: enable boot on regulator

Meng Dongyang (8):
  usb: Kconfig: config USB_XHCI_ROCKCHIP depends on DM_REGULATOR and DM_USB
  usb: host: xhci-rockchip: use fixed regulator to control vbus
  rockchip: dts: rk3328: add fixed regulator node for xhci
  rockchip: dts: rk3328: add fixed regulator node for xhci
  usb: dwc2: use dev_read_bool() instead of fdt_getprop()
  rockchip: rk3328: board: add support of dwc2 gadget
  rockchip: dts: rk3328: support and enable dwc2
  rockchip: dts: rk3399: control vbus of typec by fixed regulator

Philipp Tomsich (8):
  rockchip: pinctrl: dm: convert fdt_get to dev_read
  rockchip: spi: dm: convert fdt_get to dev_read
  rockchip: xhci: dm: convert fdt_get to dev_read
  rockchip: mmc: dm: convert fdt_get to dev_read
  rockchip: net: dm: convert fdt_get to dev_read
  rockchip: ns16550: dm: convert fdt_get to dev_read
  rockchip: dm: convert fdt_get to dev_read
  usb: Kconfig: migrate USB_DWC2 to Kconfig

Sjoerd Simons (1):
  rockchip: rk3288: Add pinctrl support for the gmac ethernet interface

Wadim Egorov (4):
  power: regulator: rk8xx: Build get_ldo_reg only for SPL
  power: regulator: rk8xx: Allow input current/charger shutdown 
configuration
  rockchip: Add basic support for phyCORE-RK3288 SoM based carrier board
  doc: rockchip: Add phyCORE-RK3288 RDK to board list

eric@rock-chips.com (3):
  rockchip: pwm: fix the register layout for the PWM controller
  rockchip: video: mipi: Modify variable type for arm32 compatibility
  rockchip: video: mipi: Modify format type for debug message

 arch/arm/dts/Makefile |   1 +
 arch/arm/dts/rk3036.dtsi  |   2 +-
 arch/arm/dts/rk3229-evb.dts   |  77 
 arch/arm/dts/rk322x.dtsi  | 710 
+
 arch/arm/dts/rk3288-phycore-rdk.dts   | 294 

 arch/arm/dts/rk3288-phycore-som.dtsi  | 506 
+++
 arch/arm/dts/rk3288.dtsi  |   8 +-
 arch/arm/dts/rk3328-evb.dts   |  33 -
 arch/arm/dts/rk3328.dtsi  |  25 +++-
 arch/arm/dts/rk3368.dtsi  |   7 ++
 arch/arm/dts/rk3399-evb.dts   |  16 ++-
 arch/arm/dts/rk3399-firefly.dts   |   7 ++
 arch/arm/include/asm/arch-rockchip/clock.h|   1 +
 arch/arm/include/asm/arch-rockchip/cru_rk322x.h   | 215 

 arch/arm/include/asm/arch-rockchip/ddr_rk3288.h   |  48 
 arch/arm/include/asm/arch-rockchip/grf_rk322x.h   | 519 

Re: [U-Boot] [PATCH] net: Mark the ip_udp_hdr struct as packed

[Dr. Philipp Tomsich]

> On 12 Jul 2017, at 16:34, Maxime Ripard  
> wrote:
> 
> The -mno-unaligned-access flag used on ARM to prevent GCC from generating
> unaligned accesses (obviously) will only do so on packed structures.
> 
> It seems like gcc 7.1 is a bit stricter than previous gcc versions on this,
> and using it lead to data abort for unaligned accesses when generating
> network traffic.
> 
> Fix this by adding the packed attribute to the ip_udp_hdr structure in
> order to let GCC do its job.
> 
> Signed-off-by: Maxime Ripard 
> —

Reviewed-by: Philipp Tomsich 

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] Data Abort with gcc 7.1

[Dr. Philipp Tomsich]

> On 12 Jul 2017, at 16:34, Tom Rini  wrote:
> 
> On Wed, Jul 12, 2017 at 04:20:52PM +0200, Maxime Ripard wrote:
>> On Tue, Jul 11, 2017 at 07:59:21PM +0200, Dr. Philipp Tomsich wrote:
>>> Maxime,
>>> 
>>>> On 11 Jul 2017, at 18:59, Tom Rini  wrote:
>>>> 
>>>> On Tue, Jul 11, 2017 at 06:54:55PM +0200, Maxime Ripard wrote:
>>>>> Hi,
>>>>> 
>>>>> I recently got a gcc 7.1 based toolchain, and it seems like it
>>>>> generates unaligned code, specifically in the net_set_ip_header
>>>>> function in my case.
>>>>> 
>>>>> Whenever some packet is sent, this data abort is triggered:
>>>>> 
>>>>> => setenv ipaddr 10.42.0.1; ping 10.42.0.254
>>>>> using musb-hdrc, OUT ep1out IN ep1in STATUS ep2in
>>>>> MAC de:ad:be:ef:00:01
>>>>> HOST MAC de:ad:be:af:00:00
>>>>> RNDIS ready
>>>>> musb-hdrc: peripheral reset irq lost!
>>>>> high speed config #2: 2 mA, Ethernet Gadget, using RNDIS
>>>>> USB RNDIS network up!
>>>>> Using usb_ether device
>>>>> data abort
>>>>> pc : [<7ff9db10>]lr : [<7ff9f00c>]
>>>>> reloc pc : [<4a043b10>]  lr : [<4a04500c>]
>>>>> sp : 7bf37cc8  ip :    fp : 7ff6236c
>>>>> r10: 7ffed2b8  r9 : 7bf39ee8   r8 : 7ffed2b8
>>>>> r7 : 0001  r6 :    r5 : 002a  r4 : 7ffed30e
>>>>> r3 : 1445  r2 : 01002a0a   r1 : fe002a0a  r0 : 7ffed30e
>>>>> Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
>>>>> Resetting CPU ...
>>>>> 
>>>>> 
>>>>> Running objdump on it gives us this:
>>>>> 
>>>>> 4a043b04 :
>>>>> 
>>>>>   /*
>>>>>*  Construct an IP header.
>>>>>*/
>>>>>   /* IP_HDR_SIZE / 4 (not including UDP) */
>>>>>   ip->ip_hl_v  = 0x45;
>>>>> 4a043b04: e59f3074ldr r3, [pc, #116]  ; 4a043b80 
>>>>> 
>>>>> {
>>>>> 4a043b08: e92d4013push{r0, r1, r4, lr}
>>>>> 4a043b0c: e1a04000mov r4, r0
>>>>>   ip->ip_hl_v  = 0x45;
>>>>> 4a043b10: e5803000str r3, [r0] < Abort
>>>>>   ip->ip_tos   = 0;
>>>>>   ip->ip_len   = htons(IP_HDR_SIZE);
>>>>>   ip->ip_id= htons(net_ip_id++);
>>>>> 4a043b14: e59f3068ldr r3, [pc, #104]  ; 4a043b84 
>>>>> 
>>>>> 
>>>>> It seems like r0 is indeed set to an unaligned address (0x7ffed30e)
>>>>> for some reason.
>>>>> 
>>>>> Using a Linaro 6.3 toolchain works on the same commit with the same
>>>>> config, so it really seems to be a compiler-related issue.
>>>>> 
>>>>> It generates this code:
>>>>> 
>>>>> 4a043ec4 :
>>>>> 
>>>>>   /*
>>>>>*  Construct an IP header.
>>>>>*/
>>>>>   /* IP_HDR_SIZE / 4 (not including UDP) */
>>>>>   ip->ip_hl_v  = 0x45;
>>>>> 4a043ec4: e3a03045mov r3, #69 ; 0x45
>>>>> {
>>>>> 4a043ec8: e92d4013push{r0, r1, r4, lr}
>>>>> 4a043ecc: e1a04000mov r4, r0
>>>>>   ip->ip_hl_v  = 0x45;
>>>>> 4a043ed0: e5c03000strbr3, [r0]
>>>>>   ip->ip_tos   = 0;
>>>>>   ip->ip_len   = htons(IP_HDR_SIZE);
>>>>> 4a043ed4: e3a03b05mov r3, #5120   ; 0x1400
>>>>>   ip->ip_tos   = 0;
>>>>> 4a043ed8: e3a0mov r0, #0
>>>>>   ip->ip_len   = htons(IP_HDR_SIZE);
>>>>> 4a043edc: e1c430b2strhr3, [r4, #2]
>>>>>   ip->ip_id= htons(net_ip_id++);
>>>>> 4a043ee0: e59f3064ldr r3, [pc, #100]  ; 4a043f4c 
>>>>> 
>>>>> 
>>>>> And it seems like it's using an strb instruction to avoid the
>>>>> unaligned access.
>>>>> 
>>>>> As far as I know, we are passing --wno-unaligned-access, so the broken
>>>>> situation should not arise, and yet it does, so I'm a bit confused,
>>>>> and not really sure what to do from there.
>>>> 
>>

Re: [U-Boot] Data Abort with gcc 7.1

[Dr. Philipp Tomsich]

Tom & Maxime,

> On 12 Jul 2017, at 16:34, Tom Rini  wrote:
> 
> On Wed, Jul 12, 2017 at 04:20:52PM +0200, Maxime Ripard wrote:
>> On Tue, Jul 11, 2017 at 07:59:21PM +0200, Dr. Philipp Tomsich wrote:
>>> Maxime,
>>> 
>>>> On 11 Jul 2017, at 18:59, Tom Rini  wrote:
>>>> 
>>>> On Tue, Jul 11, 2017 at 06:54:55PM +0200, Maxime Ripard wrote:
>>>>> Hi,
>>>>> 
>>>>> I recently got a gcc 7.1 based toolchain, and it seems like it
>>>>> generates unaligned code, specifically in the net_set_ip_header
>>>>> function in my case.
>>>>> 
>>>>> Whenever some packet is sent, this data abort is triggered:
>>>>> 
>>>>> => setenv ipaddr 10.42.0.1; ping 10.42.0.254
>>>>> using musb-hdrc, OUT ep1out IN ep1in STATUS ep2in
>>>>> MAC de:ad:be:ef:00:01
>>>>> HOST MAC de:ad:be:af:00:00
>>>>> RNDIS ready
>>>>> musb-hdrc: peripheral reset irq lost!
>>>>> high speed config #2: 2 mA, Ethernet Gadget, using RNDIS
>>>>> USB RNDIS network up!
>>>>> Using usb_ether device
>>>>> data abort
>>>>> pc : [<7ff9db10>]lr : [<7ff9f00c>]
>>>>> reloc pc : [<4a043b10>]  lr : [<4a04500c>]
>>>>> sp : 7bf37cc8  ip :    fp : 7ff6236c
>>>>> r10: 7ffed2b8  r9 : 7bf39ee8   r8 : 7ffed2b8
>>>>> r7 : 0001  r6 :    r5 : 002a  r4 : 7ffed30e
>>>>> r3 : 1445  r2 : 01002a0a   r1 : fe002a0a  r0 : 7ffed30e
>>>>> Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
>>>>> Resetting CPU ...
>>>>> 
>>>>> 
>>>>> Running objdump on it gives us this:
>>>>> 
>>>>> 4a043b04 :
>>>>> 
>>>>>   /*
>>>>>*  Construct an IP header.
>>>>>*/
>>>>>   /* IP_HDR_SIZE / 4 (not including UDP) */
>>>>>   ip->ip_hl_v  = 0x45;
>>>>> 4a043b04: e59f3074ldr r3, [pc, #116]  ; 4a043b80 
>>>>> 
>>>>> {
>>>>> 4a043b08: e92d4013push{r0, r1, r4, lr}
>>>>> 4a043b0c: e1a04000mov r4, r0
>>>>>   ip->ip_hl_v  = 0x45;
>>>>> 4a043b10: e5803000str r3, [r0] < Abort
>>>>>   ip->ip_tos   = 0;
>>>>>   ip->ip_len   = htons(IP_HDR_SIZE);
>>>>>   ip->ip_id= htons(net_ip_id++);
>>>>> 4a043b14: e59f3068ldr r3, [pc, #104]  ; 4a043b84 
>>>>> 
>>>>> 
>>>>> It seems like r0 is indeed set to an unaligned address (0x7ffed30e)
>>>>> for some reason.
>>>>> 
>>>>> Using a Linaro 6.3 toolchain works on the same commit with the same
>>>>> config, so it really seems to be a compiler-related issue.
>>>>> 
>>>>> It generates this code:
>>>>> 
>>>>> 4a043ec4 :
>>>>> 
>>>>>   /*
>>>>>*  Construct an IP header.
>>>>>*/
>>>>>   /* IP_HDR_SIZE / 4 (not including UDP) */
>>>>>   ip->ip_hl_v  = 0x45;
>>>>> 4a043ec4: e3a03045mov r3, #69 ; 0x45
>>>>> {
>>>>> 4a043ec8: e92d4013push{r0, r1, r4, lr}
>>>>> 4a043ecc: e1a04000mov r4, r0
>>>>>   ip->ip_hl_v  = 0x45;
>>>>> 4a043ed0: e5c03000strbr3, [r0]
>>>>>   ip->ip_tos   = 0;
>>>>>   ip->ip_len   = htons(IP_HDR_SIZE);
>>>>> 4a043ed4: e3a03b05mov r3, #5120   ; 0x1400
>>>>>   ip->ip_tos   = 0;
>>>>> 4a043ed8: e3a0mov r0, #0
>>>>>   ip->ip_len   = htons(IP_HDR_SIZE);
>>>>> 4a043edc: e1c430b2strhr3, [r4, #2]
>>>>>   ip->ip_id= htons(net_ip_id++);
>>>>> 4a043ee0: e59f3064ldr r3, [pc, #100]  ; 4a043f4c 
>>>>> 
>>>>> 
>>>>> And it seems like it's using an strb instruction to avoid the
>>>>> unaligned access.
>>>>> 
>>>>> As far as I know, we are passing --wno-unaligned-access, so the broken
>>>>> situation should not arise, and yet it does, so I'm a bit confused,
>>>>> and not really sure what to do from there.
>>>