Re: [PATCH 18/24] thunderbolt: Store Thunderbolt generation in the switch structure

[Mika Westerberg]

On Sun, May 21, 2017 at 11:18:01AM +, Bernat, Yehezkel wrote:
> Nothing that I'm aware of.

OK.

> Still, I like the idea of having a table-like construct somewhere in
> the code to centralize the handling of various controller-specific
> info (generation, DMA port, maybe more things in the future). 

Yup, I guess that's something we could try out in the next version of
the series.

Re: [PATCH 18/24] thunderbolt: Store Thunderbolt generation in the switch structure

[Mika Westerberg]

On Sun, May 21, 2017 at 11:18:01AM +, Bernat, Yehezkel wrote:
> Nothing that I'm aware of.

OK.

> Still, I like the idea of having a table-like construct somewhere in
> the code to centralize the handling of various controller-specific
> info (generation, DMA port, maybe more things in the future). 

Yup, I guess that's something we could try out in the next version of
the series.

RE: [PATCH 18/24] thunderbolt: Store Thunderbolt generation in the switch structure

[Bernat, Yehezkel]

> -Original Message-
> From: Mika Westerberg [mailto:mika.westerb...@linux.intel.com]
> Sent: Sunday, May 21, 2017 13:47
> To: Bernat, Yehezkel <yehezkel.ber...@intel.com>
> Cc: Levy, Amir (Jer) <amir.jer.l...@intel.com>; Lukas Wunner
> <lu...@wunner.de>; Greg Kroah-Hartman <gre...@linuxfoundation.org>;
> Andreas Noever <andreas.noe...@gmail.com>; Jamet, Michael
> <michael.ja...@intel.com>; Andy Lutomirski <l...@kernel.org>;
> mario.limoncie...@dell.com; jared.doming...@dell.com; Andy Shevchenko
> <andriy.shevche...@linux.intel.com>; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 18/24] thunderbolt: Store Thunderbolt generation in the
> switch structure
> 
> On Sun, May 21, 2017 at 09:55:55AM +, Bernat, Yehezkel wrote:
> >
> >
> > > -Original Message-
> > > From: Levy, Amir (Jer)
> > > Sent: Sunday, May 21, 2017 11:07
> > > To: Mika Westerberg <mika.westerb...@linux.intel.com>; Lukas Wunner
> > > <lu...@wunner.de>
> > > Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>; Andreas
> Noever
> > > <andreas.noe...@gmail.com>; Jamet, Michael
> > > <michael.ja...@intel.com>; Bernat, Yehezkel
> > > <yehezkel.ber...@intel.com>; Andy Lutomirski <l...@kernel.org>;
> > > mario.limoncie...@dell.com; jared.doming...@dell.com; Andy
> > > Shevchenko <andriy.shevche...@linux.intel.com>;
> > > 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 +, 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?

Nothing that I'm aware of.
Still, I like the idea of having a table-like construct somewhere in the code 
to centralize the handling of various controller-specific info (generation, DMA 
port, maybe more things in the future). 

RE: [PATCH 18/24] thunderbolt: Store Thunderbolt generation in the switch structure

[Bernat, Yehezkel]

> -Original Message-
> From: Mika Westerberg [mailto:mika.westerb...@linux.intel.com]
> Sent: Sunday, May 21, 2017 13:47
> To: Bernat, Yehezkel 
> Cc: Levy, Amir (Jer) ; Lukas Wunner
> ; Greg Kroah-Hartman ;
> Andreas Noever ; Jamet, Michael
> ; Andy Lutomirski ;
> mario.limoncie...@dell.com; jared.doming...@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 at 09:55:55AM +, 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.limoncie...@dell.com; jared.doming...@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 +, 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?

Nothing that I'm aware of.
Still, I like the idea of having a table-like construct somewhere in the code 
to centralize the handling of various controller-specific info (generation, DMA 
port, maybe more things in the future). 

Re: [PATCH 18/24] thunderbolt: Store Thunderbolt generation in the switch structure

[Mika Westerberg]

On Sun, May 21, 2017 at 09:55:55AM +, Bernat, Yehezkel wrote:
> 
> 
> > -Original Message-
> > From: Levy, Amir (Jer)
> > Sent: Sunday, May 21, 2017 11:07
> > To: Mika Westerberg <mika.westerb...@linux.intel.com>; Lukas Wunner
> > <lu...@wunner.de>
> > Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>; Andreas Noever
> > <andreas.noe...@gmail.com>; Jamet, Michael <michael.ja...@intel.com>;
> > Bernat, Yehezkel <yehezkel.ber...@intel.com>; Andy Lutomirski
> > <l...@kernel.org>; mario.limoncie...@dell.com;
> > jared.doming...@dell.com; Andy Shevchenko
> > <andriy.shevche...@linux.intel.com>; 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 +, 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?

Re: [PATCH 18/24] thunderbolt: Store Thunderbolt generation in the switch structure

[Mika Westerberg]

On Sun, May 21, 2017 at 09:55:55AM +, 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.limoncie...@dell.com;
> > jared.doming...@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 +, 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?

Re: [PATCH 18/24] thunderbolt: Store Thunderbolt generation in the switch structure

[Mika Westerberg]

On Sun, May 21, 2017 at 08:07:08AM +, Levy, Amir (Jer) wrote:
> 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 +, 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.

Indeed but what if you have a device with a Port Ridge TBT controller
(switch) connected to the host? Here we want to get the generation of
whatever Thunderbolt switch, not just root switch (where NHI ID would
indeed suffice).

Re: [PATCH 18/24] thunderbolt: Store Thunderbolt generation in the switch structure

[Mika Westerberg]

On Sun, May 21, 2017 at 08:07:08AM +, Levy, Amir (Jer) wrote:
> 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 +, 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.

Indeed but what if you have a device with a Port Ridge TBT controller
(switch) connected to the host? Here we want to get the generation of
whatever Thunderbolt switch, not just root switch (where NHI ID would
indeed suffice).

RE: [PATCH 18/24] thunderbolt: Store Thunderbolt generation in the switch structure

[Bernat, Yehezkel]

> -Original Message-
> From: Levy, Amir (Jer)
> Sent: Sunday, May 21, 2017 11:07
> To: Mika Westerberg <mika.westerb...@linux.intel.com>; Lukas Wunner
> <lu...@wunner.de>
> Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>; Andreas Noever
> <andreas.noe...@gmail.com>; Jamet, Michael <michael.ja...@intel.com>;
> Bernat, Yehezkel <yehezkel.ber...@intel.com>; Andy Lutomirski
> <l...@kernel.org>; mario.limoncie...@dell.com;
> jared.doming...@dell.com; Andy Shevchenko
> <andriy.shevche...@linux.intel.com>; 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 +, 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.

RE: [PATCH 18/24] thunderbolt: Store Thunderbolt generation in the switch structure

[Bernat, Yehezkel]

> -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.limoncie...@dell.com;
> jared.doming...@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 +, 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.

RE: [PATCH 18/24] thunderbolt: Store Thunderbolt generation in the switch structure

[Levy, Amir (Jer)]

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 +, 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.

RE: [PATCH 18/24] thunderbolt: Store Thunderbolt generation in the switch structure

[Levy, Amir (Jer)]

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 +, 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.

Re: [PATCH 18/24] thunderbolt: Store Thunderbolt generation in the switch structure

[Mika Westerberg]

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 +, 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.

Re: [PATCH 18/24] thunderbolt: Store Thunderbolt generation in the switch structure

[Mika Westerberg]

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 +, 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.

Re: [PATCH 18/24] thunderbolt: Store Thunderbolt generation in the switch structure

[Mika Westerberg]

On Sun, May 21, 2017 at 07:35:21AM +0200, Lukas Wunner wrote:
> On Sun, May 21, 2017 at 05:29:47AM +, 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.

Re: [PATCH 18/24] thunderbolt: Store Thunderbolt generation in the switch structure

[Mika Westerberg]

On Sun, May 21, 2017 at 07:35:21AM +0200, Lukas Wunner wrote:
> On Sun, May 21, 2017 at 05:29:47AM +, 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.

Re: [PATCH 18/24] thunderbolt: Store Thunderbolt generation in the switch structure

