Re: [PATCH] libsas: Fix kernel-doc headers

2018-02-27 Thread Martin K. Petersen

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

2018-02-23 Thread John Garry

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

2018-02-22 Thread Bart Van Assche
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

2018-02-22 Thread Bart Van Assche
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

2018-02-14 Thread John Garry

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

2018-02-13 Thread Bart Van Assche

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

2018-02-13 Thread John Garry

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

2018-02-12 Thread Bart Van Assche
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