On Monday 08 September 2008, David Miller wrote:
> > > > > struct rtc_device *rtc = rtc_class_open("rtc0");
> >
> > One more point: that should probably use CONFIG_RTC_HCTOSYS_DEVICE
> > instead of hard-wiring to "rtc0". Yeah, I'm sure your SPARCs have
> > lots of RTCs to choose from -- not! -- but I'd like to see you end
> > up with code that many folk can reuse/recycle/pirate. ;)
>
> Can you be more specific? Oh, you want me to use the string defined
> by that config option. Ok :-)
>
> But as far as I can tell this will only be set of RTC_HCTOSYS and
> users currently are allowed to not set that.
>
> If this code goes somewhere generic you would need to ifdef test on
> that, depending upon where you'd want to put it and how it would
> be provided generically.
OK.
> > > > I'd be tempted to cache that ... notice how you never
> > > > close it, too. That will goof lots of refcounts...
> > >
> > > Well if I cache it then we'll hold it forever and that's not
> > > so nice right?
> >
> > Why wouldn't it be, so long as it's eventually closed
> > to prevent leakage? Other code can rtc_class_open() too;
> > unlike a userspace open("/dev/rtc0", ...) this isn't an
> > exclusive operation.
>
> When would be "eventually closed" if I open it here and remember
> the pointer in a static local variable, and don't close it?
>
> I guess you need to be more specific about what you mean by
> caching :)
I'll translate that as "-ENOPATCH". :)
It'd suffice to fire a timer every 15 minutes or so, and
close it if the NTP logic hasn't refreshed the clock since
last time. You're right that the simplest scheme is to just
open/close on each call. The extra work is so infrequent it
may not be measurable.
> > If you're concerned about stuff like "rmmod my-i2c-rtc-driver"
> > losing (or "rmmod my-i2c-rtc-driver's-i2c-adapter") ... what's
> > supposed to happen is that you start getting an -ENODEV return
> > from your rtc_set_mmss() call, and then you close and null your
> > cached handle to free up its memory.
>
> I see... god that's ugly. If you want to do this in the generic
> RTC layer helper routines,
... when they get created ...
> that's fine, but I don't feel like
> adding all sorts of stuff like that to the sparc specific routine
> at the moment.
>
> I'm trying to do things that are practical and that I can check
> into sparc-next-2.6 right now.
OK by me.
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
