Hi Sergei,
Sergei Shtylyov <sergei.shtyl...@cogentembedded.com> writes:
> Hello.
>
> On 2/12/2016 8:09 PM, Vivien Didelot wrote:
>
>> The DSA drivers now have access to the VLAN prepare phase and the bridge
>> net_device. It is easier to check for overlapping bridges from within
>> the driver. Thus add such check in mv88e6xxx_port_vlan_prepare.
>>
>> Signed-off-by: Vivien Didelot <vivien.dide...@savoirfairelinux.com>
>> ---
>> drivers/net/dsa/mv88e6xxx.c | 64
>> +++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 64 insertions(+)
>>
>> diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
>> index 2e515e8..685dcb0 100644
>> --- a/drivers/net/dsa/mv88e6xxx.c
>> +++ b/drivers/net/dsa/mv88e6xxx.c
>> @@ -1471,14 +1471,78 @@ static int _mv88e6xxx_vlan_init(struct dsa_switch
>> *ds, u16 vid,
>> return 0;
>> }
>>
>> +static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
>> + u16 vid_begin, u16 vid_end)
>> +{
>> + struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
>> + struct mv88e6xxx_vtu_stu_entry vlan;
>> + int i, err;
>> +
>> + if (!vid_begin)
>> + return -EOPNOTSUPP;
>> +
>> + mutex_lock(&ps->smi_mutex);
>> +
>> + err = _mv88e6xxx_vtu_vid_write(ds, vid_begin - 1);
>> + if (err)
>> + goto unlock;
>> +
>> + do {
>> + err = _mv88e6xxx_vtu_getnext(ds, &vlan);
>> + if (err)
>> + goto unlock;
>
> Why are you not using *break*?
I use goto for explicit error handling, and break for expected flow.
>> +
>> + if (!vlan.valid)
>> + break;
>> +
>> + if (vlan.vid > vid_end)
>> + break;
>> +
>> + for (i = 0; i < ps->num_ports; ++i) {
>> + if (dsa_is_dsa_port(ds, i) || dsa_is_cpu_port(ds, i))
>> + continue;
>> +
>> + if (vlan.data[i] ==
>> + GLOBAL_VTU_DATA_MEMBER_TAG_NON_MEMBER)
>> + continue;
>> +
>> + if (ps->ports[i].bridge_dev ==
>> + ps->ports[port].bridge_dev)
>> + break; /* same bridge, check next VLAN */
>> +
>> + netdev_warn(ds->ports[port],
>> + "hardware VLAN %d already used by %s\n",
>> + vlan.vid,
>> + netdev_name(ps->ports[i].bridge_dev));
>> + err = -EOPNOTSUPP;
>> + goto unlock;
>> + }
>
> Why not *break*?
Because here it would only break the for loop, and not the while loop.
>
>> + } while (vlan.vid < vid_end);
>> +
>> +unlock:
>> + mutex_unlock(&ps->smi_mutex);
>> +
>> + return err;
>> +}
>> +
> [...]
>
> MBR, Sergei
Thanks,
-v
