Re: [PATCH v2 2/2] cmd: brcm: netXtreme commands

On 10/23/21 01:23, Roman Bacik wrote:

From: Bharat Gooty bharat.gooty@broadcom.com

Following netXtreme commands are supported:- Device probe, remove, supported speeds, get/set speeds and get/set MAC address.

Signed-off-by: Bharat Gooty bharat.gooty@broadcom.com

Signed-off-by: Roman Bacik roman.bacik@broadcom.com

Please, add a man-page for the new command in doc/usage/. Here is an example: doc/usage/loady.rst Add the new man-page to doc/usage/index.rst Test building with 'make htmldocs'.

---

(no changes since v1)

cmd/Kconfig | 2 + cmd/broadcom/Kconfig | 10 ++ cmd/broadcom/Makefile | 3 +- cmd/broadcom/bnxt.c | 237 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 251 insertions(+), 1 deletion(-) create mode 100644 cmd/broadcom/Kconfig create mode 100644 cmd/broadcom/bnxt.c

diff --git a/cmd/Kconfig b/cmd/Kconfig index 5b30b13e438f..e054292dbcd0 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1953,6 +1953,8 @@ endmenu

source "cmd/ti/Kconfig"

+source "cmd/broadcom/Kconfig"

• config CMD_BOOTSTAGE bool "Enable the 'bootstage' command" depends on BOOTSTAGE

diff --git a/cmd/broadcom/Kconfig b/cmd/broadcom/Kconfig new file mode 100644 index 000000000000..6f16b09d1425 --- /dev/null +++ b/cmd/broadcom/Kconfig @@ -0,0 +1,10 @@ +menu "Broadcom specific command line interface"

+config BNXT_ETH_CMD

• bool "BNXT commands"
• depends on BNXT_ETH
• help

•  Broadcom NXS ethernet controller commands. Commands supported are:-
  

•  Driver probe, Driver remove, Supported speeds, get/set MAC address and get/set Link speeds.
  

+endmenu diff --git a/cmd/broadcom/Makefile b/cmd/broadcom/Makefile index 62268d98d0dd..0027c1c15e5a 100644 --- a/cmd/broadcom/Makefile +++ b/cmd/broadcom/Makefile @@ -1,6 +1,7 @@ # SPDX-License-Identifier: GPL-2.0+ -# Copyright 2020 Broadcom +# Copyright 2020-2021 Broadcom

obj-y += chimp_boot.o obj-y += nitro_image_load.o obj-y += chimp_handshake.o +obj-$(CONFIG_BNXT_ETH_CMD) += bnxt.o diff --git a/cmd/broadcom/bnxt.c b/cmd/broadcom/bnxt.c new file mode 100644 index 000000000000..b9d1e59a74fb --- /dev/null +++ b/cmd/broadcom/bnxt.c @@ -0,0 +1,237 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Copyright 2021 Broadcom
• */

+#include <common.h> +#include <command.h>

+#include <asm/io.h> +#include <pci.h> +#include <broadcom/bnxt.h> +#include <broadcom/bnxt_ver.h> +#include <broadcom/bnxt_hsi.h> +#include <dm.h> +#include <env_flags.h> +#include <env_internal.h> +#include <errno.h> +#include <linux/if_ether.h> +#include <net.h> +#include <dm/device-internal.h> +#include <linux/ctype.h> +#include <linux/delay.h> +#include <linux/types.h>

+static int do_bnxt_set_link(struct bnxt *bp, char *link_str) +{

• bp->link_set = simple_strtoul(link_str, NULL, 16);
• switch (bp->link_set) {
• case LINK_SPEED_DRV_AUTONEG:

• printf("- AutoNeg Not Supported\n");
  

• return 0;
  

Please, remove the leading '- '. It just increases code size. In case of an error, please, return CMD_RET_FAILURE. Please, remove captitalization of 'Not Supported'

• case LINK_SPEED_DRV_1G:

• if (!(bp->support_speeds & PORT_QCFG_SUPPORT_SPEEDS_1GB)) {
  

• 	printf("- 1 GBPS: Link Speed is not supported\n");
  

ditto

• 	return 0;
  

• }
  

• break;
  

• case LINK_SPEED_DRV_10G:

• if (!(bp->support_speeds & PORT_QCFG_SUPPORT_SPEEDS_10GB)) {
  

• 	printf("- 10 GBPS: Link Speed is not supported\n");
  

• 	return 0;
  

ditto

• }
  

• break;
  

• case LINK_SPEED_DRV_25G:

• if (!(bp->support_speeds & PORT_QCFG_SUPPORT_SPEEDS_25GB)) {
  

• 	printf("- 25 GBPS: Link Speed is not supported\n");
  

• 	return 0;
  

ditto

• }
  

• break;
  

• case LINK_SPEED_DRV_40G:

• printf("- 40 GBPS Not Supported\n");
  

• return 0;
  

ditto

• case LINK_SPEED_DRV_50G:

• if (!(bp->support_speeds & PORT_QCFG_SUPPORT_SPEEDS_50GB)) {
  

• 	printf("- 50 GBPS: Link Speed is not supported\n");
  

• 	return 0;
  

ditto

• }
  

• break;
  

• case LINK_SPEED_DRV_100G:

• if (!(bp->support_speeds & PORT_QCFG_SUPPORT_SPEEDS_100GB)) {
  

• 	printf("- 100 GBPS: Link Speed is not supported\n");
  

• 	return 0;
  

ditto

• }
  

• break;
  

• case LINK_SPEED_DRV_200G:

• if (!(bp->support_speeds & PORT_QCFG_SUPPORT_SPEEDS_200GB)) {
  

• 	printf("- 200 GBPS: Link Speed is not supported\n");
  

• 	return 0;
  

ditto

• }
  

• break;
  

• case LINK_SPEED_DRV_2_5G:

• if (!(bp->support_speeds & PORT_QCFG_SUPPORT_SPEEDS_2_5GB)) {
  

• 	printf("- 2.5 GBPS: Link Speed is not supported\n");
  

ditto

• 	return 0;
  

• }
  

• break;
  

• case LINK_SPEED_DRV_100M:

• if (!(bp->support_speeds & PORT_QCFG_SUPPORT_SPEEDS_100MB)) {
  

• 	printf("- 100 MBPS: Link Speed is not supported\n");
  

• 	return 0;
  

ditto

• }
  

• break;
  

• default:

• printf("- Invalid Link Speed specified\n");
  

• return CMD_RET_USAGE;
  

• }
• prn_link_speed(bp->link_set, 1);
• return bnxt_set_link_speed(bp);

+}

+static int (struct bnxt *bp, char *mac_str) +{

• struct eth_pdata *plat = dev_get_plat(bp->pdev);
• u8 addr[ETH_ALEN];
• int ret = CMD_RET_USAGE;
• if (eth_validate_ethaddr_str(mac_str)) {

• printf("Invalid MAC Address %s\n", mac_str);
  

• return 0;
  

return CMD_RET_FAILURE

%s/Invalid MAC Address/Invalid MAC address/

• }
• string_to_enetaddr(mac_str, &addr[0]);
• if (!is_valid_ethaddr(&addr[0])) {

• printf("- Warning: Invalid MAC address to set\n");
  

Please, be consistent.

%s/- Warning: //

• return 0;
  

return CMD_RET_FAILURE

• }
• memcpy(bp->mac_set, addr, ETH_ALEN);
• print_mac(&bp->mac_set[0], 1);
• ret = bnxt_set_mac(bp);
• if (ret)

• return ret;
  

• bnxt_hwrm_nvm_flush(bp);
• /* copy ROM MAC to environment. */
• memcpy(plat->enetaddr, bp->mac_set, ETH_ALEN);
• bnxt_env_set_ethaddr(bp->pdev);
• return CMD_RET_SUCCESS;

+}

+static int print_drv(u8 flag) +{

• printf("bnxt - Device ");
• if (flag)

• printf("already");
  

• else

• printf("not");
  

• printf(" initialized\n");
• return CMD_RET_SUCCESS;

+}

+static int bnxt_parse_input(int argc, char *const argv[]) +{

• if (!strcmp(argv[2], "probe")) {

• return CMD_PROBE;
  

• } else if (!strcmp(argv[2], "remove")) {

• return CMD_REMOVE;
  

• } else if (!strcmp(argv[2], "get")) {

• if (!strncmp(argv[3], "supported_speed", 16))
  

• 	return CMD_GET_SUPPORTED_SPEED;
  

• else if (!strncmp(argv[3], "link_speed", 11))
  

• 	return CMD_GET_LINK_SPEED;
  

• else if (!strncmp(argv[3], "mac", 4))
  

• 	return CMD_GET_MAC;
  

• }
• return CMD_UNKNOWN;

+}

+static int do_bnxt(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) +{

Please, implement the subcommands as described in doc/develop/commands.rst. https://u-boot.readthedocs.io/en/latest/develop/commands.html This avoids reinventing the wheel and reduces code size.

• struct udevice *dev;
• struct bnxt *bp;
• char name[30];
• int ret = CMD_RET_USAGE;
• int cardnum;
• int op;
• printf("\n");
• if (argc < 2)

• return ret;
  

• cardnum = simple_strtoul(argv[1], NULL, 10);
• sprintf(name, "bnxt_eth%u", cardnum);
• ret = uclass_get_device_by_name(UCLASS_ETH, name, &dev);
• if (ret)

• return CMD_RET_USAGE;
  

• bp = dev_get_priv(dev);
• op = bnxt_parse_input(argc, argv);
• ret = CMD_RET_USAGE;
• switch (op) {
• case CMD_PROBE:

• if (!bp->drv_load)
  

• 	ret = bnxt_drv_probe(dev);
  

• else
  

• 	ret = print_drv(1);
  

• break;
  

• case CMD_REMOVE:

• ret = bnxt_drv_remove(dev);
  

• break;
  

• case CMD_GET_SUPPORTED_SPEED:

• ret = bnxt_supported_speed(bp);
  

• break;
  

• case CMD_GET_MAC:

• ret = bnxt_get_mac(bp);
  

• break;
  

• case CMD_GET_LINK_SPEED:

• ret = bnxt_get_link_speed(bp);
  

• break;
  

• case CMD_SET_MAC:

• ret = do_bnxt_set_mac(bp, argv[4]);
  

• break;
  

• case CMD_SET_LINK_SPEED:

• ret = do_bnxt_set_link(bp, argv[4]);
  

• break;
  

• default:

• if (op && !bp->drv_load)
  

• 	ret = print_drv(0);
  

• }
• printf("\n");
• return ret;

+}

+U_BOOT_CMD

• (bnxt, 5, 1, do_bnxt,
• "Broadcom NetXtreme-C/E Management",
• /* */"<bnxt_eth#> probe\n"
• " - Load Driver Instance.\n"

Please, remove capitalization and period at end of enumberation lines. Saves us a few bytes.

Best regards

Heinrich

• "bnxt <bnxt_eth#> remove\n"
• " - Unload Driver Instance.\n"
• "bnxt <bnxt_eth#> get supported_speed\n"
• " - Get Supported Link Speeds.\n"
• "bnxt <bnxt_eth#> get link_speed\n"
• " - Get Current Link Speed.\n"
• "bnxt <bnxt_eth#> get mac\n"
• " - Get MAC Address.\n"

+);