From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756399AbdEUKrY (ORCPT ); Sun, 21 May 2017 06:47:24 -0400 Received: from mga06.intel.com ([134.134.136.31]:8193 "EHLO mga06.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753008AbdEUKrV (ORCPT ); Sun, 21 May 2017 06:47:21 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.38,373,1491289200"; d="scan'208";a="971293462" Date: Sun, 21 May 2017 13:47:14 +0300 From: Mika Westerberg To: "Bernat, Yehezkel" Cc: "Levy, Amir (Jer)" , Lukas Wunner , Greg Kroah-Hartman , Andreas Noever , "Jamet, Michael" , Andy Lutomirski , "Mario.Limonciello@dell.com" , "Jared.Dominguez@dell.com" , Andy Shevchenko , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH 18/24] thunderbolt: Store Thunderbolt generation in the switch structure Message-ID: <20170521104714.GQ8541@lahna.fi.intel.com> References: <20170518143914.60902-1-mika.westerberg@linux.intel.com> <20170518143914.60902-19-mika.westerberg@linux.intel.com> <20170521044735.GA2948@wunner.de> <20170521053521.GB3003@wunner.de> <20170521074041.GK8541@lahna.fi.intel.com> <20170521080016.GN8541@lahna.fi.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo User-Agent: Mutt/1.7.1 (2016-10-04) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, May 21, 2017 at 09:55:55AM +0000, Bernat, Yehezkel wrote: > > > > -----Original Message----- > > From: Levy, Amir (Jer) > > Sent: Sunday, May 21, 2017 11:07 > > To: Mika Westerberg ; Lukas Wunner > > > > Cc: Greg Kroah-Hartman ; Andreas Noever > > ; Jamet, Michael ; > > Bernat, Yehezkel ; Andy Lutomirski > > ; Mario.Limonciello@dell.com; > > Jared.Dominguez@dell.com; Andy Shevchenko > > ; linux-kernel@vger.kernel.org > > Subject: RE: [PATCH 18/24] thunderbolt: Store Thunderbolt generation in the > > switch structure > > > > On Sun, May 21 2017, 11:00 AM, Mika Westerberg wrote: > > > On Sun, May 21, 2017 at 10:40:41AM +0300, Mika Westerberg wrote: > > > > On Sun, May 21, 2017 at 07:35:21AM +0200, Lukas Wunner wrote: > > > > > On Sun, May 21, 2017 at 05:29:47AM +0000, Levy, Amir (Jer) wrote: > > > > > > On Sun, May 21 2017, 07:47 AM, Lukas Wunner wrote: > > > > > > > On Thu, May 18, 2017 at 05:39:08PM +0300, Mika Westerberg wrote: > > > > > > > > + > > > > > > > > + default: > > > > > > > > + sw->generation = 1; > > > > > > > > + break; > > > > > > > > > > > > > > If someone adds an entry for, say, a new TB3 controller to > > > > > > > nhi_ids[] but forgets to update this function, the controller > > > > > > > is assigned the wrong generation number. It might be better > > > > > > > to make TB3 the default and list each TB1 controller instead > > > > > > > since it's less likely for Intel to introduce an older gen chip. > > > > > > > > > > > > > > Generally I think it's problematic to require that multiple > > > > > > > files are touched whenever a new controller is added. Isn't > > > > > > > the generation number or link speed (10/20/40) stored in some > > > > > > > register in PCI config space (VSEC 0x1234) or TB config space? > > > > > > > > > > > > How about setting information, that isn't available from PCI, in > > > > > > pci_device_id.driver_data when initializing nhi_ids[]? > > > > > > > > > > Right, that would also be possible, though reading the generation > > > > > number from a register would be more elegant, if such a register exists. > > > > > > > > I don't think there is such register but I can put this information > > > > to the driver_data instead. > > > > > > Actually these are Thunderbolt switch IDs, not NHI PCI IDs so I don't > > > think driver_data is the right place after all. So if no objections, > > > I'll update the function to default to TBT3 but keep the switch case > > > and add the TBT1 IDs + Win Ridge there. > > > > There is correlation between switch ID to NHI ID. > > I'm not sure defaulting to TBT3 is a good idea. ICM message format can > be changed, DMA port can be different, nothing guaranties correct > operation of the driver with newer unknown controllers. Fair enough :) Then we just need to remember to update the function here as new generations get added and tested. I suppose you don't know either if we could use the revision or similar field in the switch config space to determine generation somehow?