* [RFC PATCH 0/5] hwmon: (lm75) add I3C support
@ 2024-12-19 22:55 Wolfram Sang
2024-12-19 22:55 ` [RFC PATCH 1/5] hwmon: (lm75) simplify lm75_write_config() Wolfram Sang
` (4 more replies)
0 siblings, 5 replies; 14+ messages in thread
From: Wolfram Sang @ 2024-12-19 22:55 UTC (permalink / raw)
To: linux-renesas-soc; +Cc: Wolfram Sang, Guenter Roeck, Jean Delvare, linux-hwmon
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [RFC PATCH 1/5] hwmon: (lm75) simplify lm75_write_config()
2024-12-19 22:55 [RFC PATCH 0/5] hwmon: (lm75) add I3C support Wolfram Sang
@ 2024-12-19 22:55 ` Wolfram Sang
2024-12-20 8:48 ` Geert Uytterhoeven
2024-12-20 14:56 ` Guenter Roeck
2024-12-19 22:55 ` [RFC PATCH 2/5] hwmon: (lm75) simplify regulator handling Wolfram Sang
` (3 subsequent siblings)
4 siblings, 2 replies; 14+ messages in thread
From: Wolfram Sang @ 2024-12-19 22:55 UTC (permalink / raw)
To: linux-renesas-soc; +Cc: Wolfram Sang, Jean Delvare, Guenter Roeck, linux-hwmon
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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [RFC PATCH 2/5] hwmon: (lm75) simplify regulator handling
2024-12-19 22:55 [RFC PATCH 0/5] hwmon: (lm75) add I3C support Wolfram Sang
2024-12-19 22:55 ` [RFC PATCH 1/5] hwmon: (lm75) simplify lm75_write_config() Wolfram Sang
@ 2024-12-19 22:55 ` Wolfram Sang
2024-12-20 8:50 ` Geert Uytterhoeven
2024-12-20 14:57 ` Guenter Roeck
2024-12-19 22:55 ` [RFC PATCH 3/5] hwmon: (lm75) Remove superfluous 'client' member from private struct Wolfram Sang
` (2 subsequent siblings)
4 siblings, 2 replies; 14+ messages in thread
From: Wolfram Sang @ 2024-12-19 22:55 UTC (permalink / raw)
To: linux-renesas-soc; +Cc: Wolfram Sang, Jean Delvare, Guenter Roeck, linux-hwmon
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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [RFC PATCH 3/5] hwmon: (lm75) Remove superfluous 'client' member from private struct
2024-12-19 22:55 [RFC PATCH 0/5] hwmon: (lm75) add I3C support Wolfram Sang
2024-12-19 22:55 ` [RFC PATCH 1/5] hwmon: (lm75) simplify lm75_write_config() Wolfram Sang
2024-12-19 22:55 ` [RFC PATCH 2/5] hwmon: (lm75) simplify regulator handling Wolfram Sang
@ 2024-12-19 22:55 ` Wolfram Sang
2024-12-20 14:57 ` Guenter Roeck
2024-12-19 22:55 ` [RFC PATCH 4/5] hwmon: (lm75) separate probe into common and I2C parts Wolfram Sang
2024-12-19 22:55 ` [RFC PATCH 5/5] hwmon: (lm75) add I3C support for P3T1755 Wolfram Sang
4 siblings, 1 reply; 14+ messages in thread
From: Wolfram Sang @ 2024-12-19 22:55 UTC (permalink / raw)
To: linux-renesas-soc; +Cc: Wolfram Sang, Jean Delvare, Guenter Roeck, linux-hwmon
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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [RFC PATCH 4/5] hwmon: (lm75) separate probe into common and I2C parts
2024-12-19 22:55 [RFC PATCH 0/5] hwmon: (lm75) add I3C support Wolfram Sang
` (2 preceding siblings ...)
2024-12-19 22:55 ` [RFC PATCH 3/5] hwmon: (lm75) Remove superfluous 'client' member from private struct Wolfram Sang
@ 2024-12-19 22:55 ` Wolfram Sang
2024-12-20 14:58 ` Guenter Roeck
2024-12-19 22:55 ` [RFC PATCH 5/5] hwmon: (lm75) add I3C support for P3T1755 Wolfram Sang
4 siblings, 1 reply; 14+ messages in thread
From: Wolfram Sang @ 2024-12-19 22:55 UTC (permalink / raw)
To: linux-renesas-soc; +Cc: Wolfram Sang, Jean Delvare, Guenter Roeck, linux-hwmon
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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [RFC PATCH 5/5] hwmon: (lm75) add I3C support for P3T1755
2024-12-19 22:55 [RFC PATCH 0/5] hwmon: (lm75) add I3C support Wolfram Sang
` (3 preceding siblings ...)
2024-12-19 22:55 ` [RFC PATCH 4/5] hwmon: (lm75) separate probe into common and I2C parts Wolfram Sang
@ 2024-12-19 22:55 ` Wolfram Sang
2024-12-20 6:10 ` Guenter Roeck
4 siblings, 1 reply; 14+ messages in thread
From: Wolfram Sang @ 2024-12-19 22:55 UTC (permalink / raw)
To: linux-renesas-soc; +Cc: Wolfram Sang, Jean Delvare, Guenter Roeck, linux-hwmon
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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 5/5] hwmon: (lm75) add I3C support for P3T1755
2024-12-19 22:55 ` [RFC PATCH 5/5] hwmon: (lm75) add I3C support for P3T1755 Wolfram Sang
@ 2024-12-20 6:10 ` Guenter Roeck
2024-12-20 8:20 ` Wolfram Sang
0 siblings, 1 reply; 14+ messages in thread
From: Guenter Roeck @ 2024-12-20 6:10 UTC (permalink / raw)
To: Wolfram Sang, linux-renesas-soc; +Cc: Jean Delvare, linux-hwmon
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 5/5] hwmon: (lm75) add I3C support for P3T1755
2024-12-20 6:10 ` Guenter Roeck
@ 2024-12-20 8:20 ` Wolfram Sang
0 siblings, 0 replies; 14+ messages in thread
From: Wolfram Sang @ 2024-12-20 8:20 UTC (permalink / raw)
To: Guenter Roeck; +Cc: linux-renesas-soc, Jean Delvare, linux-hwmon
[-- 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 --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 1/5] hwmon: (lm75) simplify lm75_write_config()
2024-12-19 22:55 ` [RFC PATCH 1/5] hwmon: (lm75) simplify lm75_write_config() Wolfram Sang
@ 2024-12-20 8:48 ` Geert Uytterhoeven
2024-12-20 14:56 ` Guenter Roeck
1 sibling, 0 replies; 14+ messages in thread
From: Geert Uytterhoeven @ 2024-12-20 8:48 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-renesas-soc, Jean Delvare, Guenter Roeck, linux-hwmon
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 2/5] hwmon: (lm75) simplify regulator handling
2024-12-19 22:55 ` [RFC PATCH 2/5] hwmon: (lm75) simplify regulator handling Wolfram Sang
@ 2024-12-20 8:50 ` Geert Uytterhoeven
2024-12-20 14:57 ` Guenter Roeck
1 sibling, 0 replies; 14+ messages in thread
From: Geert Uytterhoeven @ 2024-12-20 8:50 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-renesas-soc, Jean Delvare, Guenter Roeck, linux-hwmon
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 1/5] hwmon: (lm75) simplify lm75_write_config()
2024-12-19 22:55 ` [RFC PATCH 1/5] hwmon: (lm75) simplify lm75_write_config() Wolfram Sang
2024-12-20 8:48 ` Geert Uytterhoeven
@ 2024-12-20 14:56 ` Guenter Roeck
1 sibling, 0 replies; 14+ messages in thread
From: Guenter Roeck @ 2024-12-20 14:56 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-renesas-soc, Jean Delvare, linux-hwmon
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 2/5] hwmon: (lm75) simplify regulator handling
2024-12-19 22:55 ` [RFC PATCH 2/5] hwmon: (lm75) simplify regulator handling Wolfram Sang
2024-12-20 8:50 ` Geert Uytterhoeven
@ 2024-12-20 14:57 ` Guenter Roeck
1 sibling, 0 replies; 14+ messages in thread
From: Guenter Roeck @ 2024-12-20 14:57 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-renesas-soc, Jean Delvare, linux-hwmon
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 3/5] hwmon: (lm75) Remove superfluous 'client' member from private struct
2024-12-19 22:55 ` [RFC PATCH 3/5] hwmon: (lm75) Remove superfluous 'client' member from private struct Wolfram Sang
@ 2024-12-20 14:57 ` Guenter Roeck
0 siblings, 0 replies; 14+ messages in thread
From: Guenter Roeck @ 2024-12-20 14:57 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-renesas-soc, Jean Delvare, linux-hwmon
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC PATCH 4/5] hwmon: (lm75) separate probe into common and I2C parts
2024-12-19 22:55 ` [RFC PATCH 4/5] hwmon: (lm75) separate probe into common and I2C parts Wolfram Sang
@ 2024-12-20 14:58 ` Guenter Roeck
0 siblings, 0 replies; 14+ messages in thread
From: Guenter Roeck @ 2024-12-20 14:58 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-renesas-soc, Jean Delvare, linux-hwmon
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
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2024-12-20 14:58 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-19 22:55 [RFC PATCH 0/5] hwmon: (lm75) add I3C support Wolfram Sang
2024-12-19 22:55 ` [RFC PATCH 1/5] hwmon: (lm75) simplify lm75_write_config() Wolfram Sang
2024-12-20 8:48 ` Geert Uytterhoeven
2024-12-20 14:56 ` Guenter Roeck
2024-12-19 22:55 ` [RFC PATCH 2/5] hwmon: (lm75) simplify regulator handling Wolfram Sang
2024-12-20 8:50 ` Geert Uytterhoeven
2024-12-20 14:57 ` Guenter Roeck
2024-12-19 22:55 ` [RFC PATCH 3/5] hwmon: (lm75) Remove superfluous 'client' member from private struct Wolfram Sang
2024-12-20 14:57 ` Guenter Roeck
2024-12-19 22:55 ` [RFC PATCH 4/5] hwmon: (lm75) separate probe into common and I2C parts Wolfram Sang
2024-12-20 14:58 ` Guenter Roeck
2024-12-19 22:55 ` [RFC PATCH 5/5] hwmon: (lm75) add I3C support for P3T1755 Wolfram Sang
2024-12-20 6:10 ` Guenter Roeck
2024-12-20 8:20 ` Wolfram Sang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox