Currently, code exists in the pci layer to allow userspace to specify
driver data when adding a pci dynamic id from sysfs. However, this data
is never used and there exists no way in the existing code to use it.
This patch allows device drivers to indicate that they want driver data
passed to them on dynamic id adds by initializing use_driver_data in their
pci_driver->pci_dynids struct. The documentation has also been updated
to reflect this.
Signed-off-by: Brian King <[email protected]>
---
linux-2.6.11-rc3-bk4-bjking1/Documentation/pci.txt | 8 ++++----
linux-2.6.11-rc3-bk4-bjking1/drivers/pci/pci-driver.c | 1 -
2 files changed, 4 insertions(+), 5 deletions(-)
diff -puN drivers/pci/pci-driver.c~pci_dynids_driver_data drivers/pci/pci-driver.c
--- linux-2.6.11-rc3-bk4/drivers/pci/pci-driver.c~pci_dynids_driver_data 2005-02-07 15:58:21.000000000 -0600
+++ linux-2.6.11-rc3-bk4-bjking1/drivers/pci/pci-driver.c 2005-02-07 15:58:21.000000000 -0600
@@ -115,7 +115,6 @@ static DRIVER_ATTR(new_id, S_IWUSR, NULL
static inline void
pci_init_dynids(struct pci_dynids *dynids)
{
- memset(dynids, 0, sizeof(*dynids));
spin_lock_init(&dynids->lock);
INIT_LIST_HEAD(&dynids->list);
}
diff -puN Documentation/pci.txt~pci_dynids_driver_data Documentation/pci.txt
--- linux-2.6.11-rc3-bk4/Documentation/pci.txt~pci_dynids_driver_data 2005-02-07 15:58:21.000000000 -0600
+++ linux-2.6.11-rc3-bk4-bjking1/Documentation/pci.txt 2005-02-07 15:58:21.000000000 -0600
@@ -99,10 +99,10 @@ where all fields are passed in as hexade
Users need pass only as many fields as necessary; vendor, device,
subvendor, and subdevice fields default to PCI_ANY_ID (FFFFFFFF),
class and classmask fields default to 0, and driver_data defaults to
-0UL. Device drivers must call
- pci_dynids_set_use_driver_data(pci_driver *, 1)
-in order for the driver_data field to get passed to the driver.
-Otherwise, only a 0 is passed in that field.
+0UL. Device drivers must initialize use_driver_data in the dynids struct
+in their pci_driver struct prior to calling pci_register_driver in order
+for the driver_data field to get passed to the driver. Otherwise, only a
+0 is passed in that field.
When the driver exits, it just calls pci_unregister_driver() and the PCI layer
automatically calls the remove hook for all devices handled by the driver.
_
On Mon, Feb 07, 2005 at 04:00:27PM -0600, [email protected] wrote:
>
> Currently, code exists in the pci layer to allow userspace to specify
> driver data when adding a pci dynamic id from sysfs. However, this data
> is never used and there exists no way in the existing code to use it.
Which is a good thing, right? "driver_data" is usually a pointer to
somewhere. Having userspace specify it would not be a good thing.
> This patch allows device drivers to indicate that they want driver data
> passed to them on dynamic id adds by initializing use_driver_data in their
> pci_driver->pci_dynids struct. The documentation has also been updated
> to reflect this.
What driver wants to use this?
thanks,
greg k-h
Greg KH wrote:
> On Mon, Feb 07, 2005 at 04:00:27PM -0600, [email protected] wrote:
>
>>Currently, code exists in the pci layer to allow userspace to specify
>>driver data when adding a pci dynamic id from sysfs. However, this data
>>is never used and there exists no way in the existing code to use it.
>
>
> Which is a good thing, right? "driver_data" is usually a pointer to
> somewhere. Having userspace specify it would not be a good thing.
That depends on the driver usage, and the patch allows it to be
configurable and defaults to not being used.
>>This patch allows device drivers to indicate that they want driver data
>>passed to them on dynamic id adds by initializing use_driver_data in their
>>pci_driver->pci_dynids struct. The documentation has also been updated
>>to reflect this.
>
>
> What driver wants to use this?
I am in the process of adding dynids support into the ipr scsi driver. I
originally was using driver_data as a pointer, but am changing it to be
an index instead, so that it can be specified by the user.
There are essentially 2 different types of chipsets that ipr controls,
the primary difference being the register offsets. I am using
driver_data to figure that out today.
My other option is to somehow change the driver to cope with having no
driver data, but that will result in more driver code and will
ultimately be less flexible in the new chipsets that can be added using
dynids.
-Brian
--
Brian King
eServer Storage I/O
IBM Linux Technology Center
Hello!
> >Which is a good thing, right? "driver_data" is usually a pointer to
> >somewhere. Having userspace specify it would not be a good thing.
>
> That depends on the driver usage, and the patch allows it to be
> configurable and defaults to not being used.
Maybe we could just define the operation as cloning of an entry
for another device ID, including its driver_data.
Have a nice fortnight
--
Martin `MJ' Mares <[email protected]> http://atrey.karlin.mff.cuni.cz/~mj/
Faculty of Math and Physics, Charles University, Prague, Czech Rep., Earth
Only dead fish swim with the stream.
Martin Mares wrote:
> Hello!
>
>
>>>Which is a good thing, right? "driver_data" is usually a pointer to
>>>somewhere. Having userspace specify it would not be a good thing.
>>
>>That depends on the driver usage, and the patch allows it to be
>>configurable and defaults to not being used.
>
>
> Maybe we could just define the operation as cloning of an entry
> for another device ID, including its driver_data.
Possibly. That would potentially require a lot of parameters to
userspace. We would really need to duplicate all the currently existing
sysfs parms to accomplish this.
--
Brian King
eServer Storage I/O
IBM Linux Technology Center
On Mon, Feb 07, 2005 at 02:18:20PM -0800, Greg KH wrote:
> On Mon, Feb 07, 2005 at 04:00:27PM -0600, [email protected] wrote:
> >
> > Currently, code exists in the pci layer to allow userspace to specify
> > driver data when adding a pci dynamic id from sysfs. However, this data
> > is never used and there exists no way in the existing code to use it.
>
> Which is a good thing, right? "driver_data" is usually a pointer to
> somewhere. Having userspace specify it would not be a good thing.
Yeah, I don't think it's safe to use "driver_data". Although it can be a
set of flags, it's also often used as an index in an array, or as a
pointer. An invalid value could result in an oops.
Most drivers don't use "driver_data". For those that do, it might be
helpful to have the capability of setting a few static configuration values
before probing the device. Currently "driver_data" fills this role.
Perhaps we need a new mechanism that would be more useable with sysfs?
The current code is limiting because the configuration options in
"driver_data" are not well defined. Any ideas?
Thanks,
Adam
P.S.: The pci serial driver is a good example.
Adam Belay wrote:
> On Mon, Feb 07, 2005 at 02:18:20PM -0800, Greg KH wrote:
>
>>On Mon, Feb 07, 2005 at 04:00:27PM -0600, [email protected] wrote:
>>
>>>Currently, code exists in the pci layer to allow userspace to specify
>>>driver data when adding a pci dynamic id from sysfs. However, this data
>>>is never used and there exists no way in the existing code to use it.
>>
>>Which is a good thing, right? "driver_data" is usually a pointer to
>>somewhere. Having userspace specify it would not be a good thing.
>
>
> Yeah, I don't think it's safe to use "driver_data". Although it can be a
> set of flags, it's also often used as an index in an array, or as a
> pointer. An invalid value could result in an oops.
>
> Most drivers don't use "driver_data". For those that do, it might be
> helpful to have the capability of setting a few static configuration values
> before probing the device. Currently "driver_data" fills this role.
> Perhaps we need a new mechanism that would be more useable with sysfs?
> The current code is limiting because the configuration options in
> "driver_data" are not well defined. Any ideas?
Unfortunately, from what I have gathered so far, driver_data is pretty
unique for different drivers, especially when comparing the usage in the
pci serial driver to a scsi hba driver like ipr.
Certainly, a driver that uses driver_data as a pointer cannot allow it
to be passed in from userspace. But a device driver that uses it as an
index into an array can easily allow it to be passed in and do some
simple range checking on it to verify it is a valid value before
indexing into the array.
--
Brian King
eServer Storage I/O
IBM Linux Technology Center