[PATCH v2] Create add_rtc() function to enable the RTC CMOS driver

[PATCH v2] Create add_rtc() function to enable the RTC CMOS driver

[Wade Farnsworth]

In order to use the RTC CMOS driver, each architecture must register a
platform device for the RTC.

This creates a function to register the platform device based on the RTC
device node and verifies that the RTC port against the hard-coded value
in asm/mc146818rtc.h.  It also sets the RTC to 24-hr mode as 12-hr mode
is not currently supported by the driver.

Signed-off-by: Wade Farnsworth <wfarnsworth@mvista.com>

---

 arch/powerpc/sysdev/Makefile         |    1 
 arch/powerpc/sysdev/rtc_cmos_setup.c |   54 +++++++++++++++++++++++++
 2 files changed, 55 insertions(+)

Index: linux-2.6-powerpc-8641/arch/powerpc/sysdev/Makefile
===================================================================
--- linux-2.6-powerpc-8641.orig/arch/powerpc/sysdev/Makefile
+++ linux-2.6-powerpc-8641/arch/powerpc/sysdev/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_TSI108_BRIDGE)	+= tsi108_pc
 obj-$(CONFIG_QUICC_ENGINE)	+= qe_lib/
 mv64x60-$(CONFIG_PCI)		+= mv64x60_pci.o
 obj-$(CONFIG_MV64X60)		+= $(mv64x60-y) mv64x60_pic.o mv64x60_dev.o
+obj-$(CONFIG_RTC_DRV_CMOS)	+= rtc_cmos_setup.o
 
 # contains only the suspend handler for time
 obj-$(CONFIG_PM)		+= timer.o
Index: linux-2.6-powerpc-8641/arch/powerpc/sysdev/rtc_cmos_setup.c
===================================================================
--- /dev/null
+++ linux-2.6-powerpc-8641/arch/powerpc/sysdev/rtc_cmos_setup.c
@@ -0,0 +1,54 @@
+/*
+ * Setup code for PC-style Real-Time Clock.
+ *
+ * Author: Wade Farnsworth <wfarnsworth@mvista.com>
+ *
+ * 2007 (c) MontaVista Software, Inc. This file is licensed under
+ * the terms of the GNU General Public License version 2. This program
+ * is licensed "as is" without any warranty of any kind, whether express
+ * or implied.
+ */
+
+#include <linux/platform_device.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/mc146818rtc.h>
+
+#include <asm/prom.h>
+
+static int  __init add_rtc(void)
+{
+	struct device_node *np;
+	struct platform_device *pd;
+	struct resource res;
+
+	np = of_find_compatible_node(NULL, NULL, "pnpPNP,b00");
+	if (!np)
+		return -ENODEV;
+
+	if (of_address_to_resource(np, 0, &res)) {
+		of_node_put(np);
+		return -ENODEV;
+	}
+
+	/*
+	 * RTC_PORT(x) is hardcoded in asm/mc146818rtc.h.  Verify that the
+	 * address provided by the device node matches.
+	 */
+	if (res.start != RTC_PORT(0)) {
+		of_node_put(np);
+		return -ENODEV;
+	}
+
+	pd = platform_device_register_simple("rtc_cmos", -1,
+					     &res, 1);
+	of_node_put(np);
+	if (IS_ERR(pd))
+		return PTR_ERR(pd);
+
+	/* rtc-cmos only supports 24-hr mode */
+	CMOS_WRITE(CMOS_READ(RTC_CONTROL) | RTC_24H, RTC_CONTROL);
+
+	return 0;
+}
+fs_initcall(add_rtc);

Re: [PATCH v2] Create add_rtc() function to enable the RTC CMOS driver

[Segher Boessenkool]

> This creates a function to register the platform device based on the 
> RTC
> device node and verifies that the RTC port against the hard-coded value
> in asm/mc146818rtc.h.  It also sets the RTC to 24-hr mode as 12-hr mode
> is not currently supported by the driver.

The 24h thing should be set in the driver itself.  Does the
driver handle binary vs. decimal mode btw?  If not, you'll
want to do the analogue thing for that.


Segher

Re: [PATCH v2] Create add_rtc() function to enable the RTC CMOS driver

[Wade Farnsworth]

On Wed, 2007-06-20 at 12:06 +0200, Segher Boessenkool wrote:
> > This creates a function to register the platform device based on the 
> > RTC
> > device node and verifies that the RTC port against the hard-coded value
> > in asm/mc146818rtc.h.  It also sets the RTC to 24-hr mode as 12-hr mode
> > is not currently supported by the driver.
> 
> The 24h thing should be set in the driver itself.  Does the
> driver handle binary vs. decimal mode btw?  If not, you'll
> want to do the analogue thing for that.
> 

