Re: [U-Boot] [PATCH v2 2/3] efi: Add EFI_MEMORY_{NV, MORE_RELIABLE, RO} attributes

2018-07-15 Thread Heinrich Schuchardt
On 07/14/2018 10:53 PM, Eugeniu Rosca wrote:
> With this update, the memory attributes are in sync with Linux
> kernel v4.18-rc4. They also match page 190 of UEFI 2.7 spec [1].
> 
> [1] http://www.uefi.org/sites/default/files/resources/UEFI_Spec_2_7.pdf
> 
> Suggested-by: Heinrich Schuchardt 
> Signed-off-by: Eugeniu Rosca 
> ---
> 

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


Re: [U-Boot] [PATCH v2 1/3] efi: Fix truncation of constant value

2018-07-15 Thread Heinrich Schuchardt
On 07/14/2018 10:53 PM, Eugeniu Rosca wrote:
> Starting with commit 867a6ac86dd8 ("efi: Add start-up library code"),
> sparse constantly complains about truncated constant value in efi.h:
> 
> include/efi.h:176:35: warning: cast truncates bits from constant value 
> (8000 becomes 0)
> 
> This can get quite noisy, preventing real issues to be noticed:
> 
> $ make defconfig
> *** Default configuration is based on 'sandbox_defconfig'
> $ make C=2 -j12 2>&1 | grep truncates | wc -l
> 441
> 
> After the patch is applied:
> $ make C=2 -j12 2>&1 | grep truncates | wc -l
> 0
> $ sparse --version
> v0.5.2
> 
> Following the suggestion of Heinrich Schuchardt, instead of only
> fixing the root-cause, I replaced the whole enum of _SHIFT values
> by ULL defines. This matches both the UEFI 2.7 spec and the Linux
> kernel implementation.
> 
> Some ELF size comparison before and after the patch (gcc 7.3.0):
> 
> efi-x86_payload64_defconfig:
> textdatabss dec   hex   filename
> 407174  29432   278676  715282aea12 u-boot.old
> 407152  29464   278676  715292aea1c u-boot.new
> -22 +32 0   +10
> 
> efi-x86_payload32_defconfig:
> textdatabss dec   hex   filename
> 447075  30308   280076  757459b8ed3 u-boot.old
> 447053  30340   280076  757469b8edd u-boot.new
> -22 +32 0   +10
> 
> Fixes: 867a6ac86dd8 ("efi: Add start-up library code")
> Suggested-by: Heinrich Schuchardt 
> Signed-off-by: Eugeniu Rosca 
> ---
> 
> v2:
> - Replace _SHIFT enum values by ULL defines to match UEFI 2.7 spec
> - Add ELF size comparison to patch description
> 
>  cmd/efi.c   | 22 +++---
>  include/efi.h   | 24 ++--
>  lib/efi_loader/efi_memory.c |  7 +++
>  3 files changed, 24 insertions(+), 29 deletions(-)
> 
> diff --git a/cmd/efi.c b/cmd/efi.c
> index 6c1eb88424be..92a565f71373 100644
> --- a/cmd/efi.c
> +++ b/cmd/efi.c
> @@ -28,18 +28,18 @@ static const char *const type_name[] = {
>  };
>  
>  static struct attr_info {
> - int shift;
> + u64 val;
>   const char *name;
>  } mem_attr[] = {
> - { EFI_MEMORY_UC_SHIFT, "uncached" },
> - { EFI_MEMORY_WC_SHIFT, "write-coalescing" },
> - { EFI_MEMORY_WT_SHIFT, "write-through" },
> - { EFI_MEMORY_WB_SHIFT, "write-back" },
> - { EFI_MEMORY_UCE_SHIFT, "uncached & exported" },
> - { EFI_MEMORY_WP_SHIFT, "write-protect" },
> - { EFI_MEMORY_RP_SHIFT, "read-protect" },
> - { EFI_MEMORY_XP_SHIFT, "execute-protect" },
> - { EFI_MEMORY_RUNTIME_SHIFT, "needs runtime mapping" }
> + { EFI_MEMORY_UC, "uncached" },
> + { EFI_MEMORY_WC, "write-coalescing" },
> + { EFI_MEMORY_WT, "write-through" },
> + { EFI_MEMORY_WB, "write-back" },
> + { EFI_MEMORY_UCE, "uncached & exported" },
> + { EFI_MEMORY_WP, "write-protect" },
> + { EFI_MEMORY_RP, "read-protect" },
> + { EFI_MEMORY_XP, "execute-protect" },
> + { EFI_MEMORY_RUNTIME, "needs runtime mapping" }
>  };
>  
>  /* Maximum different attribute values we can track */
> @@ -173,7 +173,7 @@ static void efi_print_mem_table(struct efi_entry_memmap 
> *map,
>   printf("%c%llx: ", attr & EFI_MEMORY_RUNTIME ? 'r' : ' ',
>  attr & ~EFI_MEMORY_RUNTIME);
>   for (j = 0, first = true; j < ARRAY_SIZE(mem_attr); j++) {
> - if (attr & (1ULL << mem_attr[j].shift)) {
> + if (attr & mem_attr[j].val) {
>   if (first)
>   first = false;
>   else
> diff --git a/include/efi.h b/include/efi.h
> index 0fe15e65c06c..eb2a569fe010 100644
> --- a/include/efi.h
> +++ b/include/efi.h
> @@ -162,20 +162,16 @@ enum efi_mem_type {
>  };
>  
>  /* Attribute values */
> -enum {
> - EFI_MEMORY_UC_SHIFT = 0,/* uncached */
> - EFI_MEMORY_WC_SHIFT = 1,/* write-coalescing */
> - EFI_MEMORY_WT_SHIFT = 2,/* write-through */
> - EFI_MEMORY_WB_SHIFT = 3,/* write-back */
> - EFI_MEMORY_UCE_SHIFT= 4,/* uncached, exported */
> - EFI_MEMORY_WP_SHIFT = 12,   /* write-protect */
> - EFI_MEMORY_RP_SHIFT = 13,   /* read-protect */
> - EFI_MEMORY_XP_SHIFT = 14,   /* execute-protect */
> - EFI_MEMORY_RUNTIME_SHIFT = 63,  /* range requires runtime mapping */
> -
> - EFI_MEMORY_RUNTIME = 1ULL << EFI_MEMORY_RUNTIME_SHIFT,
> - EFI_MEM_DESC_VERSION= 1,
> -};
> +#define EFI_MEMORY_UC((u64)0x0001ULL)/* 
> uncached */

Unsigned long long int (ULL) is at least 64bit wide and unsigned (cf.
https://gcc.gnu.org/onlinedocs/gcc/Long-Long.html). Why do you introduce
the (u64) conversion?

Otherwise

Reviewed-by: Heinrich Schuchardt 

> +#define EFI_MEMORY_WC((u64)0x0002ULL)/* 
> write-coalescing */
> +#define EFI_MEMORY_WT((u64)0x0004ULL)/* 
> 

Re: [U-Boot] [PATCH] watchdog: dm: Support manual relocation for watchdogs

2018-07-15 Thread Michal Simek
On 15.7.2018 23:21, Simon Glass wrote:
> Hi Michal,
> 
> On 11 July 2018 at 23:47, Michal Simek  wrote:
>> On 11.7.2018 22:13, Simon Glass wrote:
>>> On 11 July 2018 at 08:41, Michal Simek  wrote:
 Relocate watchdog ops as was done by:
 "dm: Add support for all targets which requires MANUAL_RELOC"
 (sha1: 484fdf5ba058b07be5ca82763aa2b72063540ef3)

 Signed-off-by: Michal Simek 
 ---

 based on https://lists.denx.de/pipermail/u-boot/2018-July/334227.html

 ---
  drivers/watchdog/wdt-uclass.c | 23 +++
  1 file changed, 23 insertions(+)

>>>
>>> Reviewed-by: Simon Glass 
>>>
>>> When will the toolchain be fixed?
>>
>> It is really several years back when I have looked at it last time but I
>> think that toolchain is fixed for quite some time and only changes in
>> microblaze u-boot code are needed but really I would have to check and
>> start to play with it.
> 
> I think someone should sort this out. It would be good to remove this
> code. Is there a toolchain group at Xilinx?

Xilinx has a toolchain group. I just looked a I was playing with it in
January 2015 but didn't finish that. It is still on my long todo list.
Will see when I have a time to look at it.

Thanks,
Michal

___
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

2018-07-15 Thread Michal Simek
On 16.7.2018 07:20, Simon Glass wrote:
> Hi Michal,
> 
> On 13 July 2018 at 03:15, Michal Simek  wrote:
>> The Linux kernel has binding for gpio-restart node.
>> This patch is adding basic support without supporting any optional
>> properties.
>> 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 
>> ---
>>
>>  drivers/sysreset/Kconfig |  6 
>>  drivers/sysreset/Makefile|  1 +
>>  drivers/sysreset/sysreset_gpio.c | 59 
>> 
>>  3 files changed, 66 insertions(+)
>>  create mode 100644 drivers/sysreset/sysreset_gpio.c
> 
> Reviewed-by: Simon Glass 
> 
> But please see below.
> 
>>
>> diff --git a/drivers/sysreset/Kconfig b/drivers/sysreset/Kconfig
>> index a6d48e8a662c..1e228b97443a 100644
>> --- a/drivers/sysreset/Kconfig
>> +++ b/drivers/sysreset/Kconfig
>> @@ -15,6 +15,12 @@ config SYSRESET
>>
>>  if SYSRESET
>>
>> +config SYSRESET_GPIO
>> +   bool "Enable support for GPIO restart driver"
>> +   select GPIO
>> +   help
>> + Restart support via GPIO pin connected reset logic.
> 
> What does this mean? Please expand this to explain what it means.
> 
> Also, what is the difference between restart and reset? If there is no
> difference please use the word 'reset'. If there is a difference,
> please explain it here.

I have taken restart name because this is what it is written Linux kernel.

Based on this explanation:
https://kb.netgear.com/1001/Defining-Terms-Power-Cycle-Boot-Reboot-Restart-Reset-and-Hard-Reset

"Unlike a reset which changes something, a restart means to turn
something on, possibly without changing settings. When upgrading
firmware or software you are often asked to restart. A restart would be
probably be used if there were a major change to functionality, while a
reset often just changes settings of existing functionality."


>> +
>>  config SYSRESET_PSCI
>> bool "Enable support for PSCI System Reset"
>> depends on ARM_PSCI_FW
>> diff --git a/drivers/sysreset/Makefile b/drivers/sysreset/Makefile
>> index 0da58a1cf6ad..ca533cfefaad 100644
>> --- a/drivers/sysreset/Makefile
>> +++ b/drivers/sysreset/Makefile
>> @@ -3,6 +3,7 @@
>>  # (C) Copyright 2016 Cadence Design Systems Inc.
>>
>>  obj-$(CONFIG_SYSRESET) += sysreset-uclass.o
>> +obj-$(CONFIG_SYSRESET_GPIO) += sysreset_gpio.o
>>  obj-$(CONFIG_SYSRESET_PSCI) += sysreset_psci.o
>>  obj-$(CONFIG_SYSRESET_SYSCON) += sysreset_syscon.o
>>  obj-$(CONFIG_SYSRESET_WATCHDOG) += sysreset_watchdog.o
>> diff --git a/drivers/sysreset/sysreset_gpio.c 
>> b/drivers/sysreset/sysreset_gpio.c
>> new file mode 100644
>> index ..4c1c1510f285
>> --- /dev/null
>> +++ b/drivers/sysreset/sysreset_gpio.c
>> @@ -0,0 +1,59 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2018 Xilinx, Inc. - Michal Simek
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +struct gpio_reboot_priv {
>> +   struct gpio_desc gpio;
>> +};
>> +
>> +static int gpio_reboot_request(struct udevice *dev, enum sysreset_t type)
>> +{
>> +   struct gpio_reboot_priv *priv = dev_get_priv(dev);
>> +
>> +   /*
>> +* When debug log is enabled please make sure that chars won't end up
>> +* in output fifo. Or you can append udelay(); to get enough time
>> +* to HW to emit output fifo.
>> +*/
>> +   debug("GPIO restart\n");
>> +
>> +   /* 1 is just setting value - based on gpio->flags 0 or 1 is written 
>> */
> 
> Do you mean that it respects polarity (active high/low)? If so, it
> might be less confusing to state that.

ok - will update.

> 
>> +   return dm_gpio_set_value(>gpio, 1);
>> +}
>> +
>> +static struct sysreset_ops gpio_reboot_ops = {
>> +   .request = gpio_reboot_request,
>> +};
>> +
>> +int gpio_reboot_probe(struct udevice *dev)
>> +{
>> +   struct gpio_reboot_priv *priv = dev_get_priv(dev);
>> +
>> +   /*
>> +* Linux kernel DT binding contain others optional properties
>> +* which are not supported now
>> +*/
>> +
>> +   return gpio_request_by_name(dev, "gpios", 0, >gpio, 
>> GPIOD_IS_OUT);
>> +}
>> +
>> +static const struct udevice_id led_gpio_ids[] = {
>> +   { .compatible = "gpio-restart" },
>> +   { }
>> +};
>> +
>> +U_BOOT_DRIVER(gpio_reboot) = {
>> +   .id = UCLASS_SYSRESET,
>> +   .name = "gpio_restart",
>> +   .of_match = led_gpio_ids,
>> +   .ops = _reboot_ops,
>> +   .priv_auto_alloc_size = sizeof(struct gpio_reboot_priv),
>> +   .probe = gpio_reboot_probe,
>> +};
>> --
>> 1.9.1
>>
> 

Thanks,
Michal

Re: [U-Boot] [PATCH v2 1/1] Sandbox: provide default dtb

2018-07-15 Thread Heinrich Schuchardt
On 07/15/2018 11:22 PM, Simon Glass wrote:
> Hi Heinrich,
> 
> On 15 June 2018 at 14:06, Heinrich Schuchardt  wrote:
>> Provide sandbox.dtb as default device tree for sandbox_defconfig.
>>
>> We can use the dtb with
>>
>> ./u-boot -d u-boot.dtb
>>
>> Signed-off-by: Heinrich Schuchardt 
>> ---
>> v2
>> adjust commit message
>> ---
>>  configs/sandbox_defconfig | 1 +
>>  1 file changed, 1 insertion(+)
> 
> I afraid I still don't understand the purpose of this patch. Can you
> please expand the commit message? Also please reference -D since that
> is supposed to provide the default DT.
> 
> Regards,
> Simon
> 

With current master ./u-boot -D provides ethernet. So we don't this patch.

Regards

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


Re: [U-Boot] [RFC PATCH] gpio: zynq: Setup bank_name to dev->name

2018-07-15 Thread Michal Simek
On 16.7.2018 07:19, Simon Glass wrote:
> On 12 July 2018 at 08:04, Michal Simek  wrote:
>> There should be proper bank name setup to distiguish between different
>> gpio drivers. Use dev->name for it.
>>
>> Signed-off-by: Michal Simek 
>> ---
>>
>>  drivers/gpio/zynq_gpio.c | 2 ++
>>  1 file changed, 2 insertions(+)
> 
> Reviewed-by: Simon Glass 
> 
> Normally these would be named A, B, C, etc. Is there no such naming
> convention on zynq?

PS(Hard) part has only one gpio controller with several banks with are
using the same address space. We are using from the beginning flat
number scheme that's why only one name is used for all banks.

Thanks,
Michal

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


Re: [U-Boot] [PATCH] fdt: fix fdtdec_setup_memory_banksize()

2018-07-15 Thread Simon Glass
On 13 July 2018 at 04:12, Jens Wiklander  wrote:
> Prior to this patch is fdtdec_setup_memory_banksize() incorrectly
> ignoring the "status" field. This patch fixes that by testing the status
> with fdtdec_get_is_enabled() before using a memory node.
>
> Signed-off-by: Jens Wiklander 
> ---
>  lib/fdtdec.c | 20 +++-
>  1 file changed, 15 insertions(+), 5 deletions(-)

Reviewed-by: Simon Glass 

But can you convert this to livetree at the same time? E.g. use ofnode
functions.

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


Re: [U-Boot] [PATCH 2/4] Revert "dm: led: auto probe() LEDs with "default-state""

2018-07-15 Thread Simon Glass
On 13 July 2018 at 09:21, Patrick Delaunay  wrote:
> This reverts commit bc882f5d5c7b4d6ed5e927bf838863af43c786e7.
>
> Signed-off-by: Patrick Delaunay 
> ---
>
>  drivers/led/led_gpio.c | 9 -
>  1 file changed, 9 deletions(-)

Reviewed-by: Simon Glass 
___
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

2018-07-15 Thread Simon Glass
Hi Michal,

On 13 July 2018 at 03:15, Michal Simek  wrote:
> The Linux kernel has binding for gpio-restart node.
> This patch is adding basic support without supporting any optional
> properties.
> 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 
> ---
>
>  drivers/sysreset/Kconfig |  6 
>  drivers/sysreset/Makefile|  1 +
>  drivers/sysreset/sysreset_gpio.c | 59 
> 
>  3 files changed, 66 insertions(+)
>  create mode 100644 drivers/sysreset/sysreset_gpio.c

Reviewed-by: Simon Glass 

But please see below.

>
> diff --git a/drivers/sysreset/Kconfig b/drivers/sysreset/Kconfig
> index a6d48e8a662c..1e228b97443a 100644
> --- a/drivers/sysreset/Kconfig
> +++ b/drivers/sysreset/Kconfig
> @@ -15,6 +15,12 @@ config SYSRESET
>
>  if SYSRESET
>
> +config SYSRESET_GPIO
> +   bool "Enable support for GPIO restart driver"
> +   select GPIO
> +   help
> + Restart support via GPIO pin connected reset logic.

What does this mean? Please expand this to explain what it means.

Also, what is the difference between restart and reset? If there is no
difference please use the word 'reset'. If there is a difference,
please explain it here.

> +
>  config SYSRESET_PSCI
> bool "Enable support for PSCI System Reset"
> depends on ARM_PSCI_FW
> diff --git a/drivers/sysreset/Makefile b/drivers/sysreset/Makefile
> index 0da58a1cf6ad..ca533cfefaad 100644
> --- a/drivers/sysreset/Makefile
> +++ b/drivers/sysreset/Makefile
> @@ -3,6 +3,7 @@
>  # (C) Copyright 2016 Cadence Design Systems Inc.
>
>  obj-$(CONFIG_SYSRESET) += sysreset-uclass.o
> +obj-$(CONFIG_SYSRESET_GPIO) += sysreset_gpio.o
>  obj-$(CONFIG_SYSRESET_PSCI) += sysreset_psci.o
>  obj-$(CONFIG_SYSRESET_SYSCON) += sysreset_syscon.o
>  obj-$(CONFIG_SYSRESET_WATCHDOG) += sysreset_watchdog.o
> diff --git a/drivers/sysreset/sysreset_gpio.c 
> b/drivers/sysreset/sysreset_gpio.c
> new file mode 100644
> index ..4c1c1510f285
> --- /dev/null
> +++ b/drivers/sysreset/sysreset_gpio.c
> @@ -0,0 +1,59 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 Xilinx, Inc. - Michal Simek
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +struct gpio_reboot_priv {
> +   struct gpio_desc gpio;
> +};
> +
> +static int gpio_reboot_request(struct udevice *dev, enum sysreset_t type)
> +{
> +   struct gpio_reboot_priv *priv = dev_get_priv(dev);
> +
> +   /*
> +* When debug log is enabled please make sure that chars won't end up
> +* in output fifo. Or you can append udelay(); to get enough time
> +* to HW to emit output fifo.
> +*/
> +   debug("GPIO restart\n");
> +
> +   /* 1 is just setting value - based on gpio->flags 0 or 1 is written */

Do you mean that it respects polarity (active high/low)? If so, it
might be less confusing to state that.

> +   return dm_gpio_set_value(>gpio, 1);
> +}
> +
> +static struct sysreset_ops gpio_reboot_ops = {
> +   .request = gpio_reboot_request,
> +};
> +
> +int gpio_reboot_probe(struct udevice *dev)
> +{
> +   struct gpio_reboot_priv *priv = dev_get_priv(dev);
> +
> +   /*
> +* Linux kernel DT binding contain others optional properties
> +* which are not supported now
> +*/
> +
> +   return gpio_request_by_name(dev, "gpios", 0, >gpio, 
> GPIOD_IS_OUT);
> +}
> +
> +static const struct udevice_id led_gpio_ids[] = {
> +   { .compatible = "gpio-restart" },
> +   { }
> +};
> +
> +U_BOOT_DRIVER(gpio_reboot) = {
> +   .id = UCLASS_SYSRESET,
> +   .name = "gpio_restart",
> +   .of_match = led_gpio_ids,
> +   .ops = _reboot_ops,
> +   .priv_auto_alloc_size = sizeof(struct gpio_reboot_priv),
> +   .probe = gpio_reboot_probe,
> +};
> --
> 1.9.1
>

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


Re: [U-Boot] [ v2 05/10] video: add support of panel RM68200

2018-07-15 Thread Simon Glass
On 13 July 2018 at 06:11, Yannick Fertré  wrote:
> Support for Raydium RM68200 720p dsi 2dl video mode panel.
> This rm68200 panel driver is based on the Linux Kernel driver from
> drivers/gpu/drm/panel/panel-raydium-rm68200.c.
>
> Signed-off-by: Yannick Fertré 
> ---
>  drivers/video/Kconfig   |   8 +
>  drivers/video/Makefile  |   1 +
>  drivers/video/raydium-rm68200.c | 338 
> 
>  3 files changed, 347 insertions(+)
>  create mode 100644 drivers/video/raydium-rm68200.c
>

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


Re: [U-Boot] [ v2 04/10] video: add support of panel OTM8009A

2018-07-15 Thread Simon Glass
On 13 July 2018 at 06:11, Yannick Fertré  wrote:
> Support for Orise Tech otm8009a 480p dsi 2dl video mode panel.
>
> Signed-off-by: Yannick Fertré 
> ---
>  drivers/video/Kconfig  |   8 +
>  drivers/video/Makefile |   1 +
>  drivers/video/orisetech_otm8009a.c | 366 
> +
>  3 files changed, 375 insertions(+)
>  create mode 100644 drivers/video/orisetech_otm8009a.c

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


Re: [U-Boot] [PATCH 3/4] dm: led: move default state support in led uclass

2018-07-15 Thread Simon Glass
On 13 July 2018 at 09:21, Patrick Delaunay  wrote:
> This patch save common LED property "default-state" value
> in post bind of LED uclass.
> The configuration for this default state is only performed when
> led_default_state() is called;
> It can be called in your board_init()
> or it could added in init_sequence_r[] in future.
>
> Signed-off-by: Patrick Delaunay 
> ---
>
>  drivers/led/led-uclass.c | 54 
> 
>  drivers/led/led_gpio.c   |  8 ---
>  include/led.h| 23 +
>  3 files changed, 77 insertions(+), 8 deletions(-)

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


Re: [U-Boot] [ v2 01/10] dm: panel: get timings from panel

2018-07-15 Thread Simon Glass
On 13 July 2018 at 06:11, Yannick Fertré  wrote:
> Get timings from panel instead of read device tree.
>
> Signed-off-by: Yannick Fertré 
> ---
>  drivers/video/panel-uclass.c | 11 +++
>  include/panel.h  | 18 ++
>  2 files changed, 29 insertions(+)

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


Re: [U-Boot] [PATCH 4/4] stm32mp1: use new function led default state

2018-07-15 Thread Simon Glass
On 13 July 2018 at 09:21, Patrick Delaunay  wrote:
> Initialize the led with the default state defined in device tree.
>
> Signed-off-by: Patrick Delaunay 
> ---
>
>  board/st/stm32mp1/stm32mp1.c | 4 
>  1 file changed, 4 insertions(+)

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


Re: [U-Boot] [ v2 06/10] video: add MIPI DSI host controller bridge

2018-07-15 Thread Simon Glass
Hi Yannick,

On 13 July 2018 at 06:11, Yannick Fertré  wrote:
> Add a Synopsys Designware MIPI DSI host bridge driver, based on the
> Rockchip version from rockchip/dw-mipi-dsi.c with phy & bridge APIs.
>
> Signed-off-by: Yannick Fertré 
> ---
>  drivers/video/Kconfig   |   9 +
>  drivers/video/Makefile  |   1 +
>  drivers/video/dw_mipi_dsi.c | 826 
> 
>  include/dw_mipi_dsi.h   |  32 ++
>  4 files changed, 868 insertions(+)
>  create mode 100644 drivers/video/dw_mipi_dsi.c
>  create mode 100644 include/dw_mipi_dsi.h
>
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index e1029e5..3ccc8df 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -674,6 +674,15 @@ config VIDEO_DW_HDMI
>   rather requires a SoC-specific glue driver to call it), it
>   can not be enabled from the configuration menu.
>
> +config VIDEO_DW_MIPI_DSI
> +   bool
> +   help
> + Enables the common driver code for the Synopsis Designware
> + MIPI DSI block found in SoCs from various vendors.
> + As this does not provide any functionality by itself (but
> + rather requires a SoC-specific glue driver to call it), it
> + can not be enabled from the configuration menu.
> +
>  config VIDEO_SIMPLE
> bool "Simple display driver for preconfigured display"
> help
> diff --git a/drivers/video/Makefile b/drivers/video/Makefile
> index 018343f..bb2fd3c 100644
> --- a/drivers/video/Makefile
> +++ b/drivers/video/Makefile
> @@ -52,6 +52,7 @@ obj-$(CONFIG_FORMIKE) += formike.o
>  obj-$(CONFIG_LG4573) += lg4573.o
>  obj-$(CONFIG_AM335X_LCD) += am335x-fb.o
>  obj-$(CONFIG_VIDEO_DW_HDMI) += dw_hdmi.o
> +obj-$(CONFIG_VIDEO_DW_MIPI_DSI) += dw_mipi_dsi.o
>  obj-${CONFIG_VIDEO_MIPI_DSI} += mipi_display.o
>  obj-$(CONFIG_VIDEO_SIMPLE) += simplefb.o
>  obj-${CONFIG_VIDEO_TEGRA124} += tegra124/
> diff --git a/drivers/video/dw_mipi_dsi.c b/drivers/video/dw_mipi_dsi.c
> new file mode 100644
> index 000..db278c5
> --- /dev/null
> +++ b/drivers/video/dw_mipi_dsi.c
> @@ -0,0 +1,826 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2016, Fuzhou Rockchip Electronics Co., Ltd
> + * Copyright (C) 2018, STMicroelectronics - All Rights Reserved
> + * Author(s): Philippe Cornu  for STMicroelectronics.
> + *Yannick Fertre  for STMicroelectronics.
> + *
> + * This generic Synopsys DesignWare MIPI DSI host driver is inspired from
> + * the Linux Kernel driver drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

Please check the ordering here.

> +
> +#define HWVER_131  0x31333100  /* IP version 1.31 */
> +
> +#define DSI_VERSION0x00
> +#define VERSIONGENMASK(31, 8)
> +
> +#define DSI_PWR_UP 0x04
> +#define RESET  0
> +#define POWERUPBIT(0)
> +
> +#define DSI_CLKMGR_CFG 0x08
> +#define TO_CLK_DIVISION(div)   (((div) & 0xff) << 8)
> +#define TX_ESC_CLK_DIVISION(div)   ((div) & 0xff)

Instead of this can you use something like:

#define TO_CLK_DIVISION_SHIFT  8
#define TO_CLK_DIVISION_MASK (0xff << TO_CLK_DIVISION_SHIFT)

then do the shift in the code which need it below.

xxx << TO_CLK_DIVISION_SHIFT


> diff --git a/include/dw_mipi_dsi.h b/include/dw_mipi_dsi.h
> new file mode 100644
> index 000..f8482f7
> --- /dev/null
> +++ b/include/dw_mipi_dsi.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (C) 2017-2018, STMicroelectronics - All Rights Reserved
> + *
> + * Authors: Yannick Fertre 
> + *  Philippe Cornu 
> + *
> + * This generic Synopsys DesignWare MIPI DSI host include is inspired from
> + * the Linux Kernel include file include/drm/bridge/dw_mipi_dsi.h.
> + */
> +
> +#ifndef __DW_MIPI_DSI__
> +#define __DW_MIPI_DSI__
> +
> +#include 
> +
> +struct dw_mipi_dsi_phy_ops {
> +   int (*init)(void *priv_data);
> +   int (*get_lane_mbps)(void *priv_data, struct display_timing *timings,
> +u32 lanes, u32 format, unsigned int *lane_mbps);
> +};
> +
> +struct dw_mipi_dsi_plat_data {
> +   unsigned int max_data_lanes;
> +   const struct dw_mipi_dsi_phy_ops *phy_ops;
> +   struct udevice *panel;
> +};
> +
> +int dw_mipi_dsi_init_bridge(struct mipi_dsi_device *device);
> +void dw_mipi_dsi_bridge_enable(struct mipi_dsi_device *device);

This should be in a uclass I think.

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


Re: [U-Boot] [ v2 03/10] video: add support of MIPI DSI interface

2018-07-15 Thread Simon Glass
On 13 July 2018 at 06:11, Yannick Fertré  wrote:
> Mipi_display.c contains a set of dsi helpers.
> This file is a copy of file drm_mipi_dsi.c (linux kernel).
>
> Signed-off-by: Yannick Fertré 
> ---
>  drivers/video/Kconfig|   9 +
>  drivers/video/Makefile   |   1 +
>  drivers/video/mipi_display.c | 817 
> +++
>  include/mipi_display.h   | 257 +-
>  4 files changed, 1083 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/video/mipi_display.c
>

> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 5ee9032..560da1a 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -73,6 +73,15 @@ config VIDEO_ANSI
>   Enable ANSI escape sequence decoding for a more fully functional
>   console.
>
> +config VIDEO_MIPI_DSI
> +   bool "Support MIPI DSI interface"
> +   depends on DM_VIDEO
> +   default y if DM_VIDEO

Why default y? Many boards won't use MIPI.

> +   help
> + Support MIPI DSI interface for driving a MIPI compatible device.
> + The MIPI Display Serial Interface (MIPI DSI) defines a high-speed
> + serial interface between a host processor and a display module.
> +
>  config CONSOLE_NORMAL

[..]

> diff --git a/include/mipi_display.h b/include/mipi_display.h
> index ddcc8ca..5c3dbbe 100644
> --- a/include/mipi_display.h
> +++ b/include/mipi_display.h
> @@ -4,12 +4,16 @@
>   *
>   * Copyright (C) 2010 Guennadi Liakhovetski 
>   * Copyright (C) 2006 Nokia Corporation
> - * Author: Imre Deak 
> + * Copyright (C) 2018 STMicroelectronics - All Rights Reserved
> + * Author(s): Imre Deak 
> + *Yannick Fertre 
> + *Philippe Cornu 
>   *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License version 2 as
>   * published by the Free Software Foundation.
>   */
> +
>  #ifndef MIPI_DISPLAY_H
>  #define MIPI_DISPLAY_H
>
> @@ -115,6 +119,14 @@ enum {
> MIPI_DCS_READ_MEMORY_CONTINUE   = 0x3E,
> MIPI_DCS_SET_TEAR_SCANLINE  = 0x44,
> MIPI_DCS_GET_SCANLINE   = 0x45,
> +   MIPI_DCS_SET_DISPLAY_BRIGHTNESS = 0x51, /* MIPI DCS 1.3 */
> +   MIPI_DCS_GET_DISPLAY_BRIGHTNESS = 0x52, /* MIPI DCS 1.3 */
> +   MIPI_DCS_WRITE_CONTROL_DISPLAY  = 0x53, /* MIPI DCS 1.3 */
> +   MIPI_DCS_GET_CONTROL_DISPLAY= 0x54, /* MIPI DCS 1.3 */
> +   MIPI_DCS_WRITE_POWER_SAVE   = 0x55, /* MIPI DCS 1.3 */
> +   MIPI_DCS_GET_POWER_SAVE = 0x56, /* MIPI DCS 1.3 */
> +   MIPI_DCS_SET_CABC_MIN_BRIGHTNESS = 0x5E,/* MIPI DCS 1.3 */
> +   MIPI_DCS_GET_CABC_MIN_BRIGHTNESS = 0x5F,/* MIPI DCS 1.3 */
> MIPI_DCS_READ_DDB_START = 0xA1,
> MIPI_DCS_READ_DDB_CONTINUE  = 0xA8,
>  };
> @@ -127,4 +139,247 @@ enum {
>  #define MIPI_DCS_PIXEL_FMT_8BIT2
>  #define MIPI_DCS_PIXEL_FMT_3BIT1
>
> +struct mipi_dsi_host;
> +struct mipi_dsi_device;
> +
> +/* request ACK from peripheral */
> +#define MIPI_DSI_MSG_REQ_ACK   BIT(0)
> +/* use Low Power Mode to transmit message */
> +#define MIPI_DSI_MSG_USE_LPM   BIT(1)
> +
> +/**
> + * struct mipi_dsi_msg - read/write DSI buffer
> + * @channel: virtual channel id
> + * @type: payload data type
> + * @flags: flags controlling this message transmission
> + * @tx_len: length of @tx_buf
> + * @tx_buf: data to be written
> + * @rx_len: length of @rx_buf
> + * @rx_buf: data to be read, or NULL
> + */
> +struct mipi_dsi_msg {
> +   u8 channel;
> +   u8 type;
> +   u16 flags;
> +
> +   size_t tx_len;
> +   const void *tx_buf;
> +
> +   size_t rx_len;
> +   void *rx_buf;
> +};
> +
> +bool mipi_dsi_packet_format_is_short(u8 type);
> +bool mipi_dsi_packet_format_is_long(u8 type);
> +
> +/**
> + * struct mipi_dsi_packet - represents a MIPI DSI packet in protocol format
> + * @size: size (in bytes) of the packet
> + * @header: the four bytes that make up the header (Data ID, Word Count or
> + * Packet Data, and ECC)
> + * @payload_length: number of bytes in the payload
> + * @payload: a pointer to a buffer containing the payload, if any
> + */
> +struct mipi_dsi_packet {
> +   size_t size;
> +   u8 header[4];
> +   size_t payload_length;
> +   const u8 *payload;
> +};
> +
> +int mipi_dsi_create_packet(struct mipi_dsi_packet *packet,
> +  const struct mipi_dsi_msg *msg);
> +
> +/**
> + * struct mipi_dsi_host_ops - DSI bus operations
> + * @attach: attach DSI device to DSI host
> + * @detach: detach DSI device from DSI host
> + * @transfer: transmit a DSI packet
> + *
> + * DSI packets transmitted by .transfer() are passed in as mipi_dsi_msg
> + * structures. This structure contains information about the type of packet
> + * being transmitted as well as the transmit and receive buffers. When an
> + * error is 

Re: [U-Boot] [PATCH 2/4 v3] armv8: dts: fsl-ls1012a: add sata node support

2018-07-15 Thread Simon Glass
On 13 July 2018 at 03:25,   wrote:
> From: Yuantian Tang 
>
> One ls1012a, there is one SATA 3.0 advanced host controller interface
> which is a high-performance SATA solution that delivers comprehensive
> and fully-compliant generation 3 (1.5 Gb/s - 6.0 Gb/s) serial ATA
> capabilities, in accordance with the serial ATA revision 3.0 of Serial
> ATA International Organization.
> Add sata node to support this feature.
>
> Signed-off-by: Tang Yuantian 
> ---
> v3:
> - no changes
> v2:
> - add qds and 2g5rdb board support
>
>  arch/arm/dts/fsl-ls1012a-2g5rdb.dts |4 
>  arch/arm/dts/fsl-ls1012a-qds.dtsi   |4 
>  arch/arm/dts/fsl-ls1012a-rdb.dtsi   |4 
>  arch/arm/dts/fsl-ls1012a.dtsi   |8 
>  4 files changed, 20 insertions(+), 0 deletions(-)

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


Re: [U-Boot] [PATCH 1/4] stm32mp1: add gpio led support

2018-07-15 Thread Simon Glass
On 13 July 2018 at 09:21, Patrick Delaunay  wrote:
> This patch add the 4 LED available on the ED1 board and activated
> gpio led driver.
>
> Signed-off-by: Patrick Delaunay 
> ---
>
>  arch/arm/dts/stm32mp157c-ed1-u-boot.dtsi | 24 
>  configs/stm32mp15_basic_defconfig|  2 ++
>  2 files changed, 26 insertions(+)

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


Re: [U-Boot] [PATCH 1/4 v3] armv8: fsl: remove sata support

2018-07-15 Thread Simon Glass
On 13 July 2018 at 03:25,   wrote:
> From: Yuantian Tang 
>
> Remove the old implementation in order to enable DM for sata
>
> Signed-off-by: Tang Yuantian 
> ---
> v3:
> - rebase to latest code
> v2:
> - no changes
>
>  arch/arm/cpu/armv8/fsl-layerscape/soc.c|   54 
> 
>  arch/arm/include/asm/arch-fsl-layerscape/soc.h |   32 --
>  2 files changed, 0 insertions(+), 86 deletions(-)

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


Re: [U-Boot] Microblaze gpio reset handling

2018-07-15 Thread Simon Glass
Hi Michal,

On 12 July 2018 at 08:28, Michal Simek  wrote:
> Hi Simon,
>
> I have looked at converting the rest of microblaze drivers to DM and I
> am curious why I can't use top level compatible string in DM based drivers.
> I am looking at ways how to move gpio and microblaze soft reset to
> sysreset framework and I see that core is not able to find out my high
> level xlnx,microblaze compatible string.
>
>
> / {
> #address-cells = <1>;
> #size-cells = <1>;
> compatible = "xlnx,microblaze";
> model = "Xilinx MicroBlaze";
> hard-reset-gpios = <_gpio 0 1>;
>
> As far I have seen only nodes are checked but unfortunately Microblaze
> is using hard-reset-gpios in root for years and will be good to keep it
> like that.
>
> I have already tested to bind that driver from board file like
> device_bind_driver(gd->dm_root, "microblaze_sysreset", "reset", NULL);
> which is working fine but just asking if there is another solution for this.

Not at present. But I think we should actually try to bind a driver to
the root node. Probably needs changes to root.c.

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


Re: [U-Boot] [RFC PATCH] gpio: zynq: Setup bank_name to dev->name

2018-07-15 Thread Simon Glass
On 12 July 2018 at 08:04, Michal Simek  wrote:
> There should be proper bank name setup to distiguish between different
> gpio drivers. Use dev->name for it.
>
> Signed-off-by: Michal Simek 
> ---
>
>  drivers/gpio/zynq_gpio.c | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Simon Glass 

Normally these would be named A, B, C, etc. Is there no such naming
convention on zynq?

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


Re: [U-Boot] [PATCH v3 3/9] rockchip: rk3288: implement reading chip version from bootrom code

2018-07-15 Thread Simon Glass
On 12 July 2018 at 05:05, Alberto Panizzo  wrote:
> This allows rockusb code to reply correctly to K_FW_GET_CHIP_VER
> command.
>
> On RK3288 chip version is at 0x4ff0 and on tested hardware it
> corresponds at the string "320A20140813V200"
>
> Signed-off-by: Alberto Panizzo 
> ---
>  arch/arm/mach-rockchip/rk3288/Makefile |  1 +
>  arch/arm/mach-rockchip/rk3288/rockusb_rk3288.c | 30 
> ++
>  2 files changed, 31 insertions(+)
>  create mode 100644 arch/arm/mach-rockchip/rk3288/rockusb_rk3288.c

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


Re: [U-Boot] [PATCH v2 1/1] Sandbox: provide default dtb

2018-07-15 Thread Simon Glass
Hi Heinrich,

On 15 June 2018 at 14:06, Heinrich Schuchardt  wrote:
> Provide sandbox.dtb as default device tree for sandbox_defconfig.
>
> We can use the dtb with
>
> ./u-boot -d u-boot.dtb
>
> Signed-off-by: Heinrich Schuchardt 
> ---
> v2
> adjust commit message
> ---
>  configs/sandbox_defconfig | 1 +
>  1 file changed, 1 insertion(+)

I afraid I still don't understand the purpose of this patch. Can you
please expand the commit message? Also please reference -D since that
is supposed to provide the default DT.

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


Re: [U-Boot] [PATCH v3 5/9] usb: rockchip: implement K_FW_LBA_ERASE_10 command

2018-07-15 Thread Simon Glass
On 12 July 2018 at 05:05, Alberto Panizzo  wrote:
> This command is part of the write partition sequence performed by
> rkdeveloptool: one partition is first completely erased and
> than wrote.
>
> Signed-off-by: Alberto Panizzo 
> ---
>  arch/arm/include/asm/arch-rockchip/f_rockusb.h |  1 +
>  doc/README.rockusb |  1 +
>  drivers/usb/gadget/f_rockusb.c | 48 
> ++
>  3 files changed, 50 insertions(+)
>

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


Re: [U-Boot] [PATCH v3 7/9] usb: rockchip: boost up write speed from 4MB/s to 15MB/s

2018-07-15 Thread Simon Glass
On 12 July 2018 at 05:05, Alberto Panizzo  wrote:
> Speedup transfers increasing the max chunk size.
> Buffers are allocated with memalign thus developer is noticed when heap is
> full and in current configuration a buffer allocation of 64K till now
> is safe.
>
> Signed-off-by: Alberto Panizzo 
> ---
>  arch/arm/include/asm/arch-rockchip/f_rockusb.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Really your commit subject should say that you are increasing the
buffer size IMO, since that is the change. Then your commit message
can discuss motivation and impact.

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


Re: [U-Boot] [PATCH] sysreset: dm: Support manual relocation for sysreset

2018-07-15 Thread Simon Glass
On 12 July 2018 at 05:35, Michal Simek  wrote:
> Relocate sysreset ops as was done by:
> "dm: Add support for all targets which requires MANUAL_RELOC"
> (sha1: 484fdf5ba058b07be5ca82763aa2b72063540ef3)
>
> Signed-off-by: Michal Simek 
> ---
>
>  drivers/sysreset/sysreset-uclass.c | 16 
>  1 file changed, 16 insertions(+)

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


Re: [U-Boot] [PATCH 2/2] test: Fix test_vboot to call openssl without redirection

2018-07-15 Thread Simon Glass
Hi Stephen,

On 8 June 2018 at 00:02, Simon Glass  wrote:
>
> Hi Stephen,
>
> On 16 May 2018 at 12:57, Simon Glass  wrote:
> > Hi Stephen,
> >
> > On 16 May 2018 at 09:55, Stephen Warren  wrote:
> >> On 05/16/2018 01:10 AM, Simon Glass wrote:
> >>>
> >>> The redirection seems to cause this test to fail now. It isn't needed, so
> >>> drop it.
> >>
> >>
> >> I guess I have no particular objection to this, but I will point out that
> >> the test is working just fine as-is right now, so it might be worth
> >> investigating more re: what the error is and why it's happening... It'd be
> >> good to describe the details of the failure in the commit description too.
> >
> > So the test works OK for you? For me it fails. I'll update the commit 
> > message.
> >
> > # Store the output so it can be accessed if we raise an exception.
> > self.output = output
> > self.exit_status = exit_status
> > if exception:
> >>   raise exception
> > E   Exception: Exit code: 1
> >
> > test/py/multiplexed_log.py:173: Exception
> > --- Captured stdout call
> > ---
> > +openssl genpkey -algorithm RSA -out
> > /usr/local/google/home/sjg/cosarm/src/third_party/u-boot/files/build-sandbox/dev.key
> > -pkeyopt rsa_keygen_bits:2048 -pkeyopt rsa_keygen_pubexp:65537
> > 2>/dev/null
> > genpkey: Use -help for summary.
> > Exit code: 1
> >
>
> From Stephen:
>
> > Yes. I just double-checked and it's definitely running in Jenkins for me; 
> > not being skipped or anything. It's running on Denx u-boot.git, 
> > u-boot-dm.git and a slew of others too. I assume it's running on Travis CI 
> > without issue too.
>
> > I'm running Ubuntu 16.04 right now, but only recently upgraded from 14.04 
> > where I believe the test was running without issue too. Is this issue 
> > OS-specific or Python-version-specific (I have 2.7.12)?
>
> I am really not sure what is going on...I can definitely repeat this
> but only on my workstation, not my laptop.
>
> I will see if I can dig into it further.

I found a patch from someone else from a while back, which you had
reviewed :-) So I applied that.

The redirection only works if a shell is being used, and it normally
isn't. We certainly are not requesting a shell when calling
subprocess. As to why on some machines we appear to get one anyway, I
cannot comment.

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


Re: [U-Boot] [PATCH] gpio: dm: Support manual relocation for gpio

2018-07-15 Thread Simon Glass
On 12 July 2018 at 05:36, Michal Simek  wrote:
> Relocate gpio ops as was done by:
> "dm: Add support for all targets which requires MANUAL_RELOC"
> (sha1: 484fdf5ba058b07be5ca82763aa2b72063540ef3)
>
> Signed-off-by: Michal Simek 
> ---
>
>  drivers/gpio/gpio-uclass.c | 35 +++
>  1 file changed, 35 insertions(+)
>

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


Re: [U-Boot] [PATCH v4 6/6] common: Generic loader for file system

2018-07-15 Thread Simon Glass
Hi Tien Fong,

On 12 July 2018 at 01:19, Chee, Tien Fong  wrote:
> On Wed, 2018-07-11 at 14:13 -0600, Simon Glass wrote:
>> Hi Tien,
>>
>> On 6 July 2018 at 02:28,   wrote:
>> >
>> > From: Tien Fong Chee 
>> >
>> > This is file system generic loader which can be used to load
>> > the file image from the storage into target such as memory.
>> > The consumer driver would then use this loader to program whatever,
>> > ie. the FPGA device.
>> >
>> > Signed-off-by: Tien Fong Chee 
>> > ---
>> >  drivers/misc/Kconfig |  10 ++
>> >  drivers/misc/Makefile|   1 +
>> >  drivers/misc/fs_loader.c | 295
>> > +++
>> >  include/dm/uclass-id.h   |   1 +
>> >  include/fs_loader.h  |  79 +
>> >  5 files changed, 386 insertions(+)
>> >  create mode 100644 drivers/misc/fs_loader.c
>> >  create mode 100644 include/fs_loader.h
>> >
[..]

>>
>> >
>> > +   (*firmwarep)->priv = calloc(1, sizeof(struct
>> > firmware_priv));
>> > +   if (!(*firmwarep)->priv) {
>> > +   free(*firmwarep);
>> > +   return -ENOMEM;
>> > +   }
>> > +   }
>> > +
>> > +   ((struct firmware_priv *)((*firmwarep)->priv))->name =
>> > name;
>> Again this needs a local variable.
> Why?

Because these casts are really ugly and repetitive, particularly when
used for assignments :-)

[..]

>> > + * release_firmware - Release the resource associated with a
>> > firmware image
>> > + * @firmware: Firmware resource to release
>> > + */
>> > +void release_firmware(struct firmware *firmware);
>> > +
>> > +/**
>> > + * request_firmware_into_buf - Load firmware into a previously
>> > allocated buffer.
>> > + * @plat: Platform data such as storage and partition firmware
>> > loading from.
>> > + * @name: Name of firmware file.
>> > + * @buf: Address of buffer to load firmware into.
>> > + * @size: Size of buffer.
>> > + * @offset: Offset of a file for start reading into buffer.
>> > + * @firmwarep: Pointer to firmware image.
>> > + *
>> > + * The firmware is loaded directly into the buffer pointed to by
>> > @buf and
>> > + * the @firmwarep data member is pointed at @buf.
>> > + *
>> > + * Return: Size of total read, negative value when error.
>> > + */
>> > +int request_firmware_into_buf(struct device_platdata *plat,
>> > + const char *name,
>> > + void *buf, size_t size, u32 offset,
>> > + struct firmware **firmwarep);
>> Without a test it's hard to see how this is used, but I'm not keen on
>> the struct firmware ** here.
>>
>> Can you just use struct firmware * instead?
>>
>> Or maybe just return the fields individually since you only have
>> start
>> address and size, I think.
> I can try to remove struct firmware, let driver model handles all
> memory allocation, and then struct device_platdata *plat will change
> to struct udevice *dev. So, it would become like this
> int request_firmware_into_buf(struct udevice *dev,
>   const char *name,
>   void *buf, size_t size, u32 offset);
> I will remove release_firmware() after letting driver model to handle
> all memory deallocation.
> So, the API interface would looks a bit different compare to Linux
> firmware API interface. Does this change OK for you?

I believe you would need:

> int request_firmware_into_buf(struct udevice *dev,
>   const char *name,
>   void **bufp, size_t *sizep, u32 offset);

since you need to return the buffer and size?

What exactly is 'dev'? Is it the device in the FS_LOADER uclass?

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


Re: [U-Boot] [PATCH] gpio: zynq: Read of mach data in platdata with dev_get_driver_data

2018-07-15 Thread Simon Glass
On 12 July 2018 at 05:49, Michal Simek  wrote:
> Remove bogus zynq_gpio_getplat_data() and read driver data directly.
>
> Signed-off-by: Michal Simek 
> ---
>
> Driver should use platdata structure but this change should be fine
> before this is fixed.
> Simon: Is ofdata_to_platdata correct function where this should be read?

Yes.

>
> ---
>  drivers/gpio/zynq_gpio.c | 29 ++---
>  1 file changed, 2 insertions(+), 27 deletions(-)

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


Re: [U-Boot] [PATCH v2] microblaze: Do not call timer init that early

2018-07-15 Thread Simon Glass
On 12 July 2018 at 00:44, Michal Simek  wrote:
> Timer needs to be converted to DM but as of now it can't be called so
> early because intc controller is not ready. Call it later in board_r.c.
> Before this patch timer_init is called twice which is wrong.
> The patch is blocking initialization before relocation.
>
> Signed-off-by: Michal Simek 
> ---
>
> Changes in v2:
> - Do not add new ifdef to board_f and use GD_FLG_RELOC instead -
>   reported-by sjg
> - Change commit message
>
>  arch/microblaze/cpu/timer.c | 4 
>  1 file changed, 4 insertions(+)

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


Re: [U-Boot] [PATCH v3 4/9] usb: rockchip: implement K_FW_LBA_READ_10 command

2018-07-15 Thread Simon Glass
On 12 July 2018 at 05:05, Alberto Panizzo  wrote:
> This patch implement reading blocks form selected device with
> LBA addressing.
>
> Corresponding command on workstation is:
> rkdeveloptool rl   
>
> While we support reading more than one blocks per K_FW_LBA_READ_10
> request, rkdeveloptool and original rockchip tool do perform
> chunk reads limiting the maximum size per chunk far lower
> than max int values.
>
> Signed-off-by: Alberto Panizzo 
> ---
>  arch/arm/include/asm/arch-rockchip/f_rockusb.h |   3 +
>  doc/README.rockusb |   1 +
>  drivers/usb/gadget/f_rockusb.c | 104 
> -
>  3 files changed, 107 insertions(+), 1 deletion(-)

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


Re: [U-Boot] [PATCH] watchdog: dm: Support manual relocation for watchdogs

2018-07-15 Thread Simon Glass
Hi Michal,

On 11 July 2018 at 23:47, Michal Simek  wrote:
> On 11.7.2018 22:13, Simon Glass wrote:
>> On 11 July 2018 at 08:41, Michal Simek  wrote:
>>> Relocate watchdog ops as was done by:
>>> "dm: Add support for all targets which requires MANUAL_RELOC"
>>> (sha1: 484fdf5ba058b07be5ca82763aa2b72063540ef3)
>>>
>>> Signed-off-by: Michal Simek 
>>> ---
>>>
>>> based on https://lists.denx.de/pipermail/u-boot/2018-July/334227.html
>>>
>>> ---
>>>  drivers/watchdog/wdt-uclass.c | 23 +++
>>>  1 file changed, 23 insertions(+)
>>>
>>
>> Reviewed-by: Simon Glass 
>>
>> When will the toolchain be fixed?
>
> It is really several years back when I have looked at it last time but I
> think that toolchain is fixed for quite some time and only changes in
> microblaze u-boot code are needed but really I would have to check and
> start to play with it.

I think someone should sort this out. It would be good to remove this
code. Is there a toolchain group at Xilinx?

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


Re: [U-Boot] [RESEND PATCH v2 1/2] Add BOOTCOUNT_BOOTLIMIT to set reboot limit

2018-07-15 Thread Alex Kiernan
On Sun, Jul 15, 2018 at 10:36 AM Prabhakar Kushwaha
 wrote:
>
>
> > -Original Message-
> > From: U-Boot [mailto:u-boot-boun...@lists.denx.de] On Behalf Of Alex Kiernan
> > Sent: Saturday, July 14, 2018 1:30 PM
> > To: u-boot@lists.denx.de
> > Cc: Thomas Petazzoni ; Martyn Welch
> > ; Ian Ray 
> > Subject: [U-Boot] [RESEND PATCH v2 1/2] Add BOOTCOUNT_BOOTLIMIT to set
> > reboot limit
> >
> > Add ability to set environment bootlimit from Kconfig
> >
> > Signed-off-by: Alex Kiernan 
> > ---
> >
> > Changes in v2: None
> >
> >  drivers/bootcount/Kconfig | 8 
> >  include/env_default.h | 3 +++
> >  2 files changed, 11 insertions(+)
> >
> > diff --git a/drivers/bootcount/Kconfig b/drivers/bootcount/Kconfig index
> > d335ed14b9..9a0bd516d9 100644
> > --- a/drivers/bootcount/Kconfig
> > +++ b/drivers/bootcount/Kconfig
> > @@ -72,6 +72,14 @@ config BOOTCOUNT_AT91
> >
> >  endchoice
> >
> > +config BOOTCOUNT_BOOTLIMIT
> > + int "Maximum number of reboot cycles allowed"
> > + default 0
> > + help
> > +   Set the Maximum number of reboot cycles allowed without the boot
> > +   counter being cleared.
> > +   If set to 0 do not set a boot limit in the environment.
> > +
>
> Just a curiosity, if maximum number of reboot cycles expires, what will be 
> the behavior of u-boot?
>

It depends on which bootcount implementation you're using, I expect
some are undefined. For the default based on a u32, it'll wrap to 0
(which IIRC is defined behaviour in C) and you'll go back to the
bootcmd flow rather than altbootcmd.

> --pk
>
>


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


[U-Boot] Raspberry Pi / fdt_addr bug with environment

2018-07-15 Thread Pascal Vizeli
Hi,

I found a funny bug. I don't think that is a raspberry pi only bug. It
is the handling how env restore work.

We use the origin device tree from first bootloader. We store also the
environment to SSD for a fallback scenario. Now it store also the
fdt_addr to environment. On next boot, it overwrite the origin fdt
address with last known address from environment.

It look like the rpi don't change the address a lot. I see that only
after I update the firmware/first stage bootloader.

I can fix that inside script with a workaround, but maybe we need
change the handling of restore a bit?

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


Re: [U-Boot] [PATCH] serial: ns16550: Add register shift variable

2018-07-15 Thread Felix Brack
Hi Alexey,

On 15.07.2018 10:43, Alexey Brodkin wrote:
> Hi Tom, Felix, all
> 
>> -Original Message-
>> From: Tom Rini [mailto:tr...@konsulko.com]
>> Sent: Saturday, July 14, 2018 6:50 PM
>> To: Wolfgang Denk 
>> Cc: Felix Brack ; u-boot@lists.denx.de; Stefan Roese 
>> ; Alexey Brodkin ;
>> Alexander Graf ; Michal Simek 
>> Subject: Re: [U-Boot] [PATCH] serial: ns16550: Add register shift variable
>>
>> On Sat, Jul 14, 2018 at 12:47:21PM +0200, Wolfgang Denk wrote:
>>> Dear Felix,
>>>
>>> In message <1531492980-16543-1-git-send-email...@ltec.ch> you wrote:

 The motivation for writing this patch originates in the
 effort of synchronizing U-Boot DT to Linux DT for am33xx SOCs.
 The current am33xx.dtsi file from U-Boot defines the 
 property for all UART nodes. The actual (4.18+) am33xx.dtsi
 file from Linux does not define  anymore. To prevent
 (probably difficult) changes in many .dts and .dtsi files once
 the synchronization is done, one can use this new variable. For
 the pdu001 board, for example, SYS_NS16550_REG_SHIFT is set
 to 2; no need to clutter U-Boot and board specific dts files
 with  properties.
>>>
>>> Does this mean that U-Boot will not be able to use the same DTB as
>>> Linux?
>>
>> To be clear, it's the other way around.  We can't use the Linux dtb/dts
>> files as they've dropped (and in other cases, aren't adding) these
>> properties as it's handled differently.
> 
> Any chance to get a reference to the commit in Linux kernel that introduces 
> that change?
> 
In fact I believe that the  property never existed in the
am33xx.dtsi file from Linux. U-Boot commit 85cf0e6299 shows that the
property has been added to U-Boot's am33xx.dtsi file. The commit log
clearly states why this happened:

"With the commit 'c7b9686d5d48 ("ns16550: unify serial_omap")' all
TI platforms are broken with DM/DT boot as ns16550 driver expects
reg-shift from DT which is not populated for TI platforms.
Earlier it worked as it was hard coded to 2 in serial-omap
driver. So adding the reg-shift to serial nodes for dra7, am4372
and am33xx dtsi files. Tested this patch on am437x-sk-evm,
am437x-gp-evm, am335x-boneblack, dra74x-evm and dra72x-evm."

> I still see a bunch of:
> ->8--
> reg-shift = <2>;
> reg-io-width = <4>;
> ->8--
> in .dts files for ARC boards even in linux-next git tree.
> 
> -Alexey
> 

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


Re: [U-Boot] [PATCH] serial: ns16550: Add register shift variable

2018-07-15 Thread Felix Brack
Hi Tom,

On 14.07.2018 17:49, Tom Rini wrote:
> On Sat, Jul 14, 2018 at 12:47:21PM +0200, Wolfgang Denk wrote:
>> Dear Felix,
>>
>> In message <1531492980-16543-1-git-send-email...@ltec.ch> you wrote:
>>>
>>> The motivation for writing this patch originates in the
>>> effort of synchronizing U-Boot DT to Linux DT for am33xx SOCs.
>>> The current am33xx.dtsi file from U-Boot defines the 
>>> property for all UART nodes. The actual (4.18+) am33xx.dtsi
>>> file from Linux does not define  anymore. To prevent
>>> (probably difficult) changes in many .dts and .dtsi files once
>>> the synchronization is done, one can use this new variable. For
>>> the pdu001 board, for example, SYS_NS16550_REG_SHIFT is set
>>> to 2; no need to clutter U-Boot and board specific dts files
>>> with  properties.
>>
>> Does this mean that U-Boot will not be able to use the same DTB as
>> Linux?
> 
> To be clear, it's the other way around.  We can't use the Linux dtb/dts
> files as they've dropped (and in other cases, aren't adding) these
> properties as it's handled differently.
> 
This is exactly the point. These files have diverged quite a lot between
U-Boot and Linux since the last synchronization. I am trying to find a
method to re-synchronize U-Boot generating minimal collateral damage -
this patch is supposed to be part of that effort.

In fact we could even set the default value of SYS_NS16550_REG_SHIFT to
2 for _all_ am33xx SOC boards. That would probably further reduce
possible problems.

>>
>>> +config SYS_NS16550_REG_SHIFT
>>> +   int "Number of bytes to shift register offsets"
>>> +   default 0
>>> +   depends on SYS_NS16550
>>> +   help
>>> + Use this to specify the amount of bytes between discrete
>>> + device registers. If, for example, the device registers are
>>> + located at 0x00, 0x04, 0x08, 0x0C and so forth than set
>>> + this to 2. The default value is 0.
>>
>> This description is inconsistent or misleading.  How do you define
>> "space between registers"?  The unused gaps?  In the example, the
>> registers are spaced at 4 bytes intervals, so a value of 2 would
>> only make sense of we have 16 bit registters and you count the gap
>> bytes.
>>
>> But this is a very strange and uncommon way to describe such a
>> situation, especially when you write that you "shift register
>> offsets".  Here I think about something like a LSL operation, so
>> shifing by 2 bits would result in a 2^2 = 4 byte spacing.
>>
>> This needs to be rewritten.
> 
> To try and help clarify, the property in question means "quantity to
> shift the register offsets by."  It should be clear in our Kconfig help
> entry as well that this is what we're looking for.
> 
Thanks for this! The help text will be fixed in v2.

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


Re: [U-Boot] [PATCH] serial: ns16550: Add register shift variable

2018-07-15 Thread Felix Brack
Hello Wolfgang,

On 14.07.2018 12:47, Wolfgang Denk wrote:
> Dear Felix,
> 
> In message <1531492980-16543-1-git-send-email...@ltec.ch> you wrote:
>>
>> The motivation for writing this patch originates in the
>> effort of synchronizing U-Boot DT to Linux DT for am33xx SOCs.
>> The current am33xx.dtsi file from U-Boot defines the 
>> property for all UART nodes. The actual (4.18+) am33xx.dtsi
>> file from Linux does not define  anymore. To prevent
>> (probably difficult) changes in many .dts and .dtsi files once
>> the synchronization is done, one can use this new variable. For
>> the pdu001 board, for example, SYS_NS16550_REG_SHIFT is set
>> to 2; no need to clutter U-Boot and board specific dts files
>> with  properties.
> 
> Does this mean that U-Boot will not be able to use the same DTB as
> Linux?
>
This has already been answered very accurately by Tom.
I have also written an RFC dealing with this: "[RFC] arm: dts: am33xx:
Sync DTS with Linux 4.16.11", posted May 24.

>> +config SYS_NS16550_REG_SHIFT
>> +int "Number of bytes to shift register offsets"
>> +default 0
>> +depends on SYS_NS16550
>> +help
>> +  Use this to specify the amount of bytes between discrete
>> +  device registers. If, for example, the device registers are
>> +  located at 0x00, 0x04, 0x08, 0x0C and so forth than set
>> +  this to 2. The default value is 0.
> 
> This description is inconsistent or misleading.  How do you define
> "space between registers"?  The unused gaps?  In the example, the
> registers are spaced at 4 bytes intervals, so a value of 2 would
> only make sense of we have 16 bit registters and you count the gap
> bytes.
> 
> But this is a very strange and uncommon way to describe such a
> situation, especially when you write that you "shift register
> offsets".  Here I think about something like a LSL operation, so
> shifing by 2 bits would result in a 2^2 = 4 byte spacing.
> 
> This needs to be rewritten.
> 
You are right, sorry. It seems I used my brain to mix up two different
explanations and that's the result -  will be fixed.
> 
> Best regards,
> 
> Wolfgang Denk
> 
regards, Felix
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [RESEND PATCH v2 1/2] Add BOOTCOUNT_BOOTLIMIT to set reboot limit

2018-07-15 Thread Prabhakar Kushwaha

> -Original Message-
> From: U-Boot [mailto:u-boot-boun...@lists.denx.de] On Behalf Of Alex Kiernan
> Sent: Saturday, July 14, 2018 1:30 PM
> To: u-boot@lists.denx.de
> Cc: Thomas Petazzoni ; Martyn Welch
> ; Ian Ray 
> Subject: [U-Boot] [RESEND PATCH v2 1/2] Add BOOTCOUNT_BOOTLIMIT to set
> reboot limit
> 
> Add ability to set environment bootlimit from Kconfig
> 
> Signed-off-by: Alex Kiernan 
> ---
> 
> Changes in v2: None
> 
>  drivers/bootcount/Kconfig | 8 
>  include/env_default.h | 3 +++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/drivers/bootcount/Kconfig b/drivers/bootcount/Kconfig index
> d335ed14b9..9a0bd516d9 100644
> --- a/drivers/bootcount/Kconfig
> +++ b/drivers/bootcount/Kconfig
> @@ -72,6 +72,14 @@ config BOOTCOUNT_AT91
> 
>  endchoice
> 
> +config BOOTCOUNT_BOOTLIMIT
> + int "Maximum number of reboot cycles allowed"
> + default 0
> + help
> +   Set the Maximum number of reboot cycles allowed without the boot
> +   counter being cleared.
> +   If set to 0 do not set a boot limit in the environment.
> +

Just a curiosity, if maximum number of reboot cycles expires, what will be the 
behavior of u-boot?

--pk


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


Re: [U-Boot] [PATCH] serial: ns16550: Add register shift variable

2018-07-15 Thread Alexey Brodkin
Hi Tom, Felix, all

> -Original Message-
> From: Tom Rini [mailto:tr...@konsulko.com]
> Sent: Saturday, July 14, 2018 6:50 PM
> To: Wolfgang Denk 
> Cc: Felix Brack ; u-boot@lists.denx.de; Stefan Roese 
> ; Alexey Brodkin ;
> Alexander Graf ; Michal Simek 
> Subject: Re: [U-Boot] [PATCH] serial: ns16550: Add register shift variable
> 
> On Sat, Jul 14, 2018 at 12:47:21PM +0200, Wolfgang Denk wrote:
> > Dear Felix,
> >
> > In message <1531492980-16543-1-git-send-email...@ltec.ch> you wrote:
> > >
> > > The motivation for writing this patch originates in the
> > > effort of synchronizing U-Boot DT to Linux DT for am33xx SOCs.
> > > The current am33xx.dtsi file from U-Boot defines the 
> > > property for all UART nodes. The actual (4.18+) am33xx.dtsi
> > > file from Linux does not define  anymore. To prevent
> > > (probably difficult) changes in many .dts and .dtsi files once
> > > the synchronization is done, one can use this new variable. For
> > > the pdu001 board, for example, SYS_NS16550_REG_SHIFT is set
> > > to 2; no need to clutter U-Boot and board specific dts files
> > > with  properties.
> >
> > Does this mean that U-Boot will not be able to use the same DTB as
> > Linux?
> 
> To be clear, it's the other way around.  We can't use the Linux dtb/dts
> files as they've dropped (and in other cases, aren't adding) these
> properties as it's handled differently.

Any chance to get a reference to the commit in Linux kernel that introduces 
that change?

I still see a bunch of:
->8--
reg-shift = <2>;
reg-io-width = <4>;
->8--
in .dts files for ARC boards even in linux-next git tree.

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