[PATCH net-next] sfc: correct kernel-doc complaints

[PATCH net-next] sfc: correct kernel-doc complaints

[Randy Dunlap]

Fix kernel-doc warnings by adding 3 missing struct member descriptions
in struct efx_ef10_nic_data and removing preprocessor directives (which
are not handled by kernel-doc).

Fixes these 5 warnings:
Warning: drivers/net/ethernet/sfc/nic.h:158 bad line: #ifdef CONFIG_SFC_SRIOV
Warning: drivers/net/ethernet/sfc/nic.h:160 bad line: #endif
Warning: drivers/net/ethernet/sfc/nic.h:204 struct member 'port_id'
 not described in 'efx_ef10_nic_data'
Warning: drivers/net/ethernet/sfc/nic.h:204 struct member 'vf_index'
 not described in 'efx_ef10_nic_data'
Warning: drivers/net/ethernet/sfc/nic.h:204 struct member 'licensed_features'
 not described in 'efx_ef10_nic_data'

Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
---
Cc: Edward Cree <ecree.xilinx@gmail.com>
Cc: Andrew Lunn <andrew+netdev@lunn.ch>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Simon Horman <horms@kernel.org>
Cc: linux-net-drivers@amd.com
---
 drivers/net/ethernet/sfc/nic.h |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

