[PATCH 00/13] eeprom: ee1004: improvements

[PATCH 00/13] eeprom: ee1004: improvements

[Heiner Kallweit]

This series includes a number of improvements to the ee1004 driver.

Heiner Kallweit (13):
  eeprom: ee1004: Use kobj_to_i2c_client to simplify the code
  eeprom: ee1004: Remove not needed check in ee1004_read
  eeprom: ee1004: Remove not needed check in ee1004_eeprom_read
  eeprom: ee1004: Remove usage of i2c_adapter_id in adapter comparison
  eeprom: ee1004: Improve check for SMBUS features
  eeprom: ee1004: Improve creating dummy devices
  eeprom: ee1004: Switch to i2c probe_new callback
  eeprom: ee1004: Cache current page at initialization of first device
    only
  eeprom: ee1004: Factor out setting page to ee1004_set_current_page
  eeprom: ee1004: Improve error handling in ee1004_read
  eeprom: ee1004: Move call to ee1004_set_current_page to
    ee1004_eeprom_read
  eeprom: ee1004: Add constant EE1004_NUM_PAGES
  eeprom: ee1004: Add helper ee1004_cleanup

 drivers/misc/eeprom/ee1004.c | 191 +++++++++++++++--------------------
 1 file changed, 80 insertions(+), 111 deletions(-)

-- 
2.31.1

[PATCH 01/13] eeprom: ee1004: Use kobj_to_i2c_client to simplify the code

[Heiner Kallweit]

Switch to helper kobj_to_i2c_client() to simplify the code.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/misc/eeprom/ee1004.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/misc/eeprom/ee1004.c b/drivers/misc/eeprom/ee1004.c
index 0950d4d9d..0613a5300 100644
--- a/drivers/misc/eeprom/ee1004.c
+++ b/drivers/misc/eeprom/ee1004.c
@@ -93,8 +93,7 @@ static ssize_t eeprom_read(struct file *filp, struct kobject *kobj,
 			   struct bin_attribute *bin_attr,
 			   char *buf, loff_t off, size_t count)
 {
-	struct device *dev = kobj_to_dev(kobj);
-	struct i2c_client *client = to_i2c_client(dev);
+	struct i2c_client *client = kobj_to_i2c_client(kobj);
 	size_t requested = count;
 	int page;
 
-- 
2.31.1

[PATCH 02/13] eeprom: ee1004: Remove not needed check in ee1004_read

[Heiner Kallweit]

sysfs_kf_bin_read() checks this for us already. In addition
the function works correctly also w/o this check.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/misc/eeprom/ee1004.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/misc/eeprom/ee1004.c b/drivers/misc/eeprom/ee1004.c
index 0613a5300..6aff333ff 100644
--- a/drivers/misc/eeprom/ee1004.c
+++ b/drivers/misc/eeprom/ee1004.c
@@ -97,9 +97,6 @@ static ssize_t eeprom_read(struct file *filp, struct kobject *kobj,
 	size_t requested = count;
 	int page;
 
-	if (unlikely(!count))
-		return count;
-
 	page = off >> EE1004_PAGE_SHIFT;
 	if (unlikely(page > 1))
 		return 0;
-- 
2.31.1

[PATCH 03/13] eeprom: ee1004: Remove not needed check in ee1004_eeprom_read

[Heiner Kallweit]

i2c_smbus_read_i2c_block_data_or_emulated() checks its length argument,
so we don't have to do it. In addition remove the unlikely hint from
the checks, we do i2c reads and therefore are in a slow path.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/misc/eeprom/ee1004.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/misc/eeprom/ee1004.c b/drivers/misc/eeprom/ee1004.c
index 6aff333ff..2824dba76 100644
--- a/drivers/misc/eeprom/ee1004.c
+++ b/drivers/misc/eeprom/ee1004.c
@@ -76,10 +76,8 @@ static ssize_t ee1004_eeprom_read(struct i2c_client *client, char *buf,
 {
 	int status;
 
-	if (count > I2C_SMBUS_BLOCK_MAX)
-		count = I2C_SMBUS_BLOCK_MAX;
 	/* Can't cross page boundaries */
-	if (unlikely(offset + count > EE1004_PAGE_SIZE))
+	if (offset + count > EE1004_PAGE_SIZE)
 		count = EE1004_PAGE_SIZE - offset;
 
 	status = i2c_smbus_read_i2c_block_data_or_emulated(client, offset,
-- 
2.31.1

[PATCH 04/13] eeprom: ee1004: Remove usage of i2c_adapter_id in adapter comparison

[Heiner Kallweit]

We can compare the adapter pointers directly instead of using
i2c_adapter_id().

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/misc/eeprom/ee1004.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/misc/eeprom/ee1004.c b/drivers/misc/eeprom/ee1004.c
index 2824dba76..b991ab250 100644
--- a/drivers/misc/eeprom/ee1004.c
+++ b/drivers/misc/eeprom/ee1004.c
@@ -199,8 +199,7 @@ static int ee1004_probe(struct i2c_client *client,
 				goto err_clients;
 			}
 		}
-	} else if (i2c_adapter_id(client->adapter) !=
-		   i2c_adapter_id(ee1004_set_page[0]->adapter)) {
+	} 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;
-- 
2.31.1

[PATCH 05/13] eeprom: ee1004: Improve check for SMBUS features

[Heiner Kallweit]

We have to read 512 bytes only, therefore read performance isn't really
a concern. Don't bother the user if i2c block read isn't supported.

For i2c_smbus_read_i2c_block_data_or_emulated() to work it's sufficient
if I2C_FUNC_SMBUS_READ_I2C_BLOCK or I2C_FUNC_SMBUS_READ_BYTE_DATA is
supported. Therefore remove the check for I2C_FUNC_SMBUS_READ_WORD_DATA.

In addition check for I2C_FUNC_SMBUS_WRITE_BYTE (included in
I2C_FUNC_SMBUS_BYTE) which is needed for setting the page.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/misc/eeprom/ee1004.c | 22 ++++------------------
 1 file changed, 4 insertions(+), 18 deletions(-)

diff --git a/drivers/misc/eeprom/ee1004.c b/drivers/misc/eeprom/ee1004.c
index b991ab250..0d497e0e4 100644
--- a/drivers/misc/eeprom/ee1004.c
+++ b/drivers/misc/eeprom/ee1004.c
@@ -167,23 +167,13 @@ static int ee1004_probe(struct i2c_client *client,
 			const struct i2c_device_id *id)
 {
 	int err, cnr = 0;
-	const char *slow = NULL;
 
 	/* Make sure we can operate on this adapter */
 	if (!i2c_check_functionality(client->adapter,
-				     I2C_FUNC_SMBUS_READ_BYTE |
-				     I2C_FUNC_SMBUS_READ_I2C_BLOCK)) {
-		if (i2c_check_functionality(client->adapter,
-				     I2C_FUNC_SMBUS_READ_BYTE |
-				     I2C_FUNC_SMBUS_READ_WORD_DATA))
-			slow = "word";
-		else if (i2c_check_functionality(client->adapter,
-				     I2C_FUNC_SMBUS_READ_BYTE |
-				     I2C_FUNC_SMBUS_READ_BYTE_DATA))
-			slow = "byte";
-		else
-			return -EPFNOSUPPORT;
-	}
+				     I2C_FUNC_SMBUS_BYTE | I2C_FUNC_SMBUS_READ_I2C_BLOCK) &&
+	    !i2c_check_functionality(client->adapter,
+				     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);
@@ -218,10 +208,6 @@ static int ee1004_probe(struct i2c_client *client,
 	dev_info(&client->dev,
 		 "%u byte EE1004-compliant SPD EEPROM, read-only\n",
 		 EE1004_EEPROM_SIZE);
-	if (slow)
-		dev_notice(&client->dev,
-			   "Falling back to %s reads, performance will suffer\n",
-			   slow);
 
 	return 0;
 
-- 
2.31.1

[PATCH 06/13] eeprom: ee1004: Improve creating dummy devices

[Heiner Kallweit]

i2c_new_dummy_device() calls i2c_new_client_device() that complains
if it fails to create the device. Therefore we don't have to emit an
error message in case of failure. In addition ensure that
ee1004_set_page is only set if creating the device succeeded.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/misc/eeprom/ee1004.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/misc/eeprom/ee1004.c b/drivers/misc/eeprom/ee1004.c
index 0d497e0e4..4b2c60a18 100644
--- a/drivers/misc/eeprom/ee1004.c
+++ b/drivers/misc/eeprom/ee1004.c
@@ -179,15 +179,14 @@ static int ee1004_probe(struct i2c_client *client,
 	mutex_lock(&ee1004_bus_lock);
 	if (++ee1004_dev_count == 1) {
 		for (cnr = 0; cnr < 2; cnr++) {
-			ee1004_set_page[cnr] = i2c_new_dummy_device(client->adapter,
-						EE1004_ADDR_SET_PAGE + cnr);
-			if (IS_ERR(ee1004_set_page[cnr])) {
-				dev_err(&client->dev,
-					"address 0x%02x unavailable\n",
-					EE1004_ADDR_SET_PAGE + cnr);
-				err = PTR_ERR(ee1004_set_page[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;
 		}
 	} else if (client->adapter != ee1004_set_page[0]->adapter) {
 		dev_err(&client->dev,
-- 
2.31.1

[PATCH 07/13] eeprom: ee1004: Switch to i2c probe_new callback

[Heiner Kallweit]

Switch to the new i2c_driver probe callback version.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/misc/eeprom/ee1004.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/misc/eeprom/ee1004.c b/drivers/misc/eeprom/ee1004.c
index 4b2c60a18..460cc22ea 100644
--- a/drivers/misc/eeprom/ee1004.c
+++ b/drivers/misc/eeprom/ee1004.c
@@ -163,8 +163,7 @@ static struct bin_attribute *ee1004_attrs[] = {
 
 BIN_ATTRIBUTE_GROUPS(ee1004);
 
-static int ee1004_probe(struct i2c_client *client,
-			const struct i2c_device_id *id)
+static int ee1004_probe(struct i2c_client *client)
 {
 	int err, cnr = 0;
 
@@ -246,7 +245,7 @@ static struct i2c_driver ee1004_driver = {
 		.name = "ee1004",
 		.dev_groups = ee1004_groups,
 	},
-	.probe = ee1004_probe,
+	.probe_new = ee1004_probe,
 	.remove = ee1004_remove,
 	.id_table = ee1004_ids,
 };
-- 
2.31.1

[PATCH 08/13] eeprom: ee1004: Cache current page at initialization of first device only

[Heiner Kallweit]

The value of ee1004_current_page applies to all SPD eeproms connected
to the adapter. Therefore it's sufficient if we set ee1004_current_page
when the first device is added.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/misc/eeprom/ee1004.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/misc/eeprom/ee1004.c b/drivers/misc/eeprom/ee1004.c
index 460cc22ea..d7c693b26 100644
--- a/drivers/misc/eeprom/ee1004.c
+++ b/drivers/misc/eeprom/ee1004.c
@@ -187,20 +187,19 @@ static int ee1004_probe(struct i2c_client *client)
 			}
 			ee1004_set_page[cnr] = cl;
 		}
+
+		/* 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;
 	}
-
-	/* Remember current page to avoid unneeded page select */
-	err = ee1004_get_current_page();
-	if (err < 0)
-		goto err_clients;
-	ee1004_current_page = err;
-	dev_dbg(&client->dev, "Currently selected page: %d\n",
-		ee1004_current_page);
 	mutex_unlock(&ee1004_bus_lock);
 
 	dev_info(&client->dev,
-- 
2.31.1

[PATCH 09/13] eeprom: ee1004: Factor out setting page to ee1004_set_current_page

[Heiner Kallweit]

Factor out setting the page, this makes the code better readable.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/misc/eeprom/ee1004.c | 52 +++++++++++++++++++++---------------
 1 file changed, 30 insertions(+), 22 deletions(-)

diff --git a/drivers/misc/eeprom/ee1004.c b/drivers/misc/eeprom/ee1004.c
index d7c693b26..33855e459 100644
--- a/drivers/misc/eeprom/ee1004.c
+++ b/drivers/misc/eeprom/ee1004.c
@@ -71,6 +71,32 @@ static int ee1004_get_current_page(void)
 	return 0;
 }
 
+static int ee1004_set_current_page(struct device *dev, int page)
+{
+	int ret;
+
+	if (page == ee1004_current_page)
+		return 0;
+
+	/* Data is ignored */
+	ret = i2c_smbus_write_byte(ee1004_set_page[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)
+		ret = 0;
+	if (ret < 0) {
+		dev_err(dev, "Failed to select page %d (%d)\n", page, ret);
+		return ret;
+	}
+
+	dev_dbg(dev, "Selected page %d\n", page);
+	ee1004_current_page = page;
+
+	return 0;
+}
+
 static ssize_t ee1004_eeprom_read(struct i2c_client *client, char *buf,
 				  unsigned int offset, size_t count)
 {
@@ -110,28 +136,10 @@ static ssize_t eeprom_read(struct file *filp, struct kobject *kobj,
 		int status;
 
 		/* Select page */
-		if (page != ee1004_current_page) {
-			/* 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);
-				mutex_unlock(&ee1004_bus_lock);
-				return status;
-			}
-			dev_dbg(dev, "Selected page %d\n", page);
-			ee1004_current_page = page;
+		status = ee1004_set_current_page(dev, page);
+		if (status) {
+			mutex_unlock(&ee1004_bus_lock);
+			return status;
 		}
 
 		status = ee1004_eeprom_read(client, buf, off, count);
-- 
2.31.1

[PATCH 10/13] eeprom: ee1004: Improve error handling in ee1004_read

[Heiner Kallweit]

Simplify the error handling and make it better readable. No functional
change intended.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/misc/eeprom/ee1004.c | 31 +++++++++++++------------------
 1 file changed, 13 insertions(+), 18 deletions(-)

diff --git a/drivers/misc/eeprom/ee1004.c b/drivers/misc/eeprom/ee1004.c
index 33855e459..d18348ee4 100644
--- a/drivers/misc/eeprom/ee1004.c
+++ b/drivers/misc/eeprom/ee1004.c
@@ -119,7 +119,7 @@ static ssize_t eeprom_read(struct file *filp, struct kobject *kobj,
 {
 	struct i2c_client *client = kobj_to_i2c_client(kobj);
 	size_t requested = count;
-	int page;
+	int page, ret = 0;
 
 	page = off >> EE1004_PAGE_SHIFT;
 	if (unlikely(page > 1))
@@ -133,33 +133,28 @@ static ssize_t eeprom_read(struct file *filp, struct kobject *kobj,
 	mutex_lock(&ee1004_bus_lock);
 
 	while (count) {
-		int status;
-
 		/* Select page */
-		status = ee1004_set_current_page(dev, page);
-		if (status) {
-			mutex_unlock(&ee1004_bus_lock);
-			return status;
-		}
+		ret = ee1004_set_current_page(dev, page);
+		if (ret)
+			goto out;
 
-		status = ee1004_eeprom_read(client, buf, off, count);
-		if (status < 0) {
-			mutex_unlock(&ee1004_bus_lock);
-			return status;
-		}
-		buf += status;
-		off += status;
-		count -= status;
+		ret = ee1004_eeprom_read(client, buf, off, count);
+		if (ret < 0)
+			goto out;
+
+		buf += ret;
+		off += ret;
+		count -= ret;
 
 		if (off == EE1004_PAGE_SIZE) {
 			page++;
 			off = 0;
 		}
 	}
-
+out:
 	mutex_unlock(&ee1004_bus_lock);
 
-	return requested;
+	return ret < 0 ? ret : requested;
 }
 
 static BIN_ATTR_RO(eeprom, EE1004_EEPROM_SIZE);
-- 
2.31.1

[PATCH 11/13] eeprom: ee1004: Move call to ee1004_set_current_page to ee1004_eeprom_read

[Heiner Kallweit]

Moving the call to ee1004_set_current_page() to ee1004_eeprom_read()
allows to simplify the code.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/misc/eeprom/ee1004.c | 26 +++++++++-----------------
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/drivers/misc/eeprom/ee1004.c b/drivers/misc/eeprom/ee1004.c
index d18348ee4..65fe11d8f 100644
--- a/drivers/misc/eeprom/ee1004.c
+++ b/drivers/misc/eeprom/ee1004.c
@@ -100,7 +100,14 @@ static int ee1004_set_current_page(struct device *dev, int page)
 static ssize_t ee1004_eeprom_read(struct i2c_client *client, char *buf,
 				  unsigned int offset, size_t count)
 {
-	int status;
+	int status, page;
+
+	page = offset >> EE1004_PAGE_SHIFT;
+	offset &= (1 << EE1004_PAGE_SHIFT) - 1;
+
+	status = ee1004_set_current_page(&client->dev, page);
+	if (status)
+		return status;
 
 	/* Can't cross page boundaries */
 	if (offset + count > EE1004_PAGE_SIZE)
@@ -119,12 +126,7 @@ static ssize_t eeprom_read(struct file *filp, struct kobject *kobj,
 {
 	struct i2c_client *client = kobj_to_i2c_client(kobj);
 	size_t requested = count;
-	int page, ret = 0;
-
-	page = off >> EE1004_PAGE_SHIFT;
-	if (unlikely(page > 1))
-		return 0;
-	off &= (1 << EE1004_PAGE_SHIFT) - 1;
+	int ret = 0;
 
 	/*
 	 * Read data from chip, protecting against concurrent access to
@@ -133,11 +135,6 @@ static ssize_t eeprom_read(struct file *filp, struct kobject *kobj,
 	mutex_lock(&ee1004_bus_lock);
 
 	while (count) {
-		/* Select page */
-		ret = ee1004_set_current_page(dev, page);
-		if (ret)
-			goto out;
-
 		ret = ee1004_eeprom_read(client, buf, off, count);
 		if (ret < 0)
 			goto out;
@@ -145,11 +142,6 @@ static ssize_t eeprom_read(struct file *filp, struct kobject *kobj,
 		buf += ret;
 		off += ret;
 		count -= ret;
-
-		if (off == EE1004_PAGE_SIZE) {
-			page++;
-			off = 0;
-		}
 	}
 out:
 	mutex_unlock(&ee1004_bus_lock);
-- 
2.31.1

[PATCH 12/13] eeprom: ee1004: Add constant EE1004_NUM_PAGES

[Heiner Kallweit]

Add a constant for the number of pages.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/misc/eeprom/ee1004.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/misc/eeprom/ee1004.c b/drivers/misc/eeprom/ee1004.c
index 65fe11d8f..5173d040c 100644
--- a/drivers/misc/eeprom/ee1004.c
+++ b/drivers/misc/eeprom/ee1004.c
@@ -32,16 +32,17 @@
  */
 
 #define EE1004_ADDR_SET_PAGE		0x36
-#define EE1004_EEPROM_SIZE		512
+#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[2];
+static struct i2c_client *ee1004_set_page[EE1004_NUM_PAGES];
 static unsigned int ee1004_dev_count;
 static int ee1004_current_page;
 
@@ -172,7 +173,7 @@ static int ee1004_probe(struct i2c_client *client)
 	/* Use 2 dummy devices for page select command */
 	mutex_lock(&ee1004_bus_lock);
 	if (++ee1004_dev_count == 1) {
-		for (cnr = 0; cnr < 2; cnr++) {
+		for (cnr = 0; cnr < EE1004_NUM_PAGES; cnr++) {
 			struct i2c_client *cl;
 
 			cl = i2c_new_dummy_device(client->adapter, EE1004_ADDR_SET_PAGE + cnr);
@@ -222,7 +223,7 @@ static int ee1004_remove(struct i2c_client *client)
 	/* Remove page select clients if this is the last device */
 	mutex_lock(&ee1004_bus_lock);
 	if (--ee1004_dev_count == 0) {
-		for (i = 0; i < 2; i++) {
+		for (i = 0; i < EE1004_NUM_PAGES; i++) {
 			i2c_unregister_device(ee1004_set_page[i]);
 			ee1004_set_page[i] = NULL;
 		}
-- 
2.31.1

[PATCH 13/13] eeprom: ee1004: Add helper ee1004_cleanup

[Heiner Kallweit]

Factor out the cleanup code to a new helper ee1004_cleanup().

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/misc/eeprom/ee1004.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/misc/eeprom/ee1004.c b/drivers/misc/eeprom/ee1004.c
index 5173d040c..00f61a83d 100644
--- a/drivers/misc/eeprom/ee1004.c
+++ b/drivers/misc/eeprom/ee1004.c
@@ -159,6 +159,15 @@ static struct bin_attribute *ee1004_attrs[] = {
 
 BIN_ATTRIBUTE_GROUPS(ee1004);
 
+static void ee1004_cleanup(int idx)
+{
+	if (--ee1004_dev_count == 0)
+		while (--idx >= 0) {
+			i2c_unregister_device(ee1004_set_page[idx]);
+			ee1004_set_page[idx] = NULL;
+		}
+}
+
 static int ee1004_probe(struct i2c_client *client)
 {
 	int err, cnr = 0;
@@ -205,12 +214,7 @@ static int ee1004_probe(struct i2c_client *client)
 	return 0;
 
  err_clients:
-	if (--ee1004_dev_count == 0) {
-		for (cnr--; cnr >= 0; cnr--) {
-			i2c_unregister_device(ee1004_set_page[cnr]);
-			ee1004_set_page[cnr] = NULL;
-		}
-	}
+	ee1004_cleanup(cnr);
 	mutex_unlock(&ee1004_bus_lock);
 
 	return err;
@@ -218,16 +222,9 @@ static int ee1004_probe(struct i2c_client *client)
 
 static int ee1004_remove(struct i2c_client *client)
 {
-	int i;
-
 	/* Remove page select clients if this is the last device */
 	mutex_lock(&ee1004_bus_lock);
-	if (--ee1004_dev_count == 0) {
-		for (i = 0; i < EE1004_NUM_PAGES; i++) {
-			i2c_unregister_device(ee1004_set_page[i]);
-			ee1004_set_page[i] = NULL;
-		}
-	}
+	ee1004_cleanup(EE1004_NUM_PAGES);
 	mutex_unlock(&ee1004_bus_lock);
 
 	return 0;
-- 
2.31.1