2022-02-18 16:17:43

by Hans S

[permalink] [raw]
Subject: [PATCH net-next v3 4/5] net: dsa: mv88e6xxx: Add support for bridge port locked mode

Supporting bridge ports in locked mode using the drop on lock
feature 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 <[email protected]>
---
drivers/net/dsa/mv88e6xxx/chip.c | 9 ++++++++-
drivers/net/dsa/mv88e6xxx/port.c | 33 ++++++++++++++++++++++++++++++++
drivers/net/dsa/mv88e6xxx/port.h | 9 ++++++++-
3 files changed, 49 insertions(+), 2 deletions(-)

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..46b7381899a0 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_SA_FILT_MASK;
+ if (locked)
+ reg |= MV88E6XXX_PORT_CTL0_SA_FILT_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..3f70557f0d48 100644
--- a/drivers/net/dsa/mv88e6xxx/port.h
+++ b/drivers/net/dsa/mv88e6xxx/port.h
@@ -142,7 +142,11 @@
/* Offset 0x04: Port Control Register */
#define MV88E6XXX_PORT_CTL0 0x04
#define MV88E6XXX_PORT_CTL0_USE_CORE_TAG 0x8000
-#define MV88E6XXX_PORT_CTL0_DROP_ON_LOCK 0x4000
+#define MV88E6XXX_PORT_CTL0_SA_FILT_MASK 0xc000
+#define MV88E6XXX_PORT_CTL0_SA_FILT_DISABLED 0x0000
+#define MV88E6XXX_PORT_CTL0_SA_FILT_DROP_ON_LOCK 0x4000
+#define MV88E6XXX_PORT_CTL0_SA_FILT_DROP_ON_UNLOCK 0x8000
+#define MV88E6XXX_PORT_CTL0_SA_FILT_DROP_ON_CPU 0xc000
#define MV88E6XXX_PORT_CTL0_EGRESS_MODE_MASK 0x3000
#define MV88E6XXX_PORT_CTL0_EGRESS_MODE_UNMODIFIED 0x0000
#define MV88E6XXX_PORT_CTL0_EGRESS_MODE_UNTAGGED 0x1000
@@ -365,6 +369,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);
--
2.30.2


2022-02-20 11:40:42

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH net-next v3 4/5] net: dsa: mv88e6xxx: Add support for bridge port locked mode

On Fri, Feb 18, 2022 at 04:51:47PM +0100, Hans Schultz wrote:
> diff --git a/drivers/net/dsa/mv88e6xxx/port.c b/drivers/net/dsa/mv88e6xxx/port.c
> index ab41619a809b..46b7381899a0 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_SA_FILT_MASK;
> + if (locked)
> + reg |= MV88E6XXX_PORT_CTL0_SA_FILT_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);

return mv88e6xxx_port_write(...);

> + if (err)
> + return err;
> +
> + return 0;
> +}

2022-02-21 03:42:41

by Ido Schimmel

[permalink] [raw]
Subject: Re: [PATCH net-next v3 4/5] net: dsa: mv88e6xxx: Add support for bridge port locked mode

On Sat, Feb 19, 2022 at 12:00:34PM +0200, Vladimir Oltean wrote:
> On Fri, Feb 18, 2022 at 04:51:47PM +0100, Hans Schultz wrote:
> > diff --git a/drivers/net/dsa/mv88e6xxx/port.c b/drivers/net/dsa/mv88e6xxx/port.c
> > index ab41619a809b..46b7381899a0 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_SA_FILT_MASK;
> > + if (locked)
> > + reg |= MV88E6XXX_PORT_CTL0_SA_FILT_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);
>
> return mv88e6xxx_port_write(...);

Not familiar with mv88e6xxx, but shouldn't there be a rollback of
previous operations? Specifically mv88e6xxx_port_write()

>
> > + if (err)
> > + return err;
> > +
> > + return 0;
> > +}

2022-02-22 05:20:30

by Hans S

[permalink] [raw]
Subject: Re: [PATCH net-next v3 4/5] net: dsa: mv88e6xxx: Add support for bridge port locked mode

On sön, feb 20, 2022 at 11:21, Ido Schimmel <[email protected]> wrote:
> On Sat, Feb 19, 2022 at 12:00:34PM +0200, Vladimir Oltean wrote:
>> On Fri, Feb 18, 2022 at 04:51:47PM +0100, Hans Schultz wrote:
>> > diff --git a/drivers/net/dsa/mv88e6xxx/port.c b/drivers/net/dsa/mv88e6xxx/port.c
>> > index ab41619a809b..46b7381899a0 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_SA_FILT_MASK;
>> > + if (locked)
>> > + reg |= MV88E6XXX_PORT_CTL0_SA_FILT_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);
>>
>> return mv88e6xxx_port_write(...);
>
> Not familiar with mv88e6xxx, but shouldn't there be a rollback of
> previous operations? Specifically mv88e6xxx_port_write()
>

If a register write function fails, I don't think that it would make
sense to try and resolve the situation by additional register write
calls (rollback).

>>
>> > + if (err)
>> > + return err;
>> > +
>> > + return 0;
>> > +}