* [U-Boot] Commit "ARM: CPU: arm926ejs: Consolidate cache routines to common file" breaks u-boot on Dreamplug
@ 2019-02-27 11:32 Leigh Brown
2019-02-27 12:50 ` Adam Ford
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Leigh Brown @ 2019-02-27 11:32 UTC (permalink / raw)
To: u-boot
Hello,
Vagrant Cascadian asked for people to test the version of u-boot
packaged
for Debian Buster. I tested u-boot on my Dreamplug and found it was not
working correctly. I raised a bug for Debian[1] but I have also tested
with the mainline version of u-boot and found the same issues.
The first issue is that the following commit caused u-boot to no longer
be able to access usb storage on the Dreamplug:
commit 93b283d49f933f95f3a6f40762936f454ac655a8
Author: Adam Ford <aford173@gmail.com>
Date: Thu Aug 16 13:23:11 2018 -0500
ARM: CPU: arm926ejs: Consolidate cache routines to common file
Four different boards had different options for enabling cache
that were virtually all the same. This consolidates these
common functions into arch/arm/cpu/arm926ejs/cache.c
This also has the positive side-effect of enabling cache on
the Davinci (da850) boards.
Signed-off-by: Adam Ford <aford173@gmail.com>
[trini: Add mach-at91 to the list of consolidations]
Signed-off-by: Tom Rini <trini@konsulko.com>
I don't have much knowledge of ARM caching, but the following patch
makes
it work again on my Dreamplug.
diff --git a/arch/arm/mach-kirkwood/cpu.c b/arch/arm/mach-kirkwood/cpu.c
index d54de53f31..8a065d73ae 100644
--- a/arch/arm/mach-kirkwood/cpu.c
+++ b/arch/arm/mach-kirkwood/cpu.c
@@ -291,7 +291,6 @@ int arch_misc_init(void)
temp |= (1 << 22);
writefr_extra_feature_reg(temp);
- icache_enable();
/* Change reset vector to address 0x0 */
temp = get_cr();
set_cr(temp & ~CR_V);
diff --git a/include/configs/dreamplug.h b/include/configs/dreamplug.h
index f4d717213c..6348935c68 100644
--- a/include/configs/dreamplug.h
+++ b/include/configs/dreamplug.h
@@ -79,4 +79,6 @@
#define CONFIG_SYS_ATA_IDE0_OFFSET MV_SATA_PORT0_OFFSET
#endif /*CONFIG_MVSATA_IDE*/
+#define CONFIG_SYS_DCACHE_OFF
+
#endif /* _CONFIG_DREAMPLUG_H */
I'd be grateful if someone could take a look. If you need me to do any
testing or provide any more information, please let me know.
I found another issue (documented in the same bug). I'll send a
separate
email about that.
Regards,
Leigh.
--
[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=923379
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dreamplug_dcache.patch
Type: text/x-diff
Size: 736 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190227/29a5b7b2/attachment.patch>
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] Commit "ARM: CPU: arm926ejs: Consolidate cache routines to common file" breaks u-boot on Dreamplug
2019-02-27 11:32 [U-Boot] Commit "ARM: CPU: arm926ejs: Consolidate cache routines to common file" breaks u-boot on Dreamplug Leigh Brown
@ 2019-02-27 12:50 ` Adam Ford
2019-02-27 14:52 ` Leigh Brown
2019-02-28 8:24 ` Chris Packham
2019-02-28 23:55 ` Tom Rini
2 siblings, 1 reply; 6+ messages in thread
From: Adam Ford @ 2019-02-27 12:50 UTC (permalink / raw)
To: u-boot
On Wed, Feb 27, 2019 at 5:32 AM Leigh Brown <leigh@solinno.co.uk> wrote:
>
> Hello,
>
> Vagrant Cascadian asked for people to test the version of u-boot
> packaged
> for Debian Buster. I tested u-boot on my Dreamplug and found it was not
> working correctly. I raised a bug for Debian[1] but I have also tested
> with the mainline version of u-boot and found the same issues.
>
> The first issue is that the following commit caused u-boot to no longer
> be able to access usb storage on the Dreamplug:
>
> commit 93b283d49f933f95f3a6f40762936f454ac655a8
> Author: Adam Ford <aford173@gmail.com>
> Date: Thu Aug 16 13:23:11 2018 -0500
>
Sorry about that.
> ARM: CPU: arm926ejs: Consolidate cache routines to common file
>
> Four different boards had different options for enabling cache
> that were virtually all the same. This consolidates these
> common functions into arch/arm/cpu/arm926ejs/cache.c
>
> This also has the positive side-effect of enabling cache on
> the Davinci (da850) boards.
>
> Signed-off-by: Adam Ford <aford173@gmail.com>
> [trini: Add mach-at91 to the list of consolidations]
> Signed-off-by: Tom Rini <trini@konsulko.com>
>
> I don't have much knowledge of ARM caching, but the following patch
> makes
> it work again on my Dreamplug.
I am not that familiar with the ARM caching either, I was just hoping
to enable it on the da850 board and compared the various code chunks
between ARM 926 boards and noticed a common thread. Tom took it and
added some more.
>
> diff --git a/arch/arm/mach-kirkwood/cpu.c b/arch/arm/mach-kirkwood/cpu.c
> index d54de53f31..8a065d73ae 100644
> --- a/arch/arm/mach-kirkwood/cpu.c
> +++ b/arch/arm/mach-kirkwood/cpu.c
> @@ -291,7 +291,6 @@ int arch_misc_init(void)
> temp |= (1 << 22);
> writefr_extra_feature_reg(temp);
>
#ifndef CONFIG_SYS_ICACHE_OFF
> - icache_enable();
#endif
Instead of commenting out that line, try defining
CONFIG_SYS_ICACHE_OFF in your header like you did for the
CONFIG_SYS_DCACHE_OFF and encapsulate the function call with ifdef's
so any other kirkwood processors can enable/disable it independently.
> /* Change reset vector to address 0x0 */
> temp = get_cr();
> set_cr(temp & ~CR_V);
> diff --git a/include/configs/dreamplug.h b/include/configs/dreamplug.h
> index f4d717213c..6348935c68 100644
> --- a/include/configs/dreamplug.h
> +++ b/include/configs/dreamplug.h
> @@ -79,4 +79,6 @@
> #define CONFIG_SYS_ATA_IDE0_OFFSET MV_SATA_PORT0_OFFSET
> #endif /*CONFIG_MVSATA_IDE*/
>
> +#define CONFIG_SYS_DCACHE_OFF
#define CONFIG_SYS_ICACHE_OFF
> +
> #endif /* _CONFIG_DREAMPLUG_H */
>
> I'd be grateful if someone could take a look. If you need me to do any
> testing or provide any more information, please let me know.
>
> I found another issue (documented in the same bug). I'll send a
> separate
> email about that.
>
> Regards,
>
> Leigh.
>
> --
> [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=923379
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] Commit "ARM: CPU: arm926ejs: Consolidate cache routines to common file" breaks u-boot on Dreamplug
2019-02-27 12:50 ` Adam Ford
@ 2019-02-27 14:52 ` Leigh Brown
2019-02-27 15:21 ` Adam Ford
0 siblings, 1 reply; 6+ messages in thread
From: Leigh Brown @ 2019-02-27 14:52 UTC (permalink / raw)
To: u-boot
Hi Adam,
Thanks very much for your response.
On 2019-02-27 12:50, Adam Ford wrote:
> On Wed, Feb 27, 2019 at 5:32 AM Leigh Brown <leigh@solinno.co.uk>
> wrote:
>>
>> Hello,
>>
>> Vagrant Cascadian asked for people to test the version of u-boot
>> packaged
>> for Debian Buster. I tested u-boot on my Dreamplug and found it was
>> not
>> working correctly. I raised a bug for Debian[1] but I have also
>> tested
>> with the mainline version of u-boot and found the same issues.
>>
>> The first issue is that the following commit caused u-boot to no
>> longer
>> be able to access usb storage on the Dreamplug:
>>
>> commit 93b283d49f933f95f3a6f40762936f454ac655a8
>> Author: Adam Ford <aford173@gmail.com>
>> Date: Thu Aug 16 13:23:11 2018 -0500
>>
>
> Sorry about that.
>
>> ARM: CPU: arm926ejs: Consolidate cache routines to common file
>>
>> Four different boards had different options for enabling cache
>> that were virtually all the same. This consolidates these
>> common functions into arch/arm/cpu/arm926ejs/cache.c
>>
>> This also has the positive side-effect of enabling cache on
>> the Davinci (da850) boards.
>>
>> Signed-off-by: Adam Ford <aford173@gmail.com>
>> [trini: Add mach-at91 to the list of consolidations]
>> Signed-off-by: Tom Rini <trini@konsulko.com>
>>
>> I don't have much knowledge of ARM caching, but the following patch
>> makes
>> it work again on my Dreamplug.
>
> I am not that familiar with the ARM caching either, I was just hoping
> to enable it on the da850 board and compared the various code chunks
> between ARM 926 boards and noticed a common thread. Tom took it and
> added some more.
Okay. Hopefully Tom can comment on my proposed fixes.
>
>>
>> diff --git a/arch/arm/mach-kirkwood/cpu.c
>> b/arch/arm/mach-kirkwood/cpu.c
>> index d54de53f31..8a065d73ae 100644
>> --- a/arch/arm/mach-kirkwood/cpu.c
>> +++ b/arch/arm/mach-kirkwood/cpu.c
>> @@ -291,7 +291,6 @@ int arch_misc_init(void)
>> temp |= (1 << 22);
>> writefr_extra_feature_reg(temp);
>>
> #ifndef CONFIG_SYS_ICACHE_OFF
>> - icache_enable();
> #endif
>
> Instead of commenting out that line, try defining
> CONFIG_SYS_ICACHE_OFF in your header like you did for the
> CONFIG_SYS_DCACHE_OFF and encapsulate the function call with ifdef's
> so any other kirkwood processors can enable/disable it independently.
The reason I removed that line is because icache_enable() is called from
enable_caches() which is called from initr_caches() which is in the list
of functions in init_sequence_r[] prior to arch_misc_init(). In other
words, it will already have been called by then if CONFIG_SYS_ICACHE_OFF
is not set. Does that make sense?
>> /* Change reset vector to address 0x0 */
>> temp = get_cr();
>> set_cr(temp & ~CR_V);
>> diff --git a/include/configs/dreamplug.h b/include/configs/dreamplug.h
>> index f4d717213c..6348935c68 100644
>> --- a/include/configs/dreamplug.h
>> +++ b/include/configs/dreamplug.h
>> @@ -79,4 +79,6 @@
>> #define CONFIG_SYS_ATA_IDE0_OFFSET MV_SATA_PORT0_OFFSET
>> #endif /*CONFIG_MVSATA_IDE*/
>>
>> +#define CONFIG_SYS_DCACHE_OFF
> #define CONFIG_SYS_ICACHE_OFF
The reason I have not done this is because the Kirkwood arch_misc_init()
function was already unconditionally enabling the instruction cache, so
we want to retain that behaviour - I think. I hope that makes sense.
>> +
>> #endif /* _CONFIG_DREAMPLUG_H */
>>
>> I'd be grateful if someone could take a look. If you need me to do
>> any
>> testing or provide any more information, please let me know.
>>
>> I found another issue (documented in the same bug). I'll send a
>> separate
>> email about that.
>>
>> Regards,
>>
>> Leigh.
>>
>> --
>> [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=923379
Regards,
Leigh.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] Commit "ARM: CPU: arm926ejs: Consolidate cache routines to common file" breaks u-boot on Dreamplug
2019-02-27 14:52 ` Leigh Brown
@ 2019-02-27 15:21 ` Adam Ford
0 siblings, 0 replies; 6+ messages in thread
From: Adam Ford @ 2019-02-27 15:21 UTC (permalink / raw)
To: u-boot
On Wed, Feb 27, 2019 at 8:52 AM Leigh Brown <leigh@solinno.co.uk> wrote:
>
> Hi Adam,
>
> Thanks very much for your response.
>
> On 2019-02-27 12:50, Adam Ford wrote:
> > On Wed, Feb 27, 2019 at 5:32 AM Leigh Brown <leigh@solinno.co.uk>
> > wrote:
> >>
> >> Hello,
> >>
> >> Vagrant Cascadian asked for people to test the version of u-boot
> >> packaged
> >> for Debian Buster. I tested u-boot on my Dreamplug and found it was
> >> not
> >> working correctly. I raised a bug for Debian[1] but I have also
> >> tested
> >> with the mainline version of u-boot and found the same issues.
> >>
> >> The first issue is that the following commit caused u-boot to no
> >> longer
> >> be able to access usb storage on the Dreamplug:
> >>
> >> commit 93b283d49f933f95f3a6f40762936f454ac655a8
> >> Author: Adam Ford <aford173@gmail.com>
> >> Date: Thu Aug 16 13:23:11 2018 -0500
> >>
> >
> > Sorry about that.
> >
> >> ARM: CPU: arm926ejs: Consolidate cache routines to common file
> >>
> >> Four different boards had different options for enabling cache
> >> that were virtually all the same. This consolidates these
> >> common functions into arch/arm/cpu/arm926ejs/cache.c
> >>
> >> This also has the positive side-effect of enabling cache on
> >> the Davinci (da850) boards.
> >>
> >> Signed-off-by: Adam Ford <aford173@gmail.com>
> >> [trini: Add mach-at91 to the list of consolidations]
> >> Signed-off-by: Tom Rini <trini@konsulko.com>
> >>
> >> I don't have much knowledge of ARM caching, but the following patch
> >> makes
> >> it work again on my Dreamplug.
> >
> > I am not that familiar with the ARM caching either, I was just hoping
> > to enable it on the da850 board and compared the various code chunks
> > between ARM 926 boards and noticed a common thread. Tom took it and
> > added some more.
>
> Okay. Hopefully Tom can comment on my proposed fixes.
>
> >
> >>
> >> diff --git a/arch/arm/mach-kirkwood/cpu.c
> >> b/arch/arm/mach-kirkwood/cpu.c
> >> index d54de53f31..8a065d73ae 100644
> >> --- a/arch/arm/mach-kirkwood/cpu.c
> >> +++ b/arch/arm/mach-kirkwood/cpu.c
> >> @@ -291,7 +291,6 @@ int arch_misc_init(void)
> >> temp |= (1 << 22);
> >> writefr_extra_feature_reg(temp);
> >>
> > #ifndef CONFIG_SYS_ICACHE_OFF
> >> - icache_enable();
> > #endif
> >
> > Instead of commenting out that line, try defining
> > CONFIG_SYS_ICACHE_OFF in your header like you did for the
> > CONFIG_SYS_DCACHE_OFF and encapsulate the function call with ifdef's
> > so any other kirkwood processors can enable/disable it independently.
>
> The reason I removed that line is because icache_enable() is called from
> enable_caches() which is called from initr_caches() which is in the list
> of functions in init_sequence_r[] prior to arch_misc_init(). In other
> words, it will already have been called by then if CONFIG_SYS_ICACHE_OFF
> is not set. Does that make sense?
That makes a lot of sense to me.
>
> >> /* Change reset vector to address 0x0 */
> >> temp = get_cr();
> >> set_cr(temp & ~CR_V);
> >> diff --git a/include/configs/dreamplug.h b/include/configs/dreamplug.h
> >> index f4d717213c..6348935c68 100644
> >> --- a/include/configs/dreamplug.h
> >> +++ b/include/configs/dreamplug.h
> >> @@ -79,4 +79,6 @@
> >> #define CONFIG_SYS_ATA_IDE0_OFFSET MV_SATA_PORT0_OFFSET
> >> #endif /*CONFIG_MVSATA_IDE*/
> >>
> >> +#define CONFIG_SYS_DCACHE_OFF
> > #define CONFIG_SYS_ICACHE_OFF
>
> The reason I have not done this is because the Kirkwood arch_misc_init()
> function was already unconditionally enabling the instruction cache, so
> we want to retain that behaviour - I think. I hope that makes sense.
That makes a lot of sense too.
adam
>
> >> +
> >> #endif /* _CONFIG_DREAMPLUG_H */
> >>
> >> I'd be grateful if someone could take a look. If you need me to do
> >> any
> >> testing or provide any more information, please let me know.
> >>
> >> I found another issue (documented in the same bug). I'll send a
> >> separate
> >> email about that.
> >>
> >> Regards,
> >>
> >> Leigh.
> >>
> >> --
> >> [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=923379
>
> Regards,
>
> Leigh.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] Commit "ARM: CPU: arm926ejs: Consolidate cache routines to common file" breaks u-boot on Dreamplug
2019-02-27 11:32 [U-Boot] Commit "ARM: CPU: arm926ejs: Consolidate cache routines to common file" breaks u-boot on Dreamplug Leigh Brown
2019-02-27 12:50 ` Adam Ford
@ 2019-02-28 8:24 ` Chris Packham
2019-02-28 23:55 ` Tom Rini
2 siblings, 0 replies; 6+ messages in thread
From: Chris Packham @ 2019-02-28 8:24 UTC (permalink / raw)
To: u-boot
Hi Leigh,
On Thu, Feb 28, 2019 at 1:11 AM Leigh Brown <leigh@solinno.co.uk> wrote:
>
> Hello,
>
> Vagrant Cascadian asked for people to test the version of u-boot
> packaged
> for Debian Buster. I tested u-boot on my Dreamplug and found it was not
> working correctly. I raised a bug for Debian[1] but I have also tested
> with the mainline version of u-boot and found the same issues.
>
> The first issue is that the following commit caused u-boot to no longer
> be able to access usb storage on the Dreamplug:
>
> commit 93b283d49f933f95f3a6f40762936f454ac655a8
> Author: Adam Ford <aford173@gmail.com>
> Date: Thu Aug 16 13:23:11 2018 -0500
>
> ARM: CPU: arm926ejs: Consolidate cache routines to common file
>
> Four different boards had different options for enabling cache
> that were virtually all the same. This consolidates these
> common functions into arch/arm/cpu/arm926ejs/cache.c
>
> This also has the positive side-effect of enabling cache on
> the Davinci (da850) boards.
>
> Signed-off-by: Adam Ford <aford173@gmail.com>
> [trini: Add mach-at91 to the list of consolidations]
> Signed-off-by: Tom Rini <trini@konsulko.com>
>
> I don't have much knowledge of ARM caching, but the following patch
> makes
> it work again on my Dreamplug.
>
> diff --git a/arch/arm/mach-kirkwood/cpu.c b/arch/arm/mach-kirkwood/cpu.c
> index d54de53f31..8a065d73ae 100644
> --- a/arch/arm/mach-kirkwood/cpu.c
> +++ b/arch/arm/mach-kirkwood/cpu.c
> @@ -291,7 +291,6 @@ int arch_misc_init(void)
> temp |= (1 << 22);
> writefr_extra_feature_reg(temp);
>
> - icache_enable();
> /* Change reset vector to address 0x0 */
> temp = get_cr();
> set_cr(temp & ~CR_V);
> diff --git a/include/configs/dreamplug.h b/include/configs/dreamplug.h
> index f4d717213c..6348935c68 100644
> --- a/include/configs/dreamplug.h
> +++ b/include/configs/dreamplug.h
> @@ -79,4 +79,6 @@
> #define CONFIG_SYS_ATA_IDE0_OFFSET MV_SATA_PORT0_OFFSET
> #endif /*CONFIG_MVSATA_IDE*/
>
> +#define CONFIG_SYS_DCACHE_OFF
> +
> #endif /* _CONFIG_DREAMPLUG_H */
>
> I'd be grateful if someone could take a look. If you need me to do any
> testing or provide any more information, please let me know.
>
Thanks for pointing this out. I'd been having a problem like this with
the Ethernet interface. I'd been blaming the hardware I was using but
your change makes things work for me.
> I found another issue (documented in the same bug). I'll send a
> separate
> email about that.
>
> Regards,
>
> Leigh.
>
> --
> [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=923379_______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] Commit "ARM: CPU: arm926ejs: Consolidate cache routines to common file" breaks u-boot on Dreamplug
2019-02-27 11:32 [U-Boot] Commit "ARM: CPU: arm926ejs: Consolidate cache routines to common file" breaks u-boot on Dreamplug Leigh Brown
2019-02-27 12:50 ` Adam Ford
2019-02-28 8:24 ` Chris Packham
@ 2019-02-28 23:55 ` Tom Rini
2 siblings, 0 replies; 6+ messages in thread
From: Tom Rini @ 2019-02-28 23:55 UTC (permalink / raw)
To: u-boot
On Wed, Feb 27, 2019 at 11:32:16AM +0000, Leigh Brown wrote:
> Hello,
>
> Vagrant Cascadian asked for people to test the version of u-boot packaged
> for Debian Buster. I tested u-boot on my Dreamplug and found it was not
> working correctly. I raised a bug for Debian[1] but I have also tested
> with the mainline version of u-boot and found the same issues.
>
> The first issue is that the following commit caused u-boot to no longer
> be able to access usb storage on the Dreamplug:
>
> commit 93b283d49f933f95f3a6f40762936f454ac655a8
> Author: Adam Ford <aford173@gmail.com>
> Date: Thu Aug 16 13:23:11 2018 -0500
>
> ARM: CPU: arm926ejs: Consolidate cache routines to common file
>
> Four different boards had different options for enabling cache
> that were virtually all the same. This consolidates these
> common functions into arch/arm/cpu/arm926ejs/cache.c
>
> This also has the positive side-effect of enabling cache on
> the Davinci (da850) boards.
>
> Signed-off-by: Adam Ford <aford173@gmail.com>
> [trini: Add mach-at91 to the list of consolidations]
> Signed-off-by: Tom Rini <trini@konsulko.com>
>
> I don't have much knowledge of ARM caching, but the following patch makes
> it work again on my Dreamplug.
>
> diff --git a/arch/arm/mach-kirkwood/cpu.c b/arch/arm/mach-kirkwood/cpu.c
> index d54de53f31..8a065d73ae 100644
Thanks for digging into this. Based on the whole thread, yes, this
makes sense.
Reviewed-by: Tom Rini <trini@konsulko.com>
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190228/e6c8caab/attachment.sig>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-02-28 23:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-02-27 11:32 [U-Boot] Commit "ARM: CPU: arm926ejs: Consolidate cache routines to common file" breaks u-boot on Dreamplug Leigh Brown
2019-02-27 12:50 ` Adam Ford
2019-02-27 14:52 ` Leigh Brown
2019-02-27 15:21 ` Adam Ford
2019-02-28 8:24 ` Chris Packham
2019-02-28 23:55 ` Tom Rini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox