mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH v4] ARM: i.MX8M: smccc: allow SIP_BUILDINFO call on all imx8m variants
@ 2023-01-12  8:15 Johannes Schneider
  2023-01-12 12:56 ` Ahmad Fatoum
  0 siblings, 1 reply; 4+ messages in thread
From: Johannes Schneider @ 2023-01-12  8:15 UTC (permalink / raw)
  To: barebox; +Cc: Johannes Schneider

By adding a common config switch, the imx8m_init code becomes generic
for all board variants (and slightly less confusing).

Signed-off-by: Johannes Schneider <johannes.schneider@leica-geosystems.com>

---
v4: add missing common switch

v3: redid patch with suggested common config switch

v2: was:
[PATCH v2 1/1] ARM: i.MX8M: smccc: fix firmware_atf check

v1: or'ed all IS_ENABLED(FIRMWARE_IMX8M[mnpq]_ATF
---
 arch/arm/mach-imx/imx8m.c | 2 +-
 firmware/Kconfig          | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-imx/imx8m.c b/arch/arm/mach-imx/imx8m.c
index 9758525b54..4399dc7dd5 100644
--- a/arch/arm/mach-imx/imx8m.c
+++ b/arch/arm/mach-imx/imx8m.c
@@ -61,7 +61,7 @@ static int imx8m_init(const char *cputypestr)
 	pr_info("%s unique ID: %llx\n", cputypestr, imx8m_uid());
 
 	if (IS_ENABLED(CONFIG_ARM_SMCCC) &&
-	    IS_ENABLED(CONFIG_FIRMWARE_IMX8MQ_ATF)) {
+	    IS_ENABLED(CONFIG_FIRMWARE_IMX8M_ATF)) {
 		arm_smccc_smc(IMX_SIP_BUILDINFO,
 			      IMX_SIP_BUILDINFO_GET_COMMITHASH,
 			      0, 0, 0, 0, 0, 0, &res);
diff --git a/firmware/Kconfig b/firmware/Kconfig
index b4a6fd9137..09ad85c539 100644
--- a/firmware/Kconfig
+++ b/firmware/Kconfig
@@ -12,17 +12,24 @@ config FIRMWARE_IMX_LPDDR4_PMU_TRAIN
 config FIRMWARE_IMX_DDR4_PMU_TRAIN
        	bool
 
+config FIRMWARE_IMX8M_ATF
+	bool
+
 config FIRMWARE_IMX8MM_ATF
         bool
+	select FIRMWARE_IMX8M_ATF
 
 config FIRMWARE_IMX8MN_ATF
         bool
+	select FIRMWARE_IMX8M_ATF
 
 config FIRMWARE_IMX8MP_ATF
         bool
+	select FIRMWARE_IMX8M_ATF
 
 config FIRMWARE_IMX8MQ_ATF
         bool
+	select FIRMWARE_IMX8M_ATF
 
 config FIRMWARE_CCBV2_OPTEE
 	bool
-- 
2.25.1




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

* Re: [PATCH v4] ARM: i.MX8M: smccc: allow SIP_BUILDINFO call on all imx8m variants
  2023-01-12  8:15 [PATCH v4] ARM: i.MX8M: smccc: allow SIP_BUILDINFO call on all imx8m variants Johannes Schneider
@ 2023-01-12 12:56 ` Ahmad Fatoum
  2023-01-12 13:57   ` SCHNEIDER Johannes
  0 siblings, 1 reply; 4+ messages in thread
From: Ahmad Fatoum @ 2023-01-12 12:56 UTC (permalink / raw)
  To: Johannes Schneider, barebox

Hello Johannes,

On 12.01.23 09:15, Johannes Schneider wrote:
> By adding a common config switch, the imx8m_init code becomes generic
> for all board variants (and slightly less confusing).
> 
> Signed-off-by: Johannes Schneider <johannes.schneider@leica-geosystems.com>
> 
> ---
> v4: add missing common switch
> 
> v3: redid patch with suggested common config switch

I am fine with either current_el() check or what you do here,
but given that you switched between them in revisions, I am
curious what the reasoning is.

Cheers,
Ahmad

> 
> v2: was:
> [PATCH v2 1/1] ARM: i.MX8M: smccc: fix firmware_atf check
> 
> v1: or'ed all IS_ENABLED(FIRMWARE_IMX8M[mnpq]_ATF
> ---
>  arch/arm/mach-imx/imx8m.c | 2 +-
>  firmware/Kconfig          | 7 +++++++
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-imx/imx8m.c b/arch/arm/mach-imx/imx8m.c
> index 9758525b54..4399dc7dd5 100644
> --- a/arch/arm/mach-imx/imx8m.c
> +++ b/arch/arm/mach-imx/imx8m.c
> @@ -61,7 +61,7 @@ static int imx8m_init(const char *cputypestr)
>  	pr_info("%s unique ID: %llx\n", cputypestr, imx8m_uid());
>  
>  	if (IS_ENABLED(CONFIG_ARM_SMCCC) &&
> -	    IS_ENABLED(CONFIG_FIRMWARE_IMX8MQ_ATF)) {
> +	    IS_ENABLED(CONFIG_FIRMWARE_IMX8M_ATF)) {
>  		arm_smccc_smc(IMX_SIP_BUILDINFO,
>  			      IMX_SIP_BUILDINFO_GET_COMMITHASH,
>  			      0, 0, 0, 0, 0, 0, &res);
> diff --git a/firmware/Kconfig b/firmware/Kconfig
> index b4a6fd9137..09ad85c539 100644
> --- a/firmware/Kconfig
> +++ b/firmware/Kconfig
> @@ -12,17 +12,24 @@ config FIRMWARE_IMX_LPDDR4_PMU_TRAIN
>  config FIRMWARE_IMX_DDR4_PMU_TRAIN
>         	bool
>  
> +config FIRMWARE_IMX8M_ATF
> +	bool
> +
>  config FIRMWARE_IMX8MM_ATF
>          bool
> +	select FIRMWARE_IMX8M_ATF

Your whitespacing are ok, but I see now that the file has
a mix of whitespace and tabs. I just sent out a patch for
this.

Cheers,
Ahmad

>  
>  config FIRMWARE_IMX8MN_ATF
>          bool
> +	select FIRMWARE_IMX8M_ATF
>  
>  config FIRMWARE_IMX8MP_ATF
>          bool
> +	select FIRMWARE_IMX8M_ATF
>  
>  config FIRMWARE_IMX8MQ_ATF
>          bool
> +	select FIRMWARE_IMX8M_ATF
>  
>  config FIRMWARE_CCBV2_OPTEE
>  	bool

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |




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

* Re: [PATCH v4] ARM: i.MX8M: smccc: allow SIP_BUILDINFO call on all imx8m variants
  2023-01-12 12:56 ` Ahmad Fatoum
