All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next 0/3] Marvell Prestera Switchdev initial updates for
@ 2021-04-23 15:59 Vadym Kochan
  2021-04-23 15:59 ` [PATCH net-next 1/3] net: marvell: prestera: bump supported firmware version to 3.0 Vadym Kochan
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Vadym Kochan @ 2021-04-23 15:59 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, netdev
  Cc: Vadym Kochan, Taras Chornyi, linux-kernel, Mickey Rachamim, Vadym Kochan

From: Vadym Kochan <vkochan@marvell.com>

This series adds minimal support for firmware version 3.0 which
has such changes like:

    - initial routing support

    - LAG support

    - events interrupt handling changes

Changes just make able to work with new firmware version but
supported features in driver will be added later.

New firmware version was recently merged into linux-firmware tree.

Vadym Kochan (3):
  net: marvell: prestera: bump supported firmware version to 3.0
  net: marvell: prestera: disable events interrupt while handling
  net: marvell: prestera: align flood setting according to latest
    firmware version

 .../ethernet/marvell/prestera/prestera_hw.c   | 37 ++++++++++++--
 .../ethernet/marvell/prestera/prestera_hw.h   |  3 +-
 .../ethernet/marvell/prestera/prestera_pci.c  | 21 +++++++-
 .../marvell/prestera/prestera_switchdev.c     | 50 +++++++++++++++----
 4 files changed, 96 insertions(+), 15 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH net-next 1/3] net: marvell: prestera: bump supported firmware version to 3.0
  2021-04-23 15:59 [PATCH net-next 0/3] Marvell Prestera Switchdev initial updates for Vadym Kochan
@ 2021-04-23 15:59 ` Vadym Kochan
  2021-04-23 16:49   ` Andrew Lunn
  2021-04-23 15:59 ` [PATCH net-next 2/3] net: marvell: prestera: disable events interrupt while handling Vadym Kochan
  2021-04-23 15:59 ` [PATCH net-next 3/3] net: marvell: prestera: align flood setting according to latest firmware version Vadym Kochan
  2 siblings, 1 reply; 11+ messages in thread
From: Vadym Kochan @ 2021-04-23 15:59 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, netdev
  Cc: Vadym Kochan, Taras Chornyi, linux-kernel, Mickey Rachamim, Vadym Kochan

From: Vadym Kochan <vkochan@marvell.com>

New firmware version has some ABI and feature changes like:

    - LAG support
    - initial L3 support
    - changed events handling logic

Signed-off-by: Vadym Kochan <vkochan@marvell.com>
---
 drivers/net/ethernet/marvell/prestera/prestera_pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/prestera/prestera_pci.c b/drivers/net/ethernet/marvell/prestera/prestera_pci.c
index 298110119272..80fb5daf1da8 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_pci.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_pci.c
@@ -13,7 +13,7 @@
 
 #define PRESTERA_MSG_MAX_SIZE 1500
 
-#define PRESTERA_SUPP_FW_MAJ_VER	2
+#define PRESTERA_SUPP_FW_MAJ_VER	3
 #define PRESTERA_SUPP_FW_MIN_VER	0
 
 #define PRESTERA_FW_PATH_FMT	"mrvl/prestera/mvsw_prestera_fw-v%u.%u.img"
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH net-next 2/3] net: marvell: prestera: disable events interrupt while handling
  2021-04-23 15:59 [PATCH net-next 0/3] Marvell Prestera Switchdev initial updates for Vadym Kochan
  2021-04-23 15:59 ` [PATCH net-next 1/3] net: marvell: prestera: bump supported firmware version to 3.0 Vadym Kochan
@ 2021-04-23 15:59 ` Vadym Kochan
  2021-04-23 15:59 ` [PATCH net-next 3/3] net: marvell: prestera: align flood setting according to latest firmware version Vadym Kochan
  2 siblings, 0 replies; 11+ messages in thread
From: Vadym Kochan @ 2021-04-23 15:59 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, netdev
  Cc: Vadym Kochan, Taras Chornyi, linux-kernel, Mickey Rachamim, Vadym Kochan

From: Vadym Kochan <vkochan@marvell.com>

There are change in firmware which requires that receiver will
disable event interrupts before handling them and enable them
after finish with handling. Events still may come into the queue
but without receiver interruption.

Signed-off-by: Vadym Kochan <vkochan@marvell.com>
---
 .../ethernet/marvell/prestera/prestera_pci.c  | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/net/ethernet/marvell/prestera/prestera_pci.c b/drivers/net/ethernet/marvell/prestera/prestera_pci.c
index 80fb5daf1da8..5edd4d2ac672 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_pci.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_pci.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0
 /* Copyright (c) 2019-2020 Marvell International Ltd. All rights reserved */
 
+#include <linux/bitfield.h>
 #include <linux/circ_buf.h>
 #include <linux/device.h>
 #include <linux/firmware.h>
@@ -144,6 +145,11 @@ struct prestera_fw_regs {
 /* PRESTERA_CMD_RCV_CTL_REG flags */
 #define PRESTERA_CMD_F_REPL_SENT	BIT(0)
 
+#define PRESTERA_FW_EVT_CTL_STATUS_MASK	GENMASK(1, 0)
+
+#define PRESTERA_FW_EVT_CTL_STATUS_ON	0
+#define PRESTERA_FW_EVT_CTL_STATUS_OFF	1
+
 #define PRESTERA_EVTQ_REG_OFFSET(q, f)			\
 	(PRESTERA_FW_REG_OFFSET(evtq_list) +		\
 	 (q) * sizeof(struct prestera_fw_evtq_regs) +	\
@@ -260,6 +266,15 @@ static u8 prestera_fw_evtq_pick(struct prestera_fw *fw)
 	return PRESTERA_EVT_QNUM_MAX;
 }
 
+static void prestera_fw_evt_ctl_status_set(struct prestera_fw *fw, u32 val)
+{
+	u32 status = prestera_fw_read(fw, PRESTERA_FW_STATUS_REG);
+
+	u32p_replace_bits(&status, val, PRESTERA_FW_EVT_CTL_STATUS_MASK);
+
+	prestera_fw_write(fw, PRESTERA_FW_STATUS_REG, status);
+}
+
 static void prestera_fw_evt_work_fn(struct work_struct *work)
 {
 	struct prestera_fw *fw;
@@ -269,6 +284,8 @@ static void prestera_fw_evt_work_fn(struct work_struct *work)
 	fw = container_of(work, struct prestera_fw, evt_work);
 	msg = fw->evt_msg;
 
+	prestera_fw_evt_ctl_status_set(fw, PRESTERA_FW_EVT_CTL_STATUS_OFF);
+
 	while ((qid = prestera_fw_evtq_pick(fw)) < PRESTERA_EVT_QNUM_MAX) {
 		u32 idx;
 		u32 len;
@@ -288,6 +305,8 @@ static void prestera_fw_evt_work_fn(struct work_struct *work)
 		if (fw->dev.recv_msg)
 			fw->dev.recv_msg(&fw->dev, msg, len);
 	}
+
+	prestera_fw_evt_ctl_status_set(fw, PRESTERA_FW_EVT_CTL_STATUS_ON);
 }
 
 static int prestera_fw_wait_reg32(struct prestera_fw *fw, u32 reg, u32 cmp,
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH net-next 3/3] net: marvell: prestera: align flood setting according to latest firmware version
  2021-04-23 15:59 [PATCH net-next 0/3] Marvell Prestera Switchdev initial updates for Vadym Kochan
  2021-04-23 15:59 ` [PATCH net-next 1/3] net: marvell: prestera: bump supported firmware version to 3.0 Vadym Kochan
  2021-04-23 15:59 ` [PATCH net-next 2/3] net: marvell: prestera: disable events interrupt while handling Vadym Kochan
@ 2021-04-23 15:59 ` Vadym Kochan
  2 siblings, 0 replies; 11+ messages in thread
From: Vadym Kochan @ 2021-04-23 15:59 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, netdev
  Cc: Vadym Kochan, Taras Chornyi, linux-kernel, Mickey Rachamim, Vadym Kochan

From: Vadym Kochan <vkochan@marvell.com>

Latest FW IPC flood message format was changed to configure uc/mc
flooding separately, so change code according to this.

Signed-off-by: Vadym Kochan <vkochan@marvell.com>
---
 .../ethernet/marvell/prestera/prestera_hw.c   | 37 ++++++++++++--
 .../ethernet/marvell/prestera/prestera_hw.h   |  3 +-
 .../marvell/prestera/prestera_switchdev.c     | 50 +++++++++++++++----
 3 files changed, 76 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/marvell/prestera/prestera_hw.c b/drivers/net/ethernet/marvell/prestera/prestera_hw.c
index 0424718d5998..4afef6e14db3 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_hw.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_hw.c
@@ -85,6 +85,11 @@ enum {
 	PRESTERA_PORT_TP_AUTO,
 };
 
+enum {
+	PRESTERA_PORT_FLOOD_TYPE_UC = 0,
+	PRESTERA_PORT_FLOOD_TYPE_MC = 1,
+};
+
 enum {
 	PRESTERA_PORT_GOOD_OCTETS_RCV_CNT,
 	PRESTERA_PORT_BAD_OCTETS_RCV_CNT,
@@ -188,6 +193,11 @@ struct prestera_msg_port_mdix_param {
 	u8 admin_mode;
 };
 
+struct prestera_msg_port_flood_param {
+	u8 type;
+	u8 enable;
+};
+
 union prestera_msg_port_param {
 	u8  admin_state;
 	u8  oper_state;
@@ -196,7 +206,6 @@ union prestera_msg_port_param {
 	u8  accept_frm_type;
 	u32 speed;
 	u8 learning;
-	u8 flood;
 	u32 link_mode;
 	u8  type;
 	u8  duplex;
@@ -205,6 +214,7 @@ union prestera_msg_port_param {
 	struct prestera_msg_port_mdix_param mdix;
 	struct prestera_msg_port_autoneg_param autoneg;
 	struct prestera_msg_port_cap_param cap;
+	struct prestera_msg_port_flood_param flood;
 };
 
 struct prestera_msg_port_attr_req {
@@ -988,14 +998,35 @@ int prestera_hw_port_learning_set(struct prestera_port *port, bool enable)
 			    &req.cmd, sizeof(req));
 }
 
-int prestera_hw_port_flood_set(struct prestera_port *port, bool flood)
+int prestera_hw_port_uc_flood_set(struct prestera_port *port, bool flood)
+{
+	struct prestera_msg_port_attr_req req = {
+		.attr = PRESTERA_CMD_PORT_ATTR_FLOOD,
+		.port = port->hw_id,
+		.dev = port->dev_id,
+		.param = {
+			.flood = {
+				.type = PRESTERA_PORT_FLOOD_TYPE_UC,
+				.enable = flood,
+			}
+		}
+	};
+
+	return prestera_cmd(port->sw, PRESTERA_CMD_TYPE_PORT_ATTR_SET,
+			    &req.cmd, sizeof(req));
+}
+
+int prestera_hw_port_mc_flood_set(struct prestera_port *port, bool flood)
 {
 	struct prestera_msg_port_attr_req req = {
 		.attr = PRESTERA_CMD_PORT_ATTR_FLOOD,
 		.port = port->hw_id,
 		.dev = port->dev_id,
 		.param = {
-			.flood = flood,
+			.flood = {
+				.type = PRESTERA_PORT_FLOOD_TYPE_MC,
+				.enable = flood,
+			}
 		}
 	};
 
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_hw.h b/drivers/net/ethernet/marvell/prestera/prestera_hw.h
index b2b5ac95b4e3..109a677951cc 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_hw.h
+++ b/drivers/net/ethernet/marvell/prestera/prestera_hw.h
@@ -138,7 +138,8 @@ int prestera_hw_port_mdix_get(const struct prestera_port *port, u8 *status,
 int prestera_hw_port_mdix_set(const struct prestera_port *port, u8 mode);
 int prestera_hw_port_speed_get(const struct prestera_port *port, u32 *speed);
 int prestera_hw_port_learning_set(struct prestera_port *port, bool enable);
-int prestera_hw_port_flood_set(struct prestera_port *port, bool flood);
+int prestera_hw_port_uc_flood_set(struct prestera_port *port, bool flood);
+int prestera_hw_port_mc_flood_set(struct prestera_port *port, bool flood);
 int prestera_hw_port_accept_frm_type(struct prestera_port *port,
 				     enum prestera_accept_frm_type type);
 /* Vlan API */
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_switchdev.c b/drivers/net/ethernet/marvell/prestera/prestera_switchdev.c
index cb564890a3dc..f615ebe683f0 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_switchdev.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_switchdev.c
@@ -404,9 +404,13 @@ prestera_bridge_1d_port_join(struct prestera_bridge_port *br_port)
 	if (err)
 		return err;
 
-	err = prestera_hw_port_flood_set(port, br_port->flags & BR_FLOOD);
+	err = prestera_hw_port_uc_flood_set(port, br_port->flags & BR_FLOOD);
 	if (err)
-		goto err_port_flood_set;
+		goto err_port_uc_flood_set;
+
+	err = prestera_hw_port_mc_flood_set(port, br_port->flags & BR_MCAST_FLOOD);
+	if (err)
+		goto err_port_mc_flood_set;
 
 	err = prestera_hw_port_learning_set(port, br_port->flags & BR_LEARNING);
 	if (err)
@@ -415,8 +419,10 @@ prestera_bridge_1d_port_join(struct prestera_bridge_port *br_port)
 	return 0;
 
 err_port_learning_set:
-	prestera_hw_port_flood_set(port, false);
-err_port_flood_set:
+	prestera_hw_port_mc_flood_set(port, false);
+err_port_mc_flood_set:
+	prestera_hw_port_uc_flood_set(port, false);
+err_port_uc_flood_set:
 	prestera_hw_bridge_port_delete(port, bridge->bridge_id);
 
 	return err;
@@ -528,7 +534,8 @@ static void prestera_port_bridge_leave(struct prestera_port *port,
 		prestera_bridge_1d_port_leave(br_port);
 
 	prestera_hw_port_learning_set(port, false);
-	prestera_hw_port_flood_set(port, false);
+	prestera_hw_port_uc_flood_set(port, false);
+	prestera_hw_port_mc_flood_set(port, false);
 	prestera_port_vid_stp_set(port, PRESTERA_VID_ALL, BR_STATE_FORWARDING);
 	prestera_bridge_port_put(br_port);
 }
@@ -591,21 +598,36 @@ static int prestera_port_attr_br_flags_set(struct prestera_port *port,
 		return 0;
 
 	if (flags.mask & BR_FLOOD) {
-		err = prestera_hw_port_flood_set(port, flags.val & BR_FLOOD);
+		err = prestera_hw_port_uc_flood_set(port,
+						    flags.val & BR_FLOOD);
+		if (err)
+			goto err_port_uc_flood_set;
+	}
+
+	if (flags.mask & BR_MCAST_FLOOD) {
+		err = prestera_hw_port_mc_flood_set(port,
+						    flags.val & BR_MCAST_FLOOD);
 		if (err)
-			return err;
+			goto err_port_mc_flood_set;
 	}
 
 	if (flags.mask & BR_LEARNING) {
 		err = prestera_hw_port_learning_set(port,
 						    flags.val & BR_LEARNING);
 		if (err)
-			return err;
+			goto err_port_learning_set;
 	}
 
 	memcpy(&br_port->flags, &flags.val, sizeof(flags.val));
 
 	return 0;
+
+err_port_learning_set:
+	prestera_hw_port_mc_flood_set(port, false);
+err_port_mc_flood_set:
+	prestera_hw_port_uc_flood_set(port, false);
+err_port_uc_flood_set:
+	return err;
 }
 
 static int prestera_port_attr_br_ageing_set(struct prestera_port *port,
@@ -901,9 +923,13 @@ prestera_port_vlan_bridge_join(struct prestera_port_vlan *port_vlan,
 	if (port_vlan->br_port)
 		return 0;
 
-	err = prestera_hw_port_flood_set(port, br_port->flags & BR_FLOOD);
+	err = prestera_hw_port_uc_flood_set(port, br_port->flags & BR_FLOOD);
 	if (err)
-		return err;
+		goto err_port_uc_flood_set;
+
+	err = prestera_hw_port_mc_flood_set(port, br_port->flags & BR_MCAST_FLOOD);
+	if (err)
+		goto err_port_mc_flood_set;
 
 	err = prestera_hw_port_learning_set(port, br_port->flags & BR_LEARNING);
 	if (err)
@@ -934,6 +960,10 @@ prestera_port_vlan_bridge_join(struct prestera_port_vlan *port_vlan,
 err_port_vid_stp_set:
 	prestera_hw_port_learning_set(port, false);
 err_port_learning_set:
+	prestera_hw_port_mc_flood_set(port, false);
+err_port_mc_flood_set:
+	prestera_hw_port_uc_flood_set(port, false);
+err_port_uc_flood_set:
 	return err;
 }
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next 1/3] net: marvell: prestera: bump supported firmware version to 3.0
  2021-04-23 15:59 ` [PATCH net-next 1/3] net: marvell: prestera: bump supported firmware version to 3.0 Vadym Kochan
@ 2021-04-23 16:49   ` Andrew Lunn
  2021-04-23 17:04     ` Vadym Kochan
  0 siblings, 1 reply; 11+ messages in thread
From: Andrew Lunn @ 2021-04-23 16:49 UTC (permalink / raw)
  To: Vadym Kochan
  Cc: David S. Miller, Jakub Kicinski, netdev, Taras Chornyi,
	linux-kernel, Mickey Rachamim, Vadym Kochan

On Fri, Apr 23, 2021 at 06:59:31PM +0300, Vadym Kochan wrote:
> From: Vadym Kochan <vkochan@marvell.com>
> 
> New firmware version has some ABI and feature changes like:
> 
>     - LAG support
>     - initial L3 support
>     - changed events handling logic
> 
> Signed-off-by: Vadym Kochan <vkochan@marvell.com>
> ---
>  drivers/net/ethernet/marvell/prestera/prestera_pci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_pci.c b/drivers/net/ethernet/marvell/prestera/prestera_pci.c
> index 298110119272..80fb5daf1da8 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_pci.c
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_pci.c
> @@ -13,7 +13,7 @@
>  
>  #define PRESTERA_MSG_MAX_SIZE 1500
>  
> -#define PRESTERA_SUPP_FW_MAJ_VER	2
> +#define PRESTERA_SUPP_FW_MAJ_VER	3
>  #define PRESTERA_SUPP_FW_MIN_VER	0

I could be reading the code wrong, but it looks like anybody with
firmware version 2 on their machine and this new driver version
results in the switch not probing? And if the switch does not probe,
do they have any networking to go get the new firmware version?

I think you need to provide some degree of backwards compatibly to
older firmware. Support version 2 and 3. When version 4 comes out,
drop support for version 2 in the driver etc.

     Andrew

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next 1/3] net: marvell: prestera: bump supported firmware version to 3.0
  2021-04-23 16:49   ` Andrew Lunn
@ 2021-04-23 17:04     ` Vadym Kochan
  2021-04-23 18:01       ` Andrew Lunn
  0 siblings, 1 reply; 11+ messages in thread
From: Vadym Kochan @ 2021-04-23 17:04 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: David S. Miller, Jakub Kicinski, netdev, Taras Chornyi,
	linux-kernel, Mickey Rachamim, Vadym Kochan

Hi Andrew,

On Fri, Apr 23, 2021 at 06:49:01PM +0200, Andrew Lunn wrote:
> On Fri, Apr 23, 2021 at 06:59:31PM +0300, Vadym Kochan wrote:
> > From: Vadym Kochan <vkochan@marvell.com>
> > 
> > New firmware version has some ABI and feature changes like:
> > 
> >     - LAG support
> >     - initial L3 support
> >     - changed events handling logic
> > 
> > Signed-off-by: Vadym Kochan <vkochan@marvell.com>
> > ---
> >  drivers/net/ethernet/marvell/prestera/prestera_pci.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/ethernet/marvell/prestera/prestera_pci.c b/drivers/net/ethernet/marvell/prestera/prestera_pci.c
> > index 298110119272..80fb5daf1da8 100644
> > --- a/drivers/net/ethernet/marvell/prestera/prestera_pci.c
> > +++ b/drivers/net/ethernet/marvell/prestera/prestera_pci.c
> > @@ -13,7 +13,7 @@
> >  
> >  #define PRESTERA_MSG_MAX_SIZE 1500
> >  
> > -#define PRESTERA_SUPP_FW_MAJ_VER	2
> > +#define PRESTERA_SUPP_FW_MAJ_VER	3
> >  #define PRESTERA_SUPP_FW_MIN_VER	0
> 
> I could be reading the code wrong, but it looks like anybody with
> firmware version 2 on their machine and this new driver version
> results in the switch not probing? And if the switch does not probe,
> do they have any networking to go get the new firmware version?
> 

Existing boards have management port which is separated from the PP.

> I think you need to provide some degree of backwards compatibly to
> older firmware. Support version 2 and 3. When version 4 comes out,
> drop support for version 2 in the driver etc.
> 
>      Andrew

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next 1/3] net: marvell: prestera: bump supported firmware version to 3.0
  2021-04-23 17:04     ` Vadym Kochan
@ 2021-04-23 18:01       ` Andrew Lunn
  2021-04-28 13:47         ` Vadym Kochan
  0 siblings, 1 reply; 11+ messages in thread
From: Andrew Lunn @ 2021-04-23 18:01 UTC (permalink / raw)
  To: Vadym Kochan
  Cc: David S. Miller, Jakub Kicinski, netdev, Taras Chornyi,
	linux-kernel, Mickey Rachamim, Vadym Kochan

On Fri, Apr 23, 2021 at 08:04:37PM +0300, Vadym Kochan wrote:
> Hi Andrew,
> 
> On Fri, Apr 23, 2021 at 06:49:01PM +0200, Andrew Lunn wrote:
> > On Fri, Apr 23, 2021 at 06:59:31PM +0300, Vadym Kochan wrote:
> > > From: Vadym Kochan <vkochan@marvell.com>
> > > 
> > > New firmware version has some ABI and feature changes like:
> > > 
> > >     - LAG support
> > >     - initial L3 support
> > >     - changed events handling logic
> > > 
> > > Signed-off-by: Vadym Kochan <vkochan@marvell.com>
> > > ---
> > >  drivers/net/ethernet/marvell/prestera/prestera_pci.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/net/ethernet/marvell/prestera/prestera_pci.c b/drivers/net/ethernet/marvell/prestera/prestera_pci.c
> > > index 298110119272..80fb5daf1da8 100644
> > > --- a/drivers/net/ethernet/marvell/prestera/prestera_pci.c
> > > +++ b/drivers/net/ethernet/marvell/prestera/prestera_pci.c
> > > @@ -13,7 +13,7 @@
> > >  
> > >  #define PRESTERA_MSG_MAX_SIZE 1500
> > >  
> > > -#define PRESTERA_SUPP_FW_MAJ_VER	2
> > > +#define PRESTERA_SUPP_FW_MAJ_VER	3
> > >  #define PRESTERA_SUPP_FW_MIN_VER	0
> > 
> > I could be reading the code wrong, but it looks like anybody with
> > firmware version 2 on their machine and this new driver version
> > results in the switch not probing? And if the switch does not probe,
> > do they have any networking to go get the new firmware version?
> > 
> 
> Existing boards have management port which is separated from the PP.

