Skip to contentSkip to conversation

eeprom: at24: new DT property "address-width"

PATCH In review Johannes Poehlmann 2 participants 7 messages Jan 25, 2018 – Feb 2, 2018 1 message not in the archive

Subsystems (3)
Device tree bindings Char and misc drivers I2C subsystem host drivers
lore
Share & tools
lore

Johannes Poehlmann

author
#0 lore raw

PATCH 1/1 eeprom: at24: new DT property "address-width"

Hi Bartosz,

The patch was build against recent linus master
commit 5b7d27967dabfb17c21b0d98b29153b9e3ee71e5
Merge: f165495 624ca9c
Author: Linus Torvalds <[email protected]>
Date: Wed Jan 24 17:24:30 2018 -0800

checkpatch.pl run to 0 Errors
tested on powerpc 32bit

Any recommendations/comments ?

-------------------------------------------------------------------

Signed-off-by Johannes Poehlmann <[email protected]>

needed with some eeprom chips
modelled after at25

---
 .../devicetree/bindings/eeprom/eeprom.txt          |  3 +++
 drivers/misc/eeprom/at24.c                         | 24 ++++++++++++++++++++--
 2 files changed, 25 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/eeprom/eeprom.txt +3 -0
index 27f2bc1..8f3293c 100644
@@ -38,10 +38,13 @@ Optional properties:
 
   - size: total eeprom size in bytes
 
+  - address-width : number of address bits (8 or 16)
+
 Example:
 
 eeprom@52 {
 	compatible = "atmel,24c32";
 	reg = <0x52>;
 	pagesize = <32>;
+	address-width = <16>;
 };
drivers/misc/eeprom/at24.c +22 -2
index 4d63ac8..2a6d476 100644
@@ -651,11 +651,27 @@ static int at24_write(void *priv, unsigned int off, void *val, size_t count)
 	return 0;
 }
 
-static void at24_get_pdata(struct device *dev, struct at24_platform_data *chip)
+static int at24_get_pdata(struct device *dev, struct at24_platform_data *chip)
 {
 	int err;
 	u32 val;
 
+	if (device_property_read_u32(dev, "address-width", &val) == 0) {
+		switch (val) {
+		case 8:
+			chip->flags &= ~AT24_FLAG_ADDR16;
+			break;
+		case 16:
+			chip->flags |= AT24_FLAG_ADDR16;
+			break;
+		default:
+			dev_err(dev,
+				"Error: bad \"address-width\" property: %u\n",
+				val);
+			return -ENODEV;
+		}
+	}
+
 	if (device_property_present(dev, "read-only"))
 		chip->flags |= AT24_FLAG_READONLY;
 

@@ -674,6 +690,7 @@ static void at24_get_pdata(struct device *dev, struct at24_platform_data *chip)
 		 */
 		chip->page_size = 1;
 	}
+	return 0;
 }
 
 static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)

@@ -716,7 +733,10 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		magic >>= AT24_SIZE_BYTELEN;
 		chip.flags = magic & AT24_BITMASK(AT24_SIZE_FLAGS);
 
-		at24_get_pdata(&client->dev, &chip);
+		err = at24_get_pdata(&client->dev, &chip);
+		if (err)
+			return err;
+
 
 		chip.setup = NULL;
 		chip.context = NULL;
Signature · 1 line
2.1.4
#1 lore raw

2018-01-25 14:40 GMT+01:00 Johannes Poehlmann <[email protected]>:

Quoted 97 lines

Hi Bartosz,

The patch was build against recent linus master
commit 5b7d27967dabfb17c21b0d98b29153b9e3ee71e5
Merge: f165495 624ca9c
Author: Linus Torvalds <[email protected]>
Date: Wed Jan 24 17:24:30 2018 -0800

checkpatch.pl run to 0 Errors
tested on powerpc 32bit

Any recommendations/comments ?

-------------------------------------------------------------------

Signed-off-by Johannes Poehlmann <[email protected]>

needed with some eeprom chips
modelled after at25

---
 .../devicetree/bindings/eeprom/eeprom.txt          |  3 +++
 drivers/misc/eeprom/at24.c                         | 24 ++++++++++++++++++++--
 2 files changed, 25 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/eeprom/eeprom.txt +3 -0
