Linux Device Drivers, 2nd Edition by Alessandro Rubini & Jonathan Corbet This errata page lists errors outstanding in the most recent printing. If you have technical questions or error reports, you can send them to booktech@oreilly.com. Please specify the printing date of your copy. This page was last modified on November 5, 2004. Here's a key to the markup: [page-number]: serious technical mistake {page-number}: minor technical mistake : important language/formatting problem (page-number): language change or minor formatting problem ?page-number?: reader question or request for clarification {xv} last paragraph; Original href to http://www.linuxdoc.org has now become out of date. The new href is http://www.tldp.org (The Linux Documentation Project) (27) the line in Rules.make file; ifeq ($(KERNELDIR)/.config,$(wildcard $(KERNELDIR))/.config) Should be: ^ ifeq ($(KERNELDIR)/.config,$(wildcard $(KERNELDIR)/.config)) ^ {32} in init_module(void); Currently the first if statement in the functions reads: if(!item2 || !item2) Should be changed to if(!item1 || !item2) [88] The first example of code ; In the second edition of yours "Linux Device Drivers" book there is an example of supporting devfs in the module code. The excerpt from ldd2-samples-1.0.1/scull/main.c (the int scull_init_module(void)) looks like: #ifdef CONFIG_DEVFS_FS sprintf(devname, "%i", i); devfs_register(scull_devfs_dir, devname, DEVFS_FL_AUTO_DEVNUM, 0, 0, S_IFCHR | S_IRUGO | S_IWUGO, &scull_fops, scull_devices+i); #endif But the devfs_register doesn't know the format of the structure Scull_Dev, so it can't initialize the member 'handle' in it ( (scull_devices+i)->handle), so it remains uninitialized. In the scull_cleanup_module(void) there is explicit use of an uninitialized handle (due to the fact scull_devices members are initialized by zeroes this just do nothing): devfs_unregister(scull_devices[i].handle); So it means, the scull_devices[i].handle __must__ be initialized correctly to unregister a device. So the "fix" is just like: #ifdef CONFIG_DEVFS_FS sprintf(devname, "%i", i); scull_devices[i].handle = devfs_register(scull_devfs_dir, devname, DEVFS_FL_AUTO_DEVNUM, 0, 0, S_IFCHR | S_IRUGO | S_IWUGO, &scull_fops, scull_devices+i); #endif (94) 1st paragraph, last line.; SET_FILE_OWNER should be SET_MODULE_OWNER. (151) Last Paragraph; Last paragraph of Pg 151 : 'The if statement that follows interruptible_sleep_on takes care of signal handling.' should be: 'The if statement that follows wait_event_interruptible takes care of signal handling.' (206) Description of DECLARE_TASK_QUEUE.; It should read something like: This macro declares a new task queue named variablename and initializes it. (231) Second line from the bottom.; "or" should be "of" [247] Middle of page, code for probing expansion ROM.; add += (size & ~2048) - 2048; /* skip it */ should be add += (size + 2047) & ~2047; /* skip it */ {287} Second-to-last paragraph, second sentence; The text states "The current state is set to TASK_RUNNING to reflect the fact that we are no longer asleep; this is necessary because if we exited the loop without ever sleeping, we may still be in TASK_INTERRUPTIBLE." it should say: "The current state is set to TASK_RUNNING to reflect the fact that we are no longer asleep; this is necessary because when we exit the loop current->state will be TASK_INTERRUPTIBLE. (Although after schedule() returns the process is always in TASK_RUNNING). {282} In the second edition (June 2001) of your book, Linux Device Drivers (ISBN 0-596-00008-1), on page 282 under the description of "spin_trylock" I see: "To attempt to acquire a lock without waiting, use spin_trylock, which returns nonzero if the operation failed (the lock was busy)." This is, in fact, false. The return value is zero when the operation fails and nonzero when it succeeds. {286} it states that atomic_inc_and_test (and other functions) return the "previous" value of the atomic data type. In the 2.4 source code, the comments claim that the value returned is either TRUE if the "new" value of the data type is zero, or FALSE otherwise. (Which makes these routines fairly useless, but...) [358] Paragraph 2 and following 2 lines of code.; Seems this interface changed early in 2.4. From genhd.c: /* * Global kernel list of partitioning information. * * XXX: you should _never_ access this directly. * the only reason this is exported is source compatiblity. */ /*static*/ struct gendisk *gendisk_head; static struct gendisk *gendisk_array[MAX_BLKDEV]; static rwlock_t gendisk_lock = RW_LOCK_UNLOCKED; /** * add_gendisk - add partitioning information to kernel list * @gp: per-device partitioning information * * This function registers the partitioning information in @gp * with the kernel. */ void add_gendisk(struct gendisk *gp) If you want to do this by hand, you should be holding gendisk_lock. AUTHOR: The author is right that the add_gendisk() and del_gendisk() functions were added after the book came out - in 2.4.10, actually. I have no objection to adding them to the errata so that people know about them. The method described in the book still works - for 2.4 - however. It all breaks in 2.5, of course. (390) 1st paragraph; last line of example -- remap_page_range(vma_->vm_start... should be remap_page_range(vma->vm_start... {407} Prototype of pci_free_consistent(); The type of the last argument should be dma_addr_t. AUTHOR: Right. Same applies to page 409 and the two entries in the reference at the end of the chapter. [434] 2nd paragraph; kfree( snull_devs[i].priv) is called before unregister_netdev(). But unregister_netdev is later calling the snull_stats() function once again that accesses snull[devs].priv. This may lead to a segmentation fault. Detected on uClinux-2.4.20 AUTHOR: The solution is to reverse the two lines in snull_cleanup: void snull_cleanup(void) { int i; for (i=0; i<2; i++) { unregister_netdev(snull_devs + i); kfree(snull_devs[i].priv); } return; } (453) 3rd paragraph; ether_type_trans should be eth_type_trans (455) last paragraph; dev->addr should be dev->dev_addr {475} 2nd paragraph, line 1.; "address" Would be clearer as: "configuration" {481} 2nd code sample; unsigned char jail_get_revision(unsigned char bus, unsigned char fn) { unsigned char *revision; pci_read_config_byte(bus, fn, PCI_REVISION_ID, &revision); return revision; } Should be: unsigned char jail_get_revision(struct pci_dev *dev) { unsigned char revision; pci_read_config_byte(dev, PCI_REVISION_ID, &revision); return revision; } {483} First paragraph.; Since pcidata.c is GPLed, it should include MODULE_LICENSE("GPL"); to avoid the taint warning from recent 2.4 kernels. AUTHOR'S NOTE: "Errata" is perhaps the wrong word; MODULE_LICENSE was introduced in 2.4.10, and so came well after the publication of the book. And, of course, the module will work just as well without the license declaration. I have no objection to adding it to the list, though; all modules should have a MODULE_LICENSE line in them now. {488} paragraph 3, line 2; pci_resource_size should be pci_resource_len {504} Middle.; The third argument to the three pci_write_config_*() functions should be integer types, not point\ ers to these types. int pci_write_config_byte (struct pci_dev *dev, int where, u8 *val); int pci_write_config_word (struct pci_dev *dev, int where, u16 *val); int pci_write_config_dword (struct pci_dev *dev, int where, u32 *val); Should be: int pci_write_config_byte (struct pci_dev *dev, int where, u8 val); int pci_write_config_word (struct pci_dev *dev, int where, u16 val); int pci_write_config_dword (struct pci_dev *dev, int where, u32 val);