mbox series

[net-next,0/3] net: dsa: Allow default tag protocol to be overridden from DT

Message ID 20210326105648.2492411-1-tobias@waldekranz.com
Headers show
Series net: dsa: Allow default tag protocol to be overridden from DT | expand

Message

Tobias Waldekranz March 26, 2021, 10:56 a.m. UTC
This is logically the v2 of this patch:
https://lore.kernel.org/netdev/20210323102326.3677940-1-tobias@waldekranz.com/

In addition to the mv88e6xxx support to dynamically change the
protocol, it is now possible to override the protocol from the device
tree. This means that when a board vendor finds an incompatibility,
they can specify a working protocol in the DT, and users will not have
to worry about it.

Some background information:

In a system using an NXP T1023 SoC connected to a 6390X switch, we
noticed that TO_CPU frames where not reaching the CPU. This only
happened on hardware port 8. Looking at the DSA master interface
(dpaa-ethernet) we could see that an Rx error counter was bumped at
the same rate. The logs indicated a parser error.

It just so happens that a TO_CPU coming in on device 0, port 8, will
result in the first two bytes of the DSA tag being one of:

00 40
00 44
00 46

My guess was that since these values looked like 802.3 length fields,
the controller's parser would signal an error if the frame length did
not match what was in the header.

This was later confirmed using two different workarounds provided by
Vladimir. Unfortunately these either bypass or ignore the hardware
parser and thus robs working combinations of the ability to do RSS and
other nifty things. It was therefore decided to go with the option of
a DT override.

Tobias Waldekranz (3):
  net: dsa: mv88e6xxx: Allow dynamic reconfiguration of tag protocol
  net: dsa: Allow default tag protocol to be overridden from DT
  dt-bindings: net: dsa: Document dsa,tag-protocol property

 .../devicetree/bindings/net/dsa/dsa.yaml      |  7 ++
 drivers/net/dsa/mv88e6xxx/chip.c              | 41 +++++++-
 drivers/net/dsa/mv88e6xxx/chip.h              |  3 +
 include/net/dsa.h                             |  5 +
 net/dsa/dsa2.c                                | 95 +++++++++++++++----
 5 files changed, 132 insertions(+), 19 deletions(-)

Comments

Rob Herring (Arm) March 27, 2021, 6:13 p.m. UTC | #1
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

>
Tobias Waldekranz April 6, 2021, 9:52 a.m. UTC | #2
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
>>
Andrew Lunn April 6, 2021, 1:30 p.m. UTC | #3
> 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
Vladimir Oltean April 7, 2021, 11:34 p.m. UTC | #4
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.