From: Milton Miller <miltonm@bga•com>
To: Mark Zhan <rongkai.zhan@windriver•com>
Cc: ppcdev <linuxppc-dev@ozlabs•org>
Subject: Re: [PATCH] Add the support of ST M48T59 RTC chip in rtc-class driver subsystem
Date: Mon, 11 Jun 2007 09:35:42 -0500 [thread overview]
Message-ID: <8009e52995468cb801ebd94b144fa629@bga.com> (raw)
In-Reply-To: <1181548600.5217.16.camel@mark>
On Mon Jun 11 17:56:40 EST 2007, Mark Zhan wrote:
> Add the support of ST M48T59 RTC chip driver in RTC class subsystem for
> Wind River SBC PowerQUICCII 82xx board
...
> +#define M48T59_CNTL 0x1ff8
> +#define M48T59_WATCHDOG 0x1ff7
> +#define M48T59_INTR 0x1ff6
> +#define M48T59_ALARM_DATE 0x1ff5
> +#define M48T59_ALARM_HOUR 0x1ff4
> +#define M48T59_ALARM_MIN 0x1ff3
> +#define M48T59_ALARM_SEC 0x1ff2
> +#define M48T59_UNUSED 0x1ff1
> +#define M48T59_FLAGS 0x1ff0
> +
> +#define M48T59_WDAY_CB 0x20 /* Century Bit */
> +#define M48T59_WDAY_CEB 0x10 /* Century Enable Bit
> */
> +
> +#define M48T59_CNTL_READ 0x40;
> +#define M48T59_CNTL_WRITE 0x80;
> +
> +#define M48T59_FLAGS_WDT 0x80 /* watchdog timer expired */
> +#define M48T59_FLAGS_AF 0x40 /* alarm */
> +#define M48T59_FLAGS_BF 0x10 /* low battery */
> +
> +#define M48T59_INTR_AFE 0x80 /* Alarm Interrupt
> Enable */
> +#define M48T59_INTR_ABE 0x20
>
Another style is to put the flag and control register values
immediately after the register, indenting the values an additional tab
to distinguish them from the list of registers. Either way is ok with
me.
> +/**
> + * NOTE: M48T59 only uses BCD mode
> + */
> +static int m48t59_rtc_read_time(struct device *dev, struct rtc_time
> *tm)
> +{
> + unsigned char val;
> +
> + /* Issue the READ command */
> + M48T59_WRITE((M48T59_READ(M48T59_CNTL) | 0x40), M48T59_CNTL);
> +
...
> + /* Clear the READ bit to restore the update */
> + M48T59_WRITE((M48T59_READ(M48T59_CNTL) & ~0x40), M48T59_CNTL);
Should you clear READ and WRITE when your driver starts in case the
previous driver got interrupted (say the system crashed)? Or is it ok
for READ and WRITE to be set?
You aren't using the READ and WRITE flag bits you defined above. If
the line gets too long, you might create a SET_BITS / CLEAR_BITS macro.
milton
prev parent reply other threads:[~2007-06-11 15:27 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-06-11 7:56 [PATCH] Add the support of ST M48T59 RTC chip in rtc-class driver subsystem Mark Zhan
2007-06-11 11:25 ` [rtc-linux] " Alessandro Zummo
2007-06-11 12:11 ` Gabriel Paubert
2007-06-12 13:59 ` Mark Zhan
2007-06-12 14:12 ` Mark Zhan
2007-06-19 12:29 ` Alessandro Zummo
2007-06-14 10:32 ` Gabriel Paubert
2007-06-11 14:35 ` Milton Miller [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8009e52995468cb801ebd94b144fa629@bga.com \
--to=miltonm@bga$(echo .)com \
--cc=linuxppc-dev@ozlabs$(echo .)org \
--cc=rongkai.zhan@windriver$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox