Re: [PATCH 1/3] thunderbolt: Read DP IN adapter first two dwords in one go

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 1/10/19 6:29 pm, Mika Westerberg wrote:
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@xxxxxxxxxxxxxxx>

Tested-by: Brad Campbell <lists2009@xxxxxxxxxxxxxxx>

Signed-off-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
---
  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 */





[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux