| Message ID | VI1PR04MB14552C43C6707ECFBD3845A698000@VI1PR04MB1455.eurprd04.prod.outlook.com |
|---|---|
| State | Not Applicable |
| Delegated to: | York Sun |
| Headers | show |
On 04/10/2017 10:54 PM, Sumit Garg wrote: >> -----Original Message----- >> From: York Sun [mailto:york.sun@nxp.com] >> Sent: Monday, April 10, 2017 10:39 PM >> To: Sumit Garg <sumit.garg@nxp.com>; u-boot@lists.denx.de >> Cc: Ruchika Gupta <ruchika.gupta@nxp.com>; Prabhakar Kushwaha >> <prabhakar.kushwaha@nxp.com>; Vini Pillai <vinitha.pillai@nxp.com>; Udit >> Agarwal <udit.agarwal@nxp.com> >> Subject: Re: [PATCH 1/3] fsl: PPA: add support PPA image validation from >> NAND and SD >> >> On 04/07/2017 04:41 AM, Sumit Garg wrote: >>> Signed-off-by: Sumit Garg <sumit.garg@nxp.com> >>> Signed-off-by: Udit Agarwal <udit.agarwal@nxp.com> >>> Tested-by: Vinitha Pillai <vinitha.pillai@nxp.com> >>> --- >>> arch/arm/cpu/armv8/fsl-layerscape/ppa.c | 67 >>> ++++++++++++++++++++++++++++++++- >>> 1 file changed, 66 insertions(+), 1 deletion(-) >>> >>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/ppa.c >>> b/arch/arm/cpu/armv8/fsl-layerscape/ppa.c >>> index 7f87bb8..d8f1d36 100644 >>> --- a/arch/arm/cpu/armv8/fsl-layerscape/ppa.c >>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/ppa.c >>> @@ -39,6 +39,10 @@ int ppa_init(void) >>> #ifdef CONFIG_CHAIN_OF_TRUST >>> uintptr_t ppa_esbc_hdr = CONFIG_SYS_LS_PPA_ESBC_ADDR; >> >> For MMC and NAND, this CONFIG_SYS_LS_PPA_ESBC_ADDR is actually not >> used. >> Shall we move the assignment down to XIP section? The Kconfig should also be >> updated. >> >> York > > As per PPA verification patch for eMMC/SD and NAND: > > --- a/arch/arm/cpu/armv8/fsl-layerscape/Kconfig > +++ b/arch/arm/cpu/armv8/fsl-layerscape/Kconfig > @@ -179,12 +179,22 @@ config SYS_LS_PPA_ESBC_ADDR > default 0x40740000 if SYS_LS_PPA_FW_IN_XIP && ARCH_LS1046A > default 0x40480000 if SYS_LS_PPA_FW_IN_XIP && ARCH_LS1012A > default 0x580c40000 if SYS_LS_PPA_FW_IN_XIP && FSL_LSCH3 > + default 0x700000 if SYS_LS_PPA_FW_IN_MMC > + default 0x700000 if SYS_LS_PPA_FW_IN_NAND > help > If the PPA header firmware locate at XIP flash, such as NOR or > QSPI flash, this address is a directly memory-mapped. > If it is in a serial accessed flash, such as NAND and SD > card, it is a byte offset. > > CONFIG_SYS_LS_PPA_ESBC_ADDR is used to provide offset on eMMC/SD and NAND. > That's exactly what I was referring to. You are _NOT_ using CONFIG_SYS_LS_PPA_ESBC_ADDR value for eMMC/SD or NAND. Instead, you use malloc to get a new memory for the header, which I agree it is right to do. So the macro CONFIG_SYS_LS_PPA_ESBC_ADDR is not used for either MMC or NAND. York
> -----Original Message----- > From: York Sun [mailto:york.sun@nxp.com] > Sent: Tuesday, April 11, 2017 11:44 AM > To: Sumit Garg <sumit.garg@nxp.com>; u-boot@lists.denx.de > Cc: Ruchika Gupta <ruchika.gupta@nxp.com>; Prabhakar Kushwaha > <prabhakar.kushwaha@nxp.com>; Vini Pillai <vinitha.pillai@nxp.com>; Udit > Agarwal <udit.agarwal@nxp.com> > Subject: Re: [PATCH 1/3] fsl: PPA: add support PPA image validation from > NAND and SD > > On 04/10/2017 10:54 PM, Sumit Garg wrote: > >> -----Original Message----- > >> From: York Sun [mailto:york.sun@nxp.com] > >> Sent: Monday, April 10, 2017 10:39 PM > >> To: Sumit Garg <sumit.garg@nxp.com>; u-boot@lists.denx.de > >> Cc: Ruchika Gupta <ruchika.gupta@nxp.com>; Prabhakar Kushwaha > >> <prabhakar.kushwaha@nxp.com>; Vini Pillai <vinitha.pillai@nxp.com>; > >> Udit Agarwal <udit.agarwal@nxp.com> > >> Subject: Re: [PATCH 1/3] fsl: PPA: add support PPA image validation > >> from NAND and SD > >> > >> On 04/07/2017 04:41 AM, Sumit Garg wrote: > >>> Signed-off-by: Sumit Garg <sumit.garg@nxp.com> > >>> Signed-off-by: Udit Agarwal <udit.agarwal@nxp.com> > >>> Tested-by: Vinitha Pillai <vinitha.pillai@nxp.com> > >>> --- > >>> arch/arm/cpu/armv8/fsl-layerscape/ppa.c | 67 > >>> ++++++++++++++++++++++++++++++++- > >>> 1 file changed, 66 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/ppa.c > >>> b/arch/arm/cpu/armv8/fsl-layerscape/ppa.c > >>> index 7f87bb8..d8f1d36 100644 > >>> --- a/arch/arm/cpu/armv8/fsl-layerscape/ppa.c > >>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/ppa.c > >>> @@ -39,6 +39,10 @@ int ppa_init(void) #ifdef CONFIG_CHAIN_OF_TRUST > >>> uintptr_t ppa_esbc_hdr = CONFIG_SYS_LS_PPA_ESBC_ADDR; > >> > >> For MMC and NAND, this CONFIG_SYS_LS_PPA_ESBC_ADDR is actually not > >> used. > >> Shall we move the assignment down to XIP section? The Kconfig should > >> also be updated. > >> > >> York > > > > As per PPA verification patch for eMMC/SD and NAND: > > > > --- a/arch/arm/cpu/armv8/fsl-layerscape/Kconfig > > +++ b/arch/arm/cpu/armv8/fsl-layerscape/Kconfig > > @@ -179,12 +179,22 @@ config SYS_LS_PPA_ESBC_ADDR > > default 0x40740000 if SYS_LS_PPA_FW_IN_XIP && ARCH_LS1046A > > default 0x40480000 if SYS_LS_PPA_FW_IN_XIP && ARCH_LS1012A > > default 0x580c40000 if SYS_LS_PPA_FW_IN_XIP && FSL_LSCH3 > > + default 0x700000 if SYS_LS_PPA_FW_IN_MMC > > + default 0x700000 if SYS_LS_PPA_FW_IN_NAND > > help > > If the PPA header firmware locate at XIP flash, such as NOR or > > QSPI flash, this address is a directly memory-mapped. > > If it is in a serial accessed flash, such as NAND and SD > > card, it is a byte offset. > > > > CONFIG_SYS_LS_PPA_ESBC_ADDR is used to provide offset on eMMC/SD > and NAND. > > > > That's exactly what I was referring to. You are _NOT_ using > CONFIG_SYS_LS_PPA_ESBC_ADDR value for eMMC/SD or NAND. Instead, you > use malloc to get a new memory for the header, which I agree it is right to do. > So the macro CONFIG_SYS_LS_PPA_ESBC_ADDR is not used for either MMC or > NAND. > > York Actually I am using CONFIG_SYS_LS_PPA_ESBC_ADDR in case of eMMC/SD or NAND as follows: For eMMC/SD: + blk = CONFIG_SYS_LS_PPA_ESBC_ADDR >> 9; + cnt = DIV_ROUND_UP(CONFIG_LS_PPA_ESBC_HDR_SIZE, 512); + ret = mmc->block_dev.block_read(&mmc->block_dev, blk, cnt, ppa_hdr_ddr); + if (ret != cnt) { + free(ppa_hdr_ddr); + printf("MMC/SD read of PPA header failed\n"); + return -EIO; + } For NAND: + ret = nand_read(nand_info[0], (loff_t)CONFIG_SYS_LS_PPA_ESBC_ADDR, + &fw_length, (u_char *)ppa_hdr_ddr); + if (ret == -EUCLEAN) { + free(ppa_hdr_ddr); + printf("NAND read of PPA firmware at offset 0x%x failed\n", + CONFIG_SYS_LS_PPA_FW_ADDR); + return -EIO; + } Sumit
On 04/11/2017 04:59 AM, Sumit Garg wrote: >> -----Original Message----- >> From: York Sun [mailto:york.sun@nxp.com] >> Sent: Tuesday, April 11, 2017 11:44 AM >> To: Sumit Garg <sumit.garg@nxp.com>; u-boot@lists.denx.de >> Cc: Ruchika Gupta <ruchika.gupta@nxp.com>; Prabhakar Kushwaha >> <prabhakar.kushwaha@nxp.com>; Vini Pillai <vinitha.pillai@nxp.com>; Udit >> Agarwal <udit.agarwal@nxp.com> >> Subject: Re: [PATCH 1/3] fsl: PPA: add support PPA image validation from >> NAND and SD >> >> On 04/10/2017 10:54 PM, Sumit Garg wrote: >>>> -----Original Message----- >>>> From: York Sun [mailto:york.sun@nxp.com] >>>> Sent: Monday, April 10, 2017 10:39 PM >>>> To: Sumit Garg <sumit.garg@nxp.com>; u-boot@lists.denx.de >>>> Cc: Ruchika Gupta <ruchika.gupta@nxp.com>; Prabhakar Kushwaha >>>> <prabhakar.kushwaha@nxp.com>; Vini Pillai <vinitha.pillai@nxp.com>; >>>> Udit Agarwal <udit.agarwal@nxp.com> >>>> Subject: Re: [PATCH 1/3] fsl: PPA: add support PPA image validation >>>> from NAND and SD >>>> >>>> On 04/07/2017 04:41 AM, Sumit Garg wrote: >>>>> Signed-off-by: Sumit Garg <sumit.garg@nxp.com> >>>>> Signed-off-by: Udit Agarwal <udit.agarwal@nxp.com> >>>>> Tested-by: Vinitha Pillai <vinitha.pillai@nxp.com> >>>>> --- >>>>> arch/arm/cpu/armv8/fsl-layerscape/ppa.c | 67 >>>>> ++++++++++++++++++++++++++++++++- >>>>> 1 file changed, 66 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/ppa.c >>>>> b/arch/arm/cpu/armv8/fsl-layerscape/ppa.c >>>>> index 7f87bb8..d8f1d36 100644 >>>>> --- a/arch/arm/cpu/armv8/fsl-layerscape/ppa.c >>>>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/ppa.c >>>>> @@ -39,6 +39,10 @@ int ppa_init(void) #ifdef CONFIG_CHAIN_OF_TRUST >>>>> uintptr_t ppa_esbc_hdr = CONFIG_SYS_LS_PPA_ESBC_ADDR; >>>> >>>> For MMC and NAND, this CONFIG_SYS_LS_PPA_ESBC_ADDR is actually not >>>> used. >>>> Shall we move the assignment down to XIP section? The Kconfig should >>>> also be updated. >>>> >>>> York >>> >>> As per PPA verification patch for eMMC/SD and NAND: >>> >>> --- a/arch/arm/cpu/armv8/fsl-layerscape/Kconfig >>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/Kconfig >>> @@ -179,12 +179,22 @@ config SYS_LS_PPA_ESBC_ADDR >>> default 0x40740000 if SYS_LS_PPA_FW_IN_XIP && ARCH_LS1046A >>> default 0x40480000 if SYS_LS_PPA_FW_IN_XIP && ARCH_LS1012A >>> default 0x580c40000 if SYS_LS_PPA_FW_IN_XIP && FSL_LSCH3 >>> + default 0x700000 if SYS_LS_PPA_FW_IN_MMC >>> + default 0x700000 if SYS_LS_PPA_FW_IN_NAND >>> help >>> If the PPA header firmware locate at XIP flash, such as NOR or >>> QSPI flash, this address is a directly memory-mapped. >>> If it is in a serial accessed flash, such as NAND and SD >>> card, it is a byte offset. >>> >>> CONFIG_SYS_LS_PPA_ESBC_ADDR is used to provide offset on eMMC/SD >> and NAND. >>> >> >> That's exactly what I was referring to. You are _NOT_ using >> CONFIG_SYS_LS_PPA_ESBC_ADDR value for eMMC/SD or NAND. Instead, you >> use malloc to get a new memory for the header, which I agree it is right to do. >> So the macro CONFIG_SYS_LS_PPA_ESBC_ADDR is not used for either MMC or >> NAND. >> >> York > > Actually I am using CONFIG_SYS_LS_PPA_ESBC_ADDR in case of eMMC/SD or NAND as follows: > > For eMMC/SD: > + blk = CONFIG_SYS_LS_PPA_ESBC_ADDR >> 9; > + cnt = DIV_ROUND_UP(CONFIG_LS_PPA_ESBC_HDR_SIZE, 512); > + ret = mmc->block_dev.block_read(&mmc->block_dev, blk, cnt, ppa_hdr_ddr); > + if (ret != cnt) { > + free(ppa_hdr_ddr); > + printf("MMC/SD read of PPA header failed\n"); > + return -EIO; > + } > > For NAND: > + ret = nand_read(nand_info[0], (loff_t)CONFIG_SYS_LS_PPA_ESBC_ADDR, > + &fw_length, (u_char *)ppa_hdr_ddr); > + if (ret == -EUCLEAN) { > + free(ppa_hdr_ddr); > + printf("NAND read of PPA firmware at offset 0x%x failed\n", > + CONFIG_SYS_LS_PPA_FW_ADDR); > + return -EIO; > + } > So you are using this macro, but not using it directly as the way for XIP. It is confusing to see the assignment ppa_esbc_hdr = CONFIG_SYS_LS_PPA_ESBC_ADDR; I was suggesting to move this assignment down to the XIP section. York
> -----Original Message----- > From: York Sun [mailto:york.sun@nxp.com] > Sent: Tuesday, April 11, 2017 9:09 PM > To: Sumit Garg <sumit.garg@nxp.com>; u-boot@lists.denx.de > Cc: Ruchika Gupta <ruchika.gupta@nxp.com>; Prabhakar Kushwaha > <prabhakar.kushwaha@nxp.com>; Vini Pillai <vinitha.pillai@nxp.com>; Udit > Agarwal <udit.agarwal@nxp.com> > Subject: Re: [PATCH 1/3] fsl: PPA: add support PPA image validation from > NAND and SD > > On 04/11/2017 04:59 AM, Sumit Garg wrote: > >> -----Original Message----- > >> From: York Sun [mailto:york.sun@nxp.com] > >> Sent: Tuesday, April 11, 2017 11:44 AM > >> To: Sumit Garg <sumit.garg@nxp.com>; u-boot@lists.denx.de > >> Cc: Ruchika Gupta <ruchika.gupta@nxp.com>; Prabhakar Kushwaha > >> <prabhakar.kushwaha@nxp.com>; Vini Pillai <vinitha.pillai@nxp.com>; > >> Udit Agarwal <udit.agarwal@nxp.com> > >> Subject: Re: [PATCH 1/3] fsl: PPA: add support PPA image validation > >> from NAND and SD > >> > >> On 04/10/2017 10:54 PM, Sumit Garg wrote: > >>>> -----Original Message----- > >>>> From: York Sun [mailto:york.sun@nxp.com] > >>>> Sent: Monday, April 10, 2017 10:39 PM > >>>> To: Sumit Garg <sumit.garg@nxp.com>; u-boot@lists.denx.de > >>>> Cc: Ruchika Gupta <ruchika.gupta@nxp.com>; Prabhakar Kushwaha > >>>> <prabhakar.kushwaha@nxp.com>; Vini Pillai <vinitha.pillai@nxp.com>; > >>>> Udit Agarwal <udit.agarwal@nxp.com> > >>>> Subject: Re: [PATCH 1/3] fsl: PPA: add support PPA image validation > >>>> from NAND and SD > >>>> > >>>> On 04/07/2017 04:41 AM, Sumit Garg wrote: > >>>>> Signed-off-by: Sumit Garg <sumit.garg@nxp.com> > >>>>> Signed-off-by: Udit Agarwal <udit.agarwal@nxp.com> > >>>>> Tested-by: Vinitha Pillai <vinitha.pillai@nxp.com> > >>>>> --- > >>>>> arch/arm/cpu/armv8/fsl-layerscape/ppa.c | 67 > >>>>> ++++++++++++++++++++++++++++++++- > >>>>> 1 file changed, 66 insertions(+), 1 deletion(-) > >>>>> > >>>>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/ppa.c > >>>>> b/arch/arm/cpu/armv8/fsl-layerscape/ppa.c > >>>>> index 7f87bb8..d8f1d36 100644 > >>>>> --- a/arch/arm/cpu/armv8/fsl-layerscape/ppa.c > >>>>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/ppa.c > >>>>> @@ -39,6 +39,10 @@ int ppa_init(void) #ifdef > CONFIG_CHAIN_OF_TRUST > >>>>> uintptr_t ppa_esbc_hdr = CONFIG_SYS_LS_PPA_ESBC_ADDR; > >>>> > >>>> For MMC and NAND, this CONFIG_SYS_LS_PPA_ESBC_ADDR is actually > not > >>>> used. > >>>> Shall we move the assignment down to XIP section? The Kconfig > >>>> should also be updated. > >>>> > >>>> York > >>> > >>> As per PPA verification patch for eMMC/SD and NAND: > >>> > >>> --- a/arch/arm/cpu/armv8/fsl-layerscape/Kconfig > >>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/Kconfig > >>> @@ -179,12 +179,22 @@ config SYS_LS_PPA_ESBC_ADDR > >>> default 0x40740000 if SYS_LS_PPA_FW_IN_XIP && ARCH_LS1046A > >>> default 0x40480000 if SYS_LS_PPA_FW_IN_XIP && ARCH_LS1012A > >>> default 0x580c40000 if SYS_LS_PPA_FW_IN_XIP && FSL_LSCH3 > >>> + default 0x700000 if SYS_LS_PPA_FW_IN_MMC > >>> + default 0x700000 if SYS_LS_PPA_FW_IN_NAND > >>> help > >>> If the PPA header firmware locate at XIP flash, such as NOR or > >>> QSPI flash, this address is a directly memory-mapped. > >>> If it is in a serial accessed flash, such as NAND and SD > >>> card, it is a byte offset. > >>> > >>> CONFIG_SYS_LS_PPA_ESBC_ADDR is used to provide offset on eMMC/SD > >> and NAND. > >>> > >> > >> That's exactly what I was referring to. You are _NOT_ using > >> CONFIG_SYS_LS_PPA_ESBC_ADDR value for eMMC/SD or NAND. Instead, > you > >> use malloc to get a new memory for the header, which I agree it is right to > do. > >> So the macro CONFIG_SYS_LS_PPA_ESBC_ADDR is not used for either MMC > >> or NAND. > >> > >> York > > > > Actually I am using CONFIG_SYS_LS_PPA_ESBC_ADDR in case of eMMC/SD or > NAND as follows: > > > > For eMMC/SD: > > + blk = CONFIG_SYS_LS_PPA_ESBC_ADDR >> 9; > > + cnt = DIV_ROUND_UP(CONFIG_LS_PPA_ESBC_HDR_SIZE, 512); > > + ret = mmc->block_dev.block_read(&mmc->block_dev, blk, cnt, > ppa_hdr_ddr); > > + if (ret != cnt) { > > + free(ppa_hdr_ddr); > > + printf("MMC/SD read of PPA header failed\n"); > > + return -EIO; > > + } > > > > For NAND: > > + ret = nand_read(nand_info[0], > (loff_t)CONFIG_SYS_LS_PPA_ESBC_ADDR, > > + &fw_length, (u_char *)ppa_hdr_ddr); > > + if (ret == -EUCLEAN) { > > + free(ppa_hdr_ddr); > > + printf("NAND read of PPA firmware at offset 0x%x failed\n", > > + CONFIG_SYS_LS_PPA_FW_ADDR); > > + return -EIO; > > + } > > > > So you are using this macro, but not using it directly as the way for XIP. It is > confusing to see the assignment > > ppa_esbc_hdr = CONFIG_SYS_LS_PPA_ESBC_ADDR; > > I was suggesting to move this assignment down to the XIP section. > > York Yes you are correct, I will move this assignment to XIP section in next version of this patch-set. Sumit
--- a/arch/arm/cpu/armv8/fsl-layerscape/Kconfig +++ b/arch/arm/cpu/armv8/fsl-layerscape/Kconfig @@ -179,12 +179,22 @@ config SYS_LS_PPA_ESBC_ADDR default 0x40740000 if SYS_LS_PPA_FW_IN_XIP && ARCH_LS1046A default 0x40480000 if SYS_LS_PPA_FW_IN_XIP && ARCH_LS1012A default 0x580c40000 if SYS_LS_PPA_FW_IN_XIP && FSL_LSCH3 + default 0x700000 if SYS_LS_PPA_FW_IN_MMC + default 0x700000 if SYS_LS_PPA_FW_IN_NAND help If the PPA header firmware locate at XIP flash, such as NOR or QSPI flash, this address is a directly memory-mapped. If it is in a serial accessed flash, such as NAND and SD card, it is a byte offset. CONFIG_SYS_LS_PPA_ESBC_ADDR is used to provide offset on eMMC/SD and NAND. Sumit _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot