arm: mvebu: a38x: serdes: Don't configure PCIe cards in SerDes init code

This code is trying to parse PCIe config space of PCIe card connected on
the other end of link and then is trying to force 5.0 GT/s speed via Target
Link Speed bits in PCIe Root Port Link Control 2 Register on the local part
of link if it sees that card supports 5.0 GT/s via Max Link Speed bits in
Link Capabilities Register.

The code is incorrect for more reasons:
- Accessing config space of an endpoint card cannot be done immediately.
  If the PCIe link is not up, reading vendor/device ID registers will
  return all ones.
- Parsing is incomplete, so it can cause issues even for working cards.

Moreover there is no need to force speed to 5.0 GT/s via Target Link Speed
bits on PCIe Root Port Link Control 2 Register. Hardware changes speed from
2.5 GT/s to 5.0 GT/s autonomously when it is supported.

Most importantly, this code does not change link speed at all, since
because after updating Target Link Speed bits on PCIe Root Port Link
Control 2 Register, it is required to retrain the link, and the code for
that is completely missing.

The code was probably needed for making buggy endpoint cards work. Such a
workaround, though, should be implemented via PCIe subsystem (via quirks,
for example), as buggy cards could also affect other PCIe controllers.

Note that this code is fully unrelated to a38x SerDes code and really
should not have been included in SerDes initialization. Usage of magic
constants without names and comments made this SerDes code hard to read and
understand.

Remove this PCIe application code from low level SerDes code. As this code
is configuring only 5.0 GT/s part, in the worst case, it could leave buggy
cards at the initial speed of 2.5 GT/s (if somehow before this change they
could have been "upgraded" to 5.0 GT/s speed even with missing link
retraining). Compliant cards which just need longer initialization should
work better after this change.

Signed-off-by: Pali Rohár <pali@kernel.org>
Reviewed-by: Marek Behún <marek.behun@nic.cz>
Reviewed-by: Stefan Roese <sr@denx.de>

diff --git a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c
index a7e45a5550ddcfbf9a3365e2e3e1c5b9e6b095b4..0445b43deffa93c9976987b5f760208e308b610f 100644 (file)
--- a/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c
+++ b/arch/arm/mach-mvebu/serdes/a38x/ctrl_pex.c
@@ -21,10 +21,8 @@ __weak void board_pex_config(void)
 
 int hws_pex_config(const struct serdes_map *serdes_map, u8 count)
 {
-       u32 pex_idx, tmp, next_busno, first_busno, temp_pex_reg,
-           temp_reg, addr, dev_id, ctrl_mode;
        enum serdes_type serdes_type;
-       u32 idx;
+       u32 idx, tmp;
 
        DEBUG_INIT_FULL_S("\n### hws_pex_config ###\n");
 
@@ -60,132 +58,8 @@ int hws_pex_config(const struct serdes_map *serdes_map, u8 count)
 
        reg_write(SOC_CONTROL_REG1, tmp);
 
-       /* Support gen1/gen2 */
-       DEBUG_INIT_FULL_S("Support gen1/gen2\n");
-
        board_pex_config();
 
-       next_busno = 0;
-       mdelay(150);
-
-       for (idx = 0; idx < count; idx++) {
-               serdes_type = serdes_map[idx].serdes_type;
-               DEBUG_INIT_FULL_S(" serdes_type=0x");
-               DEBUG_INIT_FULL_D(serdes_type, 8);
-               DEBUG_INIT_FULL_S("\n");
-               DEBUG_INIT_FULL_S(" idx=0x");
-               DEBUG_INIT_FULL_D(idx, 8);
-               DEBUG_INIT_FULL_S("\n");
-
-               /* Configuration for PEX only */
-               if ((serdes_type != PEX0) && (serdes_type != PEX1) &&
-                   (serdes_type != PEX2) && (serdes_type != PEX3))
-                       continue;
-
-               if ((serdes_type != PEX0) &&
-                   ((serdes_map[idx].serdes_mode == PEX_ROOT_COMPLEX_X4) ||
-                    (serdes_map[idx].serdes_mode == PEX_END_POINT_X4))) {
-                       /* for PEX by4 - relevant for the first port only */
-                       continue;
-               }
-
-               pex_idx = serdes_type - PEX0;
-               tmp = reg_read(PEX_DBG_STATUS_REG(pex_idx));
-
-               first_busno = next_busno;
-               if ((tmp & 0x7f) != 0x7e) {
-                       DEBUG_INIT_S("PCIe, Idx ");
-                       DEBUG_INIT_D(pex_idx, 1);
-                       DEBUG_INIT_S(": detected no link\n");
-                       continue;
-               }
-
-               next_busno++;
-
-               /*
-                * Read maximum link speed. It must be 0x2 (5.0 GT/s) as this
-                * value was set in serdes_power_up_ctrl() function.
-                */
-               temp_pex_reg = reg_read((PEX_CFG_DIRECT_ACCESS
-                                        (pex_idx, PEX_LINK_CAPABILITY_REG)));
-               temp_pex_reg &= 0xf;
-               if (temp_pex_reg != 0x2)
-                       continue;
-
-               /* Read negotiated link speed */
-               temp_reg = (reg_read(PEX_CFG_DIRECT_ACCESS(
-                                            pex_idx,
-                                            PEX_LINK_CTRL_STAT_REG)) &
-                           0xf0000) >> 16;
-
-               /* Check if the link established is GEN1 */
-               DEBUG_INIT_FULL_S
-                       ("Checking if the link established is gen1\n");
-               if (temp_reg != 0x1)
-                       continue;
-
-               pex_local_bus_num_set(pex_idx, first_busno);
-               pex_local_dev_num_set(pex_idx, 1);
-               DEBUG_INIT_FULL_S("PCIe, Idx ");
-               DEBUG_INIT_FULL_D(pex_idx, 1);
-
-               DEBUG_INIT_S(":** Link is Gen1, check the EP capability\n");
-               /* link is Gen1, check the EP capability */
-               addr = pex_config_read(pex_idx, first_busno, 0, 0, 0x34) & 0xff;
-               DEBUG_INIT_FULL_C("pex_config_read: return addr=0x%x", addr, 4);
-               if (addr == 0xff) {
-                       DEBUG_INIT_FULL_C
-                               ("pex_config_read: return 0xff -->PCIe (%d): Detected No Link.",
-                                pex_idx, 1);
-                       continue;
-               }
-
-               /* Find start of the PCI Express Capability registers */
-               while ((pex_config_read(pex_idx, first_busno, 0, 0, addr)
-                       & 0xff) != 0x10) {
-                       addr = (pex_config_read(pex_idx, first_busno, 0,
-                                               0, addr) & 0xff00) >> 8;
-               }
-
-               /* Check for Gen2 and above */
-               if ((pex_config_read(pex_idx, first_busno, 0, 0,
-                                    addr + 0xc) & 0xf) < 0x2) {
-                       DEBUG_INIT_S("PCIe, Idx ");
-                       DEBUG_INIT_D(pex_idx, 1);
-                       DEBUG_INIT_S(": remains Gen1\n");
-                       continue;
-               }
-
-               tmp = reg_read(PEX_LINK_CTRL_STATUS2_REG(pex_idx));
-               DEBUG_RD_REG(PEX_LINK_CTRL_STATUS2_REG(pex_idx), tmp);
-               tmp &= ~(BIT(0) | BIT(1));
-               tmp |= BIT(1);  /* Force Target Link Speed to 5.0 GT/s */
-               tmp |= BIT(6);  /* Select Deemphasize (-3.5d_b) */
-               reg_write(PEX_LINK_CTRL_STATUS2_REG(pex_idx), tmp);
-               DEBUG_WR_REG(PEX_LINK_CTRL_STATUS2_REG(pex_idx), tmp);
-
-               /*
-                * Enable Auto Speed change. When set, link will issue link
-                * speed change to max possible link speed.
-                */
-               tmp = reg_read(PEX_CTRL_REG(pex_idx));
-               DEBUG_RD_REG(PEX_CTRL_REG(pex_idx), tmp);
-               tmp |= BIT(10);
-               reg_write(PEX_CTRL_REG(pex_idx), tmp);
-               DEBUG_WR_REG(PEX_CTRL_REG(pex_idx), tmp);
-
-               /*
-                * We need to wait 10ms before reading the PEX_DBG_STATUS_REG
-                * in order not to read the status of the former state
-                */
-               mdelay(10);
-
-               DEBUG_INIT_S("PCIe, Idx ");
-               DEBUG_INIT_D(pex_idx, 1);
-               DEBUG_INIT_S
-                       (": Link upgraded to Gen2 based on client capabilities\n");
-       }
-
        return MV_OK;
 }