Add GPIO-based MMC/SD driver

This driver hooks up the mmc_spi and spi_gpio modules so that MMC/SD cards can be used on a GPIO based bus by bitbanging the SPI protocol in software.

This driver provides a sysfs interface to dynamically create and destroy GPIO-based MMC/SD card interfaces. It also provides a platform device interface API.
See Documentation/gpiommc.txt for details.

Signed-off-by Michael Buesch <[email protected]>

---

This driver is used in OpenWrt since quite some time, so please consider for inclusion in mainline.

Index: linux-next/drivers/mmc/host/gpiommc.c
===================================================================

drivers/mmc/host/gpiommc.c new file +251 -0

@@ -0,0 +1,328 @@
+/*
+ * Driver an MMC/SD card on a bitbanging GPIO SPI bus.
+ * This module hooks up the mmc_spi and spi_gpio modules and also
+ * provides a sysfs interface.
+ *
+ * Copyright 2008 Michael Buesch <[email protected]>
+ *
+ * Licensed under the GNU/GPL. See COPYING for details.
+ */
+
+#include <linux/mmc/gpiommc.h>
+#include <linux/platform_device.h>
+#include <linux/list.h>
+#include <linux/mutex.h>
+#include <linux/spi/spi_gpio.h>
+
+
+#define PFX				"gpio-mmc: "
+#define GPIOMMC_MAX_NAMELEN_STR		__stringify(GPIOMMC_MAX_NAMELEN)
+
+struct gpiommc_device {
+	struct platform_device *pdev;
+	struct platform_device *spi_pdev;
+	struct spi_board_info boardinfo;
+};
+
+
+MODULE_DESCRIPTION("GPIO based MMC driver");
+MODULE_AUTHOR("Michael Buesch");
+MODULE_LICENSE("GPL");
+
+
+static int gpiommc_boardinfo_setup(struct spi_board_info *bi,
+				   struct spi_master *master,
+				   void *data)
+{
+	struct gpiommc_device *d = data;
+	struct gpiommc_platform_data *pdata = d->pdev->dev.platform_data;
+
+	/* Bind the SPI master to the MMC-SPI host driver. */
+	strlcpy(bi->modalias, "mmc_spi", sizeof(bi->modalias));
+
+	bi->max_speed_hz = pdata->max_bus_speed;
+	bi->bus_num = master->bus_num;
+	bi->mode = pdata->mode;
+
+	return 0;
+}
+
+static int gpiommc_probe(struct platform_device *pdev)
+{
+	struct gpiommc_platform_data *mmc_pdata = pdev->dev.platform_data;
+	struct spi_gpio_platform_data spi_pdata;
+	struct gpiommc_device *d;
+	int err;
+
+	err = -ENXIO;
+	if (!mmc_pdata)
+		goto error;
+
+	/* Allocate the GPIO-MMC device */
+	err = -ENOMEM;
+	d = kzalloc(sizeof(*d), GFP_KERNEL);
+	if (!d)
+		goto error;
+	d->pdev = pdev;
+
+	/* Create the SPI-GPIO device */
+	d->spi_pdev = platform_device_alloc(SPI_GPIO_PLATDEV_NAME,
+					    spi_gpio_next_id());
+	if (!d->spi_pdev)
+		goto err_free_d;
+
+	memset(&spi_pdata, 0, sizeof(spi_pdata));
+	spi_pdata.pin_clk = mmc_pdata->pins.gpio_clk;
+	spi_pdata.pin_miso = mmc_pdata->pins.gpio_do;
+	spi_pdata.pin_mosi = mmc_pdata->pins.gpio_di;
+	spi_pdata.pin_cs = mmc_pdata->pins.gpio_cs;
+	spi_pdata.cs_activelow = mmc_pdata->pins.cs_activelow;
+	spi_pdata.no_spi_delay = mmc_pdata->no_spi_delay;
+	spi_pdata.boardinfo_setup = gpiommc_boardinfo_setup;
+	spi_pdata.boardinfo_setup_data = d;
+
+	err = platform_device_add_data(d->spi_pdev, &spi_pdata,
+				       sizeof(spi_pdata));
+	if (err)
+		goto err_free_pdev;
+	err = platform_device_add(d->spi_pdev);
+	if (err)
+		goto err_free_pdata;
+	platform_set_drvdata(pdev, d);
+
+	printk(KERN_INFO PFX "MMC-Card \"%s\" "
+	       "attached to GPIO pins di=%u, do=%u, clk=%u, cs=%u\n",
+	       mmc_pdata->name, mmc_pdata->pins.gpio_di,
+	       mmc_pdata->pins.gpio_do,
+	       mmc_pdata->pins.gpio_clk,
+	       mmc_pdata->pins.gpio_cs);
+
+	return 0;
+
+err_free_pdata:
+	kfree(d->spi_pdev->dev.platform_data);
+	d->spi_pdev->dev.platform_data = NULL;
+err_free_pdev:
+	platform_device_put(d->spi_pdev);
+err_free_d:
+	kfree(d);
+error:
+	return err;
+}
+
+static int gpiommc_remove(struct platform_device *pdev)
+{
+	struct gpiommc_device *d = platform_get_drvdata(pdev);
+	struct gpiommc_platform_data *pdata = d->pdev->dev.platform_data;
+
+	platform_device_unregister(d->spi_pdev);
+	printk(KERN_INFO PFX "GPIO based MMC-Card \"%s\" removed\n", pdata->name);
+	platform_device_put(d->spi_pdev);
+
+	return 0;
+}
+
+/* Wrapper for the platform data with context data for the sysfs interface. */
+struct gpiommc_sysfs_platform_data {
+	struct gpiommc_platform_data p; /* Keep as first element */
+
+	/* The platform device that we allocated. */
+	struct platform_device *pdev;
+	/* gpiommc_sysfs_list */
+	struct list_head list;
+};
+
+static LIST_HEAD(gpiommc_sysfs_list);
+static DEFINE_MUTEX(gpiommc_sysfs_mutex);
+
+static struct gpiommc_sysfs_platform_data *gpiommc_sysfs_find_dev(const char *name)
+{
+	struct gpiommc_sysfs_platform_data *pdata;
+
+	list_for_each_entry(pdata, &gpiommc_sysfs_list, list) {
+		if (strcmp(pdata->p.name, name) == 0)
+			return pdata;
+	}
+
+	return NULL;
+}
+
+static ssize_t gpiommc_add_store(struct device_driver *drv,
+				 const char *buf, size_t count)
+{
+	int res, err;
+	struct gpiommc_sysfs_platform_data pdata_local, *pdata;
+	struct platform_device *pdev;
+	unsigned int no_spi_delay = 0, mode = 0, csactivelow = 0;
+
+	mutex_lock(&gpiommc_sysfs_mutex);
+
+	pdata = &pdata_local;
+	memset(pdata, 0, sizeof(*pdata));
+
+	err = -EINVAL;
+	res = sscanf(buf, "%" GPIOMMC_MAX_NAMELEN_STR "s %u %u %u %u %u %u %u %u",
+		     pdata->p.name,
+		     &pdata->p.pins.gpio_di,
+		     &pdata->p.pins.gpio_do,
+		     &pdata->p.pins.gpio_clk,
+		     &pdata->p.pins.gpio_cs,
+		     &mode,
+		     &pdata->p.max_bus_speed,
+		     &no_spi_delay,
+		     &csactivelow);
+	pdata->p.mode = mode;
+	pdata->p.no_spi_delay = !!no_spi_delay;
+	pdata->p.pins.cs_activelow = !!csactivelow;
+	if (res < 9)
+		pdata->p.pins.cs_activelow = 1; /* Default: CS = activelow */
+	if (res < 8)
+		pdata->p.no_spi_delay = 0; /* Default: Delay turned on */
+	if (res < 7)
+		pdata->p.max_bus_speed = 5000000; /* Default: 5Mhz */
+	if (res < 6)
+		pdata->p.mode = 0; /* Default: SPI mode 0 */
+	if (res < 5 || res > 9)
+		goto out; /* First 5 args are mandatory. */
+
+	/* Convert mode so that the SPI subsystem does understand it. */
+	switch (pdata->p.mode) {
+	case 0:
+		pdata->p.mode = SPI_MODE_0;
+		break;
+	case 1:
+		pdata->p.mode = SPI_MODE_1;
+		break;
+	case 2:
+		pdata->p.mode = SPI_MODE_2;
+		break;
+	case 3:
+		pdata->p.mode = SPI_MODE_3;
+		break;
+	default:
+		goto out; /* Invalid mode */
+	}
+
+	err = -EEXIST;
+	if (gpiommc_sysfs_find_dev(pdata->p.name))
+		goto out;
+
+	err = -ENOMEM;
+	pdev = platform_device_alloc(GPIOMMC_PLATDEV_NAME, gpiommc_next_id());
+	if (!pdev)
+		goto out;
+
+	err = platform_device_add_data(pdev, pdata, sizeof(*pdata));
+	if (err)
+		goto err_free_pdev;
+	pdata = pdev->dev.platform_data;
+
+	err = platform_device_add(pdev);
+	if (err)
+		goto err_free_pdev;
+
+	pdata->pdev = pdev;
+	INIT_LIST_HEAD(&pdata->list);
+	list_add(&pdata->list, &gpiommc_sysfs_list);
+
+	err = 0;
+out:
+	mutex_unlock(&gpiommc_sysfs_mutex);
+
+	return err ? err : count;
+
+err_free_pdev:
+	platform_device_put(pdev);
+	goto out;
+}
+
+static ssize_t gpiommc_remove_store(struct device_driver *drv,
+				    const char *buf, size_t count)
+{
+	struct gpiommc_sysfs_platform_data *pdata;
+	int err;
+
+	mutex_lock(&gpiommc_sysfs_mutex);
+
+	err = -ENODEV;
+	pdata = gpiommc_sysfs_find_dev(buf);
+	if (!pdata)
+		goto out;
+

drivers/mmc/host/gpiommc.c new file +67 -0

+		   NULL, gpiommc_add_store);
+static DRIVER_ATTR(remove, 0200,
+		   NULL, gpiommc_remove_store);
+
+static struct platform_driver gpiommc_plat_driver = {
+	.probe	= gpiommc_probe,
+	.remove	= gpiommc_remove,
+	.driver	= {
+		.name	= GPIOMMC_PLATDEV_NAME,
+		.owner	= THIS_MODULE,
+	},
+};
+
+int gpiommc_next_id(void)
+{
+	static atomic_t counter = ATOMIC_INIT(-1);
+
+	return atomic_inc_return(&counter);
+}
+EXPORT_SYMBOL(gpiommc_next_id);
+
+static int __init gpiommc_modinit(void)
+{
+	int err;
+
+	err = platform_driver_register(&gpiommc_plat_driver);
+	if (err)
+		return err;
+	err = driver_create_file(&gpiommc_plat_driver.driver,
+				 &driver_attr_add);
+	if (err)
+		goto err_drv_unreg;
+	err = driver_create_file(&gpiommc_plat_driver.driver,
+				 &driver_attr_remove);
+	if (err)
+		goto err_remove_add;
+
+	return 0;
+
+err_remove_add:
+	driver_remove_file(&gpiommc_plat_driver.driver,
+			   &driver_attr_add);
+err_drv_unreg:
+	platform_driver_unregister(&gpiommc_plat_driver);
+	return err;
+}
+module_init(gpiommc_modinit);
+
+static void __exit gpiommc_modexit(void)
+{
+	struct gpiommc_sysfs_platform_data *pdata, *pdata_tmp;
+
+	driver_remove_file(&gpiommc_plat_driver.driver,
+			   &driver_attr_remove);
+	driver_remove_file(&gpiommc_plat_driver.driver,
+			   &driver_attr_add);
+
+	mutex_lock(&gpiommc_sysfs_mutex);
+	list_for_each_entry_safe(pdata, pdata_tmp, &gpiommc_sysfs_list, list) {
+		list_del(&pdata->list);
+		platform_device_unregister(pdata->pdev);
+	}
+	mutex_unlock(&gpiommc_sysfs_mutex);
+
+	platform_driver_unregister(&gpiommc_plat_driver);
+}
+module_exit(gpiommc_modexit);

Index: linux-next/drivers/mmc/host/Kconfig
===================================================================

drivers/mmc/host/Kconfig +20 -0

@@ -164,3 +164,23 @@ config MMC_S3C
 
 	  If unsure, say N.
 
+config GPIOMMC
+	tristate "MMC/SD over GPIO-based SPI"
+	depends on MMC && MMC_SPI && SPI_GPIO
+	help
+	  This driver hooks up the mmc_spi and spi_gpio modules so that
+	  MMC/SD cards can be used on a GPIO based bus by bitbanging
+	  the SPI protocol in software.
+
+	  This driver provides a sysfs interface to dynamically create
+	  and destroy GPIO-based MMC/SD card interfaces. It also provides
+	  a platform device interface API.
+	  See Documentation/gpiommc.txt for details.
+
+	  The module will be called gpiommc.
+
+	  If unsure, say N.
+
+config MMC_S3C
+	tristate "Samsung S3C SD/MMC Card Interface support"
+	depends on ARCH_S3C2410 && MMC

Index: linux-next/include/linux/mmc/gpiommc.h
===================================================================

include/linux/mmc/gpiommc.h new file +6 -0

@@ -0,0 +1,62 @@
+/*
+ * Device driver for MMC/SD cards driven over a GPIO bus.
+ *
+ * Copyright (c) 2008 Michael Buesch
+ *
+ * Licensed under the GNU/GPL version 2.

include/linux/mmc/gpiommc.h new file +46 -0

+ * @gpio_di: The GPIO number of the DATA IN pin
+ * @gpio_do: The GPIO number of the DATA OUT pin
+ * @gpio_clk: The GPIO number of the CLOCK pin
+ * @gpio_cs: The GPIO number of the CHIPSELECT pin
+ * @cs_activelow: If true, the chip is considered selected if @gpio_cs is low.
+ */
+struct gpiommc_pins {
+	unsigned int gpio_di;
+	unsigned int gpio_do;
+	unsigned int gpio_clk;
+	unsigned int gpio_cs;
+	bool cs_activelow;
+};
+
+/** struct gpiommc_platform_data - Platform data for a MMC-over-SPI-GPIO device.
+ * @name: The unique name string of the device.
+ * @pins: The hardware pin assignments.
+ * @mode: The hardware mode. This is either SPI_MODE_0,
+ *        SPI_MODE_1, SPI_MODE_2 or SPI_MODE_3. See the SPI documentation.
+ * @no_spi_delay: Do not use delays in the lowlevel SPI bitbanging code.
+ *                This is not standards compliant, but may be required for some
+ *                embedded machines to gain reasonable speed.
+ * @max_bus_speed: The maximum speed of the SPI bus, in Hertz.
+ */
+struct gpiommc_platform_data {
+	char name[GPIOMMC_MAX_NAMELEN + 1];
+	struct gpiommc_pins pins;
+	u8 mode;
+	bool no_spi_delay;
+	unsigned int max_bus_speed;
+};
+
+/** GPIOMMC_PLATDEV_NAME - The platform device name string.
+ * The name string that has to be used for platform_device_alloc
+ * when allocating a gpiommc device.
+ */
+#define GPIOMMC_PLATDEV_NAME	"gpiommc"
+
+/** gpiommc_next_id - Get another platform device ID number.
+ * This returns the next platform device ID number that has to be used
+ * for platform_device_alloc. The ID is opaque and should not be used for
+ * anything else.
+ */
+int gpiommc_next_id(void);
+
+#endif /* LINUX_GPIOMMC_H_ */

