[linux@roeck-us.net: Re: Functional enhancement of max31827]

[linux@roeck-us.net: Re: Functional enhancement of max31827]

[Daniel Matyas]

----- Forwarded message from Guenter Roeck <linux@roeck-us.net> -----

Date: Sat, 29 Jul 2023 07:25:18 -0700
From: Guenter Roeck <linux@roeck-us.net>
To: Daniel Matyas <daniel.matyas23@gmail.com>, linux-hwmon@vger.kernel.org
Subject: Re: Functional enhancement of max31827
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0

On 7/27/23 23:49, Daniel Matyas wrote:
> Dear hwmon community,
> 
> I developed the driver for the max31827 temperature sensor, but when
> doing so I omitted some functionalities of the chip, namely: PEC Error,
> Fault Queue, Comp/Int, Alarm Polarity, Resolution, Timeout and PEC
> Enable. I did not implement these, because there are no predefined
> attributes in the linux kernel which have similar meaning to these
> funcionalities.
> 
> I recently got a request from the client to implement these
> functionalities. I want to use SENSOR_DEVICE_ATTR to implement these. I
> did some reading and saw that using debufs is also a possibility.
> 
> I would be really greatful, if you could provide me with some advices.
> Should I proceed with device attributes defined by me? Should I use
> debufs? Maybe there is another and better way...
> 

pec related attributes should be attached to the i2c interface. Other
drivers already do that; look for pec_{show,store}. The same is true
for timeout.

Comp/Int and Alarm polarity are system properties which should be set
via devicetree.

For others I am not sure. Fault Queue also smells like a system property
since normally one would only modify it if a given system requires it
(for example because the sensors are noisy). The same is true for sensor
resolution, though that one is closest for considering a sysfs attribute.
For most sensors, the resolution is closely related to conversion time
and can be handled with that attribute. I think that isn't the case here,
and there are numerous chips where the resolution is configurable, so it
may make sense to consider a sysfs attribute.

Guenter


----- End forwarded message -----

[linux@roeck-us.net: Re: Functional enhancement of max31827]

[Daniel Matyas]

----- Forwarded message from Guenter Roeck <linux@roeck-us.net> -----

Date: Sat, 29 Jul 2023 07:25:18 -0700
From: Guenter Roeck <linux@roeck-us.net>
To: Daniel Matyas <daniel.matyas23@gmail.com>, linux-hwmon@vger.kernel.org
Subject: Re: Functional enhancement of max31827
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0

On 7/27/23 23:49, Daniel Matyas wrote:
> Dear hwmon community,
> 
> I developed the driver for the max31827 temperature sensor, but when
> doing so I omitted some functionalities of the chip, namely: PEC Error,
> Fault Queue, Comp/Int, Alarm Polarity, Resolution, Timeout and PEC
> Enable. I did not implement these, because there are no predefined
> attributes in the linux kernel which have similar meaning to these
> funcionalities.
> 
> I recently got a request from the client to implement these
> functionalities. I want to use SENSOR_DEVICE_ATTR to implement these. I
> did some reading and saw that using debufs is also a possibility.
> 
> I would be really greatful, if you could provide me with some advices.
> Should I proceed with device attributes defined by me? Should I use
> debufs? Maybe there is another and better way...
> 

pec related attributes should be attached to the i2c interface. Other
drivers already do that; look for pec_{show,store}. The same is true
for timeout.

Comp/Int and Alarm polarity are system properties which should be set
via devicetree.

For others I am not sure. Fault Queue also smells like a system property
since normally one would only modify it if a given system requires it
(for example because the sensors are noisy). The same is true for sensor
resolution, though that one is closest for considering a sysfs attribute.
For most sensors, the resolution is closely related to conversion time
and can be handled with that attribute. I think that isn't the case here,
and there are numerous chips where the resolution is configurable, so it
may make sense to consider a sysfs attribute.

Guenter


----- End forwarded message -----