2020-07-14 13:37:07

by Ioana Ciornei

[permalink] [raw]
Subject: [PATCH 0/6] staging: dpaa2-ethsw: cleanup of link state and MAC addresses

This patch set is cleaning up the link state handling of the switch
ports in patches 1-4. The last two patches are setting up the MAC
addresses of the switch ports automatically so that the user is not
forced to manually add them before adding them to a bridge.

Ioana Ciornei (6):
staging: dpaa2-ethsw: fix reported link state
staging: dpaa2-ethsw: ignore state interrupts when the interface is
not running
staging: dpaa2-ethsw: use netif_running when checking for port up
staging: dpaa2-ethsw: disable switch ports are probe time
staging: dpaa2-ethsw: store version information of the DPSW object
staging: dpaa2-ethsw: setup MAC address of switch netdevices

drivers/staging/fsl-dpaa2/ethsw/dpsw-cmd.h | 14 +++
drivers/staging/fsl-dpaa2/ethsw/dpsw.c | 106 +++++++++++++++++++++
drivers/staging/fsl-dpaa2/ethsw/dpsw.h | 9 ++
drivers/staging/fsl-dpaa2/ethsw/ethsw.c | 102 +++++++++++++++++---
drivers/staging/fsl-dpaa2/ethsw/ethsw.h | 4 +
5 files changed, 221 insertions(+), 14 deletions(-)

--
2.25.1


2020-07-14 13:37:19

by Ioana Ciornei

[permalink] [raw]
Subject: [PATCH 3/6] staging: dpaa2-ethsw: use netif_running when checking for port up

There are some cases where the switch interface needs to be disabled so
that changes in the configuration can be made. In such cases, we should
check for a running interface (bit __LINK_STATE_START of the netdev)
instead of netif_carrier_ok(). This is because on open() we enable the
switch interface even though the link up has not come out yet.

Signed-off-by: Ioana Ciornei <[email protected]>
---
drivers/staging/fsl-dpaa2/ethsw/ethsw.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
index b57bc705c2ee..a1917842536e 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
+++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
@@ -48,7 +48,7 @@ static int ethsw_port_set_pvid(struct ethsw_port_priv *port_priv, u16 pvid)
struct ethsw_core *ethsw = port_priv->ethsw_data;
struct net_device *netdev = port_priv->netdev;
struct dpsw_tci_cfg tci_cfg = { 0 };
- bool is_oper;
+ bool up;
int err, ret;

err = dpsw_if_get_tci(ethsw->mc_io, 0, ethsw->dpsw_handle,
@@ -61,8 +61,8 @@ static int ethsw_port_set_pvid(struct ethsw_port_priv *port_priv, u16 pvid)
tci_cfg.vlan_id = pvid;

/* Interface needs to be down to change PVID */
- is_oper = netif_oper_up(netdev);
- if (is_oper) {
+ up = netif_running(netdev);
+ if (up) {
err = dpsw_if_disable(ethsw->mc_io, 0,
ethsw->dpsw_handle,
port_priv->idx);
@@ -85,7 +85,7 @@ static int ethsw_port_set_pvid(struct ethsw_port_priv *port_priv, u16 pvid)
port_priv->pvid = pvid;

set_tci_error:
- if (is_oper) {
+ if (up) {
ret = dpsw_if_enable(ethsw->mc_io, 0,
ethsw->dpsw_handle,
port_priv->idx);
@@ -188,7 +188,7 @@ static int ethsw_port_set_stp_state(struct ethsw_port_priv *port_priv, u8 state)
};
int err;

- if (!netif_oper_up(port_priv->netdev) || state == port_priv->stp_state)
+ if (!netif_running(port_priv->netdev) || state == port_priv->stp_state)
return 0; /* Nothing to do */

err = dpsw_if_set_stp(port_priv->ethsw_data->mc_io, 0,
--
2.25.1

2020-07-14 13:37:30

by Ioana Ciornei

[permalink] [raw]
Subject: [PATCH 5/6] staging: dpaa2-ethsw: store version information of the DPSW object

Store the major and minor versions of the DPSW object in the ethsw
structure. This will be used in a subsequent patch to make sure some
commands are only called on the appropriate version of object.

Signed-off-by: Ioana Ciornei <[email protected]>
---
drivers/staging/fsl-dpaa2/ethsw/ethsw.c | 16 ++++++++--------
drivers/staging/fsl-dpaa2/ethsw/ethsw.h | 1 +
2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
index f283ce195c1e..a8fc9bcf3b8a 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
+++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
@@ -1368,9 +1368,9 @@ static int ethsw_init(struct fsl_mc_device *sw_dev)
{
struct device *dev = &sw_dev->dev;
struct ethsw_core *ethsw = dev_get_drvdata(dev);
- u16 version_major, version_minor, i;
struct dpsw_stp_cfg stp_cfg;
int err;
+ u16 i;

ethsw->dev_id = sw_dev->obj_desc.id;

@@ -1388,20 +1388,20 @@ static int ethsw_init(struct fsl_mc_device *sw_dev)
}

err = dpsw_get_api_version(ethsw->mc_io, 0,
- &version_major,
- &version_minor);
+ &ethsw->major,
+ &ethsw->minor);
if (err) {
dev_err(dev, "dpsw_get_api_version err %d\n", err);
goto err_close;
}

/* Minimum supported DPSW version check */
- if (version_major < DPSW_MIN_VER_MAJOR ||
- (version_major == DPSW_MIN_VER_MAJOR &&
- version_minor < DPSW_MIN_VER_MINOR)) {
+ if (ethsw->major < DPSW_MIN_VER_MAJOR ||
+ (ethsw->major == DPSW_MIN_VER_MAJOR &&
+ ethsw->minor < DPSW_MIN_VER_MINOR)) {
dev_err(dev, "DPSW version %d:%d not supported. Use %d.%d or greater.\n",
- version_major,
- version_minor,
+ ethsw->major,
+ ethsw->minor,
DPSW_MIN_VER_MAJOR, DPSW_MIN_VER_MINOR);
err = -ENOTSUPP;
goto err_close;
diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.h b/drivers/staging/fsl-dpaa2/ethsw/ethsw.h
index a0244f7d5003..0e520fd94dbc 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/ethsw.h
+++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.h
@@ -61,6 +61,7 @@ struct ethsw_core {
struct fsl_mc_io *mc_io;
u16 dpsw_handle;
struct dpsw_attr sw_attr;
+ u16 major, minor;
int dev_id;
struct ethsw_port_priv **ports;

--
2.25.1

2020-07-14 13:38:24

by Ioana Ciornei

[permalink] [raw]
Subject: [PATCH 4/6] staging: dpaa2-ethsw: disable switch ports are probe time

The MC firmware will enable the switch interfaces at DPSW creation
without waiting for an 'ifconfig up' on the switch interfaces. When this
happens, the states held by the Linux software vs the firmware are not
in sync. Make sure to disable the switch ports at probe time to not
encounter this issue.

Signed-off-by: Ioana Ciornei <[email protected]>
---
drivers/staging/fsl-dpaa2/ethsw/ethsw.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
index a1917842536e..f283ce195c1e 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
+++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
@@ -1672,6 +1672,10 @@ static int ethsw_probe(struct fsl_mc_device *sw_dev)
goto err_free_ports;
}

+ /* Make sure the switch ports are disabled at probe time */
+ for (i = 0; i < ethsw->sw_attr.num_ifs; i++)
+ dpsw_if_disable(ethsw->mc_io, 0, ethsw->dpsw_handle, i);
+
/* Setup IRQs */
err = ethsw_setup_irqs(sw_dev);
if (err)
--
2.25.1

2020-07-14 13:38:39

by Ioana Ciornei

[permalink] [raw]
Subject: [PATCH 2/6] staging: dpaa2-ethsw: ignore state interrupts when the interface is not running

Link state interrupts will be transmitted to the DPSW object even though
the user has not yet issued a 'ifconfig up' on a switch interface. Don't
act on those interrupts since there are of no interrest.

Signed-off-by: Ioana Ciornei <[email protected]>
---
drivers/staging/fsl-dpaa2/ethsw/ethsw.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
index 46aa37093e86..b57bc705c2ee 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
+++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
@@ -445,6 +445,12 @@ static int port_carrier_state_sync(struct net_device *netdev)
struct dpsw_link_state state;
int err;

+ /* Interrupts are received even though no one issued an 'ifconfig up'
+ * on the switch interface. Ignore these link state update interrupts
+ */
+ if (!netif_running(netdev))
+ return 0;
+
err = dpsw_if_get_link_state(port_priv->ethsw_data->mc_io, 0,
port_priv->ethsw_data->dpsw_handle,
port_priv->idx, &state);
--
2.25.1

2020-07-15 15:09:11

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 0/6] staging: dpaa2-ethsw: cleanup of link state and MAC addresses

On Tue, Jul 14, 2020 at 04:34:25PM +0300, Ioana Ciornei wrote:
> This patch set is cleaning up the link state handling of the switch
> ports in patches 1-4. The last two patches are setting up the MAC
> addresses of the switch ports automatically so that the user is not
> forced to manually add them before adding them to a bridge.

This feels like adding new functionality to this code. What is keeping
it from getting out of staging at this point in time? I would prefer
for it to be moved out before adding new stuff to it.

thanks,

greg k-h

2020-07-15 15:09:23

by Ioana Ciornei

[permalink] [raw]
Subject: RE: [PATCH 0/6] staging: dpaa2-ethsw: cleanup of link state and MAC addresses

> Subject: Re: [PATCH 0/6] staging: dpaa2-ethsw: cleanup of link state and MAC
> addresses
>
> On Tue, Jul 14, 2020 at 04:34:25PM +0300, Ioana Ciornei wrote:
> > This patch set is cleaning up the link state handling of the switch
> > ports in patches 1-4. The last two patches are setting up the MAC
> > addresses of the switch ports automatically so that the user is not
> > forced to manually add them before adding them to a bridge.
>
> This feels like adding new functionality to this code. What is keeping it from
> getting out of staging at this point in time? I would prefer for it to be moved out
> before adding new stuff to it.
>
> thanks,
>
> greg k-h

We still have some work do to on this driver before moving it out of staging unfortunately, mainly integrating the Rx/Tx functionality for switch ports[1] and general cleanup on the driver. Sorry I did not mention this in the cover letter.

This patch set just does some cleanup of the link state handling since the state showed to the user in the ifconfig output was not consistent all the time with what actually was happening in the hardware.

Thanks,
Ioana

[1] https://lkml.org/lkml/2019/11/5/548