* [PATCH v3] PCI: hotplug: Fix kernel-doc formatting and add missing documentation
@ 2021-07-02 23:15 Krzysztof Wilczyński
2021-07-03 6:48 ` Lukas Wunner
0 siblings, 1 reply; 3+ messages in thread
From: Krzysztof Wilczyński @ 2021-07-02 23:15 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Scott Murray, linux-pci
Fix kernel-doc formatting and add missing documentation for the
parameters "bus" of the get_slot_mapping() function, "t" of the
cpqhp_pushbutton_thread() function and "controller" of the
inband_presence_disabled() function.
Additionally, fix formatting and add missing documentation for the
members "slot_list" and "pci_slot" of the struct hotplug_slot.
Also remove surplus parameter "slot" from the description of the
function cpqhp_pushbutton_thread().
Thus, resolve build time warnings related to kernel-doc:
drivers/pci/hotplug/cpqphp_core.c:308: warning: Function parameter or member 'bus' not described in 'get_slot_mapping'
drivers/pci/hotplug/cpqphp_core.c:308: warning: Function parameter or member 'bus_num' not described in 'get_slot_mapping'
drivers/pci/hotplug/cpqphp_core.c:308: warning: Function parameter or member 'dev_num' not described in 'get_slot_mapping'
drivers/pci/hotplug/cpqphp_core.c:308: warning: Function parameter or member 'slot' not described in 'get_slot_mapping'
drivers/pci/hotplug/cpqphp_ctrl.c:1887: warning: Function parameter or member 't' not described in 'cpqhp_pushbutton_thread'
drivers/pci/hotplug/cpqphp_ctrl.c:1887: warning: Excess function parameter 'slot' description in 'cpqhp_pushbutton_thread'
drivers/pci/hotplug/pciehp.h:110: warning: Function parameter or member 'inband_presence_disabled' not described in 'controller'
include/linux/pci_hotplug.h:64: warning: Function parameter or member 'slot_list' not described in 'hotplug_slot'
include/linux/pci_hotplug.h:64: warning: Function parameter or member 'pci_slot' not described in 'hotplug_slot'
No change to functionality intended.
Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
---
Changes in v3:
Added kernel-doc fixes for the file include/linux/pci_hotplug.h.
Changes in v2:
Added kernel-doc fixes for the files drivers/pci/hotplug/cpqphp_ctrl.c
and drivers/pci/hotplug/pciehp.h.
drivers/pci/hotplug/cpqphp_core.c | 16 ++++---
drivers/pci/hotplug/cpqphp_ctrl.c | 4 +-
drivers/pci/hotplug/pciehp.h | 80 ++++++++++++++++++-------------
include/linux/pci_hotplug.h | 12 +++--
4 files changed, 67 insertions(+), 45 deletions(-)
diff --git a/drivers/pci/hotplug/cpqphp_core.c b/drivers/pci/hotplug/cpqphp_core.c
index b8aacb41a83c..efe6031c1e68 100644
--- a/drivers/pci/hotplug/cpqphp_core.c
+++ b/drivers/pci/hotplug/cpqphp_core.c
@@ -292,15 +292,17 @@ static int ctrl_slot_cleanup(struct controller *ctrl)
/**
- * get_slot_mapping - determine logical slot mapping for PCI device
+ * get_slot_mapping - Determine logical slot mapping for PCI device.
+ * @bus: Pointer to the PCI bus structure.
+ * @bus_num: Bus number of the PCI device.
+ * @dev_num: Device number of the PCI device.
+ * @slot: Pointer to where slot number will be returned.
*
- * Won't work for more than one PCI-PCI bridge in a slot.
+ * Will not work for more than one PCI-PCI bridge in a slot.
*
- * @bus_num - bus number of PCI device
- * @dev_num - device number of PCI device
- * @slot - Pointer to u8 where slot number will be returned
- *
- * Output: SUCCESS or FAILURE
+ * Return:
+ * * 0 - Logical slot mapping has been found for this PCI device.
+ * * < 0 - Unable to find entry in the routing table for this PCI device.
*/
static int
get_slot_mapping(struct pci_bus *bus, u8 bus_num, u8 dev_num, u8 *slot)
diff --git a/drivers/pci/hotplug/cpqphp_ctrl.c b/drivers/pci/hotplug/cpqphp_ctrl.c
index 68de958a9be8..035114423ebb 100644
--- a/drivers/pci/hotplug/cpqphp_ctrl.c
+++ b/drivers/pci/hotplug/cpqphp_ctrl.c
@@ -1876,8 +1876,8 @@ static void interrupt_event_handler(struct controller *ctrl)
/**
- * cpqhp_pushbutton_thread - handle pushbutton events
- * @slot: target slot (struct)
+ * cpqhp_pushbutton_thread - Handle pushbutton events.
+ * @t: Pointer to struct timer_list which holds all timer related callbacks.
*
* Scheduled procedure to handle blocking stuff for the pushbuttons.
* Handles all pending events and exits.
diff --git a/drivers/pci/hotplug/pciehp.h b/drivers/pci/hotplug/pciehp.h
index 4fd200d8b0a9..c8b2b1e046e8 100644
--- a/drivers/pci/hotplug/pciehp.h
+++ b/drivers/pci/hotplug/pciehp.h
@@ -44,38 +44,54 @@ extern int pciehp_poll_time;
#define SLOT_NAME_SIZE 10
/**
- * struct controller - PCIe hotplug controller
- * @pcie: pointer to the controller's PCIe port service device
- * @slot_cap: cached copy of the Slot Capabilities register
- * @slot_ctrl: cached copy of the Slot Control register
- * @ctrl_lock: serializes writes to the Slot Control register
- * @cmd_started: jiffies when the Slot Control register was last written;
- * the next write is allowed 1 second later, absent a Command Completed
- * interrupt (PCIe r4.0, sec 6.7.3.2)
- * @cmd_busy: flag set on Slot Control register write, cleared by IRQ handler
- * on reception of a Command Completed event
- * @queue: wait queue to wake up on reception of a Command Completed event,
- * used for synchronous writes to the Slot Control register
- * @pending_events: used by the IRQ handler to save events retrieved from the
- * Slot Status register for later consumption by the IRQ thread
- * @notification_enabled: whether the IRQ was requested successfully
- * @power_fault_detected: whether a power fault was detected by the hardware
- * that has not yet been cleared by the user
- * @poll_thread: thread to poll for slot events if no IRQ is available,
- * enabled with pciehp_poll_mode module parameter
- * @state: current state machine position
- * @state_lock: protects reads and writes of @state;
- * protects scheduling, execution and cancellation of @button_work
- * @button_work: work item to turn the slot on or off after 5 seconds
- * in response to an Attention Button press
- * @hotplug_slot: structure registered with the PCI hotplug core
- * @reset_lock: prevents access to the Data Link Layer Link Active bit in the
- * Link Status register and to the Presence Detect State bit in the Slot
- * Status register during a slot reset which may cause them to flap
- * @ist_running: flag to keep user request waiting while IRQ thread is running
- * @request_result: result of last user request submitted to the IRQ thread
- * @requester: wait queue to wake up on completion of user request,
- * used for synchronous slot enable/disable request via sysfs
+ * struct controller - PCIe hotplug controller.
+ * @pcie: Pointer to the controller's PCIe port service
+ * device.
+ * @slot_cap: Cached copy of the Slot Capabilities register.
+ * @inband_presence_disabled: Flag to used to track whether the in-band
+ * presence detection is disabled.
+ * @slot_ctrl: Cached copy of the Slot Control register.
+ * @ctrl_lock: Serializes writes to the Slot Control register.
+ * @cmd_started: Jiffies when the Slot Control register was last
+ * written; the next write is allowed 1 second
+ * later, absent a Command Completed interrupt
+ * (PCIe r4.0, sec 6.7.3.2).
+ * @cmd_busy: Flag set on Slot Control register write, cleared
+ * by IRQ handler on reception of a Command
+ * Completed event.
+ * @queue: Wait queue to wake up on reception of a Command
+ * Completed event, used for synchronous writes to
+ * the Slot Control register.
+ * @pending_events: Used by the IRQ handler to save events retrieved
+ * from the Slot Status register for later
+ * consumption by the IRQ thread.
+ * @notification_enabled: Whether the IRQ was requested successfully.
+ * @power_fault_detected: Whether a power fault was detected by the
+ * hardware that has not yet been cleared by the
+ * user.
+ * @poll_thread: Thread to poll for slot events if no IRQ is
+ * available, enabled with pciehp_poll_mode module
+ * parameter.
+ * @state: Current state machine position.
+ * @state_lock: Protects reads and writes of @state; protects
+ * scheduling, execution and cancellation of
+ * @button_work.
+ * @button_work: Work item to turn the slot on or off after
+ * 5 seconds in response to an Attention Button
+ * press.
+ * @hotplug_slot: Structure registered with the PCI hotplug core.
+ * @reset_lock: Prevents access to the Data Link Layer Link
+ * Active bit in the Link Status register and to
+ * the Presence Detect State bit in the Slot Status
+ * register during a slot reset which may cause
+ * them to flap.
+ * @ist_running: Flag to keep user request waiting while IRQ
+ * thread is running.
+ * @request_result: Result of last user request submitted to the IRQ
+ * thread.
+ * @requester: Wait queue to wake up on completion of user
+ * request, used for synchronous slot
+ * enable/disable request via sysfs.
*
* PCIe hotplug has a 1:1 relationship between controller and slot, hence
* unlike other drivers, the two aren't represented by separate structures.
diff --git a/include/linux/pci_hotplug.h b/include/linux/pci_hotplug.h
index b482e42d7153..a73851884b81 100644
--- a/include/linux/pci_hotplug.h
+++ b/include/linux/pci_hotplug.h
@@ -48,10 +48,14 @@ struct hotplug_slot_ops {
};
/**
- * struct hotplug_slot - used to register a physical slot with the hotplug pci core
- * @ops: pointer to the &struct hotplug_slot_ops to be used for this slot
- * @owner: The module owner of this structure
- * @mod_name: The module name (KBUILD_MODNAME) of this structure
+ * struct hotplug_slot - Used to register a physical slot with the hotplug PCI
+ * core.
+ * @ops: Pointer to the &struct hotplug_slot_ops to be used for this
+ * slot.
+ * @slot_list: Internal list used to track hotplug PCI slots.
+ * @pci_slot: Pepresents a physical slot.
+ * @owner: The module owner of this structure.
+ * @mod_name: The module name (KBUILD_MODNAME) of this structure.
*/
struct hotplug_slot {
const struct hotplug_slot_ops *ops;
--
2.32.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v3] PCI: hotplug: Fix kernel-doc formatting and add missing documentation
2021-07-02 23:15 [PATCH v3] PCI: hotplug: Fix kernel-doc formatting and add missing documentation Krzysztof Wilczyński
@ 2021-07-03 6:48 ` Lukas Wunner
2021-07-03 8:51 ` Krzysztof Wilczy??ski
0 siblings, 1 reply; 3+ messages in thread
From: Lukas Wunner @ 2021-07-03 6:48 UTC (permalink / raw)
To: Krzysztof Wilczy??ski; +Cc: Bjorn Helgaas, Scott Murray, linux-pci
On Fri, Jul 02, 2021 at 11:15:41PM +0000, Krzysztof Wilczy??ski wrote:
> Fix kernel-doc formatting and add missing documentation for the
[...]
> drivers/pci/hotplug/pciehp.h:110: warning: Function parameter or member 'inband_presence_disabled' not described in 'controller'
[...]
> diff --git a/drivers/pci/hotplug/pciehp.h b/drivers/pci/hotplug/pciehp.h
> index 4fd200d8b0a9..c8b2b1e046e8 100644
> --- a/drivers/pci/hotplug/pciehp.h
> +++ b/drivers/pci/hotplug/pciehp.h
> @@ -44,38 +44,54 @@ extern int pciehp_poll_time;
> #define SLOT_NAME_SIZE 10
>
> /**
> - * struct controller - PCIe hotplug controller
> - * @pcie: pointer to the controller's PCIe port service device
> - * @slot_cap: cached copy of the Slot Capabilities register
> - * @slot_ctrl: cached copy of the Slot Control register
[...]
> + * struct controller - PCIe hotplug controller.
> + * @pcie: Pointer to the controller's PCIe port service
> + * device.
> + * @slot_cap: Cached copy of the Slot Capabilities register.
Is it really necessary to change the indentation and add the trailing
period *everywhere*? What value does that add?
Changes like this make it more difficult to determine the commit from
which a certain line originates.
I respectfully submit that the formatting is fine and there's nothing
to be "fixed" here (as the commit message claims).
> + * @inband_presence_disabled: Flag to used to track whether the in-band
> + * presence detection is disabled.
That's not proper English and also not very useful because the documentation
merely repeats what the flag's name says. I'd suggest something along the
lines of:
* @inband_presence_disabled: whether In-Band Presence Detect Disable is
* supported by the controller and disabled per spec recommendation
* (PCIe r5.0, appendix I implementation note)
Thanks,
Lukas
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v3] PCI: hotplug: Fix kernel-doc formatting and add missing documentation
2021-07-03 6:48 ` Lukas Wunner
@ 2021-07-03 8:51 ` Krzysztof Wilczy??ski
0 siblings, 0 replies; 3+ messages in thread
From: Krzysztof Wilczy??ski @ 2021-07-03 8:51 UTC (permalink / raw)
To: Lukas Wunner; +Cc: Bjorn Helgaas, Scott Murray, linux-pci
Hi Lukas,
Thank you for feedback!
[...]
> I respectfully submit that the formatting is fine and there's nothing
> to be "fixed" here (as the commit message claims).
The fixing it probably more relevant to the two first hunks, everything
else would be more of a style update so that kernel-doc is kept with the
following guideline:
https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html
Which is something other users of kernel-doc seldom embrace as I could
only find a handful of places where this format is somewhat followed.
Thus, like you say, keeping the scope of changes to only updating what
matters would be more appropriate. I will send another revision that
does exactly that.
> > + * @inband_presence_disabled: Flag to used to track whether the in-band
> > + * presence detection is disabled.
>
> That's not proper English and also not very useful because the documentation
> merely repeats what the flag's name says. I'd suggest something along the
> lines of:
> * @inband_presence_disabled: whether In-Band Presence Detect Disable is
> * supported by the controller and disabled per spec recommendation
> * (PCIe r5.0, appendix I implementation note)
Thank you! I will use your version going forward.
Krzysztof
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-07-03 8:51 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-02 23:15 [PATCH v3] PCI: hotplug: Fix kernel-doc formatting and add missing documentation Krzysztof Wilczyński
2021-07-03 6:48 ` Lukas Wunner
2021-07-03 8:51 ` Krzysztof Wilczy??ski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).