Re: RTC: add RTC class interface to m41t00 driver

From: Atsushi Nemoto
Date: Fri Aug 04 2006 - 11:59:29 EST

On Fri, 4 Aug 2006 16:01:02 +0200, Alexander Bigga <ab@xxxxxxxxxx> wrote:
> like you, I started recently with Mark's m41t00.c driver to add support for
> the new rtc-subsystem. Mark reviewed it and I added his changes.

Thank you. Though my patch for m41t00.c intended to keep original
code as is as possible, I like your approach. I'll work with your new

> There is still the question, if the code for the interrupt context
> (workqueues) should stay or not. You bracketed all this with CONFIG_GEN_RTC.
> I can't say, if this is a good idea. Maybe somebody else has some good
> comments.

I think read_time and set_time routine of rtc_class never called from
the interrupt context. It looks true on current RTC class framework
and some RTC class drivers depend on it already.

> +#include <asm/time.h>
> +#include <asm/rtc.h>

The asm/time.h is not exist on some archs. And while all asm/time.h
are included by asm/rtc.h, this can be removed safely.

> +int m41txx_set_datetime(struct i2c_client *client, struct rtc_time *tm)


> +ulong m41t00_get_rtc_time(void)
> +{
> + struct rtc_time tm;
> +
> + m41txx_get_datetime(save_client, &tm);
> +
> + return mktime(tm.tm_year, tm.tm_mon,
> + tm.tm_mday, tm.tm_hour, tm.tm_min, tm.tm_sec);
> +}
> +EXPORT_SYMBOL_GPL(m41t00_get_rtc_time);

Please drop this old interface from new driver. There are other way
to glue new driver to existing platform, as hctosys.c does.

Then we can remove save_client too.

> +static struct workqueue_struct *m41txx_wq;

As I wrote above, I think this is not needed. If this is really
needed, it should be done in RTC framework instead of lowlevel driver.

> +st_err:
> + dev_err(&client->dev, "%s: Can't clear ST bit\n", __FUNCTION__);
> + goto exit_detach;
> +ht_err:
> + dev_err(&client->dev, "%s: Can't clear HT bit\n", __FUNCTION__);
> + goto exit_detach;
> +sqw_err:
> + dev_err(&client->dev, "%s: Can't set SQW Frequency\n",
> + __FUNCTION__);
> +
> +exit_detach:
> + i2c_detach_client(client);

rtc_device_unregister() must be called somewhere in error path.

Atsushi Nemoto
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at