Re: [PATCH] drivers/rtc/rtc-palmas.c: rename charging dt property name

Re: [PATCH] drivers/rtc/rtc-palmas.c: rename charging dt property name

[Kumar Gala]

On Aug 9, 2013, at 10:47 AM, Laxman Dewangan wrote:

> Renaming the charging property names to have more meaningful:
> 
> Renaming property "ti,back-battery-charge-enable" to "ti,backup-battery-chargeable"
> to tells OS that attahced battery is chargeable and OS can do charging.
> 
> Renaming property "ti,back-battery-charge-low-current" to
> "ti,backup-battery-charge-high-current" for safe configuration as maximum charging current
> of battery depends on battery type and overcurrent charging can damage the battery. So
> enable the high current charging only if this property is available.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> ---
> The patch V4 version of the change
> [PATCH V4] drivers/rtc/rtc-palmas.c: support for backup battery charging
> is already applied but still the discussion is going on the property name.
> 
> To correct the property name, I am sending this patch on top of above patch
> as per discussion on above patch.
> 
> .../devicetree/bindings/rtc/rtc-palmas.txt         |   21 +++++++++++--------
> drivers/rtc/rtc-palmas.c                           |   14 ++++++------
> 2 files changed, 19 insertions(+), 16 deletions(-)

Are you not concerned about any dts with the old property names being around such that the code should support the old name as well?

- k

> 
> diff --git a/Documentation/devicetree/bindings/rtc/rtc-palmas.txt b/Documentation/devicetree/bindings/rtc/rtc-palmas.txt
> index 41bb445..8ef7660 100644
> --- a/Documentation/devicetree/bindings/rtc/rtc-palmas.txt
> +++ b/Documentation/devicetree/bindings/rtc/rtc-palmas.txt
> @@ -7,13 +7,16 @@ Required properties:
> - interrupts: Interrupt number of RTC submodule on device.
> 
> Optional properties:
> -- ti,back-battery-charge-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-battery-charge-low-current: Configure lower charging current. Device
> -	supports the charging current as < 100mA or >100mA. Low current will
> -	set as <100mA.
> +- ti,backup-battery-chargeable: The Palmas series device like TPS65913 or
> +	TPS80036 supports the backup battery for powering the RTC when main
> +	battery is removed or in very low power state. The backup battery
> +	can be chargeable or non-chargeable. This flag will tells whether
> +	battery is chargeable or not. If charging battery then driver can
> +	enable the charging.
> +- ti,backup-battery-charge-high-current: Enable high current charging in
> +	backup battery. Device supports the < 100mA and > 100mA charging.
> +	The high current will be > 100mA. Absence of this property will
> +	charge battery to lower current i.e. < 100mA.
> 
> Example:
> 	palmas: tps65913@58 {
> @@ -22,8 +25,8 @@ Example:
> 			compatible = "ti,palmas-rtc";
> 			interrupt-parent = <&palmas>;
> 			interrupts = <8 0>;
> -			ti,back-battery-charge-enable;
> -			ti,back-battery-charge-low-current;
> +			ti,backup-battery-chargeable;
> +			ti,backup-battery-charge-high-current;
> 		};
> 		...
> 	};
> diff --git a/drivers/rtc/rtc-palmas.c b/drivers/rtc/rtc-palmas.c
> index adb6e71..fffb7d3 100644
> --- a/drivers/rtc/rtc-palmas.c
> +++ b/drivers/rtc/rtc-palmas.c
> @@ -239,13 +239,13 @@ static int palmas_rtc_probe(struct platform_device *pdev)
> 	struct palmas_rtc *palmas_rtc = NULL;
> 	int ret;
> 	bool enable_bb_charging = false;
> -	bool low_bb_charging;
> +	bool high_bb_charging;
> 
> 	if (pdev->dev.of_node) {
> 		enable_bb_charging = of_property_read_bool(pdev->dev.of_node,
> -					"ti,back-battery-charge-enable");
> -		low_bb_charging = of_property_read_bool(pdev->dev.of_node,
> -					"ti,back-battery-charge-low-current");
> +					"ti,backup-battery-chargeable");
> +		high_bb_charging = of_property_read_bool(pdev->dev.of_node,
> +					"ti,backup-battery-charge-high-current");
> 	}
> 
> 	palmas_rtc = devm_kzalloc(&pdev->dev, sizeof(struct palmas_rtc),
> @@ -264,10 +264,10 @@ static int palmas_rtc_probe(struct platform_device *pdev)
> 	platform_set_drvdata(pdev, palmas_rtc);
> 
> 	if (enable_bb_charging) {
> -		unsigned reg = 0;
> +		unsigned reg = PALMAS_BACKUP_BATTERY_CTRL_BBS_BBC_LOW_ICHRG;
> 
> -		if (low_bb_charging)
> -			reg |= PALMAS_BACKUP_BATTERY_CTRL_BBS_BBC_LOW_ICHRG;
> +		if (high_bb_charging)
> +			reg = 0;
> 
> 		ret = palmas_update_bits(palmas, PALMAS_PMU_CONTROL_BASE,
> 			PALMAS_BACKUP_BATTERY_CTRL,
> -- 
> 1.7.1.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH] drivers/rtc/rtc-palmas.c: rename charging dt property name

[Kumar Gala]

On Aug 9, 2013, at 10:47 AM, Laxman Dewangan wrote:

> Renaming the charging property names to have more meaningful:
> 
> Renaming property "ti,back-battery-charge-enable" to "ti,backup-battery-chargeable"
> to tells OS that attahced battery is chargeable and OS can do charging.
> 
> Renaming property "ti,back-battery-charge-low-current" to
> "ti,backup-battery-charge-high-current" for safe configuration as maximum charging current
> of battery depends on battery type and overcurrent charging can damage the battery. So
> enable the high current charging only if this property is available.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> ---
> The patch V4 version of the change
> [PATCH V4] drivers/rtc/rtc-palmas.c: support for backup battery charging
> is already applied but still the discussion is going on the property name.
> 
> To correct the property name, I am sending this patch on top of above patch
> as per discussion on above patch.
> 
> .../devicetree/bindings/rtc/rtc-palmas.txt         |   21 +++++++++++--------
> drivers/rtc/rtc-palmas.c                           |   14 ++++++------
> 2 files changed, 19 insertions(+), 16 deletions(-)

Acked-by: Kumar Gala <galak@codeaurora.org>

- k

--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

[PATCH] drivers/rtc/rtc-palmas.c: rename charging dt property name

[Laxman Dewangan]

Renaming the charging property names to have more meaningful:

Renaming property "ti,back-battery-charge-enable" to "ti,backup-battery-chargeable"
to tells OS that attahced battery is chargeable and OS can do charging.

Renaming property "ti,back-battery-charge-low-current" to
"ti,backup-battery-charge-high-current" for safe configuration as maximum charging current
of battery depends on battery type and overcurrent charging can damage the battery. So
enable the high current charging only if this property is available.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
The patch V4 version of the change
[PATCH V4] drivers/rtc/rtc-palmas.c: support for backup battery charging
is already applied but still the discussion is going on the property name.

To correct the property name, I am sending this patch on top of above patch
as per discussion on above patch.

 .../devicetree/bindings/rtc/rtc-palmas.txt         |   21 +++++++++++--------
 drivers/rtc/rtc-palmas.c                           |   14 ++++++------
 2 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/Documentation/devicetree/bindings/rtc/rtc-palmas.txt b/Documentation/devicetree/bindings/rtc/rtc-palmas.txt
index 41bb445..8ef7660 100644
--- a/Documentation/devicetree/bindings/rtc/rtc-palmas.txt
+++ b/Documentation/devicetree/bindings/rtc/rtc-palmas.txt
@@ -7,13 +7,16 @@ Required properties:
 - interrupts: Interrupt number of RTC submodule on device.
 
 Optional properties:
-- ti,back-battery-charge-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-battery-charge-low-current: Configure lower charging current. Device
-	supports the charging current as < 100mA or >100mA. Low current will
-	set as <100mA.
+- ti,backup-battery-chargeable: The Palmas series device like TPS65913 or
+	TPS80036 supports the backup battery for powering the RTC when main
+	battery is removed or in very low power state. The backup battery
+	can be chargeable or non-chargeable. This flag will tells whether
+	battery is chargeable or not. If charging battery then driver can
+	enable the charging.
+- ti,backup-battery-charge-high-current: Enable high current charging in
+	backup battery. Device supports the < 100mA and > 100mA charging.
+	The high current will be > 100mA. Absence of this property will
+	charge battery to lower current i.e. < 100mA.
 
 Example:
 	palmas: tps65913@58 {
@@ -22,8 +25,8 @@ Example:
 			compatible = "ti,palmas-rtc";
 			interrupt-parent = <&palmas>;
 			interrupts = <8 0>;
-			ti,back-battery-charge-enable;
-			ti,back-battery-charge-low-current;
+			ti,backup-battery-chargeable;
+			ti,backup-battery-charge-high-current;
 		};
 		...
 	};
diff --git a/drivers/rtc/rtc-palmas.c b/drivers/rtc/rtc-palmas.c
index adb6e71..fffb7d3 100644
--- a/drivers/rtc/rtc-palmas.c
+++ b/drivers/rtc/rtc-palmas.c
@@ -239,13 +239,13 @@ static int palmas_rtc_probe(struct platform_device *pdev)
 	struct palmas_rtc *palmas_rtc = NULL;
 	int ret;
 	bool enable_bb_charging = false;
-	bool low_bb_charging;
+	bool high_bb_charging;
 
 	if (pdev->dev.of_node) {
 		enable_bb_charging = of_property_read_bool(pdev->dev.of_node,
-					"ti,back-battery-charge-enable");
-		low_bb_charging = of_property_read_bool(pdev->dev.of_node,
-					"ti,back-battery-charge-low-current");
+					"ti,backup-battery-chargeable");
+		high_bb_charging = of_property_read_bool(pdev->dev.of_node,
+					"ti,backup-battery-charge-high-current");
 	}
 
 	palmas_rtc = devm_kzalloc(&pdev->dev, sizeof(struct palmas_rtc),
@@ -264,10 +264,10 @@ static int palmas_rtc_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, palmas_rtc);
 
 	if (enable_bb_charging) {
-		unsigned reg = 0;
+		unsigned reg = PALMAS_BACKUP_BATTERY_CTRL_BBS_BBC_LOW_ICHRG;
 
-		if (low_bb_charging)
-			reg |= PALMAS_BACKUP_BATTERY_CTRL_BBS_BBC_LOW_ICHRG;
+		if (high_bb_charging)
+			reg = 0;
 
 		ret = palmas_update_bits(palmas, PALMAS_PMU_CONTROL_BASE,
 			PALMAS_BACKUP_BATTERY_CTRL,
-- 
1.7.1.1

Re: [PATCH] drivers/rtc/rtc-palmas.c: rename charging dt property name

[Laxman Dewangan]

On Friday 09 August 2013 09:07 PM, Kumar Gala wrote:
> On Aug 9, 2013, at 10:47 AM, Laxman Dewangan wrote:
>
>> Renaming the charging property names to have more meaningful:
>>
>> Renaming property "ti,back-battery-charge-enable" to "ti,backup-battery-chargeable"
>> to tells OS that attahced battery is chargeable and OS can do charging.
>>
>> Renaming property "ti,back-battery-charge-low-current" to
>> "ti,backup-battery-charge-high-current" for safe configuration as maximum charging current
>> of battery depends on battery type and overcurrent charging can damage the battery. So
>> enable the high current charging only if this property is available.
>>
>> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
>> ---
>> The patch V4 version of the change
>> [PATCH V4] drivers/rtc/rtc-palmas.c: support for backup battery charging
>> is already applied but still the discussion is going on the property name.
>>
>> To correct the property name, I am sending this patch on top of above patch
>> as per discussion on above patch.
>>
>> .../devicetree/bindings/rtc/rtc-palmas.txt         |   21 +++++++++++--------
>> drivers/rtc/rtc-palmas.c                           |   14 ++++++------
>> 2 files changed, 19 insertions(+), 16 deletions(-)
> Are you not concerned about any dts with the old property names being around such that the code should support the old name as well?
>

I introduced these properties on above patch and it applied yesterday 
only. Hence no one is using now. So no issue here.