index 27f2bc1..8f3293c 100644
@@ -38,10 +38,13 @@ Optional properties:
 
   - size: total eeprom size in bytes
 
+  - address-width : number of address bits (8 or 16)
+
 Example:
 
 eeprom@52 {
        compatible = "atmel,24c32";
        reg = <0x52>;
        pagesize = <32>;
+       address-width = <16>;
 };
drivers/misc/eeprom/at24.c +22 -3
index 4d63ac8..2a6d476 100644
@@ -651,11 +651,27 @@ static int at24_write(void *priv, unsigned int off, void *val, size_t count)
        return 0;
 }
 
-static void at24_get_pdata(struct device *dev, struct at24_platform_data *chip)
+static int at24_get_pdata(struct device *dev, struct at24_platform_data *chip)
 {
        int err;
        u32 val;
 
+       if (device_property_read_u32(dev, "address-width", &val) == 0) {
+               switch (val) {
+               case 8:
+                       chip->flags &= ~AT24_FLAG_ADDR16;
+                       break;
+               case 16:
+                       chip->flags |= AT24_FLAG_ADDR16;
+                       break;
+               default:
+                       dev_err(dev,
+                               "Error: bad \"address-width\" property: %u\n",
+                               val);
+                       return -ENODEV;
+               }
+       }
+
        if (device_property_present(dev, "read-only"))
                chip->flags |= AT24_FLAG_READONLY;
 

@@ -674,6 +690,7 @@ static void at24_get_pdata(struct device *dev, struct at24_platform_data *chip)
                 */
                chip->page_size = 1;
        }
+       return 0;
 }
 
 static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)

@@ -716,7 +733,10 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
                magic >>= AT24_SIZE_BYTELEN;
                chip.flags = magic & AT24_BITMASK(AT24_SIZE_FLAGS);
 
-               at24_get_pdata(&client->dev, &chip);
+               err = at24_get_pdata(&client->dev, &chip);
+               if (err)
+                       return err;
+
 
                chip.setup = NULL;
                chip.context = NULL;
--
Quoted 2 lines

2.1.4

What EEPROM model do you need that for? Is it not covered by all the standard types we already support in the driver?

Thanks,
Bartosz

author
#2 lore raw

Model is M24M01-D* ( was [PATCH alpha] eeprom: at24: new DT property "address-width")

Am 25.01.2018 um 17:07 schrieb Bartosz Golaszewski:

Quoted 104 lines

2018-01-25 14:40 GMT+01:00 Johannes Poehlmann <[email protected]>:

Quoted 97 lines

Hi Bartosz,

The patch was build against recent linus master
commit 5b7d27967dabfb17c21b0d98b29153b9e3ee71e5
Merge: f165495 624ca9c
Author: Linus Torvalds <[email protected]>
Date: Wed Jan 24 17:24:30 2018 -0800

checkpatch.pl run to 0 Errors
tested on powerpc 32bit

Any recommendations/comments ?

-------------------------------------------------------------------

Signed-off-by Johannes Poehlmann <[email protected]>

needed with some eeprom chips
modelled after at25

---
 .../devicetree/bindings/eeprom/eeprom.txt          |  3 +++
 drivers/misc/eeprom/at24.c                         | 24 ++++++++++++++++++++--
 2 files changed, 25 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/eeprom/eeprom.txt +3 -0
index 27f2bc1..8f3293c 100644
@@ -38,10 +38,13 @@ Optional properties:
 
   - size: total eeprom size in bytes
 
+  - address-width : number of address bits (8 or 16)
+
 Example:
 
 eeprom@52 {
        compatible = "atmel,24c32";
        reg = <0x52>;
        pagesize = <32>;
+       address-width = <16>;
 };
drivers/misc/eeprom/at24.c +22 -3
index 4d63ac8..2a6d476 100644
@@ -651,11 +651,27 @@ static int at24_write(void *priv, unsigned int off, void *val, size_t count)
        return 0;
 }
 
