On 8/23/2020 1:59 PM, Linus Walleij wrote:
The VLANs and PVIDs on the RTL8366 utilizes a "member
configuration" (MC) which is largely unexplained in the
code.
This set-up requires a special ordering: rtl8366_set_pvid()
must be called first, followed by rtl8366_set_vlan(),
else the MC will not be properly allocated. Relax this
by factoring out the code obtaining an MC and reuse
the helper in both rtl8366_set_pvid() and
rtl8366_set_vlan() so we remove this strict ordering
requirement.
In the process, add some better comments and debug prints
so people who read the code understand what is going on.
Signed-off-by: Linus Walleij <[email protected]>
---
[snip]
+/**
+ * rtl8366_obtain_mc() - retrieve or allocate a VLAN member configuration
+ * @smi: the Realtek SMI device instance
+ * @vid: the VLAN ID to look up or allocate
+ * @vlanmc: the pointer will be assigned to a pointer to a valid member config
+ * if successful
+ * @index: the pointer will be assigned to the index of the valid member config
+ * if successful
+ * @return: 0 or error number
You could return the index directly, since only negative error codes are
of interest to deal with failures?
[snip]
+
+ dev_err(smi->dev, "all VLAN member configurations are in use\n");
+ return -EINVAL;
Maybe -ENOSPC?
[snip]
vid);
+
+ ret = rtl8366_set_pvid(smi, port, vid);
+ if (ret)
+ dev_err(smi->dev,
+ "failed to set PVID on port %d to VLAN %04x",
+ port, vid);
+
+ if (!ret)
+ dev_dbg(smi->dev, "VLAN add: added VLAN %04x with PVID on
port %d\n",
+ vid, port);
Since you are changing this message to a debug print, can we bring some
consistency along the way and print the VID as decimal (not hexadecimal)
to match the prior instances?
--
Florian
