linux-kernel - Re: [PATCH 18/18] thunderbolt: Add support for Intel Titan Ridge

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180214142809.GT27191@lahna.fi.intel.com>
Date:   Wed, 14 Feb 2018 16:28:09 +0200
From:   Mika Westerberg <mika.westerberg@...ux.intel.com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Andreas Noever <andreas.noever@...il.com>,
        Michael Jamet <michael.jamet@...el.com>,
        Yehezkel Bernat <yehezkel.bernat@...el.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Mario Limonciello <Mario.Limonciello@...l.com>,
        Radion Mirchevsky <radion.mirchevsky@...el.com>
Subject: Re: [PATCH 18/18] thunderbolt: Add support for Intel Titan Ridge

On Wed, Feb 14, 2018 at 04:23:44PM +0200, Andy Shevchenko wrote:
> On Tue, Feb 13, 2018 at 7:00 PM, Mika Westerberg
> <mika.westerberg@...ux.intel.com> wrote:
> 
> > Intel Titan Ridge is the next Thunderbolt 3 controller. The ICM firmware
> > message format in Titan Ridge differs from Falcon Ridge and Alpine Ridge
> > somewhat because it is using route strings addressing devices. In
> > addition to that the DMA port of 4-channel (two port) controller is in
> > different port number than the previous controllers. There are some
> > other minor differences as well.
> >
> > This patch add support for Intel Titan Ridge and the new ICM firmware
> > message format.
> 
> >  static int dma_find_port(struct tb_switch *sw)
> >  {
> > +       static const int ports[] = { 7, 5, 3 };
> 
> Is it anything special in ordering? Otherwise I would keep it the same
> as in comment below.

I don't think there is anything special in ordering so I can update it
accordingly.

> > +       int i;
> >
> >         /*
> > +        * The DMA (NHI) port is either 3, 5 or 7 depending on the
> > +        * controller. Try all of them.
> >          */
> > +       for (i = 0; i < ARRAY_SIZE(ports); i++) {
> > +               u32 type;
> > +               int ret;
> > +
> > +               ret = dma_port_read(sw->tb->ctl, &type, tb_route(sw), ports[i],
> > +                                   2, 1, DMA_PORT_TIMEOUT);
> > +               if (!ret && (type & 0xffffff) == TB_TYPE_NHI)
> > +                       return ports[i];
> > +       }
> 
> > +static inline u64 get_parent_route(u64 route)
> > +{
> > +       int depth = tb_route_length(route);
> > +       return depth ? route & ~((u64)0xff << (depth - 1) * TB_ROUTE_SHIFT) : 0;
> 
> 0xffULL ?
> 
> > +}
> 
> > +       const struct icm_tr_event_device_connected *pkg =
> > +               (const struct icm_tr_event_device_connected *)hdr;
> 
> > +       const struct icm_tr_event_device_disconnected *pkg =
> > +               (const struct icm_tr_event_device_disconnected *)hdr;
> 
> > +       const struct icm_tr_event_xdomain_connected *pkg =
> > +               (const struct icm_tr_event_xdomain_connected *)hdr;
> 
> > +       const struct icm_tr_event_xdomain_disconnected *pkg =
> > +               (const struct icm_tr_event_xdomain_disconnected *)hdr;
> 
> 
> >  #define PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_LP_USBONLY_NHI        0x15dc
> >  #define PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_USBONLY_NHI   0x15dd
> >  #define PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_USBONLY_NHI 0x15de
> 
> > +#define PCI_DEVICE_ID_INTEL_TITAN_RIDGE_2C_NHI         0x15e8
> > +#define PCI_DEVICE_ID_INTEL_TITAN_RIDGE_2C_BRIDGE      0x15e7
> > +#define PCI_DEVICE_ID_INTEL_TITAN_RIDGE_4C_NHI         0x15eb
> > +#define PCI_DEVICE_ID_INTEL_TITAN_RIDGE_4C_BRIDGE      0x15ea
> > +#define PCI_DEVICE_ID_INTEL_TITAN_RIDGE_DD_BRIDGE      0x15ef
> 
> Can we keep it sorted?

It is sorted by the controller type ;-)

> 
> > +#define ICM_TR_INFO_SLEVEL_MASK                0x7
> 
> GENMASK() ?

OK.

> > +#define ICM_TR_INFO_BOOT_ACL_SHIFT     7
> > +#define ICM_TR_INFO_BOOT_ACL_MASK      GENMASK(12, 7)
> 
> -- 
> With Best Regards,
> Andy Shevchenko