On Mon, May 08, 2000, David Brownell <[EMAIL PROTECTED]> wrote:
> > > I applied this, and have to point out that it's got two
> > > incompatibilities with the current usbdevfs code (that I
> > > noticed):
> > >
> > > - Current names vs new ones, for the first root hub:
> > >
> > > /proc/bus/ usb/001/001 ... usbdevfs rules
> > > /devfs/ usb/bus1/device1 ... rule in this patch
> >
> > This is to keep it consistent with the rest of the devfs naming scheme.
>
> But it's a change (two!), and you'd said the only change was
> in the directory prefix.
One change of importance. The other "change" was an extension. I can
hardly see that as a problem.
> > > - Read of "device1" no longer returns the device descriptor.
> >
> > If this is true, there's a bug. It's supposed to return the device
> > descriptor, and then all of the config descriptors afterwards.
>
> I saw no device descriptors; the first two bytes of the file
> were zero, indicating much strangeness.
>
> Seeing config descriptors (order 0..N I assume?) is good, I've
> wanted ways to get them without talking to the devices, but I
> need the device descriptors first!!
Yes, yes. Please read what I said again before you get too worked up
about this.
> (The file size was zero too ... doesn't need to be.)
Good point. Since we know this before hand we can set this. I'll patch
this since devfs provides the functionality.
> > It's an extension of the API.
>
> Your original description didn't mention such extensions.
> Or the filename changes ...
I thought I did mention the extension. There were 2 extensions, GET_DRIVER
and the config descriptors cached via read()
I do see I mentioned it in my original patch, but this time around.
> > > So on grounds that it's API-incompatible, I'd say it needs
> > > more work yet. Neither of these changes was mentioned as
> > > being intended ("same API") ... what's the deal?
> >
> > It is the same API. The first problem isn't part of the API and the second
> > is a bug. I'll look into that.
>
> Is there some other way to get the device address than by
> parsing the filename? It's part of the API. It's not a
> huge one, but such changes broke all code that parsed the
> device names to get device addresses.
You have to parse the name either way, but adding an easy to use ioctl()
would probably be a good idea.
> > > I'd sure rather see "ohci@address" and "uhci@port" addresses
> > > for the busses, since that style name is not a function of the
> > > order I modprobed drivers or plugged in hardware.
>
> No response?
This is not how the rest of the system names directories/device nodes, so
I'd probably say no.
> > > Changing
> > > the device names doesn't seem like it was necessary.
> >
> > You are correct, it is not necessary, but it keeps everything
> > consistent.
>
> It's a minor change, but when you change things in the API,
> you should tell folk that their programs may be breaking.
>
> You'd advertised this as "just changes the usbdevfs prefix"
> and it's certainly not that kind of patch. It's one that
> applications need time to respond to.
You make it sound like the patch is inherently evil.
Sorry I neglected to mention that the names changed. I thought this was
self explanatory since every name changed when things were moved to
devfs. However, I can see how what I said could be misleading.
Most likely programs need to updated anyway to support the root at
/dev/usb versus /proc/bus/usb, so changing a couple of sprintf's or
other code is not all too complicated either.
Anyway, here's a patch to fix the descriptor read problem. It also
implements the file size idea you mentioned.
JE
--- drivers/usb/devio.c.old Wed May 3 16:18:19 2000
+++ drivers/usb/devio.c Mon May 8 16:39:12 2000
@@ -206,6 +206,11 @@
ret = -EFAULT;
goto err;
}
+
+ *ppos += len;
+ buf += len;
+ nbytes -= len;
+ ret += len;
}
pos = sizeof(struct usb_device_descriptor);
@@ -219,7 +224,8 @@
if (len > nbytes)
len = nbytes;
- if (copy_to_user(buf, ps->dev->rawdescriptors[i] + (*ppos -
pos), len)) {
+ if (copy_to_user(buf,
+ ps->dev->rawdescriptors[i] + (*ppos - pos), len)) {
ret = -EFAULT;
goto err;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
