-----Original Message----- From: Pali Rohár pali@kernel.org Sent: Sunday, November 7, 2021 1:44 AM To: Roman Bacik roman.bacik@broadcom.com Cc: U-Boot Mailing List u-boot@lists.denx.de; Bharat Gooty bharat.gooty@broadcom.com; Joe Hershberger joe.hershberger@ni.com; Ramon Fried rfried.dev@gmail.com Subject: Re: [PATCH v8 1/2] net: brcm: netXtreme driver
Hello!
When sending a new version of patch, try to put into CC reviewers of previous versions, so they can approve new version (if objections were fixed). Not everybody is following all emails on mailing list as there are lot of emails...
Ok, we will.
On Friday 05 November 2021 15:36:51 Roman Bacik wrote:
--- /dev/null +++ b/drivers/net/bnxt/bnxt.c @@ -0,0 +1,1710 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright 2019-2021 Broadcom.
- */
+#include <common.h>
+#include <asm/io.h> +#include <dm.h> +#include <linux/delay.h> +#include <memalign.h> +#include <net.h>
+#include "bnxt.h" +#include "bnxt_dbg.h"
+#define bnxt_down_chip(bp) bnxt_hwrm_run(down_chip, bp, 0) +#define bnxt_bring_chip(bp) bnxt_hwrm_run(bring_chip, bp, 1)
+/* Broadcom ethernet driver PCI APIs. */ +static void bnxt_bring_pci(struct bnxt *bp) +{
- u16 cmd_reg = 0;
- pci_read_word16(bp->pdev, PCI_VENDOR_ID, &bp->vendor_id);
- pci_read_word16(bp->pdev, PCI_DEVICE_ID, &bp->device_id);
- pci_read_word16(bp->pdev,
PCI_SUBSYSTEM_VENDOR_ID,&bp->subsystem_vendor);- pci_read_word16(bp->pdev, PCI_SUBSYSTEM_ID, &bp-
subsystem_device);
- pci_read_word16(bp->pdev, PCI_COMMAND, &bp->cmd_reg);
- pci_read_byte(bp->pdev, PCI_INTERRUPT_LINE, &bp->irq);
- bp->bar0 = dm_pci_map_bar(bp->pdev, PCI_BASE_ADDRESS_0,
PCI_REGION_MEM);
- bp->bar1 = dm_pci_map_bar(bp->pdev, PCI_BASE_ADDRESS_2,
PCI_REGION_MEM);
- bp->bar2 = dm_pci_map_bar(bp->pdev, PCI_BASE_ADDRESS_4,
PCI_REGION_MEM);
- cmd_reg = bp->cmd_reg | PCI_COMMAND_MEMORY |
PCI_COMMAND_MASTER;
- cmd_reg |= PCI_COMMAND_INTX_DISABLE; /* disable intr */
- pci_write_word(bp->pdev, PCI_COMMAND, cmd_reg);
- pci_read_word16(bp->pdev, PCI_COMMAND, &cmd_reg);
I cannot find any pci_read_word16() function in the current U-Boot master repository. So this patch cannot be compiled.
It is defined in bnxt.h as dm_pci_read_config16. But we will remove pci_read_/pci_write_/pci_map_ definitions from the header and replace.
Could you check that you are developing this patch on top of the recent version of U-Boot git master branch?
We rebase, compile and test with the latest before posting the patch.
Also you should use pci_dm_* functions as Driver Model API is preferred now and drivers are being converting to this new API, so old API can be later dropped.
- dbg_pci(bp, __func__, cmd_reg);
+}
This is our method declared in bnxt_dbg.h.
...
diff --git a/drivers/net/bnxt/bnxt_ver.h b/drivers/net/bnxt/bnxt_ver.h new file mode 100644 index 000000000000..fa84397338dd --- /dev/null +++ b/drivers/net/bnxt/bnxt_ver.h @@ -0,0 +1,22 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*
- Copyright 2019-2021 Broadcom.
- */
+#ifndef _BNXT_VER_H_ +#define _BNXT_VER_H_
+#ifndef BNXT_EXTRA_VER_H +#define BNXT_EXTRA_VER_H +#define DRV_MODULE_EXTRA_VER "-216.1.182.0" +#endif
+#define DRV_MODULE_NAME "bnxt" +#define DRV_MODULE_VERSION "1.0.0" DRV_MODULE_EXTRA_VER +#define UBOOT_MODULE_VER "1.0.0" +#define UBOOT_VERSION_MAJOR 1 +#define UBOOT_VERSION_MINOR 0 +#define UBOOT_VERSION_UPDATE 0 +#define DRV_MODULE_DESC "Broadcom NetXtreme-C/E driver"
+#endif /* _BNXT_VER_H_ */
It looks like that more macros from this file are completely unused. Could you re-check it? Macros which are unused and do not bring any value even for documentation purposes is dead code, which should be eliminated due to maintenance cost.
For example for documentation purposes it could make sense to define unused macro which describe some existing HE register even when this macros is not used currently in the driver.
But defining macro UBOOT_VERSION_* is suspicious as 1) U-Boot version is already defined in other global header file and 2) U-Boot version is not 1.0.0 anymore.
We will remove unused defines from the headers. Thanks,
Roman