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 <[email protected]>

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.

Reply via email to