Re: + blackfin-on-chip-rtc-controller-driver.patch added to -mm tree

[Mike Frysinger]

On 3/1/07, David Brownell <david-b@xxxxxxxxxxx> wrote:
> Look again, but at 2.6.21-rc2 instead ... ISTR that fix went into RC1,
> but that one's been fixed in various trees (like handhelds.org) for
> some time.

this driver was originally written against 2.6.19.x as we havent
gotten the Blackfin arch into mainline yet so it's hard to track the
latest git

i'll pull the latest git and look into the pointers you've posted ...
and send out a sep patch for updating the documentation ;)

> > > It looks to me like you're handling the time in set_alarm()
> > > incorrectly.  Do the conversion and test in set_alarm, not
> > > AIE_ON ... since set_alarm() needs to be able to include AIE_ON
> > > capability.
> >
> > the reason for this is so that i do not have to worry about keeping
> > two structures in sync ... there's the common RTC representation and
> > then there's the funky Blackfin representation, so by delaying the
> > conversion until RTC_AIE_ON, i didnt have to worry about keeping these
> > fields in sync
>
> Yes, Blackfin has an unusually funky representation, but that
> doesn't mean you can't do the conversion when the alarm time
> comes into the driver.  It's not like the alarm is based on
> offset-from-current-time, so that it'd need to be adjusted when
> the current time gets updated (like some RTCs)... it's just a
> strange way to encode an absolute time.

fair enough ... my other concern was that now i'll also have to
translate it back out in rtc_read_alarm so by maintaining everything
in common RTC terms, there would be no overhead

> > > The fields tm_{wday,yday,isdst} are never used,
> > > but you're testing tm_yday.  If you meant to test tm_mday in
> > > order to distinguish the WKALM_SET and ALM_SET cases, then you
> > > are mishandling a wraparound case; see how rtc-omap handles
> > > it.  (If it's 23:00 now, an 01:00 alarm means TOMORROW.  I
> > > think other RTCs may goof this case too.)
> >
> > this is exactly why i'm checking tm_yday ... the rtc-dev interface
> > sets those fields to -1 when using the WKALM ioctls and there's no
> > other way to distinguish this
>
> Thing is, "man 4 rtc" reports those fields as unused, which means
> they can come from userspace with arbitrary values.  You should
> stick to testing tm_mday, which *is* used.

i dont rely on the manpages for kernel interfaces as they get stale
too fast ... i read the files found in Documentation, rtc.txt in this
case

i guess rtc.txt needs more stuff added to it
-mike
-
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/