Re: [PATCH 1/1] cciss: resubmit export uid, model, vendor, rev to sysfs

[Kay Sievers]

On Tue, Apr 7, 2009 at 23:19, Jens Axboe <jens.axboe@xxxxxxxxxx> wrote:
> On Tue, Apr 07 2009, Mike Miller (OS Dev) wrote:

> The patch looks fine to me, but my sysfs foo is very weak though. CC'ing
> Kay, perhaps he can help take a quick look at this.

>> + * Initialize sysfs for each logical drive. ÂThis sets up and registers
>> + * the 'c#d#' directory for each individual logical drive under
>> + * /sys/bus/pci/devices/<dev/ccis#/. We also create a link from
>> + * /sys/block/cciss!c#d# to this entry.
>> + */
>> +static int cciss_create_ld_sysfs_entry(struct ctlr_info *h,
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âdrive_info_struct *drv,
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âint drv_index)
>> +{
>> + Â Â device_initialize(&drv->dev);
>> + Â Â drv->dev.type = &cciss_dev_type;
>> + Â Â dev_set_name(&drv->dev, "c%dd%d", h->ctlr, drv_index);
>> + Â Â drv->dev.parent = &h->dev;
>> + Â Â return device_add(&drv->dev);
>> +}

If I read that correctly, you are creating a hierarchy of devices
where the devices do not belong to any subsystem? This is what we need
to avoid in almost all cases, we need a "subsystem" link in sysfs.

I wold expect the cciss devices not to be a magic, silently created,
subdirectory of a pci device, but to have their own "cciss" bus in
sysfs, so the created devices get proper events at creation time. All
the cciss devices would show up in its own directory
/sys/bus/cciss/devices/*.

I think, you should name all "cciss bus devices" uniquely, and assign
them to a "cciss bus_type". We really do not want unclassified devices
in the chain of parent devices of a block device.

Or do I missing something here?

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