I don't think that is enough. You have strongly tied the kernel
version to the firmware version. Upgrade the kernel without first
upgrading linux-firmware, and things break. In Linux distributions
these are separate packages, each with their own life cycle. There is
no guarantee they will be upgraded together.

> > I think you need to provide some degree of backwards compatibly to
> > older firmware. Support version 2 and 3. When version 4 comes out,
> > drop support for version 2 in the driver etc.

The wifi driver i have for my laptop does something like this. It
first tries to load the latest version of the firmware the driver
supports, and if that fails, it goes back to older versions until it
finds a version it can load, or gives up, saying they are all too old.

      Andrew

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next 1/3] net: marvell: prestera: bump supported firmware version to 3.0
  2021-04-23 18:01       ` Andrew Lunn
@ 2021-04-28 13:47         ` Vadym Kochan
  2021-04-28 14:18           ` Andrew Lunn
  0 siblings, 1 reply; 11+ messages in thread
From: Vadym Kochan @ 2021-04-28 13:47 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: David S. Miller, Jakub Kicinski, netdev, Taras Chornyi,
	linux-kernel, Mickey Rachamim, Vadym Kochan

Hi Andrew,

On Fri, Apr 23, 2021 at 08:01:22PM +0200, Andrew Lunn wrote:
> On Fri, Apr 23, 2021 at 08:04:37PM +0300, Vadym Kochan wrote:
> > Hi Andrew,
> > 
> > On Fri, Apr 23, 2021 at 06:49:01PM +0200, Andrew Lunn wrote:
> > > On Fri, Apr 23, 2021 at 06:59:31PM +0300, Vadym Kochan wrote:
> > > > From: Vadym Kochan <vkochan@marvell.com>
> > > > 
> > > > New firmware version has some ABI and feature changes like:
> > > > 
> > > >     - LAG support
> > > >     - initial L3 support
> > > >     - changed events handling logic
> > > > 
> > > > Signed-off-by: Vadym Kochan <vkochan@marvell.com>
> > > > ---
> > > >  drivers/net/ethernet/marvell/prestera/prestera_pci.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/net/ethernet/marvell/prestera/prestera_pci.c b/drivers/net/ethernet/marvell/prestera/prestera_pci.c
> > > > index 298110119272..80fb5daf1da8 100644
> > > > --- a/drivers/net/ethernet/marvell/prestera/prestera_pci.c
> > > > +++ b/drivers/net/ethernet/marvell/prestera/prestera_pci.c
> > > > @@ -13,7 +13,7 @@
> > > >  
> > > >  #define PRESTERA_MSG_MAX_SIZE 1500
> > > >  
> > > > -#define PRESTERA_SUPP_FW_MAJ_VER	2
> > > > +#define PRESTERA_SUPP_FW_MAJ_VER	3
> > > >  #define PRESTERA_SUPP_FW_MIN_VER	0
> > > 
> > > I could be reading the code wrong, but it looks like anybody with
> > > firmware version 2 on their machine and this new driver version
> > > results in the switch not probing? And if the switch does not probe,
> > > do they have any networking to go get the new firmware version?
> > > 
> > 
> > Existing boards have management port which is separated from the PP.
> 
> I don't think that is enough. You have strongly tied the kernel
> version to the firmware version. Upgrade the kernel without first
> upgrading linux-firmware, and things break. In Linux distributions
> these are separate packages, each with their own life cycle. There is
> no guarantee they will be upgraded together.
> 
> > > I think you need to provide some degree of backwards compatibly to
> > > older firmware. Support version 2 and 3. When version 4 comes out,
> > > drop support for version 2 in the driver etc.
> 
> The wifi driver i have for my laptop does something like this. It
> first tries to load the latest version of the firmware the driver
> supports, and if that fails, it goes back to older versions until it
> finds a version it can load, or gives up, saying they are all too old.
> 
>       Andrew

Your comment is right in cases when there's no management port.
However, the all current designs use management port connected directly
to the CPU and not via the PP. This promises the Network connectivity
will remain functional all the time. As for your comment, we have a
plan of designing basic PP ports configuration that will enable
recovering the PP ports connectivity in case backward compatibility
issue will happen.

Regarding the distribution issue when the driver version might be released
earlier than the firmware, it looks like that the probability of such
case is very low because the distributor of the target Linux system will
keep track (actually this is how I see it) that driver and firmware
versions are aligned.

Thanks,
Vadym Kochan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next 1/3] net: marvell: prestera: bump supported firmware version to 3.0
  2021-04-28 13:47         ` Vadym Kochan
@ 2021-04-28 14:18           ` Andrew Lunn
  2021-04-28 14:54             ` Vadym Kochan
  0 siblings, 1 reply; 11+ messages in thread
From: Andrew Lunn @ 2021-04-28 14:18 UTC (permalink / raw)
  To: Vadym Kochan
  Cc: David S. Miller, Jakub Kicinski, netdev, Taras Chornyi,
	linux-kernel, Mickey Rachamim, Vadym Kochan

> Regarding the distribution issue when the driver version might be released
> earlier than the firmware, it looks like that the probability of such
> case is very low because the distributor of the target Linux system will
> keep track (actually this is how I see it) that driver and firmware
> versions are aligned.

You really expect Debian, Redhat, openWRT, SuSE to keep a close eye on
your kernel driver and update their packages at a time you suggest?

I'm also not sure your management port argument is valid. This is an
enterprise switch, not a TOR. It is probably installed in some broom
cupboard at a satellite facility. The management port is not likely to
have its own dedicated link back to the central management
site. Upgrades are going to be applied over the network, and you have
a real danger of turning it into a remote brick, needing local access
to restore it.

I really think you need to support two firmware generations.

  Andrew

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next 1/3] net: marvell: prestera: bump supported firmware version to 3.0
  2021-04-28 14:18           ` Andrew Lunn
@ 2021-04-28 14:54             ` Vadym Kochan
  2021-04-28 15:25               ` Andrew Lunn
  0 siblings, 1 reply; 11+ messages in thread
From: Vadym Kochan @ 2021-04-28 14:54 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: David S. Miller, Jakub Kicinski, netdev, Taras Chornyi,
	linux-kernel, Mickey Rachamim, Vadym Kochan

On Wed, Apr 28, 2021 at 04:18:52PM +0200, Andrew Lunn wrote:
> > Regarding the distribution issue when the driver version might be released
> > earlier than the firmware, it looks like that the probability of such
> > case is very low because the distributor of the target Linux system will
> > keep track (actually this is how I see it) that driver and firmware
> > versions are aligned.
> 
> You really expect Debian, Redhat, openWRT, SuSE to keep a close eye on
> your kernel driver and update their packages at a time you suggest?
> 

No, I don't think these distros will keep track it because they are
targeted for wider usages).
But I think that NOS specifc distro (which may be based on top of which
you listed) will do it (sure this is just my assumption).

> I'm also not sure your management port argument is valid. This is an
> enterprise switch, not a TOR. It is probably installed in some broom
> cupboard at a satellite facility. The management port is not likely to
> have its own dedicated link back to the central management
> site. Upgrades are going to be applied over the network, and you have
> a real danger of turning it into a remote brick, needing local access
> to restore it.
> 
> I really think you need to support two firmware generations.
> 
>   Andrew

I am just trying to clarify if it really worth of it because it will
lead to the hairy code and keep structs for previous FW version.
Ofcourse it may have not a big impact if it will be possible to handle
FW differences in prestera_hw.c only.

I really appreciate your comments, just sharing some concerns/doubts
to discuss.

Thanks,
Vadym Kochan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net-next 1/3] net: marvell: prestera: bump supported firmware version to 3.0
  2021-04-28 14:54             ` Vadym Kochan
