| Message ID | 1441757557-7266-14-git-send-email-dannenberg@ti.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On 09.09.2015 09:12, Andreas Dannenberg wrote: > Extend the bq2425x charger's device tree documentation to cover the > bq24250 and bq24257 devices as well as recent feature additions. > > Signed-off-by: Andreas Dannenberg <dannenberg@ti.com> Hi, Thanks for updates. Everything pointed previous looks good. After looking at Ramakrishna Pallala's (Cc-ed) patch ("power: bq24261_charger: Add support for TI BQ24261 charger") I have only one comment below. > --- > .../devicetree/bindings/power/bq24257.txt | 21 ------- > .../devicetree/bindings/power/bq2425x.txt | 70 ++++++++++++++++++++++ > 2 files changed, 70 insertions(+), 21 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/power/bq24257.txt > create mode 100644 Documentation/devicetree/bindings/power/bq2425x.txt > > diff --git a/Documentation/devicetree/bindings/power/bq24257.txt b/Documentation/devicetree/bindings/power/bq24257.txt > deleted file mode 100644 > index 5c9d394..0000000 > --- a/Documentation/devicetree/bindings/power/bq24257.txt > +++ /dev/null > @@ -1,21 +0,0 @@ > -Binding for TI bq24257 Li-Ion Charger > - > -Required properties: > -- compatible: Should contain one of the following: > - * "ti,bq24257" > -- reg: integer, i2c address of the device. > -- ti,battery-regulation-voltage: integer, maximum charging voltage in uV. > -- ti,charge-current: integer, maximum charging current in uA. > -- ti,termination-current: integer, charge will be terminated when current in > - constant-voltage phase drops below this value (in uA). > - > -Example: > - > -bq24257 { > - compatible = "ti,bq24257"; > - reg = <0x6a>; > - > - ti,battery-regulation-voltage = <4200000>; > - ti,charge-current = <1000000>; > - ti,termination-current = <50000>; > -}; > diff --git a/Documentation/devicetree/bindings/power/bq2425x.txt b/Documentation/devicetree/bindings/power/bq2425x.txt > new file mode 100644 > index 0000000..3e171c3 > --- /dev/null > +++ b/Documentation/devicetree/bindings/power/bq2425x.txt > @@ -0,0 +1,70 @@ > +Binding for TI bq2425x Li-Ion Charger > + > +Required properties: > +- compatible: Should contain one of the following: > + * "ti,bq24250" > + * "ti,bq24251" > + * "ti,bq24257" > +- reg: integer, i2c address of the device. > +- stat-gpios: GPIO used for the devices STAT_IN pin. Alternatively the pin can > + also be defined through the standard interrupt definition properties (see > + optional properties section below). Only use one method. > +- ti,battery-regulation-voltage: integer, maximum charging voltage in uV. > +- ti,charge-current: integer, maximum charging current in uA. > +- ti,termination-current: integer, charge will be terminated when current in > + constant-voltage phase drops below this value (in uA). > + > +Optional properties: > +- pg-gpios: GPIO used for connecting the bq2425x device PG (Power Good) pin. > + This pin is not available on all devices however it should be used if > + possible as this is the recommended way to obtain the charger's input PG > + state. If this pin is not specified a software-based approach for PG > + detection is used. > +- ti,current-limit: The maximum current to be drawn from the charger's input > + (in uA). If this property is not specified a USB D+/D- signal based charger > + type detection is used (if available) and the input limit current is set > + accordingly. If the D+/D- based detection is not available on a given device > + a default of 500,000 is used (=500mA). I understand this is different property than "ti,charge-current: integer, default charging current (in mA)" from bq24261_charger patch? This one is for setting limit on current drawn from the charger and the bq24261's is to limit current delivered to battery? Best regards, Krzysztof > +- ti,ovp-voltage: Configures the over voltage protection voltage (in uV). If > + not specified a default of 6,5000,000 (=6.5V) is used. > +- ti,in-dpm-voltage: Configures the threshold input voltage for the dynamic > + power path management (in uV). If not specified a default of 4,360,000 > + (=4.36V) is used. > +- ti,safety-timer-2x-enable: If this property is set to 1 the device's safety > + timer is extended (slowed down) by a factor of two. Setting this property > + to 0 or not providing it will leave the safety timer at its default > + setting. > +- interrupt-parent: Should be the phandle for the interrupt controller. Use in > + conjunction with "interrupts" and only in case "stat-gpios" is not used. > +- interrupts: Interrupt mapping for GPIO IRQ (configure for both edges). Use in > + conjunction with "interrupt-parent" and only in case "stat-gpios" is not > + used. > + > +Example: > + > +bq24257 { > + compatible = "ti,bq24257"; > + reg = <0x6a>; > + stat-gpios = <&gpio1 16 GPIO_ACTIVE_HIGH>; > + pg-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>; > + > + ti,battery-regulation-voltage = <4200000>; > + ti,charge-current = <1000000>; > + ti,termination-current = <50000>; > +}; > + > +Example: > + > +bq24250 { > + compatible = "ti,bq24250"; > + reg = <0x6a>; > + interrupt-parent = <&gpio1>; > + interrupts = <16 IRQ_TYPE_EDGE_BOTH>; > + > + ti,battery-regulation-voltage = <4200000>; > + ti,charge-current = <500000>; > + ti,termination-current = <50000>; > + ti,current-limit = <900000>; > + ti,ovp-voltage = <9500000>; > + ti,in-dpm-voltage = <4440000>; > +}; > -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Sep 09, 2015 at 09:27:06AM +0900, Krzysztof Kozlowski wrote: > On 09.09.2015 09:12, Andreas Dannenberg wrote: > > Extend the bq2425x charger's device tree documentation to cover the > > bq24250 and bq24257 devices as well as recent feature additions. > > > > Signed-off-by: Andreas Dannenberg <dannenberg@ti.com> > > Hi, > > Thanks for updates. Everything pointed previous looks good. After > looking at Ramakrishna Pallala's (Cc-ed) patch ("power: bq24261_charger: > Add support for TI BQ24261 charger") I have only one comment below. Thanks for all your efforts and time checking those patches! > > > --- > > .../devicetree/bindings/power/bq24257.txt | 21 ------- > > .../devicetree/bindings/power/bq2425x.txt | 70 ++++++++++++++++++++++ > > 2 files changed, 70 insertions(+), 21 deletions(-) > > delete mode 100644 Documentation/devicetree/bindings/power/bq24257.txt > > create mode 100644 Documentation/devicetree/bindings/power/bq2425x.txt > > > > diff --git a/Documentation/devicetree/bindings/power/bq24257.txt b/Documentation/devicetree/bindings/power/bq24257.txt > > deleted file mode 100644 > > index 5c9d394..0000000 > > --- a/Documentation/devicetree/bindings/power/bq24257.txt > > +++ /dev/null > > @@ -1,21 +0,0 @@ > > -Binding for TI bq24257 Li-Ion Charger > > - > > -Required properties: > > -- compatible: Should contain one of the following: > > - * "ti,bq24257" > > -- reg: integer, i2c address of the device. > > -- ti,battery-regulation-voltage: integer, maximum charging voltage in uV. > > -- ti,charge-current: integer, maximum charging current in uA. > > -- ti,termination-current: integer, charge will be terminated when current in > > - constant-voltage phase drops below this value (in uA). > > - > > -Example: > > - > > -bq24257 { > > - compatible = "ti,bq24257"; > > - reg = <0x6a>; > > - > > - ti,battery-regulation-voltage = <4200000>; > > - ti,charge-current = <1000000>; > > - ti,termination-current = <50000>; > > -}; > > diff --git a/Documentation/devicetree/bindings/power/bq2425x.txt b/Documentation/devicetree/bindings/power/bq2425x.txt > > new file mode 100644 > > index 0000000..3e171c3 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/power/bq2425x.txt > > @@ -0,0 +1,70 @@ > > +Binding for TI bq2425x Li-Ion Charger > > + > > +Required properties: > > +- compatible: Should contain one of the following: > > + * "ti,bq24250" > > + * "ti,bq24251" > > + * "ti,bq24257" > > +- reg: integer, i2c address of the device. > > +- stat-gpios: GPIO used for the devices STAT_IN pin. Alternatively the pin can > > + also be defined through the standard interrupt definition properties (see > > + optional properties section below). Only use one method. > > +- ti,battery-regulation-voltage: integer, maximum charging voltage in uV. > > +- ti,charge-current: integer, maximum charging current in uA. > > +- ti,termination-current: integer, charge will be terminated when current in > > + constant-voltage phase drops below this value (in uA). > > + > > +Optional properties: > > +- pg-gpios: GPIO used for connecting the bq2425x device PG (Power Good) pin. > > + This pin is not available on all devices however it should be used if > > + possible as this is the recommended way to obtain the charger's input PG > > + state. If this pin is not specified a software-based approach for PG > > + detection is used. > > +- ti,current-limit: The maximum current to be drawn from the charger's input > > + (in uA). If this property is not specified a USB D+/D- signal based charger > > + type detection is used (if available) and the input limit current is set > > + accordingly. If the D+/D- based detection is not available on a given device > > + a default of 500,000 is used (=500mA). > > I understand this is different property than "ti,charge-current: > integer, default charging current (in mA)" from bq24261_charger patch? Correct this is what "ti,current-limit" is for. And I borrowed that definition from bq2415x_charger.c where it's used in the same context. The reason for this property to exist is for systems where the external power supply does more than just charging the battery, such as supplying the system while it's charging or after it has finished charging. All this only makes sense of course if the "ti,current-limit" is larger than "ti,charge-current". And if you see a few lines above the bq24257 driver also has a "ti,charge-current" property. And yes this is what actually controls how much current is delivered to the battery. > This one is for setting limit on current drawn from the charger and the > bq24261's is to limit current delivered to battery? The bq24261 driver only exposes a "ti,charge-current" property which is used in accordance with how other drivers use it. And it also supports setting the input current limit but only automatically and not via DT. Regards, -- Andreas Dannenberg Texas Instruments Inc > > Best regards, > Krzysztof > > > +- ti,ovp-voltage: Configures the over voltage protection voltage (in uV). If > > + not specified a default of 6,5000,000 (=6.5V) is used. > > +- ti,in-dpm-voltage: Configures the threshold input voltage for the dynamic > > + power path management (in uV). If not specified a default of 4,360,000 > > + (=4.36V) is used. > > +- ti,safety-timer-2x-enable: If this property is set to 1 the device's safety > > + timer is extended (slowed down) by a factor of two. Setting this property > > + to 0 or not providing it will leave the safety timer at its default > > + setting. > > +- interrupt-parent: Should be the phandle for the interrupt controller. Use in > > + conjunction with "interrupts" and only in case "stat-gpios" is not used. > > +- interrupts: Interrupt mapping for GPIO IRQ (configure for both edges). Use in > > + conjunction with "interrupt-parent" and only in case "stat-gpios" is not > > + used. > > + > > +Example: > > + > > +bq24257 { > > + compatible = "ti,bq24257"; > > + reg = <0x6a>; > > + stat-gpios = <&gpio1 16 GPIO_ACTIVE_HIGH>; > > + pg-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>; > > + > > + ti,battery-regulation-voltage = <4200000>; > > + ti,charge-current = <1000000>; > > + ti,termination-current = <50000>; > > +}; > > + > > +Example: > > + > > +bq24250 { > > + compatible = "ti,bq24250"; > > + reg = <0x6a>; > > + interrupt-parent = <&gpio1>; > > + interrupts = <16 IRQ_TYPE_EDGE_BOTH>; > > + > > + ti,battery-regulation-voltage = <4200000>; > > + ti,charge-current = <500000>; > > + ti,termination-current = <50000>; > > + ti,current-limit = <900000>; > > + ti,ovp-voltage = <9500000>; > > + ti,in-dpm-voltage = <4440000>; > > +}; > > > -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09.09.2015 11:48, Andreas Dannenberg wrote: > On Wed, Sep 09, 2015 at 09:27:06AM +0900, Krzysztof Kozlowski wrote: >> On 09.09.2015 09:12, Andreas Dannenberg wrote: >>> Extend the bq2425x charger's device tree documentation to cover the >>> bq24250 and bq24257 devices as well as recent feature additions. >>> >>> Signed-off-by: Andreas Dannenberg <dannenberg@ti.com> >> >> Hi, >> >> Thanks for updates. Everything pointed previous looks good. After >> looking at Ramakrishna Pallala's (Cc-ed) patch ("power: bq24261_charger: >> Add support for TI BQ24261 charger") I have only one comment below. > > Thanks for all your efforts and time checking those patches! > >> >>> --- >>> .../devicetree/bindings/power/bq24257.txt | 21 ------- >>> .../devicetree/bindings/power/bq2425x.txt | 70 ++++++++++++++++++++++ >>> 2 files changed, 70 insertions(+), 21 deletions(-) >>> delete mode 100644 Documentation/devicetree/bindings/power/bq24257.txt >>> create mode 100644 Documentation/devicetree/bindings/power/bq2425x.txt >>> >>> diff --git a/Documentation/devicetree/bindings/power/bq24257.txt b/Documentation/devicetree/bindings/power/bq24257.txt >>> deleted file mode 100644 >>> index 5c9d394..0000000 >>> --- a/Documentation/devicetree/bindings/power/bq24257.txt >>> +++ /dev/null >>> @@ -1,21 +0,0 @@ >>> -Binding for TI bq24257 Li-Ion Charger >>> - >>> -Required properties: >>> -- compatible: Should contain one of the following: >>> - * "ti,bq24257" >>> -- reg: integer, i2c address of the device. >>> -- ti,battery-regulation-voltage: integer, maximum charging voltage in uV. >>> -- ti,charge-current: integer, maximum charging current in uA. >>> -- ti,termination-current: integer, charge will be terminated when current in >>> - constant-voltage phase drops below this value (in uA). >>> - >>> -Example: >>> - >>> -bq24257 { >>> - compatible = "ti,bq24257"; >>> - reg = <0x6a>; >>> - >>> - ti,battery-regulation-voltage = <4200000>; >>> - ti,charge-current = <1000000>; >>> - ti,termination-current = <50000>; >>> -}; >>> diff --git a/Documentation/devicetree/bindings/power/bq2425x.txt b/Documentation/devicetree/bindings/power/bq2425x.txt >>> new file mode 100644 >>> index 0000000..3e171c3 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/power/bq2425x.txt >>> @@ -0,0 +1,70 @@ >>> +Binding for TI bq2425x Li-Ion Charger >>> + >>> +Required properties: >>> +- compatible: Should contain one of the following: >>> + * "ti,bq24250" >>> + * "ti,bq24251" >>> + * "ti,bq24257" >>> +- reg: integer, i2c address of the device. >>> +- stat-gpios: GPIO used for the devices STAT_IN pin. Alternatively the pin can >>> + also be defined through the standard interrupt definition properties (see >>> + optional properties section below). Only use one method. >>> +- ti,battery-regulation-voltage: integer, maximum charging voltage in uV. >>> +- ti,charge-current: integer, maximum charging current in uA. >>> +- ti,termination-current: integer, charge will be terminated when current in >>> + constant-voltage phase drops below this value (in uA). >>> + >>> +Optional properties: >>> +- pg-gpios: GPIO used for connecting the bq2425x device PG (Power Good) pin. >>> + This pin is not available on all devices however it should be used if >>> + possible as this is the recommended way to obtain the charger's input PG >>> + state. If this pin is not specified a software-based approach for PG >>> + detection is used. >>> +- ti,current-limit: The maximum current to be drawn from the charger's input >>> + (in uA). If this property is not specified a USB D+/D- signal based charger >>> + type detection is used (if available) and the input limit current is set >>> + accordingly. If the D+/D- based detection is not available on a given device >>> + a default of 500,000 is used (=500mA). >> >> I understand this is different property than "ti,charge-current: >> integer, default charging current (in mA)" from bq24261_charger patch? > > Correct this is what "ti,current-limit" is for. And I borrowed that > definition from bq2415x_charger.c where it's used in the same context. > The reason for this property to exist is for systems where the external > power supply does more than just charging the battery, such as supplying > the system while it's charging or after it has finished charging. All > this only makes sense of course if the "ti,current-limit" is larger than > "ti,charge-current". > > And if you see a few lines above the bq24257 driver also has a > "ti,charge-current" property. And yes this is what actually controls how > much current is delivered to the battery. Right, I missed that one. Everything is clear now. So we have different bindings. Existing ones: bq2415x.txt - ti,charge-current - maximum charging current in mA bq24257.txt - ti,charge-current - maximum charging current in uA bq25890.txt - ti,charge-current - maximum charging current in uA bq24735.txt - ti,charge-current - charge current (?) in mA bq2415x.txt - ti,current-limit - maximum current to be drawn in mA New bindings: ti,charge-current (bq24261) - default charge current in mA ti,max-charge-current (bq24261) - maximum charging current in mA ti,current-limit (bq2425x) - maximum current to be drawn in uA Damn it... It's a mess. And there is no device prefixes... The bq24261's bindings look most sensible (max prefix for max charge current) but they are not compatible with existing bindings for different devices. There is no way to unify or make consistent all of these bindings. However one could try to add new stuff in a more sensible way. For example how about (for bq2425x-charger and bq24261_charger... BTW notice even the difference in using underscore and hyphen!): ti,charge-current - maximum charging current in uA (that one must be supported, it's for existing bq24257 devices) ti,default-charge-current (bq24261) - default charge current in *uA* ti,current-limit (bq2425x) - maximum current to be drawn in *uA* Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Sep 09, 2015 at 01:58:09PM +0900, Krzysztof Kozlowski wrote: > On 09.09.2015 11:48, Andreas Dannenberg wrote: > > On Wed, Sep 09, 2015 at 09:27:06AM +0900, Krzysztof Kozlowski wrote: > >> On 09.09.2015 09:12, Andreas Dannenberg wrote: > >>> Extend the bq2425x charger's device tree documentation to cover the > >>> bq24250 and bq24257 devices as well as recent feature additions. > >>> > >>> Signed-off-by: Andreas Dannenberg <dannenberg@ti.com> > >> > >> Hi, > >> > >> Thanks for updates. Everything pointed previous looks good. After > >> looking at Ramakrishna Pallala's (Cc-ed) patch ("power: bq24261_charger: > >> Add support for TI BQ24261 charger") I have only one comment below. > > > > Thanks for all your efforts and time checking those patches! > > > >> > >>> --- > >>> .../devicetree/bindings/power/bq24257.txt | 21 ------- > >>> .../devicetree/bindings/power/bq2425x.txt | 70 ++++++++++++++++++++++ > >>> 2 files changed, 70 insertions(+), 21 deletions(-) > >>> delete mode 100644 Documentation/devicetree/bindings/power/bq24257.txt > >>> create mode 100644 Documentation/devicetree/bindings/power/bq2425x.txt > >>> > >>> diff --git a/Documentation/devicetree/bindings/power/bq24257.txt b/Documentation/devicetree/bindings/power/bq24257.txt > >>> deleted file mode 100644 > >>> index 5c9d394..0000000 > >>> --- a/Documentation/devicetree/bindings/power/bq24257.txt > >>> +++ /dev/null > >>> @@ -1,21 +0,0 @@ > >>> -Binding for TI bq24257 Li-Ion Charger > >>> - > >>> -Required properties: > >>> -- compatible: Should contain one of the following: > >>> - * "ti,bq24257" > >>> -- reg: integer, i2c address of the device. > >>> -- ti,battery-regulation-voltage: integer, maximum charging voltage in uV. > >>> -- ti,charge-current: integer, maximum charging current in uA. > >>> -- ti,termination-current: integer, charge will be terminated when current in > >>> - constant-voltage phase drops below this value (in uA). > >>> - > >>> -Example: > >>> - > >>> -bq24257 { > >>> - compatible = "ti,bq24257"; > >>> - reg = <0x6a>; > >>> - > >>> - ti,battery-regulation-voltage = <4200000>; > >>> - ti,charge-current = <1000000>; > >>> - ti,termination-current = <50000>; > >>> -}; > >>> diff --git a/Documentation/devicetree/bindings/power/bq2425x.txt b/Documentation/devicetree/bindings/power/bq2425x.txt > >>> new file mode 100644 > >>> index 0000000..3e171c3 > >>> --- /dev/null > >>> +++ b/Documentation/devicetree/bindings/power/bq2425x.txt > >>> @@ -0,0 +1,70 @@ > >>> +Binding for TI bq2425x Li-Ion Charger > >>> + > >>> +Required properties: > >>> +- compatible: Should contain one of the following: > >>> + * "ti,bq24250" > >>> + * "ti,bq24251" > >>> + * "ti,bq24257" > >>> +- reg: integer, i2c address of the device. > >>> +- stat-gpios: GPIO used for the devices STAT_IN pin. Alternatively the pin can > >>> + also be defined through the standard interrupt definition properties (see > >>> + optional properties section below). Only use one method. > >>> +- ti,battery-regulation-voltage: integer, maximum charging voltage in uV. > >>> +- ti,charge-current: integer, maximum charging current in uA. > >>> +- ti,termination-current: integer, charge will be terminated when current in > >>> + constant-voltage phase drops below this value (in uA). > >>> + > >>> +Optional properties: > >>> +- pg-gpios: GPIO used for connecting the bq2425x device PG (Power Good) pin. > >>> + This pin is not available on all devices however it should be used if > >>> + possible as this is the recommended way to obtain the charger's input PG > >>> + state. If this pin is not specified a software-based approach for PG > >>> + detection is used. > >>> +- ti,current-limit: The maximum current to be drawn from the charger's input > >>> + (in uA). If this property is not specified a USB D+/D- signal based charger > >>> + type detection is used (if available) and the input limit current is set > >>> + accordingly. If the D+/D- based detection is not available on a given device > >>> + a default of 500,000 is used (=500mA). > >> > >> I understand this is different property than "ti,charge-current: > >> integer, default charging current (in mA)" from bq24261_charger patch? > > > > Correct this is what "ti,current-limit" is for. And I borrowed that > > definition from bq2415x_charger.c where it's used in the same context. > > The reason for this property to exist is for systems where the external > > power supply does more than just charging the battery, such as supplying > > the system while it's charging or after it has finished charging. All > > this only makes sense of course if the "ti,current-limit" is larger than > > "ti,charge-current". > > > > And if you see a few lines above the bq24257 driver also has a > > "ti,charge-current" property. And yes this is what actually controls how > > much current is delivered to the battery. > > Right, I missed that one. Everything is clear now. > > So we have different bindings. Existing ones: > bq2415x.txt - ti,charge-current - maximum charging current in mA > bq24257.txt - ti,charge-current - maximum charging current in uA > bq25890.txt - ti,charge-current - maximum charging current in uA > bq24735.txt - ti,charge-current - charge current (?) in mA I just spent some time with the bq24735 datasheet and the way that device appears to work is putting the user in control of the charging process rather than implementing a fully automatic control loop, but either way I still think it's valid to call the property ti,charge-current and use a description of "maximum charging current" for that device (there is no DT bindings doc however). And yes the unit is mA as one can see from reverse-engineering the register settings. On a related note the datasheet for that device says you have to periodically re-send the charge current setting every 44...175s to keep charging with the configured current or disable the device's watchdog timer. Neither of which the driver seems to do. I can probably go back and get some HW and test if that driver actually works as advertised. (also see http://www.ti.com/lit/ds/symlink/bq24735.pdf, Page 21) > bq2415x.txt - ti,current-limit - maximum current to be drawn in mA > > New bindings: > ti,charge-current (bq24261) - default charge current in mA > ti,max-charge-current (bq24261) - maximum charging current in mA This ti,max-charge-current is actually an interesting one. It's not a device setting as it does not impact any of the device registers at all. Instead, it's an artificial limit that can be set through DT that prevents somebody from going into sysfs and configuring a charge current higher than ti,max-charge-current. In other drivers I have seen that the sysfs property reflecting that max charge current is just read-only and gives you the maximum the HW is capable of. From a device point of view there is nothing configurable about this property. > ti,current-limit (bq2425x) - maximum current to be drawn in uA > > > Damn it... It's a mess. And there is no device prefixes... ACK :) Let's see how we can bring a little sense into it. > The bq24261's bindings look most sensible (max prefix for max charge > current) but they are not compatible with existing bindings for > different devices. Hmm I think they are compatible, it's just a question making the DT bindings description for the bq24261 fit better into what's already there. For example, like this: (1) ti,charge-current: integer, maximum charging current in mA. For this device as for others this setting controls the max current the device uses to charge the battery so the established description is good however the general use of this property name itself is not 100% accurate (too late for that). (2) ti,max-charge-current: Unless there is a good reason to keep it, REMOVE this property (alongside ti,max-charge-voltage). Instead, have the charger directly report back the maximum device charge current (BQ24261_MAX_CC) via sysfs like most other charger drivers do (bq24190, bq24257, bq25890, rt9455). > There is no way to unify or make consistent all of these bindings. > However one could try to add new stuff in a more sensible way. For > example how about (for bq2425x-charger and bq24261_charger... BTW notice > even the difference in using underscore and hyphen!): :( You are talking about the driver .name, right? I saw that too. If the bq24261 charger was to change it's device name to use a hyphen at least it would be consistent with bq2415x and bq2425x. > ti,charge-current - maximum charging current in uA > (that one must be supported, it's for existing bq24257 devices) Agreed. As discussed earlier this one is pretty established -- but in mA (not uA). > ti,default-charge-current (bq24261) - default charge current in *uA* We don't need that. If you look where it goes (registers) this should be called ti,charge-current for the bq24261 (like it already is). Exactly the same name/usage as the other bqxxxx chargers. We just need to update the description. > ti,current-limit (bq2425x) - maximum current to be drawn in *uA* Ok. Again here my preference would be mA. Like already on the bq2415x. If we can change the bq2425x driver to mA (see separate thread) we'd be closer to a more unified set of properties. Otherwise we would have properties with the same name but different units (is this even possible?). Regards, -- Andreas Dannenberg Texas Instruments Inc -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10.09.2015 05:15, Andreas Dannenberg wrote: > On Wed, Sep 09, 2015 at 01:58:09PM +0900, Krzysztof Kozlowski wrote: >> >> So we have different bindings. Existing ones: >> bq2415x.txt - ti,charge-current - maximum charging current in mA >> bq24257.txt - ti,charge-current - maximum charging current in uA >> bq25890.txt - ti,charge-current - maximum charging current in uA >> bq24735.txt - ti,charge-current - charge current (?) in mA > > I just spent some time with the bq24735 datasheet and the way that > device appears to work is putting the user in control of the charging > process rather than implementing a fully automatic control loop, but > either way I still think it's valid to call the property > ti,charge-current and use a description of "maximum charging current" > for that device (there is no DT bindings doc however). And yes the unit > is mA as one can see from reverse-engineering the register settings. Just for the record: the units used by device registers are not related to units used in DT binding. You can use whatever you want. The driver should just convert them. > > On a related note the datasheet for that device says you have to > periodically re-send the charge current setting every 44...175s to keep > charging with the configured current or disable the device's watchdog > timer. Neither of which the driver seems to do. I can probably go back > and get some HW and test if that driver actually works as advertised. > (also see http://www.ti.com/lit/ds/symlink/bq24735.pdf, Page 21) I mentioned existing bindings for reference. To show that it's a mess. However you cannot change them now (at least easily). You could add new bindings and mark old as deprecated (still they would have to be supported by the driver)... > >> bq2415x.txt - ti,current-limit - maximum current to be drawn in mA >> >> New bindings: >> ti,charge-current (bq24261) - default charge current in mA >> ti,max-charge-current (bq24261) - maximum charging current in mA > > This ti,max-charge-current is actually an interesting one. It's not a > device setting as it does not impact any of the device registers at all. > Instead, it's an artificial limit that can be set through DT that > prevents somebody from going into sysfs and configuring a charge current > higher than ti,max-charge-current. In other drivers I have seen that the > sysfs property reflecting that max charge current is just read-only and > gives you the maximum the HW is capable of. From a device point of view > there is nothing configurable about this property. Hmmm, so now I wonder whether this should be a DT binding. The purpose of DT isn't the control of the driver like enable some stuff, set some value used by the driver. The DT provides information about hardware so the driver could properly configure the device and work with it. Reason to put this into DT would be: For example one device configuration (device, board, connected battery) could handle one maximum value and the other configuration would require lower one. The device and its capabilities (bq24257 for example) are the same but configuration changes. If all configurations of bq24261 device have the same maximum value then it should not be a DT property but it should be hard-coded in the driver instead. Ramakrishna, Could you describe the reason behind "ti,max-charge-current" and "pdata.max_cc" in bq24261 driver? > >> ti,current-limit (bq2425x) - maximum current to be drawn in uA >> >> >> Damn it... It's a mess. And there is no device prefixes... > > ACK :) Let's see how we can bring a little sense into it. > >> The bq24261's bindings look most sensible (max prefix for max charge >> current) but they are not compatible with existing bindings for >> different devices. > > Hmm I think they are compatible, it's just a question making the DT > bindings description for the bq24261 fit better into what's already > there. For example, like this: > > (1) ti,charge-current: integer, maximum charging current in mA. For this > device as for others this setting controls the max current the device > uses to charge the battery so the established description is good > however the general use of this property name itself is not 100% > accurate (too late for that). I agree. > (2) ti,max-charge-current: Unless there is a good reason to keep it, > REMOVE this property (alongside ti,max-charge-voltage). Instead, have > the charger directly report back the maximum device charge current > (BQ24261_MAX_CC) via sysfs like most other charger drivers do (bq24190, > bq24257, bq25890, rt9455). I agree but wait for some more data from Ramakrishna. > >> There is no way to unify or make consistent all of these bindings. >> However one could try to add new stuff in a more sensible way. For >> example how about (for bq2425x-charger and bq24261_charger... BTW notice >> even the difference in using underscore and hyphen!): > > :( You are talking about the driver .name, right? I saw that too. If > the bq24261 charger was to change it's device name to use a hyphen at > least it would be consistent with bq2415x and bq2425x. The name of file and name of driver. Most of existing bq* drivers have "_" in file name and "-" in driver name. So maybe use it in bq2425x-charger (file name: bq2425x_charger)? To prevent future issues in naming and bindings consistency maybe there should be a dedicated maintainer for bq/TI charger devices? Pali Rohar recently took care for devices related to Nokia N900 but maybe someone from TI should also take care of rest or all of them (if TI is interested in this)? > >> ti,charge-current - maximum charging current in uA >> (that one must be supported, it's for existing bq24257 devices) > > Agreed. As discussed earlier this one is pretty established -- but in > mA (not uA). Okay. > >> ti,default-charge-current (bq24261) - default charge current in *uA* > > We don't need that. If you look where it goes (registers) this should be > called ti,charge-current for the bq24261 (like it already is). Exactly > the same name/usage as the other bqxxxx chargers. We just need to update > the description. Okay. > >> ti,current-limit (bq2425x) - maximum current to be drawn in *uA* > > Ok. Again here my preference would be mA. Like already on the bq2415x. > If we can change the bq2425x driver to mA (see separate thread) we'd be > closer to a more unified set of properties. Otherwise we would have > properties with the same name but different units (is this even > possible?). mA would be nice... but bq2425x driver must support existing device which means it must support existing bindings. Unfortunately the existing binding for bq24257 is in uA. Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Sep 10, 2015 at 09:15:09AM +0900, Krzysztof Kozlowski wrote: > On 10.09.2015 05:15, Andreas Dannenberg wrote: > > On Wed, Sep 09, 2015 at 01:58:09PM +0900, Krzysztof Kozlowski wrote: > >> > >> So we have different bindings. Existing ones: > >> bq2415x.txt - ti,charge-current - maximum charging current in mA > >> bq24257.txt - ti,charge-current - maximum charging current in uA > >> bq25890.txt - ti,charge-current - maximum charging current in uA > >> bq24735.txt - ti,charge-current - charge current (?) in mA > > > > I just spent some time with the bq24735 datasheet and the way that > > device appears to work is putting the user in control of the charging > > process rather than implementing a fully automatic control loop, but > > either way I still think it's valid to call the property > > ti,charge-current and use a description of "maximum charging current" > > for that device (there is no DT bindings doc however). And yes the unit > > is mA as one can see from reverse-engineering the register settings. > > Just for the record: the units used by device registers are not related > to units used in DT binding. You can use whatever you want. The driver > should just convert them. > > > > > On a related note the datasheet for that device says you have to > > periodically re-send the charge current setting every 44...175s to keep > > charging with the configured current or disable the device's watchdog > > timer. Neither of which the driver seems to do. I can probably go back > > and get some HW and test if that driver actually works as advertised. > > (also see http://www.ti.com/lit/ds/symlink/bq24735.pdf, Page 21) > > I mentioned existing bindings for reference. To show that it's a mess. > However you cannot change them now (at least easily). You could add new > bindings and mark old as deprecated (still they would have to be > supported by the driver)... > > > > >> bq2415x.txt - ti,current-limit - maximum current to be drawn in mA > >> > >> New bindings: > >> ti,charge-current (bq24261) - default charge current in mA > >> ti,max-charge-current (bq24261) - maximum charging current in mA > > > > This ti,max-charge-current is actually an interesting one. It's not a > > device setting as it does not impact any of the device registers at all. > > Instead, it's an artificial limit that can be set through DT that > > prevents somebody from going into sysfs and configuring a charge current > > higher than ti,max-charge-current. In other drivers I have seen that the > > sysfs property reflecting that max charge current is just read-only and > > gives you the maximum the HW is capable of. From a device point of view > > there is nothing configurable about this property. > > Hmmm, so now I wonder whether this should be a DT binding. The purpose > of DT isn't the control of the driver like enable some stuff, set some > value used by the driver. The DT provides information about hardware so > the driver could properly configure the device and work with it. > > Reason to put this into DT would be: > For example one device configuration (device, board, connected battery) > could handle one maximum value and the other configuration would require > lower one. The device and its capabilities (bq24257 for example) are the > same but configuration changes. > > > If all configurations of bq24261 device have the same maximum value then > it should not be a DT property but it should be hard-coded in the driver > instead. > > Ramakrishna, > Could you describe the reason behind "ti,max-charge-current" and > "pdata.max_cc" in bq24261 driver? > > > > >> ti,current-limit (bq2425x) - maximum current to be drawn in uA > >> > >> > >> Damn it... It's a mess. And there is no device prefixes... > > > > ACK :) Let's see how we can bring a little sense into it. > > > >> The bq24261's bindings look most sensible (max prefix for max charge > >> current) but they are not compatible with existing bindings for > >> different devices. > > > > Hmm I think they are compatible, it's just a question making the DT > > bindings description for the bq24261 fit better into what's already > > there. For example, like this: > > > > (1) ti,charge-current: integer, maximum charging current in mA. For this > > device as for others this setting controls the max current the device > > uses to charge the battery so the established description is good > > however the general use of this property name itself is not 100% > > accurate (too late for that). > > I agree. > > > (2) ti,max-charge-current: Unless there is a good reason to keep it, > > REMOVE this property (alongside ti,max-charge-voltage). Instead, have > > the charger directly report back the maximum device charge current > > (BQ24261_MAX_CC) via sysfs like most other charger drivers do (bq24190, > > bq24257, bq25890, rt9455). > > I agree but wait for some more data from Ramakrishna. > > > > >> There is no way to unify or make consistent all of these bindings. > >> However one could try to add new stuff in a more sensible way. For > >> example how about (for bq2425x-charger and bq24261_charger... BTW notice > >> even the difference in using underscore and hyphen!): > > > > :( You are talking about the driver .name, right? I saw that too. If > > the bq24261 charger was to change it's device name to use a hyphen at > > least it would be consistent with bq2415x and bq2425x. > > The name of file and name of driver. Most of existing bq* drivers have > "_" in file name and "-" in driver name. So maybe use it in > bq2425x-charger (file name: bq2425x_charger)? > > To prevent future issues in naming and bindings consistency maybe there > should be a dedicated maintainer for bq/TI charger devices? Pali Rohar > recently took care for devices related to Nokia N900 but maybe someone > from TI should also take care of rest or all of them (if TI is > interested in this)? > > > > >> ti,charge-current - maximum charging current in uA > >> (that one must be supported, it's for existing bq24257 devices) > > > > Agreed. As discussed earlier this one is pretty established -- but in > > mA (not uA). > > Okay. > > > > >> ti,default-charge-current (bq24261) - default charge current in *uA* > > > > We don't need that. If you look where it goes (registers) this should be > > called ti,charge-current for the bq24261 (like it already is). Exactly > > the same name/usage as the other bqxxxx chargers. We just need to update > > the description. > > Okay. > > > > >> ti,current-limit (bq2425x) - maximum current to be drawn in *uA* > > > > Ok. Again here my preference would be mA. Like already on the bq2415x. > > If we can change the bq2425x driver to mA (see separate thread) we'd be > > closer to a more unified set of properties. Otherwise we would have > > properties with the same name but different units (is this even > > possible?). > > mA would be nice... but bq2425x driver must support existing device > which means it must support existing bindings. Unfortunately the > existing binding for bq24257 is in uA. Oh boy... apparently this unit discrepancy mess for TI chargers was my doing. :/ I replied on the other thread already on my bindings choice... As I said there, all we can do now is agree on the binding names to be consistent. I'm afraid the units (as Krzysztof pointed) are pretty much settled... :| laurentiu -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Sep 10, 2015 at 09:15:09AM +0900, Krzysztof Kozlowski wrote: > On 10.09.2015 05:15, Andreas Dannenberg wrote: > > On Wed, Sep 09, 2015 at 01:58:09PM +0900, Krzysztof Kozlowski wrote: > >> > >> So we have different bindings. Existing ones: > >> bq2415x.txt - ti,charge-current - maximum charging current in mA > >> bq24257.txt - ti,charge-current - maximum charging current in uA > >> bq25890.txt - ti,charge-current - maximum charging current in uA > >> bq24735.txt - ti,charge-current - charge current (?) in mA > > > > I just spent some time with the bq24735 datasheet and the way that > > device appears to work is putting the user in control of the charging > > process rather than implementing a fully automatic control loop, but > > either way I still think it's valid to call the property > > ti,charge-current and use a description of "maximum charging current" > > for that device (there is no DT bindings doc however). And yes the unit > > is mA as one can see from reverse-engineering the register settings. > > Just for the record: the units used by device registers are not related > to units used in DT binding. You can use whatever you want. The driver > should just convert them. > > > > > On a related note the datasheet for that device says you have to > > periodically re-send the charge current setting every 44...175s to keep > > charging with the configured current or disable the device's watchdog > > timer. Neither of which the driver seems to do. I can probably go back > > and get some HW and test if that driver actually works as advertised. > > (also see http://www.ti.com/lit/ds/symlink/bq24735.pdf, Page 21) > > I mentioned existing bindings for reference. To show that it's a mess. > However you cannot change them now (at least easily). You could add new > bindings and mark old as deprecated (still they would have to be > supported by the driver)... Understood. > >> bq2415x.txt - ti,current-limit - maximum current to be drawn in mA > >> > >> New bindings: > >> ti,charge-current (bq24261) - default charge current in mA > >> ti,max-charge-current (bq24261) - maximum charging current in mA > > > > This ti,max-charge-current is actually an interesting one. It's not a > > device setting as it does not impact any of the device registers at all. > > Instead, it's an artificial limit that can be set through DT that > > prevents somebody from going into sysfs and configuring a charge current > > higher than ti,max-charge-current. In other drivers I have seen that the > > sysfs property reflecting that max charge current is just read-only and > > gives you the maximum the HW is capable of. From a device point of view > > there is nothing configurable about this property. > > Hmmm, so now I wonder whether this should be a DT binding. The purpose > of DT isn't the control of the driver like enable some stuff, set some > value used by the driver. The DT provides information about hardware so > the driver could properly configure the device and work with it. > > Reason to put this into DT would be: > For example one device configuration (device, board, connected battery) > could handle one maximum value and the other configuration would require > lower one. The device and its capabilities (bq24257 for example) are the > same but configuration changes. Yes this could be useful in this case but this property only makes sense if userspace is allowed to change the actual charge current through sysfs as some drivers do including Ram's proposed bq24261 driver. But if we get rid of the general sysfs configurability of the charge current (which I say we should) we don't need that ti,max-charge-current property. Also see... http://marc.info/?l=linux-pm&m=143080413218161&w=2 > If all configurations of bq24261 device have the same maximum value then > it should not be a DT property but it should be hard-coded in the driver > instead. > > Ramakrishna, > Could you describe the reason behind "ti,max-charge-current" and > "pdata.max_cc" in bq24261 driver? [...] > >> There is no way to unify or make consistent all of these bindings. > >> However one could try to add new stuff in a more sensible way. For > >> example how about (for bq2425x-charger and bq24261_charger... BTW notice > >> even the difference in using underscore and hyphen!): > > > > :( You are talking about the driver .name, right? I saw that too. If > > the bq24261 charger was to change it's device name to use a hyphen at > > least it would be consistent with bq2415x and bq2425x. > > The name of file and name of driver. Most of existing bq* drivers have > "_" in file name and "-" in driver name. So maybe use it in > bq2425x-charger (file name: bq2425x_charger)? Yes the bq2425x_charger already follows the same trend - underscore in the filename and dash in the driver name. The bq26261_charger.c driver is the one that should be adopted (already provided feedback on a separate thread). > To prevent future issues in naming and bindings consistency maybe there > should be a dedicated maintainer for bq/TI charger devices? Pali Rohar > recently took care for devices related to Nokia N900 but maybe someone > from TI should also take care of rest or all of them (if TI is > interested in this)? I'd be happy to step in here. Let me know how I can help and contribute. [...] > >> ti,current-limit (bq2425x) - maximum current to be drawn in *uA* > > > > Ok. Again here my preference would be mA. Like already on the bq2415x. > > If we can change the bq2425x driver to mA (see separate thread) we'd be > > closer to a more unified set of properties. Otherwise we would have > > properties with the same name but different units (is this even > > possible?). > > mA would be nice... but bq2425x driver must support existing device > which means it must support existing bindings. Unfortunately the > existing binding for bq24257 is in uA. Ok understood. I guess I interpreted your previous statement with you being ok with either milli or micro too broadly. Will stick to what we have then. Regards, -- Andreas Dannenberg Texas Instruments Inc -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Sep 10, 2015 at 06:00:32PM +0300, Laurentiu Palcu wrote: > On Thu, Sep 10, 2015 at 09:15:09AM +0900, Krzysztof Kozlowski wrote: > > On 10.09.2015 05:15, Andreas Dannenberg wrote: [...] > > >> ti,current-limit (bq2425x) - maximum current to be drawn in *uA* > > > > > > Ok. Again here my preference would be mA. Like already on the bq2415x. > > > If we can change the bq2425x driver to mA (see separate thread) we'd be > > > closer to a more unified set of properties. Otherwise we would have > > > properties with the same name but different units (is this even > > > possible?). > > > > mA would be nice... but bq2425x driver must support existing device > > which means it must support existing bindings. Unfortunately the > > existing binding for bq24257 is in uA. > > Oh boy... apparently this unit discrepancy mess for TI chargers was my > doing. :/ > > I replied on the other thread already on my bindings choice... As I said > there, all we can do now is agree on the binding names to be consistent. > I'm afraid the units (as Krzysztof pointed) are pretty much settled... > :| Hi Laurentiu, Yes let's keep the binding names consistent. And as for the units - given the new state of the world (recent driver additions, inability to change units on existing drivers) the way this aspects looks to me now we should probably go wih uA/uV for all new bqXXX chargers. It's not a big deal, looks like micro units are widely used in other parts of the Kernel already (see IIO framework). As indicated in the other thread I'll be happy to step in as the TI/bqXXX DT bindings maintainer moving forward. Regards, -- Andreas Dannenberg Texas Instruments Inc -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11.09.2015 05:57, Andreas Dannenberg wrote: > On Thu, Sep 10, 2015 at 09:15:09AM +0900, Krzysztof Kozlowski wrote: (...) > >>>> bq2415x.txt - ti,current-limit - maximum current to be drawn in mA >>>> >>>> New bindings: >>>> ti,charge-current (bq24261) - default charge current in mA >>>> ti,max-charge-current (bq24261) - maximum charging current in mA >>> >>> This ti,max-charge-current is actually an interesting one. It's not a >>> device setting as it does not impact any of the device registers at all. >>> Instead, it's an artificial limit that can be set through DT that >>> prevents somebody from going into sysfs and configuring a charge current >>> higher than ti,max-charge-current. In other drivers I have seen that the >>> sysfs property reflecting that max charge current is just read-only and >>> gives you the maximum the HW is capable of. From a device point of view >>> there is nothing configurable about this property. >> >> Hmmm, so now I wonder whether this should be a DT binding. The purpose >> of DT isn't the control of the driver like enable some stuff, set some >> value used by the driver. The DT provides information about hardware so >> the driver could properly configure the device and work with it. >> >> Reason to put this into DT would be: >> For example one device configuration (device, board, connected battery) >> could handle one maximum value and the other configuration would require >> lower one. The device and its capabilities (bq24257 for example) are the >> same but configuration changes. > > Yes this could be useful in this case but this property only makes sense > if userspace is allowed to change the actual charge current through > sysfs as some drivers do including Ram's proposed bq24261 driver. But if we > get rid of the general sysfs configurability of the charge current > (which I say we should) we don't need that ti,max-charge-current property. > Also see... > > http://marc.info/?l=linux-pm&m=143080413218161&w=2 Right, that's my previous reply. :) I must admit that here and for bq24261 I looked mostly at bindings so I did not initially about the sysfs interface. Anyway I am not convinced to having such sysfs interfaces and definitely they should not mess with DT. I would rather expect these to be totally orthogonal. Summarizing all these bq24261 properties: - ti,enable-user-write - ti,max-charge-current - ti,max-charge-voltage are related to that sysfs interface, not to hardware configuration. If that's true, then all should be gone. > >> To prevent future issues in naming and bindings consistency maybe there >> should be a dedicated maintainer for bq/TI charger devices? Pali Rohar >> recently took care for devices related to Nokia N900 but maybe someone >> from TI should also take care of rest or all of them (if TI is >> interested in this)? > > I'd be happy to step in here. Let me know how I can help and contribute. I don't know Sebastian's opinion on that idea but I think usually a new maintainer and reviewer of particular drivers is welcomed by community. I see also Intel's interest and his contributions in these chargers. Laurentiu seems to do a thorough review as well. There can be both official reviewers. It's up to you people. Just make a first step and we'll see how other people react (and what Sebastian thinks about it :) ). Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Sep 11, 2015 at 09:34:10AM +0900, Krzysztof Kozlowski wrote: > On 11.09.2015 05:57, Andreas Dannenberg wrote: > > On Thu, Sep 10, 2015 at 09:15:09AM +0900, Krzysztof Kozlowski wrote: > > (...) > > > > >>>> bq2415x.txt - ti,current-limit - maximum current to be drawn in mA > >>>> > >>>> New bindings: > >>>> ti,charge-current (bq24261) - default charge current in mA > >>>> ti,max-charge-current (bq24261) - maximum charging current in mA > >>> > >>> This ti,max-charge-current is actually an interesting one. It's not a > >>> device setting as it does not impact any of the device registers at all. > >>> Instead, it's an artificial limit that can be set through DT that > >>> prevents somebody from going into sysfs and configuring a charge current > >>> higher than ti,max-charge-current. In other drivers I have seen that the > >>> sysfs property reflecting that max charge current is just read-only and > >>> gives you the maximum the HW is capable of. From a device point of view > >>> there is nothing configurable about this property. > >> > >> Hmmm, so now I wonder whether this should be a DT binding. The purpose > >> of DT isn't the control of the driver like enable some stuff, set some > >> value used by the driver. The DT provides information about hardware so > >> the driver could properly configure the device and work with it. > >> > >> Reason to put this into DT would be: > >> For example one device configuration (device, board, connected battery) > >> could handle one maximum value and the other configuration would require > >> lower one. The device and its capabilities (bq24257 for example) are the > >> same but configuration changes. > > > > Yes this could be useful in this case but this property only makes sense > > if userspace is allowed to change the actual charge current through > > sysfs as some drivers do including Ram's proposed bq24261 driver. But if we > > get rid of the general sysfs configurability of the charge current > > (which I say we should) we don't need that ti,max-charge-current property. > > Also see... > > > > http://marc.info/?l=linux-pm&m=143080413218161&w=2 > > Right, that's my previous reply. :) I must admit that here and for > bq24261 I looked mostly at bindings so I did not initially about the > sysfs interface. > > Anyway I am not convinced to having such sysfs interfaces and definitely > they should not mess with DT. I would rather expect these to be totally > orthogonal. > > Summarizing all these bq24261 properties: > - ti,enable-user-write > - ti,max-charge-current > - ti,max-charge-voltage > are related to that sysfs interface, not to hardware configuration. If > that's true, then all should be gone. > > > > >> To prevent future issues in naming and bindings consistency maybe there > >> should be a dedicated maintainer for bq/TI charger devices? Pali Rohar > >> recently took care for devices related to Nokia N900 but maybe someone > >> from TI should also take care of rest or all of them (if TI is > >> interested in this)? > > > > I'd be happy to step in here. Let me know how I can help and contribute. > > I don't know Sebastian's opinion on that idea but I think usually a new > maintainer and reviewer of particular drivers is welcomed by community. > I see also Intel's interest and his contributions in these chargers. > Laurentiu seems to do a thorough review as well. There can be both > official reviewers. Hi Krzysztof, If we already have good coverage then that's great. The value I can add is that I literally sit in the same building with the "battery charger people" :) so it's easy to go talk to them to clarify technical details amongst other things that might come up as drivers are getting developed/reviewed. So at a minimum folks like Laurentiu should feel free to reach out to me as needed. Regards, -- Andreas Dannenberg Texas Instruments Inc > It's up to you people. Just make a first step and we'll see how other > people react (and what Sebastian thinks about it :) ). > > Best regards, > Krzysztof > -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/power/bq24257.txt b/Documentation/devicetree/bindings/power/bq24257.txt deleted file mode 100644 index 5c9d394..0000000 --- a/Documentation/devicetree/bindings/power/bq24257.txt +++ /dev/null @@ -1,21 +0,0 @@ -Binding for TI bq24257 Li-Ion Charger - -Required properties: -- compatible: Should contain one of the following: - * "ti,bq24257" -- reg: integer, i2c address of the device. -- ti,battery-regulation-voltage: integer, maximum charging voltage in uV. -- ti,charge-current: integer, maximum charging current in uA. -- ti,termination-current: integer, charge will be terminated when current in - constant-voltage phase drops below this value (in uA). - -Example: - -bq24257 { - compatible = "ti,bq24257"; - reg = <0x6a>; - - ti,battery-regulation-voltage = <4200000>; - ti,charge-current = <1000000>; - ti,termination-current = <50000>; -}; diff --git a/Documentation/devicetree/bindings/power/bq2425x.txt b/Documentation/devicetree/bindings/power/bq2425x.txt new file mode 100644 index 0000000..3e171c3 --- /dev/null +++ b/Documentation/devicetree/bindings/power/bq2425x.txt @@ -0,0 +1,70 @@ +Binding for TI bq2425x Li-Ion Charger + +Required properties: +- compatible: Should contain one of the following: + * "ti,bq24250" + * "ti,bq24251" + * "ti,bq24257" +- reg: integer, i2c address of the device. +- stat-gpios: GPIO used for the devices STAT_IN pin. Alternatively the pin can + also be defined through the standard interrupt definition properties (see + optional properties section below). Only use one method. +- ti,battery-regulation-voltage: integer, maximum charging voltage in uV. +- ti,charge-current: integer, maximum charging current in uA. +- ti,termination-current: integer, charge will be terminated when current in + constant-voltage phase drops below this value (in uA). + +Optional properties: +- pg-gpios: GPIO used for connecting the bq2425x device PG (Power Good) pin. + This pin is not available on all devices however it should be used if + possible as this is the recommended way to obtain the charger's input PG + state. If this pin is not specified a software-based approach for PG + detection is used. +- ti,current-limit: The maximum current to be drawn from the charger's input + (in uA). If this property is not specified a USB D+/D- signal based charger + type detection is used (if available) and the input limit current is set + accordingly. If the D+/D- based detection is not available on a given device + a default of 500,000 is used (=500mA). +- ti,ovp-voltage: Configures the over voltage protection voltage (in uV). If + not specified a default of 6,5000,000 (=6.5V) is used. +- ti,in-dpm-voltage: Configures the threshold input voltage for the dynamic + power path management (in uV). If not specified a default of 4,360,000 + (=4.36V) is used. +- ti,safety-timer-2x-enable: If this property is set to 1 the device's safety + timer is extended (slowed down) by a factor of two. Setting this property + to 0 or not providing it will leave the safety timer at its default + setting. +- interrupt-parent: Should be the phandle for the interrupt controller. Use in + conjunction with "interrupts" and only in case "stat-gpios" is not used. +- interrupts: Interrupt mapping for GPIO IRQ (configure for both edges). Use in + conjunction with "interrupt-parent" and only in case "stat-gpios" is not + used. + +Example: + +bq24257 { + compatible = "ti,bq24257"; + reg = <0x6a>; + stat-gpios = <&gpio1 16 GPIO_ACTIVE_HIGH>; + pg-gpios = <&gpio1 28 GPIO_ACTIVE_HIGH>; + + ti,battery-regulation-voltage = <4200000>; + ti,charge-current = <1000000>; + ti,termination-current = <50000>; +}; + +Example: + +bq24250 { + compatible = "ti,bq24250"; + reg = <0x6a>; + interrupt-parent = <&gpio1>; + interrupts = <16 IRQ_TYPE_EDGE_BOTH>; + + ti,battery-regulation-voltage = <4200000>; + ti,charge-current = <500000>; + ti,termination-current = <50000>; + ti,current-limit = <900000>; + ti,ovp-voltage = <9500000>; + ti,in-dpm-voltage = <4440000>; +};
Extend the bq2425x charger's device tree documentation to cover the bq24250 and bq24257 devices as well as recent feature additions. Signed-off-by: Andreas Dannenberg <dannenberg@ti.com> --- .../devicetree/bindings/power/bq24257.txt | 21 ------- .../devicetree/bindings/power/bq2425x.txt | 70 ++++++++++++++++++++++ 2 files changed, 70 insertions(+), 21 deletions(-) delete mode 100644 Documentation/devicetree/bindings/power/bq24257.txt create mode 100644 Documentation/devicetree/bindings/power/bq2425x.txt