| Message ID | 1391875428-6281-4-git-send-email-carlo@caione.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> Bindings documentation for the axp20x driver. In this file also two > sub-nodes (PEK and regulators) are documented. > > Signed-off-by: Carlo Caione <carlo@caione.org> > --- > Documentation/devicetree/bindings/mfd/axp20x.txt | 87 ++++++++++++++++++++++++ You need to CC the Device Tree guys.
Hi Carlo, On Sat, Feb 08, 2014 at 05:03:48PM +0100, Carlo Caione wrote: > Bindings documentation for the axp20x driver. In this file also two > sub-nodes (PEK and regulators) are documented. > > Signed-off-by: Carlo Caione <carlo@caione.org> > --- > Documentation/devicetree/bindings/mfd/axp20x.txt | 87 ++++++++++++++++++++++++ > 1 file changed, 87 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mfd/axp20x.txt > > diff --git a/Documentation/devicetree/bindings/mfd/axp20x.txt b/Documentation/devicetree/bindings/mfd/axp20x.txt > new file mode 100644 > index 0000000..ccea6b8 > --- /dev/null > +++ b/Documentation/devicetree/bindings/mfd/axp20x.txt > @@ -0,0 +1,87 @@ > +* axp20x device tree bindings > + > +The axp20x family current members :- > +axp202 (x-powers) > +axp209 (x-powers) > + > +Required properties: > +- compatible : Should be "x-powers,axp20x" (for axp202 and axp209) "Generic" compatibles are usually a bad thing, for several reasons, mostly because there's no way to actually differentiate the two without keeping adding DT properties (and hence, being unable to actually fix something or add a quirk for one single of these devices without having to modify the DT too.) Please use the "real" compatibles. > +- interrupt-controller : axp20x has its own internal IRQs > +- #interrupt-cells : should be set to 1 > +- interrupt-parent : The parent interrupt controller Is this really required? It was not in your DTSI. > +- interrupts : Specifies the list of interrupt lines which are handled by > + the device in the parent controller's notation Hmmm, I'm not sure what you mean here. > +- reg : Specifies base physical address and size of the registers Base physical address? Isn't it a I2C device? > + > +Sub-nodes > +* regulators : Contain the regulator nodes. The regulators are bound using > + their name as listed here: dcdc2, dcdc3, ldo1, ldo2, ldo3, > + ldo4, ldo5. > + The bindings details of individual regulator device can be found in: > + Documentation/devicetree/bindings/regulator/regulator.txt with the > + exception of: I'm guessing this is where you differentiate between AXP202 and AXP209? > + > + - dcdc-freq : defines the work frequency of DC-DC where > + F=(1+dcdc-freq*5%)*1.5MHz I'd very much prefer this DCDC frequency to be in Hz, or a similar unit, and let the driver do the conversion. > + > +* axp20x-pek : Power Enable Key > + - compatible : should be "x-powers,axp20x-pek" > + - interrupts : two interrupt numbers with order defined by interrupt-names > + (one irq number for rising transition of the power key, the > + other one for falling transition) > + - interrupt-names : should be "PEK_DBR" and "PEK_DBF" Is this actually needed since you declare the resources in your driver? > + > +Example: > + > +axp { > + compatible = "x-powers,axp20x"; > + interrupt-controller; > + #interrupt-cells = <1>; No reg property ? > + > + axp20x-pek { > + compatible = "x-powers,axp20x-pek"; > + interrupts = <33>, <34>; > + interrupt-names = "PEK_DBR", "PEK_DBF"; > + }; > + > + regulators { > + dcdc-freq = "8"; > + > + axp_dcdc2: dcdc2 { > + regulator-min-microvolt = <700000>; > + regulator-max-microvolt = <2275000>; > + dcdc-workmode = <0>; And what is this dcdc-workmode property about? > + }; > + > + axp_dcdc3: dcdc3 { > + regulator-min-microvolt = <700000>; > + regulator-max-microvolt = <3500000>; > + dcdc-workmode = <0>; > + }; > + > + axp_ldo1: ldo1 { > + regulator-min-microvolt = <1300000>; > + regulator-max-microvolt = <1300000>; > + }; > + > + axp_ldo2: ldo2 { > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <3300000>; > + }; > + > + axp_ldo3: ldo3 { > + regulator-min-microvolt = <700000>; > + regulator-max-microvolt = <3500000>; > + }; > + > + axp_ldo4: ldo4 { > + regulator-min-microvolt = <1250000>; > + regulator-max-microvolt = <3300000>; > + }; > + > + axp_ldo5: ldo5 { > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <3300000>; > + }; > + }; > +}; > -- > 1.8.5.3 > Thanks for working on this! Maxime
On Mon, Feb 10, 2014 at 9:12 PM, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > Hi Carlo, > > On Sat, Feb 08, 2014 at 05:03:48PM +0100, Carlo Caione wrote: >> Bindings documentation for the axp20x driver. In this file also two >> sub-nodes (PEK and regulators) are documented. >> >> Signed-off-by: Carlo Caione <carlo@caione.org> >> --- >> Documentation/devicetree/bindings/mfd/axp20x.txt | 87 ++++++++++++++++++++++++ >> 1 file changed, 87 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/mfd/axp20x.txt >> >> diff --git a/Documentation/devicetree/bindings/mfd/axp20x.txt b/Documentation/devicetree/bindings/mfd/axp20x.txt >> new file mode 100644 >> index 0000000..ccea6b8 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/mfd/axp20x.txt >> @@ -0,0 +1,87 @@ >> +* axp20x device tree bindings >> + >> +The axp20x family current members :- >> +axp202 (x-powers) >> +axp209 (x-powers) >> + >> +Required properties: >> +- compatible : Should be "x-powers,axp20x" (for axp202 and axp209) > > "Generic" compatibles are usually a bad thing, for several reasons, > mostly because there's no way to actually differentiate the two > without keeping adding DT properties (and hence, being unable to > actually fix something or add a quirk for one single of these devices > without having to modify the DT too.) > > Please use the "real" compatibles. Ok >> +- interrupt-controller : axp20x has its own internal IRQs >> +- #interrupt-cells : should be set to 1 >> +- interrupt-parent : The parent interrupt controller > > Is this really required? It was not in your DTSI. It wasn't in my DTSI since it was supposed to be used by DTS importing this DTSI. I'll fix it in v2. >> +- interrupts : Specifies the list of interrupt lines which are handled by >> + the device in the parent controller's notation > > Hmmm, I'm not sure what you mean here. It means that the notation used to indicate the interrupt for this device has to follow the notation of the parent controller (i.e. in terms of number of interrupt cells). I'll rewrite the statement in v2. >> +- reg : Specifies base physical address and size of the registers > > Base physical address? Isn't it a I2C device? Right. cut&paste error. >> + >> +Sub-nodes >> +* regulators : Contain the regulator nodes. The regulators are bound using >> + their name as listed here: dcdc2, dcdc3, ldo1, ldo2, ldo3, >> + ldo4, ldo5. >> + The bindings details of individual regulator device can be found in: >> + Documentation/devicetree/bindings/regulator/regulator.txt with the >> + exception of: > > I'm guessing this is where you differentiate between AXP202 and > AXP209? Not really. AXP202 and AXP209 have the same regulators (with the same constrains) >> + >> + - dcdc-freq : defines the work frequency of DC-DC where >> + F=(1+dcdc-freq*5%)*1.5MHz > > I'd very much prefer this DCDC frequency to be in Hz, or a similar > unit, and let the driver do the conversion. Ok. Fix in v2. >> + >> +* axp20x-pek : Power Enable Key >> + - compatible : should be "x-powers,axp20x-pek" >> + - interrupts : two interrupt numbers with order defined by interrupt-names >> + (one irq number for rising transition of the power key, the >> + other one for falling transition) >> + - interrupt-names : should be "PEK_DBR" and "PEK_DBF" > > Is this actually needed since you declare the resources in your driver? Not needed. >> + >> +Example: >> + >> +axp { >> + compatible = "x-powers,axp20x"; >> + interrupt-controller; >> + #interrupt-cells = <1>; > > No reg property ? Also the reg property was supposed to be used in the DTS. I'll fix it. >> + >> + axp20x-pek { >> + compatible = "x-powers,axp20x-pek"; >> + interrupts = <33>, <34>; >> + interrupt-names = "PEK_DBR", "PEK_DBF"; >> + }; >> + >> + regulators { >> + dcdc-freq = "8"; >> + >> + axp_dcdc2: dcdc2 { >> + regulator-min-microvolt = <700000>; >> + regulator-max-microvolt = <2275000>; >> + dcdc-workmode = <0>; > > And what is this dcdc-workmode property about? Oh. I missed that, sorry. >> + }; >> + >> + axp_dcdc3: dcdc3 { >> + regulator-min-microvolt = <700000>; >> + regulator-max-microvolt = <3500000>; >> + dcdc-workmode = <0>; >> + }; >> + >> + axp_ldo1: ldo1 { >> + regulator-min-microvolt = <1300000>; >> + regulator-max-microvolt = <1300000>; >> + }; >> + >> + axp_ldo2: ldo2 { >> + regulator-min-microvolt = <1800000>; >> + regulator-max-microvolt = <3300000>; >> + }; >> + >> + axp_ldo3: ldo3 { >> + regulator-min-microvolt = <700000>; >> + regulator-max-microvolt = <3500000>; >> + }; >> + >> + axp_ldo4: ldo4 { >> + regulator-min-microvolt = <1250000>; >> + regulator-max-microvolt = <3300000>; >> + }; >> + >> + axp_ldo5: ldo5 { >> + regulator-min-microvolt = <1800000>; >> + regulator-max-microvolt = <3300000>; >> + }; >> + }; >> +}; >> -- >> 1.8.5.3 >> > > Thanks for working on this! Thank you,
On Mon, Feb 10, 2014 at 09:37:37PM +0100, Carlo Caione wrote: > >> +Sub-nodes > >> +* regulators : Contain the regulator nodes. The regulators are bound using > >> + their name as listed here: dcdc2, dcdc3, ldo1, ldo2, ldo3, > >> + ldo4, ldo5. > >> + The bindings details of individual regulator device can be found in: > >> + Documentation/devicetree/bindings/regulator/regulator.txt with the > >> + exception of: > > > > I'm guessing this is where you differentiate between AXP202 and > > AXP209? > > Not really. AXP202 and AXP209 have the same regulators (with the same > constrains) Ok. What's the difference between the two then? Thanks,
On Mon, Feb 10, 2014 at 11:01 PM, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > On Mon, Feb 10, 2014 at 09:37:37PM +0100, Carlo Caione wrote: >> >> +Sub-nodes >> >> +* regulators : Contain the regulator nodes. The regulators are bound using >> >> + their name as listed here: dcdc2, dcdc3, ldo1, ldo2, ldo3, >> >> + ldo4, ldo5. >> >> + The bindings details of individual regulator device can be found in: >> >> + Documentation/devicetree/bindings/regulator/regulator.txt with the >> >> + exception of: >> > >> > I'm guessing this is where you differentiate between AXP202 and >> > AXP209? >> >> Not really. AXP202 and AXP209 have the same regulators (with the same >> constrains) > > Ok. What's the difference between the two then? Honestly I have no idea. For what I have seen (core, PEK and regulators) AXP202 and AXP209 are identical.
On Sat, Feb 08, 2014 at 05:03:48PM +0100, Carlo Caione wrote: > Bindings documentation for the axp20x driver. In this file also two > sub-nodes (PEK and regulators) are documented. You probably want to CC bindings to the maintianers for the relevant subsystes too. > + - dcdc-freq : defines the work frequency of DC-DC where > + F=(1+dcdc-freq*5%)*1.5MHz It sees a bit nicer to either have this be the frequency and work back to the register setting or name it after the register field that gets updated as a result. I'm also not sure what *5% means - I'm guessing it's *0.05 but it'd be clearer to just write that.
On Mon, Feb 10, 2014 at 11:38:47PM +0100, Carlo Caione wrote: > On Mon, Feb 10, 2014 at 11:01 PM, Maxime Ripard > > Ok. What's the difference between the two then? > Honestly I have no idea. For what I have seen (core, PEK and > regulators) AXP202 and AXP209 are identical. I'd guess something in the electrical or mechanical specs (eg, maximum current draws or packaging which are often related anyway), it's quite common to get multiple PMICs that are silicon identical but differently packaged or binned.
diff --git a/Documentation/devicetree/bindings/mfd/axp20x.txt b/Documentation/devicetree/bindings/mfd/axp20x.txt new file mode 100644 index 0000000..ccea6b8 --- /dev/null +++ b/Documentation/devicetree/bindings/mfd/axp20x.txt @@ -0,0 +1,87 @@ +* axp20x device tree bindings + +The axp20x family current members :- +axp202 (x-powers) +axp209 (x-powers) + +Required properties: +- compatible : Should be "x-powers,axp20x" (for axp202 and axp209) +- interrupt-controller : axp20x has its own internal IRQs +- #interrupt-cells : should be set to 1 +- interrupt-parent : The parent interrupt controller +- interrupts : Specifies the list of interrupt lines which are handled by + the device in the parent controller's notation +- reg : Specifies base physical address and size of the registers + +Sub-nodes +* regulators : Contain the regulator nodes. The regulators are bound using + their name as listed here: dcdc2, dcdc3, ldo1, ldo2, ldo3, + ldo4, ldo5. + The bindings details of individual regulator device can be found in: + Documentation/devicetree/bindings/regulator/regulator.txt with the + exception of: + + - dcdc-freq : defines the work frequency of DC-DC where + F=(1+dcdc-freq*5%)*1.5MHz + +* axp20x-pek : Power Enable Key + - compatible : should be "x-powers,axp20x-pek" + - interrupts : two interrupt numbers with order defined by interrupt-names + (one irq number for rising transition of the power key, the + other one for falling transition) + - interrupt-names : should be "PEK_DBR" and "PEK_DBF" + +Example: + +axp { + compatible = "x-powers,axp20x"; + interrupt-controller; + #interrupt-cells = <1>; + + axp20x-pek { + compatible = "x-powers,axp20x-pek"; + interrupts = <33>, <34>; + interrupt-names = "PEK_DBR", "PEK_DBF"; + }; + + regulators { + dcdc-freq = "8"; + + axp_dcdc2: dcdc2 { + regulator-min-microvolt = <700000>; + regulator-max-microvolt = <2275000>; + dcdc-workmode = <0>; + }; + + axp_dcdc3: dcdc3 { + regulator-min-microvolt = <700000>; + regulator-max-microvolt = <3500000>; + dcdc-workmode = <0>; + }; + + axp_ldo1: ldo1 { + regulator-min-microvolt = <1300000>; + regulator-max-microvolt = <1300000>; + }; + + axp_ldo2: ldo2 { + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <3300000>; + }; + + axp_ldo3: ldo3 { + regulator-min-microvolt = <700000>; + regulator-max-microvolt = <3500000>; + }; + + axp_ldo4: ldo4 { + regulator-min-microvolt = <1250000>; + regulator-max-microvolt = <3300000>; + }; + + axp_ldo5: ldo5 { + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <3300000>; + }; + }; +};
Bindings documentation for the axp20x driver. In this file also two sub-nodes (PEK and regulators) are documented. Signed-off-by: Carlo Caione <carlo@caione.org> --- Documentation/devicetree/bindings/mfd/axp20x.txt | 87 ++++++++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 Documentation/devicetree/bindings/mfd/axp20x.txt