| Message ID | 20201006193453.4069-1-linus.walleij@linaro.org |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net-next,v2] net: dsa: rtl8366rb: Roof MTU for switch | expand |
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 <lkp@intel.com>
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/kbuild-all@lists.01.org
On Wed, Oct 7, 2020 at 12:19 AM kernel test robot <lkp@intel.com> 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
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);
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 <linus.walleij@linaro.org> --- 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(-)