Re: [RFC] net: dsa: microchip: add KSZ8 change_tag_protocol support

[Krzysztof Kozlowski]

On 16/08/2024 08:59, vtpieter@xxxxxxxxx wrote:
> From: Pieter Van Trappen <pieter.van.trappen@xxxxxxx>
> 
> Add support for changing the KSZ8 switches tag protocol. In fact
> these devices can only enable or disable the tail tag, so there's
> really only three supported protocols:
> - DSA_TAG_PROTO_KSZ8795 for KSZ87xx
> - DSA_TAG_PROTO_KSZ9893 for KSZ88x3
> - DSA_TAG_PROTO_NONE
> 
> When disabled, this can be used as a workaround for the 'Common
> pitfalls using DSA setups' [1] to use the conduit network interface as
> a regular one, admittedly forgoing most DSA functionality and using
> the device as an unmanaged switch whilst allowing control
> operations (ethtool, PHY management, WoL). Implementing the new
> software-defined DSA tagging protocol tag_8021q [2] for these devices
> seems overkill for this use case at the time being.
> 
> Link: Documentation/networking/dsa/dsa.rst [1]
> Link: https://lpc.events/event/11/contributions/949/attachments/823/1555/paper.pdf [2]
> Signed-off-by: Pieter Van Trappen <pieter.van.trappen@xxxxxxx>
> ---
>  .../devicetree/bindings/net/dsa/dsa-port.yaml |  1 +
>  drivers/net/dsa/microchip/ksz8.h              |  2 ++
>  drivers/net/dsa/microchip/ksz8795.c           | 28 +++++++++++++++++++
>  drivers/net/dsa/microchip/ksz_common.c        | 19 +++++++++++--
>  drivers/net/dsa/microchip/ksz_common.h        |  2 ++
>  5 files changed, 49 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml b/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
> index 480120469953..ded8019b6ba6 100644
> --- a/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
> +++ b/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
> @@ -53,6 +53,7 @@ properties:
>      enum:
>        - dsa
>        - edsa
> +      - none
>        - ocelot
>        - ocelot-8021q
>        - rtl8_4

Please run scripts/checkpatch.pl and fix reported warnings. Then please
run `scripts/checkpatch.pl --strict` and (probably) fix more warnings.
Some warnings can be ignored, especially from --strict run, but the code
here looks like it needs a fix. Feel free to get in touch if the warning
is not clear.

Anyway, what does "none" mean in terms of protocol? Is there a "none"
protocol? Or you mean, disable tagging entirely?

Best regards,
Krzysztof