[RESEND PATCH v2 0/9] eeprom: at24: at24cs series serial number read

[RESEND PATCH v2 0/9] eeprom: at24: at24cs series serial number read

[Bartosz Golaszewski]

Chips from the at24cs EEPROM series have an additional read-only memory area
containing a factory pre-programmed serial number. In order to access it, a
dummy write must be executed before reading the serial number bytes.

This series adds support for reading the serial number through a sysfs
attribute.

While we're at it: some of the patches contain readability tweaks and code
organization fixes.

Tested with at24cs64 and at24cs02 chips (for both 16 and 8 bit address
pointers).

(rebased against 4.4-rc3)

v2:
- protect the serial number read with a mutex

v1: https://lkml.org/lkml/2015/10/20/162

Bartosz Golaszewski (9):
  eeprom: at24: platform_data: use BIT() macro
  eeprom: at24: new flag in platform_data
  eeprom: at24: tie up an additional address for at24cs series
  eeprom: at24: support reading of the serial number
  eeprom: at24: export the serial number through sysfs
  eeprom: at24: improve the device_id table readability
  eeprom: at24: add the at24cs series to the list of supported devices
  eeprom: at24: remove a reduntant if
  eeprom: at24: readability tweaks

 drivers/misc/eeprom/at24.c         | 187 +++++++++++++++++++++++++++++++------
 include/linux/platform_data/at24.h |   9 +-
 2 files changed, 166 insertions(+), 30 deletions(-)

-- 
2.1.4

[RESEND PATCH v2 1/9] eeprom: at24: platform_data: use BIT() macro

[Bartosz Golaszewski]

Use BIT() macro to replace the 0xXX constants in platform_data flags
definitions.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 include/linux/platform_data/at24.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/platform_data/at24.h b/include/linux/platform_data/at24.h
index c42aa89..8d90f52 100644
--- a/include/linux/platform_data/at24.h
+++ b/include/linux/platform_data/at24.h
@@ -43,10 +43,10 @@ struct at24_platform_data {
 	u32		byte_len;		/* size (sum of all addr) */
 	u16		page_size;		/* for writes */
 	u8		flags;
-#define AT24_FLAG_ADDR16	0x80	/* address pointer is 16 bit */
-#define AT24_FLAG_READONLY	0x40	/* sysfs-entry will be read-only */
-#define AT24_FLAG_IRUGO		0x20	/* sysfs-entry will be world-readable */
-#define AT24_FLAG_TAKE8ADDR	0x10	/* take always 8 addresses (24c00) */
+#define AT24_FLAG_ADDR16	BIT(7)	/* address pointer is 16 bit */
+#define AT24_FLAG_READONLY	BIT(6)	/* sysfs-entry will be read-only */
+#define AT24_FLAG_IRUGO		BIT(5)	/* sysfs-entry will be world-readable */
+#define AT24_FLAG_TAKE8ADDR	BIT(4)	/* take always 8 addresses (24c00) */
 
 	void		(*setup)(struct memory_accessor *, void *context);
 	void		*context;
-- 
2.1.4

[RESEND PATCH v2 2/9] eeprom: at24: new flag in platform_data

[Bartosz Golaszewski]

In preparation for supporting the at24cs EEPROM series add a new flag to
platform data. When set, it should tell the driver that the chip has an
additional read-only memory area that holds a factory pre-programmed serial
number.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 include/linux/platform_data/at24.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/platform_data/at24.h b/include/linux/platform_data/at24.h
index 8d90f52..5686f91 100644
--- a/include/linux/platform_data/at24.h
+++ b/include/linux/platform_data/at24.h
@@ -47,6 +47,7 @@ struct at24_platform_data {
 #define AT24_FLAG_READONLY	BIT(6)	/* sysfs-entry will be read-only */
 #define AT24_FLAG_IRUGO		BIT(5)	/* sysfs-entry will be world-readable */
 #define AT24_FLAG_TAKE8ADDR	BIT(4)	/* take always 8 addresses (24c00) */
+#define AT24_FLAG_SERIAL	BIT(3)	/* factory-programmed serial number */
 
 	void		(*setup)(struct memory_accessor *, void *context);
 	void		*context;
-- 
2.1.4

[RESEND PATCH v2 3/9] eeprom: at24: tie up an additional address for at24cs series

[Bartosz Golaszewski]

The at24cs series EEPROM chips have an additional read-only memory area,
that is visible on a different i2c slave address. Tie it up with a dummy
device.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/misc/eeprom/at24.c | 30 ++++++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 5d7c090..08cc327 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -103,6 +103,8 @@ MODULE_PARM_DESC(write_timeout, "Time (in ms) to try writes (default 25)");
 
 #define AT24_BITMASK(x) (BIT(x) - 1)
 
+#define AT24CS_SERIAL_ADDR(addr) (addr + 0x08)
+
 /* create non-zero magic value for given eeprom parameters */
 #define AT24_DEVICE_MAGIC(_len, _flags) 		\
 	((1 << AT24_SIZE_FLAGS | (_flags)) 		\
@@ -558,6 +560,8 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
 
 	if (chip.flags & AT24_FLAG_TAKE8ADDR)
 		num_addresses = 8;
+	else if (chip.flags & AT24_FLAG_SERIAL)
+		num_addresses = 2;
 	else
 		num_addresses =	DIV_ROUND_UP(chip.byte_len,
 			(chip.flags & AT24_FLAG_ADDR16) ? 65536 : 256);
@@ -616,12 +620,30 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	at24->client[0] = client;
 
 	/* use dummy devices for multiple-address chips */
-	for (i = 1; i < num_addresses; i++) {
-		at24->client[i] = i2c_new_dummy(client->adapter,
+	if (at24->chip.flags & AT24_FLAG_TAKE8ADDR) {
+		for (i = 1; i < num_addresses; i++) {
+			at24->client[i] = i2c_new_dummy(client->adapter,
+							client->addr + i);
+			if (!at24->client[i]) {
+				dev_err(&client->dev,
+					"address 0x%02x unavailable\n",
 					client->addr + i);
-		if (!at24->client[i]) {
+				err = -EADDRINUSE;
+				goto err_clients;
+			}
+		}
+	}
+
+	/*
+	 * at24cs series tie up an additional address for the memory area
+	 * contining the serial number
+	 */
+	if (at24->chip.flags & AT24_FLAG_SERIAL) {
+		at24->client[1] = i2c_new_dummy(client->adapter,
+					AT24CS_SERIAL_ADDR(client->addr));
+		if (!at24->client[1]) {
 			dev_err(&client->dev, "address 0x%02x unavailable\n",
-					client->addr + i);
+				AT24CS_SERIAL_ADDR(client->addr));
 			err = -EADDRINUSE;
 			goto err_clients;
 		}
-- 
2.1.4

[RESEND PATCH v2 4/9] eeprom: at24: support reading of the serial number

[Bartosz Golaszewski]

The at24cs series EEPROM chips have an additional read-only memory area
containing a factory pre-programmed serial number. In order to access
it, one has to perform a dummy write before reading the serial number
bytes.

Add a function that allows to access the serial number.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/misc/eeprom/at24.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 08cc327..6182f47 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -163,6 +163,77 @@ static struct i2c_client *at24_translate_offset(struct at24_data *at24,
 	return at24->client[i];
 }
 
+static int __attribute__((unused)) at24cs_eeprom_serial_read(
+						struct at24_data *at24,
+						char *buf, unsigned offset,
+						size_t count)
+{
+	unsigned long timeout, read_time;
+	struct i2c_client *client;
+	struct i2c_msg msg[2];
+	u8 addrbuf[2];
+	int status;
+
+	mutex_lock(&at24->lock);
+
+	client = at24->client[1];
+
+	memset(msg, 0, sizeof(msg));
+	msg[0].addr = client->addr;
+	msg[0].buf = addrbuf;
+
+	/*
+	 * The address pointer of the device is shared between the regular
+	 * EEPROM array and the serial number block. The dummy write (part of
+	 * the sequential read protocol) ensures the address pointer is reset
+	 * to the desired position.
+	 */
+	if (at24->chip.flags & AT24_FLAG_ADDR16) {
+		/*
+		 * For 16 bit address pointers, the word address must contain
+		 * a '10' sequence in bits 11 and 10 regardless of the
+		 * intended position of the address pointer.
+		 */
+		addrbuf[0] = 0x08;
+		addrbuf[1] = offset;
+		msg[0].len = 2;
+	} else {
+		/*
+		 * Otherwise the word address must begin with a '10' sequence,
+		 * regardless of the intended address.
+		 */
+		addrbuf[0] = 0x80 + offset;
+		msg[0].len = 1;
+	}
+
+	msg[1].addr = client->addr;
+	msg[1].flags = I2C_M_RD;
+	msg[1].buf = buf;
+	msg[1].len = count;
+
+	/*
+	 * Reads fail if the previous write didn't complete yet. We may
+	 * loop a few times until this one succeeds, waiting at least
+	 * long enough for one entire page write to work.
+	 */
+	timeout = jiffies + msecs_to_jiffies(write_timeout);
+	do {
+		read_time = jiffies;
+		status = i2c_transfer(client->adapter, msg, 2);
+		if (status == 2) {
+			mutex_unlock(&at24->lock);
+			return count;
+		}
+
+		/* REVISIT: at HZ=100, this is sloooow */
+		msleep(1);
+	} while (time_before(read_time, timeout));
+
+	mutex_unlock(&at24->lock);
+
+	return -ETIMEDOUT;
+}
+
 static ssize_t at24_eeprom_read(struct at24_data *at24, char *buf,
 		unsigned offset, size_t count)
 {
-- 
2.1.4

[RESEND PATCH v2 5/9] eeprom: at24: export the serial number through sysfs

[Bartosz Golaszewski]

The at24 driver is now capable of reading the serial number from at24cs
EEPROM chips. Export the serial number through sysfs.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/misc/eeprom/at24.c | 44 ++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 40 insertions(+), 4 deletions(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 6182f47..65fca1e 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -65,6 +65,7 @@ struct at24_data {
 	 */
 	struct mutex lock;
 	struct bin_attribute bin;
+	struct bin_attribute *bin_serial;
 
 	u8 *writebuf;
 	unsigned write_max;
@@ -103,6 +104,7 @@ MODULE_PARM_DESC(write_timeout, "Time (in ms) to try writes (default 25)");
 
 #define AT24_BITMASK(x) (BIT(x) - 1)
 
+#define AT24CS_SERIAL_SIZE 16
 #define AT24CS_SERIAL_ADDR(addr) (addr + 0x08)
 
 /* create non-zero magic value for given eeprom parameters */
@@ -163,10 +165,8 @@ static struct i2c_client *at24_translate_offset(struct at24_data *at24,
 	return at24->client[i];
 }
 
-static int __attribute__((unused)) at24cs_eeprom_serial_read(
-						struct at24_data *at24,
-						char *buf, unsigned offset,
-						size_t count)
+static int at24cs_eeprom_serial_read(struct at24_data *at24, char *buf,
+				     unsigned offset, size_t count)
 {
 	unsigned long timeout, read_time;
 	struct i2c_client *client;
@@ -234,6 +234,16 @@ static int __attribute__((unused)) at24cs_eeprom_serial_read(
 	return -ETIMEDOUT;
 }
 
+static ssize_t at24cs_bin_serial_read(struct file *filp, struct kobject *kobj,
+				      struct bin_attribute *attr,
+				      char *buf, loff_t off, size_t count)
+{
+	struct at24_data *at24;
+
+	at24 = dev_get_drvdata(container_of(kobj, struct device, kobj));
+	return at24cs_eeprom_serial_read(at24, buf, off, count);
+}
+
 static ssize_t at24_eeprom_read(struct at24_data *at24, char *buf,
 		unsigned offset, size_t count)
 {
@@ -658,6 +668,30 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	at24->bin.read = at24_bin_read;
 	at24->bin.size = chip.byte_len;
 
+	if (at24->chip.flags & AT24_FLAG_SERIAL) {
+		/*
+		 * For EEPROMs containing the serial number export an
+		 * additional file allowing allowing convenvient access
+		 * to it from user-space.
+		 */
+		at24->bin_serial = devm_kzalloc(&client->dev,
+						sizeof(struct bin_attribute),
+						GFP_KERNEL);
+		if (!at24->bin_serial)
+			return -ENOMEM;
+
+		sysfs_bin_attr_init(at24->bin_serial);
+		at24->bin_serial->attr.name = "serial";
+		at24->bin_serial->attr.mode = S_IRUSR;
+		at24->bin_serial->read = at24cs_bin_serial_read;
+		at24->bin_serial->size = AT24CS_SERIAL_SIZE;
+
+		err = sysfs_create_bin_file(&client->dev.kobj,
+					    at24->bin_serial);
+		if (err)
+			goto err_clients;
+	}
+
 	at24->macc.read = at24_macc_read;
 
 	writable = !(chip.flags & AT24_FLAG_READONLY);
@@ -757,6 +791,8 @@ static int at24_remove(struct i2c_client *client)
 
 	at24 = i2c_get_clientdata(client);
 	sysfs_remove_bin_file(&client->dev.kobj, &at24->bin);
+	if (at24->bin_serial)
+		sysfs_remove_bin_file(&client->dev.kobj, at24->bin_serial);
 
 	for (i = 1; i < at24->num_addresses; i++)
 		i2c_unregister_device(at24->client[i]);
-- 
2.1.4

[RESEND PATCH v2 6/9] eeprom: at24: improve the device_id table readability

[Bartosz Golaszewski]

Improve the readability of the device table by separating columns with
tabs.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/misc/eeprom/at24.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 65fca1e..d474b6d 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -114,23 +114,23 @@ MODULE_PARM_DESC(write_timeout, "Time (in ms) to try writes (default 25)");
 
 static const struct i2c_device_id at24_ids[] = {
 	/* needs 8 addresses as A0-A2 are ignored */
-	{ "24c00", AT24_DEVICE_MAGIC(128 / 8, AT24_FLAG_TAKE8ADDR) },
+	{ "24c00",	AT24_DEVICE_MAGIC(128 / 8,	AT24_FLAG_TAKE8ADDR) },
 	/* old variants can't be handled with this generic entry! */
-	{ "24c01", AT24_DEVICE_MAGIC(1024 / 8, 0) },
-	{ "24c02", AT24_DEVICE_MAGIC(2048 / 8, 0) },
+	{ "24c01",	AT24_DEVICE_MAGIC(1024 / 8,	0) },
+	{ "24c02",	AT24_DEVICE_MAGIC(2048 / 8,	0) },
 	/* spd is a 24c02 in memory DIMMs */
-	{ "spd", AT24_DEVICE_MAGIC(2048 / 8,
-		AT24_FLAG_READONLY | AT24_FLAG_IRUGO) },
-	{ "24c04", AT24_DEVICE_MAGIC(4096 / 8, 0) },
+	{ "spd",	AT24_DEVICE_MAGIC(2048 / 8,
+				AT24_FLAG_READONLY | AT24_FLAG_IRUGO) },
+	{ "24c04",	AT24_DEVICE_MAGIC(4096 / 8,	0) },
 	/* 24rf08 quirk is handled at i2c-core */
-	{ "24c08", AT24_DEVICE_MAGIC(8192 / 8, 0) },
-	{ "24c16", AT24_DEVICE_MAGIC(16384 / 8, 0) },
-	{ "24c32", AT24_DEVICE_MAGIC(32768 / 8, AT24_FLAG_ADDR16) },
-	{ "24c64", AT24_DEVICE_MAGIC(65536 / 8, AT24_FLAG_ADDR16) },
-	{ "24c128", AT24_DEVICE_MAGIC(131072 / 8, AT24_FLAG_ADDR16) },
-	{ "24c256", AT24_DEVICE_MAGIC(262144 / 8, AT24_FLAG_ADDR16) },
-	{ "24c512", AT24_DEVICE_MAGIC(524288 / 8, AT24_FLAG_ADDR16) },
-	{ "24c1024", AT24_DEVICE_MAGIC(1048576 / 8, AT24_FLAG_ADDR16) },
+	{ "24c08",	AT24_DEVICE_MAGIC(8192 / 8,	0) },
+	{ "24c16",	AT24_DEVICE_MAGIC(16384 / 8,	0) },
+	{ "24c32",	AT24_DEVICE_MAGIC(32768 / 8,	AT24_FLAG_ADDR16) },
+	{ "24c64",	AT24_DEVICE_MAGIC(65536 / 8,	AT24_FLAG_ADDR16) },
+	{ "24c128",	AT24_DEVICE_MAGIC(131072 / 8,	AT24_FLAG_ADDR16) },
+	{ "24c256",	AT24_DEVICE_MAGIC(262144 / 8,	AT24_FLAG_ADDR16) },
+	{ "24c512",	AT24_DEVICE_MAGIC(524288 / 8,	AT24_FLAG_ADDR16) },
+	{ "24c1024",	AT24_DEVICE_MAGIC(1048576 / 8,	AT24_FLAG_ADDR16) },
 	{ "at24", 0 },
 	{ /* END OF LIST */ }
 };
-- 
2.1.4

[RESEND PATCH v2 7/9] eeprom: at24: add the at24cs series to the list of supported devices

[Bartosz Golaszewski]

The infrastructure for reading of the factory-programmed serial number
for at24cs EEPROM series is now in place. Add the chips that are actually
equipped with the serial number memory area to the list of supported
devices.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/misc/eeprom/at24.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index d474b6d..6e28b02 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -117,16 +117,25 @@ static const struct i2c_device_id at24_ids[] = {
 	{ "24c00",	AT24_DEVICE_MAGIC(128 / 8,	AT24_FLAG_TAKE8ADDR) },
 	/* old variants can't be handled with this generic entry! */
 	{ "24c01",	AT24_DEVICE_MAGIC(1024 / 8,	0) },
+	{ "24cs01",	AT24_DEVICE_MAGIC(1024 / 8,	AT24_FLAG_SERIAL) },
 	{ "24c02",	AT24_DEVICE_MAGIC(2048 / 8,	0) },
+	{ "24cs02",	AT24_DEVICE_MAGIC(2048 / 8,	AT24_FLAG_SERIAL) },
 	/* spd is a 24c02 in memory DIMMs */
 	{ "spd",	AT24_DEVICE_MAGIC(2048 / 8,
 				AT24_FLAG_READONLY | AT24_FLAG_IRUGO) },
 	{ "24c04",	AT24_DEVICE_MAGIC(4096 / 8,	0) },
+	{ "24cs04",	AT24_DEVICE_MAGIC(4096 / 8,	AT24_FLAG_SERIAL) },
 	/* 24rf08 quirk is handled at i2c-core */
 	{ "24c08",	AT24_DEVICE_MAGIC(8192 / 8,	0) },
+	{ "24cs08",	AT24_DEVICE_MAGIC(8192 / 8,	AT24_FLAG_SERIAL) },
 	{ "24c16",	AT24_DEVICE_MAGIC(16384 / 8,	0) },
+	{ "24cs16",	AT24_DEVICE_MAGIC(16384 / 8,	AT24_FLAG_SERIAL) },
 	{ "24c32",	AT24_DEVICE_MAGIC(32768 / 8,	AT24_FLAG_ADDR16) },
+	{ "24cs32",	AT24_DEVICE_MAGIC(32768 / 8,
+				AT24_FLAG_ADDR16 | AT24_FLAG_SERIAL) },
 	{ "24c64",	AT24_DEVICE_MAGIC(65536 / 8,	AT24_FLAG_ADDR16) },
+	{ "24cs64",	AT24_DEVICE_MAGIC(65536 / 8,
+				AT24_FLAG_ADDR16 | AT24_FLAG_SERIAL) },
 	{ "24c128",	AT24_DEVICE_MAGIC(131072 / 8,	AT24_FLAG_ADDR16) },
 	{ "24c256",	AT24_DEVICE_MAGIC(262144 / 8,	AT24_FLAG_ADDR16) },
 	{ "24c512",	AT24_DEVICE_MAGIC(524288 / 8,	AT24_FLAG_ADDR16) },
-- 
2.1.4

[RESEND PATCH v2 8/9] eeprom: at24: remove a reduntant if

[Bartosz Golaszewski]

It seems as if the second check for I2C_FUNC_I2C functionality had been
introduced accidentally during a merge. Tt's reduntant, so remove it.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/misc/eeprom/at24.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 6e28b02..1288193 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -634,10 +634,7 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		} else {
 			return -EPFNOSUPPORT;
 		}
-	}
 
-	/* Use I2C operations unless we're stuck with SMBus extensions. */
-	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
 		if (i2c_check_functionality(client->adapter,
 				I2C_FUNC_SMBUS_WRITE_I2C_BLOCK)) {
 			use_smbus_write = I2C_SMBUS_I2C_BLOCK_DATA;
-- 
2.1.4

[RESEND PATCH v2 9/9] eeprom: at24: readability tweaks

[Bartosz Golaszewski]

Move the macro definitions above the struct definitions and add some
tabs for better readability.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 drivers/misc/eeprom/at24.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 1288193..3238bf6 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -53,6 +53,14 @@
  * which won't work on pure SMBus systems.
  */
 
+#define AT24_SIZE_BYTELEN		5
+#define AT24_SIZE_FLAGS			8
+
+#define AT24_BITMASK(x)			(BIT(x) - 1)
+
+#define AT24CS_SERIAL_SIZE		16
+#define AT24CS_SERIAL_ADDR(addr)	(addr + 0x08)
+
 struct at24_data {
 	struct at24_platform_data chip;
 	struct memory_accessor macc;
@@ -99,14 +107,6 @@ static unsigned write_timeout = 25;
 module_param(write_timeout, uint, 0);
 MODULE_PARM_DESC(write_timeout, "Time (in ms) to try writes (default 25)");
 
-#define AT24_SIZE_BYTELEN 5
-#define AT24_SIZE_FLAGS 8
-
-#define AT24_BITMASK(x) (BIT(x) - 1)
-
-#define AT24CS_SERIAL_SIZE 16
-#define AT24CS_SERIAL_ADDR(addr) (addr + 0x08)
-
 /* create non-zero magic value for given eeprom parameters */
 #define AT24_DEVICE_MAGIC(_len, _flags) 		\
 	((1 << AT24_SIZE_FLAGS | (_flags)) 		\
-- 
2.1.4

Re: [RESEND PATCH v2 0/9] eeprom: at24: at24cs series serial number read

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 458 bytes --]

On Wed, Dec 02, 2015 at 11:25:17AM +0100, Bartosz Golaszewski wrote:
> Chips from the at24cs EEPROM series have an additional read-only memory area
> containing a factory pre-programmed serial number. In order to access it, a
> dummy write must be executed before reading the serial number bytes.

Can't you instantiate a read-only EEPROM on this second address? Or a
seperate driver attaching to this address? What is the advantage of
having this in at24?


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RESEND PATCH v2 0/9] eeprom: at24: at24cs series serial number read

[Bartosz Golaszewski]

2015-12-11 13:08 GMT+01:00 Wolfram Sang <wsa@the-dreams.de>:
> On Wed, Dec 02, 2015 at 11:25:17AM +0100, Bartosz Golaszewski wrote:
>> Chips from the at24cs EEPROM series have an additional read-only memory area
>> containing a factory pre-programmed serial number. In order to access it, a
>> dummy write must be executed before reading the serial number bytes.
>
> Can't you instantiate a read-only EEPROM on this second address? Or a
> seperate driver attaching to this address? What is the advantage of
> having this in at24?
>

The regular memory area and serial number read-only block share the
internal address pointer. We must ensure that there's no race
conditions between normal EEPROM reads/writes and serial number reads.

Best regards,
Bartosz Golaszewski

Re: [RESEND PATCH v2 0/9] eeprom: at24: at24cs series serial number read

[Bartosz Golaszewski]

2015-12-11 14:55 GMT+01:00 Bartosz Golaszewski <bgolaszewski@baylibre.com>:
> 2015-12-11 13:08 GMT+01:00 Wolfram Sang <wsa@the-dreams.de>:
>> On Wed, Dec 02, 2015 at 11:25:17AM +0100, Bartosz Golaszewski wrote:
>>> Chips from the at24cs EEPROM series have an additional read-only memory area
>>> containing a factory pre-programmed serial number. In order to access it, a
>>> dummy write must be executed before reading the serial number bytes.
>>
>> Can't you instantiate a read-only EEPROM on this second address? Or a
>> seperate driver attaching to this address? What is the advantage of
>> having this in at24?
>>
>
> The regular memory area and serial number read-only block share the
> internal address pointer. We must ensure that there's no race
> conditions between normal EEPROM reads/writes and serial number reads.
>
> Best regards,
> Bartosz Golaszewski

Hi Wolfram,

any chance of getting it in for 4.5?

Best regards,
Bartosz Golaszewski

Re: [RESEND PATCH v2 0/9] eeprom: at24: at24cs series serial number read

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1201 bytes --]

On Fri, Dec 11, 2015 at 02:55:10PM +0100, Bartosz Golaszewski wrote:
> 2015-12-11 13:08 GMT+01:00 Wolfram Sang <wsa@the-dreams.de>:
> > On Wed, Dec 02, 2015 at 11:25:17AM +0100, Bartosz Golaszewski wrote:
> >> Chips from the at24cs EEPROM series have an additional read-only memory area
> >> containing a factory pre-programmed serial number. In order to access it, a
> >> dummy write must be executed before reading the serial number bytes.
> >
> > Can't you instantiate a read-only EEPROM on this second address? Or a
> > seperate driver attaching to this address? What is the advantage of
> > having this in at24?
> >
> 
> The regular memory area and serial number read-only block share the
> internal address pointer. We must ensure that there's no race
> conditions between normal EEPROM reads/writes and serial number reads.

I don't get it. Both, regular at24 reads and the serial read, setup the
pointer every time by using two messages, first write to set the
pointer, then read. The per-adapter lock makes sure those two messages
will not get interrupted. So, it looks to me that it would be OK if a
serial read access gets inbetween a eeprom read access. Am I wrong?


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RESEND PATCH v2 0/9] eeprom: at24: at24cs series serial number read

[Bartosz Golaszewski]

2016-01-02 21:50 GMT+01:00 Wolfram Sang <wsa@the-dreams.de>:
> On Fri, Dec 11, 2015 at 02:55:10PM +0100, Bartosz Golaszewski wrote:
>> 2015-12-11 13:08 GMT+01:00 Wolfram Sang <wsa@the-dreams.de>:
>> > On Wed, Dec 02, 2015 at 11:25:17AM +0100, Bartosz Golaszewski wrote:
>> >> Chips from the at24cs EEPROM series have an additional read-only memory area
>> >> containing a factory pre-programmed serial number. In order to access it, a
>> >> dummy write must be executed before reading the serial number bytes.
>> >
>> > Can't you instantiate a read-only EEPROM on this second address? Or a
>> > seperate driver attaching to this address? What is the advantage of
>> > having this in at24?
>> >
>>
>> The regular memory area and serial number read-only block share the
>> internal address pointer. We must ensure that there's no race
>> conditions between normal EEPROM reads/writes and serial number reads.
>
> I don't get it. Both, regular at24 reads and the serial read, setup the
> pointer every time by using two messages, first write to set the
> pointer, then read. The per-adapter lock makes sure those two messages
> will not get interrupted.

If that's correct, then is there any need to have an additional mutex
for at24_data?

> So, it looks to me that it would be OK if a
> serial read access gets inbetween a eeprom read access. Am I wrong?
>

In that case would the preferred method be to access the regular
memory area like before - by allocating, for example, a 24c02 device -
while allocating a second device - in that case 24cs02 - on the
corresponding serial number address would give the user access to the
serial number via the eeprom sysfs attribute (which for the latter
would be read-only and 16 bytes in size)?

Best regards,
Bartosz Golaszewski

Re: [RESEND PATCH v2 0/9] eeprom: at24: at24cs series serial number read

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 2241 bytes --]

On Mon, Jan 04, 2016 at 03:01:54PM +0100, Bartosz Golaszewski wrote:
> 2016-01-02 21:50 GMT+01:00 Wolfram Sang <wsa@the-dreams.de>:
> > On Fri, Dec 11, 2015 at 02:55:10PM +0100, Bartosz Golaszewski wrote:
> >> 2015-12-11 13:08 GMT+01:00 Wolfram Sang <wsa@the-dreams.de>:
> >> > On Wed, Dec 02, 2015 at 11:25:17AM +0100, Bartosz Golaszewski wrote:
> >> >> Chips from the at24cs EEPROM series have an additional read-only memory area
> >> >> containing a factory pre-programmed serial number. In order to access it, a
> >> >> dummy write must be executed before reading the serial number bytes.
> >> >
> >> > Can't you instantiate a read-only EEPROM on this second address? Or a
> >> > seperate driver attaching to this address? What is the advantage of
> >> > having this in at24?
> >> >
> >>
> >> The regular memory area and serial number read-only block share the
> >> internal address pointer. We must ensure that there's no race
> >> conditions between normal EEPROM reads/writes and serial number reads.
> >
> > I don't get it. Both, regular at24 reads and the serial read, setup the
> > pointer every time by using two messages, first write to set the
> > pointer, then read. The per-adapter lock makes sure those two messages
> > will not get interrupted.
> 
> If that's correct, then is there any need to have an additional mutex
> for at24_data?

I can't see a need, yes.

> In that case would the preferred method be to access the regular
> memory area like before - by allocating, for example, a 24c02 device -
> while allocating a second device - in that case 24cs02 - on the
> corresponding serial number address would give the user access to the
> serial number via the eeprom sysfs attribute (which for the latter
> would be read-only and 16 bytes in size)?

Yes, a seperate driver for the second address is what I meant to suggest
in the above paragraph. Only that the data should probably be exported
via the NVMEM framework, not directly via sysfs. We have patches pending
doing that for at24.

What happens if you assign another at24 instance (read-only) to the
second address? I mean, there is not only the serial number, but also a
MAC address IIRC.

Regards,

   Wolfram


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RESEND PATCH v2 0/9] eeprom: at24: at24cs series serial number read

[Bartosz Golaszewski]

2016-01-05 19:58 GMT+01:00 Wolfram Sang <wsa@the-dreams.de>:
> On Mon, Jan 04, 2016 at 03:01:54PM +0100, Bartosz Golaszewski wrote:
>> 2016-01-02 21:50 GMT+01:00 Wolfram Sang <wsa@the-dreams.de>:
>> > On Fri, Dec 11, 2015 at 02:55:10PM +0100, Bartosz Golaszewski wrote:
>> >> 2015-12-11 13:08 GMT+01:00 Wolfram Sang <wsa@the-dreams.de>:
>> >> > On Wed, Dec 02, 2015 at 11:25:17AM +0100, Bartosz Golaszewski wrote:
>> >> >> Chips from the at24cs EEPROM series have an additional read-only memory area
>> >> >> containing a factory pre-programmed serial number. In order to access it, a
>> >> >> dummy write must be executed before reading the serial number bytes.
>> >> >
>> >> > Can't you instantiate a read-only EEPROM on this second address? Or a
>> >> > seperate driver attaching to this address? What is the advantage of
>> >> > having this in at24?
>> >> >
>> >>
>> >> The regular memory area and serial number read-only block share the
>> >> internal address pointer. We must ensure that there's no race
>> >> conditions between normal EEPROM reads/writes and serial number reads.
>> >
>> > I don't get it. Both, regular at24 reads and the serial read, setup the
>> > pointer every time by using two messages, first write to set the
>> > pointer, then read. The per-adapter lock makes sure those two messages
>> > will not get interrupted.
>>
>> If that's correct, then is there any need to have an additional mutex
>> for at24_data?
>
> I can't see a need, yes.

Then I'll see if it can be safely removed in the next iteration.

>> In that case would the preferred method be to access the regular
>> memory area like before - by allocating, for example, a 24c02 device -
>> while allocating a second device - in that case 24cs02 - on the
>> corresponding serial number address would give the user access to the
>> serial number via the eeprom sysfs attribute (which for the latter
>> would be read-only and 16 bytes in size)?
>
> Yes, a seperate driver for the second address is what I meant to suggest
> in the above paragraph. Only that the data should probably be exported
> via the NVMEM framework, not directly via sysfs. We have patches pending
> doing that for at24.

Right, but then these patches keep the driver backwards compatible in
that they keep the 'eeprom' sysfs attribute, so it's still a viable
option.

> What happens if you assign another at24 instance (read-only) to the
> second address? I mean, there is not only the serial number, but also a
> MAC address IIRC.

Nothing - it can't be read with the regular driver. Its protocol
requires certain bits set just like in the function from patch 4/9 in
this series.

As for the MAC address - I can't find anything in the datasheet, and
haven't heard about it.

> Regards,
>
>    Wolfram
>

Best regards,
Bartosz Golaszewski

Re: [RESEND PATCH v2 0/9] eeprom: at24: at24cs series serial number read

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1602 bytes --]


> >> If that's correct, then is there any need to have an additional mutex
> >> for at24_data?
> >
> > I can't see a need, yes.
> 
> Then I'll see if it can be safely removed in the next iteration.

That would be great, thanks!

> > Yes, a seperate driver for the second address is what I meant to suggest
> > in the above paragraph. Only that the data should probably be exported
> > via the NVMEM framework, not directly via sysfs. We have patches pending
> > doing that for at24.
> 
> Right, but then these patches keep the driver backwards compatible in
> that they keep the 'eeprom' sysfs attribute, so it's still a viable
> option.

Yes, they do it for backwards compatibility. If you do something new,
you can't really claim that ;)

> > What happens if you assign another at24 instance (read-only) to the
> > second address? I mean, there is not only the serial number, but also a
> > MAC address IIRC.
> 
> Nothing - it can't be read with the regular driver. Its protocol
> requires certain bits set just like in the function from patch 4/9 in
> this series.

Maybe it might work if you seek to the right offset and read the right
number of bytes, but this is clumsy, I agree.

> As for the MAC address - I can't find anything in the datasheet, and
> haven't heard about it.

http://www.atmel.com/images/atmel-8807-seeprom-at24mac402-602-datasheet.pdf

That was the first data sheet I found when looking for documentation.
So, we should keep in mind that there might be more than a serial number
in this extra memory space.

Thanks,

   Wolfram


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RESEND PATCH v2 0/9] eeprom: at24: at24cs series serial number read

[Bartosz Golaszewski]

2016-01-09 22:09 GMT+01:00 Wolfram Sang <wsa@the-dreams.de>:
>
>> >> If that's correct, then is there any need to have an additional mutex
>> >> for at24_data?
>> >
>> > I can't see a need, yes.
>>
>> Then I'll see if it can be safely removed in the next iteration.
>
> That would be great, thanks!
>
>> > Yes, a seperate driver for the second address is what I meant to suggest
>> > in the above paragraph. Only that the data should probably be exported
>> > via the NVMEM framework, not directly via sysfs. We have patches pending
>> > doing that for at24.
>>
>> Right, but then these patches keep the driver backwards compatible in
>> that they keep the 'eeprom' sysfs attribute, so it's still a viable
>> option.
>
> Yes, they do it for backwards compatibility. If you do something new,
> you can't really claim that ;)
>
>> > What happens if you assign another at24 instance (read-only) to the
>> > second address? I mean, there is not only the serial number, but also a
>> > MAC address IIRC.
>>
>> Nothing - it can't be read with the regular driver. Its protocol
>> requires certain bits set just like in the function from patch 4/9 in
>> this series.
>
> Maybe it might work if you seek to the right offset and read the right
> number of bytes, but this is clumsy, I agree.

You not only need to reset the address pointer to the right value, but
also prefix the word address with the right sequence just like in the
following snippet:

200         if (at24->chip.flags & AT24_FLAG_ADDR16) {
201                 /*
202                  * For 16 bit address pointers, the word address
must contain
203                  * a '10' sequence in bits 11 and 10 regardless of the
204                  * intended position of the address pointer.
205                  */
206                 addrbuf[0] = 0x08;
207                 addrbuf[1] = offset;
208                 msg[0].len = 2;
209         } else {
210                 /*
211                  * Otherwise the word address must begin with a
'10' sequence,
212                  * regardless of the intended address.
213                  */
214                 addrbuf[0] = 0x80 + offset;
215                 msg[0].len = 1;
216         }

>> As for the MAC address - I can't find anything in the datasheet, and
>> haven't heard about it.
>
> http://www.atmel.com/images/atmel-8807-seeprom-at24mac402-602-datasheet.pdf
>
> That was the first data sheet I found when looking for documentation.
> So, we should keep in mind that there might be more than a serial number
> in this extra memory space.

Right. I'll keep that in mind, but unfortunately I have no means of testing it.

Best regards,
Bartosz Golaszewski

> Thanks,
>
>    Wolfram
>

[RESEND PATCH v2 0/9] eeprom: at24: at24cs series serial number read

[Bartosz Golaszewski]

Chips from the at24cs EEPROM series have an additional read-only memory area
containing a factory pre-programmed serial number. In order to access it, a
dummy write must be executed before reading the serial number bytes.

This series adds support for reading the serial number through a sysfs
attribute.

While we're at it: some of the patches contain readability tweaks and code
organization fixes.

Tested with at24cs64 and at24cs02 chips (for both 16 and 8 bit address
pointers).

(rebased against 4.4-rc2)

v2:
- protect the serial number read with a mutex

v1: https://lkml.org/lkml/2015/10/20/162


Bartosz Golaszewski (9):
  eeprom: at24: platform_data: use BIT() macro
  eeprom: at24: new flag in platform_data
  eeprom: at24: tie up an additional address for at24cs series
  eeprom: at24: support reading of the serial number
  eeprom: at24: export the serial number through sysfs
  eeprom: at24: improve the device_id table readability
  eeprom: at24: add the at24cs series to the list of supported devices
  eeprom: at24: remove a reduntant if
  eeprom: at24: readability tweaks

 drivers/misc/eeprom/at24.c         | 187 +++++++++++++++++++++++++++++++------
 include/linux/platform_data/at24.h |   9 +-
 2 files changed, 166 insertions(+), 30 deletions(-)

-- 
2.1.4

[RESEND PATCH v2 0/9] eeprom: at24: at24cs series serial number read

[Bartosz Golaszewski]

Chips from the at24cs EEPROM series have an additional read-only memory area
containing a factory pre-programmed serial number. In order to access it, a
dummy write must be executed before reading the serial number bytes.

This series adds support for reading the serial number through a sysfs
attribute.

While we're at it: some of the patches contain readability tweaks and code
organization fixes.

Tested with at24cs64 and at24cs02 chips (for both 16 and 8 bit address
pointers).

(rebased against 4.4-rc1)

v2:
- protect the serial number read with a mutex

v1: https://lkml.org/lkml/2015/10/20/162

Bartosz Golaszewski (9):
  eeprom: at24: platform_data: use BIT() macro
  eeprom: at24: new flag in platform_data
  eeprom: at24: tie up an additional address for at24cs series
  eeprom: at24: support reading of the serial number
  eeprom: at24: export the serial number through sysfs
  eeprom: at24: improve the device_id table readability
  eeprom: at24: add the at24cs series to the list of supported devices
  eeprom: at24: remove a reduntant if
  eeprom: at24: readability tweaks

 drivers/misc/eeprom/at24.c         | 187 +++++++++++++++++++++++++++++++------
 include/linux/platform_data/at24.h |   9 +-
 2 files changed, 166 insertions(+), 30 deletions(-)

-- 
2.1.4