[patch 0/6] PS3 Storage Drivers for 2.6.23, take 2

[patch 0/6] PS3 Storage Drivers for 2.6.23, take 2

[Geert Uytterhoeven]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1290 bytes --]

	Hi,

This is the second submission of the new PS3 storage drivers:
  [1] ps3: Preallocate bootmem memory for the PS3 FLASH ROM storage driver
  [2] ps3: Storage Driver Core
  [3] ps3: Storage device registration routines.
  [4] ps3: Disk Storage Driver
  [5] ps3: ROM Storage Driver
  [6] ps3: FLASH ROM Storage Driver

They are based on the current Linux kernel source tree, plus the PS3 patches
already submitted by Geoff Levand on linuxppc-dev.

All issues raised during the previous review round should be fixed.

If there are no more comments, please apply for 2.6.23.  Since they depend on
other PS3 work, I think it's most logical to let Paul integrate them.

Thanks in advance!

With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center 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/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

[patch 1/6] ps3: Preallocate bootmem memory for the PS3 FLASH ROM storage driver

[Geert Uytterhoeven]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2772 bytes --]

Preallocate 256 KiB of bootmem memory for the PS3 FLASH ROM storage driver.

Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
Signed-off-by: Geoff Levand <geoffrey.levand@am.sony.com>
---
 arch/powerpc/platforms/ps3/setup.c |   19 ++++++++++++++++++-
 include/asm-powerpc/ps3.h          |    1 +
 2 files changed, 19 insertions(+), 1 deletion(-)

--- a/arch/powerpc/platforms/ps3/setup.c
+++ b/arch/powerpc/platforms/ps3/setup.c
@@ -107,7 +107,8 @@ static void ps3_panic(char *str)
 	while(1);
 }
 
-#if defined(CONFIG_FB_PS3) || defined(CONFIG_FB_PS3_MODULE)
+#if defined(CONFIG_FB_PS3) || defined(CONFIG_FB_PS3_MODULE) || \
+    defined(CONFIG_PS3_FLASH) || defined(CONFIG_PS3_FLASH_MODULE)
 static void prealloc(struct ps3_prealloc *p)
 {
 	if (!p->size)
@@ -123,7 +124,9 @@ static void prealloc(struct ps3_prealloc
 	printk(KERN_INFO "%s: %lu bytes at %p\n", p->name, p->size,
 	       p->address);
 }
+#endif
 
+#if defined(CONFIG_FB_PS3) || defined(CONFIG_FB_PS3_MODULE)
 struct ps3_prealloc ps3fb_videomemory = {
 	.name = "ps3fb videomemory",
 	.size = CONFIG_FB_PS3_DEFAULT_SIZE_M*1024*1024,
@@ -146,6 +149,18 @@ early_param("ps3fb", early_parse_ps3fb);
 #define prealloc_ps3fb_videomemory()	do { } while (0)
 #endif
 
+#if defined(CONFIG_PS3_FLASH) || defined(CONFIG_PS3_FLASH_MODULE)
+struct ps3_prealloc ps3flash_bounce_buffer = {
+	.name = "ps3flash bounce buffer",
+	.size = 256*1024,
+	.align = 256*1024
+};
+EXPORT_SYMBOL_GPL(ps3flash_bounce_buffer);
+#define prealloc_ps3flash_bounce_buffer()	prealloc(&ps3flash_bounce_buffer)
+#else
+#define prealloc_ps3flash_bounce_buffer()	do { } while (0)
+#endif
+
 static int ps3_set_dabr(u64 dabr)
 {
 	enum {DABR_USER = 1, DABR_KERNEL = 2,};
@@ -175,6 +190,8 @@ static void __init ps3_setup_arch(void)
 #endif
 
 	prealloc_ps3fb_videomemory();
+	prealloc_ps3flash_bounce_buffer();
+
 	ppc_md.power_save = ps3_power_save;
 
 	DBG(" <- %s:%d\n", __func__, __LINE__);
--- a/include/asm-powerpc/ps3.h
+++ b/include/asm-powerpc/ps3.h
@@ -427,6 +427,7 @@ struct ps3_prealloc {
 };
 
 extern struct ps3_prealloc ps3fb_videomemory;
+extern struct ps3_prealloc ps3flash_bounce_buffer;
 
 
 #endif

-- 
With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center 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/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

Re: [patch 1/6] ps3: Preallocate bootmem memory for the PS3 FLASH ROM storage driver

[Benjamin Herrenschmidt]

On Fri, 2007-06-15 at 13:39 +0200, Geert Uytterhoeven wrote:
> plain text document attachment (ps3-stable)
> Preallocate 256 KiB of bootmem memory for the PS3 FLASH ROM storage driver.

I still very much dislike the #ifdef xxx_MODULE in main kernel code.

At the end of the day, is it realistic to ever use a PS3 without the
storage driver ? I would suggest just allocating those unconditionally.

Ben.

> Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
> Signed-off-by: Geoff Levand <geoffrey.levand@am.sony.com>
> ---
>  arch/powerpc/platforms/ps3/setup.c |   19 ++++++++++++++++++-
>  include/asm-powerpc/ps3.h          |    1 +
>  2 files changed, 19 insertions(+), 1 deletion(-)
> 
> --- a/arch/powerpc/platforms/ps3/setup.c
> +++ b/arch/powerpc/platforms/ps3/setup.c
> @@ -107,7 +107,8 @@ static void ps3_panic(char *str)
>  	while(1);
>  }
>  
> -#if defined(CONFIG_FB_PS3) || defined(CONFIG_FB_PS3_MODULE)
> +#if defined(CONFIG_FB_PS3) || defined(CONFIG_FB_PS3_MODULE) || \
> +    defined(CONFIG_PS3_FLASH) || defined(CONFIG_PS3_FLASH_MODULE)
>  static void prealloc(struct ps3_prealloc *p)
>  {
>  	if (!p->size)
> @@ -123,7 +124,9 @@ static void prealloc(struct ps3_prealloc
>  	printk(KERN_INFO "%s: %lu bytes at %p\n", p->name, p->size,
>  	       p->address);
>  }
> +#endif
>  
> +#if defined(CONFIG_FB_PS3) || defined(CONFIG_FB_PS3_MODULE)
>  struct ps3_prealloc ps3fb_videomemory = {
>  	.name = "ps3fb videomemory",
>  	.size = CONFIG_FB_PS3_DEFAULT_SIZE_M*1024*1024,
> @@ -146,6 +149,18 @@ early_param("ps3fb", early_parse_ps3fb);
>  #define prealloc_ps3fb_videomemory()	do { } while (0)
>  #endif
>  
> +#if defined(CONFIG_PS3_FLASH) || defined(CONFIG_PS3_FLASH_MODULE)
> +struct ps3_prealloc ps3flash_bounce_buffer = {
> +	.name = "ps3flash bounce buffer",
> +	.size = 256*1024,
> +	.align = 256*1024
> +};
> +EXPORT_SYMBOL_GPL(ps3flash_bounce_buffer);
> +#define prealloc_ps3flash_bounce_buffer()	prealloc(&ps3flash_bounce_buffer)
> +#else
> +#define prealloc_ps3flash_bounce_buffer()	do { } while (0)
> +#endif
> +
>  static int ps3_set_dabr(u64 dabr)
>  {
>  	enum {DABR_USER = 1, DABR_KERNEL = 2,};
> @@ -175,6 +190,8 @@ static void __init ps3_setup_arch(void)
>  #endif
>  
>  	prealloc_ps3fb_videomemory();
> +	prealloc_ps3flash_bounce_buffer();
> +
>  	ppc_md.power_save = ps3_power_save;
>  
>  	DBG(" <- %s:%d\n", __func__, __LINE__);
> --- a/include/asm-powerpc/ps3.h
> +++ b/include/asm-powerpc/ps3.h
> @@ -427,6 +427,7 @@ struct ps3_prealloc {
>  };
>  
>  extern struct ps3_prealloc ps3fb_videomemory;
> +extern struct ps3_prealloc ps3flash_bounce_buffer;
>  
> 
>  #endif
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [patch 1/6] ps3: Preallocate bootmem memory for the PS3 FLASH ROM storage driver

[Geert Uytterhoeven]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1312 bytes --]

On Fri, 15 Jun 2007, Benjamin Herrenschmidt wrote:
> On Fri, 2007-06-15 at 13:39 +0200, Geert Uytterhoeven wrote:
> > plain text document attachment (ps3-stable)
> > Preallocate 256 KiB of bootmem memory for the PS3 FLASH ROM storage driver.
> 
> I still very much dislike the #ifdef xxx_MODULE in main kernel code.

Me too, but there's no other reliable way to allocate it after boot up...

> At the end of the day, is it realistic to ever use a PS3 without the
> storage driver ? I would suggest just allocating those unconditionally.

If you don't plan to (a) boot into GameOS or (b) update the kboot kernel, then
the answer is: yes, you can live without the FLASH ROM driver, and save one
promille of RAM.

With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center 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/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

Re: [patch 1/6] ps3: Preallocate bootmem memory for the PS3 FLASH

[Milton Miller]

On Fri Jun 15 23:32:41 EST 2007, Geert Uytterhoeven wrote:
> On Fri, 15 Jun 2007, Benjamin Herrenschmidt wrote:
>> On Fri, 2007-06-15 at 13:39 +0200, Geert Uytterhoeven wrote:
>>> Preallocate 256 KiB of bootmem memory for the PS3 FLASH ROM storage 
>>> driver.
>>
>> I still very much dislike the #ifdef xxx_MODULE in main kernel code.
>
> Me too, but there's no other reliable way to allocate it after boot 
> up...
>
>> At the end of the day, is it realistic to ever use a PS3 without the
>> storage driver ? I would suggest just allocating those 
>> unconditionally.
>
> If you don't plan to (a) boot into GameOS or (b) update the kboot 
> kernel, then the answer is: yes, you can live without the FLASH ROM 
> driver, and save one promille of RAM.
>

In that case, why not make it an early_param, and only change the 
default based on the config varables?   It sounds like I might want to 
reclaim this memory even if I compiled in the driver.

Statement (a) is because one has to write the flag to say don't' boot 
OtherOS to go back to GameOS (or reset all defaults) I presume.

milton

Re: [patch 1/6] ps3: Preallocate bootmem memory for the PS3 FLASH

[Geert Uytterhoeven]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1946 bytes --]

On Mon, 18 Jun 2007, Milton Miller wrote:
> On Fri Jun 15 23:32:41 EST 2007, Geert Uytterhoeven wrote:
> > On Fri, 15 Jun 2007, Benjamin Herrenschmidt wrote:
> > > On Fri, 2007-06-15 at 13:39 +0200, Geert Uytterhoeven wrote:
> > > > Preallocate 256 KiB of bootmem memory for the PS3 FLASH ROM storage
> > > > driver.
> > > 
> > > I still very much dislike the #ifdef xxx_MODULE in main kernel code.
> > 
> > Me too, but there's no other reliable way to allocate it after boot up...
> > 
> > > At the end of the day, is it realistic to ever use a PS3 without the
> > > storage driver ? I would suggest just allocating those unconditionally.
> > 
> > If you don't plan to (a) boot into GameOS or (b) update the kboot kernel,
> > then the answer is: yes, you can live without the FLASH ROM driver, and save
> > one promille of RAM.
> 
> In that case, why not make it an early_param, and only change the default
> based on the config varables?   It sounds like I might want to reclaim this
> memory even if I compiled in the driver.

Like `ps3flash=off' on the kernel command line to disable it? Can be done.

> Statement (a) is because one has to write the flag to say don't' boot OtherOS
> to go back to GameOS (or reset all defaults) I presume.

Indeed, there's a flag in the FLASH ROM to choose between booting GameOS and
OtherOS (e.g. Linux).

With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center 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/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

[patch 2/6] ps3: Storage Driver Core

[Geert Uytterhoeven]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 13406 bytes --]

From: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>

Add storage driver core support for the PS3.
PS3 storage devices are a special kind of PS3 system bus devices

Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
Signed-off-by: Geoff Levand <geoffrey.levand@am.sony.com>
---
Changes since previous submission:
  - Remove the `name' parameters of ps3stor_setup(), as it can be obtained from
    the driver structure
  - Cleanup struct ps3_storage_device

 arch/powerpc/platforms/ps3/Kconfig |    4 
 drivers/ps3/Makefile               |    1 
 drivers/ps3/ps3stor_lib.c          |  326 +++++++++++++++++++++++++++++++++++++
 include/asm-powerpc/ps3stor.h      |   70 +++++++
 4 files changed, 401 insertions(+)

--- a/arch/powerpc/platforms/ps3/Kconfig
+++ b/arch/powerpc/platforms/ps3/Kconfig
@@ -98,4 +98,8 @@ config PS3_SYS_MANAGER
 	  This support is required for system control.  In
 	  general, all users will say Y or M.
 
+config PS3_STORAGE
+	depends on PPC_PS3
+	tristate
+
 endmenu
--- a/drivers/ps3/Makefile
+++ b/drivers/ps3/Makefile
@@ -3,3 +3,4 @@ obj-$(CONFIG_PS3_PS3AV) += ps3av_mod.o
 ps3av_mod-objs		+= ps3av.o ps3av_cmd.o
 obj-$(CONFIG_PPC_PS3) += sys-manager-core.o
 obj-$(CONFIG_PS3_SYS_MANAGER) += sys-manager.o
+obj-$(CONFIG_PS3_STORAGE) += ps3stor_lib.o
--- /dev/null
+++ b/drivers/ps3/ps3stor_lib.c
@@ -0,0 +1,326 @@
+/*
+ * PS3 Storage Library
+ *
+ * Copyright (C) 2007 Sony Computer Entertainment Inc.
+ * Copyright 2007 Sony Corp.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published
+ * by the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include <linux/dma-mapping.h>
+#include <linux/interrupt.h>
+
+#include <asm/lv1call.h>
+#include <asm/ps3stor.h>
+
+
+static irqreturn_t ps3stor_interrupt(int irq, void *data)
+{
+	struct ps3_storage_device *dev = data;
+	int res;
+	u64 tag, status;
+
+	res = lv1_storage_get_async_status(dev->sbd.dev_id, &tag, &status);
+
+	if (tag != dev->tag)
+		dev_err(&dev->sbd.core,
+			"%s:%u: tag mismatch, got %lx, expected %lx\n",
+			__func__, __LINE__, tag, dev->tag);
+
+	if (res) {
+		dev_err(&dev->sbd.core, "%s:%u: res=%d status=0x%lx\n",
+			__func__, __LINE__, res, status);
+	} else {
+		dev->lv1_status = status;
+		complete(&dev->done);
+	}
+	return IRQ_HANDLED;
+}
+
+
+static int ps3stor_probe_access(struct ps3_storage_device *dev)
+{
+	int res, error;
+	unsigned int i;
+	unsigned long n;
+
+	if (dev->sbd.match_id == PS3_MATCH_ID_STOR_ROM) {
+		/* special case: CD-ROM is assumed always accessible */
+		dev->accessible_regions = 1;
+		return 0;
+	}
+
+	error = -EPERM;
+	for (i = 0; i < dev->num_regions; i++) {
+		dev_dbg(&dev->sbd.core,
+			"%s:%u: checking accessibility of region %u\n",
+			__func__, __LINE__, i);
+
+		dev->region_idx = i;
+		res = ps3stor_read_write_sectors(dev, dev->bounce_lpar, 0, 1,
+						 0);
+		if (res) {
+			dev_dbg(&dev->sbd.core, "%s:%u: read failed, "
+				"region %u is not accessible\n", __func__,
+				__LINE__, i);
+			continue;
+		}
+
+		dev_dbg(&dev->sbd.core, "%s:%u: region %u is accessible\n",
+			__func__, __LINE__, i);
+		set_bit(i, &dev->accessible_regions);
+
+		/* We can access at least one region */
+		error = 0;
+	}
+	if (error)
+		return error;
+
+	n = hweight_long(dev->accessible_regions);
+	if (n > 1)
+		dev_info(&dev->sbd.core,
+			 "%s:%u: %lu accessible regions found. Only the first "
+			 "one will be used",
+			 __func__, __LINE__, n);
+	dev->region_idx = __ffs(dev->accessible_regions);
+	dev_info(&dev->sbd.core,
+		 "First accessible region has index %u start %lu size %lu\n",
+		 dev->region_idx, dev->regions[dev->region_idx].start,
+		 dev->regions[dev->region_idx].size);
+
+	return 0;
+}
+
+
+/**
+ *	ps3stor_setup - Setup a storage device before use
+ *	@dev: Pointer to a struct ps3_storage_device
+ *
+ *	Returns 0 for success, or an error code
+ */
+int ps3stor_setup(struct ps3_storage_device *dev)
+{
+	int error, res, alignment;
+	enum ps3_dma_page_size page_size;
+
+	error = ps3_open_hv_device(&dev->sbd);
+	if (error) {
+		dev_err(&dev->sbd.core,
+			"%s:%u: ps3_open_hv_device failed %d\n", __func__,
+			__LINE__, error);
+		goto fail;
+	}
+
+	error = ps3_sb_event_receive_port_setup(&dev->sbd, PS3_BINDING_CPU_ANY,
+						&dev->irq);
+	if (error) {
+		dev_err(&dev->sbd.core,
+			"%s:%u: ps3_sb_event_receive_port_setup failed %d\n",
+		       __func__, __LINE__, error);
+		goto fail_close_device;
+	}
+
+	error = request_irq(dev->irq, ps3stor_interrupt, IRQF_DISABLED,
+			    dev->sbd.core.driver->name, dev);
+	if (error) {
+		dev_err(&dev->sbd.core, "%s:%u: request_irq failed %d\n",
+			__func__, __LINE__, error);
+		goto fail_sb_event_receive_port_destroy;
+	}
+
+	alignment = min(__ffs(dev->bounce_size),
+			__ffs((unsigned long)dev->bounce_buf));
+	if (alignment < 12) {
+		dev_err(&dev->sbd.core,
+			"%s:%u: bounce buffer not aligned (%lx at 0x%p)\n",
+			__func__, __LINE__, dev->bounce_size, dev->bounce_buf);
+		error = -EINVAL;
+		goto fail_free_irq;
+	} else if (alignment < 16)
+		page_size = PS3_DMA_4K;
+	else
+		page_size = PS3_DMA_64K;
+	dev->sbd.d_region = &dev->dma_region;
+	ps3_dma_region_init(&dev->sbd, &dev->dma_region, page_size,
+			    PS3_DMA_OTHER, dev->bounce_buf, dev->bounce_size);
+	res = ps3_dma_region_create(&dev->dma_region);
+	if (res) {
+		dev_err(&dev->sbd.core, "%s:%u: cannot create DMA region\n",
+			__func__, __LINE__);
+		error = -ENOMEM;
+		goto fail_free_irq;
+	}
+
+	dev->bounce_lpar = ps3_mm_phys_to_lpar(__pa(dev->bounce_buf));
+	dev->bounce_dma = dma_map_single(&dev->sbd.core, dev->bounce_buf,
+					 dev->bounce_size, DMA_BIDIRECTIONAL);
+	if (!dev->bounce_dma) {
+		dev_err(&dev->sbd.core, "%s:%u: map DMA region failed\n",
+			__func__, __LINE__);
+		error = -ENODEV;
+		goto fail_free_dma;
+	}
+
+	error = ps3stor_probe_access(dev);
+	if (error) {
+		dev_err(&dev->sbd.core, "%s:%u: No accessible regions found\n",
+			__func__, __LINE__);
+		goto fail_unmap_dma;
+	}
+	return 0;
+
+fail_unmap_dma:
+	dma_unmap_single(&dev->sbd.core, dev->bounce_dma, dev->bounce_size,
+			 DMA_BIDIRECTIONAL);
+fail_free_dma:
+	ps3_dma_region_free(&dev->dma_region);
+fail_free_irq:
+	free_irq(dev->irq, dev);
+fail_sb_event_receive_port_destroy:
+	ps3_sb_event_receive_port_destroy(&dev->sbd, dev->irq);
+fail_close_device:
+	ps3_close_hv_device(&dev->sbd);
+fail:
+	return error;
+}
+EXPORT_SYMBOL_GPL(ps3stor_setup);
+
+
+/**
+ *	ps3stor_teardown - Tear down a storage device after use
+ *	@dev: Pointer to a struct ps3_storage_device
+ */
+void ps3stor_teardown(struct ps3_storage_device *dev)
+{
+	int error;
+
+	dma_unmap_single(&dev->sbd.core, dev->bounce_dma, dev->bounce_size,
+			 DMA_BIDIRECTIONAL);
+	ps3_dma_region_free(&dev->dma_region);
+
+	free_irq(dev->irq, dev);
+
+	error = ps3_sb_event_receive_port_destroy(&dev->sbd, dev->irq);
+	if (error)
+		dev_err(&dev->sbd.core,
+			"%s:%u: destroy event receive port failed %d\n",
+			__func__, __LINE__, error);
+
+	error = ps3_close_hv_device(&dev->sbd);
+	if (error)
+		dev_err(&dev->sbd.core,
+			"%s:%u: ps3_close_hv_device failed %d\n", __func__,
+			__LINE__, error);
+}
+EXPORT_SYMBOL_GPL(ps3stor_teardown);
+
+
+/**
+ *	ps3stor_read_write_sectors - read/write from/to a storage device
+ *	@dev: Pointer to a struct ps3_storage_device
+ *	@lpar: HV logical partition address
+ *	@start_sector: First sector to read/write
+ *	@sectors: Number of sectors to read/write
+ *	@write: Flag indicating write (non-zero) or read (zero)
+ *
+ *	Returns 0 for success, -1 in case of failure to submit the command, or
+ *	an LV1 status value in case of other errors
+ */
+u64 ps3stor_read_write_sectors(struct ps3_storage_device *dev, u64 lpar,
+			       u64 start_sector, u64 sectors, int write)
+{
+	unsigned int region_id = dev->regions[dev->region_idx].id;
+	const char *op = write ? "write" : "read";
+	int res;
+
+	dev_dbg(&dev->sbd.core, "%s:%u: %s %lu sectors starting at %lu\n",
+		__func__, __LINE__, op, sectors, start_sector);
+
+	init_completion(&dev->done);
+	res = write ? lv1_storage_write(dev->sbd.dev_id, region_id,
+					start_sector, sectors, 0, lpar,
+					&dev->tag)
+		    : lv1_storage_read(dev->sbd.dev_id, region_id,
+				       start_sector, sectors, 0, lpar,
+				       &dev->tag);
+	if (res) {
+		dev_dbg(&dev->sbd.core, "%s:%u: %s failed %d\n", __func__,
+			__LINE__, op, res);
+		return -1;
+	}
+
+	wait_for_completion(&dev->done);
+	if (dev->lv1_status) {
+		dev_dbg(&dev->sbd.core, "%s:%u: %s failed 0x%lx\n", __func__,
+			__LINE__, op, dev->lv1_status);
+		return dev->lv1_status;
+	}
+
+	dev_dbg(&dev->sbd.core, "%s:%u: %s completed\n", __func__, __LINE__,
+		op);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(ps3stor_read_write_sectors);
+
+
+/**
+ *	ps3stor_send_command - send a device command to a storage device
+ *	@dev: Pointer to a struct ps3_storage_device
+ *	@cmd: Command number
+ *	@arg1: First command argument
+ *	@arg2: Second command argument
+ *	@arg3: Third command argument
+ *	@arg4: Fourth command argument
+ *
+ *	Returns 0 for success, -1 in case of failure to submit the command, or
+ *	an LV1 status value in case of other errors
+ */
+u64 ps3stor_send_command(struct ps3_storage_device *dev, u64 cmd, u64 arg1,
+			 u64 arg2, u64 arg3, u64 arg4)
+{
+	int res;
+
+	dev_dbg(&dev->sbd.core, "%s:%u: send device command 0x%lx\n", __func__,
+		__LINE__, cmd);
+
+	init_completion(&dev->done);
+
+	res = lv1_storage_send_device_command(dev->sbd.dev_id, cmd, arg1,
+					      arg2, arg3, arg4, &dev->tag);
+	if (res) {
+		dev_err(&dev->sbd.core,
+			"%s:%u: send_device_command 0x%lx failed %d\n",
+			__func__, __LINE__, cmd, res);
+		return -1;
+	}
+
+	wait_for_completion(&dev->done);
+	if (dev->lv1_status) {
+		dev_dbg(&dev->sbd.core, "%s:%u: command 0x%lx failed 0x%lx\n",
+			__func__, __LINE__, cmd, dev->lv1_status);
+		return dev->lv1_status;
+	}
+
+	dev_dbg(&dev->sbd.core, "%s:%u: command 0x%lx completed\n", __func__,
+		__LINE__, cmd);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(ps3stor_send_command);
+
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("PS3 Storage Bus Library");
+MODULE_AUTHOR("Sony Corporation");
--- /dev/null
+++ b/include/asm-powerpc/ps3stor.h
@@ -0,0 +1,70 @@
+/*
+ * PS3 Storage Devices
+ *
+ * Copyright (C) 2007 Sony Computer Entertainment Inc.
+ * Copyright 2007 Sony Corp.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published
+ * by the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#ifndef _ASM_POWERPC_PS3STOR_H_
+#define _ASM_POWERPC_PS3STOR_H_
+
+#include <linux/irqreturn.h>
+
+#include <asm/ps3.h>
+
+
+struct ps3_storage_region {
+	unsigned int id;
+	u64 start;
+	u64 size;
+};
+
+struct ps3_storage_device {
+	struct ps3_system_bus_device sbd;
+
+	struct ps3_dma_region dma_region;
+	unsigned int irq;
+	u64 blk_size;
+
+	u64 tag;
+	u64 lv1_status;
+	struct completion done;
+
+	unsigned long bounce_size;
+	void *bounce_buf;
+	u64 bounce_lpar;
+	dma_addr_t bounce_dma;
+
+	unsigned int num_regions;
+	unsigned long accessible_regions;
+	unsigned int region_idx;		/* first accessible region */
+	struct ps3_storage_region regions[0];	/* Must be last */
+};
+
+static inline struct ps3_storage_device *to_ps3_storage_device(struct device *dev)
+{
+	return container_of(dev, struct ps3_storage_device, sbd.core);
+}
+
+extern int ps3stor_setup(struct ps3_storage_device *dev);
+extern void ps3stor_teardown(struct ps3_storage_device *dev);
+extern u64 ps3stor_read_write_sectors(struct ps3_storage_device *dev, u64 lpar,
+				      u64 start_sector, u64 sectors,
+				      int write);
+extern u64 ps3stor_send_command(struct ps3_storage_device *dev, u64 cmd,
+				u64 arg1, u64 arg2, u64 arg3, u64 arg4);
+
+#endif /* _ASM_POWERPC_PS3STOR_H_ */

-- 
With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center 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/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

[patch 3/6] PS3: Storage device registration routines.

[Geert Uytterhoeven]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 9391 bytes --]

Add support for storage devices to the device probe code.

Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
Signed-off-by: Geoff Levand <geoffrey.levand@am.sony.com>
---
Changes since previous submission:
      o use msleep() instead of schedule_timeout()

 arch/powerpc/platforms/ps3/device-init.c |  286 +++++++++++++++++++++++++++++++
 1 files changed, 286 insertions(+)

--- a/arch/powerpc/platforms/ps3/device-init.c
+++ b/arch/powerpc/platforms/ps3/device-init.c
@@ -26,6 +26,7 @@
 
 #include <asm/firmware.h>
 #include <asm/lv1call.h>
+#include <asm/ps3stor.h>
 
 #include "platform.h"
 
@@ -237,6 +238,262 @@ static int __init ps3_setup_vuart_device
 	return result;
 }
 
+static int ps3stor_wait_for_completion(u64 dev_id, u64 tag,
+				       unsigned int timeout)
+{
+	int result = -1;
+	unsigned int retries = 0;
+	u64 status;
+
+	for (retries = 0; retries < timeout; retries++) {
+		result = lv1_storage_check_async_status(dev_id, tag, &status);
+		if (!result)
+			break;
+
+		msleep(1);
+	}
+
+	if (result)
+		pr_debug("%s:%u: check_async_status: %s, status %lx\n",
+			 __func__, __LINE__, ps3_result(result), status);
+
+	return result;
+}
+
+/**
+ * ps3_storage_wait_for_device - Wait for a storage device to become ready.
+ * @repo: The repository device to wait for.
+ *
+ * Uses the hypervisor's storage device notification mechanism to wait until
+ * a storage device is ready.  The device notification mechanism uses a
+ * psuedo device (id = -1) to asynchronously notify the guest when storage
+ * devices become ready.  The notification device has a block size of 512
+ * bytes.
+ */
+
+static int ps3_storage_wait_for_device(const struct ps3_repository_device *repo)
+{
+	int result;
+	const u64 notification_dev_id = (u64)-1LL;
+	const unsigned int timeout = HZ;
+	u64 lpar;
+	u64 tag;
+	struct {
+		u64 operation_code;	/* must be zero */
+		u64 event_mask;		/* 1 = device ready */
+	} *notify_cmd;
+	struct {
+		u64 event_type;		/* notify_device_ready */
+		u64 bus_id;
+		u64 dev_id;
+		u64 dev_type;
+		u64 dev_port;
+	} *notify_event;
+	enum {
+		notify_device_ready = 1
+	};
+
+	pr_debug(" -> %s:%u: bus_id %u, dev_id %u, dev_type %u\n", __func__,
+		 __LINE__, repo->bus_id, repo->dev_id, repo->dev_type);
+
+	notify_cmd = kzalloc(512, GFP_KERNEL);
+	notify_event = (void *)notify_cmd;
+	if (!notify_cmd)
+		return -ENOMEM;
+
+	lpar = ps3_mm_phys_to_lpar(__pa(notify_cmd));
+
+	result = lv1_open_device(repo->bus_id, notification_dev_id, 0);
+	if (result) {
+		printk(KERN_ERR "%s:%u: lv1_open_device %s\n", __func__,
+		       __LINE__, ps3_result(result));
+		result = -ENODEV;
+		goto fail_free;
+	}
+
+	/* Setup and write the request for device notification. */
+
+	notify_cmd->operation_code = 0;	/* must be zero */
+	notify_cmd->event_mask = 0x01;	/* device ready */
+
+	result = lv1_storage_write(notification_dev_id, 0, 0, 1, 0, lpar,
+				   &tag);
+	if (result) {
+		printk(KERN_ERR "%s:%u: write failed %s\n", __func__, __LINE__,
+		       ps3_result(result));
+		result = -ENODEV;
+		goto fail_close;
+	}
+
+	/* Wait for the write completion */
+
+	result = ps3stor_wait_for_completion(notification_dev_id, tag,
+					     timeout);
+	if (result) {
+		printk(KERN_ERR "%s:%u: write not completed %s\n", __func__,
+		       __LINE__, ps3_result(result));
+		result = -ENODEV;
+		goto fail_close;
+	}
+
+	/* Loop here processing the requested notification events. */
+
+	result = -ENODEV;
+	while (1) {
+		memset(notify_event, 0, sizeof(*notify_event));
+
+		result = lv1_storage_read(notification_dev_id, 0, 0, 1, 0,
+					  lpar, &tag);
+		if (result) {
+			printk(KERN_ERR "%s:%u: write failed %s\n", __func__,
+			       __LINE__, ps3_result(result));
+			break;
+		}
+
+		result = ps3stor_wait_for_completion(notification_dev_id, tag,
+						     timeout);
+		if (result) {
+			printk(KERN_ERR "%s:%u: read not completed %s\n",
+			       __func__, __LINE__, ps3_result(result));
+			break;
+		}
+
+		if (notify_event->event_type != notify_device_ready ||
+		    notify_event->bus_id != repo->bus_id) {
+			pr_debug("%s:%u: bad notify_event: event %lu, "
+				 "dev_id %lu, dev_type %lu\n",
+				 __func__, __LINE__, notify_event->event_type,
+				 notify_event->dev_id, notify_event->dev_type);
+			break;
+		}
+
+		if (notify_event->dev_id == repo->dev_id &&
+		    notify_event->dev_type == repo->dev_type) {
+			pr_debug("%s:%u: device ready: dev_id %u\n", __func__,
+				 __LINE__, repo->dev_id);
+			result = 0;
+			break;
+		}
+
+		if (notify_event->dev_id == repo->dev_id &&
+		    notify_event->dev_type == PS3_DEV_TYPE_NOACCESS) {
+			pr_debug("%s:%u: no access: dev_id %u\n", __func__,
+				 __LINE__, repo->dev_id);
+			break;
+		}
+	}
+
+fail_close:
+	lv1_close_device(repo->bus_id, notification_dev_id);
+fail_free:
+	kfree(notify_cmd);
+	pr_debug(" <- %s:%u\n", __func__, __LINE__);
+	return result;
+}
+
+static int ps3_setup_storage_dev(const struct ps3_repository_device *repo,
+				 enum ps3_match_id match_id)
+{
+	int result;
+	struct ps3_storage_device *p;
+	u64 port, blk_size, num_blocks;
+	unsigned int num_regions, i;
+
+	pr_debug(" -> %s:%u: match_id %u\n", __func__, __LINE__, match_id);
+
+	result = ps3_repository_read_stor_dev_info(repo->bus_index,
+						   repo->dev_index, &port,
+						   &blk_size, &num_blocks,
+						   &num_regions);
+	if (result) {
+		printk(KERN_ERR "%s:%u: _read_stor_dev_info failed %d\n",
+		       __func__, __LINE__, result);
+		return -ENODEV;
+	}
+
+	pr_debug("%s:%u: index %u:%u: port %lu blk_size %lu num_blocks %lu "
+		 "num_regions %u\n", __func__, __LINE__, repo->bus_index,
+		 repo->dev_index, port, blk_size, num_blocks, num_regions);
+
+	p = kzalloc(sizeof(struct ps3_storage_device) +
+		    num_regions * sizeof(struct ps3_storage_region),
+		    GFP_KERNEL);
+	if (!p) {
+		result = -ENOMEM;
+		goto fail_malloc;
+	}
+
+	p->sbd.match_id = match_id;
+	p->sbd.dev_type = PS3_DEVICE_TYPE_SB;
+	p->sbd.bus_id = repo->bus_id;
+	p->sbd.dev_id = repo->dev_id;
+	p->sbd.d_region = &p->dma_region;
+	p->blk_size = blk_size;
+	p->num_regions = num_regions;
+
+	result = ps3_repository_find_interrupt(repo,
+					       PS3_INTERRUPT_TYPE_EVENT_PORT,
+					       &p->sbd.interrupt_id);
+	if (result) {
+		printk(KERN_ERR "%s:%u: find_interrupt failed %d\n", __func__,
+		       __LINE__, result);
+		result = -ENODEV;
+		goto fail_find_interrupt;
+	}
+
+	/* FIXME: Arrange to only do this on a 'cold' boot */
+
+	result = ps3_storage_wait_for_device(repo);
+	if (result) {
+		printk(KERN_ERR "%s:%u: storage_notification failed %d\n",
+		       __func__, __LINE__, result);
+		result = -ENODEV;
+		goto fail_probe_notification;
+	}
+
+	for (i = 0; i < num_regions; i++) {
+		unsigned int id;
+		u64 start, size;
+
+		result = ps3_repository_read_stor_dev_region(repo->bus_index,
+							     repo->dev_index,
+							     i, &id, &start,
+							     &size);
+		if (result) {
+			printk(KERN_ERR
+			       "%s:%u: read_stor_dev_region failed %d\n",
+			       __func__, __LINE__, result);
+			result = -ENODEV;
+			goto fail_read_region;
+		}
+		pr_debug("%s:%u: region %u: id %u start %lu size %lu\n",
+			 __func__, __LINE__, i, id, start, size);
+
+		p->regions[i].id = id;
+		p->regions[i].start = start;
+		p->regions[i].size = size;
+	}
+
+	result = ps3_system_bus_device_register(&p->sbd);
+	if (result) {
+		pr_debug("%s:%u ps3_system_bus_device_register failed\n",
+			 __func__, __LINE__);
+		goto fail_device_register;
+	}
+
+	pr_debug(" <- %s:%u\n", __func__, __LINE__);
+	return 0;
+
+fail_device_register:
+fail_read_region:
+fail_probe_notification:
+fail_find_interrupt:
+	kfree(p);
+fail_malloc:
+	pr_debug(" <- %s:%u: fail.\n", __func__, __LINE__);
+	return result;
+}
+
 static int __init ps3_register_vuart_devices(void)
 {
 	int result;
@@ -356,6 +613,35 @@ static int ps3_register_repository_devic
 				__func__, __LINE__);
 		}
 		break;
+	case PS3_DEV_TYPE_STOR_DISK:
+		result = ps3_setup_storage_dev(repo, PS3_MATCH_ID_STOR_DISK);
+
+		/* Some devices are not accessable from the Other OS lpar. */
+		if (result == -ENODEV) {
+			result = 0;
+			pr_debug("%s:%u: not accessable\n", __func__,
+				 __LINE__);
+		}
+
+		if (result)
+			pr_debug("%s:%u ps3_setup_storage_dev failed\n",
+				 __func__, __LINE__);
+		break;
+
+	case PS3_DEV_TYPE_STOR_ROM:
+		result = ps3_setup_storage_dev(repo, PS3_MATCH_ID_STOR_ROM);
+		if (result)
+			pr_debug("%s:%u ps3_setup_storage_dev failed\n",
+				 __func__, __LINE__);
+		break;
+
+	case PS3_DEV_TYPE_STOR_FLASH:
+		result = ps3_setup_storage_dev(repo, PS3_MATCH_ID_STOR_FLASH);
+		if (result)
+			pr_debug("%s:%u ps3_setup_storage_dev failed\n",
+				 __func__, __LINE__);
+		break;
+
 	default:
 		result = 0;
 		pr_debug("%s:%u: unsupported dev_type %u\n", __func__, __LINE__,

-- 
With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center 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/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

[patch 4/6] ps3: Disk Storage Driver

[Geert Uytterhoeven]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 16217 bytes --]

From: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>

Add a Disk Storage Driver for the PS3:
  - Implemented as a block device driver with a dynamic major
  - Disk names (and partitions) are of the format ps3d%c(%u)
  - Uses software scatter-gather with a 64 KiB bounce buffer as the hypervisor
    doesn't support scatter-gather

CC: Geoff Levand <geoffrey.levand@am.sony.com>
Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
---
Changes since previous submission:
      o Don't use `default y' in Kconfig
      o Remove the thread for handling requests
      o Set up sysfs links between block device and PS3 system device:
          . /sys/block/ps3da/device -> ../../devices/ps3_system/sb_02
	  . /sys/devices/ps3_system/sb_02/block:ps3da -> ../../../block/ps3da
      o Fix all FIXMEs
      o Implement proper barrier support
      o Call add_disk_randomness()
      o Add a test to verify we are running on a PS3
      o Fix error path in ps3disk_init()
      o Fix cleanup order in ps3disk_exit()

 arch/powerpc/platforms/ps3/Kconfig |   10 
 drivers/block/Makefile             |    1 
 drivers/block/ps3disk.c            |  526 +++++++++++++++++++++++++++++++++++++
 3 files changed, 537 insertions(+)

--- a/arch/powerpc/platforms/ps3/Kconfig
+++ b/arch/powerpc/platforms/ps3/Kconfig
@@ -102,4 +102,14 @@ config PS3_STORAGE
 	depends on PPC_PS3
 	tristate
 
+config PS3_DISK
+	tristate "PS3 Disk Storage Driver"
+	depends on PPC_PS3 && BLOCK
+	select PS3_STORAGE
+	help
+	  Include support for the PS3 Disk Storage.
+
+	  This support is required to access the PS3 hard disk.
+	  In general, all users will say Y or M.
+
 endmenu
--- a/drivers/block/Makefile
+++ b/drivers/block/Makefile
@@ -29,3 +29,4 @@ obj-$(CONFIG_VIODASD)		+= viodasd.o
 obj-$(CONFIG_BLK_DEV_SX8)	+= sx8.o
 obj-$(CONFIG_BLK_DEV_UB)	+= ub.o
 
+obj-$(CONFIG_PS3_DISK)		+= ps3disk.o
--- /dev/null
+++ b/drivers/block/ps3disk.c
@@ -0,0 +1,526 @@
+/*
+ * PS3 Disk Storage Driver
+ *
+ * Copyright (C) 2007 Sony Computer Entertainment Inc.
+ * Copyright 2007 Sony Corp.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published
+ * by the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include <linux/dma-mapping.h>
+#include <linux/blkdev.h>
+#include <linux/freezer.h>
+#include <linux/hdreg.h>
+#include <linux/interrupt.h>
+#include <linux/kthread.h>
+
+#include <asm/lv1call.h>
+#include <asm/ps3stor.h>
+#include <asm/firmware.h>
+
+
+#define DEVICE_NAME		"ps3disk"
+
+#define BOUNCE_SIZE		(64*1024)
+
+#define PS3DISK_MAJOR		0
+
+#define PS3DISK_MAX_DISKS	16
+#define PS3DISK_MINORS		16
+
+#define KERNEL_SECTOR_SIZE	512
+
+
+#define PS3DISK_NAME		"ps3d%c"
+
+#define LV1_STORAGE_ATA_HDDOUT		(0x23)
+
+
+struct ps3disk_private {
+	spinlock_t lock;		/* Request queue spinlock */
+	struct request_queue *queue;
+	struct gendisk *gendisk;
+	unsigned int blocking_factor;
+	struct request *req;
+};
+#define ps3disk_priv(dev)	((dev)->sbd.core.driver_data)
+
+static int ps3disk_major = PS3DISK_MAJOR;
+
+
+static int ps3disk_open(struct inode *inode, struct file *file)
+{
+	struct ps3_storage_device *dev = inode->i_bdev->bd_disk->private_data;
+
+	file->private_data = dev;
+	return 0;
+}
+
+static struct block_device_operations ps3disk_fops = {
+	.owner		= THIS_MODULE,
+	.open		= ps3disk_open,
+};
+
+
+static void ps3disk_scatter_gather(struct ps3_storage_device *dev,
+				   struct request *req, int gather)
+{
+	unsigned int sectors = 0, offset = 0;
+	struct bio *bio;
+	sector_t sector;
+	struct bio_vec *bvec;
+	unsigned int i = 0, j;
+	size_t size;
+	void *buf;
+
+	rq_for_each_bio(bio, req) {
+		sector = bio->bi_sector;
+		dev_dbg(&dev->sbd.core,
+			"%s:%u: bio %u: %u segs %u sectors from %lu\n",
+			__func__, __LINE__, i, bio_segments(bio),
+			bio_sectors(bio), sector);
+		bio_for_each_segment(bvec, bio, j) {
+			size = bio_cur_sectors(bio)*KERNEL_SECTOR_SIZE;
+			buf = __bio_kmap_atomic(bio, j, KM_USER0);
+			if (gather)
+				memcpy(dev->bounce_buf+offset, buf, size);
+			else
+				memcpy(buf, dev->bounce_buf+offset, size);
+			offset += size;
+			__bio_kunmap_atomic(bio, KM_USER0);
+		}
+		sectors += bio_sectors(bio);
+		i++;
+	}
+}
+
+static u64 ps3disk_read_write_sectors(struct ps3_storage_device *dev, u64 lpar,
+				      u64 start_sector, u64 sectors, int write)
+{
+	unsigned int region_id = dev->regions[dev->region_idx].id;
+	const char *op = write ? "write" : "read";
+	int res;
+
+	dev_dbg(&dev->sbd.core, "%s:%u: %s %lu sectors starting at %lu\n",
+		__func__, __LINE__, op, sectors, start_sector);
+
+	res = write ? lv1_storage_write(dev->sbd.dev_id, region_id,
+					start_sector, sectors, 0, lpar,
+					&dev->tag)
+		    : lv1_storage_read(dev->sbd.dev_id, region_id,
+				       start_sector, sectors, 0, lpar,
+				       &dev->tag);
+	if (res) {
+		dev_dbg(&dev->sbd.core, "%s:%u: %s failed %d\n", __func__,
+			__LINE__, op, res);
+		return -1;
+	}
+
+	return 0;
+}
+
+static int ps3disk_submit_request_sg(struct ps3_storage_device *dev,
+				     struct request *req)
+{
+	struct ps3disk_private *priv = ps3disk_priv(dev);
+	int write = rq_data_dir(req);
+	const char *op = write ? "write" : "read";
+	u64 res;
+
+#ifdef DEBUG
+	unsigned int n = 0;
+	struct bio *bio;
+	rq_for_each_bio(bio, req)
+		n++;
+	dev_dbg(&dev->sbd.core,
+		"%s:%u: %s req has %u bios for %lu sectors %lu hard sectors\n",
+		__func__, __LINE__, op, n, req->nr_sectors,
+		req->hard_nr_sectors);
+#endif
+
+	if (write)
+		ps3disk_scatter_gather(dev, req, 1);
+
+	res = ps3disk_read_write_sectors(dev, dev->bounce_lpar,
+					 req->sector*priv->blocking_factor,
+					 req->nr_sectors*priv->blocking_factor,
+					 write);
+	if (res) {
+		dev_err(&dev->sbd.core, "%s:%u: %s failed 0x%lx\n", __func__,
+			__LINE__, op, res);
+		end_request(req, 0);
+		return 0;
+	}
+
+	priv->req = req;
+	return 1;
+}
+
+static int ps3disk_submit_flush_request(struct ps3_storage_device *dev,
+					struct request *req)
+{
+	struct ps3disk_private *priv = ps3disk_priv(dev);
+	u64 res;
+
+	dev_dbg(&dev->sbd.core, "%s:%u: flush request\n", __func__, __LINE__);
+
+	res = lv1_storage_send_device_command(dev->sbd.dev_id,
+					      LV1_STORAGE_ATA_HDDOUT, 0, 0, 0,
+					      0, &dev->tag);
+	if (res) {
+		dev_err(&dev->sbd.core, "%s:%u: sync cache failed 0x%lx\n",
+			__func__, __LINE__, res);
+		end_request(req, 0);
+		return 0;
+	}
+
+	priv->req = req;
+	return 1;
+}
+
+static void ps3disk_do_request(struct ps3_storage_device *dev,
+			       request_queue_t *q)
+{
+	struct request *req;
+
+	dev_dbg(&dev->sbd.core, "%s:%u\n", __func__, __LINE__);
+
+	while ((req = elv_next_request(q))) {
+		if (blk_fs_request(req)) {
+			if (ps3disk_submit_request_sg(dev, req))
+				break;
+		} else if (req->cmd_type == REQ_TYPE_FLUSH) {
+			if (ps3disk_submit_flush_request(dev, req))
+				break;
+		} else {
+			blk_dump_rq_flags(req, DEVICE_NAME " bad request");
+			end_request(req, 0);
+			continue;
+		}
+	}
+}
+
+static void ps3disk_request(request_queue_t *q)
+{
+	struct ps3_storage_device *dev = q->queuedata;
+	struct ps3disk_private *priv = ps3disk_priv(dev);
+
+	if (priv->req) {
+		dev_dbg(&dev->sbd.core, "%s:%u busy\n", __func__, __LINE__);
+		return;
+	}
+
+	ps3disk_do_request(dev, q);
+}
+
+static irqreturn_t ps3disk_interrupt(int irq, void *data)
+{
+	struct ps3_storage_device *dev = data;
+	struct ps3disk_private *priv;
+	struct request *req;
+	int res, read, uptodate;
+	u64 tag, status;
+	unsigned long num_sectors;
+	const char *op;
+
+	res = lv1_storage_get_async_status(dev->sbd.dev_id, &tag, &status);
+
+	if (tag != dev->tag)
+		dev_err(&dev->sbd.core,
+			"%s:%u: tag mismatch, got %lx, expected %lx\n",
+			__func__, __LINE__, tag, dev->tag);
+
+	if (res) {
+		dev_err(&dev->sbd.core, "%s:%u: res=%d status=0x%lx\n",
+			__func__, __LINE__, res, status);
+		return IRQ_HANDLED;
+	}
+
+	priv = ps3disk_priv(dev);
+	req = priv->req;
+	if (!req) {
+		dev_dbg(&dev->sbd.core,
+			"%s:%u non-block layer request completed\n", __func__,
+			__LINE__);
+		dev->lv1_status = status;
+		complete(&dev->done);
+		return IRQ_HANDLED;
+	}
+
+	if (req->cmd_type == REQ_TYPE_FLUSH) {
+		read = 0;
+		num_sectors = req->hard_cur_sectors;
+		op = "flush";
+	} else {
+		read = !rq_data_dir(req);
+		num_sectors = req->nr_sectors;
+		op = read ? "read" : "write";
+	}
+	if (status) {
+		dev_dbg(&dev->sbd.core, "%s:%u: %s failed 0x%lx\n", __func__,
+			__LINE__, op, status);
+		uptodate = 0;
+	} else {
+		dev_dbg(&dev->sbd.core, "%s:%u: %s completed\n", __func__,
+			__LINE__, op);
+		uptodate = 1;
+		if (read)
+			ps3disk_scatter_gather(dev, req, 0);
+	}
+
+	spin_lock(&priv->lock);
+	if (!end_that_request_first(req, uptodate, num_sectors)) {
+		add_disk_randomness(req->rq_disk);
+		blkdev_dequeue_request(req);
+		end_that_request_last(req, uptodate);
+	}
+	priv->req = NULL;
+	ps3disk_do_request(dev, priv->queue);
+	spin_unlock(&priv->lock);
+
+	return IRQ_HANDLED;
+}
+
+static int ps3disk_sync_cache(struct ps3_storage_device *dev)
+{
+	u64 res;
+
+	dev_dbg(&dev->sbd.core, "%s:%u: sync cache\n", __func__, __LINE__);
+
+	res = ps3stor_send_command(dev, LV1_STORAGE_ATA_HDDOUT, 0, 0, 0, 0);
+	if (res) {
+		dev_err(&dev->sbd.core, "%s:%u: sync cache failed 0x%lx\n",
+			__func__, __LINE__, res);
+		return -EIO;
+	}
+	return 0;
+}
+
+static void ps3disk_prepare_flush(request_queue_t *q, struct request *req)
+{
+	struct ps3_storage_device *dev = q->queuedata;
+
+	dev_dbg(&dev->sbd.core, "%s:%u\n", __func__, __LINE__);
+
+	memset(req->cmd, 0, sizeof(req->cmd));
+	req->cmd_type = REQ_TYPE_FLUSH;
+}
+
+static int ps3disk_issue_flush(request_queue_t *q, struct gendisk *gendisk,
+			       sector_t *sector)
+{
+	struct ps3_storage_device *dev = q->queuedata;
+	struct request *req;
+	int res;
+
+	dev_dbg(&dev->sbd.core, "%s:%u\n", __func__, __LINE__);
+
+	req = blk_get_request(q, WRITE, __GFP_WAIT);
+	ps3disk_prepare_flush(q, req);
+	res = blk_execute_rq(q, gendisk, req, 0);
+	if (res)
+		dev_err(&dev->sbd.core, "%s:%u: flush request failed %d\n",
+			__func__, __LINE__, res);
+	blk_put_request(req);
+	return res;
+}
+
+
+static unsigned long ps3disk_mask;
+
+static int __devinit ps3disk_probe(struct ps3_system_bus_device *_dev)
+{
+	struct ps3_storage_device *dev = to_ps3_storage_device(&_dev->core);
+	struct ps3disk_private *priv;
+	int error;
+	unsigned int devidx;
+	struct request_queue *queue;
+	struct gendisk *gendisk;
+
+	if (dev->blk_size < KERNEL_SECTOR_SIZE) {
+		dev_err(&dev->sbd.core,
+			"%s:%u: cannot handle block size %lu\n", __func__,
+			__LINE__, dev->blk_size);
+		return -EINVAL;
+	}
+
+	BUILD_BUG_ON(PS3DISK_MAX_DISKS > BITS_PER_LONG);
+	devidx = find_first_zero_bit(&ps3disk_mask, PS3DISK_MAX_DISKS);
+	if (devidx >= PS3DISK_MAX_DISKS) {
+		dev_err(&dev->sbd.core, "%s:%u: Too many disks\n", __func__,
+			__LINE__);
+		return -ENOSPC;
+	}
+	__set_bit(devidx, &ps3disk_mask);
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv) {
+		error = -ENOMEM;
+		goto fail;
+	}
+
+	ps3disk_priv(dev) = priv;
+	spin_lock_init(&priv->lock);
+
+	dev->bounce_size = BOUNCE_SIZE;
+	dev->bounce_buf = kmalloc(BOUNCE_SIZE, GFP_DMA);
+	if (!dev->bounce_buf) {
+		error = -ENOMEM;
+		goto fail_free_priv;
+	}
+
+	error = ps3stor_setup(dev);
+	if (error)
+		goto fail_free_bounce;
+
+	/* override the interrupt handler */
+	free_irq(dev->irq, dev);
+	error = request_irq(dev->irq, ps3disk_interrupt, IRQF_DISABLED,
+			    dev->sbd.core.driver->name, dev);
+	if (error) {
+		dev_err(&dev->sbd.core, "%s:%u: request_irq failed %d\n",
+			__func__, __LINE__, error);
+		goto fail_teardown;
+	}
+
+	queue = blk_init_queue(ps3disk_request, &priv->lock);
+	if (!queue) {
+		dev_err(&dev->sbd.core, "%s:%u: blk_init_queue failed\n",
+			__func__, __LINE__);
+		error = -ENOMEM;
+		goto fail_teardown;
+	}
+
+	priv->queue = queue;
+	queue->queuedata = dev;
+
+	blk_queue_bounce_limit(queue, BLK_BOUNCE_HIGH);
+
+	blk_queue_max_sectors(queue, dev->bounce_size/KERNEL_SECTOR_SIZE);
+	blk_queue_segment_boundary(queue, -1UL);
+	blk_queue_dma_alignment(queue, dev->blk_size-1);
+	blk_queue_hardsect_size(queue, dev->blk_size);
+
+	blk_queue_issue_flush_fn(queue, ps3disk_issue_flush);
+	blk_queue_ordered(queue, QUEUE_ORDERED_DRAIN_FLUSH,
+			  ps3disk_prepare_flush);
+
+	blk_queue_max_phys_segments(queue, -1);
+	blk_queue_max_hw_segments(queue, -1);
+	blk_queue_max_segment_size(queue, dev->bounce_size);
+
+	gendisk = alloc_disk(PS3DISK_MINORS);
+	if (!gendisk) {
+		dev_err(&dev->sbd.core, "%s:%u: alloc_disk failed\n", __func__,
+			__LINE__);
+		error = -ENOMEM;
+		goto fail_cleanup_queue;
+	}
+
+	priv->gendisk = gendisk;
+	gendisk->major = ps3disk_major;
+	gendisk->first_minor = devidx * PS3DISK_MINORS;
+	gendisk->fops = &ps3disk_fops;
+	gendisk->queue = queue;
+	gendisk->private_data = dev;
+	gendisk->driverfs_dev = &dev->sbd.core;
+	snprintf(gendisk->disk_name, sizeof(gendisk->disk_name), PS3DISK_NAME,
+		 devidx+'a');
+	priv->blocking_factor = dev->blk_size/KERNEL_SECTOR_SIZE;
+	set_capacity(gendisk,
+		     dev->regions[dev->region_idx].size*priv->blocking_factor);
+
+	add_disk(gendisk);
+	return 0;
+
+fail_cleanup_queue:
+	blk_cleanup_queue(queue);
+fail_teardown:
+	ps3stor_teardown(dev);
+fail_free_bounce:
+	kfree(dev->bounce_buf);
+fail_free_priv:
+	kfree(priv);
+fail:
+	__clear_bit(devidx, &ps3disk_mask);
+	return error;
+}
+
+static int ps3disk_remove(struct ps3_system_bus_device *_dev)
+{
+	struct ps3_storage_device *dev = to_ps3_storage_device(&_dev->core);
+	struct ps3disk_private *priv = ps3disk_priv(dev);
+
+	__clear_bit(priv->gendisk->first_minor / PS3DISK_MINORS,
+		    &ps3disk_mask);
+	del_gendisk(priv->gendisk);
+	put_disk(priv->gendisk);
+	blk_cleanup_queue(priv->queue);
+	dev_notice(&dev->sbd.core, "Synchronizing disk cache\n");
+	ps3disk_sync_cache(dev);
+	ps3stor_teardown(dev);
+	kfree(dev->bounce_buf);
+	kfree(priv);
+	return 0;
+}
+
+static struct ps3_system_bus_driver ps3disk = {
+	.match_id	= PS3_MATCH_ID_STOR_DISK,
+	.core.name	= DEVICE_NAME,
+	.core.owner	= THIS_MODULE,
+	.probe		= ps3disk_probe,
+	.remove		= ps3disk_remove,
+	.shutdown	= ps3disk_remove,
+};
+
+
+static int __init ps3disk_init(void)
+{
+	int error;
+
+	if (!firmware_has_feature(FW_FEATURE_PS3_LV1))
+		return -ENODEV;
+
+	error = register_blkdev(ps3disk_major, DEVICE_NAME);
+	if (error <= 0) {
+		printk(KERN_ERR "%s:%u: register_blkdev failed %d\n", __func__,
+		       __LINE__, error);
+		return error;
+	}
+	if (!ps3disk_major)
+		ps3disk_major = error;
+
+	pr_info("%s:%u: registered block device major %d\n", __func__,
+		__LINE__, ps3disk_major);
+
+	error = ps3_system_bus_driver_register(&ps3disk);
+	if (error)
+		unregister_blkdev(ps3disk_major, DEVICE_NAME);
+
+	return error;
+}
+
+static void __exit ps3disk_exit(void)
+{
+	ps3_system_bus_driver_unregister(&ps3disk);
+	unregister_blkdev(ps3disk_major, DEVICE_NAME);
+}
+
+module_init(ps3disk_init);
+module_exit(ps3disk_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("PS3 Disk Storage Driver");
+MODULE_AUTHOR("Sony Corporation");
+MODULE_ALIAS(PS3_MODULE_ALIAS_STOR_DISK);

-- 
With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center 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/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

Re: [patch 4/6] ps3: Disk Storage Driver

[David Woodhouse]

On Fri, 2007-06-15 at 13:39 +0200, Geert Uytterhoeven wrote:
> Add a Disk Storage Driver for the PS3:
>   - Implemented as a block device driver with a dynamic major
>   - Disk names (and partitions) are of the format ps3d%c(%u)
>   - Uses software scatter-gather with a 64 KiB bounce buffer as the hypervisor
>     doesn't support scatter-gather

Any particular reason why this is done as a separate block device driver
rather than as SCSI?

-- 
dwmw2

Re: [patch 4/6] ps3: Disk Storage Driver

[Arnd Bergmann]

On Friday 15 June 2007, David Woodhouse wrote:
>=20
> On Fri, 2007-06-15 at 13:39 +0200, Geert Uytterhoeven wrote:
> > Add a Disk Storage Driver for the PS3:
> > =A0 - Implemented as a block device driver with a dynamic major
> > =A0 - Disk names (and partitions) are of the format ps3d%c(%u)
> > =A0 - Uses software scatter-gather with a 64 KiB bounce buffer as the h=
ypervisor
> > =A0 =A0 doesn't support scatter-gather
>=20
> Any particular reason why this is done as a separate block device driver
> rather than as SCSI?

Because the hypervisor interface is based on tranferring blocks, not based
on SCSI commands like the one for the optical storage drive.

The first version of this driver actually was based on SCSI command emulati=
on,
and in an earlier round of reviews it was decided to rewrite it to use the
simpler block interface directly.

	Arnd <><

Re: [patch 4/6] ps3: Disk Storage Driver

[Geert Uytterhoeven]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1162 bytes --]

On Fri, 15 Jun 2007, David Woodhouse wrote:
> On Fri, 2007-06-15 at 13:39 +0200, Geert Uytterhoeven wrote:
> > Add a Disk Storage Driver for the PS3:
> >   - Implemented as a block device driver with a dynamic major
> >   - Disk names (and partitions) are of the format ps3d%c(%u)
> >   - Uses software scatter-gather with a 64 KiB bounce buffer as the hypervisor
> >     doesn't support scatter-gather
> 
> Any particular reason why this is done as a separate block device driver
> rather than as SCSI?

Because no new fake SCSI drivers are accepted anymore.

With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center 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/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

Re: [patch 4/6] ps3: Disk Storage Driver

[Alan Cox]

> > Any particular reason why this is done as a separate block device driver
> > rather than as SCSI?
> 
> Because no new fake SCSI drivers are accepted anymore.

Where did drivers/ata come from ;)

How about making it a fake ata driver if James is being fussy 8)

Re: [patch 4/6] ps3: Disk Storage Driver

[Geert Uytterhoeven]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1046 bytes --]

On Fri, 15 Jun 2007, Alan Cox wrote:
> > > Any particular reason why this is done as a separate block device driver
> > > rather than as SCSI?
> > 
> > Because no new fake SCSI drivers are accepted anymore.
> 
> Where did drivers/ata come from ;)

I was told that one got in just before the moratorium on fake SCSI.

> How about making it a fake ata driver if James is being fussy 8)

I don't think Christoph likes fake ATA drivers neither ;-)

With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center 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/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

Re: [patch 4/6] ps3: Disk Storage Driver

[David Miller]

From: Alan Cox <alan@lxorguk.ukuu.org.uk>
Date: Fri, 15 Jun 2007 17:15:45 +0100

> > > Any particular reason why this is done as a separate block device driver
> > > rather than as SCSI?
> > 
> > Because no new fake SCSI drivers are accepted anymore.
> 
> Where did drivers/ata come from ;)
> 
> How about making it a fake ata driver if James is being fussy 8)

That sounds like a good idea for my virtual I/O case on
Niagara too actually :-)

Another quirk I have to deal with is that under LDOMs you
can export full disks and also just slices.  So I'll have
to get down into the partition machinery to support that
somehow.

Re: [patch 4/6] ps3: Disk Storage Driver

[James Bottomley]

On Fri, 2007-06-15 at 14:19 -0700, David Miller wrote:
> From: Alan Cox <alan@lxorguk.ukuu.org.uk>
> Date: Fri, 15 Jun 2007 17:15:45 +0100
> 
> > > > Any particular reason why this is done as a separate block device driver
> > > > rather than as SCSI?
> > > 
> > > Because no new fake SCSI drivers are accepted anymore.
> > 
> > Where did drivers/ata come from ;)
> > 
> > How about making it a fake ata driver if James is being fussy 8)
> 
> That sounds like a good idea for my virtual I/O case on
> Niagara too actually :-)

I have no objections to VIO servers speaking SCSI ... if you want to do
that, I suggest you look at an existing example, like
drivers/scsi/ibmvscsi (the actual communication piece is very ppc
hypervisor specific, though).

However, I do strongly believe that using SCSI for devices which truly
aren't SCSI (i.e. you have to code a shim in the driver between the SCSI
commands and whatever your device speaks) is wrong ... if SCSI has
features that the generic block layer doesn't, then we need to move the
function from SCSI to block to give you access ... in theory this was
the rationale for having libata as the last "fake" SCSI driver, so we
could identify this functionality and begin to move it.

> Another quirk I have to deal with is that under LDOMs you
> can export full disks and also just slices.  So I'll have
> to get down into the partition machinery to support that
> somehow.

For this, it sounds like you might find nbd a more enticing
proposition ... it already is partition independent and is basically a
block to net socket exporter.

James

Re: [patch 4/6] ps3: Disk Storage Driver

[David Miller]

From: James Bottomley <James.Bottomley@SteelEye.com>
Date: Fri, 15 Jun 2007 15:40:42 -0700

> On Fri, 2007-06-15 at 14:19 -0700, David Miller wrote:
> > Another quirk I have to deal with is that under LDOMs you
> > can export full disks and also just slices.  So I'll have
> > to get down into the partition machinery to support that
> > somehow.
> 
> For this, it sounds like you might find nbd a more enticing
> proposition ... it already is partition independent and is basically a
> block to net socket exporter.

That's not gonna work, it's a totally different model.

I have a predefined protocol over hypervisor provided "channels" and
page flipping also done by the hypervisor for the bulk data transfer.
For the client side I cannot change the hypervisor nor the server
speaking on the other end.  And when I do write a server I do want
it to be able to speak to all of the existing clients.

There's SCSI command pass through as well, as I keep mentioning as
it's an important reason I don't want to go with any of the non-SCSI
solutions (other than perhaps ATA) being suggested.

Re: [patch 4/6] ps3: Disk Storage Driver

[James Bottomley]

On Fri, 2007-06-15 at 16:08 -0700, David Miller wrote:
> From: James Bottomley <James.Bottomley@SteelEye.com>
> Date: Fri, 15 Jun 2007 15:40:42 -0700
> 
> > On Fri, 2007-06-15 at 14:19 -0700, David Miller wrote:
> > > Another quirk I have to deal with is that under LDOMs you
> > > can export full disks and also just slices.  So I'll have
> > > to get down into the partition machinery to support that
> > > somehow.
> > 
> > For this, it sounds like you might find nbd a more enticing
> > proposition ... it already is partition independent and is basically a
> > block to net socket exporter.
> 
> That's not gonna work, it's a totally different model.
> 
> I have a predefined protocol over hypervisor provided "channels" and
> page flipping also done by the hypervisor for the bulk data transfer.
> For the client side I cannot change the hypervisor nor the server
> speaking on the other end.  And when I do write a server I do want
> it to be able to speak to all of the existing clients.
> 
> There's SCSI command pass through as well, as I keep mentioning as
> it's an important reason I don't want to go with any of the non-SCSI
> solutions (other than perhaps ATA) being suggested.

Then sure, use SCSI ... the ibmvscsi client originally talked to some
type of hypervisor interface too before IBM extracted it and open
sourced the server.  If actual SCSI commands are going in somewhere ...
be it a real device, a RAID firmware emulation or a hypervisor input,
then I'm happy with the driver being in SCSI.

James

Re: [patch 4/6] ps3: Disk Storage Driver

[David Miller]

From: James Bottomley <James.Bottomley@SteelEye.com>
Date: Fri, 15 Jun 2007 16:28:03 -0700

> On Fri, 2007-06-15 at 16:08 -0700, David Miller wrote:
> > From: James Bottomley <James.Bottomley@SteelEye.com>
> > Date: Fri, 15 Jun 2007 15:40:42 -0700
> > 
> > > On Fri, 2007-06-15 at 14:19 -0700, David Miller wrote:
> > > > Another quirk I have to deal with is that under LDOMs you
> > > > can export full disks and also just slices.  So I'll have
> > > > to get down into the partition machinery to support that
> > > > somehow.
> > > 
> > > For this, it sounds like you might find nbd a more enticing
> > > proposition ... it already is partition independent and is basically a
> > > block to net socket exporter.
> > 
> > That's not gonna work, it's a totally different model.
> > 
> > I have a predefined protocol over hypervisor provided "channels" and
> > page flipping also done by the hypervisor for the bulk data transfer.
> > For the client side I cannot change the hypervisor nor the server
> > speaking on the other end.  And when I do write a server I do want
> > it to be able to speak to all of the existing clients.
> > 
> > There's SCSI command pass through as well, as I keep mentioning as
> > it's an important reason I don't want to go with any of the non-SCSI
> > solutions (other than perhaps ATA) being suggested.
> 
> Then sure, use SCSI ... the ibmvscsi client originally talked to some
> type of hypervisor interface too before IBM extracted it and open
> sourced the server.  If actual SCSI commands are going in somewhere ...
> be it a real device, a RAID firmware emulation or a hypervisor input,
> then I'm happy with the driver being in SCSI.

