Re: [PATCH 1/5] Update Documentation/pci.txt
> On Mon, 18 Dec 2006 00:11:33 -0700 Grant Grundler wrote: ... >> +4.1 Stop IRQs on the device >> +~~~ >> +How to do this is chip/device specific. If it's not done, it opens >> +the possibility of a "screaming interrupt" if (and only if) >> +the IRQ is shared with another device. >> + >> +When the shared IRQ handler is "unhoooked", the remaining devices ^^^ -> unhooked ... >> +11. MMIO Space and "Write Posting" >> +~~ >> +Converting a driver from using I/O Port space to using MMIO space >> +often requires some additional changes. Specifically, "write posting" >> +needs to be handled. Many drivers (e.g. tg3, acenic, sym53c8xx_2) >> +already do. I/O Port space guarantees write transactions reach the PCI > >already do this. > >> +device before the CPU can continue. Writes to MMIO space allow to CPU ^^ >> +continue before the transaction reaches the PCI device. HW weenies ^ -> allow the CPU to continue -- Stefan Richter -=-=-==- ==-- =-==- http://arcgraph.de/sr/ - 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
Re: [PATCH 1/5] Update Documentation/pci.txt
On Mon, 18 Dec 2006 00:11:33 -0700 Grant Grundler wrote: > Full version of pci.txt (v6 is 677 lines): > http://www.parisc-linux.org/~grundler/Documentation/pci.txt-06 > > I've appended patch v6 (823 lines!) and commit log entry is below. > > > > diff --git a/Documentation/pci.txt b/Documentation/pci.txt > index 2b395e4..dce829e 100644 > --- a/Documentation/pci.txt > +++ b/Documentation/pci.txt > @@ -1,147 +1,235 @@ > - How To Write Linux PCI Drivers > > -by Martin Mares <[EMAIL PROTECTED]> on 07-Feb-2000 > + How To Write Linux PCI Drivers > + > + by Martin Mares <[EMAIL PROTECTED]> on 07-Feb-2000 > + updatedby Grant Grundler <[EMAIL PROTECTED]> on 17-Dec-2006 updated by > > > +pci_register_driver() leaves most of the probing for devices to > +the PCI layer and supports online insertion/removal of devices [thus > +supporting PCI, hot-pluggable PCI and CardBus in a single driver]. ExpressCard ? > +pci_register_driver() call requires passing in a table of function > +calls and thus dictates the high level structure of a driver. s/calls/pointers/ ? > + > +Once the driver knows about a PCI device and takes ownership, the > +driver generally needs to perform the following initialization: > > Enable the device > - Access device configuration space > - Discover resources (addresses and IRQ numbers) provided by the device > - Allocate these resources > - Communicate with the device > + request MMIO/IOP resources > + set the DMA mask size (for both coherent and streaming DMA) > + allocate and initialize shared control data (pci_allocate_coherent()) > + Access device configuration space (if needed) > + register IRQ handler (request_irq()) > + Initialize non-PCI (ie LAN/SCSI/etc parts of the chip) > + enable DMA/processing engines. Please capitalize the first word of each list item. (i.e., be consistent) > +When done using the device, and perhaps the module needs to be unloaded, > +the driver needs to take the follow steps: > + disable the device from generating IRQs > + release the IRQ (free_irq()) > + stop all DMA activity > + release DMA buffers (both streaming and coherent) > + unregister from other subsystems (e.g. scsi or netdev) > + release MMIO/IOP resources > Disable the device Be consistent. > +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 > + -- -- > + shutdownHook into reboot_notifier_list (kernel/sys.c). > + Intended to stop any idling DMA operations. > + Useful for enabling wake-on-lan (NIC) or change s/change/changing/ or /to change/ > + the power state of a device before reboot. > + e.g. drivers/net/e100.c. > + > + multithread_probe Enable multi-threaded probe/scan. Driver is > + required to provide it's own locking/syncronization s/it's/its/ s/syncronization/synchronization/ > + for init operations if this is enabled. > + > + > +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, > + > > > + > +1.1 "Marks" for driver functions/data Markers, Attributes, Tags ? (I prefer Attributes.) > + > Please mark the initialization and cleanup functions where appropriate > (the corresponding macros are defined in ): > > > +2. How to find PCI devices manually > +~~~ > + > +A manual search may be performed using the following constructs: > > Searching by vendor and device ID: > > - struct pci_dev *dev = NULL; > - while (dev = pci_get_device(VENDOR_ID, DEVICE_ID, dev)) > + structpci_dev *dev = NULL; missing some spaces: struct pci_dev > + while(dev = pci_get_device(VENDOR_ID, DEVICE_ID, dev)) while ( > configure_device(dev); > > +3. Device Initialization Steps > +~~ > + > +As noted in the introduction, most PCI drivers need the following steps > +for device initialization: > > - If you want to use the PCI Memory-Write-Invalidate transaction, > + Enable the device > + request MMIO/IOP resources > + set the DMA mask size (for both coherent and streaming DMA) > + allocate and initialize shared control data (pci_allocate_coherent()) > + Access device
Re: [PATCH 1/5] Update Documentation/pci.txt
On Fri, Dec 15, 2006 at 09:02:07AM -0800, Greg KH wrote: > On Sun, Dec 10, 2006 at 12:25:08AM -0700, Grant Grundler wrote: ... [ re __devinit/__devexit ] > > o drivers should use __dev* exactly because HOTPLUG is defined. > > Yes, they should, but it is confusing as to why it should be used in > places, and __init used in others. If you want to detail the > differences better than the current documentation does, please do. I took a whack at this but I suspect in general you are right. It's easy to get confused about when to use one or the other. I tried to make it clearer in version 6 of pci.txt. > > o Why does everyone get __dev* wrong? Bad API? Missing or bad Documentation? > > [ This is not a free-for-all...I'd like a clear answer from > > Greg what would help driver writers get this right. ] > > They get it wrong usually because they cut-and-paste from others. The > proof of that is seeing __dev* used in pci hotplug controller drivers :) > > I'm just pointing out that about every 6 months I have to sweep the tree > and fix up all of the improper usages. And the whole __devexit_p() > stuff is usually used incorrectly too. The __devexit_p() is a non-obvious thing and was badly documented. I've rewritten that bit completely and hope using an example makes a difference. I wouldn't be offended if someone wanted to rewrite it again. But my fear is this doc is starting to get so long as to miss it's primary goal: be a _short_ introduction to Kernel PCI API. Full version of pci.txt (v6 is 677 lines): http://www.parisc-linux.org/~grundler/Documentation/pci.txt-06 I've appended patch v6 (823 lines!) and commit log entry is below. thanks, grant Rewrite Documentation/pci.txt: o restructure document to match how API is used when writing init code. o update to reflect changes in struct pci_driver function calls. o removed language on "new style vs old style" device discovery. "Old style" is now deprecated. Don't use it. Left description in to document existing driver behaviors. o add section "Legacy I/O Port free driver" by Kenji Kaneshige http://lkml.org/lkml/2006/11/22/25 o add "MMIO space and write posting" section to help avoid common pitfall when converting drivers from IO Port space to MMIO space. Orignally posted http://lkml.org/lkml/2006/2/27/24 Signed-off-by: Grant Grundler <[EMAIL PROTECTED]> diff --git a/Documentation/pci.txt b/Documentation/pci.txt index 2b395e4..dce829e 100644 --- a/Documentation/pci.txt +++ b/Documentation/pci.txt @@ -1,147 +1,235 @@ -How To Write Linux PCI Drivers - by Martin Mares <[EMAIL PROTECTED]> on 07-Feb-2000 + How To Write Linux PCI Drivers + + by Martin Mares <[EMAIL PROTECTED]> on 07-Feb-2000 + updatedby Grant Grundler <[EMAIL PROTECTED]> on 17-Dec-2006 -The world of PCI is vast and it's full of (mostly unpleasant) surprises. -Different PCI devices have different requirements and different bugs -- -because of this, the PCI support layer in Linux kernel is not as trivial -as one would wish. This short pamphlet tries to help all potential driver -authors find their way through the deep forests of PCI handling. +The world of PCI is vast and full of (mostly unpleasant) surprises. +Since each CPU architecture implements different chipsets and PCI devices +have different requirements (erm, "features"), the result is the PCI support +in the Linux kernel is not as trivial as one would wish. This short paper +tries to introduce all potential driver authors to Linux APIs for +PCI device drivers. + +A more complete resource is the third edition of "Linux Device Drivers" +by Jonathan Corbet, Alessandro Rubini, and Greg Kroah-Hartman. +LDD3 is available for free (under Creative Commons License) from: + + http://lwn.net/Kernel/LDD3/ + +However, keep in mind that all documents are subject to "bit rot". +Refer to the source code if things are not working as described here. + +Please send questions/comments/patches about Linux PCI API to the +"Linux PCI" <[EMAIL PROTECTED]> mailing list. + 0. Structure of PCI drivers ~~~ -There exist two kinds of PCI drivers: new-style ones (which leave most of -probing for devices to the PCI layer and support online insertion and removal -of devices [thus supporting PCI, hot-pluggable PCI and CardBus in a single -driver]) and old-style ones which just do all the probing themselves. Unless -you have a very good reason to do so, please don't use the old way of probing -in any new code. After the driver finds the devices it wishes to operate -on (either the old or the new way), it needs to perform the following steps: +PCI drivers "discover" PCI devices in a system via pci_register_driver(). +Actually, it's the other way around. When the PCI generic code discovers +a new device, the driver with a matching "description
Re: [PATCH 1/5] Update Documentation/pci.txt
On Sun, Dec 10, 2006 at 12:25:08AM -0700, Grant Grundler wrote: > On Tue, Dec 05, 2006 at 11:26:51PM -0800, Greg KH wrote: > ... > > I do have a few minor comments: > ... > > > Please mark the initialization and cleanup functions where appropriate > > > (the corresponding macros are defined in ): > > > > > > __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", > > agreed - removed that bullet item. > > > and since CONFIG_HOTPLUG is really hard to turn off these days, > > don't confuse people with the devinit stuff. Everyone gets it wrong... > > While revisiting this bit, I started thinking: > > o While I agree HOTPLUG is essential to desktop and server, > I don't think that's true for "embedded" (e.g. routers/switches). Agreed, but those are the minority. > o drivers should use __dev* exactly because HOTPLUG is defined. Yes, they should, but it is confusing as to why it should be used in places, and __init used in others. If you want to detail the differences better than the current documentation does, please do. > o Why does everyone get __dev* wrong? Bad API? Missing or bad Documentation? > [ This is not a free-for-all...I'd like a clear answer from > Greg what would help driver writers get this right. ] They get it wrong usually because they cut-and-paste from others. The proof of that is seeing __dev* used in pci hotplug controller drivers :) I'm just pointing out that about every 6 months I have to sweep the tree and fix up all of the improper usages. And the whole __devexit_p() stuff is usually used incorrectly too. > o Prefer a seperate patch to clean this up? > Take what I have for now and sort out the __devinit handling in > another round? > > o Note what I have is essentially the original text - just reformatted > to be a bit more readable. > > o Hrm...what did greg mean with "it"? All of the markers? > Or just the __dev* markers? Just the __dev stuff. > > > 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... > > It's a bit too late for that, no? > And even if it's more of a PITA than it's worth, we do save something: > > # hppa64-linux-gnu-readelf -S vmlinux > ... > [26] .init.textPROGBITS 40598000 00457000 > 00022280 AX 0 0 8 > [27] .init.dataPROGBITS 405ba280 00479280 > 0001faf0 WA 0 0 8 > ... > > Reality is they are used in _alot_ of drivers. > I checked 2.6.19: > grundler <514>find -name \*.c | xargs fgrep __devinit | wc >2723 16812 235863 > > I'd prefer to keep the short references here if you > don't mind too much. At least until you can get some > consensus that __init and __exit should go away > or get replaced by easier-to-get-right markers. Ok, just describe them properly and we should be fine. thanks, greg k-h - 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
Re: [PATCH 1/5] Update Documentation/pci.txt
On Tue, Dec 05, 2006 at 11:26:51PM -0800, Greg KH wrote:
> > If this looks good, I'll post a diff for gregkh.
>
> This looks very good, thanks for doing this work.
welcome - feels like the most significant contribution I've made
to kernel in a long time.
> I do have a few minor comments:
I'll fix up the comments and post a diff in a few days.
Try to answer your questions below.
...
> > 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.
I'll review all the callbacks and verify we aren't missing any too.
Definitely want to update this. I'll add them but ask someone
else whose used those entry points rewrite my crap with a
seperate patch. I don't want to rathole on this patch.
...
> > 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?
Yes. Will add that.
> > 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 :)
ditto.
> > 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 (),
> > oclass and classmask fields default to 0
> > odriver_data defaults to 0UL.
>
> What's with the "o"s here?
Those are supposed to be bullets.
Cute Story: I lost the first blank space on _every_ line when I messed up
my attempt to remove trailing whitespace in vi with ":1,$ s/ *//".
I left out the "$". :(
I didn't realize the mistake until a few minutes later and had to
hand edit every line to fix it up again. *ouch*
Lesson is to be more careful and save state before making global changes.
>
> > 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 it's removed, the patch can remove that paragraph.
This is a seperate issue.
...
> > 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...
Yeah. I'll drop the "is not a hotplug driver" bullet item.
> > 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...
Ok. fgrep found 321 instances of __devexit_p and thus I think we should
document it's use at a minimum. I'll just add "Most PCI drivers don't need
to use __devexit."
thanks for the comments!
Expect the next rev this weekend sometime.
grant
-
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
Re: [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_tablePointer 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 devic
