Add Nuvoton NCT7718W IIO driver

[PATCH v1 0/2] Add Nuvoton NCT7718W IIO driver

Posted by Ming Yu 1 year, 2 months ago

NCT7718W is an I2C based thermal sensor chip from Nuvoton.

Ming Yu (2):
  dt-bindings: iio: temperature: Add support for NCT7718W
  iio: temperature: Add Nuvoton NCT7718W support

 .../iio/temperature/nuvoton,nct7718.yaml      |  44 ++
 MAINTAINERS                                   |   7 +
 drivers/iio/temperature/Kconfig               |  10 +
 drivers/iio/temperature/Makefile              |   1 +
 drivers/iio/temperature/nct7718.c             | 505 ++++++++++++++++++
 5 files changed, 567 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/temperature/nuvoton,nct7718.yaml
 create mode 100644 drivers/iio/temperature/nct7718.c

-- 
2.34.1

Re: [PATCH v1 0/2] Add Nuvoton NCT7718W IIO driver

Posted by Jonathan Cameron 1 year, 2 months ago

On Tue, 26 Nov 2024 15:40:03 +0800
Ming Yu <a0282524688@gmail.com> wrote:

> NCT7718W is an I2C based thermal sensor chip from Nuvoton.
Hi Ming Yu,

+CC Jean and Guenter,

Why an IIO driver rather than a HWMON one?  Superficially this looks like a hwmon
chip.  We do have the means to put a generic driver in IIO and bridge to hwmon, but
when a device is very much intended for monitoring of hardware temperatures etc
the IIO driver rarely has any purpose and a simpler hwmon only solution makes sense.

For temperature sensors IIO normally makes sense if:
1) They are part of a series of devices some of which have more functionality than temp
2) Fast devices where hwmon sysfs interfaces become a bottleneck - note you have to have
a usecase for reading them fast, not simply a device that is capable of it.
3) 'Unusual' temperature sensors such as infrared thermometers or very high precision
   thermocouple interfaces.

Any of those apply here?

Note that hwmon has better threshold and critical temperature handling than we can do
in IIO and it seems your part has those as well.

Thanks,

Jonathan

> 
> Ming Yu (2):
>   dt-bindings: iio: temperature: Add support for NCT7718W
>   iio: temperature: Add Nuvoton NCT7718W support
> 
>  .../iio/temperature/nuvoton,nct7718.yaml      |  44 ++
>  MAINTAINERS                                   |   7 +
>  drivers/iio/temperature/Kconfig               |  10 +
>  drivers/iio/temperature/Makefile              |   1 +
>  drivers/iio/temperature/nct7718.c             | 505 ++++++++++++++++++
>  5 files changed, 567 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/temperature/nuvoton,nct7718.yaml
>  create mode 100644 drivers/iio/temperature/nct7718.c
>

Re: [PATCH v1 0/2] Add Nuvoton NCT7718W IIO driver

Posted by Ming Yu 1 year, 2 months ago

Dear Jonathan,

Thank you for your reply, I'll move the driver to HWMON.
Additionally, for conventional ADC, Thermal sensor, and tachometer
like chips, would it be more appropriate to implement them in HWMON?

Best regards
Ming


Jonathan Cameron <jic23@kernel.org> 於 2024年12月1日 週日 上午4:28寫道：
>
> On Tue, 26 Nov 2024 15:40:03 +0800
> Ming Yu <a0282524688@gmail.com> wrote:
>
> > NCT7718W is an I2C based thermal sensor chip from Nuvoton.
> Hi Ming Yu,
>
> +CC Jean and Guenter,
>
> Why an IIO driver rather than a HWMON one?  Superficially this looks like a hwmon
> chip.  We do have the means to put a generic driver in IIO and bridge to hwmon, but
> when a device is very much intended for monitoring of hardware temperatures etc
> the IIO driver rarely has any purpose and a simpler hwmon only solution makes sense.
>
> For temperature sensors IIO normally makes sense if:
> 1) They are part of a series of devices some of which have more functionality than temp
> 2) Fast devices where hwmon sysfs interfaces become a bottleneck - note you have to have
> a usecase for reading them fast, not simply a device that is capable of it.
> 3) 'Unusual' temperature sensors such as infrared thermometers or very high precision
>    thermocouple interfaces.
>
> Any of those apply here?
>
> Note that hwmon has better threshold and critical temperature handling than we can do
> in IIO and it seems your part has those as well.
>
> Thanks,
>
> Jonathan
>
>
> >
> > Ming Yu (2):
> >   dt-bindings: iio: temperature: Add support for NCT7718W
> >   iio: temperature: Add Nuvoton NCT7718W support
> >
> >  .../iio/temperature/nuvoton,nct7718.yaml      |  44 ++
> >  MAINTAINERS                                   |   7 +
> >  drivers/iio/temperature/Kconfig               |  10 +
> >  drivers/iio/temperature/Makefile              |   1 +
> >  drivers/iio/temperature/nct7718.c             | 505 ++++++++++++++++++
> >  5 files changed, 567 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/iio/temperature/nuvoton,nct7718.yaml
> >  create mode 100644 drivers/iio/temperature/nct7718.c
> >
>

Re: [PATCH v1 0/2] Add Nuvoton NCT7718W IIO driver

Posted by Guenter Roeck 1 year, 2 months ago

On Mon, Dec 02, 2024 at 05:22:52PM +0800, Ming Yu wrote:
> Dear Jonathan,
> 
> Thank you for your reply, I'll move the driver to HWMON.

AFAICS the chip is compatible to lm90, so please add support for
it to the lm90 driver. Note that this also applies to NCT7717, which
looks like an internal-sensor-only version of the same chip.

> Additionally, for conventional ADC, Thermal sensor, and tachometer
> like chips, would it be more appropriate to implement them in HWMON?
> 

Thermal sensor and tachometer, yes. For ADC it depends on the use case.
If the ADC is used for hardware monitoring (which is typically the case
if it has limit and alert support), yes. If it is a pure ADC, IIO
is a better place.

Thanks,
Guenter

Re: [PATCH v1 0/2] Add Nuvoton NCT7718W IIO driver

Posted by Guenter Roeck 1 year, 2 months ago

On 11/30/24 12:28, Jonathan Cameron wrote:
> On Tue, 26 Nov 2024 15:40:03 +0800
> Ming Yu <a0282524688@gmail.com> wrote:
> 
>> NCT7718W is an I2C based thermal sensor chip from Nuvoton.
> Hi Ming Yu,
> 
> +CC Jean and Guenter,
> 
> Why an IIO driver rather than a HWMON one?  Superficially this looks like a hwmon
> chip.  We do have the means to put a generic driver in IIO and bridge to hwmon, but
> when a device is very much intended for monitoring of hardware temperatures etc
> the IIO driver rarely has any purpose and a simpler hwmon only solution makes sense.
> 
> For temperature sensors IIO normally makes sense if:
> 1) They are part of a series of devices some of which have more functionality than temp
> 2) Fast devices where hwmon sysfs interfaces become a bottleneck - note you have to have
> a usecase for reading them fast, not simply a device that is capable of it.
> 3) 'Unusual' temperature sensors such as infrared thermometers or very high precision
>     thermocouple interfaces.
> 
> Any of those apply here?
> 

Also, it looks like this chip is compatible to LM90. It isn't entirely clear to me
why this would require a new driver.

Guenter

> Note that hwmon has better threshold and critical temperature handling than we can do
> in IIO and it seems your part has those as well.
> 
> Thanks,
> 
> Jonathan
> 
> 
>>
>> Ming Yu (2):
>>    dt-bindings: iio: temperature: Add support for NCT7718W
>>    iio: temperature: Add Nuvoton NCT7718W support
>>
>>   .../iio/temperature/nuvoton,nct7718.yaml      |  44 ++
>>   MAINTAINERS                                   |   7 +
>>   drivers/iio/temperature/Kconfig               |  10 +
>>   drivers/iio/temperature/Makefile              |   1 +
>>   drivers/iio/temperature/nct7718.c             | 505 ++++++++++++++++++
>>   5 files changed, 567 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/iio/temperature/nuvoton,nct7718.yaml
>>   create mode 100644 drivers/iio/temperature/nct7718.c
>>
>

Re: [PATCH v1 0/2] Add Nuvoton NCT7718W IIO driver

Posted by Ming Yu 1 year, 2 months ago

Dear Guenter,

Thank you for your reply. I will add a patch to the LM90 driver to
support NCT7718W.

Best regards,
Ming

Guenter Roeck <linux@roeck-us.net> 於 2024年12月1日 週日 上午4:50寫道：
>
> On 11/30/24 12:28, Jonathan Cameron wrote:
> > On Tue, 26 Nov 2024 15:40:03 +0800
> > Ming Yu <a0282524688@gmail.com> wrote:
> >
> >> NCT7718W is an I2C based thermal sensor chip from Nuvoton.
> > Hi Ming Yu,
> >
> > +CC Jean and Guenter,
> >
> > Why an IIO driver rather than a HWMON one?  Superficially this looks like a hwmon
> > chip.  We do have the means to put a generic driver in IIO and bridge to hwmon, but
> > when a device is very much intended for monitoring of hardware temperatures etc
> > the IIO driver rarely has any purpose and a simpler hwmon only solution makes sense.
> >
> > For temperature sensors IIO normally makes sense if:
> > 1) They are part of a series of devices some of which have more functionality than temp
> > 2) Fast devices where hwmon sysfs interfaces become a bottleneck - note you have to have
> > a usecase for reading them fast, not simply a device that is capable of it.
> > 3) 'Unusual' temperature sensors such as infrared thermometers or very high precision
> >     thermocouple interfaces.
> >
> > Any of those apply here?
> >
>
> Also, it looks like this chip is compatible to LM90. It isn't entirely clear to me
> why this would require a new driver.
>
> Guenter
>
> > Note that hwmon has better threshold and critical temperature handling than we can do
> > in IIO and it seems your part has those as well.
> >
> > Thanks,
> >
> > Jonathan
> >
> >
> >>
> >> Ming Yu (2):
> >>    dt-bindings: iio: temperature: Add support for NCT7718W
> >>    iio: temperature: Add Nuvoton NCT7718W support
> >>
> >>   .../iio/temperature/nuvoton,nct7718.yaml      |  44 ++
> >>   MAINTAINERS                                   |   7 +
> >>   drivers/iio/temperature/Kconfig               |  10 +
> >>   drivers/iio/temperature/Makefile              |   1 +
> >>   drivers/iio/temperature/nct7718.c             | 505 ++++++++++++++++++
> >>   5 files changed, 567 insertions(+)
> >>   create mode 100644 Documentation/devicetree/bindings/iio/temperature/nuvoton,nct7718.yaml
> >>   create mode 100644 drivers/iio/temperature/nct7718.c
> >>
> >
>