On Tue, Jul 17, 2007 at 07:13:52AM +0200, Hans de Goede wrote: > I think this will be rather tricky, its esp tricky to define what is a sane > value. <snip> > -Notice that the msleep, now will only be triggered if update_device gets > called within TEMP_COLLECTION_TIME of driver loading, assuming that > REFRESH_INTERVAL > TEMP_COLLECTION_TIME. I did it this way hoping that the > startup scripts of the PC will have enough time between insmod and the > first > read, thus not delaying PC startup. As I understand your pseudocode, you want to start the temperature reading at the end of a call to update_device and init, and pick up the temp values the next time update_device gets called, sleeping as necessary. I've a few questions about this approach: (1) Each sysfs attribute calls update_device before calling sprintf to write the sysfs attribute. If a user decides to read all sysfs attributes at once (for example, via that grep trick), won't this cause a new round of temperature sensor reading every time a sysfs attribute is read, as well as a sleep for the next sysfs attribute read? (2) Let's say a user reads a sensor and causes the temperature sensors to start reading. Then, the user goes to sleep for an hour, and reads the temperature sensor via sysfs. Since the last temperature sensor refresh was an hour previously, won't this cause the user to see temperature readings that are an hour old? (3) If the startup scripts load the driver and immediately begin querying the sensor readings, the scripts will take the msleep hit anyway. > In a releated note, depending on where the 0ther 0.7 seconds are, you could > try todo less reading, for example each time you start / stop the temp > reading you first read the cfg register, why not just use a cached value, > is there a reason to assumee it will change? Esp when stopping I would be > using the value already read when starting (in the current code). I suspect that would work fine. It looks like the bits in the config register only get changed by the driver, so I'd only need to read it once. > Also you read the limits and other non sensor reading registers each > update, why not just read these once at driver initialisation, is there a > reason to assume they will change? Nope. I can make these two changes. > I saw your longer read intervals and make temps signed patches, since the > driver hasn't been merged yet, maybe its a good idea todo a v4 ? Not hard, though I'll have to collapse the two subsequent incremental patches. It'd be less work to just make one more incremental patch and send the entire patchset to lm-sensors. --D -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 189 bytes Desc: Digital signature Url : http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20070724/15c0c134/attachment.bin
