diff mbox series

[net-next,2/5] net: dsa: mv88e6xxx: Remove some bureaucracy around querying the VTU

Message ID 20210315211400.2805330-3-tobias@waldekranz.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net: dsa: mv88e6xxx: Offload bridge port flags | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 98 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link

Commit Message

Tobias Waldekranz March 15, 2021, 9:13 p.m. UTC 
The hardware has a somewhat quirky protocol for reading out the VTU
entry for a particular VID. But there is no reason why we cannot
create a better API for ourselves in the driver.

Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 45 ++++++++++++++------------------
 1 file changed, 20 insertions(+), 25 deletions(-)

Comments

Vladimir Oltean March 16, 2021, 9:17 a.m. UTC | #1 
On Mon, Mar 15, 2021 at 10:13:57PM +0100, Tobias Waldekranz wrote:
> +static int mv88e6xxx_vtu_get(struct mv88e6xxx_chip *chip, u16 vid,
> +			     struct mv88e6xxx_vtu_entry *entry)
>  {
> +	int err;
> +
>  	if (!chip->info->ops->vtu_getnext)
>  		return -EOPNOTSUPP;
>  
> -	return chip->info->ops->vtu_getnext(chip, entry);
> +	entry->vid = vid - 1;

Should the vtu_get API not work with vid 0 as well? Shouldn't we
initialize entry->vid to mv88e6xxx_max_vid(chip) in that case?

> +	entry->valid = false;
> +
> +	err = chip->info->ops->vtu_getnext(chip, entry);
> +
> +	if (entry->vid != vid)
> +		entry->valid = false;
> +
> +	return err;
>  }
Tobias Waldekranz March 17, 2021, 9:46 a.m. UTC | #2 
On Tue, Mar 16, 2021 at 11:17, Vladimir Oltean <olteanv@gmail.com> wrote:
> On Mon, Mar 15, 2021 at 10:13:57PM +0100, Tobias Waldekranz wrote:
>> +static int mv88e6xxx_vtu_get(struct mv88e6xxx_chip *chip, u16 vid,
>> +			     struct mv88e6xxx_vtu_entry *entry)
>>  {
>> +	int err;
>> +
>>  	if (!chip->info->ops->vtu_getnext)
>>  		return -EOPNOTSUPP;
>>  
>> -	return chip->info->ops->vtu_getnext(chip, entry);
>> +	entry->vid = vid - 1;
>
> Should the vtu_get API not work with vid 0 as well? Shouldn't we
> initialize entry->vid to mv88e6xxx_max_vid(chip) in that case?

Good catch. We never load VID 0 to the VTU today, but this function
should be ready for it, if that ever changes. Fixing in v2.

>> +	entry->valid = false;
>> +
>> +	err = chip->info->ops->vtu_getnext(chip, entry);
>> +
>> +	if (entry->vid != vid)
>> +		entry->valid = false;
>> +
>> +	return err;
>>  }
diff mbox series

Patch

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index c70d4b7db4f5..86027e98d83d 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1472,13 +1472,23 @@  static int mv88e6xxx_vtu_setup(struct mv88e6xxx_chip *chip)
 	return mv88e6xxx_g1_vtu_flush(chip);
 }
 
-static int mv88e6xxx_vtu_getnext(struct mv88e6xxx_chip *chip,
-				 struct mv88e6xxx_vtu_entry *entry)
+static int mv88e6xxx_vtu_get(struct mv88e6xxx_chip *chip, u16 vid,
+			     struct mv88e6xxx_vtu_entry *entry)
 {
+	int err;
+
 	if (!chip->info->ops->vtu_getnext)
 		return -EOPNOTSUPP;
 
-	return chip->info->ops->vtu_getnext(chip, entry);
+	entry->vid = vid - 1;
+	entry->valid = false;
+
+	err = chip->info->ops->vtu_getnext(chip, entry);
+
+	if (entry->vid != vid)
+		entry->valid = false;
+
+	return err;
 }
 
 static int mv88e6xxx_vtu_walk(struct mv88e6xxx_chip *chip,
@@ -1585,19 +1595,13 @@  static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
 	if (dsa_is_dsa_port(ds, port) || dsa_is_cpu_port(ds, port))
 		return 0;
 
-	vlan.vid = vid - 1;
-	vlan.valid = false;
-
-	err = mv88e6xxx_vtu_getnext(chip, &vlan);
+	err = mv88e6xxx_vtu_get(chip, vid, &vlan);
 	if (err)
 		return err;
 
 	if (!vlan.valid)
 		return 0;
 
-	if (vlan.vid != vid)
-		return 0;
-
 	for (i = 0; i < mv88e6xxx_num_ports(chip); ++i) {
 		if (dsa_is_dsa_port(ds, i) || dsa_is_cpu_port(ds, i))
 			continue;
@@ -1679,15 +1683,12 @@  static int mv88e6xxx_port_db_load_purge(struct mv88e6xxx_chip *chip, int port,
 		if (err)
 			return err;
 	} else {
-		vlan.vid = vid - 1;
-		vlan.valid = false;
-
-		err = mv88e6xxx_vtu_getnext(chip, &vlan);
+		err = mv88e6xxx_vtu_get(chip, vid, &vlan);
 		if (err)
 			return err;
 
 		/* switchdev expects -EOPNOTSUPP to honor software VLANs */
-		if (vlan.vid != vid || !vlan.valid)
+		if (!vlan.valid)
 			return -EOPNOTSUPP;
 
 		fid = vlan.fid;
@@ -1964,14 +1965,11 @@  static int mv88e6xxx_port_vlan_join(struct mv88e6xxx_chip *chip, int port,
 	struct mv88e6xxx_vtu_entry vlan;
 	int i, err;
 
-	vlan.vid = vid - 1;
-	vlan.valid = false;
-
-	err = mv88e6xxx_vtu_getnext(chip, &vlan);
+	err = mv88e6xxx_vtu_get(chip, vid, &vlan);
 	if (err)
 		return err;
 
-	if (vlan.vid != vid || !vlan.valid) {
+	if (!vlan.valid) {
 		memset(&vlan, 0, sizeof(vlan));
 
 		err = mv88e6xxx_atu_new(chip, &vlan.fid);
@@ -2067,17 +2065,14 @@  static int mv88e6xxx_port_vlan_leave(struct mv88e6xxx_chip *chip,
 	if (!vid)
 		return -EOPNOTSUPP;
 
-	vlan.vid = vid - 1;
-	vlan.valid = false;
-
-	err = mv88e6xxx_vtu_getnext(chip, &vlan);
+	err = mv88e6xxx_vtu_get(chip, vid, &vlan);
 	if (err)
 		return err;
 
 	/* If the VLAN doesn't exist in hardware or the port isn't a member,
 	 * tell switchdev that this VLAN is likely handled in software.
 	 */
-	if (vlan.vid != vid || !vlan.valid ||
+	if (!vlan.valid ||
 	    vlan.member[port] == MV88E6XXX_G1_VTU_DATA_MEMBER_TAG_NON_MEMBER)
 		return -EOPNOTSUPP;