rtc-parisc cleanup

PATCH In review dann frazier 4 participants 16 messages Jan 15, 2009 – Jan 17, 2009

Subsystems (2)

Real time clock (RTC) PA-RISC architecture

lore

Share & tools

lore

dann frazier

author

2009-01-15 23:46 UTC #0 lore raw

cover rtc-parisc cleanup

I recently submitted an rtc-efi driver having used rtc-parisc as a template. I received several suggestions from Alessandro Zummo and Andrew Morton that also apply to rtc-parisc. This series merges the relevant changes back into the rtc-parisc driver.

dann frazier (9):
Add a missing include for linux/rtc.h
Remove redundant locking
remove struct parisc_rtc
Use rtc_valid_tm() in parisc_get_time
Use platform_driver_probe
declare rtc_parisc_dev as static
remove unnecessary ret variable
remove a couple unnecessary variables
rename p pointer to rtc

 arch/parisc/kernel/time.c |    7 +----
 drivers/rtc/rtc-parisc.c  |   56 ++++++++++++---------------------------------
 2 files changed, 17 insertions(+), 46 deletions(-)

dann frazier

author

2009-01-15 23:46 UTC #1 lore raw

PATCH 1/9 Add a missing include for linux/rtc.h

Signed-off-by dann frazier <[email protected]>

---
 drivers/rtc/rtc-parisc.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

drivers/rtc/rtc-parisc.c +1 -0

index c6bfa6f..319bb5d 100644
@@ -7,6 +7,7 @@
 #include <linux/module.h>
 #include <linux/time.h>
 #include <linux/platform_device.h>
+#include <linux/rtc.h>
 
 #include <asm/rtc.h>

Signature · 1 line

1.5.6.5

dann frazier

author

2009-01-15 23:46 UTC #2 lore raw

PATCH 2/9 Remove redundant locking

The RTC subsystem proides ops locking, no need to implement our own

Signed-off-by dann frazier <[email protected]>

---
 drivers/rtc/rtc-parisc.c |   12 +-----------
 1 files changed, 1 insertions(+), 11 deletions(-)

drivers/rtc/rtc-parisc.c +1 ~~-10~~

index 319bb5d..cb087ad 100644
@@ -14,17 +14,13 @@
 /* as simple as can be, and no simpler. */
 struct parisc_rtc {
 	struct rtc_device *rtc;
-	spinlock_t lock;
 };
 
 static int parisc_get_time(struct device *dev, struct rtc_time *tm)
 {
-	struct parisc_rtc *p = dev_get_drvdata(dev);
-	unsigned long flags, ret;
+	unsigned long ret;
 
-	spin_lock_irqsave(&p->lock, flags);
 	ret = get_rtc_time(tm);
-	spin_unlock_irqrestore(&p->lock, flags);
 
 	if (ret & RTC_BATT_BAD)
 		return -EOPNOTSUPP;

@@ -34,13 +30,9 @@ static int parisc_get_time(struct device *dev, struct rtc_time *tm)
 
 static int parisc_set_time(struct device *dev, struct rtc_time *tm)
 {
-	struct parisc_rtc *p = dev_get_drvdata(dev);
-	unsigned long flags;
 	int ret;
 
-	spin_lock_irqsave(&p->lock, flags);
 	ret = set_rtc_time(tm);
-	spin_unlock_irqrestore(&p->lock, flags);
 
 	if (ret < 0)
 		return -EOPNOTSUPP;

@@ -61,8 +53,6 @@ static int __devinit parisc_rtc_probe(struct platform_device *dev)
 	if (!p)
 		return -ENOMEM;
 
-	spin_lock_init(&p->lock);

dann frazier

author

2009-01-15 23:46 UTC #3 lore raw

PATCH 3/9 remove struct parisc_rtc

parisc_rtc now only includes an rtc_device pointer, so let's just use the rtc_device type directly.

Signed-off-by dann frazier <[email protected]>

---
 drivers/rtc/rtc-parisc.c |   19 +++++++------------
 1 files changed, 7 insertions(+), 12 deletions(-)

drivers/rtc/rtc-parisc.c +7 ~~-12~~

index cb087ad..ee4e9a3 100644
@@ -11,11 +11,6 @@
 
 #include <asm/rtc.h>
 
-/* as simple as can be, and no simpler. */
-struct parisc_rtc {
-	struct rtc_device *rtc;
-};
-
 static int parisc_get_time(struct device *dev, struct rtc_time *tm)
 {
 	unsigned long ret;

@@ -47,16 +42,16 @@ static const struct rtc_class_ops parisc_rtc_ops = {
 
 static int __devinit parisc_rtc_probe(struct platform_device *dev)
 {
-	struct parisc_rtc *p;
+	struct rtc_device *p;
 
 	p = kzalloc(sizeof (*p), GFP_KERNEL);
 	if (!p)
 		return -ENOMEM;
 
-	p->rtc = rtc_device_register("rtc-parisc", &dev->dev, &parisc_rtc_ops,
-					THIS_MODULE);
-	if (IS_ERR(p->rtc)) {
-		int err = PTR_ERR(p->rtc);
+	p = rtc_device_register("rtc-parisc", &dev->dev, &parisc_rtc_ops,
+				THIS_MODULE);
+	if (IS_ERR(p)) {
+		int err = PTR_ERR(p);
 		kfree(p);
 		return err;
 	}

@@ -68,9 +63,9 @@ static int __devinit parisc_rtc_probe(struct platform_device *dev)
 
 static int __devexit parisc_rtc_remove(struct platform_device *dev)
 {
-	struct parisc_rtc *p = platform_get_drvdata(dev);
+	struct rtc_device *p = platform_get_drvdata(dev);
 
-	rtc_device_unregister(p->rtc);
+	rtc_device_unregister(p);
 	kfree(p);
 
 	return 0;

Signature · 1 line

1.5.6.5

dann frazier

author

2009-01-15 23:46 UTC #4 lore raw

PATCH 4/9 Use rtc_valid_tm() in parisc_get_time

Use the return value of rtc_valid_tm() instead of just returning 0.

Signed-off-by dann frazier <[email protected]>

---
 drivers/rtc/rtc-parisc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/rtc/rtc-parisc.c +1 -1

index ee4e9a3..0477cc1 100644
@@ -20,7 +20,7 @@ static int parisc_get_time(struct device *dev, struct rtc_time *tm)
 	if (ret & RTC_BATT_BAD)
 		return -EOPNOTSUPP;
 
-	return 0;
+	return rtc_valid_tm(tm);
 }
 
 static int parisc_set_time(struct device *dev, struct rtc_time *tm)

Signature · 1 line

1.5.6.5

dann frazier

author

2009-01-15 23:46 UTC #5 lore raw

PATCH 5/9 Use platform_driver_probe

This isn't a hotpluggable device, so call platform_driver_probe directly in parisc_rtc_init

Signed-off-by dann frazier <[email protected]>

---
 drivers/rtc/rtc-parisc.c |   12 +++---------
 1 files changed, 3 insertions(+), 9 deletions(-)

drivers/rtc/rtc-parisc.c +3 -9

index 0477cc1..a2ca07a 100644
@@ -40,19 +40,14 @@ static const struct rtc_class_ops parisc_rtc_ops = {
 	.set_time = parisc_set_time,
 };
 
-static int __devinit parisc_rtc_probe(struct platform_device *dev)
+static int __init parisc_rtc_probe(struct platform_device *dev)
 {
 	struct rtc_device *p;
 
-	p = kzalloc(sizeof (*p), GFP_KERNEL);
-	if (!p)
-		return -ENOMEM;
-
 	p = rtc_device_register("rtc-parisc", &dev->dev, &parisc_rtc_ops,
 				THIS_MODULE);
 	if (IS_ERR(p)) {
 		int err = PTR_ERR(p);
-		kfree(p);
 		return err;
 	}

@@ -61,12 +56,11 @@ static int __devinit parisc_rtc_probe(struct platform_device *dev)
 	return 0;
 }
 
-static int __devexit parisc_rtc_remove(struct platform_device *dev)
+static int __exit parisc_rtc_remove(struct platform_device *dev)
 {
 	struct rtc_device *p = platform_get_drvdata(dev);
 
 	rtc_device_unregister(p);
-	kfree(p);
 
 	return 0;
 }

@@ -82,7 +76,7 @@ static struct platform_driver parisc_rtc_driver = {
 
 static int __init parisc_rtc_init(void)
 {
-	return platform_driver_register(&parisc_rtc_driver);
+	return platform_driver_probe(&parisc_rtc_driver, parisc_rtc_probe);
 }
 
 static void __exit parisc_rtc_fini(void)

Signature · 1 line

1.5.6.5

dann frazier

author

2009-01-15 23:46 UTC #6 lore raw

PATCH 6/9 declare rtc_parisc_dev as static

Signed-off-by dann frazier <[email protected]>

---
 arch/parisc/kernel/time.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

arch/parisc/kernel/time.c +1 -1

index 9d46c43..a479d08 100644
@@ -216,7 +216,7 @@ void __init start_cpu_itimer(void)
 	per_cpu(cpu_data, cpu).it_value = next_tick;
 }
 
-struct platform_device rtc_parisc_dev = {
+static struct platform_device rtc_parisc_dev = {
 	.name = "rtc-parisc",
 	.id = -1,
 };

Signature · 1 line

1.5.6.5

dann frazier

author

2009-01-15 23:46 UTC #7 lore raw

PATCH 7/9 remove unnecessary ret variable

Signed-off-by dann frazier <[email protected]>

---
 arch/parisc/kernel/time.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

arch/parisc/kernel/time.c +0 -1

index a479d08..e75cae6 100644
@@ -223,10 +223,7 @@ static struct platform_device rtc_parisc_dev = {
 
 static int __init rtc_init(void)
 {
-	int ret;

dann frazier

author

2009-01-15 23:46 UTC #8 lore raw

PATCH 8/9 remove a couple unnecessary variables

Signed-off-by dann frazier <[email protected]>

---
 drivers/rtc/rtc-parisc.c |   12 +++---------
 1 files changed, 3 insertions(+), 9 deletions(-)

drivers/rtc/rtc-parisc.c +0 -3

index a2ca07a..c29e918 100644
@@ -25,11 +25,7 @@ static int parisc_get_time(struct device *dev, struct rtc_time *tm)
 
 static int parisc_set_time(struct device *dev, struct rtc_time *tm)
 {
-	int ret;
-
-	ret = set_rtc_time(tm);

dann frazier

author

2009-01-15 23:46 UTC #9 lore raw

PATCH 9/9 rename p pointer to rtc

Signed-off-by dann frazier <[email protected]>

---
 drivers/rtc/rtc-parisc.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

drivers/rtc/rtc-parisc.c +8 -8

index c29e918..b966f56 100644
@@ -38,23 +38,23 @@ static const struct rtc_class_ops parisc_rtc_ops = {
 
 static int __init parisc_rtc_probe(struct platform_device *dev)
 {
-	struct rtc_device *p;
+	struct rtc_device *rtc;
 
-	p = rtc_device_register("rtc-parisc", &dev->dev, &parisc_rtc_ops,
-				THIS_MODULE);
-	if (IS_ERR(p))
-		return PTR_ERR(p);
+	rtc = rtc_device_register("rtc-parisc", &dev->dev, &parisc_rtc_ops,
+				  THIS_MODULE);
+	if (IS_ERR(rtc))
+		return PTR_ERR(rtc);
 
-	platform_set_drvdata(dev, p);
+	platform_set_drvdata(dev, rtc);
 
 	return 0;
 }
 
 static int __exit parisc_rtc_remove(struct platform_device *dev)
 {
-	struct rtc_device *p = platform_get_drvdata(dev);
+	struct rtc_device *rtc = platform_get_drvdata(dev);
 
-	rtc_device_unregister(p);
+	rtc_device_unregister(rtc);
 
 	return 0;
 }

Signature · 1 line

1.5.6.5

Alessandro Zummo

Acked-by on the v1 series

2009-01-16 00:06 UTC #10 lore raw

In reply to dann frazier

Quoted 4 lines · On Thu, 15 Jan 2009 16:46:47 -0700 dann frazier <[email protected]> wrote:

I recently submitted an rtc-efi driver having used rtc-parisc as a template. I received several suggestions from Alessandro Zummo and Andrew Morton that also apply to rtc-parisc. This series merges the relevant changes back into the rtc-parisc driver.

Acked-by Alessandro Zummo <[email protected]>

on the whole series. Thank you Dann!

Signature · 6 lines

 Best regards,

 Alessandro Zummo,
  Tower Technologies - Torino, Italy

  http://www.towertech.it

Kyle McMartin

2009-01-16 00:36 UTC #11 lore raw

Quoted 12 lines · Alessandro Zummo

Quoted 4 lines · On Thu, 15 Jan 2009 16:46:47 -0700 dann frazier <[email protected]> wrote:
I recently submitted an rtc-efi driver having used rtc-parisc as a template. I received several suggestions from Alessandro Zummo and Andrew Morton that also apply to rtc-parisc. This series merges the relevant changes back into the rtc-parisc driver.
Acked-by Alessandro Zummo <[email protected]>
on the whole series. Thank you Dann!

How does this jive with Paul Mundt's work that kills a bunch of these rtc- drivers? (added Paul to CC)

These patches are cool with me. (I noticed you kept the same comment in the ia64 time.c code. ;-)

Signed-off-by Kyle McMartin <[email protected]>

Alessandro Zummo

2009-01-16 01:48 UTC #12 lore raw

Quoted 2 lines · On Thu, 15 Jan 2009 19:36:06 -0500 Kyle McMartin <[email protected]> wrote:

How does this jive with Paul Mundt's work that kills a bunch of these rtc- drivers? (added Paul to CC)

You mean rtc-firmware? I'm not comfortable in having another legacy bridge. archs need to move to the rtc subsystem as a whole.

I saw a lot of improvements in this direction in the last few kernels, and I don't want to have something that will have to be removed in a couple of months.

Signature · 6 lines

 Best regards,

 Alessandro Zummo,
  Tower Technologies - Torino, Italy

  http://www.towertech.it

Andrew Morton

2009-01-16 20:47 UTC #13 lore raw

In reply to Alessandro Zummo

Quoted 12 lines · On Fri, 16 Jan 2009 01:06:33 +0100 Alessandro Zummo…

Quoted 4 lines · On Thu, 15 Jan 2009 16:46:47 -0700 dann frazier <[email protected]> wrote:
I recently submitted an rtc-efi driver having used rtc-parisc as a template. I received several suggestions from Alessandro Zummo and Andrew Morton that also apply to rtc-parisc. This series merges the relevant changes back into the rtc-parisc driver.
Acked-by Alessandro Zummo <[email protected]>
on the whole series. Thank you Dann!

I don't have a copy of these patches anywhere.

Whoever cc'ed "[email protected]", please slap thyself!

Alessandro Zummo

2009-01-17 01:10 UTC #14 lore raw

Quoted 4 lines · On Fri, 16 Jan 2009 12:47:30 -0800 Andrew Morton <[email protected]>…

I don't have a copy of these patches anywhere.
Whoever cc'ed "[email protected]", please slap thyself!

Here's the series in mbox format
http://patchwork.ozlabs.org/user/bundle/74/mbox/

Signature · 6 lines

 Best regards,

 Alessandro Zummo,
  Tower Technologies - Torino, Italy

  http://www.towertech.it

Andrew Morton

2009-01-17 01:49 UTC #15 lore raw

Quoted 11 lines · On Sat, 17 Jan 2009 02:10:26 +0100 Alessandro Zummo…

Quoted 4 lines · On Fri, 16 Jan 2009 12:47:30 -0800 Andrew Morton <[email protected]>…
I don't have a copy of these patches anywhere.
Whoever cc'ed "[email protected]", please slap thyself!
Here's the series in mbox format
http://patchwork.ozlabs.org/user/bundle/74/mbox/

From: dann frazier <[email protected]>
Subject: [1/9] Add a missing include for linux/rtc.h
Date: Thu, 15 Jan 2009 13:46:48 -0000

That's a bit awkward - if I do a reply-to-all with review comments, it goes privately.

Obviously I can manually fix that up in this case, but...