Linux-IIO Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] iio: imu: st_lsm6dsx: make IIO_CHAN_INFO_SCALE shared by type
@ 2019-08-01 14:39 Martin Kepplinger
  2019-08-05 14:21 ` Jonathan Cameron
  0 siblings, 1 reply; 4+ messages in thread
From: Martin Kepplinger @ 2019-08-01 14:39 UTC (permalink / raw)
  To: lorenzo.bianconi83, jic23, knaack.h, lars, pmeerw
  Cc: linux-iio, linux-kernel, Martin Kepplinger

in_accel_x_scale, in_accel_y_scale and in_accel_z_scale are always
the same. The scale is still defined to be in "info_mask_separate".

Userspace (iio-sensor-proxy and others) is not used to that and only
looks for "in_accel_scale" for the scaling factor to apply.

Change IIO_CHAN_INFO_SCALE from being separate in all channel to be
shared by type.

This removes in_accel_x_scale, in_accel_y_scale and in_accel_z_scale and
makes available in_accel_scale.

Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>
---

AFAIK in all other drivers, IIO_CHAN_INFO_SCALE is "shared by type". Sure
devices are different, but LSM6DSX devices still don't have different
scales for x/y/z channels :)

thanks,

                              martin



 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
index af379a5429ed..59c3ab7cbb6f 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
@@ -56,8 +56,8 @@ enum st_lsm6dsx_hw_id {
 	.address = addr,						\
 	.modified = 1,							\
 	.channel2 = mod,						\
-	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |			\
-			      BIT(IIO_CHAN_INFO_SCALE),			\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),		\
 	.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
 	.scan_index = scan_idx,						\
 	.scan_type = {							\
-- 
2.20.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] iio: imu: st_lsm6dsx: make IIO_CHAN_INFO_SCALE shared by type
  2019-08-01 14:39 [PATCH] iio: imu: st_lsm6dsx: make IIO_CHAN_INFO_SCALE shared by type Martin Kepplinger
@ 2019-08-05 14:21 ` Jonathan Cameron
  2019-08-05 15:04   ` Lorenzo Bianconi
  0 siblings, 1 reply; 4+ messages in thread
From: Jonathan Cameron @ 2019-08-05 14:21 UTC (permalink / raw)
  To: Martin Kepplinger
  Cc: lorenzo.bianconi83, knaack.h, lars, pmeerw, linux-iio, linux-kernel

On Thu,  1 Aug 2019 16:39:08 +0200
Martin Kepplinger <martin.kepplinger@puri.sm> wrote:

> in_accel_x_scale, in_accel_y_scale and in_accel_z_scale are always
> the same. The scale is still defined to be in "info_mask_separate".
> 
> Userspace (iio-sensor-proxy and others) is not used to that and only
> looks for "in_accel_scale" for the scaling factor to apply.
> 
> Change IIO_CHAN_INFO_SCALE from being separate in all channel to be
> shared by type.
> 
> This removes in_accel_x_scale, in_accel_y_scale and in_accel_z_scale and
> makes available in_accel_scale.
> 
> Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>
> ---
> 
> AFAIK in all other drivers, IIO_CHAN_INFO_SCALE is "shared by type". Sure
> devices are different, but LSM6DSX devices still don't have different
> scales for x/y/z channels :)

I'm fine with this, but would like a Lorenzo ack as we have had
devices in other series where these are not equal.   It used to
be common in accelerometers as I think it was hard to get a large
range in the vertical direction.  Doubt that applies on these modern
parts though!

Thanks,

Jonathan


> 
> thanks,
> 
>                               martin
> 
> 
> 
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> index af379a5429ed..59c3ab7cbb6f 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> @@ -56,8 +56,8 @@ enum st_lsm6dsx_hw_id {
>  	.address = addr,						\
>  	.modified = 1,							\
>  	.channel2 = mod,						\
> -	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |			\
> -			      BIT(IIO_CHAN_INFO_SCALE),			\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),		\
>  	.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
>  	.scan_index = scan_idx,						\
>  	.scan_type = {							\


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] iio: imu: st_lsm6dsx: make IIO_CHAN_INFO_SCALE shared by type
  2019-08-05 14:21 ` Jonathan Cameron
@ 2019-08-05 15:04   ` Lorenzo Bianconi
  2019-08-05 15:41     ` Jonathan Cameron
  0 siblings, 1 reply; 4+ messages in thread
From: Lorenzo Bianconi @ 2019-08-05 15:04 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Martin Kepplinger, lorenzo.bianconi83, knaack.h, lars, pmeerw,
	linux-iio, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2460 bytes --]

> On Thu,  1 Aug 2019 16:39:08 +0200
> Martin Kepplinger <martin.kepplinger@puri.sm> wrote:
> 
> > in_accel_x_scale, in_accel_y_scale and in_accel_z_scale are always
> > the same. The scale is still defined to be in "info_mask_separate".
> > 
> > Userspace (iio-sensor-proxy and others) is not used to that and only
> > looks for "in_accel_scale" for the scaling factor to apply.
> > 
> > Change IIO_CHAN_INFO_SCALE from being separate in all channel to be
> > shared by type.
> > 
> > This removes in_accel_x_scale, in_accel_y_scale and in_accel_z_scale and
> > makes available in_accel_scale.
> > 
> > Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>
> > ---
> > 
> > AFAIK in all other drivers, IIO_CHAN_INFO_SCALE is "shared by type". Sure
> > devices are different, but LSM6DSX devices still don't have different
> > scales for x/y/z channels :)
> 
> I'm fine with this, but would like a Lorenzo ack as we have had
> devices in other series where these are not equal.   It used to
> be common in accelerometers as I think it was hard to get a large
> range in the vertical direction.  Doubt that applies on these modern
> parts though!
> 
> Thanks,
> 
> Jonathan

AFAIK all the supported sensors have the same sensitivity on all axis and so I
am fine with this patch (it should be done in this way from day 0 actually :))
but is it going to break uapi? if not feel free to add:

Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>

Regards,
Lorenzo

> 
> 
> > 
> > thanks,
> > 
> >                               martin
> > 
> > 
> > 
> >  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> > index af379a5429ed..59c3ab7cbb6f 100644
> > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> > @@ -56,8 +56,8 @@ enum st_lsm6dsx_hw_id {
> >  	.address = addr,						\
> >  	.modified = 1,							\
> >  	.channel2 = mod,						\
> > -	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |			\
> > -			      BIT(IIO_CHAN_INFO_SCALE),			\
> > +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
> > +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),		\
> >  	.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
> >  	.scan_index = scan_idx,						\
> >  	.scan_type = {							\
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] iio: imu: st_lsm6dsx: make IIO_CHAN_INFO_SCALE shared by type
  2019-08-05 15:04   ` Lorenzo Bianconi
@ 2019-08-05 15:41     ` Jonathan Cameron
  0 siblings, 0 replies; 4+ messages in thread
From: Jonathan Cameron @ 2019-08-05 15:41 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: Martin Kepplinger, lorenzo.bianconi83, knaack.h, lars, pmeerw,
	linux-iio, linux-kernel

On Mon, 5 Aug 2019 17:04:35 +0200
Lorenzo Bianconi <lorenzo@kernel.org> wrote:

> > On Thu,  1 Aug 2019 16:39:08 +0200
> > Martin Kepplinger <martin.kepplinger@puri.sm> wrote:
> >   
> > > in_accel_x_scale, in_accel_y_scale and in_accel_z_scale are always
> > > the same. The scale is still defined to be in "info_mask_separate".
> > > 
> > > Userspace (iio-sensor-proxy and others) is not used to that and only
> > > looks for "in_accel_scale" for the scaling factor to apply.
> > > 
> > > Change IIO_CHAN_INFO_SCALE from being separate in all channel to be
> > > shared by type.
> > > 
> > > This removes in_accel_x_scale, in_accel_y_scale and in_accel_z_scale and
> > > makes available in_accel_scale.
> > > 
> > > Signed-off-by: Martin Kepplinger <martin.kepplinger@puri.sm>
> > > ---
> > > 
> > > AFAIK in all other drivers, IIO_CHAN_INFO_SCALE is "shared by type". Sure
> > > devices are different, but LSM6DSX devices still don't have different
> > > scales for x/y/z channels :)  
> > 
> > I'm fine with this, but would like a Lorenzo ack as we have had
> > devices in other series where these are not equal.   It used to
> > be common in accelerometers as I think it was hard to get a large
> > range in the vertical direction.  Doubt that applies on these modern
> > parts though!
> > 
> > Thanks,
> > 
> > Jonathan  
> 
> AFAIK all the supported sensors have the same sensitivity on all axis and so I
> am fine with this patch (it should be done in this way from day 0 actually :))
> but is it going to break uapi? if not feel free to add:
It's a gamble, but one we've made a number of times before when tidying this
sort of thing up.

Most code uses libraries that are capable of coping with both options so chances
are fairly low that anyone will notice.  It seems some code doesn't cope
with independent versions and will actually be fixed by this.

I'm not proposing to push this as a fix, but as a tidy up.
We may break a few custom scripts built against particular setups though.

I think it's a worthwhile enough tidy up to take the risk, particularly
if it fixes the interactions with some code bases (that incidentally
should have coped with this either way).

*crosses fingers*

Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.

Thanks,

Jonathan

> 
> Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>
> 
> Regards,
> Lorenzo
> 
> > 
> >   
> > > 
> > > thanks,
> > > 
> > >                               martin
> > > 
> > > 
> > > 
> > >  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> > > index af379a5429ed..59c3ab7cbb6f 100644
> > > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> > > @@ -56,8 +56,8 @@ enum st_lsm6dsx_hw_id {
> > >  	.address = addr,						\
> > >  	.modified = 1,							\
> > >  	.channel2 = mod,						\
> > > -	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |			\
> > > -			      BIT(IIO_CHAN_INFO_SCALE),			\
> > > +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),			\
> > > +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),		\
> > >  	.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
> > >  	.scan_index = scan_idx,						\
> > >  	.scan_type = {							\  
> >   


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-01 14:39 [PATCH] iio: imu: st_lsm6dsx: make IIO_CHAN_INFO_SCALE shared by type Martin Kepplinger
2019-08-05 14:21 ` Jonathan Cameron
2019-08-05 15:04   ` Lorenzo Bianconi
2019-08-05 15:41     ` Jonathan Cameron

Linux-IIO Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-iio/0 linux-iio/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-iio linux-iio/ https://lore.kernel.org/linux-iio \
		linux-iio@vger.kernel.org linux-iio@archiver.kernel.org
	public-inbox-index linux-iio


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-iio


AGPL code for this site: git clone https://public-inbox.org/ public-inbox