[patch 0/6] ps3vram driver patches

[patch 0/6] ps3vram driver patches

[Geoff Levand]

Hi Ben,

This is a set of patches for the ps3vram driver.

Patch 1 adds the driver.  It has already been reviewed on the cbe-oss-dev ML
and has been acked by David Woodhouse, so should be ready to merge.

Patch 2 is a minor update to reflect changes to the ps3-system-bus that have
been merged into mainline since this driver was posted, and so can be merged.

Patches 3-6 are minor cleanups suggested by Arnd Bergmann.  I have left two
of Arnd's sugestions, implementing the driver as a block device, similar to
the axonram driver, and eliminating the need to ioremap.  These will require
a significant re-work.


 [patch 1/6] powerpc/ps3: ps3vram driver for accessing video RAM as MTD
 [patch 2/6] powerpc/ps3: Add modalias support to the ps3vram driver
 [patch 3/6] mtd/ps3vram: Remove ps3vram debug routines
 [patch 4/6] mtd/ps3vram: Cleanup ps3vram driver messages
 [patch 5/6] mtd/ps3vram: Use kernel types
 [patch 6/6] mtd/ps3vram: Use msleep in waits

-Geoff


-- 

Re: [patch 0/6] ps3vram driver patches

[Arnd Bergmann]

On Tuesday 06 January 2009, Geoff Levand wrote:
> Patches 3-6 are minor cleanups suggested by Arnd Bergmann. =A0I have left=
 two
> of Arnd's sugestions, implementing the driver as a block device, similar =
to
> the axonram driver, and eliminating the need to ioremap. =A0These will re=
quire
> a significant re-work.

My complaint about the ioremap was just about the type of the mapping,
not something fundamental. Please just replace ioremap(addr, size) with
ioremap_flags(addr, size, _PAGE_NO_CACHE) in order to get a mapping without
the guarded bit.

If everyone else thinks that doing the ps3vram driver as an MTD rather than
a block device is acceptable, I have no further objections. Thanks for
following up on my other comments.

Acked-by: Arnd Bergmann <arnd@arndb.de>

	Arnd <><

Re: [patch 0/6] ps3vram driver patches

[Geoff Levand]

Arnd Bergmann wrote:
> On Tuesday 06 January 2009, Geoff Levand wrote:
>> Patches 3-6 are minor cleanups suggested by Arnd Bergmann.  I have left two
>> of Arnd's sugestions, implementing the driver as a block device, similar to
>> the axonram driver, and eliminating the need to ioremap.  These will require
>> a significant re-work.
> 
> My complaint about the ioremap was just about the type of the mapping,
> not something fundamental. Please just replace ioremap(addr, size) with
> ioremap_flags(addr, size, _PAGE_NO_CACHE) in order to get a mapping without
> the guarded bit.

OK, I see.  I'll post a patch.

Geert and I were discussing actually removing the direct write to the XDR
memory, and so the need for that ioremap, as the mapping is just used in
ps3vram_erase(), which seems could be removed.

> If everyone else thinks that doing the ps3vram driver as an MTD rather than
> a block device is acceptable, I have no further objections. Thanks for
> following up on my other comments.

I want to get it converted to a block device, and I will work towards
that, but it will take some time.  As it is, it is very useful for typical
systems that are running full desktops like gmome or KDE and do a lot of
swapping.  Many of the distros now use it, and users want it.

-Geoff

Re: [patch 0/6] ps3vram driver patches

[Arnd Bergmann]

On Wednesday 07 January 2009, Geoff Levand wrote:

> Geert and I were discussing actually removing the direct write to the XDR
> memory, and so the need for that ioremap, as the mapping is just used in
> ps3vram_erase(), which seems could be removed.

Ah, I see. I also forgot to mention that the ioremap_flags change should
only be done for actual memory regions, but *not* for memory mapped I/O
registers.

> I want to get it converted to a block device, and I will work towards
> that, but it will take some time. =A0As it is, it is very useful for typi=
cal
> systems that are running full desktops like gmome or KDE and do a lot of
> swapping. =A0Many of the distros now use it, and users want it.

