* [PATCH 0/9] Summit SMB3xx driver & device-tree @ 2020-03-29 16:15 David Heidelberg 2020-03-29 16:21 ` [PATCH 4/9] dt-bindings: power: supply: Add device-tree binding for Summit SMB3xx David Heidelberg 2020-03-29 16:21 ` [PATCH 9/9] arm: dts: qcom: apq8064-nexus7: Add smb345 charger node David Heidelberg 0 siblings, 2 replies; 9+ messages in thread From: David Heidelberg @ 2020-03-29 16:15 UTC (permalink / raw) To: Sebastian Reichel, Jonghwa Lee, Chanwoo Choi, Myungjoo Ham, Sumit Semwal, John Stultz, Vinay Simha BN, mika.westerberg, ramakrishna.pallala, Dmitry Osipenko, linux-pm, linux-kernel Cc: David Heidelberg, Rob Herring, devicetree We gathered existing patches, fixed and improved what we could and final result is an working charging driver with device-tree support for Nexus 7. At this moment charging works with: - Nexus 7 2012 (grouper and tilapia) - Nexus 7 2013 (flo and deb) - ... and there is more devices equiped with these chargers. David Heidelberg (7): power: supply: smb347-charger: Add delay before getting IRQSTAT power: supply: smb347-charger: Use resource-managed API dt-bindings: power: supply: Add device-tree binding for Summit SMB3xx power: supply: smb347-charger: Implement device-tree support power: supply: smb347-charger: Support SMB345 and SMB358 power: supply: smb347-charger: Remove virtual smb347-battery arm: dts: qcom: apq8064-nexus7: Add smb345 charger node Dmitry Osipenko (2): power: supply: smb347-charger: IRQSTAT_D is volatile power: supply: smb347-charger: Replace mutex with IRQ disable/enable .../power/supply/summit,smb347-charger.yaml | 224 ++++++++ .../boot/dts/qcom-apq8064-asus-nexus7-flo.dts | 22 +- drivers/power/supply/Kconfig | 6 +- drivers/power/supply/smb347-charger.c | 510 +++++++++--------- .../dt-bindings/power/summit,smb347-charger.h | 19 + 5 files changed, 526 insertions(+), 255 deletions(-) create mode 100644 Documentation/devicetree/bindings/power/supply/summit,smb347-charger.yaml create mode 100644 include/dt-bindings/power/summit,smb347-charger.h -- 2.25.0 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 4/9] dt-bindings: power: supply: Add device-tree binding for Summit SMB3xx 2020-03-29 16:15 [PATCH 0/9] Summit SMB3xx driver & device-tree David Heidelberg @ 2020-03-29 16:21 ` David Heidelberg 2020-04-10 16:49 ` Rob Herring 2020-03-29 16:21 ` [PATCH 9/9] arm: dts: qcom: apq8064-nexus7: Add smb345 charger node David Heidelberg 1 sibling, 1 reply; 9+ messages in thread From: David Heidelberg @ 2020-03-29 16:21 UTC (permalink / raw) To: Sebastian Reichel, Jonghwa Lee, Chanwoo Choi, Myungjoo Ham, Sumit Semwal, John Stultz, Vinay Simha BN, mika.westerberg, ramakrishna.pallala, Dmitry Osipenko, linux-pm, linux-kernel Cc: David Heidelberg, Rob Herring, devicetree Summit SMB3xx series is a Programmable Switching Li+ Battery Charger. This device-tree binding documents Summit SMB345, SMB347 and SMB358 chargers. Signed-off-by: David Heidelberg <david@ixit.cz> --- .../power/supply/summit,smb347-charger.yaml | 224 ++++++++++++++++++ .../dt-bindings/power/summit,smb347-charger.h | 19 ++ 2 files changed, 243 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/supply/summit,smb347-charger.yaml create mode 100644 include/dt-bindings/power/summit,smb347-charger.h diff --git a/Documentation/devicetree/bindings/power/supply/summit,smb347-charger.yaml b/Documentation/devicetree/bindings/power/supply/summit,smb347-charger.yaml new file mode 100644 index 000000000000..1d6bccdcd233 --- /dev/null +++ b/Documentation/devicetree/bindings/power/supply/summit,smb347-charger.yaml @@ -0,0 +1,224 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/power/supply/summit,smb347-charger.yaml#" +$schema: "http://devicetree.org/meta-schemas/core.yaml#" + +title: Battery charger driver for SMB345, SMB347 and SMB358 + +maintainers: + - David Heidelberg <david@ixit.cz> + +properties: + compatible: + enum: + - summit,smb345 + - summit,smb347 + - summit,smb358 + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + summit,enable-usb-charging: + type: boolean + description: Enable charging trough USB. + + summit,enable-otg-charging: + type: boolean + description: Provide power for USB OTG + + summit,enable-mains-charging: + type: boolean + description: Enable charging trough mains + + summit,enable-chg-ctrl: + description: Enable charging control + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32 + - enum: + - 0 # SMB3XX_CHG_ENABLE_SW SW (I2C interface) + - 1 # SMB3XX_CHG_ENABLE_PIN_ACTIVE_LOW Pin control (Active Low) + - 2 # SMB3XX_CHG_ENABLE_PIN_ACTIVE_HIGH Pin control (Active High) + + summit,max-chg-curr: + description: Maximum current for charging (in uA) + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32 + + summit,max-chg-volt: + description: Maximum voltage for charging (in uV) + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 3500000 + maximum: 4500000 + + summit,pre-chg-curr: + description: Pre-charging current for charging (in uA) + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32 + + summit,term-curr: + description: Charging cycle termination current (in uA) + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32 + + summit,fast-volt-threshold: + description: Voltage threshold to transit to fast charge mode (in uV) + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 2400000 + maximum: 3000000 + + summit,mains-curr-limit: + description: Maximum input current from AC/DC input (in uA) + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32 + + summit,usb-curr-limit: + description: Maximum input current from USB input (in uA) + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32 + + summit,chg-curr-comp: + description: Charge current compensation (in uA) + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32 + + summit,chip-temp-threshold: + description: Chip temperature for thermal regulation in °C. + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32 + enum: [100, 110, 120, 130] + + summit,soft-cold-temp-limit: + description: Cold battery temperature in °C for soft alarm. + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32 + enum: [0, 5, 10, 15] + + summit,soft-hot-temp-limit: + description: Hot battery temperature in °C for soft alarm. + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32 + enum: [40, 45, 50, 55] + + summit,hard-cold-temp-limit: + description: Cold battery temperature in °C for hard alarm. + allOf: + - $ref: /schemas/types.yaml#/definitions/int32 + enum: [-5, 0, 5, 10] + + summit,hard-hot-temp-limit: + description: Hot battery temperature in °C for hard alarm. + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32 + enum: [50, 55, 60, 65] + + summit,soft-comp-method: + description: Soft temperature limit compensation method + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32 + - enum: + - 0 # SMB3XX_SOFT_TEMP_COMPENSATE_NONE Compensation none + - 1 # SMB3XX_SOFT_TEMP_COMPENSATE_CURRENT Current compensation + - 2 # SMB3XX_SOFT_TEMP_COMPENSATE_VOLTAGE Voltage compensation + +allOf: + - if: + properties: + compatible: + enum: + - summit,smb345 + - summit,smb358 + + then: + properties: + summit,max-chg-curr: + enum: [ 200000, 450000, 600000, 900000, + 1300000, 1500000, 1800000, 2000000] + + summit,pre-chg-curr: + enum: [150000, 250000, 350000, 450000] + + summit,term-curr: + enum: [ 30000, 40000, 60000, 80000, + 100000, 125000, 150000, 200000] + + summit,mains-curr-limit: + enum: [ 300000, 500000, 700000, 1000000, + 1500000, 1800000, 2000000] + + summit,usb-curr-limit: + enum: [ 300000, 500000, 700000, 1000000, + 1500000, 1800000, 2000000] + + summit,chg-curr-comp: + enum: [200000, 450000, 600000, 900000] + + else: + properties: + summit,max-chg-curr: + enum: [ 700000, 900000, 1200000, 1500000, + 1800000, 2000000, 2200000, 2500000] + + summit,pre-chg-curr: + enum: [100000, 150000, 200000, 250000] + + summit,term-curr: + enum: [ 37500, 50000, 100000, 150000, + 200000, 250000, 500000, 600000] + + summit,mains-curr-limit: + enum: [ 300000, 500000, 700000, 900000, 1200000, + 1500000, 1800000, 2000000, 2200000, 2500000] + + summit,usb-curr-limit: + enum: [ 300000, 500000, 700000, 900000, 1200000, + 1500000, 1800000, 2000000, 2200000, 2500000] + + summit,chg-curr-comp: + enum: [250000, 700000, 900000, 1200000] + +required: + - compatible + - reg + +anyOf: + - required: + - summit,enable-usb-charging + - required: + - summit,enable-otg-charging + - required: + - summit,enable-mains-charging + +additionalProperties: false + +examples: + - | + #include <dt-bindings/power/summit,smb347-charger.h> + + i2c { + #address-cells = <1>; + #size-cells = <0>; + + charger@7f { + compatible = "summit,smb347"; + reg = <0x7f>; + status = "okay"; + + summit,max-chg-curr = <1800000>; + summit,mains-curr-limit = <2000000>; + summit,usb-curr-limit = <500000>; + + summit,chip-temp-threshold = <110>; + summit,soft-cold-temp-limit = <5>; + + summit,enable-usb-charging; + summit,enable-mains-charging; + + summit,enable-chg-ctrl = <SMB3XX_CHG_ENABLE_PIN_ACTIVE_HIGH>; + }; + }; diff --git a/include/dt-bindings/power/summit,smb347-charger.h b/include/dt-bindings/power/summit,smb347-charger.h new file mode 100644 index 000000000000..d918bf321a71 --- /dev/null +++ b/include/dt-bindings/power/summit,smb347-charger.h @@ -0,0 +1,19 @@ +/* SPDX-License-Identifier: (GPL-2.0-or-later or MIT) */ +/* + * Author: David Heidelberg <david@ixit.cz> + */ + +#ifndef _DT_BINDINGS_SMB347_CHARGER_H +#define _DT_BINDINGS_SMB347_CHARGER_H + +/* Charging compensation method */ +#define SMB3XX_SOFT_TEMP_COMPENSATE_NONE 0 +#define SMB3XX_SOFT_TEMP_COMPENSATE_CURRENT 1 +#define SMB3XX_SOFT_TEMP_COMPENSATE_VOLTAGE 2 + +/* Charging enable control */ +#define SMB3XX_CHG_ENABLE_SW 0 +#define SMB3XX_CHG_ENABLE_PIN_ACTIVE_LOW 1 +#define SMB3XX_CHG_ENABLE_PIN_ACTIVE_HIGH 2 + +#endif -- 2.25.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 4/9] dt-bindings: power: supply: Add device-tree binding for Summit SMB3xx 2020-03-29 16:21 ` [PATCH 4/9] dt-bindings: power: supply: Add device-tree binding for Summit SMB3xx David Heidelberg @ 2020-04-10 16:49 ` Rob Herring 2020-04-10 19:02 ` Dmitry Osipenko 0 siblings, 1 reply; 9+ messages in thread From: Rob Herring @ 2020-04-10 16:49 UTC (permalink / raw) To: David Heidelberg Cc: Sebastian Reichel, Jonghwa Lee, Chanwoo Choi, Myungjoo Ham, Sumit Semwal, John Stultz, Vinay Simha BN, mika.westerberg, ramakrishna.pallala, Dmitry Osipenko, linux-pm, linux-kernel, devicetree On Sun, Mar 29, 2020 at 06:21:23PM +0200, David Heidelberg wrote: > Summit SMB3xx series is a Programmable Switching Li+ Battery Charger. > This device-tree binding documents Summit SMB345, SMB347 and SMB358 chargers. > > Signed-off-by: David Heidelberg <david@ixit.cz> > --- > .../power/supply/summit,smb347-charger.yaml | 224 ++++++++++++++++++ > .../dt-bindings/power/summit,smb347-charger.h | 19 ++ > 2 files changed, 243 insertions(+) > create mode 100644 Documentation/devicetree/bindings/power/supply/summit,smb347-charger.yaml > create mode 100644 include/dt-bindings/power/summit,smb347-charger.h > > diff --git a/Documentation/devicetree/bindings/power/supply/summit,smb347-charger.yaml b/Documentation/devicetree/bindings/power/supply/summit,smb347-charger.yaml > new file mode 100644 > index 000000000000..1d6bccdcd233 > --- /dev/null > +++ b/Documentation/devicetree/bindings/power/supply/summit,smb347-charger.yaml > @@ -0,0 +1,224 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: "http://devicetree.org/schemas/power/supply/summit,smb347-charger.yaml#" > +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > + > +title: Battery charger driver for SMB345, SMB347 and SMB358 > + > +maintainers: > + - David Heidelberg <david@ixit.cz> > + > +properties: > + compatible: > + enum: > + - summit,smb345 > + - summit,smb347 > + - summit,smb358 > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + summit,enable-usb-charging: > + type: boolean > + description: Enable charging trough USB. > + > + summit,enable-otg-charging: > + type: boolean > + description: Provide power for USB OTG > + > + summit,enable-mains-charging: > + type: boolean > + description: Enable charging trough mains > + > + summit,enable-chg-ctrl: > + description: Enable charging control > + allOf: > + - $ref: /schemas/types.yaml#/definitions/uint32 > + - enum: > + - 0 # SMB3XX_CHG_ENABLE_SW SW (I2C interface) > + - 1 # SMB3XX_CHG_ENABLE_PIN_ACTIVE_LOW Pin control (Active Low) > + - 2 # SMB3XX_CHG_ENABLE_PIN_ACTIVE_HIGH Pin control (Active High) > + > + summit,max-chg-curr: > + description: Maximum current for charging (in uA) > + allOf: > + - $ref: /schemas/types.yaml#/definitions/uint32 > + > + summit,max-chg-volt: > + description: Maximum voltage for charging (in uV) > + allOf: > + - $ref: /schemas/types.yaml#/definitions/uint32 > + minimum: 3500000 > + maximum: 4500000 > + > + summit,pre-chg-curr: > + description: Pre-charging current for charging (in uA) > + allOf: > + - $ref: /schemas/types.yaml#/definitions/uint32 > + > + summit,term-curr: > + description: Charging cycle termination current (in uA) > + allOf: > + - $ref: /schemas/types.yaml#/definitions/uint32 > + > + summit,fast-volt-threshold: > + description: Voltage threshold to transit to fast charge mode (in uV) > + allOf: > + - $ref: /schemas/types.yaml#/definitions/uint32 > + minimum: 2400000 > + maximum: 3000000 > + > + summit,mains-curr-limit: > + description: Maximum input current from AC/DC input (in uA) > + allOf: > + - $ref: /schemas/types.yaml#/definitions/uint32 > + > + summit,usb-curr-limit: > + description: Maximum input current from USB input (in uA) > + allOf: > + - $ref: /schemas/types.yaml#/definitions/uint32 These are all properties of the battery attached and we have standard properties for some/all of these. Also, any property with units should have a unit suffix as defined in property-units.txt. And with that, you don't need to define the type. > + > + summit,chg-curr-comp: > + description: Charge current compensation (in uA) > + allOf: > + - $ref: /schemas/types.yaml#/definitions/uint32 > + > + summit,chip-temp-threshold: > + description: Chip temperature for thermal regulation in °C. > + allOf: > + - $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [100, 110, 120, 130] > + > + summit,soft-cold-temp-limit: > + description: Cold battery temperature in °C for soft alarm. > + allOf: > + - $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [0, 5, 10, 15] > + > + summit,soft-hot-temp-limit: > + description: Hot battery temperature in °C for soft alarm. > + allOf: > + - $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [40, 45, 50, 55] > + > + summit,hard-cold-temp-limit: > + description: Cold battery temperature in °C for hard alarm. > + allOf: > + - $ref: /schemas/types.yaml#/definitions/int32 > + enum: [-5, 0, 5, 10] > + > + summit,hard-hot-temp-limit: > + description: Hot battery temperature in °C for hard alarm. > + allOf: > + - $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [50, 55, 60, 65] > + > + summit,soft-comp-method: > + description: Soft temperature limit compensation method > + allOf: > + - $ref: /schemas/types.yaml#/definitions/uint32 > + - enum: > + - 0 # SMB3XX_SOFT_TEMP_COMPENSATE_NONE Compensation none > + - 1 # SMB3XX_SOFT_TEMP_COMPENSATE_CURRENT Current compensation > + - 2 # SMB3XX_SOFT_TEMP_COMPENSATE_VOLTAGE Voltage compensation > + > +allOf: > + - if: > + properties: > + compatible: > + enum: > + - summit,smb345 > + - summit,smb358 > + > + then: > + properties: > + summit,max-chg-curr: > + enum: [ 200000, 450000, 600000, 900000, > + 1300000, 1500000, 1800000, 2000000] > + > + summit,pre-chg-curr: > + enum: [150000, 250000, 350000, 450000] > + > + summit,term-curr: > + enum: [ 30000, 40000, 60000, 80000, > + 100000, 125000, 150000, 200000] > + > + summit,mains-curr-limit: > + enum: [ 300000, 500000, 700000, 1000000, > + 1500000, 1800000, 2000000] > + > + summit,usb-curr-limit: > + enum: [ 300000, 500000, 700000, 1000000, > + 1500000, 1800000, 2000000] > + > + summit,chg-curr-comp: > + enum: [200000, 450000, 600000, 900000] > + > + else: > + properties: > + summit,max-chg-curr: > + enum: [ 700000, 900000, 1200000, 1500000, > + 1800000, 2000000, 2200000, 2500000] > + > + summit,pre-chg-curr: > + enum: [100000, 150000, 200000, 250000] > + > + summit,term-curr: > + enum: [ 37500, 50000, 100000, 150000, > + 200000, 250000, 500000, 600000] > + > + summit,mains-curr-limit: > + enum: [ 300000, 500000, 700000, 900000, 1200000, > + 1500000, 1800000, 2000000, 2200000, 2500000] > + > + summit,usb-curr-limit: > + enum: [ 300000, 500000, 700000, 900000, 1200000, > + 1500000, 1800000, 2000000, 2200000, 2500000] > + > + summit,chg-curr-comp: > + enum: [250000, 700000, 900000, 1200000] > + > +required: > + - compatible > + - reg > + > +anyOf: > + - required: > + - summit,enable-usb-charging > + - required: > + - summit,enable-otg-charging > + - required: > + - summit,enable-mains-charging > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/power/summit,smb347-charger.h> > + > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + charger@7f { > + compatible = "summit,smb347"; > + reg = <0x7f>; > + status = "okay"; > + > + summit,max-chg-curr = <1800000>; > + summit,mains-curr-limit = <2000000>; > + summit,usb-curr-limit = <500000>; > + > + summit,chip-temp-threshold = <110>; > + summit,soft-cold-temp-limit = <5>; > + > + summit,enable-usb-charging; > + summit,enable-mains-charging; > + > + summit,enable-chg-ctrl = <SMB3XX_CHG_ENABLE_PIN_ACTIVE_HIGH>; > + }; > + }; > diff --git a/include/dt-bindings/power/summit,smb347-charger.h b/include/dt-bindings/power/summit,smb347-charger.h > new file mode 100644 > index 000000000000..d918bf321a71 > --- /dev/null > +++ b/include/dt-bindings/power/summit,smb347-charger.h > @@ -0,0 +1,19 @@ > +/* SPDX-License-Identifier: (GPL-2.0-or-later or MIT) */ > +/* > + * Author: David Heidelberg <david@ixit.cz> > + */ > + > +#ifndef _DT_BINDINGS_SMB347_CHARGER_H > +#define _DT_BINDINGS_SMB347_CHARGER_H > + > +/* Charging compensation method */ > +#define SMB3XX_SOFT_TEMP_COMPENSATE_NONE 0 > +#define SMB3XX_SOFT_TEMP_COMPENSATE_CURRENT 1 > +#define SMB3XX_SOFT_TEMP_COMPENSATE_VOLTAGE 2 > + > +/* Charging enable control */ > +#define SMB3XX_CHG_ENABLE_SW 0 > +#define SMB3XX_CHG_ENABLE_PIN_ACTIVE_LOW 1 > +#define SMB3XX_CHG_ENABLE_PIN_ACTIVE_HIGH 2 > + > +#endif > -- > 2.25.0 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 4/9] dt-bindings: power: supply: Add device-tree binding for Summit SMB3xx 2020-04-10 16:49 ` Rob Herring @ 2020-04-10 19:02 ` Dmitry Osipenko 2020-04-15 14:27 ` Rob Herring 0 siblings, 1 reply; 9+ messages in thread From: Dmitry Osipenko @ 2020-04-10 19:02 UTC (permalink / raw) To: Rob Herring, David Heidelberg Cc: Sebastian Reichel, Jonghwa Lee, Chanwoo Choi, Myungjoo Ham, Sumit Semwal, John Stultz, Vinay Simha BN, mika.westerberg, ramakrishna.pallala, linux-pm, linux-kernel, devicetree 10.04.2020 19:49, Rob Herring пишет: ... >> + summit,max-chg-curr: >> + description: Maximum current for charging (in uA) >> + allOf: >> + - $ref: /schemas/types.yaml#/definitions/uint32 >> + >> + summit,max-chg-volt: >> + description: Maximum voltage for charging (in uV) >> + allOf: >> + - $ref: /schemas/types.yaml#/definitions/uint32 >> + minimum: 3500000 >> + maximum: 4500000 >> + >> + summit,pre-chg-curr: >> + description: Pre-charging current for charging (in uA) >> + allOf: >> + - $ref: /schemas/types.yaml#/definitions/uint32 >> + >> + summit,term-curr: >> + description: Charging cycle termination current (in uA) >> + allOf: >> + - $ref: /schemas/types.yaml#/definitions/uint32 ... > These are all properties of the battery attached and we have standard > properties for some/all of these. Looks like only four properties seem to be matching the properties of the battery.txt binding. Are you suggesting that these matching properties should be renamed after the properties in battery.txt? > Also, any property with units should have a unit suffix as defined in > property-units.txt. And with that, you don't need to define the type. Indeed, thank you for the suggestion. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 4/9] dt-bindings: power: supply: Add device-tree binding for Summit SMB3xx 2020-04-10 19:02 ` Dmitry Osipenko @ 2020-04-15 14:27 ` Rob Herring 2020-04-15 15:30 ` Dmitry Osipenko 0 siblings, 1 reply; 9+ messages in thread From: Rob Herring @ 2020-04-15 14:27 UTC (permalink / raw) To: Dmitry Osipenko Cc: David Heidelberg, Sebastian Reichel, Jonghwa Lee, Chanwoo Choi, Myungjoo Ham, Sumit Semwal, John Stultz, Vinay Simha BN, Mika Westerberg, ramakrishna.pallala, open list:THERMAL, linux-kernel@vger.kernel.org, devicetree On Fri, Apr 10, 2020 at 2:02 PM Dmitry Osipenko <digetx@gmail.com> wrote: > > 10.04.2020 19:49, Rob Herring пишет: > ... > >> + summit,max-chg-curr: > >> + description: Maximum current for charging (in uA) > >> + allOf: > >> + - $ref: /schemas/types.yaml#/definitions/uint32 > >> + > >> + summit,max-chg-volt: > >> + description: Maximum voltage for charging (in uV) > >> + allOf: > >> + - $ref: /schemas/types.yaml#/definitions/uint32 > >> + minimum: 3500000 > >> + maximum: 4500000 > >> + > >> + summit,pre-chg-curr: > >> + description: Pre-charging current for charging (in uA) > >> + allOf: > >> + - $ref: /schemas/types.yaml#/definitions/uint32 > >> + > >> + summit,term-curr: > >> + description: Charging cycle termination current (in uA) > >> + allOf: > >> + - $ref: /schemas/types.yaml#/definitions/uint32 > ... > > These are all properties of the battery attached and we have standard > > properties for some/all of these. > > Looks like only four properties seem to be matching the properties of > the battery.txt binding. > > Are you suggesting that these matching properties should be renamed > after the properties in battery.txt? Yes, and that there should be a battery node. Possibly you should add new properties battery.txt. It's curious that different properties are needed. Ultimately, for a given battery technology I would expect there's a fixed set of properties needed to describe how to charge them. Perhaps some of these properties can just be derived from other properties and folks are just picking what a specific charger wants. Unfortunately, we have just a mess of stuff made up for each charger out there. I don't have the time nor the experience in this area to do much more than say do better. Rob ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 4/9] dt-bindings: power: supply: Add device-tree binding for Summit SMB3xx 2020-04-15 14:27 ` Rob Herring @ 2020-04-15 15:30 ` Dmitry Osipenko 2020-05-09 1:14 ` Sebastian Reichel 0 siblings, 1 reply; 9+ messages in thread From: Dmitry Osipenko @ 2020-04-15 15:30 UTC (permalink / raw) To: Rob Herring, David Heidelberg, Sebastian Reichel Cc: Jonghwa Lee, Chanwoo Choi, Myungjoo Ham, Sumit Semwal, John Stultz, Vinay Simha BN, Mika Westerberg, ramakrishna.pallala, open list:THERMAL, linux-kernel@vger.kernel.org, devicetree 15.04.2020 17:27, Rob Herring пишет: > On Fri, Apr 10, 2020 at 2:02 PM Dmitry Osipenko <digetx@gmail.com> wrote: >> >> 10.04.2020 19:49, Rob Herring пишет: >> ... >>>> + summit,max-chg-curr: >>>> + description: Maximum current for charging (in uA) >>>> + allOf: >>>> + - $ref: /schemas/types.yaml#/definitions/uint32 >>>> + >>>> + summit,max-chg-volt: >>>> + description: Maximum voltage for charging (in uV) >>>> + allOf: >>>> + - $ref: /schemas/types.yaml#/definitions/uint32 >>>> + minimum: 3500000 >>>> + maximum: 4500000 >>>> + >>>> + summit,pre-chg-curr: >>>> + description: Pre-charging current for charging (in uA) >>>> + allOf: >>>> + - $ref: /schemas/types.yaml#/definitions/uint32 >>>> + >>>> + summit,term-curr: >>>> + description: Charging cycle termination current (in uA) >>>> + allOf: >>>> + - $ref: /schemas/types.yaml#/definitions/uint32 >> ... >>> These are all properties of the battery attached and we have standard >>> properties for some/all of these. >> >> Looks like only four properties seem to be matching the properties of >> the battery.txt binding. >> >> Are you suggesting that these matching properties should be renamed >> after the properties in battery.txt? > > Yes, and that there should be a battery node. Usually, it's a battery that has a phandle to the power-supply. Isn't it? > Possibly you should add > new properties battery.txt. It's curious that different properties are > needed. I guess it should be possible to make all these properties generic. Sebastian, will you be okay if we will add all the required properties to the power_supply_core? > Ultimately, for a given battery technology I would expect > there's a fixed set of properties needed to describe how to charge > them. Please notice that the charger doesn't "only charge" the battery, usually it also supplies power to the whole device. For example, when battery is fully-charged and charger is connected to the power source (USB or mains), then battery may not draw any current at all. > Perhaps some of these properties can just be derived from other > properties and folks are just picking what a specific charger wants. Could be so, but I don't know for sure. Even if some properties could be derived from the others, it won't hurt if we will specify everything explicitly in the device-tree. > Unfortunately, we have just a mess of stuff made up for each charger > out there. I don't have the time nor the experience in this area to do > much more than say do better. I don't think it's a mess in the kernel. For example, it's common that embedded controllers are exposed to the system as "just a battery", while in fact it's a combined charger + battery controller and the charger parameters just couldn't be changed by SW. In a case of the Nexus 7 devices, the battery controller and charger controller are two separate entities in the system. The battery controller (bq27541) only monitors status of the battery (charge level, temperature and etc). ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 4/9] dt-bindings: power: supply: Add device-tree binding for Summit SMB3xx 2020-04-15 15:30 ` Dmitry Osipenko @ 2020-05-09 1:14 ` Sebastian Reichel 2020-05-14 19:34 ` Dmitry Osipenko 0 siblings, 1 reply; 9+ messages in thread From: Sebastian Reichel @ 2020-05-09 1:14 UTC (permalink / raw) To: Dmitry Osipenko Cc: Rob Herring, David Heidelberg, Jonghwa Lee, Chanwoo Choi, Myungjoo Ham, Sumit Semwal, John Stultz, Vinay Simha BN, Mika Westerberg, ramakrishna.pallala, open list:THERMAL, linux-kernel@vger.kernel.org, devicetree [-- Attachment #1: Type: text/plain, Size: 5893 bytes --] Hi, On Wed, Apr 15, 2020 at 06:30:02PM +0300, Dmitry Osipenko wrote: > 15.04.2020 17:27, Rob Herring пишет: > > On Fri, Apr 10, 2020 at 2:02 PM Dmitry Osipenko <digetx@gmail.com> wrote: > >> > >> 10.04.2020 19:49, Rob Herring пишет: > >> ... > >>>> + summit,max-chg-curr: > >>>> + description: Maximum current for charging (in uA) > >>>> + allOf: > >>>> + - $ref: /schemas/types.yaml#/definitions/uint32 > >>>> + > >>>> + summit,max-chg-volt: > >>>> + description: Maximum voltage for charging (in uV) > >>>> + allOf: > >>>> + - $ref: /schemas/types.yaml#/definitions/uint32 > >>>> + minimum: 3500000 > >>>> + maximum: 4500000 > >>>> + > >>>> + summit,pre-chg-curr: > >>>> + description: Pre-charging current for charging (in uA) > >>>> + allOf: > >>>> + - $ref: /schemas/types.yaml#/definitions/uint32 > >>>> + > >>>> + summit,term-curr: > >>>> + description: Charging cycle termination current (in uA) > >>>> + allOf: > >>>> + - $ref: /schemas/types.yaml#/definitions/uint32 > >> ... > >>> These are all properties of the battery attached and we have standard > >>> properties for some/all of these. > >> > >> Looks like only four properties seem to be matching the properties of > >> the battery.txt binding. > >> > >> Are you suggesting that these matching properties should be renamed > >> after the properties in battery.txt? > > > > Yes, and that there should be a battery node. > > Usually, it's a battery that has a phandle to the power-supply. Isn't it? There are two things: The infrastructure described by Documentation/devicetree/bindings/power/supply/power-supply.yaml is used for telling the operating system, that a battery is charged by some charger. This is done by adding a power-supplies = <&phandle> in the battery fuel gauge node referencing the charger and probably what you mean here. Then we have the infrastructure described by Documentation/devicetree/bindings/power/supply/battery.txt, which provides data about the battery cell. In an ideal world we would have only smart batteries providing this data, but we don't live in such a world. So what we currently have is a binding looking like this: bat: dumb-battery { compatible = "simple-battery"; // data about battery cell(s) }; fuel-gauge { // fuel-gauge specific data supplies = <&charger>; monitored-battery = <&bat>; }; charger: charger { // charger specific data monitored-battery = <&bat>; }; In an ideal world, charger should possibly reference fuel-gauge node, which could provide combined data. Right now we do not have the infrastructure for that, so it needs to directly reference the simple-battery node. > > Possibly you should add > > new properties battery.txt. It's curious that different properties are > > needed. > > I guess it should be possible to make all these properties generic. > > Sebastian, will you be okay if we will add all the required properties > to the power_supply_core? Extending battery.txt is possible when something is missing. As Rob mentioned quite a few are already described, though: summit,max-chg-curr => constant-charge-current-max-microamp summit,max-chg-volt => constant-charge-voltage-max-microvolt summit,pre-chg-curr => precharge-current-microamp summit,term-curr => charge-term-current-microamp I think at least the battery temperature limits are something, that should be added to the generic code. > > Ultimately, for a given battery technology I would expect > > there's a fixed set of properties needed to describe how to charge > > them. > > Please notice that the charger doesn't "only charge" the battery, > usually it also supplies power to the whole device. > > For example, when battery is fully-charged and charger is connected to > the power source (USB or mains), then battery may not draw any current > at all. It is also a question of how good the charging process should be. Technically I can charge a single cell Li-ion battery without knowing much, but it can reduce battery life and/or be very slow. It might even be dangerous, if charging is done at high temperatures. Also some of the properties in the battery binding are not about charging, but about gauging. Some devices basically have only options to measure voltage and voltage drop over a resistor and everything else must be done by the operating system. > > Perhaps some of these properties can just be derived from other > > properties and folks are just picking what a specific charger wants. > > Could be so, but I don't know for sure. I don't think we have things, that can be derived with a reasonable amount of effort in the existing simple-battery binding, except for energy-full-design-microwatt-hours & charge-full-design-microamp-hours. > Even if some properties could be derived from the others, it won't hurt > if we will specify everything explicitly in the device-tree. > > > Unfortunately, we have just a mess of stuff made up for each charger > > out there. I don't have the time nor the experience in this area to do > > much more than say do better. > > I don't think it's a mess in the kernel. For example, it's common that > embedded controllers are exposed to the system as "just a battery", > while in fact it's a combined charger + battery controller and the > charger parameters just couldn't be changed by SW. A good EC driver exposes a charger and a battery device, so that userspace can easily identify if a charger is connected. > In a case of the Nexus 7 devices, the battery controller and charger > controller are two separate entities in the system. The battery > controller (bq27541) only monitors status of the battery (charge level, > temperature and etc). -- Sebastian [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 4/9] dt-bindings: power: supply: Add device-tree binding for Summit SMB3xx 2020-05-09 1:14 ` Sebastian Reichel @ 2020-05-14 19:34 ` Dmitry Osipenko 0 siblings, 0 replies; 9+ messages in thread From: Dmitry Osipenko @ 2020-05-14 19:34 UTC (permalink / raw) To: Sebastian Reichel Cc: Rob Herring, David Heidelberg, Jonghwa Lee, Chanwoo Choi, Myungjoo Ham, Sumit Semwal, John Stultz, Vinay Simha BN, Mika Westerberg, ramakrishna.pallala, open list:THERMAL, linux-kernel@vger.kernel.org, devicetree 09.05.2020 04:14, Sebastian Reichel пишет: > Hi, > > On Wed, Apr 15, 2020 at 06:30:02PM +0300, Dmitry Osipenko wrote: >> 15.04.2020 17:27, Rob Herring пишет: >>> On Fri, Apr 10, 2020 at 2:02 PM Dmitry Osipenko <digetx@gmail.com> wrote: >>>> >>>> 10.04.2020 19:49, Rob Herring пишет: >>>> ... >>>>>> + summit,max-chg-curr: >>>>>> + description: Maximum current for charging (in uA) >>>>>> + allOf: >>>>>> + - $ref: /schemas/types.yaml#/definitions/uint32 >>>>>> + >>>>>> + summit,max-chg-volt: >>>>>> + description: Maximum voltage for charging (in uV) >>>>>> + allOf: >>>>>> + - $ref: /schemas/types.yaml#/definitions/uint32 >>>>>> + minimum: 3500000 >>>>>> + maximum: 4500000 >>>>>> + >>>>>> + summit,pre-chg-curr: >>>>>> + description: Pre-charging current for charging (in uA) >>>>>> + allOf: >>>>>> + - $ref: /schemas/types.yaml#/definitions/uint32 >>>>>> + >>>>>> + summit,term-curr: >>>>>> + description: Charging cycle termination current (in uA) >>>>>> + allOf: >>>>>> + - $ref: /schemas/types.yaml#/definitions/uint32 >>>> ... >>>>> These are all properties of the battery attached and we have standard >>>>> properties for some/all of these. >>>> >>>> Looks like only four properties seem to be matching the properties of >>>> the battery.txt binding. >>>> >>>> Are you suggesting that these matching properties should be renamed >>>> after the properties in battery.txt? >>> >>> Yes, and that there should be a battery node. >> >> Usually, it's a battery that has a phandle to the power-supply. Isn't it? > > There are two things: The infrastructure described by > Documentation/devicetree/bindings/power/supply/power-supply.yaml is > used for telling the operating system, that a battery is charged > by some charger. This is done by adding a power-supplies = <&phandle> > in the battery fuel gauge node referencing the charger and probably > what you mean here. > > Then we have the infrastructure described by > Documentation/devicetree/bindings/power/supply/battery.txt, which > provides data about the battery cell. In an ideal world we would > have only smart batteries providing this data, but we don't live > in such a world. So what we currently have is a binding looking > like this: > > bat: dumb-battery { > compatible = "simple-battery"; > > // data about battery cell(s) > }; > > fuel-gauge { > // fuel-gauge specific data > > supplies = <&charger>; > monitored-battery = <&bat>; > }; > > charger: charger { > // charger specific data > > monitored-battery = <&bat>; > }; > > In an ideal world, charger should possibly reference fuel-gauge > node, which could provide combined data. Right now we do not have > the infrastructure for that, so it needs to directly reference > the simple-battery node. > >>> Possibly you should add >>> new properties battery.txt. It's curious that different properties are >>> needed. >> >> I guess it should be possible to make all these properties generic. >> >> Sebastian, will you be okay if we will add all the required properties >> to the power_supply_core? > > Extending battery.txt is possible when something is missing. As Rob > mentioned quite a few are already described, though: > > summit,max-chg-curr => constant-charge-current-max-microamp > summit,max-chg-volt => constant-charge-voltage-max-microvolt > summit,pre-chg-curr => precharge-current-microamp > summit,term-curr => charge-term-current-microamp > > I think at least the battery temperature limits are something, that > should be added to the generic code. > >>> Ultimately, for a given battery technology I would expect >>> there's a fixed set of properties needed to describe how to charge >>> them. >> >> Please notice that the charger doesn't "only charge" the battery, >> usually it also supplies power to the whole device. >> >> For example, when battery is fully-charged and charger is connected to >> the power source (USB or mains), then battery may not draw any current >> at all. > > It is also a question of how good the charging process should be. > Technically I can charge a single cell Li-ion battery without > knowing much, but it can reduce battery life and/or be very slow. > It might even be dangerous, if charging is done at high > temperatures. Also some of the properties in the battery binding > are not about charging, but about gauging. Some devices basically > have only options to measure voltage and voltage drop over a > resistor and everything else must be done by the operating system. > >>> Perhaps some of these properties can just be derived from other >>> properties and folks are just picking what a specific charger wants. >> >> Could be so, but I don't know for sure. > > I don't think we have things, that can be derived with a reasonable > amount of effort in the existing simple-battery binding, except for > energy-full-design-microwatt-hours & charge-full-design-microamp-hours. > >> Even if some properties could be derived from the others, it won't hurt >> if we will specify everything explicitly in the device-tree. >> >>> Unfortunately, we have just a mess of stuff made up for each charger >>> out there. I don't have the time nor the experience in this area to do >>> much more than say do better. >> >> I don't think it's a mess in the kernel. For example, it's common that >> embedded controllers are exposed to the system as "just a battery", >> while in fact it's a combined charger + battery controller and the >> charger parameters just couldn't be changed by SW. > > A good EC driver exposes a charger and a battery device, so that > userspace can easily identify if a charger is connected. > >> In a case of the Nexus 7 devices, the battery controller and charger >> controller are two separate entities in the system. The battery >> controller (bq27541) only monitors status of the battery (charge level, >> temperature and etc). Hello Sebastian, Thank you very much for the comments! We'll prepare the v2. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 9/9] arm: dts: qcom: apq8064-nexus7: Add smb345 charger node 2020-03-29 16:15 [PATCH 0/9] Summit SMB3xx driver & device-tree David Heidelberg 2020-03-29 16:21 ` [PATCH 4/9] dt-bindings: power: supply: Add device-tree binding for Summit SMB3xx David Heidelberg @ 2020-03-29 16:21 ` David Heidelberg 1 sibling, 0 replies; 9+ messages in thread From: David Heidelberg @ 2020-03-29 16:21 UTC (permalink / raw) To: Sebastian Reichel, Jonghwa Lee, Chanwoo Choi, Myungjoo Ham, Sumit Semwal, John Stultz, Vinay Simha BN, mika.westerberg, ramakrishna.pallala, Dmitry Osipenko, linux-pm, linux-kernel Cc: David Heidelberg, Rob Herring, devicetree Add smb345 charger node to Nexus 7 2013 DTS. Proper charger initialization also prevents battery from overcharging. Original author: Vinay Simha BN <simhavcs@gmail.com> Signed-off-by: David Heidelberg <david@ixit.cz> --- .../boot/dts/qcom-apq8064-asus-nexus7-flo.dts | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts b/arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts index a701d4bac320..9f14216a22f1 100644 --- a/arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts +++ b/arch/arm/boot/dts/qcom-apq8064-asus-nexus7-flo.dts @@ -3,6 +3,7 @@ #include <dt-bindings/gpio/gpio.h> #include <dt-bindings/input/input.h> #include <dt-bindings/pinctrl/qcom,pmic-gpio.h> +#include <dt-bindings/power/summit,smb347-charger.h> / { model = "Asus Nexus7(flo)"; compatible = "asus,nexus7-flo", "qcom,apq8064"; @@ -293,11 +294,30 @@ eeprom@52 { pagesize = <32>; }; - bq27541@55 { + bat: battery@55 { compatible = "ti,bq27541"; reg = <0x55>; + power-supplies = <&power_supply>; }; + power_supply: charger@6a { + compatible = "summit,smb345"; + reg = <0x6a>; + + interrupt-parent = <&tlmm_pinmux>; + interrupts = <23 IRQ_TYPE_EDGE_BOTH>; + + summit,max-chg-curr = <1800000>; + summit,usb-curr-limit = <500000>; + + summit,chip-temp-threshold = <110>; + + summit,enable-usb-charging; + summit,enable-otg-charging; + + summit,enable-chg-ctrl = + <SMB3XX_CHG_ENABLE_SW>; + }; }; }; -- 2.25.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-05-14 19:35 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-03-29 16:15 [PATCH 0/9] Summit SMB3xx driver & device-tree David Heidelberg 2020-03-29 16:21 ` [PATCH 4/9] dt-bindings: power: supply: Add device-tree binding for Summit SMB3xx David Heidelberg 2020-04-10 16:49 ` Rob Herring 2020-04-10 19:02 ` Dmitry Osipenko 2020-04-15 14:27 ` Rob Herring 2020-04-15 15:30 ` Dmitry Osipenko 2020-05-09 1:14 ` Sebastian Reichel 2020-05-14 19:34 ` Dmitry Osipenko 2020-03-29 16:21 ` [PATCH 9/9] arm: dts: qcom: apq8064-nexus7: Add smb345 charger node David Heidelberg
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).