public inbox for linux-mtd@lists.infradead.org
 help / color / mirror / Atom feed
* [RFC/PATCH] map drivers. DCACHE option for physmap and mphysmap drivers
@ 2006-01-26 17:24 Korolev, Alexey
  2006-01-26 17:48 ` Josh Boyer
  0 siblings, 1 reply; 9+ messages in thread
From: Korolev, Alexey @ 2006-01-26 17:24 UTC (permalink / raw)
  To: linux-mtd

Hi All,
 
Some mapping drivers of linux-mtd has feature of cached mapping. This
can improve read performance on FLASH significantly . 
I think adding the option of data cache mapping would also be helpful
for physmap and mphysmap drivers.
 
Here is a perf. data I measured on Mainstone with enabled and disabled
DCACHE mapping option:
 
DCAHCE is off:
time dd if=/dev/mtd2 of=/dev/null bs=4k count=4k
4096+0 records in
4096+0 records out
real    0m 7.21s
user    0m 0.00s
sys     0m 7.21s
 
DCACHE is on:
time dd if=/dev/mtd2 of=/dev/null bs=4k count=4k
4096+0 records in
4096+0 records out
real    0m 0.69s
user    0m 0.01s
sys     0m 0.69s
 
 
Please find diff file below. 
 
========================================================================
=====
diff -uNr a/drivers/mtd/maps/Kconfig b/drivers/mtd/maps/Kconfig
--- a/drivers/mtd/maps/Kconfig 2005-12-22 15:06:38.000000000 +0300
+++ b/drivers/mtd/maps/Kconfig 2005-12-22 15:05:24.000000000 +0300
@@ -60,6 +60,12 @@
    Ignore this option if you use run-time physmap configuration
    (i.e., run-time calling physmap_configure()).
 
+config MTD_PHYSMAP_DCACHE
+ bool "Enable dcache for flash mapping"
+ depends on MTD_PHYSMAP
+ help
+   This option enables CPU data cache for flash mapping
+   This improves flash read speed significantly. 
 config MTD_MULTI_PHYSMAP
  tristate "multiple CFI Flash devices in physical memory map"
  depends on MTD_CFI
@@ -86,6 +92,12 @@
  depends on MTD_MULTI_PHYSMAP
  default "2"
 
+config MTD_MULTI_PHYSMAP_1_DCACHE
+ bool "Enable dcache for 1st flash mapping"
+ depends on MTD_MULTI_PHYSMAP
+ help
+   This option enables CPU data cache for 1st flash mapping
+   This improves flash read speed significantly.
 config MTD_MULTI_PHYSMAP_2_NAME
  string "2nd mapping name"
  depends on MTD_MULTI_PHYSMAP
@@ -106,6 +118,12 @@
  depends on MTD_MULTI_PHYSMAP
  default "2"
 
+config MTD_MULTI_PHYSMAP_2_DCACHE
+ bool "Enable dcache for 2nd flash mapping"
+ depends on MTD_MULTI_PHYSMAP
+ help
+   This option enables CPU data cache for 2nd flash mapping
+   This improves flash read speed significantly.
 config MTD_MULTI_PHYSMAP_3_NAME
  string "3rd mapping name"
  depends on MTD_MULTI_PHYSMAP
@@ -126,6 +144,12 @@
  depends on MTD_MULTI_PHYSMAP
  default "2"
 
+config MTD_MULTI_PHYSMAP_3_DCACHE
+ bool "Enable dcache for 3rd flash mapping"
+ depends on MTD_MULTI_PHYSMAP
+ help
+   This option enables CPU data cache for 3rd flash mapping
+   This improves flash read speed significantly.
 config MTD_MULTI_PHYSMAP_4_NAME
  string "4th mapping name"
  depends on MTD_MULTI_PHYSMAP
@@ -146,6 +170,12 @@
  depends on MTD_MULTI_PHYSMAP
  default "2"
 
+config MTD_MULTI_PHYSMAP_4_DCACHE
+ bool "Enable dcache for 4th flash mapping"
+ depends on MTD_MULTI_PHYSMAP
+ help
+   This option enables CPU data cache for 4th flash mapping
+   This improves flash read speed significantly.
 config MTD_SUN_UFLASH
  tristate "Sun Microsystems userflash support"
  depends on (SPARC32 || SPARC64) && MTD_CFI
diff -uNr a/drivers/mtd/maps/mphysmap.c b/drivers/mtd/maps/mphysmap.c
--- a/drivers/mtd/maps/mphysmap.c 2005-12-22 15:06:38.000000000 +0300
+++ b/drivers/mtd/maps/mphysmap.c 2005-12-22 15:05:24.000000000 +0300
@@ -9,9 +9,40 @@
 #include <linux/module.h>
 #include <linux/mtd/map.h>
 #include <linux/mtd/mtd.h>
+#if defined(CONFIG_MTD_MULTI_PHYSMAP_1_DCACHE) || \
+    defined(CONFIG_MTD_MULTI_PHYSMAP_2_DCACHE) || \
+    defined(CONFIG_MTD_MULTI_PHYSMAP_3_DCACHE) || \
+    defined(CONFIG_MTD_MULTI_PHYSMAP_4_DCACHE)
+#define CONFIG_MTD_MULTI_PHYSMAP_DCACHE
+#endif    
 #ifdef CONFIG_MTD_PARTITIONS
 #include <linux/mtd/partitions.h>
 #endif
+#ifdef CONFIG_MTD_MULTI_PHYSMAP_DCACHE
+#include <asm/cacheflush.h>
+#include <linux/dma-mapping.h>
+#endif
+
+
+#ifdef CONFIG_MTD_MULTI_PHYSMAP_DCACHE
+static void mphysmap_inval_cache(struct map_info *map, unsigned long
from, ssize_t len)
+{unsigned long end;
+   /* flush_cache_all() works, but seriously degrades performance */
+   /* flush_cache_all(); */
+   /**/
+   /* round to full pages */
+   end=(from+len);
+   end=(end / PAGE_SIZE + 1) * PAGE_SIZE;
+   from=(from / PAGE_SIZE) * PAGE_SIZE;
+   len=end-from;
+#ifdef CONFIG_ARM        
+   consistent_sync((char *)map->cached + from, len, DMA_FROM_DEVICE);
+#else
+   /* Non ARM architectures has not been tested */
+   dma_cache_sync((char *)map->cached + from, len, DMA_FROM_DEVICE);
+#endif
+}
+#endif
 
 static struct map_info mphysmap_static_maps[] = {
 #if CONFIG_MTD_MULTI_PHYSMAP_1_WIDTH
@@ -20,6 +57,9 @@
   .phys  = CONFIG_MTD_MULTI_PHYSMAP_1_START,
   .size  = CONFIG_MTD_MULTI_PHYSMAP_1_LEN,
   .bankwidth = CONFIG_MTD_MULTI_PHYSMAP_1_WIDTH,
+#ifdef CONFIG_MTD_MULTI_PHYSMAP_1_DCACHE
+  .inval_cache = mphysmap_inval_cache,
+#endif
  },
 #endif
 #if CONFIG_MTD_MULTI_PHYSMAP_2_WIDTH
@@ -28,6 +68,9 @@
   .phys  = CONFIG_MTD_MULTI_PHYSMAP_2_START,
   .size  = CONFIG_MTD_MULTI_PHYSMAP_2_LEN,
   .bankwidth = CONFIG_MTD_MULTI_PHYSMAP_2_WIDTH,
+#ifdef CONFIG_MTD_MULTI_PHYSMAP_2_DCACHE
+  .inval_cache = mphysmap_inval_cache,
+#endif
  },
 #endif
 #if CONFIG_MTD_MULTI_PHYSMAP_3_WIDTH
@@ -36,6 +79,9 @@
   .phys  = CONFIG_MTD_MULTI_PHYSMAP_3_START,
   .size  = CONFIG_MTD_MULTI_PHYSMAP_3_LEN,
   .bankwidth = CONFIG_MTD_MULTI_PHYSMAP_3_WIDTH,
+#ifdef CONFIG_MTD_MULTI_PHYSMAP_3_DCACHE
+  .inval_cache = mphysmap_inval_cache,
+#endif
  },
 #endif
 #if CONFIG_MTD_MULTI_PHYSMAP_4_WIDTH
@@ -44,6 +90,9 @@
   .phys  = CONFIG_MTD_MULTI_PHYSMAP_4_START,
   .size  = CONFIG_MTD_MULTI_PHYSMAP_4_LEN,
   .bankwidth = CONFIG_MTD_MULTI_PHYSMAP_4_WIDTH,
+#ifdef CONFIG_MTD_MULTI_PHYSMAP_4_DCACHE
+  .inval_cache = mphysmap_inval_cache,
+#endif
  },
 #endif
 };
@@ -68,10 +117,20 @@
 #endif
        NULL};
 #endif
- map->virt = ioremap(map->phys, map->size);
+ map->virt = ioremap_nocache(map->phys, map->size);
  if (!map->virt)
   return -EIO;
-
+        map->cached=NULL;        
+#ifdef CONFIG_MTD_MULTI_PHYSMAP_DCACHE
+        if (map->inval_cache)
+        {
+#ifdef CONFIG_ARM        
+           map->cached = ioremap_cached(map->phys, map->size);
+#else
+           map->cached = __ioremap(map->phys, map->size,
L_PTE_CACHEABLE);
+#endif
+        };   
+#endif
  simple_map_init(map);
  mtd = NULL;
  type = rom_probe_types;
@@ -81,6 +140,10 @@
 
  if (!mtd) {
   iounmap(map->virt);
+#ifdef CONFIG_MTD_MULTI_PHYSMAP_DCACHE
+  if (map->cached)
+      iounmap(map->cached);
+#endif
   return -ENXIO;
  }
 
@@ -134,6 +197,12 @@
  map->map_priv_1 = 0;
  map->map_priv_2 = 0;
  map->virt = NULL;
+#ifdef CONFIG_MTD_MULTI_PHYSMAP_DCACHE
+ if (map->cached) {
+     iounmap(map->cached);
+     map->cached = NULL;
+ }
+#endif
 }
 
 
diff -uNr a/drivers/mtd/maps/physmap.c b/drivers/mtd/maps/physmap.c
--- a/drivers/mtd/maps/physmap.c 2005-12-22 15:06:38.000000000 +0300
+++ b/drivers/mtd/maps/physmap.c 2005-12-22 15:05:24.000000000 +0300
@@ -19,15 +19,41 @@
 #include <linux/mtd/map.h>
 #include <linux/config.h>
 #include <linux/mtd/partitions.h>
-#include <linux/mtd/physmap.h>
+#ifdef CONFIG_MTD_PHYSMAP_DCACHE
+#include <asm/cacheflush.h>
+#include <linux/dma-mapping.h>
+#endif
 
 static struct mtd_info *mymtd;
 
+#ifdef CONFIG_MTD_PHYSMAP_DCACHE
+static void physmap_inval_cache(struct map_info *map, unsigned long
from, ssize_t len)
+{unsigned long end;
+   /* flush_cache_all() works, but seriously degrades performance */
+   /* flush_cache_all(); */
+   /**/
+   /* round to full pages */
+   end=(from+len);
+   end=(end / PAGE_SIZE + 1) * PAGE_SIZE;
+   from=(from / PAGE_SIZE) * PAGE_SIZE;
+   len=end-from;
+#ifdef CONFIG_ARM        
+   consistent_sync((char *)map->cached + from, len, DMA_FROM_DEVICE);
+#else
+   /* Non ARM architectures are not tested */
+   dma_cache_sync((char *)map->cached + from, len, DMA_FROM_DEVICE);
+#endif
+}
+#endif
+
 struct map_info physmap_map = {
  .name = "phys_mapped_flash",
  .phys = CONFIG_MTD_PHYSMAP_START,
  .size = CONFIG_MTD_PHYSMAP_LEN,
  .bankwidth = CONFIG_MTD_PHYSMAP_BANKWIDTH,
+#ifdef CONFIG_MTD_PHYSMAP_DCACHE
+ .inval_cache = physmap_inval_cache,
+#endif
 };
 
 #ifdef CONFIG_MTD_PARTITIONS
@@ -52,13 +78,22 @@
  const char **type;
 
         printk(KERN_NOTICE "physmap flash device: %lx at %lx\n",
physmap_map.size, physmap_map.phys);
- physmap_map.virt = ioremap(physmap_map.phys, physmap_map.size);
+ physmap_map.virt = ioremap_nocache(physmap_map.phys,
physmap_map.size);
 
  if (!physmap_map.virt) {
   printk("Failed to ioremap\n");
   return -EIO;
  }
-
+#ifdef CONFIG_MTD_PHYSMAP_DCACHE
+#ifdef CONFIG_ARM
+ physmap_map.cached = ioremap_cached(physmap_map.phys,
physmap_map.size);
+#else        
+ physmap_map.cached = __ioremap(physmap_map.phys, physmap_map.size,
L_PTE_CACHEABLE);
+#endif        
+ if (!physmap_map.cached) {
+  printk("Failed to ioremap cached\n");
+ }
+#endif
  simple_map_init(&physmap_map);
 
  mymtd = NULL;
@@ -94,6 +129,10 @@
  }
 
  iounmap(physmap_map.virt);
+#ifdef CONFIG_MTD_PHYSMAP_DCACHE
+ if (physmap_map.cached)
+  iounmap(physmap_map.cached);
+#endif
  return -ENXIO;
 }
 
@@ -115,6 +154,12 @@
 
  iounmap(physmap_map.virt);
  physmap_map.virt = NULL;
+#ifdef CONFIG_MTD_PHYSMAP_DCACHE
+ if (physmap_map.cached) {
+  iounmap(physmap_map.cached);
+  physmap_map.cached = NULL;
+ }
+#endif
 }
 
 module_init(init_physmap);
=========================================================
 
Thanks,
Alexey

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

* Re: [RFC/PATCH] map drivers. DCACHE option for physmap and mphysmap drivers
  2006-01-26 17:24 [RFC/PATCH] map drivers. DCACHE option for physmap and mphysmap drivers Korolev, Alexey
@ 2006-01-26 17:48 ` Josh Boyer
  2006-01-27 15:31   ` Alexey, Korolev
  0 siblings, 1 reply; 9+ messages in thread
From: Josh Boyer @ 2006-01-26 17:48 UTC (permalink / raw)
  To: Korolev, Alexey; +Cc: linux-mtd

On 1/26/06, Korolev, Alexey <alexey.korolev@intel.com> wrote:
> Hi All,
>
> Some mapping drivers of linux-mtd has feature of cached mapping. This
> can improve read performance on FLASH significantly .
> I think adding the option of data cache mapping would also be helpful
> for physmap and mphysmap drivers.

Is this a read-only caching?  I know several architectures and
hardware configurations that would quickly throw a machine check if
trying to flush the cache back to flash since they don't support
bursted writes.


> ========================================================================
> =====
> diff -uNr a/drivers/mtd/maps/Kconfig b/drivers/mtd/maps/Kconfig
> --- a/drivers/mtd/maps/Kconfig 2005-12-22 15:06:38.000000000 +0300
> +++ b/drivers/mtd/maps/Kconfig 2005-12-22 15:05:24.000000000 +0300
> @@ -60,6 +60,12 @@
>     Ignore this option if you use run-time physmap configuration
>     (i.e., run-time calling physmap_configure()).
>
> +config MTD_PHYSMAP_DCACHE
> + bool "Enable dcache for flash mapping"
> + depends on MTD_PHYSMAP
> + help
> +   This option enables CPU data cache for flash mapping
> +   This improves flash read speed significantly.

I would recommend adding a dependency on ARM for all of this for now. 
As other architectures are proven to be working, they can be added.


> +#ifdef CONFIG_MTD_MULTI_PHYSMAP_DCACHE
> +        if (map->inval_cache)
> +        {
> +#ifdef CONFIG_ARM
> +           map->cached = ioremap_cached(map->phys, map->size);
> +#else
> +           map->cached = __ioremap(map->phys, map->size,
> L_PTE_CACHEABLE);
> +#endif

Gah.  The above doesn't work.  L_PTE_CACHEABLE seems to be an ARM only
flag.  And it wouldn't work on PPC for example.  That version of
__ioremap makes everything implicitly cached unless _PAGE_NO_CACHED is
passed in.

As I said before, I would make this an ARM only option for now.

> +#ifdef CONFIG_MTD_PHYSMAP_DCACHE
> +#ifdef CONFIG_ARM
> + physmap_map.cached = ioremap_cached(physmap_map.phys,
> physmap_map.size);
> +#else
> + physmap_map.cached = __ioremap(physmap_map.phys, physmap_map.size,
> L_PTE_CACHEABLE);
> +#endif

Same issues as in mphysmap.

josh

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

* Re: [RFC/PATCH] map drivers. DCACHE option for physmap and mphysmap drivers
  2006-01-26 17:48 ` Josh Boyer
@ 2006-01-27 15:31   ` Alexey, Korolev
  2006-01-27 19:15     ` Jared Hulbert
  0 siblings, 1 reply; 9+ messages in thread
From: Alexey, Korolev @ 2006-01-27 15:31 UTC (permalink / raw)
  To: Josh Boyer; +Cc: linux-mtd

Josh,

Thanks a lot for your feedback.


Josh Boyer wrote:

> On 1/26/06, Korolev, Alexey <alexey.korolev@intel.com> wrote:
> > Hi All,
> >
> > Some mapping drivers of linux-mtd has feature of cached mapping. This
> > can improve read performance on FLASH significantly .
> > I think adding the option of data cache mapping would also be helpful
> > for physmap and mphysmap drivers.
>
> Is this a read-only caching?  I know several architectures and
> hardware configurations that would quickly throw a machine check if
> trying to flush the cache back to flash since they don't support
> bursted writes.
>
In general case not.  I've never heard about the such issue. Is there 
any way to define has platform flash devices with bursted writes or not?
What architectures have this problem?

> I would recommend adding a dependency on ARM for all of this for now.
> As other architectures are proven to be working, they can be added.
>
Oh. It's a good point. I'll add ARM dependency to Kconfig file. Thanks.

> > +#ifdef CONFIG_MTD_MULTI_PHYSMAP_DCACHE
> > +        if (map->inval_cache)
> > +        {
> > +#ifdef CONFIG_ARM
> > +           map->cached = ioremap_cached(map->phys, map->size);
> > +#else
> > +           map->cached = __ioremap(map->phys, map->size,
> > L_PTE_CACHEABLE);
> > +#endif
>
> Gah.  The above doesn't work.  L_PTE_CACHEABLE seems to be an ARM only
> flag.  And it wouldn't work on PPC for example.  That version of
> __ioremap makes everything implicitly cached unless _PAGE_NO_CACHED is
> passed in.
>
> As I said before, I would make this an ARM only option for now.
>
> > +#ifdef CONFIG_MTD_PHYSMAP_DCACHE
> > +#ifdef CONFIG_ARM
> > + physmap_map.cached = ioremap_cached(physmap_map.phys,
> > physmap_map.size);
> > +#else
> > + physmap_map.cached = __ioremap(physmap_map.phys, physmap_map.size,
> > L_PTE_CACHEABLE);
> > +#endif
>
> Same issues as in mphysmap.
>
> josh
>
Thanks,
Alexey

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

* Re: [RFC/PATCH] map drivers. DCACHE option for physmap and mphysmap drivers
  2006-01-27 15:31   ` Alexey, Korolev
@ 2006-01-27 19:15     ` Jared Hulbert
  2006-01-27 19:51       ` Josh Boyer
  2006-01-27 20:39       ` Nicolas Pitre
  0 siblings, 2 replies; 9+ messages in thread
From: Jared Hulbert @ 2006-01-27 19:15 UTC (permalink / raw)
  To: Alexey, Korolev; +Cc: Josh Boyer, linux-mtd

> > Is this a read-only caching?  I know several architectures and
> > hardware configurations that would quickly throw a machine check if
> > trying to flush the cache back to flash since they don't support
> > bursted writes.
> >
> In general case not.  I've never heard about the such issue. Is there
> any way to define has platform flash devices with bursted writes or not?
> What architectures have this problem?

When I first did a patch like this I used a very platform dependent
embedded assembly loop to _invalidate_ NOT _flush_ the specific
cachelines possibly affected by the flash program or erase. 
consistent_sync() is a great arm specific option to do this but then
ioremap_cached() is very arm specific too.  Invalidating on the arm
arch marks a cacheline as invalid but doesn't try to write it back
while a flush typically refers to a writeback followed by an
invalidate.

 In this case the point is the flash was changed by a program or erase
that  does not change the state of possible contents of the cache so
the flash and cache are out of sync.  We don't care what the cache
contents that point to the affected regions of cache are, we don't
ever want to read them again, so we mark them as invalid and wait for
those lines to be replaced.

Flushing the caches back _could_ be a very bad thing, it could cause
flash to do many bad things because you would be write arbitrary data
back to the flash bus.  I'm not sure why it would machine check.  If
your processor was configured properly I'm not sure why flushing would
trigger burst writes to a memory not capable of bursting, but then
hardware can have its mysterious ways.  Either way you would likely do
bad things to the flash by flushing garbage back to it.  However, from
a higher level look at it you should never be writing directly to the
cached mapping of flash as only the mtd really knows about it and only
uses it read.  Therefore the cachelines are always clean and don't get
written back so it's not really a problem.

However I don't see the value of keeping the commented out lines with
the flushing calls around unless there was a more generic alternative
to ioremap_cached().

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

* Re: [RFC/PATCH] map drivers. DCACHE option for physmap and mphysmap drivers
  2006-01-27 19:15     ` Jared Hulbert
@ 2006-01-27 19:51       ` Josh Boyer
  2006-01-27 20:47         ` Nicolas Pitre
  2006-01-27 20:39       ` Nicolas Pitre
  1 sibling, 1 reply; 9+ messages in thread
From: Josh Boyer @ 2006-01-27 19:51 UTC (permalink / raw)
  To: Jared Hulbert; +Cc: Alexey, Korolev, linux-mtd

On 1/27/06, Jared Hulbert <jaredeh@gmail.com> wrote:
> > > Is this a read-only caching?  I know several architectures and
> > > hardware configurations that would quickly throw a machine check if
> > > trying to flush the cache back to flash since they don't support
> > > bursted writes.
> > >
> > In general case not.  I've never heard about the such issue. Is there
> > any way to define has platform flash devices with bursted writes or not?
> > What architectures have this problem?
>
> When I first did a patch like this I used a very platform dependent
> embedded assembly loop to _invalidate_ NOT _flush_ the specific
> cachelines possibly affected by the flash program or erase.

Yep, that would be better.

>  In this case the point is the flash was changed by a program or erase
> that  does not change the state of possible contents of the cache so
> the flash and cache are out of sync.  We don't care what the cache
> contents that point to the affected regions of cache are, we don't
> ever want to read them again, so we mark them as invalid and wait for
> those lines to be replaced.

Right.

>
> Flushing the caches back _could_ be a very bad thing, it could cause
> flash to do many bad things because you would be write arbitrary data
> back to the flash bus.  I'm not sure why it would machine check.  If
> your processor was configured properly I'm not sure why flushing would
> trigger burst writes to a memory not capable of bursting, but then
> hardware can have its mysterious ways.  Either way you would likely do

I didn't say it was sane hardware :).  But some ppc 4xx boards can't
handle bursted writes to the EBCs.  I'm not sure if they've all been
fixed out there or not.

> bad things to the flash by flushing garbage back to it.  However, from
> a higher level look at it you should never be writing directly to the
> cached mapping of flash as only the mtd really knows about it and only
> uses it read.  Therefore the cachelines are always clean and don't get
> written back so it's not really a problem.

I agree.  As long as things are getting invalidated instead of
flushed, then there should be no issue.  flushing == bad, invalidating
== sane :)

josh

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

* Re: [RFC/PATCH] map drivers. DCACHE option for physmap and mphysmap drivers
  2006-01-27 19:15     ` Jared Hulbert
  2006-01-27 19:51       ` Josh Boyer
@ 2006-01-27 20:39       ` Nicolas Pitre
  1 sibling, 0 replies; 9+ messages in thread
From: Nicolas Pitre @ 2006-01-27 20:39 UTC (permalink / raw)
  To: Jared Hulbert; +Cc: Alexey, Korolev, Josh Boyer, linux-mtd

On Fri, 27 Jan 2006, Jared Hulbert wrote:

> Flushing the caches back _could_ be a very bad thing, it could cause
> flash to do many bad things because you would be write arbitrary data
> back to the flash bus.  I'm not sure why it would machine check.  If
> your processor was configured properly I'm not sure why flushing would
> trigger burst writes to a memory not capable of bursting, but then
> hardware can have its mysterious ways.  Either way you would likely do
> bad things to the flash by flushing garbage back to it.  However, from
> a higher level look at it you should never be writing directly to the
> cached mapping of flash as only the mtd really knows about it and only
> uses it read.  Therefore the cachelines are always clean and don't get
> written back so it's not really a problem.

And this is _VERY_ fundamental.  That cached memory must _never_ be 
written to.  In fact, the memory protection could be set read-only as 
well for such mapping.  Hence it must never be written back.


Nicolas

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

* Re: [RFC/PATCH] map drivers. DCACHE option for physmap and mphysmap drivers
  2006-01-27 19:51       ` Josh Boyer
