x86, intel_mid: ADC management

This ended in a discussion of where shall we put it and then silence so resending this because we still need it and we've got patches that depend upon it getting backlogged.

From: Alan Cox <[email protected]>

This is a fold up of the following ported to 3.3

commit f81040398e4b45323d4144fb05c126245e86866d

Author: Mika Westerberg <[email protected]>
Date:   Thu Jan 12 11:51:18 2012 +0200

x86, mrst: add platform data for gpadc driver

Now that the driver is converted to use intel_msic interfaces we can add the platform data back.

commit 6d9d2d390f6e18ce1d419b9e2762c9bf139a80ea

Author: Mika Westerberg <[email protected]>
Date:   Thu Jan 12 11:51:17 2012 +0200

gpadc: convert to use intel_msic

In Medfield all the subdevices in MSIC chip are handled by intel_msic MFD driver. This patch converts gpadc driver to use interfaces provided by intel_msic driver.

This will enable all the users (including sn95031 audio codec) to use gpadc again.

commit f1532c8454b8958aee933a9915862345d62ee3f8

Author: Mika Westerberg <[email protected]>
Date:   Thu Jan 12 11:51:16 2012 +0200

x86, mrst: remove gpadc platform data

Nowadays these devices are handled by intel_msic MFD driver which has slightly different way of passing platform data to the drivers.

So as a first step we remove the old platform data for gpadc driver. In subsequent patches we add it back in such format suitable for intel_msic driver.

commit 8444512429fd93786f114824bb5c258272aa1d2d

Author: Bin Yang <[email protected]>
Date:   Tue Oct 18 14:16:31 2011 +0100

gpadc: add reg dump after timeout

Sometimes, it is very hard to duplicate ADC timeout issue. Some issue only can be duplicated on a small quantity of boards. To add the registers dump after timeout will help debugging.

commit 26fc6e4e72d1cac189064b9c85290e3c8a907cf0

Author: Bin Yang <[email protected]>
Date:   Tue Oct 18 14:16:30 2011 +0100

gpadc: fix battery temp accuracy

From oscilloscope result, BPTHERM takes <1ms to stabilize. So it needs to add 1ms delay after VBUSREF is enabled.

commit 9dc5019e570c59e70349b34ec864bfc447a0d0c5

Author: Bin Yang <[email protected]>
Date:   Tue Oct 18 14:16:30 2011 +0100

gpadc: optimize driver initialization

ADC trimming cost a long time. It blocks the kernel boot sequence. Change to do trimming in a single workqueue.

commit 16149d8e3dfe0d663851318390ebafdede760281

Author: Bin Yang <[email protected]>
Date:   Tue Oct 18 14:16:29 2011 +0100

gpadc: read sample result in one loop

RR bit is used to inform HW for sample result read. it needs to hold the RR bit for all channels result read.

this patch moves the lines to set/clear RR bit outside the loop.

commit d910dd3b07b6e0766c96c59e48de5bcce1175f0c

Author: Bin Yang <[email protected]>
Date:   Tue Oct 18 14:16:29 2011 +0100

gpadc: fix logic err of last addr checking

it has logic error to check last addr which is used.

commit 1b688193bd96331da73482399925d51cbf535780

Author: Bin Yang <[email protected]>
Date:   Tue Oct 18 14:16:28 2011 +0100

gpadc: update the calibration algorithm

gpadc document had updated this algorithm in new version.

This patch updates the driver base on new document.

commit baa74900a34ad17de1619a8f009702e66eb4fbd4

Author: Bin Yang <[email protected]>
Date:   Tue Oct 18 14:16:28 2011 +0100

gpadc: fix sample result error

During stress test, ADC results have some bad valules.
Driver has a race condition issue. It initializes the output data to 0 first without any lock. And the caller may access the output pointer before function return.

This patch moves the output data initializing code inside mutex protected period. And it also adds a limitation which forbid caller to access output pointer before function return.

commit 9c53a361051541a7cb24a8fde90d5c75c5d52d91