[Lukas Wunner]

On Sun, May 21, 2017 at 05:29:47AM +, 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.

Thanks,

Lukas

Re: [PATCH 18/24] thunderbolt: Store Thunderbolt generation in the switch structure

[Lukas Wunner]

On Sun, May 21, 2017 at 05:29:47AM +, 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.

Thanks,

Lukas

RE: [PATCH 18/24] thunderbolt: Store Thunderbolt generation in the switch structure

[Levy, Amir (Jer)]

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?
> 
> Thanks,
> 
> Lukas
> 

How about setting information, that isn't available from PCI, in 
pci_device_id.driver_data when initializing nhi_ids[]?

RE: [PATCH 18/24] thunderbolt: Store Thunderbolt generation in the switch structure

[Levy, Amir (Jer)]

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?
> 
> Thanks,
> 
> Lukas
> 

How about setting information, that isn't available from PCI, in 
pci_device_id.driver_data when initializing nhi_ids[]?

Re: [PATCH 18/24] thunderbolt: Store Thunderbolt generation in the switch structure

[Lukas Wunner]

On Thu, May 18, 2017 at 05:39:08PM +0300, Mika Westerberg wrote:
> In some cases it is useful to know what is the Thunderbolt generation
> the switch supports. This introduces a new field to struct switch that
> stores the generation of the switch based on the device ID.
> 
> Signed-off-by: Mika Westerberg 
> Reviewed-by: Yehezkel Bernat 
> Reviewed-by: Michael Jamet 
> ---
>  drivers/thunderbolt/switch.c | 24 
>  drivers/thunderbolt/tb.h |  2 ++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
> index 396e00ab7723..9c91d397d3b3 100644
> --- a/drivers/thunderbolt/switch.c
> +++ b/drivers/thunderbolt/switch.c
> @@ -382,6 +382,28 @@ struct device_type tb_switch_type = {
>   .release = tb_switch_release,
>  };
>  
> +static void tb_switch_set_generation(struct tb_switch *sw)
> +{
> + switch (sw->config.device_id) {
> + case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_LP_BRIDGE:
> + case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_2C_BRIDGE:
> + case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_4C_BRIDGE:
> + case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_2C_BRIDGE:
> + case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_4C_BRIDGE:
> + sw->generation = 3;
> + break;
> +
> + case PCI_DEVICE_ID_INTEL_FALCON_RIDGE_2C_BRIDGE:
> + case PCI_DEVICE_ID_INTEL_FALCON_RIDGE_4C_BRIDGE:
> + sw->generation = 2;
> + break;

This is missing PCI_DEVICE_ID_INTEL_WIN_RIDGE_2C_BRIDGE (0x157e).


> +
> + 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?

Thanks,

Lukas

> + }
> +}
> +
>  /**
>   * tb_switch_alloc() - allocate a switch
>   * @tb: Pointer to the owning domain
> @@ -442,6 +464,8 @@ struct tb_switch *tb_switch_alloc(struct tb *tb, struct 
> device *parent,
>   }
>   sw->cap_plug_events = cap;
>  
> + tb_switch_set_generation(sw);
> +
>   device_initialize(>dev);
>   sw->dev.parent = parent;
>   sw->dev.bus = _bus_type;
> diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
> index 0be989069941..b3cda7605619 100644
> --- a/drivers/thunderbolt/tb.h
> +++ b/drivers/thunderbolt/tb.h
> @@ -25,6 +25,7 @@
>   * @device: Device ID of the switch
>   * @vendor_name: Name of the vendor (or %NULL if not known)
>   * @device_name: Name of the device (or %NULL if not known)
> + * @generation: Switch Thunderbolt generation
>   * @cap_plug_events: Offset to the plug events capability (%0 if not found)
>   * @is_unplugged: The switch is going away
>   * @drom: DROM of the switch (%NULL if not found)
> @@ -40,6 +41,7 @@ struct tb_switch {
>   u16 device;
>   const char *vendor_name;
>   const char *device_name;
> + unsigned int generation;
>   int cap_plug_events;
>   bool is_unplugged;
>   u8 *drom;
> -- 
> 2.11.0
> 

Re: [PATCH 18/24] thunderbolt: Store Thunderbolt generation in the switch structure

[Lukas Wunner]

On Thu, May 18, 2017 at 05:39:08PM +0300, Mika Westerberg wrote:
> In some cases it is useful to know what is the Thunderbolt generation
> the switch supports. This introduces a new field to struct switch that
> stores the generation of the switch based on the device ID.
> 
> Signed-off-by: Mika Westerberg 
> Reviewed-by: Yehezkel Bernat 
> Reviewed-by: Michael Jamet 
> ---
>  drivers/thunderbolt/switch.c | 24 
>  drivers/thunderbolt/tb.h |  2 ++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
> index 396e00ab7723..9c91d397d3b3 100644
> --- a/drivers/thunderbolt/switch.c
> +++ b/drivers/thunderbolt/switch.c
> @@ -382,6 +382,28 @@ struct device_type tb_switch_type = {
>   .release = tb_switch_release,
>  };
>  
> +static void tb_switch_set_generation(struct tb_switch *sw)
> +{
> + switch (sw->config.device_id) {
> + case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_LP_BRIDGE:
> + case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_2C_BRIDGE:
> + case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_4C_BRIDGE:
> + case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_2C_BRIDGE:
> + case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_4C_BRIDGE:
> + sw->generation = 3;
> + break;
> +
> + case PCI_DEVICE_ID_INTEL_FALCON_RIDGE_2C_BRIDGE:
> + case PCI_DEVICE_ID_INTEL_FALCON_RIDGE_4C_BRIDGE:
> + sw->generation = 2;
> + break;

This is missing PCI_DEVICE_ID_INTEL_WIN_RIDGE_2C_BRIDGE (0x157e).


> +
> + 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?

Thanks,

Lukas

> + }
> +}
> +
>  /**
>   * tb_switch_alloc() - allocate a switch
>   * @tb: Pointer to the owning domain
> @@ -442,6 +464,8 @@ struct tb_switch *tb_switch_alloc(struct tb *tb, struct 
> device *parent,
>   }
>   sw->cap_plug_events = cap;
>  
> + tb_switch_set_generation(sw);
> +
>   device_initialize(>dev);
>   sw->dev.parent = parent;
>   sw->dev.bus = _bus_type;
> diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
> index 0be989069941..b3cda7605619 100644
> --- a/drivers/thunderbolt/tb.h
> +++ b/drivers/thunderbolt/tb.h
> @@ -25,6 +25,7 @@
>   * @device: Device ID of the switch
>   * @vendor_name: Name of the vendor (or %NULL if not known)
>   * @device_name: Name of the device (or %NULL if not known)
> + * @generation: Switch Thunderbolt generation
>   * @cap_plug_events: Offset to the plug events capability (%0 if not found)
>   * @is_unplugged: The switch is going away
>   * @drom: DROM of the switch (%NULL if not found)
> @@ -40,6 +41,7 @@ struct tb_switch {
>   u16 device;
>   const char *vendor_name;
>   const char *device_name;
> + unsigned int generation;
>   int cap_plug_events;
>   bool is_unplugged;
>   u8 *drom;
> -- 
> 2.11.0
> 

[PATCH 18/24] thunderbolt: Store Thunderbolt generation in the switch structure

[Mika Westerberg]

In some cases it is useful to know what is the Thunderbolt generation
the switch supports. This introduces a new field to struct switch that
stores the generation of the switch based on the device ID.

Signed-off-by: Mika Westerberg 
Reviewed-by: Yehezkel Bernat 
Reviewed-by: Michael Jamet 
---
 drivers/thunderbolt/switch.c | 24 
 drivers/thunderbolt/tb.h |  2 ++
 2 files changed, 26 insertions(+)

diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 396e00ab7723..9c91d397d3b3 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -382,6 +382,28 @@ struct device_type tb_switch_type = {
.release = tb_switch_release,
 };
 
+static void tb_switch_set_generation(struct tb_switch *sw)
+{
+   switch (sw->config.device_id) {
+   case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_LP_BRIDGE:
+   case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_2C_BRIDGE:
+   case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_4C_BRIDGE:
+   case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_2C_BRIDGE:
+   case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_4C_BRIDGE:
+   sw->generation = 3;
+   break;
+
+   case PCI_DEVICE_ID_INTEL_FALCON_RIDGE_2C_BRIDGE:
+   case PCI_DEVICE_ID_INTEL_FALCON_RIDGE_4C_BRIDGE:
+   sw->generation = 2;
+   break;
+
+   default:
+   sw->generation = 1;
+   break;
+   }
+}
+
 /**
  * tb_switch_alloc() - allocate a switch
  * @tb: Pointer to the owning domain
@@ -442,6 +464,8 @@ struct tb_switch *tb_switch_alloc(struct tb *tb, struct 
device *parent,
}
sw->cap_plug_events = cap;
 
+   tb_switch_set_generation(sw);
+
device_initialize(>dev);
sw->dev.parent = parent;
sw->dev.bus = _bus_type;
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 0be989069941..b3cda7605619 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -25,6 +25,7 @@
  * @device: Device ID of the switch
  * @vendor_name: Name of the vendor (or %NULL if not known)
  * @device_name: Name of the device (or %NULL if not known)
+ * @generation: Switch Thunderbolt generation
  * @cap_plug_events: Offset to the plug events capability (%0 if not found)
  * @is_unplugged: The switch is going away
  * @drom: DROM of the switch (%NULL if not found)
@@ -40,6 +41,7 @@ struct tb_switch {
u16 device;
const char *vendor_name;
const char *device_name;
+   unsigned int generation;
int cap_plug_events;
bool is_unplugged;
u8 *drom;
-- 
2.11.0

[PATCH 18/24] thunderbolt: Store Thunderbolt generation in the switch structure

[Mika Westerberg]

In some cases it is useful to know what is the Thunderbolt generation
the switch supports. This introduces a new field to struct switch that
stores the generation of the switch based on the device ID.

Signed-off-by: Mika Westerberg 
Reviewed-by: Yehezkel Bernat 
Reviewed-by: Michael Jamet 
---
 drivers/thunderbolt/switch.c | 24 
 drivers/thunderbolt/tb.h |  2 ++
 2 files changed, 26 insertions(+)

diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 396e00ab7723..9c91d397d3b3 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -382,6 +382,28 @@ struct device_type tb_switch_type = {
.release = tb_switch_release,
 };
 
+static void tb_switch_set_generation(struct tb_switch *sw)
+{
+   switch (sw->config.device_id) {
+   case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_LP_BRIDGE:
+   case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_2C_BRIDGE:
+   case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_4C_BRIDGE:
+   case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_2C_BRIDGE:
+   case PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_4C_BRIDGE:
+   sw->generation = 3;
+   break;
+
+   case PCI_DEVICE_ID_INTEL_FALCON_RIDGE_2C_BRIDGE:
+   case PCI_DEVICE_ID_INTEL_FALCON_RIDGE_4C_BRIDGE:
+   sw->generation = 2;
+   break;
+
+   default:
+   sw->generation = 1;
+   break;
+   }
+}
+
 /**
  * tb_switch_alloc() - allocate a switch
  * @tb: Pointer to the owning domain
@@ -442,6 +464,8 @@ struct tb_switch *tb_switch_alloc(struct tb *tb, struct 
device *parent,
}
sw->cap_plug_events = cap;
 
+   tb_switch_set_generation(sw);
+
device_initialize(>dev);
sw->dev.parent = parent;
sw->dev.bus = _bus_type;
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 0be989069941..b3cda7605619 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -25,6 +25,7 @@
  * @device: Device ID of the switch
  * @vendor_name: Name of the vendor (or %NULL if not known)
  * @device_name: Name of the device (or %NULL if not known)
+ * @generation: Switch Thunderbolt generation
  * @cap_plug_events: Offset to the plug events capability (%0 if not found)
  * @is_unplugged: The switch is going away
  * @drom: DROM of the switch (%NULL if not found)
@@ -40,6 +41,7 @@ struct tb_switch {
u16 device;
const char *vendor_name;
const char *device_name;
+   unsigned int generation;
int cap_plug_events;
bool is_unplugged;
u8 *drom;
-- 
2.11.0