On Tue, 16 Oct 2007, Matthew Dharm wrote:

quoted text
> On Tue, Oct 16, 2007 at 02:04:43PM -0400, Alan Stern wrote:
> > On Tue, 16 Oct 2007, Matthew Dharm wrote:
> > 
> > > I haven't looked at this code at all, but neither approach feels right to
> > > me.
> > > 
> > > How does this work at all?  Even if you load a driver later, wouldn't it
> > > call usb_set_interface(), which would call usb_create_sysfs_intf_files()
> > > and hit the same issue?
> > 
> > usb_set_interface() is smart enough to remove the old interface files
> > before creating new ones, since it expects them to exist already.  
> > Hence there's no problem in that scenario.
> > 
> > But usb_set_configuration doesn't expect there to be any pre-existing
> > interface files, because there isn't even an interface until the
> > registration is performed.
> 
> And I'm guessing that you can't call usb_create_sysfs_intf_files() until
> registration is performed, right?

Right.

quoted text
> > The most important reason has to do with the endpoint pseudo-devices.  
> > Different altsettings can have different endpoints, so those have to be
> > removed and re-created whenever the altsetting changes.
> 
> Right, altsettings.  I forgot about those.  I only ever think in terms of
> multiple configurations.
> 
> *grumble*
> 
> If usb_set_interface() has to be smart enough to remove existing files
> first already, then I guess it's reasonably symmetric to have
> usb_set_configuration() have the same smarts.  Maybe they can share some
> common code, even.

It's not a big deal to remove the files first.  In fact, here's a patch 
to do it.  Dave, see if this doesn't fix your problem.  I don't like it 
much because it does an unnecessary remove/create cycle, but that's 
better than doing something wrong.

It's slightly odd that the sysfs core logs an error when you try to 
create the same file twice but it doesn't when you try to remove a 
non-existent file (or try to remove an existing file twice).  Oh 
well...

Alan Stern



Index: usb-2.6/drivers/usb/core/message.c
===================================================================
--- usb-2.6.orig/drivers/usb/core/message.c
+++ usb-2.6/drivers/usb/core/message.c
@@ -1643,7 +1643,13 @@ free_interfaces:
 				intf->dev.bus_id, ret);
 			continue;
 		}
-		usb_create_sysfs_intf_files (intf);
+
+		/* The driver's probe method can call usb_set_interface(),
+		 * which would mean the interface's sysfs files are already
+		 * created.  Just in case, we'll remove them first.
+		 */
+		usb_remove_sysfs_intf_files(intf);
+		usb_create_sysfs_intf_files(intf);
 	}
 
 	usb_autosuspend_device(dev);

-
unsubscribe notice
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/