Author: He Bo <[email protected]>
Date:   Tue Oct 18 14:16:26 2011 +0100

gpadc: system does not enter S3 after incoming call

intel_mid_gpadc driver causes deadlock in S3, fix it.

commit f25e645c187d3b9943ad0180e52984f51c77e64b

Author: Bin Yang <[email protected]>
Date:   Tue Oct 18 14:16:25 2011 +0100

gpadc: prevent CPU from S0i3 when ADC is active

Under the following scenario:
1. IPC1 request sent to SCU
2. PM_CMD s0ix entry request sent to SCU
3. MWAIT C6 abort (or no attempt at MWAIT C6)
If steps #1 an #2 are sufficiently close (maybe within 200us) there is
a possibility that SCU will handle the PM_CMD first and begin waiting on an ack_c6 response from the MWAIT that is expected. Since the MWAIT never comes (or is aborted) SCU will have to eventually time out before being able to move on and handle the IPC1 request.

commit fdd0adef95e6b5836cc4ed5764a398b81841945d

Author: Bin Yang <[email protected]>
Date:   Tue Oct 18 14:16:23 2011 +0100

The coulomb count interrupt is not used by kernel driver. It needs to be handled inside firmware.

commit 1f4f11af853a74c015a9ffb5bc22d3f9140ce6d5

Author: Bin Yang <[email protected]>
Date:   Tue Oct 18 14:16:19 2011 +0100

gpadc: add gpadc driver support

The general purpose ADC inside MSIC is used by battery and thermal. This driver provides the channel allocation and basic ADC functions.

---

 arch/x86/include/asm/intel_mid_gpadc.h   |   13 +
 arch/x86/platform/mrst/Makefile          |    1 
 arch/x86/platform/mrst/intel_mid_gpadc.c |  645 ++++++++++++++++++++++++++++++
 arch/x86/platform/mrst/mrst.c            |    6 
 4 files changed, 665 insertions(+), 0 deletions(-)
 create mode 100644 arch/x86/include/asm/intel_mid_gpadc.h
 create mode 100644 arch/x86/platform/mrst/intel_mid_gpadc.c

new file mode 100644
index 0000000..ae79309
@@ -0,0 +1,13 @@
+#ifndef __INTEL_MID_GPADC_H__
+#define __INTEL_MID_GPADC_H__
+
+#define CH_NEED_VREF		(1 << 8)
+#define CH_NEED_VCALIB		(1 << 9)
+#define CH_NEED_ICALIB		(1 << 10)
+
+int intel_mid_gpadc_gsmpulse_sample(int *vol, int *cur);
+int intel_mid_gpadc_sample(void *handle, int sample_count, ...);
+void intel_mid_gpadc_free(void *handle);
+void *intel_mid_gpadc_alloc(int count, ...);
+#endif
+

index af1da7e..66e006c 100644
@@ -1,3 +1,4 @@
 obj-$(CONFIG_X86_INTEL_MID)	+= mrst.o
 obj-$(CONFIG_X86_INTEL_MID)	+= vrtc.o
 obj-$(CONFIG_EARLY_PRINTK_INTEL_MID)	+= early_printk_mrst.o
+obj-$(CONFIG_X86_INTEL_MID)	+= intel_mid_gpadc.o

index a810696..8cefd1b 100644
@@ -792,6 +792,11 @@ static void *msic_generic_platform_data(void *info, enum intel_msic_block block)
 	return no_platform_data(info);
 }
 
