Skip to content

Commit

Permalink
thunderbolt: Read DP IN adapter first two dwords in one go
Browse files Browse the repository at this point in the history
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 <[email protected]>
Tested-by: Brad Campbell <[email protected]>
Signed-off-by: Mika Westerberg <[email protected]>
  • Loading branch information
westeri committed Oct 8, 2019
1 parent da0c9ea commit fd5c46b
Showing 1 changed file with 11 additions and 8 deletions.
19 changes: 11 additions & 8 deletions drivers/thunderbolt/switch.c
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

/**
Expand All @@ -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 */
Expand Down

0 comments on commit fd5c46b

Please sign in to comment.