The current driver works with mcp9601, but emits a warning because it
does not recognize the chip id.
MCP9601 is a superset of MCP9600. The drivers works without changes
on this chipset.
However, the 9601 chip supports open/closed-circuit detection if wired
properly, so we'll need to be able to differentiate between them.
Signed-off-by: Ben Collins <bcollins@watter.com>
---
drivers/iio/temperature/Kconfig | 8 +++--
drivers/iio/temperature/mcp9600.c | 55 +++++++++++++++++++++++++------
2 files changed, 50 insertions(+), 13 deletions(-)
diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
index 1244d8e17d504..9328b2250aced 100644
--- a/drivers/iio/temperature/Kconfig
+++ b/drivers/iio/temperature/Kconfig
@@ -173,11 +173,13 @@ config MAX31865
will be called max31865.
config MCP9600
- tristate "MCP9600 thermocouple EMF converter"
+ tristate "MCP9600 and similar thermocouple EMF converters"
depends on I2C
help
- If you say yes here you get support for MCP9600
- thermocouple EMF converter connected via I2C.
+ If you say yes here you get support for...
+ - MCP9600
+ - MCP9601
+ ...thermocouple EMF converters connected via I2C.
This driver can also be built as a module. If so, the module
will be called mcp9600.
diff --git a/drivers/iio/temperature/mcp9600.c b/drivers/iio/temperature/mcp9600.c
index 40906bb200ec9..54de38a39292e 100644
--- a/drivers/iio/temperature/mcp9600.c
+++ b/drivers/iio/temperature/mcp9600.c
@@ -42,6 +42,7 @@
/* MCP9600 device id value */
#define MCP9600_DEVICE_ID_MCP9600 0x40
+#define MCP9600_DEVICE_ID_MCP9601 0x41
#define MCP9600_ALERT_COUNT 4
@@ -123,6 +124,11 @@ static const struct iio_chan_spec mcp9600_channels[][2] = {
MCP9600_CHANNELS(2, 0, 2, 0), /* Alerts: 1 2 3 4 */
};
+struct mcp_chip_info {
+ u8 chip_id;
+ const char *chip_name;
+};
+
struct mcp9600_data {
struct i2c_client *client;
};
@@ -416,16 +422,33 @@ static int mcp9600_probe_alerts(struct iio_dev *indio_dev)
static int mcp9600_probe(struct i2c_client *client)
{
+ const struct mcp_chip_info *chip_info = i2c_get_match_data(client);
struct iio_dev *indio_dev;
struct mcp9600_data *data;
- int ret, ch_sel;
+ int ch_sel, dev_id, ret;
+
+ if (chip_info == NULL)
+ return dev_err_probe(&client->dev, -EINVAL,
+ "No chip-info found for device\n");
+
+ dev_id = i2c_smbus_read_byte_data(client, MCP9600_DEVICE_ID);
+ if (dev_id < 0)
+ return dev_err_probe(&client->dev, dev_id,
+ "Failed to read device ID\n");
+
+ switch (dev_id) {
+ case MCP9600_DEVICE_ID_MCP9600:
+ case MCP9600_DEVICE_ID_MCP9601:
+ if (dev_id != chip_info->chip_id)
+ dev_warn(&client->dev,
+ "Expected id %02x, but device responded with %02x\n",
+ chip_info->chip_id, dev_id);
+ break;
- ret = i2c_smbus_read_byte_data(client, MCP9600_DEVICE_ID);
- if (ret < 0)
- return dev_err_probe(&client->dev, ret, "Failed to read device ID\n");
- if (ret != MCP9600_DEVICE_ID_MCP9600)
- dev_warn(&client->dev, "Expected ID %x, got %x\n",
- MCP9600_DEVICE_ID_MCP9600, ret);
+ default:
+ dev_warn(&client->dev, "Unknown id %x, using %x\n", dev_id,
+ chip_info->chip_id);
+ }
indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
if (!indio_dev)
@@ -439,7 +462,7 @@ static int mcp9600_probe(struct i2c_client *client)
return ch_sel;
indio_dev->info = &mcp9600_info;
- indio_dev->name = "mcp9600";
+ indio_dev->name = chip_info->chip_name;
indio_dev->modes = INDIO_DIRECT_MODE;
indio_dev->channels = mcp9600_channels[ch_sel];
indio_dev->num_channels = ARRAY_SIZE(mcp9600_channels[ch_sel]);
@@ -447,14 +470,26 @@ static int mcp9600_probe(struct i2c_client *client)
return devm_iio_device_register(&client->dev, indio_dev);
}
+static const struct mcp_chip_info mcp9600_chip_info = {
+ .chip_id = MCP9600_DEVICE_ID_MCP9600,
+ .chip_name = "mcp9600",
+};
+
+static const struct mcp_chip_info mcp9601_chip_info = {
+ .chip_id = MCP9600_DEVICE_ID_MCP9601,
+ .chip_name = "mcp9601",
+};
+
static const struct i2c_device_id mcp9600_id[] = {
- { "mcp9600" },
+ { "mcp9600", .driver_data = (kernel_ulong_t)&mcp9600_chip_info },
+ { "mcp9601", .driver_data = (kernel_ulong_t)&mcp9601_chip_info },
{ }
};
MODULE_DEVICE_TABLE(i2c, mcp9600_id);
static const struct of_device_id mcp9600_of_match[] = {
- { .compatible = "microchip,mcp9600" },
+ { .compatible = "microchip,mcp9600", .data = &mcp9600_chip_info },
+ { .compatible = "microchip,mcp9601", .data = &mcp9601_chip_info },
{ }
};
MODULE_DEVICE_TABLE(of, mcp9600_of_match);
--
2.39.5On Sun, Aug 17, 2025 at 11:50:51PM -0400, Ben Collins wrote:
> The current driver works with mcp9601, but emits a warning because it
> does not recognize the chip id.
>
> MCP9601 is a superset of MCP9600. The drivers works without changes
> on this chipset.
>
> However, the 9601 chip supports open/closed-circuit detection if wired
> properly, so we'll need to be able to differentiate between them.
...
> static int mcp9600_probe(struct i2c_client *client)
> {
> + const struct mcp_chip_info *chip_info = i2c_get_match_data(client);
I believe I have commented on this already, please, split assignment...
> struct iio_dev *indio_dev;
> struct mcp9600_data *data;
> - int ret, ch_sel;
> + int ch_sel, dev_id, ret;
...and put it here.
> + if (chip_info == NULL)
> + return dev_err_probe(&client->dev, -EINVAL,
> + "No chip-info found for device\n");
Wrong indentation.
Besides that I have commented as well on
struct device *dev = &client->dev;
at the top that helps to make the code neater.
...
Since it seems the comments were ignored, I stopped here. Please, find previous
emails, take your time and fine grain the result for the next version.
--
With Best Regards,
Andy Shevchenko
On Thu, Aug 21, 2025 at 12:33:19PM -0500, Andy Shevchenko wrote:
> On Sun, Aug 17, 2025 at 11:50:51PM -0400, Ben Collins wrote:
> > The current driver works with mcp9601, but emits a warning because it
> > does not recognize the chip id.
> >
> > MCP9601 is a superset of MCP9600. The drivers works without changes
> > on this chipset.
> >
> > However, the 9601 chip supports open/closed-circuit detection if wired
> > properly, so we'll need to be able to differentiate between them.
>
> ...
>
> > static int mcp9600_probe(struct i2c_client *client)
> > {
> > + const struct mcp_chip_info *chip_info = i2c_get_match_data(client);
>
> I believe I have commented on this already, please, split assignment...
>
> > struct iio_dev *indio_dev;
> > struct mcp9600_data *data;
> > - int ret, ch_sel;
> > + int ch_sel, dev_id, ret;
>
> ...and put it here.
>
> > + if (chip_info == NULL)
> > + return dev_err_probe(&client->dev, -EINVAL,
> > + "No chip-info found for device\n");
>
> Wrong indentation.
>
> Besides that I have commented as well on
>
> struct device *dev = &client->dev;
>
> at the top that helps to make the code neater.
>
> ...
>
> Since it seems the comments were ignored, I stopped here. Please, find previous
> emails, take your time and fine grain the result for the next version.
You're looking at v4. The latest is v7.
--
Ben Collins
https://libjwt.io
https://github.com/benmcollins
--
3EC9 7598 1672 961A 1139 173A 5D5A 57C7 242B 22CF
On Thu, Aug 21, 2025 at 07:14:21AM -0400, Ben Collins wrote: > On Thu, Aug 21, 2025 at 12:33:19PM -0500, Andy Shevchenko wrote: > > On Sun, Aug 17, 2025 at 11:50:51PM -0400, Ben Collins wrote: ... > > Since it seems the comments were ignored, I stopped here. Please, find previous > > emails, take your time and fine grain the result for the next version. > > You're looking at v4. The latest is v7. Nice, please make sure you are not so quick with new versions. Can you Cc me for v8? Or at least give a pointer to v7? -- With Best Regards, Andy Shevchenko
© 2016 - 2025 Red Hat, Inc.