| Message ID | 20241028-hpd-v2-3-18f6e79154d7@chromium.org |
|---|---|
| State | New |
| Headers | show |
| Series | iio: hid-sensors-prox: Add support for more channels | expand |
On Mon, 28 Oct 2024 10:12:23 +0000 Ricardo Ribalda <ribalda@chromium.org> wrote: > Add a new channel type representing if the user's attention state to the > the system. This usually means if the user is looking at the screen or > not. > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > --- > Documentation/ABI/testing/sysfs-bus-iio | 7 +++++++ > drivers/iio/industrialio-core.c | 1 + > include/uapi/linux/iio/types.h | 1 + > tools/iio/iio_event_monitor.c | 2 ++ > 4 files changed, 11 insertions(+) > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio > index 89943c2d54e8..d5a2f93bd051 100644 > --- a/Documentation/ABI/testing/sysfs-bus-iio > +++ b/Documentation/ABI/testing/sysfs-bus-iio > @@ -2339,3 +2339,10 @@ KernelVersion: 6.10 > Contact: linux-iio@vger.kernel.org > Description: > The value of current sense resistor in Ohms. > + > +What: /sys/.../iio:deviceX/in_attention_raw > +KernelVersion: 6.13 > +Contact: linux-iio@vger.kernel.org > +Description: > + Boolean value representing the user's attention to the system. > + This usually means if the user is looking at the screen or not. Hmm. I should have thought of this when I replied to suggest a new channel type. The question is 'units' for a decision. Last time we hit something like this where processing is used to make a decision we decided to at least allow for the concept of 'certainty'. The idea being that smarter sensors would tell us something about how sure they are that the attention is on the device. The analogy being with activity detection. See in_activity_walking_input in Documentation/ABI/testing/sysfs-bus-iio Do you think that would be appropriate here as well? For this device it would take the values 0 and 100 rather than 0 and 1. > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c > index 6a6568d4a2cb..bdfb51275b68 100644 > --- a/drivers/iio/industrialio-core.c > +++ b/drivers/iio/industrialio-core.c > @@ -95,6 +95,7 @@ static const char * const iio_chan_type_name_spec[] = { > [IIO_DELTA_VELOCITY] = "deltavelocity", > [IIO_COLORTEMP] = "colortemp", > [IIO_CHROMATICITY] = "chromaticity", > + [IIO_ATTENTION] = "attention", > }; > > static const char * const iio_modifier_names[] = { > diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h > index f2e0b2d50e6b..12886d4465e4 100644 > --- a/include/uapi/linux/iio/types.h > +++ b/include/uapi/linux/iio/types.h > @@ -51,6 +51,7 @@ enum iio_chan_type { > IIO_DELTA_VELOCITY, > IIO_COLORTEMP, > IIO_CHROMATICITY, > + IIO_ATTENTION, > }; > > enum iio_modifier { > diff --git a/tools/iio/iio_event_monitor.c b/tools/iio/iio_event_monitor.c > index 8073c9e4fe46..ed9a677f1028 100644 > --- a/tools/iio/iio_event_monitor.c > +++ b/tools/iio/iio_event_monitor.c > @@ -63,6 +63,7 @@ static const char * const iio_chan_type_name_spec[] = { > [IIO_DELTA_VELOCITY] = "deltavelocity", > [IIO_COLORTEMP] = "colortemp", > [IIO_CHROMATICITY] = "chromaticity", > + [IIO_ATTENTION] = "attention", > }; > > static const char * const iio_ev_type_text[] = { > @@ -183,6 +184,7 @@ static bool event_is_known(struct iio_event_data *event) > case IIO_DELTA_VELOCITY: > case IIO_COLORTEMP: > case IIO_CHROMATICITY: > + case IIO_ATTENTION: > break; > default: > return false; >
Hi Jonathan On Mon, 28 Oct 2024 at 21:34, Jonathan Cameron <jic23@kernel.org> wrote: > > On Mon, 28 Oct 2024 10:12:23 +0000 > Ricardo Ribalda <ribalda@chromium.org> wrote: > > > Add a new channel type representing if the user's attention state to the > > the system. This usually means if the user is looking at the screen or > > not. > > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > > --- > > Documentation/ABI/testing/sysfs-bus-iio | 7 +++++++ > > drivers/iio/industrialio-core.c | 1 + > > include/uapi/linux/iio/types.h | 1 + > > tools/iio/iio_event_monitor.c | 2 ++ > > 4 files changed, 11 insertions(+) > > > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio > > index 89943c2d54e8..d5a2f93bd051 100644 > > --- a/Documentation/ABI/testing/sysfs-bus-iio > > +++ b/Documentation/ABI/testing/sysfs-bus-iio > > @@ -2339,3 +2339,10 @@ KernelVersion: 6.10 > > Contact: linux-iio@vger.kernel.org > > Description: > > The value of current sense resistor in Ohms. > > + > > +What: /sys/.../iio:deviceX/in_attention_raw > > +KernelVersion: 6.13 > > +Contact: linux-iio@vger.kernel.org > > +Description: > > + Boolean value representing the user's attention to the system. > > + This usually means if the user is looking at the screen or not. > > Hmm. I should have thought of this when I replied to suggest a new channel type. > The question is 'units' for a decision. > > Last time we hit something like this where processing is used to make a decision > we decided to at least allow for the concept of 'certainty'. > > The idea being that smarter sensors would tell us something about how sure they > are that the attention is on the device. > The analogy being with activity detection. See in_activity_walking_input > in Documentation/ABI/testing/sysfs-bus-iio > > Do you think that would be appropriate here as well? For this device > it would take the values 0 and 100 rather than 0 and 1. For the particular device that I want to support, they are giving me a value of 1 and 0, and the example from usb.org seems to work the same way (Logical Maximum of 1) https://www.usb.org/sites/default/files/hutrr107-humanpresenceattention_1.pdf I have no problem multiplying my value by 100 if you think there will be a use case for that. It will not have a major performance impact on the driver. You decide ;) > > > > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c > > index 6a6568d4a2cb..bdfb51275b68 100644 > > --- a/drivers/iio/industrialio-core.c > > +++ b/drivers/iio/industrialio-core.c > > @@ -95,6 +95,7 @@ static const char * const iio_chan_type_name_spec[] = { > > [IIO_DELTA_VELOCITY] = "deltavelocity", > > [IIO_COLORTEMP] = "colortemp", > > [IIO_CHROMATICITY] = "chromaticity", > > + [IIO_ATTENTION] = "attention", > > }; > > > > static const char * const iio_modifier_names[] = { > > diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h > > index f2e0b2d50e6b..12886d4465e4 100644 > > --- a/include/uapi/linux/iio/types.h > > +++ b/include/uapi/linux/iio/types.h > > @@ -51,6 +51,7 @@ enum iio_chan_type { > > IIO_DELTA_VELOCITY, > > IIO_COLORTEMP, > > IIO_CHROMATICITY, > > + IIO_ATTENTION, > > }; > > > > enum iio_modifier { > > diff --git a/tools/iio/iio_event_monitor.c b/tools/iio/iio_event_monitor.c > > index 8073c9e4fe46..ed9a677f1028 100644 > > --- a/tools/iio/iio_event_monitor.c > > +++ b/tools/iio/iio_event_monitor.c > > @@ -63,6 +63,7 @@ static const char * const iio_chan_type_name_spec[] = { > > [IIO_DELTA_VELOCITY] = "deltavelocity", > > [IIO_COLORTEMP] = "colortemp", > > [IIO_CHROMATICITY] = "chromaticity", > > + [IIO_ATTENTION] = "attention", > > }; > > > > static const char * const iio_ev_type_text[] = { > > @@ -183,6 +184,7 @@ static bool event_is_known(struct iio_event_data *event) > > case IIO_DELTA_VELOCITY: > > case IIO_COLORTEMP: > > case IIO_CHROMATICITY: > > + case IIO_ATTENTION: > > break; > > default: > > return false; > > >
On Tue, 29 Oct 2024 13:20:06 +0100 Ricardo Ribalda <ribalda@chromium.org> wrote: > Hi Jonathan > > On Mon, 28 Oct 2024 at 21:34, Jonathan Cameron <jic23@kernel.org> wrote: > > > > On Mon, 28 Oct 2024 10:12:23 +0000 > > Ricardo Ribalda <ribalda@chromium.org> wrote: > > > > > Add a new channel type representing if the user's attention state to the > > > the system. This usually means if the user is looking at the screen or > > > not. > > > > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > > > --- > > > Documentation/ABI/testing/sysfs-bus-iio | 7 +++++++ > > > drivers/iio/industrialio-core.c | 1 + > > > include/uapi/linux/iio/types.h | 1 + > > > tools/iio/iio_event_monitor.c | 2 ++ > > > 4 files changed, 11 insertions(+) > > > > > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio > > > index 89943c2d54e8..d5a2f93bd051 100644 > > > --- a/Documentation/ABI/testing/sysfs-bus-iio > > > +++ b/Documentation/ABI/testing/sysfs-bus-iio > > > @@ -2339,3 +2339,10 @@ KernelVersion: 6.10 > > > Contact: linux-iio@vger.kernel.org > > > Description: > > > The value of current sense resistor in Ohms. > > > + > > > +What: /sys/.../iio:deviceX/in_attention_raw > > > +KernelVersion: 6.13 > > > +Contact: linux-iio@vger.kernel.org > > > +Description: > > > + Boolean value representing the user's attention to the system. > > > + This usually means if the user is looking at the screen or not. > > > > Hmm. I should have thought of this when I replied to suggest a new channel type. > > The question is 'units' for a decision. > > > > Last time we hit something like this where processing is used to make a decision > > we decided to at least allow for the concept of 'certainty'. > > > > The idea being that smarter sensors would tell us something about how sure they > > are that the attention is on the device. > > The analogy being with activity detection. See in_activity_walking_input > > in Documentation/ABI/testing/sysfs-bus-iio > > > > Do you think that would be appropriate here as well? For this device > > it would take the values 0 and 100 rather than 0 and 1. > > For the particular device that I want to support, they are giving me a > value of 1 and 0, and the example from usb.org seems to work the same > way (Logical Maximum of 1) > https://www.usb.org/sites/default/files/hutrr107-humanpresenceattention_1.pdf > > I have no problem multiplying my value by 100 if you think there will > be a use case for that. It will not have a major performance impact on > the driver. Same was true (0 or 1) for the activity classification but I'm not keen on certainty :) So lets' copy that precedence and *100 > You decide ;) > > > > > > > > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c > > > index 6a6568d4a2cb..bdfb51275b68 100644 > > > --- a/drivers/iio/industrialio-core.c > > > +++ b/drivers/iio/industrialio-core.c > > > @@ -95,6 +95,7 @@ static const char * const iio_chan_type_name_spec[] = { > > > [IIO_DELTA_VELOCITY] = "deltavelocity", > > > [IIO_COLORTEMP] = "colortemp", > > > [IIO_CHROMATICITY] = "chromaticity", > > > + [IIO_ATTENTION] = "attention", > > > }; > > > > > > static const char * const iio_modifier_names[] = { > > > diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h > > > index f2e0b2d50e6b..12886d4465e4 100644 > > > --- a/include/uapi/linux/iio/types.h > > > +++ b/include/uapi/linux/iio/types.h > > > @@ -51,6 +51,7 @@ enum iio_chan_type { > > > IIO_DELTA_VELOCITY, > > > IIO_COLORTEMP, > > > IIO_CHROMATICITY, > > > + IIO_ATTENTION, > > > }; > > > > > > enum iio_modifier { > > > diff --git a/tools/iio/iio_event_monitor.c b/tools/iio/iio_event_monitor.c > > > index 8073c9e4fe46..ed9a677f1028 100644 > > > --- a/tools/iio/iio_event_monitor.c > > > +++ b/tools/iio/iio_event_monitor.c > > > @@ -63,6 +63,7 @@ static const char * const iio_chan_type_name_spec[] = { > > > [IIO_DELTA_VELOCITY] = "deltavelocity", > > > [IIO_COLORTEMP] = "colortemp", > > > [IIO_CHROMATICITY] = "chromaticity", > > > + [IIO_ATTENTION] = "attention", > > > }; > > > > > > static const char * const iio_ev_type_text[] = { > > > @@ -183,6 +184,7 @@ static bool event_is_known(struct iio_event_data *event) > > > case IIO_DELTA_VELOCITY: > > > case IIO_COLORTEMP: > > > case IIO_CHROMATICITY: > > > + case IIO_ATTENTION: > > > break; > > > default: > > > return false; > > > > > > >
On 10/29/24 9:38 AM, Jonathan Cameron wrote: > On Tue, 29 Oct 2024 13:20:06 +0100 > Ricardo Ribalda <ribalda@chromium.org> wrote: > >> Hi Jonathan >> >> On Mon, 28 Oct 2024 at 21:34, Jonathan Cameron <jic23@kernel.org> wrote: >>> >>> On Mon, 28 Oct 2024 10:12:23 +0000 >>> Ricardo Ribalda <ribalda@chromium.org> wrote: >>> >>>> Add a new channel type representing if the user's attention state to the >>>> the system. This usually means if the user is looking at the screen or >>>> not. >>>> >>>> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> >>>> --- >>>> Documentation/ABI/testing/sysfs-bus-iio | 7 +++++++ >>>> drivers/iio/industrialio-core.c | 1 + >>>> include/uapi/linux/iio/types.h | 1 + >>>> tools/iio/iio_event_monitor.c | 2 ++ >>>> 4 files changed, 11 insertions(+) >>>> >>>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio >>>> index 89943c2d54e8..d5a2f93bd051 100644 >>>> --- a/Documentation/ABI/testing/sysfs-bus-iio >>>> +++ b/Documentation/ABI/testing/sysfs-bus-iio >>>> @@ -2339,3 +2339,10 @@ KernelVersion: 6.10 >>>> Contact: linux-iio@vger.kernel.org >>>> Description: >>>> The value of current sense resistor in Ohms. >>>> + >>>> +What: /sys/.../iio:deviceX/in_attention_raw >>>> +KernelVersion: 6.13 >>>> +Contact: linux-iio@vger.kernel.org >>>> +Description: >>>> + Boolean value representing the user's attention to the system. >>>> + This usually means if the user is looking at the screen or not. >>> >>> Hmm. I should have thought of this when I replied to suggest a new channel type. >>> The question is 'units' for a decision. >>> >>> Last time we hit something like this where processing is used to make a decision >>> we decided to at least allow for the concept of 'certainty'. >>> >>> The idea being that smarter sensors would tell us something about how sure they >>> are that the attention is on the device. >>> The analogy being with activity detection. See in_activity_walking_input >>> in Documentation/ABI/testing/sysfs-bus-iio >>> >>> Do you think that would be appropriate here as well? For this device >>> it would take the values 0 and 100 rather than 0 and 1. >> >> For the particular device that I want to support, they are giving me a >> value of 1 and 0, and the example from usb.org seems to work the same >> way (Logical Maximum of 1) >> https://www.usb.org/sites/default/files/hutrr107-humanpresenceattention_1.pdf >> >> I have no problem multiplying my value by 100 if you think there will >> be a use case for that. It will not have a major performance impact on >> the driver. > Same was true (0 or 1) for the activity classification but I'm not > keen on certainty :) So lets' copy that precedence and *100 > > And I assume we would want this to be in_attention_input (processed), not in_attention_raw.
diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio index 89943c2d54e8..d5a2f93bd051 100644 --- a/Documentation/ABI/testing/sysfs-bus-iio +++ b/Documentation/ABI/testing/sysfs-bus-iio @@ -2339,3 +2339,10 @@ KernelVersion: 6.10 Contact: linux-iio@vger.kernel.org Description: The value of current sense resistor in Ohms. + +What: /sys/.../iio:deviceX/in_attention_raw +KernelVersion: 6.13 +Contact: linux-iio@vger.kernel.org +Description: + Boolean value representing the user's attention to the system. + This usually means if the user is looking at the screen or not. diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c index 6a6568d4a2cb..bdfb51275b68 100644 --- a/drivers/iio/industrialio-core.c +++ b/drivers/iio/industrialio-core.c @@ -95,6 +95,7 @@ static const char * const iio_chan_type_name_spec[] = { [IIO_DELTA_VELOCITY] = "deltavelocity", [IIO_COLORTEMP] = "colortemp", [IIO_CHROMATICITY] = "chromaticity", + [IIO_ATTENTION] = "attention", }; static const char * const iio_modifier_names[] = { diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h index f2e0b2d50e6b..12886d4465e4 100644 --- a/include/uapi/linux/iio/types.h +++ b/include/uapi/linux/iio/types.h @@ -51,6 +51,7 @@ enum iio_chan_type { IIO_DELTA_VELOCITY, IIO_COLORTEMP, IIO_CHROMATICITY, + IIO_ATTENTION, }; enum iio_modifier { diff --git a/tools/iio/iio_event_monitor.c b/tools/iio/iio_event_monitor.c index 8073c9e4fe46..ed9a677f1028 100644 --- a/tools/iio/iio_event_monitor.c +++ b/tools/iio/iio_event_monitor.c @@ -63,6 +63,7 @@ static const char * const iio_chan_type_name_spec[] = { [IIO_DELTA_VELOCITY] = "deltavelocity", [IIO_COLORTEMP] = "colortemp", [IIO_CHROMATICITY] = "chromaticity", + [IIO_ATTENTION] = "attention", }; static const char * const iio_ev_type_text[] = { @@ -183,6 +184,7 @@ static bool event_is_known(struct iio_event_data *event) case IIO_DELTA_VELOCITY: case IIO_COLORTEMP: case IIO_CHROMATICITY: + case IIO_ATTENTION: break; default: return false;
Add a new channel type representing if the user's attention state to the the system. This usually means if the user is looking at the screen or not. Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- Documentation/ABI/testing/sysfs-bus-iio | 7 +++++++ drivers/iio/industrialio-core.c | 1 + include/uapi/linux/iio/types.h | 1 + tools/iio/iio_event_monitor.c | 2 ++ 4 files changed, 11 insertions(+)