Hi Evgeniy, > > So I suspect that this update at least was never reviewed by anyone (on > > the sensors list at least). > > I have one rule - if noone answers that it means noone objects, > or it is not interesting for anyone, and thus noone objects.
Broken rule IMHO. This might be fine for your own projects, but doesn't work at all for the kernel. Silence might not mean that everyone agrees. Rather, it could mean that either people are just not interested at all, or they are too busy to review a whole new subsystem (or more) at the moment. Believe me, kernel folks are very, very busy. > I have pc8736x logical devices in my TODO list, but they are currently > preempted by acrypto, but I definitely will get it very soon. I don't think you get me. Your personal priorities are unimportant when you are asking for code review and merge into the main kernel tree. What matters is that parts are presented correctly and added in small chunks and in the right order, so as to minimize the time it takes to Greg and others to understand, review and accept it. One year ago I learnt this the hard way myself. I realize how deep my misunderstanding of the situation was back then, and now praise Greg for the good lesson - however tough it was for me at that time - each time I now send a kernel patch the right way and get it accepted. So, make yourself a favor and comply with what the kernel folks expect from you. They won't change for you. > > If you can provide a patch which adds your superio core driver and one > > which modifies the pc87360 driver to take make use of it, and only that, > > this would certainly be easier for everyone (and especially me) to > > review your superio code. Once this is in, incremental patches for the > > additional features should be easier for you to generate and for the > > rest of us to review. > > pc87360.c can not be used with superio as is, it requires big rewrite, > since you implemented it as part of i2c core, > that is why I created parts that was not touched by your driver. Again, you clearly don't get it. The pc87360 driver does *not* require a big rewrite. Without even looking at your superio code again, I can tell that it is necessarily broken if every driver which accesses the superio address space at the moment needs a big rewrite. The pc87360 driver only really uses the superio for hardware identification and then to get a couple configuration values (such as subdevice I/O addresses) at startup, and then leaves it alone forever. If we has a dedicated superio driver, it would need to provide an interface to read, and eventually write, these values, as a replacement for the savage direct accesses we do for now. Period. Anything else might be nice or useful for other logical devices (such as GPIOs) but is not strictly necessary at first. I do not deny that these Super-I/O integrated sensors are not using the SMBus and as such do not truly belong to the i2c subsystem. It just happens that, for now and for historical reasons, the i2c subsystem is also where all hardware monitoring drivers are. That might (hopefully) change in the future, but this is a completely different problem. One thing at a time please. > Your driver is part of i2c core, it just not supposed to be used > in superio, although many hardware routings could be used. Again, some drivers in the i2c subsystem use ISA accesses; others need to poke a couple PCI configuration registers. We did not move these drivers to the ISA (is there even one?) or PCI subsystems. Before anything else, these drivers are hardware monitoring drivers, and such drivers belong - for now - to the i2c subsystem, until we can separate the i2c subsystem from the hardware monitoring function more clearly. So, the pc87360, it87, smsc47m1 and w83627hf drivers are not going to move to the "superio subsystem", nor are they going to be rewritten for it. Superio chips are only a new way to access the subdevices. What we need for them is an extension of the request_region/release_region mechanism, because superios introduce a two-level addressing scheme. And this is about it. Thanks, -- Jean Delvare - 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/
