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

Conditional on Johan's comments:

Reviewed-by: Felipe Balbi <[email protected]>

-- 
balbi

Attachment: signature.asc
Description: Digital signature

Reply via email to