Hi Hans, On Wed, 21 May 2008 13:29:55 +0200, Hans de Goede wrote: > Jean Delvare wrote: > > * SMBus read cache. I submitted something already over one year ago: > > http://lists.lm-sensors.org/pipermail/lm-sensors/2007-January/018749.html > > But nobody reacted so I never committed my work. The speedup > > generated by this patch alone seemed worth the extra code. Another > > reason now is that limiting the hardware access to these devices > > seems to be a good idea in the light of recent reports we had about > > I2C/SMBus chips reacting to probes in odd ways. The most important > > protection measures have been implemented in 3.0.2 already, but I > > believe that caching the register reads in sensors-detect would > > further prevent the possibility to break something accidentally. > > Is this really worth it? The problems with the one register chip won't be > stopped by this as one read transaction is all it takes to foobar things there. I agree that this approach won't solve the one-register-only device problem, nor any other problem of that nature (but hopefully the changes that went in 3.0.2 addressed most of these.) By definition, a register cache will only avoid reads that have already happened before. There is still a thin chance that an exact sequence of reads causes problem to a given chip. Also think of concurrent accesses to a given chip by ACPI or SMM code: the less reads we do, the less likely they are to happen. Of course, ideally we would prevent these concurrent accesses in the kernel directly, but we don't have that at the moment, and I can't see it done in the next few months (nor years to be honest.) So I still see a small a value to a register cache in this area. > Also speed is hardly an argument, it is not like sensors-detect takes a long > time to run. And as long as it stays within 5 minutes speed is irrelevant IMHO. 5 minutes? You must be kidding. Any device detection that takes more than a few _seconds_ is considered too slow by most users. What worries me and motivated my work in this area, is that our current code makes the detection slower and slower over time, because we add support for new devices continuously, but the SMBus doesn't get faster in the same time. We've added 29 new I2C/SMBus chips over the last 2 years, that's a 30% increase, and I see no reason why it would stop. I was also thinking that user-friendly distributions might want to automatize the process of setting up the sensors as part of the system installation. There are many improvements needed before this can happen, such as a command line interface / non-interactive mode to sensors-detect and DMI support, but speeding up the detection itself would also help, I think. I wouldn't consider it if the patch was intrusive, but it's pretty small if you look at it. > > * Fix the bus numbering prediction magic. There's some old code to > > attempt to figure out how I2C bus will be numbered after next reboot, > > so that the "ignore" module options are correct. This code is broken > > in more than one way. It assumes that it knows about all drivers which > > create i2c buses, which is not true (all graphics and multimedia > > adapters in particular are missing.) It assumes that the user will > > reboot after running sensors-detect. It assumes that i2c bus drivers > > don't autoload, while they almost all do by now. And it assumes that > > the bus numbering is stable over reboot, which is not necessarily the > > case (I can't think of a fix for this one though.) > > I agree, with udev etc, bus numbers simply won't be stable so we should not > rely on them being stable. Unfortunately that's the part that cannot be fixed in sensors-detect alone. The hwmon module parameters assume that we know the i2c bus numbers in advance and that they don't change. I have no idea how to fix this - short of moving clone detection and preventing mis-detections inside the drivers themselves. > > * DMI integration. This is needed for automatic configuration file > > download, and could also be welcome to explicitly skip some probes > > on selected motherboards. Recent kernels export the DMI data so we > > should not even need to depend on dmidecode. Either way I don't want > > to add a hard dependency for DMI, so if it's there we use it, if not > > we'll just do as before. > (...) > All in all this sounds very good to me, I esp. look forward to having some dmi > based probing in sensors-detect, I'm afraid I have little time to spare lately > but I would be happy to do testing. Same here... I would like to see it implemented, but so far time constraints never let me look into it. -- Jean Delvare
