Skip to contentSkip to conversation

Device table matching for SPI subsystem

PATCH In review Anton Vorontsov 11 participants 43 messages Jul 31, 2009 – Jul 8, 2010

Subsystems (20)
lore
Share & tools
lore
#0 lore raw

v2 cover Device table matching for SPI subsystem

Andrew,

This new patch set overwrites following patches:

hwmon-lm70-convert-to-device-table-matching.patch hwmon-adxx-convert-to-device-table-matching.patch spi-merge-probe-and-probe_id-callbacks.patch spi-prefix-modalias-with-spi.patch of-remove-stmm25p40-alias.patch mtd-m25p80-convert-to-device-table-matching.patch spi-add-support-for-device-table-matching.patch

Changes since v1:

- Implemented Ben Dooks' idea of spi_get_device_id(), so we won't call spi_match_id() twice for drivers that don't need the id.

- "spi: Merge probe and probe_id callbacks" patch no longer needed as we don't change probe()'s arguments;

- Rename spi_device_id->data to driver_data, and turn it into kernel_ulong_t to match majority of subsystems. Most drivers don't need a pointer type anyway (e.g. m25p80 needs it, but lm70 and adcxx don't);

- SPI_NAME_SIZE now defined to 32 (as it should be, using 20 for name size was a cut-n-paste typo from I2C defines).

Thanks!

Signature · 8 lines
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
#1 lore raw

PATCH v2 1/6 spi: Add support for device table matching

With this patch spi drivers can use standard spi_driver.id_table and MODULE_DEVICE_TABLE() mechanisms to bind against the devices. Just like we do with I2C drivers.

This is useful when a single driver supports several variants of devices but it is not possible to detect them in run-time (like non-JEDEC chips probing in drivers/mtd/devices/m25p80.c), and when platform_data usage is overkill.

This patch also makes life a lot easier on OpenFirmware platforms, since with OF we extensively use proper device IDs in modaliases.

---
 drivers/spi/spi.c               |   23 +++++++++++++++++++++++
 include/linux/mod_devicetable.h |   10 ++++++++++
 include/linux/spi/spi.h         |   10 ++++++++--
 scripts/mod/file2alias.c        |   13 +++++++++++++
 4 files changed, 54 insertions(+), 2 deletions(-)
drivers/spi/spi.c +23 -0
index 70845cc..8518a6e 100644
@@ -59,9 +59,32 @@ static struct device_attribute spi_dev_attrs[] = {
  * and the sysfs version makes coldplug work too.
  */
 
+static const struct spi_device_id *spi_match_id(const struct spi_device_id *id,
+						const struct spi_device *sdev)
+{
+	while (id->name[0]) {
+		if (!strcmp(sdev->modalias, id->name))
+			return id;
+		id++;
+	}
+	return NULL;
+}
+
+const struct spi_device_id *spi_get_device_id(const struct spi_device *sdev)
+{
+	const struct spi_driver *sdrv = to_spi_driver(sdev->dev.driver);
+
+	return spi_match_id(sdrv->id_table, sdev);
+}
+EXPORT_SYMBOL_GPL(spi_get_device_id);
+
 static int spi_match_device(struct device *dev, struct device_driver *drv)
 {
 	const struct spi_device	*spi = to_spi_device(dev);
+	const struct spi_driver	*sdrv = to_spi_driver(drv);
+
+	if (sdrv->id_table)
+		return !!spi_match_id(sdrv->id_table, spi);
 
 	return strcmp(spi->modalias, drv->name) = 0;
 }
include/linux/mod_devicetable.h +10 -0
index 1bf5900..b34f1ef 100644
@@ -399,6 +399,16 @@ struct i2c_device_id {
 			__attribute__((aligned(sizeof(kernel_ulong_t))));
 };
 
+/* spi */
+
+#define SPI_NAME_SIZE	32
+
+struct spi_device_id {
+	char name[SPI_NAME_SIZE];
+	kernel_ulong_t driver_data	/* Data private to the driver */
+			__attribute__((aligned(sizeof(kernel_ulong_t))));
+};
+
 /* dmi */
 enum dmi_field {
 	DMI_NONE,
include/linux/spi/spi.h +8 -2
index c47c4b4..2b444df 100644
@@ -20,6 +20,7 @@
 #define __LINUX_SPI_H
 
 #include <linux/device.h>
+#include <linux/mod_devicetable.h>
 
 /*
  * INTERFACES between SPI master-side drivers and SPI infrastructure.
@@ -86,7 +87,7 @@ struct spi_device {
 	int			irq;
 	void			*controller_state;
 	void			*controller_data;
-	char			modalias[32];
+	char			modalias[SPI_NAME_SIZE];
 
 	/*
 	 * likely need more hooks for more protocol options affecting how
@@ -145,6 +146,7 @@ struct spi_message;
 
 /**
  * struct spi_driver - Host side "protocol" driver
+ * @id_table: List of SPI devices supported by this driver
  * @probe: Binds this driver to the spi device.  Drivers can verify
  *	that the device is actually present, and may need to configure
  *	characteristics (such as bits_per_word) which weren't needed for
@@ -170,6 +172,7 @@ struct spi_message;
  * MMC, RTC, filesystem character device nodes, and hardware monitoring.
  */
 struct spi_driver {
+	const struct spi_device_id *id_table;
 	int			(*probe)(struct spi_device *spi);
 	int			(*remove)(struct spi_device *spi);
 	void			(*shutdown)(struct spi_device *spi);
@@ -732,7 +735,7 @@ struct spi_board_info {
 	 * controller_data goes to spi_device.controller_data,
 	 * irq is copied too
 	 */
-	char		modalias[32];
+	char		modalias[SPI_NAME_SIZE];
 	const void	*platform_data;
 	void		*controller_data;
 	int		irq;
@@ -800,4 +803,7 @@ spi_unregister_device(struct spi_device *spi)
 		device_unregister(&spi->dev);
 }
 
+extern const struct spi_device_id *
+spi_get_device_id(const struct spi_device *sdev);
+
 #endif /* __LINUX_SPI_H */
scripts/mod/file2alias.c +13 -0
index 40e0045..9d446e3 100644
@@ -657,6 +657,15 @@ static int do_i2c_entry(const char *filename, struct i2c_device_id *id,
 	return 1;
 }
 
+/* Looks like: S */
+static int do_spi_entry(const char *filename, struct spi_device_id *id,
+			char *alias)
+{
+	sprintf(alias, "%s", id->name);
+
+	return 1;
+}
+
 static const struct dmifield {
 	const char *prefix;
 	int field;
@@ -853,6 +862,10 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
 		do_table(symval, sym->st_size,
 			 sizeof(struct i2c_device_id), "i2c",
 			 do_i2c_entry, mod);
+	else if (sym_is(symname, "__mod_spi_device_table"))
+		do_table(symval, sym->st_size,
+			 sizeof(struct spi_device_id), "spi",
+			 do_spi_entry, mod);
 	else if (sym_is(symname, "__mod_dmi_device_table"))
 		do_table(symval, sym->st_size,
 			 sizeof(struct dmi_system_id), "dmi",
Signature · 7 lines
1.6.3.3


_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
#2 lore raw

PATCH v2 2/6 mtd: m25p80: Convert to device table matching

In reply to Anton Vorontsov

This patch converts the m25p80 driver so that now it uses .id_table for device matching, making it properly detect devices on OpenFirmware platforms (prior to this patch the driver misdetected non-JEDEC chips, seeing all chips as "m25p80").

Also, now jedec_probe() only does jedec probing, nothing else. If it is not able to detect a chip, NULL is returned and the driver fall backs to the information specified by the platform (platform_data, or exact ID).

---
 drivers/mtd/devices/m25p80.c |  146 +++++++++++++++++++++++-------------------
 1 files changed, 80 insertions(+), 66 deletions(-)
drivers/mtd/devices/m25p80.c +80 -66
index 10ed195..0d74b38 100644
@@ -21,6 +21,7 @@
 #include <linux/interrupt.h>
 #include <linux/mutex.h>
 #include <linux/math64.h>
+#include <linux/mod_devicetable.h>
 
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/partitions.h>
@@ -462,8 +463,6 @@ static int m25p80_write(struct mtd_info *mtd, loff_t to, size_t len,
  */
 
 struct flash_info {
-	char		*name;
-
 	/* JEDEC id zero means "no ID" (most older chips); otherwise it has
 	 * a high byte of zero plus three data bytes: the manufacturer id,
 	 * then a two byte device id.
@@ -481,74 +480,83 @@ struct flash_info {
 #define	SECT_4K		0x01		/* OPCODE_BE_4K works uniformly */
 };
 
+#define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags)	\
+	((kernel_ulong_t)&(struct flash_info) {				\
+		.jedec_id = (_jedec_id),				\
+		.ext_id = (_ext_id),					\
+		.sector_size = (_sector_size),				\
+		.n_sectors = (_n_sectors),				\
+		.flags = (_flags),					\
+	})
 
 /* NOTE: double check command sets and memory organization when you add
  * more flash chips.  This current list focusses on newer chips, which
  * have been converging on command sets which including JEDEC ID.
  */
-static struct flash_info __devinitdata m25p_data [] = {
-
+static const struct spi_device_id m25p_ids[] = {
 	/* Atmel -- some are (confusingly) marketed as "DataFlash" */
-	{ "at25fs010",  0x1f6601, 0, 32 * 1024, 4, SECT_4K, },
-	{ "at25fs040",  0x1f6604, 0, 64 * 1024, 8, SECT_4K, },
+	{ "at25fs010",  INFO(0x1f6601, 0, 32 * 1024, 4, SECT_4K) },
+	{ "at25fs040",  INFO(0x1f6604, 0, 64 * 1024, 8, SECT_4K) },
 
-	{ "at25df041a", 0x1f4401, 0, 64 * 1024, 8, SECT_4K, },
-	{ "at25df641",  0x1f4800, 0, 64 * 1024, 128, SECT_4K, },
+	{ "at25df041a", INFO(0x1f4401, 0, 64 * 1024, 8, SECT_4K) },
+	{ "at25df641",  INFO(0x1f4800, 0, 64 * 1024, 128, SECT_4K) },
 
-	{ "at26f004",   0x1f0400, 0, 64 * 1024, 8, SECT_4K, },
-	{ "at26df081a", 0x1f4501, 0, 64 * 1024, 16, SECT_4K, },
-	{ "at26df161a", 0x1f4601, 0, 64 * 1024, 32, SECT_4K, },
-	{ "at26df321",  0x1f4701, 0, 64 * 1024, 64, SECT_4K, },
+	{ "at26f004",   INFO(0x1f0400, 0, 64 * 1024, 8, SECT_4K) },
+	{ "at26df081a", INFO(0x1f4501, 0, 64 * 1024, 16, SECT_4K) },
+	{ "at26df161a", INFO(0x1f4601, 0, 64 * 1024, 32, SECT_4K) },
+	{ "at26df321",  INFO(0x1f4701, 0, 64 * 1024, 64, SECT_4K) },
 
 	/* Macronix */
-	{ "mx25l12805d", 0xc22018, 0, 64 * 1024, 256, },
+	{ "mx25l12805d", INFO(0xc22018, 0, 64 * 1024, 256, 0) },
 
 	/* Spansion -- single (large) sector size only, at least
 	 * for the chips listed here (without boot sectors).
 	 */
-	{ "s25sl004a", 0x010212, 0, 64 * 1024, 8, },
-	{ "s25sl008a", 0x010213, 0, 64 * 1024, 16, },
-	{ "s25sl016a", 0x010214, 0, 64 * 1024, 32, },
-	{ "s25sl032a", 0x010215, 0, 64 * 1024, 64, },
-	{ "s25sl064a", 0x010216, 0, 64 * 1024, 128, },
-        { "s25sl12800", 0x012018, 0x0300, 256 * 1024, 64, },
-	{ "s25sl12801", 0x012018, 0x0301, 64 * 1024, 256, },
+	{ "s25sl004a",  INFO(0x010212, 0, 64 * 1024, 8, 0) },
+	{ "s25sl008a",  INFO(0x010213, 0, 64 * 1024, 16, 0) },
+	{ "s25sl016a",  INFO(0x010214, 0, 64 * 1024, 32, 0) },
+	{ "s25sl032a",  INFO(0x010215, 0, 64 * 1024, 64, 0) },
+	{ "s25sl064a",  INFO(0x010216, 0, 64 * 1024, 128, 0) },
+	{ "s25sl12800", INFO(0x012018, 0x0300, 256 * 1024, 64, 0) },
+	{ "s25sl12801", INFO(0x012018, 0x0301, 64 * 1024, 256, 0) },
 
 	/* SST -- large erase sizes are "overlays", "sectors" are 4K */
-	{ "sst25vf040b", 0xbf258d, 0, 64 * 1024, 8, SECT_4K, },
-	{ "sst25vf080b", 0xbf258e, 0, 64 * 1024, 16, SECT_4K, },
-	{ "sst25vf016b", 0xbf2541, 0, 64 * 1024, 32, SECT_4K, },
-	{ "sst25vf032b", 0xbf254a, 0, 64 * 1024, 64, SECT_4K, },
+	{ "sst25vf040b", INFO(0xbf258d, 0, 64 * 1024, 8, SECT_4K) },
+	{ "sst25vf080b", INFO(0xbf258e, 0, 64 * 1024, 16, SECT_4K) },
+	{ "sst25vf016b", INFO(0xbf2541, 0, 64 * 1024, 32, SECT_4K) },
+	{ "sst25vf032b", INFO(0xbf254a, 0, 64 * 1024, 64, SECT_4K) },
 
 	/* ST Microelectronics -- newer production may have feature updates */
-	{ "m25p05",  0x202010,  0, 32 * 1024, 2, },
-	{ "m25p10",  0x202011,  0, 32 * 1024, 4, },
-	{ "m25p20",  0x202012,  0, 64 * 1024, 4, },
-	{ "m25p40",  0x202013,  0, 64 * 1024, 8, },
-	{ "m25p80",         0,  0, 64 * 1024, 16, },
-	{ "m25p16",  0x202015,  0, 64 * 1024, 32, },
-	{ "m25p32",  0x202016,  0, 64 * 1024, 64, },
-	{ "m25p64",  0x202017,  0, 64 * 1024, 128, },
-	{ "m25p128", 0x202018, 0, 256 * 1024, 64, },
-
-	{ "m45pe10", 0x204011,  0, 64 * 1024, 2, },
-	{ "m45pe80", 0x204014,  0, 64 * 1024, 16, },
-	{ "m45pe16", 0x204015,  0, 64 * 1024, 32, },
-
-	{ "m25pe80", 0x208014,  0, 64 * 1024, 16, },
-	{ "m25pe16", 0x208015,  0, 64 * 1024, 32, SECT_4K, },
+	{ "m25p05",  INFO(0x202010,  0, 32 * 1024, 2, 0) },
+	{ "m25p10",  INFO(0x202011,  0, 32 * 1024, 4, 0) },
+	{ "m25p20",  INFO(0x202012,  0, 64 * 1024, 4, 0) },
+	{ "m25p40",  INFO(0x202013,  0, 64 * 1024, 8, 0) },
+	{ "m25p80",  INFO(0x202014,  0, 64 * 1024, 16, 0) },
+	{ "m25p16",  INFO(0x202015,  0, 64 * 1024, 32, 0) },
+	{ "m25p32",  INFO(0x202016,  0, 64 * 1024, 64, 0) },
+	{ "m25p64",  INFO(0x202017,  0, 64 * 1024, 128, 0) },
+	{ "m25p128", INFO(0x202018, 0, 256 * 1024, 64, 0) },
+
+	{ "m45pe10", INFO(0x204011,  0, 64 * 1024, 2, 0) },
+	{ "m45pe80", INFO(0x204014,  0, 64 * 1024, 16, 0) },
+	{ "m45pe16", INFO(0x204015,  0, 64 * 1024, 32, 0) },
+
+	{ "m25pe80", INFO(0x208014,  0, 64 * 1024, 16, 0) },
+	{ "m25pe16", INFO(0x208015,  0, 64 * 1024, 32, SECT_4K) },
 
 	/* Winbond -- w25x "blocks" are 64K, "sectors" are 4KiB */
-	{ "w25x10", 0xef3011, 0, 64 * 1024, 2, SECT_4K, },
-	{ "w25x20", 0xef3012, 0, 64 * 1024, 4, SECT_4K, },
-	{ "w25x40", 0xef3013, 0, 64 * 1024, 8, SECT_4K, },
-	{ "w25x80", 0xef3014, 0, 64 * 1024, 16, SECT_4K, },
-	{ "w25x16", 0xef3015, 0, 64 * 1024, 32, SECT_4K, },
-	{ "w25x32", 0xef3016, 0, 64 * 1024, 64, SECT_4K, },
-	{ "w25x64", 0xef3017, 0, 64 * 1024, 128, SECT_4K, },
+	{ "w25x10", INFO(0xef3011, 0, 64 * 1024, 2, SECT_4K) },
+	{ "w25x20", INFO(0xef3012, 0, 64 * 1024, 4, SECT_4K) },
+	{ "w25x40", INFO(0xef3013, 0, 64 * 1024, 8, SECT_4K) },
+	{ "w25x80", INFO(0xef3014, 0, 64 * 1024, 16, SECT_4K) },
+	{ "w25x16", INFO(0xef3015, 0, 64 * 1024, 32, SECT_4K) },
+	{ "w25x32", INFO(0xef3016, 0, 64 * 1024, 64, SECT_4K) },
+	{ "w25x64", INFO(0xef3017, 0, 64 * 1024, 128, SECT_4K) },
+	{ },
 };
+MODULE_DEVICE_TABLE(spi, m25p_ids);
 
-static struct flash_info *__devinit jedec_probe(struct spi_device *spi)
+static const struct spi_device_id *__devinit jedec_probe(struct spi_device *spi)
 {
 	int			tmp;
 	u8			code = OPCODE_RDID;
@@ -575,16 +583,14 @@ static struct flash_info *__devinit jedec_probe(struct spi_device *spi)
 
 	ext_jedec = id[3] << 8 | id[4];
 
-	for (tmp = 0, info = m25p_data;
-			tmp < ARRAY_SIZE(m25p_data);
-			tmp++, info++) {
+	for (tmp = 0; tmp < ARRAY_SIZE(m25p_ids) - 1; tmp++) {
+		info = (void *)m25p_ids[tmp].driver_data;
 		if (info->jedec_id = jedec) {
 			if (info->ext_id != 0 && info->ext_id != ext_jedec)
 				continue;
-			return info;
+			return &m25p_ids[tmp];
 		}
 	}
-	dev_err(&spi->dev, "unrecognized JEDEC id %06x\n", jedec);
 	return NULL;
 }
 
@@ -596,6 +602,7 @@ static struct flash_info *__devinit jedec_probe(struct spi_device *spi)
  */
 static int __devinit m25p_probe(struct spi_device *spi)
 {
+	const struct spi_device_id	*id;
 	struct flash_platform_data	*data;
 	struct m25p			*flash;
 	struct flash_info		*info;
@@ -608,32 +615,38 @@ static int __devinit m25p_probe(struct spi_device *spi)
 	 */
 	data = spi->dev.platform_data;
 	if (data && data->type) {
-		for (i = 0, info = m25p_data;
-				i < ARRAY_SIZE(m25p_data);
-				i++, info++) {
-			if (strcmp(data->type, info->name) = 0)
-				break;
+		for (i = 0; i < ARRAY_SIZE(m25p_ids) - 1; i++) {
+			id = &m25p_ids[i];
+			info = (void *)m25p_ids[i].driver_data;
+			if (strcmp(data->type, id->name))
+				continue;
+			break;
 		}
 
 		/* unrecognized chip? */
-		if (i = ARRAY_SIZE(m25p_data)) {
+		if (i = ARRAY_SIZE(m25p_ids) - 1) {
 			DEBUG(MTD_DEBUG_LEVEL0, "%s: unrecognized id %s\n",
 					dev_name(&spi->dev), data->type);
 			info = NULL;
 
 		/* recognized; is that chip really what's there? */
 		} else if (info->jedec_id) {
-			struct flash_info	*chip = jedec_probe(spi);
+			id = jedec_probe(spi);
 
-			if (!chip || chip != info) {
+			if (id != &m25p_ids[i]) {
 				dev_warn(&spi->dev, "found %s, expected %s\n",
-						chip ? chip->name : "UNKNOWN",
-						info->name);
+						id ? id->name : "UNKNOWN",
+						m25p_ids[i].name);
 				info = NULL;
 			}
 		}
-	} else
-		info = jedec_probe(spi);
+	} else {
+		id = jedec_probe(spi);
+		if (!id)
+			id = spi_get_device_id(spi);
+
+		info = (void *)id->driver_data;
+	}
 
 	if (!info)
 		return -ENODEV;
@@ -680,7 +693,7 @@ static int __devinit m25p_probe(struct spi_device *spi)
 
 	flash->mtd.dev.parent = &spi->dev;
 
-	dev_info(&spi->dev, "%s (%lld Kbytes)\n", info->name,
+	dev_info(&spi->dev, "%s (%lld Kbytes)\n", id->name,
 			(long long)flash->mtd.size >> 10);
 
 	DEBUG(MTD_DEBUG_LEVEL2,
@@ -766,6 +779,7 @@ static struct spi_driver m25p80_driver = {
 		.bus	= &spi_bus_type,
 		.owner	= THIS_MODULE,
 	},
+	.id_table	= m25p_ids,
 	.probe	= m25p_probe,
 	.remove	= __devexit_p(m25p_remove),
 
Signature · 7 lines
1.6.3.3


_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
#3 lore raw

Re: [PATCH 2/6] mtd: m25p80: Convert to device table matching

Quoted 4 lines · Anton Vorontsov

This patch converts the m25p80 driver so that now it uses .id_table for device matching, making it properly detect devices on OpenFirmware platforms (prior to this patch the driver misdetected non-JEDEC chips, seeing all chips as "m25p80").

I suspect "detect" is a misnomer there. It only "detects" JEDEC chips. All others got explicit declarations ... so if there's misbehavior for other chips, it's because those declarations were poorly handled. Maybe they were not properly flagged as non-JDEC

Quoted 4 lines

Also, now jedec_probe() only does jedec probing, nothing else. If it is not able to detect a chip, NULL is returned and the driver fall backs to the information specified by the platform (platform_data, or exact ID).

I'd rather keep the warning, so there's a clue about what's really going on: JEDEC chip found, but its ID is not handled.

Quoted 13 lines
---
 drivers/mtd/devices/m25p80.c |  146 +++++++++++++++++++++++-------------------
 1 files changed, 80 insertions(+), 66 deletions(-)
drivers/mtd/devices/m25p80.c +0 -0
index 10ed195..0d74b38 100644
			... deletia ...
drivers/mtd/devices/m25p80.c +0 -0
@@ -481,74 +480,83 @@ struct flash_info {
 #define	SECT_4K		0x01		/* OPCODE_BE_4K works uniformly */

drivers/mtd/devices/m25p80.c struct flash_info { · line 490

Quoted 10 lines
drivers/mtd/devices/m25p80.c +8 -0
 };
 
+#define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags)	\
+	((kernel_ulong_t)&(struct flash_info) {				\
+		.jedec_id = (_jedec_id),				\
+		.ext_id = (_ext_id),					\
+		.sector_size = (_sector_size),				\
+		.n_sectors = (_n_sectors),				\
+		.flags = (_flags),					\
+	})

Anonymous inlined structures ... kind of ugly, but I can understand why you might not want to declare and name a few dozen single-use structures.

Quoted 26 lines
drivers/mtd/devices/m25p80.c +4 -4
 /* NOTE: double check command sets and memory organization when you add
  * more flash chips.  This current list focusses on newer chips, which
  * have been converging on command sets which including JEDEC ID.
  */
-static struct flash_info __devinitdata m25p_data [] = {
-
+static const struct spi_device_id m25p_ids[] = {
 	/* Atmel -- some are (confusingly) marketed as "DataFlash" */
-	{ "at25fs010",  0x1f6601, 0, 32 * 1024, 4, SECT_4K, },
-	{ "at25fs040",  0x1f6604, 0, 64 * 1024, 8, SECT_4K, },
+	{ "at25fs010",  INFO(0x1f6601, 0, 32 * 1024, 4, SECT_4K) },
+	{ "at25fs040",  INFO(0x1f6604, 0, 64 * 1024, 8, SECT_4K) },
 
 		... deletia ...
 
 
@@ -596,6 +602,7 @@ static struct flash_info *__devinit jedec_probe(struct spi_device *spi)
  */
 static int __devinit m25p_probe(struct spi_device *spi)
 {
+	const struct spi_device_id	*id;
 	struct flash_platform_data	*data;
 	struct m25p			*flash;
 	struct flash_info		*info;

drivers/mtd/devices/m25p80.c static int __devinit m25p_probe(struct spi_device *spi) · line 617

Quoted 3 lines
drivers/mtd/devices/m25p80.c +0 -0
@@ -608,32 +615,38 @@ static int __devinit m25p_probe(struct spi_device *spi)
 	 */
 	data = spi->dev.platform_data;
 	if (data && data->type) {

At this point I wonder why you're not changing the probe sequence more. Get "id" and then "id" here. If it's for "m25p80" assume it's an old-style board init and do the current logic. Else just verify "info".

There's a new error case of course: new-style but data->type doesn't match id->name.

Quoted 6 lines
drivers/mtd/devices/m25p80.c +1 -5
-		for (i = 0, info = m25p_data;
-				i < ARRAY_SIZE(m25p_data);
-				i++, info++) {
-			if (strcmp(data->type, info->name) = 0)
-				break;
+		for (i = 0; i < ARRAY_SIZE(m25p_ids) - 1; i++) {

drivers/mtd/devices/m25p80.c static int __devinit m25p_probe(struct spi_device *spi) · line 627

Quoted 10 lines
drivers/mtd/devices/m25p80.c +6 -1
+			id = &m25p_ids[i];
+			info = (void *)m25p_ids[i].driver_data;
+			if (strcmp(data->type, id->name))
+				continue;
+			break;
 		}
 
 		/* unrecognized chip? */
-		if (i = ARRAY_SIZE(m25p_data)) {
+		if (i = ARRAY_SIZE(m25p_ids) - 1) {

Better: "if (info = NULL) ..." You've got all the pointers
in hand; don't use indices.

drivers/mtd/devices/m25p80.c static int __devinit m25p_probe(struct spi_device *spi) · line 636

Quoted 11 lines
drivers/mtd/devices/m25p80.c +2 -2
 			DEBUG(MTD_DEBUG_LEVEL0, "%s: unrecognized id %s\n",
 					dev_name(&spi->dev), data->type);
 			info = NULL;
 
 		/* recognized; is that chip really what's there? */
 		} else if (info->jedec_id) {
-			struct flash_info	*chip = jedec_probe(spi);
+			id = jedec_probe(spi);
 
-			if (!chip || chip != info) {
+			if (id != &m25p_ids[i]) {

Again, don't use indices except during the lookup.

Quoted 10 lines
drivers/mtd/devices/m25p80.c +2 -4
 				dev_warn(&spi->dev, "found %s, expected %s\n",
-						chip ? chip->name : "UNKNOWN",
-						info->name);
+						id ? id->name : "UNKNOWN",
+						m25p_ids[i].name);
 				info = NULL;
 			}
 		}
-	} else
-		info = jedec_probe(spi);

drivers/mtd/devices/m25p80.c static int __devinit m25p_probe(struct spi_device *spi) · line 652

Quoted 10 lines
drivers/mtd/devices/m25p80.c +7 -0
+	} else {
+		id = jedec_probe(spi);
+		if (!id)
+			id = spi_get_device_id(spi);
+
+		info = (void *)id->driver_data;
+	}
 
 	if (!info)
 		return -ENODEV;

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

#4 lore raw

Re: [PATCH 2/6] mtd: m25p80: Convert to device table matching

Hi David,

Thanks for the review, and sorry for the delayed response.

Quoted 7 lines · David Brownell
Quoted 4 lines · Anton Vorontsov

This patch converts the m25p80 driver so that now it uses .id_table for device matching, making it properly detect devices on OpenFirmware platforms (prior to this patch the driver misdetected non-JEDEC chips, seeing all chips as "m25p80").

I suspect "detect" is a misnomer there. It only "detects" JEDEC chips.

Currently the driver always tries JEDEC probing, and it wrongly "assumed" that all non-JEDEC chips are m25p80, because an entry for m25p80 chips had "0" in jedec id field, which isn't correct ID, but 0 is returned by most non-JEDEC chips. Having 0 in the m25p80 entry was a hack.

Quoted 11 lines

All others got explicit declarations ... so if there's misbehavior for other chips, it's because those declarations were poorly handled. Maybe they were not properly flagged as non-JDEC

Quoted 4 lines

Also, now jedec_probe() only does jedec probing, nothing else. If it is not able to detect a chip, NULL is returned and the driver fall backs to the information specified by the platform (platform_data, or exact ID).

I'd rather keep the warning, so there's a clue about what's really going on: JEDEC chip found, but its ID is not handled.

We can't tell if the chip was actually found.

M25Px0 chips can be JEDEC and non-JEDEC, e.g. Nymonyx manufacturing "M25P80" chips in two variants: "The RDID instruction is available only for parts made with Technology T9HX (0.11μm), ..."

Most (but not all) non-JEDEC EEPROMs will return "0" for RDID opcode though (in that case warning is misleading). And for the chips that don't return 0, we shouldn't probe them with jedec at all.

[...]

Quoted 7 lines

drivers/mtd/devices/m25p80.c static int __devinit m25p_probe(struct spi_device *spi) · line 617

Quoted 4 lines
drivers/mtd/devices/m25p80.c +0 -0
@@ -608,32 +615,38 @@ static int __devinit m25p_probe(struct spi_device *spi)
 	 */
 	data = spi->dev.platform_data;
 	if (data && data->type) {

At this point I wonder why you're not changing the probe sequence more.

Yep, I was going to do it anyway, but for another reason: to support CAT25 chips.

Quoted 2 lines

There's a new error case of course: new-style but data->type doesn't match id->name.

[...]

Quoted 4 lines

drivers/mtd/devices/m25p80.c static int __devinit m25p_probe(struct spi_device *spi) · line 627

Quoted 1 line
drivers/mtd/devices/m25p80.c +1 -0
+		if (i = ARRAY_SIZE(m25p_ids) - 1) {

Better: "if (info = NULL) ..." You've got all the pointers
in hand; don't use indices.

[...]

Quoted 3 lines

drivers/mtd/devices/m25p80.c static int __devinit m25p_probe(struct spi_device *spi) · line 636

Quoted 1 line
drivers/mtd/devices/m25p80.c +1 -0
+			if (id != &m25p_ids[i]) {

Again, don't use indices except during the lookup.

Yep, good ideas. Though, the code will vanish anyway.

Patches on the way...

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

#5 lore raw

PATCH 1/2 mtd: m25p80: Rework probing/JEDEC code

Previosly the driver always tried JEDEC probing, assuming that non-JEDEC chips will return '0'. But truly non-JEDEC chips (like CAT25) won't do that, their behaviour on RDID command is undefined, so the driver should not call jedec_probe() for these chips.

Also, be less strict on error conditions, don't fail to probe if JEDEC found a chip that is different from what platform code told, instead just print some warnings and use an information obtained via JEDEC. In that case we should not trust partitions any longer, but they might be still useful (i.e. they could protect some parts of the chip).

---
 drivers/mtd/devices/m25p80.c |   69 ++++++++++++++++++++++++-----------------
 1 files changed, 40 insertions(+), 29 deletions(-)
drivers/mtd/devices/m25p80.c +40 -29
index 0d74b38..b75e319 100644
@@ -581,6 +581,14 @@ static const struct spi_device_id *__devinit jedec_probe(struct spi_device *spi)
 	jedec = jedec << 8;
 	jedec |= id[2];
 
+	/*
+	 * Some chips (like Numonyx M25P80) have JEDEC and non-JEDEC variants,
+	 * which depend on technology process. Officially RDID command doesn't
+	 * exist for non-JEDEC chips, but for compatibility they return ID 0.
+	 */
+	if (jedec == 0)
+		return NULL;
+
 	ext_jedec = id[3] << 8 | id[4];
 
 	for (tmp = 0; tmp < ARRAY_SIZE(m25p_ids) - 1; tmp++) {
@@ -602,7 +610,7 @@ static const struct spi_device_id *__devinit jedec_probe(struct spi_device *spi)
  */
 static int __devinit m25p_probe(struct spi_device *spi)
 {
-	const struct spi_device_id	*id;
+	const struct spi_device_id	*id = spi_get_device_id(spi);
 	struct flash_platform_data	*data;
 	struct m25p			*flash;
 	struct flash_info		*info;
@@ -615,41 +623,44 @@ static int __devinit m25p_probe(struct spi_device *spi)
 	 */
 	data = spi->dev.platform_data;
 	if (data && data->type) {
+		const struct spi_device_id *plat_id;
+
 		for (i = 0; i < ARRAY_SIZE(m25p_ids) - 1; i++) {
-			id = &m25p_ids[i];
-			info = (void *)m25p_ids[i].driver_data;
-			if (strcmp(data->type, id->name))
+			plat_id = &m25p_ids[i];
+			if (strcmp(data->type, plat_id->name))
 				continue;
 			break;
 		}
 
-		/* unrecognized chip? */
-		if (i == ARRAY_SIZE(m25p_ids) - 1) {
-			DEBUG(MTD_DEBUG_LEVEL0, "%s: unrecognized id %s\n",
-					dev_name(&spi->dev), data->type);
-			info = NULL;
-
-		/* recognized; is that chip really what's there? */
-		} else if (info->jedec_id) {
-			id = jedec_probe(spi);
-
-			if (id != &m25p_ids[i]) {
-				dev_warn(&spi->dev, "found %s, expected %s\n",
-						id ? id->name : "UNKNOWN",
-						m25p_ids[i].name);
-				info = NULL;
-			}
-		}
-	} else {
-		id = jedec_probe(spi);
-		if (!id)
-			id = spi_get_device_id(spi);
-
-		info = (void *)id->driver_data;
+		if (plat_id)
+			id = plat_id;
+		else
+			dev_warn(&spi->dev, "unrecognized id %s\n", data->type);
 	}
 
-	if (!info)
-		return -ENODEV;
+	info = (void *)id->driver_data;
+
+	if (info->jedec_id) {
+		const struct spi_device_id *jid;
+
+		jid = jedec_probe(spi);
+		if (!jid) {
+			dev_info(&spi->dev, "non-JEDEC variant of %s\n",
+				 id->name);
+		} else if (jid != id) {
+			/*
+			 * JEDEC knows better, so overwrite platform ID. We
+			 * can't trust partitions any longer, but we'll let
+			 * mtd apply them anyway, since some partitions may be
+			 * marked read-only, and we don't want to lose that
+			 * information, even if it's not 100% accurate.
+			 */
+			dev_warn(&spi->dev, "found %s, expected %s\n",
+				 jid->name, id->name);
+			id = jid;
+			info = (void *)jid->driver_data;
+		}
+	}
 
 	flash = kzalloc(sizeof *flash, GFP_KERNEL);
 	if (!flash)
Signature · 1 line
1.6.3.3
#6 lore raw

Re: [PATCH 1/2] mtd: m25p80: Rework probing/JEDEC code

T24gV2VkLCBBdWcgMTksIDIwMDkgYXQgNTo0NiBBTSwgQW50b24gVm9yb250c292Cjxhdm9yb250 c292QHJ1Lm12aXN0YS5jb20+IHdyb3RlOgo+Cj4gUHJldmlvc2x5IHRoZSBkcml2ZXIgYWx3YXlz IHRyaWVkIEpFREVDIHByb2JpbmcsIGFzc3VtaW5nIHRoYXQgbm9uLUpFREVDCj4gY2hpcHMgd2ls bCByZXR1cm4gJzAnLiBCdXQgdHJ1bHkgbm9uLUpFREVDIGNoaXBzIChsaWtlIENBVDI1KSB3b24n dCBkbwo+IHRoYXQsIHRoZWlyIGJlaGF2aW91ciBvbiBSRElEIGNvbW1hbmQgaXMgdW5kZWZpbmVk LCBzbyB0aGUgZHJpdmVyIHNob3VsZAo+IG5vdCBjYWxsIGplZGVjX3Byb2JlKCkgZm9yIHRoZXNl IGNoaXBzLgo+Cj4gQWxzbywgYmUgbGVzcyBzdHJpY3Qgb24gZXJyb3IgY29uZGl0aW9ucywgZG9u J3QgZmFpbCB0byBwcm9iZSBpZiBKRURFQwo+IGZvdW5kIGEgY2hpcCB0aGF0IGlzIGRpZmZlcmVu dCBmcm9tIHdoYXQgcGxhdGZvcm0gY29kZSB0b2xkLCBpbnN0ZWFkCj4ganVzdCBwcmludCBzb21l IHdhcm5pbmdzIGFuZCB1c2UgYW4gaW5mb3JtYXRpb24gb2J0YWluZWQgdmlhIEpFREVDLiBJbgpU aGlzIHBhdGNoIGNhdXNlZCBhIHByb2JsZW06CmV2ZW4gdGhvdWdoIHRoZSBleHRlcm5hbCBmbGFz aCBkb2Vzbid0IGV4aXN0LCBpdCB3aWxsIHN0aWxsIHBhc3MgdGhlCnByb2JlKCkgYW5kIGJlIHJl Z2lzdGVycmVkIGludG8ga2VybmVsIGFuZCBnaXZlbiB0aGUgcGFydGl0aW9uIHRhYmxlLgpZb3Ug bWF5IHJlZmVyIHRvIHRoaXMgYnVnIHJlcG9ydDoKaHR0cDovL2JsYWNrZmluLnVjbGludXgub3Jn L2dmL3Byb2plY3QvdWNsaW51eC1kaXN0L3RyYWNrZXIvP2FjdGlvbj1UcmFja2VySXRlbUVkaXQm dHJhY2tlcl9pdGVtX2lkPTU5NzUmc3RhcnQ9MAoKPiB0aGF0IGNhc2Ugd2Ugc2hvdWxkIG5vdCB0 cnVzdCBwYXJ0aXRpb25zIGFueSBsb25nZXIsIGJ1dCB0aGV5IG1pZ2h0IGJlCj4gc3RpbGwgdXNl ZnVsIChpLmUuIHRoZXkgY291bGQgcHJvdGVjdCBzb21lIHBhcnRzIG9mIHRoZSBjaGlwKS4KPgo+ IFNpZ25lZC1vZmYtYnk6IEFudG9uIFZvcm9udHNvdiA8YXZvcm9udHNvdkBydS5tdmlzdGEuY29t Pgo+IC0tLQo+IMKgZHJpdmVycy9tdGQvZGV2aWNlcy9tMjVwODAuYyB8IMKgIDY5ICsrKysrKysr KysrKysrKysrKysrKysrKy0tLS0tLS0tLS0tLS0tLS0tCj4gwqAxIGZpbGVzIGNoYW5nZWQsIDQw IGluc2VydGlvbnMoKyksIDI5IGRlbGV0aW9ucygtKQo+Cj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMv bXRkL2RldmljZXMvbTI1cDgwLmMgYi9kcml2ZXJzL210ZC9kZXZpY2VzL20yNXA4MC5jCj4gaW5k ZXggMGQ3NGIzOC4uYjc1ZTMxOSAxMDA2NDQKPiAtLS0gYS9kcml2ZXJzL210ZC9kZXZpY2VzL20y NXA4MC5jCj4gKysrIGIvZHJpdmVycy9tdGQvZGV2aWNlcy9tMjVwODAuYwo+IEBAIC01ODEsNiAr NTgxLDE0IEBAIHN0YXRpYyBjb25zdCBzdHJ1Y3Qgc3BpX2RldmljZV9pZCAqX19kZXZpbml0IGpl ZGVjX3Byb2JlKHN0cnVjdCBzcGlfZGV2aWNlICpzcGkpCj4gwqAgwqAgwqAgwqBqZWRlYyA9IGpl ZGVjIDw8IDg7Cj4gwqAgwqAgwqAgwqBqZWRlYyB8PSBpZFsyXTsKPgo+ICsgwqAgwqAgwqAgLyoK PiArIMKgIMKgIMKgIMKgKiBTb21lIGNoaXBzIChsaWtlIE51bW9ueXggTTI1UDgwKSBoYXZlIEpF REVDIGFuZCBub24tSkVERUMgdmFyaWFudHMsCj4gKyDCoCDCoCDCoCDCoCogd2hpY2ggZGVwZW5k IG9uIHRlY2hub2xvZ3kgcHJvY2Vzcy4gT2ZmaWNpYWxseSBSRElEIGNvbW1hbmQgZG9lc24ndAo+ ICsgwqAgwqAgwqAgwqAqIGV4aXN0IGZvciBub24tSkVERUMgY2hpcHMsIGJ1dCBmb3IgY29tcGF0 aWJpbGl0eSB0aGV5IHJldHVybiBJRCAwLgo+ICsgwqAgwqAgwqAgwqAqLwo+ICsgwqAgwqAgwqAg aWYgKGplZGVjID09IDApCj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDCoCByZXR1cm4gTlVMTDsKPiAr Cj4gwqAgwqAgwqAgwqBleHRfamVkZWMgPSBpZFszXSA8PCA4IHwgaWRbNF07Cj4KPiDCoCDCoCDC oCDCoGZvciAodG1wID0gMDsgdG1wIDwgQVJSQVlfU0laRShtMjVwX2lkcykgLSAxOyB0bXArKykg ewo+IEBAIC02MDIsNyArNjEwLDcgQEAgc3RhdGljIGNvbnN0IHN0cnVjdCBzcGlfZGV2aWNlX2lk ICpfX2RldmluaXQgamVkZWNfcHJvYmUoc3RydWN0IHNwaV9kZXZpY2UgKnNwaSkKPiDCoCovCj4g wqBzdGF0aWMgaW50IF9fZGV2aW5pdCBtMjVwX3Byb2JlKHN0cnVjdCBzcGlfZGV2aWNlICpzcGkp Cj4gwqB7Cj4gLSDCoCDCoCDCoCBjb25zdCBzdHJ1Y3Qgc3BpX2RldmljZV9pZCDCoCDCoCDCoCpp ZDsKPiArIMKgIMKgIMKgIGNvbnN0IHN0cnVjdCBzcGlfZGV2aWNlX2lkIMKgIMKgIMKgKmlkID0g c3BpX2dldF9kZXZpY2VfaWQoc3BpKTsKPiDCoCDCoCDCoCDCoHN0cnVjdCBmbGFzaF9wbGF0Zm9y bV9kYXRhIMKgIMKgIMKgKmRhdGE7Cj4gwqAgwqAgwqAgwqBzdHJ1Y3QgbTI1cCDCoCDCoCDCoCDC oCDCoCDCoCDCoCDCoCDCoCDCoCAqZmxhc2g7Cj4gwqAgwqAgwqAgwqBzdHJ1Y3QgZmxhc2hfaW5m byDCoCDCoCDCoCDCoCDCoCDCoCDCoCAqaW5mbzsKPiBAQCAtNjE1LDQxICs2MjMsNDQgQEAgc3Rh dGljIGludCBfX2RldmluaXQgbTI1cF9wcm9iZShzdHJ1Y3Qgc3BpX2RldmljZSAqc3BpKQo+IMKg IMKgIMKgIMKgICovCj4gwqAgwqAgwqAgwqBkYXRhID0gc3BpLT5kZXYucGxhdGZvcm1fZGF0YTsK PiDCoCDCoCDCoCDCoGlmIChkYXRhICYmIGRhdGEtPnR5cGUpIHsKPiArIMKgIMKgIMKgIMKgIMKg IMKgIMKgIGNvbnN0IHN0cnVjdCBzcGlfZGV2aWNlX2lkICpwbGF0X2lkOwo+ICsKPiDCoCDCoCDC oCDCoCDCoCDCoCDCoCDCoGZvciAoaSA9IDA7IGkgPCBBUlJBWV9TSVpFKG0yNXBfaWRzKSAtIDE7 IGkrKykgewo+IC0gwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgaWQgPSAmbTI1cF9p ZHNbaV07Cj4gLSDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCBpbmZvID0gKHZvaWQg KiltMjVwX2lkc1tpXS5kcml2ZXJfZGF0YTsKPiAtIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg IMKgIMKgIGlmIChzdHJjbXAoZGF0YS0+dHlwZSwgaWQtPm5hbWUpKQo+ICsgwqAgwqAgwqAgwqAg wqAgwqAgwqAgwqAgwqAgwqAgwqAgcGxhdF9pZCA9ICZtMjVwX2lkc1tpXTsKPiArIMKgIMKgIMKg IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIGlmIChzdHJjbXAoZGF0YS0+dHlwZSwgcGxhdF9pZC0+ bmFtZSkpCj4gwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqBj b250aW51ZTsKPiDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoGJyZWFrOwo+IMKg IMKgIMKgIMKgIMKgIMKgIMKgIMKgfQo+Cj4gLSDCoCDCoCDCoCDCoCDCoCDCoCDCoCAvKiB1bnJl Y29nbml6ZWQgY2hpcD8gKi8KPiAtIMKgIMKgIMKgIMKgIMKgIMKgIMKgIGlmIChpID09IEFSUkFZ X1NJWkUobTI1cF9pZHMpIC0gMSkgewo+IC0gwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAg wqAgREVCVUcoTVREX0RFQlVHX0xFVkVMMCwgIiVzOiB1bnJlY29nbml6ZWQgaWQgJXNcbiIsCj4g LSDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC oCBkZXZfbmFtZSgmc3BpLT5kZXYpLCBkYXRhLT50eXBlKTsKPiAtIMKgIMKgIMKgIMKgIMKgIMKg IMKgIMKgIMKgIMKgIMKgIGluZm8gPSBOVUxMOwo+IC0KPiAtIMKgIMKgIMKgIMKgIMKgIMKgIMKg IC8qIHJlY29nbml6ZWQ7IGlzIHRoYXQgY2hpcCByZWFsbHkgd2hhdCdzIHRoZXJlPyAqLwo+IC0g wqAgwqAgwqAgwqAgwqAgwqAgwqAgfSBlbHNlIGlmIChpbmZvLT5qZWRlY19pZCkgewo+IC0gwqAg wqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgaWQgPSBqZWRlY19wcm9iZShzcGkpOwo+IC0K PiAtIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIGlmIChpZCAhPSAmbTI1cF9pZHNb aV0pIHsKPiAtIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIGRl dl93YXJuKCZzcGktPmRldiwgImZvdW5kICVzLCBleHBlY3RlZCAlc1xuIiwKPiAtIMKgIMKgIMKg IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg IMKgIGlkID8gaWQtPm5hbWUgOiAiVU5LTk9XTiIsCj4gLSDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC oCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCBtMjVwX2lkc1tp XS5uYW1lKTsKPiAtIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg IGluZm8gPSBOVUxMOwo+IC0gwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgwqAgfQo+IC0g wqAgwqAgwqAgwqAgwqAgwqAgwqAgfQo+IC0gwqAgwqAgwqAgfSBlbHNlIHsKPiAtIMKgIMKgIMKg IMKgIMKgIMKgIMKgIGlkID0gamVkZWNfcHJvYmUoc3BpKTsKPiAtIMKgIMKgIMKgIMKgIMKgIMKg IMKgIGlmICghaWQpCj4gLSDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCBpZCA9IHNw aV9nZXRfZGV2aWNlX2lkKHNwaSk7Cj4gLQo+IC0gwqAgwqAgwqAgwqAgwqAgwqAgwqAgaW5mbyA9 ICh2b2lkICopaWQtPmRyaXZlcl9kYXRhOwo+ICsgwqAgwqAgwqAgwqAgwqAgwqAgwqAgaWYgKHBs YXRfaWQpCj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCBpZCA9IHBsYXRfaWQ7 Cj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDCoCBlbHNlCj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC oCDCoCDCoCDCoCBkZXZfd2Fybigmc3BpLT5kZXYsICJ1bnJlY29nbml6ZWQgaWQgJXNcbiIsIGRh dGEtPnR5cGUpOwo+IMKgIMKgIMKgIMKgfQo+Cj4gLSDCoCDCoCDCoCBpZiAoIWluZm8pCj4gLSDC oCDCoCDCoCDCoCDCoCDCoCDCoCByZXR1cm4gLUVOT0RFVjsKPiArIMKgIMKgIMKgIGluZm8gPSAo dm9pZCAqKWlkLT5kcml2ZXJfZGF0YTsKPiArCj4gKyDCoCDCoCDCoCBpZiAoaW5mby0+amVkZWNf aWQpIHsKPiArIMKgIMKgIMKgIMKgIMKgIMKgIMKgIGNvbnN0IHN0cnVjdCBzcGlfZGV2aWNlX2lk ICpqaWQ7Cj4gKwo+ICsgwqAgwqAgwqAgwqAgwqAgwqAgwqAgamlkID0gamVkZWNfcHJvYmUoc3Bp KTsKPiArIMKgIMKgIMKgIMKgIMKgIMKgIMKgIGlmICghamlkKSB7Cj4gKyDCoCDCoCDCoCDCoCDC oCDCoCDCoCDCoCDCoCDCoCDCoCBkZXZfaW5mbygmc3BpLT5kZXYsICJub24tSkVERUMgdmFyaWFu dCBvZiAlc1xuIiwKPiArIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg IMKgIMKgaWQtPm5hbWUpOwo+ICsgwqAgwqAgwqAgwqAgwqAgwqAgwqAgfSBlbHNlIGlmIChqaWQg IT0gaWQpIHsKPiArIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIC8qCj4gKyDCoCDC oCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCogSkVERUMga25vd3MgYmV0dGVyLCBzbyBv dmVyd3JpdGUgcGxhdGZvcm0gSUQuIFdlCj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDC oCDCoCDCoCogY2FuJ3QgdHJ1c3QgcGFydGl0aW9ucyBhbnkgbG9uZ2VyLCBidXQgd2UnbGwgbGV0 Cj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCogbXRkIGFwcGx5IHRoZW0g YW55d2F5LCBzaW5jZSBzb21lIHBhcnRpdGlvbnMgbWF5IGJlCj4gKyDCoCDCoCDCoCDCoCDCoCDC oCDCoCDCoCDCoCDCoCDCoCDCoCogbWFya2VkIHJlYWQtb25seSwgYW5kIHdlIGRvbid0IHdhbnQg dG8gbG9zZSB0aGF0Cj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCogaW5m b3JtYXRpb24sIGV2ZW4gaWYgaXQncyBub3QgMTAwJSBhY2N1cmF0ZS4KPiArIMKgIMKgIMKgIMKg IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgKi8KPiArIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKg IMKgIMKgIGRldl93YXJuKCZzcGktPmRldiwgImZvdW5kICVzLCBleHBlY3RlZCAlc1xuIiwKPiAr IMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgamlkLT5uYW1l LCBpZC0+bmFtZSk7Cj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCDCoCBpZCA9IGpp ZDsKPiArIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIMKgIGluZm8gPSAodm9pZCAqKWpp ZC0+ZHJpdmVyX2RhdGE7Cj4gKyDCoCDCoCDCoCDCoCDCoCDCoCDCoCB9Cj4gKyDCoCDCoCDCoCB9 Cj4KPiDCoCDCoCDCoCDCoGZsYXNoID0ga3phbGxvYyhzaXplb2YgKmZsYXNoLCBHRlBfS0VSTkVM KTsKPiDCoCDCoCDCoCDCoGlmICghZmxhc2gpCj4gLS0KPiAxLjYuMy4zCj4KPiAtLQo+IFRvIHVu c3Vic2NyaWJlIGZyb20gdGhpcyBsaXN0OiBzZW5kIHRoZSBsaW5lICJ1bnN1YnNjcmliZSBsaW51 eC1rZXJuZWwiIGluCj4gdGhlIGJvZHkgb2YgYSBtZXNzYWdlIHRvIG1ham9yZG9tb0B2Z2VyLmtl cm5lbC5vcmcKPiBNb3JlIG1ham9yZG9tbyBpbmZvIGF0IMKgaHR0cDovL3ZnZXIua2VybmVsLm9y Zy9tYWpvcmRvbW8taW5mby5odG1sCj4gUGxlYXNlIHJlYWQgdGhlIEZBUSBhdCDCoGh0dHA6Ly93 d3cudHV4Lm9yZy9sa21sLwo=

#7 lore raw

Re: [PATCH 1/2] mtd: m25p80: Rework probing/JEDEC code

Quoted 8 lines · Barry Song
Quoted 6 lines · Anton Vorontsov

Previosly the driver always tried JEDEC probing, assuming that non-JEDEC chips will return '0'. But truly non-JEDEC chips (like CAT25) won't do that, their behaviour on RDID command is undefined, so the driver should not call jedec_probe() for these chips.

Quoted 8 lines
Quoted 3 lines

Also, be less strict on error conditions, don't fail to probe if JEDEC found a chip that is different from what platform code told, instead just print some warnings and use an information obtained via JEDEC. In

This patch caused a problem:
even though the external flash doesn't exist, it will still pass the probe() and be registerred into kernel and given the partition table. You may refer to this bug report:
http://blackfin.uclinux.org/gf/project/uclinux-dist/tracker/?action=TrackerItemEdit&tracker_item_id=5975&start=0

Thanks for the report.

There's little we can do about it. Platform code asked us to register the device, and JEDEC probing of M25Pxx chips isn't reliable (thanks to various vendors that make these JEDEC and non-JEDEC variants), so the best thing we can do is to register the chip anyway.

OTOH, if the board pulls MISO line up, then the following patch should help.

If this won't work, we'll have to add some flag to the platform data, i.e. to force JEDEC probing, and not trust platform data.

---

drivers/mtd/devices/m25p80.c +7 -4
index 81e49a9..a307929 100644
@@ -16,6 +16,7 @@
  */
 
 #include <linux/init.h>
+#include <linux/errno.h>
 #include <linux/module.h>
 #include <linux/device.h>
 #include <linux/interrupt.h>
@@ -723,7 +724,7 @@ static const struct spi_device_id *__devinit jedec_probe(struct spi_device *spi)
 	if (tmp < 0) {
 		DEBUG(MTD_DEBUG_LEVEL0, "%s: error %d reading JEDEC ID\n",
 			dev_name(&spi->dev), tmp);
-		return NULL;
+		return ERR_PTR(tmp);
 	}
 	jedec = id[0];
 	jedec = jedec << 8;
@@ -737,7 +738,7 @@ static const struct spi_device_id *__devinit jedec_probe(struct spi_device *spi)
 	 * exist for non-JEDEC chips, but for compatibility they return ID 0.
 	 */
 	if (jedec == 0)
-		return NULL;
+		return ERR_PTR(-EEXIST);
 
 	ext_jedec = id[3] << 8 | id[4];
 
@@ -749,7 +750,7 @@ static const struct spi_device_id *__devinit jedec_probe(struct spi_device *spi)
 			return &m25p_ids[tmp];
 		}
 	}
-	return NULL;
+	return ERR_PTR(-ENODEV);
 }
 
 
@@ -794,9 +795,11 @@ static int __devinit m25p_probe(struct spi_device *spi)
 		const struct spi_device_id *jid;
 
 		jid = jedec_probe(spi);
-		if (!jid) {
+		if (IS_ERR(jid) && PTR_ERR(jid) == -EEXIST) {
 			dev_info(&spi->dev, "non-JEDEC variant of %s\n",
 				 id->name);
+		} else if (IS_ERR(jid)) {
+			return PTR_ERR(jid);
 		} else if (jid != id) {
 			/*
 			 * JEDEC knows better, so overwrite platform ID. We
#8 lore raw

RE: [Uclinux-dist-devel] [PATCH 1/2] mtd: m25p80: Reworkprobing/JEDEC code

Quoted 43 lines

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Anton Vorontsov
Sent: Friday, June 18, 2010 9:32 PM
To: Barry Song
Cc: David Brownell; Artem Bityutskiy; [email protected]; [email protected]; [email protected]; [email protected]; Andrew Morton
Subject: Re: [Uclinux-dist-devel] [PATCH 1/2] mtd: m25p80:
Reworkprobing/JEDEC code

Quoted 4 lines · Barry Song
Quoted 2 lines · Anton Vorontsov

Previosly the driver always tried JEDEC probing, assuming

that non-JEDEC

Quoted 1 line
Quoted 1 line

chips will return '0'. But truly non-JEDEC chips (like

CAT25) won't do

Quoted 1 line
Quoted 1 line

that, their behaviour on RDID command is undefined, so the

driver should

Quoted 3 lines
Quoted 3 lines

not call jedec_probe() for these chips.

Also, be less strict on error conditions, don't fail to

probe if JEDEC

Quoted 1 line
Quoted 1 line

found a chip that is different from what platform code

told, instead

Quoted 1 line
Quoted 1 line

just print some warnings and use an information obtained

via JEDEC. In

Quoted 5 lines

This patch caused a problem:
even though the external flash doesn't exist, it will still pass the probe() and be registerred into kernel and given the partition table. You may refer to this bug report:

http://blackfin.uclinux.org/gf/project/uclinux-dist/tracker/?ac
tion=TrackerItemEdit&tracker_item_id=5975&start=0

Thanks for the report.

Quoted 8 lines

There's little we can do about it. Platform code asked us to register the device, and JEDEC probing of M25Pxx chips isn't reliable (thanks to various vendors that make these JEDEC and non-JEDEC variants), so the best thing we can do is to register the chip anyway.

OTOH, if the board pulls MISO line up, then the following patch should help.

Make sense with pullup to keep the value high while external device doesn't exist.

Quoted 3 lines

If this won't work, we'll have to add some flag to the platform data, i.e. to force JEDEC probing, and not trust platform data.

How about we add a non_jedec flag in platform_data, if the flag is 1, we let the detection pass even though the ID is 0? Otherwise, we need a valid ID?

Quoted 69 lines

---

drivers/mtd/devices/m25p80.c +2 -1
index 81e49a9..a307929 100644
@@ -16,6 +16,7 @@
 */
 
 #include <linux/init.h>
+#include <linux/errno.h>
 #include <linux/module.h>
 #include <linux/device.h>
 #include <linux/interrupt.h>
@@ -723,7 +724,7 @@ static const struct spi_device_id 
 *__devinit jedec_probe(struct spi_device *spi)
 	if (tmp < 0) {
 		DEBUG(MTD_DEBUG_LEVEL0, "%s: error %d reading 
 JEDEC ID\n",
 			dev_name(&spi->dev), tmp);
-		return NULL;
+		return ERR_PTR(tmp);
 	}
	jedec = id[0];
	jedec = jedec << 8;
drivers/mtd/devices/m25p80.c +0 -0
@@ -737,7 +738,7 @@ static const struct spi_device_id 
 *__devinit jedec_probe(struct spi_device *spi)
 	 * exist for non-JEDEC chips, but for compatibility 

they return ID 0.

	 */
	if (jedec == 0)
drivers/mtd/devices/m25p80.c +1 -1
-		return NULL;
+		return ERR_PTR(-EEXIST);
	ext_jedec = id[3] << 8 | id[4];
drivers/mtd/devices/m25p80.c +4 -2
@@ -749,7 +750,7 @@ static const struct spi_device_id 
 *__devinit jedec_probe(struct spi_device *spi)
 			return &m25p_ids[tmp];
 		}
 	}
-	return NULL;
+	return ERR_PTR(-ENODEV);
 }
 
@@ -794,9 +795,11 @@ static int __devinit m25p_probe(struct 
 spi_device *spi)
 		const struct spi_device_id *jid;
 
 		jid = jedec_probe(spi);
-		if (!jid) {
+		if (IS_ERR(jid) && PTR_ERR(jid) == -EEXIST) {
 			dev_info(&spi->dev, "non-JEDEC variant of %s\n",
 				 id->name);
+		} else if (IS_ERR(jid)) {
+			return PTR_ERR(jid);
 		} else if (jid != id) {
 			/*

* JEDEC knows better, so overwrite
platform ID. We
_______________________________________________
Uclinux-dist-devel mailing list [email protected]
https://blackfin.uclinux.org/mailman/listinfo/uclinux-dist-devel

#9 lore raw

Re: [Uclinux-dist-devel] [PATCH 1/2] mtd: m25p80: Reworkprobing/JEDEC code

On Mon, Jun 21, 2010 at 10:42 AM, Song, Barry <[email protected]> wrote= :

Quoted 54 lines
Quoted 43 lines

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Anton Vorontsov
Sent: Friday, June 18, 2010 9:32 PM
To: Barry Song
Cc: David Brownell; Artem Bityutskiy; [email protected]; [email protected]; [email protected]; [email protected]; Andrew Morton
Subject: Re: [Uclinux-dist-devel] [PATCH 1/2] mtd: m25p80:
Reworkprobing/JEDEC code

Quoted 4 lines · Barry Song
Quoted 2 lines · Anton Vorontsov

Previosly the driver always tried JEDEC probing, assuming

that non-JEDEC

Quoted 1 line
Quoted 1 line

chips will return '0'. But truly non-JEDEC chips (like

CAT25) won't do

Quoted 1 line
Quoted 1 line

that, their behaviour on RDID command is undefined, so the

driver should

Quoted 3 lines
Quoted 3 lines

not call jedec_probe() for these chips.

Also, be less strict on error conditions, don't fail to

probe if JEDEC

Quoted 1 line
Quoted 1 line

found a chip that is different from what platform code

told, instead

Quoted 1 line
Quoted 1 line

just print some warnings and use an information obtained

via JEDEC. In

Quoted 5 lines

This patch caused a problem:
even though the external flash doesn't exist, it will still pass the probe() and be registerred into kernel and given the partition table. You may refer to this bug report:

http://blackfin.uclinux.org/gf/project/uclinux-dist/tracker/?ac
tion=3DTrackerItemEdit&tracker_item_id=3D5975&start=3D0

Thanks for the report.

Quoted 8 lines

There's little we can do about it. Platform code asked us to register the device, and JEDEC probing of M25Pxx chips isn't reliable (thanks to various vendors that make these JEDEC and non-JEDEC variants), so the best thing we can do is to register the chip anyway.

OTOH, if the board pulls MISO line up, then the following patch should help.

Make sense with pullup to keep the value high while external device doesn't exist.

Quoted 1 line
Quoted 6 lines
Quoted 2 lines

If this won't work, we'll have to add some flag to the platform data, i.e. to force JEDEC probing, and not trust platform data.

How about we add a non_jedec flag in platform_data, if the flag is 1, we let the detection pass even though the ID is 0? Otherwise, we need a valid ID?

Here i mean:
Index: drivers/mtd/devices/m25p80.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D

mtd/devices/m25p80.c +7 -2
@@ -795,8 +795,13 @@
 
 		jid =3D jedec_probe(spi);
 		if (!jid) {
-			dev_info(&spi->dev, "non-JEDEC variant of %s\n",
-				 id->name);
+			if (!data->non_jedec) {
+				dev_err(&spi->dev, "fail to detect%s\n",
+						id->name);
+				return -ENODEV;
+			} else
+				dev_info(&spi->dev, "non-JEDEC variant of %s\n",
+						id->name);
 		} else if (jid !=3D id) {
 			/*
 			 * JEDEC knows better, so overwrite platform ID. We

Index: include/linux/spi/flash.h =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D

linux/spi/flash.h +5 -0
@@ -25,6 +25,11 @@
 
 	char		*type;
 
+	/*
+	 * For non-JEDEC, id will be 0. In this case, we can't be sure
+	 * whether the flash exists with runtime probing.
+	 */
+	int             non_jedec;
 	/* we'll likely add more ... use JEDEC IDs, etc */
 };
 
Quoted 13 lines
Quoted 13 lines

---

drivers/mtd/devices/m25p80.c +0 -0
index 81e49a9..a307929 100644
@@ -16,6 +16,7 @@

=C2=A0*/

Quoted 9 lines
Quoted 9 lines

#include <linux/init.h>
+#include <linux/errno.h>
#include <linux/module.h>
#include <linux/device.h>
#include <linux/interrupt.h>

drivers/mtd/devices/m25p80.c +0 -0
@@ -723,7 +724,7 @@ static const struct spi_device_id
 *__devinit jedec_probe(struct spi_device *spi)
 =C2=A0 =C2=A0 =C2=A0 if (tmp < 0) {

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 DEBUG(MTD_DEBUG_LEVEL0,=

"%s: error %d reading

Quoted 2 lines
Quoted 2 lines

JEDEC ID\n",
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =

=C2=A0 dev_name(&spi->dev), tmp);

Quoted 8 lines
Quoted 8 lines

- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return NULL;
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return ERR_PTR(tmp); =C2=A0 =C2=A0 =C2=A0 }
=C2=A0 =C2=A0 =C2=A0 jedec =3D id[0];
=C2=A0 =C2=A0 =C2=A0 jedec =3D jedec << 8;

drivers/mtd/devices/m25p80.c +0 -0
@@ -737,7 +738,7 @@ static const struct spi_device_id
 *__devinit jedec_probe(struct spi_device *spi)

=C2=A0 =C2=A0 =C2=A0 =C2=A0* exist for non-JEDEC chips, but for compatib=

ility

Quoted 5 lines
Quoted 5 lines

they return ID 0.
=C2=A0 =C2=A0 =C2=A0 =C2=A0*/
=C2=A0 =C2=A0 =C2=A0 if (jedec =3D=3D 0)
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return NULL;
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return ERR_PTR(-EEXIST)=

;

Quoted 6 lines
Quoted 6 lines

=C2=A0 =C2=A0 =C2=A0 ext_jedec =3D id[3] << 8 | id[4];

drivers/mtd/devices/m25p80.c +0 -0
@@ -749,7 +750,7 @@ static const struct spi_device_id
 *__devinit jedec_probe(struct spi_device *spi)

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =

=C2=A0 return &m25p_ids[tmp];

Quoted 10 lines
Quoted 10 lines

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 } =C2=A0 =C2=A0 =C2=A0 }
- =C2=A0 =C2=A0 =C2=A0return NULL;
+ =C2=A0 =C2=A0 =C2=A0return ERR_PTR(-ENODEV); }

drivers/mtd/devices/m25p80.c +0 -0
@@ -794,9 +795,11 @@ static int __devinit m25p_probe(struct
 spi_device *spi)

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 const struct spi_device=

_id *jid;

Quoted 2 lines
Quoted 2 lines

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 jid =3D jedec_probe(spi=

);

Quoted 2 lines
Quoted 2 lines

- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (!jid) {
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (IS_ERR(jid) && PTR_=

ERR(jid) =3D=3D -EEXIST) {

Quoted 1 line
Quoted 1 line

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =

=C2=A0 dev_info(&spi->dev, "non-JEDEC variant of %s\n",

Quoted 1 line
Quoted 1 line

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0id->name);

Quoted 1 line
Quoted 1 line

+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0} else if (IS_ERR(jid))=

{

Quoted 1 line
Quoted 1 line

+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =

=C2=A0return PTR_ERR(jid);

Quoted 1 line
Quoted 1 line

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 } else if (jid !=3D id)=

{

Quoted 1 line
Quoted 1 line

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =

=C2=A0 /*

Quoted 1 line
Quoted 1 line

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =

=C2=A0 =C2=A0* JEDEC knows better, so overwrite

Quoted 7 lines
Quoted 6 lines

platform ID. We
_______________________________________________
Uclinux-dist-devel mailing list
[email protected]
https://blackfin.uclinux.org/mailman/listinfo/uclinux-dist-devel

#10 lore raw

Re: [Uclinux-dist-devel] [PATCH 1/2] mtd: m25p80: Reworkprobing/JEDEC code

On Mon, Jun 21, 2010 at 11:27:31AM +0800, Barry Song wrote:
[...]

Quoted 4 lines
Quoted 3 lines

How about we add a non_jedec flag in platform_data, if the flag is 1, we let the detection pass even though the ID is 0? Otherwise, we need a valid ID?

Here i mean:

This will break at least OF-enabled platforms (e.g. PowerPC), they assume that the driver will success for non-JEDEC flashes. OF platforms don't pass platform data, and even if they did, device tree doesn't specify if the flash is JEDEC or non-JEDEC.

Which is why I think that, by default, the driver should successfully register the flash even if JEDEC probe fails. So, instead of checking for "!non_jedec", I would recommend "force_jedec" check.

Quoted 18 lines

Index: drivers/mtd/devices/m25p80.c
===================================================================

mtd/devices/m25p80.c +7 -2
@@ -795,8 +795,13 @@
 
 		jid = jedec_probe(spi);
 		if (!jid) {
-			dev_info(&spi->dev, "non-JEDEC variant of %s\n",
-				 id->name);
+			if (!data->non_jedec) {
+				dev_err(&spi->dev, "fail to detect%s\n",
+						id->name);
+				return -ENODEV;
+			} else
+				dev_info(&spi->dev, "non-JEDEC variant of %s\n",
+						id->name);
 		} else if (jid != id) {
Signature · 3 lines
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
#11 lore raw

Re: [Uclinux-dist-devel] [PATCH 1/2] mtd: m25p80: Reworkprobing/JEDEC code

Quoted 12 lines · Anton Vorontsov

On Mon, Jun 21, 2010 at 11:27:31AM +0800, Barry Song wrote:
[...]

Quoted 4 lines
Quoted 3 lines

How about we add a non_jedec flag in platform_data, if the flag is 1, we let the detection pass even though the ID is 0? Otherwise, we need a valid ID?

Here i mean:

This will break at least OF-enabled platforms (e.g. PowerPC), they assume that the driver will success for non-JEDEC flashes. OF platforms don't pass platform data, and even if they did, device tree doesn't specify if the flash is JEDEC or non-JEDEC.

Quoted 4 lines

Which is why I think that, by default, the driver should successfully register the flash even if JEDEC probe fails. So, instead of checking for "!non_jedec", I would recommend "force_jedec" check.

Mike Frysinger suggested to use non_jedec since most devices are standard jedec devices. Only if non_jedec=1, we let the detection pass if ID is 0.

Quoted 25 lines
Quoted 9 lines

Index: drivers/mtd/devices/m25p80.c
===================================================================

mtd/devices/m25p80.c      (revision 8929) renamed from mtd/devices/m25p80.c      (revision 8927) +0 -0
@@ -795,8 +795,13 @@
 
               jid = jedec_probe(spi);
               if (!jid) {

mtd/devices/m25p80.c      (revision 8929) line 805

Quoted 10 lines
mtd/devices/m25p80.c      (revision 8929) renamed from mtd/devices/m25p80.c      (revision 8927) +7 -2
-                     dev_info(&spi->dev, "non-JEDEC variant of %s\n",
-                              id->name);
+                     if (!data->non_jedec) {
+                             dev_err(&spi->dev, "fail to detect%s\n",
+                                             id->name);
+                             return -ENODEV;
+                     } else
+                             dev_info(&spi->dev, "non-JEDEC variant of %s\n",
+                                             id->name);
               } else if (jid != id) {

--
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2

#12 lore raw

Re: [Uclinux-dist-devel] [PATCH 1/2] mtd: m25p80: Reworkprobing/JEDEC code

Quoted 13 lines · Barry Song
Quoted 12 lines · Anton Vorontsov

On Mon, Jun 21, 2010 at 11:27:31AM +0800, Barry Song wrote:
[...]

Quoted 4 lines
Quoted 3 lines

How about we add a non_jedec flag in platform_data, if the flag is 1, we let the detection pass even though the ID is 0? Otherwise, we need a valid ID?

Here i mean:

This will break at least OF-enabled platforms (e.g. PowerPC), they assume that the driver will success for non-JEDEC flashes. OF platforms don't pass platform data, and even if they did, device tree doesn't specify if the flash is JEDEC or non-JEDEC.

Quoted 7 lines
Quoted 4 lines

Which is why I think that, by default, the driver should successfully register the flash even if JEDEC probe fails. So, instead of checking for "!non_jedec", I would recommend "force_jedec" check.

Mike Frysinger suggested to use non_jedec since most devices are standard jedec devices.

Well, on OF platforms most devices that I'm aware of are non-JEDEC.

Quoted 2 lines

Only if non_jedec=1, we let the detection pass
if ID is 0.

Then please #ifdef it with CONFIG_OF.

Thanks,

Quoted 24 lines
Quoted 24 lines
Quoted 8 lines

Index: drivers/mtd/devices/m25p80.c
===================================================================

mtd/devices/m25p80.c      (revision 8929) renamed from mtd/devices/m25p80.c      (revision 8927) +0 -0
@@ -795,8 +795,13 @@
 
               jid = jedec_probe(spi);
               if (!jid) {

mtd/devices/m25p80.c      (revision 8929) line 805

Quoted 10 lines
mtd/devices/m25p80.c      (revision 8929) renamed from mtd/devices/m25p80.c      (revision 8927) +7 -2
-                     dev_info(&spi->dev, "non-JEDEC variant of %s\n",
-                              id->name);
+                     if (!data->non_jedec) {
+                             dev_err(&spi->dev, "fail to detect%s\n",
+                                             id->name);
+                             return -ENODEV;
+                     } else
+                             dev_info(&spi->dev, "non-JEDEC variant of %s\n",
+                                             id->name);
               } else if (jid != id) {

--
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2

Signature · 3 lines
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
#13 lore raw

Re: [Uclinux-dist-devel] [PATCH 1/2] mtd: m25p80: Reworkprobing/JEDEC code

On Mon, Jun 21, 2010 at 3:39 PM, Anton Vorontsov <[email protected]> w= rote:

Quoted 6 lines · Barry Song
Quoted 1 line

On Mon, Jun 21, 2010 at 3:15 PM, Anton Vorontsov <[email protected]=

Quoted 3 lines · wrote:
Quoted 3 lines · Barry Song

[...]

Quoted 1 line
Quoted 1 line

How about we add a non_jedec flag in platform_data, if the flag is =

1, we

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

let the detection pass even though the ID is 0? Otherwise, we need =

a

Quoted 16 lines
Quoted 8 lines
Quoted 8 lines
Quoted 2 lines
Quoted 1 line

valid ID?

Here i mean:

This will break at least OF-enabled platforms (e.g. PowerPC), they assume that the driver will success for non-JEDEC flashes. OF platforms don't pass platform data, and even if they did, device tree doesn't specify if the flash is JEDEC or non-JEDEC.

Quoted 7 lines
Quoted 4 lines

Which is why I think that, by default, the driver should successfully register the flash even if JEDEC probe fails. So, instead of checking for "!non_jedec", I would recommend "force_jedec" check.

Mike Frysinger suggested to use non_jedec since most devices are standard jedec devices.

Quoted 6 lines

Well, on OF platforms most devices that I'm aware of are non-JEDEC.

Quoted 2 lines

Only if non_jedec=3D1, we let the detection pass
if ID is 0.

Then please #ifdef it with CONFIG_OF.

I think the patch has nothing to do with platform. Here SPI Flash is a peripherals, doesn't depend on any platform. Adding a CONFIG_OF doesn't make sense very much.
If you think most devices are non-JEDEC, we can change non_JEDEC to force_JEDEC as you said. But anyway, is that real that most devices are non_JEDEC? If not, I think we should change OF platform codes to fit with this patch.

Quoted 5 lines

Thanks,

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

Index: drivers/mtd/devices/m25p80.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=

=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
mtd/devices/m25p80.c =C2=A0 =C2=A0 =C2=A0(revision 8929) renamed from mtd/devices/m25p80.c =C2=A0 =C2=A0 =C2=A0(revision 8927) +0 -0
@@ -795,8 +795,13 @@
 

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 jid =3D jedec_probe(=

spi);

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!jid) {
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=

=A0 dev_info(&spi->dev, "non-JEDEC variant of %s\n",

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=

=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0id->name);

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=

=A0 if (!data->non_jedec) {

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=

=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 dev_err(&spi->dev, "fail to detect%s\n",

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=

=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 id->name);

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=

=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return -ENODEV;

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=

=A0 } else

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=

=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 dev_info(&spi->dev, "non-JEDEC variant of %= s\n",

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=

=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 id->name);

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 } else if (jid !=3D =

id) {

Quoted 12 lines
Quoted 6 lines
Quoted 6 lines

--
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2

--
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2

#14 lore raw

Re: [Uclinux-dist-devel] [PATCH 1/2] mtd: m25p80: Reworkprobing/JEDEC code

Quoted 25 lines · Barry Song
Quoted 24 lines · Anton Vorontsov
Quoted 13 lines · Barry Song
Quoted 12 lines · Anton Vorontsov

On Mon, Jun 21, 2010 at 11:27:31AM +0800, Barry Song wrote:
[...]

Quoted 4 lines
Quoted 3 lines

How about we add a non_jedec flag in platform_data, if the flag is 1, we let the detection pass even though the ID is 0? Otherwise, we need a valid ID?

Here i mean:

This will break at least OF-enabled platforms (e.g. PowerPC), they assume that the driver will success for non-JEDEC flashes. OF platforms don't pass platform data, and even if they did, device tree doesn't specify if the flash is JEDEC or non-JEDEC.

Quoted 7 lines
Quoted 4 lines

Which is why I think that, by default, the driver should successfully register the flash even if JEDEC probe fails. So, instead of checking for "!non_jedec", I would recommend "force_jedec" check.

Mike Frysinger suggested to use non_jedec since most devices are standard jedec devices.

Well, on OF platforms most devices that I'm aware of are non-JEDEC.

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

Only if non_jedec=1, we let the detection pass
if ID is 0.

Then please #ifdef it with CONFIG_OF.

I think the patch has nothing to do with platform. Here SPI Flash is a peripherals, doesn't depend on any platform. Adding a CONFIG_OF doesn't make sense very much.

With OF we don't place non-existent devices into the device tree (or we mark them with status = "not-ok/disabled/absent" property).

Quoted 3 lines

If you think most devices are non-JEDEC, we can change non_JEDEC to force_JEDEC as you said.
But anyway, is that real that most devices are non_JEDEC?

Why would this matter? We have to support both.

Quoted 2 lines

If not, I think we should change OF platform codes to fit with this patch.

You can't easily change OF. It's like "let's change ACPI tables or BIOS in these PCs". Doable, but involves things like reflashing. And we usually have to support old BIOSes as well.

OTOH, I see (git grep m25p arch/powerpc/boot/dts/) that in mainline kernel only MPC8569 board has a correct m25p node, and it is STMicro variant (it is JEDEC capable).

As we don't really have to support out of tree code, I'd just go with this patch, assuming that we have to change device tree for boards with non-JEDEC flashes. It's effectively the same thing as platform data flag, except that it works automatically for OF platforms.

---

drivers/mtd/devices/m25p80.c +11 -2
index 81e49a9..a610ca9 100644
@@ -680,6 +680,16 @@ static const struct spi_device_id m25p_ids[] = {
 	{ "m25p64",  INFO(0x202017,  0,  64 * 1024, 128, 0) },
 	{ "m25p128", INFO(0x202018,  0, 256 * 1024,  64, 0) },
 
+	{ "m25p05-nonjedec",  INFO(0, 0,  32 * 1024,   2, 0) },
+	{ "m25p10-nonjedec",  INFO(0, 0,  32 * 1024,   4, 0) },
+	{ "m25p20-nonjedec",  INFO(0, 0,  64 * 1024,   4, 0) },
+	{ "m25p40-nonjedec",  INFO(0, 0,  64 * 1024,   8, 0) },
+	{ "m25p80-nonjedec",  INFO(0, 0,  64 * 1024,  16, 0) },
+	{ "m25p16-nonjedec",  INFO(0, 0,  64 * 1024,  32, 0) },
+	{ "m25p32-nonjedec",  INFO(0, 0,  64 * 1024,  64, 0) },
+	{ "m25p64-nonjedec",  INFO(0, 0,  64 * 1024, 128, 0) },
+	{ "m25p128-nonjedec", INFO(0, 0, 256 * 1024,  64, 0) },
+
 	{ "m45pe10", INFO(0x204011,  0, 64 * 1024,    2, 0) },
 	{ "m45pe80", INFO(0x204014,  0, 64 * 1024,   16, 0) },
 	{ "m45pe16", INFO(0x204015,  0, 64 * 1024,   32, 0) },
@@ -795,8 +805,7 @@ static int __devinit m25p_probe(struct spi_device *spi)
 
 		jid = jedec_probe(spi);
 		if (!jid) {
-			dev_info(&spi->dev, "non-JEDEC variant of %s\n",
-				 id->name);
+			return -ENODEV;
 		} else if (jid != id) {
 			/*
 			 * JEDEC knows better, so overwrite platform ID. We
#15 lore raw

Re: [Uclinux-dist-devel] [PATCH 1/2] mtd: m25p80: Reworkprobing/JEDEC code

Quoted 22 lines · Anton Vorontsov

You can't easily change OF. It's like "let's change ACPI tables or BIOS in these PCs". Doable, but involves things like reflashing. And we usually have to support old BIOSes as well.

OTOH, I see (git grep m25p arch/powerpc/boot/dts/) that in mainline kernel only MPC8569 board has a correct m25p node, and it is STMicro variant (it is JEDEC capable).

As we don't really have to support out of tree code, I'd just go with this patch, assuming that we have to change device tree for boards with non-JEDEC flashes. It's effectively the same thing as platform data flag, except that it works automatically for OF platforms.

drivers/mtd/devices/m25p80.c +0 -0
index 81e49a9..a610ca9 100644
@@ -680,6 +680,16 @@ static const struct spi_device_id m25p_ids[] = {
        { "m25p64",  INFO(0x202017,  0,  64 * 1024, 128, 0) },
        { "m25p128", INFO(0x202018,  0, 256 * 1024,  64, 0) },
 

drivers/mtd/devices/m25p80.c static const struct spi_device_id m25p_ids[] = { · line 692

Quoted 10 lines
drivers/mtd/devices/m25p80.c +10 -0
+       { "m25p05-nonjedec",  INFO(0, 0,  32 * 1024,   2, 0) },
+       { "m25p10-nonjedec",  INFO(0, 0,  32 * 1024,   4, 0) },
+       { "m25p20-nonjedec",  INFO(0, 0,  64 * 1024,   4, 0) },
+       { "m25p40-nonjedec",  INFO(0, 0,  64 * 1024,   8, 0) },
+       { "m25p80-nonjedec",  INFO(0, 0,  64 * 1024,  16, 0) },
+       { "m25p16-nonjedec",  INFO(0, 0,  64 * 1024,  32, 0) },
+       { "m25p32-nonjedec",  INFO(0, 0,  64 * 1024,  64, 0) },
+       { "m25p64-nonjedec",  INFO(0, 0,  64 * 1024, 128, 0) },
+       { "m25p128-nonjedec", INFO(0, 0, 256 * 1024,  64, 0) },
+

are you picking the m25p because its flash geometry matches whatever you're using, or because you have some weird variant of the m25p that has JEDEC commands removed ?
-mike

#16 lore raw

Re: [Uclinux-dist-devel] [PATCH 1/2] mtd: m25p80: Reworkprobing/JEDEC code

Quoted 23 lines · Mike Frysinger
Quoted 22 lines · Anton Vorontsov

You can't easily change OF. It's like "let's change ACPI tables or BIOS in these PCs". Doable, but involves things like reflashing. And we usually have to support old BIOSes as well.

OTOH, I see (git grep m25p arch/powerpc/boot/dts/) that in mainline kernel only MPC8569 board has a correct m25p node, and it is STMicro variant (it is JEDEC capable).

As we don't really have to support out of tree code, I'd just go with this patch, assuming that we have to change device tree for boards with non-JEDEC flashes. It's effectively the same thing as platform data flag, except that it works automatically for OF platforms.

drivers/mtd/devices/m25p80.c +0 -0
index 81e49a9..a610ca9 100644
@@ -680,6 +680,16 @@ static const struct spi_device_id m25p_ids[] = {
        { "m25p64",  INFO(0x202017,  0,  64 * 1024, 128, 0) },
        { "m25p128", INFO(0x202018,  0, 256 * 1024,  64, 0) },
 
Quoted 14 lines

drivers/mtd/devices/m25p80.c static const struct spi_device_id m25p_ids[] = { · line 692

Quoted 10 lines
drivers/mtd/devices/m25p80.c +10 -0
+       { "m25p05-nonjedec",  INFO(0, 0,  32 * 1024,   2, 0) },
+       { "m25p10-nonjedec",  INFO(0, 0,  32 * 1024,   4, 0) },
+       { "m25p20-nonjedec",  INFO(0, 0,  64 * 1024,   4, 0) },
+       { "m25p40-nonjedec",  INFO(0, 0,  64 * 1024,   8, 0) },
+       { "m25p80-nonjedec",  INFO(0, 0,  64 * 1024,  16, 0) },
+       { "m25p16-nonjedec",  INFO(0, 0,  64 * 1024,  32, 0) },
+       { "m25p32-nonjedec",  INFO(0, 0,  64 * 1024,  64, 0) },
+       { "m25p64-nonjedec",  INFO(0, 0,  64 * 1024, 128, 0) },
+       { "m25p128-nonjedec", INFO(0, 0, 256 * 1024,  64, 0) },
+

are you picking the m25p because its flash geometry matches whatever you're using, or because you have some weird variant of the m25p that has JEDEC commands removed ?

The latter. It's Numonyx M25Pxx flashes, see
http://www.numonyx.com/Documents/Datasheets/M25P80.pdf

The RDID instruction is available only for parts made with 110 nm Technology identified with Process letter '4'.

Signature · 3 lines
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
#17 lore raw

Re: [Uclinux-dist-devel] [PATCH 1/2] mtd: m25p80: Reworkprobing/JEDEC code

Quoted 10 lines · Anton Vorontsov
Quoted 9 lines · Mike Frysinger
Quoted 8 lines · Anton Vorontsov

You can't easily change OF. It's like "let's change ACPI tables or BIOS in these PCs". Doable, but involves things like reflashing. And we usually have to support old BIOSes as well.

OTOH, I see (git grep m25p arch/powerpc/boot/dts/) that in mainline kernel only MPC8569 board has a correct m25p node, and it is STMicro variant (it is JEDEC capable).

Quoted 7 lines
Quoted 7 lines

drivers/mtd/devices/m25p80=

Quoted 7 lines

As we don't really have to support out of tree code, I'd just go with this patch, assuming that we have to change device tree for boards with non-JEDEC flashes. It's effectively the same thing as platform data flag, except that it works automatically for OF platforms.

drivers/mtd/devices/m25p80= renamed from drivers/mtd/devices/m25p80.c +0 -0

.c

Quoted 4 lines
Quoted 4 lines

drivers/mtd/devices/m25p80.c static const struct spi_device_id m25p_ids[] =3D =

Quoted 4 lines

index 81e49a9..a610ca9 100644

drivers/mtd/devices/m25p80.c +0 -0
@@ -680,6 +680,16 @@ static const struct spi_device_id m25p_ids[] =3D =

{

Quoted 1 line
Quoted 1 line
Quoted 1 line

=C2=A0 =C2=A0 =C2=A0 =C2=A0{ "m25p64", =C2=A0INFO(0x202017, =C2=A00, =

=C2=A064 * 1024, 128, 0) },

Quoted 1 line
Quoted 1 line
Quoted 1 line

=C2=A0 =C2=A0 =C2=A0 =C2=A0{ "m25p128", INFO(0x202018, =C2=A00, 256 * =

1024, =C2=A064, 0) },

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

+ =C2=A0 =C2=A0 =C2=A0 { "m25p05-nonjedec", =C2=A0INFO(0, 0, =C2=A032 =

* 1024, =C2=A0 2, 0) },

Quoted 1 line
Quoted 1 line
Quoted 1 line

+ =C2=A0 =C2=A0 =C2=A0 { "m25p10-nonjedec", =C2=A0INFO(0, 0, =C2=A032 =

* 1024, =C2=A0 4, 0) },

Quoted 1 line
Quoted 1 line
Quoted 1 line

+ =C2=A0 =C2=A0 =C2=A0 { "m25p20-nonjedec", =C2=A0INFO(0, 0, =C2=A064 =

* 1024, =C2=A0 4, 0) },

Quoted 1 line
Quoted 1 line
Quoted 1 line

+ =C2=A0 =C2=A0 =C2=A0 { "m25p40-nonjedec", =C2=A0INFO(0, 0, =C2=A064 =

* 1024, =C2=A0 8, 0) },

Quoted 1 line
Quoted 1 line
Quoted 1 line

+ =C2=A0 =C2=A0 =C2=A0 { "m25p80-nonjedec", =C2=A0INFO(0, 0, =C2=A064 =

* 1024, =C2=A016, 0) },

Quoted 1 line
Quoted 1 line
Quoted 1 line

+ =C2=A0 =C2=A0 =C2=A0 { "m25p16-nonjedec", =C2=A0INFO(0, 0, =C2=A064 =

* 1024, =C2=A032, 0) },

Quoted 1 line
Quoted 1 line
Quoted 1 line

+ =C2=A0 =C2=A0 =C2=A0 { "m25p32-nonjedec", =C2=A0INFO(0, 0, =C2=A064 =

* 1024, =C2=A064, 0) },

Quoted 1 line
Quoted 1 line
Quoted 1 line

+ =C2=A0 =C2=A0 =C2=A0 { "m25p64-nonjedec", =C2=A0INFO(0, 0, =C2=A064 =

* 1024, 128, 0) },

Quoted 1 line
Quoted 1 line
Quoted 1 line

+ =C2=A0 =C2=A0 =C2=A0 { "m25p128-nonjedec", INFO(0, 0, 256 * 1024, =

=C2=A064, 0) },

Quoted 11 lines
Quoted 5 lines
Quoted 1 line

+

are you picking the m25p because its flash geometry matches whatever you're using, or because you have some weird variant of the m25p that has JEDEC commands removed ?

The latter. It's Numonyx M25Pxx flashes, see
http://www.numonyx.com/Documents/Datasheets/M25P80.pdf

=C2=A0 The RDID instruction is available only for parts made with 110 =C2=A0 nm Technology identified with Process letter '4'.

lovely. i guess this patch is the way to go to satisfy everyone's requirements. i'm also of the mindset that a mtd should not be created if the SPI flash isnt there simply because the resources say it might be.
-mike

#18 lore raw

Re: [Uclinux-dist-devel] [PATCH 1/2] mtd: m25p80: Reworkprobing/JEDEC code

In reply to Anton Vorontsov

On Mon, Jun 21, 2010 at 7:20 PM, Anton Vorontsov <[email protected]> w= rote:

Quoted 5 lines · Barry Song
Quoted 1 line

On Mon, Jun 21, 2010 at 3:39 PM, Anton Vorontsov <[email protected]=

Quoted 2 lines · wrote:
Quoted 2 lines · Barry Song
Quoted 1 line

On Mon, Jun 21, 2010 at 3:15 PM, Anton Vorontsov <cbouatmailru@gmail.=

Quoted 3 lines · com> wrote:
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines · Barry Song

[...]

Quoted 1 line
Quoted 1 line

How about we add a non_jedec flag in platform_data, if the flag =

is 1, we

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

let the detection pass even though the ID is 0? Otherwise, we ne=

ed a

Quoted 61 lines
Quoted 18 lines
Quoted 18 lines
Quoted 8 lines
Quoted 8 lines
Quoted 2 lines
Quoted 1 line

valid ID?

Here i mean:

This will break at least OF-enabled platforms (e.g. PowerPC), they assume that the driver will success for non-JEDEC flashes. OF platforms don't pass platform data, and even if they did, device tree doesn't specify if the flash is JEDEC or non-JEDEC.

Quoted 7 lines
Quoted 4 lines

Which is why I think that, by default, the driver should successfully register the flash even if JEDEC probe fails. So, instead of checking for "!non_jedec", I would recommend "force_jedec" check.

Mike Frysinger suggested to use non_jedec since most devices are standard jedec devices.

Well, on OF platforms most devices that I'm aware of are non-JEDEC.

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

Only if non_jedec=3D1, we let the detection pass
if ID is 0.

Then please #ifdef it with CONFIG_OF.

I think the patch has nothing to do with platform. Here SPI Flash is a peripherals, doesn't depend on any platform. Adding a CONFIG_OF doesn't make sense very much.

With OF we don't place non-existent devices into the device tree (or we mark them with status =3D "not-ok/disabled/absent" property).

Quoted 3 lines

If you think most devices are non-JEDEC, we can change non_JEDEC to force_JEDEC as you said.
But anyway, is that real that most devices are non_JEDEC?

Why would this matter? We have to support both.

Quoted 2 lines

If not, I think we should change OF platform codes to fit with this patch.

You can't easily change OF. It's like "let's change ACPI tables or BIOS in these PCs". Doable, but involves things like reflashing. And we usually have to support old BIOSes as well.

OTOH, I see (git grep m25p arch/powerpc/boot/dts/) that in mainline kernel only MPC8569 board has a correct m25p node, and it is STMicro variant (it is JEDEC capable).

As we don't really have to support out of tree code, I'd just go with this patch, assuming that we have to change device tree for boards with non-JEDEC flashes. It's effectively the same thing as platform data flag, except that it works automatically for OF platforms.

---

drivers/mtd/devices/m25p80.c +0 -0
index 81e49a9..a610ca9 100644
@@ -680,6 +680,16 @@ static const struct spi_device_id m25p_ids[] =3D {
Quoted 1 line

=C2=A0 =C2=A0 =C2=A0 =C2=A0{ "m25p64", =C2=A0INFO(0x202017, =C2=A00, =C2=

=A064 * 1024, 128, 0) },

Quoted 1 line

=C2=A0 =C2=A0 =C2=A0 =C2=A0{ "m25p128", INFO(0x202018, =C2=A00, 256 * 102=

4, =C2=A064, 0) },

Quoted 2 lines

+ =C2=A0 =C2=A0 =C2=A0 { "m25p05-nonjedec", =C2=A0INFO(0, 0, =C2=A032 * 1=

024, =C2=A0 2, 0) },

Quoted 1 line

+ =C2=A0 =C2=A0 =C2=A0 { "m25p10-nonjedec", =C2=A0INFO(0, 0, =C2=A032 * 1=

024, =C2=A0 4, 0) },

Quoted 1 line

+ =C2=A0 =C2=A0 =C2=A0 { "m25p20-nonjedec", =C2=A0INFO(0, 0, =C2=A064 * 1=

024, =C2=A0 4, 0) },

Quoted 1 line

+ =C2=A0 =C2=A0 =C2=A0 { "m25p40-nonjedec", =C2=A0INFO(0, 0, =C2=A064 * 1=

024, =C2=A0 8, 0) },

Quoted 1 line

+ =C2=A0 =C2=A0 =C2=A0 { "m25p80-nonjedec", =C2=A0INFO(0, 0, =C2=A064 * 1=

024, =C2=A016, 0) },

Quoted 1 line

+ =C2=A0 =C2=A0 =C2=A0 { "m25p16-nonjedec", =C2=A0INFO(0, 0, =C2=A064 * 1=

024, =C2=A032, 0) },

Quoted 1 line

+ =C2=A0 =C2=A0 =C2=A0 { "m25p32-nonjedec", =C2=A0INFO(0, 0, =C2=A064 * 1=

024, =C2=A064, 0) },

Quoted 1 line

+ =C2=A0 =C2=A0 =C2=A0 { "m25p64-nonjedec", =C2=A0INFO(0, 0, =C2=A064 * 1=

024, 128, 0) },

Quoted 1 line

+ =C2=A0 =C2=A0 =C2=A0 { "m25p128-nonjedec", INFO(0, 0, 256 * 1024, =C2=

=A064, 0) },

Quoted 2 lines

+
=C2=A0 =C2=A0 =C2=A0 =C2=A0{ "m45pe10", INFO(0x204011, =C2=A00, 64 * 1024=

, =C2=A0 =C2=A02, 0) },

Quoted 1 line

=C2=A0 =C2=A0 =C2=A0 =C2=A0{ "m45pe80", INFO(0x204014, =C2=A00, 64 * 1024=

, =C2=A0 16, 0) },

Quoted 1 line

=C2=A0 =C2=A0 =C2=A0 =C2=A0{ "m45pe16", INFO(0x204015, =C2=A00, 64 * 1024=

, =C2=A0 32, 0) },

drivers/mtd/devices/m25p80.c static int __devinit m25p_probe(struct spi_device *sp=

Quoted 1 line
drivers/mtd/devices/m25p80.c +0 -0
@@ -795,8 +805,7 @@ static int __devinit m25p_probe(struct spi_device *sp=

i)

Quoted 2 lines

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0jid =3D jedec_prob=

e(spi);

Quoted 2 lines

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (!jid) {
- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =

=C2=A0 dev_info(&spi->dev, "non-JEDEC variant of %s\n",

Quoted 1 line

- =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0id->name);

Quoted 1 line

+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =

=C2=A0 return -ENODEV;
The patch looks good to me. Only problem is NULL is also returned by spi_write_then_read() fail:
static const struct spi_device_id *__devinit jedec_probe(struct spi_device = *spi)

{
        int                     tmp;
        u8                      code =3D OPCODE_RDID;
        u8                      id[5];
        u32                     jedec;
        u16                     ext_jedec;
        struct flash_info       *info;

        /* JEDEC also defines an optional "extended device information"
         * string for after vendor-specific data, after the three bytes
         * we use here.  Supporting some chips might require using it.
         */
        tmp =3D spi_write_then_read(spi, &code, 1, id, 5);
        if (tmp < 0) {
                DEBUG(MTD_DEBUG_LEVEL0, "%s: error %d reading JEDEC ID\n",
                        dev_name(&spi->dev), tmp);
                return NULL;
        }

...
}

Here much better for -EIO (return tmp)?

Quoted 1 line

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0} else if (jid !=

=3D id) {

Quoted 1 line

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=

=A0 =C2=A0/*

Quoted 1 line

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=

=A0 =C2=A0 * JEDEC knows better, so overwrite platform ID. We

Quoted 1 line
#19 lore raw

Re: [Uclinux-dist-devel] [PATCH 1/2] mtd: m25p80: Reworkprobing/JEDEC code

On Tue, Jun 22, 2010 at 02:37:52PM +0800, Barry Song wrote:
[...]

Quoted 7 lines
Quoted 5 lines

               jid = jedec_probe(spi);
               if (!jid) {
-                       dev_info(&spi->dev, "non-JEDEC variant of %s\n",
-                                id->name);
+                       return -ENODEV;

The patch looks good to me. Only problem is NULL is also returned by spi_write_then_read() fail:

[...]

Quoted 1 line

Here much better for -EIO (return tmp)?

Agreed. Though, this is not a regression, and I guess desires its own patch.

Here are two patches, one for 2.6.35 (minimal changes to fix the JEDEC problem), another for 2.6.36.

#20 lore raw

PATCH 1/2 mtd: m25p80: Fix false-positive probing

Since commit 18c6182bae0acca220ed6611f741034d563cd19f ("Rework probing/JEDEC code"), m25p80 driver successfully registers chips even if JEDEC probing fails.

This was needed to support non-JEDEC flashes. Though, it appears that some platforms (e.g. blackfin bf533 stamp[1]) used the old behavior to detect if there's any flash connected, so the driver have to fail on JEDEC probing errors.

This patch restores the old behavior for JEDEC flashes, and adds "-nonjedec" SPI device IDs for M25Pxx flashes, so that the kernel still supports non-JEDEC flashes.

[1] http://blackfin.uclinux.org/gf/project/uclinux-dist/tracker/?action=TrackerItemEdit&tracker_item_id=5975

Reported-by Mingquan Pan

---

This is for 2.6.35.

 drivers/mtd/devices/m25p80.c |   13 +++++++++++--
 1 files changed, 11 insertions(+), 2 deletions(-)
drivers/mtd/devices/m25p80.c +11 -2
index 81e49a9..a610ca9 100644
@@ -680,6 +680,16 @@ static const struct spi_device_id m25p_ids[] = {
 	{ "m25p64",  INFO(0x202017,  0,  64 * 1024, 128, 0) },
 	{ "m25p128", INFO(0x202018,  0, 256 * 1024,  64, 0) },
 
+	{ "m25p05-nonjedec",  INFO(0, 0,  32 * 1024,   2, 0) },
+	{ "m25p10-nonjedec",  INFO(0, 0,  32 * 1024,   4, 0) },
+	{ "m25p20-nonjedec",  INFO(0, 0,  64 * 1024,   4, 0) },
+	{ "m25p40-nonjedec",  INFO(0, 0,  64 * 1024,   8, 0) },
+	{ "m25p80-nonjedec",  INFO(0, 0,  64 * 1024,  16, 0) },
+	{ "m25p16-nonjedec",  INFO(0, 0,  64 * 1024,  32, 0) },
+	{ "m25p32-nonjedec",  INFO(0, 0,  64 * 1024,  64, 0) },
+	{ "m25p64-nonjedec",  INFO(0, 0,  64 * 1024, 128, 0) },
+	{ "m25p128-nonjedec", INFO(0, 0, 256 * 1024,  64, 0) },
+
 	{ "m45pe10", INFO(0x204011,  0, 64 * 1024,    2, 0) },
 	{ "m45pe80", INFO(0x204014,  0, 64 * 1024,   16, 0) },
 	{ "m45pe16", INFO(0x204015,  0, 64 * 1024,   32, 0) },
@@ -795,8 +805,7 @@ static int __devinit m25p_probe(struct spi_device *spi)
 
 		jid = jedec_probe(spi);
 		if (!jid) {
-			dev_info(&spi->dev, "non-JEDEC variant of %s\n",
-				 id->name);
+			return -ENODEV;
 		} else if (jid != id) {
 			/*
 			 * JEDEC knows better, so overwrite platform ID. We
Signature · 1 line
1.7.0.5
Acked-by on v1 1/2
#21 lore raw

Re: [PATCH 1/2] mtd: m25p80: Fix false-positive probing

Quoted 4 lines · Anton Vorontsov

Since commit 18c6182bae0acca220ed6611f741034d563cd19f ("Rework probing/JEDEC code"), m25p80 driver successfully registers chips even if JEDEC probing fails.

Quoted 8 lines

This was needed to support non-JEDEC flashes. Though, it appears that some platforms (e.g. blackfin bf533 stamp[1]) used the old behavior to detect if there's any flash connected, so the driver have to fail on JEDEC probing errors.

This patch restores the old behavior for JEDEC flashes, and adds "-nonjedec" SPI device IDs for M25Pxx flashes, so that the kernel still supports non-JEDEC flashes.

Quoted 1 line

[1] http://blackfin.uclinux.org/gf/project/uclinux-dist/tracker/?action=TrackerItemEdit&tracker_item_id=5975

http://blackfin.uclinux.org/gf/tracker/5975

Quoted 1 line
Reported-by Mingquan Pan

<[email protected]>
-mike

#22 lore raw

Re: [PATCH 1/2] mtd: m25p80: Fix false-positive probing

In reply to Anton Vorontsov
Quoted 13 lines · Anton Vorontsov

Since commit 18c6182bae0acca220ed6611f741034d563cd19f ("Rework probing/JEDEC code"), m25p80 driver successfully registers chips even if JEDEC probing fails.

This was needed to support non-JEDEC flashes. Though, it appears that some platforms (e.g. blackfin bf533 stamp[1]) used the old behavior to detect if there's any flash connected, so the driver have to fail on JEDEC probing errors.

This patch restores the old behavior for JEDEC flashes, and adds "-nonjedec" SPI device IDs for M25Pxx flashes, so that the kernel still supports non-JEDEC flashes.

Quoted 6 lines

[1] http://blackfin.uclinux.org/gf/project/uclinux-dist/tracker/?action=TrackerItemEdit&tracker_item_id=5975

Reported-by Mingquan Pan

---

Pushed both patches to my l2-mtd-2.6.git / dunno, added Mike's ack.

Signature · 2 lines
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
#23 lore raw

PATCH 2/2 mtd: m25p80: Make jedec_probe() return proper errno values

In reply to Anton Vorontsov

spi_write_then_read() may return its own return codes (e.g. -EIO), so let's propagate the value down to the probe().

Also, remove jedec == 0 check, it isn't needed as nowadays we use dedicated SPI device IDs for non-JEDEC flashes.

---

This is for 2.6.36.

 drivers/mtd/devices/m25p80.c |   18 ++++++------------
 1 files changed, 6 insertions(+), 12 deletions(-)
drivers/mtd/devices/m25p80.c +6 -12
index a610ca9..7bf5c45 100644
@@ -16,6 +16,8 @@
  */
 
 #include <linux/init.h>
+#include <linux/err.h>
+#include <linux/errno.h>
 #include <linux/module.h>
 #include <linux/device.h>
 #include <linux/interrupt.h>
@@ -733,7 +735,7 @@ static const struct spi_device_id *__devinit jedec_probe(struct spi_device *spi)
 	if (tmp < 0) {
 		DEBUG(MTD_DEBUG_LEVEL0, "%s: error %d reading JEDEC ID\n",
 			dev_name(&spi->dev), tmp);
-		return NULL;
+		return ERR_PTR(tmp);
 	}
 	jedec = id[0];
 	jedec = jedec << 8;
@@ -741,14 +743,6 @@ static const struct spi_device_id *__devinit jedec_probe(struct spi_device *spi)
 	jedec = jedec << 8;
 	jedec |= id[2];
 
-	/*
-	 * Some chips (like Numonyx M25P80) have JEDEC and non-JEDEC variants,
-	 * which depend on technology process. Officially RDID command doesn't
-	 * exist for non-JEDEC chips, but for compatibility they return ID 0.
-	 */
-	if (jedec == 0)
-		return NULL;
-
 	ext_jedec = id[3] << 8 | id[4];
 
 	for (tmp = 0; tmp < ARRAY_SIZE(m25p_ids) - 1; tmp++) {
@@ -759,7 +753,7 @@ static const struct spi_device_id *__devinit jedec_probe(struct spi_device *spi)
 			return &m25p_ids[tmp];
 		}
 	}
-	return NULL;
+	return ERR_PTR(-ENODEV);
 }
 
 
@@ -804,8 +798,8 @@ static int __devinit m25p_probe(struct spi_device *spi)
 		const struct spi_device_id *jid;
 
 		jid = jedec_probe(spi);
-		if (!jid) {
-			return -ENODEV;
+		if (IS_ERR(jid)) {
+			return PTR_ERR(jid);
 		} else if (jid != id) {
 			/*
 			 * JEDEC knows better, so overwrite platform ID. We
Signature · 1 line
1.7.0.5
#24 lore raw

PATCH 2/2 mtd: m25p80: Add support for CAT25xxx serial EEPROMs

In reply to Anton Vorontsov

CAT25 chips (as manufactured by On Semiconductor, previously Catalyst Semiconductor) are similar to the original M25Px0 chips, except:

- Address width can vary (1-2 bytes, in contrast to 3 bytes in M25P chips). So, implement convenient m25p_addr2cmd() and m25p_cmdsz() calls, and place address width information into flash_info struct;

- Page size can vary, therefore we shouldn't hardcode it, so get rid of FLASH_PAGESIZE definition, and place the page size information into flash_info struct;

- CAT25 EEPROMs don't need to be erased, so add NO_ERASE flag, and propagate it to the mtd subsystem.

---
 drivers/mtd/devices/m25p80.c |  172 ++++++++++++++++++++++++------------------
 1 files changed, 97 insertions(+), 75 deletions(-)
drivers/mtd/devices/m25p80.c +97 -75
index b75e319..8930266 100644
@@ -29,9 +29,6 @@
 #include <linux/spi/spi.h>
 #include <linux/spi/flash.h>
 
-
-#define FLASH_PAGESIZE		256
-
 /* Flash opcodes. */
 #define	OPCODE_WREN		0x06	/* Write enable */
 #define	OPCODE_RDSR		0x05	/* Read status register */
@@ -56,7 +53,7 @@
 
 /* Define max times to check status register before we give up. */
 #define	MAX_READY_WAIT_JIFFIES	(40 * HZ)	/* M25P16 specs 40s max chip erase */
-#define	CMD_SIZE		4
+#define	MAX_CMD_SIZE		4
 
 #ifdef CONFIG_M25PXX_USE_FAST_READ
 #define OPCODE_READ 	OPCODE_FAST_READ
@@ -73,8 +70,10 @@ struct m25p {
 	struct mutex		lock;
 	struct mtd_info		mtd;
 	unsigned		partitioned:1;
+	u16			page_size;
+	u16			addr_width;
 	u8			erase_opcode;
-	u8			command[CMD_SIZE + FAST_READ_DUMMY_BYTE];
+	u8			command[MAX_CMD_SIZE + FAST_READ_DUMMY_BYTE];
 };
 
 static inline struct m25p *mtd_to_m25p(struct mtd_info *mtd)
@@ -184,6 +183,19 @@ static int erase_chip(struct m25p *flash)
 	return 0;
 }
 
+static void m25p_addr2cmd(struct m25p *flash, unsigned int addr, u8 *cmd)
+{
+	/* opcode is in cmd[0] */
+	cmd[1] = addr >> (flash->addr_width * 8 -  8);
+	cmd[2] = addr >> (flash->addr_width * 8 - 16);
+	cmd[3] = addr >> (flash->addr_width * 8 - 24);
+}
+
+static int m25p_cmdsz(struct m25p *flash)
+{
+	return 1 + flash->addr_width;
+}
+
 /*
  * Erase one sector of flash memory at offset ``offset'' which is any
  * address within the sector which should be erased.
@@ -205,11 +217,9 @@ static int erase_sector(struct m25p *flash, u32 offset)
 
 	/* Set up command buffer. */
 	flash->command[0] = flash->erase_opcode;
-	flash->command[1] = offset >> 16;
-	flash->command[2] = offset >> 8;
-	flash->command[3] = offset;
+	m25p_addr2cmd(flash, offset, flash->command);
 
-	spi_write(flash->spi, flash->command, CMD_SIZE);
+	spi_write(flash->spi, flash->command, m25p_cmdsz(flash));
 
 	return 0;
 }
@@ -311,7 +321,7 @@ static int m25p80_read(struct mtd_info *mtd, loff_t from, size_t len,
 	 * Should add 1 byte DUMMY_BYTE.
 	 */
 	t[0].tx_buf = flash->command;
-	t[0].len = CMD_SIZE + FAST_READ_DUMMY_BYTE;
+	t[0].len = m25p_cmdsz(flash) + FAST_READ_DUMMY_BYTE;
 	spi_message_add_tail(&t[0], &m);
 
 	t[1].rx_buf = buf;
@@ -338,13 +348,11 @@ static int m25p80_read(struct mtd_info *mtd, loff_t from, size_t len,
 
 	/* Set up the write data buffer. */
 	flash->command[0] = OPCODE_READ;
-	flash->command[1] = from >> 16;
-	flash->command[2] = from >> 8;
-	flash->command[3] = from;
+	m25p_addr2cmd(flash, from, flash->command);
 
 	spi_sync(flash->spi, &m);
 
-	*retlen = m.actual_length - CMD_SIZE - FAST_READ_DUMMY_BYTE;
+	*retlen = m.actual_length - m25p_cmdsz(flash) - FAST_READ_DUMMY_BYTE;
 
 	mutex_unlock(&flash->lock);
 
@@ -382,7 +390,7 @@ static int m25p80_write(struct mtd_info *mtd, loff_t to, size_t len,
 	memset(t, 0, (sizeof t));
 
 	t[0].tx_buf = flash->command;
-	t[0].len = CMD_SIZE;
+	t[0].len = m25p_cmdsz(flash);
 	spi_message_add_tail(&t[0], &m);
 
 	t[1].tx_buf = buf;
@@ -400,41 +408,36 @@ static int m25p80_write(struct mtd_info *mtd, loff_t to, size_t len,
 
 	/* Set up the opcode in the write buffer. */
 	flash->command[0] = OPCODE_PP;
-	flash->command[1] = to >> 16;
-	flash->command[2] = to >> 8;
-	flash->command[3] = to;
+	m25p_addr2cmd(flash, to, flash->command);
 
-	/* what page do we start with? */
-	page_offset = to % FLASH_PAGESIZE;
+	page_offset = to & (flash->page_size - 1);
 
 	/* do all the bytes fit onto one page? */
-	if (page_offset + len <= FLASH_PAGESIZE) {
+	if (page_offset + len <= flash->page_size) {
 		t[1].len = len;
 
 		spi_sync(flash->spi, &m);
 
-		*retlen = m.actual_length - CMD_SIZE;
+		*retlen = m.actual_length - m25p_cmdsz(flash);
 	} else {
 		u32 i;
 
 		/* the size of data remaining on the first page */
-		page_size = FLASH_PAGESIZE - page_offset;
+		page_size = flash->page_size - page_offset;
 
 		t[1].len = page_size;
 		spi_sync(flash->spi, &m);
 
-		*retlen = m.actual_length - CMD_SIZE;
+		*retlen = m.actual_length - m25p_cmdsz(flash);
 
-		/* write everything in PAGESIZE chunks */
+		/* write everything in flash->page_size chunks */
 		for (i = page_size; i < len; i += page_size) {
 			page_size = len - i;
-			if (page_size > FLASH_PAGESIZE)
-				page_size = FLASH_PAGESIZE;
+			if (page_size > flash->page_size)
+				page_size = flash->page_size;
 
 			/* write the next page to flash */
-			flash->command[1] = (to + i) >> 16;
-			flash->command[2] = (to + i) >> 8;
-			flash->command[3] = (to + i);
+			m25p_addr2cmd(flash, to + i, flash->command);
 
 			t[1].tx_buf = buf + i;
 			t[1].len = page_size;
@@ -446,7 +449,7 @@ static int m25p80_write(struct mtd_info *mtd, loff_t to, size_t len,
 			spi_sync(flash->spi, &m);
 
 			if (retlen)
-				*retlen += m.actual_length - CMD_SIZE;
+				*retlen += m.actual_length - m25p_cmdsz(flash);
 		}
 	}
 
@@ -476,16 +479,23 @@ struct flash_info {
 	unsigned	sector_size;
 	u16		n_sectors;
 
+	u16		page_size;
+	u16		addr_width;
+
 	u16		flags;
 #define	SECT_4K		0x01		/* OPCODE_BE_4K works uniformly */
+#define	M25P_NO_ERASE	0x02		/* No erase command needed */
 };
 
-#define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags)	\
+#define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _page_size,	\
+		_addr_width, _flags)					\
 	((kernel_ulong_t)&(struct flash_info) {				\
 		.jedec_id = (_jedec_id),				\
 		.ext_id = (_ext_id),					\
 		.sector_size = (_sector_size),				\
 		.n_sectors = (_n_sectors),				\
+		.page_size = (_page_size),				\
+		.addr_width = (_addr_width),				\
 		.flags = (_flags),					\
 	})
 
@@ -495,63 +505,70 @@ struct flash_info {
  */
 static const struct spi_device_id m25p_ids[] = {
 	/* Atmel -- some are (confusingly) marketed as "DataFlash" */
-	{ "at25fs010",  INFO(0x1f6601, 0, 32 * 1024, 4, SECT_4K) },
-	{ "at25fs040",  INFO(0x1f6604, 0, 64 * 1024, 8, SECT_4K) },
+	{ "at25fs010",  INFO(0x1f6601, 0, 32 * 1024, 4, 256, 3, SECT_4K) },
+	{ "at25fs040",  INFO(0x1f6604, 0, 64 * 1024, 8, 256, 3, SECT_4K) },
 
-	{ "at25df041a", INFO(0x1f4401, 0, 64 * 1024, 8, SECT_4K) },
-	{ "at25df641",  INFO(0x1f4800, 0, 64 * 1024, 128, SECT_4K) },
+	{ "at25df041a", INFO(0x1f4401, 0, 64 * 1024, 8, 256, 3, SECT_4K) },
+	{ "at25df641",  INFO(0x1f4800, 0, 64 * 1024, 128, 256, 3, SECT_4K) },
 
-	{ "at26f004",   INFO(0x1f0400, 0, 64 * 1024, 8, SECT_4K) },
-	{ "at26df081a", INFO(0x1f4501, 0, 64 * 1024, 16, SECT_4K) },
-	{ "at26df161a", INFO(0x1f4601, 0, 64 * 1024, 32, SECT_4K) },
-	{ "at26df321",  INFO(0x1f4701, 0, 64 * 1024, 64, SECT_4K) },
+	{ "at26f004",   INFO(0x1f0400, 0, 64 * 1024, 8, 256, 3, SECT_4K) },
+	{ "at26df081a", INFO(0x1f4501, 0, 64 * 1024, 16, 256, 3, SECT_4K) },
+	{ "at26df161a", INFO(0x1f4601, 0, 64 * 1024, 32, 256, 3, SECT_4K) },
+	{ "at26df321",  INFO(0x1f4701, 0, 64 * 1024, 64, 256, 3, SECT_4K) },
 
 	/* Macronix */
-	{ "mx25l12805d", INFO(0xc22018, 0, 64 * 1024, 256, 0) },
+	{ "mx25l12805d", INFO(0xc22018, 0, 64 * 1024, 256, 256, 3, 0) },
 
 	/* Spansion -- single (large) sector size only, at least
 	 * for the chips listed here (without boot sectors).
 	 */
-	{ "s25sl004a",  INFO(0x010212, 0, 64 * 1024, 8, 0) },
-	{ "s25sl008a",  INFO(0x010213, 0, 64 * 1024, 16, 0) },
-	{ "s25sl016a",  INFO(0x010214, 0, 64 * 1024, 32, 0) },
-	{ "s25sl032a",  INFO(0x010215, 0, 64 * 1024, 64, 0) },
-	{ "s25sl064a",  INFO(0x010216, 0, 64 * 1024, 128, 0) },
-	{ "s25sl12800", INFO(0x012018, 0x0300, 256 * 1024, 64, 0) },
-	{ "s25sl12801", INFO(0x012018, 0x0301, 64 * 1024, 256, 0) },
+	{ "s25sl004a",  INFO(0x010212, 0, 64 * 1024, 8, 256, 3, 0) },
+	{ "s25sl008a",  INFO(0x010213, 0, 64 * 1024, 16, 256, 3, 0) },
+	{ "s25sl016a",  INFO(0x010214, 0, 64 * 1024, 32, 256, 3, 0) },
+	{ "s25sl032a",  INFO(0x010215, 0, 64 * 1024, 64, 256, 3, 0) },
+	{ "s25sl064a",  INFO(0x010216, 0, 64 * 1024, 128, 256, 3, 0) },
+	{ "s25sl12800", INFO(0x012018, 0x0300, 256 * 1024, 64, 256, 3, 0) },
+	{ "s25sl12801", INFO(0x012018, 0x0301, 64 * 1024, 256, 256, 3, 0) },
 
 	/* SST -- large erase sizes are "overlays", "sectors" are 4K */
-	{ "sst25vf040b", INFO(0xbf258d, 0, 64 * 1024, 8, SECT_4K) },
-	{ "sst25vf080b", INFO(0xbf258e, 0, 64 * 1024, 16, SECT_4K) },
-	{ "sst25vf016b", INFO(0xbf2541, 0, 64 * 1024, 32, SECT_4K) },
-	{ "sst25vf032b", INFO(0xbf254a, 0, 64 * 1024, 64, SECT_4K) },
+	{ "sst25vf040b", INFO(0xbf258d, 0, 64 * 1024, 8, 256, 3, SECT_4K) },
+	{ "sst25vf080b", INFO(0xbf258e, 0, 64 * 1024, 16, 256, 3, SECT_4K) },
+	{ "sst25vf016b", INFO(0xbf2541, 0, 64 * 1024, 32, 256, 3, SECT_4K) },
+	{ "sst25vf032b", INFO(0xbf254a, 0, 64 * 1024, 64, 256, 3, SECT_4K) },
 
 	/* ST Microelectronics -- newer production may have feature updates */
-	{ "m25p05",  INFO(0x202010,  0, 32 * 1024, 2, 0) },
-	{ "m25p10",  INFO(0x202011,  0, 32 * 1024, 4, 0) },
-	{ "m25p20",  INFO(0x202012,  0, 64 * 1024, 4, 0) },
-	{ "m25p40",  INFO(0x202013,  0, 64 * 1024, 8, 0) },
-	{ "m25p80",  INFO(0x202014,  0, 64 * 1024, 16, 0) },
-	{ "m25p16",  INFO(0x202015,  0, 64 * 1024, 32, 0) },
-	{ "m25p32",  INFO(0x202016,  0, 64 * 1024, 64, 0) },
-	{ "m25p64",  INFO(0x202017,  0, 64 * 1024, 128, 0) },
-	{ "m25p128", INFO(0x202018, 0, 256 * 1024, 64, 0) },
-
-	{ "m45pe10", INFO(0x204011,  0, 64 * 1024, 2, 0) },
-	{ "m45pe80", INFO(0x204014,  0, 64 * 1024, 16, 0) },
-	{ "m45pe16", INFO(0x204015,  0, 64 * 1024, 32, 0) },
-
-	{ "m25pe80", INFO(0x208014,  0, 64 * 1024, 16, 0) },
-	{ "m25pe16", INFO(0x208015,  0, 64 * 1024, 32, SECT_4K) },
+	{ "m25p05",  INFO(0x202010,  0, 32 * 1024, 2, 256, 3, 0) },
+	{ "m25p10",  INFO(0x202011,  0, 32 * 1024, 4, 256, 3, 0) },
+	{ "m25p20",  INFO(0x202012,  0, 64 * 1024, 4, 256, 3, 0) },
+	{ "m25p40",  INFO(0x202013,  0, 64 * 1024, 8, 256, 3, 0) },
+	{ "m25p80",  INFO(0x202014,  0, 64 * 1024, 16, 256, 3, 0) },
+	{ "m25p16",  INFO(0x202015,  0, 64 * 1024, 32, 256, 3, 0) },
+	{ "m25p32",  INFO(0x202016,  0, 64 * 1024, 64, 256, 3, 0) },
+	{ "m25p64",  INFO(0x202017,  0, 64 * 1024, 128, 256, 3, 0) },
+	{ "m25p128", INFO(0x202018, 0, 256 * 1024, 64, 256, 3, 0) },
+
+	{ "m45pe10", INFO(0x204011,  0, 64 * 1024, 2, 256, 3, 0) },
+	{ "m45pe80", INFO(0x204014,  0, 64 * 1024, 16, 256, 3, 0) },
+	{ "m45pe16", INFO(0x204015,  0, 64 * 1024, 32, 256, 3, 0) },
+
+	{ "m25pe80", INFO(0x208014,  0, 64 * 1024, 16, 256, 3, 0) },
+	{ "m25pe16", INFO(0x208015,  0, 64 * 1024, 32, 256, 3, SECT_4K) },
 
 	/* Winbond -- w25x "blocks" are 64K, "sectors" are 4KiB */
-	{ "w25x10", INFO(0xef3011, 0, 64 * 1024, 2, SECT_4K) },
-	{ "w25x20", INFO(0xef3012, 0, 64 * 1024, 4, SECT_4K) },
-	{ "w25x40", INFO(0xef3013, 0, 64 * 1024, 8, SECT_4K) },
-	{ "w25x80", INFO(0xef3014, 0, 64 * 1024, 16, SECT_4K) },
-	{ "w25x16", INFO(0xef3015, 0, 64 * 1024, 32, SECT_4K) },
-	{ "w25x32", INFO(0xef3016, 0, 64 * 1024, 64, SECT_4K) },
-	{ "w25x64", INFO(0xef3017, 0, 64 * 1024, 128, SECT_4K) },
+	{ "w25x10", INFO(0xef3011, 0, 64 * 1024, 2, 256, 3, SECT_4K) },
+	{ "w25x20", INFO(0xef3012, 0, 64 * 1024, 4, 256, 3, SECT_4K) },
+	{ "w25x40", INFO(0xef3013, 0, 64 * 1024, 8, 256, 3, SECT_4K) },
+	{ "w25x80", INFO(0xef3014, 0, 64 * 1024, 16, 256, 3, SECT_4K) },
+	{ "w25x16", INFO(0xef3015, 0, 64 * 1024, 32, 256, 3, SECT_4K) },
+	{ "w25x32", INFO(0xef3016, 0, 64 * 1024, 64, 256, 3, SECT_4K) },
+	{ "w25x64", INFO(0xef3017, 0, 64 * 1024, 128, 256, 3, SECT_4K) },
+
+	/* Catalyst / On Semiconductor -- non-JEDEC */
+	{ "cat25c11",  INFO(0x0, 0,   16, 8, 16, 1, M25P_NO_ERASE) },
+	{ "cat25c03",  INFO(0x0, 0,   32, 8, 16, 2, M25P_NO_ERASE) },
+	{ "cat25c09",  INFO(0x0, 0,  128, 8, 32, 2, M25P_NO_ERASE) },
+	{ "cat25c17",  INFO(0x0, 0,  256, 8, 32, 2, M25P_NO_ERASE) },
+	{ "cat25128",  INFO(0x0, 0, 2048, 8, 64, 2, M25P_NO_ERASE) },
 	{ },
 };
 MODULE_DEVICE_TABLE(spi, m25p_ids);
@@ -702,7 +719,12 @@ static int __devinit m25p_probe(struct spi_device *spi)
 		flash->mtd.erasesize = info->sector_size;
 	}
 
+	if (info->flags & M25P_NO_ERASE)
+		flash->mtd.flags |= MTD_NO_ERASE;
+
 	flash->mtd.dev.parent = &spi->dev;
+	flash->page_size = info->page_size;
+	flash->addr_width = info->addr_width;
 
 	dev_info(&spi->dev, "%s (%lld Kbytes)\n", id->name,
 			(long long)flash->mtd.size >> 10);
Signature · 1 line
1.6.3.3
#25 lore raw

Re: [PATCH 2/2] mtd: m25p80: Add support for CAT25xxx serial EEPROMs

Quoted 7 lines · On Wed, 19 Aug 2009 01:46:28 +0400 Anton Vorontsov <[email protected]>…

CAT25 chips (as manufactured by On Semiconductor, previously Catalyst Semiconductor) are similar to the original M25Px0 chips, except:

- Address width can vary (1-2 bytes, in contrast to 3 bytes in M25P chips). So, implement convenient m25p_addr2cmd() and m25p_cmdsz() calls, and place address width information into flash_info struct;

Quoted 6 lines

- Page size can vary, therefore we shouldn't hardcode it, so get rid of FLASH_PAGESIZE definition, and place the page size information into flash_info struct;

- CAT25 EEPROMs don't need to be erased, so add NO_ERASE flag, and propagate it to the mtd subsystem.

This patch (still) doesn't know about the mx25l3205d, mx25l12805d and mx25l12855e devices.

I randomly did this:

-> { "mx25l3205d", INFO(0xc22016, 0, 64 * 1024, 64, 256, 3, 0) },
-> { "mx25l6405d", INFO(0xc22017, 0, 64 * 1024, 128, 256, 3, 0) },

	{ "mx25l12805d", INFO(0xc22018, 0, 64 * 1024, 256, 256, 3, 0) },

-> { "mx25l12855e", INFO(0xc22618, 0, 64 * 1024, 256, 256, 3, 0) },

#26 lore raw

Re: [PATCH 2/2] mtd: m25p80: Add support for CAT25xxx serial EEPROMs

Quoted 14 lines · Andrew Morton
Quoted 11 lines · On Wed, 19 Aug 2009 01:46:28 +0400 Anton Vorontsov <[email protected]>…

CAT25 chips (as manufactured by On Semiconductor, previously Catalyst Semiconductor) are similar to the original M25Px0 chips, except:

- Address width can vary (1-2 bytes, in contrast to 3 bytes in M25P chips). So, implement convenient m25p_addr2cmd() and m25p_cmdsz() calls, and place address width information into flash_info struct;

- Page size can vary, therefore we shouldn't hardcode it, so get rid of FLASH_PAGESIZE definition, and place the page size information into flash_info struct;

Quoted 5 lines
Quoted 2 lines

- CAT25 EEPROMs don't need to be erased, so add NO_ERASE flag, and propagate it to the mtd subsystem.

This patch (still) doesn't know about the mx25l3205d, mx25l12805d and mx25l12855e devices.

Yes, support for these chips commited on Fri, 4 Sep 2009 08:40:27 +0000 (09:40 +0100). And the patch dated 19 Aug.

Quoted 6 lines

I randomly did this:

-> { "mx25l3205d", INFO(0xc22016, 0, 64 * 1024, 64, 256, 3, 0) },
-> { "mx25l6405d", INFO(0xc22017, 0, 64 * 1024, 128, 256, 3, 0) },

	{ "mx25l12805d", INFO(0xc22018, 0, 64 * 1024, 256, 256, 3, 0) },

-> { "mx25l12855e", INFO(0xc22618, 0, 64 * 1024, 256, 256, 3, 0) },

Looks correct, thanks a lot.

Signature · 3 lines
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
#27 lore raw

Re: [PATCH 2/6] mtd: m25p80: Convert to device table matching

In reply to David Brownell
Quoted 12 lines · On Mon, 3 Aug 2009 19:54:50 -0700 David Brownell <[email protected]> wrote:
Quoted 4 lines · Anton Vorontsov

This patch converts the m25p80 driver so that now it uses .id_table for device matching, making it properly detect devices on OpenFirmware platforms (prior to this patch the driver misdetected non-JEDEC chips, seeing all chips as "m25p80").

I suspect "detect" is a misnomer there. It only "detects" JEDEC chips. All others got explicit declarations ... so if there's misbehavior for other chips, it's because those declarations were poorly handled. Maybe they were not properly flagged as non-JDEC

Quoted 8 lines
Quoted 4 lines

Also, now jedec_probe() only does jedec probing, nothing else. If it is not able to detect a chip, NULL is returned and the driver fall backs to the information specified by the platform (platform_data, or exact ID).

I'd rather keep the warning, so there's a clue about what's really going on: JEDEC chip found, but its ID is not handled.

afaik there was no response to David's review comments, so this patch is in the "stuck" state.

Quoted 123 lines
Quoted 13 lines
---
 drivers/mtd/devices/m25p80.c |  146 +++++++++++++++++++++++-------------------
 1 files changed, 80 insertions(+), 66 deletions(-)
drivers/mtd/devices/m25p80.c +0 -0
index 10ed195..0d74b38 100644
			... deletia ...
drivers/mtd/devices/m25p80.c +0 -0
@@ -481,74 +480,83 @@ struct flash_info {
 #define	SECT_4K		0x01		/* OPCODE_BE_4K works uniformly */

drivers/mtd/devices/m25p80.c struct flash_info { · line 490

Quoted 10 lines
drivers/mtd/devices/m25p80.c +8 -0
 };
 
+#define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags)	\
+	((kernel_ulong_t)&(struct flash_info) {				\
+		.jedec_id = (_jedec_id),				\
+		.ext_id = (_ext_id),					\
+		.sector_size = (_sector_size),				\
+		.n_sectors = (_n_sectors),				\
+		.flags = (_flags),					\
+	})

Anonymous inlined structures ... kind of ugly, but I can understand why you might not want to declare and name a few dozen single-use structures.

Quoted 26 lines
drivers/mtd/devices/m25p80.c +4 -4
 /* NOTE: double check command sets and memory organization when you add
  * more flash chips.  This current list focusses on newer chips, which
  * have been converging on command sets which including JEDEC ID.
  */
-static struct flash_info __devinitdata m25p_data [] = {
-
+static const struct spi_device_id m25p_ids[] = {
 	/* Atmel -- some are (confusingly) marketed as "DataFlash" */
-	{ "at25fs010",  0x1f6601, 0, 32 * 1024, 4, SECT_4K, },
-	{ "at25fs040",  0x1f6604, 0, 64 * 1024, 8, SECT_4K, },
+	{ "at25fs010",  INFO(0x1f6601, 0, 32 * 1024, 4, SECT_4K) },
+	{ "at25fs040",  INFO(0x1f6604, 0, 64 * 1024, 8, SECT_4K) },
 
 		... deletia ...
 
 
@@ -596,6 +602,7 @@ static struct flash_info *__devinit jedec_probe(struct spi_device *spi)
  */
 static int __devinit m25p_probe(struct spi_device *spi)
 {
+	const struct spi_device_id	*id;
 	struct flash_platform_data	*data;
 	struct m25p			*flash;
 	struct flash_info		*info;

drivers/mtd/devices/m25p80.c static int __devinit m25p_probe(struct spi_device *spi) · line 617

Quoted 3 lines
drivers/mtd/devices/m25p80.c +0 -0
@@ -608,32 +615,38 @@ static int __devinit m25p_probe(struct spi_device *spi)
 	 */
 	data = spi->dev.platform_data;
 	if (data && data->type) {

At this point I wonder why you're not changing the probe sequence more. Get "id" and then "id" here. If it's for "m25p80" assume it's an old-style board init and do the current logic. Else just verify "info".

There's a new error case of course: new-style but data->type doesn't match id->name.

Quoted 6 lines
drivers/mtd/devices/m25p80.c +1 -5
-		for (i = 0, info = m25p_data;
-				i < ARRAY_SIZE(m25p_data);
-				i++, info++) {
-			if (strcmp(data->type, info->name) == 0)
-				break;
+		for (i = 0; i < ARRAY_SIZE(m25p_ids) - 1; i++) {

drivers/mtd/devices/m25p80.c static int __devinit m25p_probe(struct spi_device *spi)

Quoted 10 lines
drivers/mtd/devices/m25p80.c +6 -1
+			id = &m25p_ids[i];
+			info = (void *)m25p_ids[i].driver_data;
+			if (strcmp(data->type, id->name))
+				continue;
+			break;
 		}
 
 		/* unrecognized chip? */
-		if (i == ARRAY_SIZE(m25p_data)) {
+		if (i == ARRAY_SIZE(m25p_ids) - 1) {

Better: "if (info == NULL) ..." You've got all the pointers
in hand; don't use indices.

drivers/mtd/devices/m25p80.c static int __devinit m25p_probe(struct spi_device *spi) · line 636

Quoted 11 lines
drivers/mtd/devices/m25p80.c +2 -2
 			DEBUG(MTD_DEBUG_LEVEL0, "%s: unrecognized id %s\n",
 					dev_name(&spi->dev), data->type);
 			info = NULL;
 
 		/* recognized; is that chip really what's there? */
 		} else if (info->jedec_id) {
-			struct flash_info	*chip = jedec_probe(spi);
+			id = jedec_probe(spi);
 
-			if (!chip || chip != info) {
+			if (id != &m25p_ids[i]) {

Again, don't use indices except during the lookup.

Quoted 20 lines
drivers/mtd/devices/m25p80.c +9 -4
 				dev_warn(&spi->dev, "found %s, expected %s\n",
-						chip ? chip->name : "UNKNOWN",
-						info->name);
+						id ? id->name : "UNKNOWN",
+						m25p_ids[i].name);
 				info = NULL;
 			}
 		}
-	} else
-		info = jedec_probe(spi);
+	} else {
+		id = jedec_probe(spi);
+		if (!id)
+			id = spi_get_device_id(spi);
+
+		info = (void *)id->driver_data;
+	}
 
 	if (!info)
 		return -ENODEV;
#28 lore raw

Re: [PATCH 2/6] mtd: m25p80: Convert to device table matching

Quoted 20 lines · Andrew Morton
Quoted 17 lines · On Mon, 3 Aug 2009 19:54:50 -0700 David Brownell <[email protected]> wrote:
Quoted 4 lines · Anton Vorontsov

This patch converts the m25p80 driver so that now it uses .id_table for device matching, making it properly detect devices on OpenFirmware platforms (prior to this patch the driver misdetected non-JEDEC chips, seeing all chips as "m25p80").

I suspect "detect" is a misnomer there. It only "detects" JEDEC chips. All others got explicit declarations ... so if there's misbehavior for other chips, it's because those declarations were poorly handled. Maybe they were not properly flagged as non-JDEC

Quoted 4 lines

Also, now jedec_probe() only does jedec probing, nothing else. If it is not able to detect a chip, NULL is returned and the driver fall backs to the information specified by the platform (platform_data, or exact ID).

Quoted 6 lines
Quoted 3 lines

I'd rather keep the warning, so there's a clue about what's really going on: JEDEC chip found, but its ID is not handled.

afaik there was no response to David's review comments, so this patch is in the "stuck" state.

Hm? Response:

http://lkml.org/lkml/2009/8/18/363

And the two patches I sent on top:

http://lkml.org/lkml/2009/8/18/364
http://lkml.org/lkml/2009/8/18/366

Signature · 3 lines
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
#31 lore raw

Re: [PATCH 2/6] mtd: m25p80: Convert to device table matching

In reply to David Woodhouse
Quoted 9 lines · David Woodhouse
Quoted 5 lines · Anton Vorontsov

And the two patches I sent on top:

http://lkml.org/lkml/2009/8/18/364
http://lkml.org/lkml/2009/8/18/366

Got versions of those which apply to the mtd-2.6.git tree (which I'm about to ask Linus to pull)?

I'd love to, but they depend on a bunch of SPI patches that are still in -mm tree. As soon as SPI core changes hit Linus' tree, I think Andrew will send all m25p80 patches to you anyway.

Thanks,

Signature · 3 lines
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
#32 lore raw

Re: [PATCH 2/6] mtd: m25p80: Convert to device table matching

Quoted 5 lines · On Wed, 23 Sep 2009 03:55:34 +0400 Anton Vorontsov <[email protected]>…
Quoted 4 lines · David Woodhouse
Quoted 3 lines · Anton Vorontsov

And the two patches I sent on top:

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

http://lkml.org/lkml/2009/8/18/364
http://lkml.org/lkml/2009/8/18/366

Got versions of those which apply to the mtd-2.6.git tree (which I'm about to ask Linus to pull)?

I'd love to, but they depend on a bunch of SPI patches that are still in -mm tree.

oh, is that why I queued them up where I did. Sigh.

Quoted 2 lines

As soon as SPI core changes hit Linus' tree, I think Andrew will send all m25p80 patches to you anyway.

Or David can ack them and I'll send 'em up.

#33 lore raw

Re: [PATCH 2/6] mtd: m25p80: Convert to device table matching

In reply to Anton Vorontsov

Hello Anton,

Is m25p_probe now valid with dev.platform_data = NULL, for of platforms?

Then shouldn't you have the following change as well, near the end of the function?

- } else if (data->nr_parts)
+ } else if (data && data->nr_parts)

		dev_warn(&spi->dev, "ignoring %d default partitions on %s\n",
				data->nr_parts, data->name);

Or am I misunderstanding something?

Signature · 8 lines
Michael Barkowski
RuggedCom


_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
#34 lore raw

Re: [PATCH 2/6] mtd: m25p80: Convert to device table matching

Quoted 12 lines · Michael Barkowski

Hello Anton,

Is m25p_probe now valid with dev.platform_data = NULL, for of platforms?

Then shouldn't you have the following change as well, near the end of the function?

(quoted diff) +1 -1
-  	} else if (data->nr_parts)
+  	} else if (data && data->nr_parts)
		dev_warn(&spi->dev, "ignoring %d default partitions on %s\n",
				data->nr_parts, data->name);
Quoted 1 line

Or am I misunderstanding something?

Yeah, you missed this patch:
http://patchwork.kernel.org/patch/38179/

Thanks,

Signature · 8 lines
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
#35 lore raw

Re: [PATCH 2/6] mtd: m25p80: Convert to device table matching

Quoted 13 lines · Anton Vorontsov
Quoted 12 lines · Michael Barkowski

Hello Anton,

Is m25p_probe now valid with dev.platform_data = NULL, for of platforms?

Then shouldn't you have the following change as well, near the end of the function?

(quoted diff) +1 -1
-  	} else if (data->nr_parts)
+  	} else if (data && data->nr_parts)
		dev_warn(&spi->dev, "ignoring %d default partitions on %s\n",
				data->nr_parts, data->name);
Quoted 8 lines
Quoted 1 line

Or am I misunderstanding something?

Yeah, you missed this patch:
http://patchwork.kernel.org/patch/38179/

Thanks,

Beautiful - thanks - sorry to interrupt

Signature · 7 lines
Michael Barkowski
905-482-4577

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
#36 lore raw

PATCH v2 3/6 of: Remove "stm,m25p40" alias

In reply to Anton Vorontsov

The alias isn't needed any longer since the m25p80 driver converted to the module device table matching.

---
 drivers/of/base.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)
drivers/of/base.c +0 -1
index 69f85c0..ddf224d 100644
@@ -447,7 +447,6 @@ struct of_modalias_table {
 static struct of_modalias_table of_modalias_table[] = {
 	{ "fsl,mcu-mpc8349emitx", "mcu-mpc8349emitx" },
 	{ "mmc-spi-slot", "mmc_spi" },
-	{ "stm,m25p40", "m25p80" },
 };
 
 /**
Signature · 7 lines
1.6.3.3


_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
#37 lore raw

PATCH v2 4/6 hwmon: adxx: Convert to device table matching

In reply to Anton Vorontsov

This patch makes the code a little bit nicer, and shorter.

---
 drivers/hwmon/adcxx.c |  101 ++++++++-----------------------------------------
 1 files changed, 16 insertions(+), 85 deletions(-)
drivers/hwmon/adcxx.c +16 -85
index 242294d..5e9e095 100644
@@ -43,6 +43,7 @@
 #include <linux/hwmon.h>
 #include <linux/hwmon-sysfs.h>
 #include <linux/mutex.h>
+#include <linux/mod_devicetable.h>
 #include <linux/spi/spi.h>
 
 #define DRVNAME		"adcxx"
@@ -157,8 +158,9 @@ static struct sensor_device_attribute ad_input[] = {
 
 /*----------------------------------------------------------------------*/
 
-static int __devinit adcxx_probe(struct spi_device *spi, int channels)
+static int __devinit adcxx_probe(struct spi_device *spi)
 {
+	int channels = spi_get_device_id(spi)->driver_data;
 	struct adcxx *adc;
 	int status;
 	int i;
@@ -204,26 +206,6 @@ out_err:
 	return status;
 }
 
-static int __devinit adcxx1s_probe(struct spi_device *spi)
-{
-	return adcxx_probe(spi, 1);
-}
-
-static int __devinit adcxx2s_probe(struct spi_device *spi)
-{
-	return adcxx_probe(spi, 2);
-}
-
-static int __devinit adcxx4s_probe(struct spi_device *spi)
-{
-	return adcxx_probe(spi, 4);
-}
-
-static int __devinit adcxx8s_probe(struct spi_device *spi)
-{
-	return adcxx_probe(spi, 8);
-}
-
 static int __devexit adcxx_remove(struct spi_device *spi)
 {
 	struct adcxx *adc = dev_get_drvdata(&spi->dev);
@@ -241,79 +223,33 @@ static int __devexit adcxx_remove(struct spi_device *spi)
 	return 0;
 }
 
-static struct spi_driver adcxx1s_driver = {
-	.driver = {
-		.name	= "adcxx1s",
-		.owner	= THIS_MODULE,
-	},
-	.probe	= adcxx1s_probe,
-	.remove	= __devexit_p(adcxx_remove),
+static const struct spi_device_id adcxx_ids[] = {
+	{ "adcxx1s", 1 },
+	{ "adcxx2s", 2 },
+	{ "adcxx4s", 4 },
+	{ "adcxx8s", 8 },
+	{ },
 };
+MODULE_DEVICE_TABLE(spi, adcxx_ids);
 
-static struct spi_driver adcxx2s_driver = {
+static struct spi_driver adcxx_driver = {
 	.driver = {
-		.name	= "adcxx2s",
+		.name	= "adcxx",
 		.owner	= THIS_MODULE,
 	},
-	.probe	= adcxx2s_probe,
-	.remove	= __devexit_p(adcxx_remove),
-};
-
-static struct spi_driver adcxx4s_driver = {
-	.driver = {
-		.name	= "adcxx4s",
-		.owner	= THIS_MODULE,
-	},
-	.probe	= adcxx4s_probe,
-	.remove	= __devexit_p(adcxx_remove),
-};
-
-static struct spi_driver adcxx8s_driver = {
-	.driver = {
-		.name	= "adcxx8s",
-		.owner	= THIS_MODULE,
-	},
-	.probe	= adcxx8s_probe,
+	.id_table = adcxx_ids,
+	.probe	= adcxx_probe,
 	.remove	= __devexit_p(adcxx_remove),
 };
 
 static int __init init_adcxx(void)
 {
-	int status;
-	status = spi_register_driver(&adcxx1s_driver);
-	if (status)
-		goto reg_1_failed;
-
-	status = spi_register_driver(&adcxx2s_driver);
-	if (status)
-		goto reg_2_failed;
-
-	status = spi_register_driver(&adcxx4s_driver);
-	if (status)
-		goto reg_4_failed;
-
-	status = spi_register_driver(&adcxx8s_driver);
-	if (status)
-		goto reg_8_failed;
-
-	return status;
-
-reg_8_failed:
-	spi_unregister_driver(&adcxx4s_driver);
-reg_4_failed:
-	spi_unregister_driver(&adcxx2s_driver);
-reg_2_failed:
-	spi_unregister_driver(&adcxx1s_driver);
-reg_1_failed:
-	return status;
+	return spi_register_driver(&adcxx_driver);
 }
 
 static void __exit exit_adcxx(void)
 {
-	spi_unregister_driver(&adcxx1s_driver);
-	spi_unregister_driver(&adcxx2s_driver);
-	spi_unregister_driver(&adcxx4s_driver);
-	spi_unregister_driver(&adcxx8s_driver);
+	spi_unregister_driver(&adcxx_driver);
 }
 
 module_init(init_adcxx);
@@ -322,8 +258,3 @@ module_exit(exit_adcxx);
 MODULE_AUTHOR("Marc Pignat");
 MODULE_DESCRIPTION("National Semiconductor adcxx8sxxx Linux driver");
 MODULE_LICENSE("GPL");
-
-MODULE_ALIAS("adcxx1s");
-MODULE_ALIAS("adcxx2s");
-MODULE_ALIAS("adcxx4s");
-MODULE_ALIAS("adcxx8s");
Signature · 7 lines
1.6.3.3


_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
#38 lore raw

PATCH v2 5/6 hwmon: lm70: Convert to device table matching

In reply to Anton Vorontsov

This patch makes the code a little bit nicer, and shorter.

---
 drivers/hwmon/lm70.c |   55 +++++++++++++++++--------------------------------
 1 files changed, 19 insertions(+), 36 deletions(-)
drivers/hwmon/lm70.c +19 -36
index ae6204f..ab8a5d3 100644
@@ -32,6 +32,7 @@
 #include <linux/sysfs.h>
 #include <linux/hwmon.h>
 #include <linux/mutex.h>
+#include <linux/mod_devicetable.h>
 #include <linux/spi/spi.h>
 
 
@@ -130,11 +131,20 @@ static DEVICE_ATTR(name, S_IRUGO, lm70_show_name, NULL);
 
 /*----------------------------------------------------------------------*/
 
-static int __devinit common_probe(struct spi_device *spi, int chip)
+static int __devinit lm70_probe(struct spi_device *spi)
 {
+	int chip = spi_get_device_id(spi)->driver_data;
 	struct lm70 *p_lm70;
 	int status;
 
+	/* signaling is SPI_MODE_0 for both LM70 and TMP121 */
+	if (spi->mode & (SPI_CPOL | SPI_CPHA))
+		return -EINVAL;
+
+	/* 3-wire link (shared SI/SO) for LM70 */
+	if (chip = LM70_CHIP_LM70 && !(spi->mode & SPI_3WIRE))
+		return -EINVAL;
+
 	/* NOTE:  we assume 8-bit words, and convert to 16 bits manually */
 
 	p_lm70 = kzalloc(sizeof *p_lm70, GFP_KERNEL);
@@ -170,24 +180,6 @@ out_dev_reg_failed:
 	return status;
 }
 
-static int __devinit lm70_probe(struct spi_device *spi)
-{
-	/* signaling is SPI_MODE_0 on a 3-wire link (shared SI/SO) */
-	if ((spi->mode & (SPI_CPOL | SPI_CPHA)) || !(spi->mode & SPI_3WIRE))
-		return -EINVAL;
-
-	return common_probe(spi, LM70_CHIP_LM70);
-}
-
-static int __devinit tmp121_probe(struct spi_device *spi)
-{
-	/* signaling is SPI_MODE_0 with only MISO connected */
-	if (spi->mode & (SPI_CPOL | SPI_CPHA))
-		return -EINVAL;
-
-	return common_probe(spi, LM70_CHIP_TMP121);
-}
-
 static int __devexit lm70_remove(struct spi_device *spi)
 {
 	struct lm70 *p_lm70 = dev_get_drvdata(&spi->dev);
@@ -201,41 +193,32 @@ static int __devexit lm70_remove(struct spi_device *spi)
 	return 0;
 }
 
-static struct spi_driver tmp121_driver = {
-	.driver = {
-		.name	= "tmp121",
-		.owner	= THIS_MODULE,
-	},
-	.probe	= tmp121_probe,
-	.remove	= __devexit_p(lm70_remove),
+
+static const struct spi_device_id lm70_ids[] = {
+	{ "lm70",   LM70_CHIP_LM70 },
+	{ "tmp121", LM70_CHIP_TMP121 },
+	{ },
 };
+MODULE_DEVICE_TABLE(spi, lm70_ids);
 
 static struct spi_driver lm70_driver = {
 	.driver = {
 		.name	= "lm70",
 		.owner	= THIS_MODULE,
 	},
+	.id_table = lm70_ids,
 	.probe	= lm70_probe,
 	.remove	= __devexit_p(lm70_remove),
 };
 
 static int __init init_lm70(void)
 {
-	int ret = spi_register_driver(&lm70_driver);
-	if (ret)
-		return ret;
-
-	ret = spi_register_driver(&tmp121_driver);
-	if (ret)
-		spi_unregister_driver(&lm70_driver);
-
-	return ret;
+	return spi_register_driver(&lm70_driver);
 }
 
 static void __exit cleanup_lm70(void)
 {
 	spi_unregister_driver(&lm70_driver);
-	spi_unregister_driver(&tmp121_driver);
 }
 
 module_init(init_lm70);
Signature · 7 lines
1.6.3.3


_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
#39 lore raw

PATCH v2 6/6 spi: Prefix modalias with "spi:"

In reply to Anton Vorontsov

This makes it consistent with other buses (platform, i2c, vio, ...). I'm not sure why we use the prefixes, but there must be a reason.

This was easy enough to do it, and I did it.

---
 drivers/gpio/max7301.c                 |    1 +
 drivers/gpio/mcp23s08.c                |    1 +
 drivers/hwmon/lis3lv02d_spi.c          |    2 +-
 drivers/hwmon/max1111.c                |    1 +
 drivers/input/touchscreen/ad7877.c     |    1 +
 drivers/input/touchscreen/ad7879.c     |    1 +
 drivers/input/touchscreen/ads7846.c    |    1 +
 drivers/leds/leds-dac124s085.c         |    1 +
 drivers/mfd/ezx-pcap.c                 |    1 +
 drivers/misc/eeprom/at25.c             |    2 +-
 drivers/mmc/host/mmc_spi.c             |    1 +
 drivers/mtd/devices/mtd_dataflash.c    |    1 +
 drivers/net/enc28j60.c                 |    1 +
 drivers/net/ks8851.c                   |    1 +
 drivers/net/wireless/libertas/if_spi.c |    1 +
 drivers/net/wireless/p54/p54spi.c      |    1 +
 drivers/net/wireless/wl12xx/main.c     |    1 +
 drivers/rtc/rtc-ds1305.c               |    1 +
 drivers/rtc/rtc-ds1390.c               |    1 +
 drivers/rtc/rtc-ds3234.c               |    1 +
 drivers/rtc/rtc-m41t94.c               |    1 +
 drivers/rtc/rtc-max6902.c              |    1 +
 drivers/rtc/rtc-r9701.c                |    1 +
 drivers/rtc/rtc-rs5c348.c              |    1 +
 drivers/serial/max3100.c               |    1 +
 drivers/spi/spi.c                      |    3 ++-
 drivers/spi/spidev.c                   |    1 +
 drivers/spi/tle62x0.c                  |    1 +
 drivers/staging/stlc45xx/stlc45xx.c    |    1 +
 drivers/video/backlight/corgi_lcd.c    |    1 +
 drivers/video/backlight/ltv350qv.c     |    1 +
 drivers/video/backlight/tdo24m.c       |    1 +
 drivers/video/backlight/tosa_lcd.c     |    2 +-
 drivers/video/backlight/vgg2432a4.c    |    3 +--
 include/linux/mod_devicetable.h        |    1 +
 scripts/mod/file2alias.c               |    4 ++--
 36 files changed, 38 insertions(+), 8 deletions(-)
drivers/gpio/max7301.c +1 -0
index 7b82eaa..480956f 100644
@@ -339,3 +339,4 @@ module_exit(max7301_exit);
 MODULE_AUTHOR("Juergen Beisert");
 MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("MAX7301 SPI based GPIO-Expander");
+MODULE_ALIAS("spi:" DRIVER_NAME);
drivers/gpio/mcp23s08.c +1 -0
index f6fae0e..c6c7aa1 100644
@@ -433,3 +433,4 @@ static void __exit mcp23s08_exit(void)
 module_exit(mcp23s08_exit);
 
 MODULE_LICENSE("GPL");
+MODULE_ALIAS("spi:mcp23s08");
drivers/hwmon/lis3lv02d_spi.c +1 -1
index 3827ff0..b7aed07 100644
@@ -112,4 +112,4 @@ module_exit(lis302dl_exit);
 MODULE_AUTHOR("Daniel Mack <[email protected]>");
 MODULE_DESCRIPTION("lis3lv02d SPI glue layer");
 MODULE_LICENSE("GPL");
-
+MODULE_ALIAS("spi:" DRV_NAME);
drivers/hwmon/max1111.c +1 -0
index bfaa665..9ac4972 100644
@@ -242,3 +242,4 @@ module_exit(max1111_exit);
 MODULE_AUTHOR("Eric Miao <[email protected]>");
 MODULE_DESCRIPTION("MAX1111 ADC Driver");
 MODULE_LICENSE("GPL");
+MODULE_ALIAS("spi:max1111");
drivers/input/touchscreen/ad7877.c +1 -0
index ecaeb7e..eb83939 100644
@@ -842,3 +842,4 @@ module_exit(ad7877_exit);
 MODULE_AUTHOR("Michael Hennerich <[email protected]>");
 MODULE_DESCRIPTION("AD7877 touchscreen Driver");
 MODULE_LICENSE("GPL");
+MODULE_ALIAS("spi:ad7877");
drivers/input/touchscreen/ad7879.c +1 -0
index 5d8a703..19b4db7 100644
@@ -779,3 +779,4 @@ module_exit(ad7879_exit);
 MODULE_AUTHOR("Michael Hennerich <[email protected]>");
 MODULE_DESCRIPTION("AD7879(-1) touchscreen Driver");
 MODULE_LICENSE("GPL");
+MODULE_ALIAS("spi:ad7879");
drivers/input/touchscreen/ads7846.c +1 -0
index ba9d38c..09c8109 100644
@@ -1256,3 +1256,4 @@ module_exit(ads7846_exit);
 
 MODULE_DESCRIPTION("ADS7846 TouchScreen Driver");
 MODULE_LICENSE("GPL");
+MODULE_ALIAS("spi:ads7846");
drivers/leds/leds-dac124s085.c +1 -0
index 098d9aa..2913d76 100644
@@ -148,3 +148,4 @@ module_exit(dac124s085_leds_exit);
 MODULE_AUTHOR("Guennadi Liakhovetski <[email protected]>");
 MODULE_DESCRIPTION("DAC124S085 LED driver");
 MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("spi:dac124s085");
drivers/mfd/ezx-pcap.c +1 -0
index c1de4af..1672f30 100644
@@ -505,3 +505,4 @@ module_exit(ezx_pcap_exit);
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Daniel Ribeiro / Harald Welte");
 MODULE_DESCRIPTION("Motorola PCAP2 ASIC Driver");
+MODULE_ALIAS("spi:ezx-pcap");
drivers/misc/eeprom/at25.c +1 -1
index b34cb5f..d564de0 100644
@@ -417,4 +417,4 @@ module_exit(at25_exit);
 MODULE_DESCRIPTION("Driver for most SPI EEPROMs");
 MODULE_AUTHOR("David Brownell");
 MODULE_LICENSE("GPL");
-
+MODULE_ALIAS("spi:at25");
drivers/mmc/host/mmc_spi.c +1 -0
index a461017..d55fe4f 100644
@@ -1562,3 +1562,4 @@ MODULE_AUTHOR("Mike Lavender, David Brownell, "
 		"Hans-Peter Nilsson, Jan Nikitenko");
 MODULE_DESCRIPTION("SPI SD/MMC host driver");
 MODULE_LICENSE("GPL");
+MODULE_ALIAS("spi:mmc_spi");
drivers/mtd/devices/mtd_dataflash.c +1 -0
index 43976aa..211c27a 100644
@@ -966,3 +966,4 @@ module_exit(dataflash_exit);
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Andrew Victor, David Brownell");
 MODULE_DESCRIPTION("MTD DataFlash driver");
+MODULE_ALIAS("spi:mtd_dataflash");
drivers/net/enc28j60.c +1 -0
index fc6cc03..c709571 100644
@@ -1665,3 +1665,4 @@ MODULE_AUTHOR("Claudio Lanconelli <[email protected]>");
 MODULE_LICENSE("GPL");
 module_param_named(debug, debug.msg_enable, int, 0);
 MODULE_PARM_DESC(debug, "Debug verbosity level (0=none, ..., ffff=all)");
+MODULE_ALIAS("spi:" DRV_NAME);
drivers/net/ks8851.c +1 -0
index 9a1dea6..fe7cf4f 100644
@@ -1320,3 +1320,4 @@ MODULE_LICENSE("GPL");
 
 module_param_named(message, msg_enable, int, 0);
 MODULE_PARM_DESC(message, "Message verbosity level (0=none, 31=all)");
+MODULE_ALIAS("spi:ks8851");
drivers/net/wireless/libertas/if_spi.c +1 -0
index 6564282..ea45765 100644
@@ -1222,3 +1222,4 @@ MODULE_DESCRIPTION("Libertas SPI WLAN Driver");
 MODULE_AUTHOR("Andrey Yurovsky <[email protected]>, "
 	      "Colin McCabe <[email protected]>");
 MODULE_LICENSE("GPL");
+MODULE_ALIAS("spi:libertas_spi");
drivers/net/wireless/p54/p54spi.c +1 -0
index 72c7dbd..63bcdd1 100644
@@ -767,3 +767,4 @@ module_exit(p54spi_exit);
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Christian Lamparter <[email protected]>");
+MODULE_ALIAS("spi:cx3110x");
drivers/net/wireless/wl12xx/main.c +1 -0
index 603d611..6416406 100644
@@ -1356,3 +1356,4 @@ module_exit(wl12xx_exit);
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Kalle Valo <[email protected]>, "
 		"Luciano Coelho <[email protected]>");
+MODULE_ALIAS("spi:wl12xx");
drivers/rtc/rtc-ds1305.c +1 -0
index 8f410e5..2736b11 100644
@@ -841,3 +841,4 @@ module_exit(ds1305_exit);
 
 MODULE_DESCRIPTION("RTC driver for DS1305 and DS1306 chips");
 MODULE_LICENSE("GPL");
+MODULE_ALIAS("spi:rtc-ds1305");
drivers/rtc/rtc-ds1390.c +1 -0
index e01b955..cdb7050 100644
@@ -189,3 +189,4 @@ module_exit(ds1390_exit);
 MODULE_DESCRIPTION("Dallas/Maxim DS1390/93/94 SPI RTC driver");
 MODULE_AUTHOR("Mark Jackson <[email protected]>");
 MODULE_LICENSE("GPL");
+MODULE_ALIAS("spi:rtc-ds1390");
drivers/rtc/rtc-ds3234.c +1 -0
index c51589e..a774ca3 100644
@@ -188,3 +188,4 @@ module_exit(ds3234_exit);
 MODULE_DESCRIPTION("DS3234 SPI RTC driver");
 MODULE_AUTHOR("Dennis Aberilla <[email protected]>");
 MODULE_LICENSE("GPL");
+MODULE_ALIAS("spi:ds3234");
drivers/rtc/rtc-m41t94.c +1 -0
index c3a18c5..c8c97a4 100644
@@ -171,3 +171,4 @@ module_exit(m41t94_exit);
 MODULE_AUTHOR("Kim B. Heino <[email protected]>");
 MODULE_DESCRIPTION("Driver for ST M41T94 SPI RTC");
 MODULE_LICENSE("GPL");
+MODULE_ALIAS("spi:rtc-m41t94");
drivers/rtc/rtc-max6902.c +1 -0
index 36a8ea9..657403e 100644
@@ -175,3 +175,4 @@ module_exit(max6902_exit);
 MODULE_DESCRIPTION ("max6902 spi RTC driver");
 MODULE_AUTHOR ("Raphael Assenat");
 MODULE_LICENSE ("GPL");
+MODULE_ALIAS("spi:rtc-max6902");
drivers/rtc/rtc-r9701.c +1 -0
index 42028f2..9beba49 100644
@@ -174,3 +174,4 @@ module_exit(r9701_exit);
 MODULE_DESCRIPTION("r9701 spi RTC driver");
 MODULE_AUTHOR("Magnus Damm <[email protected]>");
 MODULE_LICENSE("GPL");
+MODULE_ALIAS("spi:rtc-r9701");
drivers/rtc/rtc-rs5c348.c +1 -0
index dd1e2bc..2099037 100644
@@ -251,3 +251,4 @@ MODULE_AUTHOR("Atsushi Nemoto <[email protected]>");
 MODULE_DESCRIPTION("Ricoh RS5C348 RTC driver");
 MODULE_LICENSE("GPL");
 MODULE_VERSION(DRV_VERSION);
+MODULE_ALIAS("spi:rtc-rs5c348");
drivers/serial/max3100.c +1 -0
index 9fd33e5..05d36e2 100644
@@ -925,3 +925,4 @@ module_exit(max3100_exit);
 MODULE_DESCRIPTION("MAX3100 driver");
 MODULE_AUTHOR("Christian Pellegrin <[email protected]>");
 MODULE_LICENSE("GPL");
+MODULE_ALIAS("spi:max3100");
drivers/spi/spi.c +2 -1
index 8518a6e..49e8486 100644
@@ -23,6 +23,7 @@
 #include <linux/init.h>
 #include <linux/cache.h>
 #include <linux/mutex.h>
+#include <linux/mod_devicetable.h>
 #include <linux/spi/spi.h>
 
 
@@ -93,7 +94,7 @@ 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", spi->modalias);
+	add_uevent_var(env, "MODALIAS=%s%s", SPI_MODULE_PREFIX, spi->modalias);
 	return 0;
 }
 
drivers/spi/spidev.c +1 -0
index 606e7a4..f921bd1 100644
@@ -688,3 +688,4 @@ module_exit(spidev_exit);
 MODULE_AUTHOR("Andrea Paterniani, <[email protected]>");
 MODULE_DESCRIPTION("User mode SPI device interface");
 MODULE_LICENSE("GPL");
+MODULE_ALIAS("spi:spidev");
drivers/spi/tle62x0.c +1 -0
index 455991f..bf9540f 100644
@@ -329,3 +329,4 @@ module_exit(tle62x0_exit);
 MODULE_AUTHOR("Ben Dooks <[email protected]>");
 MODULE_DESCRIPTION("TLE62x0 SPI driver");
 MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("spi:tle62x0");
drivers/staging/stlc45xx/stlc45xx.c +1 -0
index a137c78..38d0b24 100644
@@ -2590,3 +2590,4 @@ module_exit(stlc45xx_exit);
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Kalle Valo <[email protected]>");
+MODULE_ALIAS("spi:cx3110x");
drivers/video/backlight/corgi_lcd.c +1 -0
index f8a4bb2..2211a85 100644
@@ -639,3 +639,4 @@ module_exit(corgi_lcd_exit);
 MODULE_DESCRIPTION("LCD and backlight driver for SHARP C7x0/Cxx00");
 MODULE_AUTHOR("Eric Miao <[email protected]>");
 MODULE_LICENSE("GPL");
+MODULE_ALIAS("spi:corgi-lcd");
drivers/video/backlight/ltv350qv.c +1 -0
index 2eb206b..4631ca8 100644
@@ -328,3 +328,4 @@ module_exit(ltv350qv_exit);
 MODULE_AUTHOR("Haavard Skinnemoen <[email protected]>");
 MODULE_DESCRIPTION("Samsung LTV350QV LCD Driver");
 MODULE_LICENSE("GPL");
+MODULE_ALIAS("spi:ltv350qv");
drivers/video/backlight/tdo24m.c +1 -0
index 51422fc..bbfb502 100644
@@ -472,3 +472,4 @@ module_exit(tdo24m_exit);
 MODULE_AUTHOR("Eric Miao <[email protected]>");
 MODULE_DESCRIPTION("Driver for Toppoly TDO24M LCD Panel");
 MODULE_LICENSE("GPL");
+MODULE_ALIAS("spi:tdo24m");
drivers/video/backlight/tosa_lcd.c +1 -1
index b7fbc75..50ec17d 100644
@@ -300,4 +300,4 @@ module_exit(tosa_lcd_exit);
 MODULE_AUTHOR("Dmitry Baryshkov");
 MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("LCD/Backlight control for Sharp SL-6000 PDA");
-
+MODULE_ALIAS("spi:tosa-lcd");
drivers/video/backlight/vgg2432a4.c +1 -2
index 8e653b8..b49063c 100644
@@ -280,5 +280,4 @@ module_exit(vgg2432a4_exit);
 MODULE_AUTHOR("Ben Dooks <[email protected]>");
 MODULE_DESCRIPTION("VGG2432A4 LCD Driver");
 MODULE_LICENSE("GPL v2");
-
-
+MODULE_ALIAS("spi:VGG2432A4");
include/linux/mod_devicetable.h +1 -0
index b34f1ef..f58e9d8 100644
@@ -402,6 +402,7 @@ struct i2c_device_id {
 /* spi */
 
 #define SPI_NAME_SIZE	32
+#define SPI_MODULE_PREFIX "spi:"
 
 struct spi_device_id {
 	char name[SPI_NAME_SIZE];
scripts/mod/file2alias.c +2 -2
index 9d446e3..62a9025 100644
@@ -657,11 +657,11 @@ static int do_i2c_entry(const char *filename, struct i2c_device_id *id,
 	return 1;
 }
 
-/* Looks like: S */
+/* Looks like: spi:S */
 static int do_spi_entry(const char *filename, struct spi_device_id *id,
 			char *alias)
 {
-	sprintf(alias, "%s", id->name);
+	sprintf(alias, SPI_MODULE_PREFIX "%s", id->name);
 
 	return 1;
 }
Signature · 6 lines
1.6.3.3

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
#40 lore raw
In reply to Anton Vorontsov
Quoted 11 lines · Anton Vorontsov

Andrew,

This new patch set overwrites following patches:

hwmon-lm70-convert-to-device-table-matching.patch hwmon-adxx-convert-to-device-table-matching.patch spi-merge-probe-and-probe_id-callbacks.patch spi-prefix-modalias-with-spi.patch of-remove-stmm25p40-alias.patch mtd-m25p80-convert-to-device-table-matching.patch spi-add-support-for-device-table-matching.patch

Are you going to send v3 and address David's comments? Do you want some of these patches to go via the MTD tree or they better go as a series via some other tree?

Signature · 8 lines
Best Regards,
Artem Bityutskiy (Артём Битюцкий)


_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
#41 lore raw
Quoted 13 lines · Artem Bityutskiy
Quoted 11 lines · Anton Vorontsov

Andrew,

This new patch set overwrites following patches:

hwmon-lm70-convert-to-device-table-matching.patch hwmon-adxx-convert-to-device-table-matching.patch spi-merge-probe-and-probe_id-callbacks.patch spi-prefix-modalias-with-spi.patch of-remove-stmm25p40-alias.patch mtd-m25p80-convert-to-device-table-matching.patch spi-add-support-for-device-table-matching.patch

Quoted 1 line

Are you going to send v3 and address David's comments?

No v3, but I'm going to address David's comments in a follow up patch set where I'll change the probing code anyway.

Quoted 2 lines

Do you want some of these patches to go via the MTD tree or they better go as a series via some other tree?

Um.. The MTD patches depend on SPI subsystem changes... If David and Andrew are OK with SPI patches going through MTD tree, then I'm fine with it as well.

Thanks,

Signature · 8 lines
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
#42 lore raw
Quoted 19 lines · Anton Vorontsov
Quoted 13 lines · Artem Bityutskiy
Quoted 11 lines · Anton Vorontsov

Andrew,

This new patch set overwrites following patches:

hwmon-lm70-convert-to-device-table-matching.patch hwmon-adxx-convert-to-device-table-matching.patch spi-merge-probe-and-probe_id-callbacks.patch spi-prefix-modalias-with-spi.patch of-remove-stmm25p40-alias.patch mtd-m25p80-convert-to-device-table-matching.patch spi-add-support-for-device-table-matching.patch

Quoted 1 line

Are you going to send v3 and address David's comments?

No v3, but I'm going to address David's comments in a follow up patch set where I'll change the probing code anyway.

Quoted 6 lines
Quoted 2 lines

Do you want some of these patches to go via the MTD tree or they better go as a series via some other tree?

Um.. The MTD patches depend on SPI subsystem changes... If David and Andrew are OK with SPI patches going through MTD tree, then I'm fine with it as well.

If you are not sure, then I suggest to make these go through something else (not MTD tree).

Signature · 8 lines
Best Regards,
Artem Bityutskiy (Артём Битюцкий)


_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors