2014-07-07 11:36:58

by Roman Fietze

[permalink] [raw]
Subject: [PATCH 1/2] Allow to override the hctosys RTC using a kernel parameter

Hello RTC maintainers and list members,

I would like to hear your criticism about the following two small
patches.

These patches allow to overwrite the hctosys RTC specified in the
kernel config using CONFIG_RTC_HCTOSYS_DEVICE.

The background ist, that we have two almost identical x86 boards from
MEN in Nuernberg. The first one buffers the CMOS RTC, the other one
buffers an additional Epson RX8581 I2C RTC. We would like to use the
same kernel, but the only parameter we could not overwrite when
starting the kernel or system was the RTC used to initialize the
system clock.


>From 1e302072919594da0f5e71b38e7254ebfa4243f7 Mon Sep 17 00:00:00 2001
From: Roman Fietze <[email protected]>
Date: Thu, 3 Jul 2014 14:20:40 +0200
Subject: [PATCH 1/2] rtc: define maximum size of RTC device name in rtc.h

Signed-off-by: Roman Fietze <[email protected]>
---
drivers/rtc/rtc-proc.c | 10 ++++------
include/linux/rtc.h | 1 +
2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/rtc/rtc-proc.c b/drivers/rtc/rtc-proc.c
index ffa69e1..a77ca46 100644
--- a/drivers/rtc/rtc-proc.c
+++ b/drivers/rtc/rtc-proc.c
@@ -18,19 +18,17 @@

#include "rtc-core.h"

-#define NAME_SIZE 10
-
#if defined(CONFIG_RTC_HCTOSYS_DEVICE)
static bool is_rtc_hctosys(struct rtc_device *rtc)
{
int size;
- char name[NAME_SIZE];
+ char name[RTC_HCTOSYS_DEVICE_SIZE];

- size = scnprintf(name, NAME_SIZE, "rtc%d", rtc->id);
- if (size > NAME_SIZE)
+ size = scnprintf(name, RTC_HCTOSYS_DEVICE_SIZE, "rtc%d", rtc->id);
+ if (size > RTC_HCTOSYS_DEVICE_SIZE)
return false;

- return !strncmp(name, CONFIG_RTC_HCTOSYS_DEVICE, NAME_SIZE);
+ return !strncmp(name, rtc_hctosys_device, RTC_HCTOSYS_DEVICE_SIZE);
}
#else
static bool is_rtc_hctosys(struct rtc_device *rtc)
diff --git a/include/linux/rtc.h b/include/linux/rtc.h
index c2c2897..0a115b5 100644
--- a/include/linux/rtc.h
+++ b/include/linux/rtc.h
@@ -192,6 +192,7 @@ static inline bool is_leap_year(unsigned int year)
}

#ifdef CONFIG_RTC_HCTOSYS_DEVICE
+#define RTC_HCTOSYS_DEVICE_SIZE 10
extern int rtc_hctosys_ret;
#else
#define rtc_hctosys_ret -ENODEV
--
1.8.4.5



Roman

--
Roman Fietze Telemotive AG Buero Muehlhausen
Breitwiesen 73347 Muehlhausen
Tel.: +49 7335 18493-45 http://www.telemotive.de


2014-07-07 11:51:20

by Roman Fietze

[permalink] [raw]
Subject: [PATCH 2/2] Allow to override the hctosys RTC using a kernel parameter

Hello list members,

And here the second part.


>From e523006a34db26c274d3b71de5b914f476fb029e Mon Sep 17 00:00:00 2001
From: Roman Fietze <[email protected]>
Date: Fri, 4 Jul 2014 10:05:08 +0200
Subject: [PATCH 2/2] rtc: add kernel parameter hctosys, use it instead of
CONFIG_RTC_HCTOSYS_DEVICE

This change allows to overwrite the default of the hctosys RTC
specified in the kernnel configuration by using a kernel parameter in
the form of

hctosys=rtc<n>

Signed-off-by: Roman Fietze <[email protected]>
---
Documentation/kernel-parameters.txt | 2 ++
drivers/rtc/class.c | 23 +++++++++++++++++++++--
drivers/rtc/hctosys.c | 4 ++--
drivers/rtc/rtc-sysfs.c | 2 +-
drivers/rtc/systohc.c | 2 +-
include/linux/rtc.h | 1 +
6 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 30a8ad0d..ab1672f 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1110,6 +1110,8 @@ bytes respectively. Such letter suffixes can also be entirely omitted.

hcl= [IA-64] SGI's Hardware Graph compatibility layer

+ hctosys= [RTC_HCTOSYS_DEVICE] Sets the hctosys RTC
+
hd= [EIDE] (E)IDE hard drive subsystem geometry
Format: <cyl>,<head>,<sect>

diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c
index 589351e..b39434d 100644
--- a/drivers/rtc/class.c
+++ b/drivers/rtc/class.c
@@ -34,7 +34,26 @@ static void rtc_device_release(struct device *dev)
kfree(rtc);
}

+
#ifdef CONFIG_RTC_HCTOSYS_DEVICE
+
+char rtc_hctosys_device[RTC_HCTOSYS_DEVICE_SIZE] = CONFIG_RTC_HCTOSYS_DEVICE;
+
+static int __init parse_hctosys(char *str)
+{
+ if (!str)
+ return -EINVAL;
+
+ if (strlen(str) >= RTC_HCTOSYS_DEVICE_SIZE)
+ return -ENOMEM;
+
+ strcpy(rtc_hctosys_device, str);
+
+ return 0;
+}
+
+__setup("hctosys=", parse_hctosys);
+
/* Result of the last RTC to system clock attempt. */
int rtc_hctosys_ret = -ENODEV;
#endif
@@ -57,7 +76,7 @@ static int rtc_suspend(struct device *dev)
if (has_persistent_clock())
return 0;

- if (strcmp(dev_name(&rtc->dev), CONFIG_RTC_HCTOSYS_DEVICE) != 0)
+ if (strcmp(dev_name(&rtc->dev), rtc_hctosys_device) != 0)
return 0;

/* snapshot the current RTC and system time at suspend*/
@@ -99,7 +118,7 @@ static int rtc_resume(struct device *dev)
return 0;

rtc_hctosys_ret = -ENODEV;
- if (strcmp(dev_name(&rtc->dev), CONFIG_RTC_HCTOSYS_DEVICE) != 0)
+ if (strcmp(dev_name(&rtc->dev), rtc_hctosys_device) != 0)
return 0;

/* snapshot the current rtc and system time at resume */
diff --git a/drivers/rtc/hctosys.c b/drivers/rtc/hctosys.c
index 4aa60d7..ea79018 100644
--- a/drivers/rtc/hctosys.c
+++ b/drivers/rtc/hctosys.c
@@ -29,11 +29,11 @@ static int __init rtc_hctosys(void)
struct timespec tv = {
.tv_nsec = NSEC_PER_SEC >> 1,
};
- struct rtc_device *rtc = rtc_class_open(CONFIG_RTC_HCTOSYS_DEVICE);
+ struct rtc_device *rtc = rtc_class_open(rtc_hctosys_device);

if (rtc == NULL) {
pr_err("%s: unable to open rtc device (%s)\n",
- __FILE__, CONFIG_RTC_HCTOSYS_DEVICE);
+ __FILE__, rtc_hctosys_device);
goto err_open;
}

diff --git a/drivers/rtc/rtc-sysfs.c b/drivers/rtc/rtc-sysfs.c
index babd43b..69c6a9f 100644
--- a/drivers/rtc/rtc-sysfs.c
+++ b/drivers/rtc/rtc-sysfs.c
@@ -114,7 +114,7 @@ hctosys_show(struct device *dev, struct device_attribute *attr, char *buf)
#ifdef CONFIG_RTC_HCTOSYS_DEVICE
if (rtc_hctosys_ret == 0 &&
strcmp(dev_name(&to_rtc_device(dev)->dev),
- CONFIG_RTC_HCTOSYS_DEVICE) == 0)
+ rtc_hctosys_device) == 0)
return sprintf(buf, "1\n");
else
#endif
diff --git a/drivers/rtc/systohc.c b/drivers/rtc/systohc.c
index bf3e242..db9e74a 100644
--- a/drivers/rtc/systohc.c
+++ b/drivers/rtc/systohc.c
@@ -31,7 +31,7 @@ int rtc_set_ntp_time(struct timespec now)
else
rtc_time_to_tm(now.tv_sec + 1, &tm);

- rtc = rtc_class_open(CONFIG_RTC_HCTOSYS_DEVICE);
+ rtc = rtc_class_open(rtc_hctosys_device);
if (rtc) {
/* rtc_hctosys exclusively uses UTC, so we call set_time here,
* not set_mmss. */
diff --git a/include/linux/rtc.h b/include/linux/rtc.h
index 0a115b5..6feb68a 100644
--- a/include/linux/rtc.h
+++ b/include/linux/rtc.h
@@ -194,6 +194,7 @@ static inline bool is_leap_year(unsigned int year)
#ifdef CONFIG_RTC_HCTOSYS_DEVICE
#define RTC_HCTOSYS_DEVICE_SIZE 10
extern int rtc_hctosys_ret;
+extern char rtc_hctosys_device[RTC_HCTOSYS_DEVICE_SIZE];
#else
#define rtc_hctosys_ret -ENODEV
#endif
--
1.8.4.5



--
Roman Fietze Telemotive AG Buero Muehlhausen
Breitwiesen 73347 Muehlhausen
Tel.: +49 7335 18493-45 http://www.telemotive.de

2014-07-07 12:21:20

by Alexander Holler

[permalink] [raw]
Subject: Re: [rtc-linux] [PATCH 1/2] Allow to override the hctosys RTC using a kernel parameter

Am 07.07.2014 13:30, schrieb Roman Fietze:
> Hello RTC maintainers and list members,
>
> I would like to hear your criticism about the following two small
> patches.
>
> These patches allow to overwrite the hctosys RTC specified in the
> kernel config using CONFIG_RTC_HCTOSYS_DEVICE.
>
> The background ist, that we have two almost identical x86 boards from
> MEN in Nuernberg. The first one buffers the CMOS RTC, the other one
> buffers an additional Epson RX8581 I2C RTC. We would like to use the
> same kernel, but the only parameter we could not overwrite when
> starting the kernel or system was the RTC used to initialize the
> system clock.

You might have a look at a year old thread with working patches which
does a bit more too:

https://lkml.org/lkml/2014/6/13/6

Regards,

Alexander Holler

2014-07-07 13:03:11

by Alexander Holler

[permalink] [raw]
Subject: Re: [PATCH 2/2] Allow to override the hctosys RTC using a kernel parameter

Am 07.07.2014 13:50, schrieb Roman Fietze:
> Hello list members,
>
> And here the second part.
>
>
> From e523006a34db26c274d3b71de5b914f476fb029e Mon Sep 17 00:00:00 2001
> From: Roman Fietze <[email protected]>
> Date: Fri, 4 Jul 2014 10:05:08 +0200
> Subject: [PATCH 2/2] rtc: add kernel parameter hctosys, use it instead of
> CONFIG_RTC_HCTOSYS_DEVICE
>
> This change allows to overwrite the default of the hctosys RTC
> specified in the kernnel configuration by using a kernel parameter in
> the form of
>
> hctosys=rtc<n>

Sorry for the confusion with my replies to the other patch. The
Thunderbird on the used box got confused or confused me ("reply to list"
just replied to the rtc-list and "reply to all" didn't add the the
sender (Roman Fietze) to recipients. :/

Anyway, I think the whole rtcN mechanism is broken as you never can be
sure which driver/RTC gets which N without testing every built kernel.

I've fixed that with the already mentioned patches here:

https://lkml.org/lkml/2014/6/13/6

Besides that, I wish you luck with your patches. ;)

2014-07-15 23:16:45

by Andrew Morton

[permalink] [raw]
Subject: Re: [rtc-linux] [PATCH 2/2] Allow to override the hctosys RTC using a kernel parameter

On Mon, 07 Jul 2014 13:50:12 +0200 Roman Fietze <[email protected]> wrote:

> This change allows to overwrite the default of the hctosys RTC
> specified in the kernnel configuration by using a kernel parameter in
> the form of
>
> hctosys=rtc<n>
>

Seems reasonable to me, but...

> +char rtc_hctosys_device[RTC_HCTOSYS_DEVICE_SIZE] = CONFIG_RTC_HCTOSYS_DEVICE;
> +
> +static int __init parse_hctosys(char *str)
> +{
> + if (!str)
> + return -EINVAL;
> +
> + if (strlen(str) >= RTC_HCTOSYS_DEVICE_SIZE)
> + return -ENOMEM;
> +
> + strcpy(rtc_hctosys_device, str);

Can we just strdup() this thing and do away with all this futzing around
with the maximum string length?

> + return 0;
> +}