@ 2006-01-27 20:47         ` Nicolas Pitre
  2006-01-27 22:19           ` Josh Boyer
  0 siblings, 1 reply; 9+ messages in thread
From: Nicolas Pitre @ 2006-01-27 20:47 UTC (permalink / raw)
  To: Josh Boyer; +Cc: Alexey, Korolev, linux-mtd

On Fri, 27 Jan 2006, Josh Boyer wrote:

> On 1/27/06, Jared Hulbert <jaredeh@gmail.com> wrote:
> > When I first did a patch like this I used a very platform dependent
> > embedded assembly loop to _invalidate_ NOT _flush_ the specific
> > cachelines possibly affected by the flash program or erase.
> 
> Yep, that would be better.

No it wouldn't.  It would quickly become crufty and unreadable as more 
architectures add their stuff.

Also note that some architectures might have issues having two virtual 
areas mapped to the same physical area (like i386 if I remember 
correctly).  In that case the only way to have a cached mapping is to 
rely on the fact that the flash chip is likely to have aliases in the 
memory map and use one of them for the cached mapping.  Those tricks are 
really platform dependent and better expressed in a platform specific 
map driver.


Nicolas

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

* Re: [RFC/PATCH] map drivers. DCACHE option for physmap and mphysmap drivers
  2006-01-27 20:47         ` Nicolas Pitre
@ 2006-01-27 22:19           ` Josh Boyer
  2006-01-31 11:03             ` Alexey, Korolev
  0 siblings, 1 reply; 9+ messages in thread
From: Josh Boyer @ 2006-01-27 22:19 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Alexey, Korolev, linux-mtd

On 1/27/06, Nicolas Pitre <nico@cam.org> wrote:
> On Fri, 27 Jan 2006, Josh Boyer wrote:
>
> > On 1/27/06, Jared Hulbert <jaredeh@gmail.com> wrote:
> > > When I first did a patch like this I used a very platform dependent
> > > embedded assembly loop to _invalidate_ NOT _flush_ the specific
> > > cachelines possibly affected by the flash program or erase.
> >
> > Yep, that would be better.
>
> No it wouldn't.  It would quickly become crufty and unreadable as more
> architectures add their stuff.

/me sighs.

Yes, I misread what Jared had said.  I was solely focusing on
"_invalidate_ NOT _flush_" and missed the embedded assembly loop part.

I think you're probably right in that there is no clean way to do this
in a non-platform specific driver.  At least not a way that would be
worthwhile.

josh

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

* Re: [RFC/PATCH] map drivers. DCACHE option for physmap and mphysmap drivers
  2006-01-27 22:19           ` Josh Boyer
@ 2006-01-31 11:03             ` Alexey, Korolev
  0 siblings, 0 replies; 9+ messages in thread
From: Alexey, Korolev @ 2006-01-31 11:03 UTC (permalink / raw)
  To: Josh Boyer, Nicolas Pitre, Jared Hulbert; +Cc: linux-mtd, timofey.kutergin

Jared, Josh

I looked at the map.h code. There is the only one function which 
operates cached regions. It is map_copy_from function.
This function is used for read operations only for almost all flash 
devices.
I found the only device driver which uses map_copy_from for write. It is 
AMDSTD device driver.
Josh I can bet that the cache write issues you faced were on AMDSTD 
devices. All other devices use cached regions in read only mode.
AMDSTD is obsolete and depends on CONFIG_BROKEN.
So functionally we can guarantee read only mode for cache. We can avoid 
data Cache mappings for AMDSTD or fix driver or do nothing because it's 
obsolete.

Nicolas,

Non standardized cache invalidation and mapping is a big issue for the 
kernel. I guess it's possible to provide universal interface for cahce 
operating. I don't think that it is a reason why we have to decline 
cache mapping for flash devices. I think eventually cache interfaces 
will be stadartized. I think we can do this for the first time for the 
ARM platforms. Solution with physmap and mphysmap are very flexible. For 
example we have platforms where the flash type, number of flash devices, 
band width, and map size may vary. The only good solution for these 
cases are physmap and mphysmap drivers.

Thanks,
Alexey

> On 1/27/06, Nicolas Pitre <nico@cam.org> wrote:
> > On Fri, 27 Jan 2006, Josh Boyer wrote:
> >
> > > On 1/27/06, Jared Hulbert <jaredeh@gmail.com> wrote:
> > > > When I first did a patch like this I used a very platform dependent
> > > > embedded assembly loop to _invalidate_ NOT _flush_ the specific
> > > > cachelines possibly affected by the flash program or erase.
> > >
> > > Yep, that would be better.
> >
> > No it wouldn't.  It would quickly become crufty and unreadable as more
> > architectures add their stuff.
>
> /me sighs.
>
> Yes, I misread what Jared had said.  I was solely focusing on
> "_invalidate_ NOT _flush_" and missed the embedded assembly loop part.
>
> I think you're probably right in that there is no clean way to do this
> in a non-platform specific driver.  At least not a way that would be
> worthwhile.
>
> josh
>

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

end of thread, other threads:[~2006-01-31 11:05 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-01-26 17:24 [RFC/PATCH] map drivers. DCACHE option for physmap and mphysmap drivers Korolev, Alexey
2006-01-26 17:48 ` Josh Boyer
2006-01-27 15:31   ` Alexey, Korolev
2006-01-27 19:15     ` Jared Hulbert
2006-01-27 19:51       ` Josh Boyer
2006-01-27 20:47         ` Nicolas Pitre
2006-01-27 22:19           ` Josh Boyer
2006-01-31 11:03             ` Alexey, Korolev
2006-01-27 20:39       ` Nicolas Pitre

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