poweroff-source DT property renaming

From: Romain Perier <[email protected]>

It reverts commit a4b4e0461ec5 ("of: Add standard property for poweroff capability"). As discussed on the mailing list, it makes more sense to rename back to the old established property name, without the vendor prefix. Problem being that the word "source" usually tends to be used for inputs and that is out of control of the OS. The poweroff capability is an output which simply turns the system-power off. Also, this property might be used by drivers which power-off the system and power back on subsequent RTC alarms. This seems to suggest to remove "poweroff" from the property name and to choose "system-power-controller" as the more generic name. This patchs adds the required renaming changes and defines an helper function which is compatible with both properties, the old one which was only used by tps65910 and the new one without vendor-prefix.

Signed-off-by Romain Perier <[email protected]>

---
 .../bindings/power/{poweroff.txt => power-controller.txt}        | 0
 .../devicetree/bindings/regulator/act8865-regulator.txt          | 4 ++--
 drivers/mfd/tps65910.c                                           | 9 ++++++++-
 drivers/regulator/act8865-regulator.c                            | 2 +-
 include/linux/of.h                                               | 6 +++---
 5 files changed, 14 insertions(+), 7 deletions(-)
 rename Documentation/devicetree/bindings/power/{poweroff.txt => power-controller.txt} (100%)

Documentation/devicetree/bindings/power/power-controller.txt renamed from Documentation/devicetree/bindings/power/poweroff.txt +0 -0

similarity index 100%
rename from Documentation/devicetree/bindings/power/poweroff.txt
rename to Documentation/devicetree/bindings/power/power-controller.txt

Documentation/devicetree/bindings/regulator/act8865-regulator.txt +2 -2

index 01a5b07..dad6358 100644
@@ -6,8 +6,8 @@ Required properties:
 - reg: I2C slave address
 
 Optional properties:
-- poweroff-source: Telling whether or not this pmic is controlling
-  the system power. See Documentation/devicetree/bindings/power/poweroff.txt .
+- system-power-controller: Telling whether or not this pmic is controlling
+  the system power. See Documentation/devicetree/bindings/power/power-controller.txt .
 
 Any standard regulator properties can be used to configure the single regulator.
 

drivers/mfd/tps65910.c +8 -1

index b8dca8a..3a796a5 100644
@@ -386,6 +386,13 @@ static const struct of_device_id tps65910_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, tps65910_of_match);
 
+static inline bool tps65910_is_pm_off_found(struct device_node *np)
+{
+	if (of_property_read_bool(np, "ti,system-power-controller"))
+		return true;
+	return of_is_system_power_controller(np);
+}
+
 static struct tps65910_board *tps65910_parse_dt(struct i2c_client *client,
 						unsigned long *chip_id)
 {

@@ -423,7 +430,7 @@ static struct tps65910_board *tps65910_parse_dt(struct i2c_client *client,
 
 	board_info->irq = client->irq;
 	board_info->irq_base = -1;
-	board_info->pm_off = of_system_has_poweroff_source(np);
+	board_info->pm_off = tps65910_is_pm_off_found(np);
 
 	return board_info;
 }

drivers/regulator/act8865-regulator.c +1 -1

index 76301ed..435aba1 100644
@@ -365,7 +365,7 @@ static int act8865_pmic_probe(struct i2c_client *client,
 		return ret;
 	}
 
-	if (of_system_has_poweroff_source(dev->of_node)) {
+	if (of_is_system_power_controller(dev->of_node)) {
 		if (!pm_power_off) {
 			act8865_i2c_client = client;
 			act8865->off_reg = off_reg;

include/linux/of.h +3 -3

index d933717..f9541bc 100644
@@ -965,14 +965,14 @@ static inline int of_changeset_update_property(struct of_changeset *ocs,
 extern int of_resolve_phandles(struct device_node *tree);
 
 /**
- * of_system_has_poweroff_source - Tells if poweroff-source is found for device_node
+ * of_is_system_power_controller - Tells if system-power-controller is found for device_node
  * @np: Pointer to the given device_node
  *
  * return true if present false otherwise
  */
-static inline bool of_system_has_poweroff_source(const struct device_node *np)
+static inline bool of_is_system_power_controller(const struct device_node *np)
 {
-	return of_property_read_bool(np, "poweroff-source");
+	return of_property_read_bool(np, "system-power-controller");
 }
 
 #endif /* _LINUX_OF_H */

Signature · 1 line

1.9.1

Acked-by Grant Likely <[email protected]>

Please merge via whichever tree needs this change.

g.

Hi,

Who should merge this serie ? as Mark merged the previous one it would probably make sense to do the same here (at least, in my opinion)

Thanks for your feedbacks.

Have a nice day,
Romain

2014-11-13 21:55 GMT+01:00 Grant Likely <[email protected]>:

igned-off-by Romain Perier <[email protected]>

Quoted 6 lines

Acked-by Grant Likely <[email protected]>

Please merge via whichever tree needs this change.

g.

@Most of the people who were ranting about this serie: could you send feedbacks please ?

2014-11-14 8:22 GMT+01:00 Romain Perier <[email protected]>:

Quoted 18 lines

Hi,

Who should merge this serie ? as Mark merged the previous one it would probably make sense to do the same here (at least, in my opinion)

Thanks for your feedbacks.

Have a nice day,
Romain

2014-11-13 21:55 GMT+01:00 Grant Likely <[email protected]>:

igned-off-by Romain Perier <[email protected]>

Quoted 6 lines

Acked-by Grant Likely <[email protected]>

Please merge via whichever tree needs this change.

g.

From: Romain Perier <[email protected]>

It reverts commit a4b4e0461ec5 ("of: Add standard property for poweroff capability"). As discussed on the mailing list, it makes more sense to rename back to the old established property name, without the vendor prefix. Problem being that the word "source" usually tends to be used for inputs and that is out of control of the OS. The poweroff capability is an output which simply turns the system-power off. Also, this property might be used by drivers which power-off the system and power back on subsequent RTC alarms. This seems to suggest to remove "poweroff" from the property name and to choose "system-power-controller" as the more generic name. This patchs adds the required renaming changes and defines an helper function which is compatible with both properties, the old one which was only used by tps65910 and the new one without vendor-prefix.

Signed-off-by Romain Perier <[email protected]>

---
 .../bindings/power/{poweroff.txt => power-controller.txt}        | 0
 .../devicetree/bindings/regulator/act8865-regulator.txt          | 4 ++--
 drivers/mfd/tps65910.c                                           | 9 ++++++++-
 drivers/regulator/act8865-regulator.c                            | 2 +-
 include/linux/of.h                                               | 6 +++---
 5 files changed, 14 insertions(+), 7 deletions(-)

Quoted 13 lines

From: Romain Perier <[email protected]>

It reverts commit a4b4e0461ec5 ("of: Add standard property for poweroff capability"). As discussed on the mailing list, it makes more sense to rename back to the old established property name, without the vendor prefix. Problem being that the word "source" usually tends to be used for inputs and that is out of control of the OS. The poweroff capability is an output which simply turns the system-power off. Also, this property might be used by drivers which power-off the system and power back on subsequent RTC alarms. This seems to suggest to remove "poweroff" from the property name and to choose "system-power-controller" as the more generic name. This patchs adds the required renaming changes and defines an helper function which is compatible with both properties, the old one which was only used by tps65910 and the new one without vendor-prefix.

Now this is a bit of a mess.

There's a commit in the mfd tree, 25f833c1171d ("mfd: tps65910: Convert ti,system-power-controller DT property to poweroff-source"), which breaks all dts using tps65910 since these are never updated to the now retracted property name ("poweroff-source").

This one should simply be reverted ASAP.

Quoted 8 lines

Signed-off-by Romain Perier <[email protected]>

---
 .../bindings/power/{poweroff.txt => power-controller.txt}        | 0
 .../devicetree/bindings/regulator/act8865-regulator.txt          | 4 ++--
 drivers/mfd/tps65910.c                                           | 9 ++++++++-
 drivers/regulator/act8865-regulator.c                            | 2 +-
 include/linux/of.h                                               | 6 +++---
 5 files changed, 14 insertions(+), 7 deletions(-)

Romain, what tree is this patch against? The changes to the tps65910 driver appears not to even be in the regulator tree, yet you asked Mark to merge this. And the MFD maintainer is not even on CC.

Let's fix the breakage by reverting the offending commit in mfd. Then the new standard name can be introduced in regulator alone (e.g. this patch without the tps65910 bits) as nothing outside of regulator should be using the new power-off feature (or binding) for act8865. Then other drivers and dts can be converted to use the new property name (while retaining backwards compatibility) for 3.20.

[ We should probably also consider adding an "of_device_is_" prefix to the helper name for consistency. ]

Johan

it was merged by Lee Jones (so it was merged onto
the mfd tree)

Quoted 13 lines · Auto Configured

From: Romain Perier <[email protected]>

It reverts commit a4b4e0461ec5 ("of: Add standard property for poweroff capability"). As discussed on the mailing list, it makes more sense to rename back to the old established property name, without the vendor prefix. Problem being that the word "source" usually tends to be used for inputs and that is out of control of the OS. The poweroff capability is an output which simply turns the system-power off. Also, this property might be used by drivers which power-off the system and power back on subsequent RTC alarms. This seems to suggest to remove "poweroff" from the property name and to choose "system-power-controller" as the more generic name. This patchs adds the required renaming changes and defines an helper function which is compatible with both properties, the old one which was only used by tps65910 and the new one without vendor-prefix.

Now this is a bit of a mess.

There's a commit in the mfd tree, 25f833c1171d ("mfd: tps65910: Convert ti,system-power-controller DT property to poweroff-source"), which breaks all dts using tps65910 since these are never updated to the now retracted property name ("poweroff-source").

This one should simply be reverted ASAP.

Quoted 8 lines

Signed-off-by Romain Perier <[email protected]>

---
 .../bindings/power/{poweroff.txt => power-controller.txt}        | 0
 .../devicetree/bindings/regulator/act8865-regulator.txt          | 4 ++--
 drivers/mfd/tps65910.c                                           | 9 ++++++++-
 drivers/regulator/act8865-regulator.c                            | 2 +-
 include/linux/of.h                                               | 6 +++---
 5 files changed, 14 insertions(+), 7 deletions(-)

Romain, what tree is this patch against? The changes to the tps65910 driver appears not to even be in the regulator tree, yet you asked Mark to merge this. And the MFD maintainer is not even on CC.

Let's fix the breakage by reverting the offending commit in mfd. Then the new standard name can be introduced in regulator alone (e.g. this patch without the tps65910 bits) as nothing outside of regulator should be using the new power-off feature (or binding) for act8865. Then other drivers and dts can be converted to use the new property name (while retaining backwards compatibility) for 3.20.

[ We should probably also consider adding an "of_device_is_" prefix to the helper name for consistency. ]

Johan

Quoted 17 lines · Johan Hovold

Quoted 13 lines · Auto Configured

From: Romain Perier <[email protected]>

It reverts commit a4b4e0461ec5 ("of: Add standard property for poweroff capability"). As discussed on the mailing list, it makes more sense to rename back to the old established property name, without the vendor prefix. Problem being that the word "source" usually tends to be used for inputs and that is out of control of the OS. The poweroff capability is an output which simply turns the system-power off. Also, this property might be used by drivers which power-off the system and power back on subsequent RTC alarms. This seems to suggest to remove "poweroff" from the property name and to choose "system-power-controller" as the more generic name. This patchs adds the required renaming changes and defines an helper function which is compatible with both properties, the old one which was only used by tps65910 and the new one without vendor-prefix.

Now this is a bit of a mess.

Quoted 4 lines

There's a commit in the mfd tree, 25f833c1171d ("mfd: tps65910: Convert ti,system-power-controller DT property to poweroff-source"), which breaks all dts using tps65910 since these are never updated to the now retracted property name ("poweroff-source").

My word!

Romain, what conversation on the MLs are you talking about?

Quoted 1 line

This one should simply be reverted ASAP.

No need to revert, I can just remove the patch from the MFD tree.

Quoted 20 lines

Quoted 8 lines

Signed-off-by Romain Perier <[email protected]>

---
 .../bindings/power/{poweroff.txt => power-controller.txt}        | 0
 .../devicetree/bindings/regulator/act8865-regulator.txt          | 4 ++--
 drivers/mfd/tps65910.c                                           | 9 ++++++++-
 drivers/regulator/act8865-regulator.c                            | 2 +-
 include/linux/of.h                                               | 6 +++---
 5 files changed, 14 insertions(+), 7 deletions(-)

Romain, what tree is this patch against? The changes to the tps65910 driver appears not to even be in the regulator tree, yet you asked Mark to merge this. And the MFD maintainer is not even on CC.

Let's fix the breakage by reverting the offending commit in mfd. Then the new standard name can be introduced in regulator alone (e.g. this patch without the tps65910 bits) as nothing outside of regulator should be using the new power-off feature (or binding) for act8865. Then other drivers and dts can be converted to use the new property name (while retaining backwards compatibility) for 3.20.

Quoted 2 lines

[ We should probably also consider adding an "of_device_is_" prefix to the helper name for consistency. ]

Quoted 23 lines · Johan Hovold

Quoted 21 lines · Auto Configured

Quoted 13 lines

From: Romain Perier <[email protected]>

It reverts commit a4b4e0461ec5 ("of: Add standard property for poweroff capability"). As discussed on the mailing list, it makes more sense to rename back to the old established property name, without the vendor prefix. Problem being that the word "source" usually tends to be used for inputs and that is out of control of the OS. The poweroff capability is an output which simply turns the system-power off. Also, this property might be used by drivers which power-off the system and power back on subsequent RTC alarms. This seems to suggest to remove "poweroff" from the property name and to choose "system-power-controller" as the more generic name. This patchs adds the required renaming changes and defines an helper function which is compatible with both properties, the old one which was only used by tps65910 and the new one without vendor-prefix.

Now this is a bit of a mess.

There's a commit in the mfd tree, 25f833c1171d ("mfd: tps65910: Convert ti,system-power-controller DT property to poweroff-source"), which breaks all dts using tps65910 since these are never updated to the now retracted property name ("poweroff-source").

Quoted 3 lines

My word!

Romain, what conversation on the MLs are you talking about?

I think Romain is referring to this thread:

        https://lkml.org/lkml/2014/10/23/161

Quoted 4 lines

Quoted 2 lines

This one should simply be reverted ASAP.

No need to revert, I can just remove the patch from the MFD tree.

Ok, good. Then this is limited to the regulator tree, and we could proceed as I outlined below.

Romain, care to resend this patch without the tps65910 chunks?

You should also fix the commit message, which claims to define a "helper function which is compatible with both properties", something which was no longer the case.

Thanks,
Johan

2014-11-24 12:35 GMT+01:00 Johan Hovold <[email protected]>:

Quoted 24 lines · Lee Jones

Quoted 23 lines · Johan Hovold

Quoted 21 lines · Auto Configured

Quoted 13 lines

From: Romain Perier <[email protected]>

It reverts commit a4b4e0461ec5 ("of: Add standard property for poweroff capability"). As discussed on the mailing list, it makes more sense to rename back to the old established property name, without the vendor prefix. Problem being that the word "source" usually tends to be used for inputs and that is out of control of the OS. The poweroff capability is an output which simply turns the system-power off. Also, this property might be used by drivers which power-off the system and power back on subsequent RTC alarms. This seems to suggest to remove "poweroff" from the property name and to choose "system-power-controller" as the more generic name. This patchs adds the required renaming changes and defines an helper function which is compatible with both properties, the old one which was only used by tps65910 and the new one without vendor-prefix.

Now this is a bit of a mess.

There's a commit in the mfd tree, 25f833c1171d ("mfd: tps65910: Convert ti,system-power-controller DT property to poweroff-source"), which breaks all dts using tps65910 since these are never updated to the now retracted property name ("poweroff-source").

Quoted 7 lines

Quoted 3 lines

My word!

Romain, what conversation on the MLs are you talking about?

I think Romain is referring to this thread:

        https://lkml.org/lkml/2014/10/23/161

Yes, this is this one.

Quoted 10 lines

Quoted 4 lines

Quoted 2 lines

This one should simply be reverted ASAP.

No need to revert, I can just remove the patch from the MFD tree.

Ok, good. Then this is limited to the regulator tree, and we could proceed as I outlined below.

Romain, care to resend this patch without the tps65910 chunks?

If you can just drop the patch from mfd tree Lee, please do. Yes Johan, np, what I want is that we find a smart solution, nothing more.

Quoted 5 lines

You should also fix the commit message, which claims to define a "helper function which is compatible with both properties", something which was no longer the case.

Quoted 2 lines

Thanks,
Johan

So I need to resend this patch which would only introduce an helper function which checks for "system-power-controller" property (it would also have a new commit message).
As you suggested, this helper might be named "of_device_is_system_power_controller" ?

2014-11-24 13:58 GMT+01:00 Romain Perier <[email protected]>:

Quoted 5 lines

So I need to resend this patch which would only introduce an helper function which checks for "system-power-controller" property (it would also have a new commit message).
As you suggested, this helper might be named "of_device_is_system_power_controller" ?

I mean, the backward compatibility can be handled directly from the device driver which would use the new property, no ?

2014-11-24 12:35 GMT+01:00 Johan Hovold <[email protected]>:

Quoted 4 lines

You should also fix the commit message, which claims to define a "helper function which is compatible with both properties", something which was no longer the case.

So I need to resend this patch which would only introduce an helper function which checks for "system-power-controller" property (it would also have a new commit message).
As you suggested, this helper might be named "of_device_is_system_power_controller" ?