[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 at nxp.com>
> Signed-off-by: Mingkai Hu <mingkai.hu at 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