Re: [PATCH net-next v2 05/10] net: dsa: realtek: rtl8365mb: describe the ports of each chip

["Alvin Šipraga" <alvin.sipraga@analog.com>]

On Thu, Nov 12, 2026 at 09:37:48AM +0100, Oleksij Rempel wrote:
> Record which ports each chip has, report one more than its highest
> port to DSA as the port count, and reject a devicetree port the chip
> does not have. Lower the family maximum to 8 to match.
> 
> Commit ca5ecd4246d4 ("net: dsa: realtek: rtl8365mb: correct the max
> number of ports") raised the port count of all chips to 11, but no
> supported chip has a port above 7.
> 
> Since commit 51a4a22301b0 ("net: dsa: realtek: rtl8365mb: use dsa
> helpers for port iteration"), setup also programs unused ports, so
> every probe writes per-port registers of ports 8..10. Their learn-limit
> writes go to 0x0A28..0x0A2A, above the learn-limit registers of ports
> 0..7. We have not seen a visible effect from these writes.

I'm not quite sure what the point of the patch is (sorry, maybe it's
clear later in the series, but I didn't get that far yet). Was the setup
of nonexistent ports clobbering some other registers?

Of course in practice none of the supported chips are >8 so there's
nothing technically wrong with this patch, but just wondering.

Also in the blamed commit there was:

-/* RTL8370MB and RTL8310SR, possibly suportable by this driver, have 10 ports */
-#define RTL8365MB_MAX_NUM_PORTS                10
+#define RTL8365MB_MAX_NUM_PORTS                11

So maybe it needs bumping in the future if support is added for those
chips. That's why I'm wondering if you saw something off?

> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
> Changes in v2:
> - New in v2. The QoS patches after it handle ports 0..7 only.
> ---
>  drivers/net/dsa/realtek/rtl8365mb_main.c | 28 ++++++++++++++++++++++++++--
>  1 file changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/dsa/realtek/rtl8365mb_main.c b/drivers/net/dsa/realtek/rtl8365mb_main.c
> index da06f952a053d1497647a2c6f15a861fc21860f9..1e6cdc61f6bd3b3f74e422dbbcfdcb67cc0532a7 100644
> --- a/drivers/net/dsa/realtek/rtl8365mb_main.c
> +++ b/drivers/net/dsa/realtek/rtl8365mb_main.c
> @@ -115,7 +115,7 @@
>  #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_PORTS		8
>  /* Valid for the whole family except RTL8370B, which has 4160 entries.
>   * RTL8370B is mentioned in vendor code but it might not even belong
>   * to the same RTL8367C family.
> @@ -707,6 +707,7 @@ struct rtl8365mb_extint {
>   * @name: human-readable chip name
>   * @chip_id: chip identifier
>   * @chip_ver: chip silicon revision
> + * @port_mask: ports the chip has
>   * @extints: available external interfaces
>   * @jam_table: chip-specific initialization jam table
>   * @jam_size: size of the chip's jam table
> @@ -719,6 +720,7 @@ struct rtl8365mb_chip_info {
>  	const char *name;
>  	u32 chip_id;
>  	u32 chip_ver;
> +	u32 port_mask;
>  	const struct rtl8365mb_extint extints[RTL8365MB_MAX_NUM_EXTINTS];
>  	const struct rtl8365mb_jam_tbl_entry *jam_table;
>  	size_t jam_size;
> @@ -726,11 +728,20 @@ struct rtl8365mb_chip_info {
>  
>  /* Chip info for each supported switch in the family */
>  #define PHY_INTF(_mode) (RTL8365MB_PHY_INTERFACE_MODE_ ## _mode)
> +#define PORT_MASK(_m) \
> +	((_m) + \
> +	 BUILD_BUG_ON_ZERO((_m) & ~GENMASK(RTL8365MB_MAX_NUM_PORTS - 1, 0)))
> +
> +/* Copper ports are 0..3 on the RTL8365MB-VC and 0..4 on the others. Extension
> + * interfaces are port 6 on the VC and ports 6 and 7 on the others, as the
> + * extints rows show.
> + */
>  static const struct rtl8365mb_chip_info rtl8365mb_chip_infos[] = {
>  	{
>  		.name = "RTL8365MB-VC",
>  		.chip_id = 0x6367,
>  		.chip_ver = 0x0040,
> +		.port_mask = PORT_MASK(GENMASK(3, 0) | BIT(6)),
>  		.extints = {
>  			{ 6, 1, PHY_INTF(MII) | PHY_INTF(TMII) |
>  				PHY_INTF(RMII) | PHY_INTF(RGMII) },
> @@ -742,6 +753,7 @@ static const struct rtl8365mb_chip_info rtl8365mb_chip_infos[] = {
>  		.name = "RTL8367S",
>  		.chip_id = 0x6367,
>  		.chip_ver = 0x00A0,
> +		.port_mask = PORT_MASK(GENMASK(4, 0) | BIT(6) | BIT(7)),
>  		.extints = {
>  			{ 6, 1, PHY_INTF(SGMII) | PHY_INTF(HSGMII) },
>  			{ 7, 2, PHY_INTF(MII) | PHY_INTF(TMII) |
> @@ -754,6 +766,7 @@ static const struct rtl8365mb_chip_info rtl8365mb_chip_infos[] = {
>  		.name = "RTL8367SB",
>  		.chip_id = 0x6367,
>  		.chip_ver = 0x0010,
> +		.port_mask = PORT_MASK(GENMASK(4, 0) | BIT(6) | BIT(7)),
>  		.extints = {
>  			{ 6, 1, PHY_INTF(MII) | PHY_INTF(TMII) |
>  				PHY_INTF(RMII) | PHY_INTF(RGMII) |
> @@ -768,6 +781,7 @@ static const struct rtl8365mb_chip_info rtl8365mb_chip_infos[] = {
>  		.name = "RTL8367RB-VB",
>  		.chip_id = 0x6367,
>  		.chip_ver = 0x0020,
> +		.port_mask = PORT_MASK(GENMASK(4, 0) | BIT(6) | BIT(7)),
>  		.extints = {
>  			{ 6, 1, PHY_INTF(MII) | PHY_INTF(TMII) |
>  				PHY_INTF(RMII) | PHY_INTF(RGMII) },
> @@ -3140,6 +3154,16 @@ static int rtl8365mb_setup(struct dsa_switch *ds)
>  		dev_info(priv->dev, "no interrupt support\n");
>  
>  	dsa_switch_for_each_port(dp, ds) {
> +		if (dsa_port_is_unused(dp))
> +			continue;

You can use dsa_switch_for_each_available_port() here.

> +
> +		if (!(mb->chip_info->port_mask & BIT(dp->index))) {
> +			dev_err(priv->dev, "port %d does not exist on %s\n",
> +				dp->index, mb->chip_info->name);
> +			ret = -EINVAL;
> +			goto out_teardown_irq;
> +		}
> +
>  		/* Cascading (DSA links) is not supported yet.
>  		 * Historically, the driver has always been broken
>  		 * without a dedicated CPU port because CPU tagging
> @@ -3349,7 +3373,7 @@ static int rtl8365mb_detect(struct realtek_priv *priv)
>  
>  	dev_info(priv->dev, "found an %s switch\n", mb->chip_info->name);
>  
> -	priv->num_ports = RTL8365MB_MAX_NUM_PORTS;
> +	priv->num_ports = fls(mb->chip_info->port_mask);
>  	mb->priv = priv;
>  	mb->cpu.trap_port = RTL8365MB_MAX_NUM_PORTS;
>  	mb->cpu.insert = RTL8365MB_CPU_INSERT_TO_ALL;
> 
> -- 
> 2.47.3
>