On Fri, 4 Aug 2006 16:01:02 +0200, Alexander Bigga <[email protected]> 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
driver.
> 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)
static.
> +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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
