Re: [ANNOUNCE] hotplug-ng 002 release

[Per Svennerbrandt <per.svennerbrandt@lbi.se>]

* Greg KH (greg@kroah.com) wrote:
> On Sat, May 14, 2005 at 01:21:33AM +0200, Per Svennerbrandt wrote:
> > * On Fri, 13 May 2005, Greg KH (greg@kroah.com) wrote:
> > > Ok, as you never posted your patch, I had to do it myself :)
> > 
> > Oh, crap! Seems like I'm forever doomed to be sitting on my patches for
> > six months thinking they arn't good enough, only to then repeatedly 
> > getting beaten by couple of hours when finally deciding on submitting
> > them then... ;) ;)
> > 
> > I guess I'l just have to dedicate more time if I'm ever going to get any
> > code into the kernel.
> 
> Or just send those patches earlier :)

Well, it's not sending the patches that's the hard part. :) It's getting to
the point where you're feeling a 100% confident that the code is a 100% 
correct and without side effects that is. And since I'm so inexperienced in
kernel programming getting to that point simply involves reading just
lots and lots of code, and hence ends up taking a lot of time...
But enough about that. :)

Personal TODO list:
# prt-get install aspell
$ code++
$ talk--

> > > +static ssize_t show_modalias(struct device *dev, char *buf)
> > > +{
> > > +	struct usb_interface *intf;
> > > +	struct usb_device *udev;
> > > +
> > > +	intf = to_usb_interface(dev);
> > > +	udev = interface_to_usbdev(intf);
> > > +	if (udev->descriptor.bDeviceClass == 0) {
> > > +		struct usb_host_interface *alt = intf->cur_altsetting;
> > > +
> > > +		return sprintf(buf, "usb:v%04Xp%04Xd%04Xdc%02Xdsc%02Xdp%02Xic%02Xisc%02Xip%02X\n",
> > > +			       le16_to_cpu(udev->descriptor.idVendor),
> > > +			       le16_to_cpu(udev->descriptor.idProduct),
> > > +			       le16_to_cpu(udev->descriptor.bcdDevice),
> > > +			       udev->descriptor.bDeviceClass,
> > > +			       udev->descriptor.bDeviceSubClass,
> > > +			       udev->descriptor.bDeviceProtocol,
> > > +			       alt->desc.bInterfaceClass,
> > > +			       alt->desc.bInterfaceSubClass,
> > > +			       alt->desc.bInterfaceProtocol);
> > 
> > Are you sure this is correct?
> 
> Works for me :)
> 
> > I had problems with alt (intf->cur_altsetting) beeing null and actually 
> > ended up ignoring the interface bits altogether. I'm bretty sure the 
> > above will crash repeatedly on at least some of my machines.
> 
> Please let me know if it does.  Did you put the modalias on the
> usb_device or the interface?  It belongs on the interface, as this patch
> does.

Here's what I did:

--- linux-2.6.12-rc2/drivers/usb/core/sysfs.c.orig	2005-04-12 14:25:44.000000000 +0200
+++ linux-2.6.12-rc2/drivers/usb/core/sysfs.c	2005-04-12 15:14:24.000000000 +0200
@@ -164,6 +164,22 @@ show_maxchild (struct device *dev, char 
 }
 static DEVICE_ATTR(maxchild, S_IRUGO, show_maxchild, NULL);
 
+static ssize_t
+show_modalias (struct device *dev, char *buf)
+{
+	struct usb_device *udev = to_usb_device (dev);
+	/* FIXME: Add proper interface support */
+	return sprintf (buf, "usb:v%04Xp%04Xdl%04Xdh%04Xdc%02Xdsc%02Xdp%02Xic*isc*ip*\n",
+			le16_to_cpu(udev->descriptor.idVendor),
+			le16_to_cpu(udev->descriptor.idProduct),
+			le16_to_cpu(udev->descriptor.bcdDevice),
+			udev->descriptor.bcdDevice,
+			udev->descriptor.bDeviceClass,
+			udev->descriptor.bDeviceSubClass,
+			udev->descriptor.bDeviceProtocol);
+}
+static DEVICE_ATTR(modalias, S_IRUGO, show_modalias, NULL);
+
 /* Descriptor fields */
 #define usb_descriptor_attr_le16(field, format_string)			\
 static ssize_t								\
@@ -211,6 +245,7 @@ static struct attribute *dev_attrs[] = {
 	&dev_attr_bDeviceSubClass.attr,
 	&dev_attr_bDeviceProtocol.attr,
 	&dev_attr_bNumConfigurations.attr,
+	&dev_attr_modalias.attr,
 	&dev_attr_speed.attr,
 	&dev_attr_devnum.attr,
 	&dev_attr_version.attr,

So, yeah, it was on the device and your version is obviously better. It
does, however, have one little "problem" in common with mine:

Plug in GPS.

$ grep MODALIAS /tmp/hotplug.log
MODALIAS=usb:v067Bp2303dl0202dh0202dc00dsc00dp00icFFisc00ip00
                                                ^^^^^^^^^^^^^
$ cat /sys/devices/pci0000\:00/0000\:00\:07.2/usb1/1-1/modalias
usb:v067Bp2303dl0202dh0202dc00dsc00dp00ic*isc*ip*
                                       ^^^^^^^^^^
> cur_altsetting could be NULL pretty early in the initialization phase of
> a USB device, but by the time these files are created, it should be fine
> (otherwise this same check in the hotplug call would also fail, right?)

> > So now that I'm not able to submit it toghether with a mixture of other,
> > at least slightly, related things that I actually *do believe* have a
> > small possibility of beeing accepted: How do I get my request_modalias
> > patch in? ;) ;)
> 
> Send them on, let's see what you have, and we can take it from there.

I'll do that.

Pelle