diff mbox series

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

Message ID 20190506151656.47494e56@endymion
State Awaiting Upstream
Headers show
Series [1/2] eeprom: ee1004: Move selected page detection to a separate function | expand

Commit Message

Jean Delvare May 6, 2019, 1:16 p.m. UTC
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.

Signed-off-by: Jean Delvare <jdelvare@suse.de>
Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/misc/eeprom/ee1004.c |   12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Comments

Jarkko Nikula May 6, 2019, 2:03 p.m. UTC | #1
On 5/6/19 4:16 PM, Jean Delvare wrote:
> 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.
> 
> Signed-off-by: Jean Delvare <jdelvare@suse.de>
> Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>   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)
Dreamcat4 May 6, 2019, 2:30 p.m. UTC | #2
Don't mind either way, please do as you see fit

On Mon, May 6, 2019 at 3:04 PM Jarkko Nikula
<jarkko.nikula@linux.intel.com> wrote:
>
> On 5/6/19 4:16 PM, Jean Delvare wrote:
> > 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.
> >
> > Signed-off-by: Jean Delvare <jdelvare@suse.de>
> > Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> >   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
Jean Delvare May 6, 2019, 3:16 p.m. UTC | #3
On Mon, 6 May 2019 17:03:20 +0300, Jarkko Nikula wrote:
> On 5/6/19 4:16 PM, Jean Delvare wrote:
> > 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.
> > 
> > Signed-off-by: Jean Delvare <jdelvare@suse.de>
> > Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> >   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?

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

> (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.
diff mbox series

Patch

--- linux-5.0.orig/drivers/misc/eeprom/ee1004.c	2019-05-06 11:57:14.333572368 +0200
+++ linux-5.0/drivers/misc/eeprom/ee1004.c	2019-05-06 15:11:06.009718220 +0200
@@ -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);