diff mbox

[v4,00/15] Thunderbolt driver for Apple MacBooks

Message ID 1401580281.7663.14.camel@x230 (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Matthew Garrett May 31, 2014, 11:51 p.m. UTC
On Sat, 2014-05-31 at 16:27 +0200, Andreas Noever wrote:
> Hi
> 
> This is version 4 of my Thunderbolt driver for Apple hardware (see [1] for v3).
> 
> Changes since v3:
>  - Fix typos and style problems caught by Bjorn.
>  - Changed the #ifdef CONFIG_ACPI block to cover the whole pci quirk.

Hi Andreas,

This seems to be working well on my MBP. It appears to broadly work on
my Mac Pro, which has Thunderbolt 2 hardware - I added the PCI ID, and
loading the thunderbolt driver after the device is plugged in works, but
it won't recognise hotplug events. I don't appear to get any interrupts
from the Thunderbolt controller. Any idea what might be happening there?

As far as the quirks go - perhaps something like this would be
reasonable, rather than maintaining a list of machines?

  *
@@ -3041,7 +3012,7 @@ static void
quirk_apple_poweroff_thunderbolt(struct pci_dev *dev)
 {
 	acpi_handle bridge, SXIO, SXFP, SXLV;
 
-	if (!dmi_check_system(apple_thunderbolt_whitelist))
+	if (!dmi_match(DMI_BOARD_VENDOR, "Apple Inc."))
 		return;
 	if (pci_pcie_type(dev) != PCI_EXP_TYPE_UPSTREAM)
 		return;
@@ -3084,7 +3055,7 @@ static void
quirk_apple_wait_for_thunderbolt(struct pci_dev *dev)
 	struct pci_dev *sibling = NULL;
 	struct pci_dev *nhi = NULL;
 
-	if (!dmi_check_system(apple_thunderbolt_whitelist))
+	if (!dmi_match(DMI_BOARD_VENDOR, "Apple Inc."))
 		return;
 	if (pci_pcie_type(dev) != PCI_EXP_TYPE_DOWNSTREAM)
 		return;


-- 
Matthew Garrett <matthew.garrett@nebula.com>

Comments

Andreas Noever June 1, 2014, 10:11 a.m. UTC | #1
On Sun, Jun 1, 2014 at 1:51 AM, Matthew Garrett
<matthew.garrett@nebula.com> wrote:
> On Sat, 2014-05-31 at 16:27 +0200, Andreas Noever wrote:
>> Hi
>>
>> This is version 4 of my Thunderbolt driver for Apple hardware (see [1] for v3).
>>
>> Changes since v3:
>>  - Fix typos and style problems caught by Bjorn.
>>  - Changed the #ifdef CONFIG_ACPI block to cover the whole pci quirk.
>
> Hi Andreas,
>
> This seems to be working well on my MBP. It appears to broadly work on
> my Mac Pro, which has Thunderbolt 2 hardware - I added the PCI ID, and
> loading the thunderbolt driver after the device is plugged in works, but
> it won't recognise hotplug events. I don't appear to get any interrupts
> from the Thunderbolt controller. Any idea what might be happening there?
So the communication with the controller works (dmesg dumps a list of
ports etc.)? Do you get plug events ("resetting error on port ...")?
You could try to play around with tb_plug_events_active, if you want
to experiment. I can also take another look at what OS X does once I
get back to my workstation (when I worked on this part falcon ridge
was not jet released, so maybe they do things differently now).

> As far as the quirks go - perhaps something like this would be
> reasonable, rather than maintaining a list of machines?
I have obtained ACPI dumps from a late 2013 MBP and from a MacPro
(both are falcon ridge devices) and these contain a few firmware
changes. For example SXIO, SXIL and SXLV are gone so the shutdown
quirk will fail. With some luck that means that the shutdown quirk is
no longer required for falcon ridge hardware.

Can you try the following on your MacPro?
 - Boot with your ACPI patch, nothing plugged in, no thunderbolt driver
 - Verify that NHI is present (lspci)
 - Plug in a tb device
 - Suspend and resume
 - Check that the NHI is still responding using lspci -vv
On my cactus ridge MBP the device id of the NHI now reads 0xffff (the
device is gone, Linux just doesn't know). If the NHI is still alive
then they fixed their hardware :)

Andreas


> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index c9d6b90..c3170d4 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -2993,35 +2993,6 @@ DECLARE_PCI_FIXUP_HEADER(0x1814, 0x0601, /*
> Ralink RT2800 802.11n PCI */
>                          quirk_broken_intx_masking);
>
>  #ifdef CONFIG_ACPI
> -/* Apple systems with a Cactus Ridge Thunderbolt controller. */
> -static struct dmi_system_id apple_thunderbolt_whitelist[] = {
> -       {
> -               .matches = {
> -                       DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
> -                       DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro9"),
> -               },
> -       },
> -       {
> -               .matches = {
> -                       DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
> -                       DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro10"),
> -               },
> -       },
> -       {
> -               .matches = {
> -                       DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
> -                       DMI_MATCH(DMI_PRODUCT_NAME, "MacBookAir5"),
> -               },
> -       },
> -       {
> -               .matches = {
> -                       DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
> -                       DMI_MATCH(DMI_PRODUCT_NAME, "MacBookAir6"),
> -               },
> -       },
> -       { }
> -};
> -
>  /*
>   * Apple: Shutdown Cactus Ridge Thunderbolt controller.
>   *
> @@ -3041,7 +3012,7 @@ static void
> quirk_apple_poweroff_thunderbolt(struct pci_dev *dev)
>  {
>         acpi_handle bridge, SXIO, SXFP, SXLV;
>
> -       if (!dmi_check_system(apple_thunderbolt_whitelist))
> +       if (!dmi_match(DMI_BOARD_VENDOR, "Apple Inc."))
>                 return;
>         if (pci_pcie_type(dev) != PCI_EXP_TYPE_UPSTREAM)
>                 return;
> @@ -3084,7 +3055,7 @@ static void
> quirk_apple_wait_for_thunderbolt(struct pci_dev *dev)
>         struct pci_dev *sibling = NULL;
>         struct pci_dev *nhi = NULL;
>
> -       if (!dmi_check_system(apple_thunderbolt_whitelist))
> +       if (!dmi_match(DMI_BOARD_VENDOR, "Apple Inc."))
>                 return;
>         if (pci_pcie_type(dev) != PCI_EXP_TYPE_DOWNSTREAM)
>                 return;
>
>
> --
> Matthew Garrett <matthew.garrett@nebula.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Matthew Garrett June 1, 2014, 4:07 p.m. UTC | #2
On Sun, 2014-06-01 at 12:11 +0200, Andreas Noever wrote:
> On Sun, Jun 1, 2014 at 1:51 AM, Matthew Garrett

> <matthew.garrett@nebula.com> wrote:

> > This seems to be working well on my MBP. It appears to broadly work on

> > my Mac Pro, which has Thunderbolt 2 hardware - I added the PCI ID, and

> > loading the thunderbolt driver after the device is plugged in works, but

> > it won't recognise hotplug events. I don't appear to get any interrupts

> > from the Thunderbolt controller. Any idea what might be happening there?

> So the communication with the controller works (dmesg dumps a list of

> ports etc.)? Do you get plug events ("resetting error on port ...")?

> You could try to play around with tb_plug_events_active, if you want

> to experiment. I can also take another look at what OS X does once I

> get back to my workstation (when I worked on this part falcon ridge

> was not jet released, so maybe they do things differently now).


Yeah, that was it. I'll mail the patch separately.


> > As far as the quirks go - perhaps something like this would be

> > reasonable, rather than maintaining a list of machines?

> I have obtained ACPI dumps from a late 2013 MBP and from a MacPro

> (both are falcon ridge devices) and these contain a few firmware

> changes. For example SXIO, SXIL and SXLV are gone so the shutdown

> quirk will fail. With some luck that means that the shutdown quirk is

> no longer required for falcon ridge hardware.


Yeah, it seems I don't need the suspend quirk - the NHI is still there
without it. I still think we should make the quirk general rather than
tying it to the machines, the worst case is that it'll just do nothing.

-- 
Matthew Garrett <matthew.garrett@nebula.com>
Andreas Noever June 1, 2014, 8:45 p.m. UTC | #3
On Sun, Jun 1, 2014 at 6:07 PM, Matthew Garrett
<matthew.garrett@nebula.com> wrote:
> On Sun, 2014-06-01 at 12:11 +0200, Andreas Noever wrote:
>> On Sun, Jun 1, 2014 at 1:51 AM, Matthew Garrett
>> <matthew.garrett@nebula.com> wrote:
>> > This seems to be working well on my MBP. It appears to broadly work on
>> > my Mac Pro, which has Thunderbolt 2 hardware - I added the PCI ID, and
>> > loading the thunderbolt driver after the device is plugged in works, but
>> > it won't recognise hotplug events. I don't appear to get any interrupts
>> > from the Thunderbolt controller. Any idea what might be happening there?
>> So the communication with the controller works (dmesg dumps a list of
>> ports etc.)? Do you get plug events ("resetting error on port ...")?
>> You could try to play around with tb_plug_events_active, if you want
>> to experiment. I can also take another look at what OS X does once I
>> get back to my workstation (when I worked on this part falcon ridge
>> was not jet released, so maybe they do things differently now).
>
> Yeah, that was it. I'll mail the patch separately.
Great
>
>> > As far as the quirks go - perhaps something like this would be
>> > reasonable, rather than maintaining a list of machines?
>> I have obtained ACPI dumps from a late 2013 MBP and from a MacPro
>> (both are falcon ridge devices) and these contain a few firmware
>> changes. For example SXIO, SXIL and SXLV are gone so the shutdown
>> quirk will fail. With some luck that means that the shutdown quirk is
>> no longer required for falcon ridge hardware.
>
> Yeah, it seems I don't need the suspend quirk - the NHI is still there
> without it. I still think we should make the quirk general rather than
> tying it to the machines, the worst case is that it'll just do nothing.
Ok, agreed. The "wait" quirk has to run on all machines and the other
one will fail if the ACPI methods are not there. Should I resend the
series or just the patch or should I (or do you want to) make a
separate patch?

Andreas
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Matthew Garrett June 1, 2014, 10:02 p.m. UTC | #4
On Sun, 2014-06-01 at 22:45 +0200, Andreas Noever wrote:
> On Sun, Jun 1, 2014 at 6:07 PM, Matthew Garrett

> <matthew.garrett@nebula.com> wrote:

> > Yeah, it seems I don't need the suspend quirk - the NHI is still there

> > without it. I still think we should make the quirk general rather than

> > tying it to the machines, the worst case is that it'll just do nothing.

> Ok, agreed. The "wait" quirk has to run on all machines and the other

> one will fail if the ACPI methods are not there. Should I resend the

> series or just the patch or should I (or do you want to) make a

> separate patch?


Probably best to ask Greg - I'm fine with this stuff going through his
tree (note to Greg: the Apple ACPI patches I just sent need to be merged
before this stuff will work properly)

-- 
Matthew Garrett <matthew.garrett@nebula.com>
Greg KH June 1, 2014, 10:57 p.m. UTC | #5
On Sun, Jun 01, 2014 at 10:02:04PM +0000, Matthew Garrett wrote:
> On Sun, 2014-06-01 at 22:45 +0200, Andreas Noever wrote:
> > On Sun, Jun 1, 2014 at 6:07 PM, Matthew Garrett
> > <matthew.garrett@nebula.com> wrote:
> > > Yeah, it seems I don't need the suspend quirk - the NHI is still there
> > > without it. I still think we should make the quirk general rather than
> > > tying it to the machines, the worst case is that it'll just do nothing.
> > Ok, agreed. The "wait" quirk has to run on all machines and the other
> > one will fail if the ACPI methods are not there. Should I resend the
> > series or just the patch or should I (or do you want to) make a
> > separate patch?
> 
> Probably best to ask Greg - I'm fine with this stuff going through his
> tree (note to Greg: the Apple ACPI patches I just sent need to be merged
> before this stuff will work properly)

Ok, I'll still queue these up, I don't want the to be lost at all, and
will wait for the ACPI patches to land before starting to complain that
things don't work on my box :)

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index c9d6b90..c3170d4 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -2993,35 +2993,6 @@  DECLARE_PCI_FIXUP_HEADER(0x1814, 0x0601, /*
Ralink RT2800 802.11n PCI */
 			 quirk_broken_intx_masking);
 
 #ifdef CONFIG_ACPI
-/* Apple systems with a Cactus Ridge Thunderbolt controller. */
-static struct dmi_system_id apple_thunderbolt_whitelist[] = {
-	{
-		.matches = {
-			DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
-			DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro9"),
-		},
-	},
-	{
-		.matches = {
-			DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
-			DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro10"),
-		},
-	},
-	{
-		.matches = {
-			DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
-			DMI_MATCH(DMI_PRODUCT_NAME, "MacBookAir5"),
-		},
-	},
-	{
-		.matches = {
-			DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
-			DMI_MATCH(DMI_PRODUCT_NAME, "MacBookAir6"),
-		},
-	},
-	{ }
-};
-
 /*
  * Apple: Shutdown Cactus Ridge Thunderbolt controller.