Index: linux-next/Documentation/gpiommc.txt
===================================================================

Documentation/gpiommc.txt new file +12 -0

+
+To register a new device, define an instance of struct gpiommc_platform_data.
+This structure holds any information about how the device is hooked up to the
+GPIO pins and what hardware modes the device supports. See the docbook-style
+documentation in the header file for more information on the struct fields.
+
+Then allocate a new instance of a platform device by doing:
+
+	pdev = platform_device_alloc(GPIOMMC_PLATDEV_NAME, gpiommc_next_id());
+
+This will allocate the platform device data structures and hook it up to the
+gpiommc driver.

Documentation/gpiommc.txt new file +15 -0

+So we provide a dynamic interface to conveniently handle adding and removing
+devices from userspace, without the need to recompile the kernel.
+
+There are two sysfs files responsible for that:
+export ADD=/sys/bus/platform/drivers/gpiommc/add
+export REMOVE=/sys/bus/platform/drivers/gpiommc/remove
+
+To add a new device, simply echo the configuration string to the "add" file.
+The config string is composed out of the following elements:
+
+DEVNAME DIpin DOpin CLKpin CSpin SPIMODE MAXBUSSPEED NO_SPI_DELAY CSACTIVELOW
+
+DEVNAME is a unique name string for the device.
+DIpin is the SPI DI GPIO pin.
+DOpin is the SPI DO GPIO pin.

