Add dynamic MMC-over-SPI-GPIO driver
PATCH 1/1 Add dynamic MMC-over-SPI-GPIO driver
This driver provides a sysfs interface to dynamically create and destroy GPIO-based MMC/SD card interfaces.
So an MMC or SD card can be connected to generic GPIO pins and be configured dynamically from userspace.
---
This driver is used in OpenWrt since quite some time, so please consider for inclusion in mainline.
See the attached OpenWrt initscript for an example on how to use the sysfs interface. Documentation should also be added. I'll submit a patch for that, later.
drivers/mmc/host/mmc_over_spigpio.c new file +339 -0
@@ -0,0 +1,339 @@ +/* +* Driver for driving an MMC card over a bitbanging GPIO SPI bus. +* +* Copyright 2008 Michael Buesch <[email protected]> +* +* Licensed under the GNU/GPL. See COPYING for details. +*/ + +#include <linux/platform_device.h> +#include <linux/list.h> +#include <linux/mutex.h> +#include <linux/spi/spi_gpio.h> + + +/* This is the maximum speed in Hz */ +#define GPIOMMC_MAXSPEED 5000000/* Hz */ + + +#define DRIVER_NAME "spi-gpio-mmc" +#define PFX DRIVER_NAME ": " + + +#define GPIOMMC_MAX_NAMELEN 15 +#define GPIOMMC_MAX_NAMELEN_STR __stringify(GPIOMMC_MAX_NAMELEN) + +struct gpiommc_pins { +unsigned int gpio_di;/* Card DI pin */ +unsigned int gpio_do;/* Card DO pin */ +unsigned int gpio_clk;/* Card CLK pin */ +unsigned int gpio_cs;/* Card CS pin */ +}; + +struct gpiommc_device { +char name[GPIOMMC_MAX_NAMELEN +1 ]; +struct platform_device *pdev; +struct platform_device *spi_pdev; +struct gpiommc_pins pins; +u8 mode;/* SPI_MODE_X */ +struct spi_board_info boardinfo; + +struct list_head list; +}; + + +static LIST_HEAD (gpiommc_devices_list); +static DEFINE_MUTEX (gpiommc_mutex); + + +MODULE_DESCRIPTION ("SPI-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; + +/* Bind the SPI master to the MMC-SPI host driver. */ +strlcpy (bi->modalias,"mmc_spi" ,sizeof (bi->modalias)); + + bi->max_speed_hz =GPIOMMC_MAXSPEED ; + bi->bus_num = master->bus_num; + bi->mode = d->mode; + +return 0 ; +} + +static int gpiommc_probe (struct platform_device *pdev) +{ +static int instance; +struct gpiommc_device *d =platform_get_drvdata (pdev); +struct spi_gpio_platform_data pdata; +int err = -ENOMEM ; + + d->spi_pdev =platform_device_alloc ("spi-gpio" , instance++); +if (!d->spi_pdev) + goto out; + +memset (&pdata,0 ,sizeof (pdata)); + pdata.pin_clk = d->pins.gpio_clk; + pdata.pin_miso = d->pins.gpio_do; + pdata.pin_mosi = d->pins.gpio_di; + pdata.pin_cs = d->pins.gpio_cs; + pdata.cs_activelow =1 ; + pdata.no_spi_delay =1 ; + pdata.boardinfo_setup = gpiommc_boardinfo_setup; + pdata.boardinfo_setup_data = d; + + err =platform_device_add_data (d->spi_pdev, &pdata,sizeof (pdata)); +if (err) + goto err_free_pdev; + err =platform_device_register (d->spi_pdev); +if (err) + goto err_free_pdata; + + printk(KERN_INFO PFX "MMC-Card \"%s\" " +"attached to GPIO pins %u,%u,%u,%u\n" , + d->name, d->pins.gpio_di, d->pins.gpio_do, + d->pins.gpio_clk, d->pins.gpio_cs); +out: +return err; + +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); +return err; +} + +static int gpiommc_remove (struct platform_device *pdev) +{ +struct gpiommc_device *d =platform_get_drvdata (pdev); + +platform_device_unregister (d->spi_pdev); + printk(KERN_INFO PFX "MMC-Card \"%s\" removed\n", d->name); + +return 0 ; +} + +static void gpiommc_free (struct gpiommc_device *d) +{ +kfree (d); +} + +static struct gpiommc_device *gpiommc_alloc (struct platform_device *pdev, +const char *name, +const struct gpiommc_pins *pins, +u8 mode) +{ +struct gpiommc_device *d; + + d =kmalloc (sizeof (*d),GFP_KERNEL ); +if (!d) +return NULL ; + +strcpy (d->name, name); +memcpy (&d->pins, pins,sizeof (d->pins)); + d->mode = mode; +INIT_LIST_HEAD (&d->list); + +return d; +} + +/* List must be locked. */ +static struct gpiommc_device *gpiommc_find_device (const char *name) +{ +struct gpiommc_device *d; + +list_for_each_entry (d, &gpiommc_devices_list, list) { +if (strcmp (d->name, name) ==0 ) +return d; + } + +return NULL ; +} + +static void gpiommc_do_destroy_device (struct gpiommc_device *d) +{ +list_del (&d->list); +platform_device_unregister (d->pdev); +gpiommc_free (d); +} + +static int gpiommc_destroy_device (const char *name) +{ +struct gpiommc_device *d; +int err = -ENODEV ; + +mutex_lock (&gpiommc_mutex); + d =gpiommc_find_device (name); +if (!d) + goto out_unlock; +gpiommc_do_destroy_device (d); + err =0 ; +out_unlock: +mutex_unlock (&gpiommc_mutex); + +return err; +} + +static int gpiommc_create_device (const char *name, +const struct gpiommc_pins *pins, +u8 mode) +{ +static int instance; +struct platform_device *pdev; +struct gpiommc_device *d; +int err; + +mutex_lock (&gpiommc_mutex); + err = -EEXIST ; +if (gpiommc_find_device (name)) + goto out_unlock; + err = -ENOMEM ; + pdev =platform_device_alloc (DRIVER_NAME , instance++); +if (!pdev) + goto out_unlock; + d =gpiommc_alloc (pdev, name, pins, mode); +if (!d) + goto err_free_pdev; +platform_set_drvdata (pdev, d); + d->pdev = pdev; + err =platform_device_register (pdev); +if (err) + goto err_free_mdev; +list_add (&d->list, &gpiommc_devices_list); + + err =0 ; +out_unlock: +mutex_unlock (&gpiommc_mutex); + +return err; + +err_free_mdev: +gpiommc_free (d); +err_free_pdev: +platform_device_put (pdev); + goto out_unlock; +} + +static ssize_t gpiommc_add_show (struct device_driver *drv, +char *buf) +{ +return snprintf (buf,PAGE_SIZE ,"NAME DI_pin,DO_pin,CLK_pin,CS_pin [MODE]\n" ); +} + +static ssize_t gpiommc_add_store (struct device_driver *drv, +const char *buf,size_t count) +{ +int res, err; +char name[GPIOMMC_MAX_NAMELEN +1 ]; +struct gpiommc_pins pins; +unsigned int mode; + + res =sscanf (buf,"%" GPIOMMC_MAX_NAMELEN_STR "s %u,%u,%u,%u %u" , + name, &pins.gpio_di, &pins.gpio_do, + &pins.gpio_clk, &pins.gpio_cs, &mode); +if (res ==5 ) + mode =0 ; +else if (res !=6 ) +return -EINVAL ; +switch (mode) { +case 0 : + mode =SPI_MODE_0 ; +break ; +case 1 : + mode =SPI_MODE_1 ; +break ; +case 2 : + mode =SPI_MODE_2 ; +break ; +case 3 : + mode =SPI_MODE_3 ; +break ; +default : +return -EINVAL ; + } + err =gpiommc_create_device (name, &pins, mode); + +return err ? err : count; +} + +static ssize_t gpiommc_remove_show (struct device_driver *drv, +char *buf) +{ +return snprintf (buf,PAGE_SIZE ,"write device-name to remove the device\n" ); +} + +static ssize_t gpiommc_remove_store (struct device_driver *drv, +const char *buf,size_t count) +{ +int err; + + err =gpiommc_destroy_device (buf); + +return err ? err : count; +} + +static DRIVER_ATTR(add, 0600, + gpiommc_add_show, gpiommc_add_store); +static DRIVER_ATTR(remove, 0600, + gpiommc_remove_show, gpiommc_remove_store); + +static struct platform_driver gpiommc_plat_driver = { + .probe = gpiommc_probe, + .remove = gpiommc_remove, + .driver = { + .name =DRIVER_NAME , + .owner =THIS_MODULE , + }, +}; + +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_device *d, *tmp; + +driver_remove_file (&gpiommc_plat_driver.driver, + &driver_attr_remove); +driver_remove_file (&gpiommc_plat_driver.driver, + &driver_attr_add); + +mutex_lock (&gpiommc_mutex); +list_for_each_entry_safe (d, tmp, &gpiommc_devices_list, list) +gpiommc_do_destroy_device (d); +mutex_unlock (&gpiommc_mutex); + +platform_driver_unregister (&gpiommc_plat_driver); +} +module_exit (gpiommc_modexit);
drivers/mmc/host/Kconfig +12 -0
@@ -153,6 +153,18 @@ config MMC_SPI If unsure, or if your system has no SPI master driver, say N. +config MMC_OVER_SPIGPIO + tristate "MMC/SD over GPIO-based SPI" + depends on MMC && MMC_SPI && SPI_GPIO + help + This driver provides a sysfs interface to dynamically create + and destroy GPIO-based MMC/SD card interfaces. + So an MMC or SD card can be connected to generic GPIO pins + and be configured dynamically from userspace. + The module will be called mmc_over_spigpio. + + If unsure, say N. + config MMC_S3C tristate "Samsung S3C SD/MMC Card Interface support" depends on ARCH_S3C2410 && MMC
drivers/mmc/host/Makefile +1 -0
@@ -20,3 +20,4 @@ obj-$(CONFIG_MMC_ATMELMCI) += atmel-mci. obj-$(CONFIG_MMC_TIFM_SD) += tifm_sd.o obj-$(CONFIG_MMC_SPI) += mmc_spi.o obj-$(CONFIG_MMC_S3C) += s3cmci.o +obj-$(CONFIG_MMC_OVER_SPIGPIO) += mmc_over_spigpio.o
Signature · 1 line
Greetings Michael.
Quoted 18 lines · On Mon, 14 Jul 2008 21:09:18 +0200 Michael Buesch <[email protected]> wrote:
This driver provides a sysfs interface to dynamically create and destroy GPIO-based MMC/SD card interfaces.
So an MMC or SD card can be connected to generic GPIO pins and be configured dynamically from userspace.Signed-off-by Michael Buesch <[email protected]>---
This driver is used in OpenWrt since quite some time, so please consider for inclusion in mainline.
See the attached OpenWrt initscript for an example on how to use the sysfs interface. Documentation should also be added. I'll submit a patch for that, later.
...
drivers/mmc/host/mmc_over_spigpio.c line 79
Quoted 10 lines
drivers/mmc/host/mmc_over_spigpio.c new file +10
-0+static int gpiommc_probe (struct platform_device *pdev) +{ +static int instance; +struct gpiommc_device *d =platform_get_drvdata (pdev); +struct spi_gpio_platform_data pdata; +int err = -ENOMEM ; + + d->spi_pdev =platform_device_alloc ("spi-gpio" , instance++); +if (!d->spi_pdev) + goto out;
I guess that incrementing `instance' even if the allocation failed is somewhat wrong.
Quoted 37 lines
drivers/mmc/host/mmc_over_spigpio.c new file +34
-0+memset (&pdata,0 ,sizeof (pdata)); + pdata.pin_clk = d->pins.gpio_clk; + pdata.pin_miso = d->pins.gpio_do; + pdata.pin_mosi = d->pins.gpio_di; + pdata.pin_cs = d->pins.gpio_cs; + pdata.cs_activelow =1 ; + pdata.no_spi_delay =1 ; + pdata.boardinfo_setup = gpiommc_boardinfo_setup; + pdata.boardinfo_setup_data = d; + + err =platform_device_add_data (d->spi_pdev, &pdata,sizeof (pdata)); +if (err) + goto err_free_pdev; + err =platform_device_register (d->spi_pdev); +if (err) + goto err_free_pdata; + + printk(KERN_INFO PFX "MMC-Card \"%s\" " +"attached to GPIO pins %u,%u,%u,%u\n" , + d->name, d->pins.gpio_di, d->pins.gpio_do, + d->pins.gpio_clk, d->pins.gpio_cs); +out: +return err; + +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); +return err; +} + +static struct gpiommc_device *gpiommc_alloc(struct platform_device *pdev, + const char *name,
drivers/mmc/host/mmc_over_spigpio.c line 139
Quoted 10 lines
drivers/mmc/host/mmc_over_spigpio.c new file +10
-0+ const struct gpiommc_pins *pins, + u8 mode) +{ +struct gpiommc_device *d; + + d =kmalloc (sizeof (*d),GFP_KERNEL ); +if (!d) +return NULL ; + +strcpy (d->name, name);
No check for overruns?
drivers/mmc/host/mmc_over_spigpio.c line 140
Quoted 1 line
drivers/mmc/host/mmc_over_spigpio.c new file +1
-0+memcpy (&d->pins, pins,sizeof (d->pins));
If this had used the typesafe
d->pins = *pins;
I wouldn't have needed to run all around the place working out if overflow/underflow checks were needed here.
Quoted 10 lines
drivers/mmc/host/mmc_over_spigpio.c new file +7
-0+ d->mode = mode; +INIT_LIST_HEAD (&d->list); + +return d; +} + +static ssize_t gpiommc_add_store(struct device_driver *drv,
drivers/mmc/host/mmc_over_spigpio.c line 240
Quoted 10 lines
drivers/mmc/host/mmc_over_spigpio.c new file +10
-0+ const char *buf, size_t count) +{ + int res, err; +char name[GPIOMMC_MAX_NAMELEN +1 ]; +struct gpiommc_pins pins; +unsigned int mode; + + res =sscanf (buf,"%" GPIOMMC_MAX_NAMELEN_STR "s %u,%u,%u,%u %u" , + name, &pins.gpio_di, &pins.gpio_do, + &pins.gpio_clk, &pins.gpio_cs, &mode);
What's going on here? So new kernel/userspace ABI. Not documented in changelog, not documented in code comments, not documented in Documentation/ABI. This forces reviewers to reverse-engineer the interface design from the implementation and then attempt to review that design. Reviewers not happy!
Userspace interfaces are the things which we care about most, because they are the things which we can never change. Please document them prominently.
Quoted 20 lines
drivers/mmc/host/mmc_over_spigpio.c new file +20
-0+if (res ==5 ) + mode =0 ; +else if (res !=6 ) +return -EINVAL ; +switch (mode) { +case 0 : + mode =SPI_MODE_0 ; +break ; +case 1 : + mode =SPI_MODE_1 ; +break ; +case 2 : + mode =SPI_MODE_2 ; +break ; +case 3 : + mode =SPI_MODE_3 ; +break ; +default : +return -EINVAL ; + }
drivers/mmc/host/mmc_over_spigpio.c line 270
Quoted 10 lines
drivers/mmc/host/mmc_over_spigpio.c new file +10
-0+ err =gpiommc_create_device (name, &pins, mode); + +return err ? err : count; +} + +static ssize_t gpiommc_remove_show (struct device_driver *drv, +char *buf) +{ +return snprintf (buf,PAGE_SIZE ,"write device-name to remove the device\n" ); +}
Now that is one weird way in which to document the interface! What a waste of kernel text :(
Quoted 14 lines
drivers/mmc/host/mmc_over_spigpio.c new file +14
-0+static ssize_t gpiommc_remove_store (struct device_driver *drv, +const char *buf,size_t count) +{ +int err; + + err =gpiommc_destroy_device (buf); + +return err ? err : count; +} + +static DRIVER_ATTR(add, 0600, + gpiommc_add_show, gpiommc_add_store); +static DRIVER_ATTR(remove, 0600, + gpiommc_remove_show, gpiommc_remove_store);
drivers/mmc/host/mmc_over_spigpio.c line 295
Quoted 10 lines
drivers/mmc/host/mmc_over_spigpio.c new file +10
-0+ +static struct platform_driver gpiommc_plat_driver = { + .probe = gpiommc_probe, + .remove = gpiommc_remove, + .driver = { + .name =DRIVER_NAME , + .owner =THIS_MODULE , + }, +}; +
I'll skip this, pending suitable documentation of the proposed interface design, and review of that design.
Quoted 5 lines · On Monday 14 July 2008 22:54:41 Andrew Morton wrote:
Quoted 5 lines
drivers/mmc/host/mmc_over_spigpio.c new file +5
-0+static int gpiommc_probe (struct platform_device *pdev) +{ +static int instance; +struct gpiommc_device *d =platform_get_drvdata (pdev); +struct spi_gpio_platform_data pdata;
Quoted 8 lines
drivers/mmc/host/mmc_over_spigpio.c line 79
Quoted 5 lines
drivers/mmc/host/mmc_over_spigpio.c new file +5
-0+int err = -ENOMEM ; + + d->spi_pdev =platform_device_alloc ("spi-gpio" , instance++); +if (!d->spi_pdev) + goto out;I guess that incrementing `instance' even if the allocation failed is somewhat wrong.
Well, I guess it doesn't matter much. The number is pretty random anyway.
Quoted 6 lines
Quoted 6 lines
drivers/mmc/host/mmc_over_spigpio.c new file +6
-0+static struct gpiommc_device *gpiommc_alloc (struct platform_device *pdev, +const char *name, +const struct gpiommc_pins *pins, +u8 mode) +{ +struct gpiommc_device *d;
Quoted 8 lines
drivers/mmc/host/mmc_over_spigpio.c line 139
Quoted 6 lines
drivers/mmc/host/mmc_over_spigpio.c new file +6
-0+ + d =kmalloc (sizeof (*d),GFP_KERNEL ); +if (!d) +return NULL ; + +strcpy (d->name, name);No check for overruns?
The caller checks the length, but it is a good idea to double-check here. Good catch.
Quoted 8 lines
drivers/mmc/host/mmc_over_spigpio.c line 140
Quoted 1 line
drivers/mmc/host/mmc_over_spigpio.c new file +1
-0+memcpy (&d->pins, pins,sizeof (d->pins));If this had used the typesafe
d->pins = *pins;I wouldn't have needed to run all around the place working out if overflow/underflow checks were needed here.
Yeah well, can use this.
Quoted 5 lines
Quoted 5 lines
drivers/mmc/host/mmc_over_spigpio.c new file +5
-0+static ssize_t gpiommc_add_store (struct device_driver *drv, +const char *buf,size_t count) +{ +int res, err; +char name[GPIOMMC_MAX_NAMELEN +1 ];
Quoted 8 lines
drivers/mmc/host/mmc_over_spigpio.c line 240
Quoted 6 lines
drivers/mmc/host/mmc_over_spigpio.c new file +6
-0+struct gpiommc_pins pins; +unsigned int mode; + + res =sscanf (buf,"%" GPIOMMC_MAX_NAMELEN_STR "s %u,%u,%u,%u %u" , + name, &pins.gpio_di, &pins.gpio_do, + &pins.gpio_clk, &pins.gpio_cs, &mode);What's going on here? So new kernel/userspace ABI.
The whole point of the module is to create a new userspace interface for creating the device. The module does just glue several modules together and create an actual device.
Quoted 5 lines
Not documented in
changelog, not documented in code comments, not documented inDocumentation/ABI. This forces reviewers to reverse-engineer the interface design from the implementation and then attempt to review that design. Reviewers not happy!
Yeah well, as I said, I will do docs later. I didn't have any time to write documentation, yet.
by. ;)
Quoted 8 lines
drivers/mmc/host/mmc_over_spigpio.c line 270
Quoted 5 lines
drivers/mmc/host/mmc_over_spigpio.c new file +5
-0+static ssize_t gpiommc_remove_show (struct device_driver *drv, +char *buf) +{ +return snprintf (buf,PAGE_SIZE ,"write device-name to remove the device\n" ); +}Now that is one weird way in which to document the interface! What a waste of kernel text :(
Yeah, well. Better than nothing ;)
As I already said in the original patch announcement. This is by no way the final version of the patch. Docs will be moved to Documentation/
Signature · 1 line
Greetings Michael.
Quoted 4 lines · Michael Buesch
This driver provides a sysfs interface to dynamically create and destroy GPIO-based MMC/SD card interfaces.
So an MMC or SD card can be connected to generic GPIO pins and be configured dynamically from userspace.
Can you use mmc-spi attached to spi-gpio and be done with it? Sure you won't have the dynamic alloc capability but you won't be reinventing the wheel either. You don't have dynamic creation for any other platform device and if you need it then, IMO, it'd be better solved in a more generic way.
btw, what's this spi-gpio thing? I can't see it in mainline except in a s3c24xx specific way. My domestic blindness??
--Ben.
Quoted 8 lines · On Tuesday 15 July 2008 07:06:49 Ben Nizette wrote:
Quoted 4 lines · Michael Buesch
This driver provides a sysfs interface to dynamically create and destroy GPIO-based MMC/SD card interfaces.
So an MMC or SD card can be connected to generic GPIO pins and be configured dynamically from userspace.Can you use mmc-spi attached to spi-gpio and be done with it?
This is _exactly_ what this module does. It combines mmc-spi and spi-gpio and creates an actual device for them.
Quoted 3 lines
Sure you
won't have the dynamic alloc capability but you won't be reinventing the wheel either.
This does not reinvent anything. It just wires up the two modules.
Signature · 1 line
Greetings Michael.
Quoted 8 lines · Ben Nizette
Quoted 4 lines · Michael Buesch
This driver provides a sysfs interface to dynamically create and destroy GPIO-based MMC/SD card interfaces.
So an MMC or SD card can be connected to generic GPIO pins and be configured dynamically from userspace.Can you use mmc-spi attached to spi-gpio and be done with it?
Yes, that's how some of the early mmc-spi work was done. ;)
Quoted 8 lines
Sure you
won't have the dynamic alloc capability but you won't be reinventing the wheel either. You don't have dynamic creation for any other platform device and if you need it then, IMO, it'd be better solved in a more generic way.btw, what's this spi-gpio thing? I can't see it in mainline except in a s3c24xx specific way. My domestic blindness??
ISTR a patch for that, sitting somewhere in my mailbox waiting for review cycles. I've sent examples of such stuff around before too.
I've held back submitting a spi-gpio driver until I could come up with a way to configure it which would let the GPIO calls get inlined ... because otherwise, there's a nasty speed penalty.
It's the difference between a function call per GPIO operation and an instruction per GPIO operation. Consider that each SPI clock cycle requires four such operations, and GPIO function calls often take 30 instructions ... there's a big penalty to the non-inlined bitbangers, which is only partly addressed by having those function calls sitting in I-cache.
- Dave
Quoted 4 lines · On Monday 14 July 2008 21:09:18 Michael Buesch wrote:
This driver provides a sysfs interface to dynamically create and destroy GPIO-based MMC/SD card interfaces.
So an MMC or SD card can be connected to generic GPIO pins and be configured dynamically from userspace.
Ok, I had requests in private to also allow registering the platform device from outside of the module (and use platform data). This is used for certain embedded hardware from the arch
code to statically create a GPIO-based MMC device.
I'll add that and resend along with the proposed fixes. Thanks for the review! :)
Signature · 1 line
Greetings Michael.