Re: [PATCH v3 12/16] rtc: powerpc: provide rtc_class_ops directly
On Tuesday 03 May 2016 14:05:09 Michael Ellerman wrote:
> On Thu, 2016-04-28 at 00:34 +0200, Arnd Bergmann wrote:
>
> > The rtc-generic driver provides an architecture specific
> > wrapper on top of the generic rtc_class_ops abstraction,
> > and powerpc has another abstraction on top, which is a bit
> > silly.
> >
> > This changes the powerpc rtc-generic device to provide its
> > rtc_class_ops directly, to reduce the number of layers
> > by one.
> >
> > Signed-off-by: Arnd Bergmann
> > ---
> > arch/powerpc/kernel/time.c | 29 -
> > drivers/rtc/rtc-generic.c | 2 +-
> > 2 files changed, 29 insertions(+), 2 deletions(-)
>
> If this hits linux-next it will go through my automated boot testing, which
> hopefully would be sufficient to catch any bugs in this patch, cross fingers.
>
> I don't know jack about all the layers of RTC mess, so my ack is basically
> worthless here. But if you like you can have one anyway
>
> Acked-by: Michael Ellerman
Thanks!
The main thing that could use testing here is for patch 12/16 to see if
/sbin/hwclock can still read and write the time on a machine that
uses one of the genrtc backends on powerpc, this includes
rtas_get_rtc_time, mpc8xx_get_rtc_time, maple_get_rtc_time,
ps3_get_rtc_time and pmac_get_rtc_time. Testing on a pSeries with rtas
should be sufficient, and if I made a mistake, it probably fails
spectacularly.
Just for reference, if anyone ever wants to clean this up further on
powerpc to remove all of the rtc handling from architecture code,
it has gotten easier after my series:
- The update_persistent_clock() and read_persistent_clock() callbacks
are now unnecessary: as long as the RTC driver is built into the
kernel, drivers/rtc/hctosys.c takes care of setting the initial
time (otherwise user space has to do it), and the other users
(ntp and suspend/resume) will work fine whenever an rtc driver
is loaded. Obviously you will want to test the kernel better after
removing the two functions.
- Once they are gone, the only users of the ppc_md.{get,set}_rtc_time
callbacks are in the "const struct rtc_class_ops rtc_generic_ops".
You can move them into the five files implementing those callbacks
and call the functions directly to get rid of the function pointers.
- Lastly, after that is done, you basically have five independent
rtc device drivers that can get moved to drivers/rtc and converted
into regular platform drivers. Instead of registering the fake
"rtc-generic" device, you then register the one that is actually
there like arch/powerpc/sysdev/rtc_cmos_setup.c already does, or
use the one that gets created from DT.
Arnd
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v3 12/16] rtc: powerpc: provide rtc_class_ops directly
On Thu, 2016-04-28 at 00:34 +0200, Arnd Bergmann wrote: > The rtc-generic driver provides an architecture specific > wrapper on top of the generic rtc_class_ops abstraction, > and powerpc has another abstraction on top, which is a bit > silly. > > This changes the powerpc rtc-generic device to provide its > rtc_class_ops directly, to reduce the number of layers > by one. > > Signed-off-by: Arnd Bergmann > --- > arch/powerpc/kernel/time.c | 29 - > drivers/rtc/rtc-generic.c | 2 +- > 2 files changed, 29 insertions(+), 2 deletions(-) If this hits linux-next it will go through my automated boot testing, which hopefully would be sufficient to catch any bugs in this patch, cross fingers. I don't know jack about all the layers of RTC mess, so my ack is basically worthless here. But if you like you can have one anyway :) Acked-by: Michael Ellerman cheers ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v3 12/16] rtc: powerpc: provide rtc_class_ops directly
The rtc-generic driver provides an architecture specific
wrapper on top of the generic rtc_class_ops abstraction,
and powerpc has another abstraction on top, which is a bit
silly.
This changes the powerpc rtc-generic device to provide its
rtc_class_ops directly, to reduce the number of layers
by one.
Signed-off-by: Arnd Bergmann
---
arch/powerpc/kernel/time.c | 29 -
drivers/rtc/rtc-generic.c | 2 +-
2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 3ed9a5a21d77..7a482a7f4d8d 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -56,6 +56,7 @@
#include
#include
#include
+#include
#include
#include
@@ -1081,6 +1082,29 @@ void calibrate_delay(void)
loops_per_jiffy = tb_ticks_per_jiffy;
}
+#if IS_ENABLED(CONFIG_RTC_DRV_GENERIC)
+static int rtc_generic_get_time(struct device *dev, struct rtc_time *tm)
+{
+ ppc_md.get_rtc_time(tm);
+ return rtc_valid_tm(tm);
+}
+
+static int rtc_generic_set_time(struct device *dev, struct rtc_time *tm)
+{
+ if (!ppc_md.set_rtc_time)
+ return -EOPNOTSUPP;
+
+ if (ppc_md.set_rtc_time(tm) < 0)
+ return -EOPNOTSUPP;
+
+ return 0;
+}
+
+static const struct rtc_class_ops rtc_generic_ops = {
+ .read_time = rtc_generic_get_time,
+ .set_time = rtc_generic_set_time,
+};
+
static int __init rtc_init(void)
{
struct platform_device *pdev;
@@ -1088,9 +1112,12 @@ static int __init rtc_init(void)
if (!ppc_md.get_rtc_time)
return -ENODEV;
- pdev = platform_device_register_simple("rtc-generic", -1, NULL, 0);
+ pdev = platform_device_register_data(NULL, "rtc-generic", -1,
+&rtc_generic_ops,
+sizeof(rtc_generic_ops));
return PTR_ERR_OR_ZERO(pdev);
}
device_initcall(rtc_init);
+#endif
diff --git a/drivers/rtc/rtc-generic.c b/drivers/rtc/rtc-generic.c
index 5c82bae73b9c..efcb9833cac8 100644
--- a/drivers/rtc/rtc-generic.c
+++ b/drivers/rtc/rtc-generic.c
@@ -9,7 +9,7 @@
#include
#include
-#if defined(CONFIG_PPC)
+#if 0
#include
static int generic_get_time(struct device *dev, struct rtc_time *tm)
--
2.7.0
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
