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

Hi Nicolas,

On Tue, Aug 27, 2013 at 11:05:22AM +0200, Nicolas Ferre wrote:

On 27/08/2013 10:15, Maxime Ripard :

quoted

On Mon, Aug 26, 2013 at 06:03:31PM +0800, Josh Wu wrote:

quoted

Hi, Ludovic and Maxime

On 8/26/2013 4:32 PM, Ludovic Desroches wrote:

quoted

On Fri, Aug 23, 2013 at 06:59:36PM +0200, Maxime Ripard wrote:

quoted

Hi Ludovic, Josh,

On Fri, Aug 23, 2013 at 05:46:03PM +0200, Desroches, Ludovic wrote:

quoted

On Thu, Aug 22, 2013 at 05:53:00PM +0800, Josh Wu wrote:

quoted

On 8/22/2013 5:51 PM, Josh Wu wrote:

quoted

Hi, Maxime

On 8/16/2013 3:20 AM, Maxime Ripard wrote:

quoted

Hi Josh,

On Sun, Aug 11, 2013 at 07:04:29PM +0800, Josh Wu wrote:

quoted

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 <redacted>

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130827/155d6cfd/attachment-0001.sig>