[PATCH v8 1/2] net: brcm: netXtreme driver

[Pali Rohár]

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...

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.

Could you check that you are developing this patch on top of the recent
version of U-Boot git master branch?

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);
> +}
...
> 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.