lkml.org 
[lkml]   [2006]   [Dec]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/5] Update Documentation/pci.txt
On Thu, Nov 23, 2006 at 10:12:17PM -0700, Grant Grundler wrote:
> On Fri, Nov 24, 2006 at 09:38:00AM +0900, Hidetoshi Seto wrote:
> > Grant Grundler wrote:
> > >Hidetoshi,
> > >I have a nearly finished rewrite of Documentation/pci.txt.
> > >Can you drop this patch for now on my promise to integrate
> > >your proposed text?
> >
> > No problem at all.
>
> Thanks - I've posted pci.txt-05 on:
> http://www.parisc-linux.org/~grundler/pci.txt-05
>
> and appended it below.
>
> pci.txt-03 is the last version I posted.
> pci.txt-04 contains all feedback from Andi Kleen and Randi Dunlap
> (plus a few other minor changes)
> pci.txt-05 reverts pci_enable_device/pci_request_resource ordering to
> reflect current reality. But I've added a comment to remind us
> about the issue. Also added Section 10/11 from Hidetoshi-san.
> A few minor other changes as well.
>
> If this looks good, I'll post a diff for gregkh.

This looks very good, thanks for doing this work. I do have a few minor
comments:

> 1. pci_register_driver() call
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> PCI device drivers call pci_register_driver() during their
> initialization with a pointer to a structure describing the driver
> (struct pci_driver):
>
> field name Description
> ---------- ------------------------------------------------------
> id_table Pointer to table of device ID's the driver is
> interested in. Most drivers should export this
> table using MODULE_DEVICE_TABLE(pci,...).
>
> probe This probing function gets called (during execution
> of pci_register_driver() for already existing
> devices or later if a new device gets inserted) for
> all PCI devices which match the ID table and are not
> "owned" by the other drivers yet. This function gets
> passed a "struct pci_dev *" for each device whose
> entry in the ID table matches the device. The probe
> function returns zero when the driver chooses to
> take "ownership" of the device or an error code
> (negative number) otherwise.
> The probe function always gets called from process
> context, so it can sleep.
>
> remove The remove() function gets called whenever a device
> being handled by this driver is removed (either during
> deregistration of the driver or when it's manually
> pulled out of a hot-pluggable slot).
> The remove function always gets called from process
> context, so it can sleep.
>
> save_state Save a device's state before it is suspended.

There is no such callback. We have "suspend", "suspend_late",
"resume_early", and "resume". You might want to update this.

>
> suspend Put device into low power state.
>
> resume Wake device from low power state.
>
> enable_wake Enable device to generate wake events from a low power
> state.
>
> (Please see Documentation/power/pci.txt for descriptions
> of PCI Power Management and the related functions.)


>
>
> The ID table is an array of struct pci_device_id entries ending with an
> all-zero entry. Each entry consists of:
>
> vendor,device Vendor and device ID to match (or PCI_ANY_ID)
>
> subvendor, Subsystem vendor and device ID to match (or PCI_ANY_ID)
> subdevice,
>
> class Device class, subclass, and "interface" to match.
> See Appendix D of the PCI Local Bus Spec or
> include/linux/pci_ids.h for a full list of classes.
> Most drivers do not need to specify class/class_mask
> as vendor/device is normally sufficient.
>
> class_mask limit which sub-fields of the class field are compared.
> See drivers/scsi/sym53c8xx_2/ for example of usage.
>
> driver_data Data private to the driver.
> Most drivers don't need to use driver_data field.
> Best practice is to use driver_data as an index
> into a static list of equivalent device types,
> instead of using it as a pointer.

Perhaps mention the PCI_DEVICE() and PCI_DEVICE_CLASS() macros to set
these fields properly?

> Have a table entry {PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID}

PCI_DEVICE(PCI_ANY_ID, PCI_ANY_ID) is a bit smaller :)

> to have probe() called for every PCI device known to the system.
>
> New PCI IDs may be added to a device driver pci_ids table at runtime
> as shown below:
>
> echo "vendor device subvendor subdevice class class_mask driver_data" > \
> /sys/bus/pci/drivers/{driver}/new_id
>
> All fields are passed in as hexadecimal values (no leading 0x).
> Users need pass only as many fields as necessary:
> ovendor, device, subvendor, and subdevice fields default
> to PCI_ANY_ID (FFFFFFFF),
> oclass and classmask fields default to 0
> odriver_data defaults to 0UL.

What's with the "o"s here?

> Once added, the driver probe routine will be invoked for any unclaimed
> PCI devices listed in its (newly updated) pci_ids list.
>
> 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 (zero) is passed in that field.

Note that _no one_ uses this field, so it might go away soon...

> 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.
>
> Please mark the initialization and cleanup functions where appropriate
> (the corresponding macros are defined in <linux/init.h>):
>
> __init Initialization code. Thrown away after the driver
> initializes.
> __exit Exit code. Ignored for non-modular drivers.
> __devinit Device initialization code. Identical to __init if
> the kernel is not compiled with CONFIG_HOTPLUG, normal
> function otherwise.
> __devexit The same for __exit.
>
> Tips on marks:
> o The module_init()/module_exit() functions (and all initialization
> functions called _only_ from these) should be marked __init/__exit.
>
> o The struct pci_driver shouldn't be marked with any of these tags.
>
> o The ID table array should be marked __devinitdata.
>
> o The probe() and remove() functions (and all initialization
> functions called only from these) should be marked __devinit
> and __devexit.
>
> o If the driver is not a hotplug driver then use only
> __init/__exit and __initdata/__exitdata.

No, don't say this, pci drivers are not "hotplug drivers", and since
CONFIG_HOTPLUG is really hard to turn off these days, don't confuse
people with the devinit stuff. Everyone gets it wrong...

> o Pointers to functions marked as __devexit must be created using
> __devexit_p(function_name). That will generate the function
> name or NULL if the __devexit function will be discarded.

I really recommend just not using any of these for almost all PCI
drivers, as the space savings just really isn't there...

thanks,

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

\
 
 \ /
  Last update: 2006-12-06 08:29    [W:0.089 / U:16.256 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site