* [PATCH v4 01/10] dt-bindings: hwmon: add missing tmp421 binding
2021-10-12 9:14 [PATCH v4 00/10] Add per channel properies support in tmp421 Krzysztof Adamski
@ 2021-10-12 9:14 ` Krzysztof Adamski
2021-10-12 9:26 ` [PATCH v4 02/10] hwmon: (tmp421) introduce a channel struct Krzysztof Adamski
` (8 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Krzysztof Adamski @ 2021-10-12 9:14 UTC (permalink / raw)
To: Guenter Roeck, Jean Delvare; +Cc: Rob Herring, linux-hwmon, devicetree
Add basic description of the tmp421 driver DT bindings.
Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>
---
.../devicetree/bindings/hwmon/ti,tmp421.yaml | 43 +++++++++++++++++++
1 file changed, 43 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/ti,tmp421.yaml
diff --git a/Documentation/devicetree/bindings/hwmon/ti,tmp421.yaml b/Documentation/devicetree/bindings/hwmon/ti,tmp421.yaml
new file mode 100644
index 000000000000..47040ace4f73
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/ti,tmp421.yaml
@@ -0,0 +1,43 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwmon/ti,tmp421.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TMP42x/TMP44x temperature sensor
+
+maintainers:
+ - Guenter Roeck <linux@roeck-us.net>
+
+description: |
+ ±1°C Remote and Local temperature sensor
+ https://www.ti.com/lit/ds/symlink/tmp422.pdf
+
+properties:
+ compatible:
+ enum:
+ - ti,tmp421
+ - ti,tmp422
+ - ti,tmp423
+ - ti,tmp441
+ - ti,tmp442
+ reg:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ sensor@4c {
+ compatible = "ti,tmp422";
+ reg = <0x4c>;
+ };
+ };
--
2.31.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v4 02/10] hwmon: (tmp421) introduce a channel struct
2021-10-12 9:14 [PATCH v4 00/10] Add per channel properies support in tmp421 Krzysztof Adamski
2021-10-12 9:14 ` [PATCH v4 01/10] dt-bindings: hwmon: add missing tmp421 binding Krzysztof Adamski
@ 2021-10-12 9:26 ` Krzysztof Adamski
2021-10-12 14:25 ` Guenter Roeck
2021-10-12 9:27 ` [PATCH v4 03/10] hwmon: (tmp421) add support for defining labels from DT Krzysztof Adamski
` (7 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Krzysztof Adamski @ 2021-10-12 9:26 UTC (permalink / raw)
To: Guenter Roeck, Jean Delvare; +Cc: Rob Herring, linux-hwmon, devicetree
This is a preparatory change. Upcoming patches will introduce more
per-channel parameters so it's worth organizing them into a struct.
Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>
---
drivers/hwmon/tmp421.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
index 356c96c3588b..707310d616a4 100644
--- a/drivers/hwmon/tmp421.c
+++ b/drivers/hwmon/tmp421.c
@@ -87,6 +87,10 @@ static const struct of_device_id __maybe_unused tmp421_of_match[] = {
};
MODULE_DEVICE_TABLE(of, tmp421_of_match);
+struct tmp421_channel {
+ s16 temp;
+};
+
struct tmp421_data {
struct i2c_client *client;
struct mutex update_lock;
@@ -98,7 +102,7 @@ struct tmp421_data {
unsigned long last_updated;
unsigned long channels;
u8 config;
- s16 temp[MAX_CHANNELS];
+ struct tmp421_channel channel[MAX_CHANNELS];
};
static int temp_from_raw(u16 reg, bool extended)
@@ -133,12 +137,12 @@ static int tmp421_update_device(struct tmp421_data *data)
ret = i2c_smbus_read_byte_data(client, TMP421_TEMP_MSB[i]);
if (ret < 0)
goto exit;
- data->temp[i] = ret << 8;
+ data->channel[i].temp = ret << 8;
ret = i2c_smbus_read_byte_data(client, TMP421_TEMP_LSB[i]);
if (ret < 0)
goto exit;
- data->temp[i] |= ret;
+ data->channel[i].temp |= ret;
}
data->last_updated = jiffies;
data->valid = true;
@@ -167,7 +171,7 @@ static int tmp421_read(struct device *dev, enum hwmon_sensor_types type,
switch (attr) {
case hwmon_temp_input:
- *val = temp_from_raw(tmp421->temp[channel],
+ *val = temp_from_raw(tmp421->channel[channel].temp,
tmp421->config & TMP421_CONFIG_RANGE);
return 0;
case hwmon_temp_fault:
@@ -175,7 +179,7 @@ static int tmp421_read(struct device *dev, enum hwmon_sensor_types type,
* Any of OPEN or /PVLD bits indicate a hardware mulfunction
* and the conversion result may be incorrect
*/
- *val = !!(tmp421->temp[channel] & 0x03);
+ *val = !!(tmp421->channel[channel].temp & 0x03);
return 0;
default:
return -EOPNOTSUPP;
--
2.31.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v4 02/10] hwmon: (tmp421) introduce a channel struct
2021-10-12 9:26 ` [PATCH v4 02/10] hwmon: (tmp421) introduce a channel struct Krzysztof Adamski
@ 2021-10-12 14:25 ` Guenter Roeck
0 siblings, 0 replies; 22+ messages in thread
From: Guenter Roeck @ 2021-10-12 14:25 UTC (permalink / raw)
To: Krzysztof Adamski; +Cc: Jean Delvare, Rob Herring, linux-hwmon, devicetree
On Tue, Oct 12, 2021 at 11:26:55AM +0200, Krzysztof Adamski wrote:
> This is a preparatory change. Upcoming patches will introduce more
> per-channel parameters so it's worth organizing them into a struct.
>
> Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>
Applied to hwmon-next.
Thanks,
Guenter
> ---
> drivers/hwmon/tmp421.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> index 356c96c3588b..707310d616a4 100644
> --- a/drivers/hwmon/tmp421.c
> +++ b/drivers/hwmon/tmp421.c
> @@ -87,6 +87,10 @@ static const struct of_device_id __maybe_unused tmp421_of_match[] = {
> };
> MODULE_DEVICE_TABLE(of, tmp421_of_match);
>
> +struct tmp421_channel {
> + s16 temp;
> +};
> +
> struct tmp421_data {
> struct i2c_client *client;
> struct mutex update_lock;
> @@ -98,7 +102,7 @@ struct tmp421_data {
> unsigned long last_updated;
> unsigned long channels;
> u8 config;
> - s16 temp[MAX_CHANNELS];
> + struct tmp421_channel channel[MAX_CHANNELS];
> };
>
> static int temp_from_raw(u16 reg, bool extended)
> @@ -133,12 +137,12 @@ static int tmp421_update_device(struct tmp421_data *data)
> ret = i2c_smbus_read_byte_data(client, TMP421_TEMP_MSB[i]);
> if (ret < 0)
> goto exit;
> - data->temp[i] = ret << 8;
> + data->channel[i].temp = ret << 8;
>
> ret = i2c_smbus_read_byte_data(client, TMP421_TEMP_LSB[i]);
> if (ret < 0)
> goto exit;
> - data->temp[i] |= ret;
> + data->channel[i].temp |= ret;
> }
> data->last_updated = jiffies;
> data->valid = true;
> @@ -167,7 +171,7 @@ static int tmp421_read(struct device *dev, enum hwmon_sensor_types type,
>
> switch (attr) {
> case hwmon_temp_input:
> - *val = temp_from_raw(tmp421->temp[channel],
> + *val = temp_from_raw(tmp421->channel[channel].temp,
> tmp421->config & TMP421_CONFIG_RANGE);
> return 0;
> case hwmon_temp_fault:
> @@ -175,7 +179,7 @@ static int tmp421_read(struct device *dev, enum hwmon_sensor_types type,
> * Any of OPEN or /PVLD bits indicate a hardware mulfunction
> * and the conversion result may be incorrect
> */
> - *val = !!(tmp421->temp[channel] & 0x03);
> + *val = !!(tmp421->channel[channel].temp & 0x03);
> return 0;
> default:
> return -EOPNOTSUPP;
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4 03/10] hwmon: (tmp421) add support for defining labels from DT
2021-10-12 9:14 [PATCH v4 00/10] Add per channel properies support in tmp421 Krzysztof Adamski
2021-10-12 9:14 ` [PATCH v4 01/10] dt-bindings: hwmon: add missing tmp421 binding Krzysztof Adamski
2021-10-12 9:26 ` [PATCH v4 02/10] hwmon: (tmp421) introduce a channel struct Krzysztof Adamski
@ 2021-10-12 9:27 ` Krzysztof Adamski
2021-10-12 14:30 ` Guenter Roeck
2021-10-12 9:27 ` [PATCH v4 04/10] hwmon: (tmp421) support disabling channels " Krzysztof Adamski
` (6 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Krzysztof Adamski @ 2021-10-12 9:27 UTC (permalink / raw)
To: Guenter Roeck, Jean Delvare; +Cc: Rob Herring, linux-hwmon, devicetree
tmp42x is a multichannel temperature sensor with several external
channels. Since those channels can be used to connect diodes placed
anywhere in the system, their meaning will vary depending on the
project. For this case, the hwmon framework has an idea of labels which
allows us to assign the meaning to each channel.
The similar concept is already implemented in ina3221 - the label for
each channel can be defined via device tree. See commit a9e9dd9c6de5
("hwmon: (ina3221) Read channel input source info from DT")
This patch adds support for similar feature to tmp421.
Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>
---
drivers/hwmon/tmp421.c | 61 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 61 insertions(+)
diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
index 707310d616a4..ab64d9825ca4 100644
--- a/drivers/hwmon/tmp421.c
+++ b/drivers/hwmon/tmp421.c
@@ -88,6 +88,7 @@ static const struct of_device_id __maybe_unused tmp421_of_match[] = {
MODULE_DEVICE_TABLE(of, tmp421_of_match);
struct tmp421_channel {
+ const char *label;
s16 temp;
};
@@ -187,6 +188,16 @@ static int tmp421_read(struct device *dev, enum hwmon_sensor_types type,
}
+static int tmp421_read_string(struct device *dev, enum hwmon_sensor_types type,
+ u32 attr, int channel, const char **str)
+{
+ struct tmp421_data *data = dev_get_drvdata(dev);
+
+ *str = data->channel[channel].label;
+
+ return 0;
+}
+
static umode_t tmp421_is_visible(const void *data, enum hwmon_sensor_types type,
u32 attr, int channel)
{
@@ -194,6 +205,8 @@ static umode_t tmp421_is_visible(const void *data, enum hwmon_sensor_types type,
case hwmon_temp_fault:
case hwmon_temp_input:
return 0444;
+ case hwmon_temp_label:
+ return 0444;
default:
return 0;
}
@@ -286,9 +299,53 @@ static int tmp421_detect(struct i2c_client *client,
return 0;
}
+static int tmp421_probe_child_from_dt(struct i2c_client *client,
+ struct device_node *child,
+ struct tmp421_data *data)
+
+{
+ struct device *dev = &client->dev;
+ u32 i;
+ int err;
+
+ err = of_property_read_u32(child, "reg", &i);
+ if (err) {
+ dev_err(dev, "missing reg property of %pOFn\n", child);
+ return err;
+ }
+
+ if (i >= MAX_CHANNELS) {
+ dev_err(dev, "invalid reg %d of %pOFn\n", i, child);
+ return -EINVAL;
+ }
+
+ of_property_read_string(child, "label", &data->channel[i].label);
+ if (data->channel[i].label)
+ data->temp_config[i] |= HWMON_T_LABEL;
+
+ return 0;
+}
+
+static int tmp421_probe_from_dt(struct i2c_client *client, struct tmp421_data *data)
+{
+ struct device *dev = &client->dev;
+ const struct device_node *np = dev->of_node;
+ struct device_node *child;
+ int err;
+
+ for_each_child_of_node(np, child) {
+ err = tmp421_probe_child_from_dt(client, child, data);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
+
static const struct hwmon_ops tmp421_ops = {
.is_visible = tmp421_is_visible,
.read = tmp421_read,
+ .read_string = tmp421_read_string,
};
static int tmp421_probe(struct i2c_client *client)
@@ -317,6 +374,10 @@ static int tmp421_probe(struct i2c_client *client)
for (i = 0; i < data->channels; i++)
data->temp_config[i] = HWMON_T_INPUT | HWMON_T_FAULT;
+ err = tmp421_probe_from_dt(client, data);
+ if (err)
+ return err;
+
data->chip.ops = &tmp421_ops;
data->chip.info = data->info;
--
2.31.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v4 03/10] hwmon: (tmp421) add support for defining labels from DT
2021-10-12 9:27 ` [PATCH v4 03/10] hwmon: (tmp421) add support for defining labels from DT Krzysztof Adamski
@ 2021-10-12 14:30 ` Guenter Roeck
0 siblings, 0 replies; 22+ messages in thread
From: Guenter Roeck @ 2021-10-12 14:30 UTC (permalink / raw)
To: Krzysztof Adamski; +Cc: Jean Delvare, Rob Herring, linux-hwmon, devicetree
On Tue, Oct 12, 2021 at 11:27:14AM +0200, Krzysztof Adamski wrote:
> tmp42x is a multichannel temperature sensor with several external
> channels. Since those channels can be used to connect diodes placed
> anywhere in the system, their meaning will vary depending on the
> project. For this case, the hwmon framework has an idea of labels which
> allows us to assign the meaning to each channel.
>
> The similar concept is already implemented in ina3221 - the label for
> each channel can be defined via device tree. See commit a9e9dd9c6de5
> ("hwmon: (ina3221) Read channel input source info from DT")
>
> This patch adds support for similar feature to tmp421.
>
> Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>
> ---
> drivers/hwmon/tmp421.c | 61 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 61 insertions(+)
>
> diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> index 707310d616a4..ab64d9825ca4 100644
> --- a/drivers/hwmon/tmp421.c
> +++ b/drivers/hwmon/tmp421.c
> @@ -88,6 +88,7 @@ static const struct of_device_id __maybe_unused tmp421_of_match[] = {
> MODULE_DEVICE_TABLE(of, tmp421_of_match);
>
> struct tmp421_channel {
> + const char *label;
> s16 temp;
> };
>
> @@ -187,6 +188,16 @@ static int tmp421_read(struct device *dev, enum hwmon_sensor_types type,
>
> }
>
> +static int tmp421_read_string(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, const char **str)
> +{
> + struct tmp421_data *data = dev_get_drvdata(dev);
> +
> + *str = data->channel[channel].label;
> +
> + return 0;
> +}
> +
> static umode_t tmp421_is_visible(const void *data, enum hwmon_sensor_types type,
> u32 attr, int channel)
> {
> @@ -194,6 +205,8 @@ static umode_t tmp421_is_visible(const void *data, enum hwmon_sensor_types type,
> case hwmon_temp_fault:
> case hwmon_temp_input:
> return 0444;
> + case hwmon_temp_label:
> + return 0444;
> default:
> return 0;
> }
> @@ -286,9 +299,53 @@ static int tmp421_detect(struct i2c_client *client,
> return 0;
> }
>
> +static int tmp421_probe_child_from_dt(struct i2c_client *client,
> + struct device_node *child,
> + struct tmp421_data *data)
> +
> +{
> + struct device *dev = &client->dev;
> + u32 i;
> + int err;
> +
> + err = of_property_read_u32(child, "reg", &i);
> + if (err) {
> + dev_err(dev, "missing reg property of %pOFn\n", child);
> + return err;
> + }
> +
> + if (i >= MAX_CHANNELS) {
I think this needs to compare against data->channels. Otherwise, if
unsupported channels are listed in dt (say, reg==0x2 with tmp421),
we would end up with label attributes for those channels.
> + dev_err(dev, "invalid reg %d of %pOFn\n", i, child);
> + return -EINVAL;
> + }
> +
> + of_property_read_string(child, "label", &data->channel[i].label);
> + if (data->channel[i].label)
> + data->temp_config[i] |= HWMON_T_LABEL;
> +
> + return 0;
> +}
> +
> +static int tmp421_probe_from_dt(struct i2c_client *client, struct tmp421_data *data)
> +{
> + struct device *dev = &client->dev;
> + const struct device_node *np = dev->of_node;
> + struct device_node *child;
> + int err;
> +
> + for_each_child_of_node(np, child) {
> + err = tmp421_probe_child_from_dt(client, child, data);
> + if (err)
> + return err;
> + }
> +
> + return 0;
> +}
> +
> static const struct hwmon_ops tmp421_ops = {
> .is_visible = tmp421_is_visible,
> .read = tmp421_read,
> + .read_string = tmp421_read_string,
> };
>
> static int tmp421_probe(struct i2c_client *client)
> @@ -317,6 +374,10 @@ static int tmp421_probe(struct i2c_client *client)
> for (i = 0; i < data->channels; i++)
> data->temp_config[i] = HWMON_T_INPUT | HWMON_T_FAULT;
>
> + err = tmp421_probe_from_dt(client, data);
> + if (err)
> + return err;
> +
> data->chip.ops = &tmp421_ops;
> data->chip.info = data->info;
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4 04/10] hwmon: (tmp421) support disabling channels from DT
2021-10-12 9:14 [PATCH v4 00/10] Add per channel properies support in tmp421 Krzysztof Adamski
` (2 preceding siblings ...)
2021-10-12 9:27 ` [PATCH v4 03/10] hwmon: (tmp421) add support for defining labels from DT Krzysztof Adamski
@ 2021-10-12 9:27 ` Krzysztof Adamski
2021-10-12 14:32 ` Guenter Roeck
2021-10-12 9:28 ` [PATCH v4 05/10] hwmon: (tmp421) support specifying n-factor via DT Krzysztof Adamski
` (5 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Krzysztof Adamski @ 2021-10-12 9:27 UTC (permalink / raw)
To: Guenter Roeck, Jean Delvare; +Cc: Rob Herring, linux-hwmon, devicetree
The previous patch introduced per channel subnodes in DT that let us
specify some channel specific properties. This built a ground for easily
disabling individual channels of the sensor that may not be connected to
any external diode and thus are not returning any meaningful data.
This patch adds support for parsing the "status" property of channels DT
subnodes and makes sure the -ENODATA is returned when disabled channels
value is read.
Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>
---
drivers/hwmon/tmp421.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
index ab64d9825ca4..fffffd671e34 100644
--- a/drivers/hwmon/tmp421.c
+++ b/drivers/hwmon/tmp421.c
@@ -89,6 +89,7 @@ MODULE_DEVICE_TABLE(of, tmp421_of_match);
struct tmp421_channel {
const char *label;
+ bool enabled;
s16 temp;
};
@@ -170,6 +171,9 @@ static int tmp421_read(struct device *dev, enum hwmon_sensor_types type,
if (ret)
return ret;
+ if (!tmp421->channel[channel].enabled)
+ return -ENODATA;
+
switch (attr) {
case hwmon_temp_input:
*val = temp_from_raw(tmp421->channel[channel].temp,
@@ -323,6 +327,8 @@ static int tmp421_probe_child_from_dt(struct i2c_client *client,
if (data->channel[i].label)
data->temp_config[i] |= HWMON_T_LABEL;
+ data->channel[i].enabled = of_device_is_available(child);
+
return 0;
}
@@ -371,8 +377,10 @@ static int tmp421_probe(struct i2c_client *client)
if (err)
return err;
- for (i = 0; i < data->channels; i++)
+ for (i = 0; i < data->channels; i++) {
data->temp_config[i] = HWMON_T_INPUT | HWMON_T_FAULT;
+ data->channel[i].enabled = true;
+ }
err = tmp421_probe_from_dt(client, data);
if (err)
--
2.31.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v4 04/10] hwmon: (tmp421) support disabling channels from DT
2021-10-12 9:27 ` [PATCH v4 04/10] hwmon: (tmp421) support disabling channels " Krzysztof Adamski
@ 2021-10-12 14:32 ` Guenter Roeck
0 siblings, 0 replies; 22+ messages in thread
From: Guenter Roeck @ 2021-10-12 14:32 UTC (permalink / raw)
To: Krzysztof Adamski, Jean Delvare; +Cc: Rob Herring, linux-hwmon, devicetree
On 10/12/21 2:27 AM, Krzysztof Adamski wrote:
> The previous patch introduced per channel subnodes in DT that let us
> specify some channel specific properties. This built a ground for easily
> disabling individual channels of the sensor that may not be connected to
> any external diode and thus are not returning any meaningful data.
>
> This patch adds support for parsing the "status" property of channels DT
> subnodes and makes sure the -ENODATA is returned when disabled channels
> value is read.
>
> Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>
For my reference:
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/hwmon/tmp421.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> index ab64d9825ca4..fffffd671e34 100644
> --- a/drivers/hwmon/tmp421.c
> +++ b/drivers/hwmon/tmp421.c
> @@ -89,6 +89,7 @@ MODULE_DEVICE_TABLE(of, tmp421_of_match);
>
> struct tmp421_channel {
> const char *label;
> + bool enabled;
> s16 temp;
> };
>
> @@ -170,6 +171,9 @@ static int tmp421_read(struct device *dev, enum hwmon_sensor_types type,
> if (ret)
> return ret;
>
> + if (!tmp421->channel[channel].enabled)
> + return -ENODATA;
> +
> switch (attr) {
> case hwmon_temp_input:
> *val = temp_from_raw(tmp421->channel[channel].temp,
> @@ -323,6 +327,8 @@ static int tmp421_probe_child_from_dt(struct i2c_client *client,
> if (data->channel[i].label)
> data->temp_config[i] |= HWMON_T_LABEL;
>
> + data->channel[i].enabled = of_device_is_available(child);
> +
> return 0;
> }
>
> @@ -371,8 +377,10 @@ static int tmp421_probe(struct i2c_client *client)
> if (err)
> return err;
>
> - for (i = 0; i < data->channels; i++)
> + for (i = 0; i < data->channels; i++) {
> data->temp_config[i] = HWMON_T_INPUT | HWMON_T_FAULT;
> + data->channel[i].enabled = true;
> + }
>
> err = tmp421_probe_from_dt(client, data);
> if (err)
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4 05/10] hwmon: (tmp421) support specifying n-factor via DT
2021-10-12 9:14 [PATCH v4 00/10] Add per channel properies support in tmp421 Krzysztof Adamski
` (3 preceding siblings ...)
2021-10-12 9:27 ` [PATCH v4 04/10] hwmon: (tmp421) support disabling channels " Krzysztof Adamski
@ 2021-10-12 9:28 ` Krzysztof Adamski
2021-10-12 14:34 ` Guenter Roeck
2021-10-12 9:29 ` [PATCH v4 06/10] hwmon: (tmp421) really disable channels Krzysztof Adamski
` (4 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Krzysztof Adamski @ 2021-10-12 9:28 UTC (permalink / raw)
To: Guenter Roeck, Jean Delvare; +Cc: Rob Herring, linux-hwmon, devicetree
Previous patches added a way to specify some channel specific parameters
in DT and n-factor is definitely one of them. This calibration mechanism
is board specific as its value depends on the diodes/transistors being
connected to the sensor and thus the DT seems like a right fit for that
information. It is very similar to the value of shunt resistor that some
drivers allows specifying in DT.
This patch adds a possibility to set n-factor for each channel via
"n-factor" DT property in each channel subnode.
Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>
---
drivers/hwmon/tmp421.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
index fffffd671e34..fcd2932a6ddb 100644
--- a/drivers/hwmon/tmp421.c
+++ b/drivers/hwmon/tmp421.c
@@ -34,6 +34,7 @@ enum chips { tmp421, tmp422, tmp423, tmp441, tmp442 };
#define TMP421_STATUS_REG 0x08
#define TMP421_CONFIG_REG_1 0x09
#define TMP421_CONVERSION_RATE_REG 0x0B
+#define TMP421_N_FACTOR_REG_1 0x21
#define TMP421_MANUFACTURER_ID_REG 0xFE
#define TMP421_DEVICE_ID_REG 0xFF
@@ -310,6 +311,7 @@ static int tmp421_probe_child_from_dt(struct i2c_client *client,
{
struct device *dev = &client->dev;
u32 i;
+ s32 val;
int err;
err = of_property_read_u32(child, "reg", &i);
@@ -329,6 +331,20 @@ static int tmp421_probe_child_from_dt(struct i2c_client *client,
data->channel[i].enabled = of_device_is_available(child);
+ if (i == 0)
+ return 0; /* input 0 is internal channel */
+
+ err = of_property_read_s32(child, "ti,n-factor", &val);
+ if (!err) {
+ if (val > 127 || val < -128) {
+ dev_err(dev, "n-factor for channel %d invalid (%d)\n",
+ i, val);
+ return -EINVAL;
+ }
+ i2c_smbus_write_byte_data(client, TMP421_N_FACTOR_REG_1 + i - 1,
+ val);
+ }
+
return 0;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v4 05/10] hwmon: (tmp421) support specifying n-factor via DT
2021-10-12 9:28 ` [PATCH v4 05/10] hwmon: (tmp421) support specifying n-factor via DT Krzysztof Adamski
@ 2021-10-12 14:34 ` Guenter Roeck
0 siblings, 0 replies; 22+ messages in thread
From: Guenter Roeck @ 2021-10-12 14:34 UTC (permalink / raw)
To: Krzysztof Adamski, Jean Delvare; +Cc: Rob Herring, linux-hwmon, devicetree
On 10/12/21 2:28 AM, Krzysztof Adamski wrote:
> Previous patches added a way to specify some channel specific parameters
> in DT and n-factor is definitely one of them. This calibration mechanism
> is board specific as its value depends on the diodes/transistors being
> connected to the sensor and thus the DT seems like a right fit for that
> information. It is very similar to the value of shunt resistor that some
> drivers allows specifying in DT.
>
> This patch adds a possibility to set n-factor for each channel via
> "n-factor" DT property in each channel subnode.
>
> Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>
> ---
> drivers/hwmon/tmp421.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> index fffffd671e34..fcd2932a6ddb 100644
> --- a/drivers/hwmon/tmp421.c
> +++ b/drivers/hwmon/tmp421.c
> @@ -34,6 +34,7 @@ enum chips { tmp421, tmp422, tmp423, tmp441, tmp442 };
> #define TMP421_STATUS_REG 0x08
> #define TMP421_CONFIG_REG_1 0x09
> #define TMP421_CONVERSION_RATE_REG 0x0B
> +#define TMP421_N_FACTOR_REG_1 0x21
> #define TMP421_MANUFACTURER_ID_REG 0xFE
> #define TMP421_DEVICE_ID_REG 0xFF
>
> @@ -310,6 +311,7 @@ static int tmp421_probe_child_from_dt(struct i2c_client *client,
> {
> struct device *dev = &client->dev;
> u32 i;
> + s32 val;
> int err;
>
> err = of_property_read_u32(child, "reg", &i);
> @@ -329,6 +331,20 @@ static int tmp421_probe_child_from_dt(struct i2c_client *client,
>
> data->channel[i].enabled = of_device_is_available(child);
>
> + if (i == 0)
> + return 0; /* input 0 is internal channel */
> +
I think that should generate an error, ie
> + err = of_property_read_s32(child, "ti,n-factor", &val);
> + if (!err) {
if (!i)
return -EINVAL;
> + if (val > 127 || val < -128) {
> + dev_err(dev, "n-factor for channel %d invalid (%d)\n",
> + i, val);
> + return -EINVAL;
> + }
> + i2c_smbus_write_byte_data(client, TMP421_N_FACTOR_REG_1 + i - 1,
> + val);
> + }
> +
> return 0;
> }
>
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4 06/10] hwmon: (tmp421) really disable channels
2021-10-12 9:14 [PATCH v4 00/10] Add per channel properies support in tmp421 Krzysztof Adamski
` (4 preceding siblings ...)
2021-10-12 9:28 ` [PATCH v4 05/10] hwmon: (tmp421) support specifying n-factor via DT Krzysztof Adamski
@ 2021-10-12 9:29 ` Krzysztof Adamski
2021-10-12 14:37 ` Guenter Roeck
` (3 more replies)
2021-10-12 9:29 ` [PATCH v4 07/10] hwmon: (tmp421) support HWMON_T_ENABLE Krzysztof Adamski
` (3 subsequent siblings)
9 siblings, 4 replies; 22+ messages in thread
From: Krzysztof Adamski @ 2021-10-12 9:29 UTC (permalink / raw)
To: Guenter Roeck, Jean Delvare; +Cc: Rob Herring, linux-hwmon, devicetree
Recent patch added possibility to disable selected channels. That would
only make sure that the ENODATA is returned for those channels but would
not configure the actual hardware.
With this patch, the config register is written to make sure the
channels are disabled also at hardware level.
Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>
---
drivers/hwmon/tmp421.c | 41 +++++++++++++++++++++++++++++++++++------
1 file changed, 35 insertions(+), 6 deletions(-)
diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
index fcd2932a6ddb..45cb197cd277 100644
--- a/drivers/hwmon/tmp421.c
+++ b/drivers/hwmon/tmp421.c
@@ -33,6 +33,9 @@ enum chips { tmp421, tmp422, tmp423, tmp441, tmp442 };
/* The TMP421 registers */
#define TMP421_STATUS_REG 0x08
#define TMP421_CONFIG_REG_1 0x09
+#define TMP421_CONFIG_REG_2 0x0A
+#define TMP421_CONFIG_REG_REN(x) (BIT(3 + (x)))
+#define TMP421_CONFIG_REG_REN_MASK GENMASK(6, 3)
#define TMP421_CONVERSION_RATE_REG 0x0B
#define TMP421_N_FACTOR_REG_1 0x21
#define TMP421_MANUFACTURER_ID_REG 0xFE
@@ -162,6 +165,31 @@ static int tmp421_update_device(struct tmp421_data *data)
return 0;
}
+static int tmp421_enable_channels(struct tmp421_data *data)
+{
+ int err;
+ struct i2c_client *client = data->client;
+ struct device *dev = &client->dev;
+ int cfg = i2c_smbus_read_byte_data(client, TMP421_CONFIG_REG_2);
+ int i;
+
+ if (cfg < 0) {
+ dev_err(dev, "error reading register, can't disable channels\n");
+ return err;
+ }
+
+ cfg &= ~TMP421_CONFIG_REG_REN_MASK;
+ for (i = 0; i < data->channels; i++)
+ if (data->channel[i].enabled)
+ cfg |= TMP421_CONFIG_REG_REN(i);
+
+ err = i2c_smbus_write_byte_data(client, TMP421_CONFIG_REG_2, cfg);
+ if (err < 0)
+ dev_err(dev, "error writing register, can't disable channels\n");
+
+ return err;
+}
+
static int tmp421_read(struct device *dev, enum hwmon_sensor_types type,
u32 attr, int channel, long *val)
{
@@ -217,9 +245,10 @@ static umode_t tmp421_is_visible(const void *data, enum hwmon_sensor_types type,
}
}
-static int tmp421_init_client(struct i2c_client *client)
+static int tmp421_init_client(struct tmp421_data *data)
{
int config, config_orig;
+ struct i2c_client *client = data->client;
/* Set the conversion rate to 2 Hz */
i2c_smbus_write_byte_data(client, TMP421_CONVERSION_RATE_REG, 0x05);
@@ -240,7 +269,7 @@ static int tmp421_init_client(struct i2c_client *client)
i2c_smbus_write_byte_data(client, TMP421_CONFIG_REG_1, config);
}
- return 0;
+ return tmp421_enable_channels(data);
}
static int tmp421_detect(struct i2c_client *client,
@@ -389,10 +418,6 @@ static int tmp421_probe(struct i2c_client *client)
data->channels = i2c_match_id(tmp421_id, client)->driver_data;
data->client = client;
- err = tmp421_init_client(client);
- if (err)
- return err;
-
for (i = 0; i < data->channels; i++) {
data->temp_config[i] = HWMON_T_INPUT | HWMON_T_FAULT;
data->channel[i].enabled = true;
@@ -402,6 +427,10 @@ static int tmp421_probe(struct i2c_client *client)
if (err)
return err;
+ err = tmp421_init_client(data);
+ if (err)
+ return err;
+
data->chip.ops = &tmp421_ops;
data->chip.info = data->info;
--
2.31.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v4 06/10] hwmon: (tmp421) really disable channels
2021-10-12 9:29 ` [PATCH v4 06/10] hwmon: (tmp421) really disable channels Krzysztof Adamski
@ 2021-10-12 14:37 ` Guenter Roeck
2021-10-12 18:01 ` kernel test robot
` (2 subsequent siblings)
3 siblings, 0 replies; 22+ messages in thread
From: Guenter Roeck @ 2021-10-12 14:37 UTC (permalink / raw)
To: Krzysztof Adamski, Jean Delvare; +Cc: Rob Herring, linux-hwmon, devicetree
On 10/12/21 2:29 AM, Krzysztof Adamski wrote:
> Recent patch added possibility to disable selected channels. That would
> only make sure that the ENODATA is returned for those channels but would
> not configure the actual hardware.
>
> With this patch, the config register is written to make sure the
> channels are disabled also at hardware level.
>
> Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>
> ---
> drivers/hwmon/tmp421.c | 41 +++++++++++++++++++++++++++++++++++------
> 1 file changed, 35 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> index fcd2932a6ddb..45cb197cd277 100644
> --- a/drivers/hwmon/tmp421.c
> +++ b/drivers/hwmon/tmp421.c
> @@ -33,6 +33,9 @@ enum chips { tmp421, tmp422, tmp423, tmp441, tmp442 };
> /* The TMP421 registers */
> #define TMP421_STATUS_REG 0x08
> #define TMP421_CONFIG_REG_1 0x09
> +#define TMP421_CONFIG_REG_2 0x0A
> +#define TMP421_CONFIG_REG_REN(x) (BIT(3 + (x)))
> +#define TMP421_CONFIG_REG_REN_MASK GENMASK(6, 3)
> #define TMP421_CONVERSION_RATE_REG 0x0B
> #define TMP421_N_FACTOR_REG_1 0x21
> #define TMP421_MANUFACTURER_ID_REG 0xFE
> @@ -162,6 +165,31 @@ static int tmp421_update_device(struct tmp421_data *data)
> return 0;
> }
>
> +static int tmp421_enable_channels(struct tmp421_data *data)
> +{
> + int err;
> + struct i2c_client *client = data->client;
> + struct device *dev = &client->dev;
> + int cfg = i2c_smbus_read_byte_data(client, TMP421_CONFIG_REG_2);
> + int i;
> +
> + if (cfg < 0) {
> + dev_err(dev, "error reading register, can't disable channels\n");
> + return err;
return cfg;
> + }
> +
> + cfg &= ~TMP421_CONFIG_REG_REN_MASK;
> + for (i = 0; i < data->channels; i++)
> + if (data->channel[i].enabled)
> + cfg |= TMP421_CONFIG_REG_REN(i);
> +
> + err = i2c_smbus_write_byte_data(client, TMP421_CONFIG_REG_2, cfg);
Please make the write optional: It is only necessary if the old register
value differs from the new register value.
> + if (err < 0)
> + dev_err(dev, "error writing register, can't disable channels\n");
> +
> + return err;
> +}
> +
> static int tmp421_read(struct device *dev, enum hwmon_sensor_types type,
> u32 attr, int channel, long *val)
> {
> @@ -217,9 +245,10 @@ static umode_t tmp421_is_visible(const void *data, enum hwmon_sensor_types type,
> }
> }
>
> -static int tmp421_init_client(struct i2c_client *client)
> +static int tmp421_init_client(struct tmp421_data *data)
> {
> int config, config_orig;
> + struct i2c_client *client = data->client;
>
> /* Set the conversion rate to 2 Hz */
> i2c_smbus_write_byte_data(client, TMP421_CONVERSION_RATE_REG, 0x05);
> @@ -240,7 +269,7 @@ static int tmp421_init_client(struct i2c_client *client)
> i2c_smbus_write_byte_data(client, TMP421_CONFIG_REG_1, config);
> }
>
> - return 0;
> + return tmp421_enable_channels(data);
> }
>
> static int tmp421_detect(struct i2c_client *client,
> @@ -389,10 +418,6 @@ static int tmp421_probe(struct i2c_client *client)
> data->channels = i2c_match_id(tmp421_id, client)->driver_data;
> data->client = client;
>
> - err = tmp421_init_client(client);
> - if (err)
> - return err;
> -
> for (i = 0; i < data->channels; i++) {
> data->temp_config[i] = HWMON_T_INPUT | HWMON_T_FAULT;
> data->channel[i].enabled = true;
> @@ -402,6 +427,10 @@ static int tmp421_probe(struct i2c_client *client)
> if (err)
> return err;
>
> + err = tmp421_init_client(data);
> + if (err)
> + return err;
> +
> data->chip.ops = &tmp421_ops;
> data->chip.info = data->info;
>
>
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v4 06/10] hwmon: (tmp421) really disable channels
2021-10-12 9:29 ` [PATCH v4 06/10] hwmon: (tmp421) really disable channels Krzysztof Adamski
2021-10-12 14:37 ` Guenter Roeck
@ 2021-10-12 18:01 ` kernel test robot
2021-10-13 8:36 ` Dan Carpenter
2021-10-13 18:50 ` kernel test robot
3 siblings, 0 replies; 22+ messages in thread
From: kernel test robot @ 2021-10-12 18:01 UTC (permalink / raw)
To: Krzysztof Adamski, Guenter Roeck, Jean Delvare
Cc: llvm, kbuild-all, Rob Herring, linux-hwmon, devicetree
[-- Attachment #1: Type: text/plain, Size: 3028 bytes --]
Hi Krzysztof,
I love your patch! Perhaps something to improve:
[auto build test WARNING on groeck-staging/hwmon-next]
[also build test WARNING on next-20211012]
[cannot apply to robh/for-next v5.15-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Krzysztof-Adamski/Add-per-channel-properies-support-in-tmp421/20211012-173142
base: https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
config: arm64-randconfig-r024-20211012 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project c3dcf39554dbea780d6cb7e12239451ba47a2668)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm64 cross compiling tool for clang build
# apt-get install binutils-aarch64-linux-gnu
# https://github.com/0day-ci/linux/commit/4a0f8262fe071b0b27c6fba7455627f3c5a5209e
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Krzysztof-Adamski/Add-per-channel-properies-support-in-tmp421/20211012-173142
git checkout 4a0f8262fe071b0b27c6fba7455627f3c5a5209e
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=arm64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/hwmon/tmp421.c:178:10: warning: variable 'err' is uninitialized when used here [-Wuninitialized]
return err;
^~~
drivers/hwmon/tmp421.c:170:9: note: initialize the variable 'err' to silence this warning
int err;
^
= 0
1 warning generated.
vim +/err +178 drivers/hwmon/tmp421.c
167
168 static int tmp421_enable_channels(struct tmp421_data *data)
169 {
170 int err;
171 struct i2c_client *client = data->client;
172 struct device *dev = &client->dev;
173 int cfg = i2c_smbus_read_byte_data(client, TMP421_CONFIG_REG_2);
174 int i;
175
176 if (cfg < 0) {
177 dev_err(dev, "error reading register, can't disable channels\n");
> 178 return err;
179 }
180
181 cfg &= ~TMP421_CONFIG_REG_REN_MASK;
182 for (i = 0; i < data->channels; i++)
183 if (data->channel[i].enabled)
184 cfg |= TMP421_CONFIG_REG_REN(i);
185
186 err = i2c_smbus_write_byte_data(client, TMP421_CONFIG_REG_2, cfg);
187 if (err < 0)
188 dev_err(dev, "error writing register, can't disable channels\n");
189
190 return err;
191 }
192
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 45131 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v4 06/10] hwmon: (tmp421) really disable channels
2021-10-12 9:29 ` [PATCH v4 06/10] hwmon: (tmp421) really disable channels Krzysztof Adamski
2021-10-12 14:37 ` Guenter Roeck
2021-10-12 18:01 ` kernel test robot
@ 2021-10-13 8:36 ` Dan Carpenter
2021-10-13 18:50 ` kernel test robot
3 siblings, 0 replies; 22+ messages in thread
From: Dan Carpenter @ 2021-10-13 8:36 UTC (permalink / raw)
To: kbuild, Krzysztof Adamski, Guenter Roeck, Jean Delvare
Cc: lkp, kbuild-all, Rob Herring, linux-hwmon, devicetree
Hi Krzysztof,
url: https://github.com/0day-ci/linux/commits/Krzysztof-Adamski/Add-per-channel-properies-support-in-tmp421/20211012-173142
base: https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
config: i386-randconfig-m031-20211012 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
drivers/hwmon/tmp421.c:178 tmp421_enable_channels() error: uninitialized symbol 'err'.
vim +/err +178 drivers/hwmon/tmp421.c
4a0f8262fe071b Krzysztof Adamski 2021-10-12 168 static int tmp421_enable_channels(struct tmp421_data *data)
4a0f8262fe071b Krzysztof Adamski 2021-10-12 169 {
4a0f8262fe071b Krzysztof Adamski 2021-10-12 170 int err;
4a0f8262fe071b Krzysztof Adamski 2021-10-12 171 struct i2c_client *client = data->client;
4a0f8262fe071b Krzysztof Adamski 2021-10-12 172 struct device *dev = &client->dev;
4a0f8262fe071b Krzysztof Adamski 2021-10-12 173 int cfg = i2c_smbus_read_byte_data(client, TMP421_CONFIG_REG_2);
4a0f8262fe071b Krzysztof Adamski 2021-10-12 174 int i;
4a0f8262fe071b Krzysztof Adamski 2021-10-12 175
4a0f8262fe071b Krzysztof Adamski 2021-10-12 176 if (cfg < 0) {
4a0f8262fe071b Krzysztof Adamski 2021-10-12 177 dev_err(dev, "error reading register, can't disable channels\n");
4a0f8262fe071b Krzysztof Adamski 2021-10-12 @178 return err;
"return cfg;"
4a0f8262fe071b Krzysztof Adamski 2021-10-12 179 }
4a0f8262fe071b Krzysztof Adamski 2021-10-12 180
4a0f8262fe071b Krzysztof Adamski 2021-10-12 181 cfg &= ~TMP421_CONFIG_REG_REN_MASK;
4a0f8262fe071b Krzysztof Adamski 2021-10-12 182 for (i = 0; i < data->channels; i++)
4a0f8262fe071b Krzysztof Adamski 2021-10-12 183 if (data->channel[i].enabled)
4a0f8262fe071b Krzysztof Adamski 2021-10-12 184 cfg |= TMP421_CONFIG_REG_REN(i);
4a0f8262fe071b Krzysztof Adamski 2021-10-12 185
4a0f8262fe071b Krzysztof Adamski 2021-10-12 186 err = i2c_smbus_write_byte_data(client, TMP421_CONFIG_REG_2, cfg);
4a0f8262fe071b Krzysztof Adamski 2021-10-12 187 if (err < 0)
4a0f8262fe071b Krzysztof Adamski 2021-10-12 188 dev_err(dev, "error writing register, can't disable channels\n");
4a0f8262fe071b Krzysztof Adamski 2021-10-12 189
4a0f8262fe071b Krzysztof Adamski 2021-10-12 190 return err;
4a0f8262fe071b Krzysztof Adamski 2021-10-12 191 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v4 06/10] hwmon: (tmp421) really disable channels
2021-10-12 9:29 ` [PATCH v4 06/10] hwmon: (tmp421) really disable channels Krzysztof Adamski
` (2 preceding siblings ...)
2021-10-13 8:36 ` Dan Carpenter
@ 2021-10-13 18:50 ` kernel test robot
3 siblings, 0 replies; 22+ messages in thread
From: kernel test robot @ 2021-10-13 18:50 UTC (permalink / raw)
To: Krzysztof Adamski, Guenter Roeck, Jean Delvare
Cc: llvm, kbuild-all, Rob Herring, linux-hwmon, devicetree
[-- Attachment #1: Type: text/plain, Size: 2916 bytes --]
Hi Krzysztof,
I love your patch! Yet something to improve:
[auto build test ERROR on groeck-staging/hwmon-next]
[also build test ERROR on next-20211013]
[cannot apply to robh/for-next v5.15-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Krzysztof-Adamski/Add-per-channel-properies-support-in-tmp421/20211012-173142
base: https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
config: i386-buildonly-randconfig-r004-20211013 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project b6a8c695542b2987eb9a203d5663a0740cb4725f)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/4a0f8262fe071b0b27c6fba7455627f3c5a5209e
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Krzysztof-Adamski/Add-per-channel-properies-support-in-tmp421/20211012-173142
git checkout 4a0f8262fe071b0b27c6fba7455627f3c5a5209e
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/hwmon/tmp421.c:178:10: error: variable 'err' is uninitialized when used here [-Werror,-Wuninitialized]
return err;
^~~
drivers/hwmon/tmp421.c:170:9: note: initialize the variable 'err' to silence this warning
int err;
^
= 0
1 error generated.
vim +/err +178 drivers/hwmon/tmp421.c
167
168 static int tmp421_enable_channels(struct tmp421_data *data)
169 {
170 int err;
171 struct i2c_client *client = data->client;
172 struct device *dev = &client->dev;
173 int cfg = i2c_smbus_read_byte_data(client, TMP421_CONFIG_REG_2);
174 int i;
175
176 if (cfg < 0) {
177 dev_err(dev, "error reading register, can't disable channels\n");
> 178 return err;
179 }
180
181 cfg &= ~TMP421_CONFIG_REG_REN_MASK;
182 for (i = 0; i < data->channels; i++)
183 if (data->channel[i].enabled)
184 cfg |= TMP421_CONFIG_REG_REN(i);
185
186 err = i2c_smbus_write_byte_data(client, TMP421_CONFIG_REG_2, cfg);
187 if (err < 0)
188 dev_err(dev, "error writing register, can't disable channels\n");
189
190 return err;
191 }
192
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 40883 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4 07/10] hwmon: (tmp421) support HWMON_T_ENABLE
2021-10-12 9:14 [PATCH v4 00/10] Add per channel properies support in tmp421 Krzysztof Adamski
` (5 preceding siblings ...)
2021-10-12 9:29 ` [PATCH v4 06/10] hwmon: (tmp421) really disable channels Krzysztof Adamski
@ 2021-10-12 9:29 ` Krzysztof Adamski
2021-10-12 14:43 ` Guenter Roeck
2021-10-12 9:30 ` [PATCH v4 08/10] hwmon: (tmp421) update documentation Krzysztof Adamski
` (2 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Krzysztof Adamski @ 2021-10-12 9:29 UTC (permalink / raw)
To: Guenter Roeck, Jean Delvare; +Cc: Rob Herring, linux-hwmon, devicetree
Since the recent patches added possibility of disabling sensor channels
via DT, it only make sense to allow controlling that from userspace via
HWMON_T_ENABLE mechanism. This patches adds support for that.
Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>
---
drivers/hwmon/tmp421.c | 35 +++++++++++++++++++++++++++++++----
1 file changed, 31 insertions(+), 4 deletions(-)
diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
index 45cb197cd277..133eca1f2650 100644
--- a/drivers/hwmon/tmp421.c
+++ b/drivers/hwmon/tmp421.c
@@ -200,21 +200,27 @@ static int tmp421_read(struct device *dev, enum hwmon_sensor_types type,
if (ret)
return ret;
- if (!tmp421->channel[channel].enabled)
- return -ENODATA;
-
switch (attr) {
case hwmon_temp_input:
+ if (!tmp421->channel[channel].enabled)
+ return -ENODATA;
+
*val = temp_from_raw(tmp421->channel[channel].temp,
tmp421->config & TMP421_CONFIG_RANGE);
+
return 0;
case hwmon_temp_fault:
+ if (!tmp421->channel[channel].enabled)
+ return -ENODATA;
/*
* Any of OPEN or /PVLD bits indicate a hardware mulfunction
* and the conversion result may be incorrect
*/
*val = !!(tmp421->channel[channel].temp & 0x03);
return 0;
+ case hwmon_temp_enable:
+ *val = tmp421->channel[channel].enabled;
+ return 0;
default:
return -EOPNOTSUPP;
}
@@ -231,6 +237,24 @@ static int tmp421_read_string(struct device *dev, enum hwmon_sensor_types type,
return 0;
}
+static int tmp421_write(struct device *dev, enum hwmon_sensor_types type,
+ u32 attr, int channel, long val)
+{
+ struct tmp421_data *data = dev_get_drvdata(dev);
+ int ret;
+
+ switch (type) {
+ case hwmon_temp:
+ data->channel[channel].enabled = val;
+ ret = tmp421_enable_channels(data);
+ break;
+ default:
+ ret = -EOPNOTSUPP;
+ }
+
+ return ret;
+}
+
static umode_t tmp421_is_visible(const void *data, enum hwmon_sensor_types type,
u32 attr, int channel)
{
@@ -240,6 +264,8 @@ static umode_t tmp421_is_visible(const void *data, enum hwmon_sensor_types type,
return 0444;
case hwmon_temp_label:
return 0444;
+ case hwmon_temp_enable:
+ return 0644;
default:
return 0;
}
@@ -397,6 +423,7 @@ static const struct hwmon_ops tmp421_ops = {
.is_visible = tmp421_is_visible,
.read = tmp421_read,
.read_string = tmp421_read_string,
+ .write = tmp421_write,
};
static int tmp421_probe(struct i2c_client *client)
@@ -419,7 +446,7 @@ static int tmp421_probe(struct i2c_client *client)
data->client = client;
for (i = 0; i < data->channels; i++) {
- data->temp_config[i] = HWMON_T_INPUT | HWMON_T_FAULT;
+ data->temp_config[i] = HWMON_T_INPUT | HWMON_T_FAULT | HWMON_T_ENABLE;
data->channel[i].enabled = true;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v4 07/10] hwmon: (tmp421) support HWMON_T_ENABLE
2021-10-12 9:29 ` [PATCH v4 07/10] hwmon: (tmp421) support HWMON_T_ENABLE Krzysztof Adamski
@ 2021-10-12 14:43 ` Guenter Roeck
0 siblings, 0 replies; 22+ messages in thread
From: Guenter Roeck @ 2021-10-12 14:43 UTC (permalink / raw)
To: Krzysztof Adamski, Jean Delvare; +Cc: Rob Herring, linux-hwmon, devicetree
On 10/12/21 2:29 AM, Krzysztof Adamski wrote:
> Since the recent patches added possibility of disabling sensor channels
> via DT, it only make sense to allow controlling that from userspace via
> HWMON_T_ENABLE mechanism. This patches adds support for that.
>
> Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>
> ---
> drivers/hwmon/tmp421.c | 35 +++++++++++++++++++++++++++++++----
> 1 file changed, 31 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> index 45cb197cd277..133eca1f2650 100644
> --- a/drivers/hwmon/tmp421.c
> +++ b/drivers/hwmon/tmp421.c
> @@ -200,21 +200,27 @@ static int tmp421_read(struct device *dev, enum hwmon_sensor_types type,
> if (ret)
> return ret;
>
> - if (!tmp421->channel[channel].enabled)
> - return -ENODATA;
> -
> switch (attr) {
> case hwmon_temp_input:
> + if (!tmp421->channel[channel].enabled)
> + return -ENODATA;
> +
> *val = temp_from_raw(tmp421->channel[channel].temp,
> tmp421->config & TMP421_CONFIG_RANGE);
> +
Please drop those empty lines for consistency with the code below.
> return 0;
> case hwmon_temp_fault:
> + if (!tmp421->channel[channel].enabled)
> + return -ENODATA;
> /*
> * Any of OPEN or /PVLD bits indicate a hardware mulfunction
> * and the conversion result may be incorrect
> */
> *val = !!(tmp421->channel[channel].temp & 0x03);
> return 0;
> + case hwmon_temp_enable:
> + *val = tmp421->channel[channel].enabled;
> + return 0;
> default:
> return -EOPNOTSUPP;
> }
> @@ -231,6 +237,24 @@ static int tmp421_read_string(struct device *dev, enum hwmon_sensor_types type,
> return 0;
> }
>
> +static int tmp421_write(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, long val)
> +{
> + struct tmp421_data *data = dev_get_drvdata(dev);
> + int ret;
> +
> + switch (type) {
> + case hwmon_temp:
Please check for attr here. Checking the type (which is always
hwmon_temp) is misleading.
> + data->channel[channel].enabled = val;
> + ret = tmp421_enable_channels(data);
> + break;
> + default:
> + ret = -EOPNOTSUPP;
> + }
> +
> + return ret;
> +}
> +
> static umode_t tmp421_is_visible(const void *data, enum hwmon_sensor_types type,
> u32 attr, int channel)
> {
> @@ -240,6 +264,8 @@ static umode_t tmp421_is_visible(const void *data, enum hwmon_sensor_types type,
> return 0444;
> case hwmon_temp_label:
> return 0444;
> + case hwmon_temp_enable:
> + return 0644;
> default:
> return 0;
> }
> @@ -397,6 +423,7 @@ static const struct hwmon_ops tmp421_ops = {
> .is_visible = tmp421_is_visible,
> .read = tmp421_read,
> .read_string = tmp421_read_string,
> + .write = tmp421_write,
> };
>
> static int tmp421_probe(struct i2c_client *client)
> @@ -419,7 +446,7 @@ static int tmp421_probe(struct i2c_client *client)
> data->client = client;
>
> for (i = 0; i < data->channels; i++) {
> - data->temp_config[i] = HWMON_T_INPUT | HWMON_T_FAULT;
> + data->temp_config[i] = HWMON_T_INPUT | HWMON_T_FAULT | HWMON_T_ENABLE;
> data->channel[i].enabled = true;
> }
>
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4 08/10] hwmon: (tmp421) update documentation
2021-10-12 9:14 [PATCH v4 00/10] Add per channel properies support in tmp421 Krzysztof Adamski
` (6 preceding siblings ...)
2021-10-12 9:29 ` [PATCH v4 07/10] hwmon: (tmp421) support HWMON_T_ENABLE Krzysztof Adamski
@ 2021-10-12 9:30 ` Krzysztof Adamski
2021-10-12 14:43 ` Guenter Roeck
2021-10-12 9:30 ` [PATCH v4 09/10] hwmon: (tmp421) ignore non-channel related DT nodes Krzysztof Adamski
2021-10-12 9:30 ` [PATCH v4 10/10] dt-bindings: hwmon: allow specifying channels for tmp421 Krzysztof Adamski
9 siblings, 1 reply; 22+ messages in thread
From: Krzysztof Adamski @ 2021-10-12 9:30 UTC (permalink / raw)
To: Guenter Roeck, Jean Delvare; +Cc: Rob Herring, linux-hwmon, devicetree
Sysfs interface of the tmp421 driver was extended in the recent patches
so lets update the documentation to reflect that.
Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>
---
Documentation/hwmon/tmp421.rst | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/Documentation/hwmon/tmp421.rst b/Documentation/hwmon/tmp421.rst
index ddcd5159c75d..a3002117bbd7 100644
--- a/Documentation/hwmon/tmp421.rst
+++ b/Documentation/hwmon/tmp421.rst
@@ -64,3 +64,13 @@ the temperature values via the following sysfs files:
**temp[1-4]_input**
**temp[2-4]_fault**
+
+Each sensor can be individually disabled via Devicetree or from sysfs
+via:
+
+**temp[1-4]_enable**
+
+If labels were specified in Devicetree, additional sysfs files will
+be present:
+
+**temp[1-4]_label**
--
2.31.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v4 08/10] hwmon: (tmp421) update documentation
2021-10-12 9:30 ` [PATCH v4 08/10] hwmon: (tmp421) update documentation Krzysztof Adamski
@ 2021-10-12 14:43 ` Guenter Roeck
0 siblings, 0 replies; 22+ messages in thread
From: Guenter Roeck @ 2021-10-12 14:43 UTC (permalink / raw)
To: Krzysztof Adamski, Jean Delvare; +Cc: Rob Herring, linux-hwmon, devicetree
On 10/12/21 2:30 AM, Krzysztof Adamski wrote:
> Sysfs interface of the tmp421 driver was extended in the recent patches
> so lets update the documentation to reflect that.
>
> Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>
For my reference:
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> ---
> Documentation/hwmon/tmp421.rst | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/Documentation/hwmon/tmp421.rst b/Documentation/hwmon/tmp421.rst
> index ddcd5159c75d..a3002117bbd7 100644
> --- a/Documentation/hwmon/tmp421.rst
> +++ b/Documentation/hwmon/tmp421.rst
> @@ -64,3 +64,13 @@ the temperature values via the following sysfs files:
> **temp[1-4]_input**
>
> **temp[2-4]_fault**
> +
> +Each sensor can be individually disabled via Devicetree or from sysfs
> +via:
> +
> +**temp[1-4]_enable**
> +
> +If labels were specified in Devicetree, additional sysfs files will
> +be present:
> +
> +**temp[1-4]_label**
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4 09/10] hwmon: (tmp421) ignore non-channel related DT nodes
2021-10-12 9:14 [PATCH v4 00/10] Add per channel properies support in tmp421 Krzysztof Adamski
` (7 preceding siblings ...)
2021-10-12 9:30 ` [PATCH v4 08/10] hwmon: (tmp421) update documentation Krzysztof Adamski
@ 2021-10-12 9:30 ` Krzysztof Adamski
2021-10-12 14:44 ` Guenter Roeck
2021-10-12 9:30 ` [PATCH v4 10/10] dt-bindings: hwmon: allow specifying channels for tmp421 Krzysztof Adamski
9 siblings, 1 reply; 22+ messages in thread
From: Krzysztof Adamski @ 2021-10-12 9:30 UTC (permalink / raw)
To: Guenter Roeck, Jean Delvare; +Cc: Rob Herring, linux-hwmon, devicetree
In case the DT contains some nodes not describing the input channels,
ignore them instead of exiting with error.
Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>
---
drivers/hwmon/tmp421.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
index 133eca1f2650..d44112fe2a14 100644
--- a/drivers/hwmon/tmp421.c
+++ b/drivers/hwmon/tmp421.c
@@ -411,6 +411,9 @@ static int tmp421_probe_from_dt(struct i2c_client *client, struct tmp421_data *d
int err;
for_each_child_of_node(np, child) {
+ if (strcmp(child->name, "channel"))
+ continue;
+
err = tmp421_probe_child_from_dt(client, child, data);
if (err)
return err;
--
2.31.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v4 09/10] hwmon: (tmp421) ignore non-channel related DT nodes
2021-10-12 9:30 ` [PATCH v4 09/10] hwmon: (tmp421) ignore non-channel related DT nodes Krzysztof Adamski
@ 2021-10-12 14:44 ` Guenter Roeck
0 siblings, 0 replies; 22+ messages in thread
From: Guenter Roeck @ 2021-10-12 14:44 UTC (permalink / raw)
To: Krzysztof Adamski, Jean Delvare; +Cc: Rob Herring, linux-hwmon, devicetree
On 10/12/21 2:30 AM, Krzysztof Adamski wrote:
> In case the DT contains some nodes not describing the input channels,
> ignore them instead of exiting with error.
>
> Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>
For my reference:
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/hwmon/tmp421.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
> index 133eca1f2650..d44112fe2a14 100644
> --- a/drivers/hwmon/tmp421.c
> +++ b/drivers/hwmon/tmp421.c
> @@ -411,6 +411,9 @@ static int tmp421_probe_from_dt(struct i2c_client *client, struct tmp421_data *d
> int err;
>
> for_each_child_of_node(np, child) {
> + if (strcmp(child->name, "channel"))
> + continue;
> +
> err = tmp421_probe_child_from_dt(client, child, data);
> if (err)
> return err;
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4 10/10] dt-bindings: hwmon: allow specifying channels for tmp421
2021-10-12 9:14 [PATCH v4 00/10] Add per channel properies support in tmp421 Krzysztof Adamski
` (8 preceding siblings ...)
2021-10-12 9:30 ` [PATCH v4 09/10] hwmon: (tmp421) ignore non-channel related DT nodes Krzysztof Adamski
@ 2021-10-12 9:30 ` Krzysztof Adamski
9 siblings, 0 replies; 22+ messages in thread
From: Krzysztof Adamski @ 2021-10-12 9:30 UTC (permalink / raw)
To: Guenter Roeck, Jean Delvare; +Cc: Rob Herring, linux-hwmon, devicetree
Add binding description for the per temperature channel configuration
like labels and n-factor.
Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>
---
.../devicetree/bindings/hwmon/ti,tmp421.yaml | 67 +++++++++++++++++++
1 file changed, 67 insertions(+)
diff --git a/Documentation/devicetree/bindings/hwmon/ti,tmp421.yaml b/Documentation/devicetree/bindings/hwmon/ti,tmp421.yaml
index 47040ace4f73..36f649938fb7 100644
--- a/Documentation/devicetree/bindings/hwmon/ti,tmp421.yaml
+++ b/Documentation/devicetree/bindings/hwmon/ti,tmp421.yaml
@@ -24,12 +24,50 @@ properties:
reg:
maxItems: 1
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
required:
- compatible
- reg
additionalProperties: false
+patternProperties:
+ "^channel@([0-3])$":
+ type: object
+ description: |
+ Represents channels of the device and their specific configuration.
+
+ properties:
+ reg:
+ description: |
+ The channel number. 0 is local channel, 1-3 are remote channels
+ items:
+ minimum: 0
+ maximum: 3
+
+ label:
+ description: |
+ A descriptive name for this channel, like "ambient" or "psu".
+
+ ti,n-factor:
+ description: |
+ The value (two's complement) to be programmed in the channel specific N correction register.
+ For remote channels only.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ items:
+ minimum: 0
+ maximum: 255
+
+ required:
+ - reg
+
+ additionalProperties: false
+
examples:
- |
i2c {
@@ -41,3 +79,32 @@ examples:
reg = <0x4c>;
};
};
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ sensor@4c {
+ compatible = "ti,tmp422";
+ reg = <0x4c>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ channel@0 {
+ reg = <0x0>;
+ ti,n-factor = <0x1>;
+ label = "local";
+ };
+
+ channel@1 {
+ reg = <0x1>;
+ ti,n-factor = <0x0>;
+ label = "somelabel";
+ };
+
+ channel@2 {
+ reg = <0x2>;
+ status = "disabled";
+ };
+ };
+ };
--
2.31.1
^ permalink raw reply related [flat|nested] 22+ messages in thread