[PATCH v4 0/5] Devicetree support for misc/eeprom/eeprom_93xx46.

[PATCH v4 0/5] Devicetree support for misc/eeprom/eeprom_93xx46.

[Cory Tusar]

This series of patches adds an initial set of devicetree bindings to the
eeprom_93xx46 driver which mirror the configuration options previously
available as a platform device.  These bindings are then extended to
include support for specific Atmel devices in this family and also to
support GPIO-based selection of the device (e.g. for use with an SPI bus
mux).

Additionally, an address aliasing issue with 16-bit read and write
accesses in the eeprom_93xx46 driver discovered during testing is fixed.

Changes since v3:
  - Tested-by: Chris Healy <chris.healy@zii.aero>
    ...on a separate i.MX51 platform with Microchip 93LC46A EEPROM.

Changes since v2:
  - Changed node name to 'eeprom' in DT bindings example.
  - Simplified several bits of return logic.
  - Removed #ifdef CONFIG_OF.
  - Allow compiler to handle promotion to bool return values.
  - Reworked GPIO handling to use gpiod_* functions throughout (and
    fixed an oversight where GPIO flags were being ignored).

Changes since v1:
  - Consolidated all Documentation/devictree additions into a single patch.
  - Clarified compatible string shall be only one of the supported values.
  - Renamed the 'select-gpio' binding to 'select-gpios'.

Cory Tusar (5):
  misc: eeprom_93xx46: Fix 16-bit read and write accesses.
  Documentation: devicetree: Add DT bindings to eeprom_93xx46 driver.
  misc: eeprom_93xx46: Implement eeprom_93xx46 DT bindings.
  misc: eeprom_93xx46: Add quirks to support Atmel AT93C46D device.
  misc: eeprom_93xx46: Add support for a GPIO 'select' line.

 .../devicetree/bindings/misc/eeprom-93xx46.txt     |  25 +++
 drivers/misc/eeprom/eeprom_93xx46.c                | 212 +++++++++++++++++----
 include/linux/eeprom_93xx46.h                      |   9 +
 3 files changed, 210 insertions(+), 36 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/misc/eeprom-93xx46.txt

-- 
2.4.10

[PATCH v4 1/5] misc: eeprom_93xx46: Fix 16-bit read and write accesses.

[Cory Tusar]

Compatible at93xx46 devices from both Microchip and Atmel expect a
word-based address, regardless of whether the device is strapped for 8-
or 16-bit operation.  However, the offset parameter passed in when
reading or writing at a specific location is always specified in terms
of bytes.

This commit fixes 16-bit read and write accesses by shifting the offset
parameter to account for this difference between a byte offset and a
word-based address.

Signed-off-by: Cory Tusar <cory.tusar@pid1solutions.com>
Tested-by: Chris Healy <chris.healy@zii.aero>
---
 drivers/misc/eeprom/eeprom_93xx46.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/eeprom/eeprom_93xx46.c b/drivers/misc/eeprom/eeprom_93xx46.c
index ff63f05..e1bf0a5 100644
--- a/drivers/misc/eeprom/eeprom_93xx46.c
+++ b/drivers/misc/eeprom/eeprom_93xx46.c
@@ -54,7 +54,7 @@ eeprom_93xx46_bin_read(struct file *filp, struct kobject *kobj,
 		cmd_addr |= off & 0x7f;
 		bits = 10;
 	} else {
-		cmd_addr |= off & 0x3f;
+		cmd_addr |= (off >> 1) & 0x3f;
 		bits = 9;
 	}
 
@@ -155,7 +155,7 @@ eeprom_93xx46_write_word(struct eeprom_93xx46_dev *edev,
 		bits = 10;
 		data_len = 1;
 	} else {
-		cmd_addr |= off & 0x3f;
+		cmd_addr |= (off >> 1) & 0x3f;
 		bits = 9;
 		data_len = 2;
 	}
-- 
2.4.10

[PATCH v4 1/5] misc: eeprom_93xx46: Fix 16-bit read and write accesses.

[Cory Tusar]

Compatible at93xx46 devices from both Microchip and Atmel expect a
word-based address, regardless of whether the device is strapped for 8-
or 16-bit operation.  However, the offset parameter passed in when
reading or writing at a specific location is always specified in terms
of bytes.

This commit fixes 16-bit read and write accesses by shifting the offset
parameter to account for this difference between a byte offset and a
word-based address.

Signed-off-by: Cory Tusar <cory.tusar-J6Z/VSE8EyIAspv4Qr0y0gC/G2K4zDHf@public.gmane.org>
Tested-by: Chris Healy <chris.healy-c8ZVq/bFV1I@public.gmane.org>
---
 drivers/misc/eeprom/eeprom_93xx46.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/eeprom/eeprom_93xx46.c b/drivers/misc/eeprom/eeprom_93xx46.c
index ff63f05..e1bf0a5 100644
--- a/drivers/misc/eeprom/eeprom_93xx46.c
+++ b/drivers/misc/eeprom/eeprom_93xx46.c
@@ -54,7 +54,7 @@ eeprom_93xx46_bin_read(struct file *filp, struct kobject *kobj,
 		cmd_addr |= off & 0x7f;
 		bits = 10;
 	} else {
-		cmd_addr |= off & 0x3f;
+		cmd_addr |= (off >> 1) & 0x3f;
 		bits = 9;
 	}
 
@@ -155,7 +155,7 @@ eeprom_93xx46_write_word(struct eeprom_93xx46_dev *edev,
 		bits = 10;
 		data_len = 1;
 	} else {
-		cmd_addr |= off & 0x3f;
+		cmd_addr |= (off >> 1) & 0x3f;
 		bits = 9;
 		data_len = 2;
 	}
-- 
2.4.10

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 2/5] Documentation: devicetree: Add DT bindings to eeprom_93xx46 driver.

[Cory Tusar]

This commit documents bindings to be added to the eeprom_93xx46 driver
which will allow:

  - Device word size and read-only attributes to be specified.
  - A device-specific compatible string for use with Atmel AT93C46D
    EEPROMs.
  - Specifying a GPIO line to function as a 'select' or 'enable' signal
    prior to accessing the EEPROM.

Signed-off-by: Cory Tusar <cory.tusar@pid1solutions.com>
Acked-by: Rob Herring <robh@kernel.org>
Tested-by: Chris Healy <chris.healy@zii.aero>
---
 .../devicetree/bindings/misc/eeprom-93xx46.txt     | 25 ++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/Documentation/devicetree/bindings/misc/eeprom-93xx46.txt b/Documentation/devicetree/bindings/misc/eeprom-93xx46.txt
new file mode 100644
index 0000000..a8ebb46
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/eeprom-93xx46.txt
@@ -0,0 +1,25 @@
+EEPROMs (SPI) compatible with Microchip Technology 93xx46 family.
+
+Required properties:
+- compatible : shall be one of:
+    "atmel,at93c46d"
+    "eeprom-93xx46"
+- data-size : number of data bits per word (either 8 or 16)
+
+Optional properties:
+- read-only : parameter-less property which disables writes to the EEPROM
+- select-gpios : if present, specifies the GPIO that will be asserted prior to
+  each access to the EEPROM (e.g. for SPI bus multiplexing)
+
+Property rules described in Documentation/devicetree/bindings/spi/spi-bus.txt
+apply.  In particular, "reg" and "spi-max-frequency" properties must be given.
+
+Example:
+	eeprom@0 {
+		compatible = "eeprom-93xx46";
+		reg = <0>;
+		spi-max-frequency = <1000000>;
+		spi-cs-high;
+		data-size = <8>;
+		select-gpios = <&gpio4 4 GPIO_ACTIVE_HIGH>;
+	};
-- 
2.4.10

[PATCH v4 3/5] misc: eeprom_93xx46: Implement eeprom_93xx46 DT bindings.

[Cory Tusar]

This commit implements bindings in the eeprom_93xx46 driver allowing
device word size and read-only attributes to be specified via
devicetree.

Signed-off-by: Cory Tusar <cory.tusar@pid1solutions.com>
Tested-by: Chris Healy <chris.healy@zii.aero>
---
 drivers/misc/eeprom/eeprom_93xx46.c | 49 +++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/drivers/misc/eeprom/eeprom_93xx46.c b/drivers/misc/eeprom/eeprom_93xx46.c
index e1bf0a5..cc27e11 100644
--- a/drivers/misc/eeprom/eeprom_93xx46.c
+++ b/drivers/misc/eeprom/eeprom_93xx46.c
@@ -13,6 +13,8 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/slab.h>
 #include <linux/spi/spi.h>
 #include <linux/sysfs.h>
@@ -294,12 +296,58 @@ static ssize_t eeprom_93xx46_store_erase(struct device *dev,
 }
 static DEVICE_ATTR(erase, S_IWUSR, NULL, eeprom_93xx46_store_erase);
 
+static const struct of_device_id eeprom_93xx46_of_table[] = {
+	{ .compatible = "eeprom-93xx46", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, eeprom_93xx46_of_table);
+
+static int eeprom_93xx46_probe_dt(struct spi_device *spi)
+{
+	struct device_node *np = spi->dev.of_node;
+	struct eeprom_93xx46_platform_data *pd;
+	u32 tmp;
+	int ret;
+
+	pd = devm_kzalloc(&spi->dev, sizeof(*pd), GFP_KERNEL);
+	if (!pd)
+		return -ENOMEM;
+
+	ret = of_property_read_u32(np, "data-size", &tmp);
+	if (ret < 0) {
+		dev_err(&spi->dev, "data-size property not found\n");
+		return ret;
+	}
+
+	if (tmp == 8) {
+		pd->flags |= EE_ADDR8;
+	} else if (tmp == 16) {
+		pd->flags |= EE_ADDR16;
+	} else {
+		dev_err(&spi->dev, "invalid data-size (%d)\n", tmp);
+		return -EINVAL;
+	}
+
+	if (of_property_read_bool(np, "read-only"))
+		pd->flags |= EE_READONLY;
+
+	spi->dev.platform_data = pd;
+
+	return 0;
+}
+
 static int eeprom_93xx46_probe(struct spi_device *spi)
 {
 	struct eeprom_93xx46_platform_data *pd;
 	struct eeprom_93xx46_dev *edev;
 	int err;
 
+	if (spi->dev.of_node) {
+		err = eeprom_93xx46_probe_dt(spi);
+		if (err < 0)
+			return err;
+	}
+
 	pd = spi->dev.platform_data;
 	if (!pd) {
 		dev_err(&spi->dev, "missing platform data\n");
@@ -370,6 +418,7 @@ static int eeprom_93xx46_remove(struct spi_device *spi)
 static struct spi_driver eeprom_93xx46_driver = {
 	.driver = {
 		.name	= "93xx46",
+		.of_match_table = of_match_ptr(eeprom_93xx46_of_table),
 	},
 	.probe		= eeprom_93xx46_probe,
 	.remove		= eeprom_93xx46_remove,
-- 
2.4.10

[PATCH v4 4/5] misc: eeprom_93xx46: Add quirks to support Atmel AT93C46D device.

[Cory Tusar]

Atmel devices in this family have some quirks not found in other similar
chips - they do not support a sequential read of the entire EEPROM
contents, and the control word sent at the start of each operation
varies in bit length.

This commit adds quirk support to the driver and modifies the read
implementation to support non-sequential reads for consistency with
other misc/eeprom drivers.

Tested on a custom Freescale VF610-based platform, with an AT93C46D
device attached via dspi2.  The spi-gpio driver was used to allow the
necessary non-byte-sized transfers.

Signed-off-by: Cory Tusar <cory.tusar@pid1solutions.com>
Tested-by: Chris Healy <chris.healy@zii.aero>
---
 drivers/misc/eeprom/eeprom_93xx46.c | 126 ++++++++++++++++++++++++++----------
 include/linux/eeprom_93xx46.h       |   6 ++
 2 files changed, 97 insertions(+), 35 deletions(-)

diff --git a/drivers/misc/eeprom/eeprom_93xx46.c b/drivers/misc/eeprom/eeprom_93xx46.c
index cc27e11..d50bc17 100644
--- a/drivers/misc/eeprom/eeprom_93xx46.c
+++ b/drivers/misc/eeprom/eeprom_93xx46.c
@@ -27,6 +27,15 @@
 #define ADDR_ERAL	0x20
 #define ADDR_EWEN	0x30
 
+struct eeprom_93xx46_devtype_data {
+	unsigned int quirks;
+};
+
+static const struct eeprom_93xx46_devtype_data atmel_at93c46d_data = {
+	.quirks = EEPROM_93XX46_QUIRK_SINGLE_WORD_READ |
+		  EEPROM_93XX46_QUIRK_INSTRUCTION_LENGTH,
+};
+
 struct eeprom_93xx46_dev {
 	struct spi_device *spi;
 	struct eeprom_93xx46_platform_data *pdata;
@@ -35,6 +44,16 @@ struct eeprom_93xx46_dev {
 	int addrlen;
 };
 
+static inline bool has_quirk_single_word_read(struct eeprom_93xx46_dev *edev)
+{
+	return edev->pdata->quirks & EEPROM_93XX46_QUIRK_SINGLE_WORD_READ;
+}
+
+static inline bool has_quirk_instruction_length(struct eeprom_93xx46_dev *edev)
+{
+	return edev->pdata->quirks & EEPROM_93XX46_QUIRK_INSTRUCTION_LENGTH;
+}
+
 static ssize_t
 eeprom_93xx46_bin_read(struct file *filp, struct kobject *kobj,
 		       struct bin_attribute *bin_attr,
@@ -42,58 +61,73 @@ eeprom_93xx46_bin_read(struct file *filp, struct kobject *kobj,
 {
 	struct eeprom_93xx46_dev *edev;
 	struct device *dev;
-	struct spi_message m;
-	struct spi_transfer t[2];
-	int bits, ret;
-	u16 cmd_addr;
+	ssize_t ret = 0;
 
 	dev = container_of(kobj, struct device, kobj);
 	edev = dev_get_drvdata(dev);
 
-	cmd_addr = OP_READ << edev->addrlen;
+	mutex_lock(&edev->lock);
 
-	if (edev->addrlen == 7) {
-		cmd_addr |= off & 0x7f;
-		bits = 10;
-	} else {
-		cmd_addr |= (off >> 1) & 0x3f;
-		bits = 9;
-	}
+	if (edev->pdata->prepare)
+		edev->pdata->prepare(edev);
 
-	dev_dbg(&edev->spi->dev, "read cmd 0x%x, %d Hz\n",
-		cmd_addr, edev->spi->max_speed_hz);
+	while (count) {
+		struct spi_message m;
+		struct spi_transfer t[2] = { { 0 } };
+		u16 cmd_addr = OP_READ << edev->addrlen;
+		size_t nbytes = count;
+		int bits;
+		int err;
+
+		if (edev->addrlen == 7) {
+			cmd_addr |= off & 0x7f;
+			bits = 10;
+			if (has_quirk_single_word_read(edev))
+				nbytes = 1;
+		} else {
+			cmd_addr |= (off >> 1) & 0x3f;
+			bits = 9;
+			if (has_quirk_single_word_read(edev))
+				nbytes = 2;
+		}
 
-	spi_message_init(&m);
-	memset(t, 0, sizeof(t));
+		dev_dbg(&edev->spi->dev, "read cmd 0x%x, %d Hz\n",
+			cmd_addr, edev->spi->max_speed_hz);
 
-	t[0].tx_buf = (char *)&cmd_addr;
-	t[0].len = 2;
-	t[0].bits_per_word = bits;
-	spi_message_add_tail(&t[0], &m);
+		spi_message_init(&m);
 
-	t[1].rx_buf = buf;
-	t[1].len = count;
-	t[1].bits_per_word = 8;
-	spi_message_add_tail(&t[1], &m);
+		t[0].tx_buf = (char *)&cmd_addr;
+		t[0].len = 2;
+		t[0].bits_per_word = bits;
+		spi_message_add_tail(&t[0], &m);
 
-	mutex_lock(&edev->lock);
+		t[1].rx_buf = buf;
+		t[1].len = count;
+		t[1].bits_per_word = 8;
+		spi_message_add_tail(&t[1], &m);
 
-	if (edev->pdata->prepare)
-		edev->pdata->prepare(edev);
+		err = spi_sync(edev->spi, &m);
+		/* have to wait at least Tcsl ns */
+		ndelay(250);
 
-	ret = spi_sync(edev->spi, &m);
-	/* have to wait at least Tcsl ns */
-	ndelay(250);
-	if (ret) {
-		dev_err(&edev->spi->dev, "read %zu bytes at %d: err. %d\n",
-			count, (int)off, ret);
+		if (err) {
+			dev_err(&edev->spi->dev, "read %zu bytes at %d: err. %d\n",
+				nbytes, (int)off, err);
+			ret = err;
+			break;
+		}
+
+		buf += nbytes;
+		off += nbytes;
+		count -= nbytes;
+		ret += nbytes;
 	}
 
 	if (edev->pdata->finish)
 		edev->pdata->finish(edev);
 
 	mutex_unlock(&edev->lock);
-	return ret ? : count;
+	return ret;
 }
 
 static int eeprom_93xx46_ew(struct eeprom_93xx46_dev *edev, int is_on)
@@ -112,7 +146,13 @@ static int eeprom_93xx46_ew(struct eeprom_93xx46_dev *edev, int is_on)
 		bits = 9;
 	}
 
-	dev_dbg(&edev->spi->dev, "ew cmd 0x%04x\n", cmd_addr);
+	if (has_quirk_instruction_length(edev)) {
+		cmd_addr <<= 2;
+		bits += 2;
+	}
+
+	dev_dbg(&edev->spi->dev, "ew%s cmd 0x%04x, %d bits\n",
+			is_on ? "en" : "ds", cmd_addr, bits);
 
 	spi_message_init(&m);
 	memset(&t, 0, sizeof(t));
@@ -247,6 +287,13 @@ static int eeprom_93xx46_eral(struct eeprom_93xx46_dev *edev)
 		bits = 9;
 	}
 
+	if (has_quirk_instruction_length(edev)) {
+		cmd_addr <<= 2;
+		bits += 2;
+	}
+
+	dev_dbg(&edev->spi->dev, "eral cmd 0x%04x, %d bits\n", cmd_addr, bits);
+
 	spi_message_init(&m);
 	memset(&t, 0, sizeof(t));
 
@@ -298,12 +345,15 @@ static DEVICE_ATTR(erase, S_IWUSR, NULL, eeprom_93xx46_store_erase);
 
 static const struct of_device_id eeprom_93xx46_of_table[] = {
 	{ .compatible = "eeprom-93xx46", },
+	{ .compatible = "atmel,at93c46d", .data = &atmel_at93c46d_data, },
 	{}
 };
 MODULE_DEVICE_TABLE(of, eeprom_93xx46_of_table);
 
 static int eeprom_93xx46_probe_dt(struct spi_device *spi)
 {
+	const struct of_device_id *of_id =
+		of_match_device(eeprom_93xx46_of_table, &spi->dev);
 	struct device_node *np = spi->dev.of_node;
 	struct eeprom_93xx46_platform_data *pd;
 	u32 tmp;
@@ -331,6 +381,12 @@ static int eeprom_93xx46_probe_dt(struct spi_device *spi)
 	if (of_property_read_bool(np, "read-only"))
 		pd->flags |= EE_READONLY;
 
+	if (of_id->data) {
+		const struct eeprom_93xx46_devtype_data *data = of_id->data;
+
+		pd->quirks = data->quirks;
+	}
+
 	spi->dev.platform_data = pd;
 
 	return 0;
diff --git a/include/linux/eeprom_93xx46.h b/include/linux/eeprom_93xx46.h
index 0679181..92fa4c3 100644
--- a/include/linux/eeprom_93xx46.h
+++ b/include/linux/eeprom_93xx46.h
@@ -9,6 +9,12 @@ struct eeprom_93xx46_platform_data {
 #define EE_ADDR16	0x02		/* 16 bit addr. cfg */
 #define EE_READONLY	0x08		/* forbid writing */
 
+	unsigned int	quirks;
+/* Single word read transfers only; no sequential read. */
+#define EEPROM_93XX46_QUIRK_SINGLE_WORD_READ		(1 << 0)
+/* Instructions such as EWEN are (addrlen + 2) in length. */
+#define EEPROM_93XX46_QUIRK_INSTRUCTION_LENGTH		(1 << 1)
+
 	/*
 	 * optional hooks to control additional logic
 	 * before and after spi transfer.
-- 
2.4.10

[PATCH v4 5/5] misc: eeprom_93xx46: Add support for a GPIO 'select' line.

[Cory Tusar]

This commit adds support to the eeprom_93x46 driver allowing a GPIO line
to function as a 'select' or 'enable' signal prior to accessing the
EEPROM.

Signed-off-by: Cory Tusar <cory.tusar@pid1solutions.com>
Tested-by: Chris Healy <chris.healy@zii.aero>
---
 drivers/misc/eeprom/eeprom_93xx46.c | 35 +++++++++++++++++++++++++++++++++++
 include/linux/eeprom_93xx46.h       |  3 +++
 2 files changed, 38 insertions(+)

diff --git a/drivers/misc/eeprom/eeprom_93xx46.c b/drivers/misc/eeprom/eeprom_93xx46.c
index d50bc17..d28fac2 100644
--- a/drivers/misc/eeprom/eeprom_93xx46.c
+++ b/drivers/misc/eeprom/eeprom_93xx46.c
@@ -10,11 +10,13 @@
 
 #include <linux/delay.h>
 #include <linux/device.h>
+#include <linux/gpio/consumer.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/of_gpio.h>
 #include <linux/slab.h>
 #include <linux/spi/spi.h>
 #include <linux/sysfs.h>
@@ -343,6 +345,20 @@ static ssize_t eeprom_93xx46_store_erase(struct device *dev,
 }
 static DEVICE_ATTR(erase, S_IWUSR, NULL, eeprom_93xx46_store_erase);
 
+static void select_assert(void *context)
+{
+	struct eeprom_93xx46_dev *edev = context;
+
+	gpiod_set_value_cansleep(edev->pdata->select, 1);
+}
+
+static void select_deassert(void *context)
+{
+	struct eeprom_93xx46_dev *edev = context;
+
+	gpiod_set_value_cansleep(edev->pdata->select, 0);
+}
+
 static const struct of_device_id eeprom_93xx46_of_table[] = {
 	{ .compatible = "eeprom-93xx46", },
 	{ .compatible = "atmel,at93c46d", .data = &atmel_at93c46d_data, },
@@ -357,6 +373,8 @@ static int eeprom_93xx46_probe_dt(struct spi_device *spi)
 	struct device_node *np = spi->dev.of_node;
 	struct eeprom_93xx46_platform_data *pd;
 	u32 tmp;
+	int gpio;
+	enum of_gpio_flags of_flags;
 	int ret;
 
 	pd = devm_kzalloc(&spi->dev, sizeof(*pd), GFP_KERNEL);
@@ -381,6 +399,23 @@ static int eeprom_93xx46_probe_dt(struct spi_device *spi)
 	if (of_property_read_bool(np, "read-only"))
 		pd->flags |= EE_READONLY;
 
+	gpio = of_get_named_gpio_flags(np, "select-gpios", 0, &of_flags);
+	if (gpio_is_valid(gpio)) {
+		unsigned long flags =
+			of_flags == OF_GPIO_ACTIVE_LOW ? GPIOF_ACTIVE_LOW : 0;
+
+		ret = devm_gpio_request_one(&spi->dev, gpio, flags,
+					    "eeprom_93xx46_select");
+		if (ret)
+			return ret;
+
+		pd->select = gpio_to_desc(gpio);
+		pd->prepare = select_assert;
+		pd->finish = select_deassert;
+
+		gpiod_direction_output(pd->select, 0);
+	}
+
 	if (of_id->data) {
 		const struct eeprom_93xx46_devtype_data *data = of_id->data;
 
diff --git a/include/linux/eeprom_93xx46.h b/include/linux/eeprom_93xx46.h
index 92fa4c3..03f3435 100644
--- a/include/linux/eeprom_93xx46.h
+++ b/include/linux/eeprom_93xx46.h
@@ -3,6 +3,8 @@
  * platform description for 93xx46 EEPROMs.
  */
 
+#include <linux/of_gpio.h>
+
 struct eeprom_93xx46_platform_data {
 	unsigned char	flags;
 #define EE_ADDR8	0x01		/*  8 bit addr. cfg */
@@ -21,4 +23,5 @@ struct eeprom_93xx46_platform_data {
 	 */
 	void (*prepare)(void *);
 	void (*finish)(void *);
+	struct gpio_desc *select;
 };
-- 
2.4.10

Re: [PATCH v4 2/5] Documentation: devicetree: Add DT bindings to eeprom_93xx46 driver.

[Vladimir Zapolskiy]

Hi Cory,

On 10.12.2015 06:00, Cory Tusar wrote:
> This commit documents bindings to be added to the eeprom_93xx46 driver
> which will allow:
> 
>   - Device word size and read-only attributes to be specified.
>   - A device-specific compatible string for use with Atmel AT93C46D
>     EEPROMs.
>   - Specifying a GPIO line to function as a 'select' or 'enable' signal
>     prior to accessing the EEPROM.
> 
> Signed-off-by: Cory Tusar <cory.tusar@pid1solutions.com>
> Acked-by: Rob Herring <robh@kernel.org>
> Tested-by: Chris Healy <chris.healy@zii.aero>
> ---
>  .../devicetree/bindings/misc/eeprom-93xx46.txt     | 25 ++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/misc/eeprom-93xx46.txt b/Documentation/devicetree/bindings/misc/eeprom-93xx46.txt
> new file mode 100644
> index 0000000..a8ebb46
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/eeprom-93xx46.txt
> @@ -0,0 +1,25 @@
> +EEPROMs (SPI) compatible with Microchip Technology 93xx46 family.
> +
> +Required properties:
> +- compatible : shall be one of:
> +    "atmel,at93c46d"
> +    "eeprom-93xx46"

the second compatible property value is not recommended by ePAPR, I would
suggest to remove it from here and from the example below.

But I see that Rob acked this change though...

> +- data-size : number of data bits per word (either 8 or 16)
> +
> +Optional properties:
> +- read-only : parameter-less property which disables writes to the EEPROM
> +- select-gpios : if present, specifies the GPIO that will be asserted prior to
> +  each access to the EEPROM (e.g. for SPI bus multiplexing)
> +
> +Property rules described in Documentation/devicetree/bindings/spi/spi-bus.txt
> +apply.  In particular, "reg" and "spi-max-frequency" properties must be given.
> +
> +Example:
> +	eeprom@0 {
> +		compatible = "eeprom-93xx46";
> +		reg = <0>;
> +		spi-max-frequency = <1000000>;
> +		spi-cs-high;
> +		data-size = <8>;
> +		select-gpios = <&gpio4 4 GPIO_ACTIVE_HIGH>;
> +	};
> 

--
With best wishes,
Vladimir

Re: [PATCH v4 2/5] Documentation: devicetree: Add DT bindings to eeprom_93xx46 driver.

[Vladimir Zapolskiy]

Hi Cory,

On 10.12.2015 06:00, Cory Tusar wrote:
> This commit documents bindings to be added to the eeprom_93xx46 driver
> which will allow:
> 
>   - Device word size and read-only attributes to be specified.
>   - A device-specific compatible string for use with Atmel AT93C46D
>     EEPROMs.
>   - Specifying a GPIO line to function as a 'select' or 'enable' signal
>     prior to accessing the EEPROM.
> 
> Signed-off-by: Cory Tusar <cory.tusar-J6Z/VSE8EyIAspv4Qr0y0gC/G2K4zDHf@public.gmane.org>
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Tested-by: Chris Healy <chris.healy-c8ZVq/bFV1I@public.gmane.org>
> ---
>  .../devicetree/bindings/misc/eeprom-93xx46.txt     | 25 ++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/misc/eeprom-93xx46.txt b/Documentation/devicetree/bindings/misc/eeprom-93xx46.txt
> new file mode 100644
> index 0000000..a8ebb46
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/eeprom-93xx46.txt
> @@ -0,0 +1,25 @@
> +EEPROMs (SPI) compatible with Microchip Technology 93xx46 family.
> +
> +Required properties:
> +- compatible : shall be one of:
> +    "atmel,at93c46d"
> +    "eeprom-93xx46"

the second compatible property value is not recommended by ePAPR, I would
suggest to remove it from here and from the example below.

But I see that Rob acked this change though...

> +- data-size : number of data bits per word (either 8 or 16)
> +
> +Optional properties:
> +- read-only : parameter-less property which disables writes to the EEPROM
> +- select-gpios : if present, specifies the GPIO that will be asserted prior to
> +  each access to the EEPROM (e.g. for SPI bus multiplexing)
> +
> +Property rules described in Documentation/devicetree/bindings/spi/spi-bus.txt
> +apply.  In particular, "reg" and "spi-max-frequency" properties must be given.
> +
> +Example:
> +	eeprom@0 {
> +		compatible = "eeprom-93xx46";
> +		reg = <0>;
> +		spi-max-frequency = <1000000>;
> +		spi-cs-high;
> +		data-size = <8>;
> +		select-gpios = <&gpio4 4 GPIO_ACTIVE_HIGH>;
> +	};
> 

--
With best wishes,
Vladimir
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/5] misc: eeprom_93xx46: Implement eeprom_93xx46 DT bindings.

[Vladimir Zapolskiy]

Hi Cory,

On 10.12.2015 06:00, Cory Tusar wrote:
> This commit implements bindings in the eeprom_93xx46 driver allowing
> device word size and read-only attributes to be specified via
> devicetree.
> 
> Signed-off-by: Cory Tusar <cory.tusar@pid1solutions.com>
> Tested-by: Chris Healy <chris.healy@zii.aero>
> ---
>  drivers/misc/eeprom/eeprom_93xx46.c | 49 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
> 
> diff --git a/drivers/misc/eeprom/eeprom_93xx46.c b/drivers/misc/eeprom/eeprom_93xx46.c
> index e1bf0a5..cc27e11 100644
> --- a/drivers/misc/eeprom/eeprom_93xx46.c
> +++ b/drivers/misc/eeprom/eeprom_93xx46.c
> @@ -13,6 +13,8 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
>  #include <linux/slab.h>
>  #include <linux/spi/spi.h>
>  #include <linux/sysfs.h>
> @@ -294,12 +296,58 @@ static ssize_t eeprom_93xx46_store_erase(struct device *dev,
>  }
>  static DEVICE_ATTR(erase, S_IWUSR, NULL, eeprom_93xx46_store_erase);
>  
> +static const struct of_device_id eeprom_93xx46_of_table[] = {
> +	{ .compatible = "eeprom-93xx46", },

immediately do you want to add the second (and much, much more preferred IMO)
mentioned compatible value "atmel,at93c46d" to the list? Also see a note
below.

> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, eeprom_93xx46_of_table);
> +
> +static int eeprom_93xx46_probe_dt(struct spi_device *spi)
> +{
> +	struct device_node *np = spi->dev.of_node;
> +	struct eeprom_93xx46_platform_data *pd;
> +	u32 tmp;
> +	int ret;
> +
> +	pd = devm_kzalloc(&spi->dev, sizeof(*pd), GFP_KERNEL);
> +	if (!pd)
> +		return -ENOMEM;
> +
> +	ret = of_property_read_u32(np, "data-size", &tmp);
> +	if (ret < 0) {
> +		dev_err(&spi->dev, "data-size property not found\n");
> +		return ret;
> +	}
> +
> +	if (tmp == 8) {
> +		pd->flags |= EE_ADDR8;
> +	} else if (tmp == 16) {
> +		pd->flags |= EE_ADDR16;
> +	} else {
> +		dev_err(&spi->dev, "invalid data-size (%d)\n", tmp);
> +		return -EINVAL;
> +	}

If you insist on mandatory "data-size" property, could you please check
arch/x86/platform/ce4100/falconfalls.dts , does it require updates?

In that DTS you may find "atmel,at93c46" compatible value, is that kind
of devices covered by this driver? If yes, does it make sense to
add "atmel,at93c46" to the list of compatible values replacing
"atmel,at93c46d" ?

> +
> +	if (of_property_read_bool(np, "read-only"))
> +		pd->flags |= EE_READONLY;
> +
> +	spi->dev.platform_data = pd;
> +
> +	return 0;
> +}
> +
>  static int eeprom_93xx46_probe(struct spi_device *spi)
>  {
>  	struct eeprom_93xx46_platform_data *pd;
>  	struct eeprom_93xx46_dev *edev;
>  	int err;
>  
> +	if (spi->dev.of_node) {
> +		err = eeprom_93xx46_probe_dt(spi);
> +		if (err < 0)
> +			return err;
> +	}
> +
>  	pd = spi->dev.platform_data;
>  	if (!pd) {
>  		dev_err(&spi->dev, "missing platform data\n");
> @@ -370,6 +418,7 @@ static int eeprom_93xx46_remove(struct spi_device *spi)
>  static struct spi_driver eeprom_93xx46_driver = {
>  	.driver = {
>  		.name	= "93xx46",
> +		.of_match_table = of_match_ptr(eeprom_93xx46_of_table),
>  	},
>  	.probe		= eeprom_93xx46_probe,
>  	.remove		= eeprom_93xx46_remove,
> 

Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>

--
With best wishes,
Vladimir

Re: [PATCH v4 4/5] misc: eeprom_93xx46: Add quirks to support Atmel AT93C46D device.

[Vladimir Zapolskiy]

With best wishes,
Vladimir

On 10.12.2015 06:00, Cory Tusar wrote:
> Atmel devices in this family have some quirks not found in other similar
> chips - they do not support a sequential read of the entire EEPROM
> contents, and the control word sent at the start of each operation
> varies in bit length.
> 
> This commit adds quirk support to the driver and modifies the read
> implementation to support non-sequential reads for consistency with
> other misc/eeprom drivers.
> 
> Tested on a custom Freescale VF610-based platform, with an AT93C46D
> device attached via dspi2.  The spi-gpio driver was used to allow the
> necessary non-byte-sized transfers.
> 
> Signed-off-by: Cory Tusar <cory.tusar@pid1solutions.com>
> Tested-by: Chris Healy <chris.healy@zii.aero>
> ---
>  drivers/misc/eeprom/eeprom_93xx46.c | 126 ++++++++++++++++++++++++++----------
>  include/linux/eeprom_93xx46.h       |   6 ++
>  2 files changed, 97 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/misc/eeprom/eeprom_93xx46.c b/drivers/misc/eeprom/eeprom_93xx46.c
> index cc27e11..d50bc17 100644
> --- a/drivers/misc/eeprom/eeprom_93xx46.c
> +++ b/drivers/misc/eeprom/eeprom_93xx46.c
> @@ -27,6 +27,15 @@
>  #define ADDR_ERAL	0x20
>  #define ADDR_EWEN	0x30
>  
> +struct eeprom_93xx46_devtype_data {
> +	unsigned int quirks;
> +};
> +
> +static const struct eeprom_93xx46_devtype_data atmel_at93c46d_data = {
> +	.quirks = EEPROM_93XX46_QUIRK_SINGLE_WORD_READ |
> +		  EEPROM_93XX46_QUIRK_INSTRUCTION_LENGTH,
> +};
> +
>  struct eeprom_93xx46_dev {
>  	struct spi_device *spi;
>  	struct eeprom_93xx46_platform_data *pdata;
> @@ -35,6 +44,16 @@ struct eeprom_93xx46_dev {
>  	int addrlen;
>  };
>  
> +static inline bool has_quirk_single_word_read(struct eeprom_93xx46_dev *edev)
> +{
> +	return edev->pdata->quirks & EEPROM_93XX46_QUIRK_SINGLE_WORD_READ;
> +}
> +
> +static inline bool has_quirk_instruction_length(struct eeprom_93xx46_dev *edev)
> +{
> +	return edev->pdata->quirks & EEPROM_93XX46_QUIRK_INSTRUCTION_LENGTH;
> +}
> +
>  static ssize_t
>  eeprom_93xx46_bin_read(struct file *filp, struct kobject *kobj,
>  		       struct bin_attribute *bin_attr,
> @@ -42,58 +61,73 @@ eeprom_93xx46_bin_read(struct file *filp, struct kobject *kobj,
>  {
>  	struct eeprom_93xx46_dev *edev;
>  	struct device *dev;
> -	struct spi_message m;
> -	struct spi_transfer t[2];
> -	int bits, ret;
> -	u16 cmd_addr;
> +	ssize_t ret = 0;
>  
>  	dev = container_of(kobj, struct device, kobj);
>  	edev = dev_get_drvdata(dev);
>  
> -	cmd_addr = OP_READ << edev->addrlen;
> +	mutex_lock(&edev->lock);
>  
> -	if (edev->addrlen == 7) {
> -		cmd_addr |= off & 0x7f;
> -		bits = 10;
> -	} else {
> -		cmd_addr |= (off >> 1) & 0x3f;
> -		bits = 9;
> -	}
> +	if (edev->pdata->prepare)
> +		edev->pdata->prepare(edev);
>  
> -	dev_dbg(&edev->spi->dev, "read cmd 0x%x, %d Hz\n",
> -		cmd_addr, edev->spi->max_speed_hz);
> +	while (count) {
> +		struct spi_message m;
> +		struct spi_transfer t[2] = { { 0 } };
> +		u16 cmd_addr = OP_READ << edev->addrlen;
> +		size_t nbytes = count;
> +		int bits;
> +		int err;
> +
> +		if (edev->addrlen == 7) {
> +			cmd_addr |= off & 0x7f;
> +			bits = 10;
> +			if (has_quirk_single_word_read(edev))
> +				nbytes = 1;
> +		} else {
> +			cmd_addr |= (off >> 1) & 0x3f;
> +			bits = 9;
> +			if (has_quirk_single_word_read(edev))
> +				nbytes = 2;
> +		}
>  
> -	spi_message_init(&m);
> -	memset(t, 0, sizeof(t));
> +		dev_dbg(&edev->spi->dev, "read cmd 0x%x, %d Hz\n",
> +			cmd_addr, edev->spi->max_speed_hz);
>  
> -	t[0].tx_buf = (char *)&cmd_addr;
> -	t[0].len = 2;
> -	t[0].bits_per_word = bits;
> -	spi_message_add_tail(&t[0], &m);
> +		spi_message_init(&m);
>  
> -	t[1].rx_buf = buf;
> -	t[1].len = count;
> -	t[1].bits_per_word = 8;
> -	spi_message_add_tail(&t[1], &m);
> +		t[0].tx_buf = (char *)&cmd_addr;
> +		t[0].len = 2;
> +		t[0].bits_per_word = bits;
> +		spi_message_add_tail(&t[0], &m);
>  
> -	mutex_lock(&edev->lock);
> +		t[1].rx_buf = buf;
> +		t[1].len = count;
> +		t[1].bits_per_word = 8;
> +		spi_message_add_tail(&t[1], &m);
>  
> -	if (edev->pdata->prepare)
> -		edev->pdata->prepare(edev);
> +		err = spi_sync(edev->spi, &m);
> +		/* have to wait at least Tcsl ns */
> +		ndelay(250);
>  
> -	ret = spi_sync(edev->spi, &m);
> -	/* have to wait at least Tcsl ns */
> -	ndelay(250);
> -	if (ret) {
> -		dev_err(&edev->spi->dev, "read %zu bytes at %d: err. %d\n",
> -			count, (int)off, ret);
> +		if (err) {
> +			dev_err(&edev->spi->dev, "read %zu bytes at %d: err. %d\n",
> +				nbytes, (int)off, err);
> +			ret = err;
> +			break;
> +		}
> +
> +		buf += nbytes;
> +		off += nbytes;
> +		count -= nbytes;
> +		ret += nbytes;
>  	}
>  
>  	if (edev->pdata->finish)
>  		edev->pdata->finish(edev);
>  
>  	mutex_unlock(&edev->lock);
> -	return ret ? : count;
> +	return ret;
>  }
>  
>  static int eeprom_93xx46_ew(struct eeprom_93xx46_dev *edev, int is_on)
> @@ -112,7 +146,13 @@ static int eeprom_93xx46_ew(struct eeprom_93xx46_dev *edev, int is_on)
>  		bits = 9;
>  	}
>  
> -	dev_dbg(&edev->spi->dev, "ew cmd 0x%04x\n", cmd_addr);
> +	if (has_quirk_instruction_length(edev)) {
> +		cmd_addr <<= 2;
> +		bits += 2;
> +	}
> +
> +	dev_dbg(&edev->spi->dev, "ew%s cmd 0x%04x, %d bits\n",
> +			is_on ? "en" : "ds", cmd_addr, bits);
>  
>  	spi_message_init(&m);
>  	memset(&t, 0, sizeof(t));
> @@ -247,6 +287,13 @@ static int eeprom_93xx46_eral(struct eeprom_93xx46_dev *edev)
>  		bits = 9;
>  	}
>  
> +	if (has_quirk_instruction_length(edev)) {
> +		cmd_addr <<= 2;
> +		bits += 2;
> +	}
> +
> +	dev_dbg(&edev->spi->dev, "eral cmd 0x%04x, %d bits\n", cmd_addr, bits);
> +
>  	spi_message_init(&m);
>  	memset(&t, 0, sizeof(t));
>  
> @@ -298,12 +345,15 @@ static DEVICE_ATTR(erase, S_IWUSR, NULL, eeprom_93xx46_store_erase);
>  
>  static const struct of_device_id eeprom_93xx46_of_table[] = {
>  	{ .compatible = "eeprom-93xx46", },
> +	{ .compatible = "atmel,at93c46d", .data = &atmel_at93c46d_data, },

Ok, got the second "atmel,at93c46d" here.

>  	{}
>  };
>  MODULE_DEVICE_TABLE(of, eeprom_93xx46_of_table);
>  
>  static int eeprom_93xx46_probe_dt(struct spi_device *spi)
>  {
> +	const struct of_device_id *of_id =
> +		of_match_device(eeprom_93xx46_of_table, &spi->dev);
>  	struct device_node *np = spi->dev.of_node;
>  	struct eeprom_93xx46_platform_data *pd;
>  	u32 tmp;
> @@ -331,6 +381,12 @@ static int eeprom_93xx46_probe_dt(struct spi_device *spi)
>  	if (of_property_read_bool(np, "read-only"))
>  		pd->flags |= EE_READONLY;
>  
> +	if (of_id->data) {
> +		const struct eeprom_93xx46_devtype_data *data = of_id->data;
> +
> +		pd->quirks = data->quirks;
> +	}
> +
>  	spi->dev.platform_data = pd;
>  
>  	return 0;
> diff --git a/include/linux/eeprom_93xx46.h b/include/linux/eeprom_93xx46.h
> index 0679181..92fa4c3 100644
> --- a/include/linux/eeprom_93xx46.h
> +++ b/include/linux/eeprom_93xx46.h
> @@ -9,6 +9,12 @@ struct eeprom_93xx46_platform_data {
>  #define EE_ADDR16	0x02		/* 16 bit addr. cfg */
>  #define EE_READONLY	0x08		/* forbid writing */
>  
> +	unsigned int	quirks;
> +/* Single word read transfers only; no sequential read. */
> +#define EEPROM_93XX46_QUIRK_SINGLE_WORD_READ		(1 << 0)
> +/* Instructions such as EWEN are (addrlen + 2) in length. */
> +#define EEPROM_93XX46_QUIRK_INSTRUCTION_LENGTH		(1 << 1)
> +
>  	/*
>  	 * optional hooks to control additional logic
>  	 * before and after spi transfer.
> 

Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>

--
With best wishes,
Vladimir

Re: [PATCH v4 5/5] misc: eeprom_93xx46: Add support for a GPIO 'select' line.

[Vladimir Zapolskiy]

Hi Cory,

On 10.12.2015 06:00, Cory Tusar wrote:
> This commit adds support to the eeprom_93x46 driver allowing a GPIO line
> to function as a 'select' or 'enable' signal prior to accessing the
> EEPROM.
> 
> Signed-off-by: Cory Tusar <cory.tusar@pid1solutions.com>
> Tested-by: Chris Healy <chris.healy@zii.aero>
> ---
>  drivers/misc/eeprom/eeprom_93xx46.c | 35 +++++++++++++++++++++++++++++++++++
>  include/linux/eeprom_93xx46.h       |  3 +++
>  2 files changed, 38 insertions(+)
> 
> diff --git a/drivers/misc/eeprom/eeprom_93xx46.c b/drivers/misc/eeprom/eeprom_93xx46.c
> index d50bc17..d28fac2 100644
> --- a/drivers/misc/eeprom/eeprom_93xx46.c
> +++ b/drivers/misc/eeprom/eeprom_93xx46.c
> @@ -10,11 +10,13 @@
>  
>  #include <linux/delay.h>
>  #include <linux/device.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> +#include <linux/of_gpio.h>
>  #include <linux/slab.h>
>  #include <linux/spi/spi.h>
>  #include <linux/sysfs.h>
> @@ -343,6 +345,20 @@ static ssize_t eeprom_93xx46_store_erase(struct device *dev,
>  }
>  static DEVICE_ATTR(erase, S_IWUSR, NULL, eeprom_93xx46_store_erase);
>  
> +static void select_assert(void *context)
> +{
> +	struct eeprom_93xx46_dev *edev = context;
> +
> +	gpiod_set_value_cansleep(edev->pdata->select, 1);
> +}
> +
> +static void select_deassert(void *context)
> +{
> +	struct eeprom_93xx46_dev *edev = context;
> +
> +	gpiod_set_value_cansleep(edev->pdata->select, 0);
> +}
> +
>  static const struct of_device_id eeprom_93xx46_of_table[] = {
>  	{ .compatible = "eeprom-93xx46", },
>  	{ .compatible = "atmel,at93c46d", .data = &atmel_at93c46d_data, },
> @@ -357,6 +373,8 @@ static int eeprom_93xx46_probe_dt(struct spi_device *spi)
>  	struct device_node *np = spi->dev.of_node;
>  	struct eeprom_93xx46_platform_data *pd;
>  	u32 tmp;
> +	int gpio;
> +	enum of_gpio_flags of_flags;
>  	int ret;
>  
>  	pd = devm_kzalloc(&spi->dev, sizeof(*pd), GFP_KERNEL);
> @@ -381,6 +399,23 @@ static int eeprom_93xx46_probe_dt(struct spi_device *spi)
>  	if (of_property_read_bool(np, "read-only"))
>  		pd->flags |= EE_READONLY;
>  
> +	gpio = of_get_named_gpio_flags(np, "select-gpios", 0, &of_flags);
> +	if (gpio_is_valid(gpio)) {
> +		unsigned long flags =
> +			of_flags == OF_GPIO_ACTIVE_LOW ? GPIOF_ACTIVE_LOW : 0;
> +
> +		ret = devm_gpio_request_one(&spi->dev, gpio, flags,
> +					    "eeprom_93xx46_select");
> +		if (ret)
> +			return ret;
> +
> +		pd->select = gpio_to_desc(gpio);
> +		pd->prepare = select_assert;
> +		pd->finish = select_deassert;
> +
> +		gpiod_direction_output(pd->select, 0);
> +	}
> +
>  	if (of_id->data) {
>  		const struct eeprom_93xx46_devtype_data *data = of_id->data;
>  
> diff --git a/include/linux/eeprom_93xx46.h b/include/linux/eeprom_93xx46.h
> index 92fa4c3..03f3435 100644
> --- a/include/linux/eeprom_93xx46.h
> +++ b/include/linux/eeprom_93xx46.h
> @@ -3,6 +3,8 @@
>   * platform description for 93xx46 EEPROMs.
>   */
>  
> +#include <linux/of_gpio.h>
> +

You need just "struct gpio_desc;" instead of the include here, please fix.

>  struct eeprom_93xx46_platform_data {
>  	unsigned char	flags;
>  #define EE_ADDR8	0x01		/*  8 bit addr. cfg */
> @@ -21,4 +23,5 @@ struct eeprom_93xx46_platform_data {
>  	 */
>  	void (*prepare)(void *);
>  	void (*finish)(void *);
> +	struct gpio_desc *select;
>  };
> 

Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>

--
With best wishes,
Vladimir

Re: [PATCH v4 5/5] misc: eeprom_93xx46: Add support for a GPIO 'select' line.

[Vladimir Zapolskiy]

Hi Cory,

On 10.12.2015 06:00, Cory Tusar wrote:
> This commit adds support to the eeprom_93x46 driver allowing a GPIO line
> to function as a 'select' or 'enable' signal prior to accessing the
> EEPROM.
> 
> Signed-off-by: Cory Tusar <cory.tusar-J6Z/VSE8EyIAspv4Qr0y0gC/G2K4zDHf@public.gmane.org>
> Tested-by: Chris Healy <chris.healy-c8ZVq/bFV1I@public.gmane.org>
> ---
>  drivers/misc/eeprom/eeprom_93xx46.c | 35 +++++++++++++++++++++++++++++++++++
>  include/linux/eeprom_93xx46.h       |  3 +++
>  2 files changed, 38 insertions(+)
> 
> diff --git a/drivers/misc/eeprom/eeprom_93xx46.c b/drivers/misc/eeprom/eeprom_93xx46.c
> index d50bc17..d28fac2 100644
> --- a/drivers/misc/eeprom/eeprom_93xx46.c
> +++ b/drivers/misc/eeprom/eeprom_93xx46.c
> @@ -10,11 +10,13 @@
>  
>  #include <linux/delay.h>
>  #include <linux/device.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> +#include <linux/of_gpio.h>
>  #include <linux/slab.h>
>  #include <linux/spi/spi.h>
>  #include <linux/sysfs.h>
> @@ -343,6 +345,20 @@ static ssize_t eeprom_93xx46_store_erase(struct device *dev,
>  }
>  static DEVICE_ATTR(erase, S_IWUSR, NULL, eeprom_93xx46_store_erase);
>  
> +static void select_assert(void *context)
> +{
> +	struct eeprom_93xx46_dev *edev = context;
> +
> +	gpiod_set_value_cansleep(edev->pdata->select, 1);
> +}
> +
> +static void select_deassert(void *context)
> +{
> +	struct eeprom_93xx46_dev *edev = context;
> +
> +	gpiod_set_value_cansleep(edev->pdata->select, 0);
> +}
> +
>  static const struct of_device_id eeprom_93xx46_of_table[] = {
>  	{ .compatible = "eeprom-93xx46", },
>  	{ .compatible = "atmel,at93c46d", .data = &atmel_at93c46d_data, },
> @@ -357,6 +373,8 @@ static int eeprom_93xx46_probe_dt(struct spi_device *spi)
>  	struct device_node *np = spi->dev.of_node;
>  	struct eeprom_93xx46_platform_data *pd;
>  	u32 tmp;
> +	int gpio;
> +	enum of_gpio_flags of_flags;
>  	int ret;
>  
>  	pd = devm_kzalloc(&spi->dev, sizeof(*pd), GFP_KERNEL);
> @@ -381,6 +399,23 @@ static int eeprom_93xx46_probe_dt(struct spi_device *spi)
>  	if (of_property_read_bool(np, "read-only"))
>  		pd->flags |= EE_READONLY;
>  
> +	gpio = of_get_named_gpio_flags(np, "select-gpios", 0, &of_flags);
> +	if (gpio_is_valid(gpio)) {
> +		unsigned long flags =
> +			of_flags == OF_GPIO_ACTIVE_LOW ? GPIOF_ACTIVE_LOW : 0;
> +
> +		ret = devm_gpio_request_one(&spi->dev, gpio, flags,
> +					    "eeprom_93xx46_select");
> +		if (ret)
> +			return ret;
> +
> +		pd->select = gpio_to_desc(gpio);
> +		pd->prepare = select_assert;
> +		pd->finish = select_deassert;
> +
> +		gpiod_direction_output(pd->select, 0);
> +	}
> +
>  	if (of_id->data) {
>  		const struct eeprom_93xx46_devtype_data *data = of_id->data;
>  
> diff --git a/include/linux/eeprom_93xx46.h b/include/linux/eeprom_93xx46.h
> index 92fa4c3..03f3435 100644
> --- a/include/linux/eeprom_93xx46.h
> +++ b/include/linux/eeprom_93xx46.h
> @@ -3,6 +3,8 @@
>   * platform description for 93xx46 EEPROMs.
>   */
>  
> +#include <linux/of_gpio.h>
> +

You need just "struct gpio_desc;" instead of the include here, please fix.

>  struct eeprom_93xx46_platform_data {
>  	unsigned char	flags;
>  #define EE_ADDR8	0x01		/*  8 bit addr. cfg */
> @@ -21,4 +23,5 @@ struct eeprom_93xx46_platform_data {
>  	 */
>  	void (*prepare)(void *);
>  	void (*finish)(void *);
> +	struct gpio_desc *select;
>  };
> 

Reviewed-by: Vladimir Zapolskiy <vz-ChpfBGZJDbMAvxtiuMwx3w@public.gmane.org>

--
With best wishes,
Vladimir
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 2/5] Documentation: devicetree: Add DT bindings to eeprom_93xx46 driver.

[Cory Tusar]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 12/21/2015 07:01 PM, Vladimir Zapolskiy wrote:
> Hi Cory,
> 
> On 10.12.2015 06:00, Cory Tusar wrote:
>> This commit documents bindings to be added to the eeprom_93xx46 driver
>> which will allow:
>>
>>   - Device word size and read-only attributes to be specified.
>>   - A device-specific compatible string for use with Atmel AT93C46D
>>     EEPROMs.
>>   - Specifying a GPIO line to function as a 'select' or 'enable' signal
>>     prior to accessing the EEPROM.
>>
>> Signed-off-by: Cory Tusar <cory.tusar@pid1solutions.com>
>> Acked-by: Rob Herring <robh@kernel.org>
>> Tested-by: Chris Healy <chris.healy@zii.aero>
>> ---
>>  .../devicetree/bindings/misc/eeprom-93xx46.txt     | 25 ++++++++++++++++++++++
>>  1 file changed, 25 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/misc/eeprom-93xx46.txt b/Documentation/devicetree/bindings/misc/eeprom-93xx46.txt
>> new file mode 100644
>> index 0000000..a8ebb46
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/misc/eeprom-93xx46.txt
>> @@ -0,0 +1,25 @@
>> +EEPROMs (SPI) compatible with Microchip Technology 93xx46 family.
>> +
>> +Required properties:
>> +- compatible : shall be one of:
>> +    "atmel,at93c46d"
>> +    "eeprom-93xx46"
> 
> the second compatible property value is not recommended by ePAPR, I would
> suggest to remove it from here and from the example below.

Hi Vladimir,

Yes, I know.  It's a "generic" binding, intended to allow any existing
users of this driver to move easily to DT with the exact same
configuration parameters exposed as the current platform device
implementation.

I took the at24 bindings (i.e. "at24c00") as an example.  I suppose I
could have stuck with just "93xx46" for the model, but instead used
"eeprom-93xx46" to match the name of the driver.

> But I see that Rob acked this change though...

Rob, does this still have your ACK as-is, given the above?

Thanks and best regards,
- -Cory


>> +- data-size : number of data bits per word (either 8 or 16)
>> +
>> +Optional properties:
>> +- read-only : parameter-less property which disables writes to the EEPROM
>> +- select-gpios : if present, specifies the GPIO that will be asserted prior to
>> +  each access to the EEPROM (e.g. for SPI bus multiplexing)
>> +
>> +Property rules described in Documentation/devicetree/bindings/spi/spi-bus.txt
>> +apply.  In particular, "reg" and "spi-max-frequency" properties must be given.
>> +
>> +Example:
>> +	eeprom@0 {
>> +		compatible = "eeprom-93xx46";
>> +		reg = <0>;
>> +		spi-max-frequency = <1000000>;
>> +		spi-cs-high;
>> +		data-size = <8>;
>> +		select-gpios = <&gpio4 4 GPIO_ACTIVE_HIGH>;
>> +	};
>>
> 
> --
> With best wishes,
> Vladimir
> 


- -- 
Cory Tusar
Principal
PID 1 Solutions, Inc.


"There are two ways of constructing a software design.  One way is to
 make it so simple that there are obviously no deficiencies, and the
 other way is to make it so complicated that there are no obvious
 deficiencies."  --Sir Charles Anthony Richard Hoare

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iEYEARECAAYFAlaMfhgACgkQHT1tsfGwHJ+QRwCffOZe1SiQnQPScyC2k8xU9px/
dt8An2X0c2K9TVaKo8euOIBNP/+8Y+z8
=4vbd
-----END PGP SIGNATURE-----

Re: [PATCH v4 2/5] Documentation: devicetree: Add DT bindings to eeprom_93xx46 driver.

[Cory Tusar]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 12/21/2015 07:01 PM, Vladimir Zapolskiy wrote:
> Hi Cory,
> 
> On 10.12.2015 06:00, Cory Tusar wrote:
>> This commit documents bindings to be added to the eeprom_93xx46 driver
>> which will allow:
>>
>>   - Device word size and read-only attributes to be specified.
>>   - A device-specific compatible string for use with Atmel AT93C46D
>>     EEPROMs.
>>   - Specifying a GPIO line to function as a 'select' or 'enable' signal
>>     prior to accessing the EEPROM.
>>
>> Signed-off-by: Cory Tusar <cory.tusar-J6Z/VSE8EyIAspv4Qr0y0gC/G2K4zDHf@public.gmane.org>
>> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> Tested-by: Chris Healy <chris.healy-c8ZVq/bFV1I@public.gmane.org>
>> ---
>>  .../devicetree/bindings/misc/eeprom-93xx46.txt     | 25 ++++++++++++++++++++++
>>  1 file changed, 25 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/misc/eeprom-93xx46.txt b/Documentation/devicetree/bindings/misc/eeprom-93xx46.txt
>> new file mode 100644
>> index 0000000..a8ebb46
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/misc/eeprom-93xx46.txt
>> @@ -0,0 +1,25 @@
>> +EEPROMs (SPI) compatible with Microchip Technology 93xx46 family.
>> +
>> +Required properties:
>> +- compatible : shall be one of:
>> +    "atmel,at93c46d"
>> +    "eeprom-93xx46"
> 
> the second compatible property value is not recommended by ePAPR, I would
> suggest to remove it from here and from the example below.

Hi Vladimir,

Yes, I know.  It's a "generic" binding, intended to allow any existing
users of this driver to move easily to DT with the exact same
configuration parameters exposed as the current platform device
implementation.

I took the at24 bindings (i.e. "at24c00") as an example.  I suppose I
could have stuck with just "93xx46" for the model, but instead used
"eeprom-93xx46" to match the name of the driver.

> But I see that Rob acked this change though...

Rob, does this still have your ACK as-is, given the above?

Thanks and best regards,
- -Cory


>> +- data-size : number of data bits per word (either 8 or 16)
>> +
>> +Optional properties:
>> +- read-only : parameter-less property which disables writes to the EEPROM
>> +- select-gpios : if present, specifies the GPIO that will be asserted prior to
>> +  each access to the EEPROM (e.g. for SPI bus multiplexing)
>> +
>> +Property rules described in Documentation/devicetree/bindings/spi/spi-bus.txt
>> +apply.  In particular, "reg" and "spi-max-frequency" properties must be given.
>> +
>> +Example:
>> +	eeprom@0 {
>> +		compatible = "eeprom-93xx46";
>> +		reg = <0>;
>> +		spi-max-frequency = <1000000>;
>> +		spi-cs-high;
>> +		data-size = <8>;
>> +		select-gpios = <&gpio4 4 GPIO_ACTIVE_HIGH>;
>> +	};
>>
> 
> --
> With best wishes,
> Vladimir
> 


- -- 
Cory Tusar
Principal
PID 1 Solutions, Inc.


"There are two ways of constructing a software design.  One way is to
 make it so simple that there are obviously no deficiencies, and the
 other way is to make it so complicated that there are no obvious
 deficiencies."  --Sir Charles Anthony Richard Hoare

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iEYEARECAAYFAlaMfhgACgkQHT1tsfGwHJ+QRwCffOZe1SiQnQPScyC2k8xU9px/
dt8An2X0c2K9TVaKo8euOIBNP/+8Y+z8
=4vbd
-----END PGP SIGNATURE-----
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/5] misc: eeprom_93xx46: Implement eeprom_93xx46 DT bindings.

[Cory Tusar]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 12/21/2015 07:12 PM, Vladimir Zapolskiy wrote:
> Hi Cory,
> 
> On 10.12.2015 06:00, Cory Tusar wrote:
>> This commit implements bindings in the eeprom_93xx46 driver allowing
>> device word size and read-only attributes to be specified via
>> devicetree.
>>
>> Signed-off-by: Cory Tusar <cory.tusar@pid1solutions.com>
>> Tested-by: Chris Healy <chris.healy@zii.aero>
>> ---
>>  drivers/misc/eeprom/eeprom_93xx46.c | 49 +++++++++++++++++++++++++++++++++++++
>>  1 file changed, 49 insertions(+)
>>
>> diff --git a/drivers/misc/eeprom/eeprom_93xx46.c b/drivers/misc/eeprom/eeprom_93xx46.c
>> index e1bf0a5..cc27e11 100644
>> --- a/drivers/misc/eeprom/eeprom_93xx46.c
>> +++ b/drivers/misc/eeprom/eeprom_93xx46.c
>> @@ -13,6 +13,8 @@
>>  #include <linux/kernel.h>
>>  #include <linux/module.h>
>>  #include <linux/mutex.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>>  #include <linux/slab.h>
>>  #include <linux/spi/spi.h>
>>  #include <linux/sysfs.h>
>> @@ -294,12 +296,58 @@ static ssize_t eeprom_93xx46_store_erase(struct device *dev,
>>  }
>>  static DEVICE_ATTR(erase, S_IWUSR, NULL, eeprom_93xx46_store_erase);
>>  
>> +static const struct of_device_id eeprom_93xx46_of_table[] = {
>> +	{ .compatible = "eeprom-93xx46", },
> 
> immediately do you want to add the second (and much, much more preferred IMO)
> mentioned compatible value "atmel,at93c46d" to the list? Also see a note
> below.

Hi Vladimir,

That compatible value is included as part of the next patch; the Atmel
part required additional quirk support which was (IMO) separate from
exposing just existing platform device parameters.

> 
>> +	{}
>> +};
>> +MODULE_DEVICE_TABLE(of, eeprom_93xx46_of_table);
>> +
>> +static int eeprom_93xx46_probe_dt(struct spi_device *spi)
>> +{
>> +	struct device_node *np = spi->dev.of_node;
>> +	struct eeprom_93xx46_platform_data *pd;
>> +	u32 tmp;
>> +	int ret;
>> +
>> +	pd = devm_kzalloc(&spi->dev, sizeof(*pd), GFP_KERNEL);
>> +	if (!pd)
>> +		return -ENOMEM;
>> +
>> +	ret = of_property_read_u32(np, "data-size", &tmp);
>> +	if (ret < 0) {
>> +		dev_err(&spi->dev, "data-size property not found\n");
>> +		return ret;
>> +	}
>> +
>> +	if (tmp == 8) {
>> +		pd->flags |= EE_ADDR8;
>> +	} else if (tmp == 16) {
>> +		pd->flags |= EE_ADDR16;
>> +	} else {
>> +		dev_err(&spi->dev, "invalid data-size (%d)\n", tmp);
>> +		return -EINVAL;
>> +	}
> 
> If you insist on mandatory "data-size" property, could you please check
> arch/x86/platform/ce4100/falconfalls.dts , does it require updates?

Unknown.  I see no other documented reference to that particular
compatible string...looks like it was a placeholder back when the
CE4100 platform was first added.

> In that DTS you may find "atmel,at93c46" compatible value, is that kind
> of devices covered by this driver? If yes, does it make sense to
> add "atmel,at93c46" to the list of compatible values replacing
> "atmel,at93c46d" ?

Yes, that device _should_ generally work, but a quick look at both
datasheets leads me to believe that quirks differ enough to probably
warrant its own compatible value.

For example, EWEN for an x8 strapped AT93C46 is b'10011xxxxx and
b'10011xxxxxxx for the AT93C46D.  That difference is handled by
EEPROM_93XX46_QUIRK_INSTRUCTION_LENGTH.

> 
>> +
>> +	if (of_property_read_bool(np, "read-only"))
>> +		pd->flags |= EE_READONLY;
>> +
>> +	spi->dev.platform_data = pd;
>> +
>> +	return 0;
>> +}
>> +
>>  static int eeprom_93xx46_probe(struct spi_device *spi)
>>  {
>>  	struct eeprom_93xx46_platform_data *pd;
>>  	struct eeprom_93xx46_dev *edev;
>>  	int err;
>>  
>> +	if (spi->dev.of_node) {
>> +		err = eeprom_93xx46_probe_dt(spi);
>> +		if (err < 0)
>> +			return err;
>> +	}
>> +
>>  	pd = spi->dev.platform_data;
>>  	if (!pd) {
>>  		dev_err(&spi->dev, "missing platform data\n");
>> @@ -370,6 +418,7 @@ static int eeprom_93xx46_remove(struct spi_device *spi)
>>  static struct spi_driver eeprom_93xx46_driver = {
>>  	.driver = {
>>  		.name	= "93xx46",
>> +		.of_match_table = of_match_ptr(eeprom_93xx46_of_table),
>>  	},
>>  	.probe		= eeprom_93xx46_probe,
>>  	.remove		= eeprom_93xx46_remove,
>>
> 
> Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>

Thanks and best regards,
- -Cory


- -- 
Cory Tusar
Principal
PID 1 Solutions, Inc.


"There are two ways of constructing a software design.  One way is to
 make it so simple that there are obviously no deficiencies, and the
 other way is to make it so complicated that there are no obvious
 deficiencies."  --Sir Charles Anthony Richard Hoare

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iEYEARECAAYFAlaMh4kACgkQHT1tsfGwHJ/cpgCgmedYRGYlq+tTNzl2akHZk2C7
3s8AoJ0qI14wafGJypOfbQii3kX+6Kdd
=z4WO
-----END PGP SIGNATURE-----

Re: [PATCH v4 3/5] misc: eeprom_93xx46: Implement eeprom_93xx46 DT bindings.

[Cory Tusar]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 12/21/2015 07:12 PM, Vladimir Zapolskiy wrote:
> Hi Cory,
> 
> On 10.12.2015 06:00, Cory Tusar wrote:
>> This commit implements bindings in the eeprom_93xx46 driver allowing
>> device word size and read-only attributes to be specified via
>> devicetree.
>>
>> Signed-off-by: Cory Tusar <cory.tusar-J6Z/VSE8EyIAspv4Qr0y0gC/G2K4zDHf@public.gmane.org>
>> Tested-by: Chris Healy <chris.healy-c8ZVq/bFV1I@public.gmane.org>
>> ---
>>  drivers/misc/eeprom/eeprom_93xx46.c | 49 +++++++++++++++++++++++++++++++++++++
>>  1 file changed, 49 insertions(+)
>>
>> diff --git a/drivers/misc/eeprom/eeprom_93xx46.c b/drivers/misc/eeprom/eeprom_93xx46.c
>> index e1bf0a5..cc27e11 100644
>> --- a/drivers/misc/eeprom/eeprom_93xx46.c
>> +++ b/drivers/misc/eeprom/eeprom_93xx46.c
>> @@ -13,6 +13,8 @@
>>  #include <linux/kernel.h>
>>  #include <linux/module.h>
>>  #include <linux/mutex.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>>  #include <linux/slab.h>
>>  #include <linux/spi/spi.h>
>>  #include <linux/sysfs.h>
>> @@ -294,12 +296,58 @@ static ssize_t eeprom_93xx46_store_erase(struct device *dev,
>>  }
>>  static DEVICE_ATTR(erase, S_IWUSR, NULL, eeprom_93xx46_store_erase);
>>  
>> +static const struct of_device_id eeprom_93xx46_of_table[] = {
>> +	{ .compatible = "eeprom-93xx46", },
> 
> immediately do you want to add the second (and much, much more preferred IMO)
> mentioned compatible value "atmel,at93c46d" to the list? Also see a note
> below.

Hi Vladimir,

That compatible value is included as part of the next patch; the Atmel
part required additional quirk support which was (IMO) separate from
exposing just existing platform device parameters.

> 
>> +	{}
>> +};
>> +MODULE_DEVICE_TABLE(of, eeprom_93xx46_of_table);
>> +
>> +static int eeprom_93xx46_probe_dt(struct spi_device *spi)
>> +{
>> +	struct device_node *np = spi->dev.of_node;
>> +	struct eeprom_93xx46_platform_data *pd;
>> +	u32 tmp;
>> +	int ret;
>> +
>> +	pd = devm_kzalloc(&spi->dev, sizeof(*pd), GFP_KERNEL);
>> +	if (!pd)
>> +		return -ENOMEM;
>> +
>> +	ret = of_property_read_u32(np, "data-size", &tmp);
>> +	if (ret < 0) {
>> +		dev_err(&spi->dev, "data-size property not found\n");
>> +		return ret;
>> +	}
>> +
>> +	if (tmp == 8) {
>> +		pd->flags |= EE_ADDR8;
>> +	} else if (tmp == 16) {
>> +		pd->flags |= EE_ADDR16;
>> +	} else {
>> +		dev_err(&spi->dev, "invalid data-size (%d)\n", tmp);
>> +		return -EINVAL;
>> +	}
> 
> If you insist on mandatory "data-size" property, could you please check
> arch/x86/platform/ce4100/falconfalls.dts , does it require updates?

Unknown.  I see no other documented reference to that particular
compatible string...looks like it was a placeholder back when the
CE4100 platform was first added.

> In that DTS you may find "atmel,at93c46" compatible value, is that kind
> of devices covered by this driver? If yes, does it make sense to
> add "atmel,at93c46" to the list of compatible values replacing
> "atmel,at93c46d" ?

Yes, that device _should_ generally work, but a quick look at both
datasheets leads me to believe that quirks differ enough to probably
warrant its own compatible value.

For example, EWEN for an x8 strapped AT93C46 is b'10011xxxxx and
b'10011xxxxxxx for the AT93C46D.  That difference is handled by
EEPROM_93XX46_QUIRK_INSTRUCTION_LENGTH.

> 
>> +
>> +	if (of_property_read_bool(np, "read-only"))
>> +		pd->flags |= EE_READONLY;
>> +
>> +	spi->dev.platform_data = pd;
>> +
>> +	return 0;
>> +}
>> +
>>  static int eeprom_93xx46_probe(struct spi_device *spi)
>>  {
>>  	struct eeprom_93xx46_platform_data *pd;
>>  	struct eeprom_93xx46_dev *edev;
>>  	int err;
>>  
>> +	if (spi->dev.of_node) {
>> +		err = eeprom_93xx46_probe_dt(spi);
>> +		if (err < 0)
>> +			return err;
>> +	}
>> +
>>  	pd = spi->dev.platform_data;
>>  	if (!pd) {
>>  		dev_err(&spi->dev, "missing platform data\n");
>> @@ -370,6 +418,7 @@ static int eeprom_93xx46_remove(struct spi_device *spi)
>>  static struct spi_driver eeprom_93xx46_driver = {
>>  	.driver = {
>>  		.name	= "93xx46",
>> +		.of_match_table = of_match_ptr(eeprom_93xx46_of_table),
>>  	},
>>  	.probe		= eeprom_93xx46_probe,
>>  	.remove		= eeprom_93xx46_remove,
>>
> 
> Reviewed-by: Vladimir Zapolskiy <vz-ChpfBGZJDbMAvxtiuMwx3w@public.gmane.org>

Thanks and best regards,
- -Cory


- -- 
Cory Tusar
Principal
PID 1 Solutions, Inc.


"There are two ways of constructing a software design.  One way is to
 make it so simple that there are obviously no deficiencies, and the
 other way is to make it so complicated that there are no obvious
 deficiencies."  --Sir Charles Anthony Richard Hoare

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iEYEARECAAYFAlaMh4kACgkQHT1tsfGwHJ/cpgCgmedYRGYlq+tTNzl2akHZk2C7
3s8AoJ0qI14wafGJypOfbQii3kX+6Kdd
=z4WO
-----END PGP SIGNATURE-----
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 4/5] misc: eeprom_93xx46: Add quirks to support Atmel AT93C46D device.

[Cory Tusar]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 12/21/2015 07:19 PM, Vladimir Zapolskiy wrote:
> 
> 
> With best wishes,
> Vladimir
> 
> On 10.12.2015 06:00, Cory Tusar wrote:
>> Atmel devices in this family have some quirks not found in other similar
>> chips - they do not support a sequential read of the entire EEPROM
>> contents, and the control word sent at the start of each operation
>> varies in bit length.
>>
>> This commit adds quirk support to the driver and modifies the read
>> implementation to support non-sequential reads for consistency with
>> other misc/eeprom drivers.
>>
>> Tested on a custom Freescale VF610-based platform, with an AT93C46D
>> device attached via dspi2.  The spi-gpio driver was used to allow the
>> necessary non-byte-sized transfers.
>>
>> Signed-off-by: Cory Tusar <cory.tusar@pid1solutions.com>
>> Tested-by: Chris Healy <chris.healy@zii.aero>
>> ---
>>  drivers/misc/eeprom/eeprom_93xx46.c | 126 ++++++++++++++++++++++++++----------
>>  include/linux/eeprom_93xx46.h       |   6 ++
>>  2 files changed, 97 insertions(+), 35 deletions(-)
>>
>> diff --git a/drivers/misc/eeprom/eeprom_93xx46.c b/drivers/misc/eeprom/eeprom_93xx46.c
>> index cc27e11..d50bc17 100644
>> --- a/drivers/misc/eeprom/eeprom_93xx46.c
>> +++ b/drivers/misc/eeprom/eeprom_93xx46.c
>> @@ -27,6 +27,15 @@
>>  #define ADDR_ERAL	0x20
>>  #define ADDR_EWEN	0x30
>>  
>> +struct eeprom_93xx46_devtype_data {
>> +	unsigned int quirks;
>> +};
>> +
>> +static const struct eeprom_93xx46_devtype_data atmel_at93c46d_data = {
>> +	.quirks = EEPROM_93XX46_QUIRK_SINGLE_WORD_READ |
>> +		  EEPROM_93XX46_QUIRK_INSTRUCTION_LENGTH,
>> +};
>> +
>>  struct eeprom_93xx46_dev {
>>  	struct spi_device *spi;
>>  	struct eeprom_93xx46_platform_data *pdata;
>> @@ -35,6 +44,16 @@ struct eeprom_93xx46_dev {
>>  	int addrlen;
>>  };
>>  
>> +static inline bool has_quirk_single_word_read(struct eeprom_93xx46_dev *edev)
>> +{
>> +	return edev->pdata->quirks & EEPROM_93XX46_QUIRK_SINGLE_WORD_READ;
>> +}
>> +
>> +static inline bool has_quirk_instruction_length(struct eeprom_93xx46_dev *edev)
>> +{
>> +	return edev->pdata->quirks & EEPROM_93XX46_QUIRK_INSTRUCTION_LENGTH;
>> +}
>> +
>>  static ssize_t
>>  eeprom_93xx46_bin_read(struct file *filp, struct kobject *kobj,
>>  		       struct bin_attribute *bin_attr,
>> @@ -42,58 +61,73 @@ eeprom_93xx46_bin_read(struct file *filp, struct kobject *kobj,
>>  {
>>  	struct eeprom_93xx46_dev *edev;
>>  	struct device *dev;
>> -	struct spi_message m;
>> -	struct spi_transfer t[2];
>> -	int bits, ret;
>> -	u16 cmd_addr;
>> +	ssize_t ret = 0;
>>  
>>  	dev = container_of(kobj, struct device, kobj);
>>  	edev = dev_get_drvdata(dev);
>>  
>> -	cmd_addr = OP_READ << edev->addrlen;
>> +	mutex_lock(&edev->lock);
>>  
>> -	if (edev->addrlen == 7) {
>> -		cmd_addr |= off & 0x7f;
>> -		bits = 10;
>> -	} else {
>> -		cmd_addr |= (off >> 1) & 0x3f;
>> -		bits = 9;
>> -	}
>> +	if (edev->pdata->prepare)
>> +		edev->pdata->prepare(edev);
>>  
>> -	dev_dbg(&edev->spi->dev, "read cmd 0x%x, %d Hz\n",
>> -		cmd_addr, edev->spi->max_speed_hz);
>> +	while (count) {
>> +		struct spi_message m;
>> +		struct spi_transfer t[2] = { { 0 } };
>> +		u16 cmd_addr = OP_READ << edev->addrlen;
>> +		size_t nbytes = count;
>> +		int bits;
>> +		int err;
>> +
>> +		if (edev->addrlen == 7) {
>> +			cmd_addr |= off & 0x7f;
>> +			bits = 10;
>> +			if (has_quirk_single_word_read(edev))
>> +				nbytes = 1;
>> +		} else {
>> +			cmd_addr |= (off >> 1) & 0x3f;
>> +			bits = 9;
>> +			if (has_quirk_single_word_read(edev))
>> +				nbytes = 2;
>> +		}
>>  
>> -	spi_message_init(&m);
>> -	memset(t, 0, sizeof(t));
>> +		dev_dbg(&edev->spi->dev, "read cmd 0x%x, %d Hz\n",
>> +			cmd_addr, edev->spi->max_speed_hz);
>>  
>> -	t[0].tx_buf = (char *)&cmd_addr;
>> -	t[0].len = 2;
>> -	t[0].bits_per_word = bits;
>> -	spi_message_add_tail(&t[0], &m);
>> +		spi_message_init(&m);
>>  
>> -	t[1].rx_buf = buf;
>> -	t[1].len = count;
>> -	t[1].bits_per_word = 8;
>> -	spi_message_add_tail(&t[1], &m);
>> +		t[0].tx_buf = (char *)&cmd_addr;
>> +		t[0].len = 2;
>> +		t[0].bits_per_word = bits;
>> +		spi_message_add_tail(&t[0], &m);
>>  
>> -	mutex_lock(&edev->lock);
>> +		t[1].rx_buf = buf;
>> +		t[1].len = count;
>> +		t[1].bits_per_word = 8;
>> +		spi_message_add_tail(&t[1], &m);
>>  
>> -	if (edev->pdata->prepare)
>> -		edev->pdata->prepare(edev);
>> +		err = spi_sync(edev->spi, &m);
>> +		/* have to wait at least Tcsl ns */
>> +		ndelay(250);
>>  
>> -	ret = spi_sync(edev->spi, &m);
>> -	/* have to wait at least Tcsl ns */
>> -	ndelay(250);
>> -	if (ret) {
>> -		dev_err(&edev->spi->dev, "read %zu bytes at %d: err. %d\n",
>> -			count, (int)off, ret);
>> +		if (err) {
>> +			dev_err(&edev->spi->dev, "read %zu bytes at %d: err. %d\n",
>> +				nbytes, (int)off, err);
>> +			ret = err;
>> +			break;
>> +		}
>> +
>> +		buf += nbytes;
>> +		off += nbytes;
>> +		count -= nbytes;
>> +		ret += nbytes;
>>  	}
>>  
>>  	if (edev->pdata->finish)
>>  		edev->pdata->finish(edev);
>>  
>>  	mutex_unlock(&edev->lock);
>> -	return ret ? : count;
>> +	return ret;
>>  }
>>  
>>  static int eeprom_93xx46_ew(struct eeprom_93xx46_dev *edev, int is_on)
>> @@ -112,7 +146,13 @@ static int eeprom_93xx46_ew(struct eeprom_93xx46_dev *edev, int is_on)
>>  		bits = 9;
>>  	}
>>  
>> -	dev_dbg(&edev->spi->dev, "ew cmd 0x%04x\n", cmd_addr);
>> +	if (has_quirk_instruction_length(edev)) {
>> +		cmd_addr <<= 2;
>> +		bits += 2;
>> +	}
>> +
>> +	dev_dbg(&edev->spi->dev, "ew%s cmd 0x%04x, %d bits\n",
>> +			is_on ? "en" : "ds", cmd_addr, bits);
>>  
>>  	spi_message_init(&m);
>>  	memset(&t, 0, sizeof(t));
>> @@ -247,6 +287,13 @@ static int eeprom_93xx46_eral(struct eeprom_93xx46_dev *edev)
>>  		bits = 9;
>>  	}
>>  
>> +	if (has_quirk_instruction_length(edev)) {
>> +		cmd_addr <<= 2;
>> +		bits += 2;
>> +	}
>> +
>> +	dev_dbg(&edev->spi->dev, "eral cmd 0x%04x, %d bits\n", cmd_addr, bits);
>> +
>>  	spi_message_init(&m);
>>  	memset(&t, 0, sizeof(t));
>>  
>> @@ -298,12 +345,15 @@ static DEVICE_ATTR(erase, S_IWUSR, NULL, eeprom_93xx46_store_erase);
>>  
>>  static const struct of_device_id eeprom_93xx46_of_table[] = {
>>  	{ .compatible = "eeprom-93xx46", },
>> +	{ .compatible = "atmel,at93c46d", .data = &atmel_at93c46d_data, },
> 
> Ok, got the second "atmel,at93c46d" here.

Yeah.

> 
>>  	{}
>>  };
>>  MODULE_DEVICE_TABLE(of, eeprom_93xx46_of_table);
>>  
>>  static int eeprom_93xx46_probe_dt(struct spi_device *spi)
>>  {
>> +	const struct of_device_id *of_id =
>> +		of_match_device(eeprom_93xx46_of_table, &spi->dev);
>>  	struct device_node *np = spi->dev.of_node;
>>  	struct eeprom_93xx46_platform_data *pd;
>>  	u32 tmp;
>> @@ -331,6 +381,12 @@ static int eeprom_93xx46_probe_dt(struct spi_device *spi)
>>  	if (of_property_read_bool(np, "read-only"))
>>  		pd->flags |= EE_READONLY;
>>  
>> +	if (of_id->data) {
>> +		const struct eeprom_93xx46_devtype_data *data = of_id->data;
>> +
>> +		pd->quirks = data->quirks;
>> +	}
>> +
>>  	spi->dev.platform_data = pd;
>>  
>>  	return 0;
>> diff --git a/include/linux/eeprom_93xx46.h b/include/linux/eeprom_93xx46.h
>> index 0679181..92fa4c3 100644
>> --- a/include/linux/eeprom_93xx46.h
>> +++ b/include/linux/eeprom_93xx46.h
>> @@ -9,6 +9,12 @@ struct eeprom_93xx46_platform_data {
>>  #define EE_ADDR16	0x02		/* 16 bit addr. cfg */
>>  #define EE_READONLY	0x08		/* forbid writing */
>>  
>> +	unsigned int	quirks;
>> +/* Single word read transfers only; no sequential read. */
>> +#define EEPROM_93XX46_QUIRK_SINGLE_WORD_READ		(1 << 0)
>> +/* Instructions such as EWEN are (addrlen + 2) in length. */
>> +#define EEPROM_93XX46_QUIRK_INSTRUCTION_LENGTH		(1 << 1)
>> +
>>  	/*
>>  	 * optional hooks to control additional logic
>>  	 * before and after spi transfer.
>>
> 
> Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>

Thanks again for all the reviews.

Best regards,
- -Cory


- -- 
Cory Tusar
Principal
PID 1 Solutions, Inc.


"There are two ways of constructing a software design.  One way is to
 make it so simple that there are obviously no deficiencies, and the
 other way is to make it so complicated that there are no obvious
 deficiencies."  --Sir Charles Anthony Richard Hoare

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iEYEARECAAYFAlaMiBAACgkQHT1tsfGwHJ/MLQCgju12m/YJl2wOEz/g9WqqMF0J
22gAn37PF8DaSfoUmCF0toTjPdVhk5Ks
=/kv3
-----END PGP SIGNATURE-----

Re: [PATCH v4 5/5] misc: eeprom_93xx46: Add support for a GPIO 'select' line.

[Cory Tusar]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 12/21/2015 07:30 PM, Vladimir Zapolskiy wrote:
> Hi Cory,
> 
> On 10.12.2015 06:00, Cory Tusar wrote:
>> This commit adds support to the eeprom_93x46 driver allowing a GPIO line
>> to function as a 'select' or 'enable' signal prior to accessing the
>> EEPROM.
>>
>> Signed-off-by: Cory Tusar <cory.tusar@pid1solutions.com>
>> Tested-by: Chris Healy <chris.healy@zii.aero>
>> ---
>>  drivers/misc/eeprom/eeprom_93xx46.c | 35 +++++++++++++++++++++++++++++++++++
>>  include/linux/eeprom_93xx46.h       |  3 +++
>>  2 files changed, 38 insertions(+)
>>
>> diff --git a/drivers/misc/eeprom/eeprom_93xx46.c b/drivers/misc/eeprom/eeprom_93xx46.c
>> index d50bc17..d28fac2 100644
>> --- a/drivers/misc/eeprom/eeprom_93xx46.c
>> +++ b/drivers/misc/eeprom/eeprom_93xx46.c
>> @@ -10,11 +10,13 @@
>>  
>>  #include <linux/delay.h>
>>  #include <linux/device.h>
>> +#include <linux/gpio/consumer.h>
>>  #include <linux/kernel.h>
>>  #include <linux/module.h>
>>  #include <linux/mutex.h>
>>  #include <linux/of.h>
>>  #include <linux/of_device.h>
>> +#include <linux/of_gpio.h>
>>  #include <linux/slab.h>
>>  #include <linux/spi/spi.h>
>>  #include <linux/sysfs.h>
>> @@ -343,6 +345,20 @@ static ssize_t eeprom_93xx46_store_erase(struct device *dev,
>>  }
>>  static DEVICE_ATTR(erase, S_IWUSR, NULL, eeprom_93xx46_store_erase);
>>  
>> +static void select_assert(void *context)
>> +{
>> +	struct eeprom_93xx46_dev *edev = context;
>> +
>> +	gpiod_set_value_cansleep(edev->pdata->select, 1);
>> +}
>> +
>> +static void select_deassert(void *context)
>> +{
>> +	struct eeprom_93xx46_dev *edev = context;
>> +
>> +	gpiod_set_value_cansleep(edev->pdata->select, 0);
>> +}
>> +
>>  static const struct of_device_id eeprom_93xx46_of_table[] = {
>>  	{ .compatible = "eeprom-93xx46", },
>>  	{ .compatible = "atmel,at93c46d", .data = &atmel_at93c46d_data, },
>> @@ -357,6 +373,8 @@ static int eeprom_93xx46_probe_dt(struct spi_device *spi)
>>  	struct device_node *np = spi->dev.of_node;
>>  	struct eeprom_93xx46_platform_data *pd;
>>  	u32 tmp;
>> +	int gpio;
>> +	enum of_gpio_flags of_flags;
>>  	int ret;
>>  
>>  	pd = devm_kzalloc(&spi->dev, sizeof(*pd), GFP_KERNEL);
>> @@ -381,6 +399,23 @@ static int eeprom_93xx46_probe_dt(struct spi_device *spi)
>>  	if (of_property_read_bool(np, "read-only"))
>>  		pd->flags |= EE_READONLY;
>>  
>> +	gpio = of_get_named_gpio_flags(np, "select-gpios", 0, &of_flags);
>> +	if (gpio_is_valid(gpio)) {
>> +		unsigned long flags =
>> +			of_flags == OF_GPIO_ACTIVE_LOW ? GPIOF_ACTIVE_LOW : 0;
>> +
>> +		ret = devm_gpio_request_one(&spi->dev, gpio, flags,
>> +					    "eeprom_93xx46_select");
>> +		if (ret)
>> +			return ret;
>> +
>> +		pd->select = gpio_to_desc(gpio);
>> +		pd->prepare = select_assert;
>> +		pd->finish = select_deassert;
>> +
>> +		gpiod_direction_output(pd->select, 0);
>> +	}
>> +
>>  	if (of_id->data) {
>>  		const struct eeprom_93xx46_devtype_data *data = of_id->data;
>>  
>> diff --git a/include/linux/eeprom_93xx46.h b/include/linux/eeprom_93xx46.h
>> index 92fa4c3..03f3435 100644
>> --- a/include/linux/eeprom_93xx46.h
>> +++ b/include/linux/eeprom_93xx46.h
>> @@ -3,6 +3,8 @@
>>   * platform description for 93xx46 EEPROMs.
>>   */
>>  
>> +#include <linux/of_gpio.h>
>> +
> 
> You need just "struct gpio_desc;" instead of the include here, please fix.

Hi Vladimir,

I'll fix for v5.

> 
>>  struct eeprom_93xx46_platform_data {
>>  	unsigned char	flags;
>>  #define EE_ADDR8	0x01		/*  8 bit addr. cfg */
>> @@ -21,4 +23,5 @@ struct eeprom_93xx46_platform_data {
>>  	 */
>>  	void (*prepare)(void *);
>>  	void (*finish)(void *);
>> +	struct gpio_desc *select;
>>  };
>>
> 
> Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>

Thanks and best regards,
- -Cory


- -- 
Cory Tusar
Principal
PID 1 Solutions, Inc.


"There are two ways of constructing a software design.  One way is to
 make it so simple that there are obviously no deficiencies, and the
 other way is to make it so complicated that there are no obvious
 deficiencies."  --Sir Charles Anthony Richard Hoare

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iEYEARECAAYFAlaMiFUACgkQHT1tsfGwHJ8PvwCgmCxWi9TiL+ZXCI6IYZgaGZbT
vHsAn3FOyuJ5tAqb6JywtDY6piiQGywv
=2mcd
-----END PGP SIGNATURE-----

Re: [PATCH v4 5/5] misc: eeprom_93xx46: Add support for a GPIO 'select' line.

[Cory Tusar]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 12/21/2015 07:30 PM, Vladimir Zapolskiy wrote:
> Hi Cory,
> 
> On 10.12.2015 06:00, Cory Tusar wrote:
>> This commit adds support to the eeprom_93x46 driver allowing a GPIO line
>> to function as a 'select' or 'enable' signal prior to accessing the
>> EEPROM.
>>
>> Signed-off-by: Cory Tusar <cory.tusar-J6Z/VSE8EyIAspv4Qr0y0gC/G2K4zDHf@public.gmane.org>
>> Tested-by: Chris Healy <chris.healy-c8ZVq/bFV1I@public.gmane.org>
>> ---
>>  drivers/misc/eeprom/eeprom_93xx46.c | 35 +++++++++++++++++++++++++++++++++++
>>  include/linux/eeprom_93xx46.h       |  3 +++
>>  2 files changed, 38 insertions(+)
>>
>> diff --git a/drivers/misc/eeprom/eeprom_93xx46.c b/drivers/misc/eeprom/eeprom_93xx46.c
>> index d50bc17..d28fac2 100644
>> --- a/drivers/misc/eeprom/eeprom_93xx46.c
>> +++ b/drivers/misc/eeprom/eeprom_93xx46.c
>> @@ -10,11 +10,13 @@
>>  
>>  #include <linux/delay.h>
>>  #include <linux/device.h>
>> +#include <linux/gpio/consumer.h>
>>  #include <linux/kernel.h>
>>  #include <linux/module.h>
>>  #include <linux/mutex.h>
>>  #include <linux/of.h>
>>  #include <linux/of_device.h>
>> +#include <linux/of_gpio.h>
>>  #include <linux/slab.h>
>>  #include <linux/spi/spi.h>
>>  #include <linux/sysfs.h>
>> @@ -343,6 +345,20 @@ static ssize_t eeprom_93xx46_store_erase(struct device *dev,
>>  }
>>  static DEVICE_ATTR(erase, S_IWUSR, NULL, eeprom_93xx46_store_erase);
>>  
>> +static void select_assert(void *context)
>> +{
>> +	struct eeprom_93xx46_dev *edev = context;
>> +
>> +	gpiod_set_value_cansleep(edev->pdata->select, 1);
>> +}
>> +
>> +static void select_deassert(void *context)
>> +{
>> +	struct eeprom_93xx46_dev *edev = context;
>> +
>> +	gpiod_set_value_cansleep(edev->pdata->select, 0);
>> +}
>> +
>>  static const struct of_device_id eeprom_93xx46_of_table[] = {
>>  	{ .compatible = "eeprom-93xx46", },
>>  	{ .compatible = "atmel,at93c46d", .data = &atmel_at93c46d_data, },
>> @@ -357,6 +373,8 @@ static int eeprom_93xx46_probe_dt(struct spi_device *spi)
>>  	struct device_node *np = spi->dev.of_node;
>>  	struct eeprom_93xx46_platform_data *pd;
>>  	u32 tmp;
>> +	int gpio;
>> +	enum of_gpio_flags of_flags;
>>  	int ret;
>>  
>>  	pd = devm_kzalloc(&spi->dev, sizeof(*pd), GFP_KERNEL);
>> @@ -381,6 +399,23 @@ static int eeprom_93xx46_probe_dt(struct spi_device *spi)
>>  	if (of_property_read_bool(np, "read-only"))
>>  		pd->flags |= EE_READONLY;
>>  
>> +	gpio = of_get_named_gpio_flags(np, "select-gpios", 0, &of_flags);
>> +	if (gpio_is_valid(gpio)) {
>> +		unsigned long flags =
>> +			of_flags == OF_GPIO_ACTIVE_LOW ? GPIOF_ACTIVE_LOW : 0;
>> +
>> +		ret = devm_gpio_request_one(&spi->dev, gpio, flags,
>> +					    "eeprom_93xx46_select");
>> +		if (ret)
>> +			return ret;
>> +
>> +		pd->select = gpio_to_desc(gpio);
>> +		pd->prepare = select_assert;
>> +		pd->finish = select_deassert;
>> +
>> +		gpiod_direction_output(pd->select, 0);
>> +	}
>> +
>>  	if (of_id->data) {
>>  		const struct eeprom_93xx46_devtype_data *data = of_id->data;
>>  
>> diff --git a/include/linux/eeprom_93xx46.h b/include/linux/eeprom_93xx46.h
>> index 92fa4c3..03f3435 100644
>> --- a/include/linux/eeprom_93xx46.h
>> +++ b/include/linux/eeprom_93xx46.h
>> @@ -3,6 +3,8 @@
>>   * platform description for 93xx46 EEPROMs.
>>   */
>>  
>> +#include <linux/of_gpio.h>
>> +
> 
> You need just "struct gpio_desc;" instead of the include here, please fix.

Hi Vladimir,

I'll fix for v5.

> 
>>  struct eeprom_93xx46_platform_data {
>>  	unsigned char	flags;
>>  #define EE_ADDR8	0x01		/*  8 bit addr. cfg */
>> @@ -21,4 +23,5 @@ struct eeprom_93xx46_platform_data {
>>  	 */
>>  	void (*prepare)(void *);
>>  	void (*finish)(void *);
>> +	struct gpio_desc *select;
>>  };
>>
> 
> Reviewed-by: Vladimir Zapolskiy <vz-ChpfBGZJDbMAvxtiuMwx3w@public.gmane.org>

Thanks and best regards,
- -Cory


- -- 
Cory Tusar
Principal
PID 1 Solutions, Inc.


"There are two ways of constructing a software design.  One way is to
 make it so simple that there are obviously no deficiencies, and the
 other way is to make it so complicated that there are no obvious
 deficiencies."  --Sir Charles Anthony Richard Hoare

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iEYEARECAAYFAlaMiFUACgkQHT1tsfGwHJ8PvwCgmCxWi9TiL+ZXCI6IYZgaGZbT
vHsAn3FOyuJ5tAqb6JywtDY6piiQGywv
=2mcd
-----END PGP SIGNATURE-----
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html