diff mbox series

[net-next,v2,1/2] eth: fbnic: Add ethtool support for fbnic

Message ID 20240827205904.1944066-2-mohsin.bashr@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series eth: Add basic ethtool support for fbnic | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next, async
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 16 this patch: 16
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 17 this patch: 17
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 22 this patch: 22
netdev/checkpatch warning WARNING: Argument '_commit' is not used in function-like macro WARNING: Argument '_delim' is not used in function-like macro WARNING: Missing or malformed SPDX-License-Identifier tag in line 1 WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-08-30--06-00 (tests: 714)

Commit Message

Mohsin Bashir Aug. 27, 2024, 8:59 p.m. UTC 
Add ethtool ops support and enable 'get_drvinfo' for fbnic. The driver
provides firmware version information while the driver name and bus
information is provided by ethtool_get_drvinfo().

Signed-off-by: Mohsin Bashir <mohsin.bashr@gmail.com>
---
v2:
- Update the emptiness check for firmware version commit string
- Rebase to the latest

v1: https://lore.kernel.org/netdev/20240807002445.3833895-1-mohsin.bashr@gmail.com
---
 drivers/net/ethernet/meta/fbnic/Makefile      |  1 +
 drivers/net/ethernet/meta/fbnic/fbnic.h       |  3 +++
 .../net/ethernet/meta/fbnic/fbnic_ethtool.c   | 26 +++++++++++++++++++
 drivers/net/ethernet/meta/fbnic/fbnic_fw.c    | 13 ++++++++++
 drivers/net/ethernet/meta/fbnic/fbnic_fw.h    |  6 ++---
 .../net/ethernet/meta/fbnic/fbnic_netdev.c    |  2 ++
 .../net/ethernet/meta/fbnic/fbnic_netdev.h    |  1 +
 7 files changed, 49 insertions(+), 3 deletions(-)
 create mode 100644 drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c

Comments

Michal Swiatkowski Aug. 28, 2024, 6:29 a.m. UTC | #1 
On Tue, Aug 27, 2024 at 01:59:03PM -0700, Mohsin Bashir wrote:
> Add ethtool ops support and enable 'get_drvinfo' for fbnic. The driver
> provides firmware version information while the driver name and bus
> information is provided by ethtool_get_drvinfo().
> 
> Signed-off-by: Mohsin Bashir <mohsin.bashr@gmail.com>
> ---
> v2:
> - Update the emptiness check for firmware version commit string
> - Rebase to the latest
> 
> v1: https://lore.kernel.org/netdev/20240807002445.3833895-1-mohsin.bashr@gmail.com

Correct link:
https://lore.kernel.org/netdev/20240822184944.3882360-1-mohsin.bashr@gmail.com/

> ---
>  drivers/net/ethernet/meta/fbnic/Makefile      |  1 +
>  drivers/net/ethernet/meta/fbnic/fbnic.h       |  3 +++
>  .../net/ethernet/meta/fbnic/fbnic_ethtool.c   | 26 +++++++++++++++++++
>  drivers/net/ethernet/meta/fbnic/fbnic_fw.c    | 13 ++++++++++
>  drivers/net/ethernet/meta/fbnic/fbnic_fw.h    |  6 ++---
>  .../net/ethernet/meta/fbnic/fbnic_netdev.c    |  2 ++
>  .../net/ethernet/meta/fbnic/fbnic_netdev.h    |  1 +
>  7 files changed, 49 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
> 
> diff --git a/drivers/net/ethernet/meta/fbnic/Makefile b/drivers/net/ethernet/meta/fbnic/Makefile
> index 9373b558fdc9..37cfc34a5118 100644
> --- a/drivers/net/ethernet/meta/fbnic/Makefile
> +++ b/drivers/net/ethernet/meta/fbnic/Makefile
> @@ -8,6 +8,7 @@
>  obj-$(CONFIG_FBNIC) += fbnic.o
>  
>  fbnic-y := fbnic_devlink.o \
> +	   fbnic_ethtool.o \
>  	   fbnic_fw.o \
>  	   fbnic_irq.o \
>  	   fbnic_mac.o \
> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic.h b/drivers/net/ethernet/meta/fbnic/fbnic.h
> index ad2689bfd6cb..28d970f81bfc 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic.h
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic.h
> @@ -132,6 +132,9 @@ void fbnic_free_irq(struct fbnic_dev *dev, int nr, void *data);
>  void fbnic_free_irqs(struct fbnic_dev *fbd);
>  int fbnic_alloc_irqs(struct fbnic_dev *fbd);
>  
> +void fbnic_get_fw_ver_commit_str(struct fbnic_dev *fbd, char *fw_version,
> +				 const size_t str_sz);
> +
>  enum fbnic_boards {
>  	fbnic_board_asic
>  };
> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
> new file mode 100644
> index 000000000000..7064dfc9f5b0
> --- /dev/null
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
> @@ -0,0 +1,26 @@
> +#include <linux/ethtool.h>
> +#include <linux/netdevice.h>
> +#include <linux/pci.h>
> +
> +#include "fbnic.h"
> +#include "fbnic_netdev.h"
> +#include "fbnic_tlv.h"
> +
> +static void
> +fbnic_get_drvinfo(struct net_device *netdev, struct ethtool_drvinfo *drvinfo)
> +{
> +	struct fbnic_net *fbn = netdev_priv(netdev);
> +	struct fbnic_dev *fbd = fbn->fbd;
> +
> +	fbnic_get_fw_ver_commit_str(fbd, drvinfo->fw_version,
> +				    sizeof(drvinfo->fw_version));
> +}
> +
> +static const struct ethtool_ops fbnic_ethtool_ops = {
> +	.get_drvinfo		= fbnic_get_drvinfo,
> +};
> +
> +void fbnic_set_ethtool_ops(struct net_device *dev)
> +{
> +	dev->ethtool_ops = &fbnic_ethtool_ops;
> +}
> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_fw.c b/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
> index 0c6e1b4c119b..8f7a2a19ddf8 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
> @@ -789,3 +789,16 @@ void fbnic_mbx_flush_tx(struct fbnic_dev *fbd)
>  		count += (tx_mbx->head - head) % FBNIC_IPC_MBX_DESC_LEN;
>  	} while (count < FBNIC_IPC_MBX_DESC_LEN && --attempts);
>  }
> +
> +void fbnic_get_fw_ver_commit_str(struct fbnic_dev *fbd, char *fw_version,
> +				 const size_t str_sz)
Don't you need a prototype for this function?

