---
 drivers/net/ethernet/broadcom/Kconfig              |   19 +
 drivers/net/ethernet/broadcom/Makefile             |    1 +
 drivers/net/ethernet/broadcom/bnxt/Makefile        |    4 +
 drivers/net/ethernet/broadcom/bnxt/bnxt.c          | 5821 ++++++++++++++++++++
 drivers/net/ethernet/broadcom/bnxt/bnxt.h          |  994 ++++
 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c  | 1150 ++++
 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.h  |   18 +
 drivers/net/ethernet/broadcom/bnxt/bnxt_fw_hdr.h   |  104 +
 drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h      | 4046 ++++++++++++++
 drivers/net/ethernet/broadcom/bnxt/bnxt_nvm_defs.h |   59 +
 drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c    |  787 +++
 drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.h    |   22 +
 12 files changed, 13025 insertions(+)
 create mode 100644 drivers/net/ethernet/broadcom/bnxt/Makefile
 create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt.c
 create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt.h
 create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
 create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.h
 create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt_fw_hdr.h
 create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h
 create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt_nvm_defs.h
 create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c
 create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.h

drivers/net/ethernet/broadcom/Kconfig +19 -0

index e930aa9..67a7d52 100644
@@ -170,4 +170,23 @@ config SYSTEMPORT
 	  Broadcom BCM7xxx Set Top Box family chipset using an internal
 	  Ethernet switch.
 
+config BNXT
+	tristate "Broadcom NetXtreme-C/E support"
+	depends on PCI
+	select FW_LOADER
+	select LIBCRC32C
+	---help---
+	  This driver supports Broadcom NetXtreme-C/E 10/25/40/50 gigabit
+	  Ethernet cards.  To compile this driver as a module, choose M here:
+	  the module will be called bnxt_en.  This is recommended.
+
+config BNXT_SRIOV
+	bool "Broadcom NetXtreme-C/E SR-IOV support"
+	depends on BNXT && PCI_IOV
+	default y
+	---help---
+	  This configuration parameter enables Single Root Input Output
+	  Virtualization support in the NetXtreme-C/E products. This
+	  allows for virtual function acceleration in virtual environments.
+
 endif # NET_VENDOR_BROADCOM

drivers/net/ethernet/broadcom/Makefile +1 -0

index e2a958a..00584d7 100644
@@ -12,3 +12,4 @@ obj-$(CONFIG_SB1250_MAC) += sb1250-mac.o
 obj-$(CONFIG_TIGON3) += tg3.o
 obj-$(CONFIG_BGMAC) += bgmac.o
 obj-$(CONFIG_SYSTEMPORT) += bcmsysport.o
+obj-$(CONFIG_BNXT) += bnxt/

drivers/net/ethernet/broadcom/bnxt/Makefile new file +4 -0

new file mode 100644
index 0000000..55b9815
@@ -0,0 +1,4 @@
+obj-$(CONFIG_BNXT) += bnxt_en.o
+
+bnxt_en-y := bnxt.o bnxt_sriov.o bnxt_ethtool.o
+

drivers/net/ethernet/broadcom/bnxt/bnxt.c new file +5821 -0

2,969 lines not on this page · raw message

drivers/net/ethernet/broadcom/bnxt/bnxt.h new file +994 -0

994 lines not on this page · raw message

drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c new file +1150 -0

1,150 lines not on this page · raw message

drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.h new file +18 -0

new file mode 100644
index 0000000..0567c73

Lines of this file are not shown. View the raw message.

drivers/net/ethernet/broadcom/bnxt/bnxt_fw_hdr.h new file +104 -0

104 lines not on this page · raw message

drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h new file +4046 -0

4,046 lines not on this page · raw message

drivers/net/ethernet/broadcom/bnxt/bnxt_nvm_defs.h new file +59 -0

59 lines not on this page · raw message

drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c new file +787 -0

787 lines not on this page · raw message

drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.h new file +22 -0

new file mode 100644
index 0000000..0b430f1

Lines of this file are not shown. View the raw message.

The rest of this diff is not shown. View the raw message.

Signature · 1 line

1.8.3.1

Part of the diff is not shown here. View the raw message.

