[PATCH 0/5] hwmon: (spd5118) Add I3C support

[PATCH 0/5] hwmon: (spd5118) Add I3C support

[Guenter Roeck]

Add support for I3C to spd5118 driver.

The first patch splits the spd5118 code into common and I2C specific parts.

The second patch adds a note indicating which chips are known to take the
specification literally. This patch is purely informational and does not
change the code.

The third patch adds support for 16-bit addressing when accessing NVMEM.
16-bit addressing is configurable for I2C but mandatory for I3C.

The 4th patch adds support for detecting 16-bit addressing in I2C mode.

The last patch of the series actually adds I3C support. This patch is
untested and will not be applied until/unless test coverage is available.

----------------------------------------------------------------
Guenter Roeck (5):
      hwmon: (spd5118) Split into common and I2C specific code
      hwmon: (spd5118) Name chips taking the specification literally
      hwmon: (spd5118) Support 16-bit addressing for NVMEM accesses
      hwmon: (spd5118) Detect and support 16-bit register addressing
      hwmon: (spd5118) Add I3C support

 drivers/hwmon/Kconfig   |   2 +
 drivers/hwmon/spd5118.c | 408 +++++++++++++++++++++++++++++++-----------------
 2 files changed, 271 insertions(+), 139 deletions(-)

[PATCH 1/5] hwmon: (spd5118) Split into common and I2C specific code

[Guenter Roeck]

Split spd5118 driver into common and I2C specific code to enable
adding support for I3C.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/spd5118.c | 305 +++++++++++++++++++++-------------------
 1 file changed, 157 insertions(+), 148 deletions(-)

diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
index 358152868d96..02eb21684c3a 100644
--- a/drivers/hwmon/spd5118.c
+++ b/drivers/hwmon/spd5118.c
@@ -305,51 +305,6 @@ static bool spd5118_vendor_valid(u8 bank, u8 id)
 	return id && id != 0x7f;
 }
 
-/* Return 0 if detection is successful, -ENODEV otherwise */
-static int spd5118_detect(struct i2c_client *client, struct i2c_board_info *info)
-{
-	struct i2c_adapter *adapter = client->adapter;
-	int regval;
-
-	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA |
-				     I2C_FUNC_SMBUS_WORD_DATA))
-		return -ENODEV;
-
-	regval = i2c_smbus_read_word_swapped(client, SPD5118_REG_TYPE);
-	if (regval != 0x5118)
-		return -ENODEV;
-
-	regval = i2c_smbus_read_word_data(client, SPD5118_REG_VENDOR);
-	if (regval < 0 || !spd5118_vendor_valid(regval & 0xff, regval >> 8))
-		return -ENODEV;
-
-	regval = i2c_smbus_read_byte_data(client, SPD5118_REG_CAPABILITY);
-	if (regval < 0)
-		return -ENODEV;
-	if (!(regval & SPD5118_CAP_TS_SUPPORT) || (regval & 0xfc))
-		return -ENODEV;
-
-	regval = i2c_smbus_read_byte_data(client, SPD5118_REG_TEMP_CLR);
-	if (regval)
-		return -ENODEV;
-	regval = i2c_smbus_read_byte_data(client, SPD5118_REG_ERROR_CLR);
-	if (regval)
-		return -ENODEV;
-
-	regval = i2c_smbus_read_byte_data(client, SPD5118_REG_REVISION);
-	if (regval < 0 || (regval & 0xc1))
-		return -ENODEV;
-
-	regval = i2c_smbus_read_byte_data(client, SPD5118_REG_TEMP_CONFIG);
-	if (regval < 0)
-		return -ENODEV;
-	if (regval & ~SPD5118_TS_DISABLE)
-		return -ENODEV;
-
-	strscpy(info->type, "spd5118", I2C_NAME_SIZE);
-	return 0;
-}
-
 static const struct hwmon_channel_info *spd5118_info[] = {
 	HWMON_CHANNEL_INFO(chip,
 			   HWMON_C_REGISTER_TZ),
@@ -483,7 +438,7 @@ static bool spd5118_volatile_reg(struct device *dev, unsigned int reg)
 	}
 }
 
-static const struct regmap_range_cfg spd5118_regmap_range_cfg[] = {
+static const struct regmap_range_cfg spd5118_i2c_regmap_range_cfg[] = {
 	{
 	.selector_reg   = SPD5118_REG_I2C_LEGACY_MODE,
 	.selector_mask  = SPD5118_LEGACY_PAGE_MASK,
@@ -495,7 +450,7 @@ static const struct regmap_range_cfg spd5118_regmap_range_cfg[] = {
 	},
 };
 
-static const struct regmap_config spd5118_regmap_config = {
+static const struct regmap_config spd5118_i2c_regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 8,
 	.max_register = 0x7ff,
@@ -503,11 +458,153 @@ static const struct regmap_config spd5118_regmap_config = {
 	.volatile_reg = spd5118_volatile_reg,
 	.cache_type = REGCACHE_MAPLE,
 
-	.ranges = spd5118_regmap_range_cfg,
-	.num_ranges = ARRAY_SIZE(spd5118_regmap_range_cfg),
+	.ranges = spd5118_i2c_regmap_range_cfg,
+	.num_ranges = ARRAY_SIZE(spd5118_i2c_regmap_range_cfg),
 };
 
-static int spd5118_init(struct i2c_client *client)
+static int spd5118_suspend(struct device *dev)
+{
+	struct spd5118_data *data = dev_get_drvdata(dev);
+	struct regmap *regmap = data->regmap;
+	u32 regval;
+	int err;
+
+	/*
+	 * Make sure the configuration register in the regmap cache is current
+	 * before bypassing it.
+	 */
+	err = regmap_read(regmap, SPD5118_REG_TEMP_CONFIG, &regval);
+	if (err < 0)
+		return err;
+
+	regcache_cache_bypass(regmap, true);
+	regmap_update_bits(regmap, SPD5118_REG_TEMP_CONFIG, SPD5118_TS_DISABLE,
+			   SPD5118_TS_DISABLE);
+	regcache_cache_bypass(regmap, false);
+
+	regcache_cache_only(regmap, true);
+	regcache_mark_dirty(regmap);
+
+	return 0;
+}
+
+static int spd5118_resume(struct device *dev)
+{
+	struct spd5118_data *data = dev_get_drvdata(dev);
+	struct regmap *regmap = data->regmap;
+
+	regcache_cache_only(regmap, false);
+	return regcache_sync(regmap);
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(spd5118_pm_ops, spd5118_suspend, spd5118_resume);
+
+static int spd5118_common_probe(struct device *dev, struct regmap *regmap)
+{
+	unsigned int capability, revision, vendor, bank;
+	struct spd5118_data *data;
+	struct device *hwmon_dev;
+	int err;
+
+	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	err = regmap_read(regmap, SPD5118_REG_CAPABILITY, &capability);
+	if (err)
+		return err;
+	if (!(capability & SPD5118_CAP_TS_SUPPORT))
+		return -ENODEV;
+
+	err = regmap_read(regmap, SPD5118_REG_REVISION, &revision);
+	if (err)
+		return err;
+
+	err = regmap_read(regmap, SPD5118_REG_VENDOR, &bank);
+	if (err)
+		return err;
+	err = regmap_read(regmap, SPD5118_REG_VENDOR + 1, &vendor);
+	if (err)
+		return err;
+	if (!spd5118_vendor_valid(bank, vendor))
+		return -ENODEV;
+
+	data->regmap = regmap;
+	mutex_init(&data->nvmem_lock);
+	dev_set_drvdata(dev, data);
+
+	err = spd5118_nvmem_init(dev, data);
+	/* Ignore if NVMEM support is disabled */
+	if (err && err != -EOPNOTSUPP) {
+		dev_err_probe(dev, err, "failed to register nvmem\n");
+		return err;
+	}
+
+	hwmon_dev = devm_hwmon_device_register_with_info(dev, "spd5118",
+							 regmap, &spd5118_chip_info,
+							 NULL);
+	if (IS_ERR(hwmon_dev))
+		return PTR_ERR(hwmon_dev);
+
+	/*
+	 * From JESD300-5B
+	 *   MR2 bits [5:4]: Major revision, 1..4
+	 *   MR2 bits [3:1]: Minor revision, 0..8? Probably a typo, assume 1..8
+	 */
+	dev_info(dev, "DDR5 temperature sensor: vendor 0x%02x:0x%02x revision %d.%d\n",
+		 bank & 0x7f, vendor, ((revision >> 4) & 0x03) + 1, ((revision >> 1) & 0x07) + 1);
+
+	return 0;
+}
+
+/* I2C */
+
+/* Return 0 if detection is successful, -ENODEV otherwise */
+static int spd5118_detect(struct i2c_client *client, struct i2c_board_info *info)
+{
+	struct i2c_adapter *adapter = client->adapter;
+	int regval;
+
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA |
+				     I2C_FUNC_SMBUS_WORD_DATA))
+		return -ENODEV;
+
+	regval = i2c_smbus_read_word_swapped(client, SPD5118_REG_TYPE);
+	if (regval != 0x5118)
+		return -ENODEV;
+
+	regval = i2c_smbus_read_word_data(client, SPD5118_REG_VENDOR);
+	if (regval < 0 || !spd5118_vendor_valid(regval & 0xff, regval >> 8))
+		return -ENODEV;
+
+	regval = i2c_smbus_read_byte_data(client, SPD5118_REG_CAPABILITY);
+	if (regval < 0)
+		return -ENODEV;
+	if (!(regval & SPD5118_CAP_TS_SUPPORT) || (regval & 0xfc))
+		return -ENODEV;
+
+	regval = i2c_smbus_read_byte_data(client, SPD5118_REG_TEMP_CLR);
+	if (regval)
+		return -ENODEV;
+	regval = i2c_smbus_read_byte_data(client, SPD5118_REG_ERROR_CLR);
+	if (regval)
+		return -ENODEV;
+
+	regval = i2c_smbus_read_byte_data(client, SPD5118_REG_REVISION);
+	if (regval < 0 || (regval & 0xc1))
+		return -ENODEV;
+
+	regval = i2c_smbus_read_byte_data(client, SPD5118_REG_TEMP_CONFIG);
+	if (regval < 0)
+		return -ENODEV;
+	if (regval & ~SPD5118_TS_DISABLE)
+		return -ENODEV;
+
+	strscpy(info->type, "spd5118", I2C_NAME_SIZE);
+	return 0;
+}
+
+static int spd5118_i2c_init(struct i2c_client *client)
 {
 	struct i2c_adapter *adapter = client->adapter;
 	int err, regval, mode;
@@ -559,116 +656,28 @@ static int spd5118_init(struct i2c_client *client)
 	return 0;
 }
 
-static int spd5118_probe(struct i2c_client *client)
+static int spd5118_i2c_probe(struct i2c_client *client)
 {
 	struct device *dev = &client->dev;
-	unsigned int regval, revision, vendor, bank;
-	struct spd5118_data *data;
-	struct device *hwmon_dev;
 	struct regmap *regmap;
 	int err;
 
-	err = spd5118_init(client);
+	err = spd5118_i2c_init(client);
 	if (err)
 		return err;
 
-	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
-	if (!data)
-		return -ENOMEM;
-
-	regmap = devm_regmap_init_i2c(client, &spd5118_regmap_config);
+	regmap = devm_regmap_init_i2c(client, &spd5118_i2c_regmap_config);
 	if (IS_ERR(regmap))
 		return dev_err_probe(dev, PTR_ERR(regmap), "regmap init failed\n");
 
-	err = regmap_read(regmap, SPD5118_REG_CAPABILITY, &regval);
-	if (err)
-		return err;
-	if (!(regval & SPD5118_CAP_TS_SUPPORT))
-		return -ENODEV;
-
-	err = regmap_read(regmap, SPD5118_REG_REVISION, &revision);
-	if (err)
-		return err;
-
-	err = regmap_read(regmap, SPD5118_REG_VENDOR, &bank);
-	if (err)
-		return err;
-	err = regmap_read(regmap, SPD5118_REG_VENDOR + 1, &vendor);
-	if (err)
-		return err;
-	if (!spd5118_vendor_valid(bank, vendor))
-		return -ENODEV;
-
-	data->regmap = regmap;
-	mutex_init(&data->nvmem_lock);
-	dev_set_drvdata(dev, data);
-
-	err = spd5118_nvmem_init(dev, data);
-	/* Ignore if NVMEM support is disabled */
-	if (err && err != -EOPNOTSUPP) {
-		dev_err_probe(dev, err, "failed to register nvmem\n");
-		return err;
-	}
-
-	hwmon_dev = devm_hwmon_device_register_with_info(dev, "spd5118",
-							 regmap, &spd5118_chip_info,
-							 NULL);
-	if (IS_ERR(hwmon_dev))
-		return PTR_ERR(hwmon_dev);
-
-	/*
-	 * From JESD300-5B
-	 *   MR2 bits [5:4]: Major revision, 1..4
-	 *   MR2 bits [3:1]: Minor revision, 0..8? Probably a typo, assume 1..8
-	 */
-	dev_info(dev, "DDR5 temperature sensor: vendor 0x%02x:0x%02x revision %d.%d\n",
-		 bank & 0x7f, vendor, ((revision >> 4) & 0x03) + 1, ((revision >> 1) & 0x07) + 1);
-
-	return 0;
+	return spd5118_common_probe(dev, regmap);
 }
 
-static int spd5118_suspend(struct device *dev)
-{
-	struct spd5118_data *data = dev_get_drvdata(dev);
-	struct regmap *regmap = data->regmap;
-	u32 regval;
-	int err;
-
-	/*
-	 * Make sure the configuration register in the regmap cache is current
-	 * before bypassing it.
-	 */
-	err = regmap_read(regmap, SPD5118_REG_TEMP_CONFIG, &regval);
-	if (err < 0)
-		return err;
-
-	regcache_cache_bypass(regmap, true);
-	regmap_update_bits(regmap, SPD5118_REG_TEMP_CONFIG, SPD5118_TS_DISABLE,
-			   SPD5118_TS_DISABLE);
-	regcache_cache_bypass(regmap, false);
-
-	regcache_cache_only(regmap, true);
-	regcache_mark_dirty(regmap);
-
-	return 0;
-}
-
-static int spd5118_resume(struct device *dev)
-{
-	struct spd5118_data *data = dev_get_drvdata(dev);
-	struct regmap *regmap = data->regmap;
-
-	regcache_cache_only(regmap, false);
-	return regcache_sync(regmap);
-}
-
-static DEFINE_SIMPLE_DEV_PM_OPS(spd5118_pm_ops, spd5118_suspend, spd5118_resume);
-
-static const struct i2c_device_id spd5118_id[] = {
+static const struct i2c_device_id spd5118_i2c_id[] = {
 	{ "spd5118" },
 	{ }
 };
-MODULE_DEVICE_TABLE(i2c, spd5118_id);
+MODULE_DEVICE_TABLE(i2c, spd5118_i2c_id);
 
 static const struct of_device_id spd5118_of_ids[] = {
 	{ .compatible = "jedec,spd5118", },
@@ -676,20 +685,20 @@ static const struct of_device_id spd5118_of_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, spd5118_of_ids);
 
-static struct i2c_driver spd5118_driver = {
+static struct i2c_driver spd5118_i2c_driver = {
 	.class		= I2C_CLASS_HWMON,
 	.driver = {
 		.name	= "spd5118",
 		.of_match_table = spd5118_of_ids,
 		.pm = pm_sleep_ptr(&spd5118_pm_ops),
 	},
-	.probe		= spd5118_probe,
-	.id_table	= spd5118_id,
+	.probe		= spd5118_i2c_probe,
+	.id_table	= spd5118_i2c_id,
 	.detect		= IS_ENABLED(CONFIG_SENSORS_SPD5118_DETECT) ? spd5118_detect : NULL,
 	.address_list	= IS_ENABLED(CONFIG_SENSORS_SPD5118_DETECT) ? normal_i2c : NULL,
 };
 
-module_i2c_driver(spd5118_driver);
+module_i2c_driver(spd5118_i2c_driver);
 
 MODULE_AUTHOR("René Rebe <rene@exactcode.de>");
 MODULE_AUTHOR("Guenter Roeck <linux@roeck-us.net>");
-- 
2.45.2

[PATCH 2/5] hwmon: (spd5118) Name chips taking the specification literally

[Guenter Roeck]

The Renesas/IDT SPD5118 Hub Controller is known to take the specification
literally and does not permit access to volatile registers except for the
page register if the selected page is non-zero. Explicitly name the chip
to ensure that the information does not get lost.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/spd5118.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
index 02eb21684c3a..c5ad06b90cbf 100644
--- a/drivers/hwmon/spd5118.c
+++ b/drivers/hwmon/spd5118.c
@@ -621,7 +621,8 @@ static int spd5118_i2c_init(struct i2c_client *client)
 	 * If the device type registers return 0, it is possible that the chip
 	 * has a non-zero page selected and takes the specification literally,
 	 * i.e. disables access to volatile registers besides the page register
-	 * if the page is not 0. Try to identify such chips.
+	 * if the page is not 0. The Renesas/ITD SPD5118 Hub Controller is known
+	 * to show this behavior. Try to identify such chips.
 	 */
 	if (!regval) {
 		/* Vendor ID registers must also be 0 */
-- 
2.45.2

[PATCH 3/5] hwmon: (spd5118) Support 16-bit addressing for NVMEM accesses

[Guenter Roeck]

I3C uses 16-bit register addresses. Setting the page through the legacy
mode access pointer in the legacy mode device configuration register (MR11)
is not needed. This is similar to 16-bit addressing in legacy mode.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/spd5118.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
index c5ad06b90cbf..92a3cb0bb515 100644
--- a/drivers/hwmon/spd5118.c
+++ b/drivers/hwmon/spd5118.c
@@ -66,6 +66,9 @@ static const unsigned short normal_i2c[] = {
 #define SPD5118_EEPROM_BASE		0x80
 #define SPD5118_EEPROM_SIZE		(SPD5118_PAGE_SIZE * SPD5118_NUM_PAGES)
 
+#define PAGE_ADDR0(page)		(((page) & BIT(0)) << 6)
+#define PAGE_ADDR1_4(page)		(((page) & GENMASK(4, 1)) >> 1)
+
 /* Temperature unit in millicelsius */
 #define SPD5118_TEMP_UNIT		(MILLIDEGREE_PER_DEGREE / 4)
 /* Representable temperature range in millicelsius */
@@ -75,6 +78,7 @@ static const unsigned short normal_i2c[] = {
 struct spd5118_data {
 	struct regmap *regmap;
 	struct mutex nvmem_lock;
+	bool is_16bit;
 };
 
 /* hwmon */
@@ -331,11 +335,12 @@ static const struct hwmon_chip_info spd5118_chip_info = {
 
 /* nvmem */
 
-static ssize_t spd5118_nvmem_read_page(struct regmap *regmap, char *buf,
+static ssize_t spd5118_nvmem_read_page(struct spd5118_data *data, char *buf,
 				       unsigned int offset, size_t count)
 {
-	int addr = (offset >> SPD5118_PAGE_SHIFT) * 0x100 + SPD5118_EEPROM_BASE;
-	int err;
+	int page = offset >> SPD5118_PAGE_SHIFT;
+	struct regmap *regmap = data->regmap;
+	int err, addr;
 
 	offset &= SPD5118_PAGE_MASK;
 
@@ -343,6 +348,12 @@ static ssize_t spd5118_nvmem_read_page(struct regmap *regmap, char *buf,
 	if (offset + count > SPD5118_PAGE_SIZE)
 		count = SPD5118_PAGE_SIZE - offset;
 
+	if (data->is_16bit) {
+		addr = SPD5118_EEPROM_BASE | PAGE_ADDR0(page) |
+		  (PAGE_ADDR1_4(page) << 8);
+	} else {
+		addr = page * 0x100 + SPD5118_EEPROM_BASE;
+	}
 	err = regmap_bulk_read(regmap, addr + offset, buf, count);
 	if (err)
 		return err;
@@ -365,7 +376,7 @@ static int spd5118_nvmem_read(void *priv, unsigned int off, void *val, size_t co
 	mutex_lock(&data->nvmem_lock);
 
 	while (count) {
-		ret = spd5118_nvmem_read_page(data->regmap, buf, off, count);
+		ret = spd5118_nvmem_read_page(data, buf, off, count);
 		if (ret < 0) {
 			mutex_unlock(&data->nvmem_lock);
 			return ret;
@@ -516,6 +527,13 @@ static int spd5118_common_probe(struct device *dev, struct regmap *regmap)
 	if (!(capability & SPD5118_CAP_TS_SUPPORT))
 		return -ENODEV;
 
+	/*
+	 * 16-bit register addresses are not (yet) supported with I2C.
+	 * Therefore, if this is an I2C device, register addresses must be
+	 * 8 bit wide.
+	 */
+	data->is_16bit = !!i2c_verify_adapter(dev);
+
 	err = regmap_read(regmap, SPD5118_REG_REVISION, &revision);
 	if (err)
 		return err;
-- 
2.45.2

[PATCH 4/5] hwmon: (spd5118) Detect and support 16-bit register addressing

[Guenter Roeck]

Add support for SPD5118 compatible chips with 16-bit addressing enabled
which are connected to I2C controllers.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/spd5118.c | 75 +++++++++++++++++++++++++++++++++++------
 1 file changed, 64 insertions(+), 11 deletions(-)

diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
index 92a3cb0bb515..5da44571b6a0 100644
--- a/drivers/hwmon/spd5118.c
+++ b/drivers/hwmon/spd5118.c
@@ -461,7 +461,7 @@ static const struct regmap_range_cfg spd5118_i2c_regmap_range_cfg[] = {
 	},
 };
 
-static const struct regmap_config spd5118_i2c_regmap_config = {
+static const struct regmap_config spd5118_regmap8_config = {
 	.reg_bits = 8,
 	.val_bits = 8,
 	.max_register = 0x7ff,
@@ -473,6 +473,15 @@ static const struct regmap_config spd5118_i2c_regmap_config = {
 	.num_ranges = ARRAY_SIZE(spd5118_i2c_regmap_range_cfg),
 };
 
+static const struct regmap_config spd5118_regmap16_config = {
+	.reg_bits = 16,
+	.val_bits = 8,
+	.max_register = 0x7ff,
+	.writeable_reg = spd5118_writeable_reg,
+	.volatile_reg = spd5118_volatile_reg,
+	.cache_type = REGCACHE_MAPLE,
+};
+
 static int spd5118_suspend(struct device *dev)
 {
 	struct spd5118_data *data = dev_get_drvdata(dev);
@@ -510,7 +519,8 @@ static int spd5118_resume(struct device *dev)
 
 static DEFINE_SIMPLE_DEV_PM_OPS(spd5118_pm_ops, spd5118_suspend, spd5118_resume);
 
-static int spd5118_common_probe(struct device *dev, struct regmap *regmap)
+static int spd5118_common_probe(struct device *dev, struct regmap *regmap,
+				bool is_16bit)
 {
 	unsigned int capability, revision, vendor, bank;
 	struct spd5118_data *data;
@@ -527,12 +537,7 @@ static int spd5118_common_probe(struct device *dev, struct regmap *regmap)
 	if (!(capability & SPD5118_CAP_TS_SUPPORT))
 		return -ENODEV;
 
-	/*
-	 * 16-bit register addresses are not (yet) supported with I2C.
-	 * Therefore, if this is an I2C device, register addresses must be
-	 * 8 bit wide.
-	 */
-	data->is_16bit = !!i2c_verify_adapter(dev);
+	data->is_16bit = is_16bit;
 
 	err = regmap_read(regmap, SPD5118_REG_REVISION, &revision);
 	if (err)
@@ -675,21 +680,69 @@ static int spd5118_i2c_init(struct i2c_client *client)
 	return 0;
 }
 
+/*
+ * 16-bit addressing note:
+ *
+ * If I2C_FUNC_I2C is not supported by an I2C adapter driver, regmap uses
+ * SMBus operations as alternative. To simulate a read operation with a 16-bit
+ * address, it writes the address using i2c_smbus_write_byte_data(), followed
+ * by one or more calls to i2c_smbus_read_byte() to read the data.
+ * Per spd5118 standard, a read operation after writing the address must start
+ * with <Sr> (Repeat Start). However, a SMBus read byte operation starts with
+ * <S> (Start). This resets the register address in the spd5118 chip. As result,
+ * i2c_smbus_read_byte() always returns data from register address 0x00.
+ *
+ * A working alternative to access chips with 16-bit register addresses in the
+ * absence of I2C_FUNC_I2C support is not known.
+ *
+ * For this reason, 16-bit addressing can only be supported with I2C if the
+ * adapter supports I2C_FUNC_I2C.
+ *
+ * For I2C, the addressing mode selected by the BIOS must not be changed.
+ * Experiments show that at least some PC BIOS versions will not change the
+ * addressing mode on a soft reboot and end up in setup, claiming that some
+ * configuration change happened. This will happen again after a power cycle,
+ * which does reset the addressing mode. To prevent this from happening,
+ * detect if 16-bit addressing is enabled and always use the currently
+ * configured addressing mode.
+ */
+
 static int spd5118_i2c_probe(struct i2c_client *client)
 {
+	const struct regmap_config *config;
 	struct device *dev = &client->dev;
 	struct regmap *regmap;
-	int err;
+	int err, mode;
+	bool is_16bit;
 
 	err = spd5118_i2c_init(client);
 	if (err)
 		return err;
 
-	regmap = devm_regmap_init_i2c(client, &spd5118_i2c_regmap_config);
+	mode = i2c_smbus_read_byte_data(client, SPD5118_REG_I2C_LEGACY_MODE);
+	if (mode < 0)
+		return mode;
+
+	is_16bit = mode & SPD5118_LEGACY_MODE_ADDR;
+	if (is_16bit) {
+		/*
+		 * See 16-bit addressing note above explaining why it is
+		 * necessary to check for I2C_FUNC_I2C support here.
+		 */
+		if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
+			dev_err(dev, "Adapter does not support 16-bit register addresses\n");
+			return -ENODEV;
+		}
+		config = &spd5118_regmap16_config;
+	} else {
+		config = &spd5118_regmap8_config;
+	}
+
+	regmap = devm_regmap_init_i2c(client, config);
 	if (IS_ERR(regmap))
 		return dev_err_probe(dev, PTR_ERR(regmap), "regmap init failed\n");
 
-	return spd5118_common_probe(dev, regmap);
+	return spd5118_common_probe(dev, regmap, is_16bit);
 }
 
 static const struct i2c_device_id spd5118_i2c_id[] = {
-- 
2.45.2

[RFT PATCH 5/5] hwmon: (spd5118) Add I3C support

[Guenter Roeck]

Add code to support instantiating SPD5118 compatible hub and SPD devices
connected to I3C controllers.

Note that protecting the I3C probe function against CONFIG_I3C is not
necessary even though it calls a function which is not available in this
case. The I3C registration and unregistration functions are protected with
IS_ENABLED(CONFIG_I3C), which causes the I3C probe function to be optimized
away if CONFIG_I3C is disabled.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
RFT: Compile tested only.

 drivers/hwmon/Kconfig   |  2 ++
 drivers/hwmon/spd5118.c | 51 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index f91f713b0105..6ee302cccf48 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -2259,7 +2259,9 @@ config SENSORS_INA3221
 config SENSORS_SPD5118
 	tristate "SPD5118 Compliant Temperature Sensors"
 	depends on I2C
+	depends on I3C || I3C=n
 	select REGMAP_I2C
+	select REGMAP_I3C if I3C
 	help
 	  If you say yes here you get support for SPD5118 (JEDEC JESD300)
 	  compliant temperature sensors. Such sensors are found on DDR5 memory
diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
index 5da44571b6a0..f68848b003fb 100644
--- a/drivers/hwmon/spd5118.c
+++ b/drivers/hwmon/spd5118.c
@@ -18,6 +18,7 @@
 #include <linux/bits.h>
 #include <linux/err.h>
 #include <linux/i2c.h>
+#include <linux/i3c/device.h>
 #include <linux/hwmon.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
@@ -705,6 +706,8 @@ static int spd5118_i2c_init(struct i2c_client *client)
  * which does reset the addressing mode. To prevent this from happening,
  * detect if 16-bit addressing is enabled and always use the currently
  * configured addressing mode.
+ *
+ * I3C always uses 16-bit addressing.
  */
 
 static int spd5118_i2c_probe(struct i2c_client *client)
@@ -770,7 +773,53 @@ static struct i2c_driver spd5118_i2c_driver = {
 	.address_list	= IS_ENABLED(CONFIG_SENSORS_SPD5118_DETECT) ? normal_i2c : NULL,
 };
 
-module_i2c_driver(spd5118_i2c_driver);
+/* I3C */
+
+static int spd5118_i3c_probe(struct i3c_device *i3cdev)
+{
+	struct device *dev = i3cdev_to_dev(i3cdev);
+	struct regmap *regmap;
+
+	regmap = devm_regmap_init_i3c(i3cdev, &spd5118_regmap16_config);
+	if (IS_ERR(regmap))
+		return dev_err_probe(dev, PTR_ERR(regmap), "regmap init failed\n");
+
+	return spd5118_common_probe(dev, regmap, true);
+}
+
+/*
+ * SPD5118 compliant devices do not have to support the GETPID command.
+ * It is unknown if those chips can be auto-detected on I3C busses. It may be
+ * necessary to instantiate SPD5118 devices on I3C through devicetree or by
+ * other explicit means. See Documentation/devicetree/bindings/i3c/i3c.yaml
+ * for details on how to instantiate I3C devices from devicetree.
+ * Device IDs are provided to enable association of devicetree entries with the
+ * driver. The Renesas entry was found in a driver from Nuvoton. A reference to
+ * the IDT ID was found in a devicetree file. The entries for Rambus and Montage
+ * Technology were guessed based on assigned MIPI IDs (https://mid.mipi.org).
+ * There is also a SPD5118 compliant EEPROM/Hub/Temperature sensor (S-34HTS08AB)
+ * from Ablic. The MIPI ID for this chip might be 0x0173 (Mitsumi) for Ablic's
+ * parent company, but that is unconfirmed.
+ */
+static const struct i3c_device_id spd5118_i3c_ids[] = {
+	I3C_DEVICE(0x01e0, 0x5118, NULL),	/* IDT */
+	I3C_DEVICE(0x0266, 0x5118, NULL),	/* Renesas */
+	I3C_DEVICE(0x03fa, 0x5118, NULL),	/* Rambus (unconfirmed) */
+	I3C_DEVICE(0x0433, 0x5118, NULL),	/* Montage Technology (unconfirmed) */
+	{ }
+};
+MODULE_DEVICE_TABLE(i3c, spd5118_i3c_ids);
+
+static struct i3c_driver spd5118_i3c_driver = {
+	.driver = {
+		.name = "spd5118_i3c",
+		.pm = pm_sleep_ptr(&spd5118_pm_ops),
+	},
+	.probe = spd5118_i3c_probe,
+	.id_table = spd5118_i3c_ids,
+};
+
+module_i3c_i2c_driver(spd5118_i3c_driver, &spd5118_i2c_driver);
 
 MODULE_AUTHOR("René Rebe <rene@exactcode.de>");
 MODULE_AUTHOR("Guenter Roeck <linux@roeck-us.net>");
-- 
2.45.2