diff mbox series

ARM: at91: fix build for SAMA5D3 w/o L2 cache

Message ID b7f8dacc-5e1f-0eb2-188e-3ad9a9f7613d@axentia.se
State New
Headers
Series ARM: at91: fix build for SAMA5D3 w/o L2 cache |

Commit Message

Peter Rosin Nov. 12, 2022, 3:40 p.m. UTC 
  The L2 cache is present on the newer SAMA5D2 and SAMA5D4 families, but
apparently not for the older SAMA5D3. At least not always.

Solves a build-time regression with the following symptom:

sama5.c:(.init.text+0x48): undefined reference to `outer_cache'

Fixes: 3b5a7ca7d252 ("ARM: at91: setup outer cache .write_sec() callback if needed")
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 arch/arm/mach-at91/sama5.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Hi!

I'm not sure this is the correct solution? Maybe SAMA5D3 should bring
in CONFIG_OUTER_CACHE unconditionally instead? But that seems like a
bigger change, and not just a tweak of the regressing commit...

Cheers,
Peter

Comments

Peter Rosin Nov. 18, 2022, 9:38 p.m. UTC | #1 
Hi!

2022-11-12 at 16:40, Peter Rosin wrote:
> The L2 cache is present on the newer SAMA5D2 and SAMA5D4 families, but
> apparently not for the older SAMA5D3. At least not always.
> 
> Solves a build-time regression with the following symptom:
> 
> sama5.c:(.init.text+0x48): undefined reference to `outer_cache'
> 
> Fixes: 3b5a7ca7d252 ("ARM: at91: setup outer cache .write_sec() callback if needed")
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
>  arch/arm/mach-at91/sama5.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Hi!
> 
> I'm not sure this is the correct solution? Maybe SAMA5D3 should bring
> in CONFIG_OUTER_CACHE unconditionally instead? But that seems like a
> bigger change, and not just a tweak of the regressing commit...
> 
> Cheers,
> Peter
> 
> diff --git a/arch/arm/mach-at91/sama5.c b/arch/arm/mach-at91/sama5.c
> index 67ed68fbe3a5..bf2b5c6a18c6 100644
> --- a/arch/arm/mach-at91/sama5.c
> +++ b/arch/arm/mach-at91/sama5.c
> @@ -26,7 +26,7 @@ static void sama5_l2c310_write_sec(unsigned long val, unsigned reg)
>  static void __init sama5_secure_cache_init(void)
>  {
>  	sam_secure_init();
> -	if (sam_linux_is_optee_available())
> +	if (IS_ENABLED(CONFIG_OUTER_CACHE) && sam_linux_is_optee_available())
>  		outer_cache.write_sec = sama5_l2c310_write_sec;
>  }
>  

It's been a week or so of silence, thus CC:ing the regression bot so
that this issue is not lost.

Cheers,
Peter

#regzbot ^introduced: 3b5a7ca7d252 
#regzbot title: Build-time failure for SAMA5D3
Linux regression tracking (Thorsten Leemhuis) Nov. 22, 2022, 3:13 p.m. UTC | #2 
Hi, this is your Linux kernel regression tracker.

