diff mbox

sh: maple: add support for Visual Memory Card devices, and make consequential changes to maple input drivers - 2/3 - v5

Message ID 1234123473.6736.29.camel@localhost.localdomain (mailing list archive)
State Superseded
Delegated to: Paul Mundt
Headers show

Commit Message

Adrian McMenamin Feb. 8, 2009, 8:04 p.m. UTC
Change the maple bus driver to support the visual memory unit driver.

The maple bus driver currently only supports synchronous polling of attached devices status. These changes allow
the bus to handle asynchronous commands such as block reads and writes. 

Signed-off-by: Adrian McMenamin <adrian@mcmen.demon.co.uk>
---

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Paul Mundt Feb. 9, 2009, 6:56 a.m. UTC | #1
On Sun, Feb 08, 2009 at 08:04:33PM +0000, Adrian McMenamin wrote:
> Change the maple bus driver to support the visual memory unit driver.
> 
> The maple bus driver currently only supports synchronous polling of attached devices status. These changes allow
> the bus to handle asynchronous commands such as block reads and writes. 
> 
> Signed-off-by: Adrian McMenamin <adrian@mcmen.demon.co.uk>

The ordering of your patch series is a bit vague. Do the changes to the
maple bus code need to be made before the VMU patch can be applied? Do
the input driver changes have to be made at the same time as the changes
to the bus code, or are they ok to leave as a separate patch after the
bus changes?

All of these seem to have some interdependency issues that haven't been
noted at all, making it incredibly difficult to apply incrementally. Your
subject for the series also seems to imply you have no idea how they
logically structure, and that you simply hacked things up until the point
where everything worked, rather than paying attention to logical
incremental changes to show how you got from point A to point B without
breaking bisection along the way.

We do not want to have the tree in a state where bisection is broken, nor
do we want to apply huge monolothic changes that are unable to be clearly
broken out.

At this point the maple bus stuff I am fine with, and I have no real
objections to the driver patches either, it is more your methodology or
lack thereof that makes dealing with this rather taxing. If you want your
patches applied, small incremental patches that don't leave the tree in a
broken state are the way to go.

Presently I have no idea how to split this series up, and even if the
other subsystem folks add their Acked-bys, this will not be going in as
one large change.
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Adrian McMenamin Feb. 9, 2009, 7:17 p.m. UTC | #2
On Mon, 2009-02-09 at 15:56 +0900, Paul Mundt wrote:
> On Sun, Feb 08, 2009 at 08:04:33PM +0000, Adrian McMenamin wrote:
> > Change the maple bus driver to support the visual memory unit driver.
> > 
> > The maple bus driver currently only supports synchronous polling of attached devices status. These changes allow
> > the bus to handle asynchronous commands such as block reads and writes. 
> > 
> > Signed-off-by: Adrian McMenamin <adrian@mcmen.demon.co.uk>
> 
> The ordering of your patch series is a bit vague. Do the changes to the
> maple bus code need to be made before the VMU patch can be applied? Do
> the input driver changes have to be made at the same time as the changes
> to the bus code, or are they ok to leave as a separate patch after the
> bus changes?
> 
> All of these seem to have some interdependency issues that haven't been
> noted at all, making it incredibly difficult to apply incrementally. Your
> subject for the series also seems to imply you have no idea how they
> logically structure, and that you simply hacked things up until the point
> where everything worked, rather than paying attention to logical
> incremental changes to show how you got from point A to point B without
> breaking bisection along the way.
> 

You are right. I haven' made it fully clear in this post. But I did in
previous posts eg:

"This series of patches adds support for the Dreamcast Visual Memory
Unit, reworking the maple bus code to ensure it supports asynchronous
reads and writes. A consequential amendment to the keyboard driver is
also included."

I know you read them because you commented on the code.

The VMU will not work without changes in the bus code because the
existing bus code relies on periodic polling (eg the keyboard is polled
50 times a second). That plainly won't work with a block device.

(The keyboard and joystick changes are minor and consequential to
changes made to eliminate some of the memory hacks you disliked
previously)

I could have posted it all as one patch but given that there are three
different maintainers here - Greg KH for the bus code, Dmitry for the
input layer and David Woodhouse for the MTD device it seemed pretty
sensible to me to break the code up in that way.

Unfortunately the hardware is quite flaky and that causes race
conditions (eg block writes can be so slow it can look like the device
has been removed). But I have also had issues in getting the locking
right - particularly as clone devices (and I wrote the first round
against one) behave slightly differently from SEGA devices.



> We do not want to have the tree in a state where bisection is broken, nor
> do we want to apply huge monolothic changes that are unable to be clearly
> broken out.

They are broken out. As I said - bus, mtd device and input devices.

> 
> At this point the maple bus stuff I am fine with, and I have no real
> objections to the driver patches either, it is more your methodology or
> lack thereof that makes dealing with this rather taxing. If you want your
> patches applied, small incremental patches that don't leave the tree in a
> broken state are the way to go.

Small incremental patches won't work. The bus code has to change to
support the device. The device is obviously a new file and the input
changes are consequential to the bus changes.




--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mike Frysinger Feb. 9, 2009, 7:33 p.m. UTC | #3
On Monday 09 February 2009 14:17:33 Adrian McMenamin wrote:
> On Mon, 2009-02-09 at 15:56 +0900, Paul Mundt wrote:
> > On Sun, Feb 08, 2009 at 08:04:33PM +0000, Adrian McMenamin wrote:
> > > Change the maple bus driver to support the visual memory unit driver.
> > >
> > > The maple bus driver currently only supports synchronous polling of
> > > attached devices status. These changes allow the bus to handle
> > > asynchronous commands such as block reads and writes.
> > >
> > > Signed-off-by: Adrian McMenamin <adrian@mcmen.demon.co.uk>
> >
> > The ordering of your patch series is a bit vague. Do the changes to the
> > maple bus code need to be made before the VMU patch can be applied? Do
> > the input driver changes have to be made at the same time as the changes
> > to the bus code, or are they ok to leave as a separate patch after the
> > bus changes?
> >
> > All of these seem to have some interdependency issues that haven't been
> > noted at all, making it incredibly difficult to apply incrementally. Your
> > subject for the series also seems to imply you have no idea how they
> > logically structure, and that you simply hacked things up until the point
> > where everything worked, rather than paying attention to logical
> > incremental changes to show how you got from point A to point B without
> > breaking bisection along the way.
>
> You are right. I haven' made it fully clear in this post. But I did in
> previous posts eg:
>
> "This series of patches adds support for the Dreamcast Visual Memory
> Unit, reworking the maple bus code to ensure it supports asynchronous
> reads and writes. A consequential amendment to the keyboard driver is
> also included."