-static void at24_get_pdata(struct device *dev, struct at24_platform_data *chip)
+static int at24_get_pdata(struct device *dev, struct at24_platform_data *chip)
 {
        int err;
        u32 val;
 
+       if (device_property_read_u32(dev, "address-width", &val) == 0) {
+               switch (val) {
+               case 8:
+                       chip->flags &= ~AT24_FLAG_ADDR16;
+                       break;
+               case 16:
+                       chip->flags |= AT24_FLAG_ADDR16;
+                       break;
+               default:
+                       dev_err(dev,
+                               "Error: bad \"address-width\" property: %u\n",
+                               val);
+                       return -ENODEV;
+               }
+       }
+
        if (device_property_present(dev, "read-only"))
                chip->flags |= AT24_FLAG_READONLY;
 

@@ -674,6 +690,7 @@ static void at24_get_pdata(struct device *dev, struct at24_platform_data *chip)
                 */
                chip->page_size = 1;
        }
+       return 0;
 }
 
 static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)

@@ -716,7 +733,10 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
                magic >>= AT24_SIZE_BYTELEN;
                chip.flags = magic & AT24_BITMASK(AT24_SIZE_FLAGS);
 
-               at24_get_pdata(&client->dev, &chip);
+               err = at24_get_pdata(&client->dev, &chip);
+               if (err)
+                       return err;
+
 
                chip.setup = NULL;
                chip.context = NULL;
--
Quoted 2 lines

2.1.4

What EEPROM model do you need that for? Is it not covered by all the standard types we already support in the driver?

Quoted 3 lines

Thanks,
Bartosz

Hi Bartosz,

I need this for a M24M01-D* eeprom.
(http://www.st.com/content/ccc/resource/technical/document/datasheet/group0/cb/91/ba/7d/0b/c1/4d/f6/CD00147128/files/CD00147128.pdf/jcr:content/translations/en.CD00147128.pdf)

It is 1 Mbit sized and has 2 byte addresses.

Under another I2C address it offers a additional write lockable page of 256 Byte, but with _2 byte addresses_. I could not correctly read this "second eeprom" with one of the standard eeprom at24* models.

With the patch and using a device tree entry of

			compatible = "at,24c02";
			address-width = <16>;

reg = <0x5C>;
pagesize = <256>;
gave me a 256 Byte "eeprom" file in sysfs with no errors.

Regards

Johannes

#3 lore raw

Re: Model is M24M01-D* ( was [PATCH alpha] eeprom: at24: new DT property "address-width")

2018-01-26 11:13 GMT+01:00 Johannes Pöhlmann <[email protected]>:

Quoted 124 lines

Am 25.01.2018 um 17:07 schrieb Bartosz Golaszewski:

Quoted 104 lines

2018-01-25 14:40 GMT+01:00 Johannes Poehlmann <[email protected]>:

Quoted 97 lines

Hi Bartosz,

The patch was build against recent linus master
commit 5b7d27967dabfb17c21b0d98b29153b9e3ee71e5
Merge: f165495 624ca9c
Author: Linus Torvalds <[email protected]>
Date: Wed Jan 24 17:24:30 2018 -0800

checkpatch.pl run to 0 Errors
tested on powerpc 32bit

Any recommendations/comments ?

-------------------------------------------------------------------

Signed-off-by Johannes Poehlmann <[email protected]>

needed with some eeprom chips
modelled after at25

---
 .../devicetree/bindings/eeprom/eeprom.txt          |  3 +++
 drivers/misc/eeprom/at24.c                         | 24 ++++++++++++++++++++--
 2 files changed, 25 insertions(+), 2 deletions(-)
Documentation/devicetree/bindings/eeprom/eeprom.txt +3 -0
index 27f2bc1..8f3293c 100644
@@ -38,10 +38,13 @@ Optional properties:
 
   - size: total eeprom size in bytes
 
+  - address-width : number of address bits (8 or 16)
+
 Example:
 
 eeprom@52 {
        compatible = "atmel,24c32";
        reg = <0x52>;
        pagesize = <32>;
+       address-width = <16>;
 };
drivers/misc/eeprom/at24.c +22 -3
index 4d63ac8..2a6d476 100644
@@ -651,11 +651,27 @@ static int at24_write(void *priv, unsigned int off, void *val, size_t count)
        return 0;
 }
 
