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

[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 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 --]

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

[Lokesh Vutla]

This series adds support for regulator supply.

Changes since V3:
- Removed extra Optional properties header.
- Moved regulator get before platform_set_drvdata().
- Updated commit message for Patch 3/3.
- Added new dt properties for min and max voltages if default voltage needs to be changed.

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 |  6 +++
 drivers/rtc/Kconfig                                |  6 +--
 drivers/rtc/rtc-omap.c                             | 47 ++++++++++++++++++++--
 3 files changed, 52 insertions(+), 7 deletions(-)

-- 
1.9.1

[PATCH V4 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.

Reviewed-by: Johan Hovold <johan@kernel.org>
Reviewed-by: Felipe Balbi <balbi@ti.com>
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 V4 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 V4 3/3] rtc: omap: Support regulator supply for RTC

[Lokesh Vutla]

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.
So make the OMAP RTC driver support a power regulator.

Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
- Dropped the Reviewed-by tags as this patch is changed from previous version.
 Documentation/devicetree/bindings/rtc/rtc-omap.txt |  6 ++++
 drivers/rtc/rtc-omap.c                             | 41 +++++++++++++++++++++-
 2 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
index 750efd4..c1d84ac 100644
--- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
+++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
@@ -15,6 +15,9 @@ 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.
+- vrtc-minuV: Minimum required voltage in uV, If default voltage needs to be changed
+- vrtc-maxuV: Maximum acceptable voltage in uV, If default voltage needs to be changed
 
 Example:
 
@@ -25,4 +28,7 @@ rtc@1c23000 {
 		      19>;
 	interrupt-parent = <&intc>;
 	ti,system-power-controller;
+	vrtc-supply = <&ldo9_reg>;
+	vrtc-minuV = <1050000>;
+	vrtc-maxuV = <1050000>;
 };
diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
index d9bb5e7..7f1358a 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)
@@ -484,7 +486,7 @@ static int omap_rtc_probe(struct platform_device *pdev)
 	u8 reg, mask, new_ctrl;
 	const struct platform_device_id *id_entry;
 	const struct of_device_id *of_id;
-	int ret;
+	int ret, vrtc_minuV = 0, vrtc_maxuV = 0;
 
 	rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
 	if (!rtc)
@@ -514,6 +516,37 @@ 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) {
+		of_property_read_u32(pdev->dev.of_node, "vrtc-minuV",
+				     &vrtc_minuV);
+		of_property_read_u32(pdev->dev.of_node, "vrtc-maxuV",
+				     &vrtc_maxuV);
+		if (vrtc_minuV && vrtc_maxuV) {
+			ret = regulator_set_voltage(rtc->supply,
+						    vrtc_minuV, vrtc_maxuV);
+			if (ret) {
+				dev_err(&pdev->dev, "failed to set volt %d\n",
+					ret);
+				return ret;
+			}
+		}
+
+		ret = regulator_enable(rtc->supply);
+		if (ret) {
+			dev_err(&pdev->dev, "regulator enable failed %d\n",
+				ret);
+			return ret;
+		}
+	}
+
 	platform_set_drvdata(pdev, rtc);
 
 	/* Enable the clock/module so that we can access the registers */
@@ -624,6 +657,9 @@ err:
 	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
+	if (rtc->supply)
+		regulator_disable(rtc->supply);
+
 	return ret;
 }
 
@@ -649,6 +685,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 V4 3/3] rtc: omap: Support regulator supply for RTC

[Nishanth Menon]

On 10/28/2014 04:58 AM, Lokesh Vutla wrote:
> 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.
> So make the OMAP RTC driver support a power regulator.
> 
> Signed-off-by: Lokesh Vutla <lokeshvutla-l0cyMroinI0@public.gmane.org>
> ---
> - Dropped the Reviewed-by tags as this patch is changed from previous version.
>  Documentation/devicetree/bindings/rtc/rtc-omap.txt |  6 ++++
>  drivers/rtc/rtc-omap.c                             | 41 +++++++++++++++++++++-
>  2 files changed, 46 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> index 750efd4..c1d84ac 100644
> --- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> +++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> @@ -15,6 +15,9 @@ 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.
> +- vrtc-minuV: Minimum required voltage in uV, If default voltage needs to be changed
> +- vrtc-maxuV: Maximum acceptable voltage in uV, If default voltage needs to be changed
>  
>  Example:
>  
> @@ -25,4 +28,7 @@ rtc@1c23000 {
>  		      19>;
>  	interrupt-parent = <&intc>;
>  	ti,system-power-controller;
> +	vrtc-supply = <&ldo9_reg>;
> +	vrtc-minuV = <1050000>;
> +	vrtc-maxuV = <1050000>;


why would we want to duplicate machine constraints that regulators
already have? if there is a constant voltage needed, then it should be
compatible property as it is not really a configuration option, right?

[...]
-- 
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Felipe Balbi]

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

On Tue, Oct 28, 2014 at 03:28:52PM +0530, Lokesh Vutla wrote:
> 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.
> So make the OMAP RTC driver support a power regulator.
> 
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> ---
> - Dropped the Reviewed-by tags as this patch is changed from previous version.
>  Documentation/devicetree/bindings/rtc/rtc-omap.txt |  6 ++++
>  drivers/rtc/rtc-omap.c                             | 41 +++++++++++++++++++++-
>  2 files changed, 46 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> index 750efd4..c1d84ac 100644
> --- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> +++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> @@ -15,6 +15,9 @@ 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.
> +- vrtc-minuV: Minimum required voltage in uV, If default voltage needs to be changed
> +- vrtc-maxuV: Maximum acceptable voltage in uV, If default voltage needs to be changed

huh ? minuV and maxuV is already part of the regulator binding itself.

> @@ -514,6 +516,37 @@ 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");

I'm not sure if this is optional either, it's just that many of our
current DTS don't really pass a regulator to RTC, right ?

> +	if (IS_ERR(rtc->supply)) {
> +		if (PTR_ERR(rtc->supply) == -EPROBE_DEFER)
> +			return -EPROBE_DEFER;
> +
> +		rtc->supply = NULL;
> +	}
> +
> +	if (rtc->supply) {
> +		of_property_read_u32(pdev->dev.of_node, "vrtc-minuV",
> +				     &vrtc_minuV);
> +		of_property_read_u32(pdev->dev.of_node, "vrtc-maxuV",
> +				     &vrtc_maxuV);
> +		if (vrtc_minuV && vrtc_maxuV) {
> +			ret = regulator_set_voltage(rtc->supply,
> +						    vrtc_minuV, vrtc_maxuV);
> +			if (ret) {
> +				dev_err(&pdev->dev, "failed to set volt %d\n",
> +					ret);
> +				return ret;
> +			}
> +		}

I'd really like to Mark's comments here but I was under the impression
that if the binding already gives min_microvolt == max_microvolt then
driver shouldn't really care about a set_voltage. Mark ?

> +
> +		ret = regulator_enable(rtc->supply);
> +		if (ret) {
> +			dev_err(&pdev->dev, "regulator enable failed %d\n",
> +				ret);
> +			return ret;
> +		}
> +	}
> +
>  	platform_set_drvdata(pdev, rtc);
>  
>  	/* Enable the clock/module so that we can access the registers */
> @@ -624,6 +657,9 @@ err:
>  	pm_runtime_put_sync(&pdev->dev);
>  	pm_runtime_disable(&pdev->dev);
>  
> +	if (rtc->supply)
> +		regulator_disable(rtc->supply);
> +
>  	return ret;
>  }
>  
> @@ -649,6 +685,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 V4 3/3] rtc: omap: Support regulator supply for RTC

[Mark Brown]

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

On Tue, Oct 28, 2014 at 10:01:48AM -0500, Felipe Balbi wrote:
> On Tue, Oct 28, 2014 at 03:28:52PM +0530, Lokesh Vutla wrote:

> > +- vrtc-minuV: Minimum required voltage in uV, If default voltage needs to be changed
> > +- vrtc-maxuV: Maximum acceptable voltage in uV, If default voltage needs to be changed

> huh ? minuV and maxuV is already part of the regulator binding itself.

No, they aren't - there's bindings for setting constraints but not
bindings for setting values in consumers since consumers generally
understand why they are setting a voltage if they do so.  I'd expect
that we'd have a property for whatever system feature might cause us to
need to explicitly set the voltage if we need to vary the voltage at
runtime, or alternatively for systems to set a voltage through the
constraints on the supply rather than having properties on the consumer
- why are they here?

If for some reason we really need these properties they should be -max-uV
and -min-uV.

> > @@ -514,6 +516,37 @@ 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");

> I'm not sure if this is optional either, it's just that many of our
> current DTS don't really pass a regulator to RTC, right ?

If the RTC can run without power that would certainly be most
impressive.  I can't see any reason for this driver to use anything
other than a standard regulator_get().

> > +		if (vrtc_minuV && vrtc_maxuV) {
> > +			ret = regulator_set_voltage(rtc->supply,
> > +						    vrtc_minuV, vrtc_maxuV);
> > +			if (ret) {
> > +				dev_err(&pdev->dev, "failed to set volt %d\n",
> > +					ret);
> > +				return ret;
> > +			}
> > +		}

> I'd really like to Mark's comments here but I was under the impression
> that if the binding already gives min_microvolt == max_microvolt then
> driver shouldn't really care about a set_voltage. Mark ?

That's what happens for the standard properties on the supplying
regulator, these properties are separate to that.  Like I say I'm not
sure I understand why this is being done on a per-consumer basis.

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