[PATCH 1/2] thunderbolt: Fix resume quirk for Falcon Ridge 4C.

[PATCH 1/2] thunderbolt: Fix resume quirk for Falcon Ridge 4C.

[Andreas Noever]

The quirk 'quirk_apple_wait_for_thunderbolt' did not fire on Falcon
Ridge 4C controllers with subdevice/subvendor set to zero. This lead
to lost pci devices on system resume.

Older thunderbolt controllers (pre Falcon Ridge) used the same device id
for bridges and for the controller. On Apple hardware the subvendor- &
subdevice-ids were set for the controller, but not for bridges. So that
is what was used to differentiate between the two. Starting with Falcon
Ridge bridges and controllers received different device ids.
Additionally on some MacBookPro models (but not all) the
subvendor/subdevice was zeroed.

Starting with a42fb351c (thunderbolt: Allow loading of module on recent
Apple MacBooks with thunderbolt 2 controller) the thunderbolt driver
binds to all Falcon Ridge 4C controllers (irregardless of
subvendor/subdevice). The corresponding quirk was not updated.

This commit changes the quirk to check the device class instead of its
subvendor-/subdeviceids. This works for all generations of Thunderbolt
controllers.

Signed-off-by: Andreas Noever <andreas.noever@gmail.com>
---
 drivers/pci/quirks.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index ee72ebe..75b2105 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3326,8 +3326,7 @@ static void quirk_apple_wait_for_thunderbolt(struct pci_dev *dev)
 		    || (nhi->device != PCI_DEVICE_ID_INTEL_LIGHT_RIDGE &&
 			nhi->device != PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_4C &&
 			nhi->device != PCI_DEVICE_ID_INTEL_FALCON_RIDGE_4C_NHI)
-		    || nhi->subsystem_vendor != 0x2222
-		    || nhi->subsystem_device != 0x1111)
+		    || nhi->class != PCI_CLASS_SYSTEM_OTHER << 8)
 		goto out;
 	dev_info(&dev->dev, "quirk: waiting for thunderbolt to reestablish PCI tunnels...\n");
 	device_pm_wait_for_dev(&dev->dev, &nhi->dev);
-- 
2.9.0

[PATCH 2/2] thunderbolt: Add support for INTEL_FALCON_RIDGE_2C controller.

[Andreas Noever]

From: Xavier Gnata <xavier.gnata@gmail.com>

From: Xavier Gnata <xavier.gnata@gmail.com>

Add support to INTEL_FALCON_RIDGE_2C controller and corresponding quirk
to support suspend/resume.
Tested against 4.7 master on a MacBook Air 11" 2015.

Signed-off-by: Andreas Noever <andreas.noever@gmail.com>
---
 drivers/pci/quirks.c      | 4 ++++
 drivers/thunderbolt/nhi.c | 6 ++++++
 2 files changed, 10 insertions(+)

Rebased version of Xavier's patch.

Xavier, could you verify that this still works on your system?

Thanks,
Andreas

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 75b2105..981f17d 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3325,6 +3325,7 @@ static void quirk_apple_wait_for_thunderbolt(struct pci_dev *dev)
 	if (nhi->vendor != PCI_VENDOR_ID_INTEL
 		    || (nhi->device != PCI_DEVICE_ID_INTEL_LIGHT_RIDGE &&
 			nhi->device != PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_4C &&
+			nhi->device != PCI_DEVICE_ID_INTEL_FALCON_RIDGE_2C_NHI &&
 			nhi->device != PCI_DEVICE_ID_INTEL_FALCON_RIDGE_4C_NHI)
 		    || nhi->class != PCI_CLASS_SYSTEM_OTHER << 8)
 		goto out;
@@ -3341,6 +3342,9 @@ DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_INTEL,
 			       PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_4C,
 			       quirk_apple_wait_for_thunderbolt);
 DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_INTEL,
