* [PATCH net-next v2 0/5] net: dsa: realtek: rtl8365mb: improve handling of PHY modes @ 2022-06-10 15:38 Alvin Šipraga 2022-06-10 15:38 ` [PATCH net-next v2 1/5] net: dsa: realtek: rtl8365mb: rename macro RTL8367RB -> RTL8367RB_VB Alvin Šipraga ` (5 more replies) 0 siblings, 6 replies; 14+ messages in thread From: Alvin Šipraga @ 2022-06-10 15:38 UTC (permalink / raw) To: hauke, Linus Walleij, Alvin Šipraga, Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Russell King Cc: netdev, linux-kernel From: Alvin Šipraga <alsi@bang-olufsen.dk> This series introduces some minor cleanup of the driver and improves the handling of PHY interface modes to break the assumption that CPU ports are always over an external interface, and the assumption that user ports are always using an internal PHY. Changes v1 -> v2: - patches 1-4: no code change - add Luiz' reviewed-by to some of the patches - patch 5: put the chip_infos into a static array and get rid of the switch in the detect function; also remove the macros for various chip ID/versions and embed them directly into the array - patch 5: use array of size 3 rather than flexible array for extints in the chip_info struct; gcc complained about initialization of flexible array members in a nested context, and anyway, we know that the max number of external interfaces is 3 Alvin Šipraga (5): net: dsa: realtek: rtl8365mb: rename macro RTL8367RB -> RTL8367RB_VB net: dsa: realtek: rtl8365mb: remove port_mask private data member net: dsa: realtek: rtl8365mb: correct the max number of ports net: dsa: realtek: rtl8365mb: remove learn_limit_max private data member net: dsa: realtek: rtl8365mb: handle PHY interface modes correctly drivers/net/dsa/realtek/rtl8365mb.c | 299 ++++++++++++++++------------ 1 file changed, 177 insertions(+), 122 deletions(-) -- 2.36.1 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH net-next v2 1/5] net: dsa: realtek: rtl8365mb: rename macro RTL8367RB -> RTL8367RB_VB 2022-06-10 15:38 [PATCH net-next v2 0/5] net: dsa: realtek: rtl8365mb: improve handling of PHY modes Alvin Šipraga @ 2022-06-10 15:38 ` Alvin Šipraga 2022-06-10 15:38 ` [PATCH net-next v2 2/5] net: dsa: realtek: rtl8365mb: remove port_mask private data member Alvin Šipraga ` (4 subsequent siblings) 5 siblings, 0 replies; 14+ messages in thread From: Alvin Šipraga @ 2022-06-10 15:38 UTC (permalink / raw) To: hauke, Linus Walleij, Alvin Šipraga, Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Russell King Cc: Luiz Angelo Daros de Luca, netdev, linux-kernel From: Alvin Šipraga <alsi@bang-olufsen.dk> The official name of this switch is RTL8367RB-VB, not RTL8367RB. There is also an RTL8367RB-VC which is rather different. Change the name of the CHIP_ID/_VER macros for reasons of consistency. Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk> Reviewed-by: Luiz Angelo Daros de Luca <luizluca@gmail.com> --- drivers/net/dsa/realtek/rtl8365mb.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/dsa/realtek/rtl8365mb.c b/drivers/net/dsa/realtek/rtl8365mb.c index 769f672e9128..905056250b88 100644 --- a/drivers/net/dsa/realtek/rtl8365mb.c +++ b/drivers/net/dsa/realtek/rtl8365mb.c @@ -108,8 +108,8 @@ #define RTL8365MB_CHIP_ID_8367S 0x6367 #define RTL8365MB_CHIP_VER_8367S 0x00A0 -#define RTL8365MB_CHIP_ID_8367RB 0x6367 -#define RTL8365MB_CHIP_VER_8367RB 0x0020 +#define RTL8365MB_CHIP_ID_8367RB_VB 0x6367 +#define RTL8365MB_CHIP_VER_8367RB_VB 0x0020 /* Family-specific data and limits */ #define RTL8365MB_PHYADDRMAX 7 @@ -1988,7 +1988,7 @@ static int rtl8365mb_detect(struct realtek_priv *priv) "found an RTL8365MB-VC switch (ver=0x%04x)\n", chip_ver); break; - case RTL8365MB_CHIP_VER_8367RB: + case RTL8365MB_CHIP_VER_8367RB_VB: dev_info(priv->dev, "found an RTL8367RB-VB switch (ver=0x%04x)\n", chip_ver); -- 2.36.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH net-next v2 2/5] net: dsa: realtek: rtl8365mb: remove port_mask private data member 2022-06-10 15:38 [PATCH net-next v2 0/5] net: dsa: realtek: rtl8365mb: improve handling of PHY modes Alvin Šipraga 2022-06-10 15:38 ` [PATCH net-next v2 1/5] net: dsa: realtek: rtl8365mb: rename macro RTL8367RB -> RTL8367RB_VB Alvin Šipraga @ 2022-06-10 15:38 ` Alvin Šipraga 2022-06-10 15:38 ` [PATCH net-next v2 3/5] net: dsa: realtek: rtl8365mb: correct the max number of ports Alvin Šipraga ` (3 subsequent siblings) 5 siblings, 0 replies; 14+ messages in thread From: Alvin Šipraga @ 2022-06-10 15:38 UTC (permalink / raw) To: hauke, Linus Walleij, Alvin Šipraga, Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Russell King Cc: netdev, linux-kernel From: Alvin Šipraga <alsi@bang-olufsen.dk> There is no real need for this variable: the line change interrupt mask is sufficiently masked out when getting linkup_ind and linkdown_ind in the interrupt handler. Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk> --- drivers/net/dsa/realtek/rtl8365mb.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/net/dsa/realtek/rtl8365mb.c b/drivers/net/dsa/realtek/rtl8365mb.c index 905056250b88..42afba122bb4 100644 --- a/drivers/net/dsa/realtek/rtl8365mb.c +++ b/drivers/net/dsa/realtek/rtl8365mb.c @@ -564,7 +564,6 @@ struct rtl8365mb_port { * @irq: registered IRQ or zero * @chip_id: chip identifier * @chip_ver: chip silicon revision - * @port_mask: mask of all ports * @learn_limit_max: maximum number of L2 addresses the chip can learn * @cpu: CPU tagging and CPU port configuration for this chip * @mib_lock: prevent concurrent reads of MIB counters @@ -579,7 +578,6 @@ struct rtl8365mb { int irq; u32 chip_id; u32 chip_ver; - u32 port_mask; u32 learn_limit_max; struct rtl8365mb_cpu cpu; struct mutex mib_lock; @@ -1489,13 +1487,10 @@ static irqreturn_t rtl8365mb_irq(int irq, void *data) { struct realtek_priv *priv = data; unsigned long line_changes = 0; - struct rtl8365mb *mb; u32 stat; int line; int ret; - mb = priv->chip_data; - ret = rtl8365mb_get_and_clear_status_reg(priv, RTL8365MB_INTR_STATUS_REG, &stat); if (ret) @@ -1520,7 +1515,7 @@ static irqreturn_t rtl8365mb_irq(int irq, void *data) linkdown_ind = FIELD_GET(RTL8365MB_PORT_LINKDOWN_IND_MASK, val); - line_changes = (linkup_ind | linkdown_ind) & mb->port_mask; + line_changes = linkup_ind | linkdown_ind; } if (!line_changes) @@ -2009,7 +2004,6 @@ static int rtl8365mb_detect(struct realtek_priv *priv) mb->priv = priv; mb->chip_id = chip_id; mb->chip_ver = chip_ver; - mb->port_mask = GENMASK(priv->num_ports - 1, 0); mb->learn_limit_max = RTL8365MB_LEARN_LIMIT_MAX; mb->jam_table = rtl8365mb_init_jam_8365mb_vc; mb->jam_size = ARRAY_SIZE(rtl8365mb_init_jam_8365mb_vc); -- 2.36.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH net-next v2 3/5] net: dsa: realtek: rtl8365mb: correct the max number of ports 2022-06-10 15:38 [PATCH net-next v2 0/5] net: dsa: realtek: rtl8365mb: improve handling of PHY modes Alvin Šipraga 2022-06-10 15:38 ` [PATCH net-next v2 1/5] net: dsa: realtek: rtl8365mb: rename macro RTL8367RB -> RTL8367RB_VB Alvin Šipraga 2022-06-10 15:38 ` [PATCH net-next v2 2/5] net: dsa: realtek: rtl8365mb: remove port_mask private data member Alvin Šipraga @ 2022-06-10 15:38 ` Alvin Šipraga 2022-06-10 15:38 ` [PATCH net-next v2 4/5] net: dsa: realtek: rtl8365mb: remove learn_limit_max private data member Alvin Šipraga ` (2 subsequent siblings) 5 siblings, 0 replies; 14+ messages in thread From: Alvin Šipraga @ 2022-06-10 15:38 UTC (permalink / raw) To: hauke, Linus Walleij, Alvin Šipraga, Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Russell King Cc: Luiz Angelo Daros de Luca, netdev, linux-kernel From: Alvin Šipraga <alsi@bang-olufsen.dk> The maximum number of ports is actually 11, according to two observations: 1. The highest port ID used in the vendor driver is 10. Since port IDs are indexed from 0, and since DSA follows the same numbering system, this means up to 11 ports are to be presumed. 2. The registers with port mask fields always amount to a maximum port mask of 0x7FF, corresponding to a maximum 11 ports. In view of this, I also deleted the comment. Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk> Reviewed-by: Luiz Angelo Daros de Luca <luizluca@gmail.com> --- drivers/net/dsa/realtek/rtl8365mb.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/net/dsa/realtek/rtl8365mb.c b/drivers/net/dsa/realtek/rtl8365mb.c index 42afba122bb4..3599fa5d9f14 100644 --- a/drivers/net/dsa/realtek/rtl8365mb.c +++ b/drivers/net/dsa/realtek/rtl8365mb.c @@ -115,8 +115,7 @@ #define RTL8365MB_PHYADDRMAX 7 #define RTL8365MB_NUM_PHYREGS 32 #define RTL8365MB_PHYREGMAX (RTL8365MB_NUM_PHYREGS - 1) -/* RTL8370MB and RTL8310SR, possibly suportable by this driver, have 10 ports */ -#define RTL8365MB_MAX_NUM_PORTS 10 +#define RTL8365MB_MAX_NUM_PORTS 11 #define RTL8365MB_LEARN_LIMIT_MAX 2112 /* valid for all 6-port or less variants */ -- 2.36.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH net-next v2 4/5] net: dsa: realtek: rtl8365mb: remove learn_limit_max private data member 2022-06-10 15:38 [PATCH net-next v2 0/5] net: dsa: realtek: rtl8365mb: improve handling of PHY modes Alvin Šipraga ` (2 preceding siblings ...) 2022-06-10 15:38 ` [PATCH net-next v2 3/5] net: dsa: realtek: rtl8365mb: correct the max number of ports Alvin Šipraga @ 2022-06-10 15:38 ` Alvin Šipraga 2022-06-12 1:40 ` Luiz Angelo Daros de Luca 2022-06-10 15:38 ` [PATCH net-next v2 5/5] net: dsa: realtek: rtl8365mb: handle PHY interface modes correctly Alvin Šipraga 2022-06-15 16:55 ` [PATCH net-next v2 0/5] net: dsa: realtek: rtl8365mb: improve handling of PHY modes Alvin Šipraga 5 siblings, 1 reply; 14+ messages in thread From: Alvin Šipraga @ 2022-06-10 15:38 UTC (permalink / raw) To: hauke, Linus Walleij, Alvin Šipraga, Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Russell King Cc: netdev, linux-kernel From: Alvin Šipraga <alsi@bang-olufsen.dk> The variable is just assigned the value of a macro, so it can be removed. Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk> --- drivers/net/dsa/realtek/rtl8365mb.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/drivers/net/dsa/realtek/rtl8365mb.c b/drivers/net/dsa/realtek/rtl8365mb.c index 3599fa5d9f14..676b88798976 100644 --- a/drivers/net/dsa/realtek/rtl8365mb.c +++ b/drivers/net/dsa/realtek/rtl8365mb.c @@ -563,7 +563,6 @@ struct rtl8365mb_port { * @irq: registered IRQ or zero * @chip_id: chip identifier * @chip_ver: chip silicon revision - * @learn_limit_max: maximum number of L2 addresses the chip can learn * @cpu: CPU tagging and CPU port configuration for this chip * @mib_lock: prevent concurrent reads of MIB counters * @ports: per-port data @@ -577,7 +576,6 @@ struct rtl8365mb { int irq; u32 chip_id; u32 chip_ver; - u32 learn_limit_max; struct rtl8365mb_cpu cpu; struct mutex mib_lock; struct rtl8365mb_port ports[RTL8365MB_MAX_NUM_PORTS]; @@ -1088,15 +1086,13 @@ static void rtl8365mb_port_stp_state_set(struct dsa_switch *ds, int port, static int rtl8365mb_port_set_learning(struct realtek_priv *priv, int port, bool enable) { - struct rtl8365mb *mb = priv->chip_data; - /* Enable/disable learning by limiting the number of L2 addresses the * port can learn. Realtek documentation states that a limit of zero * disables learning. When enabling learning, set it to the chip's * maximum. */ return regmap_write(priv->map, RTL8365MB_LUT_PORT_LEARN_LIMIT_REG(port), - enable ? mb->learn_limit_max : 0); + enable ? RTL8365MB_LEARN_LIMIT_MAX : 0); } static int rtl8365mb_port_set_isolation(struct realtek_priv *priv, int port, @@ -2003,7 +1999,6 @@ static int rtl8365mb_detect(struct realtek_priv *priv) mb->priv = priv; mb->chip_id = chip_id; mb->chip_ver = chip_ver; - mb->learn_limit_max = RTL8365MB_LEARN_LIMIT_MAX; mb->jam_table = rtl8365mb_init_jam_8365mb_vc; mb->jam_size = ARRAY_SIZE(rtl8365mb_init_jam_8365mb_vc); -- 2.36.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH net-next v2 4/5] net: dsa: realtek: rtl8365mb: remove learn_limit_max private data member 2022-06-10 15:38 ` [PATCH net-next v2 4/5] net: dsa: realtek: rtl8365mb: remove learn_limit_max private data member Alvin Šipraga @ 2022-06-12 1:40 ` Luiz Angelo Daros de Luca 2022-06-12 10:56 ` Alvin Šipraga 0 siblings, 1 reply; 14+ messages in thread From: Luiz Angelo Daros de Luca @ 2022-06-12 1:40 UTC (permalink / raw) To: Alvin Šipraga Cc: Hauke Mehrtens, Linus Walleij, Alvin Šipraga, Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Russell King, open list:NETWORKING DRIVERS, open list > The variable is just assigned the value of a macro, so it can be > removed. As I commented previously, the switches in this family with 10 ports do have a different value for RTL8365MB_LEARN_LIMIT_MAX. Once we add support for one of those models, we will somewhat revert this patch. I believe learn_limit_max would fit better inside the new static chip_info structure. Regards, Luiz > Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk> > --- > drivers/net/dsa/realtek/rtl8365mb.c | 7 +------ > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/drivers/net/dsa/realtek/rtl8365mb.c b/drivers/net/dsa/realtek/rtl8365mb.c > index 3599fa5d9f14..676b88798976 100644 > --- a/drivers/net/dsa/realtek/rtl8365mb.c > +++ b/drivers/net/dsa/realtek/rtl8365mb.c > @@ -563,7 +563,6 @@ struct rtl8365mb_port { > * @irq: registered IRQ or zero > * @chip_id: chip identifier > * @chip_ver: chip silicon revision > - * @learn_limit_max: maximum number of L2 addresses the chip can learn > * @cpu: CPU tagging and CPU port configuration for this chip > * @mib_lock: prevent concurrent reads of MIB counters > * @ports: per-port data > @@ -577,7 +576,6 @@ struct rtl8365mb { > int irq; > u32 chip_id; > u32 chip_ver; > - u32 learn_limit_max; > struct rtl8365mb_cpu cpu; > struct mutex mib_lock; > struct rtl8365mb_port ports[RTL8365MB_MAX_NUM_PORTS]; > @@ -1088,15 +1086,13 @@ static void rtl8365mb_port_stp_state_set(struct dsa_switch *ds, int port, > static int rtl8365mb_port_set_learning(struct realtek_priv *priv, int port, > bool enable) > { > - struct rtl8365mb *mb = priv->chip_data; > - > /* Enable/disable learning by limiting the number of L2 addresses the > * port can learn. Realtek documentation states that a limit of zero > * disables learning. When enabling learning, set it to the chip's > * maximum. > */ > return regmap_write(priv->map, RTL8365MB_LUT_PORT_LEARN_LIMIT_REG(port), > - enable ? mb->learn_limit_max : 0); > + enable ? RTL8365MB_LEARN_LIMIT_MAX : 0); > } > > static int rtl8365mb_port_set_isolation(struct realtek_priv *priv, int port, > @@ -2003,7 +1999,6 @@ static int rtl8365mb_detect(struct realtek_priv *priv) > mb->priv = priv; > mb->chip_id = chip_id; > mb->chip_ver = chip_ver; > - mb->learn_limit_max = RTL8365MB_LEARN_LIMIT_MAX; > mb->jam_table = rtl8365mb_init_jam_8365mb_vc; > mb->jam_size = ARRAY_SIZE(rtl8365mb_init_jam_8365mb_vc); > > -- > 2.36.1 > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net-next v2 4/5] net: dsa: realtek: rtl8365mb: remove learn_limit_max private data member 2022-06-12 1:40 ` Luiz Angelo Daros de Luca @ 2022-06-12 10:56 ` Alvin Šipraga 0 siblings, 0 replies; 14+ messages in thread From: Alvin Šipraga @ 2022-06-12 10:56 UTC (permalink / raw) To: Luiz Angelo Daros de Luca Cc: Alvin Šipraga, Hauke Mehrtens, Linus Walleij, Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Russell King, open list:NETWORKING DRIVERS, open list On Sat, Jun 11, 2022 at 10:40:33PM -0300, Luiz Angelo Daros de Luca wrote: > > The variable is just assigned the value of a macro, so it can be > > removed. > > As I commented previously, the switches in this family with 10 ports > do have a different value for RTL8365MB_LEARN_LIMIT_MAX. > Once we add support for one of those models, we will somewhat revert this patch. I wouldn't call that a revert, just normal development. > > I believe learn_limit_max would fit better inside the new static > chip_info structure. Other pedants may ask me what the point of such a patch is when the hardware is not even supported. That was my main reason for not incorporating your suggestion. The other reason is that, having actually experimented with the learn limit myself, I could in fact make my RTL8365MB-VC learn more than this presupposed maximum the vendor driver uses. I think it also depends on whether IVL/SVL is in use. So there might be more to it than you think. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH net-next v2 5/5] net: dsa: realtek: rtl8365mb: handle PHY interface modes correctly 2022-06-10 15:38 [PATCH net-next v2 0/5] net: dsa: realtek: rtl8365mb: improve handling of PHY modes Alvin Šipraga ` (3 preceding siblings ...) 2022-06-10 15:38 ` [PATCH net-next v2 4/5] net: dsa: realtek: rtl8365mb: remove learn_limit_max private data member Alvin Šipraga @ 2022-06-10 15:38 ` Alvin Šipraga 2022-06-10 16:36 ` Russell King (Oracle) 2022-06-12 3:20 ` Luiz Angelo Daros de Luca 2022-06-15 16:55 ` [PATCH net-next v2 0/5] net: dsa: realtek: rtl8365mb: improve handling of PHY modes Alvin Šipraga 5 siblings, 2 replies; 14+ messages in thread From: Alvin Šipraga @ 2022-06-10 15:38 UTC (permalink / raw) To: hauke, Linus Walleij, Alvin Šipraga, Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Russell King Cc: netdev, linux-kernel From: Alvin Šipraga <alsi@bang-olufsen.dk> Realtek switches in the rtl8365mb family always have at least one port with a so-called external interface, supporting PHY interface modes such as RGMII or SGMII. The purpose of this patch is to improve the driver's handling of these ports. A new struct rtl8365mb_chip_info is introduced together with a static array of such structs. An instance of this struct is added for each supported switch, distinguished by its chip ID and version. Embedded in each chip_info struct is an array of struct rtl8365mb_extint, describing the external interfaces available. This is more specific than the old rtl8365mb_extint_port_map, which was only valid for switches with up to 6 ports. The struct rtl8365mb_extint also contains a bitmask of supported PHY interface modes, which allows the driver to distinguish which ports support RGMII. This corrects a previous mistake in the driver whereby it was assumed that any port with an external interface supports RGMII. This is not actually the case: for example, the RTL8367S has two external interfaces, only the second of which supports RGMII. The first supports only SGMII and HSGMII. This new design will make it easier to add support for other interface modes. Finally, rtl8365mb_phylink_get_caps() is fixed up to return supported capabilities based on the external interface properties described above. This allows for ports with an external interface to be treated as DSA user ports, and for ports with an internal PHY to be treated as DSA CPU ports. Link: https://lore.kernel.org/netdev/20220510192301.5djdt3ghoavxulhl@bang-olufsen.dk/ Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk> --- drivers/net/dsa/realtek/rtl8365mb.c | 281 +++++++++++++++++----------- 1 file changed, 174 insertions(+), 107 deletions(-) diff --git a/drivers/net/dsa/realtek/rtl8365mb.c b/drivers/net/dsa/realtek/rtl8365mb.c index 676b88798976..da31d8b839ac 100644 --- a/drivers/net/dsa/realtek/rtl8365mb.c +++ b/drivers/net/dsa/realtek/rtl8365mb.c @@ -101,26 +101,14 @@ #include "realtek.h" -/* Chip-specific data and limits */ -#define RTL8365MB_CHIP_ID_8365MB_VC 0x6367 -#define RTL8365MB_CHIP_VER_8365MB_VC 0x0040 - -#define RTL8365MB_CHIP_ID_8367S 0x6367 -#define RTL8365MB_CHIP_VER_8367S 0x00A0 - -#define RTL8365MB_CHIP_ID_8367RB_VB 0x6367 -#define RTL8365MB_CHIP_VER_8367RB_VB 0x0020 - /* 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 11 +#define RTL8365MB_MAX_NUM_EXTINTS 3 #define RTL8365MB_LEARN_LIMIT_MAX 2112 -/* valid for all 6-port or less variants */ -static const int rtl8365mb_extint_port_map[] = { -1, -1, -1, -1, -1, -1, 1, 2, -1, -1}; - /* Chip identification registers */ #define RTL8365MB_CHIP_ID_REG 0x1300 @@ -200,7 +188,7 @@ static const int rtl8365mb_extint_port_map[] = { -1, -1, -1, -1, -1, -1, 1, 2, /* The PHY OCP addresses of PHY registers 0~31 start here */ #define RTL8365MB_PHY_OCP_ADDR_PHYREG_BASE 0xA400 -/* EXT interface port mode values - used in DIGITAL_INTERFACE_SELECT */ +/* External interface port mode values - used in DIGITAL_INTERFACE_SELECT */ #define RTL8365MB_EXT_PORT_MODE_DISABLE 0 #define RTL8365MB_EXT_PORT_MODE_RGMII 1 #define RTL8365MB_EXT_PORT_MODE_MII_MAC 2 @@ -216,19 +204,7 @@ static const int rtl8365mb_extint_port_map[] = { -1, -1, -1, -1, -1, -1, 1, 2, #define RTL8365MB_EXT_PORT_MODE_1000X 12 #define RTL8365MB_EXT_PORT_MODE_100FX 13 -/* Realtek docs and driver uses logic number as EXT_PORT0=16, EXT_PORT1=17, - * EXT_PORT2=18, to interact with switch ports. That logic number is internally - * converted to either a physical port number (0..9) or an external interface id (0..2), - * depending on which function was called. The external interface id is calculated as - * (ext_id=logic_port-15), while the logical to physical map depends on the chip id/version. - * - * EXT_PORT0 mentioned in datasheets and rtl8367c driver is used in this driver - * as extid==1, EXT_PORT2, mentioned in Realtek rtl8367c driver for 10-port switches, - * would have an ext_id of 3 (out of range for most extint macros) and ext_id 0 does - * not seem to be used as well for this family. - */ - -/* EXT interface mode configuration registers 0~1 */ +/* External interface mode configuration registers 0~1 */ #define RTL8365MB_DIGITAL_INTERFACE_SELECT_REG0 0x1305 /* EXT1 */ #define RTL8365MB_DIGITAL_INTERFACE_SELECT_REG1 0x13C3 /* EXT2 */ #define RTL8365MB_DIGITAL_INTERFACE_SELECT_REG(_extint) \ @@ -240,7 +216,7 @@ static const int rtl8365mb_extint_port_map[] = { -1, -1, -1, -1, -1, -1, 1, 2, #define RTL8365MB_DIGITAL_INTERFACE_SELECT_MODE_OFFSET(_extint) \ (((_extint) % 2) * 4) -/* EXT interface RGMII TX/RX delay configuration registers 0~2 */ +/* External interface RGMII TX/RX delay configuration registers 0~2 */ #define RTL8365MB_EXT_RGMXF_REG0 0x1306 /* EXT0 */ #define RTL8365MB_EXT_RGMXF_REG1 0x1307 /* EXT1 */ #define RTL8365MB_EXT_RGMXF_REG2 0x13C5 /* EXT2 */ @@ -257,7 +233,7 @@ static const int rtl8365mb_extint_port_map[] = { -1, -1, -1, -1, -1, -1, 1, 2, #define RTL8365MB_PORT_SPEED_100M 1 #define RTL8365MB_PORT_SPEED_1000M 2 -/* EXT interface force configuration registers 0~2 */ +/* External interface force configuration registers 0~2 */ #define RTL8365MB_DIGITAL_INTERFACE_FORCE_REG0 0x1310 /* EXT0 */ #define RTL8365MB_DIGITAL_INTERFACE_FORCE_REG1 0x1311 /* EXT1 */ #define RTL8365MB_DIGITAL_INTERFACE_FORCE_REG2 0x13C4 /* EXT2 */ @@ -489,6 +465,95 @@ static const struct rtl8365mb_jam_tbl_entry rtl8365mb_init_jam_common[] = { { 0x1D32, 0x0002 }, }; +enum rtl8365mb_phy_interface_mode { + RTL8365MB_PHY_INTERFACE_MODE_INVAL = 0, + RTL8365MB_PHY_INTERFACE_MODE_INTERNAL = BIT(0), + RTL8365MB_PHY_INTERFACE_MODE_MII = BIT(1), + RTL8365MB_PHY_INTERFACE_MODE_TMII = BIT(2), + RTL8365MB_PHY_INTERFACE_MODE_RMII = BIT(3), + RTL8365MB_PHY_INTERFACE_MODE_RGMII = BIT(4), + RTL8365MB_PHY_INTERFACE_MODE_SGMII = BIT(5), + RTL8365MB_PHY_INTERFACE_MODE_HSGMII = BIT(6), +}; + +/** + * struct rtl8365mb_extint - external interface info + * @port: the port with an external interface + * @id: the external interface ID, which is either 0, 1, or 2 + * @supported_interfaces: a bitmask of supported PHY interface modes + * + * Represents a mapping: port -> { id, supported_interfaces }. To be embedded + * in &struct rtl8365mb_chip_info for every port with an external interface. + */ +struct rtl8365mb_extint { + int port; + int id; + unsigned int supported_interfaces; +}; + +/** + * struct rtl8365mb_chip_info - static chip-specific info + * @name: human-readable chip name + * @chip_id: chip identifier + * @chip_ver: chip silicon revision + * @extints: available external interfaces + * @jam_table: chip-specific initialization jam table + * @jam_size: size of the chip's jam table + * + * These data are specific to a given chip in the family of switches supported + * by this driver. When adding support for another chip in the family, a new + * chip info should be added to the rtl8365mb_chip_infos array. + */ +struct rtl8365mb_chip_info { + const char *name; + u32 chip_id; + u32 chip_ver; + const struct rtl8365mb_extint extints[RTL8365MB_MAX_NUM_EXTINTS]; + const struct rtl8365mb_jam_tbl_entry *jam_table; + size_t jam_size; +}; + +/* Chip info for each supported switch in the family */ +#define PHY_INTF(_mode) (RTL8365MB_PHY_INTERFACE_MODE_ ## _mode) +static const struct rtl8365mb_chip_info rtl8365mb_chip_infos[] = { + { + .name = "RTL8365MB-VC", + .chip_id = 0x6367, + .chip_ver = 0x0040, + .extints = { + { 6, 1, PHY_INTF(MII) | PHY_INTF(TMII) | + PHY_INTF(RMII) | PHY_INTF(RGMII) }, + }, + .jam_table = rtl8365mb_init_jam_8365mb_vc, + .jam_size = ARRAY_SIZE(rtl8365mb_init_jam_8365mb_vc), + }, + { + .name = "RTL8367S", + .chip_id = 0x6367, + .chip_ver = 0x00A0, + .extints = { + { 6, 1, PHY_INTF(SGMII) | PHY_INTF(HSGMII) }, + { 7, 2, PHY_INTF(MII) | PHY_INTF(TMII) | + PHY_INTF(RMII) | PHY_INTF(RGMII) }, + }, + .jam_table = rtl8365mb_init_jam_8365mb_vc, + .jam_size = ARRAY_SIZE(rtl8365mb_init_jam_8365mb_vc), + }, + { + .name = "RTL8367RB-VB", + .chip_id = 0x6367, + .chip_ver = 0x0020, + .extints = { + { 6, 1, PHY_INTF(MII) | PHY_INTF(TMII) | + PHY_INTF(RMII) | PHY_INTF(RGMII) }, + { 7, 2, PHY_INTF(MII) | PHY_INTF(TMII) | + PHY_INTF(RMII) | PHY_INTF(RGMII) }, + }, + .jam_table = rtl8365mb_init_jam_8365mb_vc, + .jam_size = ARRAY_SIZE(rtl8365mb_init_jam_8365mb_vc), + }, +}; + enum rtl8365mb_stp_state { RTL8365MB_STP_STATE_DISABLED = 0, RTL8365MB_STP_STATE_BLOCKING = 1, @@ -558,29 +623,23 @@ struct rtl8365mb_port { }; /** - * struct rtl8365mb - private chip-specific driver data + * struct rtl8365mb - driver private data * @priv: pointer to parent realtek_priv data * @irq: registered IRQ or zero - * @chip_id: chip identifier - * @chip_ver: chip silicon revision + * @chip_info: chip-specific info about the attached switch * @cpu: CPU tagging and CPU port configuration for this chip * @mib_lock: prevent concurrent reads of MIB counters * @ports: per-port data - * @jam_table: chip-specific initialization jam table - * @jam_size: size of the chip's jam table * * Private data for this driver. */ struct rtl8365mb { struct realtek_priv *priv; int irq; - u32 chip_id; - u32 chip_ver; + const struct rtl8365mb_chip_info *chip_info; struct rtl8365mb_cpu cpu; struct mutex mib_lock; struct rtl8365mb_port ports[RTL8365MB_MAX_NUM_PORTS]; - const struct rtl8365mb_jam_tbl_entry *jam_table; - size_t jam_size; }; static int rtl8365mb_phy_poll_busy(struct realtek_priv *priv) @@ -775,6 +834,26 @@ static int rtl8365mb_dsa_phy_write(struct dsa_switch *ds, int phy, int regnum, return rtl8365mb_phy_write(ds->priv, phy, regnum, val); } +static const struct rtl8365mb_extint * +rtl8365mb_get_port_extint(struct realtek_priv *priv, int port) +{ + struct rtl8365mb *mb = priv->chip_data; + int i; + + for (i = 0; i < RTL8365MB_MAX_NUM_EXTINTS; i++) { + const struct rtl8365mb_extint *extint = + &mb->chip_info->extints[i]; + + if (!extint->supported_interfaces) + continue; + + if (extint->port == port) + return extint; + } + + return NULL; +} + static enum dsa_tag_protocol rtl8365mb_get_tag_protocol(struct dsa_switch *ds, int port, enum dsa_tag_protocol mp) @@ -795,20 +874,17 @@ rtl8365mb_get_tag_protocol(struct dsa_switch *ds, int port, static int rtl8365mb_ext_config_rgmii(struct realtek_priv *priv, int port, phy_interface_t interface) { + const struct rtl8365mb_extint *extint = + rtl8365mb_get_port_extint(priv, port); struct device_node *dn; struct dsa_port *dp; int tx_delay = 0; int rx_delay = 0; - int ext_int; u32 val; int ret; - ext_int = rtl8365mb_extint_port_map[port]; - - if (ext_int <= 0) { - dev_err(priv->dev, "Port %d is not an external interface port\n", port); - return -EINVAL; - } + if (!extint) + return -ENODEV; dp = dsa_to_port(priv->ds, port); dn = dp->dn; @@ -842,7 +918,7 @@ static int rtl8365mb_ext_config_rgmii(struct realtek_priv *priv, int port, tx_delay = val / 2; else dev_warn(priv->dev, - "EXT interface TX delay must be 0 or 2 ns\n"); + "RGMII TX delay must be 0 or 2 ns\n"); } if (!of_property_read_u32(dn, "rx-internal-delay-ps", &val)) { @@ -852,11 +928,11 @@ static int rtl8365mb_ext_config_rgmii(struct realtek_priv *priv, int port, rx_delay = val; else dev_warn(priv->dev, - "EXT interface RX delay must be 0 to 2.1 ns\n"); + "RGMII RX delay must be 0 to 2.1 ns\n"); } ret = regmap_update_bits( - priv->map, RTL8365MB_EXT_RGMXF_REG(ext_int), + priv->map, RTL8365MB_EXT_RGMXF_REG(extint->id), RTL8365MB_EXT_RGMXF_TXDELAY_MASK | RTL8365MB_EXT_RGMXF_RXDELAY_MASK, FIELD_PREP(RTL8365MB_EXT_RGMXF_TXDELAY_MASK, tx_delay) | @@ -865,11 +941,11 @@ static int rtl8365mb_ext_config_rgmii(struct realtek_priv *priv, int port, return ret; ret = regmap_update_bits( - priv->map, RTL8365MB_DIGITAL_INTERFACE_SELECT_REG(ext_int), - RTL8365MB_DIGITAL_INTERFACE_SELECT_MODE_MASK(ext_int), + priv->map, RTL8365MB_DIGITAL_INTERFACE_SELECT_REG(extint->id), + RTL8365MB_DIGITAL_INTERFACE_SELECT_MODE_MASK(extint->id), RTL8365MB_EXT_PORT_MODE_RGMII << RTL8365MB_DIGITAL_INTERFACE_SELECT_MODE_OFFSET( - ext_int)); + extint->id)); if (ret) return ret; @@ -880,21 +956,18 @@ static int rtl8365mb_ext_config_forcemode(struct realtek_priv *priv, int port, bool link, int speed, int duplex, bool tx_pause, bool rx_pause) { + const struct rtl8365mb_extint *extint = + rtl8365mb_get_port_extint(priv, port); u32 r_tx_pause; u32 r_rx_pause; u32 r_duplex; u32 r_speed; u32 r_link; - int ext_int; int val; int ret; - ext_int = rtl8365mb_extint_port_map[port]; - - if (ext_int <= 0) { - dev_err(priv->dev, "Port %d is not an external interface port\n", port); - return -EINVAL; - } + if (!extint) + return -ENODEV; if (link) { /* Force the link up with the desired configuration */ @@ -942,7 +1015,7 @@ static int rtl8365mb_ext_config_forcemode(struct realtek_priv *priv, int port, r_duplex) | FIELD_PREP(RTL8365MB_DIGITAL_INTERFACE_FORCE_SPEED_MASK, r_speed); ret = regmap_write(priv->map, - RTL8365MB_DIGITAL_INTERFACE_FORCE_REG(ext_int), + RTL8365MB_DIGITAL_INTERFACE_FORCE_REG(extint->id), val); if (ret) return ret; @@ -953,7 +1026,13 @@ static int rtl8365mb_ext_config_forcemode(struct realtek_priv *priv, int port, static void rtl8365mb_phylink_get_caps(struct dsa_switch *ds, int port, struct phylink_config *config) { - if (dsa_is_user_port(ds, port)) { + const struct rtl8365mb_extint *extint = + rtl8365mb_get_port_extint(ds->priv, port); + + config->mac_capabilities = MAC_SYM_PAUSE | MAC_ASYM_PAUSE | + MAC_10 | MAC_100 | MAC_1000FD; + + if (!extint) { __set_bit(PHY_INTERFACE_MODE_INTERNAL, config->supported_interfaces); @@ -962,12 +1041,16 @@ static void rtl8365mb_phylink_get_caps(struct dsa_switch *ds, int port, */ __set_bit(PHY_INTERFACE_MODE_GMII, config->supported_interfaces); - } else if (dsa_is_cpu_port(ds, port)) { - phy_interface_set_rgmii(config->supported_interfaces); + return; } - config->mac_capabilities = MAC_SYM_PAUSE | MAC_ASYM_PAUSE | - MAC_10 | MAC_100 | MAC_1000FD; + /* Populate according to the modes supported by _this driver_, + * not necessarily the modes supported by the hardware, some of + * which remain unimplemented. + */ + + if (extint->supported_interfaces & RTL8365MB_PHY_INTERFACE_MODE_RGMII) + phy_interface_set_rgmii(config->supported_interfaces); } static void rtl8365mb_phylink_mac_config(struct dsa_switch *ds, int port, @@ -1782,14 +1865,17 @@ static int rtl8365mb_change_tag_protocol(struct dsa_switch *ds, static int rtl8365mb_switch_init(struct realtek_priv *priv) { struct rtl8365mb *mb = priv->chip_data; + const struct rtl8365mb_chip_info *ci; int ret; int i; + ci = mb->chip_info; + /* Do any chip-specific init jam before getting to the common stuff */ - if (mb->jam_table) { - for (i = 0; i < mb->jam_size; i++) { - ret = regmap_write(priv->map, mb->jam_table[i].reg, - mb->jam_table[i].val); + if (ci->jam_table) { + for (i = 0; i < ci->jam_size; i++) { + ret = regmap_write(priv->map, ci->jam_table[i].reg, + ci->jam_table[i].val); if (ret) return ret; } @@ -1962,6 +2048,7 @@ static int rtl8365mb_detect(struct realtek_priv *priv) u32 chip_id; u32 chip_ver; int ret; + int i; ret = rtl8365mb_get_chip_id_and_ver(priv->map, &chip_id, &chip_ver); if (ret) { @@ -1970,52 +2057,32 @@ static int rtl8365mb_detect(struct realtek_priv *priv) return ret; } - switch (chip_id) { - case RTL8365MB_CHIP_ID_8365MB_VC: - switch (chip_ver) { - case RTL8365MB_CHIP_VER_8365MB_VC: - dev_info(priv->dev, - "found an RTL8365MB-VC switch (ver=0x%04x)\n", - chip_ver); - break; - case RTL8365MB_CHIP_VER_8367RB_VB: - dev_info(priv->dev, - "found an RTL8367RB-VB switch (ver=0x%04x)\n", - chip_ver); - break; - case RTL8365MB_CHIP_VER_8367S: - dev_info(priv->dev, - "found an RTL8367S switch (ver=0x%04x)\n", - chip_ver); + for (i = 0; i < ARRAY_SIZE(rtl8365mb_chip_infos); i++) { + const struct rtl8365mb_chip_info *ci = &rtl8365mb_chip_infos[i]; + + if (ci->chip_id == chip_id && ci->chip_ver == chip_ver) { + mb->chip_info = ci; break; - default: - dev_err(priv->dev, "unrecognized switch version (ver=0x%04x)", - chip_ver); - return -ENODEV; } + } - priv->num_ports = RTL8365MB_MAX_NUM_PORTS; - - mb->priv = priv; - mb->chip_id = chip_id; - mb->chip_ver = chip_ver; - mb->jam_table = rtl8365mb_init_jam_8365mb_vc; - mb->jam_size = ARRAY_SIZE(rtl8365mb_init_jam_8365mb_vc); - - mb->cpu.trap_port = RTL8365MB_MAX_NUM_PORTS; - mb->cpu.insert = RTL8365MB_CPU_INSERT_TO_ALL; - mb->cpu.position = RTL8365MB_CPU_POS_AFTER_SA; - mb->cpu.rx_length = RTL8365MB_CPU_RXLEN_64BYTES; - mb->cpu.format = RTL8365MB_CPU_FORMAT_8BYTES; - - break; - default: + if (!mb->chip_info) { dev_err(priv->dev, - "found an unknown Realtek switch (id=0x%04x, ver=0x%04x)\n", - chip_id, chip_ver); + "unrecognized switch (id=0x%04x, ver=0x%04x)", chip_id, + chip_ver); return -ENODEV; } + dev_info(priv->dev, "found an %s switch\n", mb->chip_info->name); + + priv->num_ports = RTL8365MB_MAX_NUM_PORTS; + mb->priv = priv; + mb->cpu.trap_port = RTL8365MB_MAX_NUM_PORTS; + mb->cpu.insert = RTL8365MB_CPU_INSERT_TO_ALL; + mb->cpu.position = RTL8365MB_CPU_POS_AFTER_SA; + mb->cpu.rx_length = RTL8365MB_CPU_RXLEN_64BYTES; + mb->cpu.format = RTL8365MB_CPU_FORMAT_8BYTES; + return 0; } -- 2.36.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH net-next v2 5/5] net: dsa: realtek: rtl8365mb: handle PHY interface modes correctly 2022-06-10 15:38 ` [PATCH net-next v2 5/5] net: dsa: realtek: rtl8365mb: handle PHY interface modes correctly Alvin Šipraga @ 2022-06-10 16:36 ` Russell King (Oracle) 2022-06-10 17:45 ` Alvin Šipraga 2022-06-12 3:20 ` Luiz Angelo Daros de Luca 1 sibling, 1 reply; 14+ messages in thread From: Russell King (Oracle) @ 2022-06-10 16:36 UTC (permalink / raw) To: Alvin Šipraga Cc: hauke, Linus Walleij, Alvin Šipraga, Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, linux-kernel On Fri, Jun 10, 2022 at 05:38:29PM +0200, Alvin Šipraga wrote: > Finally, rtl8365mb_phylink_get_caps() is fixed up to return supported > capabilities based on the external interface properties described above. > This allows for ports with an external interface to be treated as DSA > user ports, and for ports with an internal PHY to be treated as DSA CPU > ports. I've needed to read that a few times... and I'm still not sure. You seem to be saying that: - ports with an internal PHY (which presumably provide baseT connections?) are used as DSA CPU ports. - ports with an external interface supporting a range of RGMII, SGMII and HSGMII interface modes are DSA user ports. With Marvell switches, it's the other way around - the ports with an internal PHY are normally DSA user ports. Other ports can be a user, inter-switch or CPU port. So, I'm slightly confused by your description. In any ase, looking at the get_caps() (and only that): > @@ -953,7 +1026,13 @@ static int rtl8365mb_ext_config_forcemode(struct realtek_priv *priv, int port, > static void rtl8365mb_phylink_get_caps(struct dsa_switch *ds, int port, > struct phylink_config *config) > { > - if (dsa_is_user_port(ds, port)) { > + const struct rtl8365mb_extint *extint = > + rtl8365mb_get_port_extint(ds->priv, port); > + > + config->mac_capabilities = MAC_SYM_PAUSE | MAC_ASYM_PAUSE | > + MAC_10 | MAC_100 | MAC_1000FD; MAC capabilities are constant across all interfaces - okay. > + > + if (!extint) { > __set_bit(PHY_INTERFACE_MODE_INTERNAL, > config->supported_interfaces); > > @@ -962,12 +1041,16 @@ static void rtl8365mb_phylink_get_caps(struct dsa_switch *ds, int port, > */ > __set_bit(PHY_INTERFACE_MODE_GMII, > config->supported_interfaces); > - } else if (dsa_is_cpu_port(ds, port)) { > - phy_interface_set_rgmii(config->supported_interfaces); > + return; Internal ports need to support phylib, so both internal and gmii interface modes - okay. > } > > - config->mac_capabilities = MAC_SYM_PAUSE | MAC_ASYM_PAUSE | > - MAC_10 | MAC_100 | MAC_1000FD; > + /* Populate according to the modes supported by _this driver_, > + * not necessarily the modes supported by the hardware, some of > + * which remain unimplemented. > + */ > + > + if (extint->supported_interfaces & RTL8365MB_PHY_INTERFACE_MODE_RGMII) > + phy_interface_set_rgmii(config->supported_interfaces); External ports that support RGMII get all RGMII modes - also okay. So, for the get_cops() function, I'm fine with this new code, and for this alone: Acked-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> I haven't looked at the remainder of the changes. Thanks. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net-next v2 5/5] net: dsa: realtek: rtl8365mb: handle PHY interface modes correctly 2022-06-10 16:36 ` Russell King (Oracle) @ 2022-06-10 17:45 ` Alvin Šipraga 0 siblings, 0 replies; 14+ messages in thread From: Alvin Šipraga @ 2022-06-10 17:45 UTC (permalink / raw) To: Russell King (Oracle) Cc: Alvin Šipraga, hauke, Linus Walleij, Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, linux-kernel On Fri, Jun 10, 2022 at 05:36:39PM +0100, Russell King (Oracle) wrote: > On Fri, Jun 10, 2022 at 05:38:29PM +0200, Alvin ┼áipraga wrote: > > Finally, rtl8365mb_phylink_get_caps() is fixed up to return supported > > capabilities based on the external interface properties described above. > > This allows for ports with an external interface to be treated as DSA > > user ports, and for ports with an internal PHY to be treated as DSA CPU > > ports. > > I've needed to read that a few times... and I'm still not sure. You seem > to be saying that: > - ports with an internal PHY (which presumably provide baseT connections?) > are used as DSA CPU ports. With this change they now _can_ be used as CPU ports. In practice I think most application would use ports with internal PHY as DSA user ports. But the hardware can treat any port as a CPU port, and as Vladimir pointed out in the thread I added in the Link:, the previous test (dsa_is_user_port()) was spurious. > - ports with an external interface supporting a range of RGMII, SGMII and > HSGMII interface modes are DSA user ports. Same as above: previously they could not be configured as DSA user ports. Now they can be. The utility is up for debate, but at least the code is correct. > > With Marvell switches, it's the other way around - the ports with an > internal PHY are normally DSA user ports. Other ports can be a user, > inter-switch or CPU port. > > So, I'm slightly confused by your description. After reading the above, please let me know if you are still confused. Kind regards, Alvin ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net-next v2 5/5] net: dsa: realtek: rtl8365mb: handle PHY interface modes correctly 2022-06-10 15:38 ` [PATCH net-next v2 5/5] net: dsa: realtek: rtl8365mb: handle PHY interface modes correctly Alvin Šipraga 2022-06-10 16:36 ` Russell King (Oracle) @ 2022-06-12 3:20 ` Luiz Angelo Daros de Luca 1 sibling, 0 replies; 14+ messages in thread From: Luiz Angelo Daros de Luca @ 2022-06-12 3:20 UTC (permalink / raw) To: Alvin Šipraga Cc: Hauke Mehrtens, Linus Walleij, Alvin Šipraga, Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Russell King, open list:NETWORKING DRIVERS, open list " > Realtek switches in the rtl8365mb family always have at least one port > with a so-called external interface, supporting PHY interface modes such > as RGMII or SGMII. The purpose of this patch is to improve the driver's > handling of these ports. > > A new struct rtl8365mb_chip_info is introduced together with a static > array of such structs. An instance of this struct is added for each > supported switch, distinguished by its chip ID and version. Embedded in > each chip_info struct is an array of struct rtl8365mb_extint, describing > the external interfaces available. This is more specific than the old > rtl8365mb_extint_port_map, which was only valid for switches with up to > 6 ports. > > The struct rtl8365mb_extint also contains a bitmask of supported PHY > interface modes, which allows the driver to distinguish which ports > support RGMII. This corrects a previous mistake in the driver whereby it > was assumed that any port with an external interface supports RGMII. > This is not actually the case: for example, the RTL8367S has two > external interfaces, only the second of which supports RGMII. The first > supports only SGMII and HSGMII. This new design will make it easier to > add support for other interface modes. > > Finally, rtl8365mb_phylink_get_caps() is fixed up to return supported > capabilities based on the external interface properties described above. > This allows for ports with an external interface to be treated as DSA > user ports, and for ports with an internal PHY to be treated as DSA CPU > ports. > > Link: https://lore.kernel.org/netdev/20220510192301.5djdt3ghoavxulhl@bang-olufsen.dk/ > Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk> > --- > drivers/net/dsa/realtek/rtl8365mb.c | 281 +++++++++++++++++----------- > 1 file changed, 174 insertions(+), 107 deletions(-) > > diff --git a/drivers/net/dsa/realtek/rtl8365mb.c b/drivers/net/dsa/realtek/rtl8365mb.c > index 676b88798976..da31d8b839ac 100644 > --- a/drivers/net/dsa/realtek/rtl8365mb.c > +++ b/drivers/net/dsa/realtek/rtl8365mb.c > @@ -101,26 +101,14 @@ > > #include "realtek.h" > > -/* Chip-specific data and limits */ > -#define RTL8365MB_CHIP_ID_8365MB_VC 0x6367 > -#define RTL8365MB_CHIP_VER_8365MB_VC 0x0040 > - > -#define RTL8365MB_CHIP_ID_8367S 0x6367 > -#define RTL8365MB_CHIP_VER_8367S 0x00A0 > - > -#define RTL8365MB_CHIP_ID_8367RB_VB 0x6367 > -#define RTL8365MB_CHIP_VER_8367RB_VB 0x0020 > - > /* 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 11 > +#define RTL8365MB_MAX_NUM_EXTINTS 3 > #define RTL8365MB_LEARN_LIMIT_MAX 2112 > > -/* valid for all 6-port or less variants */ > -static const int rtl8365mb_extint_port_map[] = { -1, -1, -1, -1, -1, -1, 1, 2, -1, -1}; > - > /* Chip identification registers */ > #define RTL8365MB_CHIP_ID_REG 0x1300 > > @@ -200,7 +188,7 @@ static const int rtl8365mb_extint_port_map[] = { -1, -1, -1, -1, -1, -1, 1, 2, > /* The PHY OCP addresses of PHY registers 0~31 start here */ > #define RTL8365MB_PHY_OCP_ADDR_PHYREG_BASE 0xA400 > > -/* EXT interface port mode values - used in DIGITAL_INTERFACE_SELECT */ > +/* External interface port mode values - used in DIGITAL_INTERFACE_SELECT */ > #define RTL8365MB_EXT_PORT_MODE_DISABLE 0 > #define RTL8365MB_EXT_PORT_MODE_RGMII 1 > #define RTL8365MB_EXT_PORT_MODE_MII_MAC 2 > @@ -216,19 +204,7 @@ static const int rtl8365mb_extint_port_map[] = { -1, -1, -1, -1, -1, -1, 1, 2, > #define RTL8365MB_EXT_PORT_MODE_1000X 12 > #define RTL8365MB_EXT_PORT_MODE_100FX 13 > > -/* Realtek docs and driver uses logic number as EXT_PORT0=16, EXT_PORT1=17, > - * EXT_PORT2=18, to interact with switch ports. That logic number is internally > - * converted to either a physical port number (0..9) or an external interface id (0..2), > - * depending on which function was called. The external interface id is calculated as > - * (ext_id=logic_port-15), while the logical to physical map depends on the chip id/version. > - * > - * EXT_PORT0 mentioned in datasheets and rtl8367c driver is used in this driver > - * as extid==1, EXT_PORT2, mentioned in Realtek rtl8367c driver for 10-port switches, > - * would have an ext_id of 3 (out of range for most extint macros) and ext_id 0 does > - * not seem to be used as well for this family. > - */ > - > -/* EXT interface mode configuration registers 0~1 */ > +/* External interface mode configuration registers 0~1 */ > #define RTL8365MB_DIGITAL_INTERFACE_SELECT_REG0 0x1305 /* EXT1 */ > #define RTL8365MB_DIGITAL_INTERFACE_SELECT_REG1 0x13C3 /* EXT2 */ > #define RTL8365MB_DIGITAL_INTERFACE_SELECT_REG(_extint) \ > @@ -240,7 +216,7 @@ static const int rtl8365mb_extint_port_map[] = { -1, -1, -1, -1, -1, -1, 1, 2, > #define RTL8365MB_DIGITAL_INTERFACE_SELECT_MODE_OFFSET(_extint) \ > (((_extint) % 2) * 4) > > -/* EXT interface RGMII TX/RX delay configuration registers 0~2 */ > +/* External interface RGMII TX/RX delay configuration registers 0~2 */ > #define RTL8365MB_EXT_RGMXF_REG0 0x1306 /* EXT0 */ > #define RTL8365MB_EXT_RGMXF_REG1 0x1307 /* EXT1 */ > #define RTL8365MB_EXT_RGMXF_REG2 0x13C5 /* EXT2 */ > @@ -257,7 +233,7 @@ static const int rtl8365mb_extint_port_map[] = { -1, -1, -1, -1, -1, -1, 1, 2, > #define RTL8365MB_PORT_SPEED_100M 1 > #define RTL8365MB_PORT_SPEED_1000M 2 > > -/* EXT interface force configuration registers 0~2 */ > +/* External interface force configuration registers 0~2 */ > #define RTL8365MB_DIGITAL_INTERFACE_FORCE_REG0 0x1310 /* EXT0 */ > #define RTL8365MB_DIGITAL_INTERFACE_FORCE_REG1 0x1311 /* EXT1 */ > #define RTL8365MB_DIGITAL_INTERFACE_FORCE_REG2 0x13C4 /* EXT2 */ > @@ -489,6 +465,95 @@ static const struct rtl8365mb_jam_tbl_entry rtl8365mb_init_jam_common[] = { > { 0x1D32, 0x0002 }, > }; > > +enum rtl8365mb_phy_interface_mode { > + RTL8365MB_PHY_INTERFACE_MODE_INVAL = 0, > + RTL8365MB_PHY_INTERFACE_MODE_INTERNAL = BIT(0), > + RTL8365MB_PHY_INTERFACE_MODE_MII = BIT(1), > + RTL8365MB_PHY_INTERFACE_MODE_TMII = BIT(2), > + RTL8365MB_PHY_INTERFACE_MODE_RMII = BIT(3), > + RTL8365MB_PHY_INTERFACE_MODE_RGMII = BIT(4), > + RTL8365MB_PHY_INTERFACE_MODE_SGMII = BIT(5), > + RTL8365MB_PHY_INTERFACE_MODE_HSGMII = BIT(6), > +}; > + > +/** > + * struct rtl8365mb_extint - external interface info > + * @port: the port with an external interface > + * @id: the external interface ID, which is either 0, 1, or 2 > + * @supported_interfaces: a bitmask of supported PHY interface modes > + * > + * Represents a mapping: port -> { id, supported_interfaces }. To be embedded > + * in &struct rtl8365mb_chip_info for every port with an external interface. > + */ > +struct rtl8365mb_extint { > + int port; > + int id; > + unsigned int supported_interfaces; It is more a doubt than an issue but should we use an int for a 0-11 value or a char? And maybe a short for supported_interfaces to keep it 32-bit aligned. > +}; > + > +/** > + * struct rtl8365mb_chip_info - static chip-specific info > + * @name: human-readable chip name > + * @chip_id: chip identifier > + * @chip_ver: chip silicon revision > + * @extints: available external interfaces > + * @jam_table: chip-specific initialization jam table > + * @jam_size: size of the chip's jam table > + * > + * These data are specific to a given chip in the family of switches supported > + * by this driver. When adding support for another chip in the family, a new > + * chip info should be added to the rtl8365mb_chip_infos array. > + */ > +struct rtl8365mb_chip_info { > + const char *name; > + u32 chip_id; > + u32 chip_ver; > + const struct rtl8365mb_extint extints[RTL8365MB_MAX_NUM_EXTINTS]; > + const struct rtl8365mb_jam_tbl_entry *jam_table; > + size_t jam_size; > +}; > + > +/* Chip info for each supported switch in the family */ > +#define PHY_INTF(_mode) (RTL8365MB_PHY_INTERFACE_MODE_ ## _mode) > +static const struct rtl8365mb_chip_info rtl8365mb_chip_infos[] = { > + { > + .name = "RTL8365MB-VC", > + .chip_id = 0x6367, > + .chip_ver = 0x0040, > + .extints = { > + { 6, 1, PHY_INTF(MII) | PHY_INTF(TMII) | > + PHY_INTF(RMII) | PHY_INTF(RGMII) }, > + }, > + .jam_table = rtl8365mb_init_jam_8365mb_vc, > + .jam_size = ARRAY_SIZE(rtl8365mb_init_jam_8365mb_vc), > + }, > + { > + .name = "RTL8367S", > + .chip_id = 0x6367, > + .chip_ver = 0x00A0, > + .extints = { > + { 6, 1, PHY_INTF(SGMII) | PHY_INTF(HSGMII) }, > + { 7, 2, PHY_INTF(MII) | PHY_INTF(TMII) | > + PHY_INTF(RMII) | PHY_INTF(RGMII) }, > + }, > + .jam_table = rtl8365mb_init_jam_8365mb_vc, > + .jam_size = ARRAY_SIZE(rtl8365mb_init_jam_8365mb_vc), > + }, > + { > + .name = "RTL8367RB-VB", > + .chip_id = 0x6367, > + .chip_ver = 0x0020, > + .extints = { > + { 6, 1, PHY_INTF(MII) | PHY_INTF(TMII) | > + PHY_INTF(RMII) | PHY_INTF(RGMII) }, > + { 7, 2, PHY_INTF(MII) | PHY_INTF(TMII) | > + PHY_INTF(RMII) | PHY_INTF(RGMII) }, > + }, > + .jam_table = rtl8365mb_init_jam_8365mb_vc, > + .jam_size = ARRAY_SIZE(rtl8365mb_init_jam_8365mb_vc), > + }, > +}; > + > enum rtl8365mb_stp_state { > RTL8365MB_STP_STATE_DISABLED = 0, > RTL8365MB_STP_STATE_BLOCKING = 1, > @@ -558,29 +623,23 @@ struct rtl8365mb_port { > }; > > /** > - * struct rtl8365mb - private chip-specific driver data > + * struct rtl8365mb - driver private data > * @priv: pointer to parent realtek_priv data > * @irq: registered IRQ or zero > - * @chip_id: chip identifier > - * @chip_ver: chip silicon revision > + * @chip_info: chip-specific info about the attached switch > * @cpu: CPU tagging and CPU port configuration for this chip > * @mib_lock: prevent concurrent reads of MIB counters > * @ports: per-port data > - * @jam_table: chip-specific initialization jam table > - * @jam_size: size of the chip's jam table > * > * Private data for this driver. > */ > struct rtl8365mb { > struct realtek_priv *priv; > int irq; > - u32 chip_id; > - u32 chip_ver; > + const struct rtl8365mb_chip_info *chip_info; > struct rtl8365mb_cpu cpu; > struct mutex mib_lock; > struct rtl8365mb_port ports[RTL8365MB_MAX_NUM_PORTS]; > - const struct rtl8365mb_jam_tbl_entry *jam_table; > - size_t jam_size; > }; > > static int rtl8365mb_phy_poll_busy(struct realtek_priv *priv) > @@ -775,6 +834,26 @@ static int rtl8365mb_dsa_phy_write(struct dsa_switch *ds, int phy, int regnum, > return rtl8365mb_phy_write(ds->priv, phy, regnum, val); > } > > +static const struct rtl8365mb_extint * > +rtl8365mb_get_port_extint(struct realtek_priv *priv, int port) > +{ > + struct rtl8365mb *mb = priv->chip_data; > + int i; > + > + for (i = 0; i < RTL8365MB_MAX_NUM_EXTINTS; i++) { All extint lookups use port as the search key and port is only used as a search key. If this was an array of ports, you could have avoided the loop. I believe the kernel now requires C99 and we could even initialize as a sparse array (if it is acceptable by kernel code sytle). Something like this untested code: struct rtl8365mb_extint { int id; unsigned int supported_interfaces; }; struct rtl8365mb_chip_info { ... const struct rtl8365mb_extint extints[RTL8365MB_MAX_NUM_PORTS]; ... }; .extints = { [6] = {1, PHY_INTF(MII) | PHY_INTF(TMII) | PHY_INTF(RMII) | PHY_INTF(RGMII) }, [7] = {2, PHY_INTF(MII) | PHY_INTF(TMII) | PHY_INTF(RMII) | PHY_INTF(RGMII) }, } We still need to check boundaries if the caller does not guarantee port id is within boundaries. Maybe ds->num_ports is enough to make sure the port is in range. > + const struct rtl8365mb_extint *extint = > + &mb->chip_info->extints[i]; > + > + if (!extint->supported_interfaces) > + continue; > + > + if (extint->port == port) > + return extint; > + } > + > + return NULL; > +} > + > static enum dsa_tag_protocol > rtl8365mb_get_tag_protocol(struct dsa_switch *ds, int port, > enum dsa_tag_protocol mp) > @@ -795,20 +874,17 @@ rtl8365mb_get_tag_protocol(struct dsa_switch *ds, int port, > static int rtl8365mb_ext_config_rgmii(struct realtek_priv *priv, int port, > phy_interface_t interface) > { > + const struct rtl8365mb_extint *extint = > + rtl8365mb_get_port_extint(priv, port); > struct device_node *dn; > struct dsa_port *dp; > int tx_delay = 0; > int rx_delay = 0; > - int ext_int; > u32 val; > int ret; > > - ext_int = rtl8365mb_extint_port_map[port]; > - > - if (ext_int <= 0) { > - dev_err(priv->dev, "Port %d is not an external interface port\n", port); > - return -EINVAL; > - } > + if (!extint) > + return -ENODEV; > > dp = dsa_to_port(priv->ds, port); > dn = dp->dn; > @@ -842,7 +918,7 @@ static int rtl8365mb_ext_config_rgmii(struct realtek_priv *priv, int port, > tx_delay = val / 2; > else > dev_warn(priv->dev, > - "EXT interface TX delay must be 0 or 2 ns\n"); > + "RGMII TX delay must be 0 or 2 ns\n"); > } > > if (!of_property_read_u32(dn, "rx-internal-delay-ps", &val)) { > @@ -852,11 +928,11 @@ static int rtl8365mb_ext_config_rgmii(struct realtek_priv *priv, int port, > rx_delay = val; > else > dev_warn(priv->dev, > - "EXT interface RX delay must be 0 to 2.1 ns\n"); > + "RGMII RX delay must be 0 to 2.1 ns\n"); > } > > ret = regmap_update_bits( > - priv->map, RTL8365MB_EXT_RGMXF_REG(ext_int), > + priv->map, RTL8365MB_EXT_RGMXF_REG(extint->id), > RTL8365MB_EXT_RGMXF_TXDELAY_MASK | > RTL8365MB_EXT_RGMXF_RXDELAY_MASK, > FIELD_PREP(RTL8365MB_EXT_RGMXF_TXDELAY_MASK, tx_delay) | > @@ -865,11 +941,11 @@ static int rtl8365mb_ext_config_rgmii(struct realtek_priv *priv, int port, > return ret; > > ret = regmap_update_bits( > - priv->map, RTL8365MB_DIGITAL_INTERFACE_SELECT_REG(ext_int), > - RTL8365MB_DIGITAL_INTERFACE_SELECT_MODE_MASK(ext_int), > + priv->map, RTL8365MB_DIGITAL_INTERFACE_SELECT_REG(extint->id), > + RTL8365MB_DIGITAL_INTERFACE_SELECT_MODE_MASK(extint->id), > RTL8365MB_EXT_PORT_MODE_RGMII > << RTL8365MB_DIGITAL_INTERFACE_SELECT_MODE_OFFSET( > - ext_int)); > + extint->id)); > if (ret) > return ret; > > @@ -880,21 +956,18 @@ static int rtl8365mb_ext_config_forcemode(struct realtek_priv *priv, int port, > bool link, int speed, int duplex, > bool tx_pause, bool rx_pause) > { > + const struct rtl8365mb_extint *extint = > + rtl8365mb_get_port_extint(priv, port); > u32 r_tx_pause; > u32 r_rx_pause; > u32 r_duplex; > u32 r_speed; > u32 r_link; > - int ext_int; > int val; > int ret; > > - ext_int = rtl8365mb_extint_port_map[port]; > - > - if (ext_int <= 0) { > - dev_err(priv->dev, "Port %d is not an external interface port\n", port); > - return -EINVAL; > - } > + if (!extint) > + return -ENODEV; > > if (link) { > /* Force the link up with the desired configuration */ > @@ -942,7 +1015,7 @@ static int rtl8365mb_ext_config_forcemode(struct realtek_priv *priv, int port, > r_duplex) | > FIELD_PREP(RTL8365MB_DIGITAL_INTERFACE_FORCE_SPEED_MASK, r_speed); > ret = regmap_write(priv->map, > - RTL8365MB_DIGITAL_INTERFACE_FORCE_REG(ext_int), > + RTL8365MB_DIGITAL_INTERFACE_FORCE_REG(extint->id), > val); > if (ret) > return ret; > @@ -953,7 +1026,13 @@ static int rtl8365mb_ext_config_forcemode(struct realtek_priv *priv, int port, > static void rtl8365mb_phylink_get_caps(struct dsa_switch *ds, int port, > struct phylink_config *config) > { > - if (dsa_is_user_port(ds, port)) { > + const struct rtl8365mb_extint *extint = > + rtl8365mb_get_port_extint(ds->priv, port); > + > + config->mac_capabilities = MAC_SYM_PAUSE | MAC_ASYM_PAUSE | > + MAC_10 | MAC_100 | MAC_1000FD; > + > + if (!extint) { > __set_bit(PHY_INTERFACE_MODE_INTERNAL, > config->supported_interfaces); > > @@ -962,12 +1041,16 @@ static void rtl8365mb_phylink_get_caps(struct dsa_switch *ds, int port, > */ > __set_bit(PHY_INTERFACE_MODE_GMII, > config->supported_interfaces); > - } else if (dsa_is_cpu_port(ds, port)) { > - phy_interface_set_rgmii(config->supported_interfaces); > + return; > } > > - config->mac_capabilities = MAC_SYM_PAUSE | MAC_ASYM_PAUSE | > - MAC_10 | MAC_100 | MAC_1000FD; > + /* Populate according to the modes supported by _this driver_, > + * not necessarily the modes supported by the hardware, some of > + * which remain unimplemented. > + */ > + > + if (extint->supported_interfaces & RTL8365MB_PHY_INTERFACE_MODE_RGMII) > + phy_interface_set_rgmii(config->supported_interfaces); > } > > static void rtl8365mb_phylink_mac_config(struct dsa_switch *ds, int port, > @@ -1782,14 +1865,17 @@ static int rtl8365mb_change_tag_protocol(struct dsa_switch *ds, > static int rtl8365mb_switch_init(struct realtek_priv *priv) > { > struct rtl8365mb *mb = priv->chip_data; > + const struct rtl8365mb_chip_info *ci; > int ret; > int i; > > + ci = mb->chip_info; > + > /* Do any chip-specific init jam before getting to the common stuff */ > - if (mb->jam_table) { > - for (i = 0; i < mb->jam_size; i++) { > - ret = regmap_write(priv->map, mb->jam_table[i].reg, > - mb->jam_table[i].val); > + if (ci->jam_table) { > + for (i = 0; i < ci->jam_size; i++) { > + ret = regmap_write(priv->map, ci->jam_table[i].reg, > + ci->jam_table[i].val); > if (ret) > return ret; > } > @@ -1962,6 +2048,7 @@ static int rtl8365mb_detect(struct realtek_priv *priv) > u32 chip_id; > u32 chip_ver; > int ret; > + int i; > > ret = rtl8365mb_get_chip_id_and_ver(priv->map, &chip_id, &chip_ver); > if (ret) { > @@ -1970,52 +2057,32 @@ static int rtl8365mb_detect(struct realtek_priv *priv) > return ret; > } > > - switch (chip_id) { > - case RTL8365MB_CHIP_ID_8365MB_VC: > - switch (chip_ver) { > - case RTL8365MB_CHIP_VER_8365MB_VC: > - dev_info(priv->dev, > - "found an RTL8365MB-VC switch (ver=0x%04x)\n", > - chip_ver); > - break; > - case RTL8365MB_CHIP_VER_8367RB_VB: > - dev_info(priv->dev, > - "found an RTL8367RB-VB switch (ver=0x%04x)\n", > - chip_ver); > - break; > - case RTL8365MB_CHIP_VER_8367S: > - dev_info(priv->dev, > - "found an RTL8367S switch (ver=0x%04x)\n", > - chip_ver); > + for (i = 0; i < ARRAY_SIZE(rtl8365mb_chip_infos); i++) { > + const struct rtl8365mb_chip_info *ci = &rtl8365mb_chip_infos[i]; > + > + if (ci->chip_id == chip_id && ci->chip_ver == chip_ver) { > + mb->chip_info = ci; > break; > - default: > - dev_err(priv->dev, "unrecognized switch version (ver=0x%04x)", > - chip_ver); > - return -ENODEV; > } > + } > > - priv->num_ports = RTL8365MB_MAX_NUM_PORTS; > - > - mb->priv = priv; > - mb->chip_id = chip_id; > - mb->chip_ver = chip_ver; > - mb->jam_table = rtl8365mb_init_jam_8365mb_vc; > - mb->jam_size = ARRAY_SIZE(rtl8365mb_init_jam_8365mb_vc); > - > - mb->cpu.trap_port = RTL8365MB_MAX_NUM_PORTS; > - mb->cpu.insert = RTL8365MB_CPU_INSERT_TO_ALL; > - mb->cpu.position = RTL8365MB_CPU_POS_AFTER_SA; > - mb->cpu.rx_length = RTL8365MB_CPU_RXLEN_64BYTES; > - mb->cpu.format = RTL8365MB_CPU_FORMAT_8BYTES; > - > - break; > - default: > + if (!mb->chip_info) { > dev_err(priv->dev, > - "found an unknown Realtek switch (id=0x%04x, ver=0x%04x)\n", > - chip_id, chip_ver); > + "unrecognized switch (id=0x%04x, ver=0x%04x)", chip_id, > + chip_ver); > return -ENODEV; > } > > + dev_info(priv->dev, "found an %s switch\n", mb->chip_info->name); > + > + priv->num_ports = RTL8365MB_MAX_NUM_PORTS; > + mb->priv = priv; > + mb->cpu.trap_port = RTL8365MB_MAX_NUM_PORTS; > + mb->cpu.insert = RTL8365MB_CPU_INSERT_TO_ALL; > + mb->cpu.position = RTL8365MB_CPU_POS_AFTER_SA; > + mb->cpu.rx_length = RTL8365MB_CPU_RXLEN_64BYTES; > + mb->cpu.format = RTL8365MB_CPU_FORMAT_8BYTES; > + > return 0; > } > > -- > 2.36.1 > In spite of my style comments, it looks good. Acked-by: Luiz Angelo Daros de Luca <luizluca@gmail.com> ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net-next v2 0/5] net: dsa: realtek: rtl8365mb: improve handling of PHY modes 2022-06-10 15:38 [PATCH net-next v2 0/5] net: dsa: realtek: rtl8365mb: improve handling of PHY modes Alvin Šipraga ` (4 preceding siblings ...) 2022-06-10 15:38 ` [PATCH net-next v2 5/5] net: dsa: realtek: rtl8365mb: handle PHY interface modes correctly Alvin Šipraga @ 2022-06-15 16:55 ` Alvin Šipraga 2022-06-15 22:21 ` Jakub Kicinski 5 siblings, 1 reply; 14+ messages in thread From: Alvin Šipraga @ 2022-06-15 16:55 UTC (permalink / raw) To: Alvin Šipraga, kuba, davem Cc: hauke, Linus Walleij, Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Russell King, netdev, linux-kernel David, Jakub, this series is marked Changes Requested on patchwork, but I have addressed all the comments. Do you want me to resend? Kind regards, Alvin On Fri, Jun 10, 2022 at 05:38:24PM +0200, Alvin Šipraga wrote: > From: Alvin Šipraga <alsi@bang-olufsen.dk> > > This series introduces some minor cleanup of the driver and improves the > handling of PHY interface modes to break the assumption that CPU ports > are always over an external interface, and the assumption that user > ports are always using an internal PHY. > > Changes v1 -> v2: > > - patches 1-4: no code change > > - add Luiz' reviewed-by to some of the patches > > - patch 5: put the chip_infos into a static array and get rid of the > switch in the detect function; also remove the macros for various > chip ID/versions and embed them directly into the array > > - patch 5: use array of size 3 rather than flexible array for extints > in the chip_info struct; gcc complained about initialization of > flexible array members in a nested context, and anyway, we know that > the max number of external interfaces is 3 > > Alvin Šipraga (5): > net: dsa: realtek: rtl8365mb: rename macro RTL8367RB -> RTL8367RB_VB > net: dsa: realtek: rtl8365mb: remove port_mask private data member > net: dsa: realtek: rtl8365mb: correct the max number of ports > net: dsa: realtek: rtl8365mb: remove learn_limit_max private data > member > net: dsa: realtek: rtl8365mb: handle PHY interface modes correctly > > drivers/net/dsa/realtek/rtl8365mb.c | 299 ++++++++++++++++------------ > 1 file changed, 177 insertions(+), 122 deletions(-) > > -- > 2.36.1 > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net-next v2 0/5] net: dsa: realtek: rtl8365mb: improve handling of PHY modes 2022-06-15 16:55 ` [PATCH net-next v2 0/5] net: dsa: realtek: rtl8365mb: improve handling of PHY modes Alvin Šipraga @ 2022-06-15 22:21 ` Jakub Kicinski 2022-06-15 22:32 ` Alvin Šipraga 0 siblings, 1 reply; 14+ messages in thread From: Jakub Kicinski @ 2022-06-15 22:21 UTC (permalink / raw) To: Alvin Šipraga Cc: Alvin Šipraga, davem, hauke, Linus Walleij, Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean, Eric Dumazet, Paolo Abeni, Russell King, netdev, linux-kernel On Wed, 15 Jun 2022 16:55:29 +0000 Alvin Šipraga wrote: > David, Jakub, this series is marked Changes Requested on patchwork, but I have > addressed all the comments. Do you want me to resend? Oh, that was me. I was hoping you'd respin to at least clarify the commit message on patch 5, based on Russell's questions. Perhaps that's not as important these days given we add Links to the original discussion but should be useful to person reading just the git history. Sorry for not making that clear, folks sending comments and still acking the patch in general gives me low signal. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net-next v2 0/5] net: dsa: realtek: rtl8365mb: improve handling of PHY modes 2022-06-15 22:21 ` Jakub Kicinski @ 2022-06-15 22:32 ` Alvin Šipraga 0 siblings, 0 replies; 14+ messages in thread From: Alvin Šipraga @ 2022-06-15 22:32 UTC (permalink / raw) To: Jakub Kicinski Cc: Alvin Šipraga, davem, hauke, Linus Walleij, Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean, Eric Dumazet, Paolo Abeni, Russell King, netdev, linux-kernel On Wed, Jun 15, 2022 at 03:21:27PM -0700, Jakub Kicinski wrote: > On Wed, 15 Jun 2022 16:55:29 +0000 Alvin Šipraga wrote: > > David, Jakub, this series is marked Changes Requested on patchwork, but I have > > addressed all the comments. Do you want me to resend? > > Oh, that was me. I was hoping you'd respin to at least clarify > the commit message on patch 5, based on Russell's questions. > Perhaps that's not as important these days given we add Links > to the original discussion but should be useful to person reading > just the git history. Sorry for not making that clear, folks sending > comments and still acking the patch in general gives me low signal. Ah sorry, I had actually forgotten about that. Let me send a v3 then and I will fix it up. Thanks for clarifying! ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2022-06-15 22:32 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-06-10 15:38 [PATCH net-next v2 0/5] net: dsa: realtek: rtl8365mb: improve handling of PHY modes Alvin Šipraga 2022-06-10 15:38 ` [PATCH net-next v2 1/5] net: dsa: realtek: rtl8365mb: rename macro RTL8367RB -> RTL8367RB_VB Alvin Šipraga 2022-06-10 15:38 ` [PATCH net-next v2 2/5] net: dsa: realtek: rtl8365mb: remove port_mask private data member Alvin Šipraga 2022-06-10 15:38 ` [PATCH net-next v2 3/5] net: dsa: realtek: rtl8365mb: correct the max number of ports Alvin Šipraga 2022-06-10 15:38 ` [PATCH net-next v2 4/5] net: dsa: realtek: rtl8365mb: remove learn_limit_max private data member Alvin Šipraga 2022-06-12 1:40 ` Luiz Angelo Daros de Luca 2022-06-12 10:56 ` Alvin Šipraga 2022-06-10 15:38 ` [PATCH net-next v2 5/5] net: dsa: realtek: rtl8365mb: handle PHY interface modes correctly Alvin Šipraga 2022-06-10 16:36 ` Russell King (Oracle) 2022-06-10 17:45 ` Alvin Šipraga 2022-06-12 3:20 ` Luiz Angelo Daros de Luca 2022-06-15 16:55 ` [PATCH net-next v2 0/5] net: dsa: realtek: rtl8365mb: improve handling of PHY modes Alvin Šipraga 2022-06-15 22:21 ` Jakub Kicinski 2022-06-15 22:32 ` Alvin Šipraga
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.