[PATCH 0/3] mfd: twl4030-power: Start DT conversion

[PATCH 0/3] mfd: twl4030-power: Start DT conversion

[Florian Vaussard]

Hello,

This series enables a partial DT support for twl4030-power. The
missing part is the power management scripts, as the required
binding should be defined first. It however enables the complete
shutdown of the processor at poweroff when booting with DT,
dropping the power consumption from around 350 mA on Overo+Tobi
to about 40 mA.

The poweroff callback was tested with both DT and non-DT boots.

Best regards,

Florian

Florian Vaussard (3):
  mfd: twl4030-power: Split from twl-core into a dedicated module
  mfd: twl4030-power: Start transition to DT
  mfd: twl4030-power: Simplify error path

 .../devicetree/bindings/mfd/twl4030-power.txt      |   28 ++++
 drivers/mfd/twl-core.c                             |   12 +-
 drivers/mfd/twl4030-power.c                        |  149 +++++++++++++++-----
 include/linux/i2c/twl.h                            |    1 -
 4 files changed, 148 insertions(+), 42 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/twl4030-power.txt

-- 
1.7.5.4

[PATCH 1/3] mfd: twl4030-power: Split from twl-core into a dedicated module

[Florian Vaussard]

For now, the call to twl4030-power is hard-wired inside twl-core.
To ease the future transition to DT, make twl4030-power as a
separate module, like what is already done for twl4030-audio
and others.

Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
---
 drivers/mfd/twl-core.c      |   12 ++++++---
 drivers/mfd/twl4030-power.c |   54 +++++++++++++++++++++++++++++++++++--------
 include/linux/i2c/twl.h     |    1 -
 3 files changed, 52 insertions(+), 15 deletions(-)

diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index 89ab4d9..d1b9d14 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -1023,6 +1023,14 @@ add_children(struct twl4030_platform_data *pdata, unsigned irq_base,
 			return PTR_ERR(child);
 	}
 
+	if (IS_ENABLED(CONFIG_TWL4030_POWER) && pdata->power) {
+		child = add_child(TWL_MODULE_PM_MASTER, "twl4030_power",
+				  pdata->power, sizeof(*pdata->power), false,
+				  0, 0);
+		if (IS_ERR(child))
+			return PTR_ERR(child);
+	}
+
 	return 0;
 }
 
@@ -1234,10 +1242,6 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		WARN(status < 0, "Error: reading twl_idcode register value\n");
 	}
 
-	/* load power event scripts */
-	if (IS_ENABLED(CONFIG_TWL4030_POWER) && pdata && pdata->power)
-		twl4030_power_init(pdata->power);
-
 	/* Maybe init the T2 Interrupt subsystem */
 	if (client->irq) {
 		if (twl_class_is_4030()) {
diff --git a/drivers/mfd/twl4030-power.c b/drivers/mfd/twl4030-power.c
index dd362c1..c9a2a5c 100644
--- a/drivers/mfd/twl4030-power.c
+++ b/drivers/mfd/twl4030-power.c
@@ -507,8 +507,9 @@ void twl4030_power_off(void)
 		pr_err("TWL4030 Unable to power off\n");
 }
 
-void twl4030_power_init(struct twl4030_power_data *twl4030_scripts)
+int twl4030_power_probe(struct platform_device *pdev)
 {
+	struct twl4030_power_data *pdata = pdev->dev.platform_data;
 	int err = 0;
 	int i;
 	struct twl4030_resconfig *resconfig;
@@ -524,14 +525,14 @@ void twl4030_power_init(struct twl4030_power_data *twl4030_scripts)
 	if (err)
 		goto unlock;
 
-	for (i = 0; i < twl4030_scripts->num; i++) {
-		err = load_twl4030_script(twl4030_scripts->scripts[i], address);
+	for (i = 0; i < pdata->num; i++) {
+		err = load_twl4030_script(pdata->scripts[i], address);
 		if (err)
 			goto load;
-		address += twl4030_scripts->scripts[i]->size;
+		address += pdata->scripts[i]->size;
 	}
 
-	resconfig = twl4030_scripts->resource_config;
+	resconfig = pdata->resource_config;
 	if (resconfig) {
 		while (resconfig->resource) {
 			err = twl4030_configure_resource(resconfig);
@@ -543,7 +544,7 @@ void twl4030_power_init(struct twl4030_power_data *twl4030_scripts)
 	}
 
 	/* Board has to be wired properly to use this feature */
-	if (twl4030_scripts->use_poweroff && !pm_power_off) {
+	if (pdata->use_poweroff && !pm_power_off) {
 		/* Default for SEQ_OFFSYNC is set, lets ensure this */
 		err = twl_i2c_read_u8(TWL_MODULE_PM_MASTER, &val,
 				      TWL4030_PM_MASTER_CFG_P123_TRANSITION);
@@ -568,18 +569,51 @@ relock:
 			       TWL4030_PM_MASTER_PROTECT_KEY);
 	if (err)
 		pr_err("TWL4030 Unable to relock registers\n");
-	return;
+	return err;
 
 unlock:
 	if (err)
 		pr_err("TWL4030 Unable to unlock registers\n");
-	return;
+	return err;
 load:
 	if (err)
 		pr_err("TWL4030 failed to load scripts\n");
-	return;
+	return err;
 resource:
 	if (err)
 		pr_err("TWL4030 failed to configure resource\n");
-	return;
+	return err;
+}
+
+static int twl4030_power_remove(struct platform_device *pdev)
+{
+	return 0;
+}
+
+static struct platform_driver twl4030_power_driver = {
+	.driver = {
+		.name	= "twl4030_power",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= twl4030_power_probe,
+	.remove		= twl4030_power_remove,
+};
+
+static int __init twl4030_power_init(void)
+{
+	return platform_driver_register(&twl4030_power_driver);
 }
+subsys_initcall(twl4030_power_init);
+
+static void __exit twl4030_power_exit(void)
+{
+	platform_driver_unregister(&twl4030_power_driver);
+}
+module_exit(twl4030_power_exit);
+
+MODULE_AUTHOR("Nokia Corporation");
+MODULE_AUTHOR("Texas Instruments, Inc.");
+MODULE_DESCRIPTION("Power management for TWL4030");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:twl4030_power");
+
diff --git a/include/linux/i2c/twl.h b/include/linux/i2c/twl.h
index 488debb..2167c0d0 100644
--- a/include/linux/i2c/twl.h
+++ b/include/linux/i2c/twl.h
@@ -658,7 +658,6 @@ struct twl4030_power_data {
 	bool use_poweroff;	/* Board is wired for TWL poweroff */
 };
 
-extern void twl4030_power_init(struct twl4030_power_data *triton2_scripts);
 extern int twl4030_remove_script(u8 flags);
 extern void twl4030_power_off(void);
 
-- 
1.7.5.4

[PATCH 2/3] mfd: twl4030-power: Start transition to DT

[Florian Vaussard]

Support for loading twl4030-power module via devicetree.
For now, when booting with a DT, only the poweroff callback
feature is supported through the ti,use_poweroff property.

Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
---
 .../devicetree/bindings/mfd/twl4030-power.txt      |   28 +++++++
 drivers/mfd/twl4030-power.c                        |   86 +++++++++++++++----
 2 files changed, 96 insertions(+), 18 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/twl4030-power.txt

diff --git a/Documentation/devicetree/bindings/mfd/twl4030-power.txt b/Documentation/devicetree/bindings/mfd/twl4030-power.txt
new file mode 100644
index 0000000..8e15ec3
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/twl4030-power.txt
@@ -0,0 +1,28 @@
+Texas Instruments TWL family (twl4030) reset and power management module
+
+The power management module inside the TWL family provides several facilities
+to control the power resources, including power scripts. For now, the
+binding only supports the complete shutdown of the system after poweroff.
+
+Required properties:
+- compatible : must be "ti,twl4030-power"
+
+Optional properties:
+- ti,use_poweroff: With this flag, the chip will initiates an ACTIVE-to-OFF or
+		   SLEEP-to-OFF transition when the system poweroffs.
+
+Example:
+&i2c1 {
+	clock-frequency = <2600000>;
+
+	twl: twl@48 {
+		reg = <0x48>;
+		interrupts = <7>; /* SYS_NIRQ cascaded to intc */
+		interrupt-parent = <&intc>;
+
+		twl_power: power {
+			compatible = "ti,twl4030-power";
+			ti,use_poweroff;
+		};
+	};
+};
diff --git a/drivers/mfd/twl4030-power.c b/drivers/mfd/twl4030-power.c
index c9a2a5c..d12d748 100644
--- a/drivers/mfd/twl4030-power.c
+++ b/drivers/mfd/twl4030-power.c
@@ -28,6 +28,7 @@
 #include <linux/pm.h>
 #include <linux/i2c/twl.h>
 #include <linux/platform_device.h>
+#include <linux/of.h>
 
 #include <asm/mach-types.h>
 
@@ -492,6 +493,39 @@ int twl4030_remove_script(u8 flags)
 	return err;
 }
 
+int twl4030_power_configure_scripts(struct twl4030_power_data *pdata)
+{
+	int err;
+	int i;
+	u8 address = twl4030_start_script_address;
+
+	for (i = 0; i < pdata->num; i++) {
+		err = load_twl4030_script(pdata->scripts[i], address);
+		if (err)
+			return err;
+		address += pdata->scripts[i]->size;
+	}
+
+	return 0;
+}
+
+int twl4030_power_configure_resources(struct twl4030_power_data *pdata)
+{
+	struct twl4030_resconfig *resconfig = pdata->resource_config;
+	int err;
+
+	if (resconfig) {
+		while (resconfig->resource) {
+			err = twl4030_configure_resource(resconfig);
+			if (err)
+				return err;
+			resconfig++;
+		}
+	}
+
+	return 0;
+}
+
 /*
  * In master mode, start the power off sequence.
  * After a successful execution, TWL shuts down the power to the SoC
@@ -507,13 +541,29 @@ void twl4030_power_off(void)
 		pr_err("TWL4030 Unable to power off\n");
 }
 
+static bool twl4030_power_use_poweroff(struct twl4030_power_data *pdata,
+					struct device_node *node)
+{
+	if (pdata && pdata->use_poweroff)
+		return true;
+
+	if (of_property_read_bool(node, "ti,use_poweroff"))
+		return true;
+
+	return false;
+}
+
 int twl4030_power_probe(struct platform_device *pdev)
 {
 	struct twl4030_power_data *pdata = pdev->dev.platform_data;
+	struct device_node *node = pdev->dev.of_node;
 	int err = 0;
-	int i;
-	struct twl4030_resconfig *resconfig;
-	u8 val, address = twl4030_start_script_address;
+	u8 val;
+
+	if (!pdata && !node) {
+		dev_err(&pdev->dev, "Platform data is missing\n");
+		return -EINVAL;
+	}
 
 	err = twl_i2c_write_u8(TWL_MODULE_PM_MASTER, TWL4030_PM_MASTER_KEY_CFG1,
 			       TWL4030_PM_MASTER_PROTECT_KEY);
@@ -525,26 +575,17 @@ int twl4030_power_probe(struct platform_device *pdev)
 	if (err)
 		goto unlock;
 
-	for (i = 0; i < pdata->num; i++) {
-		err = load_twl4030_script(pdata->scripts[i], address);
+	if (pdata) {
+		err = twl4030_power_configure_scripts(pdata);
 		if (err)
 			goto load;
-		address += pdata->scripts[i]->size;
-	}
-
-	resconfig = pdata->resource_config;
-	if (resconfig) {
-		while (resconfig->resource) {
-			err = twl4030_configure_resource(resconfig);
-			if (err)
-				goto resource;
-			resconfig++;
-
-		}
+		err = twl4030_power_configure_resources(pdata);
+		if (err)
+			goto resource;
 	}
 
 	/* Board has to be wired properly to use this feature */
-	if (pdata->use_poweroff && !pm_power_off) {
+	if (twl4030_power_use_poweroff(pdata, node) && !pm_power_off) {
 		/* Default for SEQ_OFFSYNC is set, lets ensure this */
 		err = twl_i2c_read_u8(TWL_MODULE_PM_MASTER, &val,
 				      TWL4030_PM_MASTER_CFG_P123_TRANSITION);
@@ -590,10 +631,19 @@ static int twl4030_power_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_OF
+static const struct of_device_id twl4030_power_of_match[] = {
+	{.compatible = "ti,twl4030-power", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, twl4030_power_of_match);
+#endif
+
 static struct platform_driver twl4030_power_driver = {
 	.driver = {
 		.name	= "twl4030_power",
 		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(twl4030_power_of_match),
 	},
 	.probe		= twl4030_power_probe,
 	.remove		= twl4030_power_remove,
-- 
1.7.5.4

[PATCH 3/3] mfd: twl4030-power: Simplify error path

[Florian Vaussard]

Remove unnecessary goto statements, causing duplicated if
conditions.

Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
---
 drivers/mfd/twl4030-power.c |   37 ++++++++++++++-----------------------
 1 files changed, 14 insertions(+), 23 deletions(-)

diff --git a/drivers/mfd/twl4030-power.c b/drivers/mfd/twl4030-power.c
index d12d748..7eed526 100644
--- a/drivers/mfd/twl4030-power.c
+++ b/drivers/mfd/twl4030-power.c
@@ -567,21 +567,25 @@ int twl4030_power_probe(struct platform_device *pdev)
 
 	err = twl_i2c_write_u8(TWL_MODULE_PM_MASTER, TWL4030_PM_MASTER_KEY_CFG1,
 			       TWL4030_PM_MASTER_PROTECT_KEY);
-	if (err)
-		goto unlock;
-
-	err = twl_i2c_write_u8(TWL_MODULE_PM_MASTER, TWL4030_PM_MASTER_KEY_CFG2,
+	err |= twl_i2c_write_u8(TWL_MODULE_PM_MASTER, TWL4030_PM_MASTER_KEY_CFG2,
 			       TWL4030_PM_MASTER_PROTECT_KEY);
-	if (err)
-		goto unlock;
+
+	if (err) {
+		pr_err("TWL4030 Unable to unlock registers\n");
+		return err;
+	}
 
 	if (pdata) {
 		err = twl4030_power_configure_scripts(pdata);
-		if (err)
-			goto load;
+		if (err) {
+			pr_err("TWL4030 failed to load scripts\n");
+			return err;
+		}
 		err = twl4030_power_configure_resources(pdata);
-		if (err)
-			goto resource;
+		if (err) {
+			pr_err("TWL4030 failed to configure resource\n");
+			return err;
+		}
 	}
 
 	/* Board has to be wired properly to use this feature */
@@ -611,19 +615,6 @@ relock:
 	if (err)
 		pr_err("TWL4030 Unable to relock registers\n");
 	return err;
-
-unlock:
-	if (err)
-		pr_err("TWL4030 Unable to unlock registers\n");
-	return err;
-load:
-	if (err)
-		pr_err("TWL4030 failed to load scripts\n");
-	return err;
-resource:
-	if (err)
-		pr_err("TWL4030 failed to configure resource\n");
-	return err;
 }
 
 static int twl4030_power_remove(struct platform_device *pdev)
-- 
1.7.5.4

Re: [PATCH 1/3] mfd: twl4030-power: Split from twl-core into a dedicated module

[Samuel Ortiz]

Hi Florian,

On Thu, May 30, 2013 at 03:51:54PM +0200, Florian Vaussard wrote:
> For now, the call to twl4030-power is hard-wired inside twl-core.
> To ease the future transition to DT, make twl4030-power as a
> separate module, like what is already done for twl4030-audio
> and others.
> 
> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
> ---
>  drivers/mfd/twl-core.c      |   12 ++++++---
>  drivers/mfd/twl4030-power.c |   54 +++++++++++++++++++++++++++++++++++--------
>  include/linux/i2c/twl.h     |    1 -
>  3 files changed, 52 insertions(+), 15 deletions(-)
Looks good, I only have one comment:

> +static struct platform_driver twl4030_power_driver = {
> +	.driver = {
> +		.name	= "twl4030_power",
> +		.owner	= THIS_MODULE,
> +	},
> +	.probe		= twl4030_power_probe,
> +	.remove		= twl4030_power_remove,
> +};
> +
> +static int __init twl4030_power_init(void)
> +{
> +	return platform_driver_register(&twl4030_power_driver);
>  }
> +subsys_initcall(twl4030_power_init);
> +
> +static void __exit twl4030_power_exit(void)
> +{
> +	platform_driver_unregister(&twl4030_power_driver);
> +}
> +module_exit(twl4030_power_exit);
Please use module_platform_driver() here.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: [PATCH 2/3] mfd: twl4030-power: Start transition to DT

[Samuel Ortiz]

Hi Florian,

On Thu, May 30, 2013 at 03:51:55PM +0200, Florian Vaussard wrote:
>  int twl4030_power_probe(struct platform_device *pdev)
>  {
>  	struct twl4030_power_data *pdata = pdev->dev.platform_data;
> +	struct device_node *node = pdev->dev.of_node;
>  	int err = 0;
> -	int i;
> -	struct twl4030_resconfig *resconfig;
> -	u8 val, address = twl4030_start_script_address;
> +	u8 val;
> +
> +	if (!pdata && !node) {
> +		dev_err(&pdev->dev, "Platform data is missing\n");
> +		return -EINVAL;
> +	}
>  
>  	err = twl_i2c_write_u8(TWL_MODULE_PM_MASTER, TWL4030_PM_MASTER_KEY_CFG1,
>  			       TWL4030_PM_MASTER_PROTECT_KEY);
> @@ -525,26 +575,17 @@ int twl4030_power_probe(struct platform_device *pdev)
>  	if (err)
>  		goto unlock;
>  
> -	for (i = 0; i < pdata->num; i++) {
> -		err = load_twl4030_script(pdata->scripts[i], address);
> +	if (pdata) {
> +		err = twl4030_power_configure_scripts(pdata);
>  		if (err)
>  			goto load;
> -		address += pdata->scripts[i]->size;
> -	}
> -
> -	resconfig = pdata->resource_config;
> -	if (resconfig) {
> -		while (resconfig->resource) {
> -			err = twl4030_configure_resource(resconfig);
> -			if (err)
> -				goto resource;
> -			resconfig++;
> -
> -		}
> +		err = twl4030_power_configure_resources(pdata);
> +		if (err)
> +			goto resource;
You're simplifying the probe routine here by defining 2
twl4030_power_configure_* functions. That's good, but it should be a
separate patch as it's not related to the DT porting effort.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: [PATCH 1/3] mfd: twl4030-power: Split from twl-core into a dedicated module

[Florian Vaussard]

Hello,

Thank you for the review.

On 06/18/2013 01:56 AM, Samuel Ortiz wrote:
> Hi Florian,
>
> On Thu, May 30, 2013 at 03:51:54PM +0200, Florian Vaussard wrote:
>> For now, the call to twl4030-power is hard-wired inside twl-core.
>> To ease the future transition to DT, make twl4030-power as a
>> separate module, like what is already done for twl4030-audio
>> and others.
>>
>> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
>> ---
>>   drivers/mfd/twl-core.c      |   12 ++++++---
>>   drivers/mfd/twl4030-power.c |   54 +++++++++++++++++++++++++++++++++++--------
>>   include/linux/i2c/twl.h     |    1 -
>>   3 files changed, 52 insertions(+), 15 deletions(-)
> Looks good, I only have one comment:
>
>> +static struct platform_driver twl4030_power_driver = {
>> +	.driver = {
>> +		.name	= "twl4030_power",
>> +		.owner	= THIS_MODULE,
>> +	},
>> +	.probe		= twl4030_power_probe,
>> +	.remove		= twl4030_power_remove,
>> +};
>> +
>> +static int __init twl4030_power_init(void)
>> +{
>> +	return platform_driver_register(&twl4030_power_driver);
>>   }
>> +subsys_initcall(twl4030_power_init);
>> +
>> +static void __exit twl4030_power_exit(void)
>> +{
>> +	platform_driver_unregister(&twl4030_power_driver);
>> +}
>> +module_exit(twl4030_power_exit);
> Please use module_platform_driver() here.
>

Sure!

Regards,

Florian

Re: [PATCH 2/3] mfd: twl4030-power: Start transition to DT

[Florian Vaussard]

Hello,

On 06/18/2013 02:02 AM, Samuel Ortiz wrote:
> Hi Florian,
>
> On Thu, May 30, 2013 at 03:51:55PM +0200, Florian Vaussard wrote:
>>   int twl4030_power_probe(struct platform_device *pdev)
>>   {
>>   	struct twl4030_power_data *pdata = pdev->dev.platform_data;
>> +	struct device_node *node = pdev->dev.of_node;
>>   	int err = 0;
>> -	int i;
>> -	struct twl4030_resconfig *resconfig;
>> -	u8 val, address = twl4030_start_script_address;
>> +	u8 val;
>> +
>> +	if (!pdata && !node) {
>> +		dev_err(&pdev->dev, "Platform data is missing\n");
>> +		return -EINVAL;
>> +	}
>>
>>   	err = twl_i2c_write_u8(TWL_MODULE_PM_MASTER, TWL4030_PM_MASTER_KEY_CFG1,
>>   			       TWL4030_PM_MASTER_PROTECT_KEY);
>> @@ -525,26 +575,17 @@ int twl4030_power_probe(struct platform_device *pdev)
>>   	if (err)
>>   		goto unlock;
>>
>> -	for (i = 0; i < pdata->num; i++) {
>> -		err = load_twl4030_script(pdata->scripts[i], address);
>> +	if (pdata) {
>> +		err = twl4030_power_configure_scripts(pdata);
>>   		if (err)
>>   			goto load;
>> -		address += pdata->scripts[i]->size;
>> -	}
>> -
>> -	resconfig = pdata->resource_config;
>> -	if (resconfig) {
>> -		while (resconfig->resource) {
>> -			err = twl4030_configure_resource(resconfig);
>> -			if (err)
>> -				goto resource;
>> -			resconfig++;
>> -
>> -		}
>> +		err = twl4030_power_configure_resources(pdata);
>> +		if (err)
>> +			goto resource;
> You're simplifying the probe routine here by defining 2
> twl4030_power_configure_* functions. That's good, but it should be a
> separate patch as it's not related to the DT porting effort.
>

I agree. I will post a v2 with the changes.

Regards,
Florian