Re: [PATCH net-next v2 11/13] net: dsa: realtek: rtl8365mb: use DSA CPU port
From: Vladimir Oltean <olteanv@gmail.com>
Date: 2021-12-19 22:19:19
On Sat, Dec 18, 2021 at 05:14:23AM -0300, Luiz Angelo Daros de Luca wrote:
Instead of a fixed CPU port, assume that DSA is correct. Tested-by: Arınç ÜNAL <redacted> Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com> ---
I don't necessarily see the value added by this change. Since (I think) only a single port can be a CPU port, a sanity check seems to be missing that the CPU port in the device tree is the expected one. This seems to be missing with or without your patch. You are unnaturally splitting the initialization of a data structure between rtl8365mb_setup() and rtl8365mb_detect(). Maybe what you should do is keep everything in rtl8365mb_detect() where it is right now, and check in rtl8365mb_setup that the cpu_dp->index is equal to priv->cpu_port?
quoted hunk ↗ jump to hunk
drivers/net/dsa/realtek/rtl8365mb.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-)diff --git a/drivers/net/dsa/realtek/rtl8365mb.c b/drivers/net/dsa/realtek/rtl8365mb.c index a8f44538a87a..b79a4639b283 100644 --- a/drivers/net/dsa/realtek/rtl8365mb.c +++ b/drivers/net/dsa/realtek/rtl8365mb.c@@ -103,14 +103,13 @@ /* Chip-specific data and limits */ #define RTL8365MB_CHIP_ID_8365MB_VC 0x6367 -#define RTL8365MB_CPU_PORT_NUM_8365MB_VC 6 #define RTL8365MB_LEARN_LIMIT_MAX_8365MB_VC 2112 /* Family-specific data and limits */ #define RTL8365MB_PHYADDRMAX 7 #define RTL8365MB_NUM_PHYREGS 32 #define RTL8365MB_PHYREGMAX (RTL8365MB_NUM_PHYREGS - 1) -#define RTL8365MB_MAX_NUM_PORTS (RTL8365MB_CPU_PORT_NUM_8365MB_VC + 1) +#define RTL8365MB_MAX_NUM_PORTS 7 /* Chip identification registers */ #define RTL8365MB_CHIP_ID_REG 0x1300@@ -1827,9 +1826,18 @@ static int rtl8365mb_setup(struct dsa_switch *ds) dev_info(priv->dev, "no interrupt support\n"); /* Configure CPU tagging */ - ret = rtl8365mb_cpu_config(priv); - if (ret) - goto out_teardown_irq; + for (i = 0; i < priv->num_ports; i++) { + if (!(dsa_is_cpu_port(priv->ds, i))) + continue;
dsa_switch_for_each_cpu_port(cpu_dp, ds)
quoted hunk ↗ jump to hunk
+ priv->cpu_port = i; + mb->cpu.mask = BIT(priv->cpu_port); + mb->cpu.trap_port = priv->cpu_port; + ret = rtl8365mb_cpu_config(priv); + if (ret) + goto out_teardown_irq; + + break; + } /* Configure ports */ for (i = 0; i < priv->num_ports; i++) {@@ -1960,8 +1968,7 @@ static int rtl8365mb_detect(struct realtek_priv *priv) "found an RTL8365MB-VC switch (ver=0x%04x)\n", chip_ver); - priv->cpu_port = RTL8365MB_CPU_PORT_NUM_8365MB_VC; - priv->num_ports = priv->cpu_port + 1; + priv->num_ports = RTL8365MB_MAX_NUM_PORTS; mb->priv = priv; mb->chip_id = chip_id;@@ -1972,8 +1979,6 @@ static int rtl8365mb_detect(struct realtek_priv *priv) mb->jam_size = ARRAY_SIZE(rtl8365mb_init_jam_8365mb_vc); mb->cpu.enable = 1; - mb->cpu.mask = BIT(priv->cpu_port); - mb->cpu.trap_port = priv->cpu_port; mb->cpu.insert = RTL8365MB_CPU_INSERT_TO_ALL; mb->cpu.position = RTL8365MB_CPU_POS_AFTER_SA; mb->cpu.rx_length = RTL8365MB_CPU_RXLEN_64BYTES;-- 2.34.0