[PATCH 0/2] m68k: bvme6000: rtc: convert to rtc class

[PATCH 0/2] m68k: bvme6000: rtc: convert to rtc class

[Thadeu Lima de Souza Cascardo]

This converts the DP8570A RTC driver to the RTC class, instead of directly
implementing the /dev/rtc RTC_RD_TIME and RTC_SET_TIME.

This will get rid of the last RTC_MINOR user in the kernel tree.

This has only been built-tested.

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>
---
Thadeu Lima de Souza Cascardo (2):
      m68k: rtc: dp8570a: split read_time and set_time
      m68k: rtc: dp8570a: make it a proper RTC class driver

 arch/m68k/bvme6000/rtc.c | 207 +++++++++++++++++++----------------------------
 1 file changed, 82 insertions(+), 125 deletions(-)
---
base-commit: d082ecbc71e9e0bf49883ee4afd435a77a5101b6
change-id: 20250225-m68k-rtc-c4635a8e0160

Best regards,
-- 
Thadeu Lima de Souza Cascardo <cascardo@igalia.com>

[PATCH 1/2] m68k: rtc: dp8570a: split read_time and set_time

[Thadeu Lima de Souza Cascardo]

Give each ioctl cmd its own function. There should be no other functional
changes.

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>
---
 arch/m68k/bvme6000/rtc.c | 148 ++++++++++++++++++++++++++---------------------
 1 file changed, 83 insertions(+), 65 deletions(-)

diff --git a/arch/m68k/bvme6000/rtc.c b/arch/m68k/bvme6000/rtc.c
index b43eeef0982c117fdb39f63a6feab59e18e99502..a84996bd1491da3c1d9bd8dae7e1a92805c735e0 100644
--- a/arch/m68k/bvme6000/rtc.c
+++ b/arch/m68k/bvme6000/rtc.c
@@ -36,43 +36,103 @@ static unsigned char days_in_mo[] =
 
 static atomic_t rtc_status = ATOMIC_INIT(1);
 
