All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v1] imx6sx: Fix i.MX6SX HAB api function table offset
@ 2014-09-30 14:43 Nitin Garg
  2014-09-30 15:17 ` Fabio Estevam
  0 siblings, 1 reply; 8+ messages in thread
From: Nitin Garg @ 2014-09-30 14:43 UTC (permalink / raw)
  To: u-boot

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.garg@freescale.com>

---

 arch/arm/include/asm/arch-mx6/hab.h |   33 +++++++++++++++++++++++----------
 include/configs/mx6sxsabresd.h      |    1 +
 2 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/arch/arm/include/asm/arch-mx6/hab.h b/arch/arm/include/asm/arch-mx6/hab.h
index 1f12695..4c9734e 100644
--- a/arch/arm/include/asm/arch-mx6/hab.h
+++ b/arch/arm/include/asm/arch-mx6/hab.h
@@ -53,17 +53,30 @@ 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_REPORT_EVENT_NEW               (*(uint32_t *)0x000000B8)
-#define HAB_RVT_REPORT_STATUS_NEW              (*(uint32_t *)0x000000BC)
-#define HAB_RVT_AUTHENTICATE_IMAGE_NEW         (*(uint32_t *)0x000000A8)
-#define HAB_RVT_ENTRY_NEW                      (*(uint32_t *)0x0000009C)
-#define HAB_RVT_EXIT_NEW                       (*(uint32_t *)0x000000A0)
+#define HAB_RVT_BASE			0x00000100
+#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))
+
+#else
+
+#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)
+
+#endif
+
+#define HAB_RVT_REPORT_EVENT_NEW	(*(uint32_t *)0x000000B8)
+#define HAB_RVT_REPORT_STATUS_NEW	(*(uint32_t *)0x000000BC)
+#define HAB_RVT_AUTHENTICATE_IMAGE_NEW	(*(uint32_t *)0x000000A8)
+#define HAB_RVT_ENTRY_NEW		(*(uint32_t *)0x0000009C)
+#define HAB_RVT_EXIT_NEW		(*(uint32_t *)0x000000A0)
 
 #define HAB_CID_ROM 0 /**< ROM Caller ID */
 #define HAB_CID_UBOOT 1 /**< UBOOT Caller ID*/
diff --git a/include/configs/mx6sxsabresd.h b/include/configs/mx6sxsabresd.h
index 1eda65e..6394667 100644
--- a/include/configs/mx6sxsabresd.h
+++ b/include/configs/mx6sxsabresd.h
@@ -15,6 +15,7 @@
 #include "mx6_common.h"
 
 #define CONFIG_MX6
+#define CONFIG_ROM_UNIFIED_SECTIONS
 #define CONFIG_DISPLAY_CPUINFO
 #define CONFIG_DISPLAY_BOARDINFO
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH v1] imx6sx: Fix i.MX6SX HAB api function table offset
  2014-09-30 14:43 Nitin Garg
@ 2014-09-30 15:17 ` Fabio Estevam
  0 siblings, 0 replies; 8+ messages in thread
From: Fabio Estevam @ 2014-09-30 15:17 UTC (permalink / raw)
  To: u-boot

On Tue, Sep 30, 2014 at 11:43 AM, Nitin Garg <nitin.garg@freescale.com> 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.garg@freescale.com>

It fixes the issue for me, thanks. I have one comment though.

> --- a/include/configs/mx6sxsabresd.h
> +++ b/include/configs/mx6sxsabresd.h
> @@ -15,6 +15,7 @@
>  #include "mx6_common.h"
>
>  #define CONFIG_MX6
> +#define CONFIG_ROM_UNIFIED_SECTIONS
>  #define CONFIG_DISPLAY_CPUINFO
>  #define CONFIG_DISPLAY_BOARDINFO

It would be better to add this CONFIG_ROM_UNIFIED_SECTIONS inside mx6_common.h:

#ifdef CONFIG_MX6SX
#define CONFIG_ROM_UNIFIED_SECTIONS
#endif

with that in place you can add my:

Tested-by: Fabio Estevam <fabio.estevam@freescale.com>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH v1] imx6sx: Fix i.MX6SX HAB api function table offset
@ 2014-09-30 15:40 Nitin Garg
  2014-09-30 15:56 ` Fabio Estevam
  2014-09-30 16:08 ` Stefano Babic
  0 siblings, 2 replies; 8+ messages in thread
From: Nitin Garg @ 2014-09-30 15:40 UTC (permalink / raw)
  To: u-boot

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.garg@freescale.com>
Tested-by: Fabio Estevam <fabio.estevam@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
+
+#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
+#endif
+
 #endif
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH v1] imx6sx: Fix i.MX6SX HAB api function table offset
  2014-09-30 15:40 [U-Boot] [PATCH v1] imx6sx: Fix i.MX6SX HAB api function table offset Nitin Garg
@ 2014-09-30 15:56 ` Fabio Estevam
  2014-09-30 16:08 ` Stefano Babic
  1 sibling, 0 replies; 8+ messages in thread
From: Fabio Estevam @ 2014-09-30 15:56 UTC (permalink / raw)
  To: u-boot

On Tue, Sep 30, 2014 at 12:40 PM, Nitin Garg <nitin.garg@freescale.com> 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.garg@freescale.com>
> Tested-by: Fabio Estevam <fabio.estevam@freescale.com>

You should have marked this patch as v2 and described the changes
below the --- line.

Thanks

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH v1] imx6sx: Fix i.MX6SX HAB api function table offset
  2014-09-30 15:40 [U-Boot] [PATCH v1] imx6sx: Fix i.MX6SX HAB api function table offset Nitin Garg
  2014-09-30 15:56 ` Fabio Estevam
@ 2014-09-30 16:08 ` Stefano Babic
  2014-09-30 16:31   ` Fabio Estevam
  1 sibling, 1 reply; 8+ messages in thread
From: Stefano Babic @ 2014-09-30 16:08 UTC (permalink / raw)
  To: u-boot

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.garg@freescale.com>
> Tested-by: Fabio Estevam <fabio.estevam@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: sbabic at denx.de
=====================================================================

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH v1] imx6sx: Fix i.MX6SX HAB api function table offset
  2014-09-30 16:08 ` Stefano Babic
@ 2014-09-30 16:31   ` Fabio Estevam
  2014-09-30 17:43     ` Stefano Babic
  0 siblings, 1 reply; 8+ messages in thread
From: Fabio Estevam @ 2014-09-30 16:31 UTC (permalink / raw)
  To: u-boot

Hi Stefano,

On Tue, Sep 30, 2014 at 1:08 PM, Stefano Babic <sbabic@denx.de> wrote:

> 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.

Yes, I agree that we should avoid introducing a new config like
CONFIG_ROM_UNIFIED_SECTIONS.

What about this?

diff --git a/arch/arm/include/asm/arch-mx6/hab.h b/arch/arm/include/asm/arch-mx6
index 1f12695..c9e5318 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_
                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_MX6SX
+#define HAB_RVT_BASE                   0x00000100
+#else
+#define HAB_RVT_BASE                   0x00000094
+#endif
+
+#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)

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH v1] imx6sx: Fix i.MX6SX HAB api function table offset
  2014-09-30 16:31   ` Fabio Estevam
@ 2014-09-30 17:43     ` Stefano Babic
  2014-09-30 17:51       ` Fabio Estevam
  0 siblings, 1 reply; 8+ messages in thread
From: Stefano Babic @ 2014-09-30 17:43 UTC (permalink / raw)
  To: u-boot

Hi Fabio,

On 30/09/2014 18:31, Fabio Estevam wrote:
> Hi Stefano,
> 
> On Tue, Sep 30, 2014 at 1:08 PM, Stefano Babic <sbabic@denx.de> wrote:
> 
>> 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.
> 
> Yes, I agree that we should avoid introducing a new config like
> CONFIG_ROM_UNIFIED_SECTIONS.
> 
> What about this?
> 
> diff --git a/arch/arm/include/asm/arch-mx6/hab.h b/arch/arm/include/asm/arch-mx6
> index 1f12695..c9e5318 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_
>                 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_MX6SX
> +#define HAB_RVT_BASE                   0x00000100
> +#else
> +#define HAB_RVT_BASE                   0x00000094
> +#endif
> +
> +#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)
> 


ok - I have seen V3, I will apply it.

Regards,
Stefano

-- 
=====================================================================
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: sbabic at denx.de
=====================================================================

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [U-Boot] [PATCH v1] imx6sx: Fix i.MX6SX HAB api function table offset
  2014-09-30 17:43     ` Stefano Babic
@ 2014-09-30 17:51       ` Fabio Estevam
  0 siblings, 0 replies; 8+ messages in thread
From: Fabio Estevam @ 2014-09-30 17:51 UTC (permalink / raw)
  To: u-boot

On Tue, Sep 30, 2014 at 2:43 PM, Stefano Babic <sbabic@denx.de> wrote:

> ok - I have seen V3, I will apply it.

Thanks, Stefano!

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-09-30 17:51 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-30 15:40 [U-Boot] [PATCH v1] imx6sx: Fix i.MX6SX HAB api function table offset Nitin Garg
2014-09-30 15:56 ` Fabio Estevam
2014-09-30 16:08 ` Stefano Babic
2014-09-30 16:31   ` Fabio Estevam
2014-09-30 17:43     ` Stefano Babic
2014-09-30 17:51       ` Fabio Estevam
  -- strict thread matches above, loose matches on Subject: below --
2014-09-30 14:43 Nitin Garg
2014-09-30 15:17 ` Fabio Estevam

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.