Stephen Hemminger

2015-10-19 05:17 UTC #1 lore raw

drivers/net/ethernet/broadcom/bnxt/bnxt.c line 93

Quoted 11 lines · On Sat, 17 Oct 2015 00:21:44 -0400 Michael Chan <[email protected]> wrote:

drivers/net/ethernet/broadcom/bnxt/bnxt.c new file +11 -0

+/* indexed by enum above */
+static struct {
+	char *name;
+} board_info[] = {
+	{ "Broadcom BCM57302 NetXtreme-C Single-port 10Gb/25Gb/40Gb/50Gb Ethernet" },
+	{ "Broadcom BCM57304 NetXtreme-C Dual-port 10Gb/25Gb/40Gb/50Gb Ethernet" },
+	{ "Broadcom BCM57404 NetXtreme-E Dual-port 10Gb/25Gb Ethernet" },
+	{ "Broadcom BCM57406 NetXtreme-E Dual-port 10Gb Ethernet" },
+	{ "Broadcom BCM57304 NetXtreme-C Ethernet Virtual Function" },
+	{ "Broadcom BCM57404 NetXtreme-E Ethernet Virtual Function" },
+};

Tables like this should be declared const.

Stephen Hemminger

2015-10-19 05:19 UTC #2 lore raw

In reply to Michael Chan

drivers/net/ethernet/broadcom/bnxt/bnxt.c line 127

Quoted 7 lines · On Sat, 17 Oct 2015 00:21:44 -0400 Michael Chan <[email protected]> wrote:

drivers/net/ethernet/broadcom/bnxt/bnxt.c new file +7 -0

+static bool bnxt_vf_pciid(int idx)
+{
+	if (idx == BCM57304_VF || idx == BCM57404_VF)
+		return true;
+	return false;
+}
+

I prefer just returning result of logical operation rather than adding unnecessary if statement.
And never use (signed) int when unsigned is the real data type. Also avoid any unnecessary expansion of 16 bit value.

static bool bnxt_vf_pciid(u16 idx)

{
	return (idx == BCM57304_VF || idx == BCM57404_VF);
}

Michael Chan

author

2015-10-19 23:04 UTC #3 lore raw

Quoted 17 lines · Stephen Hemminger

drivers/net/ethernet/broadcom/bnxt/bnxt.c line 127
Quoted 7 lines · On Sat, 17 Oct 2015 00:21:44 -0400 Michael Chan <[email protected]> wrote:
drivers/net/ethernet/broadcom/bnxt/bnxt.c new file +7 -0
+static bool bnxt_vf_pciid(int idx)
+{
+	if (idx == BCM57304_VF || idx == BCM57404_VF)
+		return true;
+	return false;
+}
+
I prefer just returning result of logical operation rather than adding unnecessary if statement.
And never use (signed) int when unsigned is the real data type. Also avoid any unnecessary expansion of 16 bit value.

Quoted 4 lines

static bool bnxt_vf_pciid(u16 idx)
{
	return (idx == BCM57304_VF || idx == BCM57404_VF);
}

idx is actually type enum. Other points well taken.

David Laight

2015-10-20 15:49 UTC #4 lore raw

In reply to Stephen Hemminger

From: Stephen Hemminger
...

Quoted 16 lines

drivers/net/ethernet/broadcom/bnxt/bnxt.c line 127
Quoted 7 lines · On Sat, 17 Oct 2015 00:21:44 -0400 Michael Chan <[email protected]> wrote:
drivers/net/ethernet/broadcom/bnxt/bnxt.c new file +7 -0
+static bool bnxt_vf_pciid(int idx)
+{
+	if (idx == BCM57304_VF || idx == BCM57404_VF)
+		return true;
+	return false;
+}
+
I prefer just returning result of logical operation rather than adding unnecessary if statement.
And never use (signed) int when unsigned is the real data type. Also avoid any unnecessary expansion of 16 bit value.

Actually 16-bit (and 8-bit) values always have to be extended to 'int' before any arithmetic operations.
On systems with 16-bit arithmetic instructions (like x86) the compiler can only use them if it can determine that result would be the same (as if they were extended).

