[PATCH V3 0/3] rtc: omap: Add support for regulator supply

[PATCH V3 0/3] rtc: omap: Add support for regulator supply

[Lokesh Vutla]

This series adds support for regulator supply.

Changes since v2:
 - Rebased on top of Johan Hovold's recent rtc cleanup series[1]
 - Addressed Johan Hovold's comments.

[1] https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg748519.html

Lokesh Vutla (2):
  rtc: omap: use module_platform_driver
  rtc: omap: Support regulator supply for RTC

Tero Kristo (1):
  rtc: omap: Update Kconfig for OMAP RTC

 Documentation/devicetree/bindings/rtc/rtc-omap.txt |  3 +++
 drivers/rtc/Kconfig                                |  6 ++---
 drivers/rtc/rtc-omap.c                             | 30 +++++++++++++++++++---
 3 files changed, 33 insertions(+), 6 deletions(-)

-- 
1.9.1

[PATCH V3 1/3] rtc: omap: use module_platform_driver

[Lokesh Vutla]

module_platform_driver_probe() prevents driver from requesting probe deferral.
So using module_platform_drive() to support probe deferral.
And also removing .owner field which is set by module_platform_driver.

Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
 drivers/rtc/rtc-omap.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
index e74750f..d9bb5e7 100644
--- a/drivers/rtc/rtc-omap.c
+++ b/drivers/rtc/rtc-omap.c
@@ -477,7 +477,7 @@ static const struct of_device_id omap_rtc_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, omap_rtc_of_match);
 
-static int __init omap_rtc_probe(struct platform_device *pdev)
+static int omap_rtc_probe(struct platform_device *pdev)
 {
 	struct omap_rtc	*rtc;
 	struct resource	*res;
@@ -708,18 +708,18 @@ static void omap_rtc_shutdown(struct platform_device *pdev)
 }
 
 static struct platform_driver omap_rtc_driver = {
+	.probe		= omap_rtc_probe,
 	.remove		= __exit_p(omap_rtc_remove),
 	.shutdown	= omap_rtc_shutdown,
 	.driver		= {
 		.name	= "omap_rtc",
-		.owner	= THIS_MODULE,
 		.pm	= &omap_rtc_pm_ops,
 		.of_match_table = omap_rtc_of_match,
 	},
 	.id_table	= omap_rtc_id_table,
 };
 
-module_platform_driver_probe(omap_rtc_driver, omap_rtc_probe);
+module_platform_driver(omap_rtc_driver);
 
 MODULE_ALIAS("platform:omap_rtc");
 MODULE_AUTHOR("George G. Davis (and others)");
-- 
1.9.1

[PATCH V3 2/3] rtc: omap: Update Kconfig for OMAP RTC

[Lokesh Vutla]

From: Tero Kristo <t-kristo@ti.com>

RTC is present in AM43xx and DRA7xx also. Updating the Kconfig
to depend on ARCH_OMAP or ARCH_DAVINCI

Reviewed-by: Felipe Balbi <balbi@ti.com>
Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 drivers/rtc/Kconfig | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 94ae179..8dfcb1b 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1001,11 +1001,11 @@ config RTC_DRV_IMXDI
 	   will be called "rtc-imxdi".
 
 config RTC_DRV_OMAP
-	tristate "TI OMAP1"
-	depends on ARCH_OMAP15XX || ARCH_OMAP16XX || ARCH_OMAP730 || ARCH_DAVINCI_DA8XX || SOC_AM33XX
+	tristate "TI OMAP Real Time Clock"
+	depends on ARCH_OMAP || ARCH_DAVINCI
 	help
 	  Say "yes" here to support the on chip real time clock
-	  present on TI OMAP1, AM33xx and DA8xx/OMAP-L13x.
+	  present on TI OMAP1, AM33xx, DA8xx/OMAP-L13x, AM43xx and DRA7xx.
 
 	  This driver can also be built as a module, if so, module
 	  will be called rtc-omap.
-- 
1.9.1

[PATCH V3 3/3] rtc: omap: Support regulator supply for RTC

[Lokesh Vutla]

On some Soc's RTC is powered by an external power regulator.
e.g. RTC on DRA7 SoC. Make the OMAP RTC driver support a
power regulator.

Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
 Documentation/devicetree/bindings/rtc/rtc-omap.txt |  3 +++
 drivers/rtc/rtc-omap.c                             | 24 ++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
index 750efd4..e7ad12b 100644
--- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
+++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
@@ -15,6 +15,8 @@ Required properties:
 Optional properties:
 - ti,system-power-controller: whether the rtc is controlling the system power
   through pmic_power_en
+Optional Properties:
+- vrtc-supply: phandle to the regulator device tree node if needed
 
 Example:
 
@@ -25,4 +27,5 @@ rtc@1c23000 {
 		      19>;
 	interrupt-parent = <&intc>;
 	ti,system-power-controller;
+	vrtc-supply = <&ldo9_reg>;
 };
diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
index d9bb5e7..61fe630 100644
--- a/drivers/rtc/rtc-omap.c
+++ b/drivers/rtc/rtc-omap.c
@@ -25,6 +25,7 @@
 #include <linux/of_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/io.h>
+#include <linux/regulator/consumer.h>
 
 /*
  * The OMAP RTC is a year/month/day/hours/minutes/seconds BCD clock
@@ -134,6 +135,7 @@ struct omap_rtc {
 	u8 interrupts_reg;
 	bool is_pmic_controller;
 	const struct omap_rtc_device_type *type;
+	struct regulator *supply;
 };
 
 static inline u8 rtc_read(struct omap_rtc *rtc, unsigned int reg)
@@ -516,6 +518,22 @@ static int omap_rtc_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, rtc);
 
+	rtc->supply = devm_regulator_get_optional(&pdev->dev, "vrtc");
+	if (IS_ERR(rtc->supply)) {
+		if (PTR_ERR(rtc->supply) == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+
+		rtc->supply = NULL;
+	}
+
+	if (rtc->supply) {
+		ret = regulator_enable(rtc->supply);
+		if (ret) {
+			dev_err(&pdev->dev, "regulator enable failed\n");
+			return ret;
+		}
+	}
+
 	/* Enable the clock/module so that we can access the registers */
 	pm_runtime_enable(&pdev->dev);
 	pm_runtime_get_sync(&pdev->dev);
@@ -624,6 +642,9 @@ err:
 	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
+	if (rtc->supply)
+		regulator_disable(rtc->supply);
+
 	return ret;
 }
 
@@ -649,6 +670,9 @@ static int __exit omap_rtc_remove(struct platform_device *pdev)
 	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
+	if (rtc->supply)
+		regulator_disable(rtc->supply);
+
 	return 0;
 }
 
-- 
1.9.1

Re: [PATCH V3 1/3] rtc: omap: use module_platform_driver

[Johan Hovold]

On Fri, Oct 24, 2014 at 10:23:43AM +0530, Lokesh Vutla wrote:
> module_platform_driver_probe() prevents driver from requesting probe deferral.
> So using module_platform_drive() to support probe deferral.
> And also removing .owner field which is set by module_platform_driver.
> 
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>

Reviewed-by: Johan Hovold <johan@kernel.org>

> ---
>  drivers/rtc/rtc-omap.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
> index e74750f..d9bb5e7 100644
> --- a/drivers/rtc/rtc-omap.c
> +++ b/drivers/rtc/rtc-omap.c
> @@ -477,7 +477,7 @@ static const struct of_device_id omap_rtc_of_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, omap_rtc_of_match);
>  
> -static int __init omap_rtc_probe(struct platform_device *pdev)
> +static int omap_rtc_probe(struct platform_device *pdev)
>  {
>  	struct omap_rtc	*rtc;
>  	struct resource	*res;
> @@ -708,18 +708,18 @@ static void omap_rtc_shutdown(struct platform_device *pdev)
>  }
>  
>  static struct platform_driver omap_rtc_driver = {
> +	.probe		= omap_rtc_probe,
>  	.remove		= __exit_p(omap_rtc_remove),
>  	.shutdown	= omap_rtc_shutdown,
>  	.driver		= {
>  		.name	= "omap_rtc",
> -		.owner	= THIS_MODULE,
>  		.pm	= &omap_rtc_pm_ops,
>  		.of_match_table = omap_rtc_of_match,
>  	},
>  	.id_table	= omap_rtc_id_table,
>  };
>  
> -module_platform_driver_probe(omap_rtc_driver, omap_rtc_probe);
> +module_platform_driver(omap_rtc_driver);
>  
>  MODULE_ALIAS("platform:omap_rtc");
>  MODULE_AUTHOR("George G. Davis (and others)");

Re: [PATCH V3 3/3] rtc: omap: Support regulator supply for RTC

[Johan Hovold]

On Fri, Oct 24, 2014 at 10:23:45AM +0530, Lokesh Vutla wrote:
> On some Soc's RTC is powered by an external power regulator.
> e.g. RTC on DRA7 SoC. Make the OMAP RTC driver support a
> power regulator.
> 
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> ---
>  Documentation/devicetree/bindings/rtc/rtc-omap.txt |  3 +++
>  drivers/rtc/rtc-omap.c                             | 24 ++++++++++++++++++++++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> index 750efd4..e7ad12b 100644
> --- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> +++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> @@ -15,6 +15,8 @@ Required properties:
>  Optional properties:
>  - ti,system-power-controller: whether the rtc is controlling the system power
>    through pmic_power_en
> +Optional Properties:

No need to repeat the "Optional Properties" header.

> +- vrtc-supply: phandle to the regulator device tree node if needed
>  
>  Example:
>  
> @@ -25,4 +27,5 @@ rtc@1c23000 {
>  		      19>;
>  	interrupt-parent = <&intc>;
>  	ti,system-power-controller;
> +	vrtc-supply = <&ldo9_reg>;
>  };
> diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
> index d9bb5e7..61fe630 100644
> --- a/drivers/rtc/rtc-omap.c
> +++ b/drivers/rtc/rtc-omap.c
> @@ -25,6 +25,7 @@
>  #include <linux/of_device.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/io.h>
> +#include <linux/regulator/consumer.h>
>  
>  /*
>   * The OMAP RTC is a year/month/day/hours/minutes/seconds BCD clock
> @@ -134,6 +135,7 @@ struct omap_rtc {
>  	u8 interrupts_reg;
>  	bool is_pmic_controller;
>  	const struct omap_rtc_device_type *type;
> +	struct regulator *supply;
>  };
>  
>  static inline u8 rtc_read(struct omap_rtc *rtc, unsigned int reg)
> @@ -516,6 +518,22 @@ static int omap_rtc_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, rtc);

Also could you move the regulator allocation before this
platform_set_drvdata when resending? It's not required, but that call
sort of separates the allocations from the device initialisation
currently.

> +	rtc->supply = devm_regulator_get_optional(&pdev->dev, "vrtc");
> +	if (IS_ERR(rtc->supply)) {
> +		if (PTR_ERR(rtc->supply) == -EPROBE_DEFER)
> +			return -EPROBE_DEFER;
> +
> +		rtc->supply = NULL;
> +	}
> +
> +	if (rtc->supply) {
> +		ret = regulator_enable(rtc->supply);
> +		if (ret) {
> +			dev_err(&pdev->dev, "regulator enable failed\n");
> +			return ret;
> +		}
> +	}
> +
>  	/* Enable the clock/module so that we can access the registers */
>  	pm_runtime_enable(&pdev->dev);
>  	pm_runtime_get_sync(&pdev->dev);
> @@ -624,6 +642,9 @@ err:
>  	pm_runtime_put_sync(&pdev->dev);
>  	pm_runtime_disable(&pdev->dev);
>  
> +	if (rtc->supply)
> +		regulator_disable(rtc->supply);
> +
>  	return ret;
>  }
>  
> @@ -649,6 +670,9 @@ static int __exit omap_rtc_remove(struct platform_device *pdev)
>  	pm_runtime_put_sync(&pdev->dev);
>  	pm_runtime_disable(&pdev->dev);
>  
> +	if (rtc->supply)
> +		regulator_disable(rtc->supply);
> +
>  	return 0;
>  }

Looks good otherwise. Feel free to add

Reviewed-by: Johan Hovold <johan@kernel.org>

Thanks,
Johan

Re: [PATCH V3 3/3] rtc: omap: Support regulator supply for RTC

[Lokesh Vutla]

On Friday 24 October 2014 01:23 PM, Johan Hovold wrote:
> On Fri, Oct 24, 2014 at 10:23:45AM +0530, Lokesh Vutla wrote:
>> On some Soc's RTC is powered by an external power regulator.
>> e.g. RTC on DRA7 SoC. Make the OMAP RTC driver support a
>> power regulator.
>>
>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>> ---
>>  Documentation/devicetree/bindings/rtc/rtc-omap.txt |  3 +++
>>  drivers/rtc/rtc-omap.c                             | 24 ++++++++++++++++++++++
>>  2 files changed, 27 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
>> index 750efd4..e7ad12b 100644
>> --- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
>> +++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
>> @@ -15,6 +15,8 @@ Required properties:
>>  Optional properties:
>>  - ti,system-power-controller: whether the rtc is controlling the system power
>>    through pmic_power_en
>> +Optional Properties:
> 
> No need to repeat the "Optional Properties" header.
Oops sorry. will remove it.
> 
>> +- vrtc-supply: phandle to the regulator device tree node if needed
>>  
>>  Example:
>>  
>> @@ -25,4 +27,5 @@ rtc@1c23000 {
>>  		      19>;
>>  	interrupt-parent = <&intc>;
>>  	ti,system-power-controller;
>> +	vrtc-supply = <&ldo9_reg>;
>>  };
>> diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
>> index d9bb5e7..61fe630 100644
>> --- a/drivers/rtc/rtc-omap.c
>> +++ b/drivers/rtc/rtc-omap.c
>> @@ -25,6 +25,7 @@
>>  #include <linux/of_device.h>
>>  #include <linux/pm_runtime.h>
>>  #include <linux/io.h>
>> +#include <linux/regulator/consumer.h>
>>  
>>  /*
>>   * The OMAP RTC is a year/month/day/hours/minutes/seconds BCD clock
>> @@ -134,6 +135,7 @@ struct omap_rtc {
>>  	u8 interrupts_reg;
>>  	bool is_pmic_controller;
>>  	const struct omap_rtc_device_type *type;
>> +	struct regulator *supply;
>>  };
>>  
>>  static inline u8 rtc_read(struct omap_rtc *rtc, unsigned int reg)
>> @@ -516,6 +518,22 @@ static int omap_rtc_probe(struct platform_device *pdev)
>>  
>>  	platform_set_drvdata(pdev, rtc);
> 
> Also could you move the regulator allocation before this
> platform_set_drvdata when resending? It's not required, but that call
> sort of separates the allocations from the device initialisation
> currently.
Okay.

Thanks and regards,
Lokesh
> 
>> +	rtc->supply = devm_regulator_get_optional(&pdev->dev, "vrtc");
>> +	if (IS_ERR(rtc->supply)) {
>> +		if (PTR_ERR(rtc->supply) == -EPROBE_DEFER)
>> +			return -EPROBE_DEFER;
>> +
>> +		rtc->supply = NULL;
>> +	}
>> +
>> +	if (rtc->supply) {
>> +		ret = regulator_enable(rtc->supply);
>> +		if (ret) {
>> +			dev_err(&pdev->dev, "regulator enable failed\n");
>> +			return ret;
>> +		}
>> +	}
>> +
>>  	/* Enable the clock/module so that we can access the registers */
>>  	pm_runtime_enable(&pdev->dev);
>>  	pm_runtime_get_sync(&pdev->dev);
>> @@ -624,6 +642,9 @@ err:
>>  	pm_runtime_put_sync(&pdev->dev);
>>  	pm_runtime_disable(&pdev->dev);
>>  
>> +	if (rtc->supply)
>> +		regulator_disable(rtc->supply);
>> +
>>  	return ret;
>>  }
>>  
>> @@ -649,6 +670,9 @@ static int __exit omap_rtc_remove(struct platform_device *pdev)
>>  	pm_runtime_put_sync(&pdev->dev);
>>  	pm_runtime_disable(&pdev->dev);
>>  
>> +	if (rtc->supply)
>> +		regulator_disable(rtc->supply);
>> +
>>  	return 0;
>>  }
> 
> Looks good otherwise. Feel free to add
> 
> Reviewed-by: Johan Hovold <johan@kernel.org>
> 
> Thanks,
> Johan
> 

[PATCH V4 3/3] rtc: omap: Support regulator supply for RTC

[Lokesh Vutla]

On some Soc's RTC is powered by an external power regulator.
e.g. RTC on DRA7 SoC. Make the OMAP RTC driver support a
power regulator.

Reviewed-by: Johan Hovold <johan@kernel.org>
Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
Changes since v3:
- Removed extra Optional properties header.
- Moved regulator get before platform_set_drvdatai() as suggested by Johan

 Documentation/devicetree/bindings/rtc/rtc-omap.txt |  2 ++
 drivers/rtc/rtc-omap.c                             | 24 ++++++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
index 750efd4..42ff41f 100644
--- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
+++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
@@ -15,6 +15,7 @@ Required properties:
 Optional properties:
 - ti,system-power-controller: whether the rtc is controlling the system power
   through pmic_power_en
+- vrtc-supply: phandle to the regulator device tree node if needed
 
 Example:
 
@@ -25,4 +26,5 @@ rtc@1c23000 {
 		      19>;
 	interrupt-parent = <&intc>;
 	ti,system-power-controller;
+	vrtc-supply = <&ldo9_reg>;
 };
diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
index d9bb5e7..7f1b527 100644
--- a/drivers/rtc/rtc-omap.c
+++ b/drivers/rtc/rtc-omap.c
@@ -25,6 +25,7 @@
 #include <linux/of_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/io.h>
+#include <linux/regulator/consumer.h>
 
 /*
  * The OMAP RTC is a year/month/day/hours/minutes/seconds BCD clock
@@ -134,6 +135,7 @@ struct omap_rtc {
 	u8 interrupts_reg;
 	bool is_pmic_controller;
 	const struct omap_rtc_device_type *type;
+	struct regulator *supply;
 };
 
 static inline u8 rtc_read(struct omap_rtc *rtc, unsigned int reg)
@@ -514,6 +516,22 @@ static int omap_rtc_probe(struct platform_device *pdev)
 	if (IS_ERR(rtc->base))
 		return PTR_ERR(rtc->base);
 
+	rtc->supply = devm_regulator_get_optional(&pdev->dev, "vrtc");
+	if (IS_ERR(rtc->supply)) {
+		if (PTR_ERR(rtc->supply) == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+
+		rtc->supply = NULL;
+	}
+
+	if (rtc->supply) {
+		ret = regulator_enable(rtc->supply);
+		if (ret) {
+			dev_err(&pdev->dev, "regulator enable failed\n");
+			return ret;
+		}
+	}
+
 	platform_set_drvdata(pdev, rtc);
 
 	/* Enable the clock/module so that we can access the registers */
@@ -624,6 +642,9 @@ err:
 	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
+	if (rtc->supply)
+		regulator_disable(rtc->supply);
+
 	return ret;
 }
 
@@ -649,6 +670,9 @@ static int __exit omap_rtc_remove(struct platform_device *pdev)
 	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
+	if (rtc->supply)
+		regulator_disable(rtc->supply);
+
 	return 0;
 }
 
-- 
1.9.1

Re: [PATCH V3 3/3] rtc: omap: Support regulator supply for RTC

[Nishanth Menon]

On 10/23/2014 11:53 PM, Lokesh Vutla wrote:
> On some Soc's RTC is powered by an external power regulator.
SoC ? -> could you rephrase this to indicate "certain SoCs such as
DRA7, RTC is an independent voltage domain of it's own and on
platforms such as DRA7-evm, this may be supplied by individual
regulator on it's own.

> e.g. RTC on DRA7 SoC. Make the OMAP RTC driver support a
> power regulator.

Question ofcourse is what voltage would you like that regulator to be
at? As you are aware, certain LDOs and SMPS can drive varying voltage
and just enable/disable would do just the default voltage of the
SMPS/LDO, right? OR am i missing something here?

> 
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> ---
>  Documentation/devicetree/bindings/rtc/rtc-omap.txt |  3 +++
>  drivers/rtc/rtc-omap.c                             | 24 ++++++++++++++++++++++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> index 750efd4..e7ad12b 100644
> --- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> +++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> @@ -15,6 +15,8 @@ Required properties:
>  Optional properties:
>  - ti,system-power-controller: whether the rtc is controlling the system power
>    through pmic_power_en
> +Optional Properties:
^^ already commented on..

> +- vrtc-supply: phandle to the regulator device tree node if needed

"phandle to supply regulator" ? since it is optional, "if needed" is
redundant?

>  
>  Example:
>  
> @@ -25,4 +27,5 @@ rtc@1c23000 {
>  		      19>;
>  	interrupt-parent = <&intc>;
>  	ti,system-power-controller;
> +	vrtc-supply = <&ldo9_reg>;
>  };
> diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
> index d9bb5e7..61fe630 100644
> --- a/drivers/rtc/rtc-omap.c
> +++ b/drivers/rtc/rtc-omap.c
> @@ -25,6 +25,7 @@
>  #include <linux/of_device.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/io.h>
> +#include <linux/regulator/consumer.h>
>  
>  /*
>   * The OMAP RTC is a year/month/day/hours/minutes/seconds BCD clock
> @@ -134,6 +135,7 @@ struct omap_rtc {
>  	u8 interrupts_reg;
>  	bool is_pmic_controller;
>  	const struct omap_rtc_device_type *type;
> +	struct regulator *supply;
>  };
>  
>  static inline u8 rtc_read(struct omap_rtc *rtc, unsigned int reg)
> @@ -516,6 +518,22 @@ static int omap_rtc_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, rtc);
>  
> +	rtc->supply = devm_regulator_get_optional(&pdev->dev, "vrtc");
> +	if (IS_ERR(rtc->supply)) {
> +		if (PTR_ERR(rtc->supply) == -EPROBE_DEFER)
> +			return -EPROBE_DEFER;
> +
> +		rtc->supply = NULL;
> +	}
> +
> +	if (rtc->supply) {
> +		ret = regulator_enable(rtc->supply);
> +		if (ret) {
> +			dev_err(&pdev->dev, "regulator enable failed\n");

would be nice to print the result as well - since it helps debug from
log a little easier.



-- 
Regards,
Nishanth Menon

Re: [PATCH V3 1/3] rtc: omap: use module_platform_driver

[Felipe Balbi]

[-- Attachment #1.1: Type: text/plain, Size: 1675 bytes --]

On Fri, Oct 24, 2014 at 10:23:43AM +0530, Lokesh Vutla wrote:
> module_platform_driver_probe() prevents driver from requesting probe deferral.
> So using module_platform_drive() to support probe deferral.
> And also removing .owner field which is set by module_platform_driver.
> 
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>

Reviewed-by: Felipe Balbi <balbi@ti.com>

> ---
>  drivers/rtc/rtc-omap.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
> index e74750f..d9bb5e7 100644
> --- a/drivers/rtc/rtc-omap.c
> +++ b/drivers/rtc/rtc-omap.c
> @@ -477,7 +477,7 @@ static const struct of_device_id omap_rtc_of_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, omap_rtc_of_match);
>  
> -static int __init omap_rtc_probe(struct platform_device *pdev)
> +static int omap_rtc_probe(struct platform_device *pdev)
>  {
>  	struct omap_rtc	*rtc;
>  	struct resource	*res;
> @@ -708,18 +708,18 @@ static void omap_rtc_shutdown(struct platform_device *pdev)
>  }
>  
>  static struct platform_driver omap_rtc_driver = {
> +	.probe		= omap_rtc_probe,
>  	.remove		= __exit_p(omap_rtc_remove),
>  	.shutdown	= omap_rtc_shutdown,
>  	.driver		= {
>  		.name	= "omap_rtc",
> -		.owner	= THIS_MODULE,
>  		.pm	= &omap_rtc_pm_ops,
>  		.of_match_table = omap_rtc_of_match,
>  	},
>  	.id_table	= omap_rtc_id_table,
>  };
>  
> -module_platform_driver_probe(omap_rtc_driver, omap_rtc_probe);
> +module_platform_driver(omap_rtc_driver);
>  
>  MODULE_ALIAS("platform:omap_rtc");
>  MODULE_AUTHOR("George G. Davis (and others)");
> -- 
> 1.9.1
> 

-- 
balbi

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V3 3/3] rtc: omap: Support regulator supply for RTC

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 3693 bytes --]

Hi,

On Fri, Oct 24, 2014 at 09:53:12AM +0200, Johan Hovold wrote:
> On Fri, Oct 24, 2014 at 10:23:45AM +0530, Lokesh Vutla wrote:
> > On some Soc's RTC is powered by an external power regulator.
> > e.g. RTC on DRA7 SoC. Make the OMAP RTC driver support a
> > power regulator.
> > 
> > Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> > ---
> >  Documentation/devicetree/bindings/rtc/rtc-omap.txt |  3 +++
> >  drivers/rtc/rtc-omap.c                             | 24 ++++++++++++++++++++++
> >  2 files changed, 27 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> > index 750efd4..e7ad12b 100644
> > --- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> > +++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> > @@ -15,6 +15,8 @@ Required properties:
> >  Optional properties:
> >  - ti,system-power-controller: whether the rtc is controlling the system power
> >    through pmic_power_en
> > +Optional Properties:
> 
> No need to repeat the "Optional Properties" header.
> 
> > +- vrtc-supply: phandle to the regulator device tree node if needed
> >  
> >  Example:
> >  
> > @@ -25,4 +27,5 @@ rtc@1c23000 {
> >  		      19>;
> >  	interrupt-parent = <&intc>;
> >  	ti,system-power-controller;
> > +	vrtc-supply = <&ldo9_reg>;
> >  };
> > diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
> > index d9bb5e7..61fe630 100644
> > --- a/drivers/rtc/rtc-omap.c
> > +++ b/drivers/rtc/rtc-omap.c
> > @@ -25,6 +25,7 @@
> >  #include <linux/of_device.h>
> >  #include <linux/pm_runtime.h>
> >  #include <linux/io.h>
> > +#include <linux/regulator/consumer.h>
> >  
> >  /*
> >   * The OMAP RTC is a year/month/day/hours/minutes/seconds BCD clock
> > @@ -134,6 +135,7 @@ struct omap_rtc {
> >  	u8 interrupts_reg;
> >  	bool is_pmic_controller;
> >  	const struct omap_rtc_device_type *type;
> > +	struct regulator *supply;
> >  };
> >  
> >  static inline u8 rtc_read(struct omap_rtc *rtc, unsigned int reg)
> > @@ -516,6 +518,22 @@ static int omap_rtc_probe(struct platform_device *pdev)
> >  
> >  	platform_set_drvdata(pdev, rtc);
> 
> Also could you move the regulator allocation before this
> platform_set_drvdata when resending? It's not required, but that call
> sort of separates the allocations from the device initialisation
> currently.
> 
> > +	rtc->supply = devm_regulator_get_optional(&pdev->dev, "vrtc");
> > +	if (IS_ERR(rtc->supply)) {
> > +		if (PTR_ERR(rtc->supply) == -EPROBE_DEFER)
> > +			return -EPROBE_DEFER;
> > +
> > +		rtc->supply = NULL;
> > +	}
> > +
> > +	if (rtc->supply) {
> > +		ret = regulator_enable(rtc->supply);
> > +		if (ret) {
> > +			dev_err(&pdev->dev, "regulator enable failed\n");
> > +			return ret;
> > +		}
> > +	}
> > +
> >  	/* Enable the clock/module so that we can access the registers */
> >  	pm_runtime_enable(&pdev->dev);
> >  	pm_runtime_get_sync(&pdev->dev);
> > @@ -624,6 +642,9 @@ err:
> >  	pm_runtime_put_sync(&pdev->dev);
> >  	pm_runtime_disable(&pdev->dev);
> >  
> > +	if (rtc->supply)
> > +		regulator_disable(rtc->supply);
> > +
> >  	return ret;
> >  }
> >  
> > @@ -649,6 +670,9 @@ static int __exit omap_rtc_remove(struct platform_device *pdev)
> >  	pm_runtime_put_sync(&pdev->dev);
> >  	pm_runtime_disable(&pdev->dev);
> >  
> > +	if (rtc->supply)
> > +		regulator_disable(rtc->supply);
> > +
> >  	return 0;
> >  }
> 
> Looks good otherwise. Feel free to add
> 
> Reviewed-by: Johan Hovold <johan@kernel.org>

Conditional on Johan's comments:

Reviewed-by: Felipe Balbi <balbi@ti.com>

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH V4 3/3] rtc: omap: Support regulator supply for RTC

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 3258 bytes --]

On Fri, Oct 24, 2014 at 01:37:34PM +0530, Lokesh Vutla wrote:
> On some Soc's RTC is powered by an external power regulator.
> e.g. RTC on DRA7 SoC. Make the OMAP RTC driver support a
> power regulator.
> 
> Reviewed-by: Johan Hovold <johan@kernel.org>
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>

oh sorry, there was already a new version:

Reviewed-by: Felipe Balbi <balbi@ti.com>

> ---
> Changes since v3:
> - Removed extra Optional properties header.
> - Moved regulator get before platform_set_drvdatai() as suggested by Johan
> 
>  Documentation/devicetree/bindings/rtc/rtc-omap.txt |  2 ++
>  drivers/rtc/rtc-omap.c                             | 24 ++++++++++++++++++++++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> index 750efd4..42ff41f 100644
> --- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> +++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> @@ -15,6 +15,7 @@ Required properties:
>  Optional properties:
>  - ti,system-power-controller: whether the rtc is controlling the system power
>    through pmic_power_en
> +- vrtc-supply: phandle to the regulator device tree node if needed
>  
>  Example:
>  
> @@ -25,4 +26,5 @@ rtc@1c23000 {
>  		      19>;
>  	interrupt-parent = <&intc>;
>  	ti,system-power-controller;
> +	vrtc-supply = <&ldo9_reg>;
>  };
> diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
> index d9bb5e7..7f1b527 100644
> --- a/drivers/rtc/rtc-omap.c
> +++ b/drivers/rtc/rtc-omap.c
> @@ -25,6 +25,7 @@
>  #include <linux/of_device.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/io.h>
> +#include <linux/regulator/consumer.h>
>  
>  /*
>   * The OMAP RTC is a year/month/day/hours/minutes/seconds BCD clock
> @@ -134,6 +135,7 @@ struct omap_rtc {
>  	u8 interrupts_reg;
>  	bool is_pmic_controller;
>  	const struct omap_rtc_device_type *type;
> +	struct regulator *supply;
>  };
>  
>  static inline u8 rtc_read(struct omap_rtc *rtc, unsigned int reg)
> @@ -514,6 +516,22 @@ static int omap_rtc_probe(struct platform_device *pdev)
>  	if (IS_ERR(rtc->base))
>  		return PTR_ERR(rtc->base);
>  
> +	rtc->supply = devm_regulator_get_optional(&pdev->dev, "vrtc");
> +	if (IS_ERR(rtc->supply)) {
> +		if (PTR_ERR(rtc->supply) == -EPROBE_DEFER)
> +			return -EPROBE_DEFER;
> +
> +		rtc->supply = NULL;
> +	}
> +
> +	if (rtc->supply) {
> +		ret = regulator_enable(rtc->supply);
> +		if (ret) {
> +			dev_err(&pdev->dev, "regulator enable failed\n");
> +			return ret;
> +		}
> +	}
> +
>  	platform_set_drvdata(pdev, rtc);
>  
>  	/* Enable the clock/module so that we can access the registers */
> @@ -624,6 +642,9 @@ err:
>  	pm_runtime_put_sync(&pdev->dev);
>  	pm_runtime_disable(&pdev->dev);
>  
> +	if (rtc->supply)
> +		regulator_disable(rtc->supply);
> +
>  	return ret;
>  }
>  
> @@ -649,6 +670,9 @@ static int __exit omap_rtc_remove(struct platform_device *pdev)
>  	pm_runtime_put_sync(&pdev->dev);
>  	pm_runtime_disable(&pdev->dev);
>  
> +	if (rtc->supply)
> +		regulator_disable(rtc->supply);
> +
>  	return 0;
>  }
>  
> -- 
> 1.9.1
> 

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH V3 3/3] rtc: omap: Support regulator supply for RTC

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 982 bytes --]

On Fri, Oct 24, 2014 at 08:26:43AM -0500, Nishanth Menon wrote:
> On 10/23/2014 11:53 PM, Lokesh Vutla wrote:
> > On some Soc's RTC is powered by an external power regulator.
> SoC ? -> could you rephrase this to indicate "certain SoCs such as
> DRA7, RTC is an independent voltage domain of it's own and on
> platforms such as DRA7-evm, this may be supplied by individual
> regulator on it's own.
> 
> > e.g. RTC on DRA7 SoC. Make the OMAP RTC driver support a
> > power regulator.
> 
> Question ofcourse is what voltage would you like that regulator to be
> at? As you are aware, certain LDOs and SMPS can drive varying voltage
> and just enable/disable would do just the default voltage of the
> SMPS/LDO, right? OR am i missing something here?

just pass the correct voltage through DTS as we do for all other
regulators ? It's only tricky when we have a range of acceptable
voltages but for RTC, IIRC, it's always a set voltage (1.0V or 1.8V).

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH V3 3/3] rtc: omap: Support regulator supply for RTC

[Nishanth Menon]

On 10/24/2014 10:43 AM, Felipe Balbi wrote:
> On Fri, Oct 24, 2014 at 08:26:43AM -0500, Nishanth Menon wrote:
>> On 10/23/2014 11:53 PM, Lokesh Vutla wrote:
>>> On some Soc's RTC is powered by an external power regulator.
>> SoC ? -> could you rephrase this to indicate "certain SoCs such as
>> DRA7, RTC is an independent voltage domain of it's own and on
>> platforms such as DRA7-evm, this may be supplied by individual
>> regulator on it's own.
>>
>>> e.g. RTC on DRA7 SoC. Make the OMAP RTC driver support a
>>> power regulator.
>>
>> Question ofcourse is what voltage would you like that regulator to be
>> at? As you are aware, certain LDOs and SMPS can drive varying voltage
>> and just enable/disable would do just the default voltage of the
>> SMPS/LDO, right? OR am i missing something here?
> 
> just pass the correct voltage through DTS as we do for all other
> regulators ? It's only tricky when we have a range of acceptable
> voltages but for RTC, IIRC, it's always a set voltage (1.0V or 1.8V).
> 
you mean min=max=1.8V board constraint - sure it will work, except I
might expect to see that in the example? OR expect the driver to
explicitly set it.

-- 
Regards,
Nishanth Menon

Re: [PATCH V3 3/3] rtc: omap: Support regulator supply for RTC

[Lokesh Vutla]

On Friday 24 October 2014 06:56 PM, Nishanth Menon wrote:
> On 10/23/2014 11:53 PM, Lokesh Vutla wrote:
>> On some Soc's RTC is powered by an external power regulator.
> SoC ? -> could you rephrase this to indicate "certain SoCs such as
> DRA7, RTC is an independent voltage domain of it's own and on
> platforms such as DRA7-evm, this may be supplied by individual
> regulator on it's own.
Ok. will do it.
> 
>> e.g. RTC on DRA7 SoC. Make the OMAP RTC driver support a
>> power regulator.
> 
> Question ofcourse is what voltage would you like that regulator to be
> at? As you are aware, certain LDOs and SMPS can drive varying voltage
> and just enable/disable would do just the default voltage of the
> SMPS/LDO, right? OR am i missing something here?
Yes I agree that enable/disable would just do the default voltage of SMPS/LDO.
If default voltage needs to be changed, driver should explicitly call regulator_set_voltage.
Currently this is missing. Ill update and repost it. Thanks for pointing it out.
> 
>>
>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>> ---
>>  Documentation/devicetree/bindings/rtc/rtc-omap.txt |  3 +++
>>  drivers/rtc/rtc-omap.c                             | 24 ++++++++++++++++++++++
>>  2 files changed, 27 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
>> index 750efd4..e7ad12b 100644
>> --- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
>> +++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
>> @@ -15,6 +15,8 @@ Required properties:
>>  Optional properties:
>>  - ti,system-power-controller: whether the rtc is controlling the system power
>>    through pmic_power_en
>> +Optional Properties:
> ^^ already commented on..
> 
>> +- vrtc-supply: phandle to the regulator device tree node if needed
> 
> "phandle to supply regulator" ? since it is optional, "if needed" is
> redundant?
Okay. will update it in next version.
> 
>>  
>>  Example:
>>  
>> @@ -25,4 +27,5 @@ rtc@1c23000 {
>>  		      19>;
>>  	interrupt-parent = <&intc>;
>>  	ti,system-power-controller;
>> +	vrtc-supply = <&ldo9_reg>;
>>  };
>> diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
>> index d9bb5e7..61fe630 100644
>> --- a/drivers/rtc/rtc-omap.c
>> +++ b/drivers/rtc/rtc-omap.c
>> @@ -25,6 +25,7 @@
>>  #include <linux/of_device.h>
>>  #include <linux/pm_runtime.h>
>>  #include <linux/io.h>
>> +#include <linux/regulator/consumer.h>
>>  
>>  /*
>>   * The OMAP RTC is a year/month/day/hours/minutes/seconds BCD clock
>> @@ -134,6 +135,7 @@ struct omap_rtc {
>>  	u8 interrupts_reg;
>>  	bool is_pmic_controller;
>>  	const struct omap_rtc_device_type *type;
>> +	struct regulator *supply;
>>  };
>>  
>>  static inline u8 rtc_read(struct omap_rtc *rtc, unsigned int reg)
>> @@ -516,6 +518,22 @@ static int omap_rtc_probe(struct platform_device *pdev)
>>  
>>  	platform_set_drvdata(pdev, rtc);
>>  
>> +	rtc->supply = devm_regulator_get_optional(&pdev->dev, "vrtc");
>> +	if (IS_ERR(rtc->supply)) {
>> +		if (PTR_ERR(rtc->supply) == -EPROBE_DEFER)
>> +			return -EPROBE_DEFER;
>> +
>> +		rtc->supply = NULL;
>> +	}
>> +
>> +	if (rtc->supply) {
>> +		ret = regulator_enable(rtc->supply);
>> +		if (ret) {
>> +			dev_err(&pdev->dev, "regulator enable failed\n");
> 
> would be nice to print the result as well - since it helps debug from
> log a little easier.
Will update it in next version.

Thanks and regards,
Lokesh
> 
> 
>