Yes, fine by me.

	Arnd <><

Re: [patch 0/6] ps3vram driver patches

[Geert Uytterhoeven]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: TEXT/PLAIN; charset=UTF-8, Size: 1577 bytes --]

On Wed, 7 Jan 2009, Arnd Bergmann wrote:
> On Wednesday 07 January 2009, Geoff Levand wrote:
> > Geert and I were discussing actually removing the direct write to the XDR
> > memory, and so the need for that ioremap, as the mapping is just used in
> > ps3vram_erase(), which seems could be removed.

ps3vram_erase() writes all ones to the memory, as that's what done when
erasing real FLASH. I do not know if the MTD layer can handle it if we don't
"erase" it to ones.

David?

> Ah, I see. I also forgot to mention that the ioremap_flags change should
> only be done for actual memory regions, but *not* for memory mapped I/O
> registers.

This is DDR memory for the RSX.

> > I want to get it converted to a block device, and I will work towards
> > that, but it will take some time.  As it is, it is very useful for typical
> > systems that are running full desktops like gmome or KDE and do a lot of
> > swapping.  Many of the distros now use it, and users want it.
> 
> Yes, fine by me.

One disadvantage of this approach is that it makes life more difficult for
distro maintainers: their userland has to support both the MTD and the block
version.

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis · BIC GEBABEBB · IBAN BE41293037680010

[patch] mtd/ps3vram: Use _PAGE_NO_CACHE in memory ioremap

[Geoff Levand]

Use _PAGE_NO_CACHE for gpu memory ioremap.  Also,
add __iomem attribute to gpu memory pointer and
change use of memset() to memset_io().

Signed-off-by: Geoff Levand <geoffrey.levand@am.sony.com>
---
 drivers/mtd/devices/ps3vram.c |   14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

--- a/drivers/mtd/devices/ps3vram.c
+++ b/drivers/mtd/devices/ps3vram.c
@@ -6,6 +6,7 @@
  */
 
 #include <linux/io.h>
+#include <linux/mm.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/list.h>
@@ -69,7 +70,7 @@ struct ps3vram_priv {
 	u64 context_handle;
 	u32 *ctrl;
 	u32 *reports;
-	u8 *base;
+	u8 __iomem *ddr_base;
 	u8 *xdr_buf;
 
 	u32 *fifo_base;
@@ -425,7 +426,7 @@ static int ps3vram_erase(struct mtd_info
 	ps3vram_cache_flush(mtd);
 
 	/* Set bytes to 0xFF */
-	memset(priv->base + instr->addr, 0xFF, instr->len);
+	memset_io(priv->ddr_base + instr->addr, 0xFF, instr->len);
 
 	mutex_unlock(&priv->lock);
 
@@ -628,8 +629,9 @@ static int __devinit ps3vram_probe(struc
 		goto out_free_context;
 	}
 
-	priv->base = ioremap(ddr_lpar, ddr_size);
-	if (!priv->base) {
+	priv->ddr_base = ioremap_flags(ddr_lpar, ddr_size, _PAGE_NO_CACHE);
+
+	if (!priv->ddr_base) {
 		dev_err(&dev->core, "%s:%d: ioremap failed\n", __func__,
 			__LINE__);
 		ret = -ENOMEM;
@@ -702,7 +704,7 @@ out_unmap_reports:
 out_unmap_ctrl:
 	iounmap(priv->ctrl);
 out_unmap_vram:
-	iounmap(priv->base);
+	iounmap(priv->ddr_base);
 out_free_context:
 	lv1_gpu_context_free(priv->context_handle);
 out_free_memory:
@@ -728,7 +730,7 @@ static int ps3vram_shutdown(struct ps3_s
 	ps3vram_cache_cleanup(&ps3vram_mtd);
 	iounmap(priv->reports);
 	iounmap(priv->ctrl);
-	iounmap(priv->base);
+	iounmap(priv->ddr_base);
 	lv1_gpu_context_free(priv->context_handle);
 	lv1_gpu_memory_free(priv->memory_handle);
 	ps3_close_hv_device(dev);