[U-Boot] [PATCH v6 1/2] armv8/fsl-layerscape: fdt: fixup LS1043A rev1 GIC node

[U-Boot] [PATCH v6 1/2] armv8/fsl-layerscape: fdt: fixup LS1043A rev1 GIC node

[Wenbin song]

The LS1043A rev1.1 silicon supports two types of GIC offset: 4K alignment
and 64K alignment. The bit SCFG_GIC400_ALIGN[GIC_ADDR_BIT] is used to choose
which offset will be used. If GIC_ADDR_BIT bit is set, 4K alignment is used,
or else 64K alignment is used. The rev1.0 silicon only supports the CIG offset
with 4K alignment.

GIC offset is decided by SVR and GIC_ADDR_BIT bit.

Overriding the weak smp_kick_all_cpus, the new impletment is able to detect
GIC offset.

Signed-off-by: Wenbin Song <wenbin.song@nxp.com>
Signed-off-by: Mingkai Hu <mingkai.hu@nxp.com>
---
Changes in v6:
	Add comments around checking the revision. 
Changes in v5:
	Replace fix_gic_off with get_gic_off.
	Add #if condition to check CONFIG_GICV2 and CONFIG_GICV3 on smp_kick_all_cpus.
	Fixup gic node with 64K alignment when running on rev1.1 with GIC_ADDR_BIT cleared. 
---
 arch/arm/cpu/armv8/fsl-layerscape/Kconfig          |  4 ++
 arch/arm/cpu/armv8/fsl-layerscape/fdt.c            | 71 ++++++++++++++++++++++
 arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S       | 57 +++++++++++++++--
 arch/arm/include/asm/arch-fsl-layerscape/config.h  | 26 +++++++-
 .../include/asm/arch-fsl-layerscape/immap_lsch2.h  |  3 +-
 5 files changed, 154 insertions(+), 7 deletions(-)

diff --git a/arch/arm/cpu/armv8/fsl-layerscape/Kconfig b/arch/arm/cpu/armv8/fsl-layerscape/Kconfig
index 94ec8d5..f415868 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/Kconfig
+++ b/arch/arm/cpu/armv8/fsl-layerscape/Kconfig
@@ -135,4 +135,8 @@ config SYS_FSL_DDR4
 	help
 	  Enable Freescale DDR4 controller.
 
+config HAS_FEATURE_GIC4K_ALIGN
+       bool
+       default y if ARCH_LS1043A
+
 endmenu
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/fdt.c b/arch/arm/cpu/armv8/fsl-layerscape/fdt.c
index 1a8321b..9936be1 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/fdt.c
+++ b/arch/arm/cpu/armv8/fsl-layerscape/fdt.c
@@ -126,6 +126,74 @@ void fsl_fdt_disable_usb(void *blob)
 	}
 }
 
+#ifdef CONFIG_HAS_FEATURE_GIC4K_ALIGN
+static void fdt_fixup_gic(void *blob)
+{
+	int offset, err;
+	u64 reg[8];
+	struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
+	unsigned int rev;
+	struct ccsr_scfg __iomem *scfg = (void *)CONFIG_SYS_FSL_SCFG_ADDR;
+	int align_4k = 1;
+
+	rev = gur_in32(&gur->svr) & 0xff;
+	/*
+	 * For ls1043a rev1.0, the GIC offset aligns with 4K.
+	 * For ls1043a rev1.1 or more higher, the GIC offset
+	 * is decided by gic_align register.
+	 */
+
+	if (rev > REV1_0) {
+		/*
+		 * The GIC_ADDR_BIT on gic_align was set by
+		 * PBI to select GIC offset.
+		 */
+		rev = scfg_in32(&scfg->gic_align) & (0x01 << GIC_ADDR_BIT);
+		if (!rev)
+			align_4k = 0;
+	}
+
+	offset = fdt_subnode_offset(blob, 0, "interrupt-controller at 1400000");
+	if (offset < 0) {
+		printf("WARNING: fdt_subnode_offset can't find node %s: %s\n",
+		       "interrupt-controller at 1400000", fdt_strerror(offset));
+		return;
+	}
+
+	/* Fixup gic node align with 4K */
+	if (align_4k) {
+		reg[0] = cpu_to_fdt64(GICD_BASE_4K);
+		reg[1] = cpu_to_fdt64(GICD_SIZE_4K);
+		reg[2] = cpu_to_fdt64(GICC_BASE_4K);
+		reg[3] = cpu_to_fdt64(GICC_SIZE_4K);
+		reg[4] = cpu_to_fdt64(GICH_BASE_4K);
+		reg[5] = cpu_to_fdt64(GICH_SIZE_4K);
+		reg[6] = cpu_to_fdt64(GICV_BASE_4K);
+		reg[7] = cpu_to_fdt64(GICV_SIZE_4K);
+	} else {
+	/* Fixup gic node align with 64K */
+		reg[0] = cpu_to_fdt64(GICD_BASE);
+		reg[1] = cpu_to_fdt64(GICD_SIZE);
+		reg[2] = cpu_to_fdt64(GICC_BASE);
+		reg[3] = cpu_to_fdt64(GICC_SIZE);
+		reg[4] = cpu_to_fdt64(GICH_BASE);
+		reg[5] = cpu_to_fdt64(GICH_SIZE);
+		reg[6] = cpu_to_fdt64(GICV_BASE);
+		reg[7] = cpu_to_fdt64(GICV_SIZE);
+	}
+
+	err = fdt_setprop(blob, offset, "reg", reg, sizeof(reg));
+	if (err < 0) {
+		printf("WARNING: fdt_setprop can't set %s from node %s: %s\n",
+		       "reg", "interrupt-controller@1400000",
+		       fdt_strerror(err));
+		return;
+	}
+
+	return;
+}
+#endif
+
 void ft_cpu_setup(void *blob, bd_t *bd)
 {
 #ifdef CONFIG_FSL_LSCH2
@@ -170,4 +238,7 @@ void ft_cpu_setup(void *blob, bd_t *bd)
 #endif
 	fsl_fdt_disable_usb(blob);
 
+#ifdef CONFIG_HAS_FEATURE_GIC4K_ALIGN
+	fdt_fixup_gic(blob);
+#endif
 }
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S b/arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S
index 5d0b7a4..d50096c 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S
+++ b/arch/arm/cpu/armv8/fsl-layerscape/lowlevel.S
@@ -14,6 +14,55 @@
 #include <asm/arch/mp.h>
 #endif
 
