Hi Vladimir,

On Sun, 25 Aug 2019 21:44:54 +0300, Vladimir Oltean [off-list ref] wrote:

-	if (enabled)
-		err = dsa_port_vid_add(upstream_dp, tx_vid, 0);
-	else
-		err = dsa_port_vid_del(upstream_dp, tx_vid);
+	err = dsa_8021q_vid_apply(ds, upstream, tx_vid, 0, enabled);
 	if (err) {
 		dev_err(ds->dev, "Failed to apply TX VID %d on port %d: %d\n",
 			tx_vid, upstream, err);
 		return err;
 	}
 
-	return 0;
+	if (!enabled)
+		err = dsa_8021q_restore_pvid(ds, port);
+
+	return err;
 }

I did not dig that much into tag_8021q.c yet. From seeing this portion,
I'm just wondering if these two helpers couldn't be part of the same logic
as they both act upon the "enabled" condition?

Otherwise I have no complains about the series.


Thanks,

	Vivien