Quoted 13 lines

Documentation/gpiommc.txt new file +13 -0

+To add a new device, simply echo the configuration string to the "add" file.
+The config string is composed out of the following elements:
+
+DEVNAME DIpin DOpin CLKpin CSpin SPIMODE MAXBUSSPEED NO_SPI_DELAY CSACTIVELOW
+
+DEVNAME is a unique name string for the device.
+DIpin is the SPI DI GPIO pin.
+DOpin is the SPI DO GPIO pin.
+CLKpin is the SPI CLOCK GPIO pin.
+CSpin is the SPI CHIPSELECT GPIO pin.
+SPIMODE is the hardware mode the device will run at. Can be 0-3.
+MAXBUSSPEED is the maximum bus speed in Hertz.
+NO_SPI_DELAY can be 1 or 0. If it is 1, then the lowlevel SPI delay

Documentation/gpiommc.txt line 74

Quoted 5 lines

Documentation/gpiommc.txt new file +5 -0

+will not be performed. This is not standards compliant, but may be required
+to gain reasonable speeds on embedded hardware.
+CSACTIVELOW can be 1 or 0. If it is 1, the chip is considered to be selected, if CS
+is at a logical 0.
+

Would this be better done via configfs? sysfs files are supposed to be single-value files.

Thanks for the comments.

Quoted 13 lines · On Saturday 19 July 2008 00:10:37 Randy Dunlap wrote:

Quoted 13 lines

Documentation/gpiommc.txt new file +13 -0

+To add a new device, simply echo the configuration string to the "add" file.
+The config string is composed out of the following elements:
+
+DEVNAME DIpin DOpin CLKpin CSpin SPIMODE MAXBUSSPEED NO_SPI_DELAY CSACTIVELOW
+
+DEVNAME is a unique name string for the device.
+DIpin is the SPI DI GPIO pin.
+DOpin is the SPI DO GPIO pin.
+CLKpin is the SPI CLOCK GPIO pin.
+CSpin is the SPI CHIPSELECT GPIO pin.
+SPIMODE is the hardware mode the device will run at. Can be 0-3.
+MAXBUSSPEED is the maximum bus speed in Hertz.
+NO_SPI_DELAY can be 1 or 0. If it is 1, then the lowlevel SPI delay

Quoted 8 lines

Documentation/gpiommc.txt line 74

Quoted 5 lines

Documentation/gpiommc.txt new file +5 -0

+will not be performed. This is not standards compliant, but may be required
+to gain reasonable speeds on embedded hardware.
+CSACTIVELOW can be 1 or 0. If it is 1, the chip is considered to be selected, if CS
+is at a logical 0.
+

Would this be better done via configfs? sysfs files are supposed to be single-value files.

Well, I really want to avoid over-engineering this thing.
I thought about using configfs, but that would require to keep lots of state information across the operations.
So one would have to allocate a device with mkdir. Then configure the parameters. And then somehow tell the kernel to register it. State has to be maintained over this time and I'm not sure how that "register" operation would look like. Writing a "1" to a "register" file? So why not write all config parameters to an "add" file and be done with all the stuff. ;)

It all depends on how you define "one thing per file", IMO.
This "add" file does one thing. It creates a device. We must, of course, pass some configuration parameters, too.

Quoted 35 lines · Michael Buesch

Thanks for the comments.

Quoted 13 lines · On Saturday 19 July 2008 00:10:37 Randy Dunlap wrote:

Quoted 13 lines

Documentation/gpiommc.txt new file +13 -0

+To add a new device, simply echo the configuration string to the "add" file.
+The config string is composed out of the following elements:
+
+DEVNAME DIpin DOpin CLKpin CSpin SPIMODE MAXBUSSPEED NO_SPI_DELAY CSACTIVELOW
+
+DEVNAME is a unique name string for the device.
+DIpin is the SPI DI GPIO pin.
+DOpin is the SPI DO GPIO pin.
+CLKpin is the SPI CLOCK GPIO pin.
+CSpin is the SPI CHIPSELECT GPIO pin.
+SPIMODE is the hardware mode the device will run at. Can be 0-3.
+MAXBUSSPEED is the maximum bus speed in Hertz.
+NO_SPI_DELAY can be 1 or 0. If it is 1, then the lowlevel SPI delay

Quoted 8 lines

Documentation/gpiommc.txt line 74

Quoted 5 lines

Documentation/gpiommc.txt new file +5 -0

+will not be performed. This is not standards compliant, but may be required
+to gain reasonable speeds on embedded hardware.
+CSACTIVELOW can be 1 or 0. If it is 1, the chip is considered to be selected, if CS
+is at a logical 0.
+

Would this be better done via configfs? sysfs files are supposed to be single-value files.

Well, I really want to avoid over-engineering this thing.
I thought about using configfs, but that would require to keep lots of state information across the operations.
So one would have to allocate a device with mkdir. Then configure the parameters. And then somehow tell the kernel to register it. State has to be maintained over this time and I'm not sure how that "register" operation would look like. Writing a "1" to a "register" file? So why not write all config parameters to an "add" file and be done with all the stuff. ;)

Quoted 3 lines

It all depends on how you define "one thing per file", IMO.
This "add" file does one thing. It creates a device. We must, of course, pass some configuration parameters, too.

We have precidence with this for the device id being "added" to a driver at run time in PCI and USB. Not that I really like this, I do think configfs would be better to do this with here instead of sysfs.

That being said, as this is a new sysfs file, please break this documentation up into a file in the Documentation/ABI directory instead of here.

drivers/mmc/host/gpiommc.c line 2

Quoted 5 lines · On Fri, 18 Jul 2008 22:01:33 +0200 Michael Buesch wrote:

drivers/mmc/host/gpiommc.c new file +2 -0

@@ -0,0 +1,328 @@
+/*
+ * Driver an MMC/SD card on a bitbanging GPIO SPI bus.

drivers/mmc/host/gpiommc.c line 4

Quoted 2 lines

drivers/mmc/host/gpiommc.c new file +2 -0

+ * This module hooks up the mmc_spi and spi_gpio modules and also
+ * provides a sysfs interface.

drivers/mmc/host/gpiommc.c line 164

Quoted 1 line

drivers/mmc/host/gpiommc.c new file +1 -0

+	res = sscanf(buf, "%" GPIOMMC_MAX_NAMELEN_STR "s %u %u %u %u %u %u %u %u",

drivers/mmc/host/gpiommc.c line 167

Quoted 3 lines

drivers/mmc/host/gpiommc.c new file +3 -0

+		     pdata->p.name,
+		     &pdata->p.pins.gpio_di,
+		     &pdata->p.pins.gpio_do,

drivers/mmc/host/gpiommc.c line 178

Quoted 11 lines

drivers/mmc/host/gpiommc.c new file +11 -0

+		     &pdata->p.pins.gpio_clk,
+		     &pdata->p.pins.gpio_cs,
+		     &mode,
+		     &pdata->p.max_bus_speed,
+		     &no_spi_delay,
+		     &csactivelow);
+	pdata->p.mode = mode;
+	pdata->p.no_spi_delay = !!no_spi_delay;
+	pdata->p.pins.cs_activelow = !!csactivelow;
+	if (res < 9)
+		pdata->p.pins.cs_activelow = 1; /* Default: CS = activelow */

drivers/mmc/host/gpiommc.c line 182

Quoted 4 lines

drivers/mmc/host/gpiommc.c new file +4 -0

+	if (res < 8)
+		pdata->p.no_spi_delay = 0; /* Default: Delay turned on */
+	if (res < 7)
+		pdata->p.max_bus_speed = 5000000; /* Default: 5Mhz */

drivers/mmc/host/gpiommc.c line 184

Quoted 2 lines

drivers/mmc/host/gpiommc.c new file +2 -0

+	if (res < 6)
+		pdata->p.mode = 0; /* Default: SPI mode 0 */

drivers/mmc/host/Kconfig config MMC_S3C · line 168

Quoted 8 lines

drivers/mmc/host/Kconfig +2 -0

@@ -164,3 +164,23 @@ config MMC_S3C
 
 	  If unsure, say N.
 
+config GPIOMMC
+	tristate "MMC/SD over GPIO-based SPI"

drivers/mmc/host/Kconfig config MMC_S3C · line 177

Quoted 9 lines

drivers/mmc/host/Kconfig +9 -0

+	depends on MMC && MMC_SPI && SPI_GPIO
+	help
+	  This driver hooks up the mmc_spi and spi_gpio modules so that
+	  MMC/SD cards can be used on a GPIO based bus by bitbanging
+	  the SPI protocol in software.
+
+	  This driver provides a sysfs interface to dynamically create
+	  and destroy GPIO-based MMC/SD card interfaces. It also provides
+	  a platform device interface API.

drivers/mmc/host/Kconfig config MMC_S3C · line 186

Quoted 9 lines

drivers/mmc/host/Kconfig +9 -0

+	  See Documentation/gpiommc.txt for details.
+
+	  The module will be called gpiommc.
+
+	  If unsure, say N.
+
+config MMC_S3C
+	tristate "Samsung S3C SD/MMC Card Interface support"
+	depends on ARCH_S3C2410 && MMC

Quoted 3 lines

+ * @no_spi_delay: Do not use delays in the lowlevel SPI bitbanging code.
+ * This is not standards compliant, but may be required for some
+ * embedded machines to gain reasonable speed.

Documentation/gpiommc.txt

Quoted 4 lines

Index: linux-next/Documentation/gpiommc.txt
===================================================================

Documentation/gpiommc.txt new file +0 -0

Documentation/gpiommc.txt line 7

Quoted 8 lines

Documentation/gpiommc.txt new file +7 -0

@@ -0,0 +1,96 @@
+GPIOMMC - Driver for an MMC/SD card on a bitbanging GPIO SPI bus
+================================================================
+
+The gpiommc module hooks up the mmc_spi and spi_gpio modules for running an
+MMC or SD card on GPIO pins.
+
+Two interfaces for registering a new MMC/SD card device are provided.

are provided:
a static

Quoted 1 line

Documentation/gpiommc.txt new file +1 -0

+A static platform-device based mechanism and a dynamic sysfs based interface.

Documentation/gpiommc.txt line 16

Quoted 8 lines

Documentation/gpiommc.txt new file +8 -0

+
+
+Registering devices via platform-device
+=======================================
+
+The platform-device interface is used for registering MMC/SD devices that are
+part of the hardware platform. This is most useful only for embedded machines
+with MMC/SD devices statically connected to the platform GPIO bus.

Documentation/gpiommc.txt line 18

Quoted 1 line

Documentation/gpiommc.txt new file +1 -0

+The data structures are declared in <linux/mmc/gpiommc.h>

Documentation/gpiommc.txt line 45

Quoted 2 lines

