Re: [PATCH 5/5] dt-bindings: iio: Add adis16475 documentation

From: Jonathan Cameron <jic23@kernel.org>
To: "Sa, Nuno" <Nuno.Sa@analog.com>
Cc: "lars@metafoo.de" <lars@metafoo.de>,
	"Ardelean, Alexandru" <alexandru.Ardelean@analog.com>,
	"linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"mark.rutland@arm.com" <mark.rutland@arm.com>,
	"pmeerw@pmeerw.net" <pmeerw@pmeerw.net>,
	"knaack.h@gmx.de" <knaack.h@gmx.de>,
	"Hennerich, Michael" <Michael.Hennerich@analog.com>,
	"robh+dt@kernel.org" <robh+dt@kernel.org>
Subject: Re: [PATCH 5/5] dt-bindings: iio: Add adis16475 documentation
Date: Sat, 7 Mar 2020 11:33:49 +0000	[thread overview]
Message-ID: <20200307113349.27e4484a@archlinux> (raw)
In-Reply-To: <a6b4c76a337f53db1ee05a03eca06751b901d04b.camel@analog.com>

On Thu, 5 Mar 2020 13:04:27 +0000
"Sa, Nuno" <Nuno.Sa@analog.com> wrote:

> On Thu, 2020-03-05 at 13:43 +0100, Lars-Peter Clausen wrote:
> > On 3/5/20 1:27 PM, Sa, Nuno wrote:  
> > > > In my opinion there is should not be a difference in the
> > > > userspace
> > > > interface for chips that do support 32-bit burst and those that
> > > > don't.
> > > > For devices that don't support 32-bit burst it should be emulated
> > > > by
> > > > reading the LSB bits registers manually.  
> > > Hmm. In terms of interface I think there is no difference. We
> > > always
> > > report 32bits channels (for accel and gyro). However, what we do
> > > right
> > > know is just to set the LSB to 0 if burst32 is not supported. So,
> > > we
> > > can be just ignoring the LSB bits if they are being used...  
> > 
> > What I meant was that somebody might still want to get the full 32-
> > bit 
> > values in buffered mode, even if the device does not support burst32.  
> 
> They are. Just that the LSB part is always set to 0 :). And that, in my
> opinion, is wrong. As you say, we should do the manual readings if
> there are any bits on the LSB registers...
> 
> - Nuno Sá
> > In 
> > that case you can first do a 16-bit burst read to get the MSBs and
> > then 
> > do manual reads of all the LSB registers and then put both into the
> > buffer.
> > - Lars
> >   
> 
Thanks Lars and Nuno, I'd not grasped exactly what this was.

Hmm.  Not allowing for variable bit widths has bitten us a few times in the
past.  Howwever, it's a really nasty thing to try and add to the core now
unfortunately.

In some cases we've just padded the smaller bitwidth buffer but that
is costly to actually do.  You get fast reads from the hardware then loose
at least some of the benefit respacing the data.

Still it is definitely a policy decision so not DT.  It's ugly but if
we want to support it and can't do it at runtime, perhaps a module parameter
is the best option?

Thanks,

Jonathan