On 27/11/15 15:08, Ludovic Desroches wrote: > Hi, > > I want to add support for the ADC device of the SAMA5D2 SoC. I may > write a new driver because they are some changes about the register > contents. Moreover, the new device doesn't fit the dt bindings (which are > not very good since there is configuration stuff which is not > relating to the SoC or to the board). > > My main concern is how to adress the configuration of my ADC, some > examples: > - choosing the resolution (some resolutions will impact the sampling > time) The nasty here comes with the buffers. Right now we are defining the resolution of a given element to be fixed. Clearly in some SoC ADCs this isn't really true (in stand alone ones it pretty much always is fixed). Partly this then depends on the data layout - if it is such that we can effectively ignore it in describing the channel (i.e. the other bits are 0 anyway) then we tend to use _scale attributes to control the resolution. I'm not against having additional ABI for this, as long as it is clear. We can add a callback to override the buffer descriptions with a runtime built version as long as the exposed ABI doesn't change. Having quickly browsed the datasheet we are talking 12,13,14 bits, done by oversampling. We have control for oversampling via an info_mask element so the control is fine. Looks like values are sign extended to 16bits which is great as well as it means the buffer logic doesn't even have to know that different resolutions are supported. > - enabling the sleep mode or not This is an irritatingly open question kernel wide. It basically boils down to a 'low power mode' or not I guess. Every now and then someone suggests a general kernel twiddle knob for this, but right now we end up with a mess of different methods. To a degree it depends on the driver writer to make a sensible decision on whether sleep mode is too costly or not. If it's not then don't turn it on... Intermediate tricks to do with allowing a window in which it is disabled (runtime pm based normally) work well sometimes. Here it looks like you could work out whether there is time to enable it for some types of trigger (timing based ones). If so enable it, if not don't. Hard for other trigger types however... > - using offset correction Ah, found this in the datasheet (wasn't initially sure what you meant). Classic offset and gain, use calibscale and calibbias for this for the simple cases. The final mode is 'novel' and will need some new ABI or perhaps just figuring out which of the gain modes will work for a given input value... Not sure. > - signed or unsigned conversions That should be defined by the hardware...? Or are we talking differential vs single ended conversion? If that then have two lots of channels for the the two cases and enable / disable as relevant - lots of standard ADC drivers do this. Something like the max1363 driver should be a good example. > - etc. > > Having a look to other adc drivers, it seems resolution only depends on the > SoC. > Is there a generic solution? Not really - sometimes it really is soc dependent - e.g. a given soc has only one option, but in the flexible case we don't have a good solution. > If not, can I create the files I need for my > device in sysfs? Yes > > Which adc driver would be the best to take as a reference? For a generic ADC I'd normally say either the max1363 or the various analog devices parts. It gets less obvious for the quirks of a SoC - they all seem to be special cases! > > If you have some interesting discussions, I am interested too. I have > browsed the iio mainling list up to september but I have not found > answers. Yeah, we aren't the best at documentation and things tend to get burried over the years on the mailing list! A few other points to note from the datasheet for this one - 1) You have touchscreen specific hardware in there. Is the plan a combined driver? Looks fairly standard so that might be the easiest option. One day I'll figure out if this stuff is generic enough across SoCs to do it as an IIO consumer with a few magic extra bits but for now a combined driver is probably easiest (though you then need to get it past both sets of reviewers!) 2) Lots of trigger options here - you can register as many triggers as you likes so this should be fine. You'll need to prevent those that never reach userspace from being used by other device however using the validate callbacks. 3) Dma support - I've not pinned down how useful this is yet, but the recent dma buffers support from Lars may be useful here. Looks like it does tightly packed channels in a nice linear buffer so looks good. I got lost in the description however so will leave figuring this out to you ;) 4) I love the bit where you can set the differential channels to be unsigned and the single ended ones signed.... Sometimes it's best to not admit that the hardware allows bonkers options ;) 5) Repeated channel reads in the sequencer. We don't currently provide any means of doing this as it's horendous to design a generic interface around and relatively rarely makes much sense in an application. 6) Events support - looks straight forward to support he threshold stuff. 7) The last channel measurement trigger stuff is 'unusual' - but not hard to support - just give it it's own integration time I guess and set this magic stuff if it differs from other channels. Anyhow, nice enough looking ADC with not to 'silly' a hardware interface. > > Thanks for you help > > Regards > > Ludovic > -- > 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 > -- 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