-static long rtc_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+static int dp8570a_rtc_read_time(struct rtc_time *wtime)
+{
+	volatile RtcPtr_t rtc = (RtcPtr_t)BVME_RTC_BASE;
+	unsigned char msr;
+	unsigned long flags;
+
+	local_irq_save(flags);
+	/* Ensure clock and real-time-mode-register are accessible */
+	msr = rtc->msr & 0xc0;
+	rtc->msr = 0x40;
+	memset(wtime, 0, sizeof(struct rtc_time));
+	do {
+		wtime->tm_sec =  bcd2bin(rtc->bcd_sec);
+		wtime->tm_min =  bcd2bin(rtc->bcd_min);
+		wtime->tm_hour = bcd2bin(rtc->bcd_hr);
+		wtime->tm_mday =  bcd2bin(rtc->bcd_dom);
+		wtime->tm_mon =  bcd2bin(rtc->bcd_mth)-1;
+		wtime->tm_year = bcd2bin(rtc->bcd_year);
+		if (wtime->tm_year < 70)
+			wtime->tm_year += 100;
+		wtime->tm_wday = bcd2bin(rtc->bcd_dow)-1;
+	} while (wtime->tm_sec != bcd2bin(rtc->bcd_sec));
+	rtc->msr = msr;
+	local_irq_restore(flags);
+	return 0;
+}
+
+static int dp8570a_rtc_set_time(struct rtc_time *rtc_tm)
 {
 	volatile RtcPtr_t rtc = (RtcPtr_t)BVME_RTC_BASE;
 	unsigned char msr;
 	unsigned long flags;
-	struct rtc_time wtime;
+	unsigned char mon, day, hrs, min, sec, leap_yr;
+	unsigned int yrs;
+
+	yrs = rtc_tm->tm_year;
+	if (yrs < 1900)
+		yrs += 1900;
+	mon = rtc_tm->tm_mon + 1;   /* tm_mon starts at zero */
+	day = rtc_tm->tm_mday;
+	hrs = rtc_tm->tm_hour;
+	min = rtc_tm->tm_min;
+	sec = rtc_tm->tm_sec;
+
+	leap_yr = ((!(yrs % 4) && (yrs % 100)) || !(yrs % 400));
+
+	if ((mon > 12) || (mon < 1) || (day == 0))
+		return -EINVAL;
+
+	if (day > (days_in_mo[mon] + ((mon == 2) && leap_yr)))
+		return -EINVAL;
+
+	if ((hrs >= 24) || (min >= 60) || (sec >= 60))
+		return -EINVAL;
+
+	if (yrs >= 2070)
+		return -EINVAL;
+
+	local_irq_save(flags);
+	/* Ensure clock and real-time-mode-register are accessible */
+	msr = rtc->msr & 0xc0;
+	rtc->msr = 0x40;
+
+	rtc->t0cr_rtmr = yrs%4;
+	rtc->bcd_tenms = 0;
+	rtc->bcd_sec   = bin2bcd(sec);
+	rtc->bcd_min   = bin2bcd(min);
+	rtc->bcd_hr    = bin2bcd(hrs);
+	rtc->bcd_dom   = bin2bcd(day);
+	rtc->bcd_mth   = bin2bcd(mon);
+	rtc->bcd_year  = bin2bcd(yrs%100);
+	if (rtc_tm->tm_wday >= 0)
+		rtc->bcd_dow = bin2bcd(rtc_tm->tm_wday+1);
+	rtc->t0cr_rtmr = yrs%4 | 0x08;
+
+	rtc->msr = msr;
+	local_irq_restore(flags);
+
+	return 0;
+}
+
+static long rtc_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
 	void __user *argp = (void __user *)arg;
 
 	switch (cmd) {
 	case RTC_RD_TIME:	/* Read the time/date from RTC	*/
 	{
-		local_irq_save(flags);
-		/* Ensure clock and real-time-mode-register are accessible */
-		msr = rtc->msr & 0xc0;
-		rtc->msr = 0x40;
-		memset(&wtime, 0, sizeof(struct rtc_time));
-		do {
-			wtime.tm_sec =  bcd2bin(rtc->bcd_sec);
-			wtime.tm_min =  bcd2bin(rtc->bcd_min);
-			wtime.tm_hour = bcd2bin(rtc->bcd_hr);
-			wtime.tm_mday =  bcd2bin(rtc->bcd_dom);
-			wtime.tm_mon =  bcd2bin(rtc->bcd_mth)-1;
-			wtime.tm_year = bcd2bin(rtc->bcd_year);
-			if (wtime.tm_year < 70)
-				wtime.tm_year += 100;
-			wtime.tm_wday = bcd2bin(rtc->bcd_dow)-1;
-		} while (wtime.tm_sec != bcd2bin(rtc->bcd_sec));
-		rtc->msr = msr;
-		local_irq_restore(flags);
+		struct rtc_time wtime;
+
+		dp8570a_rtc_read_time(&wtime);
 		return copy_to_user(argp, &wtime, sizeof wtime) ?
 								-EFAULT : 0;
 	}
 	case RTC_SET_TIME:	/* Set the RTC */
 	{
 		struct rtc_time rtc_tm;
-		unsigned char mon, day, hrs, min, sec, leap_yr;
-		unsigned int yrs;
 
 		if (!capable(CAP_SYS_ADMIN))
 			return -EACCES;
@@ -80,49 +140,7 @@ static long rtc_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		if (copy_from_user(&rtc_tm, argp, sizeof(struct rtc_time)))
 			return -EFAULT;
 
-		yrs = rtc_tm.tm_year;
-		if (yrs < 1900)
-			yrs += 1900;
-		mon = rtc_tm.tm_mon + 1;   /* tm_mon starts at zero */
-		day = rtc_tm.tm_mday;
-		hrs = rtc_tm.tm_hour;
-		min = rtc_tm.tm_min;
-		sec = rtc_tm.tm_sec;
-
-		leap_yr = ((!(yrs % 4) && (yrs % 100)) || !(yrs % 400));
-
-		if ((mon > 12) || (mon < 1) || (day == 0))
-			return -EINVAL;
-
-		if (day > (days_in_mo[mon] + ((mon == 2) && leap_yr)))
-			return -EINVAL;
-
-		if ((hrs >= 24) || (min >= 60) || (sec >= 60))
-			return -EINVAL;
-
-		if (yrs >= 2070)
-			return -EINVAL;
-
-		local_irq_save(flags);
-		/* Ensure clock and real-time-mode-register are accessible */
-		msr = rtc->msr & 0xc0;
-		rtc->msr = 0x40;
-
-		rtc->t0cr_rtmr = yrs%4;
-		rtc->bcd_tenms = 0;
-		rtc->bcd_sec   = bin2bcd(sec);
-		rtc->bcd_min   = bin2bcd(min);
-		rtc->bcd_hr    = bin2bcd(hrs);
-		rtc->bcd_dom   = bin2bcd(day);
-		rtc->bcd_mth   = bin2bcd(mon);
-		rtc->bcd_year  = bin2bcd(yrs%100);
-		if (rtc_tm.tm_wday >= 0)
-			rtc->bcd_dow = bin2bcd(rtc_tm.tm_wday+1);
-		rtc->t0cr_rtmr = yrs%4 | 0x08;
-
-		rtc->msr = msr;
-		local_irq_restore(flags);
-		return 0;
+		return dp8570a_rtc_set_time(&rtc_tm);
 	}
 	default:
 		return -EINVAL;

-- 
2.47.2

[PATCH 2/2] m68k: rtc: dp8570a: make it a proper RTC class driver

[Thadeu Lima de Souza Cascardo]

In the past, each rtc implementation had to rewrite the same ioctls in
order to be compatible. But since 2006, a common RTC interface has been
introduced. Use it for the last user of RTC_MINOR.

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>
---
 arch/m68k/bvme6000/rtc.c | 91 ++++++++----------------------------------------
 1 file changed, 15 insertions(+), 76 deletions(-)

diff --git a/arch/m68k/bvme6000/rtc.c b/arch/m68k/bvme6000/rtc.c
index a84996bd1491da3c1d9bd8dae7e1a92805c735e0..7e7b40863e5eb3423c53b44c2d63c8806e7a1bbe 100644
--- a/arch/m68k/bvme6000/rtc.c
+++ b/arch/m68k/bvme6000/rtc.c
@@ -16,7 +16,8 @@
 #include <linux/init.h>
 #include <linux/poll.h>
 #include <linux/module.h>
-#include <linux/rtc.h>	/* For struct rtc_time and ioctls, etc */
+#include <linux/rtc.h>	/* For struct rtc_time and etc */
+#include <linux/platform_device.h>
 #include <linux/bcd.h>
 #include <asm/bvme6000hw.h>
 
@@ -24,19 +25,10 @@
 #include <linux/uaccess.h>
 #include <asm/setup.h>
 
-/*
- *	We sponge a minor off of the misc major. No need slurping
- *	up another valuable major dev number for this. If you add
- *	an ioctl, make sure you don't conflict with SPARC's RTC
- *	ioctls.
- */
-
 static unsigned char days_in_mo[] =
 {0, 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31};
 
-static atomic_t rtc_status = ATOMIC_INIT(1);
-
-static int dp8570a_rtc_read_time(struct rtc_time *wtime)
+static int dp8570a_rtc_read_time(struct device *dev, struct rtc_time *wtime)
 {
 	volatile RtcPtr_t rtc = (RtcPtr_t)BVME_RTC_BASE;
 	unsigned char msr;
@@ -63,7 +55,7 @@ static int dp8570a_rtc_read_time(struct rtc_time *wtime)
 	return 0;
 }
 
-static int dp8570a_rtc_set_time(struct rtc_time *rtc_tm)
+static int dp8570a_rtc_set_time(struct device *dev, struct rtc_time *rtc_tm)
 {
 	volatile RtcPtr_t rtc = (RtcPtr_t)BVME_RTC_BASE;
 	unsigned char msr;
@@ -117,77 +109,24 @@ static int dp8570a_rtc_set_time(struct rtc_time *rtc_tm)
 	return 0;
 }
 
-static long rtc_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
-{
-	void __user *argp = (void __user *)arg;
-
-	switch (cmd) {
-	case RTC_RD_TIME:	/* Read the time/date from RTC	*/
-	{
-		struct rtc_time wtime;
-
-		dp8570a_rtc_read_time(&wtime);
-		return copy_to_user(argp, &wtime, sizeof wtime) ?
-								-EFAULT : 0;
-	}
-	case RTC_SET_TIME:	/* Set the RTC */
-	{
-		struct rtc_time rtc_tm;
-
-		if (!capable(CAP_SYS_ADMIN))
-			return -EACCES;
-
-		if (copy_from_user(&rtc_tm, argp, sizeof(struct rtc_time)))
-			return -EFAULT;
-
-		return dp8570a_rtc_set_time(&rtc_tm);
-	}
-	default:
-		return -EINVAL;
-	}
-}
-
-/*
- * We enforce only one user at a time here with the open/close.
- */
-static int rtc_open(struct inode *inode, struct file *file)
-{
-	if (!atomic_dec_and_test(&rtc_status)) {
-		atomic_inc(&rtc_status);
-		return -EBUSY;
-	}
-	return 0;
-}
-
-static int rtc_release(struct inode *inode, struct file *file)
-{
-	atomic_inc(&rtc_status);
-	return 0;
-}
-
-/*
- *	The various file operations we support.
- */
-
-static const struct file_operations rtc_fops = {
-	.unlocked_ioctl	= rtc_ioctl,
-	.open		= rtc_open,
-	.release	= rtc_release,
-	.llseek		= noop_llseek,
-};
-
-static struct miscdevice rtc_dev = {
-	.minor =	RTC_MINOR,
-	.name =		"rtc",
-	.fops =		&rtc_fops
+static const struct rtc_class_ops dp8570a_rtc_ops = {
+	.read_time	= dp8570a_rtc_read_time,
+	.set_time	= dp8570a_rtc_set_time,
 };
 
 static int __init rtc_DP8570A_init(void)
 {
+	struct platform_device *pdev;
+
 	if (!MACH_IS_BVME6000)
 		return -ENODEV;
 
 	pr_info("DP8570A Real Time Clock Driver v%s\n", RTC_VERSION);
-	return misc_register(&rtc_dev);
+
+	pdev = platform_device_register_data(NULL, "rtc-generic", -1,
+					     &dp8570a_rtc_ops,
+					     sizeof(dp8570a_rtc_ops));
+
+	return PTR_ERR_OR_ZERO(pdev);
 }
 module_init(rtc_DP8570A_init);

-- 
2.47.2

Re: [PATCH 2/2] m68k: rtc: dp8570a: make it a proper RTC class driver

[Geert Uytterhoeven]

Hi Thadeu,

On Wed, 26 Feb 2025 at 13:27, Thadeu Lima de Souza Cascardo
<cascardo@igalia.com> wrote:
> In the past, each rtc implementation had to rewrite the same ioctls in
> order to be compatible. But since 2006, a common RTC interface has been
> introduced. Use it for the last user of RTC_MINOR.
>
> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>

Thanks for your patch!

> --- a/arch/m68k/bvme6000/rtc.c
> +++ b/arch/m68k/bvme6000/rtc.c

>
>  static int __init rtc_DP8570A_init(void)
>  {
> +       struct platform_device *pdev;
> +
>         if (!MACH_IS_BVME6000)
>                 return -ENODEV;
>
>         pr_info("DP8570A Real Time Clock Driver v%s\n", RTC_VERSION);
> -       return misc_register(&rtc_dev);
> +
> +       pdev = platform_device_register_data(NULL, "rtc-generic", -1,
> +                                            &dp8570a_rtc_ops,
> +                                            sizeof(dp8570a_rtc_ops));

Doesn't this conflict with the creation of the same device in rtc_init()[1]?

On BVME6000, mach_hwclk is set:

    arch/m68k/bvme6000/config.c:    mach_hwclk           = bvme6000_hwclk;

> +
> +       return PTR_ERR_OR_ZERO(pdev);
>  }
>  module_init(rtc_DP8570A_init);

[1] https://elixir.bootlin.com/linux/v6.13.4/source/arch/m68k/kernel/time.c#L144

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/2] m68k: rtc: dp8570a: make it a proper RTC class driver

[Thadeu Lima de Souza Cascardo]

On Thu, Feb 27, 2025 at 12:05:26PM +0100, Geert Uytterhoeven wrote:
> Hi Thadeu,
> 
> On Wed, 26 Feb 2025 at 13:27, Thadeu Lima de Souza Cascardo
> <cascardo@igalia.com> wrote:
> > In the past, each rtc implementation had to rewrite the same ioctls in
> > order to be compatible. But since 2006, a common RTC interface has been
> > introduced. Use it for the last user of RTC_MINOR.
> >
> > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>
> 
> Thanks for your patch!
> 
> > --- a/arch/m68k/bvme6000/rtc.c
> > +++ b/arch/m68k/bvme6000/rtc.c
> 
> >
> >  static int __init rtc_DP8570A_init(void)
> >  {
> > +       struct platform_device *pdev;
> > +
> >         if (!MACH_IS_BVME6000)
> >                 return -ENODEV;
> >
> >         pr_info("DP8570A Real Time Clock Driver v%s\n", RTC_VERSION);
> > -       return misc_register(&rtc_dev);
> > +
> > +       pdev = platform_device_register_data(NULL, "rtc-generic", -1,
> > +                                            &dp8570a_rtc_ops,
> > +                                            sizeof(dp8570a_rtc_ops));
> 
> Doesn't this conflict with the creation of the same device in rtc_init()[1]?
> 

I didn't look too deep and just assumed that this would be one of those
cases where this was set for machines other than BVME6000.

Ah, right, that is because of this check around it.

#if defined(CONFIG_M68KCLASSIC) || defined(CONFIG_SUN3).

But CONFIG_M68KCLASSIC is set for BVME6000.

On the other hand, mach_hwclk is accessing the same registers as the rtc.c
code, so they are redundant. One thing that is missing is
local_irq_save/local_irq_restore around it. I can add that and remove
rtc.c, which would make more sense here.

Thanks.
Cascardo.

> On BVME6000, mach_hwclk is set:
> 
>     arch/m68k/bvme6000/config.c:    mach_hwclk           = bvme6000_hwclk;
> 
> > +
> > +       return PTR_ERR_OR_ZERO(pdev);
> >  }
> >  module_init(rtc_DP8570A_init);
> 
> [1] https://elixir.bootlin.com/linux/v6.13.4/source/arch/m68k/kernel/time.c#L144
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds