qca-ssdk: fix unsupported scenario with PORT1 not declared in switch bmp
authorChristian Marangi <ansuelsmth@gmail.com>
Sun, 12 Nov 2023 19:25:24 +0000 (20:25 +0100)
committerChristian Marangi <ansuelsmth@gmail.com>
Mon, 13 Nov 2023 13:27:16 +0000 (14:27 +0100)
Commit 947b44d ("ipq807x: fix wrong define for LAN and WAN ess mask")
started fixing wrong switch_lan_bmp that defined lan there weren't
actually present. This displayed a fragility in the malibu phy init code
in qca-ssdk.

Add patch to fix this. Also update each DTS with the new required
property if needed.

The new binding malibu_phy_start_addr is required with devices that
place the malibu first PHY referring port1 on a different PHY addres
than 0. The most common configuration is 0 but some device (for example
Qnap 301W) place the malibu PHY at an offset to address 16.

Refer to ipq8074-ess dtsi for extensive description on how to derive
this value.

Quoting the patch detailed description:

The usage of first_phy_addr is EXTREMELY FRAGILE and results
in dangerous results if the OEM (or anyone that by chance try to
implement things in a logical manner) deviates from the default values
from the "magical template".

To be in more details. With QSDK 12.4, some tweaks were done to improve
autoneg and now on every call of port status, the phydev is tried to
add. This resulted in the call and log spam of an error with ports that
are actually not present on the system with qsdk reporting phydev is
NULL. This itself is not an error and printing the error is correct.

What is actually an error from ages is setting generic bitmap reporting
presence of port that are actually not present. This is very common on
OEM where the switch_lan_bmp is always a variant of 0x1e (that on bitmap
results in PORT1 PORT2 PORT3 PORT4 present) or 0x3e (PORT1 PORT2 PORT3
PORT4 PORT5). Reality is that many device are used as AP with one LAN
port or one WAN port. (or even exotic configuration with PORT1 not
present and PORT2 PORT3 PORT4 present (Xiaomi 3600)

With this finding one can say... ok nice, then lets update the DT and
set the correct bitmap...

Again world is a bad place and reality is that this cause wonderful
regression in some case of by extreme luck the first ever connected
port working and the rest of the switch dead.

The problem has been bisected to all the device that doesn't have the
PORT1 declared in any of the bitmap.

With this prefaction in mind, on to the REAL problem.

malibu_phy_hw_init FOR SOME REASON, set a global variable first_phy_addr
to the first detected PHY addr that coincidentally is always PORT1.
PORT1 addr is 0x0. The entire code in malibu_phy use this variable to
derive the phy addrs in some function.

Declaring a bitmap where the PORT1 is missing (or worse PORT4 the only
one connected) result in first_phy_addr set to 1 or whatever phy addr is
detected first setting wrong value all over the init stage.

To fix this, introduce a new binding malibu_first_phy_addr to manually
declare the first phy that the malibu PHY driver should use and permit
to detach it from port bmp detection. The legacy detection is kept for
compatibility reason.

Fixes: #13945
Fixes: 947b44d9ae17 ("ipq807x: fix wrong define for LAN and WAN ess mask")
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Tested-by: Robert Marko <robimarko@gmail.com> # Qnap 301W
Reviewed-by: Robert Marko <robimarko@gmail.com>
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
package/kernel/qca-ssdk/Makefile
package/kernel/qca-ssdk/patches/100-malibu-phy-add-support-for-manual-define-of-first-ph.patch [new file with mode: 0644]
target/linux/qualcommax/files/arch/arm64/boot/dts/qcom/ipq8072-301w.dts
target/linux/qualcommax/files/arch/arm64/boot/dts/qcom/ipq8074-ess.dtsi

index f5d82605034a1dab3d063583a0ebb8755ef886c8..4107208c0eb93508b0524b750acbc41e5f93b65a 100644 (file)
@@ -1,7 +1,7 @@
 include $(TOPDIR)/rules.mk
 
 PKG_NAME:=qca-ssdk
-PKG_RELEASE:=1
+PKG_RELEASE:=2
 
 PKG_SOURCE_URL:=https://git.codelinaro.org/clo/qsdk/oss/lklm/qca-ssdk.git
 PKG_SOURCE_PROTO:=git
diff --git a/package/kernel/qca-ssdk/patches/100-malibu-phy-add-support-for-manual-define-of-first-ph.patch b/package/kernel/qca-ssdk/patches/100-malibu-phy-add-support-for-manual-define-of-first-ph.patch
new file mode 100644 (file)
index 0000000..6aaa579
--- /dev/null
@@ -0,0 +1,131 @@
+From a651d10fbd880098d7b98dee27dfd1eb15146fb2 Mon Sep 17 00:00:00 2001
+From: Christian Marangi <ansuelsmth@gmail.com>
+Date: Sun, 12 Nov 2023 18:40:22 +0100
+Subject: [PATCH] malibu-phy: add support for manual define of first phy addr
+
+The usage of first_phy_addr is EXTREMELY FRAGILE and results
+in dangerous results if the OEM (or anyone that by chance try to
+implement things in a logical manner) deviates from the default values
+from the "magical template".
+
+To be in more details. With QSDK 12.4, some tweaks were done to improve
+autoneg and now on every call of port status, the phydev is tried to
+add. This resulted in the call and log spam of an error with ports that
+are actually not present on the system with qsdk reporting phydev is
+NULL. This itself is not an error and printing the error is correct.
+
+What is actually an error from ages is setting generic bitmap reporting
+presence of port that are actually not present. This is very common on
+OEM where the switch_lan_bmp is always a variant of 0x1e (that on bitmap
+results in PORT1 PORT2 PORT3 PORT4 present) or 0x3e (PORT1 PORT2 PORT3
+PORT4 PORT5). Reality is that many device are used as AP with one LAN
+port or one WAN port. (or even exotic configuration with PORT1 not
+present and PORT2 PORT3 PORT4 present (Xiaomi 3600)
+
+With this finding one can say... ok nice, then lets update the DT and
+set the correct bitmap...
+
+Again world is a bad place and reality is that this cause wonderful
+regression in some case of by extreme luck the first ever connected
+port working and the rest of the switch dead.
+
+The problem has been bisected to all the device that doesn't have the
+PORT1 declared in any of the bitmap.
+
+With this prefaction in mind, on to the REAL problem.
+
+malibu_phy_hw_init FOR SOME REASON, set a global variable first_phy_addr
+to the first detected PHY addr that coincidentally is always PORT1.
+PORT1 addr is 0x0. The entire code in malibu_phy use this variable to
+derive the phy addrs in some function.
+
+Declaring a bitmap where the PORT1 is missing (or worse PORT4 the only
+one connected) result in first_phy_addr set to 1 or whatever phy addr is
+detected first setting wrong value all over the init stage.
+
+To fix this, introduce a new binding malibu_first_phy_addr to manually
+declare the first phy that the malibu PHY driver should use and permit
+to detach it from port bmp detection. The legacy detection is kept for
+compatibility reason.
+
+Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
+---
+ include/init/ssdk_dts.h  |  1 +
+ include/init/ssdk_init.h |  2 ++
+ src/hsl/phy/malibu_phy.c |  5 +++++
+ src/init/ssdk_dts.c      | 15 +++++++++++++++
+ 4 files changed, 23 insertions(+)
+
+--- a/include/init/ssdk_dts.h
++++ b/include/init/ssdk_dts.h
+@@ -146,6 +146,7 @@ a_uint32_t ssdk_wan_bmp_get(a_uint32_t d
+ sw_error_t ssdk_lan_bmp_set(a_uint32_t dev_id, a_uint32_t lan_bmp);
+ sw_error_t ssdk_wan_bmp_set(a_uint32_t dev_id, a_uint32_t wan_bmp);
+ a_uint32_t ssdk_inner_bmp_get(a_uint32_t dev_id);
++a_uint32_t ssdk_malibu_first_phy_addr_get(a_uint32_t dev_id);
+ hsl_reg_mode ssdk_switch_reg_access_mode_get(a_uint32_t dev_id);
+ void ssdk_switch_reg_map_info_get(a_uint32_t dev_id, ssdk_reg_map_info *info);
+ a_uint32_t ssdk_switch_pcie_base_get(a_uint32_t dev_id);
+--- a/include/init/ssdk_init.h
++++ b/include/init/ssdk_init.h
+@@ -194,6 +194,7 @@ enum ssdk_port_wrapper_cfg {
+               a_uint32_t lan_bmp;
+               a_uint32_t wan_bmp;
+               a_uint32_t inner_bmp;
++              a_uint32_t malibu_first_phy_addr;
+       } ssdk_port_cfg;
+       typedef struct
+@@ -384,6 +385,7 @@ ssdk_hsl_access_mode_set(a_uint32_t dev_
+ a_uint32_t ssdk_dt_global_get_mac_mode(a_uint32_t dev_id, a_uint32_t index);
+ a_uint32_t ssdk_dt_global_set_mac_mode(a_uint32_t dev_id, a_uint32_t index, a_uint32_t mode);
++a_uint32_t ssdk_malibu_first_phy_addr_get(a_uint32_t dev_id);
+ a_uint32_t
+ qca_hppe_port_mac_type_get(a_uint32_t dev_id, a_uint32_t port_id);
+--- a/src/hsl/phy/malibu_phy.c
++++ b/src/hsl/phy/malibu_phy.c
+@@ -1945,6 +1945,11 @@ static int malibu_phy_api_ops_init(void)
+ int malibu_phy_init(a_uint32_t dev_id, a_uint32_t port_bmp)
+ {
+       static a_uint32_t phy_ops_flag = 0;
++      a_uint32_t malibu_first_phy_addr;
++
++      malibu_first_phy_addr = ssdk_malibu_first_phy_addr_get(dev_id);
++      if (malibu_first_phy_addr != MAX_PHY_ADDR)
++              first_phy_addr = malibu_first_phy_addr;
+       if(phy_ops_flag == 0) {
+               malibu_phy_api_ops_init();
+--- a/src/init/ssdk_dts.c
++++ b/src/init/ssdk_dts.c
+@@ -186,6 +186,13 @@ a_uint32_t ssdk_inner_bmp_get(a_uint32_t
+       return cfg->port_cfg.inner_bmp;
+ }
++a_uint32_t ssdk_malibu_first_phy_addr_get(a_uint32_t dev_id)
++{
++      ssdk_dt_cfg* cfg = ssdk_dt_global.ssdk_dt_switch_nodes[dev_id];
++
++      return cfg->port_cfg.malibu_first_phy_addr;
++}
++
+ hsl_reg_mode ssdk_switch_reg_access_mode_get(a_uint32_t dev_id)
+ {
+       ssdk_dt_cfg* cfg = ssdk_dt_global.ssdk_dt_switch_nodes[dev_id];
+@@ -1039,6 +1046,14 @@ static void ssdk_dt_parse_port_bmp(a_uin
+                               cfg->port_cfg.inner_bmp;
+       }
++      /* Permit to manually declare start phy addr for malibu PHY. If not found set to legacy detection. */
++      if (!of_property_read_u32(switch_node, "malibu_first_phy_addr", &cfg->port_cfg.malibu_first_phy_addr)) {
++              ssdk_dt_global.ssdk_dt_switch_nodes[dev_id]->port_cfg.malibu_first_phy_addr =
++                              cfg->port_cfg.malibu_first_phy_addr;
++      } else {
++              ssdk_dt_global.ssdk_dt_switch_nodes[dev_id]->port_cfg.malibu_first_phy_addr = MAX_PHY_ADDR;
++      }
++
+       ssdk_dt_global.ssdk_dt_switch_nodes[dev_id]->port_cfg.cpu_bmp = cfg->port_cfg.cpu_bmp;
+       ssdk_dt_global.ssdk_dt_switch_nodes[dev_id]->port_cfg.lan_bmp = cfg->port_cfg.lan_bmp;
+       ssdk_dt_global.ssdk_dt_switch_nodes[dev_id]->port_cfg.wan_bmp = cfg->port_cfg.wan_bmp;
index ce82f24ae8ad7cc7fffaf0cda154cd6199688cc3..6c7ec2e588fe225217c5aaffd983ef30b6e6b40e 100644 (file)
 
        switch_lan_bmp = <(ESS_PORT1 | ESS_PORT2 | ESS_PORT3 | ESS_PORT4 | ESS_PORT5)>; /* lan port bitmap */
        switch_wan_bmp = <ESS_PORT6>; /* wan port bitmap */
+       malibu_first_phy_addr = <16>; /* PHY addr of the first malibu PHY */
        switch_mac_mode = <0xb>; /* mac mode for uniphy instance0*/
        switch_mac_mode1 = <0xd>; /* mac mode for uniphy instance1*/
        switch_mac_mode2 = <0xd>; /* mac mode for uniphy instance2*/
index 597f7f1ff98cb749b322592e5065068098911ee3..f0c1efce9ed5091c58d8a00b89640730f0763e32 100644 (file)
                switch_access_mode = "local bus";
                switch_cpu_bmp = <ESS_PORT0>;  /* cpu port bitmap */
                switch_inner_bmp = <ESS_PORT7>; /*inner port bitmap*/
+               /* This is a special binding that controls how the malibu PHY are
+                * init. This value reflect the PHY addr of the first malibu PHY.
+                * Malibu PHY are in a bundle of 5 PHY.
+                * Some device might have some port not connected.
+                * SSDK still needs the addrs of the first PHY (even if not connected)
+                * to correctly setup the malibu PHY.
+                *
+                * This is needed as previously SSDK based this on the port bmp, but
+                * this can be problematic now that we specify correct bmp.
+                *
+                * Most common configuration have the malibu PHY placed at 0.
+                * But some device might have it placed at address 16.
+                * To drive the correct value, check the port id of the malibu PHY
+                * and try to understand what is the first one in devices where some
+                * port are missing. port_phyinfo is normally the way to go to derive
+                * this value in the few special cases.
+                */
+               malibu_first_phy_addr = <0>;
                clocks = <&gcc GCC_CMN_12GPLL_AHB_CLK>,
                         <&gcc GCC_CMN_12GPLL_SYS_CLK>,
                         <&gcc GCC_UNIPHY0_AHB_CLK>,