[PATCH v2 2/2] mpc8379erdb: Convert to DM_MMC

2020-04-03 Thread sinan
From: Sinan Akman 

Signed-off-by: Sinan Akman 
---
Changes for v2:
   - Split patches for device tree and DM_MMC

 board/freescale/mpc837xerdb/mpc837xerdb.c | 9 -
 configs/MPC837XERDB_defconfig | 5 -
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/board/freescale/mpc837xerdb/mpc837xerdb.c 
b/board/freescale/mpc837xerdb/mpc837xerdb.c
index 45038acf32..9321952674 100644
--- a/board/freescale/mpc837xerdb/mpc837xerdb.c
+++ b/board/freescale/mpc837xerdb/mpc837xerdb.c
@@ -136,8 +136,8 @@ int checkboard(void)

 int board_early_init_f(void)
 {
-#ifdef CONFIG_FSL_SERDES
immap_t *immr = (immap_t *)CONFIG_SYS_IMMR;
+#ifdef CONFIG_FSL_SERDES
u32 spridr = in_be32(>sysconf.spridr);

/* we check only part num, and don't look for CPU revisions */
@@ -164,10 +164,16 @@ int board_early_init_f(void)
break;
}
 #endif /* CONFIG_FSL_SERDES */
+
+#ifdef CONFIG_FSL_ESDHC
+   clrsetbits_be32(>sysconf.sicrl, SICRL_USB_B, SICRL_USB_B_SD);
+   clrsetbits_be32(>sysconf.sicrh, SICRH_SPI, SICRH_SPI_SD);
+#endif
return 0;
 }

 #ifdef CONFIG_FSL_ESDHC
+#if !(CONFIG_IS_ENABLED(DM_MMC))
 int board_mmc_init(bd_t *bd)
 {
struct immap __iomem *im = (struct immap __iomem *)CONFIG_SYS_IMMR;
@@ -186,6 +192,7 @@ int board_mmc_init(bd_t *bd)
return fsl_esdhc_mmc_init(bd);
 }
 #endif
+#endif

 /*
  * Miscellaneous late-boot configurations
diff --git a/configs/MPC837XERDB_defconfig b/configs/MPC837XERDB_defconfig
index fd8335ee4e..8ef193a6e5 100644
--- a/configs/MPC837XERDB_defconfig
+++ b/configs/MPC837XERDB_defconfig
@@ -160,7 +160,11 @@ CONFIG_CMD_PING=y
 CONFIG_CMD_DATE=y
 CONFIG_CMD_EXT2=y
 CONFIG_CMD_FAT=y
+CONFIG_OF_CONTROL=y
+CONFIG_DEFAULT_DEVICE_TREE="mpc8379erdb"
 CONFIG_ENV_ADDR=0xFE08
+CONFIG_DM=y
+CONFIG_DM_MMC=y
 CONFIG_FSL_SATA=y
 CONFIG_FSL_ESDHC=y
 CONFIG_MTD_NOR_FLASH=y
@@ -174,4 +178,3 @@ CONFIG_SYS_NS16550=y
 CONFIG_USB=y
 CONFIG_USB_EHCI_HCD=y
 CONFIG_USB_STORAGE=y
-CONFIG_OF_LIBFDT=y
--
2.14.5



[PATCH v2 1/2] mpc8379erdb: Add device tree

2020-04-03 Thread sinan
From: Sinan Akman 

Signed-off-by: Sinan Akman 
---
Changes for v2:
   - Split patches for device tree and DM_MMC

 arch/powerpc/dts/Makefile|  1 +
 arch/powerpc/dts/mpc8379erdb.dts | 74 
 2 files changed, 75 insertions(+)
 create mode 100644 arch/powerpc/dts/mpc8379erdb.dts

diff --git a/arch/powerpc/dts/Makefile b/arch/powerpc/dts/Makefile
index 3195351c9c..e3ec033096 100644
--- a/arch/powerpc/dts/Makefile
+++ b/arch/powerpc/dts/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0+

+dtb-$(CONFIG_TARGET_MPC837XERDB) += mpc8379erdb.dtb
 dtb-$(CONFIG_TARGET_MPC8548CDS) += mpc8548cds.dtb mpc8548cds_36b.dtb
 dtb-$(CONFIG_TARGET_P1020RDB_PC) += p1020rdb-pc.dtb p1020rdb-pc_36b.dtb
 dtb-$(CONFIG_TARGET_P1020RDB_PD) += p1020rdb-pd.dtb
diff --git a/arch/powerpc/dts/mpc8379erdb.dts b/arch/powerpc/dts/mpc8379erdb.dts
new file mode 100644
index 00..b1881b161c
--- /dev/null
+++ b/arch/powerpc/dts/mpc8379erdb.dts
@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: GPL-2.0+ OR X11
+/*
+ * MPC8379E RDB Device Tree Source
+ *
+ * Copyright 2020 NXP
+ */
+
+/dts-v1/;
+
+/ {
+   compatible = "fsl,mpc8379erdb";
+
+   #address-cells = <1>;
+   #size-cells = <1>;
+
+   cpus {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   PowerPC,8379@0 {
+   device_type = "cpu";
+   reg = <0x0>;
+   d-cache-line-size = <32>;
+   i-cache-line-size = <32>;
+   d-cache-size = <32768>;
+   i-cache-size = <32768>;
+   timebase-frequency = <0>;
+   bus-frequency = <0>;
+   clock-frequency = <0>;
+   };
+   };
+
+   memory {
+   device_type = "memory";
+   reg = <0x 0x1000>;  // 256MB at 0
+   };
+
+   localbus@e0005000 {
+   #address-cells = <2>;
+   #size-cells = <1>;
+   compatible = "fsl,elbc", "simple-bus";
+   reg = <0xe0005000 0x1000>;
+   interrupts = <77 0x8>;
+   interrupt-parent = <>;
+   };
+
+   immr@e000 {
+   #address-cells = <1>;
+   #size-cells = <1>;
+   device_type = "soc";
+   compatible = "simple-bus";
+   ranges = <0 0xe000 0x0010>;
+   reg = <0xe000 0x0200>;
+   bus-frequency = <0>;
+
+   sdhc@2e000 {
+   compatible = "fsl,esdhc";
+   reg = <0x2e000 0x1000>;
+   bus-width = <0x4>;
+   clock-frequency = <0>;
+   };
+
+   ipic: interrupt-controller@700 {
+   compatible = "fsl,ipic";
+   interrupt-controller;
+   #address-cells = <0>;
+   #interrupt-cells = <2>;
+   reg = <0x700 0x100>;
+   device_type = "ipic";
+   };
+
+   };
+
+};
--
2.14.5



[PATCH v3] serial: ns16550: Fix ordering of getting base address

2020-04-03 Thread Bin Meng
Currently the driver gets ns16550 base address in the driver
probe() routine, which may potentially break any ns16550 wrapper
driver that does additional initialization before calling
ns16550_serial_probe().

Things are complicated that we need consider ns16550 devices on
both simple-bus and PCI bus. To fix the issue we move the base
address assignment for simple-bus ns16550 device back to the
ofdata_to_platdata(), and assign base address for PCI ns16550
device in ns16550_serial_probe().

This is still not perfect. If any PCI bus based ns16550 wrapper
driver tries to access plat->base before calling probe(), it is
still subject to break.

Fixes: 720f9e1fdb0c9 ("serial: ns16550: Move PCI access from 
ofdata_to_platdata() to probe()")
Reported-by: Andy Shevchenko 
Signed-off-by: Bin Meng 
Tested-by: Andy Shevchenko 
Reviewed-by: Wolfgang Wallner 
Tested-by: Wolfgang Wallner 

---

Changes in v3:
- extract the base address assignment to a helper

Changes in v2:
- not to break Fixes, etc to two or more lines in the commit message
- add the same CONFIG_SYS_NS16550_PORT_MAPPED ifdefs in the PCI case

 drivers/serial/ns16550.c | 39 ++-
 1 file changed, 22 insertions(+), 17 deletions(-)

diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
index c1b303f..a2f1b35 100644
--- a/drivers/serial/ns16550.c
+++ b/drivers/serial/ns16550.c
@@ -479,39 +479,38 @@ static int ns16550_serial_getinfo(struct udevice *dev,
return 0;
 }
 
-#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
-static int ns1655_serial_set_base_addr(struct udevice *dev)
+static int ns16550_serial_assign_base(struct ns16550_platdata *plat, ulong 
base)
 {
-   fdt_addr_t addr;
-   struct ns16550_platdata *plat;
-
-   plat = dev_get_platdata(dev);
-
-   addr = dev_read_addr_pci(dev);
-   if (addr == FDT_ADDR_T_NONE)
+   if (base == FDT_ADDR_T_NONE)
return -EINVAL;
 
 #ifdef CONFIG_SYS_NS16550_PORT_MAPPED
-   plat->base = addr;
+   plat->base = base;
 #else
-   plat->base = (unsigned long)map_physmem(addr, 0, MAP_NOCACHE);
+   plat->base = (unsigned long)map_physmem(base, 0, MAP_NOCACHE);
 #endif
 
return 0;
 }
-#endif
 
 int ns16550_serial_probe(struct udevice *dev)
 {
+   struct ns16550_platdata *plat = dev->platdata;
struct NS16550 *const com_port = dev_get_priv(dev);
struct reset_ctl_bulk reset_bulk;
+   fdt_addr_t addr;
int ret;
 
-#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
-   ret = ns1655_serial_set_base_addr(dev);
-   if (ret)
-   return ret;
-#endif
+   /*
+* If we are on PCI bus, either directly attached to a PCI root port,
+* or via a PCI bridge, assign platdata->base before probing hardware.
+*/
+   if (device_is_on_pci_bus(dev)) {
+   addr = devfdt_get_addr_pci(dev);
+   ret = ns16550_serial_assign_base(plat, addr);
+   if (ret)
+   return ret;
+   }
 
ret = reset_get_bulk(dev, _bulk);
if (!ret)
@@ -535,9 +534,15 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
 {
struct ns16550_platdata *plat = dev->platdata;
const u32 port_type = dev_get_driver_data(dev);
+   fdt_addr_t addr;
struct clk clk;
int err;
 
+   addr = dev_read_addr(dev);
+   err = ns16550_serial_assign_base(plat, addr);
+   if (err && !device_is_on_pci_bus(dev))
+   return err;
+
plat->reg_offset = dev_read_u32_default(dev, "reg-offset", 0);
plat->reg_shift = dev_read_u32_default(dev, "reg-shift", 0);
plat->reg_width = dev_read_u32_default(dev, "reg-io-width", 1);
-- 
2.7.4



Re: [PATCH] mpc8379erdb: Convert to DM_MMC

2020-04-03 Thread Sinan Akman



  Hi Jehoon

On 2020-04-03 7:59 p.m., Jaehoon Chung wrote:

HI,

On 4/4/20 6:36 AM, si...@writeme.com wrote:

From: Sinan Akman 


I'm not sure but its subject is strange.
It's not only the converting to DM_MMC. you're adding mpc8397.dtb device-tree.
I think you don't add device-tree for only mmc.

It's better to separate patches. "Add devicetree...", " Convert to DM_MMC".
It's just my opinion.


  You are right. I will split the patch into two and for dts I will
have only the related nodes.

  Thanks for your comments.

  Regards
  Sinan Akman




Re: [PATCH] mpc8379erdb: Convert to DM_MMC

2020-04-03 Thread Jaehoon Chung
HI,

On 4/4/20 6:36 AM, si...@writeme.com wrote:
> From: Sinan Akman 
> 

I'm not sure but its subject is strange. 
It's not only the converting to DM_MMC. you're adding mpc8397.dtb device-tree.
I think you don't add device-tree for only mmc.

It's better to separate patches. "Add devicetree...", " Convert to DM_MMC". 
It's just my opinion.

Best Regards,
Jaehoon Chung

> Signed-off-by: Sinan Akman 
> Cc: mario@gdsys.cc
> ---
>  arch/powerpc/dts/Makefile |   1 +
>  arch/powerpc/dts/mpc8379erdb.dts  | 239 
> ++
>  board/freescale/mpc837xerdb/mpc837xerdb.c |   9 +-
>  configs/MPC837XERDB_defconfig |   5 +-
>  4 files changed, 252 insertions(+), 2 deletions(-)
>  create mode 100644 arch/powerpc/dts/mpc8379erdb.dts
> 
> diff --git a/arch/powerpc/dts/Makefile b/arch/powerpc/dts/Makefile
> index 3195351c9c..e3ec033096 100644
> --- a/arch/powerpc/dts/Makefile
> +++ b/arch/powerpc/dts/Makefile
> @@ -1,5 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0+
> 
> +dtb-$(CONFIG_TARGET_MPC837XERDB) += mpc8379erdb.dtb
>  dtb-$(CONFIG_TARGET_MPC8548CDS) += mpc8548cds.dtb mpc8548cds_36b.dtb
>  dtb-$(CONFIG_TARGET_P1020RDB_PC) += p1020rdb-pc.dtb p1020rdb-pc_36b.dtb
>  dtb-$(CONFIG_TARGET_P1020RDB_PD) += p1020rdb-pd.dtb
> diff --git a/arch/powerpc/dts/mpc8379erdb.dts 
> b/arch/powerpc/dts/mpc8379erdb.dts
> new file mode 100644
> index 00..33afaf9a94
> --- /dev/null
> +++ b/arch/powerpc/dts/mpc8379erdb.dts
> @@ -0,0 +1,239 @@
> +// SPDX-License-Identifier: GPL-2.0+ OR X11
> +/*
> + * MPC8379E RDB Device Tree Source
> + *
> + * Copyright 2020 NXP
> + */
> +
> +/dts-v1/;
> +
> +/ {
> + compatible = "fsl,mpc8379erdb";
> +
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + aliases {
> + serial0 = 
> + serial1 = 
> + };
> +
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + PowerPC,8379@0 {
> + device_type = "cpu";
> + reg = <0x0>;
> + d-cache-line-size = <32>;
> + i-cache-line-size = <32>;
> + d-cache-size = <32768>;
> + i-cache-size = <32768>;
> + timebase-frequency = <0>;
> + bus-frequency = <0>;
> + clock-frequency = <0>;
> + };
> + };
> +
> + memory {
> + device_type = "memory";
> + reg = <0x 0x1000>;  // 256MB at 0
> + };
> +
> + localbus@e0005000 {
> + #address-cells = <2>;
> + #size-cells = <1>;
> + compatible = "fsl,elbc", "simple-bus";
> + reg = <0xe0005000 0x1000>;
> + interrupts = <77 0x8>;
> + interrupt-parent = <>;
> + };
> +
> + immr@e000 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + device_type = "soc";
> + compatible = "simple-bus";
> + ranges = <0 0xe000 0x0010>;
> + reg = <0xe000 0x0200>;
> + bus-frequency = <0>;
> +
> + wdt@200 {
> + device_type = "watchdog";
> + compatible = "mpc83xx_wdt";
> + reg = <0x200 0x100>;
> + };
> +
> + gpio0: gpio@c00 {
> + #gpio-cells = <2>;
> + device_type = "gpio";
> + compatible = "fsl,mpc8379-gpio", "fsl,mpc8349-gpio";
> + reg = <0xc00 0x18>;
> + interrupts = <74 0x8>;
> + interrupt-parent = <>;
> + gpio-controller;
> + };
> +
> + gpio2: gpio-controller@d00 {
> + #gpio-cells = <2>;
> + device_type = "gpio";
> + compatible = "fsl,mpc8379-gpio", "fsl,mpc8349-gpio";
> + reg = <0xd00 0x100>;
> + interrupts = <75 0x8>;
> + interrupt-parent = <>;
> + gpio-controller;
> + };
> +
> + i2c@3000 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + cell-index = <0>;
> + compatible = "fsl-i2c";
> + reg = <0x3000 0x100>;
> + interrupts = <14 0x8>;
> + interrupt-parent = <>;
> + dfsrr;
> + };
> +
> + i2c@3100 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + cell-index = <1>;
> + compatible = "fsl-i2c";
> + reg = <0x3100 0x100>;
> + interrupts = <15 0x8>;
> + interrupt-parent = <>;
> + dfsrr;
> + };
> +
> +

Re: [PATCH] arm: stm32mp1: activate data cache in SPL and before relocation

2020-04-03 Thread Marek Vasut
On 4/3/20 10:03 AM, Patrick DELAUNAY wrote:
> Hi Marek,

Hi,

>> From: Marek Vasut 
>> Sent: lundi 30 mars 2020 16:04
>>
>> On 3/30/20 3:49 PM, Patrick DELAUNAY wrote:
>>> Hi Marek,
>>
>> Hi,
>>
>> [...]
>>
> - /* Enable D-cache. I-cache is already enabled in start.S */
> + /* I-cache is already enabled in start.S */
>>>
>>> Not needed for arm V7 (I copy this function from other platfrom ... I
>>> don't remember which one)
>>
>> Maybe this needs to be de-duplicated if it's a copy ?
> 
> I don't remember 
> As I mixed several references
> 
> But I found the same content in many arm arch;
> 
> arch/arm/mach-imx/mx5/soc.c:67
> arch/arm/mach-rockchip/board.c:47
> arch/arm/mach-tegra/board.c:271
> arch/arm/mach-sunxi/board.c:347
> arch/arm/mach-exynos/soc.c:30:
> arch/arm/mach-zynq/cpu.c:88:
> arch/arm/cpu/armv7/iproc-common/hwinit-common.c:1
> arch/arm/mach-u8500/cache.c:14
> arch/arm/mach-keystone/init.c:206
> 
> And different implementation in 
> arch/arm/mach-socfpga/misc.c:55

On SoCFPGA, we are sure both need to be enabled, except SPL might not
want to enable dcache, which is why it's implemented there that way.
I dunno about the other platforms.

> mach-omap2/omap-cache.c:49
> void enable_caches(void)
> {
> 
>   /* Enable I cache if not enabled */
>   if (!icache_status())
>   icache_enable();
> 
>   dcache_enable();
> }
> 
> the issue the weak function empty, so it is mandatory to
> redefine the real implementation for each platform.
> 
> arch/arm/lib/cache.c:35
> /*
>  * Default implementation of enable_caches()
>  * Real implementation should be in platform code
>  */
> __weak void enable_caches(void)
> {
>   puts("WARNING: Caches not enabled\n");
> }

Hm, that's a valid point. Then I think we're stuck with
re-reimplementing this one.

> [...]
> 
>
> +static void set_mmu_dcache(u32 addr, u32 size) {
> + int i;
> + u32 start, end;
> +
> + start = addr >> MMU_SECTION_SHIFT;
> + end = ((u64)addr + (u64)size) >> MMU_SECTION_SHIFT;

 Is this a copy of dram_bank_mmu_setup() in arch/arm/lib/cache-cp15.c ?
 Why ?
>>>
>>> It is not just a copy...
>>>
>>> set__mmu_dache is only a static helper for  function
>>> dram_bank_mmu_setup()
>>>
>>> I override the default implementation of the weak functon
>>> dram_bank_mmu_setup()
>>
>> Can you instead augment the original implementation to cater for this 
>> usecase or
>> would that be too difficult ?
> 
> Have a generic behavior...
> 
> I will propose to protect the access to bd->bi_dram[bank] in 
> dram_bank_mmu_setup

Thanks!

[...]

>>> SYRAM content (board_f)
>>> - SPL code
>>> - SPL data
>>> - SPL stack (reversed order)
>>> - TTB
>>>
>>> But I can move it in BSS as global apl variable, I need to think about 
>>> it
>>> It is probably more clean.
>>
>> Please do :)
> 
> I willl move it in ".data" section in V2 for SPL and U-Boot.
> 
> Even in binary size increase and the SPL load time
> by ROM code is increase by 30ms.

Can it be mallocated instead ? If it's in initialized data, it will add
to the binary size, and I don't think you need it to be initialized data.


Re: [PATCH 3/3] arm: caches: manage phys_addr_t overflow in mmu_set_region_dcache_behaviour

2020-04-03 Thread Marek Vasut
On 4/3/20 10:28 AM, Patrick Delaunay wrote:
> Detect and solve the overflow on phys_addr_t type for start + size in
> mmu_set_region_dcache_behaviour() function.
> 
> This issue occurs for example with ARM32, start = 0xC000 and
> size = 0x4000: start + size = 0x1 and end = 0x0.
> 
> Overflow is detected when end < start.
> In normal case the previous behavior is still used: when start is not
> aligned on MMU section, the end address is only aligned after the sum
> start + size.
> 
> Signed-off-by: Patrick Delaunay 
> ---
> 
>  arch/arm/lib/cache-cp15.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/arm/lib/cache-cp15.c b/arch/arm/lib/cache-cp15.c
> index d15144188b..e5a7fd0ef4 100644
> --- a/arch/arm/lib/cache-cp15.c
> +++ b/arch/arm/lib/cache-cp15.c
> @@ -63,6 +63,11 @@ void mmu_set_region_dcache_behaviour(phys_addr_t start, 
> size_t size,
>  
>   end = ALIGN(start + size, MMU_SECTION_SIZE) >> MMU_SECTION_SHIFT;
>   start = start >> MMU_SECTION_SHIFT;
> +
> + /* phys_addr_t overflow detected */
> + if (end < start)
> + end = (~(phys_addr_t)0x0 >> MMU_SECTION_SHIFT) + 1;
> +

Or, you can divide $start and $size separately by MMU_SECTION_SIZE and
then add them up .


[PATCH] mpc8379erdb: Convert to DM_MMC

2020-04-03 Thread sinan
From: Sinan Akman 

Signed-off-by: Sinan Akman 
Cc: mario@gdsys.cc
---
 arch/powerpc/dts/Makefile |   1 +
 arch/powerpc/dts/mpc8379erdb.dts  | 239 ++
 board/freescale/mpc837xerdb/mpc837xerdb.c |   9 +-
 configs/MPC837XERDB_defconfig |   5 +-
 4 files changed, 252 insertions(+), 2 deletions(-)
 create mode 100644 arch/powerpc/dts/mpc8379erdb.dts

diff --git a/arch/powerpc/dts/Makefile b/arch/powerpc/dts/Makefile
index 3195351c9c..e3ec033096 100644
--- a/arch/powerpc/dts/Makefile
+++ b/arch/powerpc/dts/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0+

+dtb-$(CONFIG_TARGET_MPC837XERDB) += mpc8379erdb.dtb
 dtb-$(CONFIG_TARGET_MPC8548CDS) += mpc8548cds.dtb mpc8548cds_36b.dtb
 dtb-$(CONFIG_TARGET_P1020RDB_PC) += p1020rdb-pc.dtb p1020rdb-pc_36b.dtb
 dtb-$(CONFIG_TARGET_P1020RDB_PD) += p1020rdb-pd.dtb
diff --git a/arch/powerpc/dts/mpc8379erdb.dts b/arch/powerpc/dts/mpc8379erdb.dts
new file mode 100644
index 00..33afaf9a94
--- /dev/null
+++ b/arch/powerpc/dts/mpc8379erdb.dts
@@ -0,0 +1,239 @@
+// SPDX-License-Identifier: GPL-2.0+ OR X11
+/*
+ * MPC8379E RDB Device Tree Source
+ *
+ * Copyright 2020 NXP
+ */
+
+/dts-v1/;
+
+/ {
+   compatible = "fsl,mpc8379erdb";
+
+   #address-cells = <1>;
+   #size-cells = <1>;
+
+   aliases {
+   serial0 = 
+   serial1 = 
+   };
+
+   cpus {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   PowerPC,8379@0 {
+   device_type = "cpu";
+   reg = <0x0>;
+   d-cache-line-size = <32>;
+   i-cache-line-size = <32>;
+   d-cache-size = <32768>;
+   i-cache-size = <32768>;
+   timebase-frequency = <0>;
+   bus-frequency = <0>;
+   clock-frequency = <0>;
+   };
+   };
+
+   memory {
+   device_type = "memory";
+   reg = <0x 0x1000>;  // 256MB at 0
+   };
+
+   localbus@e0005000 {
+   #address-cells = <2>;
+   #size-cells = <1>;
+   compatible = "fsl,elbc", "simple-bus";
+   reg = <0xe0005000 0x1000>;
+   interrupts = <77 0x8>;
+   interrupt-parent = <>;
+   };
+
+   immr@e000 {
+   #address-cells = <1>;
+   #size-cells = <1>;
+   device_type = "soc";
+   compatible = "simple-bus";
+   ranges = <0 0xe000 0x0010>;
+   reg = <0xe000 0x0200>;
+   bus-frequency = <0>;
+
+   wdt@200 {
+   device_type = "watchdog";
+   compatible = "mpc83xx_wdt";
+   reg = <0x200 0x100>;
+   };
+
+   gpio0: gpio@c00 {
+   #gpio-cells = <2>;
+   device_type = "gpio";
+   compatible = "fsl,mpc8379-gpio", "fsl,mpc8349-gpio";
+   reg = <0xc00 0x18>;
+   interrupts = <74 0x8>;
+   interrupt-parent = <>;
+   gpio-controller;
+   };
+
+   gpio2: gpio-controller@d00 {
+   #gpio-cells = <2>;
+   device_type = "gpio";
+   compatible = "fsl,mpc8379-gpio", "fsl,mpc8349-gpio";
+   reg = <0xd00 0x100>;
+   interrupts = <75 0x8>;
+   interrupt-parent = <>;
+   gpio-controller;
+   };
+
+   i2c@3000 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   cell-index = <0>;
+   compatible = "fsl-i2c";
+   reg = <0x3000 0x100>;
+   interrupts = <14 0x8>;
+   interrupt-parent = <>;
+   dfsrr;
+   };
+
+   i2c@3100 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   cell-index = <1>;
+   compatible = "fsl-i2c";
+   reg = <0x3100 0x100>;
+   interrupts = <15 0x8>;
+   interrupt-parent = <>;
+   dfsrr;
+   };
+
+   spi@7000 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   cell-index = <0>;
+   compatible = "fsl,spi";
+   reg = <0x7000 0x1000>;
+   interrupts = <16 0x8>;
+   interrupt-parent = <>;
+   mode = "cpu";
+   };
+
+   sdhc@2e000 {
+   compatible = 

Re: [PATCH v2 2/2] arm: stm32mp: activate data cache on DDR in SPL

2020-04-03 Thread Marek Vasut
On 4/3/20 11:25 AM, Patrick Delaunay wrote:
> Activate cache on DDR to improves the accesses to DDR used by SPL:
> - CONFIG_SPL_BSS_START_ADDR
> - CONFIG_SYS_SPL_MALLOC_START
> 
> Cache is configured only when DDR is fully initialized,
> to avoid speculative access and issue in get_ram_size().
> Data cache is deactivated at the end of SPL, to flush the data cache
> and the TLB.
> 
> Signed-off-by: Patrick Delaunay 
> ---
> 
> Changes in v2:
> - new
> 
>  arch/arm/mach-stm32mp/spl.c | 21 +
>  1 file changed, 21 insertions(+)
> 
> diff --git a/arch/arm/mach-stm32mp/spl.c b/arch/arm/mach-stm32mp/spl.c
> index 9cd7b418a4..279121af75 100644
> --- a/arch/arm/mach-stm32mp/spl.c
> +++ b/arch/arm/mach-stm32mp/spl.c
> @@ -4,6 +4,7 @@
>   */
>  
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -117,4 +118,24 @@ void board_init_f(ulong dummy)
>   printf("DRAM init failed: %d\n", ret);
>   hang();
>   }
> +
> + /*
> +  * activate cache on DDR only when DDR is fully initialized
> +  * to avoid speculative access and issue in get_ram_size()
> +  */
> + if (!CONFIG_IS_ENABLED(SYS_DCACHE_OFF))
> + mmu_set_region_dcache_behaviour(STM32_DDR_BASE, STM32_DDR_SIZE,
> + DCACHE_DEFAULT_OPTION);
> +}
> +
> +void spl_board_prepare_for_boot(void)
> +{
> + dcache_disable();
> + debug("SPL bye\n");
> +}
> +
> +void spl_board_prepare_for_boot_linux(void)
> +{
> + dcache_disable();
> + debug("SPL bye\n");

Is the debug() statement really needed ? I think the common SPL code
already has some.


Re: [PATCH v2 1/2] arm: stm32mp: activate data cache in SPL and before relocation

2020-04-03 Thread Marek Vasut
On 4/3/20 11:25 AM, Patrick Delaunay wrote:
[...]
> diff --git a/arch/arm/mach-stm32mp/cpu.c b/arch/arm/mach-stm32mp/cpu.c
> index 36a9205819..c22c1a9bbc 100644
> --- a/arch/arm/mach-stm32mp/cpu.c
> +++ b/arch/arm/mach-stm32mp/cpu.c
> @@ -75,6 +75,12 @@
>  #define PKG_SHIFT27
>  #define PKG_MASK GENMASK(2, 0)
>  
> +/*
> + * early TLB into the .data section so that it not get cleared
> + * with 16kB allignment (see TTBR0_BASE_ADDR_MASK)
> + */
> +u8 early_tlb[PGTABLE_SIZE] __section(".data") __aligned(0x4000);

Can you early-malloc this one ?
(why do you need this in __section("data") ?)

[...]


Re: [PATCH 2/3] arm: caches: add DCACHE_DEFAULT_OPTION

2020-04-03 Thread Marek Vasut
On 4/3/20 10:28 AM, Patrick Delaunay wrote:
> Add the new flags DCACHE_DEFAULT_OPTION to define the default
> option to use according the compilation flags
> CONFIG_SYS_ARM_CACHE_WRITETHROUGH or CONFIG_SYS_ARM_CACHE_WRITEALLOC.

Can't you unify these macros into a single Kconfig "select" statement
instead , and then just select the matching cache configuration in Kconfig ?

Or better yet, can't you extract this info from DT ?


Re: [PATCH 1/3] arm: caches: protect dram_bank_mmu_setup access to bi_dram

2020-04-03 Thread Marek Vasut
On 4/3/20 10:28 AM, Patrick Delaunay wrote:
> Add protection in dram_bank_mmu_setup() to avoid access to bd->bi_dram
> before relocation.
> 
> This patch allow to use the generic weak function dram_bank_mmu_setup
> to activate the MMU and the data cache in SPL or in U-Boot before
> relocation, when bd->bi_dram is not yet initialized.
> 
> In this cases, the MMU must be initialized explicitly with
> mmu_set_region_dcache_behaviour function.
> 
> Signed-off-by: Patrick Delaunay 
> ---
> 
>  arch/arm/lib/cache-cp15.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm/lib/cache-cp15.c b/arch/arm/lib/cache-cp15.c
> index f8d20960da..54509f11c3 100644
> --- a/arch/arm/lib/cache-cp15.c
> +++ b/arch/arm/lib/cache-cp15.c
> @@ -91,6 +91,10 @@ __weak void dram_bank_mmu_setup(int bank)
>   bd_t *bd = gd->bd;
>   int i;
>  
> + /* bd->bi_dram is available only after relocation */
> + if ((gd->flags & GD_FLG_RELOC) == 0)
> + return;

Why not just set the bd->bi_dram correctly before this is called ?


Re: latest u-boot branch for Marvell Armada 88F3720

2020-04-03 Thread Marek Behun
Hi Moritz,

which version of EspressoBin do you have? How much RAM, is it DDR3 or
DDR4? How many RAM chips are there?

Marek


Pull request, u-boot-tegra/master

2020-04-03 Thread Tom Warren
 Tom,

Please pull u-boot-tegra/master into U-Boot/master. Thanks.

All Tegra builds are OK on my system, and Stephen's test frame reports that
all tests pass.
This adds support for Jetson Nano, plus miscellaneous other fixes found
during Nano bringup.
It also adds Igor's update_uboot wrapper patches.

The following changes since commit e0718b3ab754860bd47677e6b4fc5b70da42c4ab:

  Merge tag 'dm-pull-1apr20' of git://git.denx.de/u-boot-dm (2020-04-01
14:29:21 -0400)

are available in the git repository at:

  ssh://twarren@git-mirror-santaclara:12001/m/denx.de/u-boot-tegra.git
master

for you to fetch changes up to 7c02bc9649f6d3afd272ac4a94b280495473834c:

  ARM: tegra: Add NVIDIA Jetson Nano Developer Kit support (2020-04-02
14:30:02 -0700)


Igor Opaniuk (4):
  apalis-tk1: add update_uboot wrapper
  apalis_t30: add update_uboot wrapper
  colibri_t20: add update_uboot wrapper
  colibri_t30: add update_uboot wrapper

JC Kuo (1):
  t210: do not enable PLLE and UPHY PLL HW PWRSEQ

Stephen Warren (1):
  ARM: tegra: p2371-2180: add I2C nodes to DT

Tom Warren (11):
  t210: pinmux: Remove pinmux/GPIO init from T210 boards
  i2c: t210: Add VI_I2C clock source support
  tegra: Enable CONFIG_BOOTP_PREFER_SERVERIP for all Jetson boards
  mmc: t210: Add autocal and tap/trim updates for SDMMC1/3
  mmc: t210: Fix 'bad' SD-card clock when doing 400KHz card detect
  qspi: t210: Fix claim_bus's use of the wrong bus/device
  qspi: t210: Fix QSPI clock and tap delays
  qspi: t210: Use dev_read calls to get FDT data like base, freq
  t210: Adjust ramdisk_addr_r/fdt_addr_r to allow for large kernels
  mtd: spi: Add Macronix MX25U3235F device
  ARM: tegra: Add NVIDIA Jetson Nano Developer Kit support

Vishruth (1):
  ARM: tegra: p2771-: enable PIE relocation

 arch/arm/cpu/armv8/cpu.c   |   5 +
 arch/arm/dts/Makefile  |   3 +-
 arch/arm/dts/tegra210-p2371-2180.dts   |  12 +
 arch/arm/dts/tegra210-p3450-.dts   | 147 +++
 arch/arm/include/asm/arch-tegra/tegra_mmc.h|  20 +-
 arch/arm/include/asm/arch-tegra/xusb-padctl.h  |   1 +
 arch/arm/mach-tegra/board2.c   |  31 +++
 arch/arm/mach-tegra/tegra210/Kconfig   |   7 +
 arch/arm/mach-tegra/tegra210/Makefile  |   3 +-
 arch/arm/mach-tegra/tegra210/clock.c   |  27 +-
 arch/arm/mach-tegra/tegra210/pinmux.c  | 194 --
 arch/arm/mach-tegra/tegra210/xusb-padctl.c |  68 +++--
 arch/arm/mach-tegra/xusb-padctl-dummy.c|   4 +
 board/nvidia/e2220-1170/e2220-1170.c   |  21 +-
 board/nvidia/e2220-1170/pinmux-config-e2220-1170.h | 276

 board/nvidia/p2371-/p2371-.c   |  21 +-
 board/nvidia/p2371-/pinmux-config-p2371-.h | 267

 board/nvidia/p2371-2180/p2371-2180.c   |  21 +-
 board/nvidia/p2371-2180/pinmux-config-p2371-2180.h | 278
-
 board/nvidia/p2571/p2571.c |  21 +-
 board/nvidia/p2571/pinmux-config-p2571.h   | 242 --
 board/nvidia/p3450-/Kconfig|  12 +
 board/nvidia/p3450-/MAINTAINERS|   6 +
 board/nvidia/p3450-/Makefile   |   8 +
 board/nvidia/p3450-/p3450-.c   | 178 +
 configs/e2220-1170_defconfig   |   1 +
 configs/p2371-_defconfig   |   1 +
 configs/p2371-2180_defconfig   |   1 +
 configs/p2571_defconfig|   1 +
 configs/p2771--000_defconfig   |   2 +
 configs/p2771--500_defconfig   |   2 +
 configs/p3450-_defconfig   |  64 +
 drivers/mmc/tegra_mmc.c| 103 +++-
 drivers/mtd/spi/spi-nor-ids.c  |   1 +
 drivers/spi/tegra210_qspi.c|  39 +--
 include/configs/apalis-tk1.h   |   9 +
 include/configs/apalis_t30.h   |  11 +
 include/configs/colibri_t20.h  |   8 +-
 include/configs/colibri_t30.h  |  11 +
 include/configs/p3450-.h   |  46 
 include/configs/tegra210-common.h  |   4 +-
 41 files changed, 757 insertions(+), 1420 deletions(-)
 create mode 100644 arch/arm/dts/tegra210-p3450-.dts
 delete mode 100644 arch/arm/mach-tegra/tegra210/pinmux.c
 delete mode 100644 board/nvidia/e2220-1170/pinmux-config-e2220-1170.h
 delete mode 100644 board/nvidia/p2371-/pinmux-config-p2371-.h
 delete mode 100644 board/nvidia/p2371-2180/pinmux-config-p2371-2180.h
 delete mode 100644 board/nvidia/p2571/pinmux-config-p2571.h
 create mode 100644 

[PATCH v2 u-boot-marvell 3/4] arm: mvebu: turris_mox: Setup Linux's device tree before boot

2020-04-03 Thread Marek Behún
Patch Linux's device tree according to which Mox modules are connected.
Linux's device tree has all possible Mox module nodes preprogrammed, but
in disabled state.

If MOX B, MOX F or MOX G module is present, this code enables the PCI
node.

For the network modules (MOX C, MOX D and MOX E) are present, the code
enables corresponding ethernet and swtich nodes and DSA connections.
For the SFP cage the SFP GPIO controller node and SFP node are also
enabled.

Signed-off-by: Marek Behún 
---
 board/CZ.NIC/turris_mox/turris_mox.c | 263 +++
 configs/turris_mox_defconfig |   1 +
 2 files changed, 264 insertions(+)

diff --git a/board/CZ.NIC/turris_mox/turris_mox.c 
b/board/CZ.NIC/turris_mox/turris_mox.c
index 0b13d1d190..bc0d067889 100644
--- a/board/CZ.NIC/turris_mox/turris_mox.c
+++ b/board/CZ.NIC/turris_mox/turris_mox.c
@@ -3,6 +3,7 @@
  * Copyright (C) 2018 Marek Behun 
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -34,7 +35,11 @@
 #define ARMADA_37XX_SPI_DOUT   0xd0010608
 #define ARMADA_37XX_SPI_DIN0xd001060c
 
+#define ETH1_PATH  "/soc/internal-regs@d000/ethernet@4"
+#define MDIO_PATH  "/soc/internal-regs@d000/mdio@32004"
+#define SFP_GPIO_PATH  "/soc/internal-regs@d000/spi@10600/moxtet@1/gpio@0"
 #define PCIE_PATH  "/soc/pcie@d007"
+#define SFP_PATH   "/sfp"
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -551,3 +556,261 @@ int last_stage_init(void)
 
return 0;
 }
