Re: [PATCH v2] iio: light: introduce si1133

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

 



On Tue, 3 Jul 2018 12:28:39 -0400
Maxime Roussin-Belanger <maxime.roussinbelanger@xxxxxxxxx> wrote:

> On Sat, Jun 30, 2018 at 05:37:22PM +0100, Jonathan Cameron wrote:
> > On Thu, 28 Jun 2018 10:34:05 -0400
> > Maxime Roussin-Belanger <maxime.roussinbelanger@xxxxxxxxx> wrote:
> >   
> > > Asked a couple questions about IIO in general.
> > > 
> > > On Wed, Jun 27, 2018 at 10:02:59PM +0200, Peter Meerwald-Stadler wrote:  
> > > > 
> > > > comments below
> > > >     
> > > > > Signed-off-by: Maxime Roussin-Bélanger <maxime.roussinbelanger@xxxxxxxxx>
> > > > > Reviewed-by: Jean-Francois Dagenais <dagenaisj@xxxxxxxxxxxx>
> > > > > ---
> > > > > Changes in v2:
> > > > > 	- Add ABI documentation
> > > > > 	- Drop part abstraction
> > > > > 	- Clean up error handling style
> > > > > 
> > > > >  .../ABI/testing/sysfs-bus-iio-light-si1133    |  57 ++
> > > > >  drivers/iio/light/Kconfig                     |  11 +
> > > > >  drivers/iio/light/Makefile                    |   1 +
> > > > >  drivers/iio/light/si1133.c                    | 809 ++++++++++++++++++
> > > > >  4 files changed, 878 insertions(+)
> > > > >  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-light-si1133
> > > > >  create mode 100644 drivers/iio/light/si1133.c
> > > > > 
> > > > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-light-si1133 b/Documentation/ABI/testing/sysfs-bus-iio-light-si1133
> > > > > new file mode 100644
> > > > > index 000000000000..4533b5699c87
> > > > > --- /dev/null
> > > > > +++ b/Documentation/ABI/testing/sysfs-bus-iio-light-si1133
> > > > > @@ -0,0 +1,57 @@
> > > > > +What:		/sys/bus/iio/devices/iio:deviceX/in_intensity0_ir_small_raw    
> > > > 
> > > > do we need the 0?    
> > > 
> > > Hopefully not, comments below.  
> > 
> > Given the several channels only distinguished by extended name, yes if you
> > might potentially have events at some later date.
> >  
> 
> I'm a bit confused on what "events" are. I'm wondering if you are talking about the
> autonomous mode or just the IRQ?

I wasn't really addressing this device, just the concepts around why we need to
keep indexes.  Events in IIO are thresholds of something or other, reported on
a character device (of a sorts).

>  
> > > >     
> > > > > +KernelVersion:	4.18
> > > > > +Contact:	linux-iio@xxxxxxxxxxxxxxx
> > > > > +Description:
> > > > > +		Unit-less infrared intensity. The intensity is measured from 1
> > > > > +		dark photodiode. "small" indicate the surface area capturing
> > > > > +		infrared.
> > > > > +
> > > > > +What:		/sys/bus/iio/devices/iio:deviceX/in_intensity1_ir_med_raw
> > > > > +KernelVersion:	4.18
> > > > > +Contact:	linux-iio@xxxxxxxxxxxxxxx
> > > > > +Description:
> > > > > +		Unit-less infrared intensity. The intensity is measured from 2
> > > > > +		dark photodiode. "med" indicate the surface area capturing    
> > > > 
> > > > photodiodes
> > > >     
> > > > > +		infrared.
> > > > > +
> > > > > +What:		/sys/bus/iio/devices/iio:deviceX/in_intensity2_ir_large_raw
> > > > > +KernelVersion:	4.18
> > > > > +Contact:	linux-iio@xxxxxxxxxxxxxxx
> > > > > +Description:
> > > > > +		Unit-less infrared intensity. The intensity is measured from 4
> > > > > +		dark photodiode. "large" indicate the surface area capturing    
> > > > 
> > > > photodiodes
> > > >     
> > > > > +		infrared.
> > > > > +
> > > > > +What:		/sys/bus/iio/devices/iio:deviceX/in_intensity11_raw    
> > > > 
> > > > what does 11 mean?    
> > > 
> > > First time using iio subsystem... I think I can remove all the numbers, but the
> > > number were used to differentiate the different channels with "large", "med"...
> > > 
> > > Now that I use the "extend_name" field, maybe I can get rid of it?  
> > 
> > No.  Potentially causes all sorts of problems if there are events on this part
> > or on some similar part in future.  Extend_name should not be used to make channels
> > unique.
> >   
> 
> I wasn't using the extend_name to make them unique, but trying to use the iio framework
> to give a name to this property instead of a number that doesn't mean much. That's how I
> interpreted the documentation for the "extend_name" field.

That is the intent, but the trade off is that it breaks most standard userspace.
So we generally try to avoid using them if we can convey the meaning some other way.
(or if the meaning isn't that important). Basically it's a horrible interface we should
never have introduced in the first place (I'm fairly sure it was my fault long ago!)

> 
> If it makes it unique, it's probably a coincidence.
It's necessary to maintain uniqueness otherwise you can't create the files.
> 
> 
...


--
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