On 02/04/11 15:07, Hennerich, Michael wrote: > Jonathan Cameron wrote on 2011-02-04: >> On 02/04/11 13:48, Hennerich, Michael wrote: >>> Jonathan Cameron wrote on 2011-02-03: >>>> On 02/03/11 14:51, michael.hennerich@xxxxxxxxxx wrote: >>>>> From: Michael Hennerich <michael.hennerich@xxxxxxxxxx> >>>>> >>>>> This patch adds support for the: AD7606/AD7606-6/AD7606-4 >>>>> 8/6/4-Channel Data Acquisition system (DAS) with 16-Bit, Bipolar, >>>>> Simultaneous Sampling ADC. >>>> >>>> Hi Michael, >>>> >>>> Another nice clean driver, various points inline with the code. >>>> >>>> With the parallel interface, I'll admit I've not seen one of these >>>> before so my questions may be somewhat uninformed ;) On that note I'd >>>> also like to have some more eyes on that element if possible. (looks >>>> fine to me) >>>> >>>> The big one is whether the assumption that all such interfaces are >>>> going to be memory mappable is reasonable or whether there should >>>> be some abstraction in below the driver? >>> >>> If these parallel interfaces are memory mappable or not depends on the >>> device in question. In general there are two types of interfaces. One >>> with just a parallel bus and a clock. Or ones with a parallel bus and >>> chip/memory select and read/write strobes. Later ones doesn't differ in >>> any form from a external Flash, USB or NIC/MACPHY, and are therefore >>> memory mappable. The asynchronous parallel bus timing requirements by >>> the device must match the capabilities of the processor. Most >>> processors allow you to set the timings for setup, access and hold >>> cycles. >>> >>> The AD7606 is not very demanding here, and easily connects to any host >>> featuring such interface. >>> >>> Additional abstraction doesn't make sense for memory mapped. >>> >>> However some parts have a parallel interface with only a clock. These >>> devices typically connect to some special Machine/Host dependant >>> peripheral. For example - on Blackfin it would be the PPI (Parallel >>> Peripheral Interface) >>> >>> http://en.wikipedia.org/wiki/Parallel_Peripheral_Interface >>> >>> OMAP calls it Universal Parallel Port (uPP), and so on. >>> These are not abstracted by a standard bus in Linux. >>> >>> Bus abstraction here makes absolute sense... >>> >>> >>>> I'd like to see some more >>>> documentation on the requirements of that interface, even if just a >>>> quick explanation for those not familiar with this sort of >>>> interface within the kernel. >>> >>> Memory mapped parallel interfaces are nothing new. They actually become >>> rare. And the datasheet of the device in question is typically the >>> documentation. >> >> So let me just confirm I have understood this correctly. The two >> parallel bus approaches are not compatible enough to share a common >> interface. One could not for example use PPI to talk to this chip? > > This chip is flexible enough to also interface to an PPI. > The Read and Chip Select strobe can be wired together. > /CS to /RD setup and hold time can be zero. > > The combined /CSRD can be connected to a PPI CLK. > > However PPI and ASYNC Bus serves different purposes. > The PPI typically continuously streams data. While the ASYNC memory bus, > is a real bus, since it is shared between different memory mapped devices. > Such as NOR Flash, MACPHY, SRAM, etc. > > This part generates an interrupt, and the readout per interrupt is always > less or equal to 8 words or 16 bytes. > > On Blackfin PPI can only be used with DMA. > There is no good reason setting up a DMA transfer for only 16bytes. That's all fair enough. I only ask to see if it was worth running even this simple memory mapped bus through an abstraction so the same code would work on the more complext buses. Sounds like it isn't. > >> Thanks for the clarifying email and pointers to more docs. >>> >>> >>>>> >>>>> Signed-off-by: Michael Hennerich <michael.hennerich@xxxxxxxxxx> >>>>> --- >>>>> drivers/staging/iio/Documentation/sysfs-bus-iio | 25 + >>>>> drivers/staging/iio/adc/Kconfig | 27 ++ >>>>> drivers/staging/iio/adc/Makefile | 6 + >>>>> drivers/staging/iio/adc/ad7606.h | 119 +++++ >>>>> drivers/staging/iio/adc/ad7606_core.c | 560 >>>>> +++++++++++++++++++++++ drivers/staging/iio/adc/ad7606_par.c >>>>> | 191 ++++++++ drivers/staging/iio/adc/ad7606_ring.c | >>>>> 261 +++++++++++ drivers/staging/iio/adc/ad7606_spi.c | >>>>> 133 ++++++ 8 files changed, 1322 insertions(+), 0 deletions(-) >>>>> create mode >>>>> 100644 drivers/staging/iio/adc/ad7606.h create mode 100644 >>>>> drivers/staging/iio/adc/ad7606_core.c >>>>> create mode 100644 drivers/staging/iio/adc/ad7606_par.c >>>>> create mode 100644 drivers/staging/iio/adc/ad7606_ring.c >>>>> create mode 100644 drivers/staging/iio/adc/ad7606_spi.c >>>>> diff --git a/drivers/staging/iio/Documentation/sysfs-bus-iio >>>>> b/drivers/staging/iio/Documentation/sysfs-bus-iio >>>>> index 8e5d8d1..b415019 100644 >>>>> --- a/drivers/staging/iio/Documentation/sysfs-bus-iio >>>>> +++ b/drivers/staging/iio/Documentation/sysfs-bus-iio >>>>> @@ -53,6 +53,31 @@ Description: >>>>> When the internal sampling clock can only take a small >>>>> discrete set of values, this file lists those >> available. >>>>> +What: /sys/bus/iio/devices/deviceX/range >>>>> +KernelVersion: 2.6.38 >>>>> +Contact: linux-iio@xxxxxxxxxxxxxxx >>>>> +Description: >>>>> + Hardware dependent ADC Full Scale Range. >>>> Could you give an illustrative example of a usecase where this is >>>> necessary? >>> >>> The AD7606 features true bipolar analog inputs (ranges are +/-10 and >>> +/- 5Volt) While being powered from a single 5Volt supply. >>> >>> The range is selected by a special logic input strobe. >>> >>>> I'm generally a bit dubious about the obviously considerable >>>> overlap between this and the _scale and _type attributes. I guess >>>> having them yoked together as you have here works out fine. We >>>> might want to set a preference though. Perhaps always have _scale >>>> as the controlable option and range as the dependant value? >>> >>> In this case range directly influences the maximum voltage that can be >>> applied to the inputs. Setting range however also influences the scale. >>> Since +/- 10 Volt are now distributed over 16-bit resolution. Actually >>> there is a bug in my scale implementation, since the ABS >> value is 20V. >> >> Yes, clearly range and scale are related, but my question is slightly >> different. The two are yoked together (i.e. changing one clearly changes >> the other). I was wondering if we want to specify which should be the >> 'controlling' attribute? That is which of scale and range should >> userspace expect to be writable if it wants to configure this? Just >> seems to me that not having this choice specified may just lead to >> future confusion. > > The term 'range' is typically used here, and should be the controlling attribute. On ADCs. Part of the original reason for going with scale is that some other sensors (Ambient light sensors for example) actually combine on chip several different signals (subtract one from another) and hence the 'range' is fairly meaningless concept whereas scale still makes sense. There are also weird and wonderful tracking ADCs out there where the range of measurement possible is dependent on what the previous value was. Again scale works, whereas range does not. If the documentation makes it clear that, in the case where both range and scale are provided, either may be the controlling parameter then that would work for me. The userspace tools will just have to try one and if it doesn't allow writing, try the other. -- 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