Documentation/gpiommc.txt new file +2 -0

+Registering devices via sysfs
+=============================

Documentation/gpiommc.txt line 49

Quoted 4 lines

Documentation/gpiommc.txt new file +4 -0

+
+MMC/SD cards connected via GPIO often are a pretty dynamic thing. For example
+selfmade hacks for soldering an MMC/SD card to standard GPIO pins on embedded
+hardware are a common situation.

I think that the two sentences above/below would be better if joined with a comma...

[ I see "v3" switches to configfs ... same comments apply. ]

Quoted 4 lines

Documentation/gpiommc.txt

Quoted 4 lines

Index: linux-next/Documentation/gpiommc.txt
===================================================================

Documentation/gpiommc.txt new file +0 -0

Should there be a Documentation/mmc/... for such stuff, instead?

The toplevel Documentation directory is getting unmanageable.

Quoted 1 line

So what this driver adds is mostly a userspace interface, yes?

Mostly. It also provides a simple platform device.

Quoted 5 lines

Please switch to the more conventional "MOSI" (Master Out, Slave In) and "MISO" (Master In, Slave Out). That's unambiguous. If you want to be a bit more clear you could say that MOSI goes to the MMC/SD card pin 2 (card data in), and MISO goes to MMC/SD card pin 7 (card data out).

Maybe.

Quoted 8 lines

Quoted 5 lines

drivers/mmc/host/gpiommc.c line 182

Quoted 5 lines

drivers/mmc/host/gpiommc.c new file +4 -0

+	if (res < 8)
+		pdata->p.no_spi_delay = 0; /* Default: Delay turned on */
+	if (res < 7)
+		pdata->p.max_bus_speed = 5000000; /* Default: 5Mhz */

So I'd default to leaving the delay off

How to?

Quoted 5 lines

Quoted 2 lines

drivers/mmc/host/gpiommc.c line 184

Quoted 2 lines

drivers/mmc/host/gpiommc.c new file +2 -0

+	if (res < 6)
+		pdata->p.mode = 0; /* Default: SPI mode 0 */

Don't need to do this. The mmc_spi driver forces mode 0 in any case...

Where's that documented?

Quoted 7 lines

Quoted 2 lines

drivers/mmc/host/Kconfig config MMC_S3C · line 168

Quoted 2 lines

drivers/mmc/host/Kconfig +2 -0

+config GPIOMMC
+	tristate "MMC/SD over GPIO-based SPI"

"Sysfs interface for ..."

Because we can already do MMC/SD over GPIO-SPI,
without using this code.

This also adds a platform device interface for convenience (as it also uses that internally).

Quoted 7 lines

Quoted 3 lines

Quoted 3 lines

+ * @no_spi_delay: Do not use delays in the lowlevel SPI bitbanging code.
+ * This is not standards compliant, but may be required for some
+ * embedded machines to gain reasonable speed.

Rather than emphasize "not standards compliant", I'd instead emphasize that bitbanged SPI is so slow that you probably don't want to slow it down any more by additional delays between per-bit operations!

I'd say you cannot tell how fast the GPIOs are. They can be pretty fast on a huge machine.

Quoted 1 line

Again, please don't promote a *SECOND* platform-device based mechanism

It is not a second mechanism, it is a mechanism on top of the first two mechanisms.

Quoted 8 lines

Quoted 6 lines

Documentation/gpiommc.txt line 16

Quoted 6 lines

Documentation/gpiommc.txt new file +6 -0

+Registering devices via platform-device
+=======================================
+
+The platform-device interface is used for registering MMC/SD devices that are
+part of the hardware platform. This is most useful only for embedded machines
+with MMC/SD devices statically connected to the platform GPIO bus.

There is no "platform GPIO bus" ...

hm??

Quoted 1 line

This is the interface I'd rather just not see exposed ...

People asked me to expose it. It was hidden in the first patch that I submitted. I'm not going to change this just to see the next one yelling "I want to see this interface in public" again.

Quoted 5 lines

Quoted 3 lines

Quoted 3 lines

+Registering devices via sysfs
+=============================

This is the interface I don't have any particular issues with.

Yeah, but it's gone. See updated patches.

You are not allowed to post to this mailing list, and your message has been automatically rejected.  If you think that your messages are being rejected in error, contact the mailing list owner at [email protected].

Quoted 1 line

How to?

Simplest to just rip out the support for using one! And in any case:
as a rule, any delay should be a function of the target bus speed.

Quoted 1 line

Where's that documented?

UTSL ... mmc_spi_probe(), first thing. :)

Don't export that. It's a (less expressive) subset of the standard way to do that.

Quoted 4 lines

Quoted 2 lines

There is no "platform GPIO bus" ...

hm??

