Hi Nitin, hi Fabio,
thanks for immediately fix the issue - I have some comments.
On 30/09/2014 17:40, Nitin Garg wrote:
> i.MX6SX ROM implements unified table sections.
> The HAB function table is at offset 0x100. Update
> the HAB function pointers accordingly.
>
> Signed-off-by: Nitin Garg <nitin.g...@freescale.com>
> Tested-by: Fabio Estevam <fabio.este...@freescale.com>
>
> ---
>
> arch/arm/include/asm/arch-mx6/hab.h | 16 +++++++++++-----
> include/configs/mx6_common.h | 4 ++++
> 2 files changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/include/asm/arch-mx6/hab.h
> b/arch/arm/include/asm/arch-mx6/hab.h
> index 1f12695..c53709b 100644
> --- a/arch/arm/include/asm/arch-mx6/hab.h
> +++ b/arch/arm/include/asm/arch-mx6/hab.h
> @@ -53,11 +53,17 @@ typedef void *hab_rvt_authenticate_image_t(uint8_t,
> ptrdiff_t,
> void **, size_t *, hab_loader_callback_f_t);
> typedef void hapi_clock_init_t(void);
>
> -#define HAB_RVT_REPORT_EVENT (*(uint32_t *)0x000000B4)
> -#define HAB_RVT_REPORT_STATUS (*(uint32_t *)0x000000B8)
> -#define HAB_RVT_AUTHENTICATE_IMAGE (*(uint32_t *)0x000000A4)
> -#define HAB_RVT_ENTRY (*(uint32_t *)0x00000098)
> -#define HAB_RVT_EXIT (*(uint32_t *)0x0000009C)
> +#ifdef CONFIG_ROM_UNIFIED_SECTIONS
> +#define HAB_RVT_BASE 0x00000100
> +#else
> +#define HAB_RVT_BASE 0x00000094
> +#endif
It would be nice to understand how the choice is done. In my
understanding, even HAB_RVT_REPORT_EVENT_NEW and
HAB_RVT_REPORT_STATUS_NEW can be dropped and substituted by an offset.
I see then three cases:
mx6sx HAB_RVT_BASE = 0x00000100
mx6q with soc_rev() > 1.5 HAB_RVT_BASE = 0x0000098
or
mx6dl with soc_rev > 1.2
mx6q with soc_rev() < 1.5 HAB_RVT_BASE = 0x0000094
or
mx6d with soc_rev() < 1.2
then the single entries can be calculated from the base address.
> +
> +#define HAB_RVT_ENTRY (*(uint32_t *)(HAB_RVT_BASE +
> 0x04))
> +#define HAB_RVT_EXIT (*(uint32_t *)(HAB_RVT_BASE + 0x08))
> +#define HAB_RVT_AUTHENTICATE_IMAGE (*(uint32_t *)(HAB_RVT_BASE + 0x10))
> +#define HAB_RVT_REPORT_EVENT (*(uint32_t *)(HAB_RVT_BASE + 0x20))
> +#define HAB_RVT_REPORT_STATUS (*(uint32_t *)(HAB_RVT_BASE +
> 0x24))
>
> #define HAB_RVT_REPORT_EVENT_NEW (*(uint32_t *)0x000000B8)
> #define HAB_RVT_REPORT_STATUS_NEW (*(uint32_t *)0x000000BC)
> diff --git a/include/configs/mx6_common.h b/include/configs/mx6_common.h
> index 135a3f5..824e73f 100644
> --- a/include/configs/mx6_common.h
> +++ b/include/configs/mx6_common.h
> @@ -30,4 +30,8 @@
>
> #define CONFIG_MP
>
> +#ifdef CONFIG_MX6SX
> +#define CONFIG_ROM_UNIFIED_SECTIONS
I do not like this approach because we do not need an additional
CONFIG_, that remains undocumented. If CONFIG_MX6SX, HAB_RVT_BASE is
always 0x100 - CONFIG_ROM_UNIFIED_SECTIONS is like a redundant
information, because it is not possible (or is it ?) to have a sx
processor with a different base address.
Best regards,
Stefano Babic
--
=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=====================================================================
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
