Hi all, > As previously mentioned, these patches have had that, on the sensors > mailing list. Lots of review and comments went into them there, and > the code was reworked quite a lot based on it.
That's right, I did actually review Evgeniy's code some times ago, as can be seen here: http://archives.andrew.net.au/lm-sensors/msg27817.html I might however add the following: 1* This was 5 months ago. I'd expect Evgeniy's code to have significantly evolved since, so an additional review now would certainly be welcome. 2* I only reviewed the superio code. The acb part is completely new so I obviously couldn't comment on it back then, and I skipped the gpio part because I admittedly have no particular interest in this part. 3* Some of my objections have been ignored by Evgeniy. Among others, the choice of "sc" as a prefix for the superio stuff is definitely poor and has to be changed. http://archives.andrew.net.au/lm-sensors/msg27847.html I don't think that getting the whole stuff (superio, acb and gpio) merged at once is a good idea. Preferably we would merge superio alone first, then update the drivers that are already in the kernel and could make use of it (it87, w83627hf, pc87360 and smsc47m1, all of i2c/sensors fame, come to mind). This would help ensure that Evgeniy's interface choices were correct, and would additionally be a very good example of how this interface must be used. Then, and only then IMVHO, should the gpio and acb stuff be merged. Before all this happens, I really would like Evgeniy to present an overall plan of his current superio implementation. Last time we discussed about this, we obviously had different views on the interface level that should be proposed by the superio core, as well as how chip-specific values should be handled (or, according to me, mostly not handled). Please note that I am certainly not the most qualified of us all to review this code. What I can do is check whether I will be able to use the new superio interface in the sensor chip drivers I mentioned above, and that's about it. I am not familiar enough with kernel core architectures to decide whether Evgeniy's approach is correct or not. I am willing to help, but I can do so only within my own current skills. Thanks, -- Jean Delvare http://khali.linux-fr.org/ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
