[PATCH 4/5] iio: mcp9600: Add support for thermocouple-type

Ben Collins posted 5 patches 1 month, 2 weeks ago
There is a newer version of this series
[PATCH 4/5] iio: mcp9600: Add support for thermocouple-type
Posted by Ben Collins 1 month, 2 weeks ago
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
Re: [PATCH 4/5] iio: mcp9600: Add support for thermocouple-type
Posted by David Lechner 1 month, 2 weeks ago
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;
Re: [PATCH 4/5] iio: mcp9600: Add support for thermocouple-type
Posted by Ben Collins 1 month, 2 weeks ago
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
Re: [PATCH 4/5] iio: mcp9600: Add support for thermocouple-type
Posted by David Lechner 1 month, 2 weeks ago
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.
Re: [PATCH 4/5] iio: mcp9600: Add support for thermocouple-type
Posted by Jonathan Cameron 1 month, 2 weeks ago
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;
Re: [PATCH 4/5] iio: mcp9600: Add support for thermocouple-type
Posted by Ben Collins 1 month, 2 weeks ago

> 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.
Re: [PATCH 4/5] iio: mcp9600: Add support for thermocouple-type
Posted by Jonathan Cameron 1 month, 2 weeks ago
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