OK, I'll look into putting the 24h fix into the driver.

It looks like binary mode is also not supported, so I'll add a fix for
that as well.

Thanks,

--Wade

[PATCH v3] Create add_rtc() function to enable the RTC CMOS driver

[Wade Farnsworth]

In order to use the RTC CMOS driver, each architecture must register a
platform device for the RTC.

This creates a function to register the platform device based on the RTC
device node and verifies that the RTC port against the hard-coded value
in asm/mc146818rtc.h.

Signed-off-by: Wade Farnsworth <wfarnsworth@mvista.com>

---

 arch/powerpc/sysdev/Makefile         |    1 
 arch/powerpc/sysdev/rtc_cmos_setup.c |   51 +++++++++++++++++++++++++
 2 files changed, 52 insertions(+)

Index: linux-2.6-powerpc-8641/arch/powerpc/sysdev/Makefile
===================================================================
--- linux-2.6-powerpc-8641.orig/arch/powerpc/sysdev/Makefile
+++ linux-2.6-powerpc-8641/arch/powerpc/sysdev/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_TSI108_BRIDGE)	+= tsi108_pc
 obj-$(CONFIG_QUICC_ENGINE)	+= qe_lib/
 mv64x60-$(CONFIG_PCI)		+= mv64x60_pci.o
 obj-$(CONFIG_MV64X60)		+= $(mv64x60-y) mv64x60_pic.o mv64x60_dev.o
+obj-$(CONFIG_RTC_DRV_CMOS)	+= rtc_cmos_setup.o
 
 # contains only the suspend handler for time
 obj-$(CONFIG_PM)		+= timer.o
Index: linux-2.6-powerpc-8641/arch/powerpc/sysdev/rtc_cmos_setup.c
===================================================================
--- /dev/null
+++ linux-2.6-powerpc-8641/arch/powerpc/sysdev/rtc_cmos_setup.c
@@ -0,0 +1,51 @@
+/*
+ * Setup code for PC-style Real-Time Clock.
+ *
+ * Author: Wade Farnsworth <wfarnsworth@mvista.com>
+ *
+ * 2007 (c) MontaVista Software, Inc. This file is licensed under
+ * the terms of the GNU General Public License version 2. This program
+ * is licensed "as is" without any warranty of any kind, whether
express
+ * or implied.
+ */
+
+#include <linux/platform_device.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/mc146818rtc.h>
+
+#include <asm/prom.h>
+
+static int  __init add_rtc(void)
+{
+	struct device_node *np;
+	struct platform_device *pd;
+	struct resource res;
+
+	np = of_find_compatible_node(NULL, NULL, "pnpPNP,b00");
+	if (!np)
+		return -ENODEV;
+
+	if (of_address_to_resource(np, 0, &res)) {
+		of_node_put(np);
+		return -ENODEV;
+	}
+
+	/*
+	 * RTC_PORT(x) is hardcoded in asm/mc146818rtc.h.  Verify that the
+	 * address provided by the device node matches.
+	 */
+	if (res.start != RTC_PORT(0)) {
+		of_node_put(np);
+		return -ENODEV;
+	}
+
+	pd = platform_device_register_simple("rtc_cmos", -1,
+					     &res, 1);
+	of_node_put(np);
+	if (IS_ERR(pd))
+		return PTR_ERR(pd);
+
+	return 0;
+}
+fs_initcall(add_rtc);

Re: [PATCH v3] Create add_rtc() function to enable the RTC CMOS driver

[Segher Boessenkool]

> +	/*
> +	 * RTC_PORT(x) is hardcoded in asm/mc146818rtc.h.  Verify that the
> +	 * address provided by the device node matches.
> +	 */
> +	if (res.start != RTC_PORT(0)) {
> +		of_node_put(np);
> +		return -ENODEV;
> +	}

You can place the of_node_put() here and remove the
previous one, and the following one.

> +	pd = platform_device_register_simple("rtc_cmos", -1,
> +					     &res, 1);
> +	of_node_put(np);

Code looks fine, with or without that change.


Segher

Re: [PATCH v3] Create add_rtc() function to enable the RTC CMOS driver

[David Gibson]

On Wed, Jun 20, 2007 at 09:59:32AM -0700, Wade Farnsworth wrote:
> In order to use the RTC CMOS driver, each architecture must register a
> platform device for the RTC.
> 
> This creates a function to register the platform device based on the RTC
> device node and verifies that the RTC port against the hard-coded value
> in asm/mc146818rtc.h.
> 
> Signed-off-by: Wade Farnsworth <wfarnsworth@mvista.com>

Hrm.  It seems rather specific.  Can we do this more generally, by
creating an of_platform device which binds to rtc nodes, then
registers an appropriate platform device for each so that the generic
rtc drivers pick them up.  Obviously we'd need some sort of table
mapping the device node compatible properties to the appropriate
platform device names.

> Index: linux-2.6-powerpc-8641/arch/powerpc/sysdev/Makefile
> ===================================================================
> --- linux-2.6-powerpc-8641.orig/arch/powerpc/sysdev/Makefile
> +++ linux-2.6-powerpc-8641/arch/powerpc/sysdev/Makefile
> @@ -18,6 +18,7 @@ obj-$(CONFIG_TSI108_BRIDGE)	+= tsi108_pc
>  obj-$(CONFIG_QUICC_ENGINE)	+= qe_lib/
>  mv64x60-$(CONFIG_PCI)		+= mv64x60_pci.o
>  obj-$(CONFIG_MV64X60)		+= $(mv64x60-y) mv64x60_pic.o mv64x60_dev.o
> +obj-$(CONFIG_RTC_DRV_CMOS)	+= rtc_cmos_setup.o
>  
>  # contains only the suspend handler for time
>  obj-$(CONFIG_PM)		+= timer.o
> Index: linux-2.6-powerpc-8641/arch/powerpc/sysdev/rtc_cmos_setup.c
> ===================================================================
> --- /dev/null
> +++ linux-2.6-powerpc-8641/arch/powerpc/sysdev/rtc_cmos_setup.c
> @@ -0,0 +1,51 @@
> +/*
> + * Setup code for PC-style Real-Time Clock.
> + *
> + * Author: Wade Farnsworth <wfarnsworth@mvista.com>
> + *
> + * 2007 (c) MontaVista Software, Inc. This file is licensed under
> + * the terms of the GNU General Public License version 2. This program
> + * is licensed "as is" without any warranty of any kind, whether
> express
> + * or implied.
> + */
> +
> +#include <linux/platform_device.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/mc146818rtc.h>
> +
> +#include <asm/prom.h>
> +
> +static int  __init add_rtc(void)
> +{
> +	struct device_node *np;
> +	struct platform_device *pd;
> +	struct resource res;
> +
> +	np = of_find_compatible_node(NULL, NULL, "pnpPNP,b00");
> +	if (!np)
> +		return -ENODEV;
> +
> +	if (of_address_to_resource(np, 0, &res)) {
> +		of_node_put(np);
> +		return -ENODEV;
> +	}
> +
> +	/*
> +	 * RTC_PORT(x) is hardcoded in asm/mc146818rtc.h.  Verify that the
> +	 * address provided by the device node matches.
> +	 */
> +	if (res.start != RTC_PORT(0)) {
> +		of_node_put(np);
> +		return -ENODEV;
> +	}

This looks totally bogus.  If we have a device tree we should be using
the address information from there, not using hardcoded magic.  Sounds
like asm/mc146818rtc.h needs some serious fixing.

> +
> +	pd = platform_device_register_simple("rtc_cmos", -1,
> +					     &res, 1);
> +	of_node_put(np);
> +	if (IS_ERR(pd))
> +		return PTR_ERR(pd);
> +
> +	return 0;
> +}
> +fs_initcall(add_rtc);
> 
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH v3] Create add_rtc() function to enable the RTC CMOS driver

[Segher Boessenkool]

> Hrm.  It seems rather specific.  Can we do this more generally, by
> creating an of_platform device which binds to rtc nodes, then
> registers an appropriate platform device for each so that the generic
> rtc drivers pick them up.  Obviously we'd need some sort of table
> mapping the device node compatible properties to the appropriate
> platform device names.

>> +	 * RTC_PORT(x) is hardcoded in asm/mc146818rtc.h.  Verify that the
>> +	 * address provided by the device node matches.
>> +	 */
>> +	if (res.start != RTC_PORT(0)) {
>> +		of_node_put(np);
>> +		return -ENODEV;
>> +	}
>
> This looks totally bogus.  If we have a device tree we should be using
> the address information from there, not using hardcoded magic.  Sounds
> like asm/mc146818rtc.h needs some serious fixing.

Both of your comments would be nice extensions (and the right
way forwards), but Wade's patch is good as-is already.


Segher

Re: [PATCH v3] Create add_rtc() function to enable the RTC CMOS driver

[David Gibson]