For normal block I/O it's just raw copies over the provided protocol.

But the service exports a service by which raw SCSI commands can be
sent, for things like disk fault probing and stuff like that.  It's
not for block I/O, it's for "all the funny stuff" scsi comands are
used for outside of actual data transfers.

Re: [patch 4/6] ps3: Disk Storage Driver

[Christoph Hellwig]

On Fri, Jun 15, 2007 at 04:08:58PM -0700, David Miller wrote:
> That's not gonna work, it's a totally different model.
> 
> I have a predefined protocol over hypervisor provided "channels" and
> page flipping also done by the hypervisor for the bulk data transfer.
> For the client side I cannot change the hypervisor nor the server
> speaking on the other end.  And when I do write a server I do want
> it to be able to speak to all of the existing clients.
> 
> There's SCSI command pass through as well, as I keep mentioning as
> it's an important reason I don't want to go with any of the non-SCSI
> solutions (other than perhaps ATA) being suggested.

A SCSI pass through is of course perfectly fine.  If you have a separate
block passthrough that has additional magic a separate block driver is
the way to go because it actually is simpler than a scsi driver decoding
command blocks and translating them to deep magic.  The ps3 storage
drivers this thread discussed are a good example for that.  We now
have a very nice and simple disk, scsi and flash chardev driver each
that don't include abstractions layers and cruft.  Combine that with
their initial scsi layer driver that was full of internal dispatches
because each of these device types speaks a completely different command
set.

Re: [patch 4/6] ps3: Disk Storage Driver

[David Miller]

From: Christoph Hellwig <hch@lst.de>
Date: Tue, 19 Jun 2007 07:56:29 +0200

> A SCSI pass through is of course perfectly fine.  If you have a separate
> block passthrough that has additional magic a separate block driver is
> the way to go because it actually is simpler than a scsi driver decoding
> command blocks and translating them to deep magic.  The ps3 storage
> drivers this thread discussed are a good example for that.  We now
> have a very nice and simple disk, scsi and flash chardev driver each
> that don't include abstractions layers and cruft.  Combine that with
> their initial scsi layer driver that was full of internal dispatches
> because each of these device types speaks a completely different command
> set.

That's how I'm currently writing my virtual disk client driver
for the Sun LDOMS stuff, as a block device.

The remaining issues are the partitioning (which we're discussing in
another thread) and how to export the scsi passthru support in such a
non-scsi block driver.

The main disk I/O block read and write is done using descriptors
sent to the disk server.  SCSI pass-through is provided (optionally)
so that disk analysis tools can do things like MODE_SENSE on the
disk.

Re: [patch 4/6] ps3: Disk Storage Driver

[Christoph Hellwig]

On Mon, Jun 18, 2007 at 11:07:31PM -0700, David Miller wrote:
> The main disk I/O block read and write is done using descriptors
> sent to the disk server.  SCSI pass-through is provided (optionally)
> so that disk analysis tools can do things like MODE_SENSE on the
> disk.

SG_IO can easily be implemented for block devices.  See cciss in
current mainline for an example.

Re: [patch 4/6] ps3: Disk Storage Driver

[Alan Cox]

> That sounds like a good idea for my virtual I/O case on
> Niagara too actually :-)

It was actually meant semi-seriously in that drivers/ata sits on top of
the abstract parts of drivers/scsi which James keeps asking for someone to
get split properly off and which would sort all these out.

> Another quirk I have to deal with is that under LDOMs you
> can export full disks and also just slices.  So I'll have
> to get down into the partition machinery to support that
> somehow.

If your slices don't fit the PC worldview you might want to look at
dmraid and just use device mapper to handle them. It is a lot more
flexible and we could actually bin all our partition code and use this
but for back compatibility.

Alan

Re: [patch 4/6] ps3: Disk Storage Driver

[Geert Uytterhoeven]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1664 bytes --]

On Fri, 15 Jun 2007, David Miller wrote:
> From: Alan Cox <alan@lxorguk.ukuu.org.uk>
> Date: Fri, 15 Jun 2007 17:15:45 +0100
> 
> > > > Any particular reason why this is done as a separate block device driver
> > > > rather than as SCSI?
> > > 
> > > Because no new fake SCSI drivers are accepted anymore.
> > 
> > Where did drivers/ata come from ;)
> > 
> > How about making it a fake ata driver if James is being fussy 8)
> 
> That sounds like a good idea for my virtual I/O case on
> Niagara too actually :-)
> 
> Another quirk I have to deal with is that under LDOMs you
> can export full disks and also just slices.  So I'll have
> to get down into the partition machinery to support that
> somehow.

The PS3 hypervisor also splits each device in regions (cfr. partitions), with
different access rights depending the OS that runs on top of it. The old
virtual SCSI driver treated each region as a separate LUN.

As currently only one region can be accessed by Linux anyway, I didn't bother
supporting multiple accessible regions in the new storage drivers.

With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center 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/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

Re: [patch 4/6] ps3: Disk Storage Driver

[Christoph Hellwig]

On Fri, Jun 15, 2007 at 02:19:17PM -0700, David Miller wrote:
> Another quirk I have to deal with is that under LDOMs you
> can export full disks and also just slices.  So I'll have
> to get down into the partition machinery to support that
> somehow.

What's the problem with that?  Partition machinery is perfectly
fine with just getting a raw disk without a partition table,
and it obviously doesn't care if the raw disk it sees is part
of a partition on the storage server.

Re: [patch 4/6] ps3: Disk Storage Driver

[David Miller]

From: Christoph Hellwig <hch@lst.de>
Date: Tue, 19 Jun 2007 07:53:58 +0200

> On Fri, Jun 15, 2007 at 02:19:17PM -0700, David Miller wrote:
> > Another quirk I have to deal with is that under LDOMs you
> > can export full disks and also just slices.  So I'll have
> > to get down into the partition machinery to support that
> > somehow.
> 
> What's the problem with that?  Partition machinery is perfectly
> fine with just getting a raw disk without a partition table,
> and it obviously doesn't care if the raw disk it sees is part
> of a partition on the storage server.

Applications care :-)

One way of managing the partition table is by asking the storage
server for it.  It supports giving back something that looks
like the Sun partition table layout, and also something that's
EFI based.

But the client can ignore that and read/write the partition
table itself.

What I really care about is what will work transparently for existing
userspace.  In particular, distribution installers and existing tools
like fdisk.

When a slice it being exported, it's not being exported like that so
that the client can just spam a disk label into it, it's for exporting
one slice.  So we might want (using the scsi naming as an arbitrary
example case) /dev/sda1 to be created but not /dev/sda

How exactly do you envision this kind of thing working?

Supporting the disk-server-provided partition table management
means providing ioctls() or whatever interface to get/set
the table.  I had one idea to intercept reads/writes to the
first block(s) and translating those into partition management
commands to the disk server, but that is just too ugly to live. :-)

Re: [patch 4/6] ps3: Disk Storage Driver

[Geert Uytterhoeven]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1244 bytes --]

On Mon, 18 Jun 2007, David Miller wrote:
> What I really care about is what will work transparently for existing
> userspace.  In particular, distribution installers and existing tools
> like fdisk.
> 
> When a slice it being exported, it's not being exported like that so
> that the client can just spam a disk label into it, it's for exporting
> one slice.  So we might want (using the scsi naming as an arbitrary
> example case) /dev/sda1 to be created but not /dev/sda
> 
> How exactly do you envision this kind of thing working?

How do distribution installers handle USB disks and memory cards? Some tend
to be partitioned, some don't.

With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center 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/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

Re: [patch 4/6] ps3: Disk Storage Driver

[David Miller]

From: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
Date: Fri, 15 Jun 2007 16:43:05 +0200 (CEST)

> On Fri, 15 Jun 2007, David Woodhouse wrote:
> > On Fri, 2007-06-15 at 13:39 +0200, Geert Uytterhoeven wrote:
> > > Add a Disk Storage Driver for the PS3:
> > >   - Implemented as a block device driver with a dynamic major
> > >   - Disk names (and partitions) are of the format ps3d%c(%u)
> > >   - Uses software scatter-gather with a 64 KiB bounce buffer as the hypervisor
> > >     doesn't support scatter-gather
> > 
> > Any particular reason why this is done as a separate block device driver
> > rather than as SCSI?
> 
> Because no new fake SCSI drivers are accepted anymore.

I'm strongly divided on this issue as I'm about to hit the same exact
thing for Sun Logical Domains on sparc64 Niagara systems.

In fact the interface I get to use allows SCSI commands to be sent
pass-through to the device, even though the basic virtual I/O API is
purely block I/O based.

It's senseless to make people build new major/minor numbers for
all these new quirky storage drivers.  People have to add support
for the new major number to installers and all kinds of other
tools.

If the SCSI guys were smart, there would be a totally generic helper
layer that allows anyone to hook into the SCSI layer as a virtual SCSI
disk provider in like 10 lines of code. :-)

Re: [patch 4/6] ps3: Disk Storage Driver

[Jeff Garzik]

David Miller wrote:
> From: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>

>> Because no new fake SCSI drivers are accepted anymore.

If you can access the firmware/hypervisor code, have that provide SCSI. 
  Then the Linux driver piece is easy, obvious and flexible SCSI, and is 
not "fake."


> If the SCSI guys were smart, there would be a totally generic helper
> layer that allows anyone to hook into the SCSI layer as a virtual SCSI
> disk provider in like 10 lines of code. :-)

The SCSI target code gives you the capability to do that (the SCSI 
transport part, the virtual storage part is easy from there).

	Jeff

Re: [patch 4/6] ps3: Disk Storage Driver

[David Miller]

From: Jeff Garzik <jeff@garzik.org>
Date: Fri, 15 Jun 2007 17:28:31 -0400

> David Miller wrote:
> > If the SCSI guys were smart, there would be a totally generic helper
> > layer that allows anyone to hook into the SCSI layer as a virtual SCSI
> > disk provider in like 10 lines of code. :-)
> 
> The SCSI target code gives you the capability to do that (the SCSI 
> transport part, the virtual storage part is easy from there).

I don't get to control the server side in my case, I'm a client
talking to something with a pre-determined interface.

Re: [patch 4/6] ps3: Disk Storage Driver

[Jeff Garzik]

David Miller wrote:
> It's senseless to make people build new major/minor numbers for
> all these new quirky storage drivers.  People have to add support
> for the new major number to installers and all kinds of other
> tools.


FWIW that's why libata followed the path it did:  faking SCSI required 
the least amount of code for the maximal amount of existing storage 
driver and platform installer support.

Well, that and the fact that ATAPI is really SCSI, but that's offtopic 
for this thread.  :)

	Jeff

Re: [patch 4/6] ps3: Disk Storage Driver

[David Miller]

From: Jeff Garzik <jeff@garzik.org>
Date: Fri, 15 Jun 2007 17:50:50 -0400

> FWIW that's why libata followed the path it did:  faking SCSI required 
> the least amount of code for the maximal amount of existing storage 
> driver and platform installer support.
> 
> Well, that and the fact that ATAPI is really SCSI, but that's offtopic 
> for this thread.  :)

Yep I absolutely understand that, and for my case as well it's
nearly always SCSI underneath that's why you can send SCSI
commands passthru to the virtual disk server if you like.

Re: [patch 4/6] ps3: Disk Storage Driver

[Christoph Hellwig]

On Fri, Jun 15, 2007 at 01:39:23PM +0200, Geert Uytterhoeven wrote:
> From: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
> 
> Add a Disk Storage Driver for the PS3:
>   - Implemented as a block device driver with a dynamic major
>   - Disk names (and partitions) are of the format ps3d%c(%u)
>   - Uses software scatter-gather with a 64 KiB bounce buffer as the hypervisor
>     doesn't support scatter-gather

Looks good to me.  Only nitpicks are:

 - ps3disk_priv should probably be an inline function instead of a macro
 - ps3disk_open is not needed at all and should be removed completely.
 - please remove PS3DISK_MAJOR - and just pass 0 to register_blkdev.
   Passing 0 to get an unassigned dev_t range is the normal convention
   and giving it a suprising symbolic names makes reading the code a
   little harder.  
 - put_disk in ps3disk_remove should be done after you finished tearing
   down the device, not before
 - calling free_irq directly followed by request_irq in ps3disk_probe
   looks rather dumb.  I'd rather move the request_irq out of
   ps3stor_setup into the low-level driver.

Re: [patch 4/6] ps3: Disk Storage Driver

[Geert Uytterhoeven]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1266 bytes --]

On Tue, 19 Jun 2007, Christoph Hellwig wrote:
> On Fri, Jun 15, 2007 at 01:39:23PM +0200, Geert Uytterhoeven wrote:
> > From: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
> > 
> > Add a Disk Storage Driver for the PS3:
> >   - Implemented as a block device driver with a dynamic major
> >   - Disk names (and partitions) are of the format ps3d%c(%u)
> >   - Uses software scatter-gather with a 64 KiB bounce buffer as the hypervisor
> >     doesn't support scatter-gather
> 
> Looks good to me.  Only nitpicks are:
> 
>  - ps3disk_priv should probably be an inline function instead of a macro

I used a macro because you can do

    ps3disk_pri(dev) = ...;

With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center 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/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

Re: [patch 4/6] ps3: Disk Storage Driver

[Christoph Hellwig]

On Tue, Jun 19, 2007 at 02:51:25PM +0200, Geert Uytterhoeven wrote:
> On Tue, 19 Jun 2007, Christoph Hellwig wrote:
> > On Fri, Jun 15, 2007 at 01:39:23PM +0200, Geert Uytterhoeven wrote:
> > > From: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
> > > 
> > > Add a Disk Storage Driver for the PS3:
> > >   - Implemented as a block device driver with a dynamic major
> > >   - Disk names (and partitions) are of the format ps3d%c(%u)
> > >   - Uses software scatter-gather with a 64 KiB bounce buffer as the hypervisor
> > >     doesn't support scatter-gather
> > 
> > Looks good to me.  Only nitpicks are:
> > 
> >  - ps3disk_priv should probably be an inline function instead of a macro
> 
> I used a macro because you can do
> 
>     ps3disk_pri(dev) = ...;

I'm not exactly a fan of macros used as lvalues, but if you really
want this it can go in.

[patch 5/6] ps3: ROM Storage Driver

[Geert Uytterhoeven]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 17842 bytes --]

From: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>

Add a CD/DVD/BD Storage Driver for the PS3:
  - Implemented as a SCSI device driver
  - Uses software scatter-gather with a 64 KiB bounce buffer as the hypervisor
    doesn't support scatter-gather

CC: Geoff Levand <geoffrey.levand@am.sony.com>
Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
---
Changes since previous submission:
      o Don't use `default y' in Kconfig
      o Add missing #include <linux/highmem.h>
      o Remove ps3rom_private.scsi_done, use scsi_cmnd.scsi_done instead
      o Use scsi_device.host.hostdata
      o Remove empty ps3rom_slave_{alloc,destroy}()
      o Kill superfluous test for command in progress
      o Move scsi_host_put() last in cleanup sequence
      o Remove scsi_command(), use scsi_print_command() for debugging
      o scsi_cmnd.use_sg is always > 0 these days
      o Allocate struct ps3rom_private using scsi_host_alloc()
      o Remove the thread for handling requests
      o Remove unused position parameter enum
      o Remove unused NA_PROTO and DIR_NA
      o Derive buffer length, data direction, and transfer protocol from the
	struct scsi_command, instead of using a big switch() statement
      o Kill superfluous spinlock
      o Remove manual request sense, as modern hypervisors always do auto sense
      o Pass all SCSI commands to the hypervisor as ATAPI commands, except for
        READ_*/WRITE_*
      o Don't print errors for SCSI commands that are not allowed for an
        Other OS by the hypervisor
      o Remove superfluous tests for data directions in
        {fill_from,fetch_to}_dev_buffer()
      o Handle errors in {fill_from,fetch_to}_dev_buffer()
      o Reorder routines
      o Manually inline ps3rom_send_atapi_command()
      o Fix all FIXMEs

 arch/powerpc/platforms/ps3/Kconfig |   10 
 drivers/scsi/Makefile              |    1 
 drivers/scsi/ps3rom.c              |  568 +++++++++++++++++++++++++++++++++++++
 3 files changed, 579 insertions(+)

--- a/arch/powerpc/platforms/ps3/Kconfig
+++ b/arch/powerpc/platforms/ps3/Kconfig
@@ -112,4 +112,14 @@ config PS3_DISK
 	  This support is required to access the PS3 hard disk.
 	  In general, all users will say Y or M.
 
+config PS3_ROM
+	tristate "PS3 ROM Storage Driver"
+	depends on PPC_PS3 && BLK_DEV_SR
+	select PS3_STORAGE
+	help
+	  Include support for the PS3 ROM Storage.
+
+	  This support is required to access the PS3 BD/DVD/CD-ROM drive.
+	  In general, all users will say Y or M.
+
 endmenu
