[RFC][PATCH] add driver matching priorities

[RFC][PATCH] add driver matching priorities

[Adam Belay]

Hi,

This patch adds initial support for driver matching priorities to the
driver model.  It is needed for my work on converting the pci bridge
driver to use "struct device_driver".  It may also be helpful for driver
with more complex (or long id lists as I've seen in many cases) matching
criteria. 

"match" has been added to "struct device_driver".  There are now two
steps in the matching process.  The first step is a bus specific filter
that determines possible driver candidates.  The second step is a driver
specific match function that verifies if the driver will work with the
hardware, and returns a priority code (how well it is able to handle the
device).  The bus layer could override the driver's match function if
necessary (similar to how it passes *probe through it's layer and then
on to the actual driver).

The current priorities are as follows:

enum {
	MATCH_PRIORITY_FAILURE = 0,
	MATCH_PRIORITY_GENERIC,
	MATCH_PRIORITY_NORMAL,
	MATCH_PRIORITY_VENDOR,
};

let me know if any of this would need to be changed.  For example, the
"struct bus_type" match function could return a priority code.

Of course this patch is not going to be effective alone.  We also need
to change the init order.  If a driver is registered early but isn't the
best available, it will be bound to the device prematurely.  This would
be a problem for carbus (yenta) bridges.

I think we may have to load all in kernel drivers first, and then begin
matching them to hardware.  Do you agree?  If so, I'd be happy to make a
patch for that too.

Thanks,
Adam


--- a/drivers/base/bus.c	2005-01-20 17:37:46.000000000 -0500
+++ b/drivers/base/bus.c	2005-01-28 16:59:00.000000000 -0500
@@ -286,6 +286,9 @@
 	if (drv->bus->match && !drv->bus->match(dev, drv))
 		return -ENODEV;
 
+	if (drv->match && !drv->match(dev))
+		return -ENODEV;
+
 	dev->driver = drv;
 	if (drv->probe) {
 		int error = drv->probe(dev);
@@ -299,6 +302,42 @@
 	return 0;
 }
 
+/**
+ *	driver_probe_device_priority - attempt to bind device & driver with a
+ *				       given match level priority 
+ *	@drv:		driver.
+ *	@dev:		device.
+ *	@priority	the match level priority
+ */
+
+static int driver_probe_device_priority(struct device_driver * drv,
+					struct device * dev, int priority)
+{
+	int matchp;
+
+	if (drv->bus->match && !drv->bus->match(dev, drv))
+		return -ENODEV;
+
+	if (drv->match) {
+		matchp = drv->match(dev);
+	} else
+		matchp = MATCH_PRIORITY_NORMAL;
+
+	if (matchp != priority)
+		return -ENODEV;
+
+	dev->driver = drv;
+	if (drv->probe) {
+		int error = drv->probe(dev);
+		if (error) {
+			dev->driver = NULL;
+			return error;
+		}
+	}
+
+	device_bind_driver(dev);
+	return 0;
+}
 
 /**
  *	device_attach - try to attach device to a driver.
@@ -312,17 +351,20 @@
 {
  	struct bus_type * bus = dev->bus;
 	struct list_head * entry;
-	int error;
+	int error, matchp = MATCH_PRIORITY_VENDOR;
 
 	if (dev->driver) {
 		device_bind_driver(dev);
 		return 1;
 	}
 
-	if (bus->match) {
+	if (!bus->match)
+		return 0;
+		
+	while (matchp > 0) {
 		list_for_each(entry, &bus->drivers.list) {
 			struct device_driver * drv = to_drv(entry);
-			error = driver_probe_device(drv, dev);
+			error = driver_probe_device_priority(drv, dev, matchp);
 			if (!error)
 				/* success, driver matched */
 				return 1;
@@ -332,6 +374,7 @@
 				    "%s: probe of %s failed with error %d\n",
 				    drv->name, dev->bus_id, error);
 		}
+		matchp--;
 	}
 
 	return 0;
--- a/include/linux/device.h	2005-01-20 17:37:26.000000000 -0500
+++ b/include/linux/device.h	2005-01-28 16:40:22.000000000 -0500
@@ -41,6 +41,13 @@
 	RESUME_ENABLE,
 };
 
