diff mbox

[7/7] powerpc/ps3: Add rtc-ps3

Message ID 1236605183-22718-8-git-send-email-Geert.Uytterhoeven@sonycom.com (mailing list archive)
State Superseded
Headers show

Commit Message

Geert Uytterhoeven March 9, 2009, 1:26 p.m. UTC
Create a real RTC driver for PS3, and unhook the deprecated
ppc_md.[gs]et_rtc_time.

Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
Cc: Geoff Levand <geoffrey.levand@am.sony.com>
---
 arch/powerpc/include/asm/ps3.h        |    3 +
 arch/powerpc/platforms/ps3/os-area.c  |    2 +
 arch/powerpc/platforms/ps3/platform.h |    2 -
 arch/powerpc/platforms/ps3/setup.c    |    2 -
 arch/powerpc/platforms/ps3/time.c     |   26 ++++-----
 drivers/rtc/Kconfig                   |    9 +++
 drivers/rtc/Makefile                  |    1 +
 drivers/rtc/rtc-ps3.c                 |  105 +++++++++++++++++++++++++++++++++
 8 files changed, 132 insertions(+), 18 deletions(-)
 create mode 100644 drivers/rtc/rtc-ps3.c

Comments

Alessandro Zummo March 9, 2009, 2:12 p.m. UTC | #1
On Mon,  9 Mar 2009 14:26:23 +0100
Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote:

 Hi,

   just a few notes:

> +
> +static int ps3_get_time(struct device *dev, struct rtc_time *tm)
> +{
> +	to_tm(read_rtc() + ps3_os_area_get_rtc_diff(), tm);
> +	tm->tm_year -= 1900;
> +	tm->tm_mon -= 1;
> +	return 0;
> +}

 this should be return rtc_valid_tm() .

 can't you use functions from rtc-lib.c instead of 
 that to_tm ?

> +
> +MODULE_AUTHOR("Sony Corporation");

 real name, if possible and a contact address
 here . Just in case I need someone to bother :)
Geoff Levand March 9, 2009, 6:04 p.m. UTC | #2
Hi,

On 03/09/2009 07:12 AM, Alessandro Zummo wrote:
> On Mon,  9 Mar 2009 14:26:23 +0100
> Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote:
>> +
>> +MODULE_AUTHOR("Sony Corporation");
> 
>  real name, if possible and a contact address
>  here . Just in case I need someone to bother :)

Please look at the MAINTAINERS file, that will give
the contact for PS3.  It is much easier to maintain
a single place for the contact than many spread
throughout the kernel sources.

-Geoff

--
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
Geoff Levand March 9, 2009, 6:35 p.m. UTC | #3
On 03/09/2009 06:26 AM, Geert Uytterhoeven wrote:
> Create a real RTC driver for PS3, and unhook the deprecated
> ppc_md.[gs]et_rtc_time.

>  8 files changed, 132 insertions(+), 18 deletions(-)

Sorry, I hadn't been following the discussion closely, but
could you explain why we are going from a generic framework
where we hook in our platform specific part to a totally
independent driver that has such an increase in code size.

Why couldn't you fix the generic part so that udev could
load it automatically?

I much prefer to have this code in the platform support
code as it was.  It is much more effort (a pain) to maintain
a separate driver were I have to cater to a subsystem's
maintainer, and with this rtc it seems everyone who was
using the generic PPC driver will need to do the same.

-Geoff

--
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
Alessandro Zummo March 9, 2009, 6:43 p.m. UTC | #4
On Mon, 9 Mar 2009 11:04:17 -0700
Geoff Levand <geoffrey.levand@am.sony.com> wrote:

> 
> Hi,
> 
> On 03/09/2009 07:12 AM, Alessandro Zummo wrote:
> > On Mon,  9 Mar 2009 14:26:23 +0100
> > Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote:
> >> +
> >> +MODULE_AUTHOR("Sony Corporation");
> > 
> >  real name, if possible and a contact address
> >  here . Just in case I need someone to bother :)
> 
> Please look at the MAINTAINERS file, that will give
> the contact for PS3.  It is much easier to maintain
> a single place for the contact than many spread
> throughout the kernel sources.

 Having it in MODULE_AUTHOR allow my scripts to automatically
 send an email when appropriate.

 MAINTAINERS lists the files with an arbitrary
 driver title so that the search must be made by an human and there's
 no field that links a person to a specific .c .

 so every time I want to address someone I need to check MODULE_AUTHOR,
 the git log and the MAINTAINERS file.
Geoff Levand March 9, 2009, 7:06 p.m. UTC | #5
Hi,

On 03/09/2009 11:43 AM, Alessandro Zummo wrote:
> Geoff Levand <geoffrey.levand@am.sony.com> wrote:
>> On 03/09/2009 07:12 AM, Alessandro Zummo wrote:
>> > On Mon,  9 Mar 2009 14:26:23 +0100
>> > Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote:
>> >> +
>> >> +MODULE_AUTHOR("Sony Corporation");
>> > 
>> >  real name, if possible and a contact address
>> >  here . Just in case I need someone to bother :)
>> 
>> Please look at the MAINTAINERS file, that will give
>> the contact for PS3.  It is much easier to maintain
>> a single place for the contact than many spread
>> throughout the kernel sources.
> 
>  Having it in MODULE_AUTHOR allow my scripts to automatically
>  send an email when appropriate.
> 
>  MAINTAINERS lists the files with an arbitrary
>  driver title so that the search must be made by an human and there's
>  no field that links a person to a specific .c .
> 
>  so every time I want to address someone I need to check MODULE_AUTHOR,
>  the git log and the MAINTAINERS file.

I see.  It seems what you want is MODULE_MAINTAINER, as author is
the author, who after some time, may not be the maintainer any more.

There was some work by Joe Perches to list the files a maintainer is
responsible for into the MAINTAINERS file.  I think that would give
you what you want, a way to automatically get the maintainer of a
file.

Joe, could you let us know the status of that work?

-Geoff

--
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
Joe Perches March 9, 2009, 7:18 p.m. UTC | #6
On Mon, 2009-03-09 at 12:06 -0700, Geoff Levand wrote:
> There was some work by Joe Perches to list the files a maintainer is
> responsible for into the MAINTAINERS file.  I think that would give
> you what you want, a way to automatically get the maintainer of a
> file.
> 
> Joe, could you let us know the status of that work?

I think it works fine, and is an acceptable
approach to finding a maintainer for either a
patch or a specific file.

The changes I've posted are probably not suitable
for merging by anyone other than Linus as
MAINTAINERS is the most heavily modified file
in the kernel tree.

I've submitted it several times after merging it
with the latest kernel without response from Linus.

I'd merge and submit it again if it could be accepted.

If someone would propose a mechanism that would
improve the possibility to get it merged, I'd
also appreciate that.

--
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
Geert Uytterhoeven March 10, 2009, 9:21 a.m. UTC | #7
On Mon, 9 Mar 2009, Geoff Levand wrote:
> On 03/09/2009 06:26 AM, Geert Uytterhoeven wrote:
> > Create a real RTC driver for PS3, and unhook the deprecated
> > ppc_md.[gs]et_rtc_time.
> 
> >  8 files changed, 132 insertions(+), 18 deletions(-)
> 
> Sorry, I hadn't been following the discussion closely, but
> could you explain why we are going from a generic framework
> where we hook in our platform specific part to a totally
> independent driver that has such an increase in code size.

Alessandro prefers not to have generic RTC drivers on top of some other
abstraction, but wants platform/chip-specific drivers under drivers/rtc/
instead. The goal is to convert all RTC drivers buried in platform code
to separate RTC drivers.

(Alessandro, please correct me if I'm wrong)

> Why couldn't you fix the generic part so that udev could
> load it automatically?

BTW, I also fixed the generic part, which is now called rtc-generic and
autoloaded (patch 4).

> I much prefer to have this code in the platform support
> code as it was.  It is much more effort (a pain) to maintain
> a separate driver were I have to cater to a subsystem's
> maintainer, and with this rtc it seems everyone who was
> using the generic PPC driver will need to do the same.

They can keep on using rtc-generic for now (patch 6).

If you do not want rtc-ps3, you can nak it, and keep on using rtc-generic :-)
But please consider before doing that...

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis · BIC GEBABEBB · IBAN BE41293037680010
--
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
Alessandro Zummo March 10, 2009, 9:39 a.m. UTC | #8
On Tue, 10 Mar 2009 10:21:14 +0100 (CET)
Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com> wrote:

> Alessandro prefers not to have generic RTC drivers on top of some other
> abstraction, but wants platform/chip-specific drivers under drivers/rtc/
> instead. The goal is to convert all RTC drivers buried in platform code
> to separate RTC drivers.
> 
> (Alessandro, please correct me if I'm wrong)

 yes, that's my dream :)
Geoff Levand March 10, 2009, 4:18 p.m. UTC | #9
On 03/09/2009 06:26 AM, Geert Uytterhoeven wrote:
> Create a real RTC driver for PS3, and unhook the deprecated
> ppc_md.[gs]et_rtc_time.
> 
> Signed-off-by: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
> Cc: Geoff Levand <geoffrey.levand@am.sony.com>
> ---
>  arch/powerpc/include/asm/ps3.h        |    3 +
>  arch/powerpc/platforms/ps3/os-area.c  |    2 +
>  arch/powerpc/platforms/ps3/platform.h |    2 -
>  arch/powerpc/platforms/ps3/setup.c    |    2 -
>  arch/powerpc/platforms/ps3/time.c     |   26 ++++-----
>  drivers/rtc/Kconfig                   |    9 +++
>  drivers/rtc/Makefile                  |    1 +
>  drivers/rtc/rtc-ps3.c                 |  105 +++++++++++++++++++++++++++++++++
>  8 files changed, 132 insertions(+), 18 deletions(-)
>  create mode 100644 drivers/rtc/rtc-ps3.c

Acked-by: Geoff Levand <geoffrey.levand@am.sony.com>

--
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/include/asm/ps3.h b/arch/powerpc/include/asm/ps3.h
index 67f1812..cdb6fd8 100644
--- a/arch/powerpc/include/asm/ps3.h
+++ b/arch/powerpc/include/asm/ps3.h
@@ -50,6 +50,9 @@  enum ps3_param_av_multi_out {
 
 enum ps3_param_av_multi_out ps3_os_area_get_av_multi_out(void);
 
+extern u64 ps3_os_area_get_rtc_diff(void);
+extern void ps3_os_area_set_rtc_diff(u64 rtc_diff);
+
 /* dma routines */
 
 enum ps3_dma_page_size {
diff --git a/arch/powerpc/platforms/ps3/os-area.c b/arch/powerpc/platforms/ps3/os-area.c
index e1c83c2..86e392b 100644
--- a/arch/powerpc/platforms/ps3/os-area.c
+++ b/arch/powerpc/platforms/ps3/os-area.c
@@ -808,6 +808,7 @@  u64 ps3_os_area_get_rtc_diff(void)
 {
 	return saved_params.rtc_diff;
 }
+EXPORT_SYMBOL(ps3_os_area_get_rtc_diff);
 
 /**
  * ps3_os_area_set_rtc_diff - Set the rtc diff value.
@@ -823,6 +824,7 @@  void ps3_os_area_set_rtc_diff(u64 rtc_diff)
 		os_area_queue_work();
 	}
 }
+EXPORT_SYMBOL(ps3_os_area_set_rtc_diff);
 
 /**
  * ps3_os_area_get_av_multi_out - Returns the default video mode.
diff --git a/arch/powerpc/platforms/ps3/platform.h b/arch/powerpc/platforms/ps3/platform.h
index 235c13e..136aa06 100644
--- a/arch/powerpc/platforms/ps3/platform.h
+++ b/arch/powerpc/platforms/ps3/platform.h
@@ -64,8 +64,6 @@  int ps3_set_rtc_time(struct rtc_time *time);
 
 void __init ps3_os_area_save_params(void);
 void __init ps3_os_area_init(void);
-u64 ps3_os_area_get_rtc_diff(void);
-void ps3_os_area_set_rtc_diff(u64 rtc_diff);
 
 /* spu */
 
diff --git a/arch/powerpc/platforms/ps3/setup.c b/arch/powerpc/platforms/ps3/setup.c
index 3331ccb..6618182 100644
--- a/arch/powerpc/platforms/ps3/setup.c
+++ b/arch/powerpc/platforms/ps3/setup.c
@@ -270,8 +270,6 @@  define_machine(ps3) {
 	.init_IRQ			= ps3_init_IRQ,
 	.panic				= ps3_panic,
 	.get_boot_time			= ps3_get_boot_time,
-	.set_rtc_time			= ps3_set_rtc_time,
-	.get_rtc_time			= ps3_get_rtc_time,
 	.set_dabr			= ps3_set_dabr,
 	.calibrate_decr			= ps3_calibrate_decr,
 	.progress			= ps3_progress,
diff --git a/arch/powerpc/platforms/ps3/time.c b/arch/powerpc/platforms/ps3/time.c
index d0daf7d..b178a1e 100644
--- a/arch/powerpc/platforms/ps3/time.c
+++ b/arch/powerpc/platforms/ps3/time.c
@@ -19,6 +19,7 @@ 
  */
 
 #include <linux/kernel.h>
+#include <linux/platform_device.h>
 
 #include <asm/rtc.h>
 #include <asm/lv1call.h>
@@ -74,23 +75,20 @@  static u64 read_rtc(void)
 	return rtc_val;
 }
 
-int ps3_set_rtc_time(struct rtc_time *tm)
+unsigned long __init ps3_get_boot_time(void)
 {
-	u64 now = mktime(tm->tm_year + 1900, tm->tm_mon + 1, tm->tm_mday,
-		tm->tm_hour, tm->tm_min, tm->tm_sec);
-
-	ps3_os_area_set_rtc_diff(now - read_rtc());
-	return 0;
+	return read_rtc() + ps3_os_area_get_rtc_diff();
 }
 
-void ps3_get_rtc_time(struct rtc_time *tm)
+static int __init ps3_rtc_init(void)
 {
-	to_tm(read_rtc() + ps3_os_area_get_rtc_diff(), tm);
-	tm->tm_year -= 1900;
-	tm->tm_mon -= 1;
-}
+	struct platform_device *pdev;
 
-unsigned long __init ps3_get_boot_time(void)
-{
-	return read_rtc() + ps3_os_area_get_rtc_diff();
+	pdev = platform_device_register_simple("rtc-ps3", -1, NULL, 0);
+	if (IS_ERR(pdev))
+		return PTR_ERR(pdev);
+
+	return 0;
 }
+
+module_init(ps3_rtc_init);
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 6488c50..7b6b63a 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -730,4 +730,13 @@  config RTC_DRV_MV
 	  This driver can also be built as a module. If so, the module
 	  will be called rtc-mv.
 
+config RTC_DRV_PS3
+	tristate "PS3 RTC"
+	depends on PPC_PS3
+	help
+	  If you say yes here you will get support for the RTC on PS3.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called rtc-ps3.
+
 endif # RTC_CLASS
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index bd209a5..d161d1d 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -75,3 +75,4 @@  obj-$(CONFIG_RTC_DRV_VR41XX)	+= rtc-vr41xx.o
 obj-$(CONFIG_RTC_DRV_WM8350)	+= rtc-wm8350.o
 obj-$(CONFIG_RTC_DRV_X1205)	+= rtc-x1205.o
 obj-$(CONFIG_RTC_DRV_PCF50633)	+= rtc-pcf50633.o
+obj-$(CONFIG_RTC_DRV_PS3)	+= rtc-ps3.o
diff --git a/drivers/rtc/rtc-ps3.c b/drivers/rtc/rtc-ps3.c
new file mode 100644
index 0000000..bacf37f
--- /dev/null
+++ b/drivers/rtc/rtc-ps3.c
@@ -0,0 +1,105 @@ 
+/*
+ * PS3 RTC Driver
+ *
+ * Copyright 2009 Sony Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.
+ * If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/rtc.h>
+
+#include <asm/lv1call.h>
+#include <asm/ps3.h>
+
+
+static u64 read_rtc(void)
+{
+	int result;
+	u64 rtc_val;
+	u64 tb_val;
+
+	result = lv1_get_rtc(&rtc_val, &tb_val);
+	BUG_ON(result);
+
+	return rtc_val;
+}
+
+static int ps3_get_time(struct device *dev, struct rtc_time *tm)
+{
+	to_tm(read_rtc() + ps3_os_area_get_rtc_diff(), tm);
+	tm->tm_year -= 1900;
+	tm->tm_mon -= 1;
+	return 0;
+}
+
+static int ps3_set_time(struct device *dev, struct rtc_time *tm)
+{
+	u64 now = mktime(tm->tm_year + 1900, tm->tm_mon + 1, tm->tm_mday,
+			 tm->tm_hour, tm->tm_min, tm->tm_sec);
+	ps3_os_area_set_rtc_diff(now - read_rtc());
+	return 0;
+}
+
+static const struct rtc_class_ops ps3_rtc_ops = {
+	.read_time = ps3_get_time,
+	.set_time = ps3_set_time,
+};
+
+static int __init ps3_rtc_probe(struct platform_device *dev)
+{
+	struct rtc_device *rtc;
+
+	rtc = rtc_device_register("rtc-ps3", &dev->dev, &ps3_rtc_ops,
+				  THIS_MODULE);
+	if (IS_ERR(rtc))
+		return PTR_ERR(rtc);
+
+	platform_set_drvdata(dev, rtc);
+	return 0;
+}
+
+static int __exit ps3_rtc_remove(struct platform_device *dev)
+{
+	rtc_device_unregister(platform_get_drvdata(dev));
+	return 0;
+}
+
+static struct platform_driver ps3_rtc_driver = {
+	.driver = {
+		.name = "rtc-ps3",
+		.owner = THIS_MODULE,
+	},
+	.remove = __exit_p(ps3_rtc_remove),
+};
+
+static int __init ps3_rtc_init(void)
+{
+	return platform_driver_probe(&ps3_rtc_driver, ps3_rtc_probe);
+}
+
+static void __exit ps3_rtc_fini(void)
+{
+	platform_driver_unregister(&ps3_rtc_driver);
+}
+
+module_init(ps3_rtc_init);
+module_exit(ps3_rtc_fini);
+
+MODULE_AUTHOR("Sony Corporation");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("ps3 RTC driver");
+MODULE_ALIAS("platform:rtc-ps3");