[RFC PATCH 0/5] hwmon: (lm75) add I3C support

[RFC PATCH 0/5] hwmon: (lm75) add I3C support

[Wolfram Sang]

This series adds I3C support to the LM75 driver to support the P3T1755
in I3C mode. This series is still RFC because there are two questions
regarding the last patch which I would like to discuss. Check the patch
for details.

The patch is based on Guenter's patch ""hwmon: (lm75) Hide register size
differences in regmap access functions". Find a branch here:

git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/g3s/i3c-broken-out-experimental

It has been tested with a Renesas RZ/G3S board. Find details here:

https://elinux.org/I3C_with_Renesas_RZG3S

Please let me know what you think!


Wolfram Sang (5):
  hwmon: (lm75) simplify lm75_write_config()
  hwmon: (lm75) simplify regulator handling
  hwmon: (lm75) Remove superfluous 'client' member from private struct
  hwmon: (lm75) separate probe into common and I2C parts
  hwmon: (lm75) add I3C support for P3T1755

 drivers/hwmon/Kconfig |   2 +
 drivers/hwmon/lm75.c  | 230 +++++++++++++++++++++++++++++-------------
 2 files changed, 163 insertions(+), 69 deletions(-)

-- 
2.45.2

[RFC PATCH 1/5] hwmon: (lm75) simplify lm75_write_config()

[Wolfram Sang]

After previous refactoring, it is now possible to make
lm75_write_config() a simple inline function.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/hwmon/lm75.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index b8889392d5da..b03e760cf3a1 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -340,17 +340,11 @@ static inline long lm75_reg_to_mc(s16 temp, u8 resolution)
 	return ((temp >> (16 - resolution)) * 1000) >> (resolution - 8);
 }
 
-static int lm75_write_config(struct lm75_data *data, u16 set_mask,
-			     u16 clr_mask)
+static inline int lm75_write_config(struct lm75_data *data, u16 set_mask,
+				    u16 clr_mask)
 {
-	int err;
-
-	err = regmap_update_bits(data->regmap, LM75_REG_CONF,
-				 clr_mask | LM75_SHUTDOWN, set_mask);
-	if (err)
-		return err;
-
-	return 0;
+	return regmap_update_bits(data->regmap, LM75_REG_CONF,
+				  clr_mask | LM75_SHUTDOWN, set_mask);
 }
 
 static irqreturn_t lm75_alarm_handler(int irq, void *private)
-- 
2.45.2

Re: [RFC PATCH 1/5] hwmon: (lm75) simplify lm75_write_config()

[Geert Uytterhoeven]

On Thu, Dec 19, 2024 at 11:55 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> After previous refactoring, it is now possible to make
> lm75_write_config() a simple inline function.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [RFC PATCH 1/5] hwmon: (lm75) simplify lm75_write_config()

[Guenter Roeck]

On Thu, Dec 19, 2024 at 11:55:23PM +0100, Wolfram Sang wrote:
> After previous refactoring, it is now possible to make
> lm75_write_config() a simple inline function.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Applied.

Thanks,
Gueter

[RFC PATCH 2/5] hwmon: (lm75) simplify regulator handling

[Wolfram Sang]

devm_regulator_get_enable() was introduced exactly to avoid open coding
regulator handling like in this driver. Make use of this helper.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/hwmon/lm75.c | 25 +++----------------------
 1 file changed, 3 insertions(+), 22 deletions(-)

diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index b03e760cf3a1..4d0fd1c93c63 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -109,7 +109,6 @@ static const unsigned short normal_i2c[] = { 0x48, 0x49, 0x4a, 0x4b, 0x4c,
 struct lm75_data {
 	struct i2c_client		*client;
 	struct regmap			*regmap;
-	struct regulator		*vs;
 	u16				orig_conf;
 	u8				resolution;	/* In bits, 9 to 16 */
 	unsigned int			sample_time;	/* In ms */
@@ -621,13 +620,6 @@ static const struct regmap_bus lm75_i2c_regmap_bus = {
 	.reg_write = lm75_i2c_reg_write,
 };
 
-static void lm75_disable_regulator(void *data)
-{
-	struct lm75_data *lm75 = data;
-
-	regulator_disable(lm75->vs);
-}
-
 static void lm75_remove(void *data)
 {
 	struct lm75_data *lm75 = data;
@@ -656,9 +648,9 @@ static int lm75_probe(struct i2c_client *client)
 	data->client = client;
 	data->kind = (uintptr_t)i2c_get_match_data(client);
 
-	data->vs = devm_regulator_get(dev, "vs");
-	if (IS_ERR(data->vs))
-		return PTR_ERR(data->vs);
+	err = devm_regulator_get_enable(dev, "vs");
+	if (err)
+		return err;
 
 	data->regmap = devm_regmap_init(dev, &lm75_i2c_regmap_bus, data,
 					&lm75_regmap_config);
@@ -675,17 +667,6 @@ static int lm75_probe(struct i2c_client *client)
 	data->sample_time = data->params->default_sample_time;
 	data->resolution = data->params->default_resolution;
 
-	/* Enable the power */
-	err = regulator_enable(data->vs);
-	if (err) {
-		dev_err(dev, "failed to enable regulator: %d\n", err);
-		return err;
-	}
-
-	err = devm_add_action_or_reset(dev, lm75_disable_regulator, data);
-	if (err)
-		return err;
-
 	/* Cache original configuration */
 	err = regmap_read(data->regmap, LM75_REG_CONF, &status);
 	if (err)
-- 
2.45.2

Re: [RFC PATCH 2/5] hwmon: (lm75) simplify regulator handling

[Geert Uytterhoeven]

On Thu, Dec 19, 2024 at 11:55 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> devm_regulator_get_enable() was introduced exactly to avoid open coding
> regulator handling like in this driver. Make use of this helper.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [RFC PATCH 2/5] hwmon: (lm75) simplify regulator handling

[Guenter Roeck]

On Thu, Dec 19, 2024 at 11:55:24PM +0100, Wolfram Sang wrote:
> devm_regulator_get_enable() was introduced exactly to avoid open coding
> regulator handling like in this driver. Make use of this helper.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Applied.

Thanks,
Guenter

[RFC PATCH 3/5] hwmon: (lm75) Remove superfluous 'client' member from private struct

[Wolfram Sang]

The regmap-only conversion allows us to store the client-pointer as the
'context' parameter for regmap. This not only makes the private struct
smaller, but also allows proper separation of I2C and I3C in the future.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/hwmon/lm75.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index 4d0fd1c93c63..0f034110daed 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -105,9 +105,7 @@ static const unsigned short normal_i2c[] = { 0x48, 0x49, 0x4a, 0x4b, 0x4c,
 #define LM75_REG_MAX		0x03
 #define PCT2075_REG_IDLE	0x04
 
-/* Each client has this additional data */
 struct lm75_data {
-	struct i2c_client		*client;
 	struct regmap			*regmap;
 	u16				orig_conf;
 	u8				resolution;	/* In bits, 9 to 16 */
@@ -572,8 +570,8 @@ static bool lm75_is_volatile_reg(struct device *dev, unsigned int reg)
 
 static int lm75_i2c_reg_read(void *context, unsigned int reg, unsigned int *val)
 {
-	struct lm75_data *data = context;
-	struct i2c_client *client = data->client;
+	struct i2c_client *client = context;
+	struct lm75_data *data = i2c_get_clientdata(client);
 	int ret;
 
 	if (reg == LM75_REG_CONF) {
@@ -592,8 +590,8 @@ static int lm75_i2c_reg_read(void *context, unsigned int reg, unsigned int *val)
 
 static int lm75_i2c_reg_write(void *context, unsigned int reg, unsigned int val)
 {
-	struct lm75_data *data = context;
-	struct i2c_client *client = data->client;
+	struct i2c_client *client = context;
+	struct lm75_data *data = i2c_get_clientdata(client);
 
 	if (reg == PCT2075_REG_IDLE ||
 	    (reg == LM75_REG_CONF && !data->params->config_reg_16bits))
@@ -645,14 +643,13 @@ static int lm75_probe(struct i2c_client *client)
 	/* needed by custom regmap callbacks */
 	dev_set_drvdata(dev, data);
 
-	data->client = client;
 	data->kind = (uintptr_t)i2c_get_match_data(client);
 
 	err = devm_regulator_get_enable(dev, "vs");
 	if (err)
 		return err;
 
-	data->regmap = devm_regmap_init(dev, &lm75_i2c_regmap_bus, data,
+	data->regmap = devm_regmap_init(dev, &lm75_i2c_regmap_bus, client,
 					&lm75_regmap_config);
 	if (IS_ERR(data->regmap))
 		return PTR_ERR(data->regmap);
-- 
2.45.2

Re: [RFC PATCH 3/5] hwmon: (lm75) Remove superfluous 'client' member from private struct

[Guenter Roeck]

On Thu, Dec 19, 2024 at 11:55:25PM +0100, Wolfram Sang wrote:
> The regmap-only conversion allows us to store the client-pointer as the
> 'context' parameter for regmap. This not only makes the private struct
> smaller, but also allows proper separation of I2C and I3C in the future.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Applied.

Thanks,
Guenter

[RFC PATCH 4/5] hwmon: (lm75) separate probe into common and I2C parts

[Wolfram Sang]

Put generic probe functionality into a separate function and let the I2C
driver call it. This is a preparation for adding I3C support which will
also use the generic probe function.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/hwmon/lm75.c | 68 +++++++++++++++++++++++++-------------------
 1 file changed, 38 insertions(+), 30 deletions(-)

diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index 0f034110daed..8b4f324524da 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -601,6 +601,11 @@ static int lm75_i2c_reg_write(void *context, unsigned int reg, unsigned int val)
 	return i2c_smbus_write_word_swapped(client, reg, val);
 }
 
+static const struct regmap_bus lm75_i2c_regmap_bus = {
+	.reg_read = lm75_i2c_reg_read,
+	.reg_write = lm75_i2c_reg_write,
+};
+
 static const struct regmap_config lm75_regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 16,
@@ -613,11 +618,6 @@ static const struct regmap_config lm75_regmap_config = {
 	.use_single_write = true,
 };
 
-static const struct regmap_bus lm75_i2c_regmap_bus = {
-	.reg_read = lm75_i2c_reg_read,
-	.reg_write = lm75_i2c_reg_write,
-};
-
 static void lm75_remove(void *data)
 {
 	struct lm75_data *lm75 = data;
@@ -625,17 +625,13 @@ static void lm75_remove(void *data)
 	regmap_write(lm75->regmap, LM75_REG_CONF, lm75->orig_conf);
 }
 
-static int lm75_probe(struct i2c_client *client)
+static int lm75_generic_probe(struct device *dev, const char *name,
+			      const void *kind_ptr, int irq, struct regmap *regmap)
 {
-	struct device *dev = &client->dev;
 	struct device *hwmon_dev;
 	struct lm75_data *data;
 	int status, err;
 
-	if (!i2c_check_functionality(client->adapter,
-			I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA))
-		return -EIO;
-
 	data = devm_kzalloc(dev, sizeof(struct lm75_data), GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
@@ -643,17 +639,13 @@ static int lm75_probe(struct i2c_client *client)
 	/* needed by custom regmap callbacks */
 	dev_set_drvdata(dev, data);
 
-	data->kind = (uintptr_t)i2c_get_match_data(client);
+	data->kind = (uintptr_t)kind_ptr;
+	data->regmap = regmap;
 
 	err = devm_regulator_get_enable(dev, "vs");
 	if (err)
 		return err;
 
-	data->regmap = devm_regmap_init(dev, &lm75_i2c_regmap_bus, client,
-					&lm75_regmap_config);
-	if (IS_ERR(data->regmap))
-		return PTR_ERR(data->regmap);
-
 	/* Set to LM75 resolution (9 bits, 1/2 degree C) and range.
 	 * Then tweak to be more precise when appropriate.
 	 */
@@ -679,20 +671,19 @@ static int lm75_probe(struct i2c_client *client)
 	if (err)
 		return err;
 
-	hwmon_dev = devm_hwmon_device_register_with_info(dev, client->name,
-							 data, &lm75_chip_info,
-							 NULL);
+	hwmon_dev = devm_hwmon_device_register_with_info(dev, name, data,
+							 &lm75_chip_info, NULL);
 	if (IS_ERR(hwmon_dev))
 		return PTR_ERR(hwmon_dev);
 
-	if (client->irq) {
+	if (irq) {
 		if (data->params->alarm) {
 			err = devm_request_threaded_irq(dev,
-							client->irq,
+							irq,
 							NULL,
 							&lm75_alarm_handler,
 							IRQF_ONESHOT,
-							client->name,
+							name,
 							hwmon_dev);
 			if (err)
 				return err;
@@ -702,12 +693,29 @@ static int lm75_probe(struct i2c_client *client)
 		}
 	}
 
-	dev_info(dev, "%s: sensor '%s'\n", dev_name(hwmon_dev), client->name);
+	dev_info(dev, "%s: sensor '%s'\n", dev_name(hwmon_dev), name);
 
 	return 0;
 }
 
-static const struct i2c_device_id lm75_ids[] = {
+static int lm75_i2c_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct regmap *regmap;
+
+	if (!i2c_check_functionality(client->adapter,
+			I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA))
+		return -EOPNOTSUPP;
+
+	regmap = devm_regmap_init(dev, &lm75_i2c_regmap_bus, client, &lm75_regmap_config);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	return lm75_generic_probe(dev, client->name, i2c_get_match_data(client),
+				  client->irq, regmap);
+}
+
+static const struct i2c_device_id lm75_i2c_ids[] = {
 	{ "adt75", adt75, },
 	{ "as6200", as6200, },
 	{ "at30ts74", at30ts74, },
@@ -740,7 +748,7 @@ static const struct i2c_device_id lm75_ids[] = {
 	{ "tmp1075", tmp1075, },
 	{ /* LIST END */ }
 };
-MODULE_DEVICE_TABLE(i2c, lm75_ids);
+MODULE_DEVICE_TABLE(i2c, lm75_i2c_ids);
 
 static const struct of_device_id __maybe_unused lm75_of_match[] = {
 	{
@@ -987,20 +995,20 @@ static const struct dev_pm_ops lm75_dev_pm_ops = {
 #define LM75_DEV_PM_OPS NULL
 #endif /* CONFIG_PM */
 
-static struct i2c_driver lm75_driver = {
+static struct i2c_driver lm75_i2c_driver = {
 	.class		= I2C_CLASS_HWMON,
 	.driver = {
 		.name	= "lm75",
 		.of_match_table = of_match_ptr(lm75_of_match),
 		.pm	= LM75_DEV_PM_OPS,
 	},
-	.probe		= lm75_probe,
-	.id_table	= lm75_ids,
+	.probe		= lm75_i2c_probe,
+	.id_table	= lm75_i2c_ids,
 	.detect		= lm75_detect,
 	.address_list	= normal_i2c,
 };
 
-module_i2c_driver(lm75_driver);
+module_i2c_driver(lm75_i2c_driver);
 
 MODULE_AUTHOR("Frodo Looijaard <frodol@dds.nl>");
 MODULE_DESCRIPTION("LM75 driver");
-- 
2.45.2

Re: [RFC PATCH 4/5] hwmon: (lm75) separate probe into common and I2C parts

[Guenter Roeck]

On Thu, Dec 19, 2024 at 11:55:26PM +0100, Wolfram Sang wrote:
> Put generic probe functionality into a separate function and let the I2C
> driver call it. This is a preparation for adding I3C support which will
> also use the generic probe function.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Applied.

Thanks,
Guenter

[RFC PATCH 5/5] hwmon: (lm75) add I3C support for P3T1755

[Wolfram Sang]

Introduce I3C support by defining I3C accessors for regmap and
implementing an I3C driver. Enable I3C for the NXP P3T1755.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

Two things I would like to discuss:

- Shall we reuse i2c_device_id's to get a proper name for the chip?
  Looks strange, but adding all the info again for I3C looks strange as
  well

- are there some suitable kernel helpers for dealing with big/little
  endianess in lm75_i3c_regmap_bus? Note: I also tried to use the
  non-*_reg callbacks for the regmap bus. But the constified
  void-pointers make it ugly as we need to change buffers because some
  registers are big endian and some little endian.

 drivers/hwmon/Kconfig |   2 +
 drivers/hwmon/lm75.c  | 114 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 115 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index dd376602f3f1..86897b4d105f 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1412,7 +1412,9 @@ config SENSORS_LM73
 config SENSORS_LM75
 	tristate "National Semiconductor LM75 and compatibles"
 	depends on I2C
+	depends on I3C || !I3C
 	select REGMAP_I2C
+	select REGMAP_I3C if I3C
 	help
 	  If you say yes here you get support for one common type of
 	  temperature sensor chip, with models including:
diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
index 8b4f324524da..2979f1746585 100644
--- a/drivers/hwmon/lm75.c
+++ b/drivers/hwmon/lm75.c
@@ -11,6 +11,7 @@
 #include <linux/slab.h>
 #include <linux/jiffies.h>
 #include <linux/i2c.h>
+#include <linux/i3c/device.h>
 #include <linux/hwmon.h>
 #include <linux/err.h>
 #include <linux/of.h>
@@ -112,6 +113,8 @@ struct lm75_data {
 	unsigned int			sample_time;	/* In ms */
 	enum lm75_type			kind;
 	const struct lm75_params	*params;
+	u8				reg_buf[1];
+	u8				val_buf[3];
 };
 
 /*-----------------------------------------------------------------------*/
@@ -606,6 +609,77 @@ static const struct regmap_bus lm75_i2c_regmap_bus = {
 	.reg_write = lm75_i2c_reg_write,
 };
 
+static int lm75_i3c_reg_read(void *context, unsigned int reg, unsigned int *val)
+{
+	struct i3c_device *i3cdev = context;
+	struct lm75_data *data = i3cdev_get_drvdata(i3cdev);
+	struct i3c_priv_xfer xfers[] = {
+		{
+			.rnw = false,
+			.len = 1,
+			.data.out = data->reg_buf,
+		},
+		{
+			.rnw = true,
+			.len = 2,
+			.data.out = data->val_buf,
+		},
+	};
+	int ret;
+
+	data->reg_buf[0] = reg;
+
+	if (reg == LM75_REG_CONF && !data->params->config_reg_16bits)
+		xfers[1].len--;
+
+	ret = i3c_device_do_priv_xfers(i3cdev, xfers, 2);
+	if (ret < 0)
+		return ret;
+
+	if (reg == LM75_REG_CONF && !data->params->config_reg_16bits)
+		*val = data->val_buf[0];
+	else if (reg == LM75_REG_CONF)
+		*val = data->val_buf[0] | (data->val_buf[1] << 8);
+	else
+		*val = data->val_buf[1] | (data->val_buf[0] << 8);
+
+	return 0;
+}
+
+static int lm75_i3c_reg_write(void *context, unsigned int reg, unsigned int val)
+{
+	struct i3c_device *i3cdev = context;
+	struct lm75_data *data = i3cdev_get_drvdata(i3cdev);
+	struct i3c_priv_xfer xfers[] = {
+		{
+			.rnw = false,
+			.len = 3,
+			.data.out = data->val_buf,
+		},
+	};
+
+	data->val_buf[0] = reg;
+
+	if (reg == PCT2075_REG_IDLE ||
+	    (reg == LM75_REG_CONF && !data->params->config_reg_16bits)) {
+		xfers[0].len--;
+		data->val_buf[1] = val & 0xff;
+	} else if (reg == LM75_REG_CONF) {
+		data->val_buf[1] = val & 0xff;
+		data->val_buf[2] = (val >> 8) & 0xff;
+	} else {
+		data->val_buf[2] = val & 0xff;
+		data->val_buf[1] = (val >> 8) & 0xff;
+	}
+
+	return i3c_device_do_priv_xfers(i3cdev, xfers, 1);
+}
+
+static const struct regmap_bus lm75_i3c_regmap_bus = {
+	.reg_read = lm75_i3c_reg_read,
+	.reg_write = lm75_i3c_reg_write,
+};
+
 static const struct regmap_config lm75_regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 16,
@@ -750,6 +824,36 @@ static const struct i2c_device_id lm75_i2c_ids[] = {
 };
 MODULE_DEVICE_TABLE(i2c, lm75_i2c_ids);
 
+static const struct i3c_device_id lm75_i3c_ids[] = {
+	I3C_DEVICE(0x011b, 0x152a, (void *)p3t1755),
+	{ /* LIST END */ }
+};
+MODULE_DEVICE_TABLE(i3c, lm75_i3c_ids);
+
+static int lm75_i3c_probe(struct i3c_device *i3cdev)
+{
+	struct device *dev = i3cdev_to_dev(i3cdev);
+	const struct i3c_device_id *id;
+	struct regmap *regmap;
+	const char *name;
+	unsigned int i;
+
+	regmap = devm_regmap_init(dev, &lm75_i3c_regmap_bus, i3cdev, &lm75_regmap_config);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	id = i3c_device_match_id(i3cdev, lm75_i3c_ids);
+
+	/* Reuse i2c_device_ids is OK? Or do we want a third copy of this data in lm75_i3c_ids? */
+	for (i = 0; lm75_i2c_ids[i].name[0]; i++)
+		if ((kernel_ulong_t)id->data == lm75_i2c_ids[i].driver_data)
+			break;
+
+	name = lm75_i2c_ids[i].name[0] ? lm75_i2c_ids[i].name : "lm75compatible";
+
+	return lm75_generic_probe(dev, name, id->data, 0, regmap);
+}
+
 static const struct of_device_id __maybe_unused lm75_of_match[] = {
 	{
 		.compatible = "adi,adt75",
@@ -1008,7 +1112,15 @@ static struct i2c_driver lm75_i2c_driver = {
 	.address_list	= normal_i2c,
 };
 
-module_i2c_driver(lm75_i2c_driver);
+static struct i3c_driver lm75_i3c_driver = {
+	.driver = {
+		.name = "lm75_i3c",
+	},
+	.probe = lm75_i3c_probe,
+	.id_table = lm75_i3c_ids,
+};
+
+module_i3c_i2c_driver(lm75_i3c_driver, &lm75_i2c_driver)
 
 MODULE_AUTHOR("Frodo Looijaard <frodol@dds.nl>");
 MODULE_DESCRIPTION("LM75 driver");
-- 
2.45.2

Re: [RFC PATCH 5/5] hwmon: (lm75) add I3C support for P3T1755

[Guenter Roeck]

Hi Wolfram,

On 12/19/24 14:55, Wolfram Sang wrote:
> Introduce I3C support by defining I3C accessors for regmap and
> implementing an I3C driver. Enable I3C for the NXP P3T1755.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
> 
> Two things I would like to discuss:
> 
> - Shall we reuse i2c_device_id's to get a proper name for the chip?
>    Looks strange, but adding all the info again for I3C looks strange as
>    well
> 

I don't think there will be many i3c devices supporting LM75 compatible chips.
For the few i3c chips which do happen do be lm75 compatible, we should have
something like

struct lm75_i3c_devices {
	enum lm75_type type;
	const char *name;
};

with an instance for each i3c device, and then point i3c_device_id->data to it.
And I think "lm75compatible" is really a terrible hwmon device name ;-).

FWIW, it is too bad that i3c_device_id doesn't have a "name" field. That would
really come handy here.

An alternative would be to just use dev_name(i3cdev_to_dev(i3cdev)),
but that would not reflect the chip name and thus be less than perfect.

> - are there some suitable kernel helpers for dealing with big/little
>    endianess in lm75_i3c_regmap_bus? Note: I also tried to use the
>    non-*_reg callbacks for the regmap bus. But the constified
>    void-pointers make it ugly as we need to change buffers because some
>    registers are big endian and some little endian.
> 

i3c doesn't seem to have any access functions (kernel helpers) similar to i2c,
other than i3c_device_do_priv_xfers(), so unless those are made available
I think we'll have to bite the bullet and use local access functions.

The other patches look good to me. If you send me a Reviewed-by: and/or
Tested-by: to my patch, I'll queue it all up (except for this patch)
for 6.14.

Thanks,
Guenter

Re: [RFC PATCH 5/5] hwmon: (lm75) add I3C support for P3T1755

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1049 bytes --]

Hi Guenter,

> For the few i3c chips which do happen do be lm75 compatible, we should have
> something like
> 
> struct lm75_i3c_devices {
> 	enum lm75_type type;
> 	const char *name;
> };

Sigh, OK, let's have that info a third time.

> And I think "lm75compatible" is really a terrible hwmon device name ;-).

The series is RFC for a reason :D

> i3c doesn't seem to have any access functions (kernel helpers) similar to i2c,
> other than i3c_device_do_priv_xfers(), so unless those are made available
> I think we'll have to bite the bullet and use local access functions.

Well, yes, there are not many I3C target drivers, so there is no
critical mass for I3C helpers yet. With helpers I meant more convesion
helpers like cpu_to_be16 and the likes. I see them rarely used in hwmon,
so I wondered about this.

> The other patches look good to me. If you send me a Reviewed-by: and/or
> Tested-by: to my patch, I'll queue it all up (except for this patch)
> for 6.14.

Done. Thank you!

All the best,

   Wolfram

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]