On 12.11.22 16:40, Peter Rosin wrote:
> The L2 cache is present on the newer SAMA5D2 and SAMA5D4 families, but
> apparently not for the older SAMA5D3. At least not always.
> 
> Solves a build-time regression with the following symptom:
> 
> sama5.c:(.init.text+0x48): undefined reference to `outer_cache'
> 
> Fixes: 3b5a7ca7d252 ("ARM: at91: setup outer cache .write_sec() callback if needed")
> Signed-off-by: Peter Rosin <peda@axentia.se>

Clément Léger and Claudiu Beznea: what's up here? Is there a particular
reason why this patch did get any feedback from you by now? It's ten
days old and Peter already sent a kind of reminder a few days ago.

Reminder, ideally this regression should be fixed by now. For details
see the section "Prioritize work on fixing regressions"  in
Documentation/process/handling-regressions.rst (
https://docs.kernel.org/process/handling-regressions.html )

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)

P.S.: As the Linux kernel's regression tracker I deal with a lot of
reports and sometimes miss something important when writing mails like
this. If that's the case here, don't hesitate to tell me in a public
reply, it's in everyone's interest to set the public record straight.

> ---
>  arch/arm/mach-at91/sama5.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Hi!
> 
> I'm not sure this is the correct solution? Maybe SAMA5D3 should bring
> in CONFIG_OUTER_CACHE unconditionally instead? But that seems like a
> bigger change, and not just a tweak of the regressing commit...
> 
> Cheers,
> Peter
> 
> diff --git a/arch/arm/mach-at91/sama5.c b/arch/arm/mach-at91/sama5.c
> index 67ed68fbe3a5..bf2b5c6a18c6 100644
> --- a/arch/arm/mach-at91/sama5.c
> +++ b/arch/arm/mach-at91/sama5.c
> @@ -26,7 +26,7 @@ static void sama5_l2c310_write_sec(unsigned long val, unsigned reg)
>  static void __init sama5_secure_cache_init(void)
>  {
>  	sam_secure_init();
> -	if (sam_linux_is_optee_available())
> +	if (IS_ENABLED(CONFIG_OUTER_CACHE) && sam_linux_is_optee_available())
>  		outer_cache.write_sec = sama5_l2c310_write_sec;
>  }
>
Clément Léger Nov. 22, 2022, 5:14 p.m. UTC | #3 
Le Tue, 22 Nov 2022 16:13:40 +0100,
Thorsten Leemhuis <regressions@leemhuis.info> a écrit :

> Hi, this is your Linux kernel regression tracker.
> 
> On 12.11.22 16:40, Peter Rosin wrote:
> > The L2 cache is present on the newer SAMA5D2 and SAMA5D4 families, but
> > apparently not for the older SAMA5D3. At least not always.
> > 
> > Solves a build-time regression with the following symptom:
> > 
> > sama5.c:(.init.text+0x48): undefined reference to `outer_cache'
> > 
> > Fixes: 3b5a7ca7d252 ("ARM: at91: setup outer cache .write_sec() callback if needed")
> > Signed-off-by: Peter Rosin <peda@axentia.se>  
> 
> Clément Léger and Claudiu Beznea: what's up here? Is there a particular
> reason why this patch did get any feedback from you by now? It's ten
> days old and Peter already sent a kind of reminder a few days ago.

Hi Thorsten,

Sorry for the lack of answer, I'm not sure about the best solution to
tackle this problem. adding IS_ENABLED(CONFIG_OUTER_CACHE) seems like a
good way to avoid modifying the whole configuration. If ok for Claudiu,
I think it is the best thing to do since it will work for all cases.

Clément

