Skip to contentSkip to conversation

Documentation improvements

PATCH Superseded Rolf Eike Beer 2 participants 6 messages Jul 12, 2006 – Jul 13, 2006

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover Documentation improvements

Hi,

I'm currently trying to get together some kernel interface to implement a device driver. These are two candidates where function documentation was either nonexistent or wrong.

The changes reflect what I found out reading the code, so I might miss something important.

Eike

#1 lore raw

PATCH 1/2 add function documentation for register_chrdev()

Documentation for register_chrdev() was missing completely.

---
commit 7471539cb5e9cdd7ca7e48a247e15797d0e53708
tree 6ecd75500615ae9a975835596afb820b0fa51786
parent e73ad26773b26c730c49f8ef3b00b10b8bcc0009
author Rolf Eike Beer <[email protected]> Tue, 11 Jul 2006 15:29:09 +0200 committer Rolf Eike Beer <[email protected]> Tue, 11 Jul 2006 15:29:09 +0200

 fs/char_dev.c |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)
fs/char_dev.c +23 -0
index a4cbc67..ac28eaa 100644
@@ -182,6 +182,29 @@ int alloc_chrdev_region(dev_t *dev, unsi
 	return 0;
 }
 
+/*
+ * Register a major number for character devices.
+ *
+ * major: major device number or 0 for dynamic allocation
+ * name: name of this range of devices
+ * fops: file operations associated with this devices
+ *
+ * If major == 0 this functions will dynamically allocate a major and return
+ * its number.
+ *
+ * If major > 0 this function will attempt to reserve a device with the given
+ * major number and will return zero on success.
+ *
+ * Returns a -ve errno on failure.
+ *
+ * The name of this device has nothing to do with the name of the device in
+ * /dev. It only helps to keep track of the different owners of devices. If
+ * your module name has only one type of devices it's ok to use e.g. the name
+ * of the module here.
+ *
+ * This function registers a range of 256 minor numbers. The first minor number
+ * is 0.
+ */
 int register_chrdev(unsigned int major, const char *name,
 		    const struct file_operations *fops)
 {
#2 lore raw

Re: [PATCH 1/2] add function documentation for register_chrdev()

Quoted 1 line · On Wed, 12 Jul 2006 09:46:16 +0200 Rolf Eike Beer wrote:

Documentation for register_chrdev() was missing completely.

I noticed that too.

Would you modify the patch to use the kernel-doc format, please?

See Documentation/kernel-doc-nano-HOWTO.txt or other source files for examples.

and "-ve" is overused/overrated. :) Please use "negative". (even though Andrew just used -ve yesterday)

Quoted 30 lines
---
 fs/char_dev.c |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)
fs/char_dev.c +16 -0
index a4cbc67..ac28eaa 100644
@@ -182,6 +182,29 @@ int alloc_chrdev_region(dev_t *dev, unsi
 	return 0;
 }
 
+/*
+ * Register a major number for character devices.
+ *
+ * major: major device number or 0 for dynamic allocation
+ * name: name of this range of devices
+ * fops: file operations associated with this devices
+ *
+ * If major == 0 this functions will dynamically allocate a major and return
+ * its number.
+ *
+ * If major > 0 this function will attempt to reserve a device with the given
+ * major number and will return zero on success.
+ *
+ * Returns a -ve errno on failure.
+ *
+ * The name of this device has nothing to do with the name of the device in

fs/char_dev.c int alloc_chrdev_region(dev_t *dev, unsi · line 210

Quoted 10 lines
fs/char_dev.c +7 -0
+ * /dev. It only helps to keep track of the different owners of devices. If
+ * your module name has only one type of devices it's ok to use e.g. the name
+ * of the module here.
+ *
+ * This function registers a range of 256 minor numbers. The first minor number
+ * is 0.
+ */
 int register_chrdev(unsigned int major, const char *name,
 		    const struct file_operations *fops)
 {

---
~Randy

#3 lore raw

Re: [PATCH 1/2] add function documentation for register_chrdev()

Quoted 6 lines · Randy.Dunlap
Quoted 1 line · On Wed, 12 Jul 2006 09:46:16 +0200 Rolf Eike Beer wrote:

Documentation for register_chrdev() was missing completely.

I noticed that too.

Would you modify the patch to use the kernel-doc format, please?

Eeks, damn. Sorry, it was meant to be this way.

Quoted 2 lines

and "-ve" is overused/overrated. :) Please use "negative". (even though Andrew just used -ve yesterday)

Just copied that from some other function in that file :)

Will resend soon.

Eike

#4 lore raw

PATCH v2 1/2 add function documentation for register_chrdev()

In reply to Rolf Eike Beer

Documentation for register_chrdev() was missing completely.

---
commit 7471539cb5e9cdd7ca7e48a247e15797d0e53708
tree 6ecd75500615ae9a975835596afb820b0fa51786
parent e73ad26773b26c730c49f8ef3b00b10b8bcc0009
author Rolf Eike Beer <[email protected]> Tue, 11 Jul 2006 15:29:09 +0200 committer Rolf Eike Beer <[email protected]> Tue, 11 Jul 2006 15:29:09 +0200

 fs/char_dev.c |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)
fs/char_dev.c +23 -0
index a4cbc67..ac28eaa 100644
@@ -182,6 +182,29 @@ int alloc_chrdev_region(dev_t *dev, unsi
 	return 0;
 }
 
+/*
+ * register_chrdev: register a major number for character devices.
+ *
+ * @major: major device number or 0 for dynamic allocation
+ * @name: name of this range of devices
+ * @fops: file operations associated with this devices
+ *
+ * If major == 0 this functions will dynamically allocate a major and return
+ * its number.
+ *
+ * If major > 0 this function will attempt to reserve a device with the given
+ * major number and will return zero on success.
+ *
+ * Returns negative error code on failure.
+ *
+ * The name of this device has nothing to do with the name of the device in
+ * /dev. It only helps to keep track of the different owners of devices. If
+ * your module name has only one type of devices it's ok to use e.g. the name
+ * of the module here.
+ *
+ * This function registers a range of 256 minor numbers. The first minor number
+ * is 0.
+ */
 int register_chrdev(unsigned int major, const char *name,
 		    const struct file_operations *fops)
 {
#5 lore raw

PATCH 2/2 Remove pci_dac_set_dma_mask() from Documentation/DMA-mapping.txt

In reply to Rolf Eike Beer

pci_dac_set_dma_mask() gives only a single match in the whole kernel tree and that's in this doc file. The best candidate for replacement is pci_dac_dma_supported().

---
commit 609c9a2c7a61f6fab9cb8507bb01e8bb8fa9f183
tree 6fccc01d3ac08c9a4bcec67a7ff430aa368b68c2
parent 7471539cb5e9cdd7ca7e48a247e15797d0e53708
author Rolf Eike Beer <[email protected]> Wed, 12 Jul 2006 09:36:05 +0200 committer Rolf Eike Beer <[email protected]> Wed, 12 Jul 2006 09:36:05 +0200

 Documentation/DMA-mapping.txt |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
Documentation/DMA-mapping.txt +4 -4
index 7c71769..63392c9 100644
@@ -698,12 +698,12 @@ these interfaces.  Remember that, as def
 always going to be SAC addressable.
 
 The first thing your driver needs to do is query the PCI platform
-layer with your devices DAC addressing capabilities:
+layer if it is capable of handling your devices DAC addressing
+capabilities:
 
-	int pci_dac_set_dma_mask(struct pci_dev *pdev, u64 mask);
+	int pci_dac_dma_supported(struct pci_dev *hwdev, u64 mask);
 
-This routine behaves identically to pci_set_dma_mask.  You may not
-use the following interfaces if this routine fails.
+You may not use the following interfaces if this routine fails.
 
 Next, DMA addresses using this API are kept track of using the
 dma64_addr_t type.  It is guaranteed to be big enough to hold any