* Re: [PATCH 0/2] Add support for Microchip EMC1812
[not found] <20250917-iio-emc1812-v1-0-0b1f74cea7ab@microchip.com>
@ 2025-09-20 11:33 ` Jonathan Cameron
2025-09-24 2:11 ` Guenter Roeck
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2025-09-20 11:33 UTC (permalink / raw)
To: Marius Cristea
Cc: David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-iio, devicetree,
linux-kernel, jdelvare, linux, linux-hwmon
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,
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] Add support for Microchip EMC1812
2025-09-20 11:33 ` [PATCH 0/2] Add support for Microchip EMC1812 Jonathan Cameron
@ 2025-09-24 2:11 ` Guenter Roeck
2025-09-25 9:09 ` Marius.Cristea
0 siblings, 1 reply; 6+ messages in thread
From: Guenter Roeck @ 2025-09-24 2:11 UTC (permalink / raw)
To: Jonathan Cameron, Marius Cristea
Cc: David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-iio, devicetree,
linux-kernel, jdelvare, linux-hwmon
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] Add support for Microchip EMC1812
2025-09-24 2:11 ` Guenter Roeck
@ 2025-09-25 9:09 ` Marius.Cristea
2025-09-25 14:32 ` Guenter Roeck
0 siblings, 1 reply; 6+ messages in thread
From: Marius.Cristea @ 2025-09-25 9:09 UTC (permalink / raw)
To: linux, jic23
Cc: dlechner, linux-hwmon, jdelvare, nuno.sa, linux-iio, devicetree,
robh, linux-kernel, andy, krzk+dt, conor+dt
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] Add support for Microchip EMC1812
2025-09-25 9:09 ` Marius.Cristea
@ 2025-09-25 14:32 ` Guenter Roeck
2025-09-27 15:04 ` Jonathan Cameron
0 siblings, 1 reply; 6+ messages in thread
From: Guenter Roeck @ 2025-09-25 14:32 UTC (permalink / raw)
To: Marius.Cristea
Cc: jic23, dlechner, linux-hwmon, jdelvare, nuno.sa, linux-iio,
devicetree, robh, linux-kernel, andy, krzk+dt, conor+dt
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] Add support for Microchip EMC1812
2025-09-25 14:32 ` Guenter Roeck
@ 2025-09-27 15:04 ` Jonathan Cameron
0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2025-09-27 15:04 UTC (permalink / raw)
To: Guenter Roeck
Cc: Marius.Cristea, dlechner, linux-hwmon, jdelvare, nuno.sa,
linux-iio, devicetree, robh, linux-kernel, andy, krzk+dt,
conor+dt
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 0/2] Add support for Microchip EMC1812
@ 2025-10-29 15:50 Marius Cristea
0 siblings, 0 replies; 6+ messages in thread
From: Marius Cristea @ 2025-10-29 15:50 UTC (permalink / raw)
To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jonathan Corbet
Cc: linux-hwmon, devicetree, linux-kernel, linux-doc, 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>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-10-29 15:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20250917-iio-emc1812-v1-0-0b1f74cea7ab@microchip.com>
2025-09-20 11:33 ` [PATCH 0/2] Add support for Microchip EMC1812 Jonathan Cameron
2025-09-24 2:11 ` Guenter Roeck
2025-09-25 9:09 ` Marius.Cristea
2025-09-25 14:32 ` Guenter Roeck
2025-09-27 15:04 ` Jonathan Cameron
2025-10-29 15:50 Marius Cristea
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox