Re: [RFC net-next 1/3] net: marvell: prestera: Add Switchdev driver for Prestera family ASIC device 98DX325x (AC3x)
From: Jiri Pirko <jiri@resnulli.us>
Date: 2020-02-26 15:54:32
Also in:
lkml
Tue, Feb 25, 2020 at 05:30:54PM CET, vadym.kochan@plvision.eu wrote:
quoted hunk ↗ jump to hunk
Marvell Prestera 98DX326x integrates up to 24 ports of 1GbE with 8 ports of 10GbE uplinks or 2 ports of 40Gbps stacking for a largely wireless SMB deployment. This driver implementation includes only L1 & basic L2 support. The core Prestera switching logic is implemented in prestera.c, there is an intermediate hw layer between core logic and firmware. It is implemented in prestera_hw.c, the purpose of it is to encapsulate hw related logic, in future there is a plan to support more devices with different HW related configurations. The following Switchdev features are supported: - VLAN-aware bridge offloading - VLAN-unaware bridge offloading - FDB offloading (learning, ageing) - Switchport configuration Signed-off-by: Vadym Kochan <redacted> Signed-off-by: Andrii Savka <redacted> Signed-off-by: Oleksandr Mazur <redacted> Signed-off-by: Serhiy Boiko <redacted> Signed-off-by: Serhiy Pshyk <redacted> Signed-off-by: Taras Chornyi <redacted> Signed-off-by: Volodymyr Mytnyk <redacted> --- drivers/net/ethernet/marvell/Kconfig | 1 + drivers/net/ethernet/marvell/Makefile | 1 + drivers/net/ethernet/marvell/prestera/Kconfig | 13 + .../net/ethernet/marvell/prestera/Makefile | 3 + .../net/ethernet/marvell/prestera/prestera.c | 1502 +++++++++++++++++ .../net/ethernet/marvell/prestera/prestera.h | 244 +++ .../marvell/prestera/prestera_drv_ver.h | 23 + .../ethernet/marvell/prestera/prestera_hw.c | 1094 ++++++++++++ .../ethernet/marvell/prestera/prestera_hw.h | 159 ++ .../marvell/prestera/prestera_switchdev.c | 1217 +++++++++++++ 10 files changed, 4257 insertions(+) create mode 100644 drivers/net/ethernet/marvell/prestera/Kconfig create mode 100644 drivers/net/ethernet/marvell/prestera/Makefile create mode 100644 drivers/net/ethernet/marvell/prestera/prestera.c create mode 100644 drivers/net/ethernet/marvell/prestera/prestera.h create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_drv_ver.h create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_hw.c create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_hw.h create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_switchdev.cdiff --git a/drivers/net/ethernet/marvell/Kconfig b/drivers/net/ethernet/marvell/Kconfig index 3d5caea096fb..74313d9e1fc0 100644 --- a/drivers/net/ethernet/marvell/Kconfig +++ b/drivers/net/ethernet/marvell/Kconfig@@ -171,5 +171,6 @@ config SKY2_DEBUGsource "drivers/net/ethernet/marvell/octeontx2/Kconfig" +source "drivers/net/ethernet/marvell/prestera/Kconfig" endif # NET_VENDOR_MARVELLdiff --git a/drivers/net/ethernet/marvell/Makefile b/drivers/net/ethernet/marvell/Makefile index 89dea7284d5b..9f88fe822555 100644 --- a/drivers/net/ethernet/marvell/Makefile +++ b/drivers/net/ethernet/marvell/Makefile@@ -12,3 +12,4 @@ obj-$(CONFIG_PXA168_ETH) += pxa168_eth.oobj-$(CONFIG_SKGE) += skge.o obj-$(CONFIG_SKY2) += sky2.o obj-y += octeontx2/ +obj-y += prestera/diff --git a/drivers/net/ethernet/marvell/prestera/Kconfig b/drivers/net/ethernet/marvell/prestera/Kconfig new file mode 100644 index 000000000000..d0b416dcb677 --- /dev/null +++ b/drivers/net/ethernet/marvell/prestera/Kconfig@@ -0,0 +1,13 @@ +# SPDX-License-Identifier: GPL-2.0-only +# +# Marvell Prestera drivers configuration +# + +config PRESTERA + tristate "Marvell Prestera Switch ASICs support" + depends on NET_SWITCHDEV && VLAN_8021Q + ---help--- + This driver supports Marvell Prestera Switch ASICs family. + + To compile this driver as a module, choose M here: the + module will be called prestera_sw.diff --git a/drivers/net/ethernet/marvell/prestera/Makefile b/drivers/net/ethernet/marvell/prestera/Makefile new file mode 100644 index 000000000000..9446298fb7f4 --- /dev/null +++ b/drivers/net/ethernet/marvell/prestera/Makefile@@ -0,0 +1,3 @@ +# SPDX-License-Identifier: GPL-2.0 +obj-$(CONFIG_PRESTERA) += prestera_sw.o +prestera_sw-objs := prestera.o prestera_hw.o prestera_switchdev.odiff --git a/drivers/net/ethernet/marvell/prestera/prestera.c b/drivers/net/ethernet/marvell/prestera/prestera.c new file mode 100644 index 000000000000..12d0eb590bbb --- /dev/null +++ b/drivers/net/ethernet/marvell/prestera/prestera.c@@ -0,0 +1,1502 @@ +/* SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0 + * + * Copyright (c) 2019-2020 Marvell International Ltd. All rights reserved. + * + */ +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/list.h> +#include <linux/netdevice.h> +#include <linux/netdev_features.h> +#include <linux/etherdevice.h> +#include <linux/ethtool.h> +#include <linux/jiffies.h> +#include <net/switchdev.h> + +#include "prestera.h" +#include "prestera_hw.h" +#include "prestera_drv_ver.h" + +#define MVSW_PR_MTU_DEFAULT 1536 + +#define PORT_STATS_CACHE_TIMEOUT_MS (msecs_to_jiffies(1000)) +#define PORT_STATS_CNT (sizeof(struct mvsw_pr_port_stats) / sizeof(u64))
Keep the prefix for all defines withing the file. "PORT_STATS_CNT" looks way to generic on the first look.
+#define PORT_STATS_IDX(name) \ + (offsetof(struct mvsw_pr_port_stats, name) / sizeof(u64)) +#define PORT_STATS_FIELD(name) \ + [PORT_STATS_IDX(name)] = __stringify(name) + +static struct list_head switches_registered; + +static const char mvsw_driver_kind[] = "prestera_sw";
Please be consistent. Make your prefixes, name, filenames the same. For example: prestera_driver_kind[] = "prestera"; Applied to the whole code.
+static const char mvsw_driver_name[] = "mvsw_switchdev";
Why is this different from kind? Also, don't mention "switchdev" anywhere.
+static const char mvsw_driver_version[] = PRESTERA_DRV_VER;
[...]
+static void mvsw_pr_port_remote_cap_get(struct ethtool_link_ksettings *ecmd,
+ struct mvsw_pr_port *port)
+{
+ u64 bitmap;
+
+ if (!mvsw_pr_hw_port_remote_cap_get(port, &bitmap)) {
+ mvsw_modes_to_eth(ecmd->link_modes.lp_advertising,
+ bitmap, 0, MVSW_PORT_TYPE_NONE);
+ }
Don't use {} for single statement. checkpatch.pl should warn you about
this.
+}
+
+static void mvsw_pr_port_duplex_get(struct ethtool_link_ksettings *ecmd,
+ struct mvsw_pr_port *port)
+{
+ u8 duplex;
+
+ if (!mvsw_pr_hw_port_duplex_get(port, &duplex)) {
+ ecmd->base.duplex = duplex == MVSW_PORT_DUPLEX_FULL ?
+ DUPLEX_FULL : DUPLEX_HALF;
+ } else {
+ ecmd->base.duplex = DUPLEX_UNKNOWN;
+ }Same here.
+}
[...]
+static void __exit mvsw_pr_module_exit(void)
+{
+ destroy_workqueue(mvsw_pr_wq);
+
+ pr_info("Unloading Marvell Prestera Switch Driver\n");No prints like this please.
+}
+
+module_init(mvsw_pr_module_init);
+module_exit(mvsw_pr_module_exit);
+
+MODULE_AUTHOR("Marvell Semi.");Does not look so :)
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Marvell Prestera switch driver");
+MODULE_VERSION(PRESTERA_DRV_VER);[...]