Re: [PATCH v2] iio: st_sensors: make scale channels also shared by type

[Bastien Nocera <hadess@hadess.net>]

On Mon, 2020-05-04 at 11:22 +0100, Jonathan Cameron wrote:
> On Mon, 4 May 2020 12:02:36 +0200
> Bastien Nocera <hadess@hadess.net> wrote:
> 
> > On Sat, 2020-05-02 at 19:07 +0100, Jonathan Cameron wrote:
> > > On Sun, 26 Apr 2020 13:19:09 +0200
> > > Bastien Nocera <hadess@hadess.net> wrote:
> > >   
> > > > On Sat, 2020-04-25 at 18:13 +0100, Jonathan Cameron wrote:  
> > > > > On Thu, 23 Apr 2020 14:17:15 +0200
> > > > > Gaëtan André <rvlander@gaetanandre.eu> wrote:
> > > > >     
> > > > > > Scale channels are available by axis. For example for
> > > > > > accelerometers,
> > > > > > in_accel_x_scale, in_accel_y_scale and in_accel_z_scale are
> > > > > > available.
> > > > > > 
> > > > > > However, they should be shared by type as documented in
> > > > > > Documentation/ABI/testing/sysfs-bus-iio.
> > > > > > 
> > > > > > For each sensor (acceleros, gyros and magnetos) only one
> > > > > > value
> > > > > > is
> > > > > > specified
> > > > > > for all the axes.
> > > > > > 
> > > > > > Existing, by axis, entries are preserved in order to to
> > > > > > leave
> > > > > > the
> > > > > > old ABI
> > > > > > untouched.    
> > > > > As I mentioned in v1, there isn't a strict ABI rule that says
> > > > > that we
> > > > > must
> > > > > do the shared form
> > > > > 
> > > > > +CC'd Bastien for comment on what userspace is assuming and
> > > > > whether
> > > > > we should
> > > > > push this back to stable or not.    
> > > > 
> > > > I have no idea what the effects of this would be on the ABI,
> > > > and
> > > > how
> > > > this would impact iio-sensor-proxy.  
> > > 
> > > There goes me being lazy ;)
> > >   
> > > > Code is here though, so it might be best to test it:
> > > > https://gitlab.freedesktop.org/hadess/iio-sensor-proxy/-/tree/master/src
> > > > 
> > > > And we accept merge requests :)  
> > > 
> > > Only looks at scale and in_accel_scale
> > > 
> > > Easy enough to fix...
> > > 
> > > Note that for some older accelerometers it has to do per channel
> > > scales btw.
> > > It used to be hard to have the same range out of the plane of the
> > > silicon
> > > than within it, so was common to have sensors with different
> > > ranges
> > > and hence
> > > scales in z direction from x and y.
> > > 
> > > I'll apply the kernel patch but good to fix up iio-sensor-proxy
> > > as
> > > well.
> > > 
> > > I would ideally like Denis to give this a quick sanity check
> > > though
> > > as I'd
> > > like to give it a stable tag and don't want any unexpected
> > > breakage.  
> > 
> > I've made this:
> > https://gitlab.freedesktop.org/hadess/iio-sensor-proxy/-/merge_requests/306
> > which should work just fine for the sensors which have a trigger
> > (the z
> > scaling was the one applied to all 3 axes).
> > 
> > What's the name of the file that contains the x/y/z axis scale
> > information for devices without triggers?
> 
> For both devices with triggers and without, if they are per axis they
> should be
> in_accel_x_scale
> in_accel_y_scale
> in_accel_z_scale
> 
> hmm. It's not in our ABI docs for accel channels (but is for
> magnetometers).
> Should fix that...
> 
> https://elixir.bootlin.com/linux/latest/source/Documentation/ABI/testing/sysfs-bus-iio#L340
> 
> In theory the other combinations you can get are
> 
> in_accel_scale
> in_scale
> scale
> 
> The in scale one tends not to make much sense for accelerometers
> though so I would
> hope there are no instances of that in the wild (none in mainline).
> Shared by direction attributes are tend to apply for things like
> sampling frequency,
> not scale.

I've updated the merge request. Would be good if somebody could test it
before I merge it.