Skip to contentSkip to conversation

eeprom: ee1004: Move selected page detection to a separate function

PATCH In review Jean Delvare 3 participants 5 messages May 6, 2019

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

PATCH 1/2 eeprom: ee1004: Move selected page detection to a separate function

No functional change, this is in preparation for future needs.

---
 drivers/misc/eeprom/ee1004.c |   31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)
drivers/misc/eeprom/ee1004.c +21 -10
@@ -57,6 +57,24 @@ MODULE_DEVICE_TABLE(i2c, ee1004_ids);
 
 /*-------------------------------------------------------------------------*/
 
+static int ee1004_get_current_page(void)
+{
+	int err;
+
+	err = i2c_smbus_read_byte(ee1004_set_page[0]);
+	if (err == -ENXIO) {
+		/* Nack means page 1 is selected */
+		return 1;
+	}
+	if (err < 0) {
+		/* Anything else is a real error, bail out */
+		return err;
+	}
+
+	/* Ack means page 0 is selected, returned value meaningless */
+	return 0;
+}
+
 static ssize_t ee1004_eeprom_read(struct i2c_client *client, char *buf,
 				  unsigned int offset, size_t count)
 {
@@ -190,17 +208,10 @@ static int ee1004_probe(struct i2c_clien
 	}
 
 	/* Remember current page to avoid unneeded page select */
-	err = i2c_smbus_read_byte(ee1004_set_page[0]);
-	if (err == -ENXIO) {
-		/* Nack means page 1 is selected */
-		ee1004_current_page = 1;
-	} else if (err < 0) {
-		/* Anything else is a real error, bail out */
+	err = ee1004_get_current_page();
+	if (err < 0)
 		goto err_clients;
-	} else {
-		/* Ack means page 0 is selected, returned value meaningless */
-		ee1004_current_page = 0;
-	}
+	ee1004_current_page = err;
 	dev_dbg(&client->dev, "Currently selected page: %d\n",
 		ee1004_current_page);
 	mutex_unlock(&ee1004_bus_lock);
Signature · 2 lines
Jean Delvare
SUSE L3 Support
#1 lore raw

PATCH 2/2 eeprom: ee1004: Deal with nack on page selection

Some EE1004 implementations will not properly ack page selection commands. They still set the page correctly, so there is no actual error. Deal with this case gracefully by checking the currently selected page after we receive a nack. If the page is set right then we can continue.

---
 drivers/misc/eeprom/ee1004.c |   12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)
drivers/misc/eeprom/ee1004.c +11 -1
@@ -1,7 +1,7 @@
 /*
  * ee1004 - driver for DDR4 SPD EEPROMs
  *
- * Copyright (C) 2017 Jean Delvare
+ * Copyright (C) 2017-2019 Jean Delvare
  *
  * Based on the at24 driver:
  * Copyright (C) 2005-2007 David Brownell
@@ -124,6 +124,16 @@ static ssize_t ee1004_read(struct file *
 			/* Data is ignored */
 			status = i2c_smbus_write_byte(ee1004_set_page[page],
 						      0x00);
+			if (status == -ENXIO) {
+				/*
+				 * 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 (ee1004_get_current_page() == page)
+					status = 0;
+			}
 			if (status < 0) {
 				dev_err(dev, "Failed to select page %d (%d)\n",
 					page, status);
Signature · 2 lines
Jean Delvare
SUSE L3 Support
#2 lore raw

Re: [PATCH 2/2] eeprom: ee1004: Deal with nack on page selection

Quoted 6 lines · Jean Delvare

Some EE1004 implementations will not properly ack page selection commands. They still set the page correctly, so there is no actual error. Deal with this case gracefully by checking the currently selected page after we receive a nack. If the page is set right then we can continue.

Quoted 8 lines
---
  drivers/misc/eeprom/ee1004.c |   12 +++++++++++-
  1 file changed, 11 insertions(+), 1 deletion(-)

Does Dreamcat4 deserve reported and tested by tags here? I guess anonymous address is fine with those tags?

(I re-tested these two patches on top of v5.1 and they make decode-dimms working on a machine with 2-4 * Crucial DD4 dimms)

Signature · 1 line
Jarkko
#3 lore raw

Re: [PATCH 2/2] eeprom: ee1004: Deal with nack on page selection

Don't mind either way, please do as you see fit

Quoted 24 lines · Jarkko Nikula
Quoted 6 lines · Jean Delvare

Some EE1004 implementations will not properly ack page selection commands. They still set the page correctly, so there is no actual error. Deal with this case gracefully by checking the currently selected page after we receive a nack. If the page is set right then we can continue.

Quoted 8 lines
---
  drivers/misc/eeprom/ee1004.c |   12 +++++++++++-
  1 file changed, 11 insertions(+), 1 deletion(-)

Does Dreamcat4 deserve reported and tested by tags here? I guess anonymous address is fine with those tags?

(I re-tested these two patches on top of v5.1 and they make decode-dimms working on a machine with 2-4 * Crucial DD4 dimms)

--
Jarkko

#4 lore raw

Re: [PATCH 2/2] eeprom: ee1004: Deal with nack on page selection

In reply to Jarkko Nikula
Quoted 15 lines · Jarkko Nikula
Quoted 6 lines · Jean Delvare

Some EE1004 implementations will not properly ack page selection commands. They still set the page correctly, so there is no actual error. Deal with this case gracefully by checking the currently selected page after we receive a nack. If the page is set right then we can continue.

Quoted 7 lines
---
  drivers/misc/eeprom/ee1004.c |   12 +++++++++++-
  1 file changed, 11 insertions(+), 1 deletion(-)
Quoted 2 lines

Does Dreamcat4 deserve reported and tested by tags here? I guess anonymous address is fine with those tags?

My assumption is that someone who posts anonymously in the first place has no desire to be credited for anything or even mentioned anywhere.

Quoted 2 lines

(I re-tested these two patches on top of v5.1 and they make decode-dimms working on a machine with 2-4 * Crucial DD4 dimms)

Thank you very much.

Signature · 2 lines
Jean Delvare
SUSE L3 Support