Re: [PATCH 0/2] Add support for Microchip EMC1812

Re: [PATCH 0/2] Add support for Microchip EMC1812

[Jonathan Cameron]

On Wed, 17 Sep 2025 15:21:56 +0300
Marius Cristea <marius.cristea@microchip.com> wrote:

> This is the iio driver for EMC1812/13/14/15/33 multichannel Low-Voltage
> Remote Diode Sensor Family. The chips in the family have one internal
> and different numbers of external channels, ranging from 1 (EMC1812) to
> 4 channels (EMC1815).
> Reading diodes in anti-parallel connection is supported by EMC1814, EMC1815
> and EMC1833.
> 
> Current version of driver does not support interrupts, events and data
> buffering.
Hi Marius,

For a temperature monitoring device like this, the opening question is
always why not HWMON?

There are various reasons we have temp sensors in IIO but mostly they are not
described as being monitors and this one is.

IIO may well be the right choice for this part, but good to lay out your
reasoning and +CC the hwmon list and maintainers.  There is an emc1403
driver already in hwmon, so perhaps compare and contrast with that.

I've +CC Jean, Guenter and list to save sending a v2 just to do that.

Jonathan


> 
> Differences related to previous patch:
> 
> v1:
> - initial version.
> 
> Signed-off-by: Marius Cristea <marius.cristea@microchip.com>
> ---
> Marius Cristea (2):
>       dt-bindings: iio: temperature: add support for EMC1812
>       iio: temperature: add support for EMC1812
> 
>  .../iio/temperature/microchip,emc1812.yaml         | 223 ++++++
>  MAINTAINERS                                        |   7 +
>  drivers/iio/temperature/Kconfig                    |  10 +
>  drivers/iio/temperature/Makefile                   |   1 +
>  drivers/iio/temperature/emc1812.c                  | 792 +++++++++++++++++++++
>  5 files changed, 1033 insertions(+)
> ---
> base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
> change-id: 20250805-iio-emc1812-e666183b07b5
> 
> Best regards,

Re: [PATCH 0/2] Add support for Microchip EMC1812

[Guenter Roeck]

On 9/20/25 04:33, Jonathan Cameron wrote:
> On Wed, 17 Sep 2025 15:21:56 +0300
> Marius Cristea <marius.cristea@microchip.com> wrote:
> 
>> This is the iio driver for EMC1812/13/14/15/33 multichannel Low-Voltage
>> Remote Diode Sensor Family. The chips in the family have one internal
>> and different numbers of external channels, ranging from 1 (EMC1812) to
>> 4 channels (EMC1815).
>> Reading diodes in anti-parallel connection is supported by EMC1814, EMC1815
>> and EMC1833.
>>
>> Current version of driver does not support interrupts, events and data
>> buffering.
> Hi Marius,
> 
> For a temperature monitoring device like this, the opening question is
> always why not HWMON?
> 
> There are various reasons we have temp sensors in IIO but mostly they are not
> described as being monitors and this one is.
> 
> IIO may well be the right choice for this part, but good to lay out your
> reasoning and +CC the hwmon list and maintainers.  There is an emc1403
> driver already in hwmon, so perhaps compare and contrast with that.
> 
> I've +CC Jean, Guenter and list to save sending a v2 just to do that.
> 

At first glance it looks like the series is (mostly ?) register compatible
to the chips supported by the emc1403 driver, so it should be straightforward
to add support for the emc180x series to that driver.

Guenter

Re: [PATCH 0/2] Add support for Microchip EMC1812

[Marius.Cristea]

Hi Guenter,

Thank you for the feedback.

On Tue, 2025-09-23 at 19:11 -0700, Guenter Roeck wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> On 9/20/25 04:33, Jonathan Cameron wrote:
> > On Wed, 17 Sep 2025 15:21:56 +0300
> > Marius Cristea <marius.cristea@microchip.com> wrote:
> > 
> > > This is the iio driver for EMC1812/13/14/15/33 multichannel Low-
> > > Voltage
> > > Remote Diode Sensor Family. The chips in the family have one
> > > internal
> > > and different numbers of external channels, ranging from 1
> > > (EMC1812) to
> > > 4 channels (EMC1815).
> > > Reading diodes in anti-parallel connection is supported by
> > > EMC1814, EMC1815
> > > and EMC1833.
> > > 
> > > Current version of driver does not support interrupts, events and
> > > data
> > > buffering.
> > Hi Marius,
> > 
> > For a temperature monitoring device like this, the opening question
> > is
> > always why not HWMON?
> > 
> > There are various reasons we have temp sensors in IIO but mostly
> > they are not
> > described as being monitors and this one is.
> > 
> > IIO may well be the right choice for this part, but good to lay out
> > your
> > reasoning and +CC the hwmon list and maintainers.  There is an
> > emc1403
> > driver already in hwmon, so perhaps compare and contrast with that.
> > 
> > I've +CC Jean, Guenter and list to save sending a v2 just to do
> > that.
> > 
> 
> At first glance it looks like the series is (mostly ?) register
> compatible
> to the chips supported by the emc1403 driver, so it should be
> straightforward
> to add support for the emc180x series to that driver.
> 
> Guenter

Most of the register address are compatible. The EMC181X is an update 
(a newer generation) then the EMC1403.

The biggest improvement is that the EMC18XX has a continuous block of
registers in order to improve the temperature reading (that means some
addresses are overlapping with the older register maps) and a new set
of registers to  handle the "Rate Of Change" functionality.
Also the older EMC14XX has some hardcoded configuration/features based
on the part number.

Considering all of the above I consider that the complexity of the
EMC1403 will increase quite a lot without any real benefit and it will
be harder to be maintained.

I have submitted this as the fist iteration from a longer list of
feature that I want to add to the driver, including events and maybe
interrupts.

If nobody has anything against, I would like to add a separate driver
for EMC18XX into the IIO.

Thanks and Best Regards,
Marius

Re: [PATCH 0/2] Add support for Microchip EMC1812

[Guenter Roeck]

On Thu, Sep 25, 2025 at 09:09:04AM +0000, Marius.Cristea@microchip.com wrote:
> Hi Guenter,
> 
> Thank you for the feedback.
> 
> On Tue, 2025-09-23 at 19:11 -0700, Guenter Roeck wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > know the content is safe
> > 
> > On 9/20/25 04:33, Jonathan Cameron wrote:
> > > On Wed, 17 Sep 2025 15:21:56 +0300
> > > Marius Cristea <marius.cristea@microchip.com> wrote:
> > > 
> > > > This is the iio driver for EMC1812/13/14/15/33 multichannel Low-
> > > > Voltage
> > > > Remote Diode Sensor Family. The chips in the family have one
> > > > internal
> > > > and different numbers of external channels, ranging from 1
> > > > (EMC1812) to
> > > > 4 channels (EMC1815).
> > > > Reading diodes in anti-parallel connection is supported by
> > > > EMC1814, EMC1815
> > > > and EMC1833.
> > > > 
> > > > Current version of driver does not support interrupts, events and
> > > > data
> > > > buffering.
> > > Hi Marius,
> > > 
> > > For a temperature monitoring device like this, the opening question
> > > is
> > > always why not HWMON?
> > > 
> > > There are various reasons we have temp sensors in IIO but mostly
> > > they are not
> > > described as being monitors and this one is.
> > > 
> > > IIO may well be the right choice for this part, but good to lay out
> > > your
> > > reasoning and +CC the hwmon list and maintainers.  There is an
> > > emc1403
> > > driver already in hwmon, so perhaps compare and contrast with that.
> > > 
> > > I've +CC Jean, Guenter and list to save sending a v2 just to do
> > > that.
> > > 
> > 
> > At first glance it looks like the series is (mostly ?) register
> > compatible
> > to the chips supported by the emc1403 driver, so it should be
> > straightforward
> > to add support for the emc180x series to that driver.
> > 
> > Guenter
> 
> Most of the register address are compatible. The EMC181X is an update 
> (a newer generation) then the EMC1403.
> 
> The biggest improvement is that the EMC18XX has a continuous block of
> registers in order to improve the temperature reading (that means some
> addresses are overlapping with the older register maps) and a new set
> of registers to  handle the "Rate Of Change" functionality.
> Also the older EMC14XX has some hardcoded configuration/features based
> on the part number.
> 
> Considering all of the above I consider that the complexity of the
> EMC1403 will increase quite a lot without any real benefit and it will
> be harder to be maintained.
> 
Ok.

> I have submitted this as the fist iteration from a longer list of
> feature that I want to add to the driver, including events and maybe
> interrupts.
> 
> If nobody has anything against, I would like to add a separate driver
> for EMC18XX into the IIO.

IMO this should be a hwmon driver.

Guenter

Re: [PATCH 0/2] Add support for Microchip EMC1812

[Jonathan Cameron]

On Thu, 25 Sep 2025 07:32:22 -0700
Guenter Roeck <linux@roeck-us.net> wrote:

