Re: [PATCH v2 2/4] iio: at91: Use different prescal, startup mask in MR for different IP

[Maxime Ripard <maxime.ripard@free-electrons.com>]

[-- Attachment #1: Type: text/plain, Size: 3976 bytes --]

Hi Nicolas,

On Tue, Aug 27, 2013 at 11:05:22AM +0200, Nicolas Ferre wrote:
> On 27/08/2013 10:15, Maxime Ripard :
> >On Mon, Aug 26, 2013 at 06:03:31PM +0800, Josh Wu wrote:
> >>Hi, Ludovic and Maxime
> >>
> >>On 8/26/2013 4:32 PM, Ludovic Desroches wrote:
> >>>On Fri, Aug 23, 2013 at 06:59:36PM +0200, Maxime Ripard wrote:
> >>>>Hi Ludovic, Josh,
> >>>>
> >>>>On Fri, Aug 23, 2013 at 05:46:03PM +0200, Desroches, Ludovic wrote:
> >>>>>On Thu, Aug 22, 2013 at 05:53:00PM +0800, Josh Wu wrote:
> >>>>>>On 8/22/2013 5:51 PM, Josh Wu wrote:
> >>>>>>>Hi, Maxime
> >>>>>>>
> >>>>>>>On 8/16/2013 3:20 AM, Maxime Ripard wrote:
> >>>>>>>>Hi Josh,
> >>>>>>>>
> >>>>>>>>On Sun, Aug 11, 2013 at 07:04:29PM +0800, Josh Wu wrote:
> >>>>>>>>>For at91 boards, there are different IPs for adc. Different IPs has
> >>>>>>>>>different STARTUP & PRESCAL mask in ADC_MR.
> >>>>>>>>>
> >>>>>>>>>This patch introduce the multiple compatible string for those
> >>>>>>>>>different IPs.
> >>>>>>>>>
> >>>>>>>>>Signed-off-by: Josh Wu <josh.wu@atmel.com>
> >>>>>>>>Overall it looks like the right ways, but I think we can take it a step
> >>>>>>>>further.
> >>>>>>>>
> >>>>>>>>I'd drop at least the atmel,adc-drdy-mask, atmel,adc-num-channels,
> >>>>>>>>atmel,adc-status-register, atmel,adc-trigger-register properties (and
> >>>>>>>>probably the triggers as well description as well).
> >>>>>>>yeah, right. Currently I want to drop following:
> >>>>>>>
> >>>>>>>atmel,adc-drdy-mask, atmel,adc-status-register,
> >>>>>>>atmel,adc-trigger-register, atmel,adc-channel-base
> >>>>>>>
> >>>>>>>For the adc-num-channels, I'd like to leave it in dt parameters.
> >>>>>>>It is a description for an adc capablity.
> >>>>>About this parameter, I'll remove it too from the dt bindings. To set it you
> >>>>>need to have a look to the datasheet and to copy a constant value into the
> >>>>>dt. From my point of view, it means than this parameter should be managed by
> >>>>>the driver and by the dt.
> >>>>>
> >>>>>On the other side since there are some dynamic allocation depending on this
> >>>>>parameter maybe it makes sense to keep it in the dt. If the user wants to use
> >>>>>only 2 channels why doing allocation for max channel number. By the way, this
> >>>>>case is only valid if he uses the two first channels.
> >>>>I don't recall it very well, is there any reason to not have it in the
> >>>>DT? Can the ADC channels be used for something else? Or is it just some
> >>>>IP-specific number of channels?
> >>>>
> >>>It is IP-specific. I don't see what benefit we could have to keep it in the DT?
> >>>But Josh seems to have arguments to keep it.
> >>
> >>I'm ok to remove the channel number. What I worried is there also
> >>has a channel-used mask in DT.
> >>This mask should be removed too if channel number is removed.
> >>So maybe we can also use the sysfs to set the mask.
> >
> >Indeed, that would make adc-channel-used irrelevant. But I'm not sure
> >the mask is useful at all. Just enable all the channels and that's it?
> 
> On the top of my head: keeping all channels enabled, won't it have
> an impact on:
> 1/ power consumption?
> 2/ minimum period of sampling for a particular channel in case of
>    continuous ADC trigger?

I don't know, you tell me ;)

This is exactly why I was asking.

> And do not forget that sysfs is an interface that is:
> - needing a userspace tool to be configured properly (even just an
>   echo xx)
> - hard to design
> - a strict ABI that can't be changed once deployed
> 
> But yes, it is true that if the user has to configure ADC at
> runtime, it is certainly an interface worth considering...

Yes, of course. But the resolution is something I'd expect to be
user-configurable, probably at runtime, and the DT doesn't make it very
convenient to achieve.

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]