| Message ID | 20240816065924.1094942-1-vtpieter@gmail.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [RFC] net: dsa: microchip: add KSZ8 change_tag_protocol support | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On 16/08/2024 08:59, vtpieter@gmail.com wrote: > From: Pieter Van Trappen <pieter.van.trappen@cern.ch> > > 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@cern.ch> > --- > .../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
On Friday 16 August 2024 at 09:12, Krzysztof Kozlowski <krzk@kernel.org> wrote: > > On 16/08/2024 08:59, vtpieter@gmail.com wrote: > > From: Pieter Van Trappen <pieter.van.trappen@cern.ch> > > > > 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 > > > > --- 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. Hi Krzysztof, thanks indeed I forgot to run it after my last modifications. I am aware that the dt-binding patch should be separate, I just thought it'd make more sense for this RFC to have these together. > Anyway, what does "none" mean in terms of protocol? Is there a "none" > protocol? Or you mean, disable tagging entirely? Indeed the 'none' protocol is DSA_TAG_PROTO_NONE which means disable tagging entirely. The concept with advantages and disadvantages is well described in the paper with link which i part of the commit message. Cheers, Pieter
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 diff --git a/drivers/net/dsa/microchip/ksz8.h b/drivers/net/dsa/microchip/ksz8.h index ae43077e76c3..802cb9d657bb 100644 --- a/drivers/net/dsa/microchip/ksz8.h +++ b/drivers/net/dsa/microchip/ksz8.h @@ -54,6 +54,8 @@ int ksz8_reset_switch(struct ksz_device *dev); int ksz8_switch_init(struct ksz_device *dev); void ksz8_switch_exit(struct ksz_device *dev); int ksz8_change_mtu(struct ksz_device *dev, int port, int mtu); +int ksz8_change_tag_protocol(struct ksz_device *dev, + enum dsa_tag_protocol proto); void ksz8_phylink_mac_link_up(struct phylink_config *config, struct phy_device *phydev, unsigned int mode, phy_interface_t interface, int speed, int duplex, diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c index d27b9c36d73f..0704d5404c5b 100644 --- a/drivers/net/dsa/microchip/ksz8795.c +++ b/drivers/net/dsa/microchip/ksz8795.c @@ -127,6 +127,34 @@ int ksz8_change_mtu(struct ksz_device *dev, int port, int mtu) return -EOPNOTSUPP; } +/** + * ksz8_change_tag_protocol - Change tag protocol + * @dev: The device structure. + * @proto: The requested protocol. + * + * This function allows changing the tag protocol. In fact the ksz8 + * devices can only enable or disable the tail tag, so there's really + * only 2 supported protocols. + * + * Return: 0 on success, -EPROTONOSUPPORT in case protocol not supported. + */ +int ksz8_change_tag_protocol(struct ksz_device *dev, + enum dsa_tag_protocol proto) +{ + const u32 *masks = dev->info->masks; + const u16 *regs = dev->info->regs; + + if ( (proto == DSA_TAG_PROTO_KSZ8795 && ksz_is_ksz87xx(dev)) || + (proto == DSA_TAG_PROTO_KSZ9893 && ksz_is_ksz88x3(dev)) ) + ksz_cfg(dev, regs[S_TAIL_TAG_CTRL], masks[SW_TAIL_TAG_ENABLE], true); + else if (proto == DSA_TAG_PROTO_NONE) + ksz_cfg(dev, regs[S_TAIL_TAG_CTRL], masks[SW_TAIL_TAG_ENABLE], false); + else + return -EPROTONOSUPPORT; + + return 0; +} + static int ksz8_port_queue_split(struct ksz_device *dev, int port, int queues) { u8 mask_4q, mask_2q; diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c index 1491099528be..a87f43908a9a 100644 --- a/drivers/net/dsa/microchip/ksz_common.c +++ b/drivers/net/dsa/microchip/ksz_common.c @@ -307,6 +307,7 @@ static const struct ksz_dev_ops ksz8_dev_ops = { .init = ksz8_switch_init, .exit = ksz8_switch_exit, .change_mtu = ksz8_change_mtu, + .change_tag_protocol = ksz8_change_tag_protocol, }; static void ksz9477_phylink_mac_link_up(struct phylink_config *config, @@ -2893,9 +2894,7 @@ static enum dsa_tag_protocol ksz_get_tag_protocol(struct dsa_switch *ds, struct ksz_device *dev = ds->priv; enum dsa_tag_protocol proto = DSA_TAG_PROTO_NONE; - if (dev->chip_id == KSZ8795_CHIP_ID || - dev->chip_id == KSZ8794_CHIP_ID || - dev->chip_id == KSZ8765_CHIP_ID) + if (ksz_is_ksz87xx(dev)) proto = DSA_TAG_PROTO_KSZ8795; if (dev->chip_id == KSZ8830_CHIP_ID || @@ -2917,12 +2916,25 @@ static enum dsa_tag_protocol ksz_get_tag_protocol(struct dsa_switch *ds, return proto; } +static int ksz_change_tag_protocol(struct dsa_switch *ds, + enum dsa_tag_protocol proto) +{ + struct ksz_device *dev = ds->priv; + + if (dev->dev_ops->change_tag_protocol) + return dev->dev_ops->change_tag_protocol(dev, proto); + else + return -EPROTONOSUPPORT; +} + static int ksz_connect_tag_protocol(struct dsa_switch *ds, enum dsa_tag_protocol proto) { struct ksz_tagger_data *tagger_data; switch (proto) { + case DSA_TAG_PROTO_NONE: + return 0; case DSA_TAG_PROTO_KSZ8795: return 0; case DSA_TAG_PROTO_KSZ9893: @@ -3974,6 +3986,7 @@ static int ksz_hsr_leave(struct dsa_switch *ds, int port, static const struct dsa_switch_ops ksz_switch_ops = { .get_tag_protocol = ksz_get_tag_protocol, + .change_tag_protocol = ksz_change_tag_protocol, .connect_tag_protocol = ksz_connect_tag_protocol, .get_phy_flags = ksz_get_phy_flags, .setup = ksz_setup, diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h index 5f0a628b9849..badae22f4613 100644 --- a/drivers/net/dsa/microchip/ksz_common.h +++ b/drivers/net/dsa/microchip/ksz_common.h @@ -354,6 +354,8 @@ struct ksz_dev_ops { void (*get_caps)(struct ksz_device *dev, int port, struct phylink_config *config); int (*change_mtu)(struct ksz_device *dev, int port, int mtu); + int (*change_tag_protocol)(struct ksz_device *dev, + enum dsa_tag_protocol proto); void (*freeze_mib)(struct ksz_device *dev, int port, bool freeze); void (*port_init_cnt)(struct ksz_device *dev, int port); void (*phylink_mac_link_up)(struct ksz_device *dev, int port,