--- linux-next-20251128.orig/drivers/net/ethernet/sfc/nic.h
+++ linux-next-20251128/drivers/net/ethernet/sfc/nic.h
@@ -156,9 +156,10 @@ enum {
  * @tx_dpcpu_fw_id: Firmware ID of the TxDPCPU
  * @must_probe_vswitching: Flag: vswitching has yet to be setup after MC reboot
  * @pf_index: The number for this PF, or the parent PF if this is a VF
-#ifdef CONFIG_SFC_SRIOV
+ * @port_id: port id (Ethernet address) if !CONFIG_SFC_SRIOV;
+ *   for CONFIG_SFC_SRIOV, the VF port id
+ * @vf_index: Index of particular VF in the VF data structure
  * @vf: Pointer to VF data structure
-#endif
  * @vport_mac: The MAC address on the vport, only for PFs; VFs will be zero
  * @vlan_list: List of VLANs added over the interface. Serialised by vlan_lock.
  * @vlan_lock: Lock to serialize access to vlan_list.
@@ -166,6 +167,7 @@ enum {
  * @udp_tunnels_dirty: flag indicating a reboot occurred while pushing
  *	@udp_tunnels to hardware and thus the push must be re-done.
  * @udp_tunnels_lock: Serialises writes to @udp_tunnels and @udp_tunnels_dirty.
+ * @licensed_features: used to enable features if the adapter is licensed for it
  */
 struct efx_ef10_nic_data {
 	struct efx_buffer mcdi_buf;

Re: [PATCH net-next] sfc: correct kernel-doc complaints

[Jakub Kicinski]

On Sat, 29 Nov 2025 14:03:51 -0800 Randy Dunlap wrote:
> + * @port_id: port id (Ethernet address) if !CONFIG_SFC_SRIOV;
> + *   for CONFIG_SFC_SRIOV, the VF port id

Thanks for the cleanup, but TBH this one sounds a little bit awkward.
Hopefully the SFC / AMD folks can suggest better wording. I'm dropping
this version from PW, I'm trying to wrap up netdev for 6.19..
-- 
pw-bot: cr

Re: [PATCH net-next] sfc: correct kernel-doc complaints

[Edward Cree]

On 29/11/2025 22:03, Randy Dunlap wrote:
> Fix kernel-doc warnings by adding 3 missing struct member descriptions
> in struct efx_ef10_nic_data and removing preprocessor directives (which
> are not handled by kernel-doc).
> 
> Fixes these 5 warnings:
> Warning: drivers/net/ethernet/sfc/nic.h:158 bad line: #ifdef CONFIG_SFC_SRIOV
> Warning: drivers/net/ethernet/sfc/nic.h:160 bad line: #endif

Does kernel-doc not complain if a member is documented but the actual
 declaration is ifdefed out?  Normal practice seems to be to move the
 doc into another comment adjacent to the declaration so it's under
 the same ifdef; is that unnecessary?

> Warning: drivers/net/ethernet/sfc/nic.h:204 struct member 'port_id'
>  not described in 'efx_ef10_nic_data'
> Warning: drivers/net/ethernet/sfc/nic.h:204 struct member 'vf_index'
>  not described in 'efx_ef10_nic_data'
> Warning: drivers/net/ethernet/sfc/nic.h:204 struct member 'licensed_features'
>  not described in 'efx_ef10_nic_data'
> 
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> ---
> Cc: Edward Cree <ecree.xilinx@gmail.com>
> Cc: Andrew Lunn <andrew+netdev@lunn.ch>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Paolo Abeni <pabeni@redhat.com>
> Cc: Simon Horman <horms@kernel.org>
> Cc: linux-net-drivers@amd.com
> ---
>  drivers/net/ethernet/sfc/nic.h |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> --- linux-next-20251128.orig/drivers/net/ethernet/sfc/nic.h
> +++ linux-next-20251128/drivers/net/ethernet/sfc/nic.h
> @@ -156,9 +156,10 @@ enum {
>   * @tx_dpcpu_fw_id: Firmware ID of the TxDPCPU
>   * @must_probe_vswitching: Flag: vswitching has yet to be setup after MC reboot
>   * @pf_index: The number for this PF, or the parent PF if this is a VF
> -#ifdef CONFIG_SFC_SRIOV
> + * @port_id: port id (Ethernet address) if !CONFIG_SFC_SRIOV;
> + *   for CONFIG_SFC_SRIOV, the VF port id

I think this is always the PF's MAC address, and is used by
 ndo_get_phys_port_id.  On EF100 that method only exists for PFs
 (the vfrep's ndo_get_port_parent_id also shows the PF's port_id),
 whereas on EF10 VFs also have a phys_port_id with the PF's MAC
 address.
I guess the best way to summarise this for the kerneldoc comment
 would be:
 * @port_id: Ethernet address of owning PF, used for phys_port_id
In our local tree we just have "@port_id: Physical port identity".

> + * @vf_index: Index of particular VF in the VF data structure

This isn't quite right; this field is the index of this VF more
 generally within the PF's set of VFs; it's provided by firmware,
 and passed back to firmware in various requests.
And when it's used as an index into the VF data structure array,
 it's the _parent PF's_ nic_data->vf that is indexed by the VF's
 nic_data->vf_index.  (The VF's nic_data->vf is %NULL afaik.)
Not really sure how to summarise this, other than just following
 the pattern of @pf_index above:
 * @vf_index: The number for this VF, or 0xFFFF if this is a VF
 which isn't greatly informative, but we could add more to @vf:

>   * @vf: Pointer to VF data structure

 * @vf: for a PF, array of VF data structures indexed by VF's
	@vf_index

> -#endif
>   * @vport_mac: The MAC address on the vport, only for PFs; VFs will be zero
>   * @vlan_list: List of VLANs added over the interface. Serialised by vlan_lock.
>   * @vlan_lock: Lock to serialize access to vlan_list.
> @@ -166,6 +167,7 @@ enum {
>   * @udp_tunnels_dirty: flag indicating a reboot occurred while pushing
>   *	@udp_tunnels to hardware and thus the push must be re-done.
>   * @udp_tunnels_lock: Serialises writes to @udp_tunnels and @udp_tunnels_dirty.
> + * @licensed_features: used to enable features if the adapter is licensed for it

In our local tree we have:
 * @licensed_features: Flags for licensed firmware features.
 which might be better as it doesn't give the impression that the
 driver can change this ('enable' things) — it's a bitmask that
 comes directly from firmware.

-ed

Re: [PATCH net-next] sfc: correct kernel-doc complaints

[Randy Dunlap]

Hi Edward,

On 12/5/25 9:19 AM, Edward Cree wrote:
> On 29/11/2025 22:03, Randy Dunlap wrote:
>> Fix kernel-doc warnings by adding 3 missing struct member descriptions
>> in struct efx_ef10_nic_data and removing preprocessor directives (which
>> are not handled by kernel-doc).
>>
>> Fixes these 5 warnings:
>> Warning: drivers/net/ethernet/sfc/nic.h:158 bad line: #ifdef CONFIG_SFC_SRIOV
>> Warning: drivers/net/ethernet/sfc/nic.h:160 bad line: #endif
> 
> Does kernel-doc not complain if a member is documented but the actual
>  declaration is ifdefed out?  Normal practice seems to be to move the
>  doc into another comment adjacent to the declaration so it's under
>  the same ifdef; is that unnecessary?

kernel-doc knows nothing about the kernel config (.config).
Inside a struct/union, it strips away (ignores) preprocessor lines.
So no, it does not complain.

>> Warning: drivers/net/ethernet/sfc/nic.h:204 struct member 'port_id'
>>  not described in 'efx_ef10_nic_data'
>> Warning: drivers/net/ethernet/sfc/nic.h:204 struct member 'vf_index'
>>  not described in 'efx_ef10_nic_data'
>> Warning: drivers/net/ethernet/sfc/nic.h:204 struct member 'licensed_features'
>>  not described in 'efx_ef10_nic_data'
>>
>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>> ---
>> Cc: Edward Cree <ecree.xilinx@gmail.com>
>> Cc: Andrew Lunn <andrew+netdev@lunn.ch>
>> Cc: "David S. Miller" <davem@davemloft.net>
>> Cc: Eric Dumazet <edumazet@google.com>
>> Cc: Jakub Kicinski <kuba@kernel.org>
>> Cc: Paolo Abeni <pabeni@redhat.com>
>> Cc: Simon Horman <horms@kernel.org>
>> Cc: linux-net-drivers@amd.com
>> ---
>>  drivers/net/ethernet/sfc/nic.h |    6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> --- linux-next-20251128.orig/drivers/net/ethernet/sfc/nic.h
>> +++ linux-next-20251128/drivers/net/ethernet/sfc/nic.h
>> @@ -156,9 +156,10 @@ enum {
>>   * @tx_dpcpu_fw_id: Firmware ID of the TxDPCPU
>>   * @must_probe_vswitching: Flag: vswitching has yet to be setup after MC reboot
>>   * @pf_index: The number for this PF, or the parent PF if this is a VF
>> -#ifdef CONFIG_SFC_SRIOV
>> + * @port_id: port id (Ethernet address) if !CONFIG_SFC_SRIOV;
>> + *   for CONFIG_SFC_SRIOV, the VF port id
> 
> I think this is always the PF's MAC address, and is used by
>  ndo_get_phys_port_id.  On EF100 that method only exists for PFs
>  (the vfrep's ndo_get_port_parent_id also shows the PF's port_id),
>  whereas on EF10 VFs also have a phys_port_id with the PF's MAC
>  address.
> I guess the best way to summarise this for the kerneldoc comment
>  would be:
>  * @port_id: Ethernet address of owning PF, used for phys_port_id
> In our local tree we just have "@port_id: Physical port identity".
> 
>> + * @vf_index: Index of particular VF in the VF data structure
> 
> This isn't quite right; this field is the index of this VF more
>  generally within the PF's set of VFs; it's provided by firmware,
>  and passed back to firmware in various requests.
> And when it's used as an index into the VF data structure array,
>  it's the _parent PF's_ nic_data->vf that is indexed by the VF's
>  nic_data->vf_index.  (The VF's nic_data->vf is %NULL afaik.)
> Not really sure how to summarise this, other than just following
>  the pattern of @pf_index above:
>  * @vf_index: The number for this VF, or 0xFFFF if this is a VF
>  which isn't greatly informative, but we could add more to @vf:
> 
>>   * @vf: Pointer to VF data structure
> 
>  * @vf: for a PF, array of VF data structures indexed by VF's
> 	@vf_index
> 
>> -#endif
>>   * @vport_mac: The MAC address on the vport, only for PFs; VFs will be zero
>>   * @vlan_list: List of VLANs added over the interface. Serialised by vlan_lock.
>>   * @vlan_lock: Lock to serialize access to vlan_list.
>> @@ -166,6 +167,7 @@ enum {
>>   * @udp_tunnels_dirty: flag indicating a reboot occurred while pushing
>>   *	@udp_tunnels to hardware and thus the push must be re-done.
>>   * @udp_tunnels_lock: Serialises writes to @udp_tunnels and @udp_tunnels_dirty.
>> + * @licensed_features: used to enable features if the adapter is licensed for it
> 
> In our local tree we have:
>  * @licensed_features: Flags for licensed firmware features.
>  which might be better as it doesn't give the impression that the
>  driver can change this ('enable' things) — it's a bitmask that
>  comes directly from firmware.
> 
> -ed

I can just use what's in your local tree (comments above), or
would you prefer to send the patch?

Thanks for the info.
-- 
~Randy