[PATCH] thunderbolt: Disable CLx state for AMD Yellow Carp and Pink Sardine

[PATCH] thunderbolt: Disable CLx state for AMD Yellow Carp and Pink Sardine

[Sanjay R Mehta]

From: Sanjay R Mehta <sanju.mehta@amd.com>

AMD Yellow Carp and Pink Sardine don't support CLx state,
hence disabling it.

Signed-off-by: Sanjay R Mehta <sanju.mehta@amd.com>
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
 drivers/thunderbolt/nhi.h    |  6 ++++++
 drivers/thunderbolt/switch.c |  6 ++++++
 drivers/thunderbolt/tb.h     | 24 ++++++++++++++++++++++++
 3 files changed, 36 insertions(+)

diff --git a/drivers/thunderbolt/nhi.h b/drivers/thunderbolt/nhi.h
index b071802..3d8cfaf 100644
--- a/drivers/thunderbolt/nhi.h
+++ b/drivers/thunderbolt/nhi.h
@@ -87,6 +87,12 @@ extern const struct tb_nhi_ops icl_nhi_ops;
 #define PCI_DEVICE_ID_INTEL_RPL_NHI0			0xa73e
 #define PCI_DEVICE_ID_INTEL_RPL_NHI1			0xa76d
 
+/* PCI IDs for AMD USB4 controllers */
+#define PCI_DEVICE_ID_AMD_YELLOW_CARP_NHI0		0x162e
+#define PCI_DEVICE_ID_AMD_YELLOW_CARP_NHI1		0x162f
+#define PCI_DEVICE_ID_AMD_PINK_SARDINE_NHI0		0x1668
+#define PCI_DEVICE_ID_AMD_PINK_SARDINE_NHI1		0x1669
+
 #define PCI_CLASS_SERIAL_USB_USB4			0x0c0340
 
 #endif
diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 363d712..93e4788 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -3574,6 +3574,12 @@ int tb_switch_enable_clx(struct tb_switch *sw, enum tb_clx clx)
 	if (root_sw->generation < 4 || tb_switch_is_tiger_lake(root_sw))
 		return 0;
 
+	/*
+	 * Disabling CLx by default on AMD USB4 platforms for Yellow Carp and Pink Sardine.
+	 */
+	if (tb_switch_is_yellow_carp(sw->tb->nhi) || tb_switch_is_pink_sardine(sw->tb->nhi))
+		return 0;
+
 	switch (clx) {
 	case TB_CL1:
 		/* CL0s and CL1 are enabled and supported together */
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index f978697..c63a023 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -905,6 +905,30 @@ static inline bool tb_switch_is_tiger_lake(const struct tb_switch *sw)
 	return false;
 }
 
+static inline bool tb_switch_is_yellow_carp(const struct tb_nhi *nhi)
+{
+	if (nhi->pdev->vendor == PCI_VENDOR_ID_AMD) {
+		switch (nhi->pdev->device) {
+		case PCI_DEVICE_ID_AMD_YELLOW_CARP_NHI0:
+		case PCI_DEVICE_ID_AMD_YELLOW_CARP_NHI1:
+			return true;
+		}
+	}
+	return false;
+}
+
+static inline bool tb_switch_is_pink_sardine(const struct tb_nhi *nhi)
+{
+	if (nhi->pdev->vendor == PCI_VENDOR_ID_AMD) {
+		switch (nhi->pdev->device) {
+		case PCI_DEVICE_ID_AMD_PINK_SARDINE_NHI0:
+		case PCI_DEVICE_ID_AMD_PINK_SARDINE_NHI1:
+			return true;
+		}
+	}
+	return false;
+}
+
 /**
  * tb_switch_is_usb4() - Is the switch USB4 compliant
  * @sw: Switch to check
-- 
2.7.4

Re: [PATCH] thunderbolt: Disable CLx state for AMD Yellow Carp and Pink Sardine

[Mika Westerberg]

Hi,

On Fri, Feb 10, 2023 at 07:32:23AM -0600, Sanjay R Mehta wrote:
> From: Sanjay R Mehta <sanju.mehta@amd.com>
> 
> AMD Yellow Carp and Pink Sardine don't support CLx state,
> hence disabling it.

The lane adapters are supposed to announce whether CL-states are
supported or not. Is that not the case with the AMD hardware?

Re: [PATCH] thunderbolt: Disable CLx state for AMD Yellow Carp and Pink Sardine

[Sanjay R Mehta]

On 2/10/2023 7:25 PM, Mika Westerberg wrote:
> Hi,
> 
> On Fri, Feb 10, 2023 at 07:32:23AM -0600, Sanjay R Mehta wrote:
>> From: Sanjay R Mehta <sanju.mehta@amd.com>
>>
>> AMD Yellow Carp and Pink Sardine don't support CLx state,
>> hence disabling it.
> 
> The lane adapters are supposed to announce whether CL-states are
> supported or not. Is that not the case with the AMD hardware?

Yes Mika. it doesn't work for AMD hardware.

- Sanjay

Re: [PATCH] thunderbolt: Disable CLx state for AMD Yellow Carp and Pink Sardine

[Mika Westerberg]

On Fri, Feb 10, 2023 at 07:32:13PM +0530, Sanjay R Mehta wrote:
> 
> 
> On 2/10/2023 7:25 PM, Mika Westerberg wrote:
> > Hi,
> > 
> > On Fri, Feb 10, 2023 at 07:32:23AM -0600, Sanjay R Mehta wrote:
> >> From: Sanjay R Mehta <sanju.mehta@amd.com>
> >>
> >> AMD Yellow Carp and Pink Sardine don't support CLx state,
> >> hence disabling it.
> > 
> > The lane adapters are supposed to announce whether CL-states are
> > supported or not. Is that not the case with the AMD hardware?
> 
> Yes Mika. it doesn't work for AMD hardware.

:-(

Okay can you then add a quirk for this to quirks.c?

Re: [PATCH] thunderbolt: Disable CLx state for AMD Yellow Carp and Pink Sardine

[Sanjay R Mehta]

On 2/10/2023 7:47 PM, Mika Westerberg wrote:
> On Fri, Feb 10, 2023 at 07:32:13PM +0530, Sanjay R Mehta wrote:
>>
>>
>> On 2/10/2023 7:25 PM, Mika Westerberg wrote:
>>> Hi,
>>>
>>> On Fri, Feb 10, 2023 at 07:32:23AM -0600, Sanjay R Mehta wrote:
>>>> From: Sanjay R Mehta <sanju.mehta@amd.com>
>>>>
>>>> AMD Yellow Carp and Pink Sardine don't support CLx state,
>>>> hence disabling it.
>>>
>>> The lane adapters are supposed to announce whether CL-states are
>>> supported or not. Is that not the case with the AMD hardware?
>>
>> Yes Mika. it doesn't work for AMD hardware.
> 
> :-(
> 
> Okay can you then add a quirk for this to quirks.c?

Did you meant like below, is this fine?

diff --git a/drivers/thunderbolt/nhi.h b/drivers/thunderbolt/nhi.h
index b071802..138c649 100644
--- a/drivers/thunderbolt/nhi.h
+++ b/drivers/thunderbolt/nhi.h
@@ -49,6 +49,7 @@ struct tb_nhi_ops {
 };

 extern const struct tb_nhi_ops icl_nhi_ops;
+extern bool clx_enabled;

......
......

diff --git a/drivers/thunderbolt/quirks.c b/drivers/thunderbolt/quirks.c
index b5f2ec7..9ceef7c 100644
--- a/drivers/thunderbolt/quirks.c
+++ b/drivers/thunderbolt/quirks.c
@@ -63,4 +63,10 @@ void tb_check_quirks(struct tb_switch *sw)

                q->hook(sw);
        }
+
+       /*
+        * CLx is not supported on AMD USB4 Yellow Carp and Pink Sardine
platforms.
+        */
+       if (tb_switch_is_yellow_carp(sw->tb->nhi) ||
tb_switch_is_pink_sardine(sw->tb->nhi))
+               clx_enabled = false;
 }
diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 363d712..d4492b5 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -26,7 +26,7 @@ struct nvm_auth_status {
        u32 status;
 };

-static bool clx_enabled = true;
+bool clx_enabled = true;
 module_param_named(clx, clx_enabled, bool, 0444);
 MODULE_PARM_DESC(clx, "allow low power states on the high-speed lanes
(default: true)");

........
........

Re: [PATCH] thunderbolt: Disable CLx state for AMD Yellow Carp and Pink Sardine

[Mika Westerberg]

On Fri, Feb 10, 2023 at 08:16:48PM +0530, Sanjay R Mehta wrote:
> 
> 
> On 2/10/2023 7:47 PM, Mika Westerberg wrote:
> > On Fri, Feb 10, 2023 at 07:32:13PM +0530, Sanjay R Mehta wrote:
> >>
> >>
> >> On 2/10/2023 7:25 PM, Mika Westerberg wrote:
> >>> Hi,
> >>>
> >>> On Fri, Feb 10, 2023 at 07:32:23AM -0600, Sanjay R Mehta wrote:
> >>>> From: Sanjay R Mehta <sanju.mehta@amd.com>
> >>>>
> >>>> AMD Yellow Carp and Pink Sardine don't support CLx state,
> >>>> hence disabling it.
> >>>
> >>> The lane adapters are supposed to announce whether CL-states are
> >>> supported or not. Is that not the case with the AMD hardware?
> >>
> >> Yes Mika. it doesn't work for AMD hardware.
> > 
> > :-(
> > 
> > Okay can you then add a quirk for this to quirks.c?
> 
> Did you meant like below, is this fine?

I mean add the affected routers to quirks.c that then sets:

	sw->quirks |= QUIRK_NO_CLX;

and check this in tb_switch_clx_is_supported(), if set return false or
something like that.

I know we have bunch of Intel specific quirks littered around the
driver but let's not add more. We should move these too into quirks.c at
some point.