Hi York, Thanks for your comments and suggestion!
> -----Original Message----- > From: York Sun [mailto:[email protected]] > Sent: 2016年5月28日 2:06 > To: Zhiqiang Hou <[email protected]>; [email protected]; > [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected]; [email protected] > Cc: Hou Zhiqiang <[email protected]> > Subject: Re: [PATCHv4 3/7] ARMv8/layerscape: Add FSL PPA support > > On 05/23/2016 04:48 AM, Zhiqiang Hou wrote: > > From: Hou Zhiqiang <[email protected]> > > > > The FSL Primary Protected Application (PPA) is a software component > > loaded during boot which runs in TrustZone and remains resident after > > boot. > > > > Signed-off-by: Hou Zhiqiang <[email protected]> > > --- > > V4: > > - Moved secure firmware validation API to this patch. > > - Moved secure firmware getting supported PSCI version API to this patch. > > > > V3: > > - Refactor the code. > > - Add PPA firmware version info output. > > > <snip> > > > +int sec_firmware_validate(void) > > +{ > > + void *ppa_addr; > > + > > +#ifdef CONFIG_SYS_LS_PPA_FW_IN_NOR > > + ppa_addr = (void *)CONFIG_SYS_LS_PPA_FW_ADDR; #else #error "No > > +CONFIG_SYS_LS_PPA_FW_IN_xxx defined" > > +#endif > > + > > + return ppa_firmware_validate(ppa_addr); } > > You are returning 0 when the image is valid. This function name is confusing. > How about declare it as bool, and change the name to is_sec_firmware_valid()? Yes, will correct the confusing name next version. > > + > > +#ifdef CONFIG_ARMV8_PSCI > > +unsigned int sec_firmware_support_psci_version(void) > > +{ > > + unsigned int psci_ver = 0; > > + > > + if (!sec_firmware_validate()) > > + psci_ver = ppa_support_psci_version(); > > + > > + return psci_ver; > > +} > > +#endif > > + > > +int ppa_init_pre(u64 *entry) > > +{ > > + u64 ppa_ram_addr; > > + const void *raw_image_addr; > > + size_t raw_image_size = 0; > > + size_t ppa_ram_size = ppa_get_dram_block_size(); > > + int ret; > > + > > + debug("fsl-ppa: ppa size(0x%lx)\n", ppa_ram_size); > > + > > + /* > > + * The PPA must be stored in secure memory. > > + * Append PPA to secure mmu table. > > + */ > > + ppa_ram_addr = (gd->secure_ram & > MEM_RESERVE_SECURE_ADDR_MASK) + > > + gd->arch.tlb_size; > > + > > Should we check "if (gd->secure_ram & MEM_RESERVE_SECURE_SECURED)" > before using the secure memory? Yes, will add the check next version. Thanks, Zhiqiang _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
