2005-11-14 13:50:14

by Andrey Volkov

[permalink] [raw]
Subject: [PATCH 1/1] Added support of ST m41t85 rtc chip

Added support of ST M41T85 RTC

Signed-off-By: Andrey Volkov <[email protected]>
---

drivers/i2c/chips/Kconfig | 88 ++++++++++++++
drivers/i2c/chips/Makefile | 1
drivers/i2c/chips/m41t85.c | 285 ++++++++++++++++++++++++++++++++++++++++++++
include/linux/i2c-id.h | 1
4 files changed, 375 insertions(+), 0 deletions(-)

diff --git a/drivers/i2c/chips/Kconfig b/drivers/i2c/chips/Kconfig
index f9fae28..6593fc5 100644
--- a/drivers/i2c/chips/Kconfig
+++ b/drivers/i2c/chips/Kconfig
@@ -111,6 +111,94 @@ config SENSORS_M41T00
This driver can also be built as a module. If so, the module
will be called m41t00.

+config SENSORS_M41T85
+ tristate "ST M41T85 RTC chip"
+ depends on I2C
+ help
+ If you say yes here you get support for the ST M41T85 RTC chip.
+
+ This driver can also be built as a module. If so, the module
+ will be called m41t85.
+
+config SENSORS_M41T85_SQW_FRQ_ENABLE
+ depends on SENSORS_M41T85
+ bool "Square Wave Output"
+ help
+ If you say yes here, then m41t85 will generate clocks on it
+ SQW pin at frequency which you are select below.
+
+choice
+ prompt "SQW frequincy"
+ depends on SENSORS_M41T85_SQW_FRQ_ENABLE
+ default SENSORS_M41T85_SQW_FRQ_32KHZ
+
+config SENSORS_M41T85_SQW_FRQ_32KHZ
+ bool "32.768 kHz"
+
+config SENSORS_M41T85_SQW_FRQ_8KHZ
+ bool "8 kHz"
+
+config SENSORS_M41T85_SQW_FRQ_4KHZ
+ bool "4 kHz"
+
+config SENSORS_M41T85_SQW_FRQ_2KHZ
+ bool "2 kHz"
+
+config SENSORS_M41T85_SQW_FRQ_1KHZ
+ bool "1 kHz"
+
+config SENSORS_M41T85_SQW_FRQ_512HZ
+ bool "512 Hz"
+
+config SENSORS_M41T85_SQW_FRQ_256HZ
+ bool "256 Hz"
+
+config SENSORS_M41T85_SQW_FRQ_128HZ
+ bool "128 Hz"
+
+config SENSORS_M41T85_SQW_FRQ_64HZ
+ bool "64 Hz"
+
+config SENSORS_M41T85_SQW_FRQ_32HZ
+ bool "32 Hz"
+
+config SENSORS_M41T85_SQW_FRQ_16HZ
+ bool "16 Hz"
+
+config SENSORS_M41T85_SQW_FRQ_8HZ
+ bool "8 Hz"
+
+config SENSORS_M41T85_SQW_FRQ_4HZ
+ bool "4 Hz"
+
+config SENSORS_M41T85_SQW_FRQ_2HZ
+ bool "2 Hz"
+
+config SENSORS_M41T85_SQW_FRQ_1HZ
+ bool "1 Hz"
+
+endchoice
+
+config SENSORS_M41T85_SQW_FRQ
+ int
+ depends on SENSORS_M41T85_SQW_FRQ_ENABLE
+ default "1" if SENSORS_M41T85_SQW_FRQ_32KHZ
+ default "2" if SENSORS_M41T85_SQW_FRQ_8KHZ
+ default "3" if SENSORS_M41T85_SQW_FRQ_4KHZ
+ default "4" if SENSORS_M41T85_SQW_FRQ_2KHZ
+ default "5" if SENSORS_M41T85_SQW_FRQ_1KHZ
+ default "6" if SENSORS_M41T85_SQW_FRQ_512HZ
+ default "7" if SENSORS_M41T85_SQW_FRQ_256HZ
+ default "8" if SENSORS_M41T85_SQW_FRQ_128HZ
+ default "9" if SENSORS_M41T85_SQW_FRQ_64HZ
+ default "10" if SENSORS_M41T85_SQW_FRQ_32HZ
+ default "11" if SENSORS_M41T85_SQW_FRQ_16HZ
+ default "12" if SENSORS_M41T85_SQW_FRQ_8HZ
+ default "13" if SENSORS_M41T85_SQW_FRQ_4HZ
+ default "14" if SENSORS_M41T85_SQW_FRQ_2HZ
+ default "15" if SENSORS_M41T85_SQW_FRQ_1HZ
+
+
config SENSORS_MAX6875
tristate "Maxim MAX6875 Power supply supervisor"
depends on I2C && EXPERIMENTAL
diff --git a/drivers/i2c/chips/Makefile b/drivers/i2c/chips/Makefile
index 46178b5..5637881 100644
--- a/drivers/i2c/chips/Makefile
+++ b/drivers/i2c/chips/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_SENSORS_RTC8564) += rtc8564
obj-$(CONFIG_ISP1301_OMAP) += isp1301_omap.o
obj-$(CONFIG_TPS65010) += tps65010.o
obj-$(CONFIG_RTC_X1205_I2C) += x1205.o
+obj-$(CONFIG_SENSORS_M41T85) += m41t85.o

ifeq ($(CONFIG_I2C_DEBUG_CHIP),y)
EXTRA_CFLAGS += -DDEBUG
diff --git a/drivers/i2c/chips/m41t85.c b/drivers/i2c/chips/m41t85.c
new file mode 100644
index 0000000..f26a115
--- /dev/null
+++ b/drivers/i2c/chips/m41t85.c
@@ -0,0 +1,285 @@
+/*
+ * drivers/i2c/chips/m41t85.c
+ *
+ * DESCRIPTION:
+ * I2C client/driver for the ST M41T85 Real-Time Clock chip.
+ *
+ * AUTHOR:
+ * Andrey Volkov <[email protected]>
+ *
+ * COPYRIGHT:
+ * 2005, Varma Electronics Oy
+ *
+ * Based on driver for ST M41T00 by Mark A. Greer <[email protected]>
+ * (see m41t00.c for copyright)
+ *
+ * LICENCE:
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License version 2. See the file COPYING in the main directory of this archive
+ * for more details.
+ *
+ * HISTORY:
+ * 2005-10-12 Created
+ */
+
+/*
+ * This i2c client/driver wedges between the drivers/char/genrtc.c RTC
+ * interface and the SMBus interface of the i2c subsystem.
+ * It would be more efficient to use i2c msgs/i2c_transfer directly but, as
+ * recommened in .../Documentation/i2c/writing-clients section
+ * "Sending and receiving", using SMBus level communication is preferred.
+ */
+
+#include <linux/config.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/i2c.h>
+#include <linux/rtc.h>
+#include <linux/bcd.h>
+
+#include <asm/time.h>
+#include <asm/rtc.h>
+
+#define M41T85_DRV_NAME "m41t85"
+
+#define RTC_CSEC_ADDR 0x00
+#define RTC_SEC_ADDR 0x01
+#define RTC_MIN_ADDR 0x02
+#define RTC_HOUR_ADDR 0x03
+#define RTC_WDAY_ADDR 0x04
+#define RTC_DAY_ADDR 0x05
+#define RTC_MONTH_ADDR 0x06
+#define RTC_YEARS_ADDR 0x07
+#define RTC_CONTROL_ADDR 0x08
+#define RTC_WATCHDOG_ADDR 0x09
+#define RTC_ALARM_MONTH_ADDR 0x0a
+#define RTC_ALARM_DATE_ADDR 0x0b
+#define RTC_ALARM_HOUR_ADDR 0x0c
+#define RTC_ALARM_MIN_ADDR 0x0d
+#define RTC_ALARM_SEC_ADDR 0x0e
+#define RTC_WDF_ADDR 0x0f
+#define RTC_SQW_ADDR 0x13
+
+
+static DECLARE_MUTEX(m41t85_mutex);
+
+static struct i2c_driver m41t85_driver;
+static struct i2c_client *save_client;
+
+static unsigned short ignore[] = { I2C_CLIENT_END };
+static unsigned short normal_addr[] = { 0x68, I2C_CLIENT_END };
+
+static struct i2c_client_address_data addr_data = {
+ .normal_i2c = normal_addr,
+ .probe = ignore,
+ .ignore = ignore,
+};
+
+ulong
+m41t85_get_rtc_time(void)
+{
+ s32 sec, min, hour, day, mon, year;
+ s32 sec1, min1, hour1, day1, mon1, year1;
+ ulong limit = 10;
+
+ sec = min = hour = day = mon = year = 0;
+ sec1 = min1 = hour1 = day1 = mon1 = year1 = 0;
+
+ down(&m41t85_mutex);
+ do {
+
+ if (((sec = i2c_smbus_read_byte_data(save_client, RTC_SEC_ADDR)) >= 0)
+ && ((min = i2c_smbus_read_byte_data(save_client, RTC_MIN_ADDR))
+ >= 0)
+ && ((hour = i2c_smbus_read_byte_data(save_client, RTC_HOUR_ADDR))
+ >= 0)
+ && ((day = i2c_smbus_read_byte_data(save_client, RTC_DAY_ADDR))
+ >= 0)
+ && ((mon = i2c_smbus_read_byte_data(save_client, RTC_MONTH_ADDR))
+ >= 0)
+ && ((year = i2c_smbus_read_byte_data(save_client, RTC_YEARS_ADDR))
+ >= 0)
+ && ((sec == sec1) && (min == min1) && (hour == hour1)
+ && (day == day1) && (mon == mon1)
+ && (year == year1)))
+
+ break;
+
+ sec1 = sec;
+ min1 = min;
+ hour1 = hour;
+ day1 = day;
+ mon1 = mon;
+ year1 = year;
+ } while (--limit > 0);
+ up(&m41t85_mutex);
+
+ if (limit == 0) {
+ dev_warn(&save_client->dev,
+ "m41t85: can't read rtc chip\n");
+ sec = min = hour = day = mon = year = 0;
+ }
+
+ sec = BCD2BIN(sec & 0x7f);
+ min = BCD2BIN(min & 0x7f);
+ hour = BCD2BIN(hour & 0x3f);
+ day = BCD2BIN(day & 0x3f);
+ mon = BCD2BIN(mon & 0x1f);
+ year = BCD2BIN(year & 0xff);
+
+ year += 1900;
+ if (year < 1970)
+ year += 100;
+
+ return mktime(year, mon, day, hour, min, sec);
+}
+
+static void
+m41t85_set_tlet(ulong arg)
+{
+ struct rtc_time tm;
+ ulong nowtime = *(ulong *)arg;
+
+ to_tm(nowtime, &tm);
+ tm.tm_year = (tm.tm_year - 1900) % 100;
+
+ tm.tm_sec = BIN2BCD(tm.tm_sec);
+ tm.tm_min = BIN2BCD(tm.tm_min);
+ tm.tm_hour = BIN2BCD(tm.tm_hour);
+ tm.tm_mon = BIN2BCD(tm.tm_mon);
+ tm.tm_mday = BIN2BCD(tm.tm_mday);
+ tm.tm_year = BIN2BCD(tm.tm_year);
+
+ down(&m41t85_mutex);
+ if ((i2c_smbus_write_byte_data(save_client, RTC_SEC_ADDR, tm.tm_sec & 0x7f) < 0)
+ || (i2c_smbus_write_byte_data(save_client, RTC_MIN_ADDR, tm.tm_min & 0x7f)
+ < 0)
+ || (i2c_smbus_write_byte_data(save_client, RTC_HOUR_ADDR, tm.tm_hour & 0x7f)
+ < 0)
+ || (i2c_smbus_write_byte_data(save_client, RTC_DAY_ADDR, tm.tm_mday & 0x7f)
+ < 0)
+ || (i2c_smbus_write_byte_data(save_client, RTC_MONTH_ADDR, tm.tm_mon & 0x7f)
+ < 0)
+ || (i2c_smbus_write_byte_data(save_client, RTC_YEARS_ADDR, tm.tm_year & 0x7f)
+ < 0))
+
+ dev_warn(&save_client->dev,"m41t85: can't write to rtc chip\n");
+
+ up(&m41t85_mutex);
+ return;
+}
+
+static ulong new_time;
+
+DECLARE_TASKLET_DISABLED(m41t85_tasklet, m41t85_set_tlet, (ulong)&new_time);
+
+int
+m41t85_set_rtc_time(ulong nowtime)
+{
+ new_time = nowtime;
+
+ if (in_interrupt())
+ tasklet_schedule(&m41t85_tasklet);
+ else
+ m41t85_set_tlet((ulong)&new_time);
+
+ return 0;
+}
+
+/*
+ *****************************************************************************
+ *
+ * Driver Interface
+ *
+ *****************************************************************************
+ */
+static int
+m41t85_probe(struct i2c_adapter *adap, int addr, int kind)
+{
+ struct i2c_client *client;
+ int ret;
+ s32 reg;
+
+ client = kzalloc(sizeof(struct i2c_client), GFP_KERNEL);
+ if (!client)
+ return -ENOMEM;
+
+ strncpy(client->name, M41T85_DRV_NAME, I2C_NAME_SIZE);
+ client->addr = addr;
+ client->adapter = adap;
+ client->driver = &m41t85_driver;
+
+ if ((ret = i2c_attach_client(client)) != 0) {
+ kfree(client);
+ return ret;
+ }
+
+ #if defined (CONFIG_SENSORS_M41T85_SQW_FRQ)
+ reg = i2c_smbus_read_byte_data(client, RTC_ALARM_MONTH_ADDR);
+ if (reg >=0 && (reg & 0x40) == 0) {
+ ret = i2c_smbus_write_byte_data(client, RTC_SQW_ADDR, CONFIG_SENSORS_M41T85_SQW_FRQ);
+ if(ret == 0)
+ i2c_smbus_write_byte_data(client, RTC_ALARM_MONTH_ADDR, reg | 0x40);
+ }
+ #endif
+ /* Check if ST flag set, and if yes, then clear it */
+ reg = i2c_smbus_read_byte_data(client, RTC_SEC_ADDR);
+ if (reg >=0 && (reg & 0x80) != 0)
+ i2c_smbus_write_byte_data(client, RTC_SEC_ADDR, reg & ~0x80);
+
+ /* Check if HT flag set, and if yes, then clear it */
+ reg = i2c_smbus_read_byte_data(client, RTC_ALARM_HOUR_ADDR);
+ if (reg >=0 && (reg & 0x40) != 0)
+ i2c_smbus_write_byte_data(client, RTC_ALARM_HOUR_ADDR, reg & ~0x40);
+
+ save_client = client;
+ return 0;
+}
+
+static int
+m41t85_attach(struct i2c_adapter *adap)
+{
+ return i2c_probe(adap, &addr_data, m41t85_probe);
+}
+
+static int
+m41t85_detach(struct i2c_client *client)
+{
+ int rc;
+
+ if ((rc = i2c_detach_client(client)) == 0) {
+ kfree(client);
+ tasklet_kill(&m41t85_tasklet);
+ }
+ return rc;
+}
+
+static struct i2c_driver m41t85_driver = {
+ .owner = THIS_MODULE,
+ .name = M41T85_DRV_NAME,
+ .id = I2C_DRIVERID_STM41T85,
+ .flags = I2C_DF_NOTIFY,
+ .attach_adapter = m41t85_attach,
+ .detach_client = m41t85_detach,
+};
+
+static int __init
+m41t85_init(void)
+{
+ return i2c_add_driver(&m41t85_driver);
+}
+
+static void __exit
+m41t85_exit(void)
+{
+ i2c_del_driver(&m41t85_driver);
+ return;
+}
+
+module_init(m41t85_init);
+module_exit(m41t85_exit);
+
+MODULE_AUTHOR("Andrey Volkov <[email protected]>");
+MODULE_DESCRIPTION("ST Microelectronics M41T85 RTC I2C Client Driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/i2c-id.h b/include/linux/i2c-id.h
index 5c05f52..ac8ffcd 100644
--- a/include/linux/i2c-id.h
+++ b/include/linux/i2c-id.h
@@ -108,6 +108,7 @@
#define I2C_DRIVERID_SAA7127 72 /* saa7124 video encoder */
#define I2C_DRIVERID_SAA711X 73 /* saa711x video encoders */
#define I2C_DRIVERID_AKITAIOEXP 74 /* IO Expander on Sharp SL-C1000 */
+#define I2C_DRIVERID_STM41T85 75 /* real time clock */

#define I2C_DRIVERID_EXP0 0xF0 /* experimental use id's */
#define I2C_DRIVERID_EXP1 0xF1


Attachments:
m41t85_rtc.diff (11.11 kB)

2005-11-15 00:41:46

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH 1/1] Added support of ST m41t85 rtc chip

Andrey Volkov <[email protected]> wrote:
>
> ...
> Added support of ST M41T85 RTC
>
> ...
>
> +ulong
> +m41t85_get_rtc_time(void)

Does this need to have global scope?

It appears to have no callers.

It is preferred that `unsigned long' be used.

Please format it this way:

unsigned long m41t85_get_rtc_time(void)


> +static void
> +m41t85_set_tlet(ulong arg)
> +{
> + struct rtc_time tm;
> + ulong nowtime = *(ulong *)arg;
> +
> + to_tm(nowtime, &tm);
> + tm.tm_year = (tm.tm_year - 1900) % 100;
> +
> + tm.tm_sec = BIN2BCD(tm.tm_sec);
> + tm.tm_min = BIN2BCD(tm.tm_min);
> + tm.tm_hour = BIN2BCD(tm.tm_hour);
> + tm.tm_mon = BIN2BCD(tm.tm_mon);
> + tm.tm_mday = BIN2BCD(tm.tm_mday);
> + tm.tm_year = BIN2BCD(tm.tm_year);
> +
> + down(&m41t85_mutex);

Cannot do down() in a tasklet handler! Enable CONFIG_DEBUG_PREEMPT and
CONFIG_DEBUG_SPINLOCK_SLEEP, retest.

schedule_work() might be an appropriate fix.

> +int
> +m41t85_set_rtc_time(ulong nowtime)
> +{
> + new_time = nowtime;
> +
> + if (in_interrupt())
> + tasklet_schedule(&m41t85_tasklet);
> + else
> + m41t85_set_tlet((ulong)&new_time);
> +
> + return 0;
> +}

hm, this function isn't referenced from within this patch either.

> + #if defined (CONFIG_SENSORS_M41T85_SQW_FRQ)

#if's normally start in column zero.

> + ret = i2c_smbus_write_byte_data(client, RTC_SQW_ADDR, CONFIG_SENSORS_M41T85_SQW_FRQ);

My, what large xterms you have ;)


2005-11-15 20:51:55

by Jean Delvare

[permalink] [raw]
Subject: Re: [PATCH 1/1] Added support of ST m41t85 rtc chip

Hi Andrey,

> Possible too late to include in 2.6.15,
> but better later then never :).

You must be kidding. It might be too late for 2.6._16_. Reviewing takes
time, reworking afterwards takes time, then you get some testing in -mm
and it takes time again.

> Comments?

Sure, although I don't really have the time right now for a complete
review. And I'd rather not review the code if it finally isn't used.

First, a question. Can't you merge the M41T85 support into the m41t00
driver?

Mark, care to comment on that possibility, and/or on the code itself?

> +config SENSORS_M41T85
> + tristate "ST M41T85 RTC chip"
> + depends on I2C

I would pretty much likt it named RTC_M41T85 or RTC_M41T85_I2C instead.
It's not a sensor in any way. And it must depend on EXPERIMENTAL to
start with.

> +config SENSORS_M41T85_SQW_FRQ_ENABLE
> + depends on SENSORS_M41T85
> + bool "Square Wave Output"

What a mess. Please just have a sysfs file for that, it's more flexible
and less intrusive.

> +/*
> + * drivers/i2c/chips/m41t85.c
> + *

Redundant, you know where the file is if you found it.

> + * HISTORY:
> + * 2005-10-12 Created
> + */

History doesn't belong to the source code.

> +/*
> + * This i2c client/driver wedges between the drivers/char/genrtc.c RTC
> + * interface and the SMBus interface of the i2c subsystem.
> + * It would be more efficient to use i2c msgs/i2c_transfer directly but, as
> + * recommened in .../Documentation/i2c/writing-clients section
> + * "Sending and receiving", using SMBus level communication is preferred.
> + */

The i2c_transfer interface wouldn't exist if nobody was ever supposed
to use it. Using the SMBus compatible interface makes it possible to
use your chip in conjunction with more I2C or SMBus masters. But maybe
you just need a specific driver for a specific case, and you just don't
care about the other cases. In that case, go with i2c_transfer for
smaller and faster code.

Also, use <file:Documentation/...> for referencing document files.

> +#define M41T85_DRV_NAME "m41t85"

Please define m41t85_driver.name directly, and use references to it
where needed. This avoids duplicating string constants.

> +static void
> +m41t85_set_tlet(ulong arg)
> +{
> + struct rtc_time tm;
> + ulong nowtime = *(ulong *)arg;

What's the idea behind this cast?

> + strncpy(client->name, M41T85_DRV_NAME, I2C_NAME_SIZE);

Please use strlcpy instead.

> +static void __exit
> +m41t85_exit(void)
> +{
> + i2c_del_driver(&m41t85_driver);
> + return;
> +}

Drop this "return" statement, it is useless.

Thanks,
--
Jean Delvare

2005-11-15 21:24:22

by Andrey Volkov

[permalink] [raw]
Subject: Re: [PATCH 1/1] Added support of ST m41t85 rtc chip

Andrew Morton wrote:
> Andrey Volkov <[email protected]> wrote:
>
>>...
>>Added support of ST M41T85 RTC
>>
>>...
>>
>>+ulong
>>+m41t85_get_rtc_time(void)
>
>
> Does this need to have global scope?
>
> It appears to have no callers.

I use this function(s) in platform driver (which still in dev stage) of
our board as platform get_rtc_time/set_rtc_time by same way as Mark in
the katana does.

May be better create special header in include/linux?
(And convert m41t80.c and arch/ppc/katana.c too)

>>+static void
>>+m41t85_set_tlet(ulong arg)
>>+{
>>+ struct rtc_time tm;
>>+ ulong nowtime = *(ulong *)arg;
>>+
>>+ to_tm(nowtime, &tm);
>>+ tm.tm_year = (tm.tm_year - 1900) % 100;
>>+
>>+ tm.tm_sec = BIN2BCD(tm.tm_sec);
>>+ tm.tm_min = BIN2BCD(tm.tm_min);
>>+ tm.tm_hour = BIN2BCD(tm.tm_hour);
>>+ tm.tm_mon = BIN2BCD(tm.tm_mon);
>>+ tm.tm_mday = BIN2BCD(tm.tm_mday);
>>+ tm.tm_year = BIN2BCD(tm.tm_year);
>>+
>>+ down(&m41t85_mutex);
>
>
> Cannot do down() in a tasklet handler! Enable CONFIG_DEBUG_PREEMPT and
> CONFIG_DEBUG_SPINLOCK_SLEEP, retest.

Oops, you're right. It's copy-paste bug from m41t00.c
(which then buggy too).

>
> schedule_work() might be an appropriate fix.
>
>
>>+int
>>+m41t85_set_rtc_time(ulong nowtime)
>>+{
>>+ new_time = nowtime;
>>+
>>+ if (in_interrupt())
>>+ tasklet_schedule(&m41t85_tasklet);
>>+ else
>>+ m41t85_set_tlet((ulong)&new_time);
>>+
>>+ return 0;
>>+}
>
>
> hm, this function isn't referenced from within this patch either.

Same as above.

>
>
>>+ #if defined (CONFIG_SENSORS_M41T85_SQW_FRQ)
>
>
> #if's normally start in column zero.
>
>
>>+ ret = i2c_smbus_write_byte_data(client, RTC_SQW_ADDR, CONFIG_SENSORS_M41T85_SQW_FRQ);
>
>
> My, what large xterms you have ;)

Tabs=4 and 1280 full screened :). Ok I fix it to 80 columns.

--
Regards
Andrey Volkov

2005-11-15 21:49:05

by Andrey Volkov

[permalink] [raw]
Subject: Re: [PATCH 1/1] Added support of ST m41t85 rtc chip

Jean Delvare wrote:
> Hi Andrey,
>
>
>>Possible too late to include in 2.6.15,
>>but better later then never :).
>
>
> You must be kidding. It might be too late for 2.6._16_. Reviewing takes
> time, reworking afterwards takes time, then you get some testing in -mm
> and it takes time again.
>
:((.

>
>>Comments?
>
>
> Sure, although I don't really have the time right now for a complete
> review. And I'd rather not review the code if it finally isn't used.
(see my prev. reply to Andrew and you)

>
> First, a question. Can't you merge the M41T85 support into the m41t00
> driver?
It was first thing what I try, but this chips are very similar only at
first glance. m41t85 have _really_ extended sets of regs and result was
very littered by #if/#else file.

>
> Mark, care to comment on that possibility, and/or on the code itself?
>
And, please, remove unnecessary PPC dependence from Kconfig.

>>+config SENSORS_M41T85_SQW_FRQ_ENABLE
>>+ depends on SENSORS_M41T85
>>+ bool "Square Wave Output"
>
>
> What a mess. Please just have a sysfs file for that, it's more flexible
> and less intrusive.

I agree, it's look messed, but if sombody use SQW,
then must exist some startup constant for some custom board.
Changing this frq may exist only as option.

--
Regards
Andrey Volkov

2005-11-16 02:56:41

by Mark A. Greer

[permalink] [raw]
Subject: Re: [PATCH 1/1] Added support of ST m41t85 rtc chip

Hi Jean,

On Tue, Nov 15, 2005 at 09:52:26PM +0100, Jean Delvare wrote:

<snip>

> First, a question. Can't you merge the M41T85 support into the m41t00
> driver?
>
> Mark, care to comment on that possibility, and/or on the code itself?

Sure.

I wrestled with the ST website for the m41t85 datasheet but lost so I
I can only guess from the patch. The drivers do look very similar.
It looks like the m41t85 is basically a m41t00 with an alarm (watchdog
timer never used AFAICT). Also there are some differences in register
offsets and [maybe] some minor differences within the registers but
nothing that serious.

I think we can combine the two into an m41txx.c and pass the exact type
in via platform_data--that would be the correct mechanism, right?
The platform_data could also be used to seed the correct SQW freq and
eliminate all the Kconfig noise.

Comments?

As for Jean's and Andrew's comments about the driver, they seem valid
to me and should be addressed. In Andrey's defense, many of them are my
fault. Once there is a consensus on the merging m41t00 & m41t85
question, I'll try to get a fixed up patch within a couple weeks.

Mark

2005-11-16 03:14:48

by Mark A. Greer

[permalink] [raw]
Subject: Re: [PATCH 1/1] Added support of ST m41t85 rtc chip

On Wed, Nov 16, 2005 at 12:48:59AM +0300, Andrey Volkov wrote:

<snip>

> >
> > Mark, care to comment on that possibility, and/or on the code itself?
> >
> And, please, remove unnecessary PPC dependence from Kconfig.

When I originally submitted the m41t00 patch, I made it clear that it
was PPC only and gave the reason why:

http://archives.andrew.net.au/lm-sensors/msg29280.html

What processor arch did you test your m41t85 driver on?
AFAICT, PPC is the only arch that uses that exact definition of
get/set_rtc_time().

Mark

2005-11-16 14:45:49

by Andrey Volkov

[permalink] [raw]
Subject: Re: [PATCH 1/1] Added support of ST m41t85 rtc chip



Mark A. Greer wrote:
> Hi Jean,
>
> On Tue, Nov 15, 2005 at 09:52:26PM +0100, Jean Delvare wrote:
>
> <snip>
>
>>First, a question. Can't you merge the M41T85 support into the m41t00
>>driver?
>>
>>Mark, care to comment on that possibility, and/or on the code itself?
>
>
> Sure.
>
> I wrestled with the ST website for the m41t85 datasheet but lost so I
> I can only guess from the patch. The drivers do look very similar.
> It looks like the m41t85 is basically a m41t00 with an alarm (watchdog
> timer never used AFAICT).
http://www.st.com/stonline/products/literature/ds/7531/m41st85w.pdf
(I agree ST's (and Maxim's too) site is for strength of mind men :) ).
Also added onchip clock, battery and poweroff time
(HT == Halt Timestamp).

> Also there are some differences in register
> offsets and [maybe] some minor differences within the registers but
> nothing that serious.
Mainly you're right, but, as I wrote before, due to _many_ little
differences I get #if/#else.. noise (half file, if be more precisely,
was under #if/#else) in unified file. But, if this noise
will be acceptable, then yes, I agree to merge this drivers, as minimum,
for better administration.

<snip>

--
Regards
Andrey Volkov

2005-11-16 14:50:34

by Andrey Volkov

[permalink] [raw]
Subject: Re: [PATCH 1/1] Added support of ST m41t85 rtc chip



Mark A. Greer wrote:

> When I originally submitted the m41t00 patch, I made it clear that it
> was PPC only and gave the reason why:
>
> http://archives.andrew.net.au/lm-sensors/msg29280.html
>
> What processor arch did you test your m41t85 driver on?
PPC32 :) (MPC5200).

> AFAICT, PPC is the only arch that uses that exact definition of
> get/set_rtc_time().
But chip (and driver) itself may be used in any i2c net with any
chip as master with slightly modification of platform driver.
Am I right?.

--
Regards
Andrey Volkov

2005-11-16 15:34:17

by Jean Delvare

[permalink] [raw]
Subject: Re: [PATCH 1/1] Added support of ST m41t85 rtc chip


Hi Andrey,

On 2005-11-16, Andrey Volkov wrote:
> > I wrestled with the ST website for the m41t85 datasheet but lost so I
> > I can only guess from the patch. The drivers do look very similar.
> > It looks like the m41t85 is basically a m41t00 with an alarm (watchdog
> > timer never used AFAICT).
>
> http://www.st.com/stonline/products/literature/ds/7531/m41st85w.pdf
> (I agree ST's (and Maxim's too) site is for strength of mind men :) ).

We're sliding off-topic, but I find Maxim's website quite good.

> > Also there are some differences in register
> > offsets and [maybe] some minor differences within the registers but
> > nothing that serious.
>
> Mainly you're right, but, as I wrote before, due to _many_ little
> differences I get #if/#else.. noise (half file, if be more precisely,
> was under #if/#else) in unified file. But, if this noise
> will be acceptable, then yes, I agree to merge this drivers, as minimum,
> for better administration.

This simply means you did it the wrong way. The use of #if/#else/#endif
suggests that you made the distinction between chips at compilation
time, and the generated driver binary ended up supporting only one of
the two chips. This doesn't make any sense. Same is true for your
output clock frequency, making it a kernel configuration option means
that the decision happens at compilation time, which is very restrictive.

Think of what will happen when a Linux distribution has to build a kernel
for their next release. How are they going to build a kernel suitable
for all systems? With your approach, they just won't be able to.
They'd have to build one kernel with m41t00 support, and one with
m41t85 support. Even worse, they would need one separate build for each
variant of the m41t85 driver, and there are over a dozen of these (one
without clock output, and one more for each allowed clock frequency).
That's obviously insane.

This is why you want to move the decision at the run time level rather
than the compilation time level whenever possible. The clock output
option can't be a configuration option. I previously suggested a sysfs
file, because this gives the greatest flexibility. If you don't like
the idea for whatever reason, you may go for a module parameter instead.

Same reasonment holds for the m41t00 vs. m41t85 choice. You can't decide
at compilation time. If we go for a common driver, it has to support
both devices at the same time. Mark suggested to use platform-specific
data. I'm not familiar with this, but it sounds reasonable.

I don't know for sure at this point whether having a single driver is
the right choice, I'll let you and Mark check it out and decide. But
the right way to determine this is definitely not through the use of
#if/#endif preprocessing stuff.

Thanks,
--
Jean Delvare

2005-11-16 16:43:38

by Andrey Volkov

[permalink] [raw]
Subject: Re: [PATCH 1/1] Added support of ST m41t85 rtc chip

Hi Jean

Jean Delvare wrote:
> Hi Andrey,
>
> On 2005-11-16, Andrey Volkov wrote:
>
>>>I wrestled with the ST website for the m41t85 datasheet but lost so I
>>>I can only guess from the patch. The drivers do look very similar.
>>>It looks like the m41t85 is basically a m41t00 with an alarm (watchdog
>>>timer never used AFAICT).
>>
>>http://www.st.com/stonline/products/literature/ds/7531/m41st85w.pdf
>>(I agree ST's (and Maxim's too) site is for strength of mind men :) ).
>
>
> We're sliding off-topic, but I find Maxim's website quite good.
Try select chip by interface :) (Timekeeping & RTC as ex).
But Maxim site is better, it's not disputed.
>
>
>>>Also there are some differences in register
>>>offsets and [maybe] some minor differences within the registers but
>>>nothing that serious.
>>
>>Mainly you're right, but, as I wrote before, due to _many_ little
>>differences I get #if/#else.. noise (half file, if be more precisely,
>>was under #if/#else) in unified file. But, if this noise
>>will be acceptable, then yes, I agree to merge this drivers, as minimum,
>>for better administration.
>

<snip>

> Same reasonment holds for the m41t00 vs. m41t85 choice. You can't decide
> at compilation time. If we go for a common driver, it has to support
> both devices at the same time. Mark suggested to use platform-specific
> data. I'm not familiar with this, but it sounds reasonable.
It's pity, but all chips of m41xx family (approx. 20 members) have same
i2c slave address (0x68) and have not some hwd model specific id
registers. Hence I couldn't made any clue about chip specific
regs/pins/... at run-time (and couldn't use two or more chips in one i2c
subnet :)). And the situation worsens, as various chips have various
time regs offsets, as ex. REG_SEC of m41t00 have offset 0, but
in m41t85 _SAME_ REG_SEC have offset 1, etc (God, who in ST was so,
hm, ...., so made such decision?).

So I coldn't see any suitable run-time method of detection which
function driver could use for current "unknown" m41xx chip (from driver
probe point of view) whithout Kconfig/module parameter prompting.

>
> I don't know for sure at this point whether having a single driver is
> the right choice, I'll let you and Mark check it out and decide. But
> the right way to determine this is definitely not through the use of
> #if/#endif preprocessing stuff.
IMHO - single driver for all m41xx will be nice thing, but I'm not
sure that it will not be monster- alike when it be released.

--
Regards
Andrey Volkov

2005-11-16 18:55:15

by Andy Isaacson

[permalink] [raw]
Subject: Re: [PATCH 1/1] Added support of ST m41t85 rtc chip

On Tue, Nov 15, 2005 at 08:15:20PM -0700, Mark A. Greer wrote:
> On Wed, Nov 16, 2005 at 12:48:59AM +0300, Andrey Volkov wrote:
> > > Mark, care to comment on that possibility, and/or on the code itself?
> > >
> > And, please, remove unnecessary PPC dependence from Kconfig.
>
> When I originally submitted the m41t00 patch, I made it clear that it
> was PPC only and gave the reason why:
>
> http://archives.andrew.net.au/lm-sensors/msg29280.html

I have a MIPS platform with M41T81 (SiByte SWARM,
arch/mips/sibyte/swarm/rtc_m41t81.c) that I would dearly love to
decruftify. (Don't pay too much attention to the kernel.org trees for
MIPS, you need to pull the git repo on linux-mips.org for the real
stuff.)

So I'm definitely interested in this work.

-andy

2005-11-16 21:24:14

by Mark A. Greer

[permalink] [raw]
Subject: Re: [PATCH 1/1] Added support of ST m41t85 rtc chip

On Wed, Nov 16, 2005 at 04:19:58PM +0100, Jean Delvare wrote:

<snip>

> This is why you want to move the decision at the run time level rather
> than the compilation time level whenever possible. The clock output
> option can't be a configuration option. I previously suggested a sysfs
> file, because this gives the greatest flexibility. If you don't like
> the idea for whatever reason, you may go for a module parameter instead.
>
> Same reasonment holds for the m41t00 vs. m41t85 choice. You can't decide
> at compilation time. If we go for a common driver, it has to support
> both devices at the same time. Mark suggested to use platform-specific
> data. I'm not familiar with this, but it sounds reasonable.

I don't know the entire history behind platform_data but my
understanding is that it was designed to provide a mechanism for
platform-specific code to pass info to drivers.

I *believe* that this would be a proper use of platform_data but I'm
hoping someone out in lkml-land who knows better than I will confirm that.

> I don't know for sure at this point whether having a single driver is
> the right choice, I'll let you and Mark check it out and decide. But
> the right way to determine this is definitely not through the use of
> #if/#endif preprocessing stuff.

I agree. We can and absolutely should do this at run-time if a merged
driver is feasible. I'll dig thru the datasheets today and start
prototyping some code if it looks like we can merge the code.

Mark

2005-11-16 21:35:51

by Mark A. Greer

[permalink] [raw]
Subject: Re: [PATCH 1/1] Added support of ST m41t85 rtc chip

On Wed, Nov 16, 2005 at 07:43:29PM +0300, Andrey Volkov wrote:

<snip>

> > Same reasonment holds for the m41t00 vs. m41t85 choice. You can't decide
> > at compilation time. If we go for a common driver, it has to support
> > both devices at the same time. Mark suggested to use platform-specific
> > data. I'm not familiar with this, but it sounds reasonable.
> It's pity, but all chips of m41xx family (approx. 20 members) have same
> i2c slave address (0x68) and have not some hwd model specific id
> registers. Hence I couldn't made any clue about chip specific
> regs/pins/... at run-time

The hope is that your platform determines the type of rtc chip and then
passes that into the m41txx driver. If your platform can have different
rtc chips and no way to determine which one is there at runtime, we have
a problem.

> (and couldn't use two or more chips in one i2c subnet :)).
> And the situation worsens, as various chips have various
> time regs offsets, as ex. REG_SEC of m41t00 have offset 0, but
> in m41t85 _SAME_ REG_SEC have offset 1, etc (God, who in ST was so,
> hm, ...., so made such decision?).

I'll peruse the datasheets to see how feasible it is to merge. At this
point, I think its possible but that may be my ignorance talking.

> So I coldn't see any suitable run-time method of detection which
> function driver could use for current "unknown" m41xx chip (from driver
> probe point of view) whithout Kconfig/module parameter prompting.

See above.

> > I don't know for sure at this point whether having a single driver is
> > the right choice, I'll let you and Mark check it out and decide. But
> > the right way to determine this is definitely not through the use of
> > #if/#endif preprocessing stuff.
> IMHO - single driver for all m41xx will be nice thing, but I'm not
> sure that it will not be monster- alike when it be released.

Understood. I'll see what I can come up with. If it looks reasonable,
I'll run it past everyone.

Mark

2005-11-16 22:23:51

by Mark A. Greer

[permalink] [raw]
Subject: Re: [PATCH 1/1] Added support of ST m41t85 rtc chip

On Wed, Nov 16, 2005 at 10:55:13AM -0800, Andy Isaacson wrote:
> On Tue, Nov 15, 2005 at 08:15:20PM -0700, Mark A. Greer wrote:
> > On Wed, Nov 16, 2005 at 12:48:59AM +0300, Andrey Volkov wrote:
> > > > Mark, care to comment on that possibility, and/or on the code itself?
> > > >
> > > And, please, remove unnecessary PPC dependence from Kconfig.
> >
> > When I originally submitted the m41t00 patch, I made it clear that it
> > was PPC only and gave the reason why:
> >
> > http://archives.andrew.net.au/lm-sensors/msg29280.html
>
> I have a MIPS platform with M41T81 (SiByte SWARM,
> arch/mips/sibyte/swarm/rtc_m41t81.c) that I would dearly love to
> decruftify. (Don't pay too much attention to the kernel.org trees for
> MIPS, you need to pull the git repo on linux-mips.org for the real
> stuff.)
>
> So I'm definitely interested in this work.

Thanks Andy. I'll check out the m41t81 datasheet and see how well it
blends with the others. Also, ppc and mips will have different
interfaces but it should be acceptable to have two sets of interface
routines with #ifdef CONFIG_PPC/MIPS around them.

I'm off to read datasheets now... :)

Mark

2005-11-17 09:34:32

by Jean Delvare

[permalink] [raw]
Subject: Re: [PATCH 1/1] Added support of ST m41t85 rtc chip


Hi Andrey,

On 2005-11-16, Andrey Volkov wrote:
> It's pity, but all chips of m41xx family (approx. 20 members) have same
> i2c slave address (0x68) and have not some hwd model specific id
> registers. Hence I couldn't made any clue about chip specific
> regs/pins/... at run-time (and couldn't use two or more chips in one i2c
> subnet :)). And the situation worsens, as various chips have various
> time regs offsets, as ex. REG_SEC of m41t00 have offset 0, but
> in m41t85 _SAME_ REG_SEC have offset 1, etc (God, who in ST was so,
> hm, ...., so made such decision?).

I wholeheartedly second your views, it would be way way better if all I2C
chips had an ID register. However, having a separate driver for every
chip doesn't actually solve the undifferenciability issue. You can't
prevent the user from loading the wrong driver. Also, the problem isn't
specific to the m41txx family of chips, many other families of chips
suffer the same.

The main problem here is that i2c-core currently lacks a mechanism which
would let us explicitely attach a given driver (and chip type within
that driver) to an arbitrary adapter, address pair. I will be working on
implementing this with the help of others in the next few weeks. RTC
drivers and media/video drivers should enjoy this new interface.

Thanks,
--
Jean Delvare

2005-11-18 20:35:13

by Mark A. Greer

[permalink] [raw]
Subject: Re: [PATCH 1/1] Added support of ST m41t85 rtc chip

On Wed, Nov 16, 2005 at 03:24:23PM -0700, Mark A. Greer wrote:

> I'm off to read datasheets now... :)

Well, the people who sign my paychecks wanted me to do something else
for the last couple days so I haven't looked at this yet. And now I'm
leaving soon and will be taking next week off so I won't even start on
this until the week of Nov 28. Sorry...

Mark

2005-11-21 12:35:40

by Andrey Volkov

[permalink] [raw]
Subject: Re: [PATCH 1/1] Added support of ST m41t85 rtc chip



Mark A. Greer wrote:
> On Wed, Nov 16, 2005 at 03:24:23PM -0700, Mark A. Greer wrote:
>
>
>>I'm off to read datasheets now... :)
>
>
> Well, the people who sign my paychecks wanted me to do something else
> for the last couple days so I haven't looked at this yet. And now I'm
> leaving soon and will be taking next week off so I won't even start on
> this until the week of Nov 28. Sorry...
>
> Mark

Ok, it's not a problem to wait for me (since anyway we couldn't release
it before 2.6.15).

Mark, may I help you?

--
Regards
Andrey Volkov

2005-12-06 21:17:20

by Mark A. Greer

[permalink] [raw]
Subject: Re: [PATCH 1/1] Added support of ST m41t85 rtc chip

On Mon, Nov 21, 2005 at 03:35:35PM +0300, Andrey Volkov wrote:
>
>
> Mark A. Greer wrote:
> > On Wed, Nov 16, 2005 at 03:24:23PM -0700, Mark A. Greer wrote:
> >
> >
> >>I'm off to read datasheets now... :)
> >
> >
> > Well, the people who sign my paychecks wanted me to do something else
> > for the last couple days so I haven't looked at this yet. And now I'm
> > leaving soon and will be taking next week off so I won't even start on
> > this until the week of Nov 28. Sorry...
> >
> > Mark
>
> Ok, it's not a problem to wait for me (since anyway we couldn't release
> it before 2.6.15).
>
> Mark, may I help you?

Sorry for ignoring you Andrey, I've been totally consumed on other
things until now.

Thanks for the offer but I doubt its a big enough project to warrant two
people. I have a couple patches to a different driver to make & submit
then I'll work on this. I hope to have a patch for this sometime next
week sometime. I would certainly appreciate a thorough review of the
patch, though.

Mark

2005-12-19 21:05:05

by Mark A. Greer

[permalink] [raw]
Subject: [RFC] i2c: Combined ST m41txx i2c rtc chip driver (was: [PATCH 1/1] Added support of ST m41t85 rtc chip)

On Tue, Nov 15, 2005 at 07:57:14PM -0700, Mark A. Greer wrote:
> I think we can combine the two into an m41txx.c and pass the exact type
> in via platform_data--that would be the correct mechanism, right?
> The platform_data could also be used to seed the correct SQW freq and
> eliminate all the Kconfig noise.
>
> Comments?
>
> As for Jean's and Andrew's comments about the driver, they seem valid
> to me and should be addressed. In Andrey's defense, many of them are my
> fault. Once there is a consensus on the merging m41t00 & m41t85
> question, I'll try to get a fixed up patch within a couple weeks.

I've made what I think should be close to an acceptable driver that
supports the m41t00, m41t81, and m41t85 i2c rtc chips. I only have hw
to test the m41t00 on a ppc platform, though.

It was suggested to me a while back to add retries when reading &
writing the rtc regs. Personally, I think its overkill but I added the
code anyway. You will see in m41txx_get_rtc_time() that 3 tries are
made to get a good read of all the regs then up to 10 tries are made to
make sure that the time didn't change while we were doing the reads.
Something similar is done in m41txx_set().

Andrey, would you please apply and test this patch on your 5200?

Andy, I apologize but I didn't take the time to look at the mips tree in
depth. I did do a quick scan and I think that you should be able to set
'rtc_get_time' to 'm41txx_get_rtc_time' and 'rtc_set_time' to
'm41txx_set_rtc_time', and it'll work. You'll have to hook up the proper
driver for your host i2c ctlr, though.

Jean, Andrew, I think I addressed your coding style, etc. issues with
the driver. If not, just point out the problems an I'll fix them.

The patch is against the 2.6.15-rc5-mm3 tree. Comments welcome.

[I will send a second patch that contains the changes I made to the ppc
katana platform that has the m41t00 that I tested with.]

Thanks,

Mark
---
drivers/i2c/chips/Kconfig | 19 +-
drivers/i2c/chips/Makefile | 2
drivers/i2c/chips/m41t00.c | 240 --------------------------------
drivers/i2c/chips/m41txx.c | 336 +++++++++++++++++++++++++++++++++++++++++++++
include/linux/i2c-id.h | 2
include/linux/m41txx.h | 28 +++
6 files changed, 376 insertions(+), 251 deletions(-)
---
diff -Nurp linux-2.6.15-rc5-mm3.orig/drivers/i2c/chips/Kconfig linux-2.6.15-rc5-mm3-m41txx/drivers/i2c/chips/Kconfig
--- linux-2.6.15-rc5-mm3.orig/drivers/i2c/chips/Kconfig 2005-12-03 22:10:42.000000000 -0700
+++ linux-2.6.15-rc5-mm3-m41txx/drivers/i2c/chips/Kconfig 2005-12-19 13:16:25.000000000 -0700
@@ -102,15 +102,6 @@ config TPS65010
This driver can also be built as a module. If so, the module
will be called tps65010.

-config SENSORS_M41T00
- tristate "ST M41T00 RTC chip"
- depends on I2C && PPC32
- help
- If you say yes here you get support for the ST M41T00 RTC chip.
-
- This driver can also be built as a module. If so, the module
- will be called m41t00.
-
config SENSORS_MAX6875
tristate "Maxim MAX6875 Power supply supervisor"
depends on I2C && EXPERIMENTAL
@@ -135,4 +126,14 @@ config RTC_X1205_I2C
This driver can also be built as a module. If so, the module
will be called x1205.

+config RTC_M41TXX_I2C
+ tristate "ST M41TXX Family of I2C RTC chips"
+ depends on I2C
+ help
+ If you say yes here you get support for the ST M41TXX family
+ of I2C RTC chips.
+
+ This driver can also be built as a module. If so, the module
+ will be called m41t00.
+
endmenu
diff -Nurp linux-2.6.15-rc5-mm3.orig/drivers/i2c/chips/m41t00.c linux-2.6.15-rc5-mm3-m41txx/drivers/i2c/chips/m41t00.c
--- linux-2.6.15-rc5-mm3.orig/drivers/i2c/chips/m41t00.c 2005-12-19 13:04:07.000000000 -0700
+++ linux-2.6.15-rc5-mm3-m41txx/drivers/i2c/chips/m41t00.c 1969-12-31 17:00:00.000000000 -0700
@@ -1,240 +0,0 @@
-/*
- * drivers/i2c/chips/m41t00.c
- *
- * I2C client/driver for the ST M41T00 Real-Time Clock chip.
- *
- * Author: Mark A. Greer <[email protected]>
- *
- * 2005 (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.
- */
-/*
- * This i2c client/driver wedges between the drivers/char/genrtc.c RTC
- * interface and the SMBus interface of the i2c subsystem.
- * It would be more efficient to use i2c msgs/i2c_transfer directly but, as
- * recommened in .../Documentation/i2c/writing-clients section
- * "Sending and receiving", using SMBus level communication is preferred.
- */
-
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/interrupt.h>
-#include <linux/i2c.h>
-#include <linux/rtc.h>
-#include <linux/bcd.h>
-
-#include <asm/time.h>
-#include <asm/rtc.h>
-
-#define M41T00_DRV_NAME "m41t00"
-
-static DECLARE_MUTEX(m41t00_mutex);
-
-static struct i2c_driver m41t00_driver;
-static struct i2c_client *save_client;
-
-static unsigned short ignore[] = { I2C_CLIENT_END };
-static unsigned short normal_addr[] = { 0x68, I2C_CLIENT_END };
-
-static struct i2c_client_address_data addr_data = {
- .normal_i2c = normal_addr,
- .probe = ignore,
- .ignore = ignore,
-};
-
-ulong
-m41t00_get_rtc_time(void)
-{
- s32 sec, min, hour, day, mon, year;
- s32 sec1, min1, hour1, day1, mon1, year1;
- ulong limit = 10;
-
- sec = min = hour = day = mon = year = 0;
- sec1 = min1 = hour1 = day1 = mon1 = year1 = 0;
-
- down(&m41t00_mutex);
- do {
- if (((sec = i2c_smbus_read_byte_data(save_client, 0)) >= 0)
- && ((min = i2c_smbus_read_byte_data(save_client, 1))
- >= 0)
- && ((hour = i2c_smbus_read_byte_data(save_client, 2))
- >= 0)
- && ((day = i2c_smbus_read_byte_data(save_client, 4))
- >= 0)
- && ((mon = i2c_smbus_read_byte_data(save_client, 5))
- >= 0)
- && ((year = i2c_smbus_read_byte_data(save_client, 6))
- >= 0)
- && ((sec == sec1) && (min == min1) && (hour == hour1)
- && (day == day1) && (mon == mon1)
- && (year == year1)))
-
- break;
-
- sec1 = sec;
- min1 = min;
- hour1 = hour;
- day1 = day;
- mon1 = mon;
- year1 = year;
- } while (--limit > 0);
- up(&m41t00_mutex);
-
- if (limit == 0) {
- dev_warn(&save_client->dev,
- "m41t00: can't read rtc chip\n");
- sec = min = hour = day = mon = year = 0;
- }
-
- sec &= 0x7f;
- min &= 0x7f;
- hour &= 0x3f;
- day &= 0x3f;
- mon &= 0x1f;
- year &= 0xff;
-
- BCD_TO_BIN(sec);
- BCD_TO_BIN(min);
- BCD_TO_BIN(hour);
- BCD_TO_BIN(day);
- BCD_TO_BIN(mon);
- BCD_TO_BIN(year);
-
- year += 1900;
- if (year < 1970)
- year += 100;
-
- return mktime(year, mon, day, hour, min, sec);
-}
-
-static void
-m41t00_set_tlet(ulong arg)
-{
- struct rtc_time tm;
- ulong nowtime = *(ulong *)arg;
-
- to_tm(nowtime, &tm);
- tm.tm_year = (tm.tm_year - 1900) % 100;
-
- BIN_TO_BCD(tm.tm_sec);
- BIN_TO_BCD(tm.tm_min);
- BIN_TO_BCD(tm.tm_hour);
- BIN_TO_BCD(tm.tm_mon);
- BIN_TO_BCD(tm.tm_mday);
- BIN_TO_BCD(tm.tm_year);
-
- down(&m41t00_mutex);
- if ((i2c_smbus_write_byte_data(save_client, 0, tm.tm_sec & 0x7f) < 0)
- || (i2c_smbus_write_byte_data(save_client, 1, tm.tm_min & 0x7f)
- < 0)
- || (i2c_smbus_write_byte_data(save_client, 2, tm.tm_hour & 0x7f)
- < 0)
- || (i2c_smbus_write_byte_data(save_client, 4, tm.tm_mday & 0x7f)
- < 0)
- || (i2c_smbus_write_byte_data(save_client, 5, tm.tm_mon & 0x7f)
- < 0)
- || (i2c_smbus_write_byte_data(save_client, 6, tm.tm_year & 0x7f)
- < 0))
-
- dev_warn(&save_client->dev,"m41t00: can't write to rtc chip\n");
-
- up(&m41t00_mutex);
- return;
-}
-
-static ulong new_time;
-
-DECLARE_TASKLET_DISABLED(m41t00_tasklet, m41t00_set_tlet, (ulong)&new_time);
-
-int
-m41t00_set_rtc_time(ulong nowtime)
-{
- new_time = nowtime;
-
- if (in_interrupt())
- tasklet_schedule(&m41t00_tasklet);
- else
- m41t00_set_tlet((ulong)&new_time);
-
- return 0;
-}
-
-/*
- *****************************************************************************
- *
- * Driver Interface
- *
- *****************************************************************************
- */
-static int
-m41t00_probe(struct i2c_adapter *adap, int addr, int kind)
-{
- struct i2c_client *client;
- int rc;
-
- client = kzalloc(sizeof(struct i2c_client), GFP_KERNEL);
- if (!client)
- return -ENOMEM;
-
- strncpy(client->name, M41T00_DRV_NAME, I2C_NAME_SIZE);
- client->addr = addr;
- client->adapter = adap;
- client->driver = &m41t00_driver;
-
- if ((rc = i2c_attach_client(client)) != 0) {
- kfree(client);
- return rc;
- }
-
- save_client = client;
- return 0;
-}
-
-static int
-m41t00_attach(struct i2c_adapter *adap)
-{
- return i2c_probe(adap, &addr_data, m41t00_probe);
-}
-
-static int
-m41t00_detach(struct i2c_client *client)
-{
- int rc;
-
- if ((rc = i2c_detach_client(client)) == 0) {
- kfree(client);
- tasklet_kill(&m41t00_tasklet);
- }
- return rc;
-}
-
-static struct i2c_driver m41t00_driver = {
- .driver = {
- .name = M41T00_DRV_NAME,
- },
- .id = I2C_DRIVERID_STM41T00,
- .attach_adapter = m41t00_attach,
- .detach_client = m41t00_detach,
-};
-
-static int __init
-m41t00_init(void)
-{
- return i2c_add_driver(&m41t00_driver);
-}
-
-static void __exit
-m41t00_exit(void)
-{
- i2c_del_driver(&m41t00_driver);
- return;
-}
-
-module_init(m41t00_init);
-module_exit(m41t00_exit);
-
-MODULE_AUTHOR("Mark A. Greer <[email protected]>");
-MODULE_DESCRIPTION("ST Microelectronics M41T00 RTC I2C Client Driver");
-MODULE_LICENSE("GPL");
diff -Nurp linux-2.6.15-rc5-mm3.orig/drivers/i2c/chips/m41txx.c linux-2.6.15-rc5-mm3-m41txx/drivers/i2c/chips/m41txx.c
--- linux-2.6.15-rc5-mm3.orig/drivers/i2c/chips/m41txx.c 1969-12-31 17:00:00.000000000 -0700
+++ linux-2.6.15-rc5-mm3-m41txx/drivers/i2c/chips/m41txx.c 2005-12-19 13:16:26.000000000 -0700
@@ -0,0 +1,336 @@
+/*
+ * I2C client/driver for the ST M41Txx family of i2c rtc chips.
+ *
+ * Author: Mark A. Greer <[email protected]>
+ *
+ * 2005 (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.
+ */
+/*
+ * This i2c client/driver wedges between the drivers/char/genrtc.c RTC
+ * interface and the SMBus interface of the i2c subsystem.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/i2c.h>
+#include <linux/rtc.h>
+#include <linux/m41txx.h>
+#include <linux/bcd.h>
+#include <linux/workqueue.h>
+#include <linux/platform_device.h>
+
+#include <asm/time.h>
+#include <asm/rtc.h>
+
+static DECLARE_MUTEX(m41txx_mutex);
+
+static struct work_struct set_rtc_time_task;
+
+static struct i2c_driver m41txx_driver;
+static struct i2c_client *save_client;
+
+static unsigned short ignore[] = { I2C_CLIENT_END };
+static unsigned short normal_addr[] = { 0, /* replaced w/ platform_data */
+ I2C_CLIENT_END };
+
+static struct i2c_client_address_data addr_data = {
+ .normal_i2c = normal_addr,
+ .probe = ignore,
+ .ignore = ignore,
+};
+
+struct m41txx_chip_info {
+ u16 type;
+ u8 sec; /* Offsets for chip regs */
+ u8 min;
+ u8 hour;
+ u8 day;
+ u8 mon;
+ u8 year;
+ u8 alarm_mon;
+ u8 alarm_hour;
+ u8 sqw;
+ u32 sqw_freq;
+};
+
+static struct m41txx_chip_info m41txx_chip_info_tbl[] = {
+ { M41TXX_TYPE_M41T00, 0, 1, 2, 4, 5, 6, 0, 0, 0, 0 },
+ { M41TXX_TYPE_M41T81, 1, 2, 3, 5, 6, 7, 0xa, 0xc, 0x13, 0 },
+ { M41TXX_TYPE_M41T85, 1, 2, 3, 5, 6, 7, 0xa, 0xc, 0x13, 0 },
+};
+static struct m41txx_chip_info *m41txx_chip;
+
+#define M41TXX_MAX_RETRIES 3 /* Really 1 try + 2 retries */
+
+unsigned long
+m41txx_get_rtc_time(void)
+{
+ s32 sec, min, hour, day, mon, year;
+ s32 sec1, min1, hour1, day1, mon1, year1;
+ u32 retries, limit = 10;
+
+ sec = min = hour = day = mon = year = 0;
+ sec1 = min1 = hour1 = day1 = mon1 = year1 = 0;
+
+ down(&m41txx_mutex);
+ do {
+ retries = M41TXX_MAX_RETRIES;
+
+ do {
+ if (((sec = i2c_smbus_read_byte_data(save_client,
+ m41txx_chip->sec)) >= 0)
+ && ((min = i2c_smbus_read_byte_data(save_client,
+ m41txx_chip->min)) >= 0)
+ && ((hour= i2c_smbus_read_byte_data(save_client,
+ m41txx_chip->hour)) >= 0)
+ && ((day = i2c_smbus_read_byte_data(save_client,
+ m41txx_chip->day)) >= 0)
+ && ((mon = i2c_smbus_read_byte_data(save_client,
+ m41txx_chip->mon)) >= 0)
+ && ((year= i2c_smbus_read_byte_data(save_client,
+ m41txx_chip->year)) >= 0))
+ break;
+ } while (--retries > 0);
+
+ if ((retries == 0) || ((sec == sec1) && (min == min1)
+ && (hour == hour1) && (day == day1)
+ && (mon == mon1) && (year == year1)))
+ break;
+
+ sec1 = sec;
+ min1 = min;
+ hour1 = hour;
+ day1 = day;
+ mon1 = mon;
+ year1 = year;
+ } while (--limit > 0);
+ up(&m41txx_mutex);
+
+ if ((limit == 0) || (retries == 0)) {
+ if (limit == 0)
+ dev_warn(&save_client->dev,
+ "m41txx: can't read rtc chip\n");
+ sec = min = hour = day = mon = year = 0;
+ }
+
+ sec &= 0x7f;
+ min &= 0x7f;
+ hour &= 0x3f;
+ day &= 0x3f;
+ mon &= 0x1f;
+ year &= 0xff;
+
+ BCD_TO_BIN(sec);
+ BCD_TO_BIN(min);
+ BCD_TO_BIN(hour);
+ BCD_TO_BIN(day);
+ BCD_TO_BIN(mon);
+ BCD_TO_BIN(year);
+
+ year += 1900;
+ if (year < 1970)
+ year += 100;
+
+ return mktime(year, mon, day, hour, min, sec);
+}
+
+static void
+m41txx_set(void *arg)
+{
+ struct rtc_time tm;
+ int nowtime = *(int *)arg;
+ u32 retries = M41TXX_MAX_RETRIES;
+
+ to_tm(nowtime, &tm);
+ tm.tm_year = (tm.tm_year - 1900) % 100;
+
+ BIN_TO_BCD(tm.tm_sec);
+ BIN_TO_BCD(tm.tm_min);
+ BIN_TO_BCD(tm.tm_hour);
+ BIN_TO_BCD(tm.tm_mday);
+ BIN_TO_BCD(tm.tm_mon);
+ BIN_TO_BCD(tm.tm_year);
+
+ down(&m41txx_mutex);
+
+ do {
+ if ((i2c_smbus_write_byte_data(save_client,
+ m41txx_chip->sec, tm.tm_sec & 0x7f) == 0)
+ && (i2c_smbus_write_byte_data(save_client,
+ m41txx_chip->min, tm.tm_min & 0x7f) == 0)
+ && (i2c_smbus_write_byte_data(save_client,
+ m41txx_chip->hour, tm.tm_hour & 0x3f) == 0)
+ && (i2c_smbus_write_byte_data(save_client,
+ m41txx_chip->day, tm.tm_mday & 0x3f) == 0)
+ && (i2c_smbus_write_byte_data(save_client,
+ m41txx_chip->mon, tm.tm_mon & 0x1f) == 0)
+ && (i2c_smbus_write_byte_data(save_client,
+ m41txx_chip->year, tm.tm_year & 0xff) == 0))
+ break;
+ } while (--retries > 0);
+
+ if (retries == 0)
+ dev_warn(&save_client->dev,"m41txx: can't write to rtc chip\n");
+
+ up(&m41txx_mutex);
+}
+
+static u32 new_time;
+
+int
+m41txx_set_rtc_time(unsigned long nowtime)
+{
+ new_time = nowtime;
+
+ if (in_interrupt())
+ schedule_work(&set_rtc_time_task);
+ else
+ m41txx_set((void *)&new_time);
+ return 0;
+}
+
+/*
+ *****************************************************************************
+ *
+ * platform_data Driver Interface
+ *
+ *****************************************************************************
+ */
+static int __init
+m41txx_platform_probe(struct platform_device *pdev)
+{
+ struct m41txx_platform_data *pdata;
+ int i;
+
+ if (pdev && (pdata = pdev->dev.platform_data)) {
+ normal_addr[0] = pdata->i2c_addr;
+
+ for (i=0; i<ARRAY_SIZE(m41txx_chip_info_tbl); i++)
+ if (m41txx_chip_info_tbl[i].type == pdata->type) {
+ m41txx_chip = &m41txx_chip_info_tbl[i];
+ m41txx_chip->sqw_freq = pdata->sqw_freq;
+ return 0;
+ }
+ }
+ return -ENODEV;
+}
+
+static int __exit
+m41txx_platform_remove(struct platform_device *pdev)
+{
+ return 0;
+}
+
+static struct platform_driver m41txx_platform_driver = {
+ .probe = m41txx_platform_probe,
+ .remove = m41txx_platform_remove,
+ .driver = {
+ .owner = THIS_MODULE,
+ .name = M41TXX_DRV_NAME,
+ },
+};
+
+/*
+ *****************************************************************************
+ *
+ * Driver Interface
+ *
+ *****************************************************************************
+ */
+static int
+m41txx_probe(struct i2c_adapter *adap, int addr, int kind)
+{
+ struct i2c_client *client;
+ int rc;
+
+ client = kzalloc(sizeof(struct i2c_client), GFP_KERNEL);
+ if (!client)
+ return -ENOMEM;
+
+ strlcpy(client->name, m41txx_driver.driver.name, I2C_NAME_SIZE);
+ client->addr = addr;
+ client->adapter = adap;
+ client->driver = &m41txx_driver;
+
+ if ((rc = i2c_attach_client(client)) != 0)
+ goto probe_err;
+
+ /* Sst SQW frequency & enable */
+ if ((m41txx_chip->type != M41TXX_TYPE_M41T00) && m41txx_chip->sqw_freq
+ && ((rc = i2c_smbus_read_byte_data(client,
+ m41txx_chip->alarm_mon)) >= 0)
+ && !(rc & 0x40) && !(i2c_smbus_write_byte_data(
+ client,m41txx_chip->sqw,m41txx_chip->sqw_freq)))
+ i2c_smbus_write_byte_data(client, m41txx_chip->alarm_mon,
+ rc | 0x40);
+
+ /* Make sure oscillator is running (i.e., clear 'ST' bit in sec reg) */
+ if (((rc = i2c_smbus_read_byte_data(client, m41txx_chip->sec)) < 0)
+ || ((rc & 0x80) && ((rc = i2c_smbus_write_byte_data(
+ client, m41txx_chip->sec, rc & 0x7f)) < 0)))
+ goto probe_err;
+
+ INIT_WORK(&set_rtc_time_task, &m41txx_set, &new_time);
+ save_client = client;
+ return 0;
+
+probe_err:
+ kfree(client);
+ return rc;
+}
+
+static int
+m41txx_attach(struct i2c_adapter *adap)
+{
+ return i2c_probe(adap, &addr_data, m41txx_probe);
+}
+
+static int
+m41txx_detach(struct i2c_client *client)
+{
+ int rc;
+
+ if ((rc = i2c_detach_client(client)) == 0) {
+ kfree(client);
+ flush_scheduled_work();
+ }
+ return rc;
+}
+
+static struct i2c_driver m41txx_driver = {
+ .driver = {
+ .owner = THIS_MODULE,
+ .name = M41TXX_DRV_NAME,
+ },
+ .id = I2C_DRIVERID_STM41TXX,
+ .attach_adapter = m41txx_attach,
+ .detach_client = m41txx_detach,
+};
+
+static int __init
+m41txx_init(void)
+{
+ int rc;
+
+ if (!(rc = platform_driver_register(&m41txx_platform_driver)))
+ rc = i2c_add_driver(&m41txx_driver);
+ return rc;
+}
+
+static void __exit
+m41txx_exit(void)
+{
+ i2c_del_driver(&m41txx_driver);
+ platform_driver_unregister(&m41txx_platform_driver);
+}
+
+module_init(m41txx_init);
+module_exit(m41txx_exit);
+
+MODULE_AUTHOR("Mark A. Greer <[email protected]>");
+MODULE_DESCRIPTION("ST Microelectronics M41TXX RTC I2C Client Driver");
+MODULE_LICENSE("GPL");
diff -Nurp linux-2.6.15-rc5-mm3.orig/drivers/i2c/chips/Makefile linux-2.6.15-rc5-mm3-m41txx/drivers/i2c/chips/Makefile
--- linux-2.6.15-rc5-mm3.orig/drivers/i2c/chips/Makefile 2005-12-03 22:10:42.000000000 -0700
+++ linux-2.6.15-rc5-mm3-m41txx/drivers/i2c/chips/Makefile 2005-12-19 13:16:26.000000000 -0700
@@ -6,7 +6,6 @@ obj-$(CONFIG_SENSORS_DS1337) += ds1337.o
obj-$(CONFIG_SENSORS_DS1374) += ds1374.o
obj-$(CONFIG_SENSORS_EEPROM) += eeprom.o
obj-$(CONFIG_SENSORS_MAX6875) += max6875.o
-obj-$(CONFIG_SENSORS_M41T00) += m41t00.o
obj-$(CONFIG_SENSORS_PCA9539) += pca9539.o
obj-$(CONFIG_SENSORS_PCF8574) += pcf8574.o
obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o
@@ -14,6 +13,7 @@ obj-$(CONFIG_SENSORS_RTC8564) += rtc8564
obj-$(CONFIG_ISP1301_OMAP) += isp1301_omap.o
obj-$(CONFIG_TPS65010) += tps65010.o
obj-$(CONFIG_RTC_X1205_I2C) += x1205.o
+obj-$(CONFIG_RTC_M41TXX_I2C) += m41txx.o

ifeq ($(CONFIG_I2C_DEBUG_CHIP),y)
EXTRA_CFLAGS += -DDEBUG
diff -Nurp linux-2.6.15-rc5-mm3.orig/include/linux/i2c-id.h linux-2.6.15-rc5-mm3-m41txx/include/linux/i2c-id.h
--- linux-2.6.15-rc5-mm3.orig/include/linux/i2c-id.h 2005-12-19 13:04:29.000000000 -0700
+++ linux-2.6.15-rc5-mm3-m41txx/include/linux/i2c-id.h 2005-12-19 13:16:26.000000000 -0700
@@ -85,7 +85,7 @@
#define I2C_DRIVERID_SAA7114 49 /* video decoder */
#define I2C_DRIVERID_ZR36120 50 /* Zoran 36120 video encoder */
#define I2C_DRIVERID_24LC32A 51 /* Microchip 24LC32A 32k EEPROM */
-#define I2C_DRIVERID_STM41T00 52 /* real time clock */
+#define I2C_DRIVERID_STM41TXX 52 /* real time clock */
#define I2C_DRIVERID_UDA1342 53 /* UDA1342 audio codec */
#define I2C_DRIVERID_ADV7170 54 /* video encoder */
#define I2C_DRIVERID_RADEON 55 /* I2C bus on Radeon boards */
diff -Nurp linux-2.6.15-rc5-mm3.orig/include/linux/m41txx.h linux-2.6.15-rc5-mm3-m41txx/include/linux/m41txx.h
--- linux-2.6.15-rc5-mm3.orig/include/linux/m41txx.h 1969-12-31 17:00:00.000000000 -0700
+++ linux-2.6.15-rc5-mm3-m41txx/include/linux/m41txx.h 2005-12-19 13:16:26.000000000 -0700
@@ -0,0 +1,28 @@
+/*
+ * Definitions for the ST M41Txx family of i2c rtc chips.
+ *
+ * Author: Mark A. Greer <[email protected]>
+ *
+ * 2005 (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.
+ */
+
+#ifndef _M41TXX_H
+#define _M41TXX_H
+
+#define M41TXX_DRV_NAME "m41txx rtc"
+#define M41TXX_I2C_ADDR 0x68
+
+#define M41TXX_TYPE_M41T00 0
+#define M41TXX_TYPE_M41T81 81
+#define M41TXX_TYPE_M41T85 85
+
+struct m41txx_platform_data {
+ u16 type;
+ u16 i2c_addr;
+ u32 sqw_freq;
+};
+
+#endif /* _M41TXX_H */

2005-12-19 21:07:38

by Mark A. Greer

[permalink] [raw]
Subject: Re: [RFC] i2c: Combined ST m41txx i2c rtc chip driver (was: [PATCH 1/1] Added support of ST m41t85 rtc chip)

On Mon, Dec 19, 2005 at 02:03:25PM -0700, Mark A. Greer wrote:
> [I will send a second patch that contains the changes I made to the ppc
> katana platform that has the m41t00 that I tested with.]

FYI, here are the code changes with an updated defconfig for the platform.

Mark
---
configs/katana_defconfig | 99 +++++++++++++++++++++++++++++++----------------
platforms/katana.c | 37 +++++++++++++++--
2 files changed, 99 insertions(+), 37 deletions(-)
---
diff -Nurp linux-2.6.15-rc5-mm3.orig/arch/ppc/configs/katana_defconfig linux-2.6.15-rc5-mm3-m41txx/arch/ppc/configs/katana_defconfig
--- linux-2.6.15-rc5-mm3.orig/arch/ppc/configs/katana_defconfig 2005-12-03 22:10:42.000000000 -0700
+++ linux-2.6.15-rc5-mm3-m41txx/arch/ppc/configs/katana_defconfig 2005-12-19 13:16:25.000000000 -0700
@@ -1,17 +1,17 @@
#
# Automatically generated make config: don't edit
-# Linux kernel version: 2.6.13-mm1
-# Thu Sep 1 17:16:03 2005
+# Linux kernel version: 2.6.15-rc5-mm1
+# Fri Dec 16 16:12:10 2005
#
CONFIG_MMU=y
CONFIG_GENERIC_HARDIRQS=y
CONFIG_RWSEM_XCHGADD_ALGORITHM=y
CONFIG_GENERIC_CALIBRATE_DELAY=y
-CONFIG_HAVE_DEC_LOCK=y
CONFIG_PPC=y
CONFIG_PPC32=y
CONFIG_GENERIC_NVRAM=y
CONFIG_SCHED_NO_NO_OMIT_FRAME_POINTER=y
+CONFIG_ARCH_MAY_HAVE_PC_FDC=y

#
# Code maturity level options
@@ -27,20 +27,21 @@ CONFIG_INIT_ENV_ARG_LIMIT=32
CONFIG_LOCALVERSION=""
CONFIG_LOCALVERSION_AUTO=y
CONFIG_SWAP=y
+CONFIG_SWAP_PREFETCH=y
CONFIG_SYSVIPC=y
# CONFIG_POSIX_MQUEUE is not set
# CONFIG_BSD_PROCESS_ACCT is not set
CONFIG_SYSCTL=y
# CONFIG_AUDIT is not set
-# CONFIG_HOTPLUG is not set
-CONFIG_KOBJECT_UEVENT=y
# CONFIG_IKCONFIG is not set
CONFIG_INITRAMFS_SOURCE=""
# CONFIG_EMBEDDED is not set
CONFIG_KALLSYMS=y
# CONFIG_KALLSYMS_EXTRA_PASS is not set
+CONFIG_HOTPLUG=y
CONFIG_PRINTK=y
CONFIG_BUG=y
+CONFIG_ELF_CORE=y
CONFIG_BASE_FULL=y
CONFIG_FUTEX=y
CONFIG_EPOLL=y
@@ -49,8 +50,11 @@ CONFIG_CC_ALIGN_FUNCTIONS=0
CONFIG_CC_ALIGN_LABELS=0
CONFIG_CC_ALIGN_LOOPS=0
CONFIG_CC_ALIGN_JUMPS=0
+CONFIG_SLAB=y
# CONFIG_TINY_SHMEM is not set
CONFIG_BASE_SMALL=0
+# CONFIG_SLOB is not set
+CONFIG_OBSOLETE_INTERMODULE=y

#
# Loadable module support
@@ -64,6 +68,25 @@ CONFIG_OBSOLETE_MODPARM=y
CONFIG_KMOD=y

#
+# Block layer
+#
+# CONFIG_LBD is not set
+# CONFIG_BLK_DEV_IO_TRACE is not set
+
+#
+# IO Schedulers
+#
+CONFIG_IOSCHED_NOOP=y
+CONFIG_IOSCHED_AS=y
+CONFIG_IOSCHED_DEADLINE=y
+CONFIG_IOSCHED_CFQ=y
+CONFIG_DEFAULT_AS=y
+# CONFIG_DEFAULT_DEADLINE is not set
+# CONFIG_DEFAULT_CFQ is not set
+# CONFIG_DEFAULT_NOOP is not set
+CONFIG_DEFAULT_IOSCHED="anticipatory"
+
+#
# Processor
#
CONFIG_6xx=y
@@ -84,11 +107,6 @@ CONFIG_PPC_STD_MMU=y
CONFIG_NOT_COHERENT_CACHE=y

#
-# Performance-monitoring counters support
-#
-# CONFIG_PERFCTR is not set
-
-#
# Platform options
#
# CONFIG_PPC_MULTIPLATFORM is not set
@@ -144,11 +162,13 @@ CONFIG_FLATMEM_MANUAL=y
CONFIG_FLATMEM=y
CONFIG_FLAT_NODE_MEM_MAP=y
# CONFIG_SPARSEMEM_STATIC is not set
+CONFIG_SPLIT_PTLOCK_CPUS=4
CONFIG_BINFMT_ELF=y
CONFIG_BINFMT_MISC=y
CONFIG_CMDLINE_BOOL=y
CONFIG_CMDLINE="console=ttyMM0 ip=on"
# CONFIG_PM is not set
+# CONFIG_SOFTWARE_SUSPEND is not set
CONFIG_SECCOMP=y
CONFIG_ISA_DMA_API=y

@@ -156,6 +176,8 @@ CONFIG_ISA_DMA_API=y
# Bus options
#
CONFIG_GENERIC_ISA_DMA=y
+# CONFIG_PPC_I8259 is not set
+CONFIG_PPC_INDIRECT_PCI=y
CONFIG_PCI=y
CONFIG_PCI_DOMAINS=y
CONFIG_PCI_LEGACY_PROC=y
@@ -241,14 +263,16 @@ CONFIG_TCP_CONG_BIC=y
# CONFIG_NET_DIVERT is not set
# CONFIG_ECONET is not set
# CONFIG_WAN_ROUTER is not set
+
+#
+# QoS and/or fair queueing
+#
# CONFIG_NET_SCHED is not set
-# CONFIG_NET_CLS_ROUTE is not set

#
# Network testing
#
# CONFIG_NET_PKTGEN is not set
-# CONFIG_NETFILTER_NETLINK is not set
# CONFIG_HAMRADIO is not set
# CONFIG_IRDA is not set
# CONFIG_BT is not set
@@ -266,6 +290,11 @@ CONFIG_PREVENT_FIRMWARE_BUILD=y
# CONFIG_FW_LOADER is not set

#
+# Connector - unified userspace <-> kernelspace linker
+#
+# CONFIG_CONNECTOR is not set
+
+#
# Memory Technology Devices (MTD)
#
CONFIG_MTD=y
@@ -283,6 +312,7 @@ CONFIG_MTD_BLOCK=y
# CONFIG_FTL is not set
# CONFIG_NFTL is not set
# CONFIG_INFTL is not set
+# CONFIG_RFD_FTL is not set

#
# RAM/ROM/Flash chip drivers
@@ -347,6 +377,11 @@ CONFIG_MTD_PHRAM=y
# CONFIG_MTD_NAND is not set

#
+# OneNAND Flash Device Drivers
+#
+# CONFIG_MTD_ONENAND is not set
+
+#
# Parallel port support
#
# CONFIG_PARPORT is not set
@@ -372,16 +407,7 @@ CONFIG_BLK_DEV_RAM=y
CONFIG_BLK_DEV_RAM_COUNT=16
CONFIG_BLK_DEV_RAM_SIZE=4096
CONFIG_BLK_DEV_INITRD=y
-# CONFIG_LBD is not set
# CONFIG_CDROM_PKTCDVD is not set
-
-#
-# IO Schedulers
-#
-CONFIG_IOSCHED_NOOP=y
-CONFIG_IOSCHED_AS=y
-CONFIG_IOSCHED_DEADLINE=y
-CONFIG_IOSCHED_CFQ=y
# CONFIG_ATA_OVER_ETH is not set

#
@@ -418,6 +444,7 @@ CONFIG_IOSCHED_CFQ=y
#
# Macintosh device drivers
#
+# CONFIG_WINDFARM is not set

#
# Network device support
@@ -445,6 +472,7 @@ CONFIG_NET_ETHERNET=y
CONFIG_MII=y
# CONFIG_HAPPYMEAL is not set
# CONFIG_SUNGEM is not set
+# CONFIG_CASSINI is not set
# CONFIG_NET_VENDOR_3COM is not set

#
@@ -599,7 +627,6 @@ CONFIG_SERIAL_MPSC=y
CONFIG_SERIAL_MPSC_CONSOLE=y
CONFIG_SERIAL_CORE=y
CONFIG_SERIAL_CORE_CONSOLE=y
-# CONFIG_SERIAL_JSM is not set
CONFIG_UNIX98_PTYS=y
CONFIG_LEGACY_PTYS=y
CONFIG_LEGACY_PTY_COUNT=256
@@ -631,6 +658,7 @@ CONFIG_GEN_RTC=y
# TPM devices
#
# CONFIG_TCG_TPM is not set
+# CONFIG_TELCLOCK is not set

#
# I2C support
@@ -682,14 +710,20 @@ CONFIG_I2C_MV64XXX=y
# CONFIG_SENSORS_PCA9539 is not set
# CONFIG_SENSORS_PCF8591 is not set
# CONFIG_SENSORS_RTC8564 is not set
-CONFIG_SENSORS_M41T00=y
# CONFIG_SENSORS_MAX6875 is not set
+# CONFIG_RTC_X1205_I2C is not set
+CONFIG_RTC_M41TXX_I2C=y
# CONFIG_I2C_DEBUG_CORE is not set
# CONFIG_I2C_DEBUG_ALGO is not set
# CONFIG_I2C_DEBUG_BUS is not set
# CONFIG_I2C_DEBUG_CHIP is not set

#
+# SPI support
+#
+# CONFIG_SPI_MASTER is not set
+
+#
# Dallas's 1-wire bus
#
# CONFIG_W1 is not set
@@ -728,6 +762,7 @@ CONFIG_HWMON=y
# CONFIG_SENSORS_SMSC47M1 is not set
# CONFIG_SENSORS_SMSC47B397 is not set
# CONFIG_SENSORS_VIA686A is not set
+# CONFIG_SENSORS_VT8231 is not set
# CONFIG_SENSORS_W83781D is not set
# CONFIG_SENSORS_W83792D is not set
# CONFIG_SENSORS_W83L785TS is not set
@@ -782,6 +817,10 @@ CONFIG_USB_ARCH_HAS_OHCI=y
# CONFIG_USB is not set

#
+# NOTE: USB_STORAGE enables SCSI, and 'SCSI disk support'
+#
+
+#
# USB Gadget Support
#
# CONFIG_USB_GADGET is not set
@@ -801,6 +840,10 @@ CONFIG_USB_ARCH_HAS_OHCI=y
#

#
+# EDAC - error detection and reporting (RAS)
+#
+
+#
# Distributed Lock Manager
#
# CONFIG_DLM is not set
@@ -816,10 +859,6 @@ CONFIG_EXT2_FS=y
# CONFIG_REISERFS_FS is not set
# CONFIG_JFS_FS is not set
# CONFIG_FS_POSIX_ACL is not set
-
-#
-# XFS support
-#
# CONFIG_XFS_FS is not set
# CONFIG_OCFS2_FS is not set
# CONFIG_MINIX_FS is not set
@@ -853,8 +892,8 @@ CONFIG_SYSFS=y
CONFIG_TMPFS=y
# CONFIG_HUGETLB_PAGE is not set
CONFIG_RAMFS=y
-# CONFIG_CONFIGFS_FS is not set
# CONFIG_RELAYFS_FS is not set
+# CONFIG_CONFIGFS_FS is not set

#
# Miscellaneous filesystems
@@ -917,10 +956,6 @@ CONFIG_MSDOS_PARTITION=y
# CONFIG_CRC16 is not set
CONFIG_CRC32=y
# CONFIG_LIBCRC32C is not set
-
-#
-# Profiling support
-#
# CONFIG_PROFILING is not set

#
diff -Nurp linux-2.6.15-rc5-mm3.orig/arch/ppc/platforms/katana.c linux-2.6.15-rc5-mm3-m41txx/arch/ppc/platforms/katana.c
--- linux-2.6.15-rc5-mm3.orig/arch/ppc/platforms/katana.c 2005-12-03 22:10:42.000000000 -0700
+++ linux-2.6.15-rc5-mm3-m41txx/arch/ppc/platforms/katana.c 2005-12-19 13:16:25.000000000 -0700
@@ -30,6 +30,7 @@
#include <linux/mtd/physmap.h>
#include <linux/mv643xx.h>
#include <linux/platform_device.h>
+#include <linux/m41txx.h>
#ifdef CONFIG_BOOTIMG
#include <linux/bootimg.h>
#endif
@@ -845,17 +846,43 @@ katana_find_end_of_memory(void)
return bdp->bi_memsize;
}

-#if defined(CONFIG_I2C_MV64XXX) && defined(CONFIG_SENSORS_M41T00)
-extern ulong m41t00_get_rtc_time(void);
-extern int m41t00_set_rtc_time(ulong);
+#if defined(CONFIG_I2C_MV64XXX) && defined(CONFIG_RTC_M41TXX_I2C)
+static struct m41txx_platform_data katana_m41txx_pdata = {
+ .type = M41TXX_TYPE_M41T00,
+ .i2c_addr = M41TXX_I2C_ADDR,
+};
+
+static struct platform_device katana_m41txx_pdev = {
+ .name = M41TXX_DRV_NAME,
+ .id = 0,
+ .num_resources = 0,
+ .dev = {
+ .platform_data = &katana_m41txx_pdata,
+ },
+};
+
+static int __init
+katana_add_pdata(void)
+{
+ int rc;
+
+ if ((rc = platform_device_register(&katana_m41txx_pdev)))
+ platform_device_unregister(&katana_m41txx_pdev);
+
+ return rc;
+}
+arch_initcall(katana_add_pdata);
+
+extern ulong m41txx_get_rtc_time(void);
+extern int m41txx_set_rtc_time(ulong);

static int __init
katana_rtc_hookup(void)
{
struct timespec tv;

- ppc_md.get_rtc_time = m41t00_get_rtc_time;
- ppc_md.set_rtc_time = m41t00_set_rtc_time;
+ ppc_md.get_rtc_time = m41txx_get_rtc_time;
+ ppc_md.set_rtc_time = m41txx_set_rtc_time;

tv.tv_nsec = 0;
tv.tv_sec = (ppc_md.get_rtc_time)();

2005-12-20 10:05:39

by Andrey Volkov

[permalink] [raw]
Subject: Re: [RFC] i2c: Combined ST m41txx i2c rtc chip driver

Hello Mark

Big Thanks, I check it on my board today-tomorrow.
But check some comments below.

Mark A. Greer wrote:
> On Tue, Nov 15, 2005 at 07:57:14PM -0700, Mark A. Greer wrote:
>
>>I think we can combine the two into an m41txx.c and pass the exact type
>>in via platform_data--that would be the correct mechanism, right?
>>The platform_data could also be used to seed the correct SQW freq and
>>eliminate all the Kconfig noise.
>>
>>Comments?
>>
>>As for Jean's and Andrew's comments about the driver, they seem valid
>>to me and should be addressed. In Andrey's defense, many of them are my
>>fault. Once there is a consensus on the merging m41t00 & m41t85
>>question, I'll try to get a fixed up patch within a couple weeks.
>
>
> I've made what I think should be close to an acceptable driver that
> supports the m41t00, m41t81, and m41t85 i2c rtc chips. I only have hw
> to test the m41t00 on a ppc platform, though.
>
> It was suggested to me a while back to add retries when reading &
> writing the rtc regs. Personally, I think its overkill but I added the
> code anyway. You will see in m41txx_get_rtc_time() that 3 tries are
> made to get a good read of all the regs then up to 10 tries are made to
> make sure that the time didn't change while we were doing the reads.
> Something similar is done in m41txx_set().
>
> Andrey, would you please apply and test this patch on your 5200?
>
> Andy, I apologize but I didn't take the time to look at the mips tree in
> depth. I did do a quick scan and I think that you should be able to set
> 'rtc_get_time' to 'm41txx_get_rtc_time' and 'rtc_set_time' to
> 'm41txx_set_rtc_time', and it'll work. You'll have to hook up the proper
> driver for your host i2c ctlr, though.
>
> Jean, Andrew, I think I addressed your coding style, etc. issues with
> the driver. If not, just point out the problems an I'll fix them.
>
> The patch is against the 2.6.15-rc5-mm3 tree. Comments welcome.
>
> [I will send a second patch that contains the changes I made to the ppc
> katana platform that has the m41t00 that I tested with.]
>
> Thanks,
>
> Mark

> + down(&m41txx_mutex);
> + do {
> + retries = M41TXX_MAX_RETRIES;
> +
> + do {
> + if (((sec = i2c_smbus_read_byte_data(save_client,
> + m41txx_chip->sec)) >= 0)
> + && ((min = i2c_smbus_read_byte_data(save_client,
> + m41txx_chip->min)) >= 0)
> + && ((hour= i2c_smbus_read_byte_data(save_client,
> + m41txx_chip->hour)) >= 0)
> + && ((day = i2c_smbus_read_byte_data(save_client,
> + m41txx_chip->day)) >= 0)
> + && ((mon = i2c_smbus_read_byte_data(save_client,
> + m41txx_chip->mon)) >= 0)
> + && ((year= i2c_smbus_read_byte_data(save_client,
> + m41txx_chip->year)) >= 0))
> + break;
> + } while (--retries > 0);
> +
> + if ((retries == 0) || ((sec == sec1) && (min == min1)
> + && (hour == hour1) && (day == day1)
> + && (mon == mon1) && (year == year1)))
> + break;

I think this code is overburdened (I forgot to point on it last time,
sorry) and may be wrong for m41t8x, since when you send i2c stop
condition (in read_byte_data), you release time registers of m41t8x,
and as a consequence, in the worst case, you must compare/read it an
undetermined number of times, but not 3 times (however, for m41t00 this
code is correct).

I think i2c_master_recv here and i2c_master_send above in m41txx_set
will be more appropriate, since for m41t00 it will have no meaning when
you send STOP (250ms stall), but for m41t8x you could drop this
while-loop completely.

Hint: quotation from m41st85w.pdf p13/34
"The system-to-user transfer of clock data will be
halted whenever the address being read is a clock
address (00h to 07h). The update will resume either
due to a Stop Condition or when the pointer
increments to a non-clock or RAM address."

Also, please, change _obsoleted_ BCD_TO_BIN to BCD2BIN
(see include/linux/bcd.h)


--
Regards
Andrey Volkov

2005-12-21 21:27:19

by Mark A. Greer

[permalink] [raw]
Subject: Re: [RFC] i2c: Combined ST m41txx i2c rtc chip driver

Hi Andrey,

On Tue, Dec 20, 2005 at 01:05:34PM +0300, Andrey Volkov wrote:
> Hello Mark
>
> Big Thanks, I check it on my board today-tomorrow.
> But check some comments below.
>
> Mark A. Greer wrote:
> > On Tue, Nov 15, 2005 at 07:57:14PM -0700, Mark A. Greer wrote:
<snip>
> > + down(&m41txx_mutex);
> > + do {
> > + retries = M41TXX_MAX_RETRIES;
> > +
> > + do {
> > + if (((sec = i2c_smbus_read_byte_data(save_client,
> > + m41txx_chip->sec)) >= 0)
> > + && ((min = i2c_smbus_read_byte_data(save_client,
> > + m41txx_chip->min)) >= 0)
> > + && ((hour= i2c_smbus_read_byte_data(save_client,
> > + m41txx_chip->hour)) >= 0)
> > + && ((day = i2c_smbus_read_byte_data(save_client,
> > + m41txx_chip->day)) >= 0)
> > + && ((mon = i2c_smbus_read_byte_data(save_client,
> > + m41txx_chip->mon)) >= 0)
> > + && ((year= i2c_smbus_read_byte_data(save_client,
> > + m41txx_chip->year)) >= 0))
> > + break;
> > + } while (--retries > 0);
> > +
> > + if ((retries == 0) || ((sec == sec1) && (min == min1)
> > + && (hour == hour1) && (day == day1)
> > + && (mon == mon1) && (year == year1)))
> > + break;
>
> I think this code is overburdened (I forgot to point on it last time,
> sorry) and may be wrong for m41t8x, since when you send i2c stop
> condition (in read_byte_data), you release time registers of m41t8x,
> and as a consequence, in the worst case, you must compare/read it an
> undetermined number of times, but not 3 times (however, for m41t00 this
> code is correct).

The 3 tries isn't to make sure that the registers didn't change, its to
make sure we actually successfully read all of the registers. There are
10 tries to make sure the registers didn't change. I doubt it will ever
take more than 2 or 3 so I don't see a problem with a limit of 10.

I *think* I understand you point, though. You would prefer I not use
the smbus calls, correct? If so, I disagree. I think its better to use
the smbus calls b/c they're the most generic (read: will work with the
most i2c host ctlr drivers). Perhaps Jean or someone else can make an
executive decision on this.

> I think i2c_master_recv here and i2c_master_send above in m41txx_set
> will be more appropriate, since for m41t00 it will have no meaning when
> you send STOP (250ms stall), but for m41t8x you could drop this
> while-loop completely.

I understand but its an issue of being more generic.

<snip>

> Also, please, change _obsoleted_ BCD_TO_BIN to BCD2BIN
> (see include/linux/bcd.h)

I figured one of those was deprecated but didn't know which one. I'll
change them.

Mark