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

[Wenbin Song <wenbin.song@nxp.com>]

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