2003-01-11 14:34:45

by Andries E. Brouwer

[permalink] [raw]
Subject: sysfs

Yesterday evening I wrote a trivial utility fd ("find device")
that gives the contents of sysfs. Mostly in order to see what
name the memory stick card reader has today.

I wondered about several things.
Is there a description of the intended hierachy, so that one can
compare present facts with intention?

In /sysfs/devices I see
1:0:6:0 2:0:0:1 2:0:0:3 3:0:0:1 4:0:0:0 4:0:0:2 ide0 legacy sys
2:0:0:0 2:0:0:2 3:0:0:0 3:0:0:2 4:0:0:1 ide-scsi ide1 pci0
many SCSI devices and some subdirectories.
Would it not be better to have subdirectories scsiN just like ideN?
One can have SCSI hosts, even when presently no devices are connected.

There are links back and forth between block and bus, so that
/sysfs/block/sda/device points at devices/1:0:6:0 and
/sysfs/devices/1:0:6:0/block points at block/sda. Good.
In the case my device is a USB device I tend to first look
at VendorID and ProductID. But it seems there are no pointers
from /sysfs/block or /sysfs/devices to the usb device hierachy.

Now /sysfs/devices/pci0/00:07.2/usb1/1-2/1-2.4/1-2.4.4
is the fourth device on some USB hub, a card reader with
idVendor=057b, and it is the scsi host scsi2. But it does not
refer to scsi2, perhaps because the category scsi host is missing
in the hierarchy?

Andries


2003-01-14 00:21:45

by Patrick Mansfield

[permalink] [raw]
Subject: Re: sysfs

Andries -

On Sat, Jan 11, 2003 at 03:43:27PM +0100, [email protected] wrote:
> Yesterday evening I wrote a trivial utility fd ("find device")
> that gives the contents of sysfs. Mostly in order to see what
> name the memory stick card reader has today.
>
> I wondered about several things.
> Is there a description of the intended hierachy, so that one can
> compare present facts with intention?
>
> In /sysfs/devices I see
> 1:0:6:0 2:0:0:1 2:0:0:3 3:0:0:1 4:0:0:0 4:0:0:2 ide0 legacy sys
> 2:0:0:0 2:0:0:2 3:0:0:0 3:0:0:2 4:0:0:1 ide-scsi ide1 pci0
> many SCSI devices and some subdirectories.
> Would it not be better to have subdirectories scsiN just like ideN?
> One can have SCSI hosts, even when presently no devices are connected.

It looks like there is a missing scsi_set_device() call in scsiglue.c,
(similiar to what happens if we handled NULL dev pointer in scis_add_host)
so all the usb scsi devices end up under /sysfs/devices.

I don't have any usb mass storage devices, this patch against 2.5 bk
compiles but otherwise is not tested. It should put the usb-scsi mass
storage devices below the usb sysfs dev (I assume in your case under
/sysfs/devices/pci0/00:07.2/usb1/1-2/1-2.4/1-2.4.4).

Maybe Matthew or Greg can comment.

--- 1.33/drivers/usb/storage/scsiglue.c Sun Nov 10 09:49:52 2002
+++ edited/drivers/usb/storage/scsiglue.c Mon Jan 13 15:33:49 2003
@@ -90,6 +90,7 @@
if (us->host) {
us->host->hostdata[0] = (unsigned long)us;
us->host_no = us->host->host_no;
+ scsi_set_device(us->host, &us->pusb_dev->dev);
return 1;
}

-- Patrick Mansfield

2003-01-14 00:49:09

by Greg KH

[permalink] [raw]
Subject: Re: sysfs

On Mon, Jan 13, 2003 at 04:51:02PM -0800, Matthew Dharm wrote:
> Really, we don't want to hang the device under USB... it's really an
> emulated SCSI device. Or, at least I think so.

Yes, and since you're looking like a scsi device, and acting like a scsi
device, and you are showing up in the sysfs tree as a scsi device, let's
at least point your device to the proper place, so that the tree shows
the proper representation for what is happening.

thanks,

greg k-h

2003-01-14 00:47:20

by Greg KH

[permalink] [raw]
Subject: Re: sysfs

On Mon, Jan 13, 2003 at 04:27:41PM -0800, Patrick Mansfield wrote:
>
> I don't have any usb mass storage devices, this patch against 2.5 bk
> compiles but otherwise is not tested. It should put the usb-scsi mass
> storage devices below the usb sysfs dev (I assume in your case under
> /sysfs/devices/pci0/00:07.2/usb1/1-2/1-2.4/1-2.4.4).

This patch works for me, nice. I'll add it to my trees.

Matt, the struct us_data only holds a pointer to the struct usb_device
for any specific usb storage device. Is there a pointer to the actual
interface that it is really bound to? That would be the proper thing to
pass to scsi_set_device() as it's really the interface that you are in
control of, not the whole device.

Hm, you do have the ifnum, how about this patch on top of Pat's patch
instead? It works for me and shows the relationship better.

thanks,

greg k-h


# USB: put the usb storage's SCSI device in the proper place in sysfs.
# Also makes usb_ifnum_to_if() a public function

diff -Nru a/drivers/usb/core/hcd.h b/drivers/usb/core/hcd.h
--- a/drivers/usb/core/hcd.h Mon Jan 13 16:54:37 2003
+++ b/drivers/usb/core/hcd.h Mon Jan 13 16:54:37 2003
@@ -353,9 +353,6 @@
extern void usb_bus_get (struct usb_bus *bus);
extern void usb_bus_put (struct usb_bus *bus);

-extern struct usb_interface *usb_ifnum_to_if (struct usb_device *dev,
- unsigned ifnum);
-
extern int usb_find_interface_driver (struct usb_device *dev,
struct usb_interface *interface);

diff -Nru a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
--- a/drivers/usb/core/usb.c Mon Jan 13 16:54:37 2003
+++ b/drivers/usb/core/usb.c Mon Jan 13 16:54:37 2003
@@ -1484,6 +1484,7 @@
EXPORT_SYMBOL(usb_driver_release_interface);
EXPORT_SYMBOL(usb_match_id);
EXPORT_SYMBOL(usb_find_interface);
+EXPORT_SYMBOL(usb_ifnum_to_if);

EXPORT_SYMBOL(usb_new_device);
EXPORT_SYMBOL(usb_reset_device);
diff -Nru a/drivers/usb/storage/scsiglue.c b/drivers/usb/storage/scsiglue.c
--- a/drivers/usb/storage/scsiglue.c Mon Jan 13 16:54:37 2003
+++ b/drivers/usb/storage/scsiglue.c Mon Jan 13 16:54:37 2003
@@ -88,9 +88,12 @@
/* register the host */
us->host = scsi_register(sht, sizeof(us));
if (us->host) {
+ struct usb_interface *iface;
us->host->hostdata[0] = (unsigned long)us;
us->host_no = us->host->host_no;
- scsi_set_device(us->host, &us->pusb_dev->dev);
+ iface = usb_ifnum_to_if(us->pusb_dev, us->ifnum);
+ if (iface)
+ scsi_set_device(us->host, &iface->dev);
return 1;
}

diff -Nru a/include/linux/usb.h b/include/linux/usb.h
--- a/include/linux/usb.h Mon Jan 13 16:54:37 2003
+++ b/include/linux/usb.h Mon Jan 13 16:54:37 2003
@@ -277,7 +277,9 @@
const struct usb_device_id *usb_match_id(struct usb_interface *interface,
const struct usb_device_id *id);

-struct usb_interface *usb_find_interface(struct usb_driver *drv, kdev_t kdev);
+extern struct usb_interface *usb_find_interface(struct usb_driver *drv, kdev_t kdev);
+extern struct usb_interface *usb_ifnum_to_if(struct usb_device *dev, unsigned ifnum);
+

/**
* usb_make_path - returns stable device path in the usb tree

2003-01-14 00:42:18

by Matthew Dharm

[permalink] [raw]
Subject: Re: sysfs

Really, we don't want to hang the device under USB... it's really an
emulated SCSI device. Or, at least I think so.

Matt

On Mon, Jan 13, 2003 at 04:27:41PM -0800, Patrick Mansfield wrote:
> Andries -
>
> On Sat, Jan 11, 2003 at 03:43:27PM +0100, [email protected] wrote:
> > Yesterday evening I wrote a trivial utility fd ("find device")
> > that gives the contents of sysfs. Mostly in order to see what
> > name the memory stick card reader has today.
> >
> > I wondered about several things.
> > Is there a description of the intended hierachy, so that one can
> > compare present facts with intention?
> >
> > In /sysfs/devices I see
> > 1:0:6:0 2:0:0:1 2:0:0:3 3:0:0:1 4:0:0:0 4:0:0:2 ide0 legacy sys
> > 2:0:0:0 2:0:0:2 3:0:0:0 3:0:0:2 4:0:0:1 ide-scsi ide1 pci0
> > many SCSI devices and some subdirectories.
> > Would it not be better to have subdirectories scsiN just like ideN?
> > One can have SCSI hosts, even when presently no devices are connected.
>
> It looks like there is a missing scsi_set_device() call in scsiglue.c,
> (similiar to what happens if we handled NULL dev pointer in scis_add_host)
> so all the usb scsi devices end up under /sysfs/devices.
>
> I don't have any usb mass storage devices, this patch against 2.5 bk
> compiles but otherwise is not tested. It should put the usb-scsi mass
> storage devices below the usb sysfs dev (I assume in your case under
> /sysfs/devices/pci0/00:07.2/usb1/1-2/1-2.4/1-2.4.4).
>
> Maybe Matthew or Greg can comment.
>
> --- 1.33/drivers/usb/storage/scsiglue.c Sun Nov 10 09:49:52 2002
> +++ edited/drivers/usb/storage/scsiglue.c Mon Jan 13 15:33:49 2003
> @@ -90,6 +90,7 @@
> if (us->host) {
> us->host->hostdata[0] = (unsigned long)us;
> us->host_no = us->host->host_no;
> + scsi_set_device(us->host, &us->pusb_dev->dev);
> return 1;
> }
>
> -- Patrick Mansfield

--
Matthew Dharm Home: [email protected]
Maintainer, Linux USB Mass Storage Driver

It was a new hope.
-- Dust Puppy
User Friendly, 12/25/1998


Attachments:
(No filename) (2.02 kB)
(No filename) (232.00 B)
Download all attachments

2003-01-14 01:00:22

by Andries E. Brouwer

[permalink] [raw]
Subject: Re: sysfs

> It looks like there is a missing scsi_set_device() call in scsiglue.c,

OK, added. Now rmmod usb-storage followed by insmod usb-storage
resulted in an oops, as usual, but after a fresh reboot:
Yes indeed, just like desired:

% ls -l /sysfs/block/sdb/device
... -> ../../devices/pci0/00:07.2/usb1/1-2/1-2.4/1-2.4.1/2:0:0:0

Good.
Now that you removed this scsi device from /sysfs/devices, I suppose
you'll also want to remove

/sysfs/devices/1:0:6:0

which is an Iomega ZIP drive on the parallel port, driver imm.c,
device sda.
(I can also do it but have no time now. Friday.)

All the best - Andries

2003-01-14 01:24:54

by Patrick Mansfield

[permalink] [raw]
Subject: Re: sysfs

On Tue, Jan 14, 2003 at 02:09:06AM +0100, [email protected] wrote:
> > It looks like there is a missing scsi_set_device() call in scsiglue.c,
>
> OK, added. Now rmmod usb-storage followed by insmod usb-storage
> resulted in an oops, as usual, but after a fresh reboot:
> Yes indeed, just like desired:
>
> % ls -l /sysfs/block/sdb/device
> ... -> ../../devices/pci0/00:07.2/usb1/1-2/1-2.4/1-2.4.1/2:0:0:0
>
> Good.
> Now that you removed this scsi device from /sysfs/devices, I suppose
> you'll also want to remove
>
> /sysfs/devices/1:0:6:0
>
> which is an Iomega ZIP drive on the parallel port, driver imm.c,
> device sda.
> (I can also do it but have no time now. Friday.)
>
> All the best - Andries

I don't know and have not used the parport code, it needs to be ported to
sysfs before we could simply call scsi_set_device() call in imm.c, or
maybe have a /sysfs/scsi/pseudo added via scsi_add_host(shost, NULL) or a
/sysfs/scsi/unported, similiar to was discussed on the linux-scsi thread
"[PATCH] allow NULL dev argument to scsi_add_host":

http://marc.theaimsgroup.com/?t=104231385300005&r=1&w=2

-- Patrick Mansfield