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

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:

quoted

+- 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.

quoted

@@ -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().

quoted

+		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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141028/33561daf/attachment.sig>