@ 2023-01-12 13:57   ` SCHNEIDER Johannes
  2023-01-12 15:14     ` Sascha Hauer
  0 siblings, 1 reply; 4+ messages in thread
From: SCHNEIDER Johannes @ 2023-01-12 13:57 UTC (permalink / raw)
  To: Ahmad Fatoum, barebox

Hoi,

reasoning is that the common config switch limits the codeblock to imx8m boards only... or at least was, since on second thought: this is inside imx8m.c... so the previous version with current_el might be better? preferences? :-)

regards

________________________________________
From: Ahmad Fatoum <a.fatoum@pengutronix.de>
Sent: Thursday, January 12, 2023 13:56
To: SCHNEIDER Johannes; barebox@lists.infradead.org
Subject: Re: [PATCH v4] ARM: i.MX8M: smccc: allow SIP_BUILDINFO call on all imx8m variants

This email is not from Hexagon’s Office 365 instance. Please be careful while clicking links, opening attachments, or replying to this email.


Hello Johannes,

On 12.01.23 09:15, Johannes Schneider wrote:
> By adding a common config switch, the imx8m_init code becomes generic
> for all board variants (and slightly less confusing).
>
> Signed-off-by: Johannes Schneider <johannes.schneider@leica-geosystems.com>
>
> ---
> v4: add missing common switch
>
> v3: redid patch with suggested common config switch

I am fine with either current_el() check or what you do here,
but given that you switched between them in revisions, I am
curious what the reasoning is.

Cheers,
Ahmad