+enum {
+	MATCH_PRIORITY_FAILURE = 0,
+	MATCH_PRIORITY_GENERIC,
+	MATCH_PRIORITY_NORMAL,
+	MATCH_PRIORITY_VENDOR,
+};
+
 struct device;
 struct device_driver;
 struct class;
@@ -108,6 +115,7 @@
 
 	struct module 		* owner;
 
+	int	(*match)	(struct device * dev);
 	int	(*probe)	(struct device * dev);
 	int 	(*remove)	(struct device * dev);
 	void	(*shutdown)	(struct device * dev);

Re: [RFC][PATCH] add driver matching priorities

[Dmitry Torokhov]

On Friday 28 January 2005 17:30, Adam Belay wrote:
> Of course this patch is not going to be effective alone.  We also need
> to change the init order.  If a driver is registered early but isn't the
> best available, it will be bound to the device prematurely.  This would
> be a problem for carbus (yenta) bridges.
> 
> I think we may have to load all in kernel drivers first, and then begin
> matching them to hardware.  Do you agree?  If so, I'd be happy to make a
> patch for that too.
> 

I disagree. The driver core should automatically unbind generic driver
from a device when native driver gets loaded. I think the only change is
that we can no longer skip devices that are bound to a driver and match
them all over again when a new driver is loaded.  

-- 
Dmitry

Re: [RFC][PATCH] add driver matching priorities

[Adam Belay]

On Fri, 2005-01-28 at 18:23 -0500, Dmitry Torokhov wrote:
> On Friday 28 January 2005 17:30, Adam Belay wrote:
> > Of course this patch is not going to be effective alone.  We also need
> > to change the init order.  If a driver is registered early but isn't the
> > best available, it will be bound to the device prematurely.  This would
> > be a problem for carbus (yenta) bridges.
> > 
> > I think we may have to load all in kernel drivers first, and then begin
> > matching them to hardware.  Do you agree?  If so, I'd be happy to make a
> > patch for that too.
> > 
> 
> I disagree. The driver core should automatically unbind generic driver
> from a device when native driver gets loaded. I think the only change is
> that we can no longer skip devices that are bound to a driver and match
> them all over again when a new driver is loaded.  
> 

That's another option.  My concern is that if a generic driver pokes
around with hardware, it may fail to initialize properly when the actual
driver is loaded.  There are other problems too.  If the system were to
be suspended while the generic driver was loaded, the restore_state code
may be incorrect, also rendering the device unusable.

I'd like to leave the option of unloading generic driver open.  I just
think we need to be aware of potential problems it might cause, before
deciding to go that direction.

Thanks,
Adam

Re: [RFC][PATCH] add driver matching priorities

[Dmitry Torokhov]

On Friday 28 January 2005 18:33, Adam Belay wrote:
> On Fri, 2005-01-28 at 18:23 -0500, Dmitry Torokhov wrote:
> > On Friday 28 January 2005 17:30, Adam Belay wrote:
> > > Of course this patch is not going to be effective alone.  We also need
> > > to change the init order.  If a driver is registered early but isn't the
> > > best available, it will be bound to the device prematurely.  This would
> > > be a problem for carbus (yenta) bridges.
> > > 
> > > I think we may have to load all in kernel drivers first, and then begin
> > > matching them to hardware.  Do you agree?  If so, I'd be happy to make a
> > > patch for that too.
> > > 
> > 
> > I disagree. The driver core should automatically unbind generic driver
> > from a device when native driver gets loaded. I think the only change is
> > that we can no longer skip devices that are bound to a driver and match
> > them all over again when a new driver is loaded.  
> > 
> 
> That's another option.  My concern is that if a generic driver pokes
> around with hardware, it may fail to initialize properly when the actual
> driver is loaded.  There are other problems too.  If the system were to
> be suspended while the generic driver was loaded, the restore_state code
> may be incorrect, also rendering the device unusable.
> 

If generic driver binds to a device that is has no idea how to drive
_at all_ then I will argue that the generic driver is broken. It should
not bind to begin with.

-- 
Dmitry

Re: [RFC][PATCH] add driver matching priorities

[Adam Belay]

On Fri, 2005-01-28 at 18:51 -0500, Dmitry Torokhov wrote:
> If generic driver binds to a device that is has no idea how to drive
> _at all_ then I will argue that the generic driver is broken. It should
> not bind to begin with.
> 

In the case of pci bridges, sometimes we can't really tell if we can
drive the hardware entirely.  It's a classcode match.  Generic drivers
may support a portion of hardware in a limited fashion.  It's not that
they have no idea what they're doing with the hardware.  It's more a
matter of not always doing the best or most complete thing.  For some
hardware this may work fine.  Because we don't support generic drivers
in the current driver model, we haven't had a chance to see how well
they would work, or where they could be used.

Also, consider this.  If the pci bridge driver binds to yenta, it will
(in theory, it also might explode) enumerate all of the cardbus devices.
If then later, it is discovered that there is a better driver for the
bridge, all of the bridge's children will have to be torn down.  Thier
drivers will be released, and  the devices removed.  This might increase
the odds of something going wrong.

Thanks,
Adam

Re: [RFC][PATCH] add driver matching priorities

[Al Viro]

On Fri, Jan 28, 2005 at 06:23:26PM -0500, Dmitry Torokhov wrote:
> On Friday 28 January 2005 17:30, Adam Belay wrote:
> > Of course this patch is not going to be effective alone.  We also need
> > to change the init order.  If a driver is registered early but isn't the
> > best available, it will be bound to the device prematurely.  This would
> > be a problem for carbus (yenta) bridges.
> > 
> > I think we may have to load all in kernel drivers first, and then begin
> > matching them to hardware.  Do you agree?  If so, I'd be happy to make a
> > patch for that too.
> > 
> 
> I disagree. The driver core should automatically unbind generic driver
> from a device when native driver gets loaded. I think the only change is
> that we can no longer skip devices that are bound to a driver and match
> them all over again when a new driver is loaded.  

And what happens if we've already got the object busy?

Re: [RFC][PATCH] add driver matching priorities

[Dmitry Torokhov]

On Friday 28 January 2005 19:11, Al Viro wrote:
> On Fri, Jan 28, 2005 at 06:23:26PM -0500, Dmitry Torokhov wrote:
> > On Friday 28 JanuarDy 2005 17:30, Adam Belay wrote:
> > > Of course this patch is not going to be effective alone.  We also need
> > > to change the init order.  If a driver is registered early but isn't the
> > > best available, it will be bound to the device prematurely.  This would
> > > be a problem for carbus (yenta) bridges.
> > > 
> > > I think we may have to load all in kernel drivers first, and then begin
> > > matching them to hardware.  Do you agree?  If so, I'd be happy to make a
> > > patch for that too.
> > > 
> > 
> > I disagree. The driver core should automatically unbind generic driver
> > from a device when native driver gets loaded. I think the only change is
> > that we can no longer skip devices that are bound to a driver and match
> > them all over again when a new driver is loaded.  
> 
> And what happens if we've already got the object busy?
> 

Mark it as dead and release structures when holder lets it go. With hotplug
pretty much everywhere more and more systems can handle it. Plus one could
argue that if an object needs a special driver to function properly it will
unlikely be busy before native driver is loaded.

Also, one still can do what Adam offers by pre-loading native drivers in
cases whent is required but still support more flexible default scheme.

-- 
Dmitry

Re: [RFC][PATCH] add driver matching priorities

[Greg KH]

On Fri, Jan 28, 2005 at 05:30:04PM -0500, Adam Belay wrote:
> Hi,
> 
> This patch adds initial support for driver matching priorities to the
> driver model.  It is needed for my work on converting the pci bridge
> driver to use "struct device_driver".  It may also be helpful for driver
> with more complex (or long id lists as I've seen in many cases) matching
> criteria. 
> 
> "match" has been added to "struct device_driver".  There are now two
> steps in the matching process.  The first step is a bus specific filter
> that determines possible driver candidates.  The second step is a driver
> specific match function that verifies if the driver will work with the
> hardware, and returns a priority code (how well it is able to handle the
> device).  The bus layer could override the driver's match function if
> necessary (similar to how it passes *probe through it's layer and then
> on to the actual driver).
> 
> The current priorities are as follows:
> 
> enum {
> 	MATCH_PRIORITY_FAILURE = 0,
> 	MATCH_PRIORITY_GENERIC,
> 	MATCH_PRIORITY_NORMAL,
> 	MATCH_PRIORITY_VENDOR,
> };
> 
> let me know if any of this would need to be changed.  For example, the
> "struct bus_type" match function could return a priority code.
> 
> Of course this patch is not going to be effective alone.  We also need
> to change the init order.  If a driver is registered early but isn't the
> best available, it will be bound to the device prematurely.  This would
> be a problem for carbus (yenta) bridges.
> 
> I think we may have to load all in kernel drivers first, and then begin
> matching them to hardware.  Do you agree?  If so, I'd be happy to make a
> patch for that too.

I think the issue that Al raises about drivers grabbing devices, and
then trying to unbind them might be a real problem.

Also, why can't this just be done in the bus specific code, in the match
function?  I don't see how putting this into the driver core helps out
any.

thanks,

greg k-h

Re: [RFC][PATCH] add driver matching priorities

[Adam Belay]

On Thu, 2005-02-10 at 00:41 -0800, Greg KH wrote:
> On Fri, Jan 28, 2005 at 05:30:04PM -0500, Adam Belay wrote:
> > Hi,
> > 
> > This patch adds initial support for driver matching priorities to the
> > driver model.  It is needed for my work on converting the pci bridge
> > driver to use "struct device_driver".  It may also be helpful for driver
> > with more complex (or long id lists as I've seen in many cases) matching
> > criteria. 
> > 
> > "match" has been added to "struct device_driver".  There are now two
> > steps in the matching process.  The first step is a bus specific filter
> > that determines possible driver candidates.  The second step is a driver
> > specific match function that verifies if the driver will work with the
> > hardware, and returns a priority code (how well it is able to handle the
> > device).  The bus layer could override the driver's match function if
> > necessary (similar to how it passes *probe through it's layer and then
> > on to the actual driver).
> > 
> > The current priorities are as follows:
> > 
> > enum {
> > 	MATCH_PRIORITY_FAILURE = 0,
> > 	MATCH_PRIORITY_GENERIC,
> > 	MATCH_PRIORITY_NORMAL,
> > 	MATCH_PRIORITY_VENDOR,
> > };
> > 
> > let me know if any of this would need to be changed.  For example, the
> > "struct bus_type" match function could return a priority code.
> > 
> > Of course this patch is not going to be effective alone.  We also need
> > to change the init order.  If a driver is registered early but isn't the
> > best available, it will be bound to the device prematurely.  This would
> > be a problem for carbus (yenta) bridges.
> > 
> > I think we may have to load all in kernel drivers first, and then begin
> > matching them to hardware.  Do you agree?  If so, I'd be happy to make a
> > patch for that too.
> 
> I think the issue that Al raises about drivers grabbing devices, and
> then trying to unbind them might be a real problem.

I agree.  Do you think registering every in-kernel driver before probing
hardware would solve this problem?

> 
> Also, why can't this just be done in the bus specific code, in the match
> function?  I don't see how putting this into the driver core helps out
> any.

The match priority is a chararistic of the driver and how it's
implemented rather than the bus's matching mechanism.  The type of match
doesn't necessarily reflect the driver's ability to control the hardware
(ex. a driver could match on a specific PCI id but only provide generic
support for the device).

Also, I think this is a feature that would be useful for all of the
buses.  Therefore, it would seem implementing it in the driver core
might result in the least code duplication.

The second "*match" function in "struct device_driver" gives the driver
a chance to evaluate it's ability of controlling the device and solves a
few problems with the current implementation.  (ex. it's not possible to
detect ISA Modems with only a list of PnP IDs, and some PCI devices
support a pool of IDs that is too large to put in an ID table).

Thanks,
Adam

Re: [RFC][PATCH] add driver matching priorities

[Dmitry Torokhov]

On Thu, 10 Feb 2005 12:18:37 -0500, Adam Belay <abelay@novell.com> wrote:
> On Thu, 2005-02-10 at 00:41 -0800, Greg KH wrote:
> > On Fri, Jan 28, 2005 at 05:30:04PM -0500, Adam Belay wrote:
> > > Hi,
> > >
> > > This patch adds initial support for driver matching priorities to the
> > > driver model.  It is needed for my work on converting the pci bridge
> > > driver to use "struct device_driver".  It may also be helpful for driver
> > > with more complex (or long id lists as I've seen in many cases) matching
> > > criteria.
> > >
> > > "match" has been added to "struct device_driver".  There are now two
> > > steps in the matching process.  The first step is a bus specific filter
> > > that determines possible driver candidates.  The second step is a driver
> > > specific match function that verifies if the driver will work with the
> > > hardware, and returns a priority code (how well it is able to handle the
> > > device).  The bus layer could override the driver's match function if
> > > necessary (similar to how it passes *probe through it's layer and then
> > > on to the actual driver).
> > >
> > > The current priorities are as follows:
> > >
> > > enum {
> > >     MATCH_PRIORITY_FAILURE = 0,
> > >     MATCH_PRIORITY_GENERIC,
> > >     MATCH_PRIORITY_NORMAL,
> > >     MATCH_PRIORITY_VENDOR,
> > > };
> > >
> > > let me know if any of this would need to be changed.  For example, the
> > > "struct bus_type" match function could return a priority code.
> > >
> > > Of course this patch is not going to be effective alone.  We also need
> > > to change the init order.  If a driver is registered early but isn't the
> > > best available, it will be bound to the device prematurely.  This would
> > > be a problem for carbus (yenta) bridges.
> > >
> > > I think we may have to load all in kernel drivers first, and then begin
> > > matching them to hardware.  Do you agree?  If so, I'd be happy to make a
> > > patch for that too.
> >
> > I think the issue that Al raises about drivers grabbing devices, and
> > then trying to unbind them might be a real problem.
> 
> I agree.  Do you think registering every in-kernel driver before probing
> hardware would solve this problem?

And what do you do with drivers that are built as modules?

-- 
Dmitry

Re: [RFC][PATCH] add driver matching priorities

[Greg KH]

On Thu, Feb 10, 2005 at 12:18:37PM -0500, Adam Belay wrote:
> On Thu, 2005-02-10 at 00:41 -0800, Greg KH wrote:
> > On Fri, Jan 28, 2005 at 05:30:04PM -0500, Adam Belay wrote:
> > > Hi,
> > > 
> > > This patch adds initial support for driver matching priorities to the
> > > driver model.  It is needed for my work on converting the pci bridge
> > > driver to use "struct device_driver".  It may also be helpful for driver
> > > with more complex (or long id lists as I've seen in many cases) matching
> > > criteria. 
> > > 
> > > "match" has been added to "struct device_driver".  There are now two
> > > steps in the matching process.  The first step is a bus specific filter
> > > that determines possible driver candidates.  The second step is a driver
> > > specific match function that verifies if the driver will work with the
> > > hardware, and returns a priority code (how well it is able to handle the
> > > device).  The bus layer could override the driver's match function if
> > > necessary (similar to how it passes *probe through it's layer and then
> > > on to the actual driver).
> > > 
> > > The current priorities are as follows:
> > > 
> > > enum {
> > > 	MATCH_PRIORITY_FAILURE = 0,
> > > 	MATCH_PRIORITY_GENERIC,
> > > 	MATCH_PRIORITY_NORMAL,
> > > 	MATCH_PRIORITY_VENDOR,
> > > };
> > > 
> > > let me know if any of this would need to be changed.  For example, the
> > > "struct bus_type" match function could return a priority code.
> > > 
> > > Of course this patch is not going to be effective alone.  We also need
> > > to change the init order.  If a driver is registered early but isn't the
> > > best available, it will be bound to the device prematurely.  This would
> > > be a problem for carbus (yenta) bridges.
> > > 
> > > I think we may have to load all in kernel drivers first, and then begin
> > > matching them to hardware.  Do you agree?  If so, I'd be happy to make a
> > > patch for that too.
> > 
> > I think the issue that Al raises about drivers grabbing devices, and
> > then trying to unbind them might be a real problem.
> 
> I agree.  Do you think registering every in-kernel driver before probing
> hardware would solve this problem?

This doesn't work for any vendor kernel as they build all drivers as
modules :(

thanks,

greg k-h

Re: [RFC][PATCH] add driver matching priorities

[Adam Belay]

On Thu, 2005-02-10 at 10:12 -0800, Greg KH wrote:
> On Thu, Feb 10, 2005 at 12:18:37PM -0500, Adam Belay wrote:
> > On Thu, 2005-02-10 at 00:41 -0800, Greg KH wrote:
> > > On Fri, Jan 28, 2005 at 05:30:04PM -0500, Adam Belay wrote:
> > > > Hi,
> > > > 
> > > > This patch adds initial support for driver matching priorities to the
> > > > driver model.  It is needed for my work on converting the pci bridge
> > > > driver to use "struct device_driver".  It may also be helpful for driver
> > > > with more complex (or long id lists as I've seen in many cases) matching
> > > > criteria. 
> > > > 
> > > > "match" has been added to "struct device_driver".  There are now two
> > > > steps in the matching process.  The first step is a bus specific filter
> > > > that determines possible driver candidates.  The second step is a driver
> > > > specific match function that verifies if the driver will work with the
> > > > hardware, and returns a priority code (how well it is able to handle the
> > > > device).  The bus layer could override the driver's match function if
> > > > necessary (similar to how it passes *probe through it's layer and then
> > > > on to the actual driver).
> > > > 
> > > > The current priorities are as follows:
> > > > 
> > > > enum {
> > > > 	MATCH_PRIORITY_FAILURE = 0,
> > > > 	MATCH_PRIORITY_GENERIC,
> > > > 	MATCH_PRIORITY_NORMAL,
> > > > 	MATCH_PRIORITY_VENDOR,
> > > > };
> > > > 
> > > > let me know if any of this would need to be changed.  For example, the
> > > > "struct bus_type" match function could return a priority code.
> > > > 
> > > > Of course this patch is not going to be effective alone.  We also need
> > > > to change the init order.  If a driver is registered early but isn't the
> > > > best available, it will be bound to the device prematurely.  This would
> > > > be a problem for carbus (yenta) bridges.
> > > > 
> > > > I think we may have to load all in kernel drivers first, and then begin
> > > > matching them to hardware.  Do you agree?  If so, I'd be happy to make a
> > > > patch for that too.
> > > 
> > > I think the issue that Al raises about drivers grabbing devices, and
> > > then trying to unbind them might be a real problem.
> > 
> > I agree.  Do you think registering every in-kernel driver before probing
> > hardware would solve this problem?
> 
> This doesn't work for any vendor kernel as they build all drivers as
> modules :(

True, but if we allowed userspace to control specifically which drivers
are bound to which devices, then we will be able to ensure the best
driver is bound.  The only problem is that modules then shouldn't
automatically bind to devices.

For non-modules, as many core drivers currently are (ex. pci bus) we can
often do the right thing using the above suggestion.

I think we can't avoid this inherent flaw in modules if we don't have a
user-space mechanism to control driver binding.  Our current approach
doesn't work because it's indirect.  Adding a device ID doesn't
specifically ensure the device in question will be bound to the correct
driver.

Thanks,
Adam

Re: [RFC][PATCH] add driver matching priorities

[Greg KH]

On Thu, Feb 10, 2005 at 12:18:37PM -0500, Adam Belay wrote:
> 
> The second "*match" function in "struct device_driver" gives the driver
> a chance to evaluate it's ability of controlling the device and solves a
> few problems with the current implementation.  (ex. it's not possible to
> detect ISA Modems with only a list of PnP IDs, and some PCI devices
> support a pool of IDs that is too large to put in an ID table).

What deficiancy in the current id tables do you see?  What driver has a
id table that is "too big"?  Is there some way we can change it to make
it work better?

thanks,

greg k-h

Re: [RFC][PATCH] add driver matching priorities

[Dmitry Torokhov]

On Thu, 10 Feb 2005 10:33:38 -0800, Greg KH <greg@kroah.com> wrote:
> On Thu, Feb 10, 2005 at 12:18:37PM -0500, Adam Belay wrote:
> >
> > The second "*match" function in "struct device_driver" gives the driver
> > a chance to evaluate it's ability of controlling the device and solves a
> > few problems with the current implementation.  (ex. it's not possible to
> > detect ISA Modems with only a list of PnP IDs, and some PCI devices
> > support a pool of IDs that is too large to put in an ID table).
> 
> What deficiancy in the current id tables do you see?  What driver has a
> id table that is "too big"?  Is there some way we can change it to make
> it work better?
> 

Stepping a bit farther away - sometimes generinc matching is not
enough to determine if driver suits for a device - actual probing is
needed (consider atkbd and psmouse - they can both attach to the same
port but we can't determine if it is a keyboard or mouse until we
started probing)
-- 
Dmitry

Re: [RFC][PATCH] add driver matching priorities

[Adam Belay]

On Thu, 2005-02-10 at 13:46 -0500, Dmitry Torokhov wrote:
> On Thu, 10 Feb 2005 10:33:38 -0800, Greg KH <greg@kroah.com> wrote:
> > On Thu, Feb 10, 2005 at 12:18:37PM -0500, Adam Belay wrote:
> > >
> > > The second "*match" function in "struct device_driver" gives the driver
> > > a chance to evaluate it's ability of controlling the device and solves a
> > > few problems with the current implementation.  (ex. it's not possible to
> > > detect ISA Modems with only a list of PnP IDs, and some PCI devices
> > > support a pool of IDs that is too large to put in an ID table).
> > 
> > What deficiancy in the current id tables do you see?  What driver has a
> > id table that is "too big"?  Is there some way we can change it to make
> > it work better?
> > 
> 
> Stepping a bit farther away - sometimes generinc matching is not
> enough to determine if driver suits for a device - actual probing is
> needed (consider atkbd and psmouse - they can both attach to the same
> port but we can't determine if it is a keyboard or mouse until we
> started probing)

Also, another example I remember seeing a while back...  Some pcmcia
devices have identical IDs but different chipsets.  The only way to find
the correct driver is to poke around.

Thanks,
Adam

Re: [RFC][PATCH] add driver matching priorities

[Russell King]

On Thu, Feb 10, 2005 at 12:18:37PM -0500, Adam Belay wrote:
> > I think the issue that Al raises about drivers grabbing devices, and
> > then trying to unbind them might be a real problem.
> 
> I agree.  Do you think registering every in-kernel driver before probing
> hardware would solve this problem?

In which case, consider whether we should be tainting the kernel if
someone loads a device driver, it binds to a device, and then they
unload that driver.

It's precisely the same situation, and precisely the same mechanics
as what I've suggested should be going on here.  If one scenario is
inherently buggy, so is the other.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: [RFC][PATCH] add driver matching priorities

[Adam Belay]

On Thu, 2005-02-10 at 18:45 +0000, Russell King wrote:
> On Thu, Feb 10, 2005 at 12:18:37PM -0500, Adam Belay wrote:
> > > I think the issue that Al raises about drivers grabbing devices, and
> > > then trying to unbind them might be a real problem.
> > 
> > I agree.  Do you think registering every in-kernel driver before probing
> > hardware would solve this problem?
> 
> In which case, consider whether we should be tainting the kernel if
> someone loads a device driver, it binds to a device, and then they
> unload that driver.
> 
> It's precisely the same situation, and precisely the same mechanics
> as what I've suggested should be going on here.  If one scenario is
> inherently buggy, so is the other.
> 

I think it would depend on whether the user makes the device busy before
the driver is unloaded.  Different device classes may have different
requirements for when and how a device can be removed.  Are there other
issues as well?  Maybe there are ways to improve driver start and stop
mechanics.

Thanks,
Adam

Re: [RFC][PATCH] add driver matching priorities

[Greg KH]

On Thu, Feb 10, 2005 at 04:37:03PM -0500, Adam Belay wrote:
> On Thu, 2005-02-10 at 18:45 +0000, Russell King wrote:
> > On Thu, Feb 10, 2005 at 12:18:37PM -0500, Adam Belay wrote:
> > > > I think the issue that Al raises about drivers grabbing devices, and
> > > > then trying to unbind them might be a real problem.
> > > 
> > > I agree.  Do you think registering every in-kernel driver before probing
> > > hardware would solve this problem?
> > 
> > In which case, consider whether we should be tainting the kernel if
> > someone loads a device driver, it binds to a device, and then they
> > unload that driver.
> > 
> > It's precisely the same situation, and precisely the same mechanics
> > as what I've suggested should be going on here.  If one scenario is
> > inherently buggy, so is the other.
> > 
> 
> I think it would depend on whether the user makes the device busy before
> the driver is unloaded.  Different device classes may have different
> requirements for when and how a device can be removed.  Are there other
> issues as well?  Maybe there are ways to improve driver start and stop
> mechanics.

We never fail a device unbind from a driver, so this isn't as big a deal
as I originally thought.  Yes, userspace can get messy, but as userspace
was the one that loaded the new driver to bind, it's acceptable.

So, care to resubmit your patch?

thanks,

greg k-h

Re: [RFC][PATCH] add driver matching priorities

[Adam Belay]

On Fri, 2005-02-25 at 15:41 -0800, Greg KH wrote:
> On Thu, Feb 10, 2005 at 04:37:03PM -0500, Adam Belay wrote:
> > On Thu, 2005-02-10 at 18:45 +0000, Russell King wrote:
> > > On Thu, Feb 10, 2005 at 12:18:37PM -0500, Adam Belay wrote:
> > > > > I think the issue that Al raises about drivers grabbing devices, and
> > > > > then trying to unbind them might be a real problem.
> > > > 
> > > > I agree.  Do you think registering every in-kernel driver before probing
> > > > hardware would solve this problem?
> > > 
> > > In which case, consider whether we should be tainting the kernel if
> > > someone loads a device driver, it binds to a device, and then they
> > > unload that driver.
> > > 
> > > It's precisely the same situation, and precisely the same mechanics
> > > as what I've suggested should be going on here.  If one scenario is
> > > inherently buggy, so is the other.
> > > 
> > 
> > I think it would depend on whether the user makes the device busy before
> > the driver is unloaded.  Different device classes may have different
> > requirements for when and how a device can be removed.  Are there other
> > issues as well?  Maybe there are ways to improve driver start and stop
> > mechanics.
> 
> We never fail a device unbind from a driver, so this isn't as big a deal
> as I originally thought.  Yes, userspace can get messy, but as userspace
> was the one that loaded the new driver to bind, it's acceptable.
> 
> So, care to resubmit your patch?
> 

Would you like me to include the portion that adds "*match" to "struct
device_driver"?  After some more thought, I began considering having
driver priority be a static quality of a device driver.  The question is
whether we want a device driver to be able to return a variable priority
based on bind device.  Also, "*match" could be used to split some
detection and validation out of "*probe".  What are your reactions to
this?

Finally, should every in-kernel driver be registered before devices are
detected?

Thanks,
Adam

Re: [RFC][PATCH] add driver matching priorities

[Greg KH]

On Mon, Feb 28, 2005 at 07:05:54PM -0500, Adam Belay wrote:
> On Fri, 2005-02-25 at 15:41 -0800, Greg KH wrote:
> > On Thu, Feb 10, 2005 at 04:37:03PM -0500, Adam Belay wrote:
> > > On Thu, 2005-02-10 at 18:45 +0000, Russell King wrote:
> > > > On Thu, Feb 10, 2005 at 12:18:37PM -0500, Adam Belay wrote:
> > > > > > I think the issue that Al raises about drivers grabbing devices, and
> > > > > > then trying to unbind them might be a real problem.
> > > > > 
> > > > > I agree.  Do you think registering every in-kernel driver before probing
> > > > > hardware would solve this problem?
> > > > 
> > > > In which case, consider whether we should be tainting the kernel if
> > > > someone loads a device driver, it binds to a device, and then they
> > > > unload that driver.
> > > > 
> > > > It's precisely the same situation, and precisely the same mechanics
> > > > as what I've suggested should be going on here.  If one scenario is
> > > > inherently buggy, so is the other.
> > > > 
> > > 
> > > I think it would depend on whether the user makes the device busy before
> > > the driver is unloaded.  Different device classes may have different
> > > requirements for when and how a device can be removed.  Are there other
> > > issues as well?  Maybe there are ways to improve driver start and stop
> > > mechanics.
> > 
> > We never fail a device unbind from a driver, so this isn't as big a deal
> > as I originally thought.  Yes, userspace can get messy, but as userspace
> > was the one that loaded the new driver to bind, it's acceptable.
> > 
> > So, care to resubmit your patch?
> > 
> 
> Would you like me to include the portion that adds "*match" to "struct
> device_driver"?  After some more thought, I began considering having
> driver priority be a static quality of a device driver.  The question is
> whether we want a device driver to be able to return a variable priority
> based on bind device.  Also, "*match" could be used to split some
> detection and validation out of "*probe".  What are your reactions to
> this?

Busses really need to be the ones doing this kind of priority logic,
right?  So shouldn't that code be in the individul bus specific driver
structures, not in "struct driver" itself?

> Finally, should every in-kernel driver be registered before devices are
> detected?

That's up to the bus.  Let's leave it as-is for now, and see what falls
apart :)

thanks,

greg k-h