[PATCH 0/3] of/spi/eeprom: Configure at25 from device tree and autoload its driver.

[PATCH 0/3] of/spi/eeprom: Configure at25 from device tree and autoload its driver.

[David Daney]

From: David Daney <david.daney@cavium.com>

As per the Subject, given a device tree fragment like this:

	spi@1070000001000 {
		compatible = "cavium,octeon-3010-spi";
		reg = <0x10700 0x00001000 0x0 0x100>;
		interrupts = <0 58>;
		#address-cells = <1>;
		#size-cells = <0>;

		eeprom@0 {
			compatible = "st,m95256", "atmel,at25";
			reg = <0>;
			spi-max-frequency = <5000000>;
			spi-cpha;
			spi-cpol;

			pagesize = <64>;
			size = <32768>;
			address-width = <16>;
		};
	};

The at25 module is autoloaded and configured from the device tree data.

1/3) Make of_modalias_node() work better for auto loading drivers.

2/3) Use MODALIAS prefixed with "spi:" for SPI drivers so modprobe can
     find the proper driver module.

3/3) Use standard eeprom device tree binding to configure at25,
     convert MODULE_ALIAS(), to equivalent MODULE_DEVICE_TABLE().

David Daney (3):
  of: Add prefix parameter to of_modalias_node().
  spi: Use consistent MODALIAS values.
  eeprom/of: Add device tree bindings to at25.

 drivers/misc/eeprom/at25.c   |   61 +++++++++++++++++++++++++++++++++++++++---
 drivers/of/base.c            |   22 +++++++++++----
 drivers/of/of_i2c.c          |    2 +-
 drivers/of/of_spi.c          |    2 +-
 drivers/spi/spi.c            |   39 +++++++++++++++++++++++---
 include/linux/of.h           |    3 +-
 sound/soc/fsl/mpc8610_hpcd.c |    2 +-
 sound/soc/fsl/p1022_ds.c     |    2 +-
 8 files changed, 113 insertions(+), 20 deletions(-)

-- 
1.7.2.3

[PATCH 1/3] of: Add prefix parameter to of_modalias_node().

[David Daney]

From: David Daney <david.daney@cavium.com>

When generating MODALIASes, it is convenient to add things like "spi:"
or "i2c:" to the front of the strings.  This allows the standard
modprobe to find the right driver when automatically populating bus
children from the device tree structure.

Add a prefix parameter, and adjust callers.  For
of_register_spi_devices() use the "spi:" prefix.

Signed-off-by: David Daney <david.daney@cavium.com>
Cc: Liam Girdwood <lrg@ti.com>
Cc: Timur Tabi <timur@freescale.com>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: Takashi Iwai <tiwai@suse.de>
Cc: alsa-devel@alsa-project.org
Cc: linuxppc-dev@lists.ozlabs.org
---
 drivers/of/base.c            |   22 ++++++++++++++++------
 drivers/of/of_i2c.c          |    2 +-
 drivers/of/of_spi.c          |    2 +-
 include/linux/of.h           |    3 ++-
 sound/soc/fsl/mpc8610_hpcd.c |    2 +-
 sound/soc/fsl/p1022_ds.c     |    2 +-
 6 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 5806449..f05a520 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -575,26 +575,36 @@ EXPORT_SYMBOL(of_find_matching_node);
 /**
  * of_modalias_node - Lookup appropriate modalias for a device node
  * @node:	pointer to a device tree node
+ * @prefix:	prefix to be added to the compatible property, may be NULL
  * @modalias:	Pointer to buffer that modalias value will be copied into
  * @len:	Length of modalias value
  *
- * Based on the value of the compatible property, this routine will attempt
- * to choose an appropriate modalias value for a particular device tree node.
- * It does this by stripping the manufacturer prefix (as delimited by a ',')
- * from the first entry in the compatible list property.
+ * Based on the value of the compatible property, this routine will
+ * attempt to choose an appropriate modalias value for a particular
+ * device tree node.  It does this by stripping the manufacturer
+ * prefix (as delimited by a ',') from the first entry in the
+ * compatible list property, and appending it to the prefix.
  *
  * This routine returns 0 on success, <0 on failure.
  */
-int of_modalias_node(struct device_node *node, char *modalias, int len)
+int of_modalias_node(struct device_node *node, const char *prefix,
+		     char *modalias, int len)
 {
 	const char *compatible, *p;
 	int cplen;
 
+	if (len < 1)
+		return -EINVAL;
+
 	compatible = of_get_property(node, "compatible", &cplen);
 	if (!compatible || strlen(compatible) > cplen)
 		return -ENODEV;
 	p = strchr(compatible, ',');
-	strlcpy(modalias, p ? p + 1 : compatible, len);
+	if (prefix)
+		strlcpy(modalias, prefix, len);
+	else
+		modalias[0] = 0;
+	strlcat(modalias, p ? p + 1 : compatible, len);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(of_modalias_node);
diff --git a/drivers/of/of_i2c.c b/drivers/of/of_i2c.c
index f37fbeb..23b05ee 100644
--- a/drivers/of/of_i2c.c
+++ b/drivers/of/of_i2c.c
@@ -37,7 +37,7 @@ void of_i2c_register_devices(struct i2c_adapter *adap)
 
 		dev_dbg(&adap->dev, "of_i2c: register %s\n", node->full_name);
 
-		if (of_modalias_node(node, info.type, sizeof(info.type)) < 0) {
+		if (of_modalias_node(node, NULL, info.type, sizeof(info.type)) < 0) {
 			dev_err(&adap->dev, "of_i2c: modalias failure on %s\n",
 				node->full_name);
 			continue;
diff --git a/drivers/of/of_spi.c b/drivers/of/of_spi.c
index 6dbc074..c329c6d 100644
--- a/drivers/of/of_spi.c
+++ b/drivers/of/of_spi.c
@@ -42,7 +42,7 @@ void of_register_spi_devices(struct spi_master *master)
 		}
 
 		/* Select device driver */
-		if (of_modalias_node(nc, spi->modalias,
+		if (of_modalias_node(nc, SPI_MODULE_PREFIX, spi->modalias,
 				     sizeof(spi->modalias)) < 0) {
 			dev_err(&master->dev, "cannot find modalias for %s\n",
 				nc->full_name);
diff --git a/include/linux/of.h b/include/linux/of.h
index fa7fb1d..ee34d76 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -233,7 +233,8 @@ extern int of_n_addr_cells(struct device_node *np);
 extern int of_n_size_cells(struct device_node *np);
 extern const struct of_device_id *of_match_node(
 	const struct of_device_id *matches, const struct device_node *node);
-extern int of_modalias_node(struct device_node *node, char *modalias, int len);
+extern int of_modalias_node(struct device_node *node,
+			    const char *prefix, char *modalias, int len);
 extern struct device_node *of_parse_phandle(struct device_node *np,
 					    const char *phandle_name,
 					    int index);
diff --git a/sound/soc/fsl/mpc8610_hpcd.c b/sound/soc/fsl/mpc8610_hpcd.c
index 3fea5a1..1fa0682 100644
--- a/sound/soc/fsl/mpc8610_hpcd.c
+++ b/sound/soc/fsl/mpc8610_hpcd.c
@@ -254,7 +254,7 @@ static int codec_node_dev_name(struct device_node *np, char *buf, size_t len)
 	char temp[DAI_NAME_SIZE];
 	struct i2c_client *i2c;
 
-	of_modalias_node(np, temp, DAI_NAME_SIZE);
+	of_modalias_node(np, NULL, temp, DAI_NAME_SIZE);
 
 	iprop = of_get_property(np, "reg", NULL);
 	if (!iprop)
diff --git a/sound/soc/fsl/p1022_ds.c b/sound/soc/fsl/p1022_ds.c
index 982a1c9..3ea51ec 100644
--- a/sound/soc/fsl/p1022_ds.c
+++ b/sound/soc/fsl/p1022_ds.c
@@ -257,7 +257,7 @@ static int codec_node_dev_name(struct device_node *np, char *buf, size_t len)
 	char temp[DAI_NAME_SIZE];
 	struct i2c_client *i2c;
 
-	of_modalias_node(np, temp, DAI_NAME_SIZE);
+	of_modalias_node(np, NULL, temp, DAI_NAME_SIZE);
 
 	iprop = of_get_property(np, "reg", NULL);
 	if (!iprop)
-- 
1.7.2.3

[PATCH 2/3] spi: Use consistent MODALIAS values.

[David Daney]

From: David Daney <david.daney@cavium.com>

For SPI devices, the MODALIAS should start with "spi:" so that the
modprobe can find the proper drivers.

Be consistent, for devices added via spi_new_device(), make sure
"spi:" is added if it is not already there.  In spi_match_device()
handle matching when the "spi:" prefix is present.

Signed-off-by: David Daney <david.daney@cavium.com>
---
 drivers/spi/spi.c |   39 ++++++++++++++++++++++++++++++++++-----
 1 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 3d8f662..8c49964 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -59,6 +59,16 @@ static struct device_attribute spi_dev_attrs[] = {
 	__ATTR_NULL,
 };
 
+/*
+ * modalias will be of the form "spi:name" or "name", match on just
+ * the name portion.
+ */
+static const char *spi_device_spidev2name(const struct spi_device *sdev)
+{
+	const char *p = strchr(sdev->modalias, ':');
+	return p ? p + 1 : sdev->modalias;
+}
+
 /* modalias support makes "modprobe $MODALIAS" new-style hotplug work,
  * and the sysfs version makes coldplug work too.
  */
@@ -66,8 +76,10 @@ static struct device_attribute spi_dev_attrs[] = {
 static const struct spi_device_id *spi_match_id(const struct spi_device_id *id,
 						const struct spi_device *sdev)
 {
+	const char *name = spi_device_spidev2name(sdev);
+
 	while (id->name[0]) {
-		if (!strcmp(sdev->modalias, id->name))
+		if (!strcmp(name, id->name))
 			return id;
 		id++;
 	}
@@ -94,14 +106,20 @@ static int spi_match_device(struct device *dev, struct device_driver *drv)
 	if (sdrv->id_table)
 		return !!spi_match_id(sdrv->id_table, spi);
 
-	return strcmp(spi->modalias, drv->name) == 0;
+	return strcmp(spi_device_spidev2name(spi), drv->name) == 0;
 }
 
 static int spi_uevent(struct device *dev, struct kobj_uevent_env *env)
 {
 	const struct spi_device		*spi = to_spi_device(dev);
 
-	add_uevent_var(env, "MODALIAS=%s%s", SPI_MODULE_PREFIX, spi->modalias);
+	if (strncmp(SPI_MODULE_PREFIX, spi->modalias,
+		    strlen(SPI_MODULE_PREFIX)) == 0)
+		add_uevent_var(env, "MODALIAS=%s", spi->modalias);
+	else
+		add_uevent_var(env, "MODALIAS=%s%s",
+			       SPI_MODULE_PREFIX, spi->modalias);
+
 	return 0;
 }
 
@@ -418,6 +436,7 @@ struct spi_device *spi_new_device(struct spi_master *master,
 {
 	struct spi_device	*proxy;
 	int			status;
+	size_t			l;
 
 	/* NOTE:  caller did any chip->bus_num checks necessary.
 	 *
@@ -430,13 +449,23 @@ struct spi_device *spi_new_device(struct spi_master *master,
 	if (!proxy)
 		return NULL;
 
-	WARN_ON(strlen(chip->modalias) >= sizeof(proxy->modalias));
+	if (strncmp(SPI_MODULE_PREFIX, chip->modalias,
+		    strlen(SPI_MODULE_PREFIX)) == 0) {
+		proxy->modalias[0] = 0;
+	} else {
+		l = strlcpy(proxy->modalias, SPI_MODULE_PREFIX,
+			    sizeof(proxy->modalias));
+		WARN_ON(l >= sizeof(proxy->modalias));
+	}
+
+	l = strlcat(proxy->modalias, chip->modalias, sizeof(proxy->modalias));
+
+	WARN_ON(l >= sizeof(proxy->modalias));
 
 	proxy->chip_select = chip->chip_select;
 	proxy->max_speed_hz = chip->max_speed_hz;
 	proxy->mode = chip->mode;
 	proxy->irq = chip->irq;
-	strlcpy(proxy->modalias, chip->modalias, sizeof(proxy->modalias));
 	proxy->dev.platform_data = (void *) chip->platform_data;
 	proxy->controller_data = chip->controller_data;
 	proxy->controller_state = NULL;
-- 
1.7.2.3

[PATCH 3/3] eeprom/of: Add device tree bindings to at25.

[David Daney]

From: David Daney <david.daney@cavium.com>

We can extract the "pagesize", "size" and "address-width" from the
device tree so that SPI eeproms can be fully specified in the device
tree.

Also add a MODULE_DEVICE_TABLE so the drivers can be automatically bound.

Signed-off-by: David Daney <david.daney@cavium.com>
Cc: Michael Hennerich <michael.hennerich@analog.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Axel Lin <axel.lin@gmail.com>
---
 drivers/misc/eeprom/at25.c |   61 +++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 57 insertions(+), 4 deletions(-)

diff --git a/drivers/misc/eeprom/at25.c b/drivers/misc/eeprom/at25.c
index 01ab3c9..609ee72 100644
--- a/drivers/misc/eeprom/at25.c
+++ b/drivers/misc/eeprom/at25.c
@@ -16,6 +16,7 @@
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/sched.h>
+#include <linux/of.h>
 
 #include <linux/spi/spi.h>
 #include <linux/spi/eeprom.h>
@@ -293,6 +294,9 @@ static int at25_probe(struct spi_device *spi)
 {
 	struct at25_data	*at25 = NULL;
 	const struct spi_eeprom *chip;
+#ifdef CONFIG_OF
+	struct spi_eeprom of_chip;
+#endif
 	int			err;
 	int			sr;
 	int			addrlen;
@@ -300,9 +304,51 @@ static int at25_probe(struct spi_device *spi)
 	/* Chip description */
 	chip = spi->dev.platform_data;
 	if (!chip) {
-		dev_dbg(&spi->dev, "no chip description\n");
-		err = -ENODEV;
-		goto fail;
+#ifdef CONFIG_OF
+		if (spi->dev.of_node) {
+			u32 val;
+			memset(&of_chip, 0, sizeof(of_chip));
+			if (of_property_read_u32(spi->dev.of_node, "pagesize", &val)) {
+				dev_dbg(&spi->dev, "no \"pagesize\" property\n");
+				err = -ENODEV;
+				goto fail;
+			}
+			of_chip.page_size = val;
+			if (of_property_read_u32(spi->dev.of_node, "size", &val)) {
+				dev_dbg(&spi->dev, "no \"size\" property\n");
+				err = -ENODEV;
+				goto fail;
+			}
+			of_chip.byte_len = val;
+			if (of_property_read_u32(spi->dev.of_node, "address-width", &val)) {
+				dev_dbg(&spi->dev, "no \"address-width\" property\n");
+				err = -ENODEV;
+				goto fail;
+			}
+			switch (val) {
+			case 8:
+				of_chip.flags |= EE_ADDR1;
+				break;
+			case 16:
+				of_chip.flags |= EE_ADDR2;
+				break;
+			case 24:
+				of_chip.flags |= EE_ADDR3;
+				break;
+			default:
+				dev_dbg(&spi->dev, "bad \"address-width\" property: %u\n", val);
+				err = -EINVAL;
+				goto fail;
+			}
+			strlcpy(of_chip.name, spi->dev.of_node->name, sizeof(of_chip.name));
+			chip = &of_chip;
+		} else
+#endif
+		{
+			dev_dbg(&spi->dev, "no chip description\n");
+			err = -ENODEV;
+			goto fail;
+		}
 	}
 
 	/* For now we only support 8/16/24 bit addressing */
@@ -396,11 +442,19 @@ static int __devexit at25_remove(struct spi_device *spi)
 
 /*-------------------------------------------------------------------------*/
 
+static const struct spi_device_id at25_id[] = {
+	{"at25", 0},
+	{"m95256", 0},
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, at25_id);
+
 static struct spi_driver at25_driver = {
 	.driver = {
 		.name		= "at25",
 		.owner		= THIS_MODULE,
 	},
+	.id_table	= at25_id,
 	.probe		= at25_probe,
 	.remove		= __devexit_p(at25_remove),
 };
@@ -410,4 +464,3 @@ module_spi_driver(at25_driver);
 MODULE_DESCRIPTION("Driver for most SPI EEPROMs");
 MODULE_AUTHOR("David Brownell");
 MODULE_LICENSE("GPL");
-MODULE_ALIAS("spi:at25");
-- 
1.7.2.3

Re: [PATCH 3/3] eeprom/of: Add device tree bindings to at25.

[Greg Kroah-Hartman]

On Fri, May 11, 2012 at 03:05:23PM -0700, David Daney wrote:
> From: David Daney <david.daney@cavium.com>
> 
> We can extract the "pagesize", "size" and "address-width" from the
> device tree so that SPI eeproms can be fully specified in the device
> tree.
> 
> Also add a MODULE_DEVICE_TABLE so the drivers can be automatically bound.
> 
> Signed-off-by: David Daney <david.daney@cavium.com>
> Cc: Michael Hennerich <michael.hennerich@analog.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Axel Lin <axel.lin@gmail.com>

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH 1/3] of: Add prefix parameter to of_modalias_node().

[Grant Likely]

On Fri, 11 May 2012 15:05:21 -0700, David Daney <ddaney.cavm@gmail.com> wrote:
> From: David Daney <david.daney@cavium.com>
> 
> When generating MODALIASes, it is convenient to add things like "spi:"
> or "i2c:" to the front of the strings.  This allows the standard
> modprobe to find the right driver when automatically populating bus
> children from the device tree structure.
> 
> Add a prefix parameter, and adjust callers.  For
> of_register_spi_devices() use the "spi:" prefix.
> 
> Signed-off-by: David Daney <david.daney@cavium.com>

Applied, thanks.  Some notes below...

> Cc: Liam Girdwood <lrg@ti.com>
> Cc: Timur Tabi <timur@freescale.com>
> Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
> Cc: Jaroslav Kysela <perex@perex.cz>
> Cc: Takashi Iwai <tiwai@suse.de>
> Cc: alsa-devel@alsa-project.org
> Cc: linuxppc-dev@lists.ozlabs.org
> ---
>  drivers/of/base.c            |   22 ++++++++++++++++------
>  drivers/of/of_i2c.c          |    2 +-
>  drivers/of/of_spi.c          |    2 +-
>  include/linux/of.h           |    3 ++-
>  sound/soc/fsl/mpc8610_hpcd.c |    2 +-
>  sound/soc/fsl/p1022_ds.c     |    2 +-
>  6 files changed, 22 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 5806449..f05a520 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -575,26 +575,36 @@ EXPORT_SYMBOL(of_find_matching_node);
>  /**
>   * of_modalias_node - Lookup appropriate modalias for a device node
>   * @node:	pointer to a device tree node
> + * @prefix:	prefix to be added to the compatible property, may be NULL
>   * @modalias:	Pointer to buffer that modalias value will be copied into
>   * @len:	Length of modalias value
>   *
> - * Based on the value of the compatible property, this routine will attempt
> - * to choose an appropriate modalias value for a particular device tree node.
> - * It does this by stripping the manufacturer prefix (as delimited by a ',')
> - * from the first entry in the compatible list property.
> + * Based on the value of the compatible property, this routine will
> + * attempt to choose an appropriate modalias value for a particular
> + * device tree node.  It does this by stripping the manufacturer
> + * prefix (as delimited by a ',') from the first entry in the
> + * compatible list property, and appending it to the prefix.

Not sure why this text block was reformatted.  I've formatted it back
to the way it was so the diff shows specifically what has changed in
the content.

I don't want to discourage cleanups, but I need to be careful that
cleanups don't obscure important changes when looking at the diff.

g.

Re: [PATCH 1/3] of: Add prefix parameter to of_modalias_node().

[Grant Likely]

On Sat, 19 May 2012 23:54:36 -0600, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Fri, 11 May 2012 15:05:21 -0700, David Daney <ddaney.cavm@gmail.com> wrote:
> > From: David Daney <david.daney@cavium.com>
> > 
> > When generating MODALIASes, it is convenient to add things like "spi:"
> > or "i2c:" to the front of the strings.  This allows the standard
> > modprobe to find the right driver when automatically populating bus
> > children from the device tree structure.
> > 
> > Add a prefix parameter, and adjust callers.  For
> > of_register_spi_devices() use the "spi:" prefix.
> > 
> > Signed-off-by: David Daney <david.daney@cavium.com>
> 
> Applied, thanks.  Some notes below...

Wait... why is this necessary?  The module type prefix isn't stored in
the modalias value for any other bus type as far as I can see, and
with this series it appears that the "spi:" prefix may or may not be
present in the modalias.  That doesn't look right.

Why isn't prefixing spi: at uevent time sufficient?  IIUC, modprobe
depends on either UEVENT or the modalias attribute to know which
driver to probe.  It does look like the attribute is missing the spi:
prefix though.  Does the following change work instead of these two
patches?

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 3d8f662..da8aac7 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -51,7 +51,7 @@ modalias_show(struct device *dev, struct device_attribute *a, char *buf)
 {
        const struct spi_device *spi = to_spi_device(dev);
 
-       return sprintf(buf, "%s\n", spi->modalias);
+       return sprintf(buf, "%s%s\n", SPI_MODULE_PREFIX, spi->modalias);
 }

So, I've dropped this patch from my tree.  If the change above works
for you then I'll push it out.

g.

Re: [PATCH 3/3] eeprom/of: Add device tree bindings to at25.

[Grant Likely]

On Fri, 11 May 2012 15:05:23 -0700, David Daney <ddaney.cavm@gmail.com> wrote:
> From: David Daney <david.daney@cavium.com>
> 
> We can extract the "pagesize", "size" and "address-width" from the
> device tree so that SPI eeproms can be fully specified in the device
> tree.
> 
> Also add a MODULE_DEVICE_TABLE so the drivers can be automatically bound.
> 
> Signed-off-by: David Daney <david.daney@cavium.com>
> Cc: Michael Hennerich <michael.hennerich@analog.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Axel Lin <axel.lin@gmail.com>
> ---
>  drivers/misc/eeprom/at25.c |   61 +++++++++++++++++++++++++++++++++++++++++---

Documentation on binding?  It needs to be there before merging.

g.

>  1 files changed, 57 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/misc/eeprom/at25.c b/drivers/misc/eeprom/at25.c
> index 01ab3c9..609ee72 100644
> --- a/drivers/misc/eeprom/at25.c
> +++ b/drivers/misc/eeprom/at25.c
> @@ -16,6 +16,7 @@
>  #include <linux/delay.h>
>  #include <linux/device.h>
>  #include <linux/sched.h>
> +#include <linux/of.h>
>  
>  #include <linux/spi/spi.h>
>  #include <linux/spi/eeprom.h>
> @@ -293,6 +294,9 @@ static int at25_probe(struct spi_device *spi)
>  {
>  	struct at25_data	*at25 = NULL;
>  	const struct spi_eeprom *chip;
> +#ifdef CONFIG_OF
> +	struct spi_eeprom of_chip;
> +#endif
>  	int			err;
>  	int			sr;
>  	int			addrlen;
> @@ -300,9 +304,51 @@ static int at25_probe(struct spi_device *spi)
>  	/* Chip description */
>  	chip = spi->dev.platform_data;
>  	if (!chip) {
> -		dev_dbg(&spi->dev, "no chip description\n");
> -		err = -ENODEV;
> -		goto fail;
> +#ifdef CONFIG_OF
> +		if (spi->dev.of_node) {
> +			u32 val;
> +			memset(&of_chip, 0, sizeof(of_chip));
> +			if (of_property_read_u32(spi->dev.of_node, "pagesize", &val)) {
> +				dev_dbg(&spi->dev, "no \"pagesize\" property\n");
> +				err = -ENODEV;
> +				goto fail;
> +			}
> +			of_chip.page_size = val;
> +			if (of_property_read_u32(spi->dev.of_node, "size", &val)) {
> +				dev_dbg(&spi->dev, "no \"size\" property\n");
> +				err = -ENODEV;
> +				goto fail;
> +			}
> +			of_chip.byte_len = val;
> +			if (of_property_read_u32(spi->dev.of_node, "address-width", &val)) {
> +				dev_dbg(&spi->dev, "no \"address-width\" property\n");
> +				err = -ENODEV;
> +				goto fail;
> +			}
> +			switch (val) {
> +			case 8:
> +				of_chip.flags |= EE_ADDR1;
> +				break;
> +			case 16:
> +				of_chip.flags |= EE_ADDR2;
> +				break;
> +			case 24:
> +				of_chip.flags |= EE_ADDR3;
> +				break;
> +			default:
> +				dev_dbg(&spi->dev, "bad \"address-width\" property: %u\n", val);
> +				err = -EINVAL;
> +				goto fail;
> +			}
> +			strlcpy(of_chip.name, spi->dev.of_node->name, sizeof(of_chip.name));
> +			chip = &of_chip;
> +		} else
> +#endif
> +		{
> +			dev_dbg(&spi->dev, "no chip description\n");
> +			err = -ENODEV;
> +			goto fail;
> +		}
>  	}
>  
>  	/* For now we only support 8/16/24 bit addressing */
> @@ -396,11 +442,19 @@ static int __devexit at25_remove(struct spi_device *spi)
>  
>  /*-------------------------------------------------------------------------*/
>  
> +static const struct spi_device_id at25_id[] = {
> +	{"at25", 0},
> +	{"m95256", 0},
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(spi, at25_id);
> +
>  static struct spi_driver at25_driver = {
>  	.driver = {
>  		.name		= "at25",
>  		.owner		= THIS_MODULE,
>  	},
> +	.id_table	= at25_id,
>  	.probe		= at25_probe,
>  	.remove		= __devexit_p(at25_remove),
>  };
> @@ -410,4 +464,3 @@ module_spi_driver(at25_driver);
>  MODULE_DESCRIPTION("Driver for most SPI EEPROMs");
>  MODULE_AUTHOR("David Brownell");
>  MODULE_LICENSE("GPL");
> -MODULE_ALIAS("spi:at25");
> -- 
> 1.7.2.3
> 

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.

Re: [PATCH 1/3] of: Add prefix parameter to of_modalias_node().

[David Daney]

On 05/19/2012 11:08 PM, Grant Likely wrote:
> On Sat, 19 May 2012 23:54:36 -0600, Grant Likely<grant.likely@secretlab.ca>  wrote:
>> On Fri, 11 May 2012 15:05:21 -0700, David Daney<ddaney.cavm@gmail.com>  wrote:
>>> From: David Daney<david.daney@cavium.com>
>>>
>>> When generating MODALIASes, it is convenient to add things like "spi:"
>>> or "i2c:" to the front of the strings.  This allows the standard
>>> modprobe to find the right driver when automatically populating bus
>>> children from the device tree structure.
>>>
>>> Add a prefix parameter, and adjust callers.  For
>>> of_register_spi_devices() use the "spi:" prefix.
>>>
>>> Signed-off-by: David Daney<david.daney@cavium.com>
>>
>> Applied, thanks.  Some notes below...
>
> Wait... why is this necessary?

Because in of_register_spi_devices() in of_spi.c, you do:

	request_module(spi->modalias);

The string passed to request_module() must have the "spi:" prefix.


> The module type prefix isn't stored in
> the modalias value for any other bus type as far as I can see,

It is only useful with the prefix, so I though I would add it to the 
stored value.

> and
> with this series it appears that the "spi:" prefix may or may not be
> present in the modalias.  That doesn't look right.

Perhaps, but the with the combination of patches 1/3 and 2/3 I tried to 
ensure that the prefix would always be present for SPI devices.

>
> Why isn't prefixing spi: at uevent time sufficient?

Because udev may not be loading the driver.

> IIUC, modprobe
> depends on either UEVENT or the modalias attribute to know which
> driver to probe.  It does look like the attribute is missing the spi:
> prefix though.  Does the following change work instead of these two
> patches?
>

No.

> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 3d8f662..da8aac7 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -51,7 +51,7 @@ modalias_show(struct device *dev, struct device_attribute *a, char *buf)
>   {
>          const struct spi_device *spi = to_spi_device(dev);
>
> -       return sprintf(buf, "%s\n", spi->modalias);
> +       return sprintf(buf, "%s%s\n", SPI_MODULE_PREFIX, spi->modalias);
>   }
>
> So, I've dropped this patch from my tree.  If the change above works
> for you then I'll push it out.
>
> g.
>

Re: [PATCH 1/3] of: Add prefix parameter to of_modalias_node().

[Grant Likely]

On Tue, May 22, 2012 at 1:45 PM, David Daney <ddaney.cavm@gmail.com> wrote:
> On 05/19/2012 11:08 PM, Grant Likely wrote:
>>
>> On Sat, 19 May 2012 23:54:36 -0600, Grant
>> Likely<grant.likely@secretlab.ca>  wrote:
>>>
>>> On Fri, 11 May 2012 15:05:21 -0700, David Daney<ddaney.cavm@gmail.com>
>>>  wrote:
>>>>
>>>> From: David Daney<david.daney@cavium.com>
>>>>
>>>> When generating MODALIASes, it is convenient to add things like "spi:"
>>>> or "i2c:" to the front of the strings.  This allows the standard
>>>> modprobe to find the right driver when automatically populating bus
>>>> children from the device tree structure.
>>>>
>>>> Add a prefix parameter, and adjust callers.  For
>>>> of_register_spi_devices() use the "spi:" prefix.
>>>>
>>>> Signed-off-by: David Daney<david.daney@cavium.com>
>>>
>>>
>>> Applied, thanks.  Some notes below...
>>
>>
>> Wait... why is this necessary?
>
>
> Because in of_register_spi_devices() in of_spi.c, you do:
>
>        request_module(spi->modalias);
>
> The string passed to request_module() must have the "spi:" prefix.

How about modifying the call to request_module() to include the prefix
also?  I think that would be a simpler change overall.  Would that
work?

g.

Re: [PATCH 1/3] of: Add prefix parameter to of_modalias_node().

[David Daney]

On 05/22/2012 01:09 PM, Grant Likely wrote:
> On Tue, May 22, 2012 at 1:45 PM, David Daney<ddaney.cavm@gmail.com>  wrote:
>> On 05/19/2012 11:08 PM, Grant Likely wrote:
>>>
>>> On Sat, 19 May 2012 23:54:36 -0600, Grant
>>> Likely<grant.likely@secretlab.ca>    wrote:
>>>>
>>>> On Fri, 11 May 2012 15:05:21 -0700, David Daney<ddaney.cavm@gmail.com>
>>>>   wrote:
>>>>>
>>>>> From: David Daney<david.daney@cavium.com>
>>>>>
>>>>> When generating MODALIASes, it is convenient to add things like "spi:"
>>>>> or "i2c:" to the front of the strings.  This allows the standard
>>>>> modprobe to find the right driver when automatically populating bus
>>>>> children from the device tree structure.
>>>>>
>>>>> Add a prefix parameter, and adjust callers.  For
>>>>> of_register_spi_devices() use the "spi:" prefix.
>>>>>
>>>>> Signed-off-by: David Daney<david.daney@cavium.com>
>>>>
>>>>
>>>> Applied, thanks.  Some notes below...
>>>
>>>
>>> Wait... why is this necessary?
>>
>>
>> Because in of_register_spi_devices() in of_spi.c, you do:
>>
>>         request_module(spi->modalias);
>>
>> The string passed to request_module() must have the "spi:" prefix.
>
> How about modifying the call to request_module() to include the prefix
> also?  I think that would be a simpler change overall.  Would that
> work?

It seems to.  I just sent such a patch in a new thread.

David Daney

Re: [PATCH 1/3] of: Add prefix parameter to of_modalias_node().

[Grant Likely]

Cool, thanks.

g.

On Tue, May 22, 2012 at 4:49 PM, David Daney <ddaney.cavm@gmail.com> wrote:
> On 05/22/2012 01:09 PM, Grant Likely wrote:
>>
>> On Tue, May 22, 2012 at 1:45 PM, David Daney<ddaney.cavm@gmail.com>
>>  wrote:
>>>
>>> On 05/19/2012 11:08 PM, Grant Likely wrote:
>>>>
>>>>
>>>> On Sat, 19 May 2012 23:54:36 -0600, Grant
>>>> Likely<grant.likely@secretlab.ca>    wrote:
>>>>>
>>>>>
>>>>> On Fri, 11 May 2012 15:05:21 -0700, David Daney<ddaney.cavm@gmail.com>
>>>>>  wrote:
>>>>>>
>>>>>>
>>>>>> From: David Daney<david.daney@cavium.com>
>>>>>>
>>>>>> When generating MODALIASes, it is convenient to add things like "spi:"
>>>>>> or "i2c:" to the front of the strings.  This allows the standard
>>>>>> modprobe to find the right driver when automatically populating bus
>>>>>> children from the device tree structure.
>>>>>>
>>>>>> Add a prefix parameter, and adjust callers.  For
>>>>>> of_register_spi_devices() use the "spi:" prefix.
>>>>>>
>>>>>> Signed-off-by: David Daney<david.daney@cavium.com>
>>>>>
>>>>>
>>>>>
>>>>> Applied, thanks.  Some notes below...
>>>>
>>>>
>>>>
>>>> Wait... why is this necessary?
>>>
>>>
>>>
>>> Because in of_register_spi_devices() in of_spi.c, you do:
>>>
>>>        request_module(spi->modalias);
>>>
>>> The string passed to request_module() must have the "spi:" prefix.
>>
>>
>> How about modifying the call to request_module() to include the prefix
>> also?  I think that would be a simpler change overall.  Would that
>> work?
>
>
> It seems to.  I just sent such a patch in a new thread.
>
> David Daney



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.