+
+#if defined(CONFIG_OF_BOARD_SETUP)
+
+static int vnode_by_path(void *blob, const char *fmt, va_list ap)
+{
+   char path[128];
+
+   vsprintf(path, fmt, ap);
+   return fdt_path_offset(blob, path);
+}
+
+static int node_by_path(void *blob, const char *fmt, ...)
+{
+   va_list ap;
+   int res;
+
+   va_start(ap, fmt);
+   res = vnode_by_path(blob, fmt, ap);
+   va_end(ap);
+
+   return res;
+}
+
+static int phandle_by_path(void *blob, const char *fmt, ...)
+{
+   va_list ap;
+   int node, phandle, res;
+
+   va_start(ap, fmt);
+   node = vnode_by_path(blob, fmt, ap);
+   va_end(ap);
+
+   if (node < 0)
+   return node;
+
+   phandle = fdt_get_phandle(blob, node);
+   if (phandle > 0)
+   return phandle;
+
+   phandle = fdt_get_max_phandle(blob);
+   if (phandle < 0)
+   return phandle;
+
+   phandle += 1;
+
+   res = fdt_setprop_u32(blob, node, "linux,phandle", phandle);
+   if (res < 0)
+   return res;
+
+   res = fdt_setprop_u32(blob, node, "phandle", phandle);
+   if (res < 0)
+   return res;
+
+   return phandle;
+}
+
+static int enable_by_path(void *blob, const char *fmt, ...)
+{
+   va_list ap;
+   int node;
+
+   va_start(ap, fmt);
+   node = vnode_by_path(blob, fmt, ap);
+   va_end(ap);
+
+   if (node < 0)
+   return node;
+
+   return fdt_setprop_string(blob, node, "status", "okay");
+}
+
+static bool is_topaz(int id)
+{
+   return topaz && id == peridot + topaz - 1;
+}
+
+static int switch_addr(int id)
+{
+   return is_topaz(id) ? 0x2 : 0x10 + id;
+}
+
+static int setup_switch(void *blob, int id)
+{
+   int res, addr, i, node, phandle;
+
+   addr = switch_addr(id);
+
+   /* first enable the switch by setting status = "okay" */
+   res = enable_by_path(blob, MDIO_PATH "/switch%i@%x", id, addr);
+   if (res < 0)
+   return res;
+
+   /*
+* now if there are more switches or a SFP module coming after,
+* enable corresponding ports
+*/
+   if (id < peridot + topaz - 1)
+   res = enable_by_path(blob,
+MDIO_PATH "/switch%i@%x/ports/port@a",
+id, addr);
+   else if (id == peridot - 1 && !topaz && sfp)
+   res = enable_by_path(blob,
+MDIO_PATH "/switch%i@%x/ports/port-sfp@a",
+id, addr);
+   else
+   res = 0;
+   if (res < 0)
+   return res;
+
+   if (id >= peridot + topaz - 1)
+   return 0;
+
+   /* finally change link property if needed */
+   node = node_by_path(blob, MDIO_PATH "/switch%i@%x/ports/port@a", id,
+   addr);
+   if (node < 0)
+   return node;
+
+   for (i = id + 1; i < peridot + topaz; ++i) {
+   phandle = phandle_by_path(blob,
+ MDIO_PATH 
"/switch%i@%x/ports/port@%x",
+ i, switch_addr(i),
+ is_topaz(i) ? 5 : 9);
+   if (phandle < 0)
+   return phandle;
+
+   if (i == id + 1)
+   res = fdt_setprop_u32(blob, node, "link", phandle);
+   else
+   res = fdt_appendprop_u32(blob, node, "link", phandle);

[PATCH v2 u-boot-marvell 4/4] arm: mvebu: dts: turris_mox: fix USB3 regulator

2020-04-03 Thread Marek Behún
Commit e8e9715df2d4 requires the USB3 regulator node to have the
enable-active-high property for the regulator to work properly. The
GPIO_ACTIVE_HIGH constant is not enough anymore.

Signed-off-by: Marek Behún 
Fixes: e8e9715df2d4 ("regulator: fixed: Modify enable-active-high...")
Reviewed-by: Stefan Roese 
---
 arch/arm/dts/armada-3720-turris-mox.dts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/dts/armada-3720-turris-mox.dts 
b/arch/arm/dts/armada-3720-turris-mox.dts
index 4c65c3e32c..a1e0ad5020 100644
--- a/arch/arm/dts/armada-3720-turris-mox.dts
+++ b/arch/arm/dts/armada-3720-turris-mox.dts
@@ -42,6 +42,7 @@
startup-delay-us = <200>;
shutdown-delay-us = <100>;
gpio = < 0 GPIO_ACTIVE_HIGH>;
+   enable-active-high;
regulator-boot-on;
};
 
-- 
2.24.1



[PATCH v2 u-boot-marvell 2/4] arm: mvebu: dts: turris_mox: update sdhci properties

2020-04-03 Thread Marek Behún
With recent changes to the mmc subsystem (chip detect code etc) update
the sdhci node of the Turris Mox device tree.

Signed-off-by: Marek Behún 
Reviewed-by: Stefan Roese 
---
 arch/arm/dts/armada-3720-turris-mox.dts | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/arch/arm/dts/armada-3720-turris-mox.dts 
b/arch/arm/dts/armada-3720-turris-mox.dts
index c36a5b8895..4c65c3e32c 100644
--- a/arch/arm/dts/armada-3720-turris-mox.dts
+++ b/arch/arm/dts/armada-3720-turris-mox.dts
@@ -45,6 +45,20 @@
regulator-boot-on;
};
 
+   vsdc_reg: vsdc-reg {
+   compatible = "regulator-gpio";
+   regulator-name = "vsdc";
+   regulator-min-microvolt = <180>;
+   regulator-max-microvolt = <330>;
+   regulator-boot-on;
+
+   gpios = < 23 GPIO_ACTIVE_HIGH>;
+   gpios-states = <0>;
+   states = <180 0x1
+ 330 0x0>;
+   enable-active-high;
+   };
+
mdio {
#address-cells = <1>;
#size-cells = <0>;
@@ -93,7 +107,11 @@
 };
 
  {
+   wp-inverted;
bus-width = <4>;
+   cd-gpios = < 10 GPIO_ACTIVE_HIGH>;
+   vqmmc-supply = <_reg>;
+   marvell,pad-type = "sd";
status = "okay";
 };
 
-- 
2.24.1



[PATCH v2 u-boot-marvell 1/4] arm: mvebu: turris_mox: Fix early SPI communication

2020-04-03 Thread Marek Behún
The SPI clock signal changes value when the SPI configuration register
is configured. This can sometimes lead to the device misinterpreting
the enablement of the SPI controller as actual clock tick.
This can be solved by first setting the SPI CS1 pin from GPIO to SPI mode,
and only after that writing the SPI configuration register.

Signed-off-by: Marek Behún 
---
 board/CZ.NIC/turris_mox/turris_mox.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/board/CZ.NIC/turris_mox/turris_mox.c 
b/board/CZ.NIC/turris_mox/turris_mox.c
index 377191baef..0b13d1d190 100644
--- a/board/CZ.NIC/turris_mox/turris_mox.c
+++ b/board/CZ.NIC/turris_mox/turris_mox.c
@@ -67,9 +67,11 @@ int board_fix_fdt(void *blob)
 * to read SPI by reading/writing SPI registers directly
 */
 
-   writel(0x563fa, ARMADA_37XX_NB_GPIO_SEL);
writel(0x10df, ARMADA_37XX_SPI_CFG);
-   writel(0x2005b, ARMADA_37XX_SPI_CTRL);
+   /* put pin from GPIO to SPI mode */
+   clrbits_le32(ARMADA_37XX_NB_GPIO_SEL, BIT(12));
+   /* enable SPI CS1 */
+   setbits_le32(ARMADA_37XX_SPI_CTRL, BIT(17));
 
while (!(readl(ARMADA_37XX_SPI_CTRL) & 0x2))
udelay(1);
@@ -89,7 +91,8 @@ int board_fix_fdt(void *blob)
 
size = i;
 
-   writel(0x5b, ARMADA_37XX_SPI_CTRL);
+   /* disable SPI CS1 */
+   clrbits_le32(ARMADA_37XX_SPI_CTRL, BIT(17));
 
if (size > 1 && (topology[1] == MOX_MODULE_PCI ||
 topology[1] == MOX_MODULE_USB3 ||
-- 
2.24.1



Re: [PATCH] mtd: spi-nor: Enable dual and quad read for s25fl256s0

2020-04-03 Thread Jagan Teki
On Sat, Mar 28, 2020 at 12:28 AM  wrote:
>
> From: Bacem Daassi 
>
> The s25fl256s0 supports dual and quad read like s25fl256s1.
> Enable it by adding SPI_NOR_DUAL_READ and SPI_NOR_QUAD_READ
> flags to the flash_info entry.
> Tested on real silicon and confirmed to be working.
>
> Signed-off-by: Bacem Daassi 
> ---

Applied to u-boot-spi/master


Re: [Patch v3 1/3] mtd: spi-nor-ids: Enable SPI_NOR_OCTAL_READ flag for mt35xu*

2020-04-03 Thread Jagan Teki
On Sat, Mar 14, 2020 at 6:24 PM Kuldeep Singh  wrote:
>
> Commit 658df8bd9464 ("mtd: spi-nor-core: Add octal mode support")
> enables octal mode(1-1-8) support in spi-nor framework.
>
> mt35xu512aba and mt35xu02g supports SINGLE and OCTAL I/O. Hence, enable
> SPI_NOR_OCTAL_READ flag for these flashes.
>
> Signed-off-by: Kuldeep Singh 
> Reviewed-by: Vignesh Raghavendra 
> ---

Applied to u-boot-spi/master


Re: [u-boot][PATCH] spi: spi-mem: Add SPI_MEM_NO_DATA to the spi_mem_data_dir enum

2020-04-03 Thread Jagan Teki
On Fri, Mar 20, 2020 at 3:05 PM  wrote:
>
> From: Tudor Ambarus 
>
> Commit: 0ebb261a0b2d ("spi: spi-mem: Add SPI_MEM_NO_DATA to the 
> spi_mem_data_dir enum")
> in linux.
>
> When defining spi_mem_op templates we don't necessarily know the size
> that will be passed when the template is actually used, and basing the
> supports_op() check on op->data.nbytes to know whether there will be
> data transferred for a specific operation is not possible.
>
> Add SPI_MEM_NO_DATA to the spi_mem_data_dir enum so that we can base
> our checks on op->data.dir instead of op->data.nbytes.
>
> This also fixes a bug identified with the atmel-quaspi driver.
> The spi-nor core, when erasing sectors, fills the spi_mem_op template
> using SPI_MEM_OP_NO_DATA, which initializes all the data members with
> value zero. This is wrong because data.dir is treated as SPI_MEM_DATA_IN,
> which translates in our driver to read accesses for erases (RICR), while
> the controller expects write accesses (WICR).
>
> Signed-off-by: Tudor Ambarus 
> ---

Reviewed-by: Jagan Teki 


Re: [PATCH] spi: cadence-qspi: Move ref clock calculation to probe

2020-04-03 Thread Jagan Teki
On Mon, Feb 24, 2020 at 12:40 PM Pratyush Yadav  wrote:
>
> "assigned-clock-parents" and "assigned-clock-rates" DT properties take
> effect only after ofdata_to_platdata() when clk_set_defaults() is called
> in device_probe(). Therefore clk get rate() would return a wrong value
> in ofdata_to_platdata() when compared with probe. Hence it needs to be
> moved to probe.
>
> Tested on u-boot-ti/next.
>
> Signed-off-by: Pratyush Yadav 
> ---

Applied to u-boot-spi/master


Re: [PATCH] spi: use is_power_of_2 instead of hweight32 in spi_nor_write()

2020-04-03 Thread Jagan Teki
On Fri, Mar 13, 2020 at 5:37 AM Rasmus Villemoes
 wrote:
>
> hweight32 is a somewhat expensive way to check for power-of-2. Use the
> is_power_of_2 helper, which does the standard and cheap idiom
> foo&(foo-1)==0.
>
> add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-96 (-96)
> Function old new   delta
> spi_nor_write388 292 -96
>
> Signed-off-by: Rasmus Villemoes 
> ---

Applied to u-boot-spi/master


Re: [PATCH v2 2/2] mtd: nand: spi: add support for Toshiba TC58CVG2S0HRAIJ

2020-04-03 Thread Jagan Teki
On Wed, Mar 4, 2020 at 12:58 AM Robert Marko  wrote:
>
> Toshiba recently launched new revisions of their serial SLC NAND series.
> TC58CVG2S0HRAIJ is a refresh of previous series with minor improvements.
> Basic parameters are same so lets add support for this new revision.
>
> Datasheet: 
> https://business.kioxia.com/info/docget.jsp?did=58601=TC58CVG2S0HRAIJ
>
> Signed-off-by: Robert Marko 
> Tested-by: Luka Kovacic 
> Cc: Luka Perkov 
> ---

Reviewed-by: Jagan Teki 


Re: [PATCH v2 1/2] mtd: spi-nand: Import Toshiba SPI-NAND support

2020-04-03 Thread Jagan Teki
On Wed, Mar 4, 2020 at 12:57 AM Robert Marko  wrote:
>
> Linux has good support for Toshiba SPI-NAND, so lets import it.
>
> Signed-off-by: Robert Marko 
> Tested-by: Luka Kovacic 
> Cc: Luka Perkov 
> ---
> Changes from v1:
>  Refresh to apply due to free() to rfree() rename

Reviewed-by: Jagan Teki 


Re: [PATCH v2 2/4] nand: raw: zynq: Do not try to probe driver if nand flash is disabled

2020-04-03 Thread Jagan Teki
On Wed, Feb 26, 2020 at 4:09 PM Michal Simek  wrote:
>
> There is no reason to continue when DT status property indicates that NAND
> flash is disabled. But that means that NOR flash should be present that's
> why try it find it out.
>
> Signed-off-by: Michal Simek 
> ---

Reviewed-by: Jagan Teki 


Re: [PATCH v2 1/4] nand: raw: Do not free xnand structure

2020-04-03 Thread Jagan Teki
On Wed, Feb 26, 2020 at 4:08 PM Michal Simek  wrote:
>
> xnand structure is private data structure and it is handled by core and
> probe shouldn't touch it.
>
> Signed-off-by: Michal Simek 

Reviewed-by: Jagan Teki 


Re: [PATCH] spi: nxp-fspi: Add 1us delay to make controller ready for next transaction

2020-04-03 Thread Jagan Teki
On Mon, Feb 24, 2020 at 4:52 PM Kuldeep Singh  wrote:
>
> Board gets reset when performing burst read/write operations. On the
> other hand, no such behaviour is observed on small size operations.
>
> In Linux, readl_poll_timeout API already add delay of 1us which is
> skipped in U-boot. Since, NXP Flexspi U-boot driver is a ported version
> of Linux driver and U-boot poll_timeout API lacks delay functionality,
> add 1us delay so as to make controller ready for other transactions.
>
> Signed-off-by: Kuldeep Singh 
> ---
>  drivers/spi/nxp_fspi.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/spi/nxp_fspi.c b/drivers/spi/nxp_fspi.c
> index 0e6c7be..9703642 100644
> --- a/drivers/spi/nxp_fspi.c
> +++ b/drivers/spi/nxp_fspi.c
> @@ -756,6 +756,7 @@ static int nxp_fspi_exec_op(struct spi_slave *slave,
> err = fspi_readl_poll_tout(f, f->iobase + FSPI_STS0,
>FSPI_STS0_ARB_IDLE, 1, POLL_TOUT, true);
> WARN_ON(err);
> +   udelay(1);

Can you add relevant comments here, why is it for udelay(1) ?


Re: [Patch v5 1/7] spi: Transform the FSL QuadSPI driver to use the SPI MEM API

2020-04-03 Thread Jagan Teki
On Thu, Feb 20, 2020 at 10:58 PM Kuldeep Singh  wrote:
>
> To support the SPI MEM API, instead of modifying the existing U-Boot
> driver, this patch adds a port of the existing Linux driver.
> This also has the advantage that porting changes and fixes from Linux will
> be easier.
> Porting of driver left most of the functions unchanged while few of the
> changes are:
> -Remove lock(mutexes) and irq handler as u-boot is a single core execution.
> -Remove invalid masterid as it was required specially for multicore
> execution in LS2088ARDB which is not the case in u-boot.
> -Remove clock support as changing spi speed is not supported in uboot and
> nor in linux.
>
> Currently tested on LS1088ARDB, LS1012ARDB, LS1046ARDB, LS1046AFRWY,
> LS1043AQDS, LS1021ATWR, LS2088ARDB, I.MX6ULL EVK.
>
> Signed-off-by: Frieder Schrempf 
> Signed-off-by: Ashish Kumar 
> Signed-off-by: Kuldeep Singh 
> Reviewed-by: Stefan Roese 
> Tested-by: Stefan Roese 
> Acked-by: Vignesh Raghavendra 
> ---

Applied to u-boot-spi/master


Re: [PATCH 1/2] drivers: spi: Add commands for Micron SPI

2020-04-03 Thread Jagan Teki
On Sat, Nov 23, 2019 at 4:39 AM Vladimir Olovyannikov
 wrote:
>
> Add commands for dual and quad SPI transfers on Micon SPI.
>
> Signed-off-by: Corneliu Doban 
> Signed-off-by: Vladimir Olovyannikov 
> ---
>  include/spi.h | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/include/spi.h b/include/spi.h
> index 6fbb4336ce..ae36835e95 100644
> --- a/include/spi.h
> +++ b/include/spi.h
> @@ -30,6 +30,10 @@
>  #define SPI_RX_SLOWBIT(11) /* receive with 1 wire slow */
>  #define SPI_RX_DUALBIT(12) /* receive with 2 wires */
>  #define SPI_RX_QUADBIT(13) /* receive with 4 wires */
> +#define SPI_RX_4X  BIT(14) /*
> +* addr on 1 wire
> +* data on 4 wires
> +*/

Not sure why flash commands will be supporting on the spi side?

Jagan.


Re: AW: AW: latest u-boot branch for Marvell Armada 88F3720

2020-04-03 Thread Stefan Roese

Hi Moritz,

On 03.04.20 16:25, Moritz Berghof wrote:

Hi Stefan, Hi Marek

I got your U-Boot mainline booting on the ESPRESSObin. First Step is done.

Your are right Stefan, the SATA makes kind of Issues. With disabling it 
is booting!


Okay, thanks for testing and reporting. But we should strive to fix
this SATA issue in mainline as well. If you find the time, it would be
great if you (or someone else) could dig into this and send a patch
to fix the real issue.

Thanks,
Stefan


You guys done a great job, the mainline works with that little changes.

It tested for my need:

USB 2.0 and USB 3.0 --- works

If I will do further tests, I will commit.

Changes are:

diff --git a/arch/arm/dts/armada-3720-espressobin.dts 
b/arch/arm/dts/armada-3720-espressobin.dts


index 84e2c2adba..f8701c0e87 100644

--- a/arch/arm/dts/armada-3720-espressobin.dts

+++ b/arch/arm/dts/armada-3720-espressobin.dts

@@ -106,9 +106,9 @@

};

  /* CON3 */

- {

-   status = "okay";

-};

+// {

+// status = "okay";

+//};

   {

     status = "okay";

I would make it clean and suggest a pull request, therefore we have a 
base, everybody can work on.


The working boot-log is:

    DLL 0xc0001074NOTICE:  Booting Trusted Firmware

NOTICE:  BL1: v1.3(release):armada-17.10.8:34247e02

NOTICE:  BL1: Built : 10:52:47, Mar 31 2NOTICE:  BL2: 
v1.3(release):armada-17.10


.8:34247e02

NOTICE:  BL2: Built : 10:52:48, Mar 31 2020

NNOTICE:  BL31: v1.3(release):armada-17.10.8:34247e02

NOTICE:  BL31:

U-Boot 2020.04-rc3-00188-g350c44dfb9-dirty (Apr 03 2020 - 12:28:10 +0200)

DRAM:  512 MiB

Comphy-0: USB3_HOST0    5 Gbps

Comphy-1: PEX0  2.5 Gbps

Comphy-2: SATA0 5 Gbps

PCIE-0: Link down

MMC:

Loading Environment from SPI Flash... SF: Detected w25q32dw with page 
size 256 Bytes, erase size 4 KiB, total 4 MiB


OK

Model: Marvell Armada 3720 Community Board ESPRESSOBin

Net:   No ethernet found.

Hit any key to stop autoboot:  0

Unknown command 'tftp' - try 'help'

Unknown command 'tftp' - try 'help'

Bad Linux ARM64 Image magic!

Thank you very much, have a great weekend,

--

Moritz Berghof

Software Engineer

Tel. +49 30 921028-209 

Fax +49 30 921028-020 

mberg...@phoenixcontact.com 

www.phoenixcontact.com 

PHOENIX CONTACT Cyber Security GmbH

Richard-Willstätter-Straße 6 
D-12489 Berlin 

Register Court: AG Charlottenburg, HR B 202908
Geschäftsführer/General Manager: Kilian Golm

--

On Thu, 2 Apr 2020 14:18:24 +0200

Stefan Roese https://lists.denx.de/listinfo/u-boot>> wrote:


/Hi Moritz,/



//



/On 02.04.20 13:30, Moritz Berghof wrote:/



/> it's really great that you answered so fast and helpfully, thank you!/



/> /



/> It's great you want get the board ported to mainline. Me too./



/> /



/> /



/> I build the U-boot mainline and uploaded on my espressobin. Used the ATF/



/> and WTMI from Marvell./



/> /



/> When I start the flashed .bin file, the U-boot crashed at this following/



/> point. "Synchronous Abort" handler, esr 0x96000210 /



//



/Looks like an issue with SATA - not sure why. You might want to try to/



/disable SATA / AHCI for testing./



//



/> Prompt is attached at this mail./



/> /



/> I think there is a problem with the RAM. For example, I build the U-Boot/



/> for 1 GB RAM with the Marvell U-boot and the mainline/master. Important/



/> constant is DDR_TOPOLOGY=2/



/> /


/> make DEBUG=0 USE_COHERENT_MEM=0 LOG_LEVEL=20 SECURE=0 
CLOCKSPRESET=CPU_1000_DDR_800 DDR_TOPOLOGY=2 WTP=... BOOTDEV=SPINOR 
PARTNUM=0 PLAT=a3700 all fip/



/> /



/> But when the u-boot mainline starts it promts:/



/> U-Boot 2020.04-rc3-00188-g350c44dfb9 (Mar 31 2020 - 10:52:01 +0200)/



/> /



/> DRAM:  512 MiB/



/> /



/> /



/> The Marvell U-boot promts DRAM: 1 GiB/



/> /



/> /



/> /



/> So my question is, where do you define the RAM Size? /



//



/Take a look at arch/arm/mach-mvebu/arm64-common.c. Perhaps this code/



/needs some changes for CONFIG_ARMADA_A3700 similar to what is done/



/for CONFIG_ARMADA_8K ?/



//



/I suggest you debug in this area a bit to see, where it goes wrong./


There is a register on A3720 which should contain information about how

much RAM the system has (0xD200 if the device contains only one

DDR chip). It is written by the TIM image. I am going to send a patch

which uses this register to determine RAM size for Turris Mox.

The thing is that without upgrade of TIM header some Moxes may report

1 GB RAM in this register, although they only have 512 MB. So a call to

get_ram_size will still be needed.

Marek

...

PHOENIX CONTACT Cyber Security GmbH 

Richard-Willstätter-Straße 6  

D-12489 Berlin  

 

Register Court: AG Charlottenburg, HR B 202908 

Geschäftsführer/General Manager: Kilian Golm

//



Viele Grüße,
Stefan

--
DENX Software Engineering GmbH,  

Re: [PATCH v2 1/2] serial: ns16550: Revert "Move PCI access from ofdata_to_platdata() to probe()"

2020-04-03 Thread Simon Glass
Hi Chee Hong,

On Fri, 3 Apr 2020 at 01:56, Ang, Chee Hong  wrote:
>
> > On Fri, Apr 3, 2020 at 6:56 AM Ang, Chee Hong 
> > wrote:
> > >
> > > > On Thu, Apr 2, 2020 at 7:28 PM Ang, Chee Hong
> > > > 
> > > > wrote:
> > > > > > On Thu, Apr 02, 2020 at 12:55:14PM +0800, Bin Meng wrote:
> > > > > > > On Thu, Apr 2, 2020 at 1:55 AM Simon Glass 
> > wrote:
> > > > > > > > On Wed, 1 Apr 2020 at 11:39, Andy Shevchenko
> >
> > > > > > > > > > > > > This reverts commit
> > > > 720f9e1fdb0c92d3fd16e1bfc25bcbd35612675c.
> >
> > > > > > Adding SoCFPGA folks to this thread as the first commit
> > > > > > (82de42fa1468) is also breaking platforms there and then their
> > > > > > fix for that problem is also causing problems, if I follow right.
> > > >
> > > > > Yes. This commit (82de42fa1468) breaks our A10 platform.
> > > > > I did submit similar patch to move some init code from
> > > > > ofdata_to_platdata() to
> > > > probe() for our clock driver as well.
> > > > > Check out the thread here:
> > > > > https://lists.denx.de/pipermail/u-boot/2020-April/405252.html
> > > >
> > > > I see half or less of the messages in the thread by above link.
> > > > Can you summarize what should have been done in order to fix this?
> > > 1) Fix in the driver (broken by this commit: 82de42fa1468) by moving some
> > code from ofdata_to_platdata() to probe().
> > > 2) Fix the DM core.
> > > Simon and Marek were discussing about these 2 options.
> > > Perhaps Simon can help shed some light here.
> >
> > I have a question. Does revert of
> > 720f9e1fdb0c92d3fd16e1bfc25bcbd35612675c makes any difference to
> > SoCFPGA?
> This commit: 82de42fa1468 break our SoCFPGA A10 clock driver.
> This commit 720f9e1fdb0c92d3fd16e1bfc25bcbd35612675c does not affect our 
> SoCFPGA platforms.
> We just want to know which way to go. Fixing our A10 clock driver or fix the 
> DM core.
> Seems like this commit 720f9e1fdb0c92d3fd16e1bfc25bcbd35612675c go with 
> option 1 (fixing the driver).

For now you should fix your clock driver as it is too late to make a
change to the DM core.

For your particular case, running ofdata_to_platdata() on parent
devices might be enough to get your clock driver running, so after the
release we can figure that out.

Regards,
SImon


Pull request: u-boot-spi/master

2020-04-03 Thread Jagan Teki
Hi Tom,

Please pull this PR for the release.

Summary:
- fix for MMIO window size (Tudor Ambarus)

thanks,
Jagan.

The following changes since commit e0718b3ab754860bd47677e6b4fc5b70da42c4ab:

  Merge tag 'dm-pull-1apr20' of git://git.denx.de/u-boot-dm (2020-04-01 
14:29:21 -0400)

are available in the Git repository at:

  https://gitlab.denx.de/u-boot/custodians/u-boot-spi master

for you to fetch changes up to 52e2565bfb5d332e53021c6ec437cdb95eaf9dde:

  spi: atmel-quadspi: Add verbose debug facilities to monitor register accesses 
(2020-04-02 17:17:09 +0530)


Tudor Ambarus (2):
  spi: atmel-quadspi: fix possible MMIO window size overrun
  spi: atmel-quadspi: Add verbose debug facilities to monitor register 
accesses

 drivers/spi/atmel-quadspi.c | 125 +---
 1 file changed, 107 insertions(+), 18 deletions(-)


Re: [PATCH v3 29/29] acpi: Add an acpi command

2020-04-03 Thread Andy Shevchenko
On Tue, Mar 31, 2020 at 07:14:18PM +0100, Leif Lindholm wrote:
> On Mon, Mar 30, 2020 at 17:13:05 -0600, Simon Glass wrote:

> > +static void list_fact(struct acpi_fadt *fadt)
> 
> Hmm, should this function be called list_facp or list_fadt?
> (The wonder that is the table called FADT with the marker FACP.)

Spec refers to some historical reasons.

-- 
With Best Regards,
Andy Shevchenko




Re: [PATCH v3 29/29] acpi: Add an acpi command

2020-04-03 Thread Andy Shevchenko
On Mon, Mar 30, 2020 at 05:13:05PM -0600, Simon Glass wrote:
> It is useful to dump ACPI tables in U-Boot to see what has been generated.
> Add a command to handle this.
> 
> To allow the command to find the tables, add a position into the global
> data.
> 
> Support subcommands to list and dump the tables.

...

> +static void dump_hdr(struct acpi_table_header *hdr)
> +{

> + bool has_hdr = memcmp(hdr->signature, "FACS", ACPI_NAME_LEN);

I believe more than one table has the same header structure. Either this
function is incorrectly called (should be dump_facs_hdr() or alike), or
you need to make it better, i.e. generic.

> + printf("%.*s %08lx %06x", ACPI_NAME_LEN, hdr->signature,
> +(ulong)map_to_sysmem(hdr), hdr->length);
> + if (has_hdr) {
> + printf(" (v%02d %.6s %.8s %u %.4s %d)\n", hdr->revision,
> +hdr->oem_id, hdr->oem_table_id, hdr->oem_revision,
> +hdr->aslc_id, hdr->aslc_revision);
> + } else {
> + printf("\n");
> + }
> +}

-- 
With Best Regards,
Andy Shevchenko




Re: [PATCH v3 27/29] acpi: Put table-setup code in its own function

2020-04-03 Thread Andy Shevchenko
On Mon, Mar 30, 2020 at 05:13:03PM -0600, Simon Glass wrote:
> We always write three basic tables to ACPI at the start. Move this into
> its own function, along with acpi_fill_header(), so we can write a test
> for this code.

...

>   /* Re-calculate checksum */
>   rsdt->header.checksum = 0;
> - rsdt->header.checksum = table_compute_checksum((u8 *)rsdt,
> + rsdt->header.checksum = table_compute_checksum(rsdt,
>  rsdt->header.length);

Why suddenly casting is not needed in this patch?
Same question to the rest.

(If it's a valid change, it should be in a separate patch)

-- 
With Best Regards,
Andy Shevchenko




Re: [PATCH v3 23/29] acpi: Convert part of acpi_table to use acpi_ctx

2020-04-03 Thread Andy Shevchenko
On Fri, Apr 03, 2020 at 04:24:06PM +0300, Andy Shevchenko wrote:
> On Mon, Mar 30, 2020 at 05:12:59PM -0600, Simon Glass wrote:
> > The current code uses an address but a pointer would result in fewer
> > casts. Also it repeats the alignment code in a lot of places so this would
> > be better done in a helper function.
> > 
> > Update write_acpi_tables() to make use of the new acpi_ctx structure,
> > adding a few helpers to clean things up.

...

> > +void acpi_inc(struct acpi_ctx *ctx, uint amount);
> 
> inc with amount is not inc, it's rather add.

...or pad if you wish.

> 
> > + * acpi_inc_align() - Increment the ACPI output pointer by a bit and align

-- 
With Best Regards,
Andy Shevchenko




Re: [PATCH v3 23/29] acpi: Convert part of acpi_table to use acpi_ctx

2020-04-03 Thread Andy Shevchenko
On Mon, Mar 30, 2020 at 05:12:59PM -0600, Simon Glass wrote:
> The current code uses an address but a pointer would result in fewer
> casts. Also it repeats the alignment code in a lot of places so this would
> be better done in a helper function.
> 
> Update write_acpi_tables() to make use of the new acpi_ctx structure,
> adding a few helpers to clean things up.

...

> +void acpi_align(struct acpi_ctx *ctx);

> +void acpi_align64(struct acpi_ctx *ctx);

In the code, it will be not understandable the difference.
align without number would be good if the function only single one.

So, align16() much better.

> +void acpi_inc(struct acpi_ctx *ctx, uint amount);

inc with amount is not inc, it's rather add.

> + * acpi_inc_align() - Increment the ACPI output pointer by a bit and align

Align how?

> +void acpi_inc_align(struct acpi_ctx *ctx, uint amount);

-- 
With Best Regards,
Andy Shevchenko




Re: [PATCH v3 22/29] acpi: Add a method to write tables for a device

2020-04-03 Thread Andy Shevchenko
On Mon, Mar 30, 2020 at 05:12:58PM -0600, Simon Glass wrote:
> A device may want to write out ACPI tables to describe itself to Linux.
> Add a method to permit this.

> +acpi_method acpi_get_method(struct udevice *dev, enum method_t method)
> +{
> + struct acpi_ops *aops;
> +
> + aops = device_get_acpi_ops(dev);
> + if (aops) {
> + switch (method) {
> + case METHOD_WRITE_TABLES:
> + return aops->write_tables;
> + }

Where is default?

> + }
> +
> + return NULL;

Perhaps,

if (!aops)
return NULL;

switch (method) {
...
default:
return NULL;
}

> +}

...

> + log_debug("\n- %s %p\n", parent->name, func);

Leading '\n' in the messages is not good idea.
It might work nicely in U-Boot, but in general better to avoid.


-- 
With Best Regards,
Andy Shevchenko




Re: [PATCH v3 20/29] acpi: Add support for DMAR

2020-04-03 Thread Andy Shevchenko
On Mon, Mar 30, 2020 at 05:12:56PM -0600, Simon Glass wrote:
> The DMA Remapping Reporting (DMAR) table contains information about DMA
> remapping.
> 
> Add a version simple version of this table with only the minimum fields
> filled out. i.e. no entries.

> +/* TODO(s...@chromium.org): Figure out how to get compiler revision */
> +#define ASL_REVISION 0

Why do you need this?

I don't see any users here either.

-- 
With Best Regards,
Andy Shevchenko




Re: [PATCH v2] serial: ns16550: Fix ordering of getting base address

2020-04-03 Thread Andy Shevchenko
On Fri, Apr 03, 2020 at 05:46:19AM -0700, Bin Meng wrote:
> Currently the driver gets ns16550 base address in the driver
> probe() routine, which may potentially break any ns16550 wrapper
> driver that does additional initialization before calling
> ns16550_serial_probe().
> 
> Things are complicated that we need consider ns16550 devices on
> both simple-bus and PCI bus. To fix the issue we move the base
> address assignment for simple-bus ns16550 device back to the
> ofdata_to_platdata(), and assign base address for PCI ns16550
> device in ns16550_serial_probe().
> 
> This is still not perfect. Ideally if any PCI bus based ns16550
> wrapper driver tries to access plat->base before calling probe(),
> it is subject to break.
> 
> Fixes: 720f9e1fdb0c9 ("serial: ns16550: Move PCI access from 
> ofdata_to_platdata() to probe()")
> Reported-by: Andy Shevchenko 
> Signed-off-by: Bin Meng 
> Tested-by: Andy Shevchenko 
> Reviewed-by: Wolfgang Wallner 
> Tested-by: Wolfgang Wallner 

...

> - if (addr == FDT_ADDR_T_NONE)
> - return -EINVAL;
> -
> -#ifdef CONFIG_SYS_NS16550_PORT_MAPPED
> - plat->base = addr;
> -#else
> - plat->base = (unsigned long)map_physmem(addr, 0, MAP_NOCACHE);
> -#endif
> -
> - return 0;

(1) for below.

...


> + addr = devfdt_get_addr_pci(dev);

> + if (addr == FDT_ADDR_T_NONE)
> + return -EINVAL;
> +#ifdef CONFIG_SYS_NS16550_PORT_MAPPED
> + plat->base = addr;
> +#else
> + plat->base = (unsigned long)map_physmem(addr, 0, MAP_NOCACHE);
>  #endif


And now is the question why we can't use above as a helper in both cases
(either with check for address correctness or without)?

> + }

...

> + addr = dev_read_addr(dev);
> + if (addr != FDT_ADDR_T_NONE) {
> +#ifdef CONFIG_SYS_NS16550_PORT_MAPPED
> + plat->base = addr;
> +#else
> + plat->base = (unsigned long)map_physmem(addr, 0, MAP_NOCACHE);
> +#endif
> + } else if (!device_is_on_pci_bus(dev)) {
> + return -EINVAL;
> + }

Something like

addr = dev_read_addr(dev);
ret = ns16550_assign_base(addr, plat);
if (ret && !device_is_on_pci_bus(...))
return ret;

-- 
With Best Regards,
Andy Shevchenko




Re: [PATCH v3 19/29] acpi: Add a central location for table version numbers

2020-04-03 Thread Andy Shevchenko
On Mon, Mar 30, 2020 at 05:12:55PM -0600, Simon Glass wrote:
> Each ACPI table has its own version number. Add the version numbers in a
> single function so we can keep them consistent and easily see what
> versions are supported.
> 
> Start a new acpi_table file in a generic directory to house this function.
> We can move things over to this file from x86 as needed.

...

> +/* FADT TABLE Revision values */
> +#define ACPI_FADT_REV_ACPI_1_0   1
> +#define ACPI_FADT_REV_ACPI_2_0   3
> +#define ACPI_FADT_REV_ACPI_3_0   4
> +#define ACPI_FADT_REV_ACPI_4_0   4
> +#define ACPI_FADT_REV_ACPI_5_0   5
> +#define ACPI_FADT_REV_ACPI_6_0   6
> +
> +/* MADT TABLE Revision values */
> +#define ACPI_MADT_REV_ACPI_3_0   2
> +#define ACPI_MADT_REV_ACPI_4_0   3
> +#define ACPI_MADT_REV_ACPI_5_0   3
> +#define ACPI_MADT_REV_ACPI_6_0   5

Is it for real this fancy numbering? I don't remember spec by heart, perhaps
this needs to be elaborated in the comment.

...

> +/* Tables defined by ACPI and generated by U-Boot */
> +enum acpi_tables {

> + ACPITAB_BERT,
> + ACPITAB_DBG2,
> + ACPITAB_DMAR,
> + ACPITAB_DSDT,
> + ACPITAB_FACS,
> + ACPITAB_FADT,
> + ACPITAB_HEST,
> + ACPITAB_HPET,
> + ACPITAB_IVRS,
> + ACPITAB_MADT,
> + ACPITAB_MCFG,
> + ACPITAB_RSDP,
> + ACPITAB_RSDT,
> + ACPITAB_SLIT,
> + ACPITAB_SRAT,
> + ACPITAB_SSDT,
> + ACPITAB_TCPA,
> + ACPITAB_TPM2,
> + ACPITAB_XSDT,
> + ACPITAB_ECDT,

This should be cleaned according to uefi.org, i.e. not all above tables are
from ACPI spec. And thus, should be rather in the below section.

> +
> + /* Additional proprietary tables */
> + ACPITAB_VFCT,
> + ACPITAB_NHLT,
> + ACPITAB_SPMI,

Where is SPCR?

> +
> + ACPITAB_COUNT,
> +};

-- 
With Best Regards,
Andy Shevchenko




Antwort: [PATCH v2] serial: ns16550: Fix ordering of getting base address

2020-04-03 Thread Wolfgang Wallner
Hi Bin,

-"Bin Meng"  schrieb: -
>Betreff: [PATCH v2] serial: ns16550: Fix ordering of getting base
>address
>
>Currently the driver gets ns16550 base address in the driver
>probe() routine, which may potentially break any ns16550 wrapper
>driver that does additional initialization before calling
>ns16550_serial_probe().
>
>Things are complicated that we need consider ns16550 devices on
>both simple-bus and PCI bus. To fix the issue we move the base
>address assignment for simple-bus ns16550 device back to the
>ofdata_to_platdata(), and assign base address for PCI ns16550
>device in ns16550_serial_probe().
>
>This is still not perfect. Ideally if any PCI bus based ns16550
>wrapper driver tries to access plat->base before calling probe(),
>it is subject to break.
>
>Fixes: 720f9e1fdb0c9 ("serial: ns16550: Move PCI access from
>ofdata_to_platdata() to probe()")
>Reported-by: Andy Shevchenko 
>Signed-off-by: Bin Meng 
>Tested-by: Andy Shevchenko 
>Reviewed-by: Wolfgang Wallner 
>Tested-by: Wolfgang Wallner 
>
>---
>
>Changes in v2:
>- not to break Fixes, etc to two or more lines in the commit message
>- add the same CONFIG_SYS_NS16550_PORT_MAPPED ifdefs in the PCI case
>
> drivers/serial/ns16550.c | 53
>
> 1 file changed, 27 insertions(+), 26 deletions(-)

I have also tested v2, just to be sure.
Works as expected.

regards, Wolfgang



Re: [PATCH v3 17/29] x86: Move acpi_table header to main include/ directory

2020-04-03 Thread Andy Shevchenko
On Mon, Mar 30, 2020 at 05:12:53PM -0600, Simon Glass wrote:
> This file is potentially useful to other architectures saddled with ACPI
> so move most of its contents to a common location.

It's not just potentially, it's definitely useful.
But this makes me think, why we don't incorporate ACPICA headers as is?

> 
> Signed-off-by: Simon Glass 
> Reviewed-by: Bin Meng 
> Reviewed-by: Wolfgang Wallner 
> ---
> 
> Changes in v3:
> - Add forward declarations for the functions
> - Move acpi_table.h to include/acpi
> - Update commit message to say that we move most of its contents
> 
> Changes in v2: None
> 
>  arch/x86/cpu/baytrail/acpi.c  |   2 +-
>  arch/x86/cpu/cpu.c|   2 +-
>  arch/x86/cpu/quark/acpi.c |   2 +-
>  arch/x86/cpu/tangier/acpi.c   |   4 +-
>  arch/x86/include/asm/acpi_table.h | 381 +
>  arch/x86/lib/acpi.c   |   2 +-
>  arch/x86/lib/acpi_s3.c|   2 +-
>  arch/x86/lib/acpi_table.c |   2 +-
>  arch/x86/lib/tables.c |   2 +-
>  arch/x86/lib/zimage.c |   2 +-
>  include/acpi/acpi_table.h | 394 ++
>  lib/efi_loader/efi_acpi.c |   2 +-
>  12 files changed, 412 insertions(+), 385 deletions(-)
>  create mode 100644 include/acpi/acpi_table.h
> 
> diff --git a/arch/x86/cpu/baytrail/acpi.c b/arch/x86/cpu/baytrail/acpi.c
> index 3c27391873c..57723109796 100644
> --- a/arch/x86/cpu/baytrail/acpi.c
> +++ b/arch/x86/cpu/baytrail/acpi.c
> @@ -7,7 +7,7 @@
>  #include 
>  #include 
>  #include 
> -#include 
> +#include 
>  #include 
>  #include 
>  #include 
> diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c
> index 246ee50948c..cec04b481b9 100644
> --- a/arch/x86/cpu/cpu.c
> +++ b/arch/x86/cpu/cpu.c
> @@ -27,8 +27,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> diff --git a/arch/x86/cpu/quark/acpi.c b/arch/x86/cpu/quark/acpi.c
> index 7b6fc2f4a53..26cda3b3376 100644
> --- a/arch/x86/cpu/quark/acpi.c
> +++ b/arch/x86/cpu/quark/acpi.c
> @@ -4,7 +4,7 @@
>   */
>  
>  #include 
> -#include 
> +#include 
>  #include 
>  #include 
>  #include 
> diff --git a/arch/x86/cpu/tangier/acpi.c b/arch/x86/cpu/tangier/acpi.c
> index 8b128138b0d..4ec8fdd6f89 100644
> --- a/arch/x86/cpu/tangier/acpi.c
> +++ b/arch/x86/cpu/tangier/acpi.c
> @@ -8,13 +8,13 @@
>  #include 
>  #include 
>  #include 
> -#include 
> -#include 
> +#include 
>  #include 
>  #include 
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  void acpi_create_fadt(struct acpi_fadt *fadt, struct acpi_facs *facs,
> void *dsdt)
> diff --git a/arch/x86/include/asm/acpi_table.h 
> b/arch/x86/include/asm/acpi_table.h
> index 7588913f937..928475cef4e 100644
> --- a/arch/x86/include/asm/acpi_table.h
> +++ b/arch/x86/include/asm/acpi_table.h
> @@ -9,381 +9,14 @@
>  #ifndef __ASM_ACPI_TABLE_H__
>  #define __ASM_ACPI_TABLE_H__
>  
> -#define RSDP_SIG "RSD PTR "  /* RSDP pointer signature */
> -#define OEM_ID   "U-BOOT"/* U-Boot */
> -#define OEM_TABLE_ID "U-BOOTBL"  /* U-Boot Table */
> -#define ASLC_ID  "INTL"  /* Intel ASL Compiler */
> -
> -#define ACPI_RSDP_REV_ACPI_1_0   0
> -#define ACPI_RSDP_REV_ACPI_2_0   2
> -
> -/*
> - * RSDP (Root System Description Pointer)
> - * Note: ACPI 1.0 didn't have length, xsdt_address, and ext_checksum
> - */
> -struct acpi_rsdp {
> - char signature[8];  /* RSDP signature */
> - u8 checksum;/* Checksum of the first 20 bytes */
> - char oem_id[6]; /* OEM ID */
> - u8 revision;/* 0 for ACPI 1.0, others 2 */
> - u32 rsdt_address;   /* Physical address of RSDT (32 bits) */
> - u32 length; /* Total RSDP length (incl. extended part) */
> - u64 xsdt_address;   /* Physical address of XSDT (64 bits) */
> - u8 ext_checksum;/* Checksum of the whole table */
> - u8 reserved[3];
> -};
> -
> -/* Generic ACPI header, provided by (almost) all tables */
> -struct __packed acpi_table_header {
> - char signature[4];  /* ACPI signature (4 ASCII characters) */
> - u32 length; /* Table length in bytes (incl. header) */
> - u8 revision;/* Table version (not ACPI version!) */
> - volatile u8 checksum;   /* To make sum of entire table == 0 */
> - char oem_id[6]; /* OEM identification */
> - char oem_table_id[8];   /* OEM table identification */
> - u32 oem_revision;   /* OEM revision number */
> - char aslc_id[4];/* ASL compiler vendor ID */
> - u32 aslc_revision;  /* ASL compiler revision number */
> -};
> -
> -/* A maximum number of 32 ACPI tables ought to be enough for now */
> -#define MAX_ACPI_TABLES  32
> -
> -/* RSDT (Root System Description Table) */
> -struct acpi_rsdt {
> - struct 

Re: [PATCH v3 16/29] x86: Move acpi_s3.h to include/acpi/

2020-04-03 Thread Andy Shevchenko
On Mon, Mar 30, 2020 at 05:12:52PM -0600, Simon Glass wrote:
> This header relates to ACPI and we are about to add some more ACPI
> headers. Move this one into a new directory so they are together.
> 

FWIW,
Reviewed-by: Andy Shevchenko 

One nit below.

> Signed-off-by: Simon Glass 
> ---
> 
> Changes in v3:
> - Add new patch to move acpi_s3.h to include/acpi/
> 
> Changes in v2: None
> 
>  arch/x86/cpu/apollolake/cpu_spl.c| 2 +-
>  arch/x86/cpu/apollolake/fsp_s.c  | 2 +-
>  arch/x86/cpu/apollolake/pmc.c| 2 +-
>  arch/x86/cpu/baytrail/acpi.c | 4 ++--
>  arch/x86/cpu/cpu.c   | 2 +-
>  arch/x86/cpu/wakeup.S| 2 +-
>  arch/x86/lib/acpi_s3.c   | 2 +-
>  arch/x86/lib/coreboot_table.c| 2 +-
>  arch/x86/lib/fsp/fsp_common.c| 2 +-
>  arch/x86/lib/fsp1/fsp_common.c   | 2 +-
>  arch/x86/lib/fsp2/fsp_dram.c | 2 +-
>  drivers/pci/pci_rom.c| 4 +---
>  drivers/power/acpi_pmc/acpi-pmc-uclass.c | 2 +-
>  drivers/sysreset/sysreset_x86.c  | 2 +-
>  include/{ => acpi}/acpi_s3.h | 0
>  15 files changed, 15 insertions(+), 17 deletions(-)
>  rename include/{ => acpi}/acpi_s3.h (100%)
> 
> diff --git a/arch/x86/cpu/apollolake/cpu_spl.c 
> b/arch/x86/cpu/apollolake/cpu_spl.c
> index 8a39c3128e0..e2509e391fa 100644
> --- a/arch/x86/cpu/apollolake/cpu_spl.c
> +++ b/arch/x86/cpu/apollolake/cpu_spl.c
> @@ -6,13 +6,13 @@
>   */
>  
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> diff --git a/arch/x86/cpu/apollolake/fsp_s.c b/arch/x86/cpu/apollolake/fsp_s.c
> index 1f22c1ea3c6..17cf1682ad0 100644
> --- a/arch/x86/cpu/apollolake/fsp_s.c
> +++ b/arch/x86/cpu/apollolake/fsp_s.c
> @@ -5,11 +5,11 @@
>   */
>  
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> diff --git a/arch/x86/cpu/apollolake/pmc.c b/arch/x86/cpu/apollolake/pmc.c
> index aec0c8394c2..4ea7c7447bc 100644
> --- a/arch/x86/cpu/apollolake/pmc.c
> +++ b/arch/x86/cpu/apollolake/pmc.c
> @@ -9,10 +9,10 @@
>  #define LOG_CATEGORY UCLASS_ACPI_PMC
>  
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> diff --git a/arch/x86/cpu/baytrail/acpi.c b/arch/x86/cpu/baytrail/acpi.c
> index f44228e6939..3c27391873c 100644
> --- a/arch/x86/cpu/baytrail/acpi.c
> +++ b/arch/x86/cpu/baytrail/acpi.c
> @@ -4,15 +4,15 @@
>   */
>  
>  #include 
> -#include 
>  #include 
>  #include 
> -#include 
> +#include 
>  #include 
>  #include 
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  void acpi_create_fadt(struct acpi_fadt *fadt, struct acpi_facs *facs,
> void *dsdt)
> diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c
> index dae06949cc6..246ee50948c 100644
> --- a/arch/x86/cpu/cpu.c
> +++ b/arch/x86/cpu/cpu.c
> @@ -19,7 +19,6 @@
>   */
>  
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> @@ -27,6 +26,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> diff --git a/arch/x86/cpu/wakeup.S b/arch/x86/cpu/wakeup.S
> index 244ca1276af..093bf3bcc5c 100644
> --- a/arch/x86/cpu/wakeup.S
> +++ b/arch/x86/cpu/wakeup.S
> @@ -5,7 +5,7 @@
>   * From coreboot src/arch/x86/wakeup.S
>   */
>  
> -#include 
> +#include 
>  #include 
>  #include 
>  
> diff --git a/arch/x86/lib/acpi_s3.c b/arch/x86/lib/acpi_s3.c
> index 197636c4b50..c3759ec8492 100644
> --- a/arch/x86/lib/acpi_s3.c
> +++ b/arch/x86/lib/acpi_s3.c
> @@ -4,7 +4,7 @@
>   */
>  
>  #include 
> -#include 
> +#include 
>  #include 
>  #include 
>  #include 
> diff --git a/arch/x86/lib/coreboot_table.c b/arch/x86/lib/coreboot_table.c
> index 2943e11d2a4..c996fc588df 100644
> --- a/arch/x86/lib/coreboot_table.c
> +++ b/arch/x86/lib/coreboot_table.c
> @@ -4,9 +4,9 @@
>   */
>  
>  #include 
> -#include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> diff --git a/arch/x86/lib/fsp/fsp_common.c b/arch/x86/lib/fsp/fsp_common.c
> index 5eff0f99aad..267527eb344 100644
> --- a/arch/x86/lib/fsp/fsp_common.c
> +++ b/arch/x86/lib/fsp/fsp_common.c
> @@ -4,11 +4,11 @@
>   */
>  
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> diff --git a/arch/x86/lib/fsp1/fsp_common.c b/arch/x86/lib/fsp1/fsp_common.c
> index aee2a05044f..0a726807c2b 100644
> --- a/arch/x86/lib/fsp1/fsp_common.c
> +++ b/arch/x86/lib/fsp1/fsp_common.c
> @@ -4,11 +4,11 @@
>   */
>  
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> diff --git a/arch/x86/lib/fsp2/fsp_dram.c b/arch/x86/lib/fsp2/fsp_dram.c
> index 90a238a2245..c8f2c09b6a7 100644
> --- a/arch/x86/lib/fsp2/fsp_dram.c
> +++ 

Re: [PATCH v3 15/29] acpi: Add a simple sandbox test

2020-04-03 Thread Andy Shevchenko
On Mon, Mar 30, 2020 at 05:12:51PM -0600, Simon Glass wrote:
> Add a sandbox test for the basic ACPI functionality we have so far.

> +U_BOOT_DRIVER(testacpi_drv) = {
> + .name   = "testacpi_drv",
> + .of_match   = testacpi_ids,
> + .id = UCLASS_TEST_ACPI,

> + acpi_ops_ptr(_ops)

I have noticed that this is not obvious why no comma here.
Perhaps, since apci_ops_ptr is a macro, you should upper case it.

> +};

-- 
With Best Regards,
Andy Shevchenko




[PATCH v2] serial: ns16550: Fix ordering of getting base address

2020-04-03 Thread Bin Meng
Currently the driver gets ns16550 base address in the driver
probe() routine, which may potentially break any ns16550 wrapper
driver that does additional initialization before calling
ns16550_serial_probe().

Things are complicated that we need consider ns16550 devices on
both simple-bus and PCI bus. To fix the issue we move the base
address assignment for simple-bus ns16550 device back to the
ofdata_to_platdata(), and assign base address for PCI ns16550
device in ns16550_serial_probe().

This is still not perfect. Ideally if any PCI bus based ns16550
wrapper driver tries to access plat->base before calling probe(),
it is subject to break.

Fixes: 720f9e1fdb0c9 ("serial: ns16550: Move PCI access from 
ofdata_to_platdata() to probe()")
Reported-by: Andy Shevchenko 
Signed-off-by: Bin Meng 
Tested-by: Andy Shevchenko 
Reviewed-by: Wolfgang Wallner 
Tested-by: Wolfgang Wallner 

---

Changes in v2:
- not to break Fixes, etc to two or more lines in the commit message
- add the same CONFIG_SYS_NS16550_PORT_MAPPED ifdefs in the PCI case

 drivers/serial/ns16550.c | 53 
 1 file changed, 27 insertions(+), 26 deletions(-)

diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
index c1b303f..cb464a5 100644
--- a/drivers/serial/ns16550.c
+++ b/drivers/serial/ns16550.c
@@ -479,39 +479,28 @@ static int ns16550_serial_getinfo(struct udevice *dev,
return 0;
 }
 
-#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
-static int ns1655_serial_set_base_addr(struct udevice *dev)
-{
-   fdt_addr_t addr;
-   struct ns16550_platdata *plat;
-
-   plat = dev_get_platdata(dev);
-
-   addr = dev_read_addr_pci(dev);
-   if (addr == FDT_ADDR_T_NONE)
-   return -EINVAL;
-
-#ifdef CONFIG_SYS_NS16550_PORT_MAPPED
-   plat->base = addr;
-#else
-   plat->base = (unsigned long)map_physmem(addr, 0, MAP_NOCACHE);
-#endif
-
-   return 0;
-}
-#endif
-
 int ns16550_serial_probe(struct udevice *dev)
 {
+   struct ns16550_platdata *plat = dev->platdata;
struct NS16550 *const com_port = dev_get_priv(dev);
struct reset_ctl_bulk reset_bulk;
+   fdt_addr_t addr;
int ret;
 
-#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
-   ret = ns1655_serial_set_base_addr(dev);
-   if (ret)
-   return ret;
+   /*
+* If we are on PCI bus, either directly attached to a PCI root port,
+* or via a PCI bridge, assign platdata->base before probing hardware.
+*/
+   if (device_is_on_pci_bus(dev)) {
+   addr = devfdt_get_addr_pci(dev);
+   if (addr == FDT_ADDR_T_NONE)
+   return -EINVAL;
+#ifdef CONFIG_SYS_NS16550_PORT_MAPPED
+   plat->base = addr;
+#else
+   plat->base = (unsigned long)map_physmem(addr, 0, MAP_NOCACHE);
 #endif
+   }
 
ret = reset_get_bulk(dev, _bulk);
if (!ret)
@@ -535,9 +524,21 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
 {
struct ns16550_platdata *plat = dev->platdata;
const u32 port_type = dev_get_driver_data(dev);
+   fdt_addr_t addr;
struct clk clk;
int err;
 
+   addr = dev_read_addr(dev);
+   if (addr != FDT_ADDR_T_NONE) {
+#ifdef CONFIG_SYS_NS16550_PORT_MAPPED
+   plat->base = addr;
+#else
+   plat->base = (unsigned long)map_physmem(addr, 0, MAP_NOCACHE);
+#endif
+   } else if (!device_is_on_pci_bus(dev)) {
+   return -EINVAL;
+   }
+
plat->reg_offset = dev_read_u32_default(dev, "reg-offset", 0);
plat->reg_shift = dev_read_u32_default(dev, "reg-shift", 0);
plat->reg_width = dev_read_u32_default(dev, "reg-io-width", 1);
-- 
2.7.4



Re: [PATCH v3 14/29] acpi: Add a binding for ACPI settings in the device tree

2020-04-03 Thread Andy Shevchenko
On Mon, Mar 30, 2020 at 05:12:50PM -0600, Simon Glass wrote:
> Devices need to report various identifiers in the ACPI tables. Rather than
> hard-coding these in drivers it is typically better to put them in the
> device tree.
> 
> Add a binding file to describe this.

> +elan_touchscreen: elan-touchscreen@10 {
> + compatible = "i2c-chip";
> + reg = <0x10>;
> + acpi,hid = "ELAN0001";
> + acpi,ddn = "ELAN Touchscreen";

> + interrupts-extended = <_gpe GPIO_21_IRQ
> + IRQ_TYPE_EDGE_FALLING>;

Can we have this on one line?

> + acpi,probed;
> +};

-- 
With Best Regards,
Andy Shevchenko




Re: [PATCH] serial: ns16550: Fix ordering of getting base address

2020-04-03 Thread Bin Meng
Hi Wolfgang,

On Fri, Apr 3, 2020 at 7:47 PM Wolfgang Wallner
 wrote:
>
> Hi Bin,
>
> Thanks for taking care of this!
>
> -"Bin Meng"  schrieb: -
>
> >An: "Simon Glass" , "Tom Rini"
> >, "Andy Shevchenko"
> >, "Wolfgang Wallner"
> >, "Chee Hong Ang"
> >, "U-Boot Mailing List"
> >
> >Von: "Bin Meng" 
> >Datum: 03.04.2020 11:58
> >Betreff: [PATCH] serial: ns16550: Fix ordering of getting base
> >address
> >
> >Currently the driver gets ns16550 base address in the driver
> >probe() routine, which may potentially break any ns16550 wrapper
> >driver that does additional initialization before calling
> >ns16550_serial_probe().
> >
> >Things are complicated that we need consider ns16550 devices on
> >both simple-bus and PCI bus. To fix the issue we move the base
> >address assignment for simple-bus ns16550 device back to the
> >ofdata_to_platdata(), and assign base address for PCI ns16550
> >device in ns16550_serial_probe().
> >
> >This is still not perfect. Ideally if any PCI bus based ns16550
> >wrapper driver tries to access plat->base before calling probe(),
> >it is subject to break.
> >
> >Fixes: 720f9e1fdb0c9 ("serial: ns16550: Move PCI access from
> >ofdata_to_platdata() to probe()")
> >Reported-by: Andy Shevchenko 
> >Signed-off-by: Bin Meng 
> >
> >---
> >
> > drivers/serial/ns16550.c | 51
> >+++-
> > 1 file changed, 24 insertions(+), 27 deletions(-)
> >
> >diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
> >index c1b303f..5e3cd1c 100644
> >--- a/drivers/serial/ns16550.c
> >+++ b/drivers/serial/ns16550.c
> >@@ -479,39 +479,24 @@ static int ns16550_serial_getinfo(struct
> >udevice *dev,
> >   return 0;
> > }
> >
> >-#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
> >-static int ns1655_serial_set_base_addr(struct udevice *dev)
> >-{
> >-  fdt_addr_t addr;
> >-  struct ns16550_platdata *plat;
> >-
> >-  plat = dev_get_platdata(dev);
> >-
> >-  addr = dev_read_addr_pci(dev);
> >-  if (addr == FDT_ADDR_T_NONE)
> >-  return -EINVAL;
> >-
> >-#ifdef CONFIG_SYS_NS16550_PORT_MAPPED
> >-  plat->base = addr;
> >-#else
> >-  plat->base = (unsigned long)map_physmem(addr, 0, MAP_NOCACHE);
> >-#endif
> >-
> >-  return 0;
> >-}
> >-#endif
> >-
> > int ns16550_serial_probe(struct udevice *dev)
> > {
> >+  struct ns16550_platdata *plat = dev->platdata;
> >   struct NS16550 *const com_port = dev_get_priv(dev);
> >   struct reset_ctl_bulk reset_bulk;
> >+  fdt_addr_t addr;
> >   int ret;
> >
> >-#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
> >-  ret = ns1655_serial_set_base_addr(dev);
> >-  if (ret)
> >-  return ret;
> >-#endif
> >+  /*
> >+   * If we are on PCI bus, either directly attached to a PCI root
> >port,
> >+   * or via a PCI bridge, assign platdata->base before probing
> >hardware.
> >+   */
> >+  if (device_is_on_pci_bus(dev)) {
> >+  addr = devfdt_get_addr_pci(dev);
> >+  if (addr == FDT_ADDR_T_NONE)
> >+  return -EINVAL;
> >+  plat->base = addr;
>
> The assignment here to plat->base does not distinguish any more based on
> CONFIG_SYS_NS16550_PORT_MAPPED. Is it not necessary any more in this case?
>

Yep, you are right. Both two cases should be handled consistently. I
will spin a v2.

> >+  }
> >
> >   ret = reset_get_bulk(dev, _bulk);
> >   if (!ret)
> >@@ -535,9 +520,21 @@ int ns16550_serial_ofdata_to_platdata(struct
> >udevice *dev)
> > {
> >   struct ns16550_platdata *plat = dev->platdata;
> >   const u32 port_type = dev_get_driver_data(dev);
> >+  fdt_addr_t addr;
> >   struct clk clk;
> >   int err;
> >
> >+  addr = dev_read_addr(dev);
> >+  if (addr != FDT_ADDR_T_NONE) {
> >+#ifdef CONFIG_SYS_NS16550_PORT_MAPPED
> >+  plat->base = addr;
> >+#else
> >+  plat->base = (unsigned long)map_physmem(addr, 0, MAP_NOCACHE);
> >+#endif
> >+  } else if (!device_is_on_pci_bus(dev)) {
> >+  return -EINVAL;
> >+  }
> >+
> >   plat->reg_offset = dev_read_u32_default(dev, "reg-offset", 0);
> >   plat->reg_shift = dev_read_u32_default(dev, "reg-shift", 0);
> >   plat->reg_width = dev_read_u32_default(dev, "reg-io-width", 1);
> >--
> >2.7.4
> >
> >
>
> Tested-by: Wolfgang Wallner 
> Tested on an Intel Apollo Lake based board booting with Coreboot and using
> U-Boot as payload.
>

Thanks for testing!

> Reviewed-by: Wolfgang Wallner 
>

Regards,
Bin


Re: [PATCH v1 1/2] clk: socfpga: Read the clock parent's register base in probe function

2020-04-03 Thread Tom Rini
On Fri, Apr 03, 2020 at 03:52:54AM +, Tan, Ley Foon wrote:
> 
> 
> > -Original Message-
> > From: Marek Vasut 
> > Sent: Friday, April 3, 2020 6:47 AM
> > To: Tom Rini 
> > Cc: Simon Glass ; Ang, Chee Hong
> > ; U-Boot Mailing List ;
> > Simon Goldschmidt ; See, Chin Liang
> > ; Tan, Ley Foon ;
> > Westergreen, Dalon 
> > Subject: Re: [PATCH v1 1/2] clk: socfpga: Read the clock parent's register
> > base in probe function
> > 
> > On 4/3/20 12:10 AM, Tom Rini wrote:
> > > On Thu, Apr 02, 2020 at 11:07:31PM +0200, Marek Vasut wrote:
> > >> On 4/2/20 10:54 PM, Tom Rini wrote:
> > >> [...]
> > >>
> >  I'm not sure it definitely should be changed. But I'll do a
> >  patch and see how it looks.
> > >>>
> > >>> Do I understand it correctly that the patch
> > >>> 82de42fa14682d408da935adfb0f935354c5008f actually completely
> > >>> breaks SoCFPGA ? Then I would say this is a release blocker ?
> > >> Yes. A10 SPL won't boot at all. It crashes during the clock manager
> > setup.
> > >
> > > This came in right at the beginning of the cycle. I thought the
> > > purpose of the 3-month cycle was to allow time to test?
> > 
> >  It was ... altera ?
> > >>>
> > >>> Sorry, I'm missing how that's an answer to the question.  This came
> > >>> in basically right at the start of the merge window.
> > >>
> > >> I don't have an A10 available right now, so what can I do ?
> > >
> > > Ah, so the answer is "I can't test this platform myself".  That's what
> > > then, thanks.
> > 
> > Clearly Altera can , since they reported this issue.
> Yes, we have Arria10 devkit here and we see the issue when testing latest 
> uboot.
> It looks like other platform [1] also encounter issue with patch 
> 82de42fa14682d40. Like Marek said, we don't know any other driver is broken 
> after this patch.
> 
> [1] https://patchwork.ozlabs.org/patch/1265188/ 

Yes, my concern here is that the platform in question wasn't tested
after -rc1 or -rc2 or -rc3 only right now just before release.  So we're
in a bit of a scramble to fix the driver.  Thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH v4 10/13] xhci: mediatek: Add support for MTK xHCI host controller

2020-04-03 Thread Marek Vasut
On 4/3/20 5:33 AM, Chunfeng Yun wrote:
[...]
> +static int xhci_mtk_ofdata_get(struct mtk_xhci *mtk)
> +{
> + struct udevice *dev = mtk->dev;
> + int ret = 0;
> +
> + mtk->hcd = devfdt_remap_addr_name(dev, "mac");
> + if (!mtk->hcd) {
> + dev_err(dev, "Failed to get xHCI base address\n");
> + return -ENXIO;
> + }
> +
> + mtk->ippc = devfdt_remap_addr_name(dev, "ippc");
> + if (!mtk->ippc) {
> + dev_err(dev, "Failed to get IPPC base address\n");
> + return -ENXIO;
> + }
> +
> + dev_info(dev, "hcd: 0x%p, ippc: 0x%p\n", mtk->hcd, mtk->ippc);
> +
> + ret = clk_get_bulk(dev, >clks);
> + if (ret) {
> + dev_err(dev, "Failed to get clocks\n");

Nitpick -- print the return value you got, it makes things easier to
debug if it's in the error output. Same for the other places which
report such errors in this driver.

> + return ret;
> + }

[...]

The rest looks good, thanks.


Antwort: [PATCH] serial: ns16550: Fix ordering of getting base address

2020-04-03 Thread Wolfgang Wallner
Hi Bin,

Thanks for taking care of this!

-"Bin Meng"  schrieb: -

>An: "Simon Glass" , "Tom Rini"
>, "Andy Shevchenko"
>, "Wolfgang Wallner"
>, "Chee Hong Ang"
>, "U-Boot Mailing List"
>
>Von: "Bin Meng" 
>Datum: 03.04.2020 11:58
>Betreff: [PATCH] serial: ns16550: Fix ordering of getting base
>address
>
>Currently the driver gets ns16550 base address in the driver
>probe() routine, which may potentially break any ns16550 wrapper
>driver that does additional initialization before calling
>ns16550_serial_probe().
>
>Things are complicated that we need consider ns16550 devices on
>both simple-bus and PCI bus. To fix the issue we move the base
>address assignment for simple-bus ns16550 device back to the
>ofdata_to_platdata(), and assign base address for PCI ns16550
>device in ns16550_serial_probe().
>
>This is still not perfect. Ideally if any PCI bus based ns16550
>wrapper driver tries to access plat->base before calling probe(),
>it is subject to break.
>
>Fixes: 720f9e1fdb0c9 ("serial: ns16550: Move PCI access from
>ofdata_to_platdata() to probe()")
>Reported-by: Andy Shevchenko 
>Signed-off-by: Bin Meng 
>
>---
>
> drivers/serial/ns16550.c | 51
>+++-
> 1 file changed, 24 insertions(+), 27 deletions(-)
>
>diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
>index c1b303f..5e3cd1c 100644
>--- a/drivers/serial/ns16550.c
>+++ b/drivers/serial/ns16550.c
>@@ -479,39 +479,24 @@ static int ns16550_serial_getinfo(struct
>udevice *dev,
>   return 0;
> }
> 
>-#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
>-static int ns1655_serial_set_base_addr(struct udevice *dev)
>-{
>-  fdt_addr_t addr;
>-  struct ns16550_platdata *plat;
>-
>-  plat = dev_get_platdata(dev);
>-
>-  addr = dev_read_addr_pci(dev);
>-  if (addr == FDT_ADDR_T_NONE)
>-  return -EINVAL;
>-
>-#ifdef CONFIG_SYS_NS16550_PORT_MAPPED
>-  plat->base = addr;
>-#else
>-  plat->base = (unsigned long)map_physmem(addr, 0, MAP_NOCACHE);
>-#endif
>-
>-  return 0;
>-}
>-#endif
>-
> int ns16550_serial_probe(struct udevice *dev)
> {
>+  struct ns16550_platdata *plat = dev->platdata;
>   struct NS16550 *const com_port = dev_get_priv(dev);
>   struct reset_ctl_bulk reset_bulk;
>+  fdt_addr_t addr;
>   int ret;
> 
>-#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
>-  ret = ns1655_serial_set_base_addr(dev);
>-  if (ret)
>-  return ret;
>-#endif
>+  /*
>+   * If we are on PCI bus, either directly attached to a PCI root
>port,
>+   * or via a PCI bridge, assign platdata->base before probing
>hardware.
>+   */
>+  if (device_is_on_pci_bus(dev)) {
>+  addr = devfdt_get_addr_pci(dev);
>+  if (addr == FDT_ADDR_T_NONE)
>+  return -EINVAL;
>+  plat->base = addr;

The assignment here to plat->base does not distinguish any more based on
CONFIG_SYS_NS16550_PORT_MAPPED. Is it not necessary any more in this case?

>+  }
> 
>   ret = reset_get_bulk(dev, _bulk);
>   if (!ret)
>@@ -535,9 +520,21 @@ int ns16550_serial_ofdata_to_platdata(struct
>udevice *dev)
> {
>   struct ns16550_platdata *plat = dev->platdata;
>   const u32 port_type = dev_get_driver_data(dev);
>+  fdt_addr_t addr;
>   struct clk clk;
>   int err;
> 
>+  addr = dev_read_addr(dev);
>+  if (addr != FDT_ADDR_T_NONE) {
>+#ifdef CONFIG_SYS_NS16550_PORT_MAPPED
>+  plat->base = addr;
>+#else
>+  plat->base = (unsigned long)map_physmem(addr, 0, MAP_NOCACHE);
>+#endif
>+  } else if (!device_is_on_pci_bus(dev)) {
>+  return -EINVAL;
>+  }
>+
>   plat->reg_offset = dev_read_u32_default(dev, "reg-offset", 0);
>   plat->reg_shift = dev_read_u32_default(dev, "reg-shift", 0);
>   plat->reg_width = dev_read_u32_default(dev, "reg-io-width", 1);
>-- 
>2.7.4
>
>

Tested-by: Wolfgang Wallner 
Tested on an Intel Apollo Lake based board booting with Coreboot and using
U-Boot as payload.

Reviewed-by: Wolfgang Wallner 



Re: [PATCH v3 12/29] dm: core: Add basic ACPI support

2020-04-03 Thread Andy Shevchenko
On Mon, Mar 30, 2020 at 05:12:48PM -0600, Simon Glass wrote:
> ACPI (Advanced Configuration and Power Interface) is a standard for
> specifying information about a platform. It is a little like device
> tree but the bindings are part of the specification and it supports an
> interpreted bytecode language.
> 
> Driver model does not use ACPI for U-Boot's configuration, but it is
> convenient to have it support generation of ACPI tables for passing to
> Linux, etc.
> 
> As a starting point, add an optional set of ACPI operations to each
> device. Initially only a single operation is available, to obtain the
> ACPI name for the device. More operations are added later.
> 
> Enable ACPI for sandbox to ensure build coverage and so that we can add
> tests.


...

> +int acpi_copy_name(char *out_name, const char *name)
> +{

> + strncpy(out_name, name, ACPI_NAME_MAX);

memcpy()?

> + out_name[ACPI_NAME_LEN] = '\0';

I dunno if compiler is clever enough to catch this and avoid any warnings.

Also it seems above should also have _LEN, and not _MAX.

> + return 0;
> +}

-- 
With Best Regards,
Andy Shevchenko




Re: Antwort: [PATCH v3 11/29] x86: apl: Add Global NVS table header

2020-04-03 Thread Andy Shevchenko
On Tue, Mar 31, 2020 at 10:07:37AM +0200, Wolfgang Wallner wrote:

> >+struct __packed acpi_global_nvs {
> >+/* Miscellaneous */
> >+u8  pcnt; /* 0x00 - Processor Count */
> >+u8  ppcm; /* 0x01 - Max PPC State */
> >+u8  lids; /* 0x02 - LID State */
> >+u8  pwrs; /* 0x03 - AC Power State */
> >+u8  dpte; /* 0x04 - Enable DPTF */
> >+u32 cbmc; /* 0x05 - 0x08 - U-Boot Console */
> >+u64 pm1i; /* 0x09 - 0x10 - System Wake Source - PM1 Index */
> >+u64 gpei; /* 0x11 - 0x18 - GPE Wake Source */
> >+u64 nhla; /* 0x19 - 0x20 - NHLT Address */
> >+u32 nhll; /* 0x21 - 0x24 - NHLT Length */
> >+u32 prt0; /* 0x25 - 0x28 - PERST_0 Address */
> >+u8  scdp; /* 0x29 - SD_CD GPIO portid */
> >+u8  scdo; /* 0x2A - GPIO pad offset relative to the community */
> >+u8  uior; /* 0x2B - UART debug controller init on S3 resume */
> >+u8  ecps; /* 0x2C - SGX Enabled status */
> >+u64 emna; /* 0x2D - 0x34 EPC base address */
> >+u64 elng; /* 0x35 - 0x3C EPC Length */
> >+u8  unused[195];
> >+u8  unused2[0xf00];
> 
> Nit 1: Something is still wrong with the indentation of unused2.
> 
> Nit 2: Could you please add comments on why the values 195 and 0xf00 were
>chosen? I would assume 195 was selected so that unused2 starts on
>a 256-byte boundary? But that is only a guess.

Better to calculate them (I mean 195). In Linux kernel, for instance, a trick
is being used for that.


The rest is not simply boundary but rather 256 vs 4096 size of NVS. The
previous (small one) is what being used by U-Boot. I don't remember what ACPI
spec dictates about this (i.o.w. if it depends to the ACPI version in use).

> >+};

-- 
With Best Regards,
Andy Shevchenko




Re: [PATCH v3 10/29] pci: Adjust dm_pci_read_bar32() to return errors correctly

2020-04-03 Thread Andy Shevchenko
On Mon, Mar 30, 2020 at 05:12:46PM -0600, Simon Glass wrote:
> At present if reading a BAR returns 0x (e.g. the device is not
> present) then the value is masked and a different value is returned.
> This makes it harder to detect the problem when debugging.

The above ('the device is not present') is actually not correct.
BAR is not mandatory register and detection is described in PCI spec.

To get device presence one may have check Vendor ID / Device ID pair rather
then BAR.

> Update the function to avoid masking in this case.
> 
> Signed-off-by: Simon Glass 
> Reviewed-by: Bin Meng 
> Reviewed-by: Wolfgang Wallner 
> ---
> 
> Changes in v3: None
> Changes in v2: None
> 
>  drivers/pci/pci-uclass.c | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
> index ceb64517047..d2e10d6868a 100644
> --- a/drivers/pci/pci-uclass.c
> +++ b/drivers/pci/pci-uclass.c
> @@ -1213,7 +1213,14 @@ u32 dm_pci_read_bar32(const struct udevice *dev, int 
> barnum)
>  
>   bar = PCI_BASE_ADDRESS_0 + barnum * 4;
>   dm_pci_read_config32(dev, bar, );
> - if (addr & PCI_BASE_ADDRESS_SPACE_IO)
> +
> + /*
> +  * If we get an invalid address, return this so that comparisons with
> +  * FDT_ADDR_T_NONE work correctly
> +  */
> + if (addr == 0x)
> + return addr;
> + else if (addr & PCI_BASE_ADDRESS_SPACE_IO)
>   return addr & PCI_BASE_ADDRESS_IO_MASK;
>   else
>   return addr & PCI_BASE_ADDRESS_MEM_MASK;
> -- 
> 2.26.0.rc2.310.g2932bb562d-goog
> 

-- 
With Best Regards,
Andy Shevchenko




Re: [PATCH] serial: ns16550: Fix ordering of getting base address

2020-04-03 Thread Andy Shevchenko
On Fri, Apr 03, 2020 at 06:16:51PM +0800, Bin Meng wrote:
> On Fri, Apr 3, 2020 at 6:05 PM Andy Shevchenko
>  wrote:
> > On Fri, Apr 03, 2020 at 02:58:08AM -0700, Bin Meng wrote:

...

> > > Fixes: 720f9e1fdb0c9 ("serial: ns16550: Move PCI access from
> > > ofdata_to_platdata() to probe()")
> >
> > I believe this should be one line.
> 
> I thought putting it in one line violates the 80 characters per column
> rule. And checkpatch.pl does not complain about this, so I think it's
> fine :)

Breaking tags to two or more lines actually breaks the scripts around this 
stuff.

JFYI, in Linux kernel it was now in documentation (not to break Fixes, etc to
two or more lines).

-- 
With Best Regards,
Andy Shevchenko




[PATCH v2 0/2] fdtdec: support multiple phandles in memory carveout

2020-04-03 Thread Laurentiu Tudor
Content-Type: text/plain; charset="us-ascii"

fdtdec_set_carveout() is limited to only one phandle. Fix this
limitation by adding support for multiple phandles and also add
an unit test for the function.

Changes in v2:
 - added a unit test for the function (Simon)
 - added a cover letter

Laurentiu Tudor (2):
  fdtdec: support multiple phandles in memory carveout
  test: fdtdec: test fdtdec_set_carveout()

 lib/fdtdec.c | 36 +
 test/dm/Makefile |  1 +
 test/dm/fdtdec.c | 60 
 3 files changed, 87 insertions(+), 10 deletions(-)
 create mode 100644 test/dm/fdtdec.c

-- 
2.17.1



[PATCH v2 1/2] fdtdec: support multiple phandles in memory carveout

2020-04-03 Thread Laurentiu Tudor
fdtdec_set_carveout() is limited to only one phandle. Fix this
limitation by adding support for multiple phandles.

Signed-off-by: Laurentiu Tudor 
---
 lib/fdtdec.c | 36 ++--
 1 file changed, 26 insertions(+), 10 deletions(-)

diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index eb11fc898e..9ecfa2a2d7 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -1433,14 +1433,9 @@ int fdtdec_set_carveout(void *blob, const char *node, 
const char *prop_name,
const struct fdt_memory *carveout)
 {
uint32_t phandle;
-   int err, offset;
+   int err, offset, len;
fdt32_t value;
-
-   /* XXX implement support for multiple phandles */
-   if (index > 0) {
-   debug("invalid index %u\n", index);
-   return -FDT_ERR_BADOFFSET;
-   }
+   void *prop;
 
err = fdtdec_add_reserved_memory(blob, name, carveout, );
if (err < 0) {
@@ -1456,10 +1451,31 @@ int fdtdec_set_carveout(void *blob, const char *node, 
const char *prop_name,
 
value = cpu_to_fdt32(phandle);
 
-   err = fdt_setprop(blob, offset, prop_name, , sizeof(value));
+   if (!fdt_getprop(blob, offset, prop_name, )) {
+   if (len == -FDT_ERR_NOTFOUND)
+   len = 0;
+   else
+   return len;
+   }
+
+   if ((index + 1) * sizeof(value) > len) {
+   err = fdt_setprop_placeholder(blob, offset, prop_name,
+ (index + 1) * sizeof(value),
+ );
+   if (err < 0) {
+   debug("failed to resize reserved memory property: %s\n",
+ fdt_strerror(err));
+   return err;
+   }
+   }
+
+   err = fdt_setprop_inplace_namelen_partial(blob, offset, prop_name,
+ strlen(prop_name),
+ index * sizeof(value),
+ , sizeof(value));
if (err < 0) {
-   debug("failed to set %s property for node %s: %d\n", prop_name,
- node, err);
+   debug("failed to update %s property for node %s: %s\n",
+ prop_name, node, fdt_strerror(err));
return err;
}
 
-- 
2.17.1



[PATCH v2 2/2] test: fdtdec: test fdtdec_set_carveout()

2020-04-03 Thread Laurentiu Tudor
Add a new test for fdtdec_set_carveout().

Signed-off-by: Laurentiu Tudor 
---
 test/dm/Makefile |  1 +
 test/dm/fdtdec.c | 60 
 2 files changed, 61 insertions(+)
 create mode 100644 test/dm/fdtdec.c

diff --git a/test/dm/Makefile b/test/dm/Makefile
index dd1ceff86c..53caa29fbb 100644
--- a/test/dm/Makefile
+++ b/test/dm/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_LED) += led.o
 obj-$(CONFIG_DM_MAILBOX) += mailbox.o
 obj-$(CONFIG_DM_MMC) += mmc.o
 obj-y += ofnode.o
+obj-y += fdtdec.o
 obj-$(CONFIG_OSD) += osd.o
 obj-$(CONFIG_DM_VIDEO) += panel.o
 obj-$(CONFIG_DM_PCI) += pci.o
diff --git a/test/dm/fdtdec.c b/test/dm/fdtdec.c
new file mode 100644
index 00..6636af48d7
--- /dev/null
+++ b/test/dm/fdtdec.c
@@ -0,0 +1,60 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2020 NXP
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+static int dm_test_fdtdec_set_carveout(struct unit_test_state *uts)
+{
+   struct fdt_memory resv;
+   void *blob;
+   const fdt32_t *prop;
+   int blob_sz, len, offset;
+
+   blob_sz = fdt_totalsize(gd->fdt_blob) + 4096;
+   blob = malloc(blob_sz);
+   ut_assertnonnull(blob);
+
+   /* Make a writtable copy of the fdt blob */
+   ut_assertok(fdt_open_into(gd->fdt_blob, blob, blob_sz));
+
+   resv.start = 0x1000;
+   resv.end = 0x2000;
+   ut_assertok(fdtdec_set_carveout(blob, "/a-test",
+   "memory-region", 2, "test_resv1",
+   ));
+
+   resv.start = 0x1;
+   resv.end = 0x2;
+   ut_assertok(fdtdec_set_carveout(blob, "/a-test",
+   "memory-region", 1, "test_resv2",
+   ));
+
+   resv.start = 0x10;
+   resv.end = 0x20;
+   ut_assertok(fdtdec_set_carveout(blob, "/a-test",
+   "memory-region", 0, "test_resv3",
+   ));
+
+   offset = fdt_path_offset(blob, "/a-test");
+   ut_assert(offset > 0);
+   prop = fdt_getprop(blob, offset, "memory-region", );
+   ut_assertnonnull(prop);
+
+   ut_asserteq(len, 12);
+   ut_assert(fdt_node_offset_by_phandle(blob, fdt32_to_cpu(prop[0])) > 0);
+   ut_assert(fdt_node_offset_by_phandle(blob, fdt32_to_cpu(prop[1])) > 0);
+   ut_assert(fdt_node_offset_by_phandle(blob, fdt32_to_cpu(prop[2])) > 0);
+
+   free(blob);
+
+   return 0;
+}
+DM_TEST(dm_test_fdtdec_set_carveout,
+   DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT | DM_TESTF_FLAT_TREE);
+
-- 
2.17.1



Re: [PATCH] serial: ns16550: Fix ordering of getting base address

2020-04-03 Thread Bin Meng
Hi Andy,

On Fri, Apr 3, 2020 at 6:05 PM Andy Shevchenko
 wrote:
>
> On Fri, Apr 03, 2020 at 02:58:08AM -0700, Bin Meng wrote:
> > Currently the driver gets ns16550 base address in the driver
> > probe() routine, which may potentially break any ns16550 wrapper
> > driver that does additional initialization before calling
> > ns16550_serial_probe().
> >
> > Things are complicated that we need consider ns16550 devices on
> > both simple-bus and PCI bus. To fix the issue we move the base
> > address assignment for simple-bus ns16550 device back to the
> > ofdata_to_platdata(), and assign base address for PCI ns16550
> > device in ns16550_serial_probe().
> >
> > This is still not perfect. Ideally if any PCI bus based ns16550
> > wrapper driver tries to access plat->base before calling probe(),
> > it is subject to break.
>
> Thank you.
> I have tested it and it fixes my case.
>
> Tested-by: Andy Shevchenko 

Thanks for testing!

>
>
> > Fixes: 720f9e1fdb0c9 ("serial: ns16550: Move PCI access from
> > ofdata_to_platdata() to probe()")
>
> I believe this should be one line.

I thought putting it in one line violates the 80 characters per column
rule. And checkpatch.pl does not complain about this, so I think it's
fine :)

>
> > Reported-by: Andy Shevchenko 
> > Signed-off-by: Bin Meng 
> >
> > ---
> >
> >  drivers/serial/ns16550.c | 51 
> > +++-
> >  1 file changed, 24 insertions(+), 27 deletions(-)
> >

Regards,
Bin


Re: [PATCH v3] serial: ns16550: Revert "Move PCI access from ofdata_to_platdata() to probe()"

2020-04-03 Thread Andy Shevchenko
On Fri, Apr 3, 2020 at 1:08 PM Bin Meng  wrote:
> On Fri, Apr 3, 2020 at 5:02 PM Wolfgang Wallner
>  wrote:

> We can't revert as that will put PCI based ns16550 in a broken state again.
>
> I've sent a patch to fix it. Please have a try.

Just did, thank you!

-- 
With Best Regards,
Andy Shevchenko


Re: [PATCH v3] serial: ns16550: Revert "Move PCI access from ofdata_to_platdata() to probe()"

2020-04-03 Thread Bin Meng
Hi Wolfgang,

On Fri, Apr 3, 2020 at 5:02 PM Wolfgang Wallner
 wrote:
>
> Hi Andy,
>
> -"Andy Shevchenko"  schrieb: -
>
> The commit 720f9e1fdb0c ("Move PCI access from ofdata_to_platdata() to
> probe()") while doing formally a right thing, actually brings a regression
> to the drivers that would like to pre-initialize hardware before calling
> ns16550_serial_probe(). In particular, the code, which gets moved out,
> is responsible for getting the address of the hardware.
>
> The commit breaks serial console on the Intel Edison, for example.
>
> Since we are very close to the release, the quick fix is to revert the
> culprit commit 720f9e1fdb0c92d3fd16e1bfc25bcbd35612675c.
>
> Note, it doesn't affect SoCFPGA case.
>
> Cc: Wolfgang Wallner 
> Signed-off-by: Andy Shevchenko 
> ---
> v3: drop wrong patch, better explanation in the commit message
>  drivers/serial/ns16550.c | 40 
>  1 file changed, 12 insertions(+), 28 deletions(-)
>
> Reviewed-by: Wolfgang Wallner 
>

We can't revert as that will put PCI based ns16550 in a broken state again.

I've sent a patch to fix it. Please have a try.

Regards,
Bin


Re: [PATCH] serial: ns16550: Fix ordering of getting base address

2020-04-03 Thread Andy Shevchenko
On Fri, Apr 03, 2020 at 02:58:08AM -0700, Bin Meng wrote:
> Currently the driver gets ns16550 base address in the driver
> probe() routine, which may potentially break any ns16550 wrapper
> driver that does additional initialization before calling
> ns16550_serial_probe().
> 
> Things are complicated that we need consider ns16550 devices on
> both simple-bus and PCI bus. To fix the issue we move the base
> address assignment for simple-bus ns16550 device back to the
> ofdata_to_platdata(), and assign base address for PCI ns16550
> device in ns16550_serial_probe().
> 
> This is still not perfect. Ideally if any PCI bus based ns16550
> wrapper driver tries to access plat->base before calling probe(),
> it is subject to break.

Thank you.
I have tested it and it fixes my case.

Tested-by: Andy Shevchenko 


> Fixes: 720f9e1fdb0c9 ("serial: ns16550: Move PCI access from
> ofdata_to_platdata() to probe()")

I believe this should be one line.

> Reported-by: Andy Shevchenko 
> Signed-off-by: Bin Meng 
> 
> ---
> 
>  drivers/serial/ns16550.c | 51 
> +++-
>  1 file changed, 24 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
> index c1b303f..5e3cd1c 100644
> --- a/drivers/serial/ns16550.c
> +++ b/drivers/serial/ns16550.c
> @@ -479,39 +479,24 @@ static int ns16550_serial_getinfo(struct udevice *dev,
>   return 0;
>  }
>  
> -#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
> -static int ns1655_serial_set_base_addr(struct udevice *dev)
> -{
> - fdt_addr_t addr;
> - struct ns16550_platdata *plat;
> -
> - plat = dev_get_platdata(dev);
> -
> - addr = dev_read_addr_pci(dev);
> - if (addr == FDT_ADDR_T_NONE)
> - return -EINVAL;
> -
> -#ifdef CONFIG_SYS_NS16550_PORT_MAPPED
> - plat->base = addr;
> -#else
> - plat->base = (unsigned long)map_physmem(addr, 0, MAP_NOCACHE);
> -#endif
> -
> - return 0;
> -}
> -#endif
> -
>  int ns16550_serial_probe(struct udevice *dev)
>  {
> + struct ns16550_platdata *plat = dev->platdata;
>   struct NS16550 *const com_port = dev_get_priv(dev);
>   struct reset_ctl_bulk reset_bulk;
> + fdt_addr_t addr;
>   int ret;
>  
> -#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
> - ret = ns1655_serial_set_base_addr(dev);
> - if (ret)
> - return ret;
> -#endif
> + /*
> +  * If we are on PCI bus, either directly attached to a PCI root port,
> +  * or via a PCI bridge, assign platdata->base before probing hardware.
> +  */
> + if (device_is_on_pci_bus(dev)) {
> + addr = devfdt_get_addr_pci(dev);
> + if (addr == FDT_ADDR_T_NONE)
> + return -EINVAL;
> + plat->base = addr;
> + }
>  
>   ret = reset_get_bulk(dev, _bulk);
>   if (!ret)
> @@ -535,9 +520,21 @@ int ns16550_serial_ofdata_to_platdata(struct udevice 
> *dev)
>  {
>   struct ns16550_platdata *plat = dev->platdata;
>   const u32 port_type = dev_get_driver_data(dev);
> + fdt_addr_t addr;
>   struct clk clk;
>   int err;
>  
> + addr = dev_read_addr(dev);
> + if (addr != FDT_ADDR_T_NONE) {
> +#ifdef CONFIG_SYS_NS16550_PORT_MAPPED
> + plat->base = addr;
> +#else
> + plat->base = (unsigned long)map_physmem(addr, 0, MAP_NOCACHE);
> +#endif
> + } else if (!device_is_on_pci_bus(dev)) {
> + return -EINVAL;
> + }
> +
>   plat->reg_offset = dev_read_u32_default(dev, "reg-offset", 0);
>   plat->reg_shift = dev_read_u32_default(dev, "reg-shift", 0);
>   plat->reg_width = dev_read_u32_default(dev, "reg-io-width", 1);
> -- 
> 2.7.4
> 

-- 
With Best Regards,
Andy Shevchenko




[PATCH] serial: ns16550: Fix ordering of getting base address

2020-04-03 Thread Bin Meng
Currently the driver gets ns16550 base address in the driver
probe() routine, which may potentially break any ns16550 wrapper
driver that does additional initialization before calling
ns16550_serial_probe().

Things are complicated that we need consider ns16550 devices on
both simple-bus and PCI bus. To fix the issue we move the base
address assignment for simple-bus ns16550 device back to the
ofdata_to_platdata(), and assign base address for PCI ns16550
device in ns16550_serial_probe().

This is still not perfect. Ideally if any PCI bus based ns16550
wrapper driver tries to access plat->base before calling probe(),
it is subject to break.

Fixes: 720f9e1fdb0c9 ("serial: ns16550: Move PCI access from
ofdata_to_platdata() to probe()")
Reported-by: Andy Shevchenko 
Signed-off-by: Bin Meng 

---

 drivers/serial/ns16550.c | 51 +++-
 1 file changed, 24 insertions(+), 27 deletions(-)

diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
index c1b303f..5e3cd1c 100644
--- a/drivers/serial/ns16550.c
+++ b/drivers/serial/ns16550.c
@@ -479,39 +479,24 @@ static int ns16550_serial_getinfo(struct udevice *dev,
return 0;
 }
 
-#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
-static int ns1655_serial_set_base_addr(struct udevice *dev)
-{
-   fdt_addr_t addr;
-   struct ns16550_platdata *plat;
-
-   plat = dev_get_platdata(dev);
-
-   addr = dev_read_addr_pci(dev);
-   if (addr == FDT_ADDR_T_NONE)
-   return -EINVAL;
-
-#ifdef CONFIG_SYS_NS16550_PORT_MAPPED
-   plat->base = addr;
-#else
-   plat->base = (unsigned long)map_physmem(addr, 0, MAP_NOCACHE);
-#endif
-
-   return 0;
-}
-#endif
-
 int ns16550_serial_probe(struct udevice *dev)
 {
+   struct ns16550_platdata *plat = dev->platdata;
struct NS16550 *const com_port = dev_get_priv(dev);
struct reset_ctl_bulk reset_bulk;
+   fdt_addr_t addr;
int ret;
 
-#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
-   ret = ns1655_serial_set_base_addr(dev);
-   if (ret)
-   return ret;
-#endif
+   /*
+* If we are on PCI bus, either directly attached to a PCI root port,
+* or via a PCI bridge, assign platdata->base before probing hardware.
+*/
+   if (device_is_on_pci_bus(dev)) {
+   addr = devfdt_get_addr_pci(dev);
+   if (addr == FDT_ADDR_T_NONE)
+   return -EINVAL;
+   plat->base = addr;
+   }
 
ret = reset_get_bulk(dev, _bulk);
if (!ret)
@@ -535,9 +520,21 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
 {
struct ns16550_platdata *plat = dev->platdata;
const u32 port_type = dev_get_driver_data(dev);
+   fdt_addr_t addr;
struct clk clk;
int err;
 
+   addr = dev_read_addr(dev);
+   if (addr != FDT_ADDR_T_NONE) {
+#ifdef CONFIG_SYS_NS16550_PORT_MAPPED
+   plat->base = addr;
+#else
+   plat->base = (unsigned long)map_physmem(addr, 0, MAP_NOCACHE);
+#endif
+   } else if (!device_is_on_pci_bus(dev)) {
+   return -EINVAL;
+   }
+
plat->reg_offset = dev_read_u32_default(dev, "reg-offset", 0);
plat->reg_shift = dev_read_u32_default(dev, "reg-shift", 0);
plat->reg_width = dev_read_u32_default(dev, "reg-io-width", 1);
-- 
2.7.4



[PATCH v2] dm: core: remove the duplicated function dm_ofnode_pre_reloc

2020-04-03 Thread Patrick Delaunay
The content dm_ofnode_pre_reloc() is identical with ofnode_pre_reloc()
defined in drivers/core/ofnode.c and used only three times:
- drivers/core/lists.c:lists_bind_fdt()
- drivers/clk/at91/pmc.c::at91_clk_sub_device_bind
- drivers/clk/altera/clk-arria10.c::socfpga_a10_clk_bind

So this function dm_ofnode_pre_reloc can be removed and replaced
by these function calls by ofnode_pre_reloc().

Signed-off-by: Patrick Delaunay 
---
Hi Simon,

It is a rebased patch for http://patchwork.ozlabs.org/patch/1035799/
marked as superseded but never resent (and I forget it).

Compilation is OK on travis:
https://travis-ci.org/github/patrickdelaunay/u-boot/builds/670029232

Patrick


Changes in v2:
- rebase on master
- udpate dm_ofnode_pre_reloc call in at91_clk_sub_device_bind and
  socfpga_a10_clk_bind

 drivers/clk/altera/clk-arria10.c |  2 +-
 drivers/clk/at91/pmc.c   |  2 +-
 drivers/core/lists.c |  2 +-
 drivers/core/util.c  | 28 
 include/dm/util.h| 27 ---
 5 files changed, 3 insertions(+), 58 deletions(-)

diff --git a/drivers/clk/altera/clk-arria10.c b/drivers/clk/altera/clk-arria10.c
index affeb31fc2..0a9bf13ac4 100644
--- a/drivers/clk/altera/clk-arria10.c
+++ b/drivers/clk/altera/clk-arria10.c
@@ -258,7 +258,7 @@ static int socfpga_a10_clk_bind(struct udevice *dev)
continue;
 
if (pre_reloc_only &&
-   !dm_ofnode_pre_reloc(offset_to_ofnode(offset)))
+   !ofnode_pre_reloc(offset_to_ofnode(offset)))
continue;
 
ret = device_bind_driver_to_node(dev, "clk-a10", name,
diff --git a/drivers/clk/at91/pmc.c b/drivers/clk/at91/pmc.c
index 6b55ec59d6..f5808449a6 100644
--- a/drivers/clk/at91/pmc.c
+++ b/drivers/clk/at91/pmc.c
@@ -61,7 +61,7 @@ int at91_clk_sub_device_bind(struct udevice *dev, const char 
*drv_name)
 offset > 0;
 offset = fdt_next_subnode(fdt, offset)) {
if (pre_reloc_only &&
-   !dm_ofnode_pre_reloc(offset_to_ofnode(offset)))
+   !ofnode_pre_reloc(offset_to_ofnode(offset)))
continue;
/*
 * If this node has "compatible" property, this is not
diff --git a/drivers/core/lists.c b/drivers/core/lists.c
index 68204c303f..c7db14ed56 100644
--- a/drivers/core/lists.c
+++ b/drivers/core/lists.c
@@ -175,7 +175,7 @@ int lists_bind_fdt(struct udevice *parent, ofnode node, 
struct udevice **devp,
continue;
 
if (pre_reloc_only) {
-   if (!dm_ofnode_pre_reloc(node) &&
+   if (!ofnode_pre_reloc(node) &&
!(entry->flags & DM_FLAG_PRE_RELOC)) {
log_debug("Skipping device pre-relocation\n");
return 0;
diff --git a/drivers/core/util.c b/drivers/core/util.c
index 69f83755f0..25b0d76f43 100644
--- a/drivers/core/util.c
+++ b/drivers/core/util.c
@@ -33,34 +33,6 @@ int list_count_items(struct list_head *head)
return count;
 }
 
-#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
-bool dm_ofnode_pre_reloc(ofnode node)
-{
-#if defined(CONFIG_SPL_BUILD) || defined(CONFIG_TPL_BUILD)
-   /* for SPL and TPL the remaining nodes after the fdtgrep 1st pass
-* had property dm-pre-reloc or u-boot,dm-spl/tpl.
-* They are removed in final dtb (fdtgrep 2nd pass)
-*/
-   return true;
-#else
-   if (ofnode_read_bool(node, "u-boot,dm-pre-reloc"))
-   return true;
-   if (ofnode_read_bool(node, "u-boot,dm-pre-proper"))
-   return true;
-
-   /*
-* In regular builds individual spl and tpl handling both
-* count as handled pre-relocation for later second init.
-*/
-   if (ofnode_read_bool(node, "u-boot,dm-spl") ||
-   ofnode_read_bool(node, "u-boot,dm-tpl"))
-   return true;
-
-   return false;
-#endif
-}
-#endif
-
 #if !CONFIG_IS_ENABLED(OF_PLATDATA)
 int pci_get_devfn(struct udevice *dev)
 {
diff --git a/include/dm/util.h b/include/dm/util.h
index 0ccb3fbadf..23f8deb14e 100644
--- a/include/dm/util.h
+++ b/include/dm/util.h
@@ -42,31 +42,4 @@ static inline void dm_dump_devres(void)
 /* Dump out a list of drivers */
 void dm_dump_drivers(void);
 
-/**
- * Check if an of node should be or was bound before relocation.
- *
- * Devicetree nodes can be marked as needed to be bound
- * in the loader stages via special devicetree properties.
- *
- * Before relocation this function can be used to check if nodes
- * are required in either SPL or TPL stages.
- *
- * After relocation and jumping into the real U-Boot binary
- * it is possible to determine if a node was bound in one of
- * SPL/TPL stages.
- *
- * There are 4 settings currently in use
- * - u-boot,dm-pre-proper: U-Boot proper pre-relocation only
- 

[PATCH v2 0/2] arm: stm32mp1: activate data cache in SPL and before relocation

2020-04-03 Thread Patrick Delaunay


V2 after first feedbacks of the previous patch
"arm: stm32mp1: activate data cache in SPL and before relocation"
http://patchwork.ozlabs.org/patch/1263815/

This new serie depends on the ARM cache serie:
http://patchwork.ozlabs.org/project/uboot/list/?series=168378

I move tlb in .data section and simplify the implementation by reusing
the default weak function dram_bank_mmu_setup() for MMU configuration
and mmu_set_region_dcache_behaviour() to setup the specific behavior.

I also activate data cache on DDR for SPL.

For information the gain of the second patch is limited (few ms) for boot
from SDCARD: the SDMMC IP use internal DMA and data cache on DDR is
not really used.

Gain should be better for other boot use-case.

Example of bootstage report on STM32MP157C-DK2, boot from SD card.

1/ For trusted boot chain with TF-A

a) Before

STM32MP> bootstage report
Timer summary in microseconds (9 records):
   MarkElapsed  Stage
  0  0  reset
583,290583,290  board_init_f
  2,348,898  1,765,608  board_init_r
  2,664,580315,682  id=64
  2,704,027 39,447  id=65
  2,704,729702  main_loop
  5,563,519  2,858,790  id=175

Accumulated time:
41,696  dm_r
   615,561  dm_f

b) After the serie

STM32MP> bootstage report
Timer summary in microseconds (9 records):
   MarkElapsed  Stage
  0  0  reset
583,401583,401  board_init_f
727,725144,324  board_init_r
  1,043,362315,637  id=64
  1,082,806 39,444  id=65
  1,083,507701  main_loop
  3,680,827  2,597,320  id=175

Accumulated time:
36,047  dm_f
41,718  dm_r

2/ And for the basic boot chain with SPL

a) Before:

STM32MP> bootstage report
Timer summary in microseconds (12 records):
   MarkElapsed  Stage
  0  0  reset
195,613195,613  SPL
837,867642,254  end SPL
840,117  2,250  board_init_f
  2,739,639  1,899,522  board_init_r
  3,066,815327,176  id=64
  3,103,377 36,562  id=65
  3,104,078701  main_loop
  3,142,171 38,093  id=175

Accumulated time:
38,124  dm_spl
41,956  dm_r
   648,861  dm_f

b) After the serie

STM32MP> bootstage report
Timer summary in microseconds (12 records):
   MarkElapsed  Stage
  0  0  reset
195,859195,859  SPL
330,190134,331  end SPL
332,408  2,218  board_init_f
482,688150,280  board_init_r
808,694326,006  id=64
845,029 36,335  id=65
845,730701  main_loop
  3,281,876  2,436,146  id=175

Accumulated time:
 3,169  dm_spl
36,041  dm_f
41,701  dm_r

STM32MP> bootstage report
Timer summary in microseconds (12 records):
   MarkElapsed  Stage
  0  0  reset
211,036211,036  SPL
343,393132,357  end SPL
345,645  2,252  board_init_f
496,596150,951  board_init_r
822,256325,660  id=64
858,451 36,195  id=65
859,153702  main_loop
  3,414,706  2,555,553  id=175

Accumulated time:
 3,132  dm_spl
36,005  dm_f
41,695  dm_r


Changes in v2:
- create a new function early_enable_caches
- use TLB in .init section
- use the default weak dram_bank_mmu_setup() and
  use mmu_set_region_dcache_behaviour() to setup
  the early MMU configuration
- enable data cache on DDR in SPL, after DDR controller initialization
- new

Patrick Delaunay (2):
  arm: stm32mp: activate data cache in SPL and before relocation
  arm: stm32mp: activate data cache on DDR in SPL

 arch/arm/mach-stm32mp/cpu.c | 43 -
 arch/arm/mach-stm32mp/spl.c | 21 ++
 2 files changed, 63 insertions(+), 1 deletion(-)

-- 
2.17.1



[PATCH v2 2/2] arm: stm32mp: activate data cache on DDR in SPL

2020-04-03 Thread Patrick Delaunay
Activate cache on DDR to improves the accesses to DDR used by SPL:
- CONFIG_SPL_BSS_START_ADDR
- CONFIG_SYS_SPL_MALLOC_START

Cache is configured only when DDR is fully initialized,
to avoid speculative access and issue in get_ram_size().
Data cache is deactivated at the end of SPL, to flush the data cache
and the TLB.

Signed-off-by: Patrick Delaunay 
---

Changes in v2:
- new

 arch/arm/mach-stm32mp/spl.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/arch/arm/mach-stm32mp/spl.c b/arch/arm/mach-stm32mp/spl.c
index 9cd7b418a4..279121af75 100644
--- a/arch/arm/mach-stm32mp/spl.c
+++ b/arch/arm/mach-stm32mp/spl.c
@@ -4,6 +4,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -117,4 +118,24 @@ void board_init_f(ulong dummy)
printf("DRAM init failed: %d\n", ret);
hang();
}
+
+   /*
+* activate cache on DDR only when DDR is fully initialized
+* to avoid speculative access and issue in get_ram_size()
+*/
+   if (!CONFIG_IS_ENABLED(SYS_DCACHE_OFF))
+   mmu_set_region_dcache_behaviour(STM32_DDR_BASE, STM32_DDR_SIZE,
+   DCACHE_DEFAULT_OPTION);
+}
+
+void spl_board_prepare_for_boot(void)
+{
+   dcache_disable();
+   debug("SPL bye\n");
+}
+
+void spl_board_prepare_for_boot_linux(void)
+{
+   dcache_disable();
+   debug("SPL bye\n");
 }
-- 
2.17.1



[PATCH v2 1/2] arm: stm32mp: activate data cache in SPL and before relocation

2020-04-03 Thread Patrick Delaunay
Activate the data cache in SPL and in U-Boot before relocation.

In arch_cpu_init(), the function early_enable_caches() sets the early
TLB, early_tlb[] located .init section, and set cacheable:
- for SPL, all the SYSRAM
- for U-Boot, all the DDR

After relocation, the function enable_caches() (called by board_r)
reconfigures the MMU with new TLB location (reserved in
board_f.c::reserve_mmu) and re-enable the data cache.

This patch allows to reduce the execution time, particularly
- for the device tree parsing in U-Boot pre-reloc stage
  (dm_extended_scan_fd =>dm_scan_fdt)
- in I2C timing computation in SPL (stm32_i2c_choose_solution())

For example, the result on STM32MP157C-DK2 board is:
   1,6s gain for trusted boot chain with TF-A
   2,2s gain for basic boot chain with SPL

As TLB is added in .data section, the binary size increased and
the SPL load time by ROM code increased (30ms on DK2).

Signed-off-by: Patrick Delaunay 
---

Changes in v2:
- create a new function early_enable_caches
- use TLB in .init section
- use the default weak dram_bank_mmu_setup() and
  use mmu_set_region_dcache_behaviour() to setup
  the early MMU configuration
- enable data cache on DDR in SPL, after DDR controller initialization

 arch/arm/mach-stm32mp/cpu.c | 43 -
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-stm32mp/cpu.c b/arch/arm/mach-stm32mp/cpu.c
index 36a9205819..c22c1a9bbc 100644
--- a/arch/arm/mach-stm32mp/cpu.c
+++ b/arch/arm/mach-stm32mp/cpu.c
@@ -75,6 +75,12 @@
 #define PKG_SHIFT  27
 #define PKG_MASK   GENMASK(2, 0)
 
+/*
+ * early TLB into the .data section so that it not get cleared
+ * with 16kB allignment (see TTBR0_BASE_ADDR_MASK)
+ */
+u8 early_tlb[PGTABLE_SIZE] __section(".data") __aligned(0x4000);
+
 #if !defined(CONFIG_SPL) || defined(CONFIG_SPL_BUILD)
 #ifndef CONFIG_STM32MP1_TRUSTED
 static void security_init(void)
@@ -186,6 +192,32 @@ u32 get_bootmode(void)
TAMP_BOOT_MODE_SHIFT;
 }
 
+/*
+ * initialize the MMU and activate cache in SPL or in U- Boot pre-reloc stage
+ * MMU/TLB is updated in enable_caches() for U-Boot after relocation
+ * or is deactivated in U-Boot entry function start.S::cpu_init_cp15
+ */
+static void early_enable_caches(void)
+{
+   /* I-cache is already enabled in start.S: cpu_init_cp15 */
+
+   if (CONFIG_IS_ENABLED(SYS_DCACHE_OFF))
+   return;
+
+   gd->arch.tlb_size = PGTABLE_SIZE;
+   gd->arch.tlb_addr = (unsigned long)_tlb;
+
+   dcache_enable();
+
+   if (IS_ENABLED(CONFIG_SPL_BUILD))
+   mmu_set_region_dcache_behaviour(STM32_SYSRAM_BASE,
+   STM32_SYSRAM_SIZE,
+   DCACHE_DEFAULT_OPTION);
+   else
+   mmu_set_region_dcache_behaviour(STM32_DDR_BASE, STM32_DDR_SIZE,
+   DCACHE_DEFAULT_OPTION);
+}
+
 /*
  * Early system init
  */
@@ -193,6 +225,8 @@ int arch_cpu_init(void)
 {
u32 boot_mode;
 
+   early_enable_caches();
+
/* early armv7 timer init: needed for polling */
timer_init();
 
@@ -225,7 +259,14 @@ int arch_cpu_init(void)
 
 void enable_caches(void)
 {
-   /* Enable D-cache. I-cache is already enabled in start.S */
+   /* I-cache is already enabled in start.S: icache_enable() not needed */
+
+   /* deactivate the data cache, early enabled in arch_cpu_init() */
+   dcache_disable();
+   /*
+* update MMU after relocation and enable the data cache
+* warning: the TLB location udpated in board_f.c::reserve_mmu
+*/
dcache_enable();
 }
 
-- 
2.17.1



Antwort: [PATCH v3] serial: ns16550: Revert "Move PCI access from ofdata_to_platdata() to probe()"

2020-04-03 Thread Wolfgang Wallner
Hi Andy,

-"Andy Shevchenko"  schrieb: -

The commit 720f9e1fdb0c ("Move PCI access from ofdata_to_platdata() to
probe()") while doing formally a right thing, actually brings a regression
to the drivers that would like to pre-initialize hardware before calling
ns16550_serial_probe(). In particular, the code, which gets moved out,
is responsible for getting the address of the hardware.

The commit breaks serial console on the Intel Edison, for example.

Since we are very close to the release, the quick fix is to revert the
culprit commit 720f9e1fdb0c92d3fd16e1bfc25bcbd35612675c.

Note, it doesn't affect SoCFPGA case.

Cc: Wolfgang Wallner 
Signed-off-by: Andy Shevchenko 
---
v3: drop wrong patch, better explanation in the commit message
 drivers/serial/ns16550.c | 40 
 1 file changed, 12 insertions(+), 28 deletions(-)

Reviewed-by: Wolfgang Wallner 




Re: [PATCH v3] serial: ns16550: Revert "Move PCI access from ofdata_to_platdata() to probe()"

2020-04-03 Thread Andy Shevchenko
On Fri, Apr 3, 2020 at 11:40 AM Andy Shevchenko
 wrote:
>
> The commit 720f9e1fdb0c ("Move PCI access from ofdata_to_platdata() to
> probe()") while doing formally a right thing, actually brings a regression
> to the drivers that would like to pre-initialize hardware before calling
> ns16550_serial_probe(). In particular, the code, which gets moved out,
> is responsible for getting the address of the hardware.
>
> The commit breaks serial console on the Intel Edison, for example.
>
> Since we are very close to the release, the quick fix is to revert the
> culprit commit 720f9e1fdb0c92d3fd16e1bfc25bcbd35612675c.
>
> Note, it doesn't affect SoCFPGA case.
>

As mentioned in another email, I'll glad to test a better solution if
it pops up in time.

> Cc: Wolfgang Wallner 
> Signed-off-by: Andy Shevchenko 
> ---
> v3: drop wrong patch, better explanation in the commit message
>  drivers/serial/ns16550.c | 40 
>  1 file changed, 12 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
> index c1b303ffcb..1fcbc35015 100644
> --- a/drivers/serial/ns16550.c
> +++ b/drivers/serial/ns16550.c
> @@ -479,40 +479,12 @@ static int ns16550_serial_getinfo(struct udevice *dev,
> return 0;
>  }
>
> -#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
> -static int ns1655_serial_set_base_addr(struct udevice *dev)
> -{
> -   fdt_addr_t addr;
> -   struct ns16550_platdata *plat;
> -
> -   plat = dev_get_platdata(dev);
> -
> -   addr = dev_read_addr_pci(dev);
> -   if (addr == FDT_ADDR_T_NONE)
> -   return -EINVAL;
> -
> -#ifdef CONFIG_SYS_NS16550_PORT_MAPPED
> -   plat->base = addr;
> -#else
> -   plat->base = (unsigned long)map_physmem(addr, 0, MAP_NOCACHE);
> -#endif
> -
> -   return 0;
> -}
> -#endif
> -
>  int ns16550_serial_probe(struct udevice *dev)
>  {
> struct NS16550 *const com_port = dev_get_priv(dev);
> struct reset_ctl_bulk reset_bulk;
> int ret;
>
> -#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
> -   ret = ns1655_serial_set_base_addr(dev);
> -   if (ret)
> -   return ret;
> -#endif
> -
> ret = reset_get_bulk(dev, _bulk);
> if (!ret)
> reset_deassert_bulk(_bulk);
> @@ -535,9 +507,21 @@ int ns16550_serial_ofdata_to_platdata(struct udevice 
> *dev)
>  {
> struct ns16550_platdata *plat = dev->platdata;
> const u32 port_type = dev_get_driver_data(dev);
> +   fdt_addr_t addr;
> struct clk clk;
> int err;
>
> +   /* try Processor Local Bus device first */
> +   addr = dev_read_addr_pci(dev);
> +   if (addr == FDT_ADDR_T_NONE)
> +   return -EINVAL;
> +
> +#ifdef CONFIG_SYS_NS16550_PORT_MAPPED
> +   plat->base = addr;
> +#else
> +   plat->base = (unsigned long)map_physmem(addr, 0, MAP_NOCACHE);
> +#endif
> +
> plat->reg_offset = dev_read_u32_default(dev, "reg-offset", 0);
> plat->reg_shift = dev_read_u32_default(dev, "reg-shift", 0);
> plat->reg_width = dev_read_u32_default(dev, "reg-io-width", 1);
> --
> 2.25.1
>


-- 
With Best Regards,
Andy Shevchenko


Re: Re: [PATCH v2 1/2] serial: ns16550: Revert "Move PCI access from ofdata_to_platdata() to probe()"

2020-04-03 Thread Andy Shevchenko
On Fri, Apr 3, 2020 at 11:35 AM Bin Meng  wrote:
> On Fri, Apr 3, 2020 at 4:26 PM Wolfgang Wallner
>  wrote:
> > > -"Andy Shevchenko"  schrieb: -
> > > On Thu, Apr 2, 2020 at 7:55 AM Bin Meng  wrote:

...

> > > I think I understand what happened, and Wolfgang's patch *is* a culprit.
> > >
> > > In serial_intel_mid.c we setup a divisor before probing the actual
> > > device. Now, since the address retrieving has been moved further in
> > > the initialization, we are writing to unknown registers and thus don't
> > > properly initialize hardware.
> >
> > Yes, you are right, mid_serial_probe() relies on plat->base being set by
> > ns16550_serial_ofdata_to_platdata().
> >
> > I was not aware that other drivers rely on ns16550 in this way.
> > And now that I know I see that there are few more:
> >
> >   $ grep -r -l --include='*.c' -e ns16550_serial_probe -e 
> > ns16550_serial_ofdata_to_platdata
> >   drivers/serial/serial_intel_mid.c
> >   drivers/serial/serial_rockchip.c
> >   drivers/serial/serial_omap.c
> >   drivers/serial/ns16550.c
> >   arch/x86/cpu/apollolake/uart.c
> >   arch/x86/cpu/slimbootloader/serial.c
> >
> > This means my patch has a wider impact than what I have taken care of.
> >
> > @Bin: I'm fine with reverting 720f9e1 for now. I will come back with a
> > new revision of this patch when I had the time to look at the other impacted
> > drivers.
> >
> > But reverting 720f9e1 is IMHO only a temporary workaround, as the underlying
> > problem (possible PCI access in ofdata_to_platdata()) should be fixed 
> > anyway,
> > at least my interpretation of the comment in drivers/pci/pci-uclass.c is 
> > that
> > this PCI access should not be there:
> >
> > "A common cause of this problem is that this function is called in the
> > ofdata_to_platdata() method of @dev. Accessing the PCI bus in that
> > method is not allowed, since it has not yet been probed. To fix this,
> > move that access to the probe() method of @dev instead."
> >
>
> Yes, when I looked at this, I wonder why the first commit was
> introduced. Simon, could you please explain more about the DM changes
> below?
>
> commit 82de42fa14682d408da935adfb0f935354c5008f
> Author: Simon Glass 
> Date:   Sun Dec 29 21:19:18 2019 -0700
>
> dm: core: Allocate parent data separate from probing parent
>
> At present the parent is probed before the child's ofdata_to_platdata()
> method is called. Adjust the logic slightly so that probing parents is
> not done until afterwards.
>
> Signed-off-by: Simon Glass 
>
> > @Andy: Regarding serial_intel_mid: I don't know the hardware, but would it 
> > be
> > possible to move the register accesses after the call to
> > ns16550_serial_probe()?
> >
>
> I suspect this does not work.

I didn't check the details, but have same feelings.

> >mid_writel(plat, UART_MUL, 96);
> >mid_writel(plat, UART_DIV, 125);
> >mid_writel(plat, UART_PS, 16);
> >
> >return ns16550_serial_probe(dev);
> >
> > > So, the proper fix would be in my opinion to introduce a call back or
> > > some other way to make ordering possible, like registering hw_init
> > > callback in probe which will be called in the ns16550, or even do this
> > > as a callback for all serial class drivers.
> > >
> > > I don't dare to dive into this anticipating that crap will hit the fan.
> > > So, please, who is knowing serial code, fix this asap!
> >
>
> I am currently working on a patch. Will send out for Andy and you for
> testing soon.

I just sent a v3 of revert, but I'll glad to test better solution.

-- 
With Best Regards,
Andy Shevchenko


[PATCH v3] serial: ns16550: Revert "Move PCI access from ofdata_to_platdata() to probe()"

2020-04-03 Thread Andy Shevchenko
The commit 720f9e1fdb0c ("Move PCI access from ofdata_to_platdata() to
probe()") while doing formally a right thing, actually brings a regression
to the drivers that would like to pre-initialize hardware before calling
ns16550_serial_probe(). In particular, the code, which gets moved out,
is responsible for getting the address of the hardware.

The commit breaks serial console on the Intel Edison, for example.

Since we are very close to the release, the quick fix is to revert the
culprit commit 720f9e1fdb0c92d3fd16e1bfc25bcbd35612675c.

Note, it doesn't affect SoCFPGA case.

Cc: Wolfgang Wallner 
Signed-off-by: Andy Shevchenko 
---
v3: drop wrong patch, better explanation in the commit message
 drivers/serial/ns16550.c | 40 
 1 file changed, 12 insertions(+), 28 deletions(-)

diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
index c1b303ffcb..1fcbc35015 100644
--- a/drivers/serial/ns16550.c
+++ b/drivers/serial/ns16550.c
@@ -479,40 +479,12 @@ static int ns16550_serial_getinfo(struct udevice *dev,
return 0;
 }
 
-#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
-static int ns1655_serial_set_base_addr(struct udevice *dev)
-{
-   fdt_addr_t addr;
-   struct ns16550_platdata *plat;
-
-   plat = dev_get_platdata(dev);
-
-   addr = dev_read_addr_pci(dev);
-   if (addr == FDT_ADDR_T_NONE)
-   return -EINVAL;
-
-#ifdef CONFIG_SYS_NS16550_PORT_MAPPED
-   plat->base = addr;
-#else
-   plat->base = (unsigned long)map_physmem(addr, 0, MAP_NOCACHE);
-#endif
-
-   return 0;
-}
-#endif
-
 int ns16550_serial_probe(struct udevice *dev)
 {
struct NS16550 *const com_port = dev_get_priv(dev);
struct reset_ctl_bulk reset_bulk;
int ret;
 
-#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
-   ret = ns1655_serial_set_base_addr(dev);
-   if (ret)
-   return ret;
-#endif
-
ret = reset_get_bulk(dev, _bulk);
if (!ret)
reset_deassert_bulk(_bulk);
@@ -535,9 +507,21 @@ int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
 {
struct ns16550_platdata *plat = dev->platdata;
const u32 port_type = dev_get_driver_data(dev);
+   fdt_addr_t addr;
struct clk clk;
int err;
 
+   /* try Processor Local Bus device first */
+   addr = dev_read_addr_pci(dev);
+   if (addr == FDT_ADDR_T_NONE)
+   return -EINVAL;
+
+#ifdef CONFIG_SYS_NS16550_PORT_MAPPED
+   plat->base = addr;
+#else
+   plat->base = (unsigned long)map_physmem(addr, 0, MAP_NOCACHE);
+#endif
+
plat->reg_offset = dev_read_u32_default(dev, "reg-offset", 0);
plat->reg_shift = dev_read_u32_default(dev, "reg-shift", 0);
plat->reg_width = dev_read_u32_default(dev, "reg-io-width", 1);
-- 
2.25.1



Re: Re: [PATCH v2 1/2] serial: ns16550: Revert "Move PCI access from ofdata_to_platdata() to probe()"

2020-04-03 Thread Bin Meng
Hi Wolfgang,

On Fri, Apr 3, 2020 at 4:26 PM Wolfgang Wallner
 wrote:
>
>
> Hi Andy, Bin,
>
> > -"Andy Shevchenko"  schrieb: -
> > On Thu, Apr 2, 2020 at 7:55 AM Bin Meng  wrote:
> > > On Thu, Apr 2, 2020 at 1:55 AM Simon Glass  wrote:
> > > > On Wed, 1 Apr 2020 at 11:39, Andy Shevchenko 
> > > >  wrote:
> > > > > On Wed, Apr 01, 2020 at 10:56:26AM -0600, Simon Glass wrote:
> > > > > > On Wed, 1 Apr 2020 at 08:45, Andy Shevchenko 
> > > > > >  wrote:
> > > > > > > On Wed, Apr 1, 2020 at 5:32 PM Bin Meng  
> > > > > > > wrote:
> > > > > > > > On Wed, Apr 1, 2020 at 9:58 PM Andy Shevchenko
> > > > > > > >  wrote:
> > > > > > > > >
> > > > > > > > > The commit breaks serial console on the Intel Edison.
> > > > > > > > >
> > > > > > > > > This reverts commit 720f9e1fdb0c92d3fd16e1bfc25bcbd35612675c.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Andy Shevchenko 
> > > > > > > > > 
> > > > > > > > > ---
> > > > > > > > >  drivers/serial/ns16550.c | 40 
> > > > > > > > > 
> > > > > > > > >  1 file changed, 12 insertions(+), 28 deletions(-)
> > > > > > > > >
> > > > > > > >
> > > > > > > > Could you please spend some time to investigate why this breaks 
> > > > > > > > Intel Edison?
> > > > > > > >
> > > > > > > > Reverting this patch would mean we break other boards too as
> > > > > > > > Wolfgang's patch wanted to fix the breakage in the first place. 
> > > > > > > > Much
> > > > > > > > appreciated!
> > > > > > >
> > > > > > > I guess I'm wrong person here. The DM code is a complete black 
> > > > > > > box to me.
> > > > > > > Nevertheless, I may test any provided fix / debug / etc patch by 
> > > > > > > request.
> > > > > > >
> > > > > > > And I think it's fair to investigate by the one who made a 
> > > > > > > regression
> > > > > > > in the first place.
> > > > > >
> > > > > > Given that we have conflicting breakages, we need to debug Edison.
> > > > >
> > > > > I would glad to test any suggested change or debug patch!
> > > > >
> > > > > > Does it enable the debug UART?
> > > > >
> > > > > If I am not mistaken, it does not.
> > > >
> > > > Looks like you are right. If you know the address you could do that -
> > > > see minnowmax for an example.
> > >
> > > Please suggest what's the best approach to proceed.
> >
> > I think I understand what happened, and Wolfgang's patch *is* a culprit.
> >
> > In serial_intel_mid.c we setup a divisor before probing the actual
> > device. Now, since the address retrieving has been moved further in
> > the initialization, we are writing to unknown registers and thus don't
> > properly initialize hardware.
>
> Yes, you are right, mid_serial_probe() relies on plat->base being set by
> ns16550_serial_ofdata_to_platdata().
>
> I was not aware that other drivers rely on ns16550 in this way.
> And now that I know I see that there are few more:
>
>   $ grep -r -l --include='*.c' -e ns16550_serial_probe -e 
> ns16550_serial_ofdata_to_platdata
>   drivers/serial/serial_intel_mid.c
>   drivers/serial/serial_rockchip.c
>   drivers/serial/serial_omap.c
>   drivers/serial/ns16550.c
>   arch/x86/cpu/apollolake/uart.c
>   arch/x86/cpu/slimbootloader/serial.c
>
> This means my patch has a wider impact than what I have taken care of.
>
> @Bin: I'm fine with reverting 720f9e1 for now. I will come back with a
> new revision of this patch when I had the time to look at the other impacted
> drivers.
>
> But reverting 720f9e1 is IMHO only a temporary workaround, as the underlying
> problem (possible PCI access in ofdata_to_platdata()) should be fixed anyway,
> at least my interpretation of the comment in drivers/pci/pci-uclass.c is that
> this PCI access should not be there:
>
> "A common cause of this problem is that this function is called in the
> ofdata_to_platdata() method of @dev. Accessing the PCI bus in that
> method is not allowed, since it has not yet been probed. To fix this,
> move that access to the probe() method of @dev instead."
>

Yes, when I looked at this, I wonder why the first commit was
introduced. Simon, could you please explain more about the DM changes
below?

commit 82de42fa14682d408da935adfb0f935354c5008f
Author: Simon Glass 
Date:   Sun Dec 29 21:19:18 2019 -0700

dm: core: Allocate parent data separate from probing parent

At present the parent is probed before the child's ofdata_to_platdata()
method is called. Adjust the logic slightly so that probing parents is
not done until afterwards.

Signed-off-by: Simon Glass 

> @Andy: Regarding serial_intel_mid: I don't know the hardware, but would it be
> possible to move the register accesses after the call to
> ns16550_serial_probe()?
>

I suspect this does not work.

>mid_writel(plat, UART_MUL, 96);
>mid_writel(plat, UART_DIV, 125);
>mid_writel(plat, UART_PS, 16);
>
>return ns16550_serial_probe(dev);
>
> > So, the proper fix would be in my opinion to introduce a call back or
> > some other way to make ordering 

Re: XHCI bringup on the Raspberry Pi 4

2020-04-03 Thread Peter Robinson
> > I'm working on enabling the VIA805 XCHI controller found on the new 
> > Raspberry
> > Pi 4. The controller sits behind a PCIe bus, which I've already 
> > implemented[1]
> > and will submit once the XCHI issues are resolved, as it's worthless 
> > otherwise.
> >
> > The XHCI initialization gets stuck after issuing the fist 'enable slot'
> > command. I've been reviewing the whole init process and comparing it to 
> > Linux's
> > for days without finding anything fishy. DMA constraints, which are 
> > important
> > on the RPi are mantained, and on top of that, I can confirm DMA trough PCIe
> > works fine as I see two 'port status change' events available in the ring
> > before completly stalling. Also note that, unsurprisingly, the CRR bit in 
> > the
> > CRCR register (which marks the running state of the Command Ring state 
> > machine)
> > is never set after ringing the relevant doorbell.
>
> This is probably caused by the required structure setup by U-Boot is
> viewed as buggy by the xHCI controller, hence there is no response to
> the first "enable slot" command.
>
> Could you please compare all the data structures, with the one set up
> by the Linux kernel?
>
> BTW: does Linux kernel xHCI driver work on this RPI 4 board?

Yes, it uses the standard xhci-hcd controller driver, it's a fairly
vanilla VIA controller.


[PATCH 2/3] arm: caches: add DCACHE_DEFAULT_OPTION

2020-04-03 Thread Patrick Delaunay
Add the new flags DCACHE_DEFAULT_OPTION to define the default
option to use according the compilation flags
CONFIG_SYS_ARM_CACHE_WRITETHROUGH or CONFIG_SYS_ARM_CACHE_WRITEALLOC.

This new compilation flag allows to simplify dram_bank_mmu_setup()
and can be used as third parameter (option=dcache option to select)
of mmu_set_region_dcache_behaviour function.

Signed-off-by: Patrick Delaunay 
---

 arch/arm/include/asm/system.h |  8 
 arch/arm/lib/cache-cp15.c | 11 ++-
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
index 81ccead112..01ea96e8ad 100644
--- a/arch/arm/include/asm/system.h
+++ b/arch/arm/include/asm/system.h
@@ -485,6 +485,14 @@ enum dcache_option {
 };
 #endif
 
+#if defined(CONFIG_SYS_ARM_CACHE_WRITETHROUGH)
+#define DCACHE_DEFAULT_OPTION  DCACHE_WRITETHROUGH
+#elif defined(CONFIG_SYS_ARM_CACHE_WRITEALLOC)
+#define DCACHE_DEFAULT_OPTION  DCACHE_WRITEALLOC
+#else
+#define DCACHE_DEFAULT_OPTION  DCACHE_WRITEBACK
+#endif
+
 /* Size of an MMU section */
 enum {
 #ifdef CONFIG_ARMV7_LPAE
diff --git a/arch/arm/lib/cache-cp15.c b/arch/arm/lib/cache-cp15.c
index 54509f11c3..d15144188b 100644
--- a/arch/arm/lib/cache-cp15.c
+++ b/arch/arm/lib/cache-cp15.c
@@ -99,15 +99,8 @@ __weak void dram_bank_mmu_setup(int bank)
for (i = bd->bi_dram[bank].start >> MMU_SECTION_SHIFT;
 i < (bd->bi_dram[bank].start >> MMU_SECTION_SHIFT) +
 (bd->bi_dram[bank].size >> MMU_SECTION_SHIFT);
-i++) {
-#if defined(CONFIG_SYS_ARM_CACHE_WRITETHROUGH)
-   set_section_dcache(i, DCACHE_WRITETHROUGH);
-#elif defined(CONFIG_SYS_ARM_CACHE_WRITEALLOC)
-   set_section_dcache(i, DCACHE_WRITEALLOC);
-#else
-   set_section_dcache(i, DCACHE_WRITEBACK);
-#endif
-   }
+i++)
+   set_section_dcache(i, DCACHE_DEFAULT_OPTION);
 }
 
 /* to activate the MMU we need to set up virtual memory: use 1M areas */
-- 
2.17.1



[PATCH 3/3] arm: caches: manage phys_addr_t overflow in mmu_set_region_dcache_behaviour

2020-04-03 Thread Patrick Delaunay
Detect and solve the overflow on phys_addr_t type for start + size in
mmu_set_region_dcache_behaviour() function.

This issue occurs for example with ARM32, start = 0xC000 and
size = 0x4000: start + size = 0x1 and end = 0x0.

Overflow is detected when end < start.
In normal case the previous behavior is still used: when start is not
aligned on MMU section, the end address is only aligned after the sum
start + size.

Signed-off-by: Patrick Delaunay 
---

 arch/arm/lib/cache-cp15.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/lib/cache-cp15.c b/arch/arm/lib/cache-cp15.c
index d15144188b..e5a7fd0ef4 100644
--- a/arch/arm/lib/cache-cp15.c
+++ b/arch/arm/lib/cache-cp15.c
@@ -63,6 +63,11 @@ void mmu_set_region_dcache_behaviour(phys_addr_t start, 
size_t size,
 
end = ALIGN(start + size, MMU_SECTION_SIZE) >> MMU_SECTION_SHIFT;
start = start >> MMU_SECTION_SHIFT;
+
+   /* phys_addr_t overflow detected */
+   if (end < start)
+   end = (~(phys_addr_t)0x0 >> MMU_SECTION_SHIFT) + 1;
+
 #ifdef CONFIG_ARMV7_LPAE
debug("%s: start=%pa, size=%zu, option=%llx\n", __func__, , size,
  option);
-- 
2.17.1



[PATCH 1/3] arm: caches: protect dram_bank_mmu_setup access to bi_dram

2020-04-03 Thread Patrick Delaunay
Add protection in dram_bank_mmu_setup() to avoid access to bd->bi_dram
before relocation.

This patch allow to use the generic weak function dram_bank_mmu_setup
to activate the MMU and the data cache in SPL or in U-Boot before
relocation, when bd->bi_dram is not yet initialized.

In this cases, the MMU must be initialized explicitly with
mmu_set_region_dcache_behaviour function.

Signed-off-by: Patrick Delaunay 
---

 arch/arm/lib/cache-cp15.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/lib/cache-cp15.c b/arch/arm/lib/cache-cp15.c
index f8d20960da..54509f11c3 100644
--- a/arch/arm/lib/cache-cp15.c
+++ b/arch/arm/lib/cache-cp15.c
@@ -91,6 +91,10 @@ __weak void dram_bank_mmu_setup(int bank)
bd_t *bd = gd->bd;
int i;
 
+   /* bd->bi_dram is available only after relocation */
+   if ((gd->flags & GD_FLG_RELOC) == 0)
+   return;
+
debug("%s: bank: %d\n", __func__, bank);
for (i = bd->bi_dram[bank].start >> MMU_SECTION_SHIFT;
 i < (bd->bi_dram[bank].start >> MMU_SECTION_SHIFT) +
-- 
2.17.1



Antwort: Re: [PATCH v2 1/2] serial: ns16550: Revert "Move PCI access from ofdata_to_platdata() to probe()"

2020-04-03 Thread Wolfgang Wallner


Hi Andy, Bin,

> -"Andy Shevchenko"  schrieb: -
> On Thu, Apr 2, 2020 at 7:55 AM Bin Meng  wrote:
> > On Thu, Apr 2, 2020 at 1:55 AM Simon Glass  wrote:
> > > On Wed, 1 Apr 2020 at 11:39, Andy Shevchenko  
> > > wrote:
> > > > On Wed, Apr 01, 2020 at 10:56:26AM -0600, Simon Glass wrote:
> > > > > On Wed, 1 Apr 2020 at 08:45, Andy Shevchenko 
> > > > >  wrote:
> > > > > > On Wed, Apr 1, 2020 at 5:32 PM Bin Meng  wrote:
> > > > > > > On Wed, Apr 1, 2020 at 9:58 PM Andy Shevchenko
> > > > > > >  wrote:
> > > > > > > >
> > > > > > > > The commit breaks serial console on the Intel Edison.
> > > > > > > >
> > > > > > > > This reverts commit 720f9e1fdb0c92d3fd16e1bfc25bcbd35612675c.
> > > > > > > >
> > > > > > > > Signed-off-by: Andy Shevchenko 
> > > > > > > > 
> > > > > > > > ---
> > > > > > > >  drivers/serial/ns16550.c | 40 
> > > > > > > > 
> > > > > > > >  1 file changed, 12 insertions(+), 28 deletions(-)
> > > > > > > >
> > > > > > >
> > > > > > > Could you please spend some time to investigate why this breaks 
> > > > > > > Intel Edison?
> > > > > > >
> > > > > > > Reverting this patch would mean we break other boards too as
> > > > > > > Wolfgang's patch wanted to fix the breakage in the first place. 
> > > > > > > Much
> > > > > > > appreciated!
> > > > > >
> > > > > > I guess I'm wrong person here. The DM code is a complete black box 
> > > > > > to me.
> > > > > > Nevertheless, I may test any provided fix / debug / etc patch by 
> > > > > > request.
> > > > > >
> > > > > > And I think it's fair to investigate by the one who made a 
> > > > > > regression
> > > > > > in the first place.
> > > > >
> > > > > Given that we have conflicting breakages, we need to debug Edison.
> > > >
> > > > I would glad to test any suggested change or debug patch!
> > > >
> > > > > Does it enable the debug UART?
> > > >
> > > > If I am not mistaken, it does not.
> > >
> > > Looks like you are right. If you know the address you could do that -
> > > see minnowmax for an example.
> >
> > Please suggest what's the best approach to proceed.
> 
> I think I understand what happened, and Wolfgang's patch *is* a culprit.
> 
> In serial_intel_mid.c we setup a divisor before probing the actual
> device. Now, since the address retrieving has been moved further in
> the initialization, we are writing to unknown registers and thus don't
> properly initialize hardware.

Yes, you are right, mid_serial_probe() relies on plat->base being set by
ns16550_serial_ofdata_to_platdata().

I was not aware that other drivers rely on ns16550 in this way.
And now that I know I see that there are few more:

  $ grep -r -l --include='*.c' -e ns16550_serial_probe -e 
ns16550_serial_ofdata_to_platdata
  drivers/serial/serial_intel_mid.c
  drivers/serial/serial_rockchip.c
  drivers/serial/serial_omap.c
  drivers/serial/ns16550.c
  arch/x86/cpu/apollolake/uart.c
  arch/x86/cpu/slimbootloader/serial.c

This means my patch has a wider impact than what I have taken care of.

@Bin: I'm fine with reverting 720f9e1 for now. I will come back with a
new revision of this patch when I had the time to look at the other impacted
drivers.

But reverting 720f9e1 is IMHO only a temporary workaround, as the underlying
problem (possible PCI access in ofdata_to_platdata()) should be fixed anyway,
at least my interpretation of the comment in drivers/pci/pci-uclass.c is that
this PCI access should not be there:

"A common cause of this problem is that this function is called in the
ofdata_to_platdata() method of @dev. Accessing the PCI bus in that
method is not allowed, since it has not yet been probed. To fix this,
move that access to the probe() method of @dev instead."

@Andy: Regarding serial_intel_mid: I don't know the hardware, but would it be
possible to move the register accesses after the call to
ns16550_serial_probe()?

   mid_writel(plat, UART_MUL, 96);
   mid_writel(plat, UART_DIV, 125);
   mid_writel(plat, UART_PS, 16);

   return ns16550_serial_probe(dev);

> So, the proper fix would be in my opinion to introduce a call back or
> some other way to make ordering possible, like registering hw_init
> callback in probe which will be called in the ns16550, or even do this
> as a callback for all serial class drivers.
> 
> I don't dare to dive into this anticipating that crap will hit the fan.
> So, please, who is knowing serial code, fix this asap!

regards, Wolfgang



Re: [PATCH v2 1/2] serial: ns16550: Revert "Move PCI access from ofdata_to_platdata() to probe()"

2020-04-03 Thread Andy Shevchenko
On Fri, Apr 3, 2020 at 10:56 AM Ang, Chee Hong  wrote:
> > On Fri, Apr 3, 2020 at 6:56 AM Ang, Chee Hong 
> > wrote:
> > >
> > > > On Thu, Apr 2, 2020 at 7:28 PM Ang, Chee Hong
> > > > 
> > > > wrote:
> > > > > > On Thu, Apr 02, 2020 at 12:55:14PM +0800, Bin Meng wrote:
> > > > > > > On Thu, Apr 2, 2020 at 1:55 AM Simon Glass 
> > wrote:
> > > > > > > > On Wed, 1 Apr 2020 at 11:39, Andy Shevchenko
> >
> > > > > > > > > > > > > This reverts commit
> > > > 720f9e1fdb0c92d3fd16e1bfc25bcbd35612675c.
> >
> > > > > > Adding SoCFPGA folks to this thread as the first commit
> > > > > > (82de42fa1468) is also breaking platforms there and then their
> > > > > > fix for that problem is also causing problems, if I follow right.
> > > >
> > > > > Yes. This commit (82de42fa1468) breaks our A10 platform.
> > > > > I did submit similar patch to move some init code from
> > > > > ofdata_to_platdata() to
> > > > probe() for our clock driver as well.
> > > > > Check out the thread here:
> > > > > https://lists.denx.de/pipermail/u-boot/2020-April/405252.html
> > > >
> > > > I see half or less of the messages in the thread by above link.
> > > > Can you summarize what should have been done in order to fix this?
> > > 1) Fix in the driver (broken by this commit: 82de42fa1468) by moving some
> > code from ofdata_to_platdata() to probe().
> > > 2) Fix the DM core.
> > > Simon and Marek were discussing about these 2 options.
> > > Perhaps Simon can help shed some light here.
> >
> > I have a question. Does revert of
> > 720f9e1fdb0c92d3fd16e1bfc25bcbd35612675c makes any difference to
> > SoCFPGA?
> This commit: 82de42fa1468 break our SoCFPGA A10 clock driver.
> This commit 720f9e1fdb0c92d3fd16e1bfc25bcbd35612675c does not affect our 
> SoCFPGA platforms.
> We just want to know which way to go. Fixing our A10 clock driver or fix the 
> DM core.
> Seems like this commit 720f9e1fdb0c92d3fd16e1bfc25bcbd35612675c go with 
> option 1 (fixing the driver).

I dunno what 720f9e is actually fixing, it breaks definitely the ordering.

So, I'm going to send a v3 of revert of 720f9e since it is not related
to SoCFPGA.

-- 
With Best Regards,
Andy Shevchenko


RE: [PATCH] arm: stm32mp1: activate data cache in SPL and before relocation

2020-04-03 Thread Patrick DELAUNAY
Hi Marek,

> From: Marek Vasut 
> Sent: lundi 30 mars 2020 16:04
> 
> On 3/30/20 3:49 PM, Patrick DELAUNAY wrote:
> > Hi Marek,
> 
> Hi,
> 
> [...]
> 
> >>> - /* Enable D-cache. I-cache is already enabled in start.S */
> >>> + /* I-cache is already enabled in start.S */
> >
> > Not needed for arm V7 (I copy this function from other platfrom ... I
> > don't remember which one)
> 
> Maybe this needs to be de-duplicated if it's a copy ?

I don't remember 
As I mixed several references

But I found the same content in many arm arch;

arch/arm/mach-imx/mx5/soc.c:67
arch/arm/mach-rockchip/board.c:47
arch/arm/mach-tegra/board.c:271
arch/arm/mach-sunxi/board.c:347
arch/arm/mach-exynos/soc.c:30:
arch/arm/mach-zynq/cpu.c:88:
arch/arm/cpu/armv7/iproc-common/hwinit-common.c:1
arch/arm/mach-u8500/cache.c:14
arch/arm/mach-keystone/init.c:206

And different implementation in 
arch/arm/mach-socfpga/misc.c:55

mach-omap2/omap-cache.c:49
void enable_caches(void)
{

/* Enable I cache if not enabled */
if (!icache_status())
icache_enable();

dcache_enable();
}

the issue the weak function empty, so it is mandatory to
redefine the real implementation for each platform.

arch/arm/lib/cache.c:35
/*
 * Default implementation of enable_caches()
 * Real implementation should be in platform code
 */
__weak void enable_caches(void)
{
puts("WARNING: Caches not enabled\n");
}

[...]

> >>>
> >>> +static void set_mmu_dcache(u32 addr, u32 size) {
> >>> + int i;
> >>> + u32 start, end;
> >>> +
> >>> + start = addr >> MMU_SECTION_SHIFT;
> >>> + end = ((u64)addr + (u64)size) >> MMU_SECTION_SHIFT;
> >>
> >> Is this a copy of dram_bank_mmu_setup() in arch/arm/lib/cache-cp15.c ?
> >> Why ?
> >
> > It is not just a copy...
> >
> > set__mmu_dache is only a static helper for  function
> > dram_bank_mmu_setup()
> >
> > I override the default implementation of the weak functon
> > dram_bank_mmu_setup()
> 
> Can you instead augment the original implementation to cater for this usecase 
> or
> would that be too difficult ?

Have a generic behavior...

I will propose to protect the access to bd->bi_dram[bank] in dram_bank_mmu_setup

[]

> >>
> >>> diff --git a/include/configs/stm32mp1.h b/include/configs/stm32mp1.h
> >>> index c34a720e0c..5203fc93ad 100644
> >>> --- a/include/configs/stm32mp1.h
> >>> +++ b/include/configs/stm32mp1.h
> >>> @@ -58,8 +58,8 @@
> >>>
> >>>  /* limit SYSRAM usage to first 128 KB */
> >>>  #define CONFIG_SPL_MAX_SIZE  0x0002
> >>> -#define CONFIG_SPL_STACK (STM32_SYSRAM_BASE + \
> >>> -  STM32_SYSRAM_SIZE)
> >>> +/* stack at STM32_SYSRAM_BASE + STM32_SYSRAM_SIZE -
> >> PGTABLE_SIZE (=16kB) */
> >>> +#define CONFIG_SPL_STACK 0x2FFFC000
> >>
> >> Can't you memalign() the pagetable area instead of this hacking around?
> >> Or use something around board_init_f_alloc_reserve().
> >
> > It was my first idea: use malloc
> >
> > But as I try to activate the data cache as soon as possible.
> > So before spl_early_init call (for spl in board_init_f) and malloc is not 
> > yet
> accessible.
> >
> > And board_init_f_alloc_reserve  is only called in U-Boot board-f.c.
> > after the MMU configuration for pre-relocation / not called in SPL.
> >
> > I don't see this address as hack but a memory configuration of SYSRAM:
> >
> > SYRAM content (board_f)
> > - SPL code
> > - SPL data
> > - SPL stack (reversed order)
> > - TTB
> >
> > But I can move it in BSS as global apl variable, I need to think about 
> > it
> > It is probably more clean.
> 
> Please do :)

I willl move it in ".data" section in V2 for SPL and U-Boot.

Even in binary size increase and the SPL load time
by ROM code is increase by 30ms.

Patrick


RE: [PATCH v2 1/2] serial: ns16550: Revert "Move PCI access from ofdata_to_platdata() to probe()"

2020-04-03 Thread Ang, Chee Hong
> On Fri, Apr 3, 2020 at 6:56 AM Ang, Chee Hong 
> wrote:
> >
> > > On Thu, Apr 2, 2020 at 7:28 PM Ang, Chee Hong
> > > 
> > > wrote:
> > > > > On Thu, Apr 02, 2020 at 12:55:14PM +0800, Bin Meng wrote:
> > > > > > On Thu, Apr 2, 2020 at 1:55 AM Simon Glass 
> wrote:
> > > > > > > On Wed, 1 Apr 2020 at 11:39, Andy Shevchenko
> 
> > > > > > > > > > > > This reverts commit
> > > 720f9e1fdb0c92d3fd16e1bfc25bcbd35612675c.
> 
> > > > > Adding SoCFPGA folks to this thread as the first commit
> > > > > (82de42fa1468) is also breaking platforms there and then their
> > > > > fix for that problem is also causing problems, if I follow right.
> > >
> > > > Yes. This commit (82de42fa1468) breaks our A10 platform.
> > > > I did submit similar patch to move some init code from
> > > > ofdata_to_platdata() to
> > > probe() for our clock driver as well.
> > > > Check out the thread here:
> > > > https://lists.denx.de/pipermail/u-boot/2020-April/405252.html
> > >
> > > I see half or less of the messages in the thread by above link.
> > > Can you summarize what should have been done in order to fix this?
> > 1) Fix in the driver (broken by this commit: 82de42fa1468) by moving some
> code from ofdata_to_platdata() to probe().
> > 2) Fix the DM core.
> > Simon and Marek were discussing about these 2 options.
> > Perhaps Simon can help shed some light here.
> 
> I have a question. Does revert of
> 720f9e1fdb0c92d3fd16e1bfc25bcbd35612675c makes any difference to
> SoCFPGA?
This commit: 82de42fa1468 break our SoCFPGA A10 clock driver.
This commit 720f9e1fdb0c92d3fd16e1bfc25bcbd35612675c does not affect our 
SoCFPGA platforms.
We just want to know which way to go. Fixing our A10 clock driver or fix the DM 
core.
Seems like this commit 720f9e1fdb0c92d3fd16e1bfc25bcbd35612675c go with option 
1 (fixing the driver).
> 
> --
> With Best Regards,
> Andy Shevchenko


Re: [PATCH v2 1/2] serial: ns16550: Revert "Move PCI access from ofdata_to_platdata() to probe()"

2020-04-03 Thread Andy Shevchenko
On Fri, Apr 3, 2020 at 6:56 AM Ang, Chee Hong  wrote:
>
> > On Thu, Apr 2, 2020 at 7:28 PM Ang, Chee Hong 
> > wrote:
> > > > On Thu, Apr 02, 2020 at 12:55:14PM +0800, Bin Meng wrote:
> > > > > On Thu, Apr 2, 2020 at 1:55 AM Simon Glass  wrote:
> > > > > > On Wed, 1 Apr 2020 at 11:39, Andy Shevchenko

> > > > > > > > > > > This reverts commit
> > 720f9e1fdb0c92d3fd16e1bfc25bcbd35612675c.

> > > > Adding SoCFPGA folks to this thread as the first commit
> > > > (82de42fa1468) is also breaking platforms there and then their fix
> > > > for that problem is also causing problems, if I follow right.
> >
> > > Yes. This commit (82de42fa1468) breaks our A10 platform.
> > > I did submit similar patch to move some init code from 
> > > ofdata_to_platdata() to
> > probe() for our clock driver as well.
> > > Check out the thread here:
> > > https://lists.denx.de/pipermail/u-boot/2020-April/405252.html
> >
> > I see half or less of the messages in the thread by above link.
> > Can you summarize what should have been done in order to fix this?
> 1) Fix in the driver (broken by this commit: 82de42fa1468) by moving some 
> code from ofdata_to_platdata() to probe().
> 2) Fix the DM core.
> Simon and Marek were discussing about these 2 options.
> Perhaps Simon can help shed some light here.

I have a question. Does revert of
720f9e1fdb0c92d3fd16e1bfc25bcbd35612675c makes any difference to
SoCFPGA?

-- 
With Best Regards,
Andy Shevchenko


Re: [PATCH v2 1/2] serial: ns16550: Revert "Move PCI access from ofdata_to_platdata() to probe()"

2020-04-03 Thread Andy Shevchenko
+Cc: Chee Hong

On Thu, Apr 2, 2020 at 10:09 PM Andy Shevchenko
 wrote:
>
> On Thu, Apr 2, 2020 at 7:55 AM Bin Meng  wrote:
> > On Thu, Apr 2, 2020 at 1:55 AM Simon Glass  wrote:
> > > On Wed, 1 Apr 2020 at 11:39, Andy Shevchenko  
> > > wrote:
> > > > On Wed, Apr 01, 2020 at 10:56:26AM -0600, Simon Glass wrote:
> > > > > On Wed, 1 Apr 2020 at 08:45, Andy Shevchenko 
> > > > >  wrote:
> > > > > > On Wed, Apr 1, 2020 at 5:32 PM Bin Meng  wrote:
> > > > > > > On Wed, Apr 1, 2020 at 9:58 PM Andy Shevchenko
> > > > > > >  wrote:
> > > > > > > >
> > > > > > > > The commit breaks serial console on the Intel Edison.
> > > > > > > >
> > > > > > > > This reverts commit 720f9e1fdb0c92d3fd16e1bfc25bcbd35612675c.
> > > > > > > >
> > > > > > > > Signed-off-by: Andy Shevchenko 
> > > > > > > > 
> > > > > > > > ---
> > > > > > > >  drivers/serial/ns16550.c | 40 
> > > > > > > > 
> > > > > > > >  1 file changed, 12 insertions(+), 28 deletions(-)
> > > > > > > >
> > > > > > >
> > > > > > > Could you please spend some time to investigate why this breaks 
> > > > > > > Intel Edison?
> > > > > > >
> > > > > > > Reverting this patch would mean we break other boards too as
> > > > > > > Wolfgang's patch wanted to fix the breakage in the first place. 
> > > > > > > Much
> > > > > > > appreciated!
> > > > > >
> > > > > > I guess I'm wrong person here. The DM code is a complete black box 
> > > > > > to me.
> > > > > > Nevertheless, I may test any provided fix / debug / etc patch by 
> > > > > > request.
> > > > > >
> > > > > > And I think it's fair to investigate by the one who made a 
> > > > > > regression
> > > > > > in the first place.
> > > > >
> > > > > Given that we have conflicting breakages, we need to debug Edison.
> > > >
> > > > I would glad to test any suggested change or debug patch!
> > > >
> > > > > Does it enable the debug UART?
> > > >
> > > > If I am not mistaken, it does not.
> > >
> > > Looks like you are right. If you know the address you could do that -
> > > see minnowmax for an example.
> >
> > Please suggest what's the best approach to proceed.
>
> I think I understand what happened, and Wolfgang's patch *is* a culprit.
>
> In serial_intel_mid.c we setup a divisor before probing the actual
> device. Now, since the address retrieving has been moved further in
> the initialization, we are writing to unknown registers and thus don't
> properly initialize hardware.
>
> So, the proper fix would be in my opinion to introduce a call back or
> some other way to make ordering possible, like registering hw_init
> callback in probe which will be called in the ns16550, or even do this
> as a callback for all serial class drivers.
>
> I don't dare to dive into this anticipating that crap will hit the fan.
> So, please, who is knowing serial code, fix this asap!
>
> --
> With Best Regards,
> Andy Shevchenko



-- 
With Best Regards,
Andy Shevchenko


[PATCH] armv8: ls1046ardb: add new 16GB udimm memory support

2020-04-03 Thread andy . tang
From: Yuantian Tang 

Add this udimm memory support on ls1046ardb board.

Signed-off-by: Yuantian Tang 
---
 board/freescale/ls1046ardb/ddr.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/board/freescale/ls1046ardb/ddr.h b/board/freescale/ls1046ardb/ddr.h
index 3b4d44d465..d401daa776 100644
--- a/board/freescale/ls1046ardb/ddr.h
+++ b/board/freescale/ls1046ardb/ddr.h
@@ -32,6 +32,7 @@ static const struct board_specific_parameters udimm0[] = {
{2,  1350, 0, 8, 6, 0x0708090B, 0x0C0D0E09,},
{2,  1666, 0, 8, 7, 0x08090A0C, 0x0D0F100B,},
{2,  1900, 0, 8, 7, 0x09090B0D, 0x0E10120B,},
+   {2,  2133, 0, 4, 7, 0x08090A0E, 0x1011120C,},
{2,  2300, 0, 8, 9, 0x0A0B0C10, 0x1213140E,},
{}
 };
-- 
2.17.1



Re: [PATCH 1/2] arm: dts: k3-am654: Increase OSPI default frequency to 50MHz

2020-04-03 Thread Pratyush Yadav
On 02/04/20 06:59PM, Vignesh Raghavendra wrote:
> In 1 bit mode OSPI can work at upto 50MHz, this provides before write
  ^
Provides _what_ before write performance? Did you mean "provides better 
write performance"? Same in the second patch too.

> performance. Therefore increase frequency from 40MHz to 50MHz
> 
> Signed-off-by: Vignesh Raghavendra 
> ---
>  arch/arm/dts/k3-am654-base-board.dts| 2 +-
>  arch/arm/dts/k3-am654-r5-base-board.dts | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/dts/k3-am654-base-board.dts 
> b/arch/arm/dts/k3-am654-base-board.dts
> index 5058b6c88e96..3ebf4af5e47a 100644
> --- a/arch/arm/dts/k3-am654-base-board.dts
> +++ b/arch/arm/dts/k3-am654-base-board.dts
> @@ -191,7 +191,7 @@
>   reg = <0x0>;
>   spi-tx-bus-width = <1>;
>   spi-rx-bus-width = <8>;
> - spi-max-frequency = <4000>;
> + spi-max-frequency = <5000>;
>   cdns,tshsl-ns = <60>;
>   cdns,tsd2d-ns = <60>;
>   cdns,tchsh-ns = <60>;
> diff --git a/arch/arm/dts/k3-am654-r5-base-board.dts 
> b/arch/arm/dts/k3-am654-r5-base-board.dts
> index 257b56a1b032..e6b78643c197 100644
> --- a/arch/arm/dts/k3-am654-r5-base-board.dts
> +++ b/arch/arm/dts/k3-am654-r5-base-board.dts
> @@ -268,7 +268,7 @@
>   reg = <0x0>;
>   spi-tx-bus-width = <1>;
>   spi-rx-bus-width = <8>;
> - spi-max-frequency = <4000>;
> + spi-max-frequency = <5000>;
>   cdns,tshsl-ns = <60>;
>   cdns,tsd2d-ns = <60>;
>   cdns,tchsh-ns = <60>;
> -- 
> 2.26.0
> 

-- 
Regards,
Pratyush Yadav
Texas Instruments India


[PATCH] mtd: spi-nor-ids: Add Spansion s25fs512s flash entry

2020-04-03 Thread Kuldeep Singh
Spansion "s25fs512s" flash is incorrectly decoded as "s25fl512s" on
various platforms as former is not present. Add the entry.

Linux already has both the flashes present. A snippet below:
{ "s25fl512s",  INFO6(0x010220, 0x4d0080, 256 * 1024, 256...},
{ "s25fs512s",  INFO6(0x010220, 0x4d0081, 256 * 1024, 256...},

Signed-off-by: Kuldeep Singh 
---
 drivers/mtd/spi/spi-nor-ids.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c
index 973b6f8..42baa84 100644
--- a/drivers/mtd/spi/spi-nor-ids.c
+++ b/drivers/mtd/spi/spi-nor-ids.c
@@ -193,7 +193,8 @@ const struct flash_info spi_nor_ids[] = {
{ INFO("s25sl064p",  0x010216, 0x4d00,  64 * 1024, 128, 
SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
{ INFO("s25fl256s0", 0x010219, 0x4d00, 256 * 1024, 128, USE_CLSR) },
{ INFO("s25fl256s1", 0x010219, 0x4d01,  64 * 1024, 512, 
SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
-   { INFO6("s25fl512s",  0x010220, 0x4d0081, 256 * 1024, 256, 
SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
+   { INFO6("s25fl512s",  0x010220, 0x4d0080, 256 * 1024, 256, 
SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
+   { INFO6("s25fs512s",  0x010220, 0x4d0081, 256 * 1024, 256, 
SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
{ INFO("s25fl512s_256k",  0x010220, 0x4d00, 256 * 1024, 256, 
SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
{ INFO("s25fl512s_64k",  0x010220, 0x4d01, 64 * 1024, 1024, 
SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
{ INFO("s25fl512s_512k", 0x010220, 0x4f00, 256 * 1024, 256, 
SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
-- 
2.7.4