public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] ARM: Generic cache ops skeleton
@ 2011-11-06  1:16 Marek Vasut
  2011-11-06  8:36 ` Mike Frysinger
  2011-11-07 12:21 ` Albert ARIBAUD
  0 siblings, 2 replies; 11+ messages in thread
From: Marek Vasut @ 2011-11-06  1:16 UTC (permalink / raw)
  To: u-boot

This patch should allow a CPU to register it's own cache ops. This shall allow
multiple CPUs with different cache handlings to be supported.

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
Cc: Albert ARIBAUD <albert.u.boot@aribaud.net>
Cc: Wolfgang Denk <wd@denx.de>
---
 arch/arm/include/asm/cache.h       |   22 ++++++++
 arch/arm/include/asm/global_data.h |    1 +
 arch/arm/lib/cache.c               |   99 ++++++++++++++++++++++++++---------
 3 files changed, 96 insertions(+), 26 deletions(-)

diff --git a/arch/arm/include/asm/cache.h b/arch/arm/include/asm/cache.h
index eef6a5a..8e1fa6c 100644
--- a/arch/arm/include/asm/cache.h
+++ b/arch/arm/include/asm/cache.h
@@ -53,4 +53,26 @@ void l2_cache_disable(void);
 #define ARCH_DMA_MINALIGN	64
 #endif
 
+enum cache_data_op {
+	CACHE_FLUSH,
+	CACHE_INVAL
+};
+
+enum cache_mgmt_op {
+	CACHE_ENABLE,
+	CACHE_DISABLE
+};
+
+struct cache_ops {
+	uint32_t cache_line_size;
+	void (*dcache_range_op)(enum cache_data_op op, u32 start, u32 end);
+	void (*dcache_whole_op)(enum cache_data_op op);
+	void (*icache_range_op)(enum cache_data_op op, u32 start, u32 end);
+	void (*icache_whole_op)(enum cache_data_op op);
+	void (*icache_ops)(enum cache_mgmt_op op);
+	void (*dcache_ops)(enum cache_mgmt_op op);
+};
+
+void cpu_register_cache_ops(struct cache_ops *ops);
+
 #endif /* _ASM_CACHE_H */
diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h
index c3ff789..e50f58f 100644
--- a/arch/arm/include/asm/global_data.h
+++ b/arch/arm/include/asm/global_data.h
@@ -38,6 +38,7 @@ typedef	struct	global_data {
 	unsigned long	flags;
 	unsigned long	baudrate;
 	unsigned long	have_console;	/* serial_init() was called */
+	struct cache_ops *cache_ops;	/* cache operations */
 #ifdef CONFIG_PRE_CONSOLE_BUFFER
 	unsigned long	precon_buf_idx;	/* Pre-Console buffer index */
 #endif
diff --git a/arch/arm/lib/cache.c b/arch/arm/lib/cache.c
index b545fb7..e8d5884 100644
--- a/arch/arm/lib/cache.c
+++ b/arch/arm/lib/cache.c
@@ -25,43 +25,90 @@
 
 #include <common.h>
 
-void  __flush_cache(unsigned long start, unsigned long size)
-{
-#if defined(CONFIG_OMAP2420) || defined(CONFIG_ARM1136)
-	void arm1136_cache_flush(void);
+DECLARE_GLOBAL_DATA_PTR;
 
-	arm1136_cache_flush();
-#endif
-#ifdef CONFIG_ARM926EJS
-	/* test and clean, page 2-23 of arm926ejs manual */
-	asm("0: mrc p15, 0, r15, c7, c10, 3\n\t" "bne 0b\n" : : : "memory");
-	/* disable write buffer as well (page 2-22) */
-	asm("mcr p15, 0, %0, c7, c10, 4" : : "r" (0));
+/* Some broken stuff which will need sorting later. */
+#if defined(CONFIG_OMAP2420) || defined(CONFIG_ARM1136) || \
+	defined(CONFIG_ARM926EJS)
+#define CONFIG_CACHE_COMPAT
 #endif
-	return;
+
+/*
+ * Generic implementation of "enable_caches()" API call.
+ *
+ * This call enable both I-cache and D-cache.
+ */
+void enable_caches(void)
+{
+	struct cache_ops *ops = gd->cache_ops;
+
+	if (!ops) {
+		puts("WARNING: Caches not enabled\n");
+		return;
+	}
+
+	ops->icache_ops(CACHE_ENABLE);
+	ops->dcache_ops(CACHE_ENABLE);
 }
-void  flush_cache(unsigned long start, unsigned long size)
-	__attribute__((weak, alias("__flush_cache")));
 
 /*
- * Default implementation:
- * do a range flush for the entire range
+ * Generic implementation of "flush_dcache_all" API call.
+ *
+ * This flushes whole D-cache.
  */
-void	__flush_dcache_all(void)
+void flush_dcache_all(void)
 {
-	flush_cache(0, ~0);
+	struct cache_ops *ops = gd->cache_ops;
+
+#ifdef CONFIG_CACHE_COMPAT
+	cache_flush_compat();
+#endif
+
+	if (!ops)
+		return;
+
+	ops->dcache_whole_op(CACHE_FLUSH);
 }
-void	flush_dcache_all(void)
-	__attribute__((weak, alias("__flush_dcache_all")));
 
+/*
+ * Generic implementation of "flush_dcache_range" API call.
+ *
+ * This flushes line in D-cache.
+ */
+void flush_dcache_range(unsigned long start, unsigned long stop)
+{
+	struct cache_ops *ops = gd->cache_ops;
+	uint32_t cache_line_mask;
+
+#ifdef CONFIG_CACHE_COMPAT
+	cache_flush_compat();
+#endif
+
+	if (!ops)
+		return;
+
+	cache_line_mask = ops->cache_line_size - 1;
+
+	if (start & cache_line_mask)
+		debug("CACHE: Unaligned start of flushed area\n");
+
+	if (stop & cache_line_mask)
+		debug("CACHE: Unaligned end of flushed area\n");
+
+	ops->dcache_range_op(CACHE_FLUSH, start, stop);
+}
 
 /*
- * Default implementation of enable_caches()
- * Real implementation should be in platform code
+ * Generic implementation of "flush_cache" API call.
+ *
+ * This flushes line in D-cache.
  */
-void __enable_caches(void)
+void flush_cache(unsigned long start, unsigned long stop)
 {
-	puts("WARNING: Caches not enabled\n");
+	flush_dcache_range(start, stop);
+}
+
+void cpu_register_cache_ops(struct cache_ops *ops)
+{
+	gd->cache_ops = ops;
 }
-void enable_caches(void)
-	__attribute__((weak, alias("__enable_caches")));
-- 
1.7.6.3

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

* [U-Boot] [PATCH] ARM: Generic cache ops skeleton
  2011-11-06  1:16 [U-Boot] [PATCH] ARM: Generic cache ops skeleton Marek Vasut
@ 2011-11-06  8:36 ` Mike Frysinger
  2011-11-06 12:39   ` Marek Vasut
  2011-11-07 12:21 ` Albert ARIBAUD
  1 sibling, 1 reply; 11+ messages in thread
From: Mike Frysinger @ 2011-11-06  8:36 UTC (permalink / raw)
  To: u-boot

On Saturday 05 November 2011 21:16:23 Marek Vasut wrote:
> This patch should allow a CPU to register it's own cache ops. This shall
> allow multiple CPUs with different cache handlings to be supported.

sorry, what's the point ?  this would make sense if you wanted to build one u-
boot image to run on multiple SoCs, but since that doesn't make sense, i don't 
see what this patch gets you.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111106/b14d0825/attachment.pgp 

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

* [U-Boot] [PATCH] ARM: Generic cache ops skeleton
  2011-11-06  8:36 ` Mike Frysinger
@ 2011-11-06 12:39   ` Marek Vasut
  2011-11-06 18:03     ` Mike Frysinger
  0 siblings, 1 reply; 11+ messages in thread
From: Marek Vasut @ 2011-11-06 12:39 UTC (permalink / raw)
  To: u-boot

> On Saturday 05 November 2011 21:16:23 Marek Vasut wrote:
> > This patch should allow a CPU to register it's own cache ops. This shall
> > allow multiple CPUs with different cache handlings to be supported.
> 
> sorry, what's the point ?  this would make sense if you wanted to build one
> u- boot image to run on multiple SoCs, but since that doesn't make sense,
> i don't see what this patch gets you.
> -mike

Well that's the ultimate goal, isn't it ...

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

* [U-Boot] [PATCH] ARM: Generic cache ops skeleton
  2011-11-06 12:39   ` Marek Vasut
@ 2011-11-06 18:03     ` Mike Frysinger
  2011-11-06 18:07       ` Marek Vasut
  0 siblings, 1 reply; 11+ messages in thread
From: Mike Frysinger @ 2011-11-06 18:03 UTC (permalink / raw)
  To: u-boot

On Sunday 06 November 2011 07:39:56 Marek Vasut wrote:
> > On Saturday 05 November 2011 21:16:23 Marek Vasut wrote:
> > > This patch should allow a CPU to register it's own cache ops. This
> > > shall allow multiple CPUs with different cache handlings to be
> > > supported.
> > 
> > sorry, what's the point ?  this would make sense if you wanted to build
> > one u- boot image to run on multiple SoCs, but since that doesn't make
> > sense, i don't see what this patch gets you.
> 
> Well that's the ultimate goal, isn't it ...

is it actually ?  i must have missed the memo.

support for multiple-SoC-support-in-a-single-image shouldn't bloat the single 
SoC case.  this code seems to do just that.  so it sounds like you should find 
a CONFIG name for this setup and start putting everything behind that.

maybe something like CONFIG_SYS_MULTI_SOC.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111106/1a4519f3/attachment.pgp 

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

* [U-Boot] [PATCH] ARM: Generic cache ops skeleton
  2011-11-06 18:03     ` Mike Frysinger
@ 2011-11-06 18:07       ` Marek Vasut
  2011-11-06 19:29         ` Simon Glass
  2011-11-07  1:03         ` Mike Frysinger
  0 siblings, 2 replies; 11+ messages in thread
From: Marek Vasut @ 2011-11-06 18:07 UTC (permalink / raw)
  To: u-boot

> On Sunday 06 November 2011 07:39:56 Marek Vasut wrote:
> > > On Saturday 05 November 2011 21:16:23 Marek Vasut wrote:
> > > > This patch should allow a CPU to register it's own cache ops. This
> > > > shall allow multiple CPUs with different cache handlings to be
> > > > supported.
> > > 
> > > sorry, what's the point ?  this would make sense if you wanted to build
> > > one u- boot image to run on multiple SoCs, but since that doesn't make
> > > sense, i don't see what this patch gets you.
> > 
> > Well that's the ultimate goal, isn't it ...
> 
> is it actually ?  i must have missed the memo.
> 
> support for multiple-SoC-support-in-a-single-image shouldn't bloat the
> single SoC case.  this code seems to do just that.  so it sounds like you
> should find a CONFIG name for this setup and start putting everything
> behind that.

That's a good point indeed. The other question is, if gcc4.6's LTO won't fix 
that for us. Surely enough though, we can't rely on that.
> 
> maybe something like CONFIG_SYS_MULTI_SOC.

Yea

> -mike

M

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

* [U-Boot] [PATCH] ARM: Generic cache ops skeleton
  2011-11-06 18:07       ` Marek Vasut
@ 2011-11-06 19:29         ` Simon Glass
  2011-11-07  1:09           ` Mike Frysinger
  2011-11-07 12:36           ` Albert ARIBAUD
  2011-11-07  1:03         ` Mike Frysinger
  1 sibling, 2 replies; 11+ messages in thread
From: Simon Glass @ 2011-11-06 19:29 UTC (permalink / raw)
  To: u-boot

Hi,

On Sun, Nov 6, 2011 at 10:07 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> On Sunday 06 November 2011 07:39:56 Marek Vasut wrote:
>> > > On Saturday 05 November 2011 21:16:23 Marek Vasut wrote:
>> > > > This patch should allow a CPU to register it's own cache ops. This
>> > > > shall allow multiple CPUs with different cache handlings to be
>> > > > supported.
>> > >
>> > > sorry, what's the point ? ?this would make sense if you wanted to build
>> > > one u- boot image to run on multiple SoCs, but since that doesn't make
>> > > sense, i don't see what this patch gets you.
>> >
>> > Well that's the ultimate goal, isn't it ...
>>
>> is it actually ? ?i must have missed the memo.

It's actually a lot more than cache ops. SOCs have there own clock
stuff, pinmux things, power mgmt, timers and the like. While it might
be desirable to provide this feature it is a fair bit of work and we
need to be careful not to make things more complex.

I would settle for an initial step of getting some sort of unified but
simple clock/pinmux support in across all ARM. Anyone interested in
that? I mean an API that covers the lot, not an implementation. Also
it would be nice to avoid the heavy-weight data structures and strings
that Linux has. If we got that far then we could make the interface
virtual as Marek has done for caches.

On this particular patch, I feel it should be more explicit about L1
cache, which is what I think it deals with. We may want to support L2
also through a similar API. And a CONFIG option is a good idea.

Finally, even the CP15/cache/MMU code is duplicated in different
arch/arm/cpu subdirs. Can we unify this a bit?

Regards,
Simon

>>
>> support for multiple-SoC-support-in-a-single-image shouldn't bloat the
>> single SoC case. ?this code seems to do just that. ?so it sounds like you
>> should find a CONFIG name for this setup and start putting everything
>> behind that.
>
> That's a good point indeed. The other question is, if gcc4.6's LTO won't fix
> that for us. Surely enough though, we can't rely on that.
>>
>> maybe something like CONFIG_SYS_MULTI_SOC.
>
> Yea
>
>> -mike
>
> M
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

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

* [U-Boot] [PATCH] ARM: Generic cache ops skeleton
  2011-11-06 18:07       ` Marek Vasut
  2011-11-06 19:29         ` Simon Glass
@ 2011-11-07  1:03         ` Mike Frysinger
  1 sibling, 0 replies; 11+ messages in thread
From: Mike Frysinger @ 2011-11-07  1:03 UTC (permalink / raw)
  To: u-boot

On Sunday 06 November 2011 13:07:29 Marek Vasut wrote:
> > On Sunday 06 November 2011 07:39:56 Marek Vasut wrote:
> > > > On Saturday 05 November 2011 21:16:23 Marek Vasut wrote:
> > > > > This patch should allow a CPU to register it's own cache ops. This
> > > > > shall allow multiple CPUs with different cache handlings to be
> > > > > supported.
> > > > 
> > > > sorry, what's the point ?  this would make sense if you wanted to
> > > > build one u- boot image to run on multiple SoCs, but since that
> > > > doesn't make sense, i don't see what this patch gets you.
> > > 
> > > Well that's the ultimate goal, isn't it ...
> > 
> > is it actually ?  i must have missed the memo.
> > 
> > support for multiple-SoC-support-in-a-single-image shouldn't bloat the
> > single SoC case.  this code seems to do just that.  so it sounds like you
> > should find a CONFIG name for this setup and start putting everything
> > behind that.
> 
> That's a good point indeed. The other question is, if gcc4.6's LTO won't
> fix that for us. Surely enough though, we can't rely on that.

i don't think LTO would help here
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111106/0681fbed/attachment.pgp 

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

* [U-Boot] [PATCH] ARM: Generic cache ops skeleton
  2011-11-06 19:29         ` Simon Glass
@ 2011-11-07  1:09           ` Mike Frysinger
  2011-11-07  3:10             ` Simon Glass
  2011-11-07 12:36           ` Albert ARIBAUD
  1 sibling, 1 reply; 11+ messages in thread
From: Mike Frysinger @ 2011-11-07  1:09 UTC (permalink / raw)
  To: u-boot

On Sunday 06 November 2011 14:29:47 Simon Glass wrote:
> On this particular patch, I feel it should be more explicit about L1
> cache, which is what I think it deals with. We may want to support L2
> also through a similar API. And a CONFIG option is a good idea.

the point of flushing caches is to make the memory coherent to other devices 
(like peripherals).  we don't differentiate between the cache levels.

> Finally, even the CP15/cache/MMU code is duplicated in different
> arch/arm/cpu subdirs. Can we unify this a bit?

things should be separated based on core and system levels.  the fact that a 
particular SoC is say armv4 doesn't mean it should have armv4 specific 
cache/mmu handling in its SoC subdir.  i think the Linux arm tree is properly 
separating things.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111106/5626c6b9/attachment.pgp 

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

* [U-Boot] [PATCH] ARM: Generic cache ops skeleton
  2011-11-07  1:09           ` Mike Frysinger
@ 2011-11-07  3:10             ` Simon Glass
  0 siblings, 0 replies; 11+ messages in thread
From: Simon Glass @ 2011-11-07  3:10 UTC (permalink / raw)
  To: u-boot

Hi Mike,

On Sun, Nov 6, 2011 at 5:09 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Sunday 06 November 2011 14:29:47 Simon Glass wrote:
>> On this particular patch, I feel it should be more explicit about L1
>> cache, which is what I think it deals with. We may want to support L2
>> also through a similar API. And a CONFIG option is a good idea.
>
> the point of flushing caches is to make the memory coherent to other devices
> (like peripherals). ?we don't differentiate between the cache levels.

Do we not have a situation where the L2 is coherent wrt on-chip
peripherals? I haven't looked into it.

>
>> Finally, even the CP15/cache/MMU code is duplicated in different
>> arch/arm/cpu subdirs. Can we unify this a bit?
>
> things should be separated based on core and system levels. ?the fact that a
> particular SoC is say armv4 doesn't mean it should have armv4 specific
> cache/mmu handling in its SoC subdir. ?i think the Linux arm tree is properly
> separating things.
> -mike
>

Actually I have this wrong - it was the CP15 stuff I was thinking of
(in start.S). For cache it is just weak functions that are overridden
in the ARMv7 case, and for MMU we only use ARMv4 features.

Going back to this patch, where will cpu_register_cache_ops() be called?

Regards,
Simon

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

* [U-Boot] [PATCH] ARM: Generic cache ops skeleton
  2011-11-06  1:16 [U-Boot] [PATCH] ARM: Generic cache ops skeleton Marek Vasut
  2011-11-06  8:36 ` Mike Frysinger
@ 2011-11-07 12:21 ` Albert ARIBAUD
  1 sibling, 0 replies; 11+ messages in thread
From: Albert ARIBAUD @ 2011-11-07 12:21 UTC (permalink / raw)
  To: u-boot

Le 06/11/2011 02:16, Marek Vasut a ?crit :
> This patch should allow a CPU to register it's own cache ops. This shall allow
> multiple CPUs with different cache handlings to be supported.

I don't really like the idea of a system of dynamic cache ops right now, 
because as discussed by Mike, we're far from a one-size-fits-all 
multiple-SoC binary, and in the current context of a single-SoC binary, 
this patch is overkill for cache management IMO.

OTOH, I am not against rationalizing the cache code, along the lines 
already discussed, i.e. factorizing cache code as much as possible (just 
as we should factorize start.S code for instance).

Amicalement,
-- 
Albert.

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

* [U-Boot] [PATCH] ARM: Generic cache ops skeleton
  2011-11-06 19:29         ` Simon Glass
  2011-11-07  1:09           ` Mike Frysinger
@ 2011-11-07 12:36           ` Albert ARIBAUD
  1 sibling, 0 replies; 11+ messages in thread
From: Albert ARIBAUD @ 2011-11-07 12:36 UTC (permalink / raw)
  To: u-boot

Le 06/11/2011 20:29, Simon Glass a ?crit :

> It's actually a lot more than cache ops. SOCs have there own clock
> stuff, pinmux things, power mgmt, timers and the like. While it might
> be desirable to provide this feature it is a fair bit of work and we
> need to be careful not to make things more complex.

Agreed.

> I would settle for an initial step of getting some sort of unified but
> simple clock/pinmux support in across all ARM. Anyone interested in
> that? I mean an API that covers the lot, not an implementation. Also
> it would be nice to avoid the heavy-weight data structures and strings
> that Linux has. If we got that far then we could make the interface
> virtual as Marek has done for caches.

On a general note, if we set our goal is to have in U-Boot what Linux 
has, U-Boot will end up being... Linux, which I don't think is a good 
thing unless we consider that U-Boot is redundant.

Besides, I don't think 'virtual' interfaces are necessarily the way to 
go either, not for the sole sake of it anyway. We might well get there 
with a driver model, and then we might want to see how things are done 
in Linux, but certainly not import the Linux model without thinking of 
what makes sense rather than what would be nice. U-Boot's basic 
principles are fundamentally different from those of an OS like Linux.

We can start, within the "single-SoC binary" model for now, by simply 
adding relevant SoC-specific or board-specific mechanisms, and using 
those already in place.

> On this particular patch, I feel it should be more explicit about L1
> cache, which is what I think it deals with. We may want to support L2
> also through a similar API. And a CONFIG option is a good idea.
>
> Finally, even the CP15/cache/MMU code is duplicated in different
> arch/arm/cpu subdirs. Can we unify this a bit?

I think our recent discussion opens the way for this unification in that 
cp15 code should be able to go away from start.S and into the (fewer) 
cache lib(s).

> Regards,
> Simon

Amicalement,
-- 
Albert.

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

end of thread, other threads:[~2011-11-07 12:36 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-11-06  1:16 [U-Boot] [PATCH] ARM: Generic cache ops skeleton Marek Vasut
2011-11-06  8:36 ` Mike Frysinger
2011-11-06 12:39   ` Marek Vasut
2011-11-06 18:03     ` Mike Frysinger
2011-11-06 18:07       ` Marek Vasut
2011-11-06 19:29         ` Simon Glass
2011-11-07  1:09           ` Mike Frysinger
2011-11-07  3:10             ` Simon Glass
2011-11-07 12:36           ` Albert ARIBAUD
2011-11-07  1:03         ` Mike Frysinger
2011-11-07 12:21 ` Albert ARIBAUD

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