-static void at24_get_pdata(struct device *dev, struct at24_platform_data *chip)
+static int at24_get_pdata(struct device *dev, struct at24_platform_data *chip)
 {
        int err;
        u32 val;
 
+       if (device_property_read_u32(dev, "address-width", &val) == 0) {
+               switch (val) {
+               case 8:
+                       chip->flags &= ~AT24_FLAG_ADDR16;
+                       break;
+               case 16:
+                       chip->flags |= AT24_FLAG_ADDR16;
+                       break;
+               default:
+                       dev_err(dev,
+                               "Error: bad \"address-width\" property: %u\n",
+                               val);
+                       return -ENODEV;
+               }
+       }
+
        if (device_property_present(dev, "read-only"))
                chip->flags |= AT24_FLAG_READONLY;
 

@@ -674,6 +690,7 @@ static void at24_get_pdata(struct device *dev, struct at24_platform_data *chip)
                 */
                chip->page_size = 1;
        }
+       return 0;
 }
 
 static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)

@@ -716,7 +733,10 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id)
                magic >>= AT24_SIZE_BYTELEN;
                chip.flags = magic & AT24_BITMASK(AT24_SIZE_FLAGS);
 
-               at24_get_pdata(&client->dev, &chip);
+               err = at24_get_pdata(&client->dev, &chip);
+               if (err)
+                       return err;
+
 
                chip.setup = NULL;
                chip.context = NULL;
--
Quoted 2 lines

2.1.4

What EEPROM model do you need that for? Is it not covered by all the standard types we already support in the driver?

Quoted 3 lines

Thanks,
Bartosz

Hi Bartosz,

I need this for a M24M01-D* eeprom.
(http://www.st.com/content/ccc/resource/technical/document/datasheet/group0/cb/91/ba/7d/0b/c1/4d/f6/CD00147128/files/CD00147128.pdf/jcr:content/translations/en.CD00147128.pdf)

It is 1 Mbit sized and has 2 byte addresses.

Under another I2C address it offers a additional write lockable page of 256 Byte, but with _2 byte addresses_. I could not correctly read this "second eeprom" with one of the standard eeprom at24* models.

With the patch and using a device tree entry of

                        compatible = "at,24c02";
                        address-width = <16>;
                        reg = <0x5C>;
                        pagesize = <256>;
Quoted 5 lines

gave me a 256 Byte "eeprom" file in sysfs with no errors.

Regards

Johannes

This model is compatible with at24cm01. We don't support it yet, but instead of a new DT property, just add a new supported model with compatible = 'atmel,at24cm01' and AT24_FLAG_ADDR16 to the driver code.

Thanks,
Bartosz

A message that is not in the archive (sent off-list or not yet received): [email protected]

#5 lore raw

Re: [PATCH V2] eeprom: at24: support second page of M24M01

2018-01-30 15:14 GMT+01:00 johannes <[email protected]>:

Quoted 19 lines · Bartosz Golaszewski
Quoted 9 lines
Quoted 8 lines
Quoted 3 lines

What EEPROM model do you need that for? Is it not covered by all the standard types we already support in the driver?

I need this for a M24M01-D* eeprom.
(http://www.st.com/content/ccc/resource/technical/document/datasheet/group0/cb/91/ba/7d/0b/c1/4d/f6/CD00147128/files/CD00147128.pdf/jcr:content/translations/en.CD00147128.pdf)

It is 1 Mbit sized and has 2 byte addresses.

Quoted 8 lines
Quoted 4 lines

Under another I2C address it offers a additional write lockable page of 256 Byte, but with _2 byte addresses_. I could not correctly read this "second eeprom" with one of the standard eeprom at24* models. [ .. ]

This model is compatible with at24cm01. We don't support it yet, but instead of a new DT property, just add a new supported model with compatible = 'atmel,at24cm01' and AT24_FLAG_ADDR16 to the driver code.

Quoted 8 lines

Hi Bartosz,

I did the patch as you recommended with a new supported model. The diff is much smaller now.

See more info and a question about naming in the patch attached.

Please send patches using git send-mail. Patchwork didn't even pick that one up.

Thanks,
Bartosz

author
#6 lore raw

PATCH v2 1/1 eeprom: at24: support second page of M24M01

The M24M01-D* eeproms are in effect 2 eeproms under 2 I2C addresses:

1. a standard 1Mbit eeprom (driver model 24c1024 is working)

2. a 256 Byte "ID page" that is a write lockable page. As the ID page demands 2 byte addresses, the 24c02 driver model can not be used.

I was thinking about not to pollute the name space of chip names with a made up name that is not the name of a real chip.

I would propose to name it "24cb02", thus making clear that we have a kind of 24c02 but with 2 byte address. (The "b" ist meant to say this)

Signed-off-by Johannes Poehlmann <[email protected]>
---
 drivers/misc/eeprom/at24.c | 1 +
 1 file changed, 1 insertion(+)
drivers/misc/eeprom/at24.c +1 -0
index 4d63ac8..debc8e3 100644
@@ -139,6 +139,7 @@ static const struct i2c_device_id at24_ids[] = {
 	{ "24cs01",	AT24_DEVICE_MAGIC(16,
 				AT24_FLAG_SERIAL | AT24_FLAG_READONLY) },
 	{ "24c02",	AT24_DEVICE_MAGIC(2048 / 8,	0) },
+	{ "24cb02",	AT24_DEVICE_MAGIC(2048 / 8,	AT24_FLAG_ADDR16) },
 	{ "24cs02",	AT24_DEVICE_MAGIC(16,
 				AT24_FLAG_SERIAL | AT24_FLAG_READONLY) },
 	{ "24mac402",	AT24_DEVICE_MAGIC(48 / 8,
Signature · 1 line
2.1.4
#7 lore raw

Re: [PATCH V2] eeprom: at24: support second page of M24M01

2018-01-30 17:50 GMT+01:00 Johannes Poehlmann <[email protected]>:

Quoted 8 lines

The M24M01-D* eeproms are in effect 2 eeproms under 2 I2C addresses:

1. a standard 1Mbit eeprom (driver model 24c1024 is working)

2. a 256 Byte "ID page" that is a write lockable page. As the ID page demands 2 byte addresses, the 24c02 driver model can not be used.

Quoted 6 lines

I was thinking about not to pollute the name space of chip names with a made up name that is not the name of a real chip.

I would propose to name it "24cb02", thus making clear that we have a kind of 24c02 but with 2 byte address. (The "b" ist meant to say this)

If anything, this is just super confusing.

Quoted 11 lines
Signed-off-by Johannes Poehlmann <[email protected]>
---
 drivers/misc/eeprom/at24.c | 1 +
 1 file changed, 1 insertion(+)
drivers/misc/eeprom/at24.c +0 -0
index 4d63ac8..debc8e3 100644

drivers/misc/eeprom/at24.c static const struct i2c_device_id at24_ids[] = { · line 142

Quoted 4 lines
drivers/misc/eeprom/at24.c +1 -0
@@ -139,6 +139,7 @@ static const struct i2c_device_id at24_ids[] = {
        { "24cs01",     AT24_DEVICE_MAGIC(16,
                                AT24_FLAG_SERIAL | AT24_FLAG_READONLY) },
        { "24c02",      AT24_DEVICE_MAGIC(2048 / 8,     0) },
+       { "24cb02",     AT24_DEVICE_MAGIC(2048 / 8,     AT24_FLAG_ADDR16) },

Please add the device tree support as well.

Quoted 6 lines
drivers/misc/eeprom/at24.c +0 -1
        { "24cs02",     AT24_DEVICE_MAGIC(16,
                                AT24_FLAG_SERIAL | AT24_FLAG_READONLY) },
        { "24mac402",   AT24_DEVICE_MAGIC(48 / 8,
--

2.1.4

Take a look at what we do with at24cs* and at24mac* models: at24cs32 stands for the serial number block, while for the regular memory area we use at24c32.

Analogously in your case: the 'regular' memory block of this EEPROM is compatible with 'atmel,24c1024', so this is the compatible you'll use to access it. For the write-lockable page, it's OK to use 'atmel,24cm01'.

Best regards,
Bartosz Golaszewski