diff mbox series

[net-next,3/4] net: dsa: mv88e6xxx: Add support for bridge port locked feature

Message ID 20220207100742.15087-4-schultz.hans+netdev@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series Add support for locked bridge ports (for 802.1X) | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch warning WARNING: From:/Signed-off-by: email subaddress mismatch: 'From: Hans Schultz <schultz.hans@gmail.com>' != 'Signed-off-by: Hans Schultz <schultz.hans+netdev@gmail.com>' WARNING: line length of 81 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Hans S Feb. 7, 2022, 10:07 a.m. UTC
Supporting bridge port locked mode using the 802.1X mode in Marvell
mv88e6xxx switchcores is described in the '88E6096/88E6097/88E6097F
Datasheet', sections 4.4.6, 4.4.7 and 5.1.2.1 (Drop on Lock).

This feature is implemented here facilitated by the locked port flag.

Signed-off-by: Hans Schultz <schultz.hans+netdev@gmail.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c |  9 ++++++++-
 drivers/net/dsa/mv88e6xxx/port.c | 33 ++++++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/port.h |  3 +++
 3 files changed, 44 insertions(+), 1 deletion(-)

Comments

Andrew Lunn Feb. 7, 2022, 2:05 p.m. UTC | #1
On Mon, Feb 07, 2022 at 11:07:41AM +0100, Hans Schultz wrote:
> Supporting bridge port locked mode using the 802.1X mode in Marvell
> mv88e6xxx switchcores is described in the '88E6096/88E6097/88E6097F
> Datasheet', sections 4.4.6, 4.4.7 and 5.1.2.1 (Drop on Lock).

This implementation seems to be incorrect for 6390X, and maybe
others. I just picked a modern devices at random, and it is different,
so didn't check any other devices.  The 6390X uses bits 14 and 15, not
just bit 14.

So either you need to narrow down support to just those devices this
actually works for, or you need to add implementations for all
generations, via an op in mv88e6xxx_ops.

    Andrew
Hans S Feb. 8, 2022, 12:14 p.m. UTC | #2
On mån, feb 07, 2022 at 15:05, Andrew Lunn <andrew@lunn.ch> wrote:
> On Mon, Feb 07, 2022 at 11:07:41AM +0100, Hans Schultz wrote:
>> Supporting bridge port locked mode using the 802.1X mode in Marvell
>> mv88e6xxx switchcores is described in the '88E6096/88E6097/88E6097F
>> Datasheet', sections 4.4.6, 4.4.7 and 5.1.2.1 (Drop on Lock).
>
> This implementation seems to be incorrect for 6390X, and maybe
> others. I just picked a modern devices at random, and it is different,
> so didn't check any other devices.  The 6390X uses bits 14 and 15, not
> just bit 14.
>
> So either you need to narrow down support to just those devices this
> actually works for, or you need to add implementations for all
> generations, via an op in mv88e6xxx_ops.
>
>     Andrew

The 6096 and 6097 also use both bits 15 and 14, with '01' being Drop On
Lock and the default being '00' No SA filtering. '11' is drop to CPU, which
can also be used for 801.1X, so 'x1' should suffice for these devices,
thus setting bit 14 seems appropriate.
Andrew Lunn Feb. 8, 2022, 1:26 p.m. UTC | #3
On Tue, Feb 08, 2022 at 01:14:45PM +0100, Hans Schultz wrote:
> On mån, feb 07, 2022 at 15:05, Andrew Lunn <andrew@lunn.ch> wrote:
> > On Mon, Feb 07, 2022 at 11:07:41AM +0100, Hans Schultz wrote:
> >> Supporting bridge port locked mode using the 802.1X mode in Marvell
> >> mv88e6xxx switchcores is described in the '88E6096/88E6097/88E6097F
> >> Datasheet', sections 4.4.6, 4.4.7 and 5.1.2.1 (Drop on Lock).
> >
> > This implementation seems to be incorrect for 6390X, and maybe
> > others. I just picked a modern devices at random, and it is different,
> > so didn't check any other devices.  The 6390X uses bits 14 and 15, not
> > just bit 14.
> >
> > So either you need to narrow down support to just those devices this
> > actually works for, or you need to add implementations for all
> > generations, via an op in mv88e6xxx_ops.
> >
> >     Andrew
> 
> The 6096 and 6097 also use both bits 15 and 14, with '01' being Drop On
> Lock and the default being '00' No SA filtering. '11' is drop to CPU, which
> can also be used for 801.1X, so 'x1' should suffice for these devices,
> thus setting bit 14 seems appropriate.

Your code does not make this clear. Please define all four values, and
then mask and set both bits as needed.

Thanks

    Andrew
diff mbox series

Patch

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 58ca684d73f7..eed3713b97ae 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -5881,7 +5881,7 @@  static int mv88e6xxx_port_pre_bridge_flags(struct dsa_switch *ds, int port,
 	const struct mv88e6xxx_ops *ops;
 
 	if (flags.mask & ~(BR_LEARNING | BR_FLOOD | BR_MCAST_FLOOD |
-			   BR_BCAST_FLOOD))
+			   BR_BCAST_FLOOD | BR_PORT_LOCKED))
 		return -EINVAL;
 
 	ops = chip->info->ops;
@@ -5939,6 +5939,13 @@  static int mv88e6xxx_port_bridge_flags(struct dsa_switch *ds, int port,
 			goto out;
 	}
 
+	if (flags.mask & BR_PORT_LOCKED) {
+		bool locked = !!(flags.val & BR_PORT_LOCKED);
+
+		err = mv88e6xxx_port_set_lock(chip, port, locked);
+		if (err)
+			goto out;
+	}
 out:
 	mv88e6xxx_reg_unlock(chip);
 
diff --git a/drivers/net/dsa/mv88e6xxx/port.c b/drivers/net/dsa/mv88e6xxx/port.c
index ab41619a809b..2279936429f9 100644
--- a/drivers/net/dsa/mv88e6xxx/port.c
+++ b/drivers/net/dsa/mv88e6xxx/port.c
@@ -1234,6 +1234,39 @@  int mv88e6xxx_port_set_mirror(struct mv88e6xxx_chip *chip, int port,
 	return err;
 }
 
+int mv88e6xxx_port_set_lock(struct mv88e6xxx_chip *chip, int port,
+			    bool locked)
+{
+	u16 reg;
+	int err;
+
+	err = mv88e6xxx_port_read(chip, port, MV88E6XXX_PORT_CTL0, &reg);
+	if (err)
+		return err;
+
+	reg &= ~MV88E6XXX_PORT_CTL0_DROP_ON_LOCK;
+	if (locked)
+		reg |= MV88E6XXX_PORT_CTL0_DROP_ON_LOCK;
+
+	err = mv88e6xxx_port_write(chip, port, MV88E6XXX_PORT_CTL0, reg);
+	if (err)
+		return err;
+
+	err = mv88e6xxx_port_read(chip, port, MV88E6XXX_PORT_ASSOC_VECTOR, &reg);
+	if (err)
+		return err;
+
+	reg &= ~MV88E6XXX_PORT_ASSOC_VECTOR_LOCKED_PORT;
+	if (locked)
+		reg |= MV88E6XXX_PORT_ASSOC_VECTOR_LOCKED_PORT;
+
+	err = mv88e6xxx_port_write(chip, port, MV88E6XXX_PORT_ASSOC_VECTOR, reg);
+	if (err)
+		return err;
+
+	return 0;
+}
+
 int mv88e6xxx_port_set_8021q_mode(struct mv88e6xxx_chip *chip, int port,
 				  u16 mode)
 {
diff --git a/drivers/net/dsa/mv88e6xxx/port.h b/drivers/net/dsa/mv88e6xxx/port.h
index 03382b66f800..655d942ac657 100644
--- a/drivers/net/dsa/mv88e6xxx/port.h
+++ b/drivers/net/dsa/mv88e6xxx/port.h
@@ -365,6 +365,9 @@  int mv88e6xxx_port_set_fid(struct mv88e6xxx_chip *chip, int port, u16 fid);
 int mv88e6xxx_port_get_pvid(struct mv88e6xxx_chip *chip, int port, u16 *pvid);
 int mv88e6xxx_port_set_pvid(struct mv88e6xxx_chip *chip, int port, u16 pvid);
 
+int mv88e6xxx_port_set_lock(struct mv88e6xxx_chip *chip, int port,
+			    bool locked);
+
 int mv88e6xxx_port_set_8021q_mode(struct mv88e6xxx_chip *chip, int port,
 				  u16 mode);
 int mv88e6095_port_tag_remap(struct mv88e6xxx_chip *chip, int port);