diff mbox series

eeprom: ee1004: Add support for multiple i2c busses

Message ID ab18528d-e0be-49f6-8ea3-1f4e73fa353a@gmail.com
State Superseded
Headers show
Series eeprom: ee1004: Add support for multiple i2c busses | expand

Commit Message

Heiner Kallweit Nov. 15, 2023, 3:30 p.m. UTC
There are systems with more than 8 memory slots where the i2c bus for
SPD is multiplexed. i2c_register_spd() isn't used yet on such systems,
but it's planned. So we need to extend ee1004 accordingly.
With this extension a maximum of 8 i2c busses is supported.

I don't have such a system for testing, therefore I just verified
that the driver still works on a system with a single i2c bus.

For the sake of simplicity the extension uses the existing global
mutex to protect access on all busses. This could be improved,
but we support 8 busses only, and SPD data is small and rarely
accessed, so it shouldn't be a problem.

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

Comments

Heiner Kallweit Nov. 17, 2023, 6:41 a.m. UTC | #1
On 15.11.2023 16:30, Heiner Kallweit wrote:
> There are systems with more than 8 memory slots where the i2c bus for
> SPD is multiplexed. i2c_register_spd() isn't used yet on such systems,
> but it's planned. So we need to extend ee1004 accordingly.
> With this extension a maximum of 8 i2c busses is supported.
> 
> I don't have such a system for testing, therefore I just verified
> that the driver still works on a system with a single i2c bus.
> 
> For the sake of simplicity the extension uses the existing global
> mutex to protect access on all busses. This could be improved,
> but we support 8 busses only, and SPD data is small and rarely
> accessed, so it shouldn't be a problem.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---

I sent a v2 that improves the code.
diff mbox series

Patch

diff --git a/drivers/misc/eeprom/ee1004.c b/drivers/misc/eeprom/ee1004.c
index a1acd7713..e154caf5a 100644
--- a/drivers/misc/eeprom/ee1004.c
+++ b/drivers/misc/eeprom/ee1004.c
@@ -31,6 +31,7 @@ 
  * over performance.
  */
 
+#define EE1004_MAX_BUSSES		8
 #define EE1004_ADDR_SET_PAGE		0x36
 #define EE1004_NUM_PAGES		2
 #define EE1004_PAGE_SIZE		256
@@ -42,9 +43,13 @@ 
  * 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;
+
+static struct ee1004_bus_data {
+	struct i2c_adapter *adap;
+	struct i2c_client *set_page[EE1004_NUM_PAGES];
+	unsigned int dev_count;
+	int current_page;
+} ee1004_bus_data[EE1004_MAX_BUSSES];
 
 static const struct i2c_device_id ee1004_ids[] = {
 	{ "ee1004", 0 },
@@ -54,11 +59,30 @@  MODULE_DEVICE_TABLE(i2c, ee1004_ids);
 
 /*-------------------------------------------------------------------------*/
 
-static int ee1004_get_current_page(void)
+static int ee1004_get_bus_data_idx(struct i2c_adapter *adap)
+{
+	int i;
+
+	for (i = 0; i < EE1004_MAX_BUSSES; i++)
+		if (ee1004_bus_data[i].adap == adap)
+			return i;
+
+	/* If not existent yet, create new entry */
+	for (i = 0; i < EE1004_MAX_BUSSES; i++)
+		if (!ee1004_bus_data[i].adap) {
+			ee1004_bus_data[i].adap = adap;
+			return i;
+		}
+
+	return -ENOSPC;
+}
+
+static int ee1004_get_current_page(int bus_data_idx)
 {
+	struct ee1004_bus_data *bd = ee1004_bus_data + bus_data_idx;
 	int err;
 
-	err = i2c_smbus_read_byte(ee1004_set_page[0]);
+	err = i2c_smbus_read_byte(bd->set_page[0]);
 	if (err == -ENXIO) {
 		/* Nack means page 1 is selected */
 		return 1;
@@ -72,28 +96,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 i2c_client *client, int page)
 {
+	int bd_idx = ee1004_get_bus_data_idx(client->adapter);
+	struct ee1004_bus_data *bd = ee1004_bus_data + bd_idx;
 	int ret;
 
-	if (page == ee1004_current_page)
+	if (page == bd->current_page)
 		return 0;
 
 	/* Data is ignored */
-	ret = i2c_smbus_write_byte(ee1004_set_page[page], 0x00);
+	ret = i2c_smbus_write_byte(bd->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)
+	if (ret == -ENXIO && ee1004_get_current_page(bd_idx) == page)
 		ret = 0;
 	if (ret < 0) {
-		dev_err(dev, "Failed to select page %d (%d)\n", page, ret);
+		dev_err(&client->dev, "Failed to select page %d (%d)\n", page, ret);
 		return ret;
 	}
 
-	dev_dbg(dev, "Selected page %d\n", page);
-	ee1004_current_page = page;
+	dev_dbg(&client->dev, "Selected page %d\n", page);
+	bd->current_page = page;
 
 	return 0;
 }
@@ -106,7 +132,7 @@  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);
+	status = ee1004_set_current_page(client, page);
 	if (status)
 		return status;
 
@@ -158,18 +184,21 @@  static struct bin_attribute *ee1004_attrs[] = {
 
 BIN_ATTRIBUTE_GROUPS(ee1004);
 
-static void ee1004_cleanup(int idx)
+static void ee1004_cleanup(int idx, int bus_data_idx)
 {
-	if (--ee1004_dev_count == 0)
-		while (--idx >= 0) {
-			i2c_unregister_device(ee1004_set_page[idx]);
-			ee1004_set_page[idx] = NULL;
-		}
+	struct ee1004_bus_data *bd = ee1004_bus_data + bus_data_idx;
+
+	if (--bd->dev_count == 0) {
+		while (--idx >= 0)
+			i2c_unregister_device(bd->set_page[idx]);
+		memset(bd, 0, sizeof(struct ee1004_bus_data));
+	}
 }
 
 static int ee1004_probe(struct i2c_client *client)
 {
-	int err, cnr = 0;
+	struct ee1004_bus_data *bd;
+	int err, bd_idx, cnr = 0;
 
 	/* Make sure we can operate on this adapter */
 	if (!i2c_check_functionality(client->adapter,
@@ -178,9 +207,19 @@  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) {
+
+	bd_idx = ee1004_get_bus_data_idx(client->adapter);
+	if (bd_idx < 0) {
+		mutex_unlock(&ee1004_bus_lock);
+		return dev_err_probe(&client->dev, bd_idx,
+				     "Only %d busses supported", EE1004_MAX_BUSSES);
+	}
+
+	bd = ee1004_bus_data + bd_idx;
+
+	if (++bd->dev_count == 1) {
+		/* Use 2 dummy devices for page select command */
 		for (cnr = 0; cnr < EE1004_NUM_PAGES; cnr++) {
 			struct i2c_client *cl;
 
@@ -189,20 +228,15 @@  static int ee1004_probe(struct i2c_client *client)
 				err = PTR_ERR(cl);
 				goto err_clients;
 			}
-			ee1004_set_page[cnr] = cl;
+			bd->set_page[cnr] = cl;
 		}
 
 		/* Remember current page to avoid unneeded page select */
-		err = ee1004_get_current_page();
+		err = ee1004_get_current_page(bd_idx);
 		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;
+		bd->current_page = err;
 	}
 	mutex_unlock(&ee1004_bus_lock);
 
@@ -213,7 +247,7 @@  static int ee1004_probe(struct i2c_client *client)
 	return 0;
 
  err_clients:
-	ee1004_cleanup(cnr);
+	ee1004_cleanup(cnr, bd_idx);
 	mutex_unlock(&ee1004_bus_lock);
 
 	return err;
@@ -221,9 +255,12 @@  static int ee1004_probe(struct i2c_client *client)
 
 static void ee1004_remove(struct i2c_client *client)
 {
+	int bd_idx;
+
 	/* Remove page select clients if this is the last device */
 	mutex_lock(&ee1004_bus_lock);
-	ee1004_cleanup(EE1004_NUM_PAGES);
+	bd_idx = ee1004_get_bus_data_idx(client->adapter);
+	ee1004_cleanup(EE1004_NUM_PAGES, bd_idx);
 	mutex_unlock(&ee1004_bus_lock);
 }