Re: [PATCH 1/2] iio:accel:bma180: Use modifier instead of index in channel specification

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello,

> > should use channel modifiers (X/Y/Z), not channel indices
> > timestamp channel has scan index 3, not 4
> > 
> Aag.  Whilst a correct fix, this is going to result in a userspace ABI
> change.  Driver has been in since 3.12.  We may need to maintain
> both ABI's for now and then deprecate it at some point.  Easiest way to do
> this
> will be to have two sets of iio_chan_specs pointing at the same underlying
> real channels.
> 
> Anyone have a strong opinion about this?
> Good spot by the way!

adding another email address of Oleksandr, x0199363@xxxxxx bounces

the IIO ABI is under "testing", not "stable" so the interface may be 
changed due to 'grave errors'

given the short timespan since 3.12 and the clear violation of the 
documented ABI, I think this should just be fixed

another suggestion:

often (at least for me) it is difficult to figure out what the user-space 
sysfs ABI is from reading the source code and I find it even more 
difficult to sometimes guess what a particular sysfs is supposed to do 
(e.g. what writing to a _scale actually sets in terms of the hardware); 

I'd like to suggest to ask for a listing of the sysfs entries relating 
to a new driver under review (e.g. the output of 'find 
/sys/bus/iio/devices/iio:deviceX'); it should be feasible to provide a
tool which compares the submitted listing against the sysfs-bus-iio 
documentation and output appropriate warnings

regards, p.
 
> > Signed-off-by: Peter Meerwald <pmeerw@xxxxxxxxxx>
> > Cc: Kravchenko Oleksandr <x0199363@xxxxxx>
> > ---
> >   drivers/iio/accel/bma180.c | 16 ++++++++--------
> >   1 file changed, 8 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/iio/accel/bma180.c b/drivers/iio/accel/bma180.c
> > index 3bec922..bfec313 100644
> > --- a/drivers/iio/accel/bma180.c
> > +++ b/drivers/iio/accel/bma180.c
> > @@ -447,14 +447,14 @@ static const struct iio_chan_spec_ext_info
> > bma180_ext_info[] = {
> >   	{ },
> >   };
> > 
> > -#define BMA180_CHANNEL(_index) {					\
> > +#define BMA180_CHANNEL(_axis) {					\
> >   	.type = IIO_ACCEL,						\
> > -	.indexed = 1,							\
> > -	.channel = (_index),						\
> > +	.modified = 1,							\
> > +	.channel2 = IIO_MOD_##_axis,					\
> >   	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |			\
> >   		BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY),	\
> >   	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),		\
> > -	.scan_index = (_index),						\
> > +	.scan_index = AXIS_##_axis,					\
> >   	.scan_type = {							\
> >   		.sign = 's',						\
> >   		.realbits = 14,						\
> > @@ -465,10 +465,10 @@ static const struct iio_chan_spec_ext_info
> > bma180_ext_info[] = {
> >   }
> > 
> >   static const struct iio_chan_spec bma180_channels[] = {
> > -	BMA180_CHANNEL(AXIS_X),
> > -	BMA180_CHANNEL(AXIS_Y),
> > -	BMA180_CHANNEL(AXIS_Z),
> > -	IIO_CHAN_SOFT_TIMESTAMP(4),
> > +	BMA180_CHANNEL(X),
> > +	BMA180_CHANNEL(Y),
> > +	BMA180_CHANNEL(Z),
> > +	IIO_CHAN_SOFT_TIMESTAMP(3),
> >   };
> > 
> >   static irqreturn_t bma180_trigger_handler(int irq, void *p)
> > 
> 

-- 

Peter Meerwald
+43-664-2444418 (mobile)
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux