Re: [PATCH] libsas: Fix kernel-doc headers
Bart, > Avoid that building with W=1 causes the kernel-doc tool to complain > about function arguments that have not been documented in the libsas > kernel-doc headers. Avoid that the short description starts with a > hyphen by changing "--" into "-" in the first line of the kernel-doc > headers. Applied to 4.17/scsi-queue. Thanks! -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH] libsas: Fix kernel-doc headers
On 22/02/2018 21:49, Bart Van Assche wrote: Avoid that building with W=1 causes the kernel-doc tool to complain about function arguments that have not been documented in the libsas kernel-doc headers. Avoid that the short description starts with a hyphen by changing "--" into "-" in the first line of the kernel-doc headers. Signed-off-by: Bart Van Assche Cc: John Garry Thanks, Reviewed-by: John Garry
[PATCH] libsas: Fix kernel-doc headers
Avoid that building with W=1 causes the kernel-doc tool to complain about function arguments that have not been documented in the libsas kernel-doc headers. Avoid that the short description starts with a hyphen by changing "--" into "-" in the first line of the kernel-doc headers. Signed-off-by: Bart Van Assche Cc: John Garry --- v2: addressed John Garry's feedback drivers/scsi/libsas/sas_ata.c | 2 +- drivers/scsi/libsas/sas_discover.c | 13 +++-- drivers/scsi/libsas/sas_expander.c | 29 +++-- drivers/scsi/libsas/sas_init.c | 2 +- drivers/scsi/libsas/sas_port.c | 5 +++-- 5 files changed, 27 insertions(+), 24 deletions(-) diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c index 2b3637b40dde..0cc1567eacc1 100644 --- a/drivers/scsi/libsas/sas_ata.c +++ b/drivers/scsi/libsas/sas_ata.c @@ -709,7 +709,7 @@ void sas_resume_sata(struct asd_sas_port *port) } /** - * sas_discover_sata -- discover an STP/SATA domain device + * sas_discover_sata - discover an STP/SATA domain device * @dev: pointer to struct domain_device of interest * * Devices directly attached to a HA port, have no parents. All other diff --git a/drivers/scsi/libsas/sas_discover.c b/drivers/scsi/libsas/sas_discover.c index e4fd078e4175..a0fa7ef3a071 100644 --- a/drivers/scsi/libsas/sas_discover.c +++ b/drivers/scsi/libsas/sas_discover.c @@ -55,7 +55,7 @@ void sas_init_dev(struct domain_device *dev) /* -- Domain device discovery -- */ /** - * sas_get_port_device -- Discover devices which caused port creation + * sas_get_port_device - Discover devices which caused port creation * @port: pointer to struct sas_port of interest * * Devices directly attached to a HA port, have no parent. This is @@ -278,8 +278,8 @@ static void sas_resume_devices(struct work_struct *work) } /** - * sas_discover_end_dev -- discover an end device (SSP, etc) - * @end: pointer to domain device of interest + * sas_discover_end_dev - discover an end device (SSP, etc) + * @dev: pointer to domain device of interest * * See comment in sas_discover_sata(). */ @@ -428,8 +428,8 @@ void sas_device_set_phy(struct domain_device *dev, struct sas_port *port) /* -- Discovery and Revalidation -- */ /** - * sas_discover_domain -- discover the domain - * @port: port to the domain of interest + * sas_discover_domain - discover the domain + * @work: work structure embedded in port domain device. * * NOTE: this process _must_ quit (return) as soon as any connection * errors are encountered. Connection recovery is done elsewhere. @@ -572,7 +572,8 @@ int sas_discover_event(struct asd_sas_port *port, enum discover_event ev) } /** - * sas_init_disc -- initialize the discovery struct in the port + * sas_init_disc - initialize the discovery struct in the port + * @disc: port discovery structure * @port: pointer to struct port * * Called when the ports are being initialized. diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c index 6a4f8198b78e..8b7114348def 100644 --- a/drivers/scsi/libsas/sas_expander.c +++ b/drivers/scsi/libsas/sas_expander.c @@ -1170,9 +1170,9 @@ static int sas_check_level_subtractive_boundary(struct domain_device *dev) return 0; } /** - * sas_ex_discover_devices -- discover devices attached to this expander - * dev: pointer to the expander domain device - * single: if you want to do a single phy, else set to -1; + * sas_ex_discover_devices - discover devices attached to this expander + * @dev: pointer to the expander domain device + * @single: if you want to do a single phy, else set to -1; * * Configure this expander for use with its devices and register the * devices of this expander. @@ -1528,10 +1528,11 @@ static int sas_configure_phy(struct domain_device *dev, int phy_id, } /** - * sas_configure_parent -- configure routing table of parent - * parent: parent expander - * child: child expander - * sas_addr: SAS port identifier of device directly attached to child + * sas_configure_parent - configure routing table of parent + * @parent: parent expander + * @child: child expander + * @sas_addr: SAS port identifier of device directly attached to child + * @include: whether or not to include @child in the expander routing table */ static int sas_configure_parent(struct domain_device *parent, struct domain_device *child, @@ -1570,9 +1571,9 @@ static int sas_configure_parent(struct domain_device *parent, } /** - * sas_configure_routing -- configure routing - * dev: expander device - * sas_addr: port identifier of device directly attached to the expander device + * sas_configure_routing - configure routing + * @dev: expander device + * @sas_addr: port identifier of device directly attached to the expander device */ static int sas_configure_routing(struct domain_device *dev, u8 *sas_addr) { @@ -1589,8 +1590,8 @@ static i
Re: [PATCH] libsas: Fix kernel-doc headers
On Wed, 2018-02-14 at 16:29 +, John Garry wrote: > On 13/02/2018 17:49, Bart Van Assche wrote: > > On 02/13/18 02:17, John Garry wrote: > > > On 12/02/2018 18:45, Bart Van Assche wrote: > > > > -/** > > > > +/* > > > > * sas_configure_parent -- configure routing table of parent > > > > - * parent: parent expander > > > > - * child: child expander > > > > - * sas_addr: SAS port identifier of device directly attached to child > > > > + * @parent: parent expander > > > > + * @child: child expander > > > > + * @sas_addr: SAS port identifier of device directly attached to child > > > > > > and no mention of @include here > > > > Also for the 'include' argument, a suggestion of how to document it > > would be welcome. > > To be honest, I am not so fimilar with this specific part of the code. > > As I see, the @include argument is to flag whether we want to setup or > tear down a routing for parent (upstream) expander device. Thanks for the feedback. I will have a look at the SAS spec to see what the meaning of this parameter is. Bart.
Re: [PATCH] libsas: Fix kernel-doc headers
On 13/02/2018 17:49, Bart Van Assche wrote: On 02/13/18 02:17, John Garry wrote: On 12/02/2018 18:45, Bart Van Assche wrote: [ ... ] -/** +/* * sas_init_disc -- initialize the discovery struct in the port * @port: pointer to struct port I wonder why you get no complaint that @disc argument is not mentioned, Hi Bart, Hello John, Since I was not sure how to document the 'disc' argument I changed /** into /* to make the kernel-doc tool skip the sas_init_disc() function. Any suggestion for how to document the 'disc' argument would be welcome. I see now. For this @disc pointer, I would just write "port discovery structure". In fact I think that we could just change the code to accept the @port argument, as the @disc argument is the port->disc. But that's another job. -/** +/* * sas_configure_parent -- configure routing table of parent - * parent: parent expander - * child: child expander - * sas_addr: SAS port identifier of device directly attached to child + * @parent: parent expander + * @child: child expander + * @sas_addr: SAS port identifier of device directly attached to child and no mention of @include here Also for the 'include' argument, a suggestion of how to document it would be welcome. To be honest, I am not so fimilar with this specific part of the code. As I see, the @include argument is to flag whether we want to setup or tear down a routing for parent (upstream) expander device. I am not 100% sure on this so you could stick with making it a non-kerneldoc comments. It is static. /** * sas_revalidate_domain -- revalidate the domain This function name seems incorrect. [ ... ] And I would write "port domain device" Thanks, will fix. -/** +/* If the build does not complain about this for W=1, then should we include it? I ask, as I see many other instances of "/**". I don't mind cleaning them up in a separate patch. The kernel-doc tool only analyzes function headers that start with "/**". Any changes of "/**" into "/*" mean that I did not know how to document the arguments about which the kernel-doc tool complained that documentation was missing. Thanks, Bart. . Regards, John
Re: [PATCH] libsas: Fix kernel-doc headers
On 02/13/18 02:17, John Garry wrote: On 12/02/2018 18:45, Bart Van Assche wrote: [ ... ] -/** +/* * sas_init_disc -- initialize the discovery struct in the port * @port: pointer to struct port I wonder why you get no complaint that @disc argument is not mentioned, Hello John, Since I was not sure how to document the 'disc' argument I changed /** into /* to make the kernel-doc tool skip the sas_init_disc() function. Any suggestion for how to document the 'disc' argument would be welcome. -/** +/* * sas_configure_parent -- configure routing table of parent - * parent: parent expander - * child: child expander - * sas_addr: SAS port identifier of device directly attached to child + * @parent: parent expander + * @child: child expander + * @sas_addr: SAS port identifier of device directly attached to child and no mention of @include here Also for the 'include' argument, a suggestion of how to document it would be welcome. /** * sas_revalidate_domain -- revalidate the domain This function name seems incorrect. [ ... ] And I would write "port domain device" Thanks, will fix. -/** +/* If the build does not complain about this for W=1, then should we include it? I ask, as I see many other instances of "/**". I don't mind cleaning them up in a separate patch. The kernel-doc tool only analyzes function headers that start with "/**". Any changes of "/**" into "/*" mean that I did not know how to document the arguments about which the kernel-doc tool complained that documentation was missing. Thanks, Bart.
Re: [PATCH] libsas: Fix kernel-doc headers
On 12/02/2018 18:45, Bart Van Assche wrote: Avoid that building with W=1 causes the kernel-doc tool to complain about the libsas kernel-doc headers. Hi Bart, A few comments, below: Signed-off-by: Bart Van Assche --- drivers/scsi/libsas/sas_discover.c | 6 +++--- drivers/scsi/libsas/sas_expander.c | 20 ++-- drivers/scsi/libsas/sas_init.c | 2 +- drivers/scsi/libsas/sas_port.c | 1 + 4 files changed, 15 insertions(+), 14 deletions(-) diff --git a/drivers/scsi/libsas/sas_discover.c b/drivers/scsi/libsas/sas_discover.c index e4fd078e4175..8a184c7f5f56 100644 --- a/drivers/scsi/libsas/sas_discover.c +++ b/drivers/scsi/libsas/sas_discover.c @@ -279,7 +279,7 @@ static void sas_resume_devices(struct work_struct *work) /** * sas_discover_end_dev -- discover an end device (SSP, etc) - * @end: pointer to domain device of interest + * @dev: pointer to domain device of interest * * See comment in sas_discover_sata(). */ @@ -429,7 +429,7 @@ void sas_device_set_phy(struct domain_device *dev, struct sas_port *port) /** * sas_discover_domain -- discover the domain - * @port: port to the domain of interest + * @work: work structure embedded in port to the domain of interest * * NOTE: this process _must_ quit (return) as soon as any connection * errors are encountered. Connection recovery is done elsewhere. @@ -571,7 +571,7 @@ int sas_discover_event(struct asd_sas_port *port, enum discover_event ev) return 0; } -/** +/* * sas_init_disc -- initialize the discovery struct in the port * @port: pointer to struct port I wonder why you get no complaint that @disc argument is not mentioned, here's the code: /** * sas_init_disc -- initialize the discovery struct in the port * @port: pointer to struct port * * Called when the ports are being initialized. */ void sas_init_disc(struct sas_discovery *disc, struct asd_sas_port *port) { * diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c index 6a4f8198b78e..6e7a128619f4 100644 --- a/drivers/scsi/libsas/sas_expander.c +++ b/drivers/scsi/libsas/sas_expander.c @@ -1171,8 +1171,8 @@ static int sas_check_level_subtractive_boundary(struct domain_device *dev) } /** * sas_ex_discover_devices -- discover devices attached to this expander - * dev: pointer to the expander domain device - * single: if you want to do a single phy, else set to -1; + * @dev: pointer to the expander domain device + * @single: if you want to do a single phy, else set to -1; * * Configure this expander for use with its devices and register the * devices of this expander. @@ -1527,11 +1527,11 @@ static int sas_configure_phy(struct domain_device *dev, int phy_id, return res; } -/** +/* * sas_configure_parent -- configure routing table of parent - * parent: parent expander - * child: child expander - * sas_addr: SAS port identifier of device directly attached to child + * @parent: parent expander + * @child: child expander + * @sas_addr: SAS port identifier of device directly attached to child and no mention of @include here */ static int sas_configure_parent(struct domain_device *parent, struct domain_device *child, @@ -1571,8 +1571,8 @@ static int sas_configure_parent(struct domain_device *parent, /** * sas_configure_routing -- configure routing - * dev: expander device - * sas_addr: port identifier of device directly attached to the expander device + * @dev: expander device + * @sas_addr: port identifier of device directly attached to the expander device */ static int sas_configure_routing(struct domain_device *dev, u8 *sas_addr) { @@ -1590,7 +1590,7 @@ static int sas_disable_routing(struct domain_device *dev, u8 *sas_addr) /** * sas_discover_expander -- expander discovery - * @ex: pointer to expander domain device + * @dev: pointer to expander domain device * * See comment in sas_discover_sata(). */ @@ -2112,7 +2112,7 @@ static int sas_rediscover(struct domain_device *dev, const int phy_id) /** * sas_revalidate_domain -- revalidate the domain This function name seems incorrect. Here is the code as I see it on linux-next: /** * sas_revalidate_domain -- revalidate the domain * @port: port to the domain of interest * * NOTE: this process _must_ quit (return) as soon as any connection * errors are encountered. Connection recovery is done elsewhere. * Discover process only interrogates devices in order to discover the * domain. */ int sas_ex_revalidate_domain(struct domain_device *port_dev) { And I would write "port domain device" - * @port: port to the domain of interest + * @port_dev: port to the domain of interest * * NOTE: this process _must_ quit (return) as soon as any connection * errors are encountered. Connection recovery is done elsewhere. diff --git a/drivers/scsi/libsas/sas_init.c b/drivers/scsi/libsas/sas_init.c index c81a63b5dc71..ede0af78144f 100644 --- a/drivers/scsi/libsas/sa
[PATCH] libsas: Fix kernel-doc headers
Avoid that building with W=1 causes the kernel-doc tool to complain about the libsas kernel-doc headers. Signed-off-by: Bart Van Assche --- drivers/scsi/libsas/sas_discover.c | 6 +++--- drivers/scsi/libsas/sas_expander.c | 20 ++-- drivers/scsi/libsas/sas_init.c | 2 +- drivers/scsi/libsas/sas_port.c | 1 + 4 files changed, 15 insertions(+), 14 deletions(-) diff --git a/drivers/scsi/libsas/sas_discover.c b/drivers/scsi/libsas/sas_discover.c index e4fd078e4175..8a184c7f5f56 100644 --- a/drivers/scsi/libsas/sas_discover.c +++ b/drivers/scsi/libsas/sas_discover.c @@ -279,7 +279,7 @@ static void sas_resume_devices(struct work_struct *work) /** * sas_discover_end_dev -- discover an end device (SSP, etc) - * @end: pointer to domain device of interest + * @dev: pointer to domain device of interest * * See comment in sas_discover_sata(). */ @@ -429,7 +429,7 @@ void sas_device_set_phy(struct domain_device *dev, struct sas_port *port) /** * sas_discover_domain -- discover the domain - * @port: port to the domain of interest + * @work: work structure embedded in port to the domain of interest * * NOTE: this process _must_ quit (return) as soon as any connection * errors are encountered. Connection recovery is done elsewhere. @@ -571,7 +571,7 @@ int sas_discover_event(struct asd_sas_port *port, enum discover_event ev) return 0; } -/** +/* * sas_init_disc -- initialize the discovery struct in the port * @port: pointer to struct port * diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c index 6a4f8198b78e..6e7a128619f4 100644 --- a/drivers/scsi/libsas/sas_expander.c +++ b/drivers/scsi/libsas/sas_expander.c @@ -1171,8 +1171,8 @@ static int sas_check_level_subtractive_boundary(struct domain_device *dev) } /** * sas_ex_discover_devices -- discover devices attached to this expander - * dev: pointer to the expander domain device - * single: if you want to do a single phy, else set to -1; + * @dev: pointer to the expander domain device + * @single: if you want to do a single phy, else set to -1; * * Configure this expander for use with its devices and register the * devices of this expander. @@ -1527,11 +1527,11 @@ static int sas_configure_phy(struct domain_device *dev, int phy_id, return res; } -/** +/* * sas_configure_parent -- configure routing table of parent - * parent: parent expander - * child: child expander - * sas_addr: SAS port identifier of device directly attached to child + * @parent: parent expander + * @child: child expander + * @sas_addr: SAS port identifier of device directly attached to child */ static int sas_configure_parent(struct domain_device *parent, struct domain_device *child, @@ -1571,8 +1571,8 @@ static int sas_configure_parent(struct domain_device *parent, /** * sas_configure_routing -- configure routing - * dev: expander device - * sas_addr: port identifier of device directly attached to the expander device + * @dev: expander device + * @sas_addr: port identifier of device directly attached to the expander device */ static int sas_configure_routing(struct domain_device *dev, u8 *sas_addr) { @@ -1590,7 +1590,7 @@ static int sas_disable_routing(struct domain_device *dev, u8 *sas_addr) /** * sas_discover_expander -- expander discovery - * @ex: pointer to expander domain device + * @dev: pointer to expander domain device * * See comment in sas_discover_sata(). */ @@ -2112,7 +2112,7 @@ static int sas_rediscover(struct domain_device *dev, const int phy_id) /** * sas_revalidate_domain -- revalidate the domain - * @port: port to the domain of interest + * @port_dev: port to the domain of interest * * NOTE: this process _must_ quit (return) as soon as any connection * errors are encountered. Connection recovery is done elsewhere. diff --git a/drivers/scsi/libsas/sas_init.c b/drivers/scsi/libsas/sas_init.c index c81a63b5dc71..ede0af78144f 100644 --- a/drivers/scsi/libsas/sas_init.c +++ b/drivers/scsi/libsas/sas_init.c @@ -234,7 +234,7 @@ int sas_try_ata_reset(struct asd_sas_phy *asd_phy) return -ENODEV; } -/** +/* * transport_sas_phy_reset - reset a phy and permit libata to manage the link * * phy reset request via sysfs in host workqueue context so we know we diff --git a/drivers/scsi/libsas/sas_port.c b/drivers/scsi/libsas/sas_port.c index f07e55d3aa73..7cd7b9f3f25e 100644 --- a/drivers/scsi/libsas/sas_port.c +++ b/drivers/scsi/libsas/sas_port.c @@ -199,6 +199,7 @@ static void sas_form_port(struct asd_sas_phy *phy) /** * sas_deform_port -- remove this phy from the port it belongs to * @phy: the phy of interest + * @gone: whether or not the port is gone * * This is called when the physical link to the other phy has been * lost (on this phy), in Event thread context. We cannot delay here. -- 2.16.1