| Message ID | 20210326105648.2492411-1-tobias@waldekranz.com |
|---|---|
| Headers | show |
| Series | net: dsa: Allow default tag protocol to be overridden from DT | expand |
On Fri, Mar 26, 2021 at 11:56:48AM +0100, Tobias Waldekranz wrote: > The 'dsa,tag-protocol' is used to force a switch tree to use a > particular tag protocol, typically because the Ethernet controller > that it is connected to is not compatible with the default one. > > Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com> > --- > Documentation/devicetree/bindings/net/dsa/dsa.yaml | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/Documentation/devicetree/bindings/net/dsa/dsa.yaml b/Documentation/devicetree/bindings/net/dsa/dsa.yaml > index 8a3494db4d8d..5dcfab049aa2 100644 > --- a/Documentation/devicetree/bindings/net/dsa/dsa.yaml > +++ b/Documentation/devicetree/bindings/net/dsa/dsa.yaml > @@ -70,6 +70,13 @@ patternProperties: > device is what the switch port is connected to > $ref: /schemas/types.yaml#/definitions/phandle > > + dsa,tag-protocol: 'dsa' is not a vendor. > + description: > + Instead of the default, the switch will use this tag protocol if > + possible. Useful when a device supports multiple protcols and > + the default is incompatible with the Ethernet device. > + $ref: /schemas/types.yaml#/definitions/string You need to define the possible strings. > + > phy-handle: true > > phy-mode: true > -- > 2.25.1 >
On Fri, Mar 26, 2021 at 11:56:46AM +0100, Tobias Waldekranz wrote: > All devices are capable of using regular DSA tags. Support for > Ethertyped DSA tags sort into three categories: > > 1. No support. Older chips fall into this category. > > 2. Full support. Datasheet explicitly supports configuring the CPU > port to receive FORWARDs with a DSA tag. > > 3. Undocumented support. Datasheet lists the configuration from > category 2 as "reserved for future use", but does empirically > behave like a category 2 device. > +static int mv88e6xxx_change_tag_protocol(struct dsa_switch *ds, int port, > + enum dsa_tag_protocol proto) > +{ > + struct mv88e6xxx_chip *chip = ds->priv; > + enum dsa_tag_protocol old_protocol; > + int err; > + > + switch (proto) { > + case DSA_TAG_PROTO_EDSA: > + if (chip->info->tag_protocol != DSA_TAG_PROTO_EDSA) > + dev_warn(chip->dev, "Relying on undocumented EDSA tagging behavior\n"); > + > + break; > + case DSA_TAG_PROTO_DSA: > + break; > + default: > + return -EPROTONOSUPPORT; > + } You are handling cases 2 and 3 here, but not 1. Which makes it a bit of a foot cannon for older devices. Now that we have chip->tag_protocol, maybe we should change chip->info->tag_protocol to mean supported protocols? BIT(0) DSA BIT(1) EDSA BIT(2) Undocumented EDSA Andrew
On Sat, Mar 27, 2021 at 12:13, Rob Herring <robh@kernel.org> wrote: > On Fri, Mar 26, 2021 at 11:56:48AM +0100, Tobias Waldekranz wrote: >> The 'dsa,tag-protocol' is used to force a switch tree to use a >> particular tag protocol, typically because the Ethernet controller >> that it is connected to is not compatible with the default one. >> >> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com> >> --- >> Documentation/devicetree/bindings/net/dsa/dsa.yaml | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/net/dsa/dsa.yaml b/Documentation/devicetree/bindings/net/dsa/dsa.yaml >> index 8a3494db4d8d..5dcfab049aa2 100644 >> --- a/Documentation/devicetree/bindings/net/dsa/dsa.yaml >> +++ b/Documentation/devicetree/bindings/net/dsa/dsa.yaml >> @@ -70,6 +70,13 @@ patternProperties: >> device is what the switch port is connected to >> $ref: /schemas/types.yaml#/definitions/phandle >> >> + dsa,tag-protocol: > > 'dsa' is not a vendor. It is not. The property is intended to be consumed by the vendor-independent driver. So should it be linux,tag-protocol? Just tag-protocol? >> + description: >> + Instead of the default, the switch will use this tag protocol if >> + possible. Useful when a device supports multiple protcols and >> + the default is incompatible with the Ethernet device. >> + $ref: /schemas/types.yaml#/definitions/string > > You need to define the possible strings. Alright. Andrew, Vladimir: I will just list dsa and edsa for now. If it is needed on other devices, people can add them to the list after they have tested their drivers. Fair? >> + >> phy-handle: true >> >> phy-mode: true >> -- >> 2.25.1 >>
> Since DSA is supported on all devices, perhaps we should just have: > > enum mv88e6xxx_edsa_support { > MV88E6XXX_EDSA_UNSUPPORTED, > MV88E6XXX_EDSA_UNDOCUMENTED, > MV88E6XXX_EDSA_SUPPORTED, > }; Yes, that is O.K. > Do we also want to default to DSA on all devices unless there is a > DT-property saying something else? Using EDSA does not really give you > anything over bare tags anymore. You have fixed the tcpdump-issue, and > the tagger drivers have been unified so there should be no risk of any > regressions there either. The regressions with be exactly what you are trying to fix here. A MAC which does not understand the DSA tag and does the wrong thing, where as currently it is using EDSA and working. So i would keep things as they are by default. Andrew
> Andrew, Vladimir: I will just list dsa and edsa for now. If it is needed > on other devices, people can add them to the list after they have tested > their drivers. Fair? O.K. Andrew
On Tue, Apr 06, 2021 at 03:30:46PM +0200, Andrew Lunn wrote: > > Andrew, Vladimir: I will just list dsa and edsa for now. If it is needed > > on other devices, people can add them to the list after they have tested > > their drivers. Fair? > > O.K. Same here.