dt-bindings documentation for this driver claims to support
thermocouple-type, but the driver does not actually make use of
the property.
Implement usage of the property to configure the chip for the
selected thermocouple-type.
Signed-off-by: Ben Collins <bcollins@watter.com>
---
drivers/iio/temperature/mcp9600.c | 69 +++++++++++++++++++++++++++++++
1 file changed, 69 insertions(+)
diff --git a/drivers/iio/temperature/mcp9600.c b/drivers/iio/temperature/mcp9600.c
index c18ae17b6d82f..361572a241f06 100644
--- a/drivers/iio/temperature/mcp9600.c
+++ b/drivers/iio/temperature/mcp9600.c
@@ -22,11 +22,15 @@
#include <linux/iio/events.h>
#include <linux/iio/iio.h>
+#include <dt-bindings/iio/temperature/thermocouple.h>
+
/* MCP9600 registers */
#define MCP9600_HOT_JUNCTION 0x00
#define MCP9600_COLD_JUNCTION 0x02
#define MCP9600_STATUS 0x04
#define MCP9600_STATUS_ALERT(x) BIT(x)
+#define MCP9600_SENSOR_CFG 0x05
+#define MCP9600_SENSOR_TYPE_MASK GENMASK(6, 4)
#define MCP9600_ALERT_CFG1 0x08
#define MCP9600_ALERT_CFG(x) (MCP9600_ALERT_CFG1 + (x - 1))
#define MCP9600_ALERT_CFG_ENABLE BIT(0)
@@ -66,6 +70,30 @@ static const char * const mcp9600_alert_name[MCP9600_ALERT_COUNT] = {
[MCP9600_ALERT4] = "alert4",
};
+/* Map between dt-bindings enum and the chip's type value */
+static const unsigned int mcp9600_type_map[] = {
+ [THERMOCOUPLE_TYPE_K] = 0,
+ [THERMOCOUPLE_TYPE_J] = 1,
+ [THERMOCOUPLE_TYPE_T] = 2,
+ [THERMOCOUPLE_TYPE_N] = 3,
+ [THERMOCOUPLE_TYPE_S] = 4,
+ [THERMOCOUPLE_TYPE_E] = 5,
+ [THERMOCOUPLE_TYPE_B] = 6,
+ [THERMOCOUPLE_TYPE_R] = 7,
+};
+
+/* Map thermocouple type to a char for for iio info in sysfs */
+static const int mcp9600_tc_types[] = {
+ [THERMOCOUPLE_TYPE_K] = 'K',
+ [THERMOCOUPLE_TYPE_J] = 'J',
+ [THERMOCOUPLE_TYPE_T] = 'T',
+ [THERMOCOUPLE_TYPE_N] = 'N',
+ [THERMOCOUPLE_TYPE_S] = 'S',
+ [THERMOCOUPLE_TYPE_E] = 'E',
+ [THERMOCOUPLE_TYPE_B] = 'B',
+ [THERMOCOUPLE_TYPE_R] = 'R',
+};
+
static const struct iio_event_spec mcp9600_events[] = {
{
.type = IIO_EV_TYPE_THRESH,
@@ -89,6 +117,7 @@ static const struct iio_event_spec mcp9600_events[] = {
.type = IIO_TEMP, \
.address = MCP9600_HOT_JUNCTION, \
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+ BIT(IIO_CHAN_INFO_THERMOCOUPLE_TYPE) | \
BIT(IIO_CHAN_INFO_SCALE), \
.event_spec = &mcp9600_events[hj_ev_spec_off], \
.num_event_specs = hj_num_ev, \
@@ -131,6 +160,7 @@ struct mcp_chip_info {
struct mcp9600_data {
struct i2c_client *client;
+ u32 thermocouple_type;
};
static int mcp9600_read(struct mcp9600_data *data,
@@ -165,11 +195,32 @@ static int mcp9600_read_raw(struct iio_dev *indio_dev,
*val = 62;
*val2 = 500000;
return IIO_VAL_INT_PLUS_MICRO;
+ case IIO_CHAN_INFO_THERMOCOUPLE_TYPE:
+ *val = mcp9600_tc_types[data->thermocouple_type];
+ return IIO_VAL_CHAR;
default:
return -EINVAL;
}
}
+static int mcp9600_config(struct mcp9600_data *data)
+{
+ struct i2c_client *client = data->client;
+ int ret;
+ u8 cfg;
+
+ cfg = FIELD_PREP(MCP9600_SENSOR_TYPE_MASK,
+ mcp9600_type_map[data->thermocouple_type]);
+
+ ret = i2c_smbus_write_byte_data(client, MCP9600_SENSOR_CFG, cfg);
+ if (ret < 0) {
+ dev_err(&client->dev, "Failed to set sensor configuration\n");
+ return ret;
+ }
+
+ return 0;
+}
+
static int mcp9600_get_alert_index(int channel2, enum iio_event_direction dir)
{
if (channel2 == IIO_MOD_TEMP_AMBIENT) {
@@ -453,6 +504,24 @@ static int mcp9600_probe(struct i2c_client *client)
data = iio_priv(indio_dev);
data->client = client;
+ /* Accept type from dt with default of Type-K. */
+ data->thermocouple_type = THERMOCOUPLE_TYPE_K;
+ ret = device_property_read_u32(&client->dev, "thermocouple-type",
+ &data->thermocouple_type);
+ if (ret < 0 && ret != -EINVAL)
+ return dev_err_probe(&client->dev, ret,
+ "Error reading thermocouple-type property\n");
+
+ if (data->thermocouple_type >= ARRAY_SIZE(mcp9600_type_map))
+ return dev_err_probe(&client->dev, -EINVAL,
+ "Invalid thermocouple-type property %u.\n",
+ data->thermocouple_type);
+
+ /* Set initial config. */
+ ret = mcp9600_config(data);
+ if (ret < 0)
+ return ret;
+
ch_sel = mcp9600_probe_alerts(indio_dev);
if (ch_sel < 0)
return ch_sel;
--
2.50.1
On 8/15/25 11:46 AM, Ben Collins wrote: > dt-bindings documentation for this driver claims to support > thermocouple-type, but the driver does not actually make use of > the property. > > Implement usage of the property to configure the chip for the > selected thermocouple-type. > > Signed-off-by: Ben Collins <bcollins@watter.com> > --- ... > @@ -453,6 +504,24 @@ static int mcp9600_probe(struct i2c_client *client) > data = iio_priv(indio_dev); > data->client = client; > > + /* Accept type from dt with default of Type-K. */ We still also need a dt-bindings patch to specify the default there as well. > + data->thermocouple_type = THERMOCOUPLE_TYPE_K; > + ret = device_property_read_u32(&client->dev, "thermocouple-type", > + &data->thermocouple_type); > + if (ret < 0 && ret != -EINVAL) > + return dev_err_probe(&client->dev, ret, > + "Error reading thermocouple-type property\n"); > + > + if (data->thermocouple_type >= ARRAY_SIZE(mcp9600_type_map)) > + return dev_err_probe(&client->dev, -EINVAL, > + "Invalid thermocouple-type property %u.\n", > + data->thermocouple_type); > + > + /* Set initial config. */ > + ret = mcp9600_config(data); > + if (ret < 0) > + return ret; > + > ch_sel = mcp9600_probe_alerts(indio_dev); > if (ch_sel < 0) > return ch_sel;
On Sat, Aug 16, 2025 at 01:24:24PM -0500, David Lechner wrote: > On 8/15/25 11:46 AM, Ben Collins wrote: > > dt-bindings documentation for this driver claims to support > > thermocouple-type, but the driver does not actually make use of > > the property. > > > > Implement usage of the property to configure the chip for the > > selected thermocouple-type. > > > > Signed-off-by: Ben Collins <bcollins@watter.com> > > --- > > ... > > > @@ -453,6 +504,24 @@ static int mcp9600_probe(struct i2c_client *client) > > data = iio_priv(indio_dev); > > data->client = client; > > > > + /* Accept type from dt with default of Type-K. */ > > We still also need a dt-bindings patch to specify the default there as well. The existing bindings file for this already states type-k is the default. Is there something else it needs? > > + data->thermocouple_type = THERMOCOUPLE_TYPE_K; > > + ret = device_property_read_u32(&client->dev, "thermocouple-type", > > + &data->thermocouple_type); > > + if (ret < 0 && ret != -EINVAL) > > + return dev_err_probe(&client->dev, ret, > > + "Error reading thermocouple-type property\n"); > > + > > + if (data->thermocouple_type >= ARRAY_SIZE(mcp9600_type_map)) > > + return dev_err_probe(&client->dev, -EINVAL, > > + "Invalid thermocouple-type property %u.\n", > > + data->thermocouple_type); > > + > > + /* Set initial config. */ > > + ret = mcp9600_config(data); > > + if (ret < 0) > > + return ret; > > + > > ch_sel = mcp9600_probe_alerts(indio_dev); > > if (ch_sel < 0) > > return ch_sel; > > -- Ben Collins https://libjwt.io https://github.com/benmcollins -- 3EC9 7598 1672 961A 1139 173A 5D5A 57C7 242B 22CF
On 8/16/25 9:54 PM, Ben Collins wrote: > On Sat, Aug 16, 2025 at 01:24:24PM -0500, David Lechner wrote: >> On 8/15/25 11:46 AM, Ben Collins wrote: >>> dt-bindings documentation for this driver claims to support >>> thermocouple-type, but the driver does not actually make use of >>> the property. >>> >>> Implement usage of the property to configure the chip for the >>> selected thermocouple-type. >>> >>> Signed-off-by: Ben Collins <bcollins@watter.com> >>> --- >> >> ... >> >>> @@ -453,6 +504,24 @@ static int mcp9600_probe(struct i2c_client *client) >>> data = iio_priv(indio_dev); >>> data->client = client; >>> >>> + /* Accept type from dt with default of Type-K. */ >> >> We still also need a dt-bindings patch to specify the default there as well. > > The existing bindings file for this already states type-k is the > default. Is there something else it needs? > default: 3 in the YAML.
On Fri, 15 Aug 2025 16:46:06 +0000 Ben Collins <bcollins@watter.com> wrote: > dt-bindings documentation for this driver claims to support > thermocouple-type, but the driver does not actually make use of > the property. > > Implement usage of the property to configure the chip for the > selected thermocouple-type. > > Signed-off-by: Ben Collins <bcollins@watter.com> Hi Ben, Just one trivial thing inline. Thanks Jonathan > --- > drivers/iio/temperature/mcp9600.c | 69 +++++++++++++++++++++++++++++++ > 1 file changed, 69 insertions(+) > > diff --git a/drivers/iio/temperature/mcp9600.c b/drivers/iio/temperature/mcp9600.c > index c18ae17b6d82f..361572a241f06 100644 > --- a/drivers/iio/temperature/mcp9600.c > +++ b/drivers/iio/temperature/mcp9600.c > @@ -22,11 +22,15 @@ > #include <linux/iio/events.h> > #include <linux/iio/iio.h> > > +#include <dt-bindings/iio/temperature/thermocouple.h> > + > /* MCP9600 registers */ > #define MCP9600_HOT_JUNCTION 0x00 > #define MCP9600_COLD_JUNCTION 0x02 > #define MCP9600_STATUS 0x04 > #define MCP9600_STATUS_ALERT(x) BIT(x) > +#define MCP9600_SENSOR_CFG 0x05 > +#define MCP9600_SENSOR_TYPE_MASK GENMASK(6, 4) > #define MCP9600_ALERT_CFG1 0x08 > #define MCP9600_ALERT_CFG(x) (MCP9600_ALERT_CFG1 + (x - 1)) > #define MCP9600_ALERT_CFG_ENABLE BIT(0) > @@ -66,6 +70,30 @@ static const char * const mcp9600_alert_name[MCP9600_ALERT_COUNT] = { > [MCP9600_ALERT4] = "alert4", > }; > > +/* Map between dt-bindings enum and the chip's type value */ > +static const unsigned int mcp9600_type_map[] = { > + [THERMOCOUPLE_TYPE_K] = 0, > + [THERMOCOUPLE_TYPE_J] = 1, > + [THERMOCOUPLE_TYPE_T] = 2, > + [THERMOCOUPLE_TYPE_N] = 3, > + [THERMOCOUPLE_TYPE_S] = 4, > + [THERMOCOUPLE_TYPE_E] = 5, > + [THERMOCOUPLE_TYPE_B] = 6, > + [THERMOCOUPLE_TYPE_R] = 7, > +}; > + > +/* Map thermocouple type to a char for for iio info in sysfs */ > +static const int mcp9600_tc_types[] = { > + [THERMOCOUPLE_TYPE_K] = 'K', > + [THERMOCOUPLE_TYPE_J] = 'J', > + [THERMOCOUPLE_TYPE_T] = 'T', > + [THERMOCOUPLE_TYPE_N] = 'N', > + [THERMOCOUPLE_TYPE_S] = 'S', > + [THERMOCOUPLE_TYPE_E] = 'E', > + [THERMOCOUPLE_TYPE_B] = 'B', > + [THERMOCOUPLE_TYPE_R] = 'R', > +}; > + > static const struct iio_event_spec mcp9600_events[] = { > { > .type = IIO_EV_TYPE_THRESH, > @@ -89,6 +117,7 @@ static const struct iio_event_spec mcp9600_events[] = { > .type = IIO_TEMP, \ > .address = MCP9600_HOT_JUNCTION, \ > .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ > + BIT(IIO_CHAN_INFO_THERMOCOUPLE_TYPE) | \ > BIT(IIO_CHAN_INFO_SCALE), \ > .event_spec = &mcp9600_events[hj_ev_spec_off], \ > .num_event_specs = hj_num_ev, \ > @@ -131,6 +160,7 @@ struct mcp_chip_info { > > struct mcp9600_data { > struct i2c_client *client; > + u32 thermocouple_type; > }; > > static int mcp9600_read(struct mcp9600_data *data, > @@ -165,11 +195,32 @@ static int mcp9600_read_raw(struct iio_dev *indio_dev, > *val = 62; > *val2 = 500000; > return IIO_VAL_INT_PLUS_MICRO; > + case IIO_CHAN_INFO_THERMOCOUPLE_TYPE: > + *val = mcp9600_tc_types[data->thermocouple_type]; > + return IIO_VAL_CHAR; > default: > return -EINVAL; > } > } > > +static int mcp9600_config(struct mcp9600_data *data) > +{ > + struct i2c_client *client = data->client; > + int ret; > + u8 cfg; > + > + cfg = FIELD_PREP(MCP9600_SENSOR_TYPE_MASK, > + mcp9600_type_map[data->thermocouple_type]); > + > + ret = i2c_smbus_write_byte_data(client, MCP9600_SENSOR_CFG, cfg); > + if (ret < 0) { > + dev_err(&client->dev, "Failed to set sensor configuration\n"); Only called from probe so use return dev_err_probe() here > + return ret; > + } > + > + return 0; > +} > + > static int mcp9600_get_alert_index(int channel2, enum iio_event_direction dir) > { > if (channel2 == IIO_MOD_TEMP_AMBIENT) { > @@ -453,6 +504,24 @@ static int mcp9600_probe(struct i2c_client *client) > data = iio_priv(indio_dev); > data->client = client; > > + /* Accept type from dt with default of Type-K. */ > + data->thermocouple_type = THERMOCOUPLE_TYPE_K; > + ret = device_property_read_u32(&client->dev, "thermocouple-type", > + &data->thermocouple_type); > + if (ret < 0 && ret != -EINVAL) > + return dev_err_probe(&client->dev, ret, > + "Error reading thermocouple-type property\n"); > + > + if (data->thermocouple_type >= ARRAY_SIZE(mcp9600_type_map)) > + return dev_err_probe(&client->dev, -EINVAL, > + "Invalid thermocouple-type property %u.\n", > + data->thermocouple_type); > + > + /* Set initial config. */ > + ret = mcp9600_config(data); > + if (ret < 0) > + return ret; > + > ch_sel = mcp9600_probe_alerts(indio_dev); > if (ch_sel < 0) > return ch_sel;
> On Aug 16, 2025, at 6:11 AM, Jonathan Cameron <jic23@kernel.org> wrote: > > On Fri, 15 Aug 2025 16:46:06 +0000 > Ben Collins <bcollins@watter.com> wrote: > >> dt-bindings documentation for this driver claims to support >> thermocouple-type, but the driver does not actually make use of >> the property. >> >> Implement usage of the property to configure the chip for the >> selected thermocouple-type. >> >> Signed-off-by: Ben Collins <bcollins@watter.com> > Hi Ben, > > Just one trivial thing inline. ... >> >> +static int mcp9600_config(struct mcp9600_data *data) >> +{ >> + struct i2c_client *client = data->client; >> + int ret; >> + u8 cfg; >> + >> + cfg = FIELD_PREP(MCP9600_SENSOR_TYPE_MASK, >> + mcp9600_type_map[data->thermocouple_type]); >> + >> + ret = i2c_smbus_write_byte_data(client, MCP9600_SENSOR_CFG, cfg); >> + if (ret < 0) { >> + dev_err(&client->dev, "Failed to set sensor configuration\n"); > > Only called from probe so use return dev_err_probe() here Hi Johnathan. That’s correct in this patch. However, in the IIR patch I call this when the filter is set, so it didn’t seem to make sense to do it that way, just to change it in the next patch. I appreciate all the feedback. I’ll get things cleaned in for v4.
On Sat, 16 Aug 2025 09:18:09 -0400 Ben Collins <bcollins@watter.com> wrote: > > On Aug 16, 2025, at 6:11 AM, Jonathan Cameron <jic23@kernel.org> wrote: > > > > On Fri, 15 Aug 2025 16:46:06 +0000 > > Ben Collins <bcollins@watter.com> wrote: > > > >> dt-bindings documentation for this driver claims to support > >> thermocouple-type, but the driver does not actually make use of > >> the property. > >> > >> Implement usage of the property to configure the chip for the > >> selected thermocouple-type. > >> > >> Signed-off-by: Ben Collins <bcollins@watter.com> > > Hi Ben, > > > > Just one trivial thing inline. > ... > >> > >> +static int mcp9600_config(struct mcp9600_data *data) > >> +{ > >> + struct i2c_client *client = data->client; > >> + int ret; > >> + u8 cfg; > >> + > >> + cfg = FIELD_PREP(MCP9600_SENSOR_TYPE_MASK, > >> + mcp9600_type_map[data->thermocouple_type]); > >> + > >> + ret = i2c_smbus_write_byte_data(client, MCP9600_SENSOR_CFG, cfg); > >> + if (ret < 0) { > >> + dev_err(&client->dev, "Failed to set sensor configuration\n"); > > > > Only called from probe so use return dev_err_probe() here > > Hi Johnathan. That’s correct in this patch. However, in the IIR patch > I call this when the filter is set, so it didn’t seem to make sense to > do it that way, just to change it in the next patch. > > I appreciate all the feedback. I’ll get things cleaned in for v4. Ah. Fair enough then. Leave it as it is and thanks for pointing this out. Jonathan
© 2016 - 2025 Red Hat, Inc.