+
+/* Get GIC offset
+* For LS1043a rev1.0, GIC base address align with 4k.
+* For LS1043a rev1.1, if DCFG_GIC400_ALIGN[GIC_ADDR_BIT]
+* is set, GIC base address align with 4K, or else align
+* with 64k.
+* output:
+*   for GICv2:
+*	x0-> the base address of GICD
+*	x1-> the base address of GICC
+*   for GICv3:
+*	 x0-> the base address of GICD
+*/
+ENTRY(get_gic_offset)
+	ldr     x0, =GICD_BASE
+#ifdef CONFIG_GICV2
+	ldr     x1, =GICC_BASE
+#endif
+#ifdef CONFIG_HAS_FEATURE_GIC4K_ALIGN
+	ldr     x3, =DCFG_CCSR_SVR
+	ldr	w3, [x3]
+	rev	w3, w3
+	ands	w3, w3, #0xff
+	cmp	w3, #REV1_0
+	b.eq	1f
+	ldr	x3, =SCFG_GIC400_ALIGN
+	ldr	w3, [x3]
+	rev	w3, w3
+	tbnz	w3, #GIC_ADDR_BIT, 1f
+	ret
+1:
+	ldr     x0, =GICD_BASE_4K
+	ldr     x1, =GICC_BASE_4K
+#endif
+	ret
+ENDPROC(get_gic_offset)
+
+ENTRY(smp_kick_all_cpus)
+	/* Kick secondary cpus up by SGI 0 interrupt */
+#if defined(CONFIG_GICV2) || defined(CONFIG_GICV3)
+	mov	x29, lr			/* Save LR */
+	bl	get_gic_offset
+	bl	gic_kick_secondary_cpus
+	mov	lr, x29			/* Restore LR */
+#endif
+	ret
+ENDPROC(smp_kick_all_cpus)
+
+
 ENTRY(lowlevel_init)
 	mov	x29, lr			/* Save LR */
 
@@ -105,15 +154,14 @@ ENTRY(lowlevel_init)
 	/* Initialize GIC Secure Bank Status */
 #if defined(CONFIG_GICV2) || defined(CONFIG_GICV3)
 	branch_if_slave x0, 1f
-	ldr	x0, =GICD_BASE
+	bl	get_gic_offset
 	bl	gic_init_secure
 1:
 #ifdef CONFIG_GICV3
 	ldr	x0, =GICR_BASE
 	bl	gic_init_secure_percpu
 #elif defined(CONFIG_GICV2)
-	ldr	x0, =GICD_BASE
-	ldr	x1, =GICC_BASE
+	bl	get_gic_offset
 	bl	gic_init_secure_percpu
 #endif
 #endif
@@ -335,7 +383,8 @@ ENTRY(secondary_boot_func)
 #if defined(CONFIG_GICV3)
 	gic_wait_for_interrupt_m x0
 #elif defined(CONFIG_GICV2)
-        ldr     x0, =GICC_BASE
+	bl	get_gic_offset
+	mov	x0, x1
         gic_wait_for_interrupt_m x0, w1
 #endif
 
diff --git a/arch/arm/include/asm/arch-fsl-layerscape/config.h b/arch/arm/include/asm/arch-fsl-layerscape/config.h
index 4201e0f..35f67dd 100644
--- a/arch/arm/include/asm/arch-fsl-layerscape/config.h
+++ b/arch/arm/include/asm/arch-fsl-layerscape/config.h
@@ -172,8 +172,30 @@
 #define SMMU_BASE		0x09000000
 
 /* Generic Interrupt Controller Definitions */
-#define GICD_BASE		0x01401000
-#define GICC_BASE		0x01402000
+#define GICD_BASE		0x01410000
+#define GICC_BASE		0x01420000
+#define GICH_BASE		0x01440000
+#define GICV_BASE		0x01460000
+#define GICD_SIZE		0x10000
+#define GICC_SIZE		0x20000
+#define GICH_SIZE		0x20000
+#define GICV_SIZE		0x20000
+#ifdef CONFIG_HAS_FEATURE_GIC4K_ALIGN
+#define GICD_BASE_4K		0x01401000
+#define GICC_BASE_4K		0x01402000
+#define GICH_BASE_4K		0x01404000
+#define GICV_BASE_4K		0x01406000
+#define GICD_SIZE_4K		0x1000
+#define GICC_SIZE_4K		0x2000
+#define GICH_SIZE_4K		0x2000
+#define GICV_SIZE_4K		0x2000
+#endif
+
+#define DCFG_CCSR_SVR		0x1ee00a4
+#define REV1_0			0x10
+#define REV1_1			0x11
+#define GIC_ADDR_BIT		31
+#define SCFG_GIC400_ALIGN	0x1570188
 
 #define CONFIG_SYS_FSL_ERRATUM_A008850
 #define CONFIG_SYS_FSL_ERRATUM_A009663
diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
index d88543d..1dfef53 100644
--- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
+++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
@@ -359,7 +359,8 @@ struct ccsr_scfg {
 	u32 qspi_cfg;
 	u8 res_160[0x180-0x160];
 	u32 dmamcr;
-	u8 res_184[0x18c-0x184];
+	u8 res_184[0x188-0x184];
+	u32 gic_align;
 	u32 debug_icid;
 	u8 res_190[0x1a4-0x190];
 	u32 snpcnfgcr;
-- 
2.1.0.27.g96db324

[U-Boot] [PATCH v6 2/2] armv8/fsl-layerscape: fdt: fixup LS1043A rev1 MSI node

[Wenbin song]

There are two types of msi node in kernel device tree, one is for
LS1043A rev1.1 silicon, the other is for rev1.0.

According to revision number, fixup the msi node.

Signed-off-by: Wenbin Song <wenbin.song@nxp.com>
Signed-off-by: Mingkai Hu <mingkai.hu@nxp.com>
---
Change in v6:
	None
Change in v5:
	Fixup the msi node used on rev1.0 when running on rev1.1.    	
---
 arch/arm/cpu/armv8/fsl-layerscape/Kconfig |   3 +
 arch/arm/cpu/armv8/fsl-layerscape/fdt.c   | 115 ++++++++++++++++++++++++++++++
 2 files changed, 118 insertions(+)

diff --git a/arch/arm/cpu/armv8/fsl-layerscape/Kconfig b/arch/arm/cpu/armv8/fsl-layerscape/Kconfig
index f415868..34ac867 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/Kconfig
+++ b/arch/arm/cpu/armv8/fsl-layerscape/Kconfig
@@ -139,4 +139,7 @@ config HAS_FEATURE_GIC4K_ALIGN
        bool
        default y if ARCH_LS1043A
 
+config HAS_FEATURE_ENHANCED_MSI
+       bool
+       default y if ARCH_LS1043A
 endmenu
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/fdt.c b/arch/arm/cpu/armv8/fsl-layerscape/fdt.c
index 9936be1..e87ba19 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/fdt.c
+++ b/arch/arm/cpu/armv8/fsl-layerscape/fdt.c
@@ -194,6 +194,118 @@ static void fdt_fixup_gic(void *blob)
 }
 #endif
 
+#ifdef CONFIG_HAS_FEATURE_ENHANCED_MSI
+static int _fdt_fixup_msi_subnode(void *blob, int offset, const char *name,
+				  int irq_0, int irq_1, int rev)
+{
+	int err, sub_offset, len;
+	u32 tmp[4][3];
+
+	sub_offset = fdt_subnode_offset(blob, offset, name);
+	if (offset < 0) {
+		printf("WARNING: fdt_subnode_offset can't find %s: %s\n",
+		       name, fdt_strerror(sub_offset));
+		return 0;
+	}
+
+	tmp[0][0] = cpu_to_fdt32(0x0);
+	tmp[0][1] = cpu_to_fdt32(irq_0);
+	tmp[0][2] = cpu_to_fdt32(0x4);
+
+	if (rev > REV1_0) {
+		tmp[1][0] = cpu_to_fdt32(0x0);
+		tmp[1][1] = cpu_to_fdt32(irq_1);
+		tmp[1][2] = cpu_to_fdt32(0x4);
+		tmp[2][0] = cpu_to_fdt32(0x0);
+		tmp[2][1] = cpu_to_fdt32(irq_1 + 1);
+		tmp[2][2] = cpu_to_fdt32(0x4);
+		tmp[3][0] = cpu_to_fdt32(0x0);
+		tmp[3][1] = cpu_to_fdt32(irq_1 + 2);
+		tmp[3][2] = cpu_to_fdt32(0x4);
+		len = sizeof(tmp);
+	} else {
+		len = sizeof(tmp[0]);
+	}
+
+	err = fdt_setprop(blob, sub_offset, "interrupts", tmp, len);
+	if (err < 0) {
+		printf("WARNING: fdt_setprop can't set %s from node %s: %s\n",
+		       "interrupts", name, fdt_strerror(err));
+		return 0;
+	}
+
+	return 1;
+}
+
+static int _fdt_fixup_pci_msi(void *blob, const char *name, int rev)
+{
+	int offset, len, err;
+	void *p;
+	int val;
+	u32 tmp[4][8];
+
+	offset = fdt_path_offset(blob, name);
+	if (offset < 0) {
+		printf("WARNING: fdt_path_offset can't find path %s: %s\n",
+		       name, fdt_strerror(offset));
+		return 0;
+	}
+
+	p = (char *)fdt_getprop(blob, offset, "interrupt-map", &len);
+	if (!p || len != sizeof(tmp)) {
+		printf("WARNING: fdt_getprop can't get %s from node %s\n",
+		       "interrupt-map", name);
+		return 0;
+	}
+
+	memcpy((char *)tmp, p, len);
+
+	val = fdt32_to_cpu(tmp[0][6]);
+	if (rev > REV1_0) {
+		tmp[1][6] = cpu_to_fdt32(val + 1);
+		tmp[2][6] = cpu_to_fdt32(val + 2);
+		tmp[3][6] = cpu_to_fdt32(val + 3);
+	} else {
+		tmp[1][6] = cpu_to_fdt32(val);
+		tmp[2][6] = cpu_to_fdt32(val);
+		tmp[3][6] = cpu_to_fdt32(val);
+	}
+
+	err = fdt_setprop(blob, offset, "interrupt-map", tmp, sizeof(tmp));
+	if (err < 0) {
+		printf("WARNING: fdt_setprop can't set %s from node %s: %s.\n",
+		       "interrupt-map", name, fdt_strerror(err));
+		return 0;
+	}
+	return 1;
+}
+
+/* Fixup msi to v1_0*/
+
+static void fdt_fixup_msi(void *blob)
+{
+	int nodeoffset;
+	struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
+	unsigned int rev;
+
+	rev = gur_in32(&gur->svr) & 0xff;
+
+	nodeoffset = fdt_path_offset(blob, "/soc/msi-controller");
+	if (nodeoffset < 0) {
+		printf("WARNING: fdt_path_offset can't find path %s: %s\n",
+		       "/soc/msi-controller", fdt_strerror(nodeoffset));
+		return;
+	}
+	_fdt_fixup_msi_subnode(blob, nodeoffset, "msi0 at 1571000", 116, 111, rev);
+	_fdt_fixup_msi_subnode(blob, nodeoffset, "msi1 at 1572000", 126, 121, rev);
+	_fdt_fixup_msi_subnode(blob, nodeoffset, "msi2 at 1573000", 160, 155, rev);
+
+	_fdt_fixup_pci_msi(blob, "/soc/pcie at 3400000", rev);
+	_fdt_fixup_pci_msi(blob, "/soc/pcie at 3500000", rev);
+	_fdt_fixup_pci_msi(blob, "/soc/pcie@3600000", rev);
+}
+#endif
+
 void ft_cpu_setup(void *blob, bd_t *bd)
 {
 #ifdef CONFIG_FSL_LSCH2
@@ -241,4 +353,7 @@ void ft_cpu_setup(void *blob, bd_t *bd)
 #ifdef CONFIG_HAS_FEATURE_GIC4K_ALIGN
 	fdt_fixup_gic(blob);
 #endif
+#ifdef CONFIG_HAS_FEATURE_ENHANCED_MSI
+	fdt_fixup_msi(blob);
+#endif
 }
-- 
2.1.0.27.g96db324

[U-Boot] [PATCH v6 1/2] armv8/fsl-layerscape: fdt: fixup LS1043A rev1 GIC node

[york sun]

On 10/31/2016 08:35 PM, Wenbin song wrote:
> The LS1043A rev1.1 silicon supports two types of GIC offset: 4K alignment
> and 64K alignment. The bit SCFG_GIC400_ALIGN[GIC_ADDR_BIT] is used to choose
> which offset will be used. If GIC_ADDR_BIT bit is set, 4K alignment is used,
> or else 64K alignment is used. The rev1.0 silicon only supports the CIG offset
> with 4K alignment.

Wenbin,

According to your patch and your explanation, the rev 1 SoC supports 4K 
alignment only. The rev 1.1 and newer SoC supports both 4K and 64K. If 
you don't do anything in PBI, the default is 64K. Does this 64k 
alignment apply to any other SoCs?

York

[U-Boot] [PATCH v6 2/2] armv8/fsl-layerscape: fdt: fixup LS1043A rev1 MSI node

[york sun]

On 10/31/2016 08:35 PM, Wenbin song wrote:
> There are two types of msi node in kernel device tree, one is for
> LS1043A rev1.1 silicon, the other is for rev1.0.

This doesn't explain the difference between the two versions. I don't 
see comments below either.

>
> According to revision number, fixup the msi node.
>
> Signed-off-by: Wenbin Song <wenbin.song@nxp.com>
> Signed-off-by: Mingkai Hu <mingkai.hu@nxp.com>
> ---
> Change in v6:
> 	None
> Change in v5:
> 	Fixup the msi node used on rev1.0 when running on rev1.1.    	
> ---
>  arch/arm/cpu/armv8/fsl-layerscape/Kconfig |   3 +
>  arch/arm/cpu/armv8/fsl-layerscape/fdt.c   | 115 ++++++++++++++++++++++++++++++
>  2 files changed, 118 insertions(+)
>
> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/Kconfig b/arch/arm/cpu/armv8/fsl-layerscape/Kconfig
> index f415868..34ac867 100644
> --- a/arch/arm/cpu/armv8/fsl-layerscape/Kconfig
> +++ b/arch/arm/cpu/armv8/fsl-layerscape/Kconfig
> @@ -139,4 +139,7 @@ config HAS_FEATURE_GIC4K_ALIGN
>         bool
>         default y if ARCH_LS1043A
>
> +config HAS_FEATURE_ENHANCED_MSI
> +       bool
> +       default y if ARCH_LS1043A
>  endmenu
> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/fdt.c b/arch/arm/cpu/armv8/fsl-layerscape/fdt.c
> index 9936be1..e87ba19 100644
> --- a/arch/arm/cpu/armv8/fsl-layerscape/fdt.c
> +++ b/arch/arm/cpu/armv8/fsl-layerscape/fdt.c
> @@ -194,6 +194,118 @@ static void fdt_fixup_gic(void *blob)
>  }
>  #endif
>
> +#ifdef CONFIG_HAS_FEATURE_ENHANCED_MSI
> +static int _fdt_fixup_msi_subnode(void *blob, int offset, const char *name,
> +				  int irq_0, int irq_1, int rev)
> +{
> +	int err, sub_offset, len;
> +	u32 tmp[4][3];
> +
> +	sub_offset = fdt_subnode_offset(blob, offset, name);
> +	if (offset < 0) {
> +		printf("WARNING: fdt_subnode_offset can't find %s: %s\n",
> +		       name, fdt_strerror(sub_offset));
> +		return 0;
> +	}
> +
> +	tmp[0][0] = cpu_to_fdt32(0x0);
> +	tmp[0][1] = cpu_to_fdt32(irq_0);
> +	tmp[0][2] = cpu_to_fdt32(0x4);
> +
> +	if (rev > REV1_0) {
> +		tmp[1][0] = cpu_to_fdt32(0x0);
> +		tmp[1][1] = cpu_to_fdt32(irq_1);
> +		tmp[1][2] = cpu_to_fdt32(0x4);
> +		tmp[2][0] = cpu_to_fdt32(0x0);
> +		tmp[2][1] = cpu_to_fdt32(irq_1 + 1);
> +		tmp[2][2] = cpu_to_fdt32(0x4);
> +		tmp[3][0] = cpu_to_fdt32(0x0);
> +		tmp[3][1] = cpu_to_fdt32(irq_1 + 2);
> +		tmp[3][2] = cpu_to_fdt32(0x4);
> +		len = sizeof(tmp);

Looks like you are adding three more interrupts. Some comments here 
would be nice.

> +	} else {
> +		len = sizeof(tmp[0]);
> +	}
> +
> +	err = fdt_setprop(blob, sub_offset, "interrupts", tmp, len);
> +	if (err < 0) {
> +		printf("WARNING: fdt_setprop can't set %s from node %s: %s\n",
> +		       "interrupts", name, fdt_strerror(err));
> +		return 0;
> +	}
> +
> +	return 1;
> +}
> +
> +static int _fdt_fixup_pci_msi(void *blob, const char *name, int rev)
> +{
> +	int offset, len, err;
> +	void *p;
> +	int val;
> +	u32 tmp[4][8];
> +
> +	offset = fdt_path_offset(blob, name);
> +	if (offset < 0) {
> +		printf("WARNING: fdt_path_offset can't find path %s: %s\n",
> +		       name, fdt_strerror(offset));
> +		return 0;
> +	}
> +
> +	p = (char *)fdt_getprop(blob, offset, "interrupt-map", &len);
> +	if (!p || len != sizeof(tmp)) {

Is the length check always accurate here?

> +		printf("WARNING: fdt_getprop can't get %s from node %s\n",
> +		       "interrupt-map", name);
> +		return 0;
> +	}
> +
> +	memcpy((char *)tmp, p, len);
> +
> +	val = fdt32_to_cpu(tmp[0][6]);
> +	if (rev > REV1_0) {
> +		tmp[1][6] = cpu_to_fdt32(val + 1);
> +		tmp[2][6] = cpu_to_fdt32(val + 2);
> +		tmp[3][6] = cpu_to_fdt32(val + 3);
> +	} else {
> +		tmp[1][6] = cpu_to_fdt32(val);
> +		tmp[2][6] = cpu_to_fdt32(val);
> +		tmp[3][6] = cpu_to_fdt32(val);
> +	}
> +
> +	err = fdt_setprop(blob, offset, "interrupt-map", tmp, sizeof(tmp));
> +	if (err < 0) {
> +		printf("WARNING: fdt_setprop can't set %s from node %s: %s.\n",
> +		       "interrupt-map", name, fdt_strerror(err));
> +		return 0;
> +	}
> +	return 1;
> +}
> +
> +/* Fixup msi to v1_0*/
> +

This comment is not accurate. You are fixing msi for both 1.0 and 1.1.

York

[U-Boot] [PATCH v6 2/2] armv8/fsl-layerscape: fdt: fixup LS1043A rev1 MSI node

[Wenbin Song]

Hi: york

There is  a bit of change for  MSI node of kernel , so I will update this patch on next version and with new comments you mentioned.     

Best Regards
Wenbin Song


> -----Original Message-----
> From: york sun
> Sent: Tuesday, November 15, 2016 5:36 AM
> To: Wenbin Song <wenbin.song@nxp.com>; albert.u.boot at aribaud.net;
> Mingkai Hu <mingkai.hu@nxp.com>; u-boot at lists.denx.de
> Subject: Re: [PATCH v6 2/2] armv8/fsl-layerscape: fdt: fixup LS1043A rev1 MSI
> node
> 
> On 10/31/2016 08:35 PM, Wenbin song wrote:
> > There are two types of msi node in kernel device tree, one is for
> > LS1043A rev1.1 silicon, the other is for rev1.0.
> 
> This doesn't explain the difference between the two versions. I don't see
> comments below either.
> 
> >
> > According to revision number, fixup the msi node.
> >
> > Signed-off-by: Wenbin Song <wenbin.song@nxp.com>
> > Signed-off-by: Mingkai Hu <mingkai.hu@nxp.com>
> > ---
> > Change in v6:
> > 	None
> > Change in v5:
> > 	Fixup the msi node used on rev1.0 when running on rev1.1.
> > ---
> >  arch/arm/cpu/armv8/fsl-layerscape/Kconfig |   3 +
> >  arch/arm/cpu/armv8/fsl-layerscape/fdt.c   | 115
> ++++++++++++++++++++++++++++++
> >  2 files changed, 118 insertions(+)
> >
> > diff --git a/arch/arm/cpu/armv8/fsl-layerscape/Kconfig
> > b/arch/arm/cpu/armv8/fsl-layerscape/Kconfig
> > index f415868..34ac867 100644
> > --- a/arch/arm/cpu/armv8/fsl-layerscape/Kconfig
> > +++ b/arch/arm/cpu/armv8/fsl-layerscape/Kconfig
> > @@ -139,4 +139,7 @@ config HAS_FEATURE_GIC4K_ALIGN
> >         bool
> >         default y if ARCH_LS1043A
> >
> > +config HAS_FEATURE_ENHANCED_MSI
> > +       bool
> > +       default y if ARCH_LS1043A
> >  endmenu
> > diff --git a/arch/arm/cpu/armv8/fsl-layerscape/fdt.c
> > b/arch/arm/cpu/armv8/fsl-layerscape/fdt.c
> > index 9936be1..e87ba19 100644
> > --- a/arch/arm/cpu/armv8/fsl-layerscape/fdt.c
> > +++ b/arch/arm/cpu/armv8/fsl-layerscape/fdt.c
> > @@ -194,6 +194,118 @@ static void fdt_fixup_gic(void *blob)  }  #endif
> >
> > +#ifdef CONFIG_HAS_FEATURE_ENHANCED_MSI static int
> > +_fdt_fixup_msi_subnode(void *blob, int offset, const char *name,
> > +				  int irq_0, int irq_1, int rev)
> > +{
> > +	int err, sub_offset, len;
> > +	u32 tmp[4][3];
> > +
> > +	sub_offset = fdt_subnode_offset(blob, offset, name);
> > +	if (offset < 0) {
> > +		printf("WARNING: fdt_subnode_offset can't find %s: %s\n",
> > +		       name, fdt_strerror(sub_offset));
> > +		return 0;
> > +	}
> > +
> > +	tmp[0][0] = cpu_to_fdt32(0x0);
> > +	tmp[0][1] = cpu_to_fdt32(irq_0);
> > +	tmp[0][2] = cpu_to_fdt32(0x4);
> > +
> > +	if (rev > REV1_0) {
> > +		tmp[1][0] = cpu_to_fdt32(0x0);
> > +		tmp[1][1] = cpu_to_fdt32(irq_1);
> > +		tmp[1][2] = cpu_to_fdt32(0x4);
> > +		tmp[2][0] = cpu_to_fdt32(0x0);
> > +		tmp[2][1] = cpu_to_fdt32(irq_1 + 1);
> > +		tmp[2][2] = cpu_to_fdt32(0x4);
> > +		tmp[3][0] = cpu_to_fdt32(0x0);
> > +		tmp[3][1] = cpu_to_fdt32(irq_1 + 2);
> > +		tmp[3][2] = cpu_to_fdt32(0x4);
> > +		len = sizeof(tmp);
> 
> Looks like you are adding three more interrupts. Some comments here would
> be nice.
> 
> > +	} else {
> > +		len = sizeof(tmp[0]);
> > +	}
> > +
> > +	err = fdt_setprop(blob, sub_offset, "interrupts", tmp, len);
> > +	if (err < 0) {
> > +		printf("WARNING: fdt_setprop can't set %s from
> node %s: %s\n",
> > +		       "interrupts", name, fdt_strerror(err));
> > +		return 0;
> > +	}
> > +
> > +	return 1;
> > +}
> > +
> > +static int _fdt_fixup_pci_msi(void *blob, const char *name, int rev)
> > +{
> > +	int offset, len, err;
> > +	void *p;
> > +	int val;
> > +	u32 tmp[4][8];
> > +
> > +	offset = fdt_path_offset(blob, name);
> > +	if (offset < 0) {
> > +		printf("WARNING: fdt_path_offset can't find path %s: %s\n",
> > +		       name, fdt_strerror(offset));
> > +		return 0;
> > +	}
> > +
> > +	p = (char *)fdt_getprop(blob, offset, "interrupt-map", &len);
> > +	if (!p || len != sizeof(tmp)) {
> 
> Is the length check always accurate here?
> 
> > +		printf("WARNING: fdt_getprop can't get %s from node %s\n",
> > +		       "interrupt-map", name);
> > +		return 0;
> > +	}
> > +
> > +	memcpy((char *)tmp, p, len);
> > +
> > +	val = fdt32_to_cpu(tmp[0][6]);
> > +	if (rev > REV1_0) {
> > +		tmp[1][6] = cpu_to_fdt32(val + 1);
> > +		tmp[2][6] = cpu_to_fdt32(val + 2);
> > +		tmp[3][6] = cpu_to_fdt32(val + 3);
> > +	} else {
> > +		tmp[1][6] = cpu_to_fdt32(val);
> > +		tmp[2][6] = cpu_to_fdt32(val);
> > +		tmp[3][6] = cpu_to_fdt32(val);
> > +	}
> > +
> > +	err = fdt_setprop(blob, offset, "interrupt-map", tmp, sizeof(tmp));
> > +	if (err < 0) {
> > +		printf("WARNING: fdt_setprop can't set %s from
> node %s: %s.\n",
> > +		       "interrupt-map", name, fdt_strerror(err));
> > +		return 0;
> > +	}
> > +	return 1;
> > +}
> > +
> > +/* Fixup msi to v1_0*/
> > +
> 
> This comment is not accurate. You are fixing msi for both 1.0 and 1.1.
> 
> York

[U-Boot] [PATCH v6 1/2] armv8/fsl-layerscape: fdt: fixup LS1043A rev1 GIC node

[Wenbin Song]

Hi: york

Best Regards
Wenbin Song


> -----Original Message-----
> From: york sun
> Sent: Tuesday, November 15, 2016 5:24 AM
> To: Wenbin Song <wenbin.song@nxp.com>; albert.u.boot at aribaud.net;
> Mingkai Hu <mingkai.hu@nxp.com>; u-boot at lists.denx.de
> Subject: Re: [PATCH v6 1/2] armv8/fsl-layerscape: fdt: fixup LS1043A rev1 GIC
> node
> 
> On 10/31/2016 08:35 PM, Wenbin song wrote:
> > The LS1043A rev1.1 silicon supports two types of GIC offset: 4K
> > alignment and 64K alignment. The bit SCFG_GIC400_ALIGN[GIC_ADDR_BIT]
> > is used to choose which offset will be used. If GIC_ADDR_BIT bit is
> > set, 4K alignment is used, or else 64K alignment is used. The rev1.0
> > silicon only supports the CIG offset with 4K alignment.
> 
> Wenbin,
> 
> According to your patch and your explanation, the rev 1 SoC supports 4K
> alignment only. The rev 1.1 and newer SoC supports both 4K and 64K. If you
> don't do anything in PBI, the default is 64K. 

[] yes. You are  correct .

Does this 64k alignment apply to
> any other SoCs?
> 
[]   This patch only apply  to ls1043a.  and the other SOCs  only support one kind of alignment style.  For example, ls1046a align with 64k, ls1012a align with 4k.
> York

[U-Boot] [PATCH v6 1/2] armv8/fsl-layerscape: fdt: fixup LS1043A rev1 GIC node

[york sun]

On 11/16/2016 01:23 AM, Wenbin Song wrote:
> Hi: york
>
> Best Regards
> Wenbin Song
>
>
>> -----Original Message-----
>> From: york sun
>> Sent: Tuesday, November 15, 2016 5:24 AM
>> To: Wenbin Song <wenbin.song@nxp.com>; albert.u.boot at aribaud.net;
>> Mingkai Hu <mingkai.hu@nxp.com>; u-boot at lists.denx.de
>> Subject: Re: [PATCH v6 1/2] armv8/fsl-layerscape: fdt: fixup LS1043A rev1 GIC
>> node
>>
>> On 10/31/2016 08:35 PM, Wenbin song wrote:
>>> The LS1043A rev1.1 silicon supports two types of GIC offset: 4K
>>> alignment and 64K alignment. The bit SCFG_GIC400_ALIGN[GIC_ADDR_BIT]
>>> is used to choose which offset will be used. If GIC_ADDR_BIT bit is
>>> set, 4K alignment is used, or else 64K alignment is used. The rev1.0
>>> silicon only supports the CIG offset with 4K alignment.
>>
>> Wenbin,
>>
>> According to your patch and your explanation, the rev 1 SoC supports 4K
>> alignment only. The rev 1.1 and newer SoC supports both 4K and 64K. If you
>> don't do anything in PBI, the default is 64K.
>
> [] yes. You are  correct .

I would prefer it has 4K alignment if you do nothing in PBI. But I guess 
it is too late to change the hardware.

>
> Does this 64k alignment apply to
>> any other SoCs?
>>
> []   This patch only apply  to ls1043a.  and the other SOCs  only support one kind of alignment style.  For example, ls1046a align with 64k, ls1012a align with 4k.

In this case, you patch is OK. You can add full SVR check if a future 
SoC needs the same fix.

York

[U-Boot] [PATCH v6 1/2] armv8/fsl-layerscape: fdt: fixup LS1043A rev1 GIC node

[Wenbin Song]

Hi, york

Best Regards
Wenbin Song


> -----Original Message-----
> From: york sun
> Sent: Thursday, November 17, 2016 1:05 AM
> To: Wenbin Song <wenbin.song@nxp.com>; albert.u.boot at aribaud.net;
> Mingkai Hu <mingkai.hu@nxp.com>; u-boot at lists.denx.de
> Subject: Re: [PATCH v6 1/2] armv8/fsl-layerscape: fdt: fixup LS1043A rev1 GIC
> node
> 
> On 11/16/2016 01:23 AM, Wenbin Song wrote:
> > Hi: york
> >
> > Best Regards
> > Wenbin Song
> >
> >
> >> -----Original Message-----
> >> From: york sun
> >> Sent: Tuesday, November 15, 2016 5:24 AM
> >> To: Wenbin Song <wenbin.song@nxp.com>; albert.u.boot at aribaud.net;
> >> Mingkai Hu <mingkai.hu@nxp.com>; u-boot at lists.denx.de
> >> Subject: Re: [PATCH v6 1/2] armv8/fsl-layerscape: fdt: fixup LS1043A
> >> rev1 GIC node
> >>
> >> On 10/31/2016 08:35 PM, Wenbin song wrote:
> >>> The LS1043A rev1.1 silicon supports two types of GIC offset: 4K
> >>> alignment and 64K alignment. The bit SCFG_GIC400_ALIGN[GIC_ADDR_BIT]
> >>> is used to choose which offset will be used. If GIC_ADDR_BIT bit is
> >>> set, 4K alignment is used, or else 64K alignment is used. The rev1.0
> >>> silicon only supports the CIG offset with 4K alignment.
> >>
> >> Wenbin,
> >>
> >> According to your patch and your explanation, the rev 1 SoC supports
> >> 4K alignment only. The rev 1.1 and newer SoC supports both 4K and
> >> 64K. If you don't do anything in PBI, the default is 64K.
> >
> > [] yes. You are  correct .
> 
> I would prefer it has 4K alignment if you do nothing in PBI. But I guess it is too
> late to change the hardware.
> 
> >
> > Does this 64k alignment apply to
> >> any other SoCs?
> >>
> > []   This patch only apply  to ls1043a.  and the other SOCs  only support one
> kind of alignment style.  For example, ls1046a align with 64k, ls1012a align with
> 4k.
> 
> In this case, you patch is OK. You can add full SVR check if a future SoC needs
> the same fix.

[]  ok, I will make this change  in the next version of the patch.
 

> 
> York

[U-Boot] [PATCH v6 1/2] armv8/fsl-layerscape: fdt: fixup LS1043A rev1 GIC node

[york sun]

On 11/16/2016 05:14 PM, Wenbin Song wrote:
> []  ok, I will make this change  in the next version of the patch.
>

Wenbin,

Have you sent an update for this set?

York

[U-Boot] [PATCH v6 1/2] armv8/fsl-layerscape: fdt: fixup LS1043A rev1 GIC node

[Wenbin Song]

Hi, york

Because the other patch [patch v6 2/2 ] in this set depends on the format of  MSI  node  which is not yet ready to upstream. 
I planed  to send them together. 

Do you have any idea?


Best Regards
Wenbin Song

> -----Original Message-----
> From: york sun
> Sent: Friday, December 09, 2016 3:07 AM
> To: Wenbin Song <wenbin.song@nxp.com>; albert.u.boot at aribaud.net;
> Mingkai Hu <mingkai.hu@nxp.com>; u-boot at lists.denx.de
> Subject: Re: [PATCH v6 1/2] armv8/fsl-layerscape: fdt: fixup LS1043A rev1 GIC
> node
> 
> On 11/16/2016 05:14 PM, Wenbin Song wrote:
> > []  ok, I will make this change  in the next version of the patch.
> >
> 
> Wenbin,
> 
> Have you sent an update for this set?
> 
> York

[U-Boot] [PATCH v6 1/2] armv8/fsl-layerscape: fdt: fixup LS1043A rev1 GIC node

[york sun]

On 12/09/2016 01:58 AM, Wenbin Song wrote:
> Hi, york
>
> Because the other patch [patch v6 2/2 ] in this set depends on the format of  MSI  node  which is not yet ready to upstream.
> I planed  to send them together.

OK.

York

[U-Boot] [PATCH v6 1/2] armv8/fsl-layerscape: fdt: fixup LS1043A rev1 GIC node

[york sun]

On 12/09/2016 01:58 AM, Wenbin Song wrote:
> Hi, york
>
> Because the other patch [patch v6 2/2 ] in this set depends on the format of  MSI  node  which is not yet ready to upstream.
> I planed  to send them together.
>
> Do you have any idea?
>

Can we separate MSI from this set and apply the GIC change first? At 
least, we can boot with this change.

York