[net-next,v2] net: dsa: rtl8366rb: Roof MTU for switch

Commit Message

Linus Walleij Oct. 6, 2020, 7:34 p.m. UTC

The MTU setting for this DSA switch is global so we need
to keep track of the MTU set for each port, then as soon
as any MTU changes, roof the MTU to the biggest common
denominator and poke that into the switch MTU setting.

To achieve this we need a per-chip-variant state container
for the RTL8366RB to use for the RTL8366RB-specific
stuff. Other SMI switches does seem to have per-port
MTU setting capabilities.

Fixes: 5f4a8ef384db ("net: dsa: rtl8366rb: Support setting MTU")
Signed-off-by: Linus Walleij <[e-mail]>
---
ChangeLog v1->v2:
- Fix a reverse-christmas-tree variable order issue.
---
 drivers/net/dsa/realtek-smi-core.c |  3 ++-
 drivers/net/dsa/realtek-smi-core.h |  2 ++
 drivers/net/dsa/rtl8366rb.c        | 36 ++++++++++++++++++++++++++----
 3 files changed, 36 insertions(+), 5 deletions(-)

Comments

kernel test robot Oct. 6, 2020, 10:18 p.m. UTC | #1

Hi Linus,

I love your patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Linus-Walleij/net-dsa-rtl8366rb-Roof-MTU-for-switch/20201007-033703
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 9faebeb2d80065926dfbc09cb73b1bb7779a89cd
config: mips-randconfig-s031-20201005 (attached as .config)
compiler: mips64-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.2-201-g24bdaac6-dirty
        # https://github.com/0day-ci/linux/commit/5b302d7e6a5f04e402853d40f77a457a9ad48198
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Linus-Walleij/net-dsa-rtl8366rb-Roof-MTU-for-switch/20201007-033703
        git checkout 5b302d7e6a5f04e402853d40f77a457a9ad48198
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=mips 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[e-mail]>

All errors (new ones prefixed by >>):

   drivers/net/dsa/rtl8366rb.c: In function 'rtl8366rb_setup':
>> drivers/net/dsa/rtl8366rb.c:720:25: error: 'smi' undeclared (first use in this function)
     720 |  struct rtl8366rb *rb = smi->chip_data;
         |                         ^~~
   drivers/net/dsa/rtl8366rb.c:720:25: note: each undeclared identifier is reported only once for each function it appears in

vim +/smi +720 drivers/net/dsa/rtl8366rb.c

   717	
   718	static int rtl8366rb_setup(struct dsa_switch *ds)
   719	{
 > 720		struct rtl8366rb *rb = smi->chip_data;
   721		struct realtek_smi *smi = ds->priv;
   722		const u16 *jam_table;
   723		u32 chip_ver = 0;
   724		u32 chip_id = 0;
   725		int jam_size;
   726		u32 val;
   727		int ret;
   728		int i;
   729	
   730		ret = regmap_read(smi->map, RTL8366RB_CHIP_ID_REG, &chip_id);
   731		if (ret) {
   732			dev_err(smi->dev, "unable to read chip id\n");
   733			return ret;
   734		}
   735	
   736		switch (chip_id) {
   737		case RTL8366RB_CHIP_ID_8366:
   738			break;
   739		default:
   740			dev_err(smi->dev, "unknown chip id (%04x)\n", chip_id);
   741			return -ENODEV;
   742		}
   743	
   744		ret = regmap_read(smi->map, RTL8366RB_CHIP_VERSION_CTRL_REG,
   745				  &chip_ver);
   746		if (ret) {
   747			dev_err(smi->dev, "unable to read chip version\n");
   748			return ret;
   749		}
   750	
   751		dev_info(smi->dev, "RTL%04x ver %u chip found\n",
   752			 chip_id, chip_ver & RTL8366RB_CHIP_VERSION_MASK);
   753	
   754		/* Do the init dance using the right jam table */
   755		switch (chip_ver) {
   756		case 0:
   757			jam_table = rtl8366rb_init_jam_ver_0;
   758			jam_size = ARRAY_SIZE(rtl8366rb_init_jam_ver_0);
   759			break;
   760		case 1:
   761			jam_table = rtl8366rb_init_jam_ver_1;
   762			jam_size = ARRAY_SIZE(rtl8366rb_init_jam_ver_1);
   763			break;
   764		case 2:
   765			jam_table = rtl8366rb_init_jam_ver_2;
   766			jam_size = ARRAY_SIZE(rtl8366rb_init_jam_ver_2);
   767			break;
   768		default:
   769			jam_table = rtl8366rb_init_jam_ver_3;
   770			jam_size = ARRAY_SIZE(rtl8366rb_init_jam_ver_3);
   771			break;
   772		}
   773	
   774		/* Special jam tables for special routers
   775		 * TODO: are these necessary? Maintainers, please test
   776		 * without them, using just the off-the-shelf tables.
   777		 */
   778		if (of_machine_is_compatible("belkin,f5d8235-v1")) {
   779			jam_table = rtl8366rb_init_jam_f5d8235;
   780			jam_size = ARRAY_SIZE(rtl8366rb_init_jam_f5d8235);
   781		}
   782		if (of_machine_is_compatible("netgear,dgn3500") ||
   783		    of_machine_is_compatible("netgear,dgn3500b")) {
   784			jam_table = rtl8366rb_init_jam_dgn3500;
   785			jam_size = ARRAY_SIZE(rtl8366rb_init_jam_dgn3500);
   786		}
   787	
   788		i = 0;
   789		while (i < jam_size) {
   790			if ((jam_table[i] & 0xBE00) == 0xBE00) {
   791				ret = regmap_read(smi->map,
   792						  RTL8366RB_PHY_ACCESS_BUSY_REG,
   793						  &val);
   794				if (ret)
   795					return ret;
   796				if (!(val & RTL8366RB_PHY_INT_BUSY)) {
   797					ret = regmap_write(smi->map,
   798							RTL8366RB_PHY_ACCESS_CTRL_REG,
   799							RTL8366RB_PHY_CTRL_WRITE);
   800					if (ret)
   801						return ret;
   802				}
   803			}
   804			dev_dbg(smi->dev, "jam %04x into register %04x\n",
   805				jam_table[i + 1],
   806				jam_table[i]);
   807			ret = regmap_write(smi->map,
   808					   jam_table[i],
   809					   jam_table[i + 1]);
   810			if (ret)
   811				return ret;
   812			i += 2;
   813		}
   814	
   815		/* Set up the "green ethernet" feature */
   816		i = 0;
   817		while (i < ARRAY_SIZE(rtl8366rb_green_jam)) {
   818			ret = regmap_read(smi->map, RTL8366RB_PHY_ACCESS_BUSY_REG,
   819					  &val);
   820			if (ret)
   821				return ret;
   822			if (!(val & RTL8366RB_PHY_INT_BUSY)) {
   823				ret = regmap_write(smi->map,
   824						   RTL8366RB_PHY_ACCESS_CTRL_REG,
   825						   RTL8366RB_PHY_CTRL_WRITE);
   826				if (ret)
   827					return ret;
   828				ret = regmap_write(smi->map,
   829						   rtl8366rb_green_jam[i][0],
   830						   rtl8366rb_green_jam[i][1]);
   831				if (ret)
   832					return ret;
   833				i++;
   834			}
   835		}
   836		ret = regmap_write(smi->map,
   837				   RTL8366RB_GREEN_FEATURE_REG,
   838				   (chip_ver == 1) ? 0x0007 : 0x0003);
   839		if (ret)
   840			return ret;
   841	
   842		/* Vendor driver sets 0x240 in registers 0xc and 0xd (undocumented) */
   843		ret = regmap_write(smi->map, 0x0c, 0x240);
   844		if (ret)
   845			return ret;
   846		ret = regmap_write(smi->map, 0x0d, 0x240);
   847		if (ret)
   848			return ret;
   849	
   850		/* Set some random MAC address */
   851		ret = rtl8366rb_set_addr(smi);
   852		if (ret)
   853			return ret;
   854	
   855		/* Enable CPU port with custom DSA tag 8899.
   856		 *
   857		 * If you set RTL8368RB_CPU_NO_TAG (bit 15) in this registers
   858		 * the custom tag is turned off.
   859		 */
   860		ret = regmap_update_bits(smi->map, RTL8368RB_CPU_CTRL_REG,
   861					 0xFFFF,
   862					 BIT(smi->cpu_port));
   863		if (ret)
   864			return ret;
   865	
   866		/* Make sure we default-enable the fixed CPU port */
   867		ret = regmap_update_bits(smi->map, RTL8366RB_PECR,
   868					 BIT(smi->cpu_port),
   869					 0);
   870		if (ret)
   871			return ret;
   872	
   873		/* Set maximum packet length to 1536 bytes */
   874		ret = regmap_update_bits(smi->map, RTL8366RB_SGCR,
   875					 RTL8366RB_SGCR_MAX_LENGTH_MASK,
   876					 RTL8366RB_SGCR_MAX_LENGTH_1536);
   877		if (ret)
   878			return ret;
   879		for (i = 0; i < RTL8366RB_NUM_PORTS; i++)
   880			/* layer 2 size, see rtl8366rb_change_mtu() */
   881			rb->max_mtu[i] = 1532;
   882	
   883		/* Enable learning for all ports */
   884		ret = regmap_write(smi->map, RTL8366RB_SSCR0, 0);
   885		if (ret)
   886			return ret;
   887	
   888		/* Enable auto ageing for all ports */
   889		ret = regmap_write(smi->map, RTL8366RB_SSCR1, 0);
   890		if (ret)
   891			return ret;
   892	
   893		/* Port 4 setup: this enables Port 4, usually the WAN port,
   894		 * common PHY IO mode is apparently mode 0, and this is not what
   895		 * the port is initialized to. There is no explanation of the
   896		 * IO modes in the Realtek source code, if your WAN port is
   897		 * connected to something exotic such as fiber, then this might
   898		 * be worth experimenting with.
   899		 */
   900		ret = regmap_update_bits(smi->map, RTL8366RB_PMC0,
   901					 RTL8366RB_PMC0_P4_IOMODE_MASK,
   902					 0 << RTL8366RB_PMC0_P4_IOMODE_SHIFT);
   903		if (ret)
   904			return ret;
   905	
   906		/* Discard VLAN tagged packets if the port is not a member of
   907		 * the VLAN with which the packets is associated.
   908		 */
   909		ret = regmap_write(smi->map, RTL8366RB_VLAN_INGRESS_CTRL2_REG,
   910				   RTL8366RB_PORT_ALL);
   911		if (ret)
   912			return ret;
   913	
   914		/* Don't drop packets whose DA has not been learned */
   915		ret = regmap_update_bits(smi->map, RTL8366RB_SSCR2,
   916					 RTL8366RB_SSCR2_DROP_UNKNOWN_DA, 0);
   917		if (ret)
   918			return ret;
   919	
   920		/* Set blinking, TODO: make this configurable */
   921		ret = regmap_update_bits(smi->map, RTL8366RB_LED_BLINKRATE_REG,
   922					 RTL8366RB_LED_BLINKRATE_MASK,
   923					 RTL8366RB_LED_BLINKRATE_56MS);
   924		if (ret)
   925			return ret;
   926	
   927		/* Set up LED activity:
   928		 * Each port has 4 LEDs, we configure all ports to the same
   929		 * behaviour (no individual config) but we can set up each
   930		 * LED separately.
   931		 */
   932		if (smi->leds_disabled) {
   933			/* Turn everything off */
   934			regmap_update_bits(smi->map,
   935					   RTL8366RB_LED_0_1_CTRL_REG,
   936					   0x0FFF, 0);
   937			regmap_update_bits(smi->map,
   938					   RTL8366RB_LED_2_3_CTRL_REG,
   939					   0x0FFF, 0);
   940			regmap_update_bits(smi->map,
   941					   RTL8366RB_INTERRUPT_CONTROL_REG,
   942					   RTL8366RB_P4_RGMII_LED,
   943					   0);
   944			val = RTL8366RB_LED_OFF;
   945		} else {
   946			/* TODO: make this configurable per LED */
   947			val = RTL8366RB_LED_FORCE;
   948		}
   949		for (i = 0; i < 4; i++) {
   950			ret = regmap_update_bits(smi->map,
   951						 RTL8366RB_LED_CTRL_REG,
   952						 0xf << (i * 4),
   953						 val << (i * 4));
   954			if (ret)
   955				return ret;
   956		}
   957	
   958		ret = rtl8366_init_vlan(smi);
   959		if (ret)
   960			return ret;
   961	
   962		ret = rtl8366rb_setup_cascaded_irq(smi);
   963		if (ret)
   964			dev_info(smi->dev, "no interrupt support\n");
   965	
   966		ret = realtek_smi_setup_mdio(smi);
   967		if (ret) {
   968			dev_info(smi->dev, "could not set up MDIO bus\n");
   969			return -ENODEV;
   970		}
   971	
   972		return 0;
   973	}
   974	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[e-mail]

Linus Walleij Oct. 7, 2020, 9:36 a.m. UTC | #2

On Wed, Oct 7, 2020 at 12:19 AM kernel test robot <[e-mail]> wrote:

> I love your patch! Yet something to improve:
>
> [auto build test ERROR on net-next/master]

Ooops fixing syntax without checking semantics, will respin.

Yours,
Linus Walleij

Patch

diff --git a/drivers/net/dsa/realtek-smi-core.c b/drivers/net/dsa/realtek-smi-core.c
index fae188c60191..8e49d4f85d48 100644
--- a/drivers/net/dsa/realtek-smi-core.c
+++ b/drivers/net/dsa/realtek-smi-core.c
@@ -394,9 +394,10 @@  static int realtek_smi_probe(struct platform_device *pdev)
 	var = of_device_get_match_data(dev);
 	np = dev->of_node;
 
-	smi = devm_kzalloc(dev, sizeof(*smi), GFP_KERNEL);
+	smi = devm_kzalloc(dev, sizeof(*smi) + var->chip_data_sz, GFP_KERNEL);
 	if (!smi)
 		return -ENOMEM;
+	smi->chip_data = (void *)smi + sizeof(*smi);
 	smi->map = devm_regmap_init(dev, NULL, smi,
 				    &realtek_smi_mdio_regmap_config);
 	if (IS_ERR(smi->map)) {
diff --git a/drivers/net/dsa/realtek-smi-core.h b/drivers/net/dsa/realtek-smi-core.h
index 6f2dab7e33d6..bc7bd47fb037 100644
--- a/drivers/net/dsa/realtek-smi-core.h
+++ b/drivers/net/dsa/realtek-smi-core.h
@@ -71,6 +71,7 @@  struct realtek_smi {
 	int			vlan4k_enabled;
 
 	char			buf[4096];
+	void			*chip_data; /* Per-chip extra variant data */
 };
 
 /**
@@ -111,6 +112,7 @@  struct realtek_smi_variant {
 	unsigned int clk_delay;
 	u8 cmd_read;
 	u8 cmd_write;
+	size_t chip_data_sz;
 };
 
 /* SMI core calls */
diff --git a/drivers/net/dsa/rtl8366rb.c b/drivers/net/dsa/rtl8366rb.c
index 053bf5041f8d..48f560c9850d 100644
--- a/drivers/net/dsa/rtl8366rb.c
+++ b/drivers/net/dsa/rtl8366rb.c
@@ -311,6 +311,13 @@ 
 #define RTL8366RB_GREEN_FEATURE_TX	BIT(0)
 #define RTL8366RB_GREEN_FEATURE_RX	BIT(2)
 
+/**
+ * struct rtl8366rb - RTL8366RB-specific data
+ */
+struct rtl8366rb {
+	unsigned int max_mtu[RTL8366RB_NUM_PORTS];
+};
+
 static struct rtl8366_mib_counter rtl8366rb_mib_counters[] = {
 	{ 0,  0, 4, "IfInOctets"				},
 	{ 0,  4, 4, "EtherStatsOctets"				},
@@ -710,6 +717,7 @@  static const u16 rtl8366rb_green_jam[][2] = {
 
 static int rtl8366rb_setup(struct dsa_switch *ds)
 {
+	struct rtl8366rb *rb = smi->chip_data;
 	struct realtek_smi *smi = ds->priv;
 	const u16 *jam_table;
 	u32 chip_ver = 0;
@@ -871,6 +879,9 @@  static int rtl8366rb_setup(struct dsa_switch *ds)
 				 RTL8366RB_SGCR_MAX_LENGTH_1536);
 	if (ret)
 		return ret;
+	for (i = 0; i < RTL8366RB_NUM_PORTS; i++)
+		/* layer 2 size, see rtl8366rb_change_mtu() */
+		rb->max_mtu[i] = 1532;
 
 	/* Enable learning for all ports */
 	ret = regmap_write(smi->map, RTL8366RB_SSCR0, 0);
@@ -1112,20 +1123,36 @@  rtl8366rb_port_disable(struct dsa_switch *ds, int port)
 static int rtl8366rb_change_mtu(struct dsa_switch *ds, int port, int new_mtu)
 {
 	struct realtek_smi *smi = ds->priv;
+	struct rtl8366rb *rb;
+	unsigned int max_mtu;
 	u32 len;
+	int i;
 
-	/* The first setting, 1522 bytes, is max IP packet 1500 bytes,
+	/* Cache the per-port MTU setting */
+	rb = smi->chip_data;
+	rb->max_mtu[port] = new_mtu;
+
+	/* Roof out the MTU for the entire switch to the greatest
+	 * common denominator: the biggest set for any one port will
+	 * be the biggest MTU for the switch.
+	 *
+	 * The first setting, 1522 bytes, is max IP packet 1500 bytes,
 	 * plus ethernet header, 1518 bytes, plus CPU tag, 4 bytes.
 	 * This function should consider the parameter an SDU, so the
 	 * MTU passed for this setting is 1518 bytes. The same logic
 	 * of subtracting the DSA tag of 4 bytes apply to the other
 	 * settings.
 	 */
-	if (new_mtu <= 1518)
+	max_mtu = 1518;
+	for (i = 0; i < RTL8366RB_NUM_PORTS; i++) {
+		if (rb->max_mtu[i] > max_mtu)
+			max_mtu = rb->max_mtu[i];
+	}
+	if (max_mtu <= 1518)
 		len = RTL8366RB_SGCR_MAX_LENGTH_1522;
-	else if (new_mtu > 1518 && new_mtu <= 1532)
+	else if (max_mtu > 1518 && max_mtu <= 1532)
 		len = RTL8366RB_SGCR_MAX_LENGTH_1536;
-	else if (new_mtu > 1532 && new_mtu <= 1548)
+	else if (max_mtu > 1532 && max_mtu <= 1548)
 		len = RTL8366RB_SGCR_MAX_LENGTH_1552;
 	else
 		len = RTL8366RB_SGCR_MAX_LENGTH_16000;
@@ -1508,5 +1535,6 @@  const struct realtek_smi_variant rtl8366rb_variant = {
 	.clk_delay = 10,
 	.cmd_read = 0xa9,
 	.cmd_write = 0xa8,
+	.chip_data_sz = sizeof(struct rtl8366rb),
 };
 EXPORT_SYMBOL_GPL(rtl8366rb_variant);