+			       PCI_DEVICE_ID_INTEL_FALCON_RIDGE_2C_BRIDGE,
+			       quirk_apple_wait_for_thunderbolt);
+DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_INTEL,
 			       PCI_DEVICE_ID_INTEL_FALCON_RIDGE_4C_BRIDGE,
 			       quirk_apple_wait_for_thunderbolt);
 #endif
diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
index 9c15344..a8c2041 100644
--- a/drivers/thunderbolt/nhi.c
+++ b/drivers/thunderbolt/nhi.c
@@ -651,6 +651,12 @@ static struct pci_device_id nhi_ids[] = {
 	{
 		.class = PCI_CLASS_SYSTEM_OTHER << 8, .class_mask = ~0,
 		.vendor = PCI_VENDOR_ID_INTEL,
+		.device = PCI_DEVICE_ID_INTEL_FALCON_RIDGE_2C_NHI,
+		.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID,
+	},
+	{
+		.class = PCI_CLASS_SYSTEM_OTHER << 8, .class_mask = ~0,
+		.vendor = PCI_VENDOR_ID_INTEL,
 		.device = PCI_DEVICE_ID_INTEL_FALCON_RIDGE_4C_NHI,
 		.subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID,
 	},
-- 
2.9.0

Re: [PATCH 2/2] thunderbolt: Add support for INTEL_FALCON_RIDGE_2C controller.

[Andreas Noever]

On Wed, Jul 27, 2016 at 10:11 AM, Xavier Gnata <xavier.gnata@gmail.com> wrote:
>
>
> On 26/07/2016 18:40, Andreas Noever wrote:
>>
>> From: Xavier Gnata <xavier.gnata@gmail.com>
>>
>> From: Xavier Gnata <xavier.gnata@gmail.com>
>>
>> Add support to INTEL_FALCON_RIDGE_2C controller and corresponding quirk
>> to support suspend/resume.
>> Tested against 4.7 master on a MacBook Air 11" 2015.
>>
>> Signed-off-by: Andreas Noever <andreas.noever@gmail.com>
>> ---
>>  drivers/pci/quirks.c      | 4 ++++
>>  drivers/thunderbolt/nhi.c | 6 ++++++
>>  2 files changed, 10 insertions(+)
>>
>> Rebased version of Xavier's patch.
>>
>> Xavier, could you verify that this still works on your system?
>>
> It does work on my system.
>
> Thanks,
> Xavier

Cool.

Greg, can we still get these into the current merge window?

Andreas

>
>> Thanks,
>> Andreas
>>
>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
>> index 75b2105..981f17d 100644
>> --- a/drivers/pci/quirks.c
>> +++ b/drivers/pci/quirks.c
>> @@ -3325,6 +3325,7 @@ static void quirk_apple_wait_for_thunderbolt(struct
>> pci_dev *dev)
>>         if (nhi->vendor != PCI_VENDOR_ID_INTEL
>>                     || (nhi->device != PCI_DEVICE_ID_INTEL_LIGHT_RIDGE &&
>>                         nhi->device != PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_4C
>> &&
>> +                       nhi->device !=
>> PCI_DEVICE_ID_INTEL_FALCON_RIDGE_2C_NHI &&
>>                         nhi->device !=
>> PCI_DEVICE_ID_INTEL_FALCON_RIDGE_4C_NHI)
>>                     || nhi->class != PCI_CLASS_SYSTEM_OTHER << 8)
>>                 goto out;
>> @@ -3341,6 +3342,9 @@ DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_INTEL,
>>                                PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_4C,
>>                                quirk_apple_wait_for_thunderbolt);
>>  DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_INTEL,
>> +                              PCI_DEVICE_ID_INTEL_FALCON_RIDGE_2C_BRIDGE,
>> +                              quirk_apple_wait_for_thunderbolt);
>> +DECLARE_PCI_FIXUP_RESUME_EARLY(PCI_VENDOR_ID_INTEL,
>>                                PCI_DEVICE_ID_INTEL_FALCON_RIDGE_4C_BRIDGE,
>>                                quirk_apple_wait_for_thunderbolt);
>>  #endif
>> diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
>> index 9c15344..a8c2041 100644
>> --- a/drivers/thunderbolt/nhi.c
>> +++ b/drivers/thunderbolt/nhi.c
>> @@ -651,6 +651,12 @@ static struct pci_device_id nhi_ids[] = {
>>         {
>>                 .class = PCI_CLASS_SYSTEM_OTHER << 8, .class_mask = ~0,
>>                 .vendor = PCI_VENDOR_ID_INTEL,
>> +               .device = PCI_DEVICE_ID_INTEL_FALCON_RIDGE_2C_NHI,
>> +               .subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID,
>> +       },
>> +       {
>> +               .class = PCI_CLASS_SYSTEM_OTHER << 8, .class_mask = ~0,
>> +               .vendor = PCI_VENDOR_ID_INTEL,
>>                 .device = PCI_DEVICE_ID_INTEL_FALCON_RIDGE_4C_NHI,
>>                 .subvendor = PCI_ANY_ID, .subdevice = PCI_ANY_ID,
>>         },
>>
>

Re: [PATCH 2/2] thunderbolt: Add support for INTEL_FALCON_RIDGE_2C controller.

[Greg KH]

On Thu, Jul 28, 2016 at 12:08:42AM +0200, Andreas Noever wrote:
> On Wed, Jul 27, 2016 at 10:11 AM, Xavier Gnata <xavier.gnata@gmail.com> wrote:
> >
> >
> > On 26/07/2016 18:40, Andreas Noever wrote:
> >>
> >> From: Xavier Gnata <xavier.gnata@gmail.com>
> >>
> >> From: Xavier Gnata <xavier.gnata@gmail.com>
> >>
> >> Add support to INTEL_FALCON_RIDGE_2C controller and corresponding quirk
> >> to support suspend/resume.
> >> Tested against 4.7 master on a MacBook Air 11" 2015.
> >>
> >> Signed-off-by: Andreas Noever <andreas.noever@gmail.com>
> >> ---
> >>  drivers/pci/quirks.c      | 4 ++++
> >>  drivers/thunderbolt/nhi.c | 6 ++++++
> >>  2 files changed, 10 insertions(+)
> >>
> >> Rebased version of Xavier's patch.
> >>
> >> Xavier, could you verify that this still works on your system?
> >>
> > It does work on my system.
> >
> > Thanks,
> > Xavier
> 
> Cool.
> 
> Greg, can we still get these into the current merge window?

Hah, no, trees closed a week or so ago.  This is all for 4.9, sorry.  My
trees will open up after 4.8-rc1 is out and I'll start working on
reviewing patches then.

thanks,

greg k-h

Re: [PATCH 1/2] thunderbolt: Fix resume quirk for Falcon Ridge 4C.

[Lukas Wunner]

On Tue, Jul 26, 2016 at 06:40:37PM +0200, Andreas Noever wrote:
> The quirk 'quirk_apple_wait_for_thunderbolt' did not fire on Falcon
> Ridge 4C controllers with subdevice/subvendor set to zero. This lead
> to lost pci devices on system resume.
> 
> Older thunderbolt controllers (pre Falcon Ridge) used the same device id
> for bridges and for the controller. On Apple hardware the subvendor- &
> subdevice-ids were set for the controller, but not for bridges. So that
> is what was used to differentiate between the two. Starting with Falcon
> Ridge bridges and controllers received different device ids.
> Additionally on some MacBookPro models (but not all) the
> subvendor/subdevice was zeroed.
> 
> Starting with a42fb351c (thunderbolt: Allow loading of module on recent
> Apple MacBooks with thunderbolt 2 controller) the thunderbolt driver
> binds to all Falcon Ridge 4C controllers (irregardless of
> subvendor/subdevice). The corresponding quirk was not updated.
> 
> This commit changes the quirk to check the device class instead of its
> subvendor-/subdeviceids. This works for all generations of Thunderbolt
> controllers.
> 
> Signed-off-by: Andreas Noever <andreas.noever@gmail.com>

FWIW, this is
Reviewed-by: Lukas Wunner <lukas@wunner.de>

I also tested it successfully on Light Ridge. I'm sending a follow-up
patch separately so that Falcon Ridge chips are no longer declared
unsupported.

Thanks,

Lukas

> ---
>  drivers/pci/quirks.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index ee72ebe..75b2105 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -3326,8 +3326,7 @@ static void quirk_apple_wait_for_thunderbolt(struct pci_dev *dev)
>  		    || (nhi->device != PCI_DEVICE_ID_INTEL_LIGHT_RIDGE &&
>  			nhi->device != PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_4C &&
>  			nhi->device != PCI_DEVICE_ID_INTEL_FALCON_RIDGE_4C_NHI)
> -		    || nhi->subsystem_vendor != 0x2222
> -		    || nhi->subsystem_device != 0x1111)
> +		    || nhi->class != PCI_CLASS_SYSTEM_OTHER << 8)
>  		goto out;
>  	dev_info(&dev->dev, "quirk: waiting for thunderbolt to reestablish PCI tunnels...\n");
>  	device_pm_wait_for_dev(&dev->dev, &nhi->dev);
> -- 
> 2.9.0

[PATCH] thunderbolt: Don't declare Falcon Ridge unsupported

[Lukas Wunner]

Falcon Ridge 4C has been supported by the driver from the beginning,
Falcon Ridge 2C support was just added. Don't irritate users with a
warning declaring the opposite.

Cc: Andreas Noever <andreas.noever@gmail.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/thunderbolt/switch.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 1e116f5..9840fde 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -372,7 +372,9 @@ struct tb_switch *tb_switch_alloc(struct tb *tb, u64 route)
 
 	if (sw->config.device_id != PCI_DEVICE_ID_INTEL_LIGHT_RIDGE &&
 	    sw->config.device_id != PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_4C &&
-	    sw->config.device_id != PCI_DEVICE_ID_INTEL_PORT_RIDGE)
+	    sw->config.device_id != PCI_DEVICE_ID_INTEL_PORT_RIDGE &&
+	    sw->config.device_id != PCI_DEVICE_ID_INTEL_FALCON_RIDGE_2C_BRIDGE &&
+	    sw->config.device_id != PCI_DEVICE_ID_INTEL_FALCON_RIDGE_4C_BRIDGE)
 		tb_sw_warn(sw, "unsupported switch device id %#x\n",
 			   sw->config.device_id);
 
-- 
2.8.1

Re: [PATCH] thunderbolt: Don't declare Falcon Ridge unsupported

