On Wed, Mar 30, 2005 at 08:32:44PM +0200, Kay Sievers wrote:
> On Thu, 2005-03-17 at 09:53 -0500, James Bottomley wrote:
> > On Wed, 2005-03-16 at 14:45 -0800, Patrick Mansfield wrote:
> > > Any comments on this? Should I resend these patches?
> > 
> > Well, the basic comment is that there are a lot of features that SCSI
> > has that the driver core lacks:
> > 
> > 1) Attribute overrides.  This is actually part of the published API for SCSI
> 
> What does this exactly mean? Handle the same attribute dynamically with
> a different source of data?

Well a different function handled by the host driver, rather than the
function in scsi core, for example, the twa_queue_depth_attr in
drivers/scsi/3w-9xxx.c overrides the simple read attr function in
scsi_syfs.c created via this line:

        sdev_rd_attr (queue_depth, "%d\n").  

The patch left this in but only for queue_depth, not for all of the
scsi_devie attributes. We have sht->change_queue_depth that can already
handle this, some drivers use it, but some still use the sdev_attrs to
override the queue_depth. IMO, we should get rid of the override, James
doesn't agree.

James - are you OK with allowing only specific overrides of attributes?

> > 2) Ability to add extra attributes---several drivers use this
> 
> What is missing in the driver core here? Why can't you add/remove
> attribute files at any time? 

The patch left this alone, but should be changed to make sure duplicate
(that we can't override) attributes get errors. It is just a nice way for
host adapters to add attributes to a scsi_device.

But, for any of these non bus_attr and class attributes, the hotplug
notification (for scsi_device or the upper level block/char device) will
still come before the attributes are created. This is also true for the
transport attributes. This is a potential problem, I know of no udev
program or hotplug helpers that use any of these attributes.

I see no easy way around this problem without a multi-step registration,
like a device_init(), device_ready(). So a device_add would just be:

        device_init()
        device_ready()

Is your kobject_hotplug patch still pending?

We also need the following, else dev_attrs will show up after the probe
and hotplug event for a matching driver (for example, an sd block hotplug
event is generated via the device_attach() call). This could be causing
problems today, AFAIR you already hack around this problem in udev.

--- bleed-2.5/drivers/base/orig-bus.c   Tue Mar 15 17:08:18 2005
+++ bleed-2.5/drivers/base/bus.c        Wed Mar 30 10:48:34 2005
@@ -458,14 +458,14 @@ int bus_add_device(struct device * dev)
        int error = 0;
 
        if (bus) {
+               device_add_attrs(bus, dev);
+               sysfs_create_link(&bus->devices.kobj, &dev->kobj, dev->bus_id);
+               sysfs_create_link(&dev->kobj, &dev->bus->subsys.kset.kobj, 
"bus");
                down_write(&dev->bus->subsys.rwsem);
                pr_debug("bus %s: add device %s\n", bus->name, dev->bus_id);
                list_add_tail(&dev->bus_list, &dev->bus->devices.list);
                device_attach(dev);
                up_write(&dev->bus->subsys.rwsem);
-               device_add_attrs(bus, dev);
-               sysfs_create_link(&bus->devices.kobj, &dev->kobj, dev->bus_id);
-               sysfs_create_link(&dev->kobj, &dev->bus->subsys.kset.kobj, 
"bus");
        }
        return error;
 }

-- Patrick Mansfield
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to