If you look in /sys/bus you won't see GPIO. It's not a bus.

Quoted 3 lines

People asked me to expose it. It was hidden in the first patch that I submitted. I'm not going to change this just to see the next one yelling "I want to see this interface in public" again.

Could you send me some URLs for that feedback?

Quoted 4 lines

Quoted 1 line

How to?

Simplest to just rip out the support for using one! And in any case:
as a rule, any delay should be a function of the target bus speed.

Well, that wasn't exactly an answer to my question. ;) So setting boardinfo->max_speed_hz to zero will do?

Quoted 3 lines

Quoted 1 line

Where's that documented?

UTSL ... mmc_spi_probe(), first thing. :)

[...]

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

There is no "platform GPIO bus" ...

hm??

If you look in /sys/bus you won't see GPIO. It's not a bus.

Oh well... This is getting boring...
Then call it "platform GPIO pins" or "platform GPIO controller" or whatever. That's really just splitting the hair into 16 parts.

Quoted 5 lines

Quoted 3 lines

People asked me to expose it. It was hidden in the first patch that I submitted. I'm not going to change this just to see the next one yelling "I want to see this interface in public" again.

Could you send me some URLs for that feedback?

It was private. One of the OpenWRT guys requested that to have a convenient way to define a hardwired MMC card in the platform code by just defining a simple data structure.

So what this driver adds is mostly a userspace interface, yes?

If so, please refocus the descriptions a bit more on that; it's not actually a "GPIO based MMC/SD driver"; it's really a "Configfs creation interface for GPIO based MMC/SD".

Quoted 1 line

drivers/mmc/host/gpiommc.c line 164

Quoted 1 line

drivers/mmc/host/gpiommc.c new file +1 -0

+	res = sscanf(buf, "%" GPIOMMC_MAX_NAMELEN_STR "s %u %u %u %u %u %u %u %u",

Kind of ugly, yes? ;)

Please switch to the more conventional "MOSI" (Master Out, Slave In) and "MISO" (Master In, Slave Out). That's unambiguous. If you want to be a bit more clear you could say that MOSI goes to the MMC/SD card pin 2 (card data in), and MISO goes to MMC/SD card pin 7 (card data out).

Quoted 4 lines

drivers/mmc/host/gpiommc.c line 182

Quoted 4 lines

drivers/mmc/host/gpiommc.c new file +4 -0

+	if (res < 8)
+		pdata->p.no_spi_delay = 0; /* Default: Delay turned on */
+	if (res < 7)
+		pdata->p.max_bus_speed = 5000000; /* Default: 5Mhz */

So I'd default to leaving the delay off

Quoted 2 lines

drivers/mmc/host/gpiommc.c line 184

Quoted 2 lines

drivers/mmc/host/gpiommc.c new file +2 -0

+	if (res < 6)
+		pdata->p.mode = 0; /* Default: SPI mode 0 */

Don't need to do this. The mmc_spi driver forces mode 0 in any case...

Quoted 2 lines

drivers/mmc/host/Kconfig config MMC_S3C · line 168

Quoted 2 lines

drivers/mmc/host/Kconfig +2 -0

+config GPIOMMC
+	tristate "MMC/SD over GPIO-based SPI"

"Sysfs interface for ..."

Because we can already do MMC/SD over GPIO-SPI,
without using this code.

Quoted 3 lines

drivers/mmc/host/Kconfig config MMC_S3C · line 186

Quoted 3 lines

drivers/mmc/host/Kconfig +3 -0

+config MMC_S3C
+	tristate "Samsung S3C SD/MMC Card Interface support"
+	depends on ARCH_S3C2410 && MMC

MMC_S3C doesn't belong in this patch...

Quoted 3 lines

Quoted 3 lines

+ * @no_spi_delay: Do not use delays in the lowlevel SPI bitbanging code.
+ * This is not standards compliant, but may be required for some
+ * embedded machines to gain reasonable speed.

Rather than emphasize "not standards compliant", I'd instead emphasize that bitbanged SPI is so slow that you probably don't want to slow it down any more by additional delays between per-bit operations!

Again, please don't promote a *SECOND* platform-device based mechanism

Quoted 6 lines

Documentation/gpiommc.txt line 16

Quoted 6 lines

Documentation/gpiommc.txt new file +6 -0

+Registering devices via platform-device
+=======================================
+
+The platform-device interface is used for registering MMC/SD devices that are
+part of the hardware platform. This is most useful only for embedded machines
+with MMC/SD devices statically connected to the platform GPIO bus.

There is no "platform GPIO bus" ...

This is the interface I'd rather just not see exposed ...

Quoted 2 lines

Quoted 2 lines

+Registering devices via sysfs
+=============================

This is the interface I don't have any particular issues with.