that isnt terribly clear.  it might be for you, but for the people attempting 
to coordinate source code, it really isnt.

using git, you can format patches and number them and include a cover letter.  
that would make everything clear for people.
git format-patch -n --cover-letter HEAD~3

then you'd end up with 4 clearly marked files (assuming your log message for 
each change is clear).  in the summary, you should then make statements like:
Patch 1 (... subject ...) in this series can be applied by itself for the bus 
driver.  Then patch 2 (... subject ...) and 3 (... subject ...) can be applied 
as they need the bus changes.
-mike
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Adrian McMenamin Feb. 9, 2009, 9:41 p.m. UTC | #4
On Mon, 2009-02-09 at 14:33 -0500, Mike Frysinger wrote:

> 
> then you'd end up with 4 clearly marked files (assuming your log message for 
> each change is clear).  in the summary, you should then make statements like:
> Patch 1 (... subject ...) in this series can be applied by itself for the bus 
> driver.  Then patch 2 (... subject ...) and 3 (... subject ...) can be applied 
> as they need the bus changes.
> -mike


Fair enough.

I ought to resubmit patches anyway because previous code has a memory
leak/race which I've now fixed with a couple of extra lines.

But the basic point here is that changing the bus code is necessary to
get the VMU driver (or *any* VMU driver) to work, but changing the bus
driver also breaks the two existing drivers in mainline that use the
bus. The breakage in terms of lines to be fixed is small, but it still
needs to be done 'all at once'.

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mike Frysinger Feb. 10, 2009, 12:11 a.m. UTC | #5
On Monday 09 February 2009 16:41:56 Adrian McMenamin wrote:
> But the basic point here is that changing the bus code is necessary to
> get the VMU driver (or *any* VMU driver) to work, but changing the bus
> driver also breaks the two existing drivers in mainline that use the
> bus. The breakage in terms of lines to be fixed is small, but it still
> needs to be done 'all at once'.

the bus change and the resulting fallout in those drivers should be 1 
changeset.  if you want to do more work than just transitioning those drivers, 
that should be additional patches that depend on that bus changeset.  that way 
bisection will not break as Paul highlighted.
-mike
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/sh/maple/maple.c b/drivers/sh/maple/maple.c
index 63f0de2..b68a06b 100644
--- a/drivers/sh/maple/maple.c
+++ b/drivers/sh/maple/maple.c
@@ -1,16 +1,10 @@ 
 /*
  * Core maple bus functionality
  *
- *  Copyright (C) 2007, 2008 Adrian McMenamin
+ *  Copyright (C) 2007 - 2009 Adrian McMenamin
  *  Copyright (C) 2001 - 2008 Paul Mundt
- *
- * Based on 2.4 code by:
- *
- *  Copyright (C) 2000-2001 YAEGASHI Takeshi
+ *  Copyright (C) 2000 - 2001 YAEGASHI Takeshi
  *  Copyright (C) 2001 M. R. Brown
- *  Copyright (C) 2001 Paul Mundt
- *
- * and others.
  *
  * This file is subject to the terms and conditions of the GNU General Public
  * License.  See the file "COPYING" in the main directory of this archive
@@ -32,7 +26,7 @@ 
 #include <mach/dma.h>
 #include <mach/sysasic.h>
 
-MODULE_AUTHOR("Yaegashi Takeshi, Paul Mundt, M. R. Brown, Adrian McMenamin");
+MODULE_AUTHOR("Adrian McMenamin <adrian@mcmen.demon.co.uk>");
 MODULE_DESCRIPTION("Maple bus driver for Dreamcast");
 MODULE_LICENSE("GPL v2");
 MODULE_SUPPORTED_DEVICE("{{SEGA, Dreamcast/Maple}}");
@@ -49,7 +43,7 @@  static LIST_HEAD(maple_sentq);
 /* mutex to protect queue of waiting packets */
 static DEFINE_MUTEX(maple_wlist_lock);
 
-static struct maple_driver maple_dummy_driver;
+static struct maple_driver maple_unsupported_device;
 static struct device maple_bus;
 static int subdevice_map[MAPLE_PORTS];
 static unsigned long *maple_sendbuf, *maple_sendptr, *maple_lastptr;
@@ -62,8 +56,9 @@  struct maple_device_specify {
 	int unit;
 };
 
-static bool checked[4];
-static struct maple_device *baseunits[4];
+static bool checked[MAPLE_PORTS];
+static bool empty[MAPLE_PORTS];
+static struct maple_device *baseunits[MAPLE_PORTS];
 
 /**
  * maple_driver_register - register a maple driver
@@ -97,12 +92,20 @@  void maple_driver_unregister(struct maple_driver *drv)
 EXPORT_SYMBOL_GPL(maple_driver_unregister);
 
 /* set hardware registers to enable next round of dma */
-static void maplebus_dma_reset(void)
+static void maple_dma_reset(void)
 {
 	ctrl_outl(MAPLE_MAGIC, MAPLE_RESET);
 	/* set trig type to 0 for software trigger, 1 for hardware (VBLANK) */
 	ctrl_outl(1, MAPLE_TRIGTYPE);
-	ctrl_outl(MAPLE_2MBPS | MAPLE_TIMEOUT(50000), MAPLE_SPEED);
+	/*
+	* Maple system register
+	* bits 31 - 16	timeout in units of 20nsec
+	* bit 12	hard trigger - set 0 to keep responding to VBLANK
+	* bits 9 - 8	set 00 for 2 Mbps, 01 for 1 Mbps
+	* bits 3 - 0	delay (in 1.3ms) between VBLANK and start of DMA
+	* max delay is 11
+	*/
+	ctrl_outl(MAPLE_2MBPS | MAPLE_TIMEOUT(0xFFFF), MAPLE_SPEED);
 	ctrl_outl(PHYSADDR(maple_sendbuf), MAPLE_DMAADDR);
 	ctrl_outl(1, MAPLE_ENABLE);
 }
@@ -134,21 +137,16 @@  static void maple_release_device(struct device *dev)
 {
 	struct maple_device *mdev;
 	struct mapleq *mq;
-	if (!dev)
-		return;
+
 	mdev = to_maple_dev(dev);
 	mq = mdev->mq;
-	if (mq) {
-		if (mq->recvbufdcsp)
-			kmem_cache_free(maple_queue_cache, mq->recvbufdcsp);
-		kfree(mq);
-		mq = NULL;
-	}
+	kmem_cache_free(maple_queue_cache, mq->recvbuf);
+	kfree(mq);
 	kfree(mdev);
 }
 
 /**
- * maple_add_packet - add a single instruction to the queue
+ * maple_add_packet - add a single instruction to the maple bus queue
  * @mdev: maple device
  * @function: function on device being queried
  * @command: maple command to add
@@ -158,68 +156,12 @@  static void maple_release_device(struct device *dev)
 int maple_add_packet(struct maple_device *mdev, u32 function, u32 command,
 	size_t length, void *data)
 {
-	int locking, ret = 0;
+	int ret = 0;
 	void *sendbuf = NULL;
 
-	mutex_lock(&maple_wlist_lock);
-	/* bounce if device already locked */
-	locking = mutex_is_locked(&mdev->mq->mutex);
-	if (locking) {
-		ret = -EBUSY;
-		goto out;
-	}
-
-	mutex_lock(&mdev->mq->mutex);
-
 	if (length) {
-		sendbuf = kmalloc(length * 4, GFP_KERNEL);
+		sendbuf = kzalloc(length * 4, GFP_KERNEL);
 		if (!sendbuf) {
-			mutex_unlock(&mdev->mq->mutex);
-			ret = -ENOMEM;
-			goto out;
-		}
-		((__be32 *)sendbuf)[0] = cpu_to_be32(function);
-	}
-
-	mdev->mq->command = command;
-	mdev->mq->length = length;
-	if (length > 1)
-		memcpy(sendbuf + 4, data, (length - 1) * 4);
-	mdev->mq->sendbuf = sendbuf;
-
-	list_add(&mdev->mq->list, &maple_waitq);
-out:
-	mutex_unlock(&maple_wlist_lock);
-	return ret;
-}
-EXPORT_SYMBOL_GPL(maple_add_packet);
-
-/**
- * maple_add_packet_sleeps - add a single instruction to the queue
- * @mdev: maple device
- * @function: function on device being queried
- * @command: maple command to add
- * @length: length of command string (in 32 bit words)
- * @data: remainder of command string
- *
- * Same as maple_add_packet(), but waits for the lock to become free.
- */
-int maple_add_packet_sleeps(struct maple_device *mdev, u32 function,
-	u32 command, size_t length, void *data)
-{
-	int locking, ret = 0;
-	void *sendbuf = NULL;
-
-	locking = mutex_lock_interruptible(&mdev->mq->mutex);
-	if (locking) {
-		ret = -EIO;
-		goto out;
-	}
-
-	if (length) {
-		sendbuf = kmalloc(length * 4, GFP_KERNEL);
-		if (!sendbuf) {
-			mutex_unlock(&mdev->mq->mutex);
 			ret = -ENOMEM;
 			goto out;
 		}
@@ -233,38 +175,35 @@  int maple_add_packet_sleeps(struct maple_device *mdev, u32 function,
 	mdev->mq->sendbuf = sendbuf;
 
 	mutex_lock(&maple_wlist_lock);
-	list_add(&mdev->mq->list, &maple_waitq);
+	list_add_tail(&mdev->mq->list, &maple_waitq);
 	mutex_unlock(&maple_wlist_lock);
 out:
 	return ret;
 }
-EXPORT_SYMBOL_GPL(maple_add_packet_sleeps);
+EXPORT_SYMBOL_GPL(maple_add_packet);
 
 static struct mapleq *maple_allocq(struct maple_device *mdev)
 {
 	struct mapleq *mq;
 
-	mq = kmalloc(sizeof(*mq), GFP_KERNEL);
+	mq = kzalloc(sizeof(*mq), GFP_KERNEL);
 	if (!mq)
 		goto failed_nomem;
 
+	INIT_LIST_HEAD(&mq->list);
 	mq->dev = mdev;
-	mq->recvbufdcsp = kmem_cache_zalloc(maple_queue_cache, GFP_KERNEL);
-	mq->recvbuf = (void *) P2SEGADDR(mq->recvbufdcsp);
+	mq->recvbuf = kmem_cache_zalloc(maple_queue_cache, GFP_KERNEL);
 	if (!mq->recvbuf)
 		goto failed_p2;
-	/*
-	 * most devices do not need the mutex - but
-	 * anything that injects block reads or writes
-	 * will rely on it
-	 */
-	mutex_init(&mq->mutex);
+	mq->recvbuf->buf = &((mq->recvbuf->bufx)[0]);
 
 	return mq;
 
 failed_p2:
 	kfree(mq);
 failed_nomem:
+	dev_err(&mdev->dev, "could not allocate memory for device (%d, %d)\n",
+		mdev->port, mdev->unit);
 	return NULL;
 }
 
@@ -272,12 +211,16 @@  static struct maple_device *maple_alloc_dev(int port, int unit)
 {
 	struct maple_device *mdev;
 
+	/* zero this out to avoid kobj subsystem
+	* thinking it has already been registered */
+
 	mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
 	if (!mdev)
 		return NULL;
 
 	mdev->port = port;
 	mdev->unit = unit;
+
 	mdev->mq = maple_allocq(mdev);
 
 	if (!mdev->mq) {
@@ -286,19 +229,14 @@  static struct maple_device *maple_alloc_dev(int port, int unit)
 	}
 	mdev->dev.bus = &maple_bus_type;
 	mdev->dev.parent = &maple_bus;
+	init_waitqueue_head(&mdev->maple_wait);
 	return mdev;
 }
 
 static void maple_free_dev(struct maple_device *mdev)
 {
-	if (!mdev)
-		return;
-	if (mdev->mq) {
-		if (mdev->mq->recvbufdcsp)
-			kmem_cache_free(maple_queue_cache,
-				mdev->mq->recvbufdcsp);
-		kfree(mdev->mq);
-	}
+	kmem_cache_free(maple_queue_cache, mdev->mq->recvbuf);
+	kfree(mdev->mq);
 	kfree(mdev);
 }
 
@@ -320,7 +258,7 @@  static void maple_build_block(struct mapleq *mq)
 	maple_lastptr = maple_sendptr;
 
 	*maple_sendptr++ = (port << 16) | len | 0x80000000;
-	*maple_sendptr++ = PHYSADDR(mq->recvbuf);
+	*maple_sendptr++ = PHYSADDR(mq->recvbuf->buf);
 	*maple_sendptr++ =
 	    mq->command | (to << 8) | (from << 16) | (len << 24);
 	while (len-- > 0)
@@ -333,20 +271,28 @@  static void maple_send(void)
 	int i, maple_packets = 0;
 	struct mapleq *mq, *nmq;
 
-	if (!list_empty(&maple_sentq))
+	if (!maple_dma_done())
 		return;
+
+	/* disable DMA */
+	ctrl_outl(0, MAPLE_ENABLE);
+
+	if (!list_empty(&maple_sentq))
+		goto finish;
+
 	mutex_lock(&maple_wlist_lock);
-	if (list_empty(&maple_waitq) || !maple_dma_done()) {
+	if (list_empty(&maple_waitq)) {
 		mutex_unlock(&maple_wlist_lock);
-		return;
+		goto finish;
 	}
-	mutex_unlock(&maple_wlist_lock);
+
 	maple_lastptr = maple_sendbuf;
 	maple_sendptr = maple_sendbuf;
-	mutex_lock(&maple_wlist_lock);
+
 	list_for_each_entry_safe(mq, nmq, &maple_waitq, list) {
 		maple_build_block(mq);
-		list_move(&mq->list, &maple_sentq);
+		list_del_init(&mq->list);
+		list_add_tail(&mq->list, &maple_sentq);
 		if (maple_packets++ > MAPLE_MAXPACKETS)
 			break;
 	}
@@ -356,10 +302,13 @@  static void maple_send(void)
 			dma_cache_sync(0, maple_sendbuf + i * PAGE_SIZE,
 				       PAGE_SIZE, DMA_BIDIRECTIONAL);
 	}
+
+finish:
+	maple_dma_reset();
 }
 
 /* check if there is a driver registered likely to match this device */
-static int check_matching_maple_driver(struct device_driver *driver,
+static int maple_check_matching_driver(struct device_driver *driver,
 					void *devptr)
 {
 	struct maple_driver *maple_drv;
@@ -374,10 +323,7 @@  static int check_matching_maple_driver(struct device_driver *driver,
 
 static void maple_detach_driver(struct maple_device *mdev)
 {
-	if (!mdev)
-		return;
 	device_unregister(&mdev->dev);
-	mdev = NULL;
 }
 
 /* process initial MAPLE_COMMAND_DEVINFO for each device or port */
@@ -385,9 +331,9 @@  static void maple_attach_driver(struct maple_device *mdev)
 {
 	char *p, *recvbuf;
 	unsigned long function;
-	int matched, retval;
+	int matched, error;
 
-	recvbuf = mdev->mq->recvbuf;
+	recvbuf = mdev->mq->recvbuf->buf;
 	/* copy the data as individual elements in
 	* case of memory optimisation */
 	memcpy(&mdev->devinfo.function, recvbuf + 4, 4);
@@ -395,7 +341,6 @@  static void maple_attach_driver(struct maple_device *mdev)
 	memcpy(&mdev->devinfo.area_code, recvbuf + 20, 1);
 	memcpy(&mdev->devinfo.connector_direction, recvbuf + 21, 1);
 	memcpy(&mdev->devinfo.product_name[0], recvbuf + 22, 30);
-	memcpy(&mdev->devinfo.product_licence[0], recvbuf + 52, 60);
 	memcpy(&mdev->devinfo.standby_power, recvbuf + 112, 2);
 	memcpy(&mdev->devinfo.max_power, recvbuf + 114, 2);
 	memcpy(mdev->product_name, mdev->devinfo.product_name, 30);
@@ -414,43 +359,41 @@  static void maple_attach_driver(struct maple_device *mdev)
 		else
 			break;
 
-	printk(KERN_INFO "Maple device detected: %s\n",
-		mdev->product_name);
-	printk(KERN_INFO "Maple device: %s\n", mdev->product_licence);
-
 	function = be32_to_cpu(mdev->devinfo.function);
 
+	dev_info(&mdev->dev, "detected %s: function 0x%lX: at (%d, %d)\n",
+		mdev->product_name, function, mdev->port, mdev->unit);
+
 	if (function > 0x200) {
 		/* Do this silently - as not a real device */
 		function = 0;
-		mdev->driver = &maple_dummy_driver;
+		mdev->driver = &maple_unsupported_device;
 		sprintf(mdev->dev.bus_id, "%d:0.port", mdev->port);
+
 	} else {
-		printk(KERN_INFO
-			"Maple bus at (%d, %d): Function 0x%lX\n",
-			mdev->port, mdev->unit, function);
 
 		matched =
 			bus_for_each_drv(&maple_bus_type, NULL, mdev,
-				check_matching_maple_driver);
+				maple_check_matching_driver);
 
 		if (matched == 0) {
 			/* Driver does not exist yet */
-			printk(KERN_INFO
-				"No maple driver found.\n");
-			mdev->driver = &maple_dummy_driver;
+			dev_info(&mdev->dev, "no driver found\n");
+			mdev->driver = &maple_unsupported_device;
 		}
 		sprintf(mdev->dev.bus_id, "%d:0%d.%lX", mdev->port,
 			mdev->unit, function);
 	}
+
 	mdev->function = function;
 	mdev->dev.release = &maple_release_device;
-	retval = device_register(&mdev->dev);
-	if (retval) {
-		printk(KERN_INFO
-		"Maple bus: Attempt to register device"
-		" (%x, %x) failed.\n",
-		mdev->port, mdev->unit);
+
+	mdev->busy = 0;
+	error = device_register(&mdev->dev);
+	if (error) {
+		dev_warn(&mdev->dev, "could not register device at"
+			" (%d, %d), with error 0x%X\n", mdev->unit,
+			mdev->port, error);
 		maple_free_dev(mdev);
 		mdev = NULL;
 		return;
@@ -462,7 +405,7 @@  static void maple_attach_driver(struct maple_device *mdev)
  * port and unit then return 1 - allows identification
  * of which devices need to be attached or detached
  */
-static int detach_maple_device(struct device *device, void *portptr)
+static int check_maple_device(struct device *device, void *portptr)
 {
 	struct maple_device_specify *ds;
 	struct maple_device *mdev;
@@ -477,21 +420,25 @@  static int detach_maple_device(struct device *device, void *portptr)
 static int setup_maple_commands(struct device *device, void *ignored)
 {
 	int add;
-	struct maple_device *maple_dev = to_maple_dev(device);
-
-	if ((maple_dev->interval > 0)
-	    && time_after(jiffies, maple_dev->when)) {
-		/* bounce if we cannot lock */
-		add = maple_add_packet(maple_dev,
-			be32_to_cpu(maple_dev->devinfo.function),
+	struct maple_device *mdev = to_maple_dev(device);
+	if ((mdev->interval > 0) && (mdev->busy == 0) &&
+		time_after(jiffies, mdev->when)) {
+		/* bounce if we cannot add */
+		add = maple_add_packet(mdev,
+			be32_to_cpu(mdev->devinfo.function),
 			MAPLE_COMMAND_GETCOND, 1, NULL);
 		if (!add)
-			maple_dev->when = jiffies + maple_dev->interval;
+			mdev->when = jiffies + mdev->interval;
 	} else {
 		if (time_after(jiffies, maple_pnp_time))
-			/* This will also bounce */
-			maple_add_packet(maple_dev, 0,
-				MAPLE_COMMAND_DEVINFO, 0, NULL);
+			/* Ensure we don't have block reads and devinfo
+			* calls interfering with one another - so flag the
+			* device as busy */
+			if (mdev->busy == 0) {
+				mdev->busy = 1;
+				maple_add_packet(mdev, 0,
+					MAPLE_COMMAND_DEVINFO, 0, NULL);
+			}
 	}
 	return 0;
 }
@@ -499,29 +446,50 @@  static int setup_maple_commands(struct device *device, void *ignored)
 /* VBLANK bottom half - implemented via workqueue */
 static void maple_vblank_handler(struct work_struct *work)
 {
-	if (!list_empty(&maple_sentq) || !maple_dma_done())
+	int x, locking;
+	struct maple_device *mdev;
+
+	if (!maple_dma_done())
 		return;
 
 	ctrl_outl(0, MAPLE_ENABLE);
 
+	if (!list_empty(&maple_sentq))
+		goto finish;
+
+	/*
+	* Set up essential commands - to fetch data and
+	* check devices are still present
+	*/
 	bus_for_each_dev(&maple_bus_type, NULL, NULL,
-			 setup_maple_commands);
+		setup_maple_commands);
+
+	if (time_after(jiffies, maple_pnp_time)) {
+		/*
+		* Scan the empty ports - bus is flakey and may have
+		* mis-reported emptyness
+		*/
+		for (x = 0; x < MAPLE_PORTS; x++) {
+			if (checked[x] && empty[x]) {
+				mdev = baseunits[x];
+				if (!mdev)
+					break;
+				mdev->busy = 1;
+				locking = maple_add_packet(mdev, 0,
+					MAPLE_COMMAND_DEVINFO, 0, NULL);
+				if (!locking)
+					break;
+				}
+			}
 
-	if (time_after(jiffies, maple_pnp_time))
 		maple_pnp_time = jiffies + MAPLE_PNP_INTERVAL;
-
-	mutex_lock(&maple_wlist_lock);
-	if (!list_empty(&maple_waitq) && list_empty(&maple_sentq)) {
-		mutex_unlock(&maple_wlist_lock);
-		maple_send();
-	} else {
-		mutex_unlock(&maple_wlist_lock);
 	}
 
-	maplebus_dma_reset();
+finish:
+	maple_send();
 }
 
-/* handle devices added via hotplugs - placing them on queue for DEVINFO*/
+/* handle devices added via hotplugs - placing them on queue for DEVINFO */
 static void maple_map_subunits(struct maple_device *mdev, int submask)
 {
 	int retval, k, devcheck;
@@ -533,7 +501,7 @@  static void maple_map_subunits(struct maple_device *mdev, int submask)
 		ds.unit = k + 1;
 		retval =
 		    bus_for_each_dev(&maple_bus_type, NULL, &ds,
-				     detach_maple_device);
+				     check_maple_device);
 		if (retval) {
 			submask = submask >> 1;
 			continue;
@@ -543,6 +511,7 @@  static void maple_map_subunits(struct maple_device *mdev, int submask)
 			mdev_add = maple_alloc_dev(mdev->port, k + 1);
 			if (!mdev_add)
 				return;
+			mdev_add->busy = 1;
 			maple_add_packet(mdev_add, 0, MAPLE_COMMAND_DEVINFO,
 				0, NULL);
 			/* mark that we are checking sub devices */
@@ -564,27 +533,45 @@  static void maple_clean_submap(struct maple_device *mdev)
 }
 
 /* handle empty port or hotplug removal */
-static void maple_response_none(struct maple_device *mdev,
-				struct mapleq *mq)
-{
-	if (mdev->unit != 0) {
-		list_del(&mq->list);
-		maple_clean_submap(mdev);
-		printk(KERN_INFO
-		       "Maple bus device detaching at (%d, %d)\n",
-		       mdev->port, mdev->unit);
+static void maple_response_none(struct maple_device *mdev)
+{
+	maple_clean_submap(mdev);
+
+	if (likely(mdev->unit != 0)) {
+		/*
+		* Block devices play up
+		* and give the impression they have
+		* been removed even when still in place or
+		* trip the mtd layer when they have
+		* really gone - this code traps that eventuality
+		* and ensures we aren't overloaded with useless
+		* error messages
+		*/
+		if (mdev->can_unload) {
+			if (!mdev->can_unload(mdev)) {
+				mdev->busy = 2;
+				wake_up(&mdev->maple_wait);
+				return;
+			}
+		}
+
+		dev_info(&mdev->dev, "detaching device at (%d, %d)\n",
+			mdev->port, mdev->unit);
 		maple_detach_driver(mdev);
 		return;
-	}
-	if (!started || !fullscan) {
-		if (checked[mdev->port] == false) {
-			checked[mdev->port] = true;
-			printk(KERN_INFO "No maple devices attached"
-				" to port %d\n", mdev->port);
+	} else {
+		if (!started || !fullscan) {
+			if (checked[mdev->port] == false) {
+				checked[mdev->port] = true;
+				empty[mdev->port] = true;
+				dev_info(&mdev->dev, "no devices"
+					" to port %d\n", mdev->port);
+			}
+			return;
 		}
-		return;
 	}
-	maple_clean_submap(mdev);
+	/* Some hardware devices generate false detach messages on unit 0 */
+	mdev->busy = 0;
 }
 
 /* preprocess hotplugs or scans */
@@ -599,8 +586,11 @@  static void maple_response_devinfo(struct maple_device *mdev,
 		} else {
 			if (mdev->unit != 0)
 				maple_attach_driver(mdev);
+			if (mdev->unit == 0) {
+				empty[mdev->port] = false;
+				maple_attach_driver(mdev);
+			}
 		}
-		return;
 	}
 	if (mdev->unit == 0) {
 		submask = recvbuf[2] & 0x1F;
@@ -611,6 +601,17 @@  static void maple_response_devinfo(struct maple_device *mdev,
 	}
 }
 
+static void maple_response_fileerr(struct maple_device *mdev, void *recvbuf)
+{
+	if (mdev->fileerr_handler) {
+		mdev->fileerr_handler(mdev, recvbuf);
+		return;
+	} else
+		dev_warn(&mdev->dev, "device at (%d, %d) reports"
+			"file error 0x%X\n", mdev->port, mdev->unit,
+			((int *)recvbuf)[1]);
+}
+
 static void maple_port_rescan(void)
 {
 	int i;
@@ -621,12 +622,6 @@  static void maple_port_rescan(void)
 		if (checked[i] == false) {
 			fullscan = 0;
 			mdev = baseunits[i];
-			/*
-			 *  test lock in case scan has failed
-			 *  but device is still locked
-			 */
-			if (mutex_is_locked(&mdev->mq->mutex))
-				mutex_unlock(&mdev->mq->mutex);
 			maple_add_packet(mdev, 0, MAPLE_COMMAND_DEVINFO,
 				0, NULL);
 		}
@@ -637,7 +632,7 @@  static void maple_port_rescan(void)
 static void maple_dma_handler(struct work_struct *work)
 {
 	struct mapleq *mq, *nmq;
-	struct maple_device *dev;
+	struct maple_device *mdev;
 	char *recvbuf;
 	enum maple_code code;
 
@@ -646,43 +641,56 @@  static void maple_dma_handler(struct work_struct *work)
 	ctrl_outl(0, MAPLE_ENABLE);
 	if (!list_empty(&maple_sentq)) {
 		list_for_each_entry_safe(mq, nmq, &maple_sentq, list) {
-			recvbuf = mq->recvbuf;
+			mdev = mq->dev;
+			recvbuf = mq->recvbuf->buf;
+			dma_cache_sync(&mdev->dev, recvbuf, 0x400,
+				DMA_FROM_DEVICE);
 			code = recvbuf[0];
-			dev = mq->dev;
 			kfree(mq->sendbuf);
-			mutex_unlock(&mq->mutex);
 			list_del_init(&mq->list);
-
 			switch (code) {
 			case MAPLE_RESPONSE_NONE:
-				maple_response_none(dev, mq);
+				maple_response_none(mdev);
 				break;
 
 			case MAPLE_RESPONSE_DEVINFO:
-				maple_response_devinfo(dev, recvbuf);
+				maple_response_devinfo(mdev, recvbuf);
+				mdev->busy = 0;
 				break;
 
 			case MAPLE_RESPONSE_DATATRF:
-				if (dev->callback)
-					dev->callback(mq);
+				if (mdev->callback)
+					mdev->callback(mq);
+				mdev->busy = 0;
+				wake_up(&mdev->maple_wait);
 				break;
 
 			case MAPLE_RESPONSE_FILEERR:
+				maple_response_fileerr(mdev, recvbuf);
+				mdev->busy = 0;
+				wake_up(&mdev->maple_wait);
+				break;
+
 			case MAPLE_RESPONSE_AGAIN:
 			case MAPLE_RESPONSE_BADCMD:
 			case MAPLE_RESPONSE_BADFUNC:
-				printk(KERN_DEBUG
-				       "Maple non-fatal error 0x%X\n",
-				       code);
+				dev_warn(&mdev->dev, "non-fatal error"
+					" 0x%X at (%d, %d)\n", code,
+					mdev->port, mdev->unit);
+				mdev->busy = 0;
 				break;
 
 			case MAPLE_RESPONSE_ALLINFO:
-				printk(KERN_DEBUG
-				       "Maple - extended device information"
-					" not supported\n");
+				dev_notice(&mdev->dev, "extended"
+				" device information request for (%d, %d)"
+				" but call is not supported\n", mdev->port,
+				mdev->unit);
+				mdev->busy = 0;
 				break;
 
 			case MAPLE_RESPONSE_OK:
+				mdev->busy = 0;
+				wake_up(&mdev->maple_wait);
 				break;
 
 			default:
@@ -699,20 +707,19 @@  static void maple_dma_handler(struct work_struct *work)
 		if (!fullscan)
 			maple_port_rescan();
 		/* mark that we have been through the first scan */
-		if (started == 0)
-			started = 1;
+		started = 1;
 	}
-	maplebus_dma_reset();
+	maple_send();
 }
 
-static irqreturn_t maplebus_dma_interrupt(int irq, void *dev_id)
+static irqreturn_t maple_dma_interrupt(int irq, void *dev_id)
 {
 	/* Load everything into the bottom half */
 	schedule_work(&maple_dma_process);
 	return IRQ_HANDLED;
 }
 
-static irqreturn_t maplebus_vblank_interrupt(int irq, void *dev_id)
+static irqreturn_t maple_vblank_interrupt(int irq, void *dev_id)
 {
 	schedule_work(&maple_vblank_process);
 	return IRQ_HANDLED;
@@ -720,14 +727,14 @@  static irqreturn_t maplebus_vblank_interrupt(int irq, void *dev_id)
 
 static int maple_set_dma_interrupt_handler(void)
 {
-	return request_irq(HW_EVENT_MAPLE_DMA, maplebus_dma_interrupt,
-		IRQF_SHARED, "maple bus DMA", &maple_dummy_driver);
+	return request_irq(HW_EVENT_MAPLE_DMA, maple_dma_interrupt,
+		IRQF_SHARED, "maple bus DMA", &maple_unsupported_device);
 }
 
 static int maple_set_vblank_interrupt_handler(void)
 {
-	return request_irq(HW_EVENT_VSYNC, maplebus_vblank_interrupt,
-		IRQF_SHARED, "maple bus VBLANK", &maple_dummy_driver);
+	return request_irq(HW_EVENT_VSYNC, maple_vblank_interrupt,
+		IRQF_SHARED, "maple bus VBLANK", &maple_unsupported_device);
 }
 
 static int maple_get_dma_buffer(void)
@@ -740,7 +747,7 @@  static int maple_get_dma_buffer(void)
 	return 0;
 }
 
-static int match_maple_bus_driver(struct device *devptr,
+static int maple_match_bus_driver(struct device *devptr,
 				  struct device_driver *drvptr)
 {
 	struct maple_driver *maple_drv = to_maple_driver(drvptr);
@@ -765,16 +772,18 @@  static void maple_bus_release(struct device *dev)
 {
 }
 
-static struct maple_driver maple_dummy_driver = {
+static struct maple_driver maple_unsupported_device = {
 	.drv = {
-		.name = "maple_dummy_driver",
+		.name = "maple_unsupported_device",
 		.bus = &maple_bus_type,
 	},
 };
-
+/**
+ * maple_bus_type - core maple bus structure
+ */
 struct bus_type maple_bus_type = {
 	.name = "maple",
-	.match = match_maple_bus_driver,
+	.match = maple_match_bus_driver,
 	.uevent = maple_bus_uevent,
 };
 EXPORT_SYMBOL_GPL(maple_bus_type);
@@ -788,7 +797,8 @@  static int __init maple_bus_init(void)
 {
 	int retval, i;
 	struct maple_device *mdev[MAPLE_PORTS];
-	ctrl_outl(0, MAPLE_STATE);
+
+	ctrl_outl(0, MAPLE_ENABLE);
 
 	retval = device_register(&maple_bus);
 	if (retval)
@@ -798,36 +808,33 @@  static int __init maple_bus_init(void)
 	if (retval)
 		goto cleanup_device;
 
-	retval = driver_register(&maple_dummy_driver.drv);
+	retval = driver_register(&maple_unsupported_device.drv);
 	if (retval)
 		goto cleanup_bus;
 
 	/* allocate memory for maple bus dma */
 	retval = maple_get_dma_buffer();
 	if (retval) {
-		printk(KERN_INFO
-		       "Maple bus: Failed to allocate Maple DMA buffers\n");
+		dev_err(&maple_bus, "failed to allocate DMA buffers\n");
 		goto cleanup_basic;
 	}
 
 	/* set up DMA interrupt handler */
 	retval = maple_set_dma_interrupt_handler();
 	if (retval) {
-		printk(KERN_INFO
-		       "Maple bus: Failed to grab maple DMA IRQ\n");
+		dev_err(&maple_bus, "bus failed to grab maple "
+			"DMA IRQ\n");
 		goto cleanup_dma;
 	}
 
 	/* set up VBLANK interrupt handler */
 	retval = maple_set_vblank_interrupt_handler();
 	if (retval) {
-		printk(KERN_INFO "Maple bus: Failed to grab VBLANK IRQ\n");
+		dev_err(&maple_bus, "bus failed to grab VBLANK IRQ\n");
 		goto cleanup_irq;
 	}
 
-	maple_queue_cache =
-	    kmem_cache_create("maple_queue_cache", 0x400, 0,
-			      SLAB_HWCACHE_ALIGN, NULL);
+	maple_queue_cache = KMEM_CACHE(maple_buffer, SLAB_HWCACHE_ALIGN);
 
 	if (!maple_queue_cache)
 		goto cleanup_bothirqs;
@@ -838,23 +845,23 @@  static int __init maple_bus_init(void)
 	/* setup maple ports */
 	for (i = 0; i < MAPLE_PORTS; i++) {
 		checked[i] = false;
+		empty[i] = false;
 		mdev[i] = maple_alloc_dev(i, 0);
-		baseunits[i] = mdev[i];
 		if (!mdev[i]) {
 			while (i-- > 0)
 				maple_free_dev(mdev[i]);
 			goto cleanup_cache;
 		}
+		baseunits[i] = mdev[i];
+		mdev[i]->busy = 1;
 		maple_add_packet(mdev[i], 0, MAPLE_COMMAND_DEVINFO, 0, NULL);
 		subdevice_map[i] = 0;
 	}
 
-	/* setup maplebus hardware */
-	maplebus_dma_reset();
-	/* initial detection */
+	maple_pnp_time = jiffies + HZ;
+	/* prepare initial queue */
 	maple_send();
-	maple_pnp_time = jiffies;
-	printk(KERN_INFO "Maple bus core now registered.\n");
+	dev_info(&maple_bus, "bus core now registered\n");
 
 	return 0;
 
@@ -871,7 +878,7 @@  cleanup_dma:
 	free_pages((unsigned long) maple_sendbuf, MAPLE_DMA_PAGES);
 
 cleanup_basic:
-	driver_unregister(&maple_dummy_driver.drv);
+	driver_unregister(&maple_unsupported_device.drv);
 
 cleanup_bus:
 	bus_unregister(&maple_bus_type);
@@ -880,7 +887,7 @@  cleanup_device:
 	device_unregister(&maple_bus);
 
 cleanup:
-	printk(KERN_INFO "Maple bus registration failed\n");
+	printk(KERN_ERR "Maple bus registration failed\n");
 	return retval;
 }
 /* Push init to later to ensure hardware gets detected */
diff --git a/include/linux/maple.h b/include/linux/maple.h
index c23d3f5..3bbad7f 100644
--- a/include/linux/maple.h
+++ b/include/linux/maple.h
@@ -8,33 +8,49 @@  extern struct bus_type maple_bus_type;
 
 /* Maple Bus command and response codes */
 enum maple_code {
-	MAPLE_RESPONSE_FILEERR = -5,
-	MAPLE_RESPONSE_AGAIN = -4,	/* request should be retransmitted */
-	MAPLE_RESPONSE_BADCMD = -3,
-	MAPLE_RESPONSE_BADFUNC = -2,
-	MAPLE_RESPONSE_NONE = -1,	/* unit didn't respond at all */
-	MAPLE_COMMAND_DEVINFO = 1,
-	MAPLE_COMMAND_ALLINFO = 2,
-	MAPLE_COMMAND_RESET = 3,
-	MAPLE_COMMAND_KILL = 4,
-	MAPLE_RESPONSE_DEVINFO = 5,
-	MAPLE_RESPONSE_ALLINFO = 6,
-	MAPLE_RESPONSE_OK = 7,
-	MAPLE_RESPONSE_DATATRF = 8,
-	MAPLE_COMMAND_GETCOND = 9,
-	MAPLE_COMMAND_GETMINFO = 10,
-	MAPLE_COMMAND_BREAD = 11,
-	MAPLE_COMMAND_BWRITE = 12,
-	MAPLE_COMMAND_SETCOND = 14
+	MAPLE_RESPONSE_FILEERR =	-5,
+	MAPLE_RESPONSE_AGAIN,	/* retransmit */
+	MAPLE_RESPONSE_BADCMD,
+	MAPLE_RESPONSE_BADFUNC,
+	MAPLE_RESPONSE_NONE,	/* unit didn't respond*/
+	MAPLE_COMMAND_DEVINFO =		1,
+	MAPLE_COMMAND_ALLINFO,
+	MAPLE_COMMAND_RESET,
+	MAPLE_COMMAND_KILL,
+	MAPLE_RESPONSE_DEVINFO,
+	MAPLE_RESPONSE_ALLINFO,
+	MAPLE_RESPONSE_OK,
+	MAPLE_RESPONSE_DATATRF,
+	MAPLE_COMMAND_GETCOND,
+	MAPLE_COMMAND_GETMINFO,
+	MAPLE_COMMAND_BREAD,
+	MAPLE_COMMAND_BWRITE,
+	MAPLE_COMMAND_BSYNC,
+	MAPLE_COMMAND_SETCOND,
+	MAPLE_COMMAND_MICCONTROL
+};
+
+enum maple_file_errors {
+	MAPLE_FILEERR_INVALID_PARTITION =	0x01000000,
+	MAPLE_FILEERR_PHASE_ERROR =		0x02000000,
+	MAPLE_FILEERR_INVALID_BLOCK =		0x04000000,
+	MAPLE_FILEERR_WRITE_ERROR =		0x08000000,
+	MAPLE_FILEERR_INVALID_WRITE_LENGTH =	0x10000000,
+	MAPLE_FILEERR_BAD_CRC = 		0x20000000
+};
+
+struct maple_buffer {
+	char bufx[0x400];
+	void *buf;
 };
 
 struct mapleq {
 	struct list_head list;
 	struct maple_device *dev;
-	void *sendbuf, *recvbuf, *recvbufdcsp;
+	struct maple_buffer *recvbuf;
+	void *sendbuf, *recvbuf_p2;
 	unsigned char length;
 	enum maple_code command;
-	struct mutex mutex;
 };
 
 struct maple_devinfo {
@@ -52,11 +68,15 @@  struct maple_device {
 	struct maple_driver *driver;
 	struct mapleq *mq;
 	void (*callback) (struct mapleq * mq);
+	void (*fileerr_handler)(struct maple_device *mdev, void *recvbuf);
+	int (*can_unload)(struct maple_device *mdev);
 	unsigned long when, interval, function;
 	struct maple_devinfo devinfo;
 	unsigned char port, unit;
 	char product_name[32];
 	char product_licence[64];
+	int busy;
+	wait_queue_head_t maple_wait;
 	struct device dev;
 };
 
@@ -72,7 +92,7 @@  void maple_getcond_callback(struct maple_device *dev,
 int maple_driver_register(struct maple_driver *);
 void maple_driver_unregister(struct maple_driver *);
 
-int maple_add_packet_sleeps(struct maple_device *mdev, u32 function,
+int maple_add_packet(struct maple_device *mdev, u32 function,
 	u32 command, u32 length, void *data);
 void maple_clear_dev(struct maple_device *mdev);