[PATCH 0/2] Fix some hotplug event issues

[PATCH 0/2] Fix some hotplug event issues

[Mario Limonciello]

On AMD Pink Sardine it's been observed that if another CM has run before
the Linux CM that some registers may not match expectations which leads
to a non-functional hotplug for PCIe devices.

Explicitly set these registers that have been observed to cause problems
with hotplug to the defaults of the USB4 spec.

Mario Limonciello (2):
  thunderbolt: Explicitly reset plug events delay back to USB4 spec
    value
  thunderbolt: Explicitly enable lane adapter hotplug events at startup

 drivers/thunderbolt/switch.c  |  5 +++++
 drivers/thunderbolt/tb.h      |  1 +
 drivers/thunderbolt/tb_regs.h |  1 +
 drivers/thunderbolt/usb4.c    | 19 +++++++++++++++++++
 4 files changed, 26 insertions(+)

-- 
2.34.1

[PATCH 1/2] thunderbolt: Explicitly reset plug events delay back to USB4 spec value

[Mario Limonciello]

If any software has interacted with the USB4 registers before the Linux
USB4 CM runs, it may have modified the plug events delay. It has been
observed that if this value too large, it's possible that hotplugged
devices will negotiate a fallback mode instead in Linux.

To prevent this, explicitly align the plug events delay with the USB4
spec value of 10ms.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/thunderbolt/switch.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index c63c1f4ff9dc..77d7f07ca075 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -2413,6 +2413,7 @@ int tb_switch_configure(struct tb_switch *sw)
 		 * additional capabilities.
 		 */
 		sw->config.cmuv = USB4_VERSION_1_0;
+		sw->config.plug_events_delay = 0xa;
 
 		/* Enumerate the switch */
 		ret = tb_sw_write(sw, (u32 *)&sw->config + 1, TB_CFG_SWITCH,
-- 
2.34.1

[PATCH 2/2] thunderbolt: Explicitly enable lane adapter hotplug events at startup

[Mario Limonciello]

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".

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/thunderbolt/switch.c  |  4 ++++
 drivers/thunderbolt/tb.h      |  1 +
 drivers/thunderbolt/tb_regs.h |  1 +
 drivers/thunderbolt/usb4.c    | 19 +++++++++++++++++++
 4 files changed, 25 insertions(+)

diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 77d7f07ca075..65f65cfe1101 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -792,6 +792,10 @@ static int tb_init_port(struct tb_port *port)
 		(port->config.nfc_credits & ADP_CS_4_TOTAL_BUFFERS_MASK) >>
 		ADP_CS_4_TOTAL_BUFFERS_SHIFT;
 
+	res = usb4_enable_hotplug(port);
+	if (res)
+		return res;
+
 	tb_dump_port(port->sw->tb, port);
 	return 0;
 }
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 5db76de40cc1..b66781ae9c16 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_enable_hotplug(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..40ba14e8e227 100644
--- a/drivers/thunderbolt/usb4.c
+++ b/drivers/thunderbolt/usb4.c
@@ -1046,6 +1046,25 @@ int usb4_port_unlock(struct tb_port *port)
 	return tb_port_write(port, &val, TB_CFG_PORT, ADP_CS_4, 1);
 }
 
+/**
+ * usb4_enable_hotplug() - Enables hotplug for a port
+ * @port: USB4 port to operate on
+ *
+ * Enables hot plug events on a given port.
+ */
+int usb4_enable_hotplug(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

Re: [PATCH 0/2] Fix some hotplug event issues

[Greg KH]

On Wed, Sep 21, 2022 at 09:54:31AM -0500, Mario Limonciello wrote:
> On AMD Pink Sardine it's been observed that if another CM has run before
> the Linux CM that some registers may not match expectations which leads
> to a non-functional hotplug for PCIe devices.
> 
> Explicitly set these registers that have been observed to cause problems
> with hotplug to the defaults of the USB4 spec.
> 
> Mario Limonciello (2):
>   thunderbolt: Explicitly reset plug events delay back to USB4 spec
>     value
>   thunderbolt: Explicitly enable lane adapter hotplug events at startup
> 
>  drivers/thunderbolt/switch.c  |  5 +++++
>  drivers/thunderbolt/tb.h      |  1 +
>  drivers/thunderbolt/tb_regs.h |  1 +
>  drivers/thunderbolt/usb4.c    | 19 +++++++++++++++++++
>  4 files changed, 26 insertions(+)

Any specific reason these were not tagged for stable backports?

thanks,

greg k-h

Re: [PATCH 0/2] Fix some hotplug event issues

[Limonciello, Mario]

On 9/21/2022 10:45, Greg KH wrote:
> On Wed, Sep 21, 2022 at 09:54:31AM -0500, Mario Limonciello wrote:
>> On AMD Pink Sardine it's been observed that if another CM has run before
>> the Linux CM that some registers may not match expectations which leads
>> to a non-functional hotplug for PCIe devices.
>>
>> Explicitly set these registers that have been observed to cause problems
>> with hotplug to the defaults of the USB4 spec.
>>
>> Mario Limonciello (2):
>>    thunderbolt: Explicitly reset plug events delay back to USB4 spec
>>      value
>>    thunderbolt: Explicitly enable lane adapter hotplug events at startup
>>
>>   drivers/thunderbolt/switch.c  |  5 +++++
>>   drivers/thunderbolt/tb.h      |  1 +
>>   drivers/thunderbolt/tb_regs.h |  1 +
>>   drivers/thunderbolt/usb4.c    | 19 +++++++++++++++++++
>>   4 files changed, 26 insertions(+)
> 
> Any specific reason these were not tagged for stable backports?
> 
> thanks,
> 
> greg k-h


No; no particular reason.  If Mika agrees with them, I think they are 
good candidates to include for stable too.  If I respin them, I'll tag 
accordingly.  If Mika picks them up as v1 I think he can add the tag 
when committing them.

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

[Mika Westerberg]

Hi Mario,

On Wed, Sep 21, 2022 at 09:54:33AM -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".
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  drivers/thunderbolt/switch.c  |  4 ++++
>  drivers/thunderbolt/tb.h      |  1 +
>  drivers/thunderbolt/tb_regs.h |  1 +
>  drivers/thunderbolt/usb4.c    | 19 +++++++++++++++++++
>  4 files changed, 25 insertions(+)
> 
> diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
> index 77d7f07ca075..65f65cfe1101 100644
> --- a/drivers/thunderbolt/switch.c
> +++ b/drivers/thunderbolt/switch.c
> @@ -792,6 +792,10 @@ static int tb_init_port(struct tb_port *port)
>  		(port->config.nfc_credits & ADP_CS_4_TOTAL_BUFFERS_MASK) >>
>  		ADP_CS_4_TOTAL_BUFFERS_SHIFT;
>  
> +	res = usb4_enable_hotplug(port);

Is this something that needs to be done only for the lane adapters? And
not say PCIe or USB adapter?

> +	if (res)
> +		return res;
> +
>  	tb_dump_port(port->sw->tb, port);
>  	return 0;
>  }
> diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
> index 5db76de40cc1..b66781ae9c16 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_enable_hotplug(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..40ba14e8e227 100644
> --- a/drivers/thunderbolt/usb4.c
> +++ b/drivers/thunderbolt/usb4.c
> @@ -1046,6 +1046,25 @@ int usb4_port_unlock(struct tb_port *port)
>  	return tb_port_write(port, &val, TB_CFG_PORT, ADP_CS_4, 1);
>  }
>  
> +/**
> + * usb4_enable_hotplug() - Enables hotplug for a port
> + * @port: USB4 port to operate on
> + *
> + * Enables hot plug events on a given port.
> + */
> +int usb4_enable_hotplug(struct tb_port *port)

usb4_port_hotplug_enable()

> +{
> +	int ret;
> +	u32 val;
> +

And this one needs to check if this is suitable adapter or if not then
at least mention it in the documentation.

> +	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

Re: [PATCH 0/2] Fix some hotplug event issues

[Mika Westerberg]

Hi Mario,

On Wed, Sep 21, 2022 at 10:48:17AM -0500, Limonciello, Mario wrote:
> On 9/21/2022 10:45, Greg KH wrote:
> > On Wed, Sep 21, 2022 at 09:54:31AM -0500, Mario Limonciello wrote:
> > > On AMD Pink Sardine it's been observed that if another CM has run before
> > > the Linux CM that some registers may not match expectations which leads
> > > to a non-functional hotplug for PCIe devices.
> > > 
> > > Explicitly set these registers that have been observed to cause problems
> > > with hotplug to the defaults of the USB4 spec.
> > > 
> > > Mario Limonciello (2):
> > >    thunderbolt: Explicitly reset plug events delay back to USB4 spec
> > >      value
> > >    thunderbolt: Explicitly enable lane adapter hotplug events at startup
> > > 
> > >   drivers/thunderbolt/switch.c  |  5 +++++
> > >   drivers/thunderbolt/tb.h      |  1 +
> > >   drivers/thunderbolt/tb_regs.h |  1 +
> > >   drivers/thunderbolt/usb4.c    | 19 +++++++++++++++++++
> > >   4 files changed, 26 insertions(+)
> > 
> > Any specific reason these were not tagged for stable backports?
> > 
> > thanks,
> > 
> > greg k-h
> 
> 
> No; no particular reason.  If Mika agrees with them, I think they are good
> candidates to include for stable too.  If I respin them, I'll tag
> accordingly.  If Mika picks them up as v1 I think he can add the tag when
> committing them.

I applied the first one to my fixes branch and tagged it for stable. For
the second one, I have a couple of comments that I would like you to
address (sent in a separate email) ;-)