Hi Andrew,
Andrew Lunn <and...@lunn.ch> writes:
> @@ -26,6 +26,7 @@ enum dsa_tag_protocol {
> DSA_TAG_PROTO_TRAILER,
> DSA_TAG_PROTO_EDSA,
> DSA_TAG_PROTO_BRCM,
> + _DSA_TAG_LAST,
> };
I would avoid _ prefixed functions or symbols, we've seen in mv88e6xxx
that it doesn't make the code really readable.
There's already an implicit "DSA_TAG_PROTO" namespace, so I suggest
"DSA_MAX_TAGS" to keep it consistent with DSA_MAX_{PORTS,SWITCHES}.
[...]
> + /*
> + * Tagging protocol operations for adding and removing an
> + * encapsulation tag.
> + */
> + const struct dsa_device_ops *tag_ops;
dsa_device_ops seems too generic for the xmit/rcv tag-related pair. That
being said, I don't really have better suggestions. Any idea?
[...]
> +const struct dsa_device_ops *dsa_device_ops[_DSA_TAG_LAST] = {
> +#ifdef CONFIG_NET_DSA_TAG_DSA
> + [DSA_TAG_PROTO_DSA] = &dsa_netdev_ops,
> +#endif
> +#ifdef CONFIG_NET_DSA_TAG_EDSA
> + [DSA_TAG_PROTO_EDSA] = &edsa_netdev_ops,
> +#endif
> +#ifdef CONFIG_NET_DSA_TAG_TRAILER
> + [DSA_TAG_PROTO_TRAILER] = &trailer_netdev_ops,
> +#endif
> +#ifdef CONFIG_NET_DSA_TAG_BRCM
> + [DSA_TAG_PROTO_BRCM] = &brcm_netdev_ops,
> +#endif
> + [DSA_TAG_PROTO_NONE] = &none_ops,
> +};
>
> /* switch driver registration
> ***********************************************/
> static DEFINE_MUTEX(dsa_switch_drivers_mutex);
> @@ -225,6 +252,20 @@ static int dsa_cpu_dsa_setups(struct dsa_switch *ds,
> struct device *dev)
> return 0;
> }
>
> +const struct dsa_device_ops *dsa_resolve_tag_protocol(int tag_protocol)
> +{
> + const struct dsa_device_ops *ops;
> +
> + if (tag_protocol >= _DSA_TAG_LAST)
> + return ERR_PTR(-EINVAL);
> + ops = dsa_device_ops[tag_protocol];
> +
> + if (!ops)
> + return ERR_PTR(-ENOPROTOOPT);
> +
> + return ops;
> +}
I don't see the need to add a dsa_device_ops array. I'd keep the switch
case on tag_protocol within this new dsa_resolve_tag_protocol function,
to make it more readable (no value checking necessary).
Thanks,
Vivien