This can mean a lot of extra instructions if you do arithmetic on 16-bit values the compiler is holding in registers.

Similarly using 16-bit values as function parameters/results can require additional masking instructions.

IMHO This really all means that you should use [unsigned] int for local variables even when the domain of the value is known to be much smaller.

David

Stephen Hemminger

2015-10-19 05:21 UTC #5 lore raw

In reply to Michael Chan

drivers/net/ethernet/broadcom/bnxt/bnxt.c line 144

Quoted 4 lines · On Sat, 17 Oct 2015 00:21:44 -0400 Michael Chan <[email protected]> wrote:

drivers/net/ethernet/broadcom/bnxt/bnxt.c new file +4 -0

+static inline u32 bnxt_tx_avail(struct bnxt *bp, struct bnxt_tx_ring_info *txr)
+{
+	/* Tell compiler to fetch tx indices from memory. */
+	barrier();

Actually, you are telling compiler not to reorder. Which may not be sufficient on some platforms.

Quoted 3 lines

drivers/net/ethernet/broadcom/bnxt/bnxt.c new file +3 -0
+	return bp->tx_ring_size -
+		((txr->tx_prod - txr->tx_cons) & bp->tx_ring_mask);
+}

Michael Chan

author

2015-10-19 22:09 UTC #6 lore raw

Quoted 10 lines · Stephen Hemminger

drivers/net/ethernet/broadcom/bnxt/bnxt.c line 144
Quoted 4 lines · On Sat, 17 Oct 2015 00:21:44 -0400 Michael Chan <[email protected]> wrote:
drivers/net/ethernet/broadcom/bnxt/bnxt.c new file +4 -0
+static inline u32 bnxt_tx_avail(struct bnxt *bp, struct bnxt_tx_ring_info *txr)
+{
+	/* Tell compiler to fetch tx indices from memory. */
+	barrier();
Actually, you are telling compiler not to reorder. Which may not be sufficient on some platforms.

Only a compiler barrier is needed here to prevent the compiler from optimizing out the memory access of tx_prod and tx_cons, in case it thinks that it has those values in some registers.

Quoted 5 lines

Quoted 5 lines
drivers/net/ethernet/broadcom/bnxt/bnxt.c new file +3 -0
+	return bp->tx_ring_size -
+		((txr->tx_prod - txr->tx_cons) & bp->tx_ring_mask);
+}

John W. Linville

2015-10-20 20:15 UTC #7 lore raw

In reply to Michael Chan

Quoted 12 lines · Michael Chan

Broadcom ethernet driver for the new family of NetXtreme-C/E ethernet devices.
v3:
- Fixed 2 more sparse warnings.
- Removed some unused structures in .h files.
v2:
- Fixed all kbuild test robot reported warnings.
- Fixed many of the checkpatch.pl errors and warnings.
- Fixed the Kconfig description.

Quoted 4 lines

Acked-by Eddie Wai <[email protected]>
Acked-by Jeffrey Huang <[email protected]>
Signed-off-by Prashant Sreedharan <[email protected]>
Signed-off-by Michael Chan <[email protected]>

...

Looks pretty good so far...

drivers/net/ethernet/broadcom/bnxt/bnxt.c line 3820

Quoted 8 lines

drivers/net/ethernet/broadcom/bnxt/bnxt.c new file +8 -0

+	if (req.hwrm_intf_maj != resp->hwrm_intf_maj ||
+	    req.hwrm_intf_min != resp->hwrm_intf_min ||
+	    req.hwrm_intf_upd != resp->hwrm_intf_upd) {
+		netdev_warn(bp->dev, "HWRM interface %d.%d.%d does not match driver interface %d.%d.%d.  Please update driver or firmware with matching interface versions.\n",
+			    resp->hwrm_intf_maj, resp->hwrm_intf_min,
+			    resp->hwrm_intf_upd, req.hwrm_intf_maj,
+			    req.hwrm_intf_min, req.hwrm_intf_upd);
+	}

Any chance we could break that string and/or make it a bit shorter? Thats a long one! (130+ characters)

John

Signature · 2 lines

John W. Linville		Someday the world will need a hero, and you
[email protected]			might be all we have.  Be ready.

John W. Linville

2015-10-20 20:15 UTC #8 lore raw

In reply to Michael Chan

Quoted 16 lines · Michael Chan

Broadcom ethernet driver for the new family of NetXtreme-C/E ethernet devices.
v3:
- Fixed 2 more sparse warnings.
- Removed some unused structures in .h files.
v2:
- Fixed all kbuild test robot reported warnings.
- Fixed many of the checkpatch.pl errors and warnings.
- Fixed the Kconfig description.
Acked-by Eddie Wai <[email protected]>
Acked-by Jeffrey Huang <[email protected]>
Signed-off-by Prashant Sreedharan <[email protected]>
Signed-off-by Michael Chan <[email protected]>

...

drivers/net/ethernet/broadcom/bnxt/bnxt.c line 5103

Quoted 10 lines

drivers/net/ethernet/broadcom/bnxt/bnxt.c new file +10 -0

+		if (netif_msg_drv(bp))
+			netdev_info(bp->dev, "[%d]: tx{fw_ring: %d prod: %x cons: %x} rx{fw_ring: %d prod: %x} rx_agg{fw_ring: %d agg_prod: %x sw_agg_prod: %x} cp{fw_ring: %d raw_cons: %x}\n",
+				    i, txr->tx_ring_struct.fw_ring_id,
+				    txr->tx_prod, txr->tx_cons,
+				    rxr->rx_ring_struct.fw_ring_id,
+				    rxr->rx_prod,
+				    rxr->rx_agg_ring_struct.fw_ring_id,
+				    rxr->rx_agg_prod, rxr->rx_sw_agg_prod,
+				    cpr->cp_ring_struct.fw_ring_id,
+				    cpr->cp_raw_cons);

Another big string -- perhaps a few \n's are in order?

John

Signature · 2 lines

John W. Linville		Someday the world will need a hero, and you
[email protected]			might be all we have.  Be ready.

John W. Linville

2015-10-20 20:15 UTC #9 lore raw

In reply to Michael Chan

Quoted 32 lines · Michael Chan

Broadcom ethernet driver for the new family of NetXtreme-C/E ethernet devices.
v3:
- Fixed 2 more sparse warnings.
- Removed some unused structures in .h files.
v2:
- Fixed all kbuild test robot reported warnings.
- Fixed many of the checkpatch.pl errors and warnings.
- Fixed the Kconfig description.
Acked-by Eddie Wai <[email protected]>
Acked-by Jeffrey Huang <[email protected]>
Signed-off-by Prashant Sreedharan <[email protected]>
Signed-off-by Michael Chan <[email protected]>
---
 drivers/net/ethernet/broadcom/Kconfig              |   19 +
 drivers/net/ethernet/broadcom/Makefile             |    1 +
 drivers/net/ethernet/broadcom/bnxt/Makefile        |    4 +
 drivers/net/ethernet/broadcom/bnxt/bnxt.c          | 5821 ++++++++++++++++++++
 drivers/net/ethernet/broadcom/bnxt/bnxt.h          |  994 ++++
 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c  | 1150 ++++
 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.h  |   18 +
 drivers/net/ethernet/broadcom/bnxt/bnxt_fw_hdr.h   |  104 +
 drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h      | 4046 ++++++++++++++
 drivers/net/ethernet/broadcom/bnxt/bnxt_nvm_defs.h |   59 +
 drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c    |  787 +++
 drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.h    |   22 +
 12 files changed, 13025 insertions(+)
 create mode 100644 drivers/net/ethernet/broadcom/bnxt/Makefile
 create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt.c

Quoted 8 lines

 create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt.h
 create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
 create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.h
 create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt_fw_hdr.h
 create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h
 create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt_nvm_defs.h
 create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c
 create mode 100644 drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.h

Well, it's a big one! But overall I'd say it looks pretty good...

I agree with Stephen's style comments, and there are a couple of strings that are crazy long -- not just a source readability problem, but things that would be hard to digest in a log as well. If you address those then I'll be happy to ACK it.

John

Signature · 2 lines

John W. Linville		Someday the world will need a hero, and you
[email protected]			might be all we have.  Be ready.