Re: [PATCH v2 16/28] thunderbolt: Discover preboot PCIe paths the boot firmware established

From: Mika Westerberg
Date: Wed Feb 13 2019 - 03:30:24 EST


On Tue, Feb 12, 2019 at 08:42:49PM +0100, Lukas Wunner wrote:
> On Wed, Feb 06, 2019 at 04:17:26PM +0300, Mika Westerberg wrote:
> > +static struct tb_port *tb_port_remote(struct tb_port *port)
> > +{
> > + struct tb_port *remote = port->remote;
> > +
> > + /*
> > + * If we have a dual link, the remote is available through the
> > + * primary link.
> > + */
> > + if (!remote && port->dual_link_port && port->dual_link_port->remote)
> > + return port->dual_link_port->remote->dual_link_port;
> > + return remote;
> > +}
>
> Yet more special-casing for dual-link ports. :-(
>
>
> > + if (tunnel->dst_port->config.type != TB_TYPE_PCIE_UP) {
> > + tb_port_warn(tunnel->dst_port,
> > + "path does not end to a PCIe adapter\n");
>
> Nit: I think the proper proposition is "on" or "at", not "to".
>
> The tunnel discovery algorithm looks solid to me, so:
> Reviewed-by: Lukas Wunner <lukas@xxxxxxxxx>

Thanks!

> When the module is unloaded, tb_stop() currently deactivates all PCI
> tunnels. Is this still a good idea now that tunnels are discovered
> on probe? We could just leave the tunnels in place and rediscover
> them when the module is reloaded. If something was unplugged in the
> meantime, pciehp will have disconnected the devices and we should
> notice on reprobe that certain tunnels cannot be rediscovered, so no
> harm no foul. Thoughts?

I agree it makes sense and that's actually what we already do with the
firmware CM.