Re: [PATCH 1/3] iio: adc: hi-843x: Holt HI-8435/8436/8437 descrete ADC

[Vladimir Barinov]

On 21.06.2015 17:14, Jonathan Cameron wrote:
> > > I'd call this a threshold detector. The device seems to have two
> > > comparators for each channel, one for the lower threshold, one for
> > > the upper threshold. If the voltage level goes above the upper
> > > threshold a FF is set, if it goes below the lower threshold the FF
> > > is cleared. Both transitions happen asynchronously as soon has the
> > > signal is below/above the threshold. And while converts a analog
> > > signal to digital one this is not what you typically call a ADC.
> > Should this be a separate/new directory in the drivers/iio/ for such driver?
> Given we could in theory have comparators for any type of channel it probably doesn't
> make sense to have a whole new directory.  Perhaps a section in the config.
> > What the type of iio_chan_spec should I use instead of IIO_VOLTAGE?
> I've been thinking about this.  Maybe we should use the same approach we already
> use for 'computed' channel values such as root sum squared accelerations that turn
> up occasionally and do this as a modifier.
>
> So the channel type would be voltage, but with the modifier comparator (shortening
> it would just get confusing).  This also maps nicely to devices that offer both
> normal adc channels and comparators on the same pin (if that ever happens!)
>
> Hence the attributes etc would be:
> in_voltage0_comparator_raw
>
> Do you think that would be clear / flexible enough?
yes
> > > > I wonder if we want to take this oportunity to add 1 bit packing to the
> > > > demux etc in the IIO core so we can have tighter packing on these
> > > > values.  Shouldn't be too hard to do and we probably do want it if we are
> > > > going to support these sorts of devices.
> > > >
> > > > Will take a bit of shuffling to pack the relevant channels together if only
> > > > a subset are enabled and to notice when no repacking at all is needed.
> > > > This will probably first one implementing in the core and pushing out into
> > > > the dummy driver to allow for testing of corner cases.
> > > Yeah, the bit shuffling gets quite cumbersome and potentially
> > > expensive. I think we should try to avoid it if at least one of the
> > > channels in the same bank is enabled all of them are read. And then
> > > let userspace figure out which bits it wants to use.
> > >
> > > But how exactly is the typical expect usage of this device. Like
> > > how would a userspace application use it? Is buffered mode where
> > > samples are taken in a continuous mode something that is really
> > > needed?
> > I was expecting to use triggered buffer for this device:
> > 1) setup threshold levels via sysfs
> > 2) enable scan elements
> > 3) setup trigger
> > 4) grab data from triggered iio buffer like the
> > tools/iio/generic_buffer.c does, f.e. ./generic_buffer -n hi-8435 -t
> > irqtrig0 -l 100 -c 1000
> Good, you are going about it the right way then.  Makes sense as this
> is what you'd do for similar devices such as a logic analyzer.
> > Actually I understand that I can just read manually the
> > /sysfs/.../in_voltageXX_raw (or new/other name) values but using of
> > iio generic irq trigger would be very good.
> What is generating the interrupt?   Are we looking at a 'dataready'
> type interrupt or some other pseudo (or actual) fixed frequency, or
> are we talking an interrupt on the state of one of the inputs changing?
>
> If the second case, then an event based approach may make more sense
> than using buffers.
Actually it is a interrupt that should occur on the state change of one 
of the inputs.

But hi8435 chip does not have it's own dedicated/hardware interrupt line 
for this purposes, so it
requires any kind of polling (hardware from pwm/gpio, or s/w polling 
inside the driver)

Probably I'm not right by trying to use "iio irq trigger" from external 
hardware line (like gpio/pwm)
and I have to implement polling inside driver.

Event based approach is good enough.
> > About bits shuffling/separating. I do think we can use banks byte
> > length for one iio channel instead of 1-bit length to avoid such
> > complexity. Then let user space separate bank's channels by itself.
> On high channel count devices this could get nasty quickly.
>
> I'd like to explore Lars' suggestion that we use shared scan_indexes
> for a set of channels, and different shift / mask values.  I don't
> think it would need any particularly substantial changes in kernel
> and obviously only usespace code using this new type of device would
> need to know about it.
>
> I'd relax the suggestion he made to have it always sample all channels
> in a given scan index, to say that it can if it makes sense.  We won't
> bother masking them out, but if it is quicker for the device to not read
> those that aren't enabled, then it is up to the device whether it does.
>
> The issue is that if we let a device in now with the 8bits per bit
> interface, we kind of commit to at least having userspace support that
> long term which isn't nice (though not too bad I suppose as it is just
> the least efficient case of what we are talking about doing anyway).
Ok, I see

Regards,
Vladimir
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/