Commit fd5c46b7 authored Sep 19, 2019 by Mika Westerberg

thunderbolt: Read DP IN adapter first two dwords in one go

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>
Tested-by: Brad Campbell <lists2009@fnarfbargle.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

parent da0c9ea1

drivers/thunderbolt/switch.c

+11 −8

Original line number	Diff line number	Diff line
		@@ -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 */

Admin message