| Message ID | 1483456430-6980-1-git-send-email-hongyan.song@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> Add new channel types support for linear accel sensor. > > Linear acceleration differs from a standard accelerometor, > its value depends on standard accel sensor and gravity sensor. > Conceptually, this three sensors have following relationship: > linear acceleration = acceleration - acceleration due to gravity > > At rest, standard accelerometer displays 1g due to earth’s > gravitational pull while a liner accelerometer will show 0g. comment below > More information can be found in: > http://www.usb.org/developers/hidpage/HUTRR59_-_Usages_for_Wearables.pdf > > Signed-off-by: Song Hongyan <hongyan.song@intel.com> > --- > Documentation/ABI/testing/sysfs-bus-iio | 10 ++++++++++ > drivers/iio/industrialio-core.c | 1 + > include/uapi/linux/iio/types.h | 1 + > tools/iio/iio_event_monitor.c | 2 ++ > 4 files changed, 14 insertions(+) > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio > index 60b7406..8091f3d 100644 > --- a/Documentation/ABI/testing/sysfs-bus-iio > +++ b/Documentation/ABI/testing/sysfs-bus-iio > @@ -170,6 +170,16 @@ Description: > Has all of the equivalent parameters as per voltageY. Units > after application of scale and offset are m/s^2. > > +What: /sys/bus/iio/devices/iio:deviceX/in_linear_accel_x_raw > +What: /sys/bus/iio/devices/iio:deviceX/in_linear_accel_y_raw > +What: /sys/bus/iio/devices/iio:deviceX/in_linear_accel_z_raw I think this would be in_linearaccel_x_raw, etc.? > +KernelVersion: 4.11 > +Contact: linux-iio@vger.kernel.org > +Description: > + Linear Acceleration in direction x, y or z (may be arbitrarily assigned > + but should match other such assignments on device). > + Has all of the equivalent parameters as per voltageY. Units > + after application of scale and offset are m/s^2. > > What: /sys/bus/iio/devices/iio:deviceX/in_gravity_x_raw > What: /sys/bus/iio/devices/iio:deviceX/in_gravity_y_raw > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c > index 72fc96a..da78c26 100644 > --- a/drivers/iio/industrialio-core.c > +++ b/drivers/iio/industrialio-core.c > @@ -82,6 +82,7 @@ struct bus_type iio_bus_type = { > [IIO_UVINDEX] = "uvindex", > [IIO_ELECTRICALCONDUCTIVITY] = "electricalconductivity", > [IIO_GRAVITY] = "gravity", > + [IIO_LINEAR_ACCEL] = "linearaccel", > }; > > static const char * const iio_modifier_names[] = { > diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h > index d3f7ba7..d6df101 100644 > --- a/include/uapi/linux/iio/types.h > +++ b/include/uapi/linux/iio/types.h > @@ -41,6 +41,7 @@ enum iio_chan_type { > IIO_UVINDEX, > IIO_ELECTRICALCONDUCTIVITY, > IIO_GRAVITY, > + IIO_LINEAR_ACCEL, > }; > > enum iio_modifier { > diff --git a/tools/iio/iio_event_monitor.c b/tools/iio/iio_event_monitor.c > index b61245e..116644a 100644 > --- a/tools/iio/iio_event_monitor.c > +++ b/tools/iio/iio_event_monitor.c > @@ -58,6 +58,7 @@ > [IIO_PH] = "ph", > [IIO_UVINDEX] = "uvindex", > [IIO_GRAVITY] = "gravity", > + [IIO_LINEAR_ACCEL] = "linearaccel", > }; > > static const char * const iio_ev_type_text[] = { > @@ -151,6 +152,7 @@ static bool event_is_known(struct iio_event_data *event) > case IIO_PH: > case IIO_UVINDEX: > case IIO_GRAVITY: > + case IIO_LINEAR_ACCEL: > break; > default: > return false; >
Hi peter, I checked and find that iio device name is usually named to be with underline if it has more than one word, so I assigned to be " linear_accel_3d". I think the device property node named to be "in_linear_accel_x_raw" is better than " in_linearaccel_x_raw" Just follow the former coding style. "linearaccel" is only a name which specify IIO type [IIO_LINEAR_ACCEL] I name [IIO_LINEAR_ACCEL] to be "linearaccel" just follow the history code style in " iio_chan_type_name_spec[]" which do not have any underline. What do you think? BR Song Hongyan -----Original Message----- From: Peter Meerwald-Stadler [mailto:pmeerw@pmeerw.net] Sent: Tuesday, January 3, 2017 3:23 PM To: Song, Hongyan <hongyan.song@intel.com> Cc: linux-input@vger.kernel.org; linux-iio@vger.kernel.org; jikos@kernel.org; jic23@kernel.org; Pandruvada, Srinivas <srinivas.pandruvada@intel.com> Subject: Re: [PATCH 1/2] iio: Add support for linear accel > Add new channel types support for linear accel sensor. > > Linear acceleration differs from a standard accelerometor, its value > depends on standard accel sensor and gravity sensor. > Conceptually, this three sensors have following relationship: > linear acceleration = acceleration - acceleration due to gravity > > At rest, standard accelerometer displays 1g due to earth’s > gravitational pull while a liner accelerometer will show 0g. comment below > More information can be found in: > http://www.usb.org/developers/hidpage/HUTRR59_-_Usages_for_Wearables.p > df > > Signed-off-by: Song Hongyan <hongyan.song@intel.com> > --- > Documentation/ABI/testing/sysfs-bus-iio | 10 ++++++++++ > drivers/iio/industrialio-core.c | 1 + > include/uapi/linux/iio/types.h | 1 + > tools/iio/iio_event_monitor.c | 2 ++ > 4 files changed, 14 insertions(+) > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio > b/Documentation/ABI/testing/sysfs-bus-iio > index 60b7406..8091f3d 100644 > --- a/Documentation/ABI/testing/sysfs-bus-iio > +++ b/Documentation/ABI/testing/sysfs-bus-iio > @@ -170,6 +170,16 @@ Description: > Has all of the equivalent parameters as per voltageY. Units > after application of scale and offset are m/s^2. > > +What: /sys/bus/iio/devices/iio:deviceX/in_linear_accel_x_raw > +What: /sys/bus/iio/devices/iio:deviceX/in_linear_accel_y_raw > +What: /sys/bus/iio/devices/iio:deviceX/in_linear_accel_z_raw I think this would be in_linearaccel_x_raw, etc.? > +KernelVersion: 4.11 > +Contact: linux-iio@vger.kernel.org > +Description: > + Linear Acceleration in direction x, y or z (may be arbitrarily assigned > + but should match other such assignments on device). > + Has all of the equivalent parameters as per voltageY. Units > + after application of scale and offset are m/s^2. > > What: /sys/bus/iio/devices/iio:deviceX/in_gravity_x_raw > What: /sys/bus/iio/devices/iio:deviceX/in_gravity_y_raw > diff --git a/drivers/iio/industrialio-core.c > b/drivers/iio/industrialio-core.c index 72fc96a..da78c26 100644 > --- a/drivers/iio/industrialio-core.c > +++ b/drivers/iio/industrialio-core.c > @@ -82,6 +82,7 @@ struct bus_type iio_bus_type = { > [IIO_UVINDEX] = "uvindex", > [IIO_ELECTRICALCONDUCTIVITY] = "electricalconductivity", > [IIO_GRAVITY] = "gravity", > + [IIO_LINEAR_ACCEL] = "linearaccel", > }; > > static const char * const iio_modifier_names[] = { diff --git > a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h > index d3f7ba7..d6df101 100644 > --- a/include/uapi/linux/iio/types.h > +++ b/include/uapi/linux/iio/types.h > @@ -41,6 +41,7 @@ enum iio_chan_type { > IIO_UVINDEX, > IIO_ELECTRICALCONDUCTIVITY, > IIO_GRAVITY, > + IIO_LINEAR_ACCEL, > }; > > enum iio_modifier { > diff --git a/tools/iio/iio_event_monitor.c > b/tools/iio/iio_event_monitor.c index b61245e..116644a 100644 > --- a/tools/iio/iio_event_monitor.c > +++ b/tools/iio/iio_event_monitor.c > @@ -58,6 +58,7 @@ > [IIO_PH] = "ph", > [IIO_UVINDEX] = "uvindex", > [IIO_GRAVITY] = "gravity", > + [IIO_LINEAR_ACCEL] = "linearaccel", > }; > > static const char * const iio_ev_type_text[] = { @@ -151,6 +152,7 @@ > static bool event_is_known(struct iio_event_data *event) > case IIO_PH: > case IIO_UVINDEX: > case IIO_GRAVITY: > + case IIO_LINEAR_ACCEL: > break; > default: > return false; > -- Peter Meerwald-Stadler +43-664-2444418 (mobile)
Hi, > I checked and find that iio device name is usually named to be with underline if it has more than one word, so I assigned to be " linear_accel_3d". > I think the device property node named to be "in_linear_accel_x_raw" is better than " in_linearaccel_x_raw" Just follow the former coding style. > "linearaccel" is only a name which specify IIO type [IIO_LINEAR_ACCEL] > I name [IIO_LINEAR_ACCEL] to be "linearaccel" just follow the history code style in " iio_chan_type_name_spec[]" which do not have any underline. > > +What: /sys/bus/iio/devices/iio:deviceX/in_linear_accel_x_raw > > +What: /sys/bus/iio/devices/iio:deviceX/in_linear_accel_y_raw > > +What: /sys/bus/iio/devices/iio:deviceX/in_linear_accel_z_raw > > I think this would be in_linearaccel_x_raw, etc.? > > @@ -82,6 +82,7 @@ struct bus_type iio_bus_type = { > > [IIO_UVINDEX] = "uvindex", > > [IIO_ELECTRICALCONDUCTIVITY] = "electricalconductivity", > > [IIO_GRAVITY] = "gravity", > > + [IIO_LINEAR_ACCEL] = "linearaccel", > > }; linearaccel vs. linear_accel is a matter of taste, I have no preference; nevertheless, the documentation should match what IIO outputs if you have "linearaccel" in iio_bus_type, I think the name of a LINEAR_ACCEL channel will end up as "in_linearaccel_x_raw" and must be documented like that -- i.e. there is a mismatch between documentation and actual code/output probably your patches should be rebased, iio/testing branch has IIO_COUNT and IIO_INDEX channels regards, p.
On 01/03/2017 09:04 AM, Peter Meerwald-Stadler wrote: > Hi, > >> I checked and find that iio device name is usually named to be with underline if it has more than one word, so I assigned to be " linear_accel_3d". >> I think the device property node named to be "in_linear_accel_x_raw" is better than " in_linearaccel_x_raw" Just follow the former coding style. >> "linearaccel" is only a name which specify IIO type [IIO_LINEAR_ACCEL] >> I name [IIO_LINEAR_ACCEL] to be "linearaccel" just follow the history code style in " iio_chan_type_name_spec[]" which do not have any underline. > >>> +What: /sys/bus/iio/devices/iio:deviceX/in_linear_accel_x_raw >>> +What: /sys/bus/iio/devices/iio:deviceX/in_linear_accel_y_raw >>> +What: /sys/bus/iio/devices/iio:deviceX/in_linear_accel_z_raw >> >> I think this would be in_linearaccel_x_raw, etc.? > >>> @@ -82,6 +82,7 @@ struct bus_type iio_bus_type = { >>> [IIO_UVINDEX] = "uvindex", >>> [IIO_ELECTRICALCONDUCTIVITY] = "electricalconductivity", >>> [IIO_GRAVITY] = "gravity", >>> + [IIO_LINEAR_ACCEL] = "linearaccel", >>> }; > > > linearaccel vs. linear_accel is a matter of taste, I have no preference; > nevertheless, the documentation should match what IIO outputs We use underscores as the field separator in the channel name. Having a underscore as part of a field adds ambiguity and makes it a lot harder to machine parse it. It's already complicated enough as it is. We should avoid adding types with underscores in them. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Lars & Peter, -----Original Message----- From: Lars-Peter Clausen [mailto:lars@metafoo.de] Sent: Tuesday, January 3, 2017 5:49 PM To: Peter Meerwald-Stadler <pmeerw@pmeerw.net>; Song, Hongyan <hongyan.song@intel.com> Cc: linux-input@vger.kernel.org; linux-iio@vger.kernel.org; jikos@kernel.org; jic23@kernel.org; Pandruvada, Srinivas <srinivas.pandruvada@intel.com> Subject: Re: [PATCH 1/2] iio: Add support for linear accel On 01/03/2017 09:04 AM, Peter Meerwald-Stadler wrote: > Hi, > >> I checked and find that iio device name is usually named to be with underline if it has more than one word, so I assigned to be " linear_accel_3d". >> I think the device property node named to be "in_linear_accel_x_raw" is better than " in_linearaccel_x_raw" Just follow the former coding style. >> "linearaccel" is only a name which specify IIO type >> [IIO_LINEAR_ACCEL] I name [IIO_LINEAR_ACCEL] to be "linearaccel" just follow the history code style in " iio_chan_type_name_spec[]" which do not have any underline. > >>> +What: /sys/bus/iio/devices/iio:deviceX/in_linear_accel_x_raw >>> +What: /sys/bus/iio/devices/iio:deviceX/in_linear_accel_y_raw >>> +What: /sys/bus/iio/devices/iio:deviceX/in_linear_accel_z_raw >> >> I think this would be in_linearaccel_x_raw, etc.? > >>> @@ -82,6 +82,7 @@ struct bus_type iio_bus_type = { >>> [IIO_UVINDEX] = "uvindex", >>> [IIO_ELECTRICALCONDUCTIVITY] = "electricalconductivity", >>> [IIO_GRAVITY] = "gravity", >>> + [IIO_LINEAR_ACCEL] = "linearaccel", >>> }; > > > >linearaccel vs. linear_accel is a matter of taste, I have no > >preference; nevertheless, the documentation should match what IIO >> outputs >We use underscores as the field separator in the channel name. Having a underscore as part of a field adds ambiguity and makes it a lot > >harder to machine parse it. It's already complicated enough as it is. We should avoid adding types with underscores in them. Thanks for your advice, I will update the patch. BR Song Hongyan -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio index 60b7406..8091f3d 100644 --- a/Documentation/ABI/testing/sysfs-bus-iio +++ b/Documentation/ABI/testing/sysfs-bus-iio @@ -170,6 +170,16 @@ Description: Has all of the equivalent parameters as per voltageY. Units after application of scale and offset are m/s^2. +What: /sys/bus/iio/devices/iio:deviceX/in_linear_accel_x_raw +What: /sys/bus/iio/devices/iio:deviceX/in_linear_accel_y_raw +What: /sys/bus/iio/devices/iio:deviceX/in_linear_accel_z_raw +KernelVersion: 4.11 +Contact: linux-iio@vger.kernel.org +Description: + Linear Acceleration in direction x, y or z (may be arbitrarily assigned + but should match other such assignments on device). + Has all of the equivalent parameters as per voltageY. Units + after application of scale and offset are m/s^2. What: /sys/bus/iio/devices/iio:deviceX/in_gravity_x_raw What: /sys/bus/iio/devices/iio:deviceX/in_gravity_y_raw diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c index 72fc96a..da78c26 100644 --- a/drivers/iio/industrialio-core.c +++ b/drivers/iio/industrialio-core.c @@ -82,6 +82,7 @@ struct bus_type iio_bus_type = { [IIO_UVINDEX] = "uvindex", [IIO_ELECTRICALCONDUCTIVITY] = "electricalconductivity", [IIO_GRAVITY] = "gravity", + [IIO_LINEAR_ACCEL] = "linearaccel", }; static const char * const iio_modifier_names[] = { diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h index d3f7ba7..d6df101 100644 --- a/include/uapi/linux/iio/types.h +++ b/include/uapi/linux/iio/types.h @@ -41,6 +41,7 @@ enum iio_chan_type { IIO_UVINDEX, IIO_ELECTRICALCONDUCTIVITY, IIO_GRAVITY, + IIO_LINEAR_ACCEL, }; enum iio_modifier { diff --git a/tools/iio/iio_event_monitor.c b/tools/iio/iio_event_monitor.c index b61245e..116644a 100644 --- a/tools/iio/iio_event_monitor.c +++ b/tools/iio/iio_event_monitor.c @@ -58,6 +58,7 @@ [IIO_PH] = "ph", [IIO_UVINDEX] = "uvindex", [IIO_GRAVITY] = "gravity", + [IIO_LINEAR_ACCEL] = "linearaccel", }; static const char * const iio_ev_type_text[] = { @@ -151,6 +152,7 @@ static bool event_is_known(struct iio_event_data *event) case IIO_PH: case IIO_UVINDEX: case IIO_GRAVITY: + case IIO_LINEAR_ACCEL: break; default: return false;
Add new channel types support for linear accel sensor. Linear acceleration differs from a standard accelerometor, its value depends on standard accel sensor and gravity sensor. Conceptually, this three sensors have following relationship: linear acceleration = acceleration - acceleration due to gravity At rest, standard accelerometer displays 1g due to earth’s gravitational pull while a liner accelerometer will show 0g. More information can be found in: http://www.usb.org/developers/hidpage/HUTRR59_-_Usages_for_Wearables.pdf Signed-off-by: Song Hongyan <hongyan.song@intel.com> --- Documentation/ABI/testing/sysfs-bus-iio | 10 ++++++++++ drivers/iio/industrialio-core.c | 1 + include/uapi/linux/iio/types.h | 1 + tools/iio/iio_event_monitor.c | 2 ++ 4 files changed, 14 insertions(+)