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

[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 at 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?

> >>>>>For the triggers, I am not decided. An obvious benifit to remove
> >>>>>trigger in dt will save many lines of code.
> >>>>>
> >>>>>>Maxime
> >>>>>>
> >>>>>Best Regards,
> >>>>>Josh Wu
> >>>Since we are talking about reworking this binding I was thinking about
> >>>resolution stuff. Filling atmel,adc-res is also copying constant value from
> >>>the device datasheet, so if I was consistent I would say it has to be removed
> >>>too. But I am not consistent! I mean by doing this the only thing the user
> >>>will have to fill is the resolution value. He can't set the value he wants,
> >>>there are only two choices. By keeping it into the dt then he will immediately
> >>>see the choices he has.
> >>But the resolution should probably be somehow user-defined, probably not
> >>really related to the DT has well. I think some other IIO ADC drivers
> >>are using sysfs files for this purpose, maybe that would be a better
> >>fit?
> >It sounds to be a good solution.
> 
> ok, I will check the other IIO ADC driver about this point.
> Maybe this sysfs replacement need a bit more time. I prefer to send
> out the patches first without the sysfs implement in v3.
> And the sysfs replacement patch will be send out serperately. What
> do you think? Maxime.

Yes, of course. The resolution rework can definitely be done later.

Maxime

-- 
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/09e64a41/attachment.sig>