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@xxxxxxxxx> > >>>>>>>>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:
signature.asc
Description: Digital signature