On Fri, Jun 22, 2007 at 09:56:24AM +0200, Segher Boessenkool wrote:
> > Hrm.  It seems rather specific.  Can we do this more generally, by
> > creating an of_platform device which binds to rtc nodes, then
> > registers an appropriate platform device for each so that the generic
> > rtc drivers pick them up.  Obviously we'd need some sort of table
> > mapping the device node compatible properties to the appropriate
> > platform device names.
> 
> >> +	 * RTC_PORT(x) is hardcoded in asm/mc146818rtc.h.  Verify that the
> >> +	 * address provided by the device node matches.
> >> +	 */
> >> +	if (res.start != RTC_PORT(0)) {
> >> +		of_node_put(np);
> >> +		return -ENODEV;
> >> +	}
> >
> > This looks totally bogus.  If we have a device tree we should be using
> > the address information from there, not using hardcoded magic.  Sounds
> > like asm/mc146818rtc.h needs some serious fixing.
> 
> Both of your comments would be nice extensions (and the right
> way forwards), but Wade's patch is good as-is already.

True enough.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH v3] Create add_rtc() function to enable the RTC CMOS driver

[Paul Mackerras]

Wade Farnsworth writes:

> In order to use the RTC CMOS driver, each architecture must register a
> platform device for the RTC.
> 
> This creates a function to register the platform device based on the RTC
> device node and verifies that the RTC port against the hard-coded value
> in asm/mc146818rtc.h.

The patch appears to be line-wrapped.  Please re-send.

Paul.

[PATCH v4] Create add_rtc() function to enable the RTC CMOS driver

[Wade Farnsworth]

In order to use the RTC CMOS driver, each architecture must register a
platform device for the RTC.

This creates a function to register the platform device based on the RTC
device node and verifies that the RTC port against the hard-coded value
in asm/mc146818rtc.h.

Signed-off-by: Wade Farnsworth <wfarnsworth@mvista.com>

---

Fixed line wrapping.

 arch/powerpc/sysdev/Makefile         |    1 
 arch/powerpc/sysdev/rtc_cmos_setup.c |   49 +++++++++++++++++++++++++
 2 files changed, 50 insertions(+)

diff --git a/arch/powerpc/sysdev/Makefile b/arch/powerpc/sysdev/Makefile
index 7d8ac1b..f65078c 100644
--- a/arch/powerpc/sysdev/Makefile
+++ b/arch/powerpc/sysdev/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_TSI108_BRIDGE)	+= tsi108_pci.o tsi108_dev.o
 obj-$(CONFIG_QUICC_ENGINE)	+= qe_lib/
 mv64x60-$(CONFIG_PCI)		+= mv64x60_pci.o
 obj-$(CONFIG_MV64X60)		+= $(mv64x60-y) mv64x60_pic.o mv64x60_dev.o
+obj-$(CONFIG_RTC_DRV_CMOS)	+= rtc_cmos_setup.o
 
 # contains only the suspend handler for time
 ifeq ($(CONFIG_RTC_CLASS),)
diff --git a/arch/powerpc/sysdev/rtc_cmos_setup.c b/arch/powerpc/sysdev/rtc_cmos_setup.c
new file mode 100644
index 0000000..e276048
--- /dev/null
+++ b/arch/powerpc/sysdev/rtc_cmos_setup.c
@@ -0,0 +1,49 @@
+/*
+ * Setup code for PC-style Real-Time Clock.
+ *
+ * Author: Wade Farnsworth <wfarnsworth@mvista.com>
+ *
+ * 2007 (c) MontaVista Software, Inc. This file is licensed under
+ * the terms of the GNU General Public License version 2. This program
+ * is licensed "as is" without any warranty of any kind, whether express
+ * or implied.
+ */
+
+#include <linux/platform_device.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/mc146818rtc.h>
+
+#include <asm/prom.h>
+
+static int  __init add_rtc(void)
+{
+	struct device_node *np;
+	struct platform_device *pd;
+	struct resource res;
+	int ret;
+
+	np = of_find_compatible_node(NULL, NULL, "pnpPNP,b00");
+	if (!np)
+		return -ENODEV;
+
+	ret = of_address_to_resource(np, 0, &res);
+	of_node_put(np);
+	if (ret)
+		return ret;
+
+	/*
+	 * RTC_PORT(x) is hardcoded in asm/mc146818rtc.h.  Verify that the
+	 * address provided by the device node matches.
+	 */
+	if (res.start != RTC_PORT(0))
+		return -EINVAL;
+
+	pd = platform_device_register_simple("rtc_cmos", -1,
+					     &res, 1);
+	if (IS_ERR(pd))
+		return PTR_ERR(pd);
+
+	return 0;
+}
+fs_initcall(add_rtc);