[05/12] infiniband: fix ulp/opa_vnic/opa_vnic_encap.h kernel-doc notation
diff mbox series

Message ID 20191010035239.890311169@gmail.com
State Accepted
Delegated to: Jason Gunthorpe
Headers show
Series
  • infiniband kernel-doc fixes & driver-api/ chapter
Related show

Commit Message

Randy Dunlap Oct. 10, 2019, 3:52 a.m. UTC
Make reserved struct fields "private:" so that they don't need to
be added to the kernel-doc notation. This removes 24 warnings.

Remove "[]" in one struct field description to that kernel-doc
won't be confused.

../drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:148: warning: Function parameter or member 'rsvd0' not described in 'opa_vesw_info' [5 rsvd members]

../drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:205: warning: Function parameter or member 'rsvd0' not described in 'opa_per_veswport_info' [4 rsvd members]

../drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:342: warning: Function parameter or member 'reserved' not described in 'opa_veswport_summary_counters'

../drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:394: warning: Function parameter or member 'rsvd0' not described in 'opa_veswport_error_counters' [10 rsvd members]

../drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:460: warning: Function parameter or member 'reserved' not described in 'opa_vnic_vema_mad'

../drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:485: warning: Function parameter or member 'reserved' not described in 'opa_vnic_notice_attr'

../drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:500: warning: Function parameter or member 'reserved' not described in 'opa_vnic_vema_mad_trap'

../drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:263: warning: Function parameter or member 'tbl_entries' not described in 'opa_veswport_mactable'

Signed-off-by: Randy Dunlap <rd.dunlab@gmail.com>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: Doug Ledford <dledford@redhat.com>
Cc: Jason Gunthorpe <jgg@mellanox.com>
Cc: linux-doc@vger.kernel.org
---
 drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h |   42 ++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)

Comments

Jason Gunthorpe Oct. 22, 2019, 5:52 p.m. UTC | #1
On Wed, Oct 09, 2019 at 08:52:44PM -0700, rd.dunlab@gmail.com wrote:
> Make reserved struct fields "private:" so that they don't need to
> be added to the kernel-doc notation. This removes 24 warnings.

> +++ linux-next-20191009/drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h
> @@ -129,21 +129,31 @@ struct opa_vesw_info {
>  	__be16  fabric_id;
>  	__be16  vesw_id;
>  
> +	/* private: */
>  	u8      rsvd0[6];
> +	/* public: */
>  	__be16  def_port_mask;

This seems overly ugly, is there some other way to handle these
reserved fields? Maybe wire protocol structures shouldn't be kdoc?

Jason
Randy Dunlap Oct. 22, 2019, 7:51 p.m. UTC | #2
On 10/22/19 10:52 AM, Jason Gunthorpe wrote:
> On Wed, Oct 09, 2019 at 08:52:44PM -0700, rd.dunlab@gmail.com wrote:
>> Make reserved struct fields "private:" so that they don't need to
>> be added to the kernel-doc notation. This removes 24 warnings.
> 
>> +++ linux-next-20191009/drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h
>> @@ -129,21 +129,31 @@ struct opa_vesw_info {
>>  	__be16  fabric_id;
>>  	__be16  vesw_id;
>>  
>> +	/* private: */
>>  	u8      rsvd0[6];
>> +	/* public: */
>>  	__be16  def_port_mask;
> 
> This seems overly ugly, is there some other way to handle these
> reserved fields? Maybe wire protocol structures shouldn't be kdoc?

I don't know of any other way to handle them with kernel-doc.
Sure, changing the /** to just /* would be one way to hide the
warnings.  Either this patch or not having them be kernel-doc
is needed just to "fix" 24 warnings.
Jani Nikula Oct. 23, 2019, 7:26 a.m. UTC | #3
On Tue, 22 Oct 2019, Randy Dunlap <rdunlap@infradead.org> wrote:
> On 10/22/19 10:52 AM, Jason Gunthorpe wrote:
>> On Wed, Oct 09, 2019 at 08:52:44PM -0700, rd.dunlab@gmail.com wrote:
>>> Make reserved struct fields "private:" so that they don't need to
>>> be added to the kernel-doc notation. This removes 24 warnings.
>> 
>>> +++ linux-next-20191009/drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h
>>> @@ -129,21 +129,31 @@ struct opa_vesw_info {
>>>  	__be16  fabric_id;
>>>  	__be16  vesw_id;
>>>  
>>> +	/* private: */
>>>  	u8      rsvd0[6];
>>> +	/* public: */
>>>  	__be16  def_port_mask;
>> 
>> This seems overly ugly, is there some other way to handle these
>> reserved fields? Maybe wire protocol structures shouldn't be kdoc?
>
> I don't know of any other way to handle them with kernel-doc.
> Sure, changing the /** to just /* would be one way to hide the
> warnings.  Either this patch or not having them be kernel-doc
> is needed just to "fix" 24 warnings.

The currently available options are:

- The patch at hand (private/public comments). Ugly and verbose.

- Document the structs using regular comments instead of
  kernel-doc. Might be suitable here, but not a generally useful
  approach. Loses all format checking and generated documentation.

- Also document the reserved fields. Ugly and verbose, also in the
  generated documentation.

Some options that I think might be relatively easy to implement:

- Add struct documentation comment indicator to not complain about
  missing member documentation. Some special tag in the struct
  comment. This would also ignore members that actually need to be
  documented.

- Add support for designating private members in the member
  documentation, i.e. require the documentation, but omit the members
  from the generated document. Something like this, with PRIVATE
  replaced with your favorite bikeshed colors:

  /**
   * @rsvd0: PRIVATE
   */

  This could be used either in the struct documentation comment or in
  the inline member documentation comment. Less ugly than the patch at
  hand, and arguably a better notation, but still requires documenting
  the members.

- Add support for a catch-all member documentation comment, for example:

  /**
   * struct foo - bar
   * @*: This member is private.
   */

  Would generate the documentation for the member with the catch-all
  documentation, which might be a generally useful feature, and would be
  easy on the source code side. This could be combined with the PRIVATE
  designation above, practically leading to the same result as the first
  option but with more flexibility.


BR,
Jani.
Dennis Dalessandro Oct. 23, 2019, 12:03 p.m. UTC | #4
On 10/22/2019 3:51 PM, Randy Dunlap wrote:
> On 10/22/19 10:52 AM, Jason Gunthorpe wrote:
>> On Wed, Oct 09, 2019 at 08:52:44PM -0700, rd.dunlab@gmail.com wrote:
>>> Make reserved struct fields "private:" so that they don't need to
>>> be added to the kernel-doc notation. This removes 24 warnings.
>>
>>> +++ linux-next-20191009/drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h
>>> @@ -129,21 +129,31 @@ struct opa_vesw_info {
>>>   	__be16  fabric_id;
>>>   	__be16  vesw_id;
>>>   
>>> +	/* private: */
>>>   	u8      rsvd0[6];
>>> +	/* public: */
>>>   	__be16  def_port_mask;
>>
>> This seems overly ugly, is there some other way to handle these
>> reserved fields? Maybe wire protocol structures shouldn't be kdoc?
> 
> I don't know of any other way to handle them with kernel-doc.
> Sure, changing the /** to just /* would be one way to hide the
> warnings.  Either this patch or not having them be kernel-doc
> is needed just to "fix" 24 warnings.
> 

I would be in favor of just not including this in kernel-doc at this 
time, but the mess of private/public tags while hard on the eyes doesn't 
really bother me either.

Also wouldn't take this as a statement that wire protocol structures not 
be in kdoc, but just that this one doesn't need to be there.

-Denny

Patch
diff mbox series

--- linux-next-20191009.orig/drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h
+++ linux-next-20191009/drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h
@@ -129,21 +129,31 @@  struct opa_vesw_info {
 	__be16  fabric_id;
 	__be16  vesw_id;
 
+	/* private: */
 	u8      rsvd0[6];
+	/* public: */
 	__be16  def_port_mask;
 
+	/* private: */
 	u8      rsvd1[2];
+	/* public: */
 	__be16  pkey;
 
+	/* private: */
 	u8      rsvd2[4];
+	/* public: */
 	__be32  u_mcast_dlid;
 	__be32  u_ucast_dlid[OPA_VESW_MAX_NUM_DEF_PORT];
 
 	__be32  rc;
 
+	/* private: */
 	u8      rsvd3[56];
+	/* public: */
 	__be16  eth_mtu;
+	/* private: */
 	u8      rsvd4[2];
+	/* public: */
 } __packed;
 
 /**
@@ -172,7 +182,9 @@  struct opa_per_veswport_info {
 	__be32  port_num;
 
 	u8      eth_link_status;
+	/* private: */
 	u8      rsvd0[3];
+	/* public: */
 
 	u8      base_mac_addr[ETH_ALEN];
 	u8      config_state;
@@ -181,7 +193,9 @@  struct opa_per_veswport_info {
 	__be16  max_mac_tbl_ent;
 	__be16  max_smac_ent;
 	__be32  mac_tbl_digest;
+	/* private: */
 	u8      rsvd1[4];
+	/* public: */
 
 	__be32  encap_slid;
 
@@ -195,12 +209,16 @@  struct opa_per_veswport_info {
 	u8      non_vlan_sc_mc;
 	u8      non_vlan_vl_mc;
 
+	/* private: */
 	u8      rsvd2[48];
+	/* public: */
 
 	__be16  uc_macs_gen_count;
 	__be16  mc_macs_gen_count;
 
+	/* private: */
 	u8      rsvd3[8];
+	/* public: */
 } __packed;
 
 /**
@@ -239,7 +257,7 @@  struct opa_veswport_mactable_entry {
  * @offset: mac table starting offset
  * @num_entries: Number of entries to get or set
  * @mac_tbl_digest: mac table digest
- * @tbl_entries[]: Array of table entries
+ * @tbl_entries: Array of table entries
  *
  * The EM sends down this structure in a MAD indicating
  * the starting offset in the forwarding table that this
@@ -337,7 +355,9 @@  struct opa_veswport_summary_counters {
 	__be64  rx_1024_1518;
 	__be64  rx_1519_max;
 
+	/* private: */
 	__be64  reserved[16];
+	/* public: */
 } __packed;
 
 /**
@@ -368,28 +388,42 @@  struct opa_veswport_error_counters {
 	__be64  tx_errors;
 	__be64  rx_errors;
 
+	/* private: */
 	__be64  rsvd0;
+	/* public: */
 	__be64  tx_smac_filt;
+	/* private: */
 	__be64  rsvd1;
 	__be64  rsvd2;
 	__be64  rsvd3;
+	/* public: */
 	__be64  tx_dlid_zero;
+	/* private: */
 	__be64  rsvd4;
+	/* public: */
 	__be64  tx_logic;
+	/* private: */
 	__be64  rsvd5;
+	/* public: */
 	__be64  tx_drop_state;
 
 	__be64  rx_bad_veswid;
+	/* private: */
 	__be64  rsvd6;
+	/* public: */
 	__be64  rx_runt;
 	__be64  rx_oversize;
+	/* private: */
 	__be64  rsvd7;
+	/* public: */
 	__be64  rx_eth_down;
 	__be64  rx_drop_state;
 	__be64  rx_logic;
+	/* private: */
 	__be64  rsvd8;
 
 	__be64  rsvd9[16];
+	/* public: */
 } __packed;
 
 /**
@@ -453,7 +487,9 @@  struct opa_veswport_iface_macs {
 struct opa_vnic_vema_mad {
 	struct ib_mad_hdr  mad_hdr;
 	struct ib_rmpp_hdr rmpp_hdr;
+	/* private: */
 	u8                 reserved;
+	/* public: */
 	u8                 oui[3];
 	u8                 data[OPA_VNIC_EMA_DATA];
 };
@@ -478,7 +514,9 @@  struct opa_vnic_notice_attr {
 	__be16 trap_num;
 	__be16 toggle_count;
 	__be32 issuer_lid;
+	/* private: */
 	__be32 reserved;
+	/* public: */
 	u8     issuer_gid[16];
 	u8     raw_data[64];
 } __packed;
@@ -493,7 +531,9 @@  struct opa_vnic_notice_attr {
 struct opa_vnic_vema_mad_trap {
 	struct ib_mad_hdr            mad_hdr;
 	struct ib_rmpp_hdr           rmpp_hdr;
+	/* private: */
 	u8                           reserved;
+	/* public: */
 	u8                           oui[3];
 	struct opa_vnic_notice_attr  notice;
 };