> 
> Reminder, ideally this regression should be fixed by now. For details
> see the section "Prioritize work on fixing regressions"  in
> Documentation/process/handling-regressions.rst (
> https://docs.kernel.org/process/handling-regressions.html )
> 
> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> 
> P.S.: As the Linux kernel's regression tracker I deal with a lot of
> reports and sometimes miss something important when writing mails like
> this. If that's the case here, don't hesitate to tell me in a public
> reply, it's in everyone's interest to set the public record straight.
> 
> > ---
> >  arch/arm/mach-at91/sama5.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > Hi!
> > 
> > I'm not sure this is the correct solution? Maybe SAMA5D3 should bring
> > in CONFIG_OUTER_CACHE unconditionally instead? But that seems like a
> > bigger change, and not just a tweak of the regressing commit...
> > 
> > Cheers,
> > Peter
> > 
> > diff --git a/arch/arm/mach-at91/sama5.c b/arch/arm/mach-at91/sama5.c
> > index 67ed68fbe3a5..bf2b5c6a18c6 100644
> > --- a/arch/arm/mach-at91/sama5.c
> > +++ b/arch/arm/mach-at91/sama5.c
> > @@ -26,7 +26,7 @@ static void sama5_l2c310_write_sec(unsigned long val, unsigned reg)
> >  static void __init sama5_secure_cache_init(void)
> >  {
> >  	sam_secure_init();
> > -	if (sam_linux_is_optee_available())
> > +	if (IS_ENABLED(CONFIG_OUTER_CACHE) && sam_linux_is_optee_available())
> >  		outer_cache.write_sec = sama5_l2c310_write_sec;
> >  }
> >
Claudiu Beznea Nov. 23, 2022, 7:19 a.m. UTC | #4 
On 22.11.2022 19:14, Clément Léger wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Le Tue, 22 Nov 2022 16:13:40 +0100,
> Thorsten Leemhuis <regressions@leemhuis.info> a écrit :
> 
>> Hi, this is your Linux kernel regression tracker.
>>
>> On 12.11.22 16:40, Peter Rosin wrote:
>>> The L2 cache is present on the newer SAMA5D2 and SAMA5D4 families, but
>>> apparently not for the older SAMA5D3. At least not always.

Peter, what do you mean by "at least not always" here? Are you talking
about the OUTER_CACHE flag?

>>>
>>> Solves a build-time regression with the following symptom:
>>>
>>> sama5.c:(.init.text+0x48): undefined reference to `outer_cache'
>>>
>>> Fixes: 3b5a7ca7d252 ("ARM: at91: setup outer cache .write_sec() callback if needed")
>>> Signed-off-by: Peter Rosin <peda@axentia.se>
>>
>> Clément Léger and Claudiu Beznea: what's up here? 

It will be in the next AT91 fixes PR.

Is there a particular
>> reason why this patch did get any feedback from you by now? It's ten

Something wrong with the email client on my side as I lost this patch. It
is back on my radar since Peter replied to it.


>> days old and Peter already sent a kind of reminder a few days ago.
> 
> Hi Thorsten,
> 
> Sorry for the lack of answer, I'm not sure about the best solution to
> tackle this problem. adding IS_ENABLED(CONFIG_OUTER_CACHE) seems like a
> good way to avoid modifying the whole configuration. If ok for Claudiu,
> I think it is the best thing to do since it will work for all cases.
> 
> Clément
> 
>>
>> Reminder, ideally this regression should be fixed by now. For details
>> see the section "Prioritize work on fixing regressions"  in
>> Documentation/process/handling-regressions.rst (
>> https://docs.kernel.org/process/handling-regressions.html )
>>
>> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
>>
>> P.S.: As the Linux kernel's regression tracker I deal with a lot of
>> reports and sometimes miss something important when writing mails like
>> this. If that's the case here, don't hesitate to tell me in a public
>> reply, it's in everyone's interest to set the public record straight.
>>
>>> ---
>>>  arch/arm/mach-at91/sama5.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> Hi!
>>>
>>> I'm not sure this is the correct solution? Maybe SAMA5D3 should bring
>>> in CONFIG_OUTER_CACHE unconditionally instead? But that seems like a
>>> bigger change, and not just a tweak of the regressing commit...
>>>
>>> Cheers,
>>> Peter
>>>
>>> diff --git a/arch/arm/mach-at91/sama5.c b/arch/arm/mach-at91/sama5.c
>>> index 67ed68fbe3a5..bf2b5c6a18c6 100644
>>> --- a/arch/arm/mach-at91/sama5.c
>>> +++ b/arch/arm/mach-at91/sama5.c
>>> @@ -26,7 +26,7 @@ static void sama5_l2c310_write_sec(unsigned long val, unsigned reg)
>>>  static void __init sama5_secure_cache_init(void)
>>>  {
>>>     sam_secure_init();
>>> -   if (sam_linux_is_optee_available())
>>> +   if (IS_ENABLED(CONFIG_OUTER_CACHE) && sam_linux_is_optee_available())
>>>             outer_cache.write_sec = sama5_l2c310_write_sec;
>>>  }
>>>
> 
> 
> 
> --
> Clément Léger,
> Embedded Linux and Kernel engineer at Bootlin
> https://bootlin.com
Peter Rosin Nov. 23, 2022, 8:38 a.m. UTC | #5 
Hi!

2022-11-23 at 08:19, Claudiu.Beznea@microchip.com wrote:
> On 22.11.2022 19:14, Clément Léger wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> Le Tue, 22 Nov 2022 16:13:40 +0100,
>> Thorsten Leemhuis <regressions@leemhuis.info> a écrit :
>>
>>> Hi, this is your Linux kernel regression tracker.
>>>
>>> On 12.11.22 16:40, Peter Rosin wrote:
>>>> The L2 cache is present on the newer SAMA5D2 and SAMA5D4 families, but
>>>> apparently not for the older SAMA5D3. At least not always.
> 
> Peter, what do you mean by "at least not always" here? Are you talking
> about the OUTER_CACHE flag?

I'm not familiar with all options for L2 caching. I was just being cautious
to not exclude the possibility that there could be some variation within
the SAMA5D3 series (I'm on SAMA5D31) or with an external L2 cache or
something such. If there's simply no possible way to have an L2 cache on
any SAMA5D3, feel free to edit that "At least not always" out while you
commit.

>>>>
>>>> Solves a build-time regression with the following symptom:
>>>>
>>>> sama5.c:(.init.text+0x48): undefined reference to `outer_cache'
>>>>
>>>> Fixes: 3b5a7ca7d252 ("ARM: at91: setup outer cache .write_sec() callback if needed")
>>>> Signed-off-by: Peter Rosin <peda@axentia.se>
>>>
>>> Clément Léger and Claudiu Beznea: what's up here? 
> 
> It will be in the next AT91 fixes PR.

Great, thanks!

Cheers,
Peter

*snip*
Nicolas Ferre Nov. 23, 2022, 10:40 a.m. UTC | #6 
On 23/11/2022 at 09:38, Peter Rosin wrote:
> Hi!
> 
> 2022-11-23 at 08:19, Claudiu.Beznea@microchip.com wrote:
>> On 22.11.2022 19:14, Clément Léger wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> Le Tue, 22 Nov 2022 16:13:40 +0100,
>>> Thorsten Leemhuis <regressions@leemhuis.info> a écrit :
>>>
>>>> Hi, this is your Linux kernel regression tracker.
>>>>
>>>> On 12.11.22 16:40, Peter Rosin wrote:
>>>>> The L2 cache is present on the newer SAMA5D2 and SAMA5D4 families, but
>>>>> apparently not for the older SAMA5D3. At least not always.
>>
>> Peter, what do you mean by "at least not always" here? Are you talking
>> about the OUTER_CACHE flag?
> 
> I'm not familiar with all options for L2 caching. I was just being cautious
> to not exclude the possibility that there could be some variation within
> the SAMA5D3 series (I'm on SAMA5D31) or with an external L2 cache or
> something such. If there's simply no possible way to have an L2 cache on
> any SAMA5D3, feel free to edit that "At least not always" out while you
> commit.

I confirm that there is no L2 cache in any variant of SAMA5D3.

[..]

Thanks, best regards,
   Nicolas
Claudiu Beznea Nov. 24, 2022, 10:52 a.m. UTC | #7 
On 12.11.2022 17:40, Peter Rosin wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> The L2 cache is present on the newer SAMA5D2 and SAMA5D4 families, but
> apparently not for the older SAMA5D3. At least not always.
> 
> Solves a build-time regression with the following symptom:
> 
> sama5.c:(.init.text+0x48): undefined reference to `outer_cache'
> 
> Fixes: 3b5a7ca7d252 ("ARM: at91: setup outer cache .write_sec() callback if needed")
> Signed-off-by: Peter Rosin <peda@axentia.se>

Applied to at91-fixes, thanks!

> ---
>  arch/arm/mach-at91/sama5.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Hi!
> 
> I'm not sure this is the correct solution? Maybe SAMA5D3 should bring
> in CONFIG_OUTER_CACHE unconditionally instead? But that seems like a
> bigger change, and not just a tweak of the regressing commit...
> 
> Cheers,
> Peter
> 
> diff --git a/arch/arm/mach-at91/sama5.c b/arch/arm/mach-at91/sama5.c
> index 67ed68fbe3a5..bf2b5c6a18c6 100644
> --- a/arch/arm/mach-at91/sama5.c
> +++ b/arch/arm/mach-at91/sama5.c
> @@ -26,7 +26,7 @@ static void sama5_l2c310_write_sec(unsigned long val, unsigned reg)
>  static void __init sama5_secure_cache_init(void)
>  {
>         sam_secure_init();
> -       if (sam_linux_is_optee_available())
> +       if (IS_ENABLED(CONFIG_OUTER_CACHE) && sam_linux_is_optee_available())
>                 outer_cache.write_sec = sama5_l2c310_write_sec;
>  }
> 
> --
> 2.20.1
>
diff mbox series

Patch

diff --git a/arch/arm/mach-at91/sama5.c b/arch/arm/mach-at91/sama5.c
index 67ed68fbe3a5..bf2b5c6a18c6 100644
--- a/arch/arm/mach-at91/sama5.c
+++ b/arch/arm/mach-at91/sama5.c
@@ -26,7 +26,7 @@  static void sama5_l2c310_write_sec(unsigned long val, unsigned reg)
 static void __init sama5_secure_cache_init(void)
 {
 	sam_secure_init();
-	if (sam_linux_is_optee_available())
+	if (IS_ENABLED(CONFIG_OUTER_CACHE) && sam_linux_is_optee_available())
 		outer_cache.write_sec = sama5_l2c310_write_sec;
 }