> +{
> +	struct fbnic_fw_ver *mgmt = &fbd->fw_cap.running.mgmt;
> +	const char *delim = "";
> +
> +	if (mgmt->commit[0])
> +		delim = "_";
> +
> +	fbnic_mk_full_fw_ver_str(mgmt->version, delim, mgmt->commit,
> +				 fw_version, str_sz);
> +}
> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_fw.h b/drivers/net/ethernet/meta/fbnic/fbnic_fw.h
> index c65bca613665..221faf8c6756 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic_fw.h
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_fw.h
> @@ -53,10 +53,10 @@ int fbnic_fw_xmit_ownership_msg(struct fbnic_dev *fbd, bool take_ownership);
>  int fbnic_fw_init_heartbeat(struct fbnic_dev *fbd, bool poll);
>  void fbnic_fw_check_heartbeat(struct fbnic_dev *fbd);
>  
> -#define fbnic_mk_full_fw_ver_str(_rev_id, _delim, _commit, _str)	\
> +#define fbnic_mk_full_fw_ver_str(_rev_id, _delim, _commit, _str, _str_sz) \
>  do {									\
>  	const u32 __rev_id = _rev_id;					\
> -	snprintf(_str, sizeof(_str), "%02lu.%02lu.%02lu-%03lu%s%s",	\
> +	snprintf(_str, _str_sz, "%02lu.%02lu.%02lu-%03lu%s%s",	\
>  		 FIELD_GET(FBNIC_FW_CAP_RESP_VERSION_MAJOR, __rev_id),	\
>  		 FIELD_GET(FBNIC_FW_CAP_RESP_VERSION_MINOR, __rev_id),	\
>  		 FIELD_GET(FBNIC_FW_CAP_RESP_VERSION_PATCH, __rev_id),	\
> @@ -65,7 +65,7 @@ do {									\
>  } while (0)
>  
>  #define fbnic_mk_fw_ver_str(_rev_id, _str) \
> -	fbnic_mk_full_fw_ver_str(_rev_id, "", "", _str)
> +	fbnic_mk_full_fw_ver_str(_rev_id, "", "", _str, sizeof(_str))
>  
>  #define FW_HEARTBEAT_PERIOD		(10 * HZ)
>  
> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
> index 571374361259..a400616a24d4 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
> @@ -521,6 +521,8 @@ struct net_device *fbnic_netdev_alloc(struct fbnic_dev *fbd)
>  	netdev->netdev_ops = &fbnic_netdev_ops;
>  	netdev->stat_ops = &fbnic_stat_ops;
>  
> +	fbnic_set_ethtool_ops(netdev);
> +
>  	fbn = netdev_priv(netdev);
>  
>  	fbn->netdev = netdev;
> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h
> index 60199e634468..6c27da09a612 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h
> @@ -58,6 +58,7 @@ int fbnic_netdev_register(struct net_device *netdev);
>  void fbnic_netdev_unregister(struct net_device *netdev);
>  void fbnic_reset_queues(struct fbnic_net *fbn,
>  			unsigned int tx, unsigned int rx);
> +void fbnic_set_ethtool_ops(struct net_device *dev);
Isn't it cleaner to have it in sth like fbnic_ethtool.h file? Probably
you will have more functions there in the future.

Thanks,
Michal

>  
>  void __fbnic_set_rx_mode(struct net_device *netdev);
>  void fbnic_clear_rx_mode(struct net_device *netdev);
> -- 
> 2.43.5
>
Mohsin Bashir Aug. 28, 2024, 5:49 p.m. UTC | #2 
On 8/27/24 11:29 PM, Michal Swiatkowski wrote:
> On Tue, Aug 27, 2024 at 01:59:03PM -0700, Mohsin Bashir wrote:
>> Add ethtool ops support and enable 'get_drvinfo' for fbnic. The driver
>> provides firmware version information while the driver name and bus
>> information is provided by ethtool_get_drvinfo().
>>
>> Signed-off-by: Mohsin Bashir <mohsin.bashr@gmail.com>
>> ---
>> v2:
>> - Update the emptiness check for firmware version commit string
>> - Rebase to the latest
>>
>> v1: https://lore.kernel.org/netdev/20240807002445.3833895-1-mohsin.bashr@gmail.com
> Correct link:
> https://lore.kernel.org/netdev/20240822184944.3882360-1-mohsin.bashr@gmail.com/
Thank you for pointing this out.
>
>> ---
>>   drivers/net/ethernet/meta/fbnic/Makefile      |  1 +
>>   drivers/net/ethernet/meta/fbnic/fbnic.h       |  3 +++
>>   .../net/ethernet/meta/fbnic/fbnic_ethtool.c   | 26 +++++++++++++++++++
>>   drivers/net/ethernet/meta/fbnic/fbnic_fw.c    | 13 ++++++++++
>>   drivers/net/ethernet/meta/fbnic/fbnic_fw.h    |  6 ++---
>>   .../net/ethernet/meta/fbnic/fbnic_netdev.c    |  2 ++
>>   .../net/ethernet/meta/fbnic/fbnic_netdev.h    |  1 +
>>   7 files changed, 49 insertions(+), 3 deletions(-)
>>   create mode 100644 drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
>>
>> diff --git a/drivers/net/ethernet/meta/fbnic/Makefile b/drivers/net/ethernet/meta/fbnic/Makefile
>> index 9373b558fdc9..37cfc34a5118 100644
>> --- a/drivers/net/ethernet/meta/fbnic/Makefile
>> +++ b/drivers/net/ethernet/meta/fbnic/Makefile
>> @@ -8,6 +8,7 @@
>>   obj-$(CONFIG_FBNIC) += fbnic.o
>>   
>>   fbnic-y := fbnic_devlink.o \
>> +	   fbnic_ethtool.o \
>>   	   fbnic_fw.o \
>>   	   fbnic_irq.o \
>>   	   fbnic_mac.o \
>> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic.h b/drivers/net/ethernet/meta/fbnic/fbnic.h
>> index ad2689bfd6cb..28d970f81bfc 100644
>> --- a/drivers/net/ethernet/meta/fbnic/fbnic.h
>> +++ b/drivers/net/ethernet/meta/fbnic/fbnic.h
>> @@ -132,6 +132,9 @@ void fbnic_free_irq(struct fbnic_dev *dev, int nr, void *data);
>>   void fbnic_free_irqs(struct fbnic_dev *fbd);
>>   int fbnic_alloc_irqs(struct fbnic_dev *fbd);
>>   
>> +void fbnic_get_fw_ver_commit_str(struct fbnic_dev *fbd, char *fw_version,
>> +				 const size_t str_sz);
>> +
>>   enum fbnic_boards {
>>   	fbnic_board_asic
>>   };
>> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
>> new file mode 100644
>> index 000000000000..7064dfc9f5b0
>> --- /dev/null
>> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
>> @@ -0,0 +1,26 @@
>> +#include <linux/ethtool.h>
>> +#include <linux/netdevice.h>
>> +#include <linux/pci.h>
>> +
>> +#include "fbnic.h"
>> +#include "fbnic_netdev.h"
>> +#include "fbnic_tlv.h"
>> +
>> +static void
>> +fbnic_get_drvinfo(struct net_device *netdev, struct ethtool_drvinfo *drvinfo)
>> +{
>> +	struct fbnic_net *fbn = netdev_priv(netdev);
>> +	struct fbnic_dev *fbd = fbn->fbd;
>> +
>> +	fbnic_get_fw_ver_commit_str(fbd, drvinfo->fw_version,
>> +				    sizeof(drvinfo->fw_version));
>> +}
>> +
>> +static const struct ethtool_ops fbnic_ethtool_ops = {
>> +	.get_drvinfo		= fbnic_get_drvinfo,
>> +};
>> +
>> +void fbnic_set_ethtool_ops(struct net_device *dev)
>> +{
>> +	dev->ethtool_ops = &fbnic_ethtool_ops;
>> +}
>> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_fw.c b/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
>> index 0c6e1b4c119b..8f7a2a19ddf8 100644
>> --- a/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
>> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
>> @@ -789,3 +789,16 @@ void fbnic_mbx_flush_tx(struct fbnic_dev *fbd)
>>   		count += (tx_mbx->head - head) % FBNIC_IPC_MBX_DESC_LEN;
>>   	} while (count < FBNIC_IPC_MBX_DESC_LEN && --attempts);
>>   }
>> +
>> +void fbnic_get_fw_ver_commit_str(struct fbnic_dev *fbd, char *fw_version,
>> +				 const size_t str_sz)
> Don't you need a prototype for this function?
We do have the prototype in fbnic.h
>> +{
>> +	struct fbnic_fw_ver *mgmt = &fbd->fw_cap.running.mgmt;
>> +	const char *delim = "";
>> +
>> +	if (mgmt->commit[0])
>> +		delim = "_";
>> +
>> +	fbnic_mk_full_fw_ver_str(mgmt->version, delim, mgmt->commit,
>> +				 fw_version, str_sz);
>> +}
>> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_fw.h b/drivers/net/ethernet/meta/fbnic/fbnic_fw.h
>> index c65bca613665..221faf8c6756 100644
>> --- a/drivers/net/ethernet/meta/fbnic/fbnic_fw.h
>> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_fw.h
>> @@ -53,10 +53,10 @@ int fbnic_fw_xmit_ownership_msg(struct fbnic_dev *fbd, bool take_ownership);
>>   int fbnic_fw_init_heartbeat(struct fbnic_dev *fbd, bool poll);
>>   void fbnic_fw_check_heartbeat(struct fbnic_dev *fbd);
>>   
>> -#define fbnic_mk_full_fw_ver_str(_rev_id, _delim, _commit, _str)	\
>> +#define fbnic_mk_full_fw_ver_str(_rev_id, _delim, _commit, _str, _str_sz) \
>>   do {									\
>>   	const u32 __rev_id = _rev_id;					\
>> -	snprintf(_str, sizeof(_str), "%02lu.%02lu.%02lu-%03lu%s%s",	\
>> +	snprintf(_str, _str_sz, "%02lu.%02lu.%02lu-%03lu%s%s",	\
>>   		 FIELD_GET(FBNIC_FW_CAP_RESP_VERSION_MAJOR, __rev_id),	\
>>   		 FIELD_GET(FBNIC_FW_CAP_RESP_VERSION_MINOR, __rev_id),	\
>>   		 FIELD_GET(FBNIC_FW_CAP_RESP_VERSION_PATCH, __rev_id),	\
>> @@ -65,7 +65,7 @@ do {									\
>>   } while (0)
>>   
>>   #define fbnic_mk_fw_ver_str(_rev_id, _str) \
>> -	fbnic_mk_full_fw_ver_str(_rev_id, "", "", _str)
>> +	fbnic_mk_full_fw_ver_str(_rev_id, "", "", _str, sizeof(_str))
>>   
>>   #define FW_HEARTBEAT_PERIOD		(10 * HZ)
>>   
>> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
>> index 571374361259..a400616a24d4 100644
>> --- a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
>> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
>> @@ -521,6 +521,8 @@ struct net_device *fbnic_netdev_alloc(struct fbnic_dev *fbd)
>>   	netdev->netdev_ops = &fbnic_netdev_ops;
>>   	netdev->stat_ops = &fbnic_stat_ops;
>>   
>> +	fbnic_set_ethtool_ops(netdev);
>> +
>>   	fbn = netdev_priv(netdev);
>>   
>>   	fbn->netdev = netdev;
>> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h
>> index 60199e634468..6c27da09a612 100644
>> --- a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h
>> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h
>> @@ -58,6 +58,7 @@ int fbnic_netdev_register(struct net_device *netdev);
>>   void fbnic_netdev_unregister(struct net_device *netdev);
>>   void fbnic_reset_queues(struct fbnic_net *fbn,
>>   			unsigned int tx, unsigned int rx);
>> +void fbnic_set_ethtool_ops(struct net_device *dev);
> Isn't it cleaner to have it in sth like fbnic_ethtool.h file? Probably
> you will have more functions there in the future.
>
> Thanks,
> Michal
It would definitely be cleaner if have a large number of functions which 
isn't the case hence, did not add fbnic_ethtool.h
>
>>   
>>   void __fbnic_set_rx_mode(struct net_device *netdev);
>>   void fbnic_clear_rx_mode(struct net_device *netdev);
>> -- 
>> 2.43.5
>>
Jakub Kicinski Aug. 28, 2024, 6:18 p.m. UTC | #3 
On Wed, 28 Aug 2024 10:49:06 -0700 Mohsin Bashir wrote:
> > Isn't it cleaner to have it in sth like fbnic_ethtool.h file? Probably
> > you will have more functions there in the future.
>
> It would definitely be cleaner if have a large number of functions which 
> isn't the case hence, did not add fbnic_ethtool.h

To further clarify, if I'm grepping right - the more fully featured
"prototype" driver out of tree only exposes this one function from 
fbnic_ethtool.c

Most ethtool functions get hooked in via ops, and ethtool code calls
out, rather than getting called by the driver itself. So it's probably
fine (and I only mean in this particular case, IDK if all the headers
in fbnic are well organized, it's been a while.)
diff mbox series

Patch

diff --git a/drivers/net/ethernet/meta/fbnic/Makefile b/drivers/net/ethernet/meta/fbnic/Makefile
index 9373b558fdc9..37cfc34a5118 100644
--- a/drivers/net/ethernet/meta/fbnic/Makefile
+++ b/drivers/net/ethernet/meta/fbnic/Makefile
@@ -8,6 +8,7 @@ 
 obj-$(CONFIG_FBNIC) += fbnic.o
 
 fbnic-y := fbnic_devlink.o \
+	   fbnic_ethtool.o \
 	   fbnic_fw.o \
 	   fbnic_irq.o \
 	   fbnic_mac.o \
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic.h b/drivers/net/ethernet/meta/fbnic/fbnic.h
index ad2689bfd6cb..28d970f81bfc 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic.h
+++ b/drivers/net/ethernet/meta/fbnic/fbnic.h
@@ -132,6 +132,9 @@  void fbnic_free_irq(struct fbnic_dev *dev, int nr, void *data);
 void fbnic_free_irqs(struct fbnic_dev *fbd);
 int fbnic_alloc_irqs(struct fbnic_dev *fbd);
 
+void fbnic_get_fw_ver_commit_str(struct fbnic_dev *fbd, char *fw_version,
+				 const size_t str_sz);
+
 enum fbnic_boards {
 	fbnic_board_asic
 };
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
new file mode 100644
index 000000000000..7064dfc9f5b0
--- /dev/null
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
@@ -0,0 +1,26 @@ 
+#include <linux/ethtool.h>
+#include <linux/netdevice.h>
+#include <linux/pci.h>
+
+#include "fbnic.h"
+#include "fbnic_netdev.h"
+#include "fbnic_tlv.h"
+
+static void
+fbnic_get_drvinfo(struct net_device *netdev, struct ethtool_drvinfo *drvinfo)
+{
+	struct fbnic_net *fbn = netdev_priv(netdev);
+	struct fbnic_dev *fbd = fbn->fbd;
+
+	fbnic_get_fw_ver_commit_str(fbd, drvinfo->fw_version,
+				    sizeof(drvinfo->fw_version));
+}
+
+static const struct ethtool_ops fbnic_ethtool_ops = {
+	.get_drvinfo		= fbnic_get_drvinfo,
+};
+
+void fbnic_set_ethtool_ops(struct net_device *dev)
+{
+	dev->ethtool_ops = &fbnic_ethtool_ops;
+}
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_fw.c b/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
index 0c6e1b4c119b..8f7a2a19ddf8 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
@@ -789,3 +789,16 @@  void fbnic_mbx_flush_tx(struct fbnic_dev *fbd)
 		count += (tx_mbx->head - head) % FBNIC_IPC_MBX_DESC_LEN;
 	} while (count < FBNIC_IPC_MBX_DESC_LEN && --attempts);
 }
+
+void fbnic_get_fw_ver_commit_str(struct fbnic_dev *fbd, char *fw_version,
+				 const size_t str_sz)
+{
+	struct fbnic_fw_ver *mgmt = &fbd->fw_cap.running.mgmt;
+	const char *delim = "";
+
+	if (mgmt->commit[0])
+		delim = "_";
+
+	fbnic_mk_full_fw_ver_str(mgmt->version, delim, mgmt->commit,
+				 fw_version, str_sz);
+}
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_fw.h b/drivers/net/ethernet/meta/fbnic/fbnic_fw.h
index c65bca613665..221faf8c6756 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_fw.h
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_fw.h
@@ -53,10 +53,10 @@  int fbnic_fw_xmit_ownership_msg(struct fbnic_dev *fbd, bool take_ownership);
 int fbnic_fw_init_heartbeat(struct fbnic_dev *fbd, bool poll);
 void fbnic_fw_check_heartbeat(struct fbnic_dev *fbd);
 
-#define fbnic_mk_full_fw_ver_str(_rev_id, _delim, _commit, _str)	\
+#define fbnic_mk_full_fw_ver_str(_rev_id, _delim, _commit, _str, _str_sz) \
 do {									\
 	const u32 __rev_id = _rev_id;					\
-	snprintf(_str, sizeof(_str), "%02lu.%02lu.%02lu-%03lu%s%s",	\
+	snprintf(_str, _str_sz, "%02lu.%02lu.%02lu-%03lu%s%s",	\
 		 FIELD_GET(FBNIC_FW_CAP_RESP_VERSION_MAJOR, __rev_id),	\
 		 FIELD_GET(FBNIC_FW_CAP_RESP_VERSION_MINOR, __rev_id),	\
 		 FIELD_GET(FBNIC_FW_CAP_RESP_VERSION_PATCH, __rev_id),	\
@@ -65,7 +65,7 @@  do {									\
 } while (0)
 
 #define fbnic_mk_fw_ver_str(_rev_id, _str) \
-	fbnic_mk_full_fw_ver_str(_rev_id, "", "", _str)
+	fbnic_mk_full_fw_ver_str(_rev_id, "", "", _str, sizeof(_str))
 
 #define FW_HEARTBEAT_PERIOD		(10 * HZ)
 
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
index 571374361259..a400616a24d4 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.c
@@ -521,6 +521,8 @@  struct net_device *fbnic_netdev_alloc(struct fbnic_dev *fbd)
 	netdev->netdev_ops = &fbnic_netdev_ops;
 	netdev->stat_ops = &fbnic_stat_ops;
 
+	fbnic_set_ethtool_ops(netdev);
+
 	fbn = netdev_priv(netdev);
 
 	fbn->netdev = netdev;
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h
index 60199e634468..6c27da09a612 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_netdev.h
@@ -58,6 +58,7 @@  int fbnic_netdev_register(struct net_device *netdev);
 void fbnic_netdev_unregister(struct net_device *netdev);
 void fbnic_reset_queues(struct fbnic_net *fbn,
 			unsigned int tx, unsigned int rx);
+void fbnic_set_ethtool_ops(struct net_device *dev);
 
 void __fbnic_set_rx_mode(struct net_device *netdev);
 void fbnic_clear_rx_mode(struct net_device *netdev);