[Andreas Noever]

On Wed, Aug 3, 2016 at 10:44 AM, Lukas Wunner <lukas@wunner.de> wrote:
> Falcon Ridge 4C has been supported by the driver from the beginning,
> Falcon Ridge 2C support was just added. Don't irritate users with a
> warning declaring the opposite.
>
> Cc: Andreas Noever <andreas.noever@gmail.com>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Andreas Noever <andreas.noever@gmail.com>

> ---
>  drivers/thunderbolt/switch.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
> index 1e116f5..9840fde 100644
> --- a/drivers/thunderbolt/switch.c
> +++ b/drivers/thunderbolt/switch.c
> @@ -372,7 +372,9 @@ struct tb_switch *tb_switch_alloc(struct tb *tb, u64 route)
>
>         if (sw->config.device_id != PCI_DEVICE_ID_INTEL_LIGHT_RIDGE &&
>             sw->config.device_id != PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_4C &&
> -           sw->config.device_id != PCI_DEVICE_ID_INTEL_PORT_RIDGE)
> +           sw->config.device_id != PCI_DEVICE_ID_INTEL_PORT_RIDGE &&
> +           sw->config.device_id != PCI_DEVICE_ID_INTEL_FALCON_RIDGE_2C_BRIDGE &&
> +           sw->config.device_id != PCI_DEVICE_ID_INTEL_FALCON_RIDGE_4C_BRIDGE)
>                 tb_sw_warn(sw, "unsupported switch device id %#x\n",
>                            sw->config.device_id);
>
> --
> 2.8.1
>