+static void *msic_adc_platform_data(void *info)
+{
+	return msic_generic_platform_data(info, INTEL_MSIC_BLOCK_ADC);
+}
+
 static void *msic_battery_platform_data(void *info)
 {
 	return msic_generic_platform_data(info, INTEL_MSIC_BLOCK_BATTERY);

@@ -880,6 +885,7 @@ static const struct devs_id __initconst device_ids[] = {
 	{"smb347", SFI_DEV_TYPE_I2C, 0, &smb347_platform_data},
 
 	/* MSIC subdevices */
+	{"msic_adc", SFI_DEV_TYPE_IPC, 1, &msic_adc_platform_data},
 	{"msic_battery", SFI_DEV_TYPE_IPC, 1, &msic_battery_platform_data},
 	{"msic_gpio", SFI_DEV_TYPE_IPC, 1, &msic_gpio_platform_data},
 	{"msic_audio", SFI_DEV_TYPE_IPC, 1, &msic_audio_platform_data},

The decision seems fairly settled that general purpose ADCs like this should go into IIO, the trick is going to be getting IIO out of staging.

It's far lower level than things like IIO. It provides very low level services to other drivers not to user space. You could in theory write an IIO driver to use the interface but at the moment all the consumers are low level hardware drivers and likely to remain so.

Alan

Right, but fundamentally it's just a general purpose ADC which looks just the same as all the other SoC/PMIC ADCs. Like I say the decision for that hardware was to push it in via IIO. If we don't do that we're just going to be stuck with two subsystems doing very similar things.

Your decision maybe. And it's one that makes no sense.

As I said this is a low level interface for driver enabling including early boot time stuff, and its on a platform that's unlikely to want or ever use and suck in the big blob of IIO code.

If you want an IIO interface to it you can write one, it's just a low level resource manager.

Alan

It's not just me, it's where all the ARM SoCs and their PMICs are going. If you look on the IIO list you'll see patches for at least the AT91

Could you be more specific about what this early boot time stuff is? Looking at the changelogs in there it all looks like the standard battery monitoring and power supply stuff that these ADCs get used for - just based on the changelogs there doesn't appear to be anything remarkable here.

There's already a standard adaptor to map IIO into at least hwmon, not sure about the power supply subsystem. There were also some other things people were doing in areas like thermal management that were using this sort of hardware in generic ways.

We can't just keep on going round adding new custom interfaces every time someone supports a new SoC - it means we end up having to sit and write lots of per-device adaptor code to integrate things and it's hard for us to factor common code out of drivers when we've not got any kind of framework for them.

If IIO is totally unsuitable then someone needs to make the case for a new subsystem for this class of hardware and provide that subsystem but it seems entirely obvious that there is a general class of hardware here which is widely implemented.

It depends on the actual device but things the like battery management are a key one.

We can't go around blocking entire platforms because of the IIO blob. I raised this point with the whole previous *generation* of Intel SoC devices about IIO and nothing has been done about it.

Get IIO out of staing and we can look at it, until then IIO is staging code, it's not part of the kernel, it may never be part of the kernel, and it should never block actual kernel code.

Alan

Right, like I say that all sounds totally standard and unremarkable.

Including by Intel, of course.

That's not where the rest of the embedded community has been coming from on this stuff and from a deployment point of view staging isn't really that big a blocker to users. We've had a lot of experience with trying to follow that approach and the results haven't been great thus far.

Frankly at this point I don't understand why we can't just lift IIO out of staging as-is, perhaps with the userspace ABI nobbled or moved into debugfs for the time being if that's still a concern. Alternatively there is the option of you proposing some other generic framework.

Given the responses I got to moving it out of staging were of the "not yet" and "no" variety it hardly got a request to be worked upon.

Non-staging code cannot depend upon staging code. That is the rule GregKH laid down. The Intel drivers involved are established non staging drivers and the gpadc layer is basically cleaning up the fact they all do this themselves in private right now without any central co-ordination or abstraction.

I've been saying this for over a year. It's still a huge blob of code that isn't needed for pure low level stuff, but that's fixable and something which can eventually be worked upon.

You are trying to make the tail wag the dog. Staging is basically out of kernel. The x86 stuff is *in kernel*, this driver is the basis of cleaning it up. We can sort out making it talk to IIO later. Right now all you are doing is blocking a pile of much needed code cleanups, and without them you *won't* have an API for the gpadc on Intel, it'll be hardcoded in each driver still. That will mean you have *zero* change of getting IIO support at all.

Whether gpadc then adopts an IIO interface is a question to ask later, after (if) IIO is ever merged. If the platform ever starts using gpadc for non internal hardware stuff then I'm pretty sure it'll end up adopting whatever generic GPADC layer eventually emerges simply because we'll want to share drivers.

Alan

The "not yet" bit of it certainly seems like something that could be worked on, presumably "not yet" included a "we need to do X first" element.

So, that's a bit different and not at all obvious from your e-mail - the diffstat shows only the code you're adding, not the code you've factored out of the existing mainline drivers. The diffstat you posted was:

| arch/x86/include/asm/intel_mid_gpadc.h   |   13 +
| arch/x86/platform/mrst/Makefile          |    1
| arch/x86/platform/mrst/intel_mid_gpadc.c |  645 ++++++++++++++++++++++++++++++
| arch/x86/platform/mrst/mrst.c            |    6
| 4 files changed, 665 insertions(+), 0 deletions(-)

which is a pure addition of code and I'm not seeing anything in the changelog about this either.

I've CCed in Greg and Jonathan here - guys, what is happening with getting IIO out of staging? It's been there since 2009 and from an outside point of view it's really difficult to see progress here, the time taken to merge the subsystem seems really long.

To me it really feels like the subsystem is pretty mature at this point and that if anything staging is blocking things rather than helping them, the subsystem feels like it's getting normal development rather than work to integrate it into the rest of the kernel and being in staging is discouraging users who don't absolutely need to use it.

For example, when we get extcon (or whatever it ends up being called) merged we'll either have to start writing a raft of auxadc specific drivers for it or we'll have to come up with *some* kind of subsystem to use to abstract out the standard DC measurement pattern.

If the code was moved out of staging today what would go wrong?

We do have to be careful about this sort of "this is a little bit of low level code" thing - it (along with "our hardware is unique") comes up rather a lot and it's often missing a good chunk of the picture.

You keep saying "you" here. To repeat once more, this is pretty much what the general embedded community has been pushing towards, this is not particularly my personal opinion.

Like I said above you'd not posted the patches which factor the code out of the existing users, if that's what you're trying to do that's a bit different to what your patch looked like.

Again, this is the sort of thing that SoC vendors routinely say but don't end up doing quite so routinely as one would hope. This is understandable as this sort of thing is more of a problem for people working in off-SoC devices and system integration than the SoC vendors themselves.

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Entirely possibly I gave my usual evasive set of a few things that were top of the pile to knock over! I've been promising a 'state of iio email' for a while now. Sorry about that.

I was anxious to not move anything out of staging that we 'knew' needed a major refactoring. There is one more such refactoring going through revisions at the moment. (Push in kernel interfaces to allow for interrupt driven users such as a client iio driver that pushes data to input). Last attempt to move out of staging unfortunately fell foul of some issues Greg KH raised in a review (correctly!) These were with the interfaces for requesting in kernel access to IIO device channels and we really needed to get that right so I pulled back from sending a pull request to Linus. Since then time has been tight on my side. Some others have been very helpful in picking up the slack.

Anyhow to repeat my whats left comments.

1) Review of code. This is crucial. If people have a little time ripping holes in the core IIO code is what we need. Arnd did a good job of this a while back. Others have done bits of it since.

2) Getting the push code tidied up and pushed out. I'll post it as an updated rfc to linux-iio shortly. All I had left that definitely wanted doing here was cleaning up the example iio to input bridge driver. That can happen later.

Other bits that are less than ideal.

* Event passing to consumers else where in the kernel. Right now an input driver can readings from a sensor, but there is no way of requesting threshold interrupts.

* Interaction between consumer drivers (e.g. hwmon or input) where some are requesting data by polling when they want it and others want a stream of data driven by interrupts. I can give details of one approach to this to anyone interested but it's basically a simply one element buffer and some changes to channel requesting (entirely in the IIO core
- - external drivers will just see requests that would previously be refused start working).

Agreed :(

Churn in interfaces is probably about it. Maybe a good use of any time would be for people to take their non IIO drivers that they think might fit (or data sheets!) and see whether there are things that they would like to be different.

An initial move would just take (possibly with some file name tidying up!)

industrialio-core.c -> core.c
industrialio-trigger.c -> trigger.c
industrialio-event.c -> event.c
inkern.c -> inkern.c
kfifo-buf.c -> buffer_kfifo.c

iio.h
buffer.h
driver.h
machine.h
consumer.h
trigger.h
trigger_consumer.h
types.h
iio_core.h
kfifo_buf.h
sysfs.h (stripped of a few legacy bits at the end that can go into a sysfs-staging.h until the users are all cleaned up).

Some name stripping etc needed and a wholesale switch of headers.

Doing it this way would be a lot less painful than the build up approach we suggested last time, but more controversial as this is large complex code drop (to anyone who hasn't followed it in staging).

The 'fun' bit for most IIO developers is that we weren't originally interested in SoCs at all. The support we've added for in kernel users of IIO has all come out of Mark and other's comments on how it would be useful to them. It's been interesting and there have been considerable gains for our other usecases, but it has take time to do.

Anyhow, if people have spare cycles, then as ever review time is what we need, even better if people are willing to dive in and propose fixes to stuff they don't like.

Now that was probably mostly irrelevant to the majority of the the thread :)

Jonathan
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJPhIw4AAoJEFSFNJnE9BaIEmAP/2l/Ks11td0SGyU1uDz394V3 PuTPtzAxtGpHPApxafHnN7ci270N2YloSGefP3qZ5bcMwbj8XBOw4VQ8M2rWuSWa xghMmc1IxfYa4BsazsvEEtlU1QZq6DTjrh51azXv6sgiJcFOttPCnIAxpJnnEQXG EVb7guUnQhzzgNCAA11up2DHohvB4CGig/jx5d7JO2puhSw483+qEzY71bW0T6FA NNXZF8zTsppcMg/YKYheCyJ2ihDUQXrhMUDc14Pz3UHiSx9q2sGrygOOnPNBoP5r HTRe3sKCpEQrPjavX3f6l0BHGd4EW4UHwQ+ZELY2SYiXqcltleuw3EjGMQA9j/6z p2opkGXMI2Zw7oI4C4XZ6ivpFQxUjpeAnNGhGHNDVQWUuXDsNmc/z6yYwW8CN1sn BgAueu9hdoxQF7AoQyQ9cl6PC93aUvAkzf5TbNAfP+8Rsboo6xqxKAJGq2ddFkad KA5Zpxe5VbalmNlMUeNmad6A1KSJe1XfPv+L1heAL+dE+w18pFUmaN8HYghDSOYV IUqxGJ2VjEA+irYndYv+qjWfojpv8xrUc2WSJaYjclAWEm+CmTHTblzro5oDZE+C tZjYGz1LobGSIO13hQqW3d1QrIDIt3Vx7Z6iROBSPbll44PaDPXV3DVZh8YiLn97 eLxK5n3x0jNJwVt8ACbO
=AbrL
-----END PGP SIGNATURE-----

For these two can we refactor in place? That's pretty much what seems to have been happening anyway...

These sound like something that can be added incrementally?

I guess the big question is then if we can live with that.

In tree there's a few auxadc and comparator drivers in drivers/mfd, plus things like arch/arm/plat-samsung/adc.c in the arch direcories. These are all broadly similar to the at91 code that's been sent to IIO already. There's also the code Alan posted at the top of this thread.

This is crucial. If people have a little time

e IIO code is what we need. Arnd did a good

Others have done bits of it since.

and pushed out. I'll post it as

All I had left that definitely

mple iio to input bridge

o can we refactor in place? That's pretty much what seems

ppening anyway...

I guess it comes down to whether Linus will pull. 2 sh=
ould be there within a week or so anyway depending mostly on analog testing=
I haven't broken any of their drivers.

e where in the kernel. Right now an

sor, but there is no way of

raction between consumer drivers (e.g. hwmon or input) where

equesting data by polling when they want it and others want a

d like something that can be added incrementally?

ved out of staging today what would go wrong?

robably about it. Maybe a good use of any

n is then if we can live with that.

Agreed.

ke their non IIO drivers that they think

d see whether there are things that they

=

=

e all broadly similar to the at91 code that's been sent to IIO

here's also the code Alan posted at the top of this thread. Quite a lot of =
things in miscellaneous as well to possibly pull in over time.

Sent =
from my Android phone with K-9 Mail. Please excuse my brevity.

This is a fairly stupid place to reply but I don't have any way of replying to the original patch whilst at work. Looking quickly at it in a web browser, looks like we fundamentally have a couple of interfaces exported. An alloc and free pair and two types of sample? The second, during gsmpulse sample is some sort of triggered capture?

Mapping to what is now in IIO (and hasn't been for that long!).
We handle alloc and free via a mapping table that registered with the core by the consumer driver (often coming from platform data).

Raw channel reads are available via polling or you can register a callback function (that's the bit that is going through review at the moment) if you have a buffered situation. So the missing bit is that we don't have a 'request n samples' call. We also don't currently have the functions to switch the triggering mechanism from consumers (though they should be easy to add).

Anyhow, those are the differences and looks like IIO needs a few small tweaks to meet the interface requirements if you did decide to go that way. The only one I'm not immediately sure how to handle is the 'read n samples' bit. A self unregistering version of the callback buffer might do it reasonably cleanly....
What you have may correspond to an arbitary scan. The question is do you ever do uneven reading of different channels? e.g. AABAAB.

Jonathan

Hm, shouldn't I be the one that moves this out of staging? :)

Anyway, all I care about to get this code out of staging is that you feel your userspace api is "sane" and not going to change. Your in-kernel stuff can radically change every kernel release with no objection from me at all.

And from what I can tell, your userspace stuff looks pretty stable now, right? So, I don't mind moving this all out of staging for 3.5 as-is.

If so, I'll be glad to make the change to my repo so it starts to show up in linux-next in the "correct" place whenever you want me to.

thanks,

greg k-h

Sorry. Still in mind set of last try where we a parrallel version planned..

There are corners of the userspace abi that aren't but they aren't related to the core so we can break the abi docs in two and just move the good stuff...

Will clear current queue under review then that would be great.

Note we will need to leave a lot of drivers in staging for now so move may require a few sed scripts to link up headers etc... quite a lot of drivers are way off.

Lets start a fresh thread on the move to make sure everyone agrees on what is ready!

Thanks as ever for all your hard work!

Sent from my Android phone with K-9 Mail. Please excuse my brevity.

Ok, that sounds like a good first step.

Why would some drivers stay? It should only be because of the userspace api, not because they don't work properly, that's never stopped drivers from entering the main part of the kernel before :)

Sounds good.

greg k-h

You got it in one. They have userspace interfaces that are miles from where they should be...

Sent from my Android phone with K-9 Mail. Please excuse my brevity.

True.. the code factoring out stuff is supposed to follow once this is sorted.

Yes. But it also helps - there is no user space API here so the only bit of IIO that needs importing is small, and if the API in kernel changes nothing userspace will break.

I don't btw think that layer should depend upon IIO though - IIO should depend upon that layer. There's no need to drag all the rest of IIO in for this, just as it can depend upon the gpio layer etc.

If that code gets pulled out of IIO dumped into drivers/adc and has a bit of a different API to the gpadc code then no problem, gpadc can follow it happily enough. IIO can use it from staging and IIO can migrate whenever.

It does make sense to think hard about userspace APIs for IIO but for kernel APIs its really being too conservative - we break kernel APIs every release. They are not set into stone.

Alan

The way I keep thinking about this (which I'm sure I've suggested before) is that we should be refactoring IIO such that the userspace ABI is just another in-kernel consumer of the device driver bit of IIO and that driver bit should be small enough to just use. I think that winds up being roughly equivalent to your suggestion but means that there's just one place to put the drivers which seems like a win.

IIO is about a heck of a lot other than ADCs. Keep that in mind. They are a substantial corner but we handle a lot of output devices and other input devices (though these might be adc's inside, that's not what your average users think of them as). We 'have' to ensure anything we do works for the other device types as well.

The intent is there, but there are an awful lot of corner cases that need working out. Mostly these don't relate to simple adc usage. Just take a look at those elements of the drivers that don't pass through the read_raw and write_raw callbacks.

It might be possible to make the userspace interfaces optional without too much pain.

I know it's not ideal, but at the end of the day IIO had a rather different target when we wrote it from SoC ADCs. That target of consistent userspace interfaces and brute force data capture still has to be met without introducing major regressions.

At the IIO layer, but an ADC layer itself needs very very little indeed.

You've got

	allocate
	deallocate
	read_samples (block/nonblock)
	setup
	->samples() callback

and devices are either polled, IRQ driven or DMA.

Now setup is a lot of different things but those can be abstracted and added as needed (and much probably taken from the IIO bits).

A pure ADC abstraction ought to be a very very thin layer of code.

I don't see the two conflicting. At one level we have a need for a simple abstraction for low level ADC access within devices (akin to gpio). At the level above we have a need for a consistent, sensible interface to userspace with a stable API.

Your simple IIO examples would just use the ADC abstraction, your complex IIO examples would use the ADC abstraction *and* layer it with IIO level code that is mixing it with all the other needed work.

Alan

To add a few more things that are common (there are others).

Read scale, read offset.
Hardware event interrupts,
Triggering control.
Filtering control.

Some adcs may only need what you specify, others need a whole lot more. You might term these setup I suppose, but the consumer of the data often needs to know about them.

Except that you then end up with simple_adc abstraction and a whole host of more complex abstractions on top.

We have that simple abstraction. Dumb polled or irq driven adc stuff can be done cleanly in minimal code.
What I disagree on is that the bit you have grouped into setup is actually separate. That needs to be abstracted as well. Consumers might not care that the gain just doubled because someone else requested it, but I suspect many of them will.

I suspect you'll end up adding more and more to your adc abstraction till you actually
end up with most of IIO. That's effectively what we did... It's big because there are
actually not that many 'simple' adc's out there.

Jonathan

At the bottom layer I'd expect a second consumer of the same data to get -EBUSY, but you are then going to tell me there are ADCs with one gain control for several channels no doubt 8)

Fair enough - you would be the expert there.

Alan

yup. If you can think of it, someone hardware guy will have implemented it! In this case often comes down to the fact that most adcs are actually a mux on the front of a single channel and the pga may be before the mux or after it (or for a laugh, sometimes both).

A real 'fun' one is devices that do scan reads. That is you can only read certain combinations of channels at a time and sometimes those combinations are far from obvious...

I just wish they were all simple!

There's also some sensible use cases like the multiple system monitoring subsystems we've got wanting to monitor the same power rail where it's useful to have multiple consumers sharing the ADC, though with those ones changing the gain probably doesn't make much sense.

I tend to agree here - I think if we try to establish a strict separation between the simple and complex abstractions it'd cause more problems than it will solve trying to split things, and from a hardware driver level it helps if there's just one upper layer.

If only it separated that well. There are numerous bits of iio that need to hack into the core structures still. I wish there weren't but they are there.

I have no problem with additional abstractions below. What I want to avoid is having two equivalent interfaces.

Just to restate this. It doesn't go in drivers/adc. That would cover a tiny corner of the scope of IIO and make the ripped out bit effectively useless for the rest. We ended up with IIO because we couldn't come up with a term that meant input / output devices that are analog (in some sense) but not hwmon or human input related. Other name suggestions welcome!

I'm not against this in principle but don't have the time and it will have to be carefully handled to not end up with something so limited that it is useless for IIO's current drivers. Now days there is actually relatively little bloat in the IIO core.

In the short term I'll look at making the sysfs interfaces a build time option for a driver or two.

As ever it comes down to man hours.

No problem with that. The recent core changes have all been about adding enough
support for the cases Mark raised (which are pretty much the same as here).