| Message ID | 20231020-topic-chipcap2-v2-2-f5c325966fdb@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | hwmon: Add support for Amphenol ChipCap 2 | expand |
On 11/8/23 07:37, Javier Carrasco wrote: > Add min_alarm and max_alarm attributes for humidityX to support devices > that can generate these alarms. > Such attributes already exist for other magnitudes such as tempX. > > Tested with a ChipCap 2 temperature-humidity sensor. > No objection, but the new attributes also need to be added to the ABI documentation at Documentation/ABI/testing/sysfs-class-hwmon and Documentation/hwmon/sysfs-interface.rst Which made me notice that humidityX_alarm isn't documented either. Please document that attribute as well while you are at it. Thanks, Guenter > Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com> > --- > drivers/hwmon/hwmon.c | 2 ++ > include/linux/hwmon.h | 4 ++++ > 2 files changed, 6 insertions(+) > > diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c > index c7dd3f5b2bd5..7f92984c37d9 100644 > --- a/drivers/hwmon/hwmon.c > +++ b/drivers/hwmon/hwmon.c > @@ -579,8 +579,10 @@ static const char * const hwmon_humidity_attr_templates[] = { > [hwmon_humidity_input] = "humidity%d_input", > [hwmon_humidity_label] = "humidity%d_label", > [hwmon_humidity_min] = "humidity%d_min", > + [hwmon_humidity_min_alarm] = "humidity%d_min_alarm", > [hwmon_humidity_min_hyst] = "humidity%d_min_hyst", > [hwmon_humidity_max] = "humidity%d_max", > + [hwmon_humidity_max_alarm] = "humidity%d_max_alarm", > [hwmon_humidity_max_hyst] = "humidity%d_max_hyst", > [hwmon_humidity_alarm] = "humidity%d_alarm", > [hwmon_humidity_fault] = "humidity%d_fault", > diff --git a/include/linux/hwmon.h b/include/linux/hwmon.h > index 8cd6a6b33593..154de35e34ac 100644 > --- a/include/linux/hwmon.h > +++ b/include/linux/hwmon.h > @@ -286,8 +286,10 @@ enum hwmon_humidity_attributes { > hwmon_humidity_input, > hwmon_humidity_label, > hwmon_humidity_min, > + hwmon_humidity_min_alarm, > hwmon_humidity_min_hyst, > hwmon_humidity_max, > + hwmon_humidity_max_alarm, > hwmon_humidity_max_hyst, > hwmon_humidity_alarm, > hwmon_humidity_fault, > @@ -299,8 +301,10 @@ enum hwmon_humidity_attributes { > #define HWMON_H_INPUT BIT(hwmon_humidity_input) > #define HWMON_H_LABEL BIT(hwmon_humidity_label) > #define HWMON_H_MIN BIT(hwmon_humidity_min) > +#define HWMON_H_MIN_ALARM BIT(hwmon_humidity_min_alarm) > #define HWMON_H_MIN_HYST BIT(hwmon_humidity_min_hyst) > #define HWMON_H_MAX BIT(hwmon_humidity_max) > +#define HWMON_H_MAX_ALARM BIT(hwmon_humidity_max_alarm) > #define HWMON_H_MAX_HYST BIT(hwmon_humidity_max_hyst) > #define HWMON_H_ALARM BIT(hwmon_humidity_alarm) > #define HWMON_H_FAULT BIT(hwmon_humidity_fault) >
Hello, On 09.11.23 01:02, Guenter Roeck wrote: > On 11/8/23 07:37, Javier Carrasco wrote: >> Add min_alarm and max_alarm attributes for humidityX to support devices >> that can generate these alarms. >> Such attributes already exist for other magnitudes such as tempX. >> >> Tested with a ChipCap 2 temperature-humidity sensor. >> > > No objection, but the new attributes also need to be added to the ABI > documentation at > Documentation/ABI/testing/sysfs-class-hwmon and > Documentation/hwmon/sysfs-interface.rst > > Which made me notice that humidityX_alarm isn't documented either. > Please document that attribute as well while you are at it. > > Thanks, > Guenter > Actually there are several attributes without ABI documentation or at least the attributes enum is much larger than the objects in the ABI documentation (in testing/sysfs-class-hwmon). For humidity there is only input, enable, rated_min and rated_max. Are some attributes not described for a good reason or should all be documented? the current humidity_attributes contains: hwmon_humidity_enable -> documented in sysfs-class-hwmon hwmon_humidity_input -> documented in sysfs-class-hwmon hwmon_humidity_label hwmon_humidity_min hwmon_humidity_min_hyst hwmon_humidity_max hwmon_humidity_max_hyst hwmon_humidity_alarm hwmon_humidity_fault hwmon_humidity_rated_min -> documented in sysfs-class-hwmon hwmon_humidity_rated_max -> documented in sysfs-class-hwmon I could not find the temperature counterparts of my new additions (temp_min_alarm and temp_max_alarm). Should all be added to sysfs-class-hwmon or am I missing some other document? I am alright adding the ones I mentioned. >> Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com> >> --- >> drivers/hwmon/hwmon.c | 2 ++ >> include/linux/hwmon.h | 4 ++++ >> 2 files changed, 6 insertions(+) >> >> diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c >> index c7dd3f5b2bd5..7f92984c37d9 100644 >> --- a/drivers/hwmon/hwmon.c >> +++ b/drivers/hwmon/hwmon.c >> @@ -579,8 +579,10 @@ static const char * const >> hwmon_humidity_attr_templates[] = { >> [hwmon_humidity_input] = "humidity%d_input", >> [hwmon_humidity_label] = "humidity%d_label", >> [hwmon_humidity_min] = "humidity%d_min", >> + [hwmon_humidity_min_alarm] = "humidity%d_min_alarm", >> [hwmon_humidity_min_hyst] = "humidity%d_min_hyst", >> [hwmon_humidity_max] = "humidity%d_max", >> + [hwmon_humidity_max_alarm] = "humidity%d_max_alarm", >> [hwmon_humidity_max_hyst] = "humidity%d_max_hyst", >> [hwmon_humidity_alarm] = "humidity%d_alarm", >> [hwmon_humidity_fault] = "humidity%d_fault", >> diff --git a/include/linux/hwmon.h b/include/linux/hwmon.h >> index 8cd6a6b33593..154de35e34ac 100644 >> --- a/include/linux/hwmon.h >> +++ b/include/linux/hwmon.h >> @@ -286,8 +286,10 @@ enum hwmon_humidity_attributes { >> hwmon_humidity_input, >> hwmon_humidity_label, >> hwmon_humidity_min, >> + hwmon_humidity_min_alarm, >> hwmon_humidity_min_hyst, >> hwmon_humidity_max, >> + hwmon_humidity_max_alarm, >> hwmon_humidity_max_hyst, >> hwmon_humidity_alarm, >> hwmon_humidity_fault, >> @@ -299,8 +301,10 @@ enum hwmon_humidity_attributes { >> #define HWMON_H_INPUT BIT(hwmon_humidity_input) >> #define HWMON_H_LABEL BIT(hwmon_humidity_label) >> #define HWMON_H_MIN BIT(hwmon_humidity_min) >> +#define HWMON_H_MIN_ALARM BIT(hwmon_humidity_min_alarm) >> #define HWMON_H_MIN_HYST BIT(hwmon_humidity_min_hyst) >> #define HWMON_H_MAX BIT(hwmon_humidity_max) >> +#define HWMON_H_MAX_ALARM BIT(hwmon_humidity_max_alarm) >> #define HWMON_H_MAX_HYST BIT(hwmon_humidity_max_hyst) >> #define HWMON_H_ALARM BIT(hwmon_humidity_alarm) >> #define HWMON_H_FAULT BIT(hwmon_humidity_fault) >> > Best regards, Javier Carrasco
On Thu, Nov 09, 2023 at 07:24:00AM +0100, Javier Carrasco wrote: > Hello, > > On 09.11.23 01:02, Guenter Roeck wrote: > > On 11/8/23 07:37, Javier Carrasco wrote: > >> Add min_alarm and max_alarm attributes for humidityX to support devices > >> that can generate these alarms. > >> Such attributes already exist for other magnitudes such as tempX. > >> > >> Tested with a ChipCap 2 temperature-humidity sensor. > >> > > > > No objection, but the new attributes also need to be added to the ABI > > documentation at > > Documentation/ABI/testing/sysfs-class-hwmon and > > Documentation/hwmon/sysfs-interface.rst > > > > Which made me notice that humidityX_alarm isn't documented either. > > Please document that attribute as well while you are at it. > > > > Thanks, > > Guenter > > > Actually there are several attributes without ABI documentation or at > least the attributes enum is much larger than the objects in the ABI > documentation (in testing/sysfs-class-hwmon). > For humidity there is only input, enable, rated_min and rated_max. Are > some attributes not described for a good reason or should all be > documented? the current humidity_attributes contains: > > hwmon_humidity_enable -> documented in sysfs-class-hwmon > hwmon_humidity_input -> documented in sysfs-class-hwmon > hwmon_humidity_label > hwmon_humidity_min > hwmon_humidity_min_hyst > hwmon_humidity_max > hwmon_humidity_max_hyst > hwmon_humidity_alarm > hwmon_humidity_fault > hwmon_humidity_rated_min -> documented in sysfs-class-hwmon > hwmon_humidity_rated_max -> documented in sysfs-class-hwmon > > I could not find the temperature counterparts of my new additions > (temp_min_alarm and temp_max_alarm). > > Should all be added to sysfs-class-hwmon or am I missing some other > document? I am alright adding the ones I mentioned. > They should all be documented. It would be great if you volunteer to add the missing ones, but that won't be a mandate. I just don't want the situation to get worse. Thanks, Guenter
On 15.11.23 14:25, Guenter Roeck wrote: > On Thu, Nov 09, 2023 at 07:24:00AM +0100, Javier Carrasco wrote: >> Hello, >> >> On 09.11.23 01:02, Guenter Roeck wrote: >>> On 11/8/23 07:37, Javier Carrasco wrote: >>>> Add min_alarm and max_alarm attributes for humidityX to support devices >>>> that can generate these alarms. >>>> Such attributes already exist for other magnitudes such as tempX. >>>> >>>> Tested with a ChipCap 2 temperature-humidity sensor. >>>> >>> >>> No objection, but the new attributes also need to be added to the ABI >>> documentation at >>> Documentation/ABI/testing/sysfs-class-hwmon and >>> Documentation/hwmon/sysfs-interface.rst >>> >>> Which made me notice that humidityX_alarm isn't documented either. >>> Please document that attribute as well while you are at it. >>> >>> Thanks, >>> Guenter >>> >> Actually there are several attributes without ABI documentation or at >> least the attributes enum is much larger than the objects in the ABI >> documentation (in testing/sysfs-class-hwmon). >> For humidity there is only input, enable, rated_min and rated_max. Are >> some attributes not described for a good reason or should all be >> documented? the current humidity_attributes contains: >> >> hwmon_humidity_enable -> documented in sysfs-class-hwmon >> hwmon_humidity_input -> documented in sysfs-class-hwmon >> hwmon_humidity_label >> hwmon_humidity_min >> hwmon_humidity_min_hyst >> hwmon_humidity_max >> hwmon_humidity_max_hyst >> hwmon_humidity_alarm >> hwmon_humidity_fault >> hwmon_humidity_rated_min -> documented in sysfs-class-hwmon >> hwmon_humidity_rated_max -> documented in sysfs-class-hwmon >> >> I could not find the temperature counterparts of my new additions >> (temp_min_alarm and temp_max_alarm). >> >> Should all be added to sysfs-class-hwmon or am I missing some other >> document? I am alright adding the ones I mentioned. >> > > They should all be documented. It would be great if you volunteer > to add the missing ones, but that won't be a mandate. I just don't want > the situation to get worse. > > Thanks, > Guenter I will document them in a separate series, so the (probably) long review of the original device driver will not have any effect on this task, which has nothing to do with the ChipCap 2 anyways. I will start asap. Best regards, Javier Carrasco
diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c index c7dd3f5b2bd5..7f92984c37d9 100644 --- a/drivers/hwmon/hwmon.c +++ b/drivers/hwmon/hwmon.c @@ -579,8 +579,10 @@ static const char * const hwmon_humidity_attr_templates[] = { [hwmon_humidity_input] = "humidity%d_input", [hwmon_humidity_label] = "humidity%d_label", [hwmon_humidity_min] = "humidity%d_min", + [hwmon_humidity_min_alarm] = "humidity%d_min_alarm", [hwmon_humidity_min_hyst] = "humidity%d_min_hyst", [hwmon_humidity_max] = "humidity%d_max", + [hwmon_humidity_max_alarm] = "humidity%d_max_alarm", [hwmon_humidity_max_hyst] = "humidity%d_max_hyst", [hwmon_humidity_alarm] = "humidity%d_alarm", [hwmon_humidity_fault] = "humidity%d_fault", diff --git a/include/linux/hwmon.h b/include/linux/hwmon.h index 8cd6a6b33593..154de35e34ac 100644 --- a/include/linux/hwmon.h +++ b/include/linux/hwmon.h @@ -286,8 +286,10 @@ enum hwmon_humidity_attributes { hwmon_humidity_input, hwmon_humidity_label, hwmon_humidity_min, + hwmon_humidity_min_alarm, hwmon_humidity_min_hyst, hwmon_humidity_max, + hwmon_humidity_max_alarm, hwmon_humidity_max_hyst, hwmon_humidity_alarm, hwmon_humidity_fault, @@ -299,8 +301,10 @@ enum hwmon_humidity_attributes { #define HWMON_H_INPUT BIT(hwmon_humidity_input) #define HWMON_H_LABEL BIT(hwmon_humidity_label) #define HWMON_H_MIN BIT(hwmon_humidity_min) +#define HWMON_H_MIN_ALARM BIT(hwmon_humidity_min_alarm) #define HWMON_H_MIN_HYST BIT(hwmon_humidity_min_hyst) #define HWMON_H_MAX BIT(hwmon_humidity_max) +#define HWMON_H_MAX_ALARM BIT(hwmon_humidity_max_alarm) #define HWMON_H_MAX_HYST BIT(hwmon_humidity_max_hyst) #define HWMON_H_ALARM BIT(hwmon_humidity_alarm) #define HWMON_H_FAULT BIT(hwmon_humidity_fault)
Add min_alarm and max_alarm attributes for humidityX to support devices that can generate these alarms. Such attributes already exist for other magnitudes such as tempX. Tested with a ChipCap 2 temperature-humidity sensor. Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com> --- drivers/hwmon/hwmon.c | 2 ++ include/linux/hwmon.h | 4 ++++ 2 files changed, 6 insertions(+)