Re: [PATCH v3] PCI: hotplug: Fix kernel-doc formatting and add missing documentation

[Lukas Wunner <lukas@wunner.de>]

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