Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 13 Feb 2019 10:30:18 +0200
From:   Mika Westerberg <mika.westerberg@linux.intel.com>
To:     Lukas Wunner <lukas@wunner.de>
Cc:     linux-kernel@vger.kernel.org,
        Michael Jamet <michael.jamet@intel.com>,
        Yehezkel Bernat <YehezkelShB@gmail.com>,
        Andreas Noever <andreas.noever@gmail.com>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Subject: Re: [PATCH v2 16/28] thunderbolt: Discover preboot PCIe paths the
 boot firmware established
Message-ID: <20190213083018.GO7875@lahna.fi.intel.com>
References: <20190206131738.43696-1-mika.westerberg@linux.intel.com>
 <20190206131738.43696-17-mika.westerberg@linux.intel.com>
 <20190212194249.2ftl2ckureow44lo@wunner.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20190212194249.2ftl2ckureow44lo@wunner.de>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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@wunner.de>

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.