--- a/drivers/scsi/Makefile
+++ b/drivers/scsi/Makefile
@@ -132,6 +132,7 @@ obj-$(CONFIG_SCSI_IBMVSCSI)	+= ibmvscsi/
 obj-$(CONFIG_SCSI_IBMVSCSIS)	+= ibmvscsi/
 obj-$(CONFIG_SCSI_HPTIOP)	+= hptiop.o
 obj-$(CONFIG_SCSI_STEX)		+= stex.o
+obj-$(CONFIG_PS3_ROM)		+= ps3rom.o
 
 obj-$(CONFIG_ARM)		+= arm/
 
--- /dev/null
+++ b/drivers/scsi/ps3rom.c
@@ -0,0 +1,568 @@
+/*
+ * PS3 ROM Storage Driver
+ *
+ * Copyright (C) 2007 Sony Computer Entertainment Inc.
+ * Copyright 2007 Sony Corp.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published
+ * by the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include <linux/cdrom.h>
+#include <linux/highmem.h>
+#include <linux/interrupt.h>
+#include <linux/kthread.h>
+
+#include <scsi/scsi.h>
+#include <scsi/scsi_cmnd.h>
+#include <scsi/scsi_dbg.h>
+#include <scsi/scsi_device.h>
+#include <scsi/scsi_host.h>
+
+#include <asm/lv1call.h>
+#include <asm/ps3stor.h>
+
+
+#define DEVICE_NAME			"ps3rom"
+
+#define BOUNCE_SIZE			(64*1024)
+
+#define PS3ROM_MAX_SECTORS		(BOUNCE_SIZE / CD_FRAMESIZE)
+
+#define LV1_STORAGE_SEND_ATAPI_COMMAND	(1)
+
+
+struct ps3rom_private {
+	struct ps3_storage_device *dev;
+	struct scsi_cmnd *cmd;
+};
+#define ps3rom_priv(dev)	((dev)->sbd.core.driver_data)
+
+struct lv1_atapi_cmnd_block {
+	u8	pkt[32];	/* packet command block           */
+	u32	pktlen;		/* should be 12 for ATAPI 8020    */
+	u32	blocks;
+	u32	block_size;
+	u32	proto;		/* transfer mode                  */
+	u32	in_out;		/* transfer direction             */
+	u64	buffer;		/* parameter except command block */
+	u32	arglen;		/* length above                   */
+};
+
+enum lv1_atapi_proto {
+	NON_DATA_PROTO     = 0,
+	PIO_DATA_IN_PROTO  = 1,
+	PIO_DATA_OUT_PROTO = 2,
+	DMA_PROTO = 3
+};
+
+enum lv1_atapi_in_out {
+	DIR_WRITE = 0, /* memory -> device */
+	DIR_READ = 1 /* device -> memory */
+};
+
+
+static int ps3rom_slave_configure(struct scsi_device *scsi_dev)
+{
+	struct ps3rom_private *priv;
+	struct ps3_storage_device *dev;
+
+	priv = (struct ps3rom_private *)scsi_dev->host->hostdata;
+	dev = priv->dev;
+	dev_dbg(&dev->sbd.core, "%s:%u: id %u, lun %u, channel %u\n", __func__,
+		__LINE__, scsi_dev->id, scsi_dev->lun, scsi_dev->channel);
+
+	/*
+	 * ATAPI SFF8020 devices use MODE_SENSE_10,
+	 * so we can prohibit MODE_SENSE_6
+	 */
+	scsi_dev->use_10_for_ms = 1;
+
+	return 0;
+}
+
+/*
+ * copy data from device into scatter/gather buffer
+ */
+static int fill_from_dev_buffer(struct scsi_cmnd *cmd, const void *buf)
+{
+	int k, req_len, act_len, len, active;
+	void *kaddr;
+	struct scatterlist *sgpnt;
+	unsigned int buflen;
+
+	buflen = cmd->request_bufflen;
+	if (!buflen)
+		return 0;
+
+	if (!cmd->request_buffer)
+		return -1;
+
+	sgpnt = cmd->request_buffer;
+	active = 1;
+	for (k = 0, req_len = 0, act_len = 0; k < cmd->use_sg; ++k, ++sgpnt) {
+		if (active) {
+			kaddr = kmap_atomic(sgpnt->page, KM_USER0);
+			if (!kaddr)
+				return -1;
+			len = sgpnt->length;
+			if ((req_len + len) > buflen) {
+				active = 0;
+				len = buflen - req_len;
+			}
+			memcpy(kaddr + sgpnt->offset, buf + req_len, len);
+			kunmap_atomic(kaddr, KM_USER0);
+			act_len += len;
+		}
+		req_len += sgpnt->length;
+	}
+	cmd->resid = req_len - act_len;
+	return 0;
+}
+
+/*
+ * copy data from scatter/gather into device's buffer
+ */
+static int fetch_to_dev_buffer(struct scsi_cmnd *cmd, void *buf)
+{
+	int k, req_len, len, fin;
+	void *kaddr;
+	struct scatterlist *sgpnt;
+	unsigned int buflen;
+
+	buflen = cmd->request_bufflen;
+	if (!buflen)
+		return 0;
+
+	if (!cmd->request_buffer)
+		return -1;
+
+	sgpnt = cmd->request_buffer;
+	for (k = 0, req_len = 0, fin = 0; k < cmd->use_sg; ++k, ++sgpnt) {
+		kaddr = kmap_atomic(sgpnt->page, KM_USER0);
+		if (!kaddr)
+			return -1;
+		len = sgpnt->length;
+		if ((req_len + len) > buflen) {
+			len = buflen - req_len;
+			fin = 1;
+		}
+		memcpy(buf + req_len, kaddr + sgpnt->offset, len);
+		kunmap_atomic(kaddr, KM_USER0);
+		if (fin)
+			return req_len + len;
+		req_len += sgpnt->length;
+	}
+	return req_len;
+}
+
+static int ps3rom_atapi_request(struct ps3_storage_device *dev,
+				struct scsi_cmnd *cmd)
+{
+	struct lv1_atapi_cmnd_block atapi_cmnd;
+	unsigned char opcode = cmd->cmnd[0];
+	int res;
+	u64 lpar;
+
+	dev_dbg(&dev->sbd.core, "%s:%u: send ATAPI command 0x%02x\n", __func__,
+		__LINE__, opcode);
+
+	memset(&atapi_cmnd, 0, sizeof(struct lv1_atapi_cmnd_block));
+	memcpy(&atapi_cmnd.pkt, cmd->cmnd, 12);
+	atapi_cmnd.pktlen = 12;
+	atapi_cmnd.block_size = 1; /* transfer size is block_size * blocks */
+	atapi_cmnd.blocks = atapi_cmnd.arglen = cmd->request_bufflen;
+	atapi_cmnd.buffer = dev->bounce_lpar;
+
+	switch (cmd->sc_data_direction) {
+	case DMA_FROM_DEVICE:
+		if (cmd->request_bufflen >= CD_FRAMESIZE)
+			atapi_cmnd.proto = DMA_PROTO;
+		else
+			atapi_cmnd.proto = PIO_DATA_IN_PROTO;
+		atapi_cmnd.in_out = DIR_READ;
+		break;
+
+	case DMA_TO_DEVICE:
+		if (cmd->request_bufflen >= CD_FRAMESIZE)
+			atapi_cmnd.proto = DMA_PROTO;
+		else
+			atapi_cmnd.proto = PIO_DATA_OUT_PROTO;
+		atapi_cmnd.in_out = DIR_WRITE;
+		res = fetch_to_dev_buffer(cmd, dev->bounce_buf);
+		if (res < 0)
+			return DID_ERROR << 16;
+		break;
+
+	default:
+		atapi_cmnd.proto = NON_DATA_PROTO;
+		break;
+	}
+
+	lpar = ps3_mm_phys_to_lpar(__pa(&atapi_cmnd));
+	res = lv1_storage_send_device_command(dev->sbd.dev_id,
+					      LV1_STORAGE_SEND_ATAPI_COMMAND,
+					      lpar, sizeof(atapi_cmnd),
+					      atapi_cmnd.buffer,
+					      atapi_cmnd.arglen, &dev->tag);
+	if (res == LV1_DENIED_BY_POLICY) {
+		dev_dbg(&dev->sbd.core,
+			"%s:%u: ATAPI command 0x%02x denied by policy\n",
+			__func__, __LINE__, opcode);
+		return DID_ERROR << 16;
+	}
+
+	if (res) {
+		dev_err(&dev->sbd.core,
+			"%s:%u: ATAPI command 0x%02x failed %d\n", __func__,
+			__LINE__, opcode, res);
+		return DID_ERROR << 16;
+	}
+
+	return 0;
+}
+
+static inline unsigned int srb6_lba(const struct scsi_cmnd *cmd)
+{
+	/* LUN is always zero */
+	return cmd->cmnd[1] << 16 | cmd->cmnd[2] << 8 | cmd->cmnd[3];
+}
+
+static inline unsigned int srb6_len(const struct scsi_cmnd *cmd)
+{
+	return cmd->cmnd[4];
+}
+
+static inline unsigned int srb10_lba(const struct scsi_cmnd *cmd)
+{
+	return cmd->cmnd[2] << 24 | cmd->cmnd[3] << 16 | cmd->cmnd[4] << 8 |
+	       cmd->cmnd[5];
+}
+
+static inline unsigned int srb10_len(const struct scsi_cmnd *cmd)
+{
+	return cmd->cmnd[7] << 8 | cmd->cmnd[8];
+}
+
+static int ps3rom_read_request(struct ps3_storage_device *dev,
+			       struct scsi_cmnd *cmd, u32 start_sector,
+			       u32 sectors)
+{
+	int res;
+
+	dev_dbg(&dev->sbd.core, "%s:%u: read %u sectors starting at %u\n",
+		__func__, __LINE__, sectors, start_sector);
+
+	res = lv1_storage_read(dev->sbd.dev_id,
+			       dev->regions[dev->region_idx].id, start_sector,
+			       sectors, 0, dev->bounce_lpar, &dev->tag);
+	if (res) {
+		dev_err(&dev->sbd.core, "%s:%u: read failed %d\n", __func__,
+			__LINE__, res);
+		return DID_ERROR << 16;
+	}
+
+	return 0;
+}
+
+static int ps3rom_write_request(struct ps3_storage_device *dev,
+				struct scsi_cmnd *cmd, u32 start_sector,
+				u32 sectors)
+{
+	int res;
+
+	dev_dbg(&dev->sbd.core, "%s:%u: write %u sectors starting at %u\n",
+		__func__, __LINE__, sectors, start_sector);
+
+	res = fetch_to_dev_buffer(cmd, dev->bounce_buf);
+	if (res < 0)
+		return DID_ERROR << 16;
+
+	res = lv1_storage_write(dev->sbd.dev_id,
+				dev->regions[dev->region_idx].id, start_sector,
+				sectors, 0, dev->bounce_lpar, &dev->tag);
+	if (res) {
+		dev_err(&dev->sbd.core, "%s:%u: write failed %d\n", __func__,
+			__LINE__, res);
+		return DID_ERROR << 16;
+	}
+
+	return 0;
+}
+
+static int ps3rom_queuecommand(struct scsi_cmnd *cmd,
+			       void (*done)(struct scsi_cmnd *))
+{
+	struct ps3rom_private *priv;
+	struct ps3_storage_device *dev;
+	unsigned char opcode;
+	int res;
+
+#ifdef DEBUG
+	scsi_print_command(cmd);
+#endif
+
+	priv = (struct ps3rom_private *)cmd->device->host->hostdata;
+	dev = priv->dev;
+	priv->cmd = cmd;
+	cmd->scsi_done = done;
+
+	opcode = cmd->cmnd[0];
+	/*
+	 * While we can submit READ/WRITE SCSI commands as ATAPI commands,
+	 * it's recommended to use lv1_storage_{read,write}() instead
+	 */
+	switch (opcode) {
+	case READ_6:
+		res = ps3rom_read_request(dev, cmd, srb6_lba(cmd),
+					  srb6_len(cmd));
+		break;
+
+	case READ_10:
+		res = ps3rom_read_request(dev, cmd, srb10_lba(cmd),
+					  srb10_len(cmd));
+		break;
+
+	case WRITE_6:
+		res = ps3rom_write_request(dev, cmd, srb6_lba(cmd),
+					   srb6_len(cmd));
+		break;
+
+	case WRITE_10:
+		res = ps3rom_write_request(dev, cmd, srb10_lba(cmd),
+					   srb10_len(cmd));
+		break;
+
+	default:
+		res = ps3rom_atapi_request(dev, cmd);
+		break;
+	}
+
+	if (res) {
+		memset(cmd->sense_buffer, 0, SCSI_SENSE_BUFFERSIZE);
+		cmd->result = res;
+		cmd->sense_buffer[0] = 0x70;
+		cmd->sense_buffer[2] = ILLEGAL_REQUEST;
+		priv->cmd = NULL;
+		cmd->scsi_done(cmd);
+	}
+
+	return 0;
+}
+
+static int decode_lv1_status(u64 status, unsigned char *sense_key,
+			     unsigned char *asc, unsigned char *ascq)
+{
+	if (((status >> 24) & 0xff) != SAM_STAT_CHECK_CONDITION)
+		return -1;
+
+	*sense_key = (status >> 16) & 0xff;
+	*asc       = (status >>  8) & 0xff;
+	*ascq      =  status        & 0xff;
+	return 0;
+}
+
+static irqreturn_t ps3rom_interrupt(int irq, void *data)
+{
+	struct ps3_storage_device *dev = data;
+	struct Scsi_Host *host;
+	struct ps3rom_private *priv;
+	struct scsi_cmnd *cmd;
+	int res;
+	u64 tag, status;
+	unsigned char sense_key, asc, ascq;
+
+	res = lv1_storage_get_async_status(dev->sbd.dev_id, &tag, &status);
+	/*
+	 * status = -1 may mean that ATAPI transport completed OK, but
+	 * ATAPI command itself resulted CHECK CONDITION
+	 * so, upper layer should issue REQUEST_SENSE to check the sense data
+	 */
+
+	if (tag != dev->tag)
+		dev_err(&dev->sbd.core,
+			"%s:%u: tag mismatch, got %lx, expected %lx\n",
+			__func__, __LINE__, tag, dev->tag);
+
+	if (res) {
+		dev_err(&dev->sbd.core, "%s:%u: res=%d status=0x%lx\n",
+			__func__, __LINE__, res, status);
+		return IRQ_HANDLED;
+	}
+
+	host = ps3rom_priv(dev);
+	priv = (struct ps3rom_private *)host->hostdata;
+	cmd = priv->cmd;
+
+	if (!status) {
+		/* OK, completed */
+		if (cmd->sc_data_direction == DMA_FROM_DEVICE) {
+			res = fill_from_dev_buffer(cmd, dev->bounce_buf);
+			if (res) {
+				cmd->result = DID_ERROR << 16;
+				goto done;
+			}
+		}
+		cmd->result = DID_OK << 16;
+		goto done;
+	}
+
+	if (cmd->cmnd[0] == REQUEST_SENSE) {
+		/* SCSI spec says request sense should never get error */
+		dev_err(&dev->sbd.core, "%s:%u: end error without autosense\n",
+			__func__, __LINE__);
+		cmd->result = DID_ERROR << 16 | SAM_STAT_CHECK_CONDITION;
+		goto done;
+	}
+
+	if (decode_lv1_status(status, &sense_key, &asc, &ascq)) {
+		cmd->result = DID_ERROR << 16;
+		goto done;
+	}
+
+	cmd->sense_buffer[0]  = 0x70;
+	cmd->sense_buffer[2]  = sense_key;
+	cmd->sense_buffer[7]  = 16 - 6;
+	cmd->sense_buffer[12] = asc;
+	cmd->sense_buffer[13] = ascq;
+	cmd->result = SAM_STAT_CHECK_CONDITION;
+
+done:
+	priv->cmd = NULL;
+	cmd->scsi_done(cmd);
+	return IRQ_HANDLED;
+}
+
+static struct scsi_host_template ps3rom_host_template = {
+	.name =			DEVICE_NAME,
+	.slave_configure =	ps3rom_slave_configure,
+	.queuecommand =		ps3rom_queuecommand,
+	.can_queue =		1,
+	.this_id =		7,
+	.sg_tablesize =		SG_ALL,
+	.cmd_per_lun =		1,
+	.emulated =             1,		/* only sg driver uses this */
+	.max_sectors =		PS3ROM_MAX_SECTORS,
+	.use_clustering =	ENABLE_CLUSTERING,
+	.module =		THIS_MODULE,
+};
+
+
+static int __devinit ps3rom_probe(struct ps3_system_bus_device *_dev)
+{
+	struct ps3_storage_device *dev = to_ps3_storage_device(&_dev->core);
+	int error;
+	struct Scsi_Host *host;
+	struct ps3rom_private *priv;
+
+	if (dev->blk_size != CD_FRAMESIZE) {
+		dev_err(&dev->sbd.core,
+			"%s:%u: cannot handle block size %lu\n", __func__,
+			__LINE__, dev->blk_size);
+		return -EINVAL;
+	}
+
+	dev->bounce_size = BOUNCE_SIZE;
+	dev->bounce_buf = kmalloc(BOUNCE_SIZE, GFP_DMA);
+	if (!dev->bounce_buf) {
+		return -ENOMEM;
+	}
+
+	error = ps3stor_setup(dev);
+	if (error)
+		goto fail_free_bounce;
+
+	/* override the interrupt handler */
+	free_irq(dev->irq, dev);
+	error = request_irq(dev->irq, ps3rom_interrupt, IRQF_DISABLED,
+			    dev->sbd.core.driver->name, dev);
+	if (error) {
+		dev_err(&dev->sbd.core, "%s:%u: request_irq failed %d\n",
+			__func__, __LINE__, error);
+		goto fail_teardown;
+	}
+
+	host = scsi_host_alloc(&ps3rom_host_template,
+			       sizeof(struct ps3rom_private));
+	if (!host) {
+		dev_err(&dev->sbd.core, "%s:%u: scsi_host_alloc failed\n",
+			__func__, __LINE__);
+		goto fail_teardown;
+	}
+
+	priv = (struct ps3rom_private *)host->hostdata;
+	ps3rom_priv(dev) = host;
+	priv->dev = dev;
+
+	/* One device/LUN per SCSI bus */
+	host->max_id = 1;
+	host->max_lun = 1;
+
+	error = scsi_add_host(host, &dev->sbd.core);
+	if (error) {
+		dev_err(&dev->sbd.core, "%s:%u: scsi_host_alloc failed %d\n",
+			__func__, __LINE__, error);
+		error = -ENODEV;
+		goto fail_host_put;
+	}
+
+	scsi_scan_host(host);
+	return 0;
+
+fail_host_put:
+	scsi_host_put(host);
+fail_teardown:
+	ps3stor_teardown(dev);
+fail_free_bounce:
+	kfree(dev->bounce_buf);
+	return error;
+}
+
+static int ps3rom_remove(struct ps3_system_bus_device *_dev)
+{
+	struct ps3_storage_device *dev = to_ps3_storage_device(&_dev->core);
+	struct Scsi_Host *host = ps3rom_priv(dev);
+
+	scsi_remove_host(host);
+	ps3stor_teardown(dev);
+	kfree(dev->bounce_buf);
+	scsi_host_put(host);
+	return 0;
+}
+
+static struct ps3_system_bus_driver ps3rom = {
+	.match_id	= PS3_MATCH_ID_STOR_ROM,
+	.core.name	= DEVICE_NAME,
+	.core.owner	= THIS_MODULE,
+	.probe		= ps3rom_probe,
+	.remove		= ps3rom_remove
+};
+
+
+static int __init ps3rom_init(void)
+{
+	return ps3_system_bus_driver_register(&ps3rom);
+}
+
+static void __exit ps3rom_exit(void)
+{
+	ps3_system_bus_driver_unregister(&ps3rom);
+}
+
+module_init(ps3rom_init);
+module_exit(ps3rom_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("PS3 ROM Storage Driver");
+MODULE_AUTHOR("Sony Corporation");
+MODULE_ALIAS(PS3_MODULE_ALIAS_STOR_ROM);

-- 
With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center 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/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

Re: [patch 5/6] ps3: ROM Storage Driver

[Milton Miller]

> From: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
>
> Add a CD/DVD/BD Storage Driver for the PS3:
>   - Implemented as a SCSI device driver
>   - Uses software scatter-gather with a 64 KiB bounce buffer as the 
> hypervisor
>     doesn't support scatter-gather
>


>
> +config PS3_ROM
> +	tristate "PS3 ROM Storage Driver"
> +	depends on PPC_PS3 && BLK_DEV_SR
> +	select PS3_STORAGE
> +	help
> +	  Include support for the PS3 ROM Storage.
> +
> +	  This support is required to access the PS3 BD/DVD/CD-ROM drive.
> +	  In general, all users will say Y or M.
> +


When I think ROM I usually dont' start with optical media.

Have you condered calling this the optical driver?

Or at least putting BD?DVD?CD-ROM on the prompt.


Why does it depend on BLK_DEV_SR?

...
> +
> +#include <linux/cdrom.h>
> +#include <linux/highmem.h>
> +#include <linux/interrupt.h>
> +#include <linux/kthread.h>
> +
> +#include <scsi/scsi.h>
> +#include <scsi/scsi_cmnd.h>
> +#include <scsi/scsi_dbg.h>
> +#include <scsi/scsi_device.h>
> +#include <scsi/scsi_host.h>
> +
> +#include <asm/lv1call.h>
> +#include <asm/ps3stor.h>
> +
> +
> +#define DEVICE_NAME			"ps3rom"
> +
> +#define BOUNCE_SIZE			(64*1024)
> +
> +#define PS3ROM_MAX_SECTORS		(BOUNCE_SIZE / CD_FRAMESIZE)
> +

the above describes the device

> +#define LV1_STORAGE_SEND_ATAPI_COMMAND	(1)
> +
>
while this one is part of the hypervisor ABI.


> +
> +struct ps3rom_private {
> +	struct ps3_storage_device *dev;
> +	struct scsi_cmnd *cmd;
> +};
> +#define ps3rom_priv(dev)	((dev)->sbd.core.driver_data)
>

The above is driver defined.

> +
> +struct lv1_atapi_cmnd_block {
> +	u8	pkt[32];	/* packet command block           */
> +	u32	pktlen;		/* should be 12 for ATAPI 8020    */
> +	u32	blocks;
> +	u32	block_size;
> +	u32	proto;		/* transfer mode                  */
> +	u32	in_out;		/* transfer direction             */
> +	u64	buffer;		/* parameter except command block */
> +	u32	arglen;		/* length above                   */
> +};
> +

Its minor, but I'd probably put the define down here with the rest of 
the ABI contants vs all defines then structs then enums.

> +enum lv1_atapi_proto {
> +	NON_DATA_PROTO     = 0,
> +	PIO_DATA_IN_PROTO  = 1,
> +	PIO_DATA_OUT_PROTO = 2,
> +	DMA_PROTO = 3
> +};
> +
> +enum lv1_atapi_in_out {
> +	DIR_WRITE = 0, /* memory -> device */
> +	DIR_READ = 1 /* device -> memory */
> +};
> +
>


...


> +
> +static int __devinit ps3rom_probe(struct ps3_system_bus_device *_dev)
> +{
> +	struct ps3_storage_device *dev = to_ps3_storage_device(&_dev->core);
> +	int error;
> +	struct Scsi_Host *host;
> +	struct ps3rom_private *priv;
> +
> +	if (dev->blk_size != CD_FRAMESIZE) {
> +		dev_err(&dev->sbd.core,
> +			"%s:%u: cannot handle block size %lu\n", __func__,
> +			__LINE__, dev->blk_size);
> +		return -EINVAL;
> +	}
> +
> +	dev->bounce_size = BOUNCE_SIZE;
> +	dev->bounce_buf = kmalloc(BOUNCE_SIZE, GFP_DMA);
> +	if (!dev->bounce_buf) {
> +		return -ENOMEM;
> +	}
> +
> +	error = ps3stor_setup(dev);
> +	if (error)
> +		goto fail_free_bounce;
> +
> +	/* override the interrupt handler */
> +	free_irq(dev->irq, dev);
> +	error = request_irq(dev->irq, ps3rom_interrupt, IRQF_DISABLED,
> +			    dev->sbd.core.driver->name, dev);
> +	if (error) {
> +		dev_err(&dev->sbd.core, "%s:%u: request_irq failed %d\n",
> +			__func__, __LINE__, error);
> +		goto fail_teardown;
> +	}
> +

Somebody's help isn't helpful?  Layering problem?


> +	host = scsi_host_alloc(&ps3rom_host_template,
> +			       sizeof(struct ps3rom_private));
> +	if (!host) {
> +		dev_err(&dev->sbd.core, "%s:%u: scsi_host_alloc failed\n",
> +			__func__, __LINE__);
> +		goto fail_teardown;
> +	}
> +
> +	priv = (struct ps3rom_private *)host->hostdata;
> +	ps3rom_priv(dev) = host;
> +	priv->dev = dev;
>

Is there code to clear ->priv when failing or unregistering?  Usually 
these fields are set to NULL to avoid accidental use of stale pointers 
to freed memory.  Discovering a NULL pointer defereence is usually 
easier than a stale usage.

> +
> +	/* One device/LUN per SCSI bus */
> +	host->max_id = 1;
> +	host->max_lun = 1;
> +
> +	error = scsi_add_host(host, &dev->sbd.core);
> +	if (error) {
> +		dev_err(&dev->sbd.core, "%s:%u: scsi_host_alloc failed %d\n",
> +			__func__, __LINE__, error);
> +		error = -ENODEV;
> +		goto fail_host_put;
> +	}
> +
> +	scsi_scan_host(host);
> +	return 0;
> +
> +fail_host_put:
> +	scsi_host_put(host);
> +fail_teardown:
> +	ps3stor_teardown(dev);
> +fail_free_bounce:
> +	kfree(dev->bounce_buf);
> +	return error;
> +}
>

Re: [patch 5/6] ps3: ROM Storage Driver

[Christoph Hellwig]

On Mon, Jun 18, 2007 at 11:30:39AM -0500, Milton Miller wrote:
> When I think ROM I usually dont' start with optical media.
> 
> Have you condered calling this the optical driver?

Actually the right name would probably be ps3atapi :)

Re: [patch 5/6] ps3: ROM Storage Driver

[Geert Uytterhoeven]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2902 bytes --]

On Mon, 18 Jun 2007, Milton Miller wrote:
> > +config PS3_ROM
> > +	tristate "PS3 ROM Storage Driver"
> > +	depends on PPC_PS3 && BLK_DEV_SR
> > +	select PS3_STORAGE
> > +	help
> > +	  Include support for the PS3 ROM Storage.
> > +
> > +	  This support is required to access the PS3 BD/DVD/CD-ROM drive.
> > +	  In general, all users will say Y or M.
> > +
> 
> 
> When I think ROM I usually dont' start with optical media.
> 
> Have you condered calling this the optical driver?

The name comes from the SCSI device type (0x05 == TYPE_ROM). But if people
prefer ps3atapi, I can change it.

> Or at least putting BD?DVD?CD-ROM on the prompt.
> 
> 
> Why does it depend on BLK_DEV_SR?

Because it needs SCSI CD-ROM support.

> > +static int __devinit ps3rom_probe(struct ps3_system_bus_device *_dev)
> > +{
> > +	struct ps3_storage_device *dev = to_ps3_storage_device(&_dev->core);
> > +	int error;
> > +	struct Scsi_Host *host;
> > +	struct ps3rom_private *priv;
> > +
> > +	if (dev->blk_size != CD_FRAMESIZE) {
> > +		dev_err(&dev->sbd.core,
> > +			"%s:%u: cannot handle block size %lu\n", __func__,
> > +			__LINE__, dev->blk_size);
> > +		return -EINVAL;
> > +	}
> > +
> > +	dev->bounce_size = BOUNCE_SIZE;
> > +	dev->bounce_buf = kmalloc(BOUNCE_SIZE, GFP_DMA);
> > +	if (!dev->bounce_buf) {
> > +		return -ENOMEM;
> > +	}
> > +
> > +	error = ps3stor_setup(dev);
> > +	if (error)
> > +		goto fail_free_bounce;
> > +
> > +	/* override the interrupt handler */
> > +	free_irq(dev->irq, dev);
> > +	error = request_irq(dev->irq, ps3rom_interrupt, IRQF_DISABLED,
> > +			    dev->sbd.core.driver->name, dev);
> > +	if (error) {
> > +		dev_err(&dev->sbd.core, "%s:%u: request_irq failed %d\n",
> > +			__func__, __LINE__, error);
> > +		goto fail_teardown;
> > +	}
> > +
> 
> Somebody's help isn't helpful?  Layering problem?

ps3stor_setup() tries to read from the device to check its accessibility,
and uses a simple interrupt routine for synchronous operations (using the
completion). This interrupt routine is also used for ps3flash.

Ps3rom and ps3disk do asynchronous device accesses, hence they use different
interrupt routines.

I guess I can add synchronous support to the ps3rom interrupt handler (ps3disk
already has this, for sync cache), so ps3stor_setup() can use them for probing,
too.

With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center 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/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

Re: [patch 5/6] ps3: ROM Storage Driver

[Milton Miller]

On Jun 19, 2007, at 7:29 AM, Geert Uytterhoeven wrote:
> On Mon, 18 Jun 2007, Milton Miller wrote:
>>> +config PS3_ROM
>>> +	tristate "PS3 ROM Storage Driver"
>>> +	depends on PPC_PS3 && BLK_DEV_SR
>>> +	select PS3_STORAGE
>>> +	help
>>> +	  Include support for the PS3 ROM Storage.
>>> +
>>> +	  This support is required to access the PS3 BD/DVD/CD-ROM drive.
>>> +	  In general, all users will say Y or M.
>>> +
>>
>> Why does it depend on BLK_DEV_SR?
>
> Because it needs SCSI CD-ROM support.
>

To compile?  I don't see any exports there.

My point is if it only relies on it to produce the device node, then 
you should provide guidance in your help text but not make the module 
build dependent on it.

milton

Re: [patch 5/6] ps3: ROM Storage Driver

[Christoph Hellwig]

Looks pretty nice.

Some more comments:

 - no need for the BLK_DEV_SR dependency, it should depend on SCSI
   instead (which might be implied by beeing in the scsi menu, don't
   remember that detail)
 - ps3rom_priv should probably be an inline
 - the cmd field in struct ps3rom_private should probably be renamed
   to curr_cmd or something along the line to make sure this is the
   current command we deal with and the driver can only deal with one
   command at a time.
 - The comment in ps3rom_queuecommand should explain why we prefer
   lv1_storage_{read,write} over just sending down the scsi cdb,
   not just that we prefer it.
 - I don't think you need to decode the 6 byte commands at all,
   ATAPI only has the 10 byte variants and you can tell the scsi
   layer not to ever send the 6 byte ones by setting the use_10_for_rw
   flag in ->slave_configure
 - same comment about the free_irq/request_irq pair as in the disk
   driver.
 - please use the new shost_priv helper in scsi-misc for accessing
   the hostdata file in struct Scsi_Host so that the ugly casts
   can go away.

[patch 6/6] ps3: FLASH ROM Storage Driver

[Geert Uytterhoeven]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 13324 bytes --]

From: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>

Add a FLASH ROM Storage Driver for the PS3:
  - Implemented as a misc character device driver
  - Uses a fixed 256 KiB buffer allocated from boot memory as the hypervisor
    requires the writing of aligned 256 KiB blocks

CC: Geoff Levand <geoffrey.levand@am.sony.com>
Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
---
Changes since previous submission:
      o Don't use `default y' in Kconfig
      o #include <linux/uaccess.h> instead of <asm/uaccess.h>
      o Set up sysfs links between misc character device and PS3 system device:
	  . /sys/class/misc/ps3flash/device -> ../../../devices/ps3_system/sb_01
	  . /sys/devices/ps3_system/sb_01/misc:ps3flash -> ../../../class/misc/ps3flash

 arch/powerpc/platforms/ps3/Kconfig |   11 +
 drivers/char/Makefile              |    2 
 drivers/char/ps3flash.c            |  404 +++++++++++++++++++++++++++++++++++++
 3 files changed, 417 insertions(+)

--- a/arch/powerpc/platforms/ps3/Kconfig
+++ b/arch/powerpc/platforms/ps3/Kconfig
@@ -122,4 +122,15 @@ config PS3_ROM
 	  This support is required to access the PS3 BD/DVD/CD-ROM drive.
 	  In general, all users will say Y or M.
 
+config PS3_FLASH
+	tristate "PS3 FLASH ROM Storage Driver"
+	depends on PPC_PS3
+	select PS3_STORAGE
+	help
+	  Include support for the PS3 FLASH ROM Storage.
+
+	  This support is required to access the PS3 FLASH ROM, which
+	  contains the boot loader and some boot options.
+	  In general, all users will say Y or M.
+
 endmenu
--- a/drivers/char/Makefile
+++ b/drivers/char/Makefile
@@ -104,6 +104,8 @@ obj-$(CONFIG_IPMI_HANDLER)	+= ipmi/
 obj-$(CONFIG_HANGCHECK_TIMER)	+= hangcheck-timer.o
 obj-$(CONFIG_TCG_TPM)		+= tpm/
 
+obj-$(CONFIG_PS3_FLASH)		+= ps3flash.o
+
 # Files generated that shall be removed upon make clean
 clean-files := consolemap_deftbl.c defkeymap.c
 
--- /dev/null
+++ b/drivers/char/ps3flash.c
@@ -0,0 +1,404 @@
+/*
+ * PS3 FLASH ROM Storage Driver
+ *
+ * Copyright (C) 2007 Sony Computer Entertainment Inc.
+ * Copyright 2007 Sony Corp.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published
+ * by the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include <linux/dma-mapping.h>
+#include <linux/interrupt.h>
+#include <linux/miscdevice.h>
+#include <linux/uaccess.h>
+
+#include <asm/ps3stor.h>
+
+
+#define DEVICE_NAME		"ps3flash"
+
+#define FLASH_BLOCK_SIZE	(256*1024)
+
+
+struct ps3flash_private {
+	struct mutex mutex;	/* Bounce buffer mutex */
+};
+#define ps3flash_priv(dev)	((dev)->sbd.core.driver_data)
+
+static struct ps3_storage_device *ps3flash_dev;
+
+static ssize_t ps3flash_read_write_sectors(struct ps3_storage_device *dev,
+					   u64 lpar, u64 start_sector,
+					   u64 sectors, int write)
+{
+	const char *op = write ? "write" : "read";
+	u64 res = ps3stor_read_write_sectors(dev, lpar, start_sector, sectors,
+					     write);
+	if (res) {
+		dev_err(&dev->sbd.core, "%s:%u: %s failed 0x%lx\n", __func__,
+			__LINE__, op, res);
+		return -EIO;
+	}
+	return sectors;
+}
+
+static ssize_t ps3flash_read_sectors(struct ps3_storage_device *dev,
+				     u64 start_sector, u64 sectors,
+				     unsigned int sector_offset)
+{
+	u64 max_sectors, lpar;
+
+	max_sectors = dev->bounce_size / dev->blk_size;
+	if (sectors > max_sectors) {
+		dev_dbg(&dev->sbd.core, "%s:%u Limiting sectors to %lu\n",
+			__func__, __LINE__, max_sectors);
+		sectors = max_sectors;
+	}
+
+	lpar = dev->bounce_lpar + sector_offset * dev->blk_size;
+	return ps3flash_read_write_sectors(dev, lpar, start_sector, sectors,
+					   0);
+}
+
+static ssize_t ps3flash_write_chunk(struct ps3_storage_device *dev,
+				    u64 start_sector)
+{
+       u64 sectors = dev->bounce_size / dev->blk_size;
+       return ps3flash_read_write_sectors(dev, dev->bounce_lpar, start_sector,
+					  sectors, 1);
+}
+
+static loff_t ps3flash_llseek(struct file *file, loff_t offset, int origin)
+{
+	struct ps3_storage_device *dev = ps3flash_dev;
+	u64 size = dev->regions[dev->region_idx].size*dev->blk_size;
+
+	switch (origin) {
+	case 1:
+		offset += file->f_pos;
+		break;
+	case 2:
+		offset += size;
+		break;
+	}
+	if (offset < 0)
+		return -EINVAL;
+
+	file->f_pos = offset;
+	return file->f_pos;
+}
+
+static ssize_t ps3flash_read(struct file *file, char __user *buf, size_t count,
+			     loff_t *pos)
+{
+	struct ps3_storage_device *dev = ps3flash_dev;
+	struct ps3flash_private *priv = ps3flash_priv(dev);
+	u64 size, start_sector, end_sector, offset;
+	ssize_t sectors_read;
+	size_t remaining, n;
+
+	dev_dbg(&dev->sbd.core,
+		"%s:%u: Reading %zu bytes at position %lld to user 0x%p\n",
+		__func__, __LINE__, count, *pos, buf);
+
+	size = dev->regions[dev->region_idx].size*dev->blk_size;
+	if (*pos >= size || !count)
+		return 0;
+
+	if (*pos+count > size) {
+		dev_dbg(&dev->sbd.core,
+			"%s:%u Truncating count from %zu to %llu\n", __func__,
+			__LINE__, count, size - *pos);
+		count = size - *pos;
+	}
+
+	start_sector = do_div_llr(*pos, dev->blk_size, &offset);
+	end_sector = DIV_ROUND_UP(*pos+count, dev->blk_size);
+
+	remaining = count;
+	do {
+		mutex_lock(&priv->mutex);
+
+		sectors_read = ps3flash_read_sectors(dev, start_sector,
+						     end_sector-start_sector,
+						     0);
+		if (sectors_read < 0) {
+			mutex_unlock(&priv->mutex);
+			return sectors_read;
+		}
+
+		n = min(remaining, sectors_read*dev->blk_size-offset);
+		dev_dbg(&dev->sbd.core,
+			"%s:%u: copy %lu bytes from 0x%p to user 0x%p\n",
+			__func__, __LINE__, n, dev->bounce_buf+offset, buf);
+		if (copy_to_user(buf, dev->bounce_buf+offset, n)) {
+			mutex_unlock(&priv->mutex);
+			return -EFAULT;
+		}
+
+		mutex_unlock(&priv->mutex);
+
+		*pos += n;
+		buf += n;
+		remaining -= n;
+		start_sector += sectors_read;
+		offset = 0;
+	} while (remaining > 0);
+
+	return count;
+}
+
+static ssize_t ps3flash_write(struct file *file, const char __user *buf,
+			      size_t count, loff_t *pos)
+{
+	struct ps3_storage_device *dev = ps3flash_dev;
+	struct ps3flash_private *priv = ps3flash_priv(dev);
+	u64 size, chunk_sectors, start_write_sector, end_write_sector,
+	    end_read_sector, start_read_sector, head, tail, offset;
+	ssize_t res;
+	size_t remaining, n;
+	unsigned int sec_off;
+
+	dev_dbg(&dev->sbd.core,
+		"%s:%u: Writing %zu bytes at position %lld from user 0x%p\n",
+		__func__, __LINE__, count, *pos, buf);
+
+	size = dev->regions[dev->region_idx].size*dev->blk_size;
+	if (*pos >= size || !count)
+		return 0;
+
+	if (*pos+count > size) {
+		dev_dbg(&dev->sbd.core,
+			"%s:%u Truncating count from %zu to %llu\n", __func__,
+			__LINE__, count, size - *pos);
+		count = size - *pos;
+	}
+
+	chunk_sectors = dev->bounce_size / dev->blk_size;
+
+	start_write_sector = do_div_llr(*pos, dev->bounce_size, &offset) *
+			     chunk_sectors;
+	end_write_sector = DIV_ROUND_UP(*pos+count, dev->bounce_size) *
+			   chunk_sectors;
+
+	end_read_sector = DIV_ROUND_UP(*pos, dev->blk_size);
+	start_read_sector = (*pos+count) / dev->blk_size;
+
+	/*
+	 * As we have to write in 256 KiB chunks, while we can read in blk_size
+	 * (usually 512 bytes) chunks, we perform the following steps:
+	 *   1. Read from start_write_sector to end_read_sector ("head")
+	 *   2. Read from start_read_sector to end_write_sector ("tail")
+	 *   3. Copy data to buffer
+	 *   4. Write from start_write_sector to end_write_sector
+	 * All of this is complicated by using only one 256 KiB bounce buffer.
+	 */
+
+	head = end_read_sector-start_write_sector;
+	tail = end_write_sector-start_read_sector;
+
+	remaining = count;
+	do {
+		mutex_lock(&priv->mutex);
+
+		if (end_read_sector >= start_read_sector) {
+			/* Merge head and tail */
+			dev_dbg(&dev->sbd.core,
+				"Merged head and tail: %lu sectors at %lu\n",
+				chunk_sectors, start_write_sector);
+			res = ps3flash_read_sectors(dev, start_write_sector,
+						    chunk_sectors, 0);
+			if (res < 0)
+				goto fail;
+		} else {
+			if (head) {
+				/* Read head */
+				dev_dbg(&dev->sbd.core,
+					"head: %lu sectors at %lu\n", head,
+					start_write_sector);
+				res = ps3flash_read_sectors(dev,
+							    start_write_sector,
+							    head, 0);
+				if (res < 0)
+					goto fail;
+			}
+			if (start_read_sector <
+			    start_write_sector+chunk_sectors) {
+				/* Read tail */
+				dev_dbg(&dev->sbd.core,
+					"tail: %lu sectors at %lu\n", tail,
+					start_read_sector);
+				sec_off = start_read_sector-start_write_sector;
+				res = ps3flash_read_sectors(dev,
+							    start_read_sector,
+							    tail, sec_off);
+				if (res < 0)
+					goto fail;
+			}
+		}
+
+		n = min(remaining, dev->bounce_size-offset);
+		dev_dbg(&dev->sbd.core,
+			"%s:%u: copy %lu bytes from user 0x%p to 0x%p\n",
+			__func__, __LINE__, n, buf, dev->bounce_buf+offset);
+		if (copy_from_user(dev->bounce_buf+offset, buf, n)) {
+			res = -EFAULT;
+			goto fail;
+		}
+
+		res = ps3flash_write_chunk(dev, start_write_sector);
+		if (res < 0)
+			goto fail;
+
+		mutex_unlock(&priv->mutex);
+
+		*pos += n;
+		buf += n;
+		remaining -= n;
+		start_write_sector += chunk_sectors;
+		head = 0;
+		offset = 0;
+	} while (remaining > 0);
+
+	return count;
+
+fail:
+	mutex_unlock(&priv->mutex);
+	return res;
+}
+
+
+static const struct file_operations ps3flash_fops = {
+	.owner	= THIS_MODULE,
+	.llseek	= ps3flash_llseek,
+	.read	= ps3flash_read,
+	.write	= ps3flash_write,
+};
+
+static struct miscdevice ps3flash_misc = {
+	.minor	= MISC_DYNAMIC_MINOR,
+	.name	= DEVICE_NAME,
+	.fops	= &ps3flash_fops,
+};
+
+static int __devinit ps3flash_probe(struct ps3_system_bus_device *_dev)
+{
+	struct ps3_storage_device *dev = to_ps3_storage_device(&_dev->core);
+	struct ps3flash_private *priv;
+	int error;
+	unsigned long tmp;
+
+	tmp = dev->regions[dev->region_idx].start*dev->blk_size;
+	if (tmp % FLASH_BLOCK_SIZE) {
+		dev_err(&dev->sbd.core,
+			"%s:%u region start %lu is not aligned\n", __func__,
+			__LINE__, tmp);
+		return -EINVAL;
+	}
+	tmp = dev->regions[dev->region_idx].size*dev->blk_size;
+	if (tmp % FLASH_BLOCK_SIZE) {
+		dev_err(&dev->sbd.core,
+			"%s:%u region size %lu is not aligned\n", __func__,
+			__LINE__, tmp);
+		return -EINVAL;
+	}
+
+	/* use static buffer, kmalloc cannot allocate 256 KiB */
+	if (!ps3flash_bounce_buffer.address)
+		return -ENOMEM;
+
+	if (ps3flash_dev) {
+		dev_err(&dev->sbd.core,
+			"Only one FLASH device is supported\n");
+		return -EBUSY;
+	}
+
+	ps3flash_dev = dev;
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv) {
+		error = -ENOMEM;
+		goto fail;
+	}
+
+	ps3flash_priv(dev) = priv;
+	mutex_init(&priv->mutex);
+
+	dev->bounce_size = ps3flash_bounce_buffer.size;
+	dev->bounce_buf = ps3flash_bounce_buffer.address;
+
+	error = ps3stor_setup(dev);
+	if (error)
+		goto fail_free_priv;
+
+	ps3flash_misc.parent = &dev->sbd.core;
+	error = misc_register(&ps3flash_misc);
+	if (error) {
+		dev_err(&dev->sbd.core, "%s:%u: misc_register failed %d\n",
+			__func__, __LINE__, error);
+		goto fail_teardown;
+	}
+
+	dev_info(&dev->sbd.core, "%s:%u: registered misc device %d\n",
+		 __func__, __LINE__, ps3flash_misc.minor);
+	return 0;
+
+fail_teardown:
+	ps3stor_teardown(dev);
+fail_free_priv:
+	kfree(priv);
+fail:
+	ps3flash_dev = NULL;
+	return error;
+}
+
+static int ps3flash_remove(struct ps3_system_bus_device *_dev)
+{
+	struct ps3_storage_device *dev = to_ps3_storage_device(&_dev->core);
+
+	misc_deregister(&ps3flash_misc);
+	ps3stor_teardown(dev);
+	kfree(ps3flash_priv(dev));
+	ps3flash_dev = NULL;
+	return 0;
+}
+
+
+static struct ps3_system_bus_driver ps3flash = {
+	.match_id	= PS3_MATCH_ID_STOR_FLASH,
+	.core.name	= DEVICE_NAME,
+	.core.owner	= THIS_MODULE,
+	.probe		= ps3flash_probe,
+	.remove		= ps3flash_remove,
+	.shutdown	= ps3flash_remove,
+};
+
+
+static int __init ps3flash_init(void)
+{
+	return ps3_system_bus_driver_register(&ps3flash);
+}
+
+static void __exit ps3flash_exit(void)
+{
+	ps3_system_bus_driver_unregister(&ps3flash);
+}
+
+module_init(ps3flash_init);
+module_exit(ps3flash_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("PS3 FLASH ROM Storage Driver");
+MODULE_AUTHOR("Sony Corporation");
+MODULE_ALIAS(PS3_MODULE_ALIAS_STOR_FLASH);

-- 
With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center 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/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

Re: [patch 6/6] ps3: FLASH ROM Storage Driver

[Milton Miller]

> From: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
>
> Add a FLASH ROM Storage Driver for the PS3:
>   - Implemented as a misc character device driver
>   - Uses a fixed 256 KiB buffer allocated from boot memory as the 
> hypervisor
>     requires the writing of aligned 256 KiB blocks
>
> CC: Geoff Levand <geoffrey.levand@am.sony.com>
> Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
> ---
> Changes since previous submission:
>       o Don't use `default y' in Kconfig
>       o #include <linux/uaccess.h> instead of <asm/uaccess.h>
>       o Set up sysfs links between misc character device and PS3 
> system device:
> 	  . /sys/class/misc/ps3flash/device -> 
> ../../../devices/ps3_system/sb_01
> 	  . /sys/devices/ps3_system/sb_01/misc:ps3flash -> 
> ../../../class/misc/ps3flash
>
>  arch/powerpc/platforms/ps3/Kconfig |   11 +
>  drivers/char/Makefile              |    2
>  drivers/char/ps3flash.c            |  404 
> +++++++++++++++++++++++++++++++++++++
>  3 files changed, 417 insertions(+)

You indicated that the primary reasons to use this driver were to boot 
back into GameOS, or reinstall the boot loader, yet your Kconfig entry 
says "In general, all users will say Y or M".  This could give a bit 
more guidance.

ps3flash_read_write_sectors chooses a string every call that is only 
used on the error path.

ps3flash_read gives priority to returning an encountered error.  If it 
made partial progress (eg partially mapped user buffer) it should 
return the count of bytes sent to the user.

ps3_write will call the hypervisor to write an aligned chunk of memory 
every call.  I'm guessing the hypervisor does no buffering of the 
flash.  If the file is written with a small block size, this will
cause unnecessary flash wear.  Have you considered requiring the 
application to write the full 256MiB, or alternatively delaying the 
actual write call until either close, the write reaches the boundary, 
or a read or write accesses another chunk?   Yes, the later would mean 
short writes would receive the error on close.

milton

Re: [patch 6/6] ps3: FLASH ROM Storage Driver

[Geert Uytterhoeven]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2207 bytes --]

	Hi Milton,

Thanks for your comments!

On Mon, 18 Jun 2007, Milton Miller wrote:
> You indicated that the primary reasons to use this driver were to boot back
> into GameOS, or reinstall the boot loader, yet your Kconfig entry says "In
> general, all users will say Y or M".  This could give a bit more guidance.

Normal users boot back into GameOS from time to time (I don't consider myself a
normal user ;-)

> ps3flash_read_write_sectors chooses a string every call that is only used on
> the error path.

Fixed.

> ps3flash_read gives priority to returning an encountered error.  If it made
> partial progress (eg partially mapped user buffer) it should return the count
> of bytes sent to the user.

Apparently all character drivers in drivers/char/mem.c immediately return
-EFAULT if copy_to_user() fails. The behavior of ps3flash is consistent with
this.

> ps3_write will call the hypervisor to write an aligned chunk of memory every
> call.  I'm guessing the hypervisor does no buffering of the flash.  If the
> file is written with a small block size, this will
> cause unnecessary flash wear.  Have you considered requiring the application
> to write the full 256MiB, or alternatively delaying the actual write call
> until either close, the write reaches the boundary, or a read or write
> accesses another chunk?   Yes, the later would mean short writes would receive
> the error on close.

I do not know how the hypervisor handles this internally.
But as the only time you write to the FLASH ROM is to change some boot flags or
upgrade the boot loader, I don't think this is a big problem.

With kind regards,
 
Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center 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/
 	
Sony Network and Software Technology Center Europe	
A division of Sony Service Centre (Europe) N.V.	
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium	
VAT BE 0413.825.160 · RPR Brussels	
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619