@ 2021-04-28 15:25               ` Andrew Lunn
  0 siblings, 0 replies; 11+ messages in thread
From: Andrew Lunn @ 2021-04-28 15:25 UTC (permalink / raw)
  To: Vadym Kochan
  Cc: David S. Miller, Jakub Kicinski, netdev, Taras Chornyi,
	linux-kernel, Mickey Rachamim, Vadym Kochan

On Wed, Apr 28, 2021 at 05:54:34PM +0300, Vadym Kochan wrote:
> On Wed, Apr 28, 2021 at 04:18:52PM +0200, Andrew Lunn wrote:
> > > Regarding the distribution issue when the driver version might be released
> > > earlier than the firmware, it looks like that the probability of such
> > > case is very low because the distributor of the target Linux system will
> > > keep track (actually this is how I see it) that driver and firmware
> > > versions are aligned.
> > 
> > You really expect Debian, Redhat, openWRT, SuSE to keep a close eye on
> > your kernel driver and update their packages at a time you suggest?
> > 
> 
> No, I don't think these distros will keep track it because they are
> targeted for wider usages).
> But I think that NOS specifc distro (which may be based on top of which
> you listed) will do it (sure this is just my assumption).

Mellanox/Nvidia says you can just run Debian on their
switches. Cumulus linux is Debian based. I've been to a few
conferences where data center managers have said they want there
switches to be just another linux machine they can upgrade whenever
they need, nothing special. So that is the TOR segment of the market.

If you look at the opposite end of the market, SOHO switches in Linux,
very few are actually used as plain boring switches. They are actually
embedded into something else. It is an inflight entertainment system
which also has a switch. It is a DSL, 4G, and Ethernet switch placed
along the side of a railway track. It is inside a bus controlling the
passenger information system, announcements, and also a switch. None
of these systems are using a NOS. They are using whatever distribution
best supports the range of devices and services the box needs to
offer.

Now, it could be Prestera will only ever be used as a plain boring
switch in a box. It never gets used for anything interesting. And
since it is a plain boring device, all it needs is a boring NOS? But
do you really want to design your driver aound the assumption nobody
will do anything interesting with Prestera?

> > I'm also not sure your management port argument is valid. This is an
> > enterprise switch, not a TOR. It is probably installed in some broom
> > cupboard at a satellite facility. The management port is not likely to
> > have its own dedicated link back to the central management
> > site. Upgrades are going to be applied over the network, and you have
> > a real danger of turning it into a remote brick, needing local access
> > to restore it.
> 
> I am just trying to clarify if it really worth of it because it will
> lead to the hairy code and keep structs for previous FW version.

Well, if you decide you really should support two generations of the
firmware, you are likely to throw away 3.0.0 and release a 3.0.1 which
is backwards compatible to 2.X.X, but adds additional calls for the
new functionality. Go look at how other drivers have handled this in
the past.

       Andrew

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-04-28 15:26 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-23 15:59 [PATCH net-next 0/3] Marvell Prestera Switchdev initial updates for Vadym Kochan
2021-04-23 15:59 ` [PATCH net-next 1/3] net: marvell: prestera: bump supported firmware version to 3.0 Vadym Kochan
2021-04-23 16:49   ` Andrew Lunn
2021-04-23 17:04     ` Vadym Kochan
2021-04-23 18:01       ` Andrew Lunn
2021-04-28 13:47         ` Vadym Kochan
2021-04-28 14:18           ` Andrew Lunn
2021-04-28 14:54             ` Vadym Kochan
2021-04-28 15:25               ` Andrew Lunn
2021-04-23 15:59 ` [PATCH net-next 2/3] net: marvell: prestera: disable events interrupt while handling Vadym Kochan
2021-04-23 15:59 ` [PATCH net-next 3/3] net: marvell: prestera: align flood setting according to latest firmware version Vadym Kochan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.