From: Geert Uytterhoeven <redacted>

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;
+}