| Message ID | 20100810155559.7620.79711.stgit@localhost.localdomain |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
Hello. Alan Cox wrote: > Intel IDE-R devices are part of the Intel AMT management setup. They don't > have any special configuration registers or settings so the ata_generic > driver will support them fully. > Rather than add a huge table of IDs for each chipset and keep sending in > new ones this patch autodetects them. > (And yes Jeff I'll resurrect the delay patches in a couple of weeks) Doesn't have to be in the changelog... > Signed-off-by: Alan Cox <alan@linux.intel.com> [...] > diff --git a/drivers/ata/ata_generic.c b/drivers/ata/ata_generic.c > index cc5f772..476f194 100644 > --- a/drivers/ata/ata_generic.c > +++ b/drivers/ata/ata_generic.c [...] > @@ -109,6 +110,44 @@ static struct ata_port_operations generic_port_ops = { > static int all_generic_ide; /* Set to claim all devices */ > > /** > + * is_intel_ider - identify intel IDE-R devices > + * @dev: PCI device > + * > + * Distinguish Intel IDE-R controller devices from other Intel IDE > + * devices. IDE-R devices have no timing registers and are in > + * most respects virtual. They should be driven by the ata_generic > + * driver. > + */ > + > +static int is_intel_ider(struct pci_dev *dev) > +{ > + /* For Intel IDE the value at 0xF8 is only zero on IDE-R > + interfaces */ Preferred style for multi-line comments is: /* * bla * bla */ > + u32 r; > + u16 t; > + > + pci_read_config_dword(dev, 0xF8, &r); > + /* Not IDE-R: punt so that ata_(old)piix gets it */ > + if (r != 0) > + return 0; > + /* 0xF8 is also be zero on some early Intel IDE devices Grammar. > + but they will have a sane timing register */ The same remark about the comment style... > + pci_read_config_word(dev, 0x40, &t); > + if (t != 0) > + return 0; > + /* Finally check if the timing register is writable so that > + we eliminate any early devices hot-docked in a docking > + station */ ... and here too. > @@ -134,6 +173,10 @@ static int ata_generic_init_one(struct pci_dev *dev, const struct pci_device_id > if ((id->driver_data & ATA_GEN_CLASS_MATCH) && all_generic_ide == 0) > return -ENODEV; > > + if (id->driver_data && ATA_GEN_INTEL_IDER) I think you mean & here, rather than &&... MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> > +static int is_intel_ider(struct pci_dev *dev) > > +{ > > + /* For Intel IDE the value at 0xF8 is only zero on IDE-R > > + interfaces */ > > Preferred style for multi-line comments is: This driver uses the style I have throughout. > > @@ -134,6 +173,10 @@ static int ata_generic_init_one(struct pci_dev > > *dev, const struct pci_device_id if ((id->driver_data & > > ATA_GEN_CLASS_MATCH) && all_generic_ide == 0) return -ENODEV; > > > > + if (id->driver_data && ATA_GEN_INTEL_IDER) > > I think you mean & here, rather than &&... Oh yes most definitely, that would have unfortunate consequences. Thanks Alan -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/10/2010 05:56 PM, Alan Cox wrote: > Intel IDE-R devices are part of the Intel AMT management setup. They don't > have any special configuration registers or settings so the ata_generic > driver will support them fully. > > Rather than add a huge table of IDs for each chipset and keep sending in > new ones this patch autodetects them. > > (And yes Jeff I'll resurrect the delay patches in a couple of weeks) > > Signed-off-by: Alan Cox <alan@linux.intel.com> ... > +static int is_intel_ider(struct pci_dev *dev) > +{ > + /* For Intel IDE the value at 0xF8 is only zero on IDE-R > + interfaces */ > + u32 r; > + u16 t; > + > + pci_read_config_dword(dev, 0xF8, &r); > + /* Not IDE-R: punt so that ata_(old)piix gets it */ > + if (r != 0) > + return 0; > + /* 0xF8 is also be zero on some early Intel IDE devices > + but they will have a sane timing register */ > + pci_read_config_word(dev, 0x40, &t); > + if (t != 0) > + return 0; > + /* Finally check if the timing register is writable so that > + we eliminate any early devices hot-docked in a docking > + station */ > + pci_write_config_word(dev, 0x40, 1); > + pci_read_config_word(dev, 0x40, &t); > + if (t) { > + pci_write_config_word(dev, 0x40, 0); > + return 0; > + } > + return 1; > +} This looks scary to me. Is this something documented somewhere? It's not like we can avoid adding PCI device IDs completely anyway, so I would suggest just doing it good old fashioned way. Thanks.
Hello, On 08/17/2010 06:42 PM, Alan Cox wrote: >> This looks scary to me. Is this something documented somewhere? It's >> not like we can avoid adding PCI device IDs completely anyway, so I >> would suggest just doing it good old fashioned way. > > The trouble with adding all the ids is it will keep needing updates every > chipset (and a lot of existing idents). But we do that for some controllers anyway so it's not something new. ata_piix already has list of all the devices it supports, so maybe it's safe to grab all intel IDE devices from ata_generic? ata_piix always has higher priority than ata_generic anyway and if a device isn't grabbed by ata_piix, we don't lose anything by grabbing it from ata_generic. > Is it documented ? The instructions it was written to came from the > people who do the chips. Yeap, that's much better, but I still think it would be better to avoid such detection magics if possible. Thanks.
> This looks scary to me. Is this something documented somewhere? It's > not like we can avoid adding PCI device IDs completely anyway, so I > would suggest just doing it good old fashioned way. The trouble with adding all the ids is it will keep needing updates every chipset (and a lot of existing idents). Is it documented ? The instructions it was written to came from the people who do the chips. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello, Alan. On 08/17/2010 07:01 PM, Alan Cox wrote: >> ata_piix already has list of all the devices it supports, so maybe >> it's safe to grab all intel IDE devices from ata_generic? ata_piix > > I can't guarantee that and I'm not sure I can find anyone at Intel with > that deep a knowledge of the earliest chip errata. But ata_generic grabbing a controller doesn't mean it's gonna be messed up in anyway. That's what windows does for unknown IDE controllers after all. The problem we want to avoid here is using ata_generic for a controller which already has a proper driver. >> always has higher priority than ata_generic anyway and if a device >> isn't grabbed by ata_piix, we don't lose anything by grabbing it from >> ata_generic. > > Priority only works if you know what is there to use, and the piix one is > for example broken if you change the bindings with sysfs. You don't for > example want to forget to compile in one driver and get generic when that > is unsafe. I don't really think it would be dangerous to grab intel IDE controllers with ata_generic. Again, that's what windows would do. And for sysfs unbind case, if the user specifically unbinds the controller from ata_piix, what is broken? > The extreme example would be if you have ata_generic binding to all > devices and you forgot to compile in RZ1000 support. On the relevant > board that just ate your file system. > > So the grab it all approach was dismissed. For RZ1000, sure, but we're talking about only intel IDEs here. >>> Is it documented ? The instructions it was written to came from the >>> people who do the chips. >> >> Yeap, that's much better, but I still think it would be better to >> avoid such detection magics if possible. > > Well yes but the logic is simple. > > 0xF8 is non zero on all later Intel ATA PCI chipsets > 0x40 is writable on all earlier Intel ATA PCI chipsets, and zero + non > writable on the IDE-R devices. Maybe it's okay now but who's gonna remember what's going on there after five years and nobody guarantees the above would continue to hold in the future. > The other thing I did consider was submitting an Intel IDE-R driver that > contained the check and matched IDE CLASS & vendor == INTEL. That has the > advantage of not getting autoloaded unnecessarily so easily, but means we > have another driver. Yeah, I think it would be better to do it in ata_generic one way or the other. Thanks.
> ata_piix already has list of all the devices it supports, so maybe > it's safe to grab all intel IDE devices from ata_generic? ata_piix I can't guarantee that and I'm not sure I can find anyone at Intel with that deep a knowledge of the earliest chip errata. > always has higher priority than ata_generic anyway and if a device > isn't grabbed by ata_piix, we don't lose anything by grabbing it from > ata_generic. Priority only works if you know what is there to use, and the piix one is for example broken if you change the bindings with sysfs. You don't for example want to forget to compile in one driver and get generic when that is unsafe. The extreme example would be if you have ata_generic binding to all devices and you forgot to compile in RZ1000 support. On the relevant board that just ate your file system. So the grab it all approach was dismissed. > > Is it documented ? The instructions it was written to came from the > > people who do the chips. > > Yeap, that's much better, but I still think it would be better to > avoid such detection magics if possible. Well yes but the logic is simple. 0xF8 is non zero on all later Intel ATA PCI chipsets 0x40 is writable on all earlier Intel ATA PCI chipsets, and zero + non writable on the IDE-R devices. The other thing I did consider was submitting an Intel IDE-R driver that contained the check and matched IDE CLASS & vendor == INTEL. That has the advantage of not getting autoloaded unnecessarily so easily, but means we have another driver. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> I don't really think it would be dangerous to grab intel IDE > controllers with ata_generic. Again, that's what windows would do. > And for sysfs unbind case, if the user specifically unbinds the > controller from ata_piix, what is broken? If the user changes the PCI id list what happens ? > > > The extreme example would be if you have ata_generic binding to all > > devices and you forgot to compile in RZ1000 support. On the relevant > > board that just ate your file system. > > > > So the grab it all approach was dismissed. > > For RZ1000, sure, but we're talking about only intel IDEs here. I don't know. I do know what the Intel folks have told me about detecting them. > > 0xF8 is non zero on all later Intel ATA PCI chipsets > > 0x40 is writable on all earlier Intel ATA PCI chipsets, and zero + non > > writable on the IDE-R devices. > > Maybe it's okay now but who's gonna remember what's going on there > after five years and nobody guarantees the above would continue to Add a comment ? > hold in the future. It did occur to me to check this would be true in the future. Either way - an id table would go out of date more often. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello, On 08/17/2010 08:23 PM, Alan Cox wrote: >> I don't really think it would be dangerous to grab intel IDE >> controllers with ata_generic. Again, that's what windows would do. >> And for sysfs unbind case, if the user specifically unbinds the >> controller from ata_piix, what is broken? > > If the user changes the PCI id list what happens ? Then that's upto the user, right? That's what the (root) user wants. You can add any random device ID to any random driver and if you do it wrong something will break. That's not different from any other driver. >> Maybe it's okay now but who's gonna remember what's going on there >> after five years and nobody guarantees the above would continue to > > Add a comment ? Yeah, definitely. Fat amount of them. >> hold in the future. > > It did occur to me to check this would be true in the future. Either way > - an id table would go out of date more often. Sure, the thing is that your patch doesn't mean we don't have to keep ata_piix device table up-to-date. We would still need to be maintaining that table whether ata_generic can detect IDE-R by itself or not. The device ID table in ata_piix is given, and ata_generic picking up the rest of intel IDE's wouldn't miss anything. So, unless IDE-R devices need some special treatment, I don't really see how the detection code would be useful. Thanks.
> Sure, the thing is that your patch doesn't mean we don't have to keep > ata_piix device table up-to-date. We would still need to be > maintaining that table whether ata_generic can detect IDE-R by itself I'm not sure I follow > or not. The device ID table in ata_piix is given, and ata_generic > picking up the rest of intel IDE's wouldn't miss anything. So, unless > IDE-R devices need some special treatment, I don't really see how the > detection code would be useful. The trend is towards AHCI so the problem goes away for the other bits. IDE-R is not ata_piix drivable, and lots of the ICH stuff really wants driving via ata_piix, so having the generic driver grab all intel stuff isn't as far as I can see going to be safe given a system may load the ata_generic module but not PIIX then meet a piix by hotplug. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello, On 08/18/2010 12:03 PM, Alan Cox wrote: >> Sure, the thing is that your patch doesn't mean we don't have to keep >> ata_piix device table up-to-date. We would still need to be >> maintaining that table whether ata_generic can detect IDE-R by itself > > I'm not sure I follow I was trying to say that IDE-R basically being the complement of Intel IDE's not drive by ata_piix, we don't need to maintain a separate PCI device ID table for IDE-R's. >> or not. The device ID table in ata_piix is given, and ata_generic >> picking up the rest of intel IDE's wouldn't miss anything. So, unless >> IDE-R devices need some special treatment, I don't really see how the >> detection code would be useful. > > The trend is towards AHCI so the problem goes away for the other bits. Yeah, it has helped a lot but ata_piix's are here to stay for the foreseeable future and we'll be maintaining its device ID table. > IDE-R is not ata_piix drivable, and lots of the ICH stuff really wants > driving via ata_piix, so having the generic driver grab all intel stuff > isn't as far as I can see going to be safe given a system may load the > ata_generic module but not PIIX then meet a piix by hotplug. That's the reason why we have module priorities. The link priority becomes module priority and modprobe will deterministically prefer ata_piix over ata_generic if a controller is supported by both. Thanks.
> That's the reason why we have module priorities. The link priority > becomes module priority and modprobe will deterministically prefer > ata_piix over ata_generic if a controller is supported by both. Sure - but if ata_generic is already compiled in or already loaded ? -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello, On 08/18/2010 05:15 PM, Alan Cox wrote: >> That's the reason why we have module priorities. The link priority >> becomes module priority and modprobe will deterministically prefer >> ata_piix over ata_generic if a controller is supported by both. > > Sure - but if ata_generic is already compiled in or already loaded ? Well, what if pata_acpi is already compiled in or already loaded? Or what if ata_piix is already compiled in or already loaded but ahci is not for an ich6 controller? We have priorities in the drivers, if the user/admin wants to use the lower priority ones that much, he/she shall. This isn't an issue. There's no way to hotplug intel IDEs anyway. Thanks.
> > Sure - but if ata_generic is already compiled in or already loaded ? > > Well, what if pata_acpi is already compiled in or already loaded? Or Yes I am aware that one also has problems but unlike IDER I don't see a cure. > what if ata_piix is already compiled in or already loaded but ahci is > not for an ich6 controller? We have priorities in the drivers, if the > user/admin wants to use the lower priority ones that much, he/she > shall. This isn't an issue. There's no way to hotplug intel IDEs > anyway. The PIIX4 in the IBM docking stations is hot plug - and its why I also have to check 0x40 and write it if zero. So the case exists. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Tejun Heo wrote: > There's no way to hotplug intel IDEs > anyway. > Won't kvm (or similar virtualisation system) + PCI passthrough allow you to hot-plug arbitrary PCI devices? Bit of a corner-case I know, but I can see people occasionally doing this... Tim. -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
(cc'ing Kay, hi!) Hello, On 08/19/2010 12:09 PM, Alan Cox wrote: >> what if ata_piix is already compiled in or already loaded but ahci is >> not for an ich6 controller? We have priorities in the drivers, if the >> user/admin wants to use the lower priority ones that much, he/she >> shall. This isn't an issue. There's no way to hotplug intel IDEs >> anyway. > > The PIIX4 in the IBM docking stations is hot plug - and its why I also > have to check 0x40 and write it if zero. So the case exists. I see. Most of the problems are solved by driver priority but yeah that's an actual corner case. Driver priority doesn't work unless all the matching drivers are loaded by the time driver matching starts, so the same problem could happen with pata_acpi depending on probe order during normal boot, so pata_acpi should be blacklisted by default. Kay, there are a few ATA drivers with overlapping hardware support. When all the drivers are built-in the kernel or none of them is loaded as modules, priority among them as defined by link order is followed; however, if one of the drivers is already loaded while other overlapping ones are not, there's nothing which guarantees driver probing would happen after modprobe for the device ID is complete, right? So, the already loaded driver may be attached to the controller even when there is a higher priority driver. Am I missing something? Thanks.
On Thu, 2010-08-19 at 13:22 +0200, Tejun Heo wrote: > Kay, there are a few ATA drivers with overlapping hardware support. > When all the drivers are built-in the kernel or none of them is loaded > as modules, priority among them as defined by link order is followed; > however, if one of the drivers is already loaded while other > overlapping ones are not, there's nothing which guarantees driver > probing would happen after modprobe for the device ID is complete, Not sure it if I get it right. You mean the first driver binds to the device already, right? But you want the later one to be used? Any driver that binds to a device will prevent any other later loaded driver from binding to, or handling the device. The only way to have the second driver seeing the device is unbinding the first driver in sysfs, and binding the second one in sysfs. Kay -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello, On 08/19/2010 01:35 PM, Kay Sievers wrote: > On Thu, 2010-08-19 at 13:22 +0200, Tejun Heo wrote: >> Kay, there are a few ATA drivers with overlapping hardware support. >> When all the drivers are built-in the kernel or none of them is loaded >> as modules, priority among them as defined by link order is followed; >> however, if one of the drivers is already loaded while other >> overlapping ones are not, there's nothing which guarantees driver >> probing would happen after modprobe for the device ID is complete, > > Not sure it if I get it right. You mean the first driver binds to the > device already, right? But you want the later one to be used? > > Any driver that binds to a device will prevent any other later loaded > driver from binding to, or handling the device. The only way to have the > second driver seeing the device is unbinding the first driver in sysfs, > and binding the second one in sysfs. It's a bit messy to explain. So, if all the drivers are already inside the kernel device - driver matching would follow the priority. If none of the drivers is in the kernel, udev will call modprobe to with device ID and modprobe will follow the link order and load the matching drivers in link order. What I was curious about is what happens if some of them are already in the kernel but not all of them. udev would still be invoked the same and modprobe too but device - driver matching can and is probably likely to happen before modprobe finishes loading all the drivers, right? Or is there a mechanism to hold off device - driver matching before the udev finishes handling the event for the controller? Thanks.
On Thu, Aug 19, 2010 at 13:42, Tejun Heo <tj@kernel.org> wrote: > What I was curious about is what > happens if some of them are already in the kernel but not all of them. > udev would still be invoked the same and modprobe too but device - > driver matching can and is probably likely to happen before modprobe > finishes loading all the drivers, right? Or is there a mechanism to > hold off device - driver matching before the udev finishes handling > the event for the controller? The compiled-in drivers have matched and bound and blocked-out any other driver of the device long before udev is even started load the other drivers. The matching compiled-in drivers will always win over the modules, regardless of their priority. Is that what you mean? :) Kay -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello, On 08/19/2010 02:24 PM, Kay Sievers wrote: > The compiled-in drivers have matched and bound and blocked-out any > other driver of the device long before udev is even started load the > other drivers. The matching compiled-in drivers will always win over > the modules, regardless of their priority. > > Is that what you mean? :) It's still a bit fuzzy to me. So, the exact scenario I'm thinking about is something like the following. 1. System discovers an unsupported IDE class controller. 2. udev kicks in and loads pata_acpi for it which can serve as a fallback driver for all IDE class controllers. 3. System discovers a controller which is supported by ata_piix. ata_piix is the correct driver for it w/ higher priority than pata_acpi. I'm curious about the followings, a. udev invokes modprobe on device added events, no? Does it matter whether there's a matching driver in the kernel (built-in or an already loaded module) to udev behavior? You seem to be implying that if there's a matching driver, udev wouldn't invoke modprobe at all. b. modprobe, when invoked, will load all the matching drivers in link order. If udev invokes modprobe regardless of existing drivers, is there a mechanism to hold off device - driver matching before modprobe is finished? Probably not, right? The whole priority thing seems kind of broken. It's good enough for simple cases but just breaks down as soon as things get closer to corner cases. Maybe it's best to leave it alone as mostly working workaround for simple problems. :-( Thanks.
On Thu, Aug 19, 2010 at 14:33, Tejun Heo <tj@kernel.org> wrote: > I'm curious about the followings, > > a. udev invokes modprobe on device added events, no? Yeah, in this case it from off of the pci devices. Most of the events for pci devices run in parallel. > Does it matter > whether there's a matching driver in the kernel (built-in or an > already loaded module) to udev behavior? If the device we are currently handling has already a driver bound, udev will not call modprobe. It's: DRIVER!="?*", ENV{MODALIAS}=="?*", RUN+="/sbin/modprobe -bv $env{MODALIAS}" which means: !DRIVER && MODALIAS -> modprobe > You seem to be implying > that if there's a matching driver, udev wouldn't invoke modprobe at > all. It doesn't call modprobe for this device in that case, yes. But some other similar device may still cause the driver to be loaded and every driver load will scan all devices and try to bind them. > b. modprobe, when invoked, will load all the matching drivers in link > order. If udev invokes modprobe regardless of existing drivers, Yeah, udev only cares about the device state, and if the device has a driver bound, not if there are other drivers (bound or unbound to devices) in the kernel already. > is > there a mechanism to hold off device - driver matching before > modprobe is finished? Probably not, right? What does 'modprobe finished' mean? The modprobe called for this specific device? The first driver loaded and initialized will win. If multiple drivers are loaded with the same modprobe call, the later ones will never see the device. Kay -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello, On 08/19/2010 02:52 PM, Kay Sievers wrote: > If the device we are currently handling has already a driver bound, > udev will not call modprobe. > It's: > DRIVER!="?*", ENV{MODALIAS}=="?*", RUN+="/sbin/modprobe -bv $env{MODALIAS}" > which means: > !DRIVER && MODALIAS -> modprobe Okay, thanks for the explanation. >> is >> there a mechanism to hold off device - driver matching before >> modprobe is finished? Probably not, right? > > What does 'modprobe finished' mean? The modprobe called for this > specific device? The first driver loaded and initialized will win. If > multiple drivers are loaded with the same modprobe call, the later > ones will never see the device. Yeap, you're right. Loading all drivers is for cases where controllers supported by different drivers share the same PCI ID (some of hpt's need this). So, the priority thing is mostly broken except for pretty simple cases. :-( Oh well, it has worked well enough till now. Let's leave it alone for now. Thanks.
Hello, again. On 08/19/2010 01:22 PM, Tejun Heo wrote: > On 08/19/2010 12:09 PM, Alan Cox wrote: >>> what if ata_piix is already compiled in or already loaded but ahci is >>> not for an ich6 controller? We have priorities in the drivers, if the >>> user/admin wants to use the lower priority ones that much, he/she >>> shall. This isn't an issue. There's no way to hotplug intel IDEs >>> anyway. >> >> The PIIX4 in the IBM docking stations is hot plug - and its why I also >> have to check 0x40 and write it if zero. So the case exists. > > I see. Most of the problems are solved by driver priority but yeah > that's an actual corner case. Driver priority doesn't work unless all > the matching drivers are loaded by the time driver matching starts, so > the same problem could happen with pata_acpi depending on probe order > during normal boot, so pata_acpi should be blacklisted by default. It seems priority thing can't handle this at all. So, to the original patch, Acked-by: Tejun Heo <tj@kernel.org> Thanks.
On Thu, Aug 19, 2010 at 14:54, Tejun Heo <tj@kernel.org> wrote: > Loading all drivers is for cases where > controllers supported by different drivers share the same PCI ID (some > of hpt's need this). So, the priority thing is mostly broken except > for pretty simple cases. :-( Oh well, it has worked well enough till > now. Let's leave it alone for now. Well, I wouldn't really call it broken. It's very helpful to have these priorities, and it solved a bunch of real problems because in most cases it produces predictable results, unlike it was before. That compiled-in and and modules don't really mix here regarding your use case does not really matter in the general picture, it's still a predictable behavior, and we really need the priorities. If we need finer-grained policy here, we need to move the driver binding to userspace. The driver core and udev can do that already. But nobody of us is so crazy to enable it, and handle all the fallout. But it's possible in theory ... :) Kay -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello, On 08/19/2010 03:08 PM, Kay Sievers wrote: > Well, I wouldn't really call it broken. It's very helpful to have > these priorities, and it solved a bunch of real problems because in > most cases it produces predictable results, unlike it was before. > > That compiled-in and and modules don't really mix here regarding your > use case does not really matter in the general picture, it's still a > predictable behavior, and we really need the priorities. Yeah, in most usual cases, it has been fine. I was just hoping that it would work better so that fallback drivers can be handled with it. For now, it seems there is no safe way to have a generic fallback driver (be it pata_acpi or ata_generic). > If we need finer-grained policy here, we need to move the driver > binding to userspace. The driver core and udev can do that already. > But nobody of us is so crazy to enable it, and handle all the fallout. > But it's possible in theory ... :) Right, userland already has enough mechanism and information to handle the driver binding problem correctly, but given how little problem it has been causing till now, I think that would be an overkill, for now at least. Thanks.
On 08/19/2010 08:56 AM, Tejun Heo wrote: > Hello, again. > > On 08/19/2010 01:22 PM, Tejun Heo wrote: >> On 08/19/2010 12:09 PM, Alan Cox wrote: >>>> what if ata_piix is already compiled in or already loaded but ahci is >>>> not for an ich6 controller? We have priorities in the drivers, if the >>>> user/admin wants to use the lower priority ones that much, he/she >>>> shall. This isn't an issue. There's no way to hotplug intel IDEs >>>> anyway. >>> >>> The PIIX4 in the IBM docking stations is hot plug - and its why I also >>> have to check 0x40 and write it if zero. So the case exists. >> >> I see. Most of the problems are solved by driver priority but yeah >> that's an actual corner case. Driver priority doesn't work unless all >> the matching drivers are loaded by the time driver matching starts, so >> the same problem could happen with pata_acpi depending on probe order >> during normal boot, so pata_acpi should be blacklisted by default. > > It seems priority thing can't handle this at all. So, to the original > patch, > > Acked-by: Tejun Heo<tj@kernel.org> Cool, though still waiting on resend to fix problems mentioned earlier in the thread, notably s/&&/&/ Or if Alan wants to sign-off on my doing that correction myself, since a resend hasn't appeared? -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/ata/ata_generic.c b/drivers/ata/ata_generic.c index cc5f772..476f194 100644 --- a/drivers/ata/ata_generic.c +++ b/drivers/ata/ata_generic.c @@ -35,6 +35,7 @@ enum { ATA_GEN_CLASS_MATCH = (1 << 0), ATA_GEN_FORCE_DMA = (1 << 1), + ATA_GEN_INTEL_IDER = (1 << 2), }; /** @@ -109,6 +110,44 @@ static struct ata_port_operations generic_port_ops = { static int all_generic_ide; /* Set to claim all devices */ /** + * is_intel_ider - identify intel IDE-R devices + * @dev: PCI device + * + * Distinguish Intel IDE-R controller devices from other Intel IDE + * devices. IDE-R devices have no timing registers and are in + * most respects virtual. They should be driven by the ata_generic + * driver. + */ + +static int is_intel_ider(struct pci_dev *dev) +{ + /* For Intel IDE the value at 0xF8 is only zero on IDE-R + interfaces */ + u32 r; + u16 t; + + pci_read_config_dword(dev, 0xF8, &r); + /* Not IDE-R: punt so that ata_(old)piix gets it */ + if (r != 0) + return 0; + /* 0xF8 is also be zero on some early Intel IDE devices + but they will have a sane timing register */ + pci_read_config_word(dev, 0x40, &t); + if (t != 0) + return 0; + /* Finally check if the timing register is writable so that + we eliminate any early devices hot-docked in a docking + station */ + pci_write_config_word(dev, 0x40, 1); + pci_read_config_word(dev, 0x40, &t); + if (t) { + pci_write_config_word(dev, 0x40, 0); + return 0; + } + return 1; +} + +/** * ata_generic_init - attach generic IDE * @dev: PCI device found * @id: match entry @@ -134,6 +173,10 @@ static int ata_generic_init_one(struct pci_dev *dev, const struct pci_device_id if ((id->driver_data & ATA_GEN_CLASS_MATCH) && all_generic_ide == 0) return -ENODEV; + if (id->driver_data && ATA_GEN_INTEL_IDER) + if (!is_intel_ider(dev)) + return -ENODEV; + /* Devices that need care */ if (dev->vendor == PCI_VENDOR_ID_UMC && dev->device == PCI_DEVICE_ID_UMC_UM8886A && @@ -186,7 +229,11 @@ static struct pci_device_id ata_generic[] = { { PCI_DEVICE(PCI_VENDOR_ID_TOSHIBA,PCI_DEVICE_ID_TOSHIBA_PICCOLO_2), }, { PCI_DEVICE(PCI_VENDOR_ID_TOSHIBA,PCI_DEVICE_ID_TOSHIBA_PICCOLO_3), }, { PCI_DEVICE(PCI_VENDOR_ID_TOSHIBA,PCI_DEVICE_ID_TOSHIBA_PICCOLO_5), }, -#endif +#endif + /* Intel, IDE class device */ + { PCI_VENDOR_ID_INTEL, PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID, + PCI_CLASS_STORAGE_IDE << 8, 0xFFFFFF00UL, + .driver_data = ATA_GEN_INTEL_IDER }, /* Must come last. If you add entries adjust this table appropriately */ { PCI_DEVICE_CLASS(PCI_CLASS_STORAGE_IDE << 8, 0xFFFFFF00UL), .driver_data = ATA_GEN_CLASS_MATCH },
Intel IDE-R devices are part of the Intel AMT management setup. They don't have any special configuration registers or settings so the ata_generic driver will support them fully. Rather than add a huge table of IDs for each chipset and keep sending in new ones this patch autodetects them. (And yes Jeff I'll resurrect the delay patches in a couple of weeks) Signed-off-by: Alan Cox <alan@linux.intel.com> --- drivers/ata/ata_generic.c | 49 ++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 48 insertions(+), 1 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html