diff mbox series

[v2,2/2] cmd: brcm: netXtreme commands

Message ID 20211022162222.v2.2.I1edaad77041c1300213c307eef6741499504047@changeid
State Superseded
Headers show
Series [v2,1/2] net: brcm: netXtreme driver | expand

Commit Message

Roman Bacik Oct. 22, 2021, 11:23 p.m. UTC
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>
---

(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

Comments

Heinrich Schuchardt Oct. 23, 2021, 8:02 a.m. UTC | #1
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"
> +);
>
Marek BehĂșn Oct. 25, 2021, 2:01 p.m. UTC | #2
On Fri, 22 Oct 2021 16:23:33 -0700
Roman Bacik <roman.bacik@broadcom.com> 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.

NAK.

- "bnxt <bnxt_eth#> get mac

  U-Boot uses the ethaddr and ethNaddr environment variables for MAC
  addresses. You don't need a new custom command for that when there is
  a generic mechanism for this.

- "<bnxt_eth#> probe" / "<bnxt_eth#> remove"

  You also shouldn't need a command for driver probe / remove. DM should
  probe your driver automatically. And if you need it for debugging,
  please add such subcommand to the dm command.

- "bnxt <bnxt_eth#> get supported_speed"
  "bnxt <bnxt_eth#> get link_speed"

  These should be available via the mdio command when you register your
  PHY driver via appropriate API.

Marek
Roman Bacik Oct. 25, 2021, 4:34 p.m. UTC | #3
> -----Original Message-----
> From: Marek BehĂșn <kabel@kernel.org>
> Sent: Monday, October 25, 2021 7:01 AM
> To: Roman Bacik <roman.bacik@broadcom.com>
> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Bharat Gooty
> <bharat.gooty@broadcom.com>; Bin Meng <bmeng.cn@gmail.com>; Franck
> LENORMAND <franck.lenormand@nxp.com>; Heinrich Schuchardt
> <xypron.glpk@gmx.de>; Kory Maincent <kory.maincent@bootlin.com>;
> Michal Simek <michal.simek@xilinx.com>; Patrick Delaunay
> <patrick.delaunay@foss.st.com>; Peng Fan <peng.fan@nxp.com>; Priyanka
> Jain <priyanka.jain@nxp.com>; Rayagonda Kokatanur
> <rayagonda.kokatanur@broadcom.com>; Sean Anderson
> <sean.anderson@seco.com>; Simon Glass <sjg@chromium.org>
> Subject: Re: [PATCH v2 2/2] cmd: brcm: netXtreme commands
>
> On Fri, 22 Oct 2021 16:23:33 -0700
> Roman Bacik <roman.bacik@broadcom.com> 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.
>
> NAK.
>
> - "bnxt <bnxt_eth#> get mac
>
>   U-Boot uses the ethaddr and ethNaddr environment variables for MAC
>   addresses. You don't need a new custom command for that when there is
>   a generic mechanism for this.
>
> - "<bnxt_eth#> probe" / "<bnxt_eth#> remove"
>
>   You also shouldn't need a command for driver probe / remove. DM should
>   probe your driver automatically. And if you need it for debugging,
>   please add such subcommand to the dm command.
>
> - "bnxt <bnxt_eth#> get supported_speed"
>   "bnxt <bnxt_eth#> get link_speed"
>
>   These should be available via the mdio command when you register your
>   PHY driver via appropriate API.
>
> Marek

Hi Marek,

Thank you very much for your feedback. We have two Ethernet drivers. One
is 10/100/1000 MB rmii driver, which is being used as you have described.
The second one is this 10/100 GB bnxt driver, which is probed and managed
on demand with these netXtreme commands. We will have a look and address
your comments.
Thanks,

Roman
Roman Bacik Oct. 25, 2021, 4:54 p.m. UTC | #4
> -----Original Message-----
> From: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Sent: Saturday, October 23, 2021 1:02 AM
> To: Roman Bacik <roman.bacik@broadcom.com>; U-Boot Mailing List <u-
> boot@lists.denx.de>
> Cc: Bharat Gooty <bharat.gooty@broadcom.com>; Bin Meng
> <bmeng.cn@gmail.com>; Franck LENORMAND
> <franck.lenormand@nxp.com>; Kory Maincent
> <kory.maincent@bootlin.com>; Michal Simek <michal.simek@xilinx.com>;
> Patrick Delaunay <patrick.delaunay@foss.st.com>; Peng Fan
> <peng.fan@nxp.com>; Priyanka Jain <priyanka.jain@nxp.com>; Rayagonda
> Kokatanur <rayagonda.kokatanur@broadcom.com>; Sean Anderson
> <sean.anderson@seco.com>; Simon Glass <sjg@chromium.org>
> Subject: 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

Heinrich,

We will address your comments in the next version.
Thank you very much for your review,

Roman

>
> > +	"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"
> > +);
> >
diff mbox series

Patch

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;
+	case LINK_SPEED_DRV_1G:
+		if (!(bp->support_speeds & PORT_QCFG_SUPPORT_SPEEDS_1GB)) {
+			printf("- 1 GBPS: Link Speed is not supported\n");
+			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;
+		}
+
+		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;
+		}
+
+		break;
+	case LINK_SPEED_DRV_40G:
+		printf("- 40 GBPS Not Supported\n");
+		return 0;
+	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;
+		}
+
+		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;
+		}
+
+		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;
+		}
+
+		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");
+			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;
+		}
+
+		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 do_bnxt_set_mac(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;
+	}
+
+	string_to_enetaddr(mac_str, &addr[0]);
+
+	if (!is_valid_ethaddr(&addr[0])) {
+		printf("- Warning: Invalid MAC address to set\n");
+		return 0;
+	}
+
+	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[])
+{
+	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"
+	"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"
+);