Re: [PATCH v2] max77693: added device tree support

[Lee Jones <lee.jones@linaro.org>]

On Mon, 19 Aug 2013, Stephen Warren wrote:

> On 08/19/2013 05:40 AM, Andrzej Hajda wrote:
> > max77693 mfd main device uses only wakeup field
> > from max77693_platform_data. This field is mapped
> > to wakeup-source common property in device tree.
> 
> > diff --git a/Documentation/devicetree/bindings/mfd/max77693.txt b/Documentation/devicetree/bindings/mfd/max77693.txt
> 
> >  Optional properties:
> >  - regulators : The regulators of max77693 have to be instantiated under subnod
> >    named "regulators" using the following format.
> > +- wakeup-source : Indicates if the device can wakeup the system from the sleep
> > +  state.
> 
> Does the property mean "can" or "should"?
> 
> "Can" implies that the property means something about the HW. What
> exactly does it mean; perhaps that some specific output pin of the chip
> has been wired to an input IRQ/GPIO of the SoC or PMIC that (can) wake
> up the system? If so, which pin, signal, ...? Also, doesn't this also
> depend on the SoC itself supporting its input IRQ/GPIO as a wakeup
> source, so isn't some co-ordination required between the SoC and chip,
> such that this property doesn't mean "can wakeup the system", but simply
> "a signal is routed to the SoC, so perhaps it can wakeup the system".
> 
> "Should" implies policy, which probably shouldn't be represented in
> device tree, since DT should describe the HW and not how it should be used.
> 
> Finally, if there was already a binding for max77693.txt, I don't think
> the patch subject "added device tree support" is entirely accurate; this
> change to the binding document seems to be more about adding a new
> feature than adding DT support to the driver...

I'm taking this as a NACK.

Once Stephen is happy I'll reapply any RESEND with his Ack.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog