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
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);
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.
> + dev_err(&client->dev, "%s: Can't clear ST bit\n", __FUNCTION__);
> + goto exit_detach;
> + dev_err(&client->dev, "%s: Can't clear HT bit\n", __FUNCTION__);
> + goto exit_detach;
> + dev_err(&client->dev, "%s: Can't set SQW Frequency\n",
> + __FUNCTION__);
> + i2c_detach_client(client);
rtc_device_unregister() must be called somewhere in error path.
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/