Re: [PATCH] drivers/rtc/rtc-palmas.c: support for backup battery charging
On Saturday 27 July 2013 02:15 AM, Tomasz Figa wrote: On Friday 26 of July 2013 09:40:15 Stephen Warren wrote: (CC'ing the new DT binding maintainers and mailing list on this reply, hence quoting the whole of the DT binding) On 07/25/2013 06:29 AM, Laxman Dewangan wrote: Palmas series device like TPS65913, TPS80036 supports the backup battery for powering the RTC when no other energy source is available. The backup battery is optional, connected to the VBACKUP pin, and can be nonrechargeable or rechargeable. The rechargeable battery can be charged from the system supply using the backup battery charger. Add support for enabling charging of this backup battery. Also add the DT binding document and the new properties to have this support. Signed-off-by: Laxman Dewangan --- .../devicetree/bindings/rtc/rtc-palmas.txt | 28 ++ drivers/rtc/rtc-palmas.c | 39 2 files changed, 67 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/rtc/rtc-palmas.txt diff --git a/Documentation/devicetree/bindings/rtc/rtc-palmas.txt b/Documentation/devicetree/bindings/rtc/rtc-palmas.txt new file mode 100644 index 000..e4b6910 --- /dev/null +++ b/Documentation/devicetree/bindings/rtc/rtc-palmas.txt @@ -0,0 +1,28 @@ +Palmas RTC controller bindings + +Required properties: +- compatible: + - "ti,palams-rtc" for palma series of the RTC controller +- interrupt-parent: Parent interrupt device, must be handle of palams node. +- interrupts: Interrupt number of RTC submodule on device. + +Optional properties: +- ti,back-bat-chg-enable: The palmas series device like TPS65913 or TPS80036 + supports the battery backup for powering the RTC when main battery is +removed or in very low power state. This flag will enable the backup + battery charging. +- ti,back-bat-chg-current: Configure charging current. Device supports the + charging current as < 100mA or >100mA. Does the HW support just two options; less-than or greater-than 100mA? If so, a Boolean property here might be better. The code below certainly implies this. Yes, it supports the two option, less than 100ma and 100ma. I will change it to bool. Given there's only 1 battery, I think "back-" is redundant in the property names. Since that shortens the names a bit, I'd suggest spelling everything out in full, perhaps: battery-charge-enable battery-charge-low-current First of all, are those even properties for the RTC binding? Doesn't this PMIC contain a charger part which handles battery charging and so requires such kind of information? There is two appropriate place for enabling battery backup charging which I can think of, one is in core and other is in RTC driver. I do not want to new driver for enabling charging as the charger driver. There is separate charger module on one of palma device which need full flashed driver. As this backup battery provides power to the RTC only in absence of main energy source, I thought of putting this on RTC. There is no issue to put this piece of code on other palma driver. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/rtc/rtc-palmas.c: support for backup battery charging
On Friday 26 July 2013 10:12 PM, Stephen Warren wrote: On 07/26/2013 10:35 AM, Mark Brown wrote: On Fri, Jul 26, 2013 at 09:40:15AM -0600, Stephen Warren wrote: Given there's only 1 battery, I think "back-" is redundant in the property names. Since that shortens the names a bit, I'd suggest spelling everything out in full, perhaps: Isn't there an integrated charger on the PMIC? If there is then it's useful to identify which is being talked about (even if it should be relatively obvious). Oh yes, that's probably true. This is of course the binding for the RTC sub-module in the PMIC though. I have currently Palma series 2 devices tps65913 and tps80036. TPS80036 supports the charging of main battery. Both chip support the backup battery charging. Hence it is good to name the back-* here. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/rtc/rtc-palmas.c: support for backup battery charging
On Friday 26 July 2013 10:12 PM, Stephen Warren wrote: On 07/26/2013 10:35 AM, Mark Brown wrote: On Fri, Jul 26, 2013 at 09:40:15AM -0600, Stephen Warren wrote: Given there's only 1 battery, I think back- is redundant in the property names. Since that shortens the names a bit, I'd suggest spelling everything out in full, perhaps: Isn't there an integrated charger on the PMIC? If there is then it's useful to identify which is being talked about (even if it should be relatively obvious). Oh yes, that's probably true. This is of course the binding for the RTC sub-module in the PMIC though. I have currently Palma series 2 devices tps65913 and tps80036. TPS80036 supports the charging of main battery. Both chip support the backup battery charging. Hence it is good to name the back-* here. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/rtc/rtc-palmas.c: support for backup battery charging
On Saturday 27 July 2013 02:15 AM, Tomasz Figa wrote: On Friday 26 of July 2013 09:40:15 Stephen Warren wrote: (CC'ing the new DT binding maintainers and mailing list on this reply, hence quoting the whole of the DT binding) On 07/25/2013 06:29 AM, Laxman Dewangan wrote: Palmas series device like TPS65913, TPS80036 supports the backup battery for powering the RTC when no other energy source is available. The backup battery is optional, connected to the VBACKUP pin, and can be nonrechargeable or rechargeable. The rechargeable battery can be charged from the system supply using the backup battery charger. Add support for enabling charging of this backup battery. Also add the DT binding document and the new properties to have this support. Signed-off-by: Laxman Dewangan ldewan...@nvidia.com --- .../devicetree/bindings/rtc/rtc-palmas.txt | 28 ++ drivers/rtc/rtc-palmas.c | 39 2 files changed, 67 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/rtc/rtc-palmas.txt diff --git a/Documentation/devicetree/bindings/rtc/rtc-palmas.txt b/Documentation/devicetree/bindings/rtc/rtc-palmas.txt new file mode 100644 index 000..e4b6910 --- /dev/null +++ b/Documentation/devicetree/bindings/rtc/rtc-palmas.txt @@ -0,0 +1,28 @@ +Palmas RTC controller bindings + +Required properties: +- compatible: + - ti,palams-rtc for palma series of the RTC controller +- interrupt-parent: Parent interrupt device, must be handle of palams node. +- interrupts: Interrupt number of RTC submodule on device. + +Optional properties: +- ti,back-bat-chg-enable: The palmas series device like TPS65913 or TPS80036 + supports the battery backup for powering the RTC when main battery is +removed or in very low power state. This flag will enable the backup + battery charging. +- ti,back-bat-chg-current: Configure charging current. Device supports the + charging current as 100mA or 100mA. Does the HW support just two options; less-than or greater-than 100mA? If so, a Boolean property here might be better. The code below certainly implies this. Yes, it supports the two option, less than 100ma and 100ma. I will change it to bool. Given there's only 1 battery, I think back- is redundant in the property names. Since that shortens the names a bit, I'd suggest spelling everything out in full, perhaps: battery-charge-enable battery-charge-low-current First of all, are those even properties for the RTC binding? Doesn't this PMIC contain a charger part which handles battery charging and so requires such kind of information? There is two appropriate place for enabling battery backup charging which I can think of, one is in core and other is in RTC driver. I do not want to new driver for enabling charging as the charger driver. There is separate charger module on one of palma device which need full flashed driver. As this backup battery provides power to the RTC only in absence of main energy source, I thought of putting this on RTC. There is no issue to put this piece of code on other palma driver. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/rtc/rtc-palmas.c: support for backup battery charging
On Friday 26 of July 2013 09:40:15 Stephen Warren wrote: > (CC'ing the new DT binding maintainers and mailing list on this reply, > hence quoting the whole of the DT binding) > > On 07/25/2013 06:29 AM, Laxman Dewangan wrote: > > Palmas series device like TPS65913, TPS80036 supports the backup > > battery for powering the RTC when no other energy source is > > available. > > > > The backup battery is optional, connected to the VBACKUP pin, and can > > be nonrechargeable or rechargeable. The rechargeable battery can be > > charged from the system supply using the backup battery charger. > > > > Add support for enabling charging of this backup battery. Also add > > the DT binding document and the new properties to have this support. > > > > Signed-off-by: Laxman Dewangan > > --- > > > > .../devicetree/bindings/rtc/rtc-palmas.txt | 28 > > ++ drivers/rtc/rtc-palmas.c | > > 39 2 files changed, 67 insertions(+), 0 > > deletions(-) > > create mode 100644 > > Documentation/devicetree/bindings/rtc/rtc-palmas.txt > > > > diff --git a/Documentation/devicetree/bindings/rtc/rtc-palmas.txt > > b/Documentation/devicetree/bindings/rtc/rtc-palmas.txt new file mode > > 100644 > > index 000..e4b6910 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/rtc/rtc-palmas.txt > > @@ -0,0 +1,28 @@ > > +Palmas RTC controller bindings > > + > > +Required properties: > > +- compatible: > > + - "ti,palams-rtc" for palma series of the RTC controller > > +- interrupt-parent: Parent interrupt device, must be handle of palams > > node. +- interrupts: Interrupt number of RTC submodule on device. > > + > > +Optional properties: > > +- ti,back-bat-chg-enable: The palmas series device like TPS65913 or > > TPS80036 + supports the battery backup for powering the RTC when main > > battery is +removed or in very low power state. This flag will > > enable the backup + battery charging. > > +- ti,back-bat-chg-current: Configure charging current. Device > > supports the + charging current as < 100mA or >100mA. > > Does the HW support just two options; less-than or greater-than 100mA? > If so, a Boolean property here might be better. The code below certainly > implies this. > > Given there's only 1 battery, I think "back-" is redundant in the > property names. Since that shortens the names a bit, I'd suggest > spelling everything out in full, perhaps: > > battery-charge-enable > battery-charge-low-current First of all, are those even properties for the RTC binding? Doesn't this PMIC contain a charger part which handles battery charging and so requires such kind of information? Best regards, Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/rtc/rtc-palmas.c: support for backup battery charging
On 07/26/2013 10:35 AM, Mark Brown wrote: > On Fri, Jul 26, 2013 at 09:40:15AM -0600, Stephen Warren wrote: > >> Given there's only 1 battery, I think "back-" is redundant in >> the property names. Since that shortens the names a bit, I'd >> suggest spelling everything out in full, perhaps: > > Isn't there an integrated charger on the PMIC? If there is then > it's useful to identify which is being talked about (even if it > should be relatively obvious). Oh yes, that's probably true. This is of course the binding for the RTC sub-module in the PMIC though. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/rtc/rtc-palmas.c: support for backup battery charging
On Fri, Jul 26, 2013 at 09:40:15AM -0600, Stephen Warren wrote: > Given there's only 1 battery, I think "back-" is redundant in the > property names. Since that shortens the names a bit, I'd suggest > spelling everything out in full, perhaps: Isn't there an integrated charger on the PMIC? If there is then it's useful to identify which is being talked about (even if it should be relatively obvious). signature.asc Description: Digital signature
Re: [PATCH] drivers/rtc/rtc-palmas.c: support for backup battery charging
On Fri, Jul 26, 2013 at 04:40:15PM +0100, Stephen Warren wrote:
> (CC'ing the new DT binding maintainers and mailing list on this reply,
> hence quoting the whole of the DT binding)
Thanks Stephen.
>
> On 07/25/2013 06:29 AM, Laxman Dewangan wrote:
> > Palmas series device like TPS65913, TPS80036 supports the backup battery
> > for powering the RTC when no other energy source is available.
> >
> > The backup battery is optional, connected to the VBACKUP pin, and can be
> > nonrechargeable or rechargeable. The rechargeable battery can be charged
> > from the system supply using the backup battery charger.
> >
> > Add support for enabling charging of this backup battery. Also add the DT
> > binding document and the new properties to have this support.
> >
> > Signed-off-by: Laxman Dewangan
> > ---
> > .../devicetree/bindings/rtc/rtc-palmas.txt | 28 ++
> > drivers/rtc/rtc-palmas.c | 39
> >
> > 2 files changed, 67 insertions(+), 0 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/rtc/rtc-palmas.txt
> >
> > diff --git a/Documentation/devicetree/bindings/rtc/rtc-palmas.txt
> > b/Documentation/devicetree/bindings/rtc/rtc-palmas.txt
> > new file mode 100644
> > index 000..e4b6910
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/rtc/rtc-palmas.txt
> > @@ -0,0 +1,28 @@
> > +Palmas RTC controller bindings
> > +
> > +Required properties:
> > +- compatible:
> > + - "ti,palams-rtc" for palma series of the RTC controller
> > +- interrupt-parent: Parent interrupt device, must be handle of palams node.
> > +- interrupts: Interrupt number of RTC submodule on device.
> > +
> > +Optional properties:
> > +- ti,back-bat-chg-enable: The palmas series device like TPS65913 or
> > TPS80036
> > + supports the battery backup for powering the RTC when main battery is
> > + removed or in very low power state. This flag will enable the backup
> > + battery charging.
Perhaps "ti,has-rechargeable-battery" would be better, as this describes
the hardware rather than an intent relating to it.
> > +- ti,back-bat-chg-current: Configure charging current. Device supports the
> > + charging current as < 100mA or >100mA.
Are either of these values unsafe or unsupported in some configurations?
>
> Does the HW support just two options; less-than or greater-than 100mA?
> If so, a Boolean property here might be better. The code below certainly
> implies this.
>
> Given there's only 1 battery, I think "back-" is redundant in the
> property names. Since that shortens the names a bit, I'd suggest
> spelling everything out in full, perhaps:
>
> battery-charge-enable
> battery-charge-low-current
>
> Both Boolean.
>
> > +Example:
> > + palmas: tps65913@58 {
> > + :::
>
> "..." is probably more common than lots of colons, or you could just
> delete this line.
>
> > + palmas_rtc: rtc {
> > + compatible = "ti,palmas-rtc";
> > + interrupt-parent = <>;
> > + interrupts = <8 0>;
> > + ti,back-bat-chg-enable;
> > + ti,back-bat-chg-current = <100>;
> > + };
> > + :::
> > + };
>
> > diff --git a/drivers/rtc/rtc-palmas.c b/drivers/rtc/rtc-palmas.c
>
> > @@ -238,6 +238,19 @@ static int palmas_rtc_probe(struct platform_device
> > *pdev)
>
> > + ret = of_property_read_u32(pdev->dev.of_node,
> > + "ti,back-bat-chg-current", );
> > + if (!ret)
> > + bb_charging_current = pval;
>
> Do you need to validate that pval is one of the legal values?
>
> > @@ -254,6 +267,32 @@ static int palmas_rtc_probe(struct platform_device
> > *pdev)
> > palmas_rtc->dev = >dev;
> > platform_set_drvdata(pdev, palmas_rtc);
> >
> > + if (enable_bb_charging) {
> > + unsigned reg = 0;
> > +
> > + if (bb_charging_current < 100)
> > + reg |= PALMAS_BACKUP_BATTERY_CTRL_BBS_BBC_LOW_ICHRG;
>
> This implies that a Boolean property would be a better representation of HW.
>
Otherwise, +1 to Stephen's comments.
Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/rtc/rtc-palmas.c: support for backup battery charging
(CC'ing the new DT binding maintainers and mailing list on this reply,
hence quoting the whole of the DT binding)
On 07/25/2013 06:29 AM, Laxman Dewangan wrote:
> Palmas series device like TPS65913, TPS80036 supports the backup battery
> for powering the RTC when no other energy source is available.
>
> The backup battery is optional, connected to the VBACKUP pin, and can be
> nonrechargeable or rechargeable. The rechargeable battery can be charged
> from the system supply using the backup battery charger.
>
> Add support for enabling charging of this backup battery. Also add the DT
> binding document and the new properties to have this support.
>
> Signed-off-by: Laxman Dewangan
> ---
> .../devicetree/bindings/rtc/rtc-palmas.txt | 28 ++
> drivers/rtc/rtc-palmas.c | 39
>
> 2 files changed, 67 insertions(+), 0 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/rtc/rtc-palmas.txt
>
> diff --git a/Documentation/devicetree/bindings/rtc/rtc-palmas.txt
> b/Documentation/devicetree/bindings/rtc/rtc-palmas.txt
> new file mode 100644
> index 000..e4b6910
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/rtc-palmas.txt
> @@ -0,0 +1,28 @@
> +Palmas RTC controller bindings
> +
> +Required properties:
> +- compatible:
> + - "ti,palams-rtc" for palma series of the RTC controller
> +- interrupt-parent: Parent interrupt device, must be handle of palams node.
> +- interrupts: Interrupt number of RTC submodule on device.
> +
> +Optional properties:
> +- ti,back-bat-chg-enable: The palmas series device like TPS65913 or TPS80036
> + supports the battery backup for powering the RTC when main battery is
> + removed or in very low power state. This flag will enable the backup
> + battery charging.
> +- ti,back-bat-chg-current: Configure charging current. Device supports the
> + charging current as < 100mA or >100mA.
Does the HW support just two options; less-than or greater-than 100mA?
If so, a Boolean property here might be better. The code below certainly
implies this.
Given there's only 1 battery, I think "back-" is redundant in the
property names. Since that shortens the names a bit, I'd suggest
spelling everything out in full, perhaps:
battery-charge-enable
battery-charge-low-current
Both Boolean.
> +Example:
> + palmas: tps65913@58 {
> + :::
"..." is probably more common than lots of colons, or you could just
delete this line.
> + palmas_rtc: rtc {
> + compatible = "ti,palmas-rtc";
> + interrupt-parent = <>;
> + interrupts = <8 0>;
> + ti,back-bat-chg-enable;
> + ti,back-bat-chg-current = <100>;
> + };
> + :::
> + };
> diff --git a/drivers/rtc/rtc-palmas.c b/drivers/rtc/rtc-palmas.c
> @@ -238,6 +238,19 @@ static int palmas_rtc_probe(struct platform_device *pdev)
> + ret = of_property_read_u32(pdev->dev.of_node,
> + "ti,back-bat-chg-current", );
> + if (!ret)
> + bb_charging_current = pval;
Do you need to validate that pval is one of the legal values?
> @@ -254,6 +267,32 @@ static int palmas_rtc_probe(struct platform_device *pdev)
> palmas_rtc->dev = >dev;
> platform_set_drvdata(pdev, palmas_rtc);
>
> + if (enable_bb_charging) {
> + unsigned reg = 0;
> +
> + if (bb_charging_current < 100)
> + reg |= PALMAS_BACKUP_BATTERY_CTRL_BBS_BBC_LOW_ICHRG;
This implies that a Boolean property would be a better representation of HW.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/rtc/rtc-palmas.c: support for backup battery charging
(CC'ing the new DT binding maintainers and mailing list on this reply,
hence quoting the whole of the DT binding)
On 07/25/2013 06:29 AM, Laxman Dewangan wrote:
Palmas series device like TPS65913, TPS80036 supports the backup battery
for powering the RTC when no other energy source is available.
The backup battery is optional, connected to the VBACKUP pin, and can be
nonrechargeable or rechargeable. The rechargeable battery can be charged
from the system supply using the backup battery charger.
Add support for enabling charging of this backup battery. Also add the DT
binding document and the new properties to have this support.
Signed-off-by: Laxman Dewangan ldewan...@nvidia.com
---
.../devicetree/bindings/rtc/rtc-palmas.txt | 28 ++
drivers/rtc/rtc-palmas.c | 39
2 files changed, 67 insertions(+), 0 deletions(-)
create mode 100644 Documentation/devicetree/bindings/rtc/rtc-palmas.txt
diff --git a/Documentation/devicetree/bindings/rtc/rtc-palmas.txt
b/Documentation/devicetree/bindings/rtc/rtc-palmas.txt
new file mode 100644
index 000..e4b6910
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/rtc-palmas.txt
@@ -0,0 +1,28 @@
+Palmas RTC controller bindings
+
+Required properties:
+- compatible:
+ - ti,palams-rtc for palma series of the RTC controller
+- interrupt-parent: Parent interrupt device, must be handle of palams node.
+- interrupts: Interrupt number of RTC submodule on device.
+
+Optional properties:
+- ti,back-bat-chg-enable: The palmas series device like TPS65913 or TPS80036
+ supports the battery backup for powering the RTC when main battery is
+ removed or in very low power state. This flag will enable the backup
+ battery charging.
+- ti,back-bat-chg-current: Configure charging current. Device supports the
+ charging current as 100mA or 100mA.
Does the HW support just two options; less-than or greater-than 100mA?
If so, a Boolean property here might be better. The code below certainly
implies this.
Given there's only 1 battery, I think back- is redundant in the
property names. Since that shortens the names a bit, I'd suggest
spelling everything out in full, perhaps:
battery-charge-enable
battery-charge-low-current
Both Boolean.
+Example:
+ palmas: tps65913@58 {
+ :::
... is probably more common than lots of colons, or you could just
delete this line.
+ palmas_rtc: rtc {
+ compatible = ti,palmas-rtc;
+ interrupt-parent = palmas;
+ interrupts = 8 0;
+ ti,back-bat-chg-enable;
+ ti,back-bat-chg-current = 100;
+ };
+ :::
+ };
diff --git a/drivers/rtc/rtc-palmas.c b/drivers/rtc/rtc-palmas.c
@@ -238,6 +238,19 @@ static int palmas_rtc_probe(struct platform_device *pdev)
+ ret = of_property_read_u32(pdev-dev.of_node,
+ ti,back-bat-chg-current, pval);
+ if (!ret)
+ bb_charging_current = pval;
Do you need to validate that pval is one of the legal values?
@@ -254,6 +267,32 @@ static int palmas_rtc_probe(struct platform_device *pdev)
palmas_rtc-dev = pdev-dev;
platform_set_drvdata(pdev, palmas_rtc);
+ if (enable_bb_charging) {
+ unsigned reg = 0;
+
+ if (bb_charging_current 100)
+ reg |= PALMAS_BACKUP_BATTERY_CTRL_BBS_BBC_LOW_ICHRG;
This implies that a Boolean property would be a better representation of HW.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/rtc/rtc-palmas.c: support for backup battery charging
On Fri, Jul 26, 2013 at 04:40:15PM +0100, Stephen Warren wrote:
(CC'ing the new DT binding maintainers and mailing list on this reply,
hence quoting the whole of the DT binding)
Thanks Stephen.
On 07/25/2013 06:29 AM, Laxman Dewangan wrote:
Palmas series device like TPS65913, TPS80036 supports the backup battery
for powering the RTC when no other energy source is available.
The backup battery is optional, connected to the VBACKUP pin, and can be
nonrechargeable or rechargeable. The rechargeable battery can be charged
from the system supply using the backup battery charger.
Add support for enabling charging of this backup battery. Also add the DT
binding document and the new properties to have this support.
Signed-off-by: Laxman Dewangan ldewan...@nvidia.com
---
.../devicetree/bindings/rtc/rtc-palmas.txt | 28 ++
drivers/rtc/rtc-palmas.c | 39
2 files changed, 67 insertions(+), 0 deletions(-)
create mode 100644 Documentation/devicetree/bindings/rtc/rtc-palmas.txt
diff --git a/Documentation/devicetree/bindings/rtc/rtc-palmas.txt
b/Documentation/devicetree/bindings/rtc/rtc-palmas.txt
new file mode 100644
index 000..e4b6910
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/rtc-palmas.txt
@@ -0,0 +1,28 @@
+Palmas RTC controller bindings
+
+Required properties:
+- compatible:
+ - ti,palams-rtc for palma series of the RTC controller
+- interrupt-parent: Parent interrupt device, must be handle of palams node.
+- interrupts: Interrupt number of RTC submodule on device.
+
+Optional properties:
+- ti,back-bat-chg-enable: The palmas series device like TPS65913 or
TPS80036
+ supports the battery backup for powering the RTC when main battery is
+ removed or in very low power state. This flag will enable the backup
+ battery charging.
Perhaps ti,has-rechargeable-battery would be better, as this describes
the hardware rather than an intent relating to it.
+- ti,back-bat-chg-current: Configure charging current. Device supports the
+ charging current as 100mA or 100mA.
Are either of these values unsafe or unsupported in some configurations?
Does the HW support just two options; less-than or greater-than 100mA?
If so, a Boolean property here might be better. The code below certainly
implies this.
Given there's only 1 battery, I think back- is redundant in the
property names. Since that shortens the names a bit, I'd suggest
spelling everything out in full, perhaps:
battery-charge-enable
battery-charge-low-current
Both Boolean.
+Example:
+ palmas: tps65913@58 {
+ :::
... is probably more common than lots of colons, or you could just
delete this line.
+ palmas_rtc: rtc {
+ compatible = ti,palmas-rtc;
+ interrupt-parent = palmas;
+ interrupts = 8 0;
+ ti,back-bat-chg-enable;
+ ti,back-bat-chg-current = 100;
+ };
+ :::
+ };
diff --git a/drivers/rtc/rtc-palmas.c b/drivers/rtc/rtc-palmas.c
@@ -238,6 +238,19 @@ static int palmas_rtc_probe(struct platform_device
*pdev)
+ ret = of_property_read_u32(pdev-dev.of_node,
+ ti,back-bat-chg-current, pval);
+ if (!ret)
+ bb_charging_current = pval;
Do you need to validate that pval is one of the legal values?
@@ -254,6 +267,32 @@ static int palmas_rtc_probe(struct platform_device
*pdev)
palmas_rtc-dev = pdev-dev;
platform_set_drvdata(pdev, palmas_rtc);
+ if (enable_bb_charging) {
+ unsigned reg = 0;
+
+ if (bb_charging_current 100)
+ reg |= PALMAS_BACKUP_BATTERY_CTRL_BBS_BBC_LOW_ICHRG;
This implies that a Boolean property would be a better representation of HW.
Otherwise, +1 to Stephen's comments.
Thanks,
Mark.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/rtc/rtc-palmas.c: support for backup battery charging
On Fri, Jul 26, 2013 at 09:40:15AM -0600, Stephen Warren wrote: Given there's only 1 battery, I think back- is redundant in the property names. Since that shortens the names a bit, I'd suggest spelling everything out in full, perhaps: Isn't there an integrated charger on the PMIC? If there is then it's useful to identify which is being talked about (even if it should be relatively obvious). signature.asc Description: Digital signature
Re: [PATCH] drivers/rtc/rtc-palmas.c: support for backup battery charging
On 07/26/2013 10:35 AM, Mark Brown wrote: On Fri, Jul 26, 2013 at 09:40:15AM -0600, Stephen Warren wrote: Given there's only 1 battery, I think back- is redundant in the property names. Since that shortens the names a bit, I'd suggest spelling everything out in full, perhaps: Isn't there an integrated charger on the PMIC? If there is then it's useful to identify which is being talked about (even if it should be relatively obvious). Oh yes, that's probably true. This is of course the binding for the RTC sub-module in the PMIC though. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/rtc/rtc-palmas.c: support for backup battery charging
On Friday 26 of July 2013 09:40:15 Stephen Warren wrote: (CC'ing the new DT binding maintainers and mailing list on this reply, hence quoting the whole of the DT binding) On 07/25/2013 06:29 AM, Laxman Dewangan wrote: Palmas series device like TPS65913, TPS80036 supports the backup battery for powering the RTC when no other energy source is available. The backup battery is optional, connected to the VBACKUP pin, and can be nonrechargeable or rechargeable. The rechargeable battery can be charged from the system supply using the backup battery charger. Add support for enabling charging of this backup battery. Also add the DT binding document and the new properties to have this support. Signed-off-by: Laxman Dewangan ldewan...@nvidia.com --- .../devicetree/bindings/rtc/rtc-palmas.txt | 28 ++ drivers/rtc/rtc-palmas.c | 39 2 files changed, 67 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/rtc/rtc-palmas.txt diff --git a/Documentation/devicetree/bindings/rtc/rtc-palmas.txt b/Documentation/devicetree/bindings/rtc/rtc-palmas.txt new file mode 100644 index 000..e4b6910 --- /dev/null +++ b/Documentation/devicetree/bindings/rtc/rtc-palmas.txt @@ -0,0 +1,28 @@ +Palmas RTC controller bindings + +Required properties: +- compatible: + - ti,palams-rtc for palma series of the RTC controller +- interrupt-parent: Parent interrupt device, must be handle of palams node. +- interrupts: Interrupt number of RTC submodule on device. + +Optional properties: +- ti,back-bat-chg-enable: The palmas series device like TPS65913 or TPS80036 + supports the battery backup for powering the RTC when main battery is +removed or in very low power state. This flag will enable the backup + battery charging. +- ti,back-bat-chg-current: Configure charging current. Device supports the + charging current as 100mA or 100mA. Does the HW support just two options; less-than or greater-than 100mA? If so, a Boolean property here might be better. The code below certainly implies this. Given there's only 1 battery, I think back- is redundant in the property names. Since that shortens the names a bit, I'd suggest spelling everything out in full, perhaps: battery-charge-enable battery-charge-low-current First of all, are those even properties for the RTC binding? Doesn't this PMIC contain a charger part which handles battery charging and so requires such kind of information? Best regards, Tomasz -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
