Re: [PATCH 1/4] Documentation: tps65086: Add DT bindings for the TPS65086 PMIC

From: Andrew F. Davis
Date: Tue Oct 20 2015 - 10:03:24 EST


On 10/20/2015 06:31 AM, Lee Jones wrote:
On Mon, 19 Oct 2015, Andrew F. Davis wrote:

On 10/19/2015 10:21 AM, Lee Jones wrote:
On Mon, 19 Oct 2015, Andrew F. Davis wrote:

On 10/19/2015 04:13 AM, Lee Jones wrote:
On Fri, 16 Oct 2015, Andrew F. Davis wrote:

The TPS65086 PMIC contains several regulators and a GPO controller.
Add bindings for the TPS65086 PMIC.

Signed-off-by: Andrew F. Davis <afd@xxxxxx>
---
.../devicetree/bindings/gpio/gpio-tps65086.txt | 17 ++++++++
Documentation/devicetree/bindings/mfd/tps65086.txt | 46 ++++++++++++++++++++++
.../bindings/regulator/tps65086-regulator.txt | 36 +++++++++++++++++

Please split these up into separate patches.

There is no functional reason to bundle them up.


ACK

3 files changed, 99 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/gpio-tps65086.txt
create mode 100644 Documentation/devicetree/bindings/mfd/tps65086.txt
create mode 100644 Documentation/devicetree/bindings/regulator/tps65086-regulator.txt

[...]

diff --git a/Documentation/devicetree/bindings/mfd/tps65086.txt b/Documentation/devicetree/bindings/mfd/tps65086.txt
new file mode 100644
index 0000000..4b6aeb4
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/tps65086.txt
@@ -0,0 +1,46 @@
+* TPS65086 Power Management Integrated Circuit bindings
+
+Required properties:
+ - compatible : Should be "ti,tps65086".

Any indication that it's a PMIC?


In the compatible string?

Ya.


Not sure what you mean then?, no one else seems to be doing that,
"xx,xxxxxxx-pmic" is usually used for matching the regulator node,
not the device itself.

Either the driver is MFD is the PMIC or it's not.

If it is, the compatible should reflect that, if isn't not then the
description in the header comment and the one above is not correct.

IMO, 'pmic' should not be used in the regulator compatible strings, as
it's a general description of the overall device. The regulators are
just a component of that device.


I agree about not using compatible in regulator nodes (Mark Brown and I had
a discussion on this topic), what I mean with "xxxxxxx-pmic" is for the MFD
core to match sub-drivers (mfd_cells). For example drivers/mfd/tps80031.c,
matches the regulator with "tps80031-pmic", the clock with "tps80031-clock",
the charger with "tps80031-charger", etc..

The core device itself is just matched with "ti,tps65086" through the I2C bus
matching.

I could change the core to be "tps65086-pmic", then call the regulator
driver "tps65086-regulator" if this works for you, this seems to be the
way new drivers name the regulator driver (max77843.c).

+ - reg : Slave address.

I2C/SPI?


ACK

+ - interrupt-parent : The parent interrupt controller.

Phandled to ...


ACK

+ - interrupts : The interrupt line the device is connected to.
+ - interrupt-controller : Marks the device node as an interrupt controller.
+ - #interrupt-cells : The number of cells to describe an IRQ, this
+ should be 2. The first cell is the IRQ number.
+ The second cell is the flags, encoded as the trigger
+ masks from ../interrupt-controller/interrupts.txt.

Masks? What masks?

Best to make a link to the header where the flags are defined here.


ACK

+Additional nodes defined in:
+ - Regulators : ../regulator/tps65086-regulator.txt.
+ - GPIO : ../gpio/gpio-tps65086.txt.

I'd suggest removing the full stops from all of the lines above.

Just treat them as bullet points like we normally do.


ACK

+Example:
+
+ pmic: tps65086@5e {
+ compatible = "ti,tps65086";
+ reg = <0x5e>;
+ interrupt-parent = <&gpio1>;
+ interrupts = <28 IRQ_TYPE_LEVEL_LOW>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+
+ regulators {
+ compatible = "ti,tps65086-regulator";
+
+ buck1 {
+ regulator-name = "vcc1";
+ regulator-min-microvolt = <1600000>;
+ regulator-max-microvolt = <1600000>;
+ regulator-boot-on;
+ ti,regulator-decay;
+ ti,regulator-step-size-25mv;
+ };
+ };
+
+ gpio4: tps65086_gpio {
+ compatible = "ti,tps65086-gpio";
+ gpio-controller;
+ #gpio-cells = <2>;
+ };
+ };

[...]





--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/