Re: mmc_spi stopped working

[David Brownell <david-b@pacbell.net>]

On Wednesday 17 October 2007, Pierre Ossman wrote:
> 
> On Wed, 17 Oct 2007 12:37:13 -0700
> David Brownell <david-b@pacbell.net> wrote:
> > That issue is 49dce689ad4ef0fd1f970ef762168e4bd46f69a3, the
> > classdev-elimination patch from Tony Jones.  It broke the
> > "does this bus have more than one device" test by relocating
> > the relevant sysfs nodes.
> > 
> > Quick workaround for that one is to disable the fault return
> > after that test.
> > 
> 
> Annoying. Feel free to ping me when you've pushed fixes to Linus.

This is another case where whacking around in sysfs seems
unavoidable, even when it breaks things.  The patch below
works around that that change.  It seems like something that
should go through you, not directly through Linus...

- Dave

================	CUT HERE
Fix mmc-over-spi regression

Patch 49dce689ad4ef0fd1f970ef762168e4bd46f69a3 changed the sysfs data
structures for SPI in a way which broke the MMC-over-SPI host driver.

This patch fixes that regression by changing the scheme used to keep
from knowingly trying to use a shared bus segment, and updates the
adjacent comments slightly to better explain the issue.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>

--- a/drivers/mmc/host/mmc_spi.c	2007-10-23 21:24:41.000000000 -0700
+++ b/drivers/mmc/host/mmc_spi.c	2007-10-24 14:46:59.000000000 -0700
@@ -1165,6 +1165,23 @@ mmc_spi_detect_irq(int irq, void *mmc)
 	return IRQ_HANDLED;
 }
 
+struct count_children {
+	unsigned	n;
+	struct bus_type	*bus;
+};
+
+static int maybe_count_child(struct device *dev, void *c)
+{
+	struct count_children *ccp = c;
+
+	if (dev->bus == ccp->bus) {
+		if (ccp->n)
+			return -EBUSY;
+		ccp->n++;
+	}
+	return 0;
+}
+
 static int mmc_spi_probe(struct spi_device *spi)
 {
 	void			*ones;
@@ -1188,33 +1205,30 @@ static int mmc_spi_probe(struct spi_devi
 		return status;
 	}
 
-	/* We can use the bus safely iff nobody else will interfere with
-	 * us.  That is, either we have the experimental exclusive access
-	 * primitives ... or else there's nobody to share it with.
+	/* We can use the bus safely iff nobody else will interfere with us.
+	 * Most commands consist of one SPI message to issue a command, then
+	 * several more to collect its response, then possibly more for data
+	 * transfer.  Clocking access to other devices during that period will
+	 * corrupt the command execution.
+	 *
+	 * Until we have software primitives which guarantee non-interference,
+	 * we'll aim for a hardware-level guarantee.
+	 *
+	 * REVISIT we can't guarantee another device won't be added later...
 	 */
 	if (spi->master->num_chipselect > 1) {
-		struct device	*parent = spi->dev.parent;
+		struct count_children cc;
 
-		/* If there are multiple devices on this bus, we
-		 * can't proceed.
-		 */
-		spin_lock(&parent->klist_children.k_lock);
-		if (parent->klist_children.k_list.next
-				!= parent->klist_children.k_list.prev)
-			status = -EMLINK;
-		else
-			status = 0;
-		spin_unlock(&parent->klist_children.k_lock);
+		cc.n = 0;
+		cc.bus = spi->dev.bus;
+		status = device_for_each_child(spi->dev.parent, &cc,
+				maybe_count_child);
 		if (status < 0) {
 			dev_err(&spi->dev, "can't share SPI bus\n");
 			return status;
 		}
 
-		/* REVISIT we can't guarantee another device won't
-		 * be added later.  It's uncommon though ... for now,
-		 * work as if this is safe.
-		 */
-		dev_warn(&spi->dev, "ASSUMING unshared SPI bus!\n");
+		dev_warn(&spi->dev, "ASSUMING SPI bus stays unshared!\n");
 	}
 
 	/* We need a supply of ones to transmit.  This is the only time
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/