diff mbox

[v7,7/7] doc: bindings: act8945a-charger: Update properties

Message ID 1471848557-27278-8-git-send-email-wenyou.yang@atmel.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Wenyou Yang Aug. 22, 2016, 6:49 a.m. UTC
Due the driver improvements, update the properties,
 - Remove "active-semi,check-battery-temperature" property.
 - Add the properties, "active-semi,irq_gpio"
   and "active-semi,lbo-gpios".

Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
Acked-by: Rob Herring <robh@kernel.org>
---

Changes in v7: None
Changes in v6: None
Changes in v5:
 - Add Acked-by tag.

Changes in v4:
 - Update the doc/binding for using "interrupts" property.

Changes in v3: None
Changes in v2: None

 .../bindings/power/supply/act8945a-charger.txt           | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

Comments

Sebastian Reichel Aug. 23, 2016, 3:03 a.m. UTC | #1
Hi,

On Mon, Aug 22, 2016 at 02:49:17PM +0800, Wenyou Yang wrote:
> Due the driver improvements, update the properties,
>  - Remove "active-semi,check-battery-temperature" property.
>  - Add the properties, "active-semi,irq_gpio"
>    and "active-semi,lbo-gpios".
> 
> Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
> Acked-by: Rob Herring <robh@kernel.org>

I wonder if it's too late to slightly break the DT API by
introducing a proper sub-device for the charger and having all those
charger specific details described there. Not being able to use the
devm_ functions because all properties are assigned to the parent
device is not very nice.

We could easily support reading timeouts and threshold information
from the both (parent and sub-device) and just mark the parent
binding deprecated. The only real API break would be
"active-semi,chglev-gpios", which is currently broken anyways in the
driver as far as I can see (I should have reviewed this more
carefully previously).

In other words I suggest the following:

pmic@5b {
    compatible = "active-semi,act8945a";
    reg = <0x5b>;
    status = "okay";

    /* the following 3 are deprecated, but still supported */
    //active-semi,input-voltage-threshold-microvolt = <6600>;
    //active-semi,precondition-timeout = <40>;
    //active-semi,total-timeout = <3>;

    /* no longer supported, never worked properly */
    //active-semi,chglev-gpios = <&pioA 12 GPIO_ACTIVE_HIGH>;

    /* this is new, mfd should register charger sub-dev independently
     * of the subnode for backwards compatibility. Then optional
     * gpios/irqs are not available. But those either never worked
     * or did not exist before this update. */
    charger {
        active-semi,chglev-gpios = <&pioA 12 GPIO_ACTIVE_HIGH>;
        active-semi,lbo-gpios = <&pioA 72 GPIO_ACTIVE_LOW>;
		interrupt-parent = <&pioA>;
        interrupts = <45 GPIO_ACTIVE_LOW>

        active-semi,input-voltage-threshold-microvolt = <6600>;
        active-semi,precondition-timeout = <40>;
        active-semi,total-timeout = <3>;
    };
};

-- Sebastian
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/power/supply/act8945a-charger.txt b/Documentation/devicetree/bindings/power/supply/act8945a-charger.txt
index bea254c..f9866c4 100644
--- a/Documentation/devicetree/bindings/power/supply/act8945a-charger.txt
+++ b/Documentation/devicetree/bindings/power/supply/act8945a-charger.txt
@@ -4,10 +4,15 @@  Required properties:
  - compatible: "active-semi,act8945a", please refer to ../mfd/act8945a.txt.
  - active-semi,chglev-gpios: charge current level phandle with args
    as described in ../gpio/gpio.txt.
+ - active-semi,lbo-gpios: specify the low battery voltage detect phandle
+   with args as as described in ../gpio/gpio.txt.
+ - interrupts: <a b> where a is the interrupt number and b is a
+   field that represents an encoding of the sense and level
+   information for the interrupt.
+ - interrupt-parent: the phandle for the interrupt controller that
+   services interrupts for this device.
 
 Optional properties:
- - active-semi,check-battery-temperature: boolean to check the battery
-   temperature or not.
  - active-semi,input-voltage-threshold-microvolt: unit: mV;
    Specifies the charger's input over-voltage threshold value;
    The value can be: 6600, 7000, 7500, 8000; default: 6600
@@ -27,9 +32,14 @@  Example:
 		status = "okay";
 
 		pinctrl-names = "default";
-		pinctrl-0 = <&pinctrl_charger_chglev>;
+		pinctrl-0 = <&pinctrl_charger_chglev &pinctrl_charger_lbo &pinctrl_charger_irq>;
+
 		active-semi,chglev-gpios = <&pioA 12 GPIO_ACTIVE_HIGH>;
+		active-semi,lbo-gpios = <&pioA 72 GPIO_ACTIVE_LOW>;
 		active-semi,input-voltage-threshold-microvolt = <6600>;
 		active-semi,precondition-timeout = <40>;
 		active-semi,total-timeout = <3>;
+		interrupt-parent = <&pioA>;
+		interrupts = <45 GPIO_ACTIVE_LOW>;
+
 	};