* Re: [PATCH V3 3/3] rtc: omap: Support regulator supply for RTC
2014-10-24 4:53 ` [PATCH V3 3/3] rtc: omap: Support regulator supply for RTC Lokesh Vutla
@ 2014-10-24 7:53 ` Johan Hovold
2014-10-24 7:57 ` Lokesh Vutla
2014-10-24 15:40 ` Felipe Balbi
2014-10-24 8:07 ` [PATCH V4 " Lokesh Vutla
2014-10-24 13:26 ` [PATCH V3 " Nishanth Menon
2 siblings, 2 replies; 19+ messages in thread
From: Johan Hovold @ 2014-10-24 7:53 UTC (permalink / raw)
To: Lokesh Vutla
Cc: rtc-linux, linux-omap, a.zummo, johan, tony, bcousson, balbi,
akpm, linux, nsekhar, t-kristo, j-keerthy, linux-arm-kernel,
devicetree
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
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH V3 3/3] rtc: omap: Support regulator supply for RTC
2014-10-24 7:53 ` Johan Hovold
@ 2014-10-24 7:57 ` Lokesh Vutla
2014-10-24 15:40 ` Felipe Balbi
1 sibling, 0 replies; 19+ messages in thread
From: Lokesh Vutla @ 2014-10-24 7:57 UTC (permalink / raw)
To: Johan Hovold
Cc: rtc-linux, linux-omap, a.zummo, tony, bcousson, balbi, akpm,
linux, nsekhar, t-kristo, j-keerthy, linux-arm-kernel, devicetree
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
>
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH V3 3/3] rtc: omap: Support regulator supply for RTC
2014-10-24 7:53 ` Johan Hovold
2014-10-24 7:57 ` Lokesh Vutla
@ 2014-10-24 15:40 ` Felipe Balbi
1 sibling, 0 replies; 19+ messages in thread
From: Felipe Balbi @ 2014-10-24 15:40 UTC (permalink / raw)
To: Johan Hovold
Cc: Lokesh Vutla, rtc-linux, linux-omap, a.zummo, tony, bcousson,
balbi, akpm, linux, nsekhar, t-kristo, j-keerthy,
linux-arm-kernel, devicetree
[-- 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 --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH V4 3/3] rtc: omap: Support regulator supply for RTC
2014-10-24 4:53 ` [PATCH V3 3/3] rtc: omap: Support regulator supply for RTC Lokesh Vutla
2014-10-24 7:53 ` Johan Hovold
@ 2014-10-24 8:07 ` Lokesh Vutla
2014-10-24 15:40 ` Felipe Balbi
2014-10-24 13:26 ` [PATCH V3 " Nishanth Menon
2 siblings, 1 reply; 19+ messages in thread
From: Lokesh Vutla @ 2014-10-24 8:07 UTC (permalink / raw)
To: rtc-linux, linux-omap, a.zummo, johan
Cc: tony, bcousson, balbi, akpm, linux, nsekhar, t-kristo, j-keerthy,
linux-arm-kernel, devicetree, lokeshvutla
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
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH V4 3/3] rtc: omap: Support regulator supply for RTC
2014-10-24 8:07 ` [PATCH V4 " Lokesh Vutla
@ 2014-10-24 15:40 ` Felipe Balbi
0 siblings, 0 replies; 19+ messages in thread
From: Felipe Balbi @ 2014-10-24 15:40 UTC (permalink / raw)
To: Lokesh Vutla
Cc: rtc-linux, linux-omap, a.zummo, johan, tony, bcousson, balbi,
akpm, linux, nsekhar, t-kristo, j-keerthy, linux-arm-kernel,
devicetree
[-- 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 --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH V3 3/3] rtc: omap: Support regulator supply for RTC
2014-10-24 4:53 ` [PATCH V3 3/3] rtc: omap: Support regulator supply for RTC Lokesh Vutla
2014-10-24 7:53 ` Johan Hovold
2014-10-24 8:07 ` [PATCH V4 " Lokesh Vutla
@ 2014-10-24 13:26 ` Nishanth Menon
2014-10-24 15:43 ` Felipe Balbi
2014-10-28 9:49 ` Lokesh Vutla
2 siblings, 2 replies; 19+ messages in thread
From: Nishanth Menon @ 2014-10-24 13:26 UTC (permalink / raw)
To: Lokesh Vutla, rtc-linux, linux-omap, a.zummo, johan
Cc: tony, bcousson, balbi, akpm, linux, nsekhar, t-kristo, j-keerthy,
linux-arm-kernel, devicetree
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
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH V3 3/3] rtc: omap: Support regulator supply for RTC
2014-10-24 13:26 ` [PATCH V3 " Nishanth Menon
@ 2014-10-24 15:43 ` Felipe Balbi
2014-10-24 15:47 ` Nishanth Menon
2014-10-28 9:49 ` Lokesh Vutla
1 sibling, 1 reply; 19+ messages in thread
From: Felipe Balbi @ 2014-10-24 15:43 UTC (permalink / raw)
To: Nishanth Menon
Cc: Lokesh Vutla, rtc-linux, linux-omap, a.zummo, johan, tony,
bcousson, balbi, akpm, linux, nsekhar, t-kristo, j-keerthy,
linux-arm-kernel, devicetree
[-- 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 --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH V3 3/3] rtc: omap: Support regulator supply for RTC
2014-10-24 15:43 ` Felipe Balbi
@ 2014-10-24 15:47 ` Nishanth Menon
0 siblings, 0 replies; 19+ messages in thread
From: Nishanth Menon @ 2014-10-24 15:47 UTC (permalink / raw)
To: balbi
Cc: Lokesh Vutla, rtc-linux, linux-omap, a.zummo, johan, tony,
bcousson, akpm, linux, nsekhar, t-kristo, j-keerthy,
linux-arm-kernel, devicetree
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH V3 3/3] rtc: omap: Support regulator supply for RTC
2014-10-24 13:26 ` [PATCH V3 " Nishanth Menon
2014-10-24 15:43 ` Felipe Balbi
@ 2014-10-28 9:49 ` Lokesh Vutla
1 sibling, 0 replies; 19+ messages in thread
From: Lokesh Vutla @ 2014-10-28 9:49 UTC (permalink / raw)
To: Nishanth Menon, rtc-linux, linux-omap, a.zummo, johan
Cc: tony, bcousson, balbi, akpm, linux, nsekhar, t-kristo, j-keerthy,
linux-arm-kernel, devicetree
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
>
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread