| Message ID | 1461796470-1291527-13-git-send-email-arnd@arndb.de |
|---|---|
| State | Superseded |
| Headers | show |
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
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
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)
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(-)