>
> v2: was:
> [PATCH v2 1/1] ARM: i.MX8M: smccc: fix firmware_atf check
>
> v1: or'ed all IS_ENABLED(FIRMWARE_IMX8M[mnpq]_ATF
> ---
>  arch/arm/mach-imx/imx8m.c | 2 +-
>  firmware/Kconfig          | 7 +++++++
>  2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-imx/imx8m.c b/arch/arm/mach-imx/imx8m.c
> index 9758525b54..4399dc7dd5 100644
> --- a/arch/arm/mach-imx/imx8m.c
> +++ b/arch/arm/mach-imx/imx8m.c
> @@ -61,7 +61,7 @@ static int imx8m_init(const char *cputypestr)
>       pr_info("%s unique ID: %llx\n", cputypestr, imx8m_uid());
>
>       if (IS_ENABLED(CONFIG_ARM_SMCCC) &&
> -         IS_ENABLED(CONFIG_FIRMWARE_IMX8MQ_ATF)) {
> +         IS_ENABLED(CONFIG_FIRMWARE_IMX8M_ATF)) {
>               arm_smccc_smc(IMX_SIP_BUILDINFO,
>                             IMX_SIP_BUILDINFO_GET_COMMITHASH,
>                             0, 0, 0, 0, 0, 0, &res);
> diff --git a/firmware/Kconfig b/firmware/Kconfig
> index b4a6fd9137..09ad85c539 100644
> --- a/firmware/Kconfig
> +++ b/firmware/Kconfig
> @@ -12,17 +12,24 @@ config FIRMWARE_IMX_LPDDR4_PMU_TRAIN
>  config FIRMWARE_IMX_DDR4_PMU_TRAIN
>               bool
>
> +config FIRMWARE_IMX8M_ATF
> +     bool
> +
>  config FIRMWARE_IMX8MM_ATF
>          bool
> +     select FIRMWARE_IMX8M_ATF

Your whitespacing are ok, but I see now that the file has
a mix of whitespace and tabs. I just sent out a patch for
this.

Cheers,
Ahmad

>
>  config FIRMWARE_IMX8MN_ATF
>          bool
> +     select FIRMWARE_IMX8M_ATF
>
>  config FIRMWARE_IMX8MP_ATF
>          bool
> +     select FIRMWARE_IMX8M_ATF
>
>  config FIRMWARE_IMX8MQ_ATF
>          bool
> +     select FIRMWARE_IMX8M_ATF
>
>  config FIRMWARE_CCBV2_OPTEE
>       bool

--
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | https://eur02.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.pengutronix.de%2F&data=05%7C01%7C%7C70cd0fd76baf40bfb3f208daf49c74d9%7C1b16ab3eb8f64fe39f3e2db7fe549f6a%7C0%7C0%7C638091250067510526%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=%2B6RznkD0xTQ5Z%2BWuYN4NC4pvjVbFIMRaffz90gnyh3s%3D&reserved=0  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |




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

* Re: [PATCH v4] ARM: i.MX8M: smccc: allow SIP_BUILDINFO call on all imx8m variants
  2023-01-12 13:57   ` SCHNEIDER Johannes
@ 2023-01-12 15:14     ` Sascha Hauer
  0 siblings, 0 replies; 4+ messages in thread
From: Sascha Hauer @ 2023-01-12 15:14 UTC (permalink / raw)
  To: SCHNEIDER Johannes; +Cc: Ahmad Fatoum, barebox

On Thu, Jan 12, 2023 at 01:57:27PM +0000, SCHNEIDER Johannes wrote:
> Hoi,
> 
> reasoning is that the common config switch limits the codeblock to
> imx8m boards only... or at least was, since on second thought: this is
> inside imx8m.c... so the previous version with current_el might be
> better? preferences? :-)

I like the current_el() approach better. As you said imx8m_init() is
called on i.MX8M only anyway, so no need to limit execution to i.MX8M.
Also just having CONFIG_FIRMWARE_IMX8MQ_ATF enabled doesn't necessarily
mean that the board code has actually loaded the TF-A binary. Using
current_el() gracefuly handles this case as well.

Sascha

> 
> regards
> 
> ________________________________________
> From: Ahmad Fatoum <a.fatoum@pengutronix.de>
> Sent: Thursday, January 12, 2023 13:56
> To: SCHNEIDER Johannes; barebox@lists.infradead.org
> Subject: Re: [PATCH v4] ARM: i.MX8M: smccc: allow SIP_BUILDINFO call on all imx8m variants
> 
> This email is not from Hexagon’s Office 365 instance. Please be careful while clicking links, opening attachments, or replying to this email.
> 
> 
> Hello Johannes,
> 
> On 12.01.23 09:15, Johannes Schneider wrote:
> > By adding a common config switch, the imx8m_init code becomes generic
> > for all board variants (and slightly less confusing).
> >
> > Signed-off-by: Johannes Schneider <johannes.schneider@leica-geosystems.com>
> >
> > ---
> > v4: add missing common switch
> >
> > v3: redid patch with suggested common config switch
> 
> I am fine with either current_el() check or what you do here,
> but given that you switched between them in revisions, I am
> curious what the reasoning is.
> 
> Cheers,
> Ahmad
> 
> >
> > v2: was:
> > [PATCH v2 1/1] ARM: i.MX8M: smccc: fix firmware_atf check
> >
> > v1: or'ed all IS_ENABLED(FIRMWARE_IMX8M[mnpq]_ATF
> > ---
> >  arch/arm/mach-imx/imx8m.c | 2 +-
> >  firmware/Kconfig          | 7 +++++++
> >  2 files changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/mach-imx/imx8m.c b/arch/arm/mach-imx/imx8m.c
> > index 9758525b54..4399dc7dd5 100644
> > --- a/arch/arm/mach-imx/imx8m.c
> > +++ b/arch/arm/mach-imx/imx8m.c
> > @@ -61,7 +61,7 @@ static int imx8m_init(const char *cputypestr)
> >       pr_info("%s unique ID: %llx\n", cputypestr, imx8m_uid());
> >
> >       if (IS_ENABLED(CONFIG_ARM_SMCCC) &&
> > -         IS_ENABLED(CONFIG_FIRMWARE_IMX8MQ_ATF)) {
> > +         IS_ENABLED(CONFIG_FIRMWARE_IMX8M_ATF)) {
> >               arm_smccc_smc(IMX_SIP_BUILDINFO,
> >                             IMX_SIP_BUILDINFO_GET_COMMITHASH,
> >                             0, 0, 0, 0, 0, 0, &res);
> > diff --git a/firmware/Kconfig b/firmware/Kconfig
> > index b4a6fd9137..09ad85c539 100644
> > --- a/firmware/Kconfig
> > +++ b/firmware/Kconfig
> > @@ -12,17 +12,24 @@ config FIRMWARE_IMX_LPDDR4_PMU_TRAIN
> >  config FIRMWARE_IMX_DDR4_PMU_TRAIN
> >               bool
> >
> > +config FIRMWARE_IMX8M_ATF
> > +     bool
> > +
> >  config FIRMWARE_IMX8MM_ATF
> >          bool
> > +     select FIRMWARE_IMX8M_ATF
> 
> Your whitespacing are ok, but I see now that the file has
> a mix of whitespace and tabs. I just sent out a patch for
> this.
> 
> Cheers,
> Ahmad
> 
> >
> >  config FIRMWARE_IMX8MN_ATF
> >          bool
> > +     select FIRMWARE_IMX8M_ATF
> >
> >  config FIRMWARE_IMX8MP_ATF
> >          bool
> > +     select FIRMWARE_IMX8M_ATF
> >
> >  config FIRMWARE_IMX8MQ_ATF
> >          bool
> > +     select FIRMWARE_IMX8M_ATF
> >
> >  config FIRMWARE_CCBV2_OPTEE
> >       bool
> 
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | https://eur02.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.pengutronix.de%2F&data=05%7C01%7C%7C70cd0fd76baf40bfb3f208daf49c74d9%7C1b16ab3eb8f64fe39f3e2db7fe549f6a%7C0%7C0%7C638091250067510526%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=%2B6RznkD0xTQ5Z%2BWuYN4NC4pvjVbFIMRaffz90gnyh3s%3D&reserved=0  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



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

end of thread, other threads:[~2023-01-12 15:16 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-12  8:15 [PATCH v4] ARM: i.MX8M: smccc: allow SIP_BUILDINFO call on all imx8m variants Johannes Schneider
2023-01-12 12:56 ` Ahmad Fatoum
2023-01-12 13:57   ` SCHNEIDER Johannes
2023-01-12 15:14     ` Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox