Re: [PATCH v2] thunderbolt: Explicitly enable lane adapter hotplug events at startup

Mika Westerberg
2022-09-23T09:16:40Z

Hi Mario,

On Thu, Sep 22, 2022 at 11:07:29AM -0500, Mario Limonciello wrote:
> Software that has run before the USB4 CM in Linux runs may have disabled
> hotplug events for a given lane adapter.
>
> Other CMs such as that one distributed with Windows 11 will enable hotplug
> events. Do the same thing in the Linux CM which fixes hotplug events on
> "AMD Pink Sardine".
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> v1->v2:
> * Only send second patch as first was merged already
> * s/usb4_enable_hotplug/usb4_port_hotplug_enable/
> * Clarify intended users in documentation comment
> * Only call for lane adapters
> * Add stable tag
>
> drivers/thunderbolt/switch.c | 4 ++++
> drivers/thunderbolt/tb.h | 1 +
> drivers/thunderbolt/tb_regs.h | 1 +
> drivers/thunderbolt/usb4.c | 20 ++++++++++++++++++++
> 4 files changed, 26 insertions(+)
>
> diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
> index 77d7f07ca075..3213239d12c8 100644
> --- a/drivers/thunderbolt/switch.c
> +++ b/drivers/thunderbolt/switch.c
> @@ -778,6 +778,10 @@ static int tb_init_port(struct tb_port *port)
>
> if (!tb_port_read(port, &hop, TB_CFG_HOPS, 0, 2))
> port->ctl_credits = hop.initial_credits;
> +
> + res = usb4_port_hotplug_enable(port);
> + if (res)

I think this does not belong here in tb_init_port(). This is called from
both FW and SW CM paths and we don't want to confuse the FW CM more than
necessary ;-)

So instead I think this should be added to tb_plug_events_active().

Otherwise looks good to me.

> + return res;
> }
> if (!port->ctl_credits)
> port->ctl_credits = 2;
> diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
> index 5db76de40cc1..332159f984fc 100644
> --- a/drivers/thunderbolt/tb.h
> +++ b/drivers/thunderbolt/tb.h
> @@ -1174,6 +1174,7 @@ int usb4_switch_add_ports(struct tb_switch *sw);
> void usb4_switch_remove_ports(struct tb_switch *sw);
>
> int usb4_port_unlock(struct tb_port *port);
> +int usb4_port_hotplug_enable(struct tb_port *port);
> int usb4_port_configure(struct tb_port *port);
> void usb4_port_unconfigure(struct tb_port *port);
> int usb4_port_configure_xdomain(struct tb_port *port);
> diff --git a/drivers/thunderbolt/tb_regs.h b/drivers/thunderbolt/tb_regs.h
> index 166054110388..bbe38b2d9057 100644
> --- a/drivers/thunderbolt/tb_regs.h
> +++ b/drivers/thunderbolt/tb_regs.h
> @@ -308,6 +308,7 @@ struct tb_regs_port_header {
> #define ADP_CS_5 0x05
> #define ADP_CS_5_LCA_MASK GENMASK(28, 22)
> #define ADP_CS_5_LCA_SHIFT 22
> +#define ADP_CS_5_DHP BIT(31)
>
> /* TMU adapter registers */
> #define TMU_ADP_CS_3 0x03
> diff --git a/drivers/thunderbolt/usb4.c b/drivers/thunderbolt/usb4.c
> index 3a2e7126db9d..f0b5a8f1ed3a 100644
> --- a/drivers/thunderbolt/usb4.c
> +++ b/drivers/thunderbolt/usb4.c
> @@ -1046,6 +1046,26 @@ int usb4_port_unlock(struct tb_port *port)
> return tb_port_write(port, &val, TB_CFG_PORT, ADP_CS_4, 1);
> }
>
> +/**
> + * usb4_port_hotplug_enable() - Enables hotplug for a port
> + * @port: USB4 port to operate on
> + *
> + * Enables hot plug events on a given port. This is only intended
> + * to be used on lane, DP-IN, and DP-OUT adapters.
> + */
> +int usb4_port_hotplug_enable(struct tb_port *port)
> +{
> + int ret;
> + u32 val;
> +
> + ret = tb_port_read(port, &val, TB_CFG_PORT, ADP_CS_5, 1);
> + if (ret)
> + return ret;
> +
> + val &= ~ADP_CS_5_DHP;
> + return tb_port_write(port, &val, TB_CFG_PORT, ADP_CS_5, 1);
> +}
> +
> static int usb4_port_set_configured(struct tb_port *port, bool configured)
> {
> int ret;
> --
> 2.34.1