Received-SPF: pass (google.com: domain of linux-kernel+bounces-165787-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Precedence: bulk
MIME-Version: 1.0
References: <CA+Y6NJGz6f8hE4kRDUTGgCj+jddUyHeD_8ocFBkARr7w90jmBw@mail.gmail.com>
 <20240416050353.GI112498@black.fi.intel.com> <CA+Y6NJF6+s5zUZeaWtagpMt8Qu0a1oE+3re3c6EsppH+ZsuMRQ@mail.gmail.com>
 <20240419044945.GR112498@black.fi.intel.com> <CA+Y6NJEpWpfPqHO6=Z1XFCXZDUq1+g6EFryB+Urq1=h0PhT+fg@mail.gmail.com>
 <7d68a112-0f48-46bf-9f6d-d99b88828761@amd.com> <20240423053312.GY112498@black.fi.intel.com>
 <7197b2ce-f815-48a1-a78e-9e139de796b7@amd.com> <20240424085608.GE112498@black.fi.intel.com>
 <CA+Y6NJFyi6e7ype6dTAjxsy5aC80NdVOt+Vg-a0O0y_JsfwSGg@mail.gmail.com> <Zi0VLrvUWH6P1_or@wunner.de>
In-Reply-To: <Zi0VLrvUWH6P1_or@wunner.de>
From: Esther Shimanovich <eshimanovich@chromium.org>
Date: Wed, 1 May 2024 18:23:28 -0400
Message-ID: <CA+Y6NJE8hA+wt+auW1wJBWA6EGMc6CGpmdExr3475E_Yys-Zdw@mail.gmail.com>
Subject: Re: [PATCH v4] PCI: Relabel JHL6540 on Lenovo X1 Carbon 7,8
To: Lukas Wunner <lukas@wunner.de>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>, 
	Mario Limonciello <mario.limonciello@amd.com>, Dmitry Torokhov <dmitry.torokhov@gmail.com>, 
	Bjorn Helgaas <bhelgaas@google.com>, linux-pci@vger.kernel.org, 
	linux-kernel@vger.kernel.org, Rajat Jain <rajatja@google.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Sat, Apr 27, 2024 at 3:17=E2=80=AFAM Lukas Wunner <lukas@wunner.de> wrot=
e:
>
> However that doesn't appear to be sufficient:  I notice that in your
> patch, you're also clearing the external_facing bit on the Root Port
> above the discrete host controller.

Rajat (rajatja@google.com) in an internal review had suggested I add
that, and leave it up to kernel maintainers to decide if it's strictly
necessary.

> Additionally you're marking the bridges leading to the NHI and XHCI
> as fixed.  You're also marking the NHI and XHCI themselves as fixed
> and external_facing.
>
> I just want to confirm whether all of this is actually necessary.
> At least marking the NHI and XHCI as external_facing seems nonsensical.
> I need to know the *minimal* set of attribute changes to fix the problem.

Labeling the NHI and XHCI external-facing was my mistake as I got the
ASCII diagram wrong in the beginning. (NHI and XHCI should not be
shown as connected to the USB-C port).  If you look at my latest draft
of the patch (included in one of my messages in this email chain), you
will see that I ended up removing that mistake.
https://lore.kernel.org/lkml/CA+Y6NJGz6f8hE4kRDUTGgCj+jddUyHeD_8ocFBkARr7w9=
0jmBw@mail.gmail.com/

I labeled the bridges leading to the NHI and XHCI as fixed because
they are technically part of the discrete chip, and fixed. I do
understand that if they were labeled as =E2=80=9Cremovable=E2=80=9D, that *=
might* not
affect anything even though that is not technically true. Happy to
leave that decision up to what you think makes more sense.

> I also need to know exactly what the user-visible issue is and how
> it comes about.  Your commit message merely says "the build-in USB-C
> ports stop working at all".  Does that mean that no driver is bound
> to the NHI or XHCI?

That is correct, when the user-visible issue occurs, no driver is
bound to the NHI and XHCI. The discrete JHL chip is not permitted to
attach to the external-facing root port because of the security
policy, so the NHI and XHCI are not seen by the computer.

When the user connects a non-thunderbolt peripheral to the USB-C port
that is downstream from the JHL chip, nothing happens in the logs, and
the computer does not see the peripheral. However, power chargers
still are able to charge the device through that port.

> The solution implemented by the above-linked branch hinges on the
> NHI driver fixing up the device attributes.  But if the NHI driver
> is not bound, it can't fix up the attributes.

I could try to experiment with your patch, see what happens, and if I
can get around that.

On Sat, Apr 27, 2024 at 11:09=E2=80=AFAM Lukas Wunner <lukas@wunner.de> wro=
te:
>
> On Thu, Apr 25, 2024 at 05:16:24PM -0400, Esther Shimanovich wrote:
> > I did find one example of a docking station that uses the DSL6540
> > chip, which has PCI IDs defined in include/linux/pci_ids.h:
> > #define PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_4C_NHI     0x1577
> > #define PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_4C_BRIDGE  0x1578
> > It seems like it has an NHI, despite being in an external, removable
> > docking station.
>
> Could you provide full output of dmesg and lspci -vvv of a machine
> with this docking station attached?
>
> Perhaps open a bug at bugzilla.kernel.org and attach it there?
>

I don=E2=80=99t have this device available at my office. I just saw that
StarTech sells a universal laptop docking station with chipset-id
Intel - Alpine Ridge DSL6540. Then I looked up the device, and found
it here: https://linux-hardware.org/?id=3Dpci:8086-1577-8086-0000

Therefore, I concluded that the DSL6540 has an NHI component.

If these logs are important, I could probably make a case to purchase
that docking station and get the info that you need. Please let me
know!

> Could you then try the below patch and see if it prevents the
> Thunderbolt driver from binding to the hot-plugged device?

I could give it a try. Thank you!