diff mbox

[v3,12/16] rtc: powerpc: provide rtc_class_ops directly

Message ID 1461796470-1291527-13-git-send-email-arnd@arndb.de (mailing list archive)
State Not Applicable
Headers show

Commit Message

Arnd Bergmann April 27, 2016, 10:34 p.m. UTC
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 <arnd@arndb.de>
---
 arch/powerpc/kernel/time.c | 29 ++++++++++++++++++++++++++++-
 drivers/rtc/rtc-generic.c  |  2 +-
 2 files changed, 29 insertions(+), 2 deletions(-)

Comments

Michael Ellerman May 3, 2016, 4:05 a.m. UTC | #1
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 <arnd@arndb.de>
> ---
>  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 <mpe@ellerman.id.au>

cheers

--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann May 3, 2016, 10:29 a.m. UTC | #2
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 <arnd@arndb.de>
> > ---
> >  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 <mpe@ellerman.id.au>

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
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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 <linux/irq_work.h>
 #include <linux/clk-provider.h>
 #include <linux/suspend.h>
+#include <linux/rtc.h>
 #include <asm/trace.h>
 
 #include <asm/io.h>
@@ -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 <linux/platform_device.h>
 #include <linux/rtc.h>
 
-#if defined(CONFIG_PPC)
+#if 0
 #include <asm/rtc.h>
 
 static int generic_get_time(struct device *dev, struct rtc_time *tm)