[PATCH v2 0/4] eeprom: ee1004: Enable devices on multiple busses

[PATCH v2 0/4] eeprom: ee1004: Enable devices on multiple busses

[Eddie James]

The driver previously prevented probing devices on more than one
bus due to locking constraints with the special page addresses. This
constraint can be removed by allocating a reference-counted bus
structure containing the lock, rather than using global variables.
In addition, add devicetree bindings for the EE1004 driver for the
AT30TSE device and add the devices to the Bonnell BMC system.

Changes since v1:
 - Add the devicetree changes

Eddie James (4):
  eeprom: ee1004: Enable devices on multiple busses
  doc: Add Atmel AT30TSE serial eeprom
  eeprom: ee1004: Add devicetree binding
  ARM: dts: aspeed: bonnell: Add DIMM SPD

 .../devicetree/bindings/trivial-devices.yaml  |   2 +
 arch/arm/boot/dts/aspeed-bmc-ibm-bonnell.dts  |  20 ++
 drivers/misc/eeprom/ee1004.c                  | 182 +++++++++++-------
 3 files changed, 135 insertions(+), 69 deletions(-)

-- 
2.31.1

[PATCH v2 1/4] eeprom: ee1004: Enable devices on multiple busses

[Eddie James]

The driver previously prevented probing devices on more than one
bus due to locking constraints with the special page addresses. This
constraint can be removed by allocating a reference-counted bus
structure containing the lock, rather than using global variables.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 drivers/misc/eeprom/ee1004.c | 175 +++++++++++++++++++++--------------
 1 file changed, 106 insertions(+), 69 deletions(-)

diff --git a/drivers/misc/eeprom/ee1004.c b/drivers/misc/eeprom/ee1004.c
index c8c6deb7ed89..950813821087 100644
--- a/drivers/misc/eeprom/ee1004.c
+++ b/drivers/misc/eeprom/ee1004.c
@@ -9,12 +9,15 @@
  * Copyright (C) 2008 Wolfram Sang, Pengutronix
  */
 
+#include <linux/err.h>
 #include <linux/i2c.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
+#include <linux/list.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/of_device.h>
 
 /*
  * DDR4 memory modules use special EEPROMs following the Jedec EE1004
@@ -31,20 +34,24 @@
  * over performance.
  */
 
-#define EE1004_ADDR_SET_PAGE		0x36
+#define EE1004_ADDR_SET_PAGE0		0x36
+#define EE1004_ADDR_SET_PAGE1		0x37
 #define EE1004_NUM_PAGES		2
 #define EE1004_PAGE_SIZE		256
 #define EE1004_PAGE_SHIFT		8
 #define EE1004_EEPROM_SIZE		(EE1004_PAGE_SIZE * EE1004_NUM_PAGES)
 
-/*
- * Mutex protects ee1004_set_page and ee1004_dev_count, and must be held
- * from page selection to end of read.
- */
-static DEFINE_MUTEX(ee1004_bus_lock);
-static struct i2c_client *ee1004_set_page[EE1004_NUM_PAGES];
-static unsigned int ee1004_dev_count;
-static int ee1004_current_page;
+struct ee1004_bus {
+	struct kref kref;
+	struct list_head list;
+	struct mutex lock;
+	struct i2c_adapter *adapter;
+	struct i2c_client *set_page_clients[EE1004_NUM_PAGES];
+	int page;
+};
+
+static LIST_HEAD(ee1004_busses);
+static DEFINE_MUTEX(ee1004_busses_lock);
 
 static const struct i2c_device_id ee1004_ids[] = {
 	{ "ee1004", 0 },
@@ -54,11 +61,11 @@ MODULE_DEVICE_TABLE(i2c, ee1004_ids);
 
 /*-------------------------------------------------------------------------*/
 
-static int ee1004_get_current_page(void)
+static int ee1004_get_current_page(struct ee1004_bus *bus)
 {
 	int err;
 
-	err = i2c_smbus_read_byte(ee1004_set_page[0]);
+	err = i2c_smbus_read_byte(bus->set_page_clients[0]);
 	if (err == -ENXIO) {
 		/* Nack means page 1 is selected */
 		return 1;
@@ -72,33 +79,30 @@ static int ee1004_get_current_page(void)
 	return 0;
 }
 
-static int ee1004_set_current_page(struct device *dev, int page)
+static int ee1004_set_current_page(struct ee1004_bus *bus, int page)
 {
 	int ret;
 
-	if (page == ee1004_current_page)
+	if (page == bus->page)
 		return 0;
 
 	/* Data is ignored */
-	ret = i2c_smbus_write_byte(ee1004_set_page[page], 0x00);
+	ret = i2c_smbus_write_byte(bus->set_page_clients[page], 0x00);
+
 	/*
 	 * Don't give up just yet. Some memory modules will select the page
 	 * but not ack the command. Check which page is selected now.
 	 */
-	if (ret == -ENXIO && ee1004_get_current_page() == page)
+	if (ret == -ENXIO && ee1004_get_current_page(bus) == page)
 		ret = 0;
-	if (ret < 0) {
-		dev_err(dev, "Failed to select page %d (%d)\n", page, ret);
+	if (ret < 0)
 		return ret;
-	}
-
-	dev_dbg(dev, "Selected page %d\n", page);
-	ee1004_current_page = page;
 
+	bus->page = page;
 	return 0;
 }
 
-static ssize_t ee1004_eeprom_read(struct i2c_client *client, char *buf,
+static ssize_t ee1004_eeprom_read(struct i2c_client *client, struct ee1004_bus *bus, char *buf,
 				  unsigned int offset, size_t count)
 {
 	int status, page;
@@ -106,9 +110,11 @@ static ssize_t ee1004_eeprom_read(struct i2c_client *client, char *buf,
 	page = offset >> EE1004_PAGE_SHIFT;
 	offset &= (1 << EE1004_PAGE_SHIFT) - 1;
 
-	status = ee1004_set_current_page(&client->dev, page);
-	if (status)
+	status = ee1004_set_current_page(bus, page);
+	if (status) {
+		dev_err(&client->dev, "Failed to select page %d (%d)\n", page, status);
 		return status;
+	}
 
 	/* Can't cross page boundaries */
 	if (offset + count > EE1004_PAGE_SIZE)
@@ -125,6 +131,7 @@ static ssize_t eeprom_read(struct file *filp, struct kobject *kobj,
 			   char *buf, loff_t off, size_t count)
 {
 	struct i2c_client *client = kobj_to_i2c_client(kobj);
+	struct ee1004_bus *bus = i2c_get_clientdata(client);
 	size_t requested = count;
 	int ret = 0;
 
@@ -132,10 +139,10 @@ static ssize_t eeprom_read(struct file *filp, struct kobject *kobj,
 	 * Read data from chip, protecting against concurrent access to
 	 * other EE1004 SPD EEPROMs on the same adapter.
 	 */
-	mutex_lock(&ee1004_bus_lock);
+	mutex_lock(&bus->lock);
 
 	while (count) {
-		ret = ee1004_eeprom_read(client, buf, off, count);
+		ret = ee1004_eeprom_read(client, bus, buf, off, count);
 		if (ret < 0)
 			goto out;
 
@@ -144,7 +151,7 @@ static ssize_t eeprom_read(struct file *filp, struct kobject *kobj,
 		count -= ret;
 	}
 out:
-	mutex_unlock(&ee1004_bus_lock);
+	mutex_unlock(&bus->lock);
 
 	return ret < 0 ? ret : requested;
 }
@@ -158,18 +165,56 @@ static struct bin_attribute *ee1004_attrs[] = {
 
 BIN_ATTRIBUTE_GROUPS(ee1004);
 
-static void ee1004_cleanup(int idx)
+static void ee1004_bus_unregister(struct ee1004_bus *bus)
 {
-	if (--ee1004_dev_count == 0)
-		while (--idx >= 0) {
-			i2c_unregister_device(ee1004_set_page[idx]);
-			ee1004_set_page[idx] = NULL;
-		}
+	i2c_unregister_device(bus->set_page_clients[1]);
+	i2c_unregister_device(bus->set_page_clients[0]);
+}
+
+static void ee1004_bus_release(struct kref *kref)
+{
+	struct ee1004_bus *bus = container_of(kref, struct ee1004_bus, kref);
+
+	ee1004_bus_unregister(bus);
+
+	mutex_lock(&ee1004_busses_lock);
+	list_del(&bus->list);
+	mutex_unlock(&ee1004_busses_lock);
+
+	kfree(bus);
+}
+
+static int ee1004_bus_initialize(struct ee1004_bus *bus, struct i2c_adapter *adapter)
+{
+	bus->set_page_clients[0] = i2c_new_dummy_device(adapter, EE1004_ADDR_SET_PAGE0);
+	if (IS_ERR(bus->set_page_clients[0]))
+		return PTR_ERR(bus->set_page_clients[0]);
+
+	bus->set_page_clients[1] = i2c_new_dummy_device(adapter, EE1004_ADDR_SET_PAGE1);
+	if (IS_ERR(bus->set_page_clients[1])) {
+		i2c_unregister_device(bus->set_page_clients[0]);
+		return PTR_ERR(bus->set_page_clients[1]);
+	}
+
+	bus->page = ee1004_get_current_page(bus);
+	if (bus->page < 0) {
+		ee1004_bus_unregister(bus);
+		return bus->page;
+	}
+
+	kref_init(&bus->kref);
+	list_add(&bus->list, &ee1004_busses);
+	mutex_init(&bus->lock);
+	bus->adapter = adapter;
+
+	return 0;
 }
 
 static int ee1004_probe(struct i2c_client *client)
 {
-	int err, cnr = 0;
+	struct ee1004_bus *bus;
+	bool found = false;
+	int rc = 0;
 
 	/* Make sure we can operate on this adapter */
 	if (!i2c_check_functionality(client->adapter,
@@ -178,53 +223,45 @@ static int ee1004_probe(struct i2c_client *client)
 				     I2C_FUNC_SMBUS_BYTE | I2C_FUNC_SMBUS_READ_BYTE_DATA))
 		return -EPFNOSUPPORT;
 
-	/* Use 2 dummy devices for page select command */
-	mutex_lock(&ee1004_bus_lock);
-	if (++ee1004_dev_count == 1) {
-		for (cnr = 0; cnr < EE1004_NUM_PAGES; cnr++) {
-			struct i2c_client *cl;
-
-			cl = i2c_new_dummy_device(client->adapter, EE1004_ADDR_SET_PAGE + cnr);
-			if (IS_ERR(cl)) {
-				err = PTR_ERR(cl);
-				goto err_clients;
-			}
-			ee1004_set_page[cnr] = cl;
+	mutex_lock(&ee1004_busses_lock);
+	list_for_each_entry(bus, &ee1004_busses, list) {
+		if (bus->adapter == client->adapter) {
+			kref_get(&bus->kref);
+			found = true;
+			break;
+		}
+	}
+
+	if (!found) {
+		bus = kzalloc(sizeof(*bus), GFP_KERNEL);
+		if (!bus) {
+			rc = -ENOMEM;
+			goto unlock;
 		}
 
-		/* Remember current page to avoid unneeded page select */
-		err = ee1004_get_current_page();
-		if (err < 0)
-			goto err_clients;
-		dev_dbg(&client->dev, "Currently selected page: %d\n", err);
-		ee1004_current_page = err;
-	} else if (client->adapter != ee1004_set_page[0]->adapter) {
-		dev_err(&client->dev,
-			"Driver only supports devices on a single I2C bus\n");
-		err = -EOPNOTSUPP;
-		goto err_clients;
+		rc = ee1004_bus_initialize(bus, client->adapter);
+		if (rc) {
+			kfree(bus);
+			goto unlock;
+		}
 	}
-	mutex_unlock(&ee1004_bus_lock);
+
+	i2c_set_clientdata(client, bus);
 
 	dev_info(&client->dev,
 		 "%u byte EE1004-compliant SPD EEPROM, read-only\n",
 		 EE1004_EEPROM_SIZE);
 
-	return 0;
-
- err_clients:
-	ee1004_cleanup(cnr);
-	mutex_unlock(&ee1004_bus_lock);
-
-	return err;
+unlock:
+	mutex_unlock(&ee1004_busses_lock);
+	return rc;
 }
 
 static void ee1004_remove(struct i2c_client *client)
 {
-	/* Remove page select clients if this is the last device */
-	mutex_lock(&ee1004_bus_lock);
-	ee1004_cleanup(EE1004_NUM_PAGES);
-	mutex_unlock(&ee1004_bus_lock);
+	struct ee1004_bus *bus = i2c_get_clientdata(client);
+
+	kref_put(&bus->kref, ee1004_bus_release);
 }
 
 /*-------------------------------------------------------------------------*/
-- 
2.31.1

Re: [PATCH v2 1/4] eeprom: ee1004: Enable devices on multiple busses

[Rob Herring]

On Tue, Mar 21, 2023 at 10:17 AM Eddie James <eajames@linux.ibm.com> wrote:
>
> The driver previously prevented probing devices on more than one
> bus due to locking constraints with the special page addresses. This
> constraint can be removed by allocating a reference-counted bus
> structure containing the lock, rather than using global variables.
>
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
>  drivers/misc/eeprom/ee1004.c | 175 +++++++++++++++++++++--------------
>  1 file changed, 106 insertions(+), 69 deletions(-)
>
> diff --git a/drivers/misc/eeprom/ee1004.c b/drivers/misc/eeprom/ee1004.c
> index c8c6deb7ed89..950813821087 100644
> --- a/drivers/misc/eeprom/ee1004.c
> +++ b/drivers/misc/eeprom/ee1004.c
> @@ -9,12 +9,15 @@
>   * Copyright (C) 2008 Wolfram Sang, Pengutronix
>   */
>
> +#include <linux/err.h>
>  #include <linux/i2c.h>
>  #include <linux/init.h>
>  #include <linux/kernel.h>
> +#include <linux/list.h>
>  #include <linux/mod_devicetable.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
> +#include <linux/of_device.h>

What do you need from here? I don't see anything.

of_device.h is a mess of implicit includes which I'm currently trying
to detangle. See the ~13 year old comment in it about removing
of_platform.h include. When I'm done, pretty much only bus
implementations should include of_device.h.

Rob

Re: [PATCH v2 1/4] eeprom: ee1004: Enable devices on multiple busses

[Eddie James]

On 3/21/23 10:39, Rob Herring wrote:
> On Tue, Mar 21, 2023 at 10:17 AM Eddie James <eajames@linux.ibm.com> wrote:
>> The driver previously prevented probing devices on more than one
>> bus due to locking constraints with the special page addresses. This
>> constraint can be removed by allocating a reference-counted bus
>> structure containing the lock, rather than using global variables.
>>
>> Signed-off-by: Eddie James <eajames@linux.ibm.com>
>> ---
>>   drivers/misc/eeprom/ee1004.c | 175 +++++++++++++++++++++--------------
>>   1 file changed, 106 insertions(+), 69 deletions(-)
>>
>> diff --git a/drivers/misc/eeprom/ee1004.c b/drivers/misc/eeprom/ee1004.c
>> index c8c6deb7ed89..950813821087 100644
>> --- a/drivers/misc/eeprom/ee1004.c
>> +++ b/drivers/misc/eeprom/ee1004.c
>> @@ -9,12 +9,15 @@
>>    * Copyright (C) 2008 Wolfram Sang, Pengutronix
>>    */
>>
>> +#include <linux/err.h>
>>   #include <linux/i2c.h>
>>   #include <linux/init.h>
>>   #include <linux/kernel.h>
>> +#include <linux/list.h>
>>   #include <linux/mod_devicetable.h>
>>   #include <linux/module.h>
>>   #include <linux/mutex.h>
>> +#include <linux/of_device.h>
> What do you need from here? I don't see anything.
>
> of_device.h is a mess of implicit includes which I'm currently trying
> to detangle. See the ~13 year old comment in it about removing
> of_platform.h include. When I'm done, pretty much only bus
> implementations should include of_device.h.


You're right, I mistakenly thought I needed it for of_device_id. I'll 
remove it in v3.

Thanks,

Eddie


>
> Rob

[PATCH v2 2/4] doc: Add Atmel AT30TSE serial eeprom

[Eddie James]

The AT30TSE is compatible with the JEDEC EE1004 standard. Document it
as a trivial I2C device.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
index 6f482a254a1d..43e26c73a95f 100644
--- a/Documentation/devicetree/bindings/trivial-devices.yaml
+++ b/Documentation/devicetree/bindings/trivial-devices.yaml
@@ -47,6 +47,8 @@ properties:
           - ams,iaq-core
             # i2c serial eeprom (24cxx)
           - at,24c08
+            # i2c serial eeprom (EE1004 standard)
+          - atmel,at30tse
             # i2c trusted platform module (TPM)
           - atmel,at97sc3204t
             # ATSHA204 - i2c h/w symmetric crypto module
-- 
2.31.1

Re: [PATCH v2 2/4] doc: Add Atmel AT30TSE serial eeprom

[Krzysztof Kozlowski]

On 21/03/2023 16:16, Eddie James wrote:
> The AT30TSE is compatible with the JEDEC EE1004 standard. Document it
> as a trivial I2C device.
> 
> Signed-off-by: Eddie James <eajames@linux.ibm.com>

Use subject prefixes matching the subsystem (which you can get for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching).

> ---
>  Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
> index 6f482a254a1d..43e26c73a95f 100644
> --- a/Documentation/devicetree/bindings/trivial-devices.yaml
> +++ b/Documentation/devicetree/bindings/trivial-devices.yaml
> @@ -47,6 +47,8 @@ properties:
>            - ams,iaq-core
>              # i2c serial eeprom (24cxx)
>            - at,24c08
> +            # i2c serial eeprom (EE1004 standard)

AT30TSE?

> +          - atmel,at30tse

Microchip does not find anything on AT30TSE. Are you sure this is the
model name?


Best regards,
Krzysztof

Re: [PATCH v2 2/4] doc: Add Atmel AT30TSE serial eeprom

[Eddie James]

On 3/21/23 10:19, Krzysztof Kozlowski wrote:
> On 21/03/2023 16:16, Eddie James wrote:
>> The AT30TSE is compatible with the JEDEC EE1004 standard. Document it
>> as a trivial I2C device.
>>
>> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> Use subject prefixes matching the subsystem (which you can get for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching).


Oops, sorry, will fix.


>
>> ---
>>   Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
>> index 6f482a254a1d..43e26c73a95f 100644
>> --- a/Documentation/devicetree/bindings/trivial-devices.yaml
>> +++ b/Documentation/devicetree/bindings/trivial-devices.yaml
>> @@ -47,6 +47,8 @@ properties:
>>             - ams,iaq-core
>>               # i2c serial eeprom (24cxx)
>>             - at,24c08
>> +            # i2c serial eeprom (EE1004 standard)
> AT30TSE?
>
>> +          - atmel,at30tse
> Microchip does not find anything on AT30TSE. Are you sure this is the
> model name?


Yes: 
https://www.microchip.com/content/dam/mchp/documents/OTH/ProductDocuments/DataSheets/Atmel-8868-DTS-AT30TSE004A-Datasheet.pdf


Maybe it's actually an 8868? Or should I include the 004A as well?

Thanks,

Eddie


>
>
> Best regards,
> Krzysztof
>

Re: [PATCH v2 2/4] doc: Add Atmel AT30TSE serial eeprom

[Eddie James]

On 3/21/23 10:46, Eddie James wrote:
>
> On 3/21/23 10:19, Krzysztof Kozlowski wrote:
>> On 21/03/2023 16:16, Eddie James wrote:
>>> The AT30TSE is compatible with the JEDEC EE1004 standard. Document it
>>> as a trivial I2C device.
>>>
>>> Signed-off-by: Eddie James <eajames@linux.ibm.com>
>> Use subject prefixes matching the subsystem (which you can get for
>> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
>> your patch is touching).
>
>
> Oops, sorry, will fix.
>
>
>>
>>> ---
>>>   Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml 
>>> b/Documentation/devicetree/bindings/trivial-devices.yaml
>>> index 6f482a254a1d..43e26c73a95f 100644
>>> --- a/Documentation/devicetree/bindings/trivial-devices.yaml
>>> +++ b/Documentation/devicetree/bindings/trivial-devices.yaml
>>> @@ -47,6 +47,8 @@ properties:
>>>             - ams,iaq-core
>>>               # i2c serial eeprom (24cxx)
>>>             - at,24c08
>>> +            # i2c serial eeprom (EE1004 standard)
>> AT30TSE?
>>
>>> +          - atmel,at30tse
>> Microchip does not find anything on AT30TSE. Are you sure this is the
>> model name?
>
>
> Yes: 
> https://www.microchip.com/content/dam/mchp/documents/OTH/ProductDocuments/DataSheets/Atmel-8868-DTS-AT30TSE004A-Datasheet.pdf
>
>
> Maybe it's actually an 8868? Or should I include the 004A as well?


I found some other AT30TSE (AT30TSE752A for example) devices that do not 
appear compatible with the EE1004 standard, so I will include the full 
model number.


>
> Thanks,
>
> Eddie
>
>
>>
>>
>> Best regards,
>> Krzysztof
>>

Re: [PATCH v2 2/4] doc: Add Atmel AT30TSE serial eeprom

[Rob Herring]

On Tue, Mar 21, 2023 at 10:55:43AM -0500, Eddie James wrote:
> 
> On 3/21/23 10:46, Eddie James wrote:
> > 
> > On 3/21/23 10:19, Krzysztof Kozlowski wrote:
> > > On 21/03/2023 16:16, Eddie James wrote:
> > > > The AT30TSE is compatible with the JEDEC EE1004 standard. Document it
> > > > as a trivial I2C device.
> > > > 
> > > > Signed-off-by: Eddie James <eajames@linux.ibm.com>
> > > Use subject prefixes matching the subsystem (which you can get for
> > > example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> > > your patch is touching).
> > 
> > 
> > Oops, sorry, will fix.
> > 
> > 
> > > 
> > > > ---
> > > >   Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++
> > > >   1 file changed, 2 insertions(+)
> > > > 
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/trivial-devices.yaml
> > > > b/Documentation/devicetree/bindings/trivial-devices.yaml
> > > > index 6f482a254a1d..43e26c73a95f 100644
> > > > --- a/Documentation/devicetree/bindings/trivial-devices.yaml
> > > > +++ b/Documentation/devicetree/bindings/trivial-devices.yaml
> > > > @@ -47,6 +47,8 @@ properties:
> > > >             - ams,iaq-core
> > > >               # i2c serial eeprom (24cxx)
> > > >             - at,24c08
> > > > +            # i2c serial eeprom (EE1004 standard)
> > > AT30TSE?
> > > 
> > > > +          - atmel,at30tse
> > > Microchip does not find anything on AT30TSE. Are you sure this is the
> > > model name?
> > 
> > 
> > Yes: https://www.microchip.com/content/dam/mchp/documents/OTH/ProductDocuments/DataSheets/Atmel-8868-DTS-AT30TSE004A-Datasheet.pdf
> > 
> > 
> > Maybe it's actually an 8868? Or should I include the 004A as well?
> 
> 
> I found some other AT30TSE (AT30TSE752A for example) devices that do not
> appear compatible with the EE1004 standard, so I will include the full model
> number.

If this standard is sufficiently complete, then you might want a EE1004 
fallback compatible. Complete would mean power supply(ies) and any extra 
i/o are defined and the exact device model is discoverable.

Rob

Re: [PATCH v2 2/4] doc: Add Atmel AT30TSE serial eeprom

[Eddie James]

On 3/27/23 10:18, Rob Herring wrote:
> On Tue, Mar 21, 2023 at 10:55:43AM -0500, Eddie James wrote:
>> On 3/21/23 10:46, Eddie James wrote:
>>> On 3/21/23 10:19, Krzysztof Kozlowski wrote:
>>>> On 21/03/2023 16:16, Eddie James wrote:
>>>>> The AT30TSE is compatible with the JEDEC EE1004 standard. Document it
>>>>> as a trivial I2C device.
>>>>>
>>>>> Signed-off-by: Eddie James <eajames@linux.ibm.com>
>>>> Use subject prefixes matching the subsystem (which you can get for
>>>> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
>>>> your patch is touching).
>>>
>>> Oops, sorry, will fix.
>>>
>>>
>>>>> ---
>>>>>    Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++
>>>>>    1 file changed, 2 insertions(+)
>>>>>
>>>>> diff --git
>>>>> a/Documentation/devicetree/bindings/trivial-devices.yaml
>>>>> b/Documentation/devicetree/bindings/trivial-devices.yaml
>>>>> index 6f482a254a1d..43e26c73a95f 100644
>>>>> --- a/Documentation/devicetree/bindings/trivial-devices.yaml
>>>>> +++ b/Documentation/devicetree/bindings/trivial-devices.yaml
>>>>> @@ -47,6 +47,8 @@ properties:
>>>>>              - ams,iaq-core
>>>>>                # i2c serial eeprom (24cxx)
>>>>>              - at,24c08
>>>>> +            # i2c serial eeprom (EE1004 standard)
>>>> AT30TSE?
>>>>
>>>>> +          - atmel,at30tse
>>>> Microchip does not find anything on AT30TSE. Are you sure this is the
>>>> model name?
>>>
>>> Yes: https://www.microchip.com/content/dam/mchp/documents/OTH/ProductDocuments/DataSheets/Atmel-8868-DTS-AT30TSE004A-Datasheet.pdf
>>>
>>>
>>> Maybe it's actually an 8868? Or should I include the 004A as well?
>>
>> I found some other AT30TSE (AT30TSE752A for example) devices that do not
>> appear compatible with the EE1004 standard, so I will include the full model
>> number.
> If this standard is sufficiently complete, then you might want a EE1004
> fallback compatible. Complete would mean power supply(ies) and any extra
> i/o are defined and the exact device model is discoverable.


I don't think this standard would meet those requirements unfortunately. 
Thanks for the suggestion!

Eddie


>
> Rob

[PATCH v2 3/4] eeprom: ee1004: Add devicetree binding

[Eddie James]

Add an OF match table for devicetree instantiation of EE1004
devices.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 drivers/misc/eeprom/ee1004.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/misc/eeprom/ee1004.c b/drivers/misc/eeprom/ee1004.c
index 950813821087..72068bb72621 100644
--- a/drivers/misc/eeprom/ee1004.c
+++ b/drivers/misc/eeprom/ee1004.c
@@ -59,6 +59,12 @@ static const struct i2c_device_id ee1004_ids[] = {
 };
 MODULE_DEVICE_TABLE(i2c, ee1004_ids);
 
+static const struct of_device_id ee1004_of_match[] = {
+	{ .compatible = "atmel,at30tse" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, ee1004_of_match);
+
 /*-------------------------------------------------------------------------*/
 
 static int ee1004_get_current_page(struct ee1004_bus *bus)
@@ -270,6 +276,7 @@ static struct i2c_driver ee1004_driver = {
 	.driver = {
 		.name = "ee1004",
 		.dev_groups = ee1004_groups,
+		.of_match_table = ee1004_of_match,
 	},
 	.probe_new = ee1004_probe,
 	.remove = ee1004_remove,
-- 
2.31.1

Re: [PATCH v2 3/4] eeprom: ee1004: Add devicetree binding

[Krzysztof Kozlowski]

On 21/03/2023 16:16, Eddie James wrote:
> Add an OF match table for devicetree instantiation of EE1004
> devices.

Subject: There is no device tree binding here. You add OF matching (or
support) to the driver.

> 
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
>  drivers/misc/eeprom/ee1004.c | 7 +++++++
>  1 file changed, 7 insertions(+)

Best regards,
Krzysztof

[PATCH v2 4/4] ARM: dts: aspeed: bonnell: Add DIMM SPD

[Eddie James]

Add the DIMM SPD to the processor I2C busses.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 arch/arm/boot/dts/aspeed-bmc-ibm-bonnell.dts | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed-bmc-ibm-bonnell.dts b/arch/arm/boot/dts/aspeed-bmc-ibm-bonnell.dts
index 79516dc21c01..72186020e75a 100644
--- a/arch/arm/boot/dts/aspeed-bmc-ibm-bonnell.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-ibm-bonnell.dts
@@ -232,18 +232,38 @@ cfam0_i2c1: i2c-bus@1 {
 
 			cfam0_i2c10: i2c-bus@a {
 				reg = <10>;	/* OP3A */
+
+				eeprom@50 {
+					compatible = "atmel,at30tse";
+					reg = <0x50>;
+				};
 			};
 
 			cfam0_i2c11: i2c-bus@b {
 				reg = <11>;	/* OP3B */
+
+				eeprom@50 {
+					compatible = "atmel,at30tse";
+					reg = <0x50>;
+				};
 			};
 
 			cfam0_i2c12: i2c-bus@c {
 				reg = <12>;	/* OP4A */
+
+				eeprom@50 {
+					compatible = "atmel,at30tse";
+					reg = <0x50>;
+				};
 			};
 
 			cfam0_i2c13: i2c-bus@d {
 				reg = <13>;	/* OP4B */
+
+				eeprom@50 {
+					compatible = "atmel,at30tse";
+					reg = <0x50>;
+				};
 			};
 
 			cfam0_i2c14: i2c-bus@e {
-- 
2.31.1