linux-sh.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] rtc: rtc-rs5c313: Fix and cleanups
@ 2020-08-14 11:07 Geert Uytterhoeven
  2020-08-14 11:07 ` [PATCH 1/3] rtc: rtc-rs5c313: Drop obsolete platform_set_drvdata() call Geert Uytterhoeven
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Geert Uytterhoeven @ 2020-08-14 11:07 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni
  Cc: Nobuhiro Iwamatsu, kogiidena, Adrian Glaubitz, linux-rtc,
	linux-sh, Geert Uytterhoeven

	Hi Ale{ss,x}andr[oe],

This patch series fixes the RS5C313 RTC driver, which is used on the I-O
DATA USL-5P aka Landisk:

    -rs5c313 rs5c313: rs5c313_rtc_read_time: timeout error
     rs5c313 rs5c313: registered as rtc0
    -rs5c313 rs5c313: rs5c313_rtc_read_time: timeout error
    -rs5c313 rs5c313: hctosys: unable to read the hardware clock
    +rs5c313 rs5c313: setting system clock to 2020-08-14T01:04:12 UTC (1597367052)

and performs a few related code cleanups.

Thanks for your comments!

Geert Uytterhoeven (3):
  rtc: rtc-rs5c313: Drop obsolete platform_set_drvdata() call
  rtc: rtc-rs5c313: Fix late hardware init
  rtc: rtc-rs5c313: Convert to module_platform_driver()

 drivers/rtc/rtc-rs5c313.c | 34 +++++++---------------------------
 1 file changed, 7 insertions(+), 27 deletions(-)

-- 
2.17.1

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/3] rtc: rtc-rs5c313: Drop obsolete platform_set_drvdata() call
  2020-08-14 11:07 [PATCH 0/3] rtc: rtc-rs5c313: Fix and cleanups Geert Uytterhoeven
@ 2020-08-14 11:07 ` Geert Uytterhoeven
  2020-08-14 11:07 ` [PATCH 2/3] rtc: rtc-rs5c313: Fix late hardware init Geert Uytterhoeven
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Geert Uytterhoeven @ 2020-08-14 11:07 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni
  Cc: Nobuhiro Iwamatsu, kogiidena, Adrian Glaubitz, linux-rtc,
	linux-sh, Geert Uytterhoeven

Commit 284e2fa1da00a998 ("rtc: rtc-rs5c313: use
devm_rtc_device_register()"), removed the last user of the
driver-specific data.  Hence there is no longer a need to set it.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/rtc/rtc-rs5c313.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/rtc/rtc-rs5c313.c b/drivers/rtc/rtc-rs5c313.c
index 89f38e3e917d3d11..00b5ef753935ec8a 100644
--- a/drivers/rtc/rtc-rs5c313.c
+++ b/drivers/rtc/rtc-rs5c313.c
@@ -369,12 +369,7 @@ static int rs5c313_rtc_probe(struct platform_device *pdev)
 	struct rtc_device *rtc = devm_rtc_device_register(&pdev->dev, "rs5c313",
 				&rs5c313_rtc_ops, THIS_MODULE);
 
-	if (IS_ERR(rtc))
-		return PTR_ERR(rtc);
-
-	platform_set_drvdata(pdev, rtc);
-
-	return 0;
+	return PTR_ERR_OR_ZERO(rtc);
 }
 
 static struct platform_driver rs5c313_rtc_platform_driver = {
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/3] rtc: rtc-rs5c313: Fix late hardware init
  2020-08-14 11:07 [PATCH 0/3] rtc: rtc-rs5c313: Fix and cleanups Geert Uytterhoeven
  2020-08-14 11:07 ` [PATCH 1/3] rtc: rtc-rs5c313: Drop obsolete platform_set_drvdata() call Geert Uytterhoeven
@ 2020-08-14 11:07 ` Geert Uytterhoeven
  2020-08-14 11:07 ` [PATCH 3/3] rtc: rtc-rs5c313: Convert to module_platform_driver() Geert Uytterhoeven
  2020-08-20 22:14 ` [PATCH 0/3] rtc: rtc-rs5c313: Fix and cleanups Alexandre Belloni
  3 siblings, 0 replies; 5+ messages in thread
From: Geert Uytterhoeven @ 2020-08-14 11:07 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni
  Cc: Nobuhiro Iwamatsu, kogiidena, Adrian Glaubitz, linux-rtc,
	linux-sh, Geert Uytterhoeven

rs5c313_rtc_init() calls platform_driver_register(), and initializes the
hardware.  This is wrong because of two reasons:
  1. As soon as the driver has been registered, the device may be
     probed.  If devm_rtc_device_register() is called before hardware
     initialization, reading the current time will fail:

	rs5c313 rs5c313: rs5c313_rtc_read_time: timeout error
	rs5c313 rs5c313: registered as rtc0
	rs5c313 rs5c313: rs5c313_rtc_read_time: timeout error
	rs5c313 rs5c313: hctosys: unable to read the hardware clock

  2. If the platform device does not exist, the driver will still write
     to a hardware device that may not be present.

Fix this by moving the hardware initialization sequence to the driver's
.probe() method.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/rtc/rtc-rs5c313.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/rtc/rtc-rs5c313.c b/drivers/rtc/rtc-rs5c313.c
index 00b5ef753935ec8a..af72e428b218173e 100644
--- a/drivers/rtc/rtc-rs5c313.c
+++ b/drivers/rtc/rtc-rs5c313.c
@@ -366,8 +366,13 @@ static const struct rtc_class_ops rs5c313_rtc_ops = {
 
 static int rs5c313_rtc_probe(struct platform_device *pdev)
 {
-	struct rtc_device *rtc = devm_rtc_device_register(&pdev->dev, "rs5c313",
-				&rs5c313_rtc_ops, THIS_MODULE);
+	struct rtc_device *rtc;
+
+	rs5c313_init_port();
+	rs5c313_check_xstp_bit();
+
+	rtc = devm_rtc_device_register(&pdev->dev, "rs5c313", &rs5c313_rtc_ops,
+				       THIS_MODULE);
 
 	return PTR_ERR_OR_ZERO(rtc);
 }
@@ -381,16 +386,7 @@ static struct platform_driver rs5c313_rtc_platform_driver = {
 
 static int __init rs5c313_rtc_init(void)
 {
-	int err;
-
-	err = platform_driver_register(&rs5c313_rtc_platform_driver);
-	if (err)
-		return err;
-
-	rs5c313_init_port();
-	rs5c313_check_xstp_bit();
-
-	return 0;
+	return platform_driver_register(&rs5c313_rtc_platform_driver);
 }
 
 static void __exit rs5c313_rtc_exit(void)
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 3/3] rtc: rtc-rs5c313: Convert to module_platform_driver()
  2020-08-14 11:07 [PATCH 0/3] rtc: rtc-rs5c313: Fix and cleanups Geert Uytterhoeven
  2020-08-14 11:07 ` [PATCH 1/3] rtc: rtc-rs5c313: Drop obsolete platform_set_drvdata() call Geert Uytterhoeven
  2020-08-14 11:07 ` [PATCH 2/3] rtc: rtc-rs5c313: Fix late hardware init Geert Uytterhoeven
@ 2020-08-14 11:07 ` Geert Uytterhoeven
  2020-08-20 22:14 ` [PATCH 0/3] rtc: rtc-rs5c313: Fix and cleanups Alexandre Belloni
  3 siblings, 0 replies; 5+ messages in thread
From: Geert Uytterhoeven @ 2020-08-14 11:07 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni
  Cc: Nobuhiro Iwamatsu, kogiidena, Adrian Glaubitz, linux-rtc,
	linux-sh, Geert Uytterhoeven

Reduce boilerplate by using the module_platform_driver() helper.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/rtc/rtc-rs5c313.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/rtc/rtc-rs5c313.c b/drivers/rtc/rtc-rs5c313.c
index af72e428b218173e..e98f85f34206f16c 100644
--- a/drivers/rtc/rtc-rs5c313.c
+++ b/drivers/rtc/rtc-rs5c313.c
@@ -384,18 +384,7 @@ static struct platform_driver rs5c313_rtc_platform_driver = {
 	.probe	= rs5c313_rtc_probe,
 };
 
-static int __init rs5c313_rtc_init(void)
-{
-	return platform_driver_register(&rs5c313_rtc_platform_driver);
-}
-
-static void __exit rs5c313_rtc_exit(void)
-{
-	platform_driver_unregister(&rs5c313_rtc_platform_driver);
-}
-
-module_init(rs5c313_rtc_init);
-module_exit(rs5c313_rtc_exit);
+module_platform_driver(rs5c313_rtc_platform_driver);
 
 MODULE_AUTHOR("kogiidena , Nobuhiro Iwamatsu <iwamatsu@nigauri.org>");
 MODULE_DESCRIPTION("Ricoh RS5C313 RTC device driver");
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/3] rtc: rtc-rs5c313: Fix and cleanups
  2020-08-14 11:07 [PATCH 0/3] rtc: rtc-rs5c313: Fix and cleanups Geert Uytterhoeven
                   ` (2 preceding siblings ...)
  2020-08-14 11:07 ` [PATCH 3/3] rtc: rtc-rs5c313: Convert to module_platform_driver() Geert Uytterhoeven
@ 2020-08-20 22:14 ` Alexandre Belloni
  3 siblings, 0 replies; 5+ messages in thread
From: Alexandre Belloni @ 2020-08-20 22:14 UTC (permalink / raw)
  To: Alessandro Zummo, Geert Uytterhoeven
  Cc: Alexandre Belloni, Nobuhiro Iwamatsu, linux-rtc, Adrian Glaubitz,
	linux-sh, kogiidena

On Fri, 14 Aug 2020 13:07:28 +0200, Geert Uytterhoeven wrote:
> 	Hi Ale{ss,x}andr[oe],
> 
> This patch series fixes the RS5C313 RTC driver, which is used on the I-O
> DATA USL-5P aka Landisk:
> 
>     -rs5c313 rs5c313: rs5c313_rtc_read_time: timeout error
>      rs5c313 rs5c313: registered as rtc0
>     -rs5c313 rs5c313: rs5c313_rtc_read_time: timeout error
>     -rs5c313 rs5c313: hctosys: unable to read the hardware clock
>     +rs5c313 rs5c313: setting system clock to 2020-08-14T01:04:12 UTC (1597367052)
> 
> [...]

Applied, thanks!

[1/3] rtc: rtc-rs5c313: Drop obsolete platform_set_drvdata() call
      commit: fc9656a370499e5a32425b715f8fed241e832458
[2/3] rtc: rtc-rs5c313: Fix late hardware init
      commit: f65e727464d7c0090f05548e8f323779eaa97eda
[3/3] rtc: rtc-rs5c313: Convert to module_platform_driver()
      commit: 163a512cd929d6db712a3021720362749653998b

Best regards,
-- 
Alexandre Belloni <alexandre.belloni@bootlin.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-08-20 22:14 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-14 11:07 [PATCH 0/3] rtc: rtc-rs5c313: Fix and cleanups Geert Uytterhoeven
2020-08-14 11:07 ` [PATCH 1/3] rtc: rtc-rs5c313: Drop obsolete platform_set_drvdata() call Geert Uytterhoeven
2020-08-14 11:07 ` [PATCH 2/3] rtc: rtc-rs5c313: Fix late hardware init Geert Uytterhoeven
2020-08-14 11:07 ` [PATCH 3/3] rtc: rtc-rs5c313: Convert to module_platform_driver() Geert Uytterhoeven
2020-08-20 22:14 ` [PATCH 0/3] rtc: rtc-rs5c313: Fix and cleanups Alexandre Belloni

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).