Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Mika Westerberg <mika.westerberg@linux.intel.com>
To:     linux-kernel@vger.kernel.org
Cc:     Andreas Noever <andreas.noever@gmail.com>,
        Michael Jamet <michael.jamet@intel.com>,
        Yehezkel Bernat <YehezkelShB@gmail.com>,
        Mika Westerberg <mika.westerberg@linux.intel.com>,
        Lukas Wunner <lukas@wunner.de>,
        Brad Campbell <lists2009@fnarfbargle.com>,
        Dominik Brodowski <linux@dominikbrodowski.net>,
        Bjorn Helgaas <bhelgaas@google.com>,
        Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au>,
        linux-pci@vger.kernel.org
Subject: [PATCH 1/3] thunderbolt: Read DP IN adapter first two dwords in one go
Date:   Tue,  1 Oct 2019 13:29:03 +0300
Message-Id: <20191001102905.21680-2-mika.westerberg@linux.intel.com>
In-Reply-To: <20191001102905.21680-1-mika.westerberg@linux.intel.com>
References: <20191001102905.21680-1-mika.westerberg@linux.intel.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

When we discover existing DP tunnels the code checks whether DP IN
adapter port is enabled by calling tb_dp_port_is_enabled() before it
continues the discovery process. On Light Ridge (gen 1) controller
reading only the first dword of the DP IN config space causes subsequent
access to the same DP IN port path config space to fail or return
invalid data as can be seen in the below splat:

  thunderbolt 0000:07:00.0: CFG_ERROR(0:d): Invalid config space or offset
  Call Trace:
   tb_cfg_read+0xb9/0xd0
   __tb_path_deactivate_hop+0x98/0x210
   tb_path_activate+0x228/0x7d0
   tb_tunnel_restart+0x95/0x200
   tb_handle_hotplug+0x30e/0x630
   process_one_work+0x1b4/0x340
   worker_thread+0x44/0x3d0
   kthread+0xeb/0x120
   ? process_one_work+0x340/0x340
   ? kthread_park+0xa0/0xa0
   ret_from_fork+0x1f/0x30

If both DP In adapter config dwords are read in one go the issue does
not reproduce. This is likely firmware bug but we can work it around by
always reading the two dwords in one go. There should be no harm for
other controllers either so can do it unconditionally.

Link: https://lkml.org/lkml/2019/8/28/160
Reported-by: Brad Campbell <lists2009@fnarfbargle.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/thunderbolt/switch.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 410bf1bceeee..8e712fbf8233 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -896,12 +896,13 @@ int tb_dp_port_set_hops(struct tb_port *port, unsigned int video,
  */
 bool tb_dp_port_is_enabled(struct tb_port *port)
 {
-	u32 data;
+	u32 data[2];
 
-	if (tb_port_read(port, &data, TB_CFG_PORT, port->cap_adap, 1))
+	if (tb_port_read(port, data, TB_CFG_PORT, port->cap_adap,
+			 ARRAY_SIZE(data)))
 		return false;
 
-	return !!(data & (TB_DP_VIDEO_EN | TB_DP_AUX_EN));
+	return !!(data[0] & (TB_DP_VIDEO_EN | TB_DP_AUX_EN));
 }
 
 /**
@@ -914,19 +915,21 @@ bool tb_dp_port_is_enabled(struct tb_port *port)
  */
 int tb_dp_port_enable(struct tb_port *port, bool enable)
 {
-	u32 data;
+	u32 data[2];
 	int ret;
 
-	ret = tb_port_read(port, &data, TB_CFG_PORT, port->cap_adap, 1);
+	ret = tb_port_read(port, data, TB_CFG_PORT, port->cap_adap,
+			   ARRAY_SIZE(data));
 	if (ret)
 		return ret;
 
 	if (enable)
-		data |= TB_DP_VIDEO_EN | TB_DP_AUX_EN;
+		data[0] |= TB_DP_VIDEO_EN | TB_DP_AUX_EN;
 	else
-		data &= ~(TB_DP_VIDEO_EN | TB_DP_AUX_EN);
+		data[0] &= ~(TB_DP_VIDEO_EN | TB_DP_AUX_EN);
 
-	return tb_port_write(port, &data, TB_CFG_PORT, port->cap_adap, 1);
+	return tb_port_write(port, data, TB_CFG_PORT, port->cap_adap,
+			     ARRAY_SIZE(data));
 }
 
 /* switch utility functions */
-- 
2.23.0