Re: [RFC] (almost) booting allyesconfig -- please don't poke super-io without request_region
Hi Hans, hi Milton, On Thu, 10 Jul 2008 16:51:33 -0500, Milton Miller wrote: > > On Jul 10, 2008, at 4:33 PM, Hans de Goede wrote: > > > Milton Miller wrote: > >> After the following patch to mark the isa region busy and applying a > >> few > >> patches[1], I was able to kexec boot into an all-yes-config kernel > >> from linux-next, with the following KCONFIG_ALLCONFIG file: > ... > >> While the first two might not be required, and the third is just > >> selecting the right platform, it would be nice to get the drivers > >> to play as well as the rest of the kernel. > >> The drivers all are for super-io chips, either irda/FIR drivers or > >> hwmon, > >> and poke at isa ports without checking request_region. > > > > Erm, > > > > The superio sensor drivers only poke the superio chip registers > > without request region during the probe phase, iow they try to detect > > the chip, using a widely document and standardized (part of isa pnp > > AFAIK) procedure on standardized ports. > > > > Let me try to explain a bit about superio chips, they have 2 superio > > control registers (an index and data register) with which things like > > a manufacturer and device id can be read, besides these id registers > > they also have a set of registers with config for different logical > > devices. Once the id is matched, the driver knows which logical device > > config to read, reads a (different) isa base address + range from the > > logical device config, and then does a request_region on the region > > actually used by the logical device. > > > > The superio control registers are thus a sort of pci configuration > > space if you want, doing a request_region on these is not such a good > > idea, as multiple drivers (for different logical devices within the > > superio device) may use these, so trying to gain exclusive access will > > lead to troubles. > > > > I hope with this info about the problem space, that you maybe have a > > suggestion on howto fix this? > > My point is that they are probing without even knowing that a such a IO > range exists. > > These are the only drivers left in the tree that still do this. (At > least that are not > blocked on a powerpc allyesconfig for 64-bit, !CONFIG_VIRT_TO_BUS). I guess we could make all these drivers depend on X86, I suspect that these chips are only used in PCs? > One could make a superio driver, and create sub-devices for the IR, > I2C, floppy, parallel, etc > nodes. There have been proposals to do this, and this would indeed be a very good idea, but unfortunately nobody took the time to implement this properly, push it upstream and volunteer to maintain it. The problem is that you don't need just a "driver", but a new subsystem, that needs to be designed and maintained. > I would even accept a check_region (horrors!), it would impove the > situation. > > But most other drivers do this by request_region, probe, then release > the region afterwards. I agree that the hwmon drivers should do this, as long as no superio subsystem exist. This should solve the problem at hand and might even help spot bad drivers which use the configuration space for longer than initialization time. > Besides, one could argue the superio region should be requested while > probing, to prevent other cpus from poking at the super io chip at the > same time. Think of it as missing locking. Agreed. Milton, care to submit a patch fixing the affected hwmon drivers? Thanks, -- Jean Delvare ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC] (almost) booting allyesconfig -- please don't poke super-io without request_region
Jean Delvare wrote: Hi Hans, hi Milton, One could make a superio driver, and create sub-devices for the IR, I2C, floppy, parallel, etc nodes. There have been proposals to do this, and this would indeed be a very good idea, but unfortunately nobody took the time to implement this properly, push it upstream and volunteer to maintain it. The problem is that you don't need just a "driver", but a new subsystem, that needs to be designed and maintained. Well, I believe there have been some lightweight superio locking coordinator patches been floating around on the lm_sensors list, and I have reviewed them and then a new version was done with my issues fixed. I kinda liked the proposed solution there, it was quite simple, moved all the generic superio stuff into generic superio code, and added locking for super io access from multiple drivers, what ever happened to those patches? If were to start using those, we could actually do a request region and then never release it, as things should be. Regards, Hans ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC] (almost) booting allyesconfig -- please don't poke super-io without request_region
On Fri, 11 Jul 2008 09:27:26 +0200, Hans de Goede wrote: > Jean Delvare wrote: > > Hi Hans, hi Milton, > > > > > > >> One could make a superio driver, and create sub-devices for the IR, > >> I2C, floppy, parallel, etc > >> nodes. > > > > There have been proposals to do this, and this would indeed be a very > > good idea, but unfortunately nobody took the time to implement this > > properly, push it upstream and volunteer to maintain it. The problem is > > that you don't need just a "driver", but a new subsystem, that needs to > > be designed and maintained. > > Well, I believe there have been some lightweight superio locking coordinator > patches been floating around on the lm_sensors list, and I have reviewed them > and then a new version was done with my issues fixed. > > I kinda liked the proposed solution there, it was quite simple, moved all the > generic superio stuff into generic superio code, and added locking for super > io > access from multiple drivers, what ever happened to those patches? As far as I know, nothing, and this is the problem. Somebody needs to step up and call him/herself the maintainer of the new code, and push it upstream and convert all the drivers (hwmon, watchdog, parallel port...) to make use of it. And I am not the one to do this, I am busy enough as is with i2c and hwmon. > If were to start using those, we could actually do a request region and then > never release it, as things should be. Yes, if we have a superio access coordinator, it can request the region and not release it. But as long as we don't have that, I agree with Milton that the individual drivers should temporarily request the Super-I/O region before accessing it. -- Jean Delvare ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC] (almost) booting allyesconfig -- please don't poke super-io without request_region
Jean Delvare wrote: On Fri, 11 Jul 2008 09:27:26 +0200, Hans de Goede wrote: Jean Delvare wrote: Hi Hans, hi Milton, One could make a superio driver, and create sub-devices for the IR, I2C, floppy, parallel, etc nodes. There have been proposals to do this, and this would indeed be a very good idea, but unfortunately nobody took the time to implement this properly, push it upstream and volunteer to maintain it. The problem is that you don't need just a "driver", but a new subsystem, that needs to be designed and maintained. Well, I believe there have been some lightweight superio locking coordinator patches been floating around on the lm_sensors list, and I have reviewed them and then a new version was done with my issues fixed. I kinda liked the proposed solution there, it was quite simple, moved all the generic superio stuff into generic superio code, and added locking for super io access from multiple drivers, what ever happened to those patches? As far as I know, nothing, and this is the problem. Somebody needs to step up and call him/herself the maintainer of the new code, and push it upstream and convert all the drivers (hwmon, watchdog, parallel port...) to make use of it. And I am not the one to do this, I am busy enough as is with i2c and hwmon. Ok, I've mailed Jim Cromie, the author of the superio access patches from end 2007 and told him we're still interested in this and asked him if he is willing to drive this forward. Regards, Hans ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC] (almost) booting allyesconfig -- please don't poke super-io without request_region
On Sun, 13 Jul 2008 15:26:56 -0600, David Hubbard wrote: > Hi Hans, > > >> I propose writing a subsystem driver. (Is that properly called "The > >> SuperIO Bus Driver"?) If no one thinks it's a really bad idea I will > >> put together some code and submit it for review, and maintain it. > >> > >> Some hwmon chips have odd / unique probe sequences. IMHO this is where > >> the design needs to be inspected. One of those is the w83627ehf, which > >> Jean and Hans are familiar enough with to check my work. > >> > >> Thoughts? > > > > I'm afraid that making this a "bus" will be a bit overkill. Jim's patches > > are quite simple and seem todo the job. > > > > Also keep in mind that most users will be platform devices which just want > > to use the superio registers to find out the baseaddress of their logical > > device, a whole bus seems overkill for this, would the hwmon driver then > > need to be a superio_driver (as well as an platform_driver) or can the bus > > be queried / used > > without having to be a bustype-driver? > > I think that's a valid point. I am willing to keep it small, but I > would prefer to follow the pattern set in other subsystems. It may be > my lack of experience in designing a subsystem showing here! What are > some alternative ways to implement it? > > In other words, Jim's patches are a good start, but maybe I am > misunderstanding them. I see it as the superio-locks module, a driver > that other drivers would depend on / auto-load. Is that something > other subsystems also do? Well, there are two approaches to the problem. The first approach (which I think Jim took in his patches? I don't really remember) is to simply solve the problem of concurrent I/O access to the Super-I/O configuration ports (typically 0x2e/0x2f or 0x4e/0x4f). That would be a simple driver requesting the ports in question and exporting an API for other drivers to access them in a safe way. The second approach is to make it a whole subsystem, as David is suggesting. The Super-I/O driver would then not only request the I/O ports, it would also identify which Super-I/O is there, and it would create devices (in the Linux device driver model sense of the term) for every logical device we are interested in (amongst which the hwmon ones.) The hwmon drivers would have to be converted from platform drivers to superio drivers. Each approach has its advantages. The first one is rather simple and also very generic in nature. It could be reused for other purposes. The second one offers automatic loading of hwmon drivers, which would be nice to have. There's probably a middle way which would keep the simplicity of the first approach while still allowing for driver auto-loading, without changing the bus type of all drivers. It would probably take some research though. Me, I don't really care which path we choose, given that I am not the one who will take care of it. All I have to say is that this has been discussed several times over the past 2 years and nothing came out of it (as far as the mainline kernel is concerned, at least) so whatever is done now, what really matters is that it makes it into the kernel quickly. We have some drivers missing features because of this (for example real-time reading of VID pin values.) This should also not prevent us from fixing the drivers now so that they temporarily request the Super-I/O ports they are using, as Milton suggested. Not only we don't know when we will have something better to offer, but it might also help us find conflicts between drivers, so that we know which drivers should make use of the future superio driver. This could also reveal conflicts with PNP BIOS reservations, etc. Milton, will you write a patch? -- Jean Delvare ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC] (almost) booting allyesconfig -- please don't poke super-io without request_region
On Jul 14, 2008, at 2:59 AM, Jean Delvare wrote: On Sun, 13 Jul 2008 15:26:56 -0600, David Hubbard wrote: Hi Hans, I propose writing a subsystem driver. (Is that properly called "The SuperIO Bus Driver"?) If no one thinks it's a really bad idea I will put together some code and submit it for review, and maintain it. Some hwmon chips have odd / unique probe sequences. IMHO this is where the design needs to be inspected. One of those is the w83627ehf, which Jean and Hans are familiar enough with to check my work. Thoughts? I'm afraid that making this a "bus" will be a bit overkill. Jim's patches are quite simple and seem todo the job. Also keep in mind that most users will be platform devices which just want to use the superio registers to find out the baseaddress of their logical device, a whole bus seems overkill for this, would the hwmon driver then need to be a superio_driver (as well as an platform_driver) or can the bus be queried / used without having to be a bustype-driver? I think that's a valid point. I am willing to keep it small, but I would prefer to follow the pattern set in other subsystems. It may be my lack of experience in designing a subsystem showing here! What are some alternative ways to implement it? In other words, Jim's patches are a good start, but maybe I am misunderstanding them. I see it as the superio-locks module, a driver that other drivers would depend on / auto-load. Is that something other subsystems also do? Well, there are two approaches to the problem. The first approach (which I think Jim took in his patches? I don't really remember) is to simply solve the problem of concurrent I/O access to the Super-I/O configuration ports (typically 0x2e/0x2f or 0x4e/0x4f). That would be a simple driver requesting the ports in question and exporting an API for other drivers to access them in a safe way. The second approach is to make it a whole subsystem, as David is suggesting. The Super-I/O driver would then not only request the I/O ports, it would also identify which Super-I/O is there, and it would create devices (in the Linux device driver model sense of the term) for every logical device we are interested in (amongst which the hwmon ones.) The hwmon drivers would have to be converted from platform drivers to superio drivers. Each approach has its advantages. The first one is rather simple and also very generic in nature. It could be reused for other purposes. The second one offers automatic loading of hwmon drivers, which would be nice to have. There's probably a middle way which would keep the simplicity of the first approach while still allowing for driver auto-loading, without changing the bus type of all drivers. It would probably take some research though. I haven't done the research, but it might be keep superio as a platform driver, and keep the clients as platform drivers. Only have the superio driver probe and discover the subcomponent addresses and then create the platform devices as children instead of having each driver create its own platform device. (This all assumes they are all platform devices in sysfs, I have not looked). This is all because in the platform bus the bus driver does not discover the addresses but relies on drivers or platform setup code. Me, I don't really care which path we choose, given that I am not the one who will take care of it. All I have to say is that this has been discussed several times over the past 2 years and nothing came out of it (as far as the mainline kernel is concerned, at least) so whatever is done now, what really matters is that it makes it into the kernel quickly. We have some drivers missing features because of this (for example real-time reading of VID pin values.) This should also not prevent us from fixing the drivers now so that they temporarily request the Super-I/O ports they are using, as Milton suggested. Not only we don't know when we will have something better to offer, but it might also help us find conflicts between drivers, so that we know which drivers should make use of the future superio driver. This could also reveal conflicts with PNP BIOS reservations, etc. Milton, will you write a patch? Well, that is the second time you asked me, so I guess I should respond. While I it is possible for me to write this patch, my schedule and priority list predict it would not be before the merge window closes. In fact, I'm not sure when it would come out. It might be argued it could go in early -rc, but I would fear somebody's chip will not be detected with the additional check. For example, the port may reserved as mother board resources or something. Also, I have no hardware to test any proposed patch, so it would be compile check only. milton ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC] (almost) booting allyesconfig -- please don't poke super-io without request_region
Hi Milton, On Mon, 14 Jul 2008 12:09:03 -0500, Milton Miller wrote: > On Jul 14, 2008, at 2:59 AM, Jean Delvare wrote: > > Well, there are two approaches to the problem. The first approach > > (which I think Jim took in his patches? I don't really remember) is to > > simply solve the problem of concurrent I/O access to the Super-I/O > > configuration ports (typically 0x2e/0x2f or 0x4e/0x4f). That would be a > > simple driver requesting the ports in question and exporting an API for > > other drivers to access them in a safe way. > > > > The second approach is to make it a whole subsystem, as David is > > suggesting. The Super-I/O driver would then not only request the I/O > > ports, it would also identify which Super-I/O is there, and it would > > create devices (in the Linux device driver model sense of the term) for > > every logical device we are interested in (amongst which the hwmon > > ones.) The hwmon drivers would have to be converted from platform > > drivers to superio drivers. > > > > Each approach has its advantages. The first one is rather simple and > > also very generic in nature. It could be reused for other purposes. The > > second one offers automatic loading of hwmon drivers, which would be > > nice to have. > > > > There's probably a middle way which would keep the simplicity of the > > first approach while still allowing for driver auto-loading, without > > changing the bus type of all drivers. It would probably take some > > research though. > > I haven't done the research, but it might be keep superio as > a platform driver, and keep the clients as platform drivers. Only > have the superio driver probe and discover the subcomponent > addresses and then create the platform devices as children > instead of having each driver create its own platform device. > (This all assumes they are all platform devices in sysfs, I have > not looked). > > This is all because in the platform bus the bus driver does not > discover the addresses but relies on drivers or platform setup code. That's more or less what I had in mind, yes. > > (...) Milton, will you write a patch? > > Well, that is the second time you asked me, so I guess I should respond. > > While I it is possible for me to write this patch, my schedule and > priority list predict it would not be before the merge window closes. > In fact, I'm not sure when it would come out. It might be argued it > could go in early -rc, but I would fear somebody's chip will not be detected > with the additional check. For example, the port may reserved as mother > board resources or something. I really don't see this as something for kernel 2.6.27, it's too late already. It doesn't fix any actual problem anyway (none that can be fixed by not loading drivers you don't need, at least.) That would be for 2.6.28, so we have plenty of time to test the changes and ensure they do not break anything. As you are the one who reported the issue as something that was bothering you personally, I expected you to also spend some time trying to address it. > (...) Also, I have no hardware to test any > proposed patch, so it would be compile check only. If you write the patches and post them to the lm-sensors list, I am certain that you will find several volunteers for testing. Me, I can offer testing of the it87, f71805f and w83627ehf drivers. Thanks, -- Jean Delvare ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC] (almost) booting allyesconfig -- please don't poke super-io without request_region
On Mon, 14 Jul 2008 11:55:01 -0600, David Hubbard wrote: > Hi Hans, > > On Mon, Jul 14, 2008 at 11:30 AM, Hans de Goede <[EMAIL PROTECTED]> wrote: > > Milton Miller wrote: > >> I haven't done the research, but it might be keep superio as > >> a platform driver, and keep the clients as platform drivers. Only > >> have the superio driver probe and discover the subcomponent > >> addresses and then create the platform devices as children > >> instead of having each driver create its own platform device. > >> (This all assumes they are all platform devices in sysfs, I have > >> not looked). > >> > >> This is all because in the platform bus the bus driver does not > >> discover the addresses but relies on drivers or platform setup code. > > > > This sounds like a good plan, rather then add a new bus type add a superio > > platform driver which does superio probing and registering of platform > > devices > > for discovered logical devices. > > > > This superio platform driver then needs to also export some functions of > > those > > few logical devices which need access to the superio registers for more then > > just finding out their own base address. > > > > I guess that it then would be best to load this superio driver by default on > > most systems. > > > > How does this all mix and match with isapnp, it feels to me we're doing > > somewhat the same as isapnp here. > > Is there any way to use lspci and start at the LPC bridge, then find > the SuperIO chip's IO address? What about ACPI tables? Perhaps probing > logic could look for an LPC bridge before probing certain IO addresses > even if the addresses are not in the LPC bridge config. I always assumed that there was no way to know in advance if a Super-I/O (LPC) chip was present or not, let alone the exact model of the chip. The I/O addresses are decoded by the Super-I/O chip itself, and in general it has no relation to PCI. And I've never seen ports 0x2e/0x2f nor 0x4e/0x4f listed in /proc/ioports. But of course if there is a way to know, we should use it. Avoiding random access to I/O ports, even if they are relatively standard in this case, is always good. > A superio platform driver is a good way to go -- it fits with the way > the platform bus does things. Also, Jim's patches are almost there > already. Good. -- Jean Delvare ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC] (almost) booting allyesconfig -- please don't poke super-io without request_region
Hi Jean, On Tue, Jul 15, 2008 at 2:36 AM, Jean Delvare <[EMAIL PROTECTED]> wrote: >> Is there any way to use lspci and start at the LPC bridge, then find >> the SuperIO chip's IO address? What about ACPI tables? Perhaps probing >> logic could look for an LPC bridge before probing certain IO addresses >> even if the addresses are not in the LPC bridge config. > > I always assumed that there was no way to know in advance if a > Super-I/O (LPC) chip was present or not, let alone the exact model of > the chip. The I/O addresses are decoded by the Super-I/O chip itself, > and in general it has no relation to PCI. And I've never seen ports > 0x2e/0x2f nor 0x4e/0x4f listed in /proc/ioports. > > But of course if there is a way to know, we should use it. Avoiding > random access to I/O ports, even if they are relatively standard in > this case, is always good. I looked at my lspci output and did a little researching, and I think the only thing we can deduce is that there is an LPC bridge, so looking for a SuperIO is a good idea. If there is no LPC bridge listed, I can't say whether probing the ports is a good idea or not. David ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC] (almost) booting allyesconfig -- please don't poke super-io without request_region
Hi David, On Tue, 15 Jul 2008 09:31:29 -0600, David Hubbard wrote: > On Tue, Jul 15, 2008 at 2:36 AM, Jean Delvare <[EMAIL PROTECTED]> wrote: > > I always assumed that there was no way to know in advance if a > > Super-I/O (LPC) chip was present or not, let alone the exact model of > > the chip. The I/O addresses are decoded by the Super-I/O chip itself, > > and in general it has no relation to PCI. And I've never seen ports > > 0x2e/0x2f nor 0x4e/0x4f listed in /proc/ioports. > > > > But of course if there is a way to know, we should use it. Avoiding > > random access to I/O ports, even if they are relatively standard in > > this case, is always good. > > I looked at my lspci output and did a little researching, and I think > the only thing we can deduce is that there is an LPC bridge, so > looking for a SuperIO is a good idea. If there is no LPC bridge > listed, I can't say whether probing the ports is a good idea or not. Machines I have here have an "ISA bridge" PCI device. Some of them have "LPC" in their name, but not all, and anyway the kernel only knows the device ID, not its user-friendly name: VIA: 00:11.0 ISA bridge: VIA Technologies, Inc. VT8237 ISA bridge [KT600/K8T800/K8T890 South] Intel: 00:01.0 ISA bridge: Intel Corporation 82371AB/EB/MB PIIX4 ISA (rev 01) 00:1f.0 ISA bridge: Intel Corporation 82801CAM ISA Bridge (LPC) (rev 01) 00:1f.0 ISA bridge: Intel Corporation 82801EB/ER (ICH5/ICH5R) LPC Interface Bridge (rev 02) nVidia: 00:01.0 ISA bridge: nVidia Corporation nForce2 ISA Bridge (rev a4) One of these machines has a LPC bridge but no (detected) Super-I/O chip. The VIA and nVidia machines do not have "LPC" in their bridge name, but they do have a Super-I/O chip, while the first Intel machine doesn't. As a conclusion, there is no clear relationship between the presence of an ISA or LPC bridge and the presence of a Super-I/O chip. All we can say is that apparently all PC systems have an ISA bridge. So indeed we can probably make the probes conditional upon the presence of an "ISA bridge" PCI device. That's very easy to test. In practice it might be equivalent to making the driver x86-only. -- Jean Delvare ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC] (almost) booting allyesconfig -- please don't poke super-io without request_region
On 16-07-08 09:46, Jean Delvare wrote: As a conclusion, there is no clear relationship between the presence of an ISA or LPC bridge and the presence of a Super-I/O chip. All we can say is that apparently all PC systems have an ISA bridge. So indeed we can probably make the probes conditional upon the presence of an "ISA bridge" PCI device. That's very easy to test. In practice it might be equivalent to making the driver x86-only. And foregoing non (pre-) PCI. Admittedly, that might not be much of an issue and I don't know if "Super-I/O" is maybe even defined such as to make it a NON issue but I do for example remember my old 386 chipset providing for extended serial rates which I thought was way cool back then. If that's Super-I/O (or something that should/could be covered by the infrastructure at least) then PCI wouldn't be helpful. Rene. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