> On Thu, Sep 25, 2025 at 09:09:04AM +0000, Marius.Cristea@microchip.com wrote:
> > Hi Guenter,
> > 
> > Thank you for the feedback.
> > 
> > On Tue, 2025-09-23 at 19:11 -0700, Guenter Roeck wrote:  
> > > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > > know the content is safe
> > > 
> > > On 9/20/25 04:33, Jonathan Cameron wrote:  
> > > > On Wed, 17 Sep 2025 15:21:56 +0300
> > > > Marius Cristea <marius.cristea@microchip.com> wrote:
> > > >   
> > > > > This is the iio driver for EMC1812/13/14/15/33 multichannel Low-
> > > > > Voltage
> > > > > Remote Diode Sensor Family. The chips in the family have one
> > > > > internal
> > > > > and different numbers of external channels, ranging from 1
> > > > > (EMC1812) to
> > > > > 4 channels (EMC1815).
> > > > > Reading diodes in anti-parallel connection is supported by
> > > > > EMC1814, EMC1815
> > > > > and EMC1833.
> > > > > 
> > > > > Current version of driver does not support interrupts, events and
> > > > > data
> > > > > buffering.  
> > > > Hi Marius,
> > > > 
> > > > For a temperature monitoring device like this, the opening question
> > > > is
> > > > always why not HWMON?
> > > > 
> > > > There are various reasons we have temp sensors in IIO but mostly
> > > > they are not
> > > > described as being monitors and this one is.
> > > > 
> > > > IIO may well be the right choice for this part, but good to lay out
> > > > your
> > > > reasoning and +CC the hwmon list and maintainers.  There is an
> > > > emc1403
> > > > driver already in hwmon, so perhaps compare and contrast with that.
> > > > 
> > > > I've +CC Jean, Guenter and list to save sending a v2 just to do
> > > > that.
> > > >   
> > > 
> > > At first glance it looks like the series is (mostly ?) register
> > > compatible
> > > to the chips supported by the emc1403 driver, so it should be
> > > straightforward
> > > to add support for the emc180x series to that driver.
> > > 
> > > Guenter  
> > 
> > Most of the register address are compatible. The EMC181X is an update 
> > (a newer generation) then the EMC1403.
> > 
> > The biggest improvement is that the EMC18XX has a continuous block of
> > registers in order to improve the temperature reading (that means some
> > addresses are overlapping with the older register maps) and a new set
> > of registers to  handle the "Rate Of Change" functionality.
> > Also the older EMC14XX has some hardcoded configuration/features based
> > on the part number.
> > 
> > Considering all of the above I consider that the complexity of the
> > EMC1403 will increase quite a lot without any real benefit and it will
> > be harder to be maintained.
> >   
> Ok.
> 
> > I have submitted this as the fist iteration from a longer list of
> > feature that I want to add to the driver, including events and maybe
> > interrupts.
> > 
> > If nobody has anything against, I would like to add a separate driver
> > for EMC18XX into the IIO.  
> 
> IMO this should be a hwmon driver.

So far I haven't seen any reason why IIO would make more sense, so agreed.

Jonathan

> 
> Guenter

[PATCH 0/2] Add support for Microchip EMC1812

[Marius Cristea]

This is the hwmon driver for EMC1812/13/14/15/33 multichannel Low-Voltage
Remote Diode Sensor Family. The chips in the family have one internal
and different numbers of external channels, ranging from 1 (EMC1812) to
4 channels (EMC1815).
Reading diodes in anti-parallel connection is supported by EMC1814, EMC1815
and EMC1833.

Signed-off-by: Marius Cristea <marius.cristea@microchip.com>
---
Marius Cristea (2):
      dt-bindings: hwmon: temperature: add support for EMC1812
      hwmon: temperature: add support for EMC1812

 .../bindings/hwmon/microchip,emc1812.yaml          | 176 ++++
 Documentation/hwmon/emc1812.rst                    |  68 ++
 MAINTAINERS                                        |   8 +
 drivers/hwmon/Kconfig                              |  11 +
 drivers/hwmon/Makefile                             |   1 +
 drivers/hwmon/emc1812.c                            | 967 +++++++++++++++++++++
 6 files changed, 1231 insertions(+)
---
base-commit: d2b2fea3503e5e12b2e28784152937e48bcca6ff
change-id: 20251002-hw_mon-emc1812-f1b806487d10

Best regards,
-- 
Marius Cristea <marius.cristea@microchip.com>