On 02/04/2019 10:32:32-0700, Tony Lindgren wrote:
> * Keerthy <j-keerthy@xxxxxx> [190402 03:43]:
> > Add an interface function to set up the rtc for a power_off
> > mode.
> >
> > Signed-off-by: Keerthy <j-keerthy@xxxxxx>
> > ---
> > drivers/rtc/interface.c | 12 ++++++++++++
> > drivers/rtc/rtc-omap.c | 1 +
> > include/linux/rtc.h | 2 ++
> > 3 files changed, 15 insertions(+)
> >
> > diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
> > index ccb7d6b4da3b..4846ec897067 100644
> > --- a/drivers/rtc/interface.c
> > +++ b/drivers/rtc/interface.c
> > @@ -1070,3 +1070,15 @@ int rtc_set_offset(struct rtc_device *rtc, long offset)
> > trace_rtc_set_offset(offset, ret);
> > return ret;
> > }
> > +
> > +/**
> > + * rtc_power_off_program - Some of the rtc are hooked on to PMIC_EN
> > + * line and can be used to power off the SoC.
> > + *
> > + * Kernel interface to program rtc to power off
> > + */
> > +int rtc_power_off_program(struct rtc_device *rtc)
> > +{
> > + return rtc->ops->power_off_program(rtc->dev.parent);
> > +}
> > +EXPORT_SYMBOL_GPL(rtc_power_off_program);
>
> This whole series looks OK to me. Not sure about the naming for
> rtc_power_off_program(), would rtc_power_off_device() be more
> generic?
>
Well, even if this is more generic, this feels way too ad-hoc to me.
The series is adding a function and a callback to the core for, from
what I understand, only one particular board.
You may as well simply export the function directly from the rtc-omap
driver as anyway, this is added as a dependency in the last patch.
Something else I would be open to but I'm not completely sure this fits
your use case is a new interface that would take an alarm index as a
parameter to allow setting any alarm on the RTC. This would at least be
usable for multiple other drivers.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
