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, andclose it if the NTP logic hasn't refreshed the clock sincelast time. You're right that the simplest scheme is to justopen/close on each call. The extra work is so infrequent itmay 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.