lkml.org 
[lkml]   [2018]   [May]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 1/2] NTB: Migrate PCI Constants to Cannonical PCI Header
    Hi Doug,

    Thanks for the patches!

    On Thu, May 17, 2018 at 05:00:12AM -0700, dmeyer@gigaio.com wrote:
    > From: Doug Meyer <dmeyer@gigaio.com>
    >
    > This is the first of two patches to implement a PCI quirk which will
    > allow the Switchtec NTB code to work with the IOMMU turned on.
    >
    > Here, the Microsemi Switchtec PCI vendor and device ID constants are
    > moved to the canonical location in pci_ids.h. Also, Microsemi class
    > constants are replaced with the standard PCI usage.
    >
    > Signed-off-by: Doug Meyer <dmeyer@gigaio.com>
    > ---
    > drivers/ntb/hw/mscc/ntb_hw_switchtec.c | 3 ++-
    > drivers/pci/switch/switchtec.c | 15 +++++++--------
    > include/linux/pci_ids.h | 32 ++++++++++++++++++++++++++++++++
    > include/linux/switchtec.h | 4 ----
    > 4 files changed, 41 insertions(+), 13 deletions(-)
    >
    > diff --git a/drivers/ntb/hw/mscc/ntb_hw_switchtec.c b/drivers/ntb/hw/mscc/ntb_hw_switchtec.c
    > index f624ae2..5ee5f40 100644
    > --- a/drivers/ntb/hw/mscc/ntb_hw_switchtec.c
    > +++ b/drivers/ntb/hw/mscc/ntb_hw_switchtec.c
    > @@ -19,6 +19,7 @@
    > #include <linux/kthread.h>
    > #include <linux/interrupt.h>
    > #include <linux/ntb.h>
    > +#include <linux/pci.h>
    >
    > MODULE_DESCRIPTION("Microsemi Switchtec(tm) NTB Driver");
    > MODULE_VERSION("0.1");
    > @@ -1487,7 +1488,7 @@ static int switchtec_ntb_add(struct device *dev,
    >
    > stdev->sndev = NULL;
    >
    > - if (stdev->pdev->class != MICROSEMI_NTB_CLASSCODE)
    > + if (stdev->pdev->class != (PCI_CLASS_BRIDGE_OTHER << 8))
    > return -ENODEV;
    >
    > sndev = kzalloc_node(sizeof(*sndev), GFP_KERNEL, dev_to_node(dev));
    > diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
    > index 47cd0c0..07a03b9 100644
    > --- a/drivers/pci/switch/switchtec.c
    > +++ b/drivers/pci/switch/switchtec.c
    > @@ -1,4 +1,3 @@
    > -// SPDX-License-Identifier: GPL-2.0

    This looks like a mistake? I doubt you intended to remove the SPDX
    header.

    > diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
    > index cc608fc5..7b04ca95 100644
    > --- a/include/linux/pci_ids.h
    > +++ b/include/linux/pci_ids.h
    > @@ -3073,4 +3073,36 @@
    >
    > #define PCI_VENDOR_ID_OCZ 0x1b85
    >
    > +#define PCI_VENDOR_ID_MICROSEMI 0x11f8

    1) Please move the PCI_VENDOR_ID_MICROSEMI definition to keep the file
    sorted by vendor ID. It should end up next to the
    PCI_VENDOR_ID_PMC_Sierra definition, which I guess makes sense
    because it looks like Microsemi acquired PMC-Sierra.

    The XEN and OCZ definitions got added out of sequence. I'll fix
    those separately.

    2) We no longer add device IDs to pci_ids.h (unless they're shared
    between multiple drivers). These don't look shared, so just use
    the raw hex IDs in quirks.c.

    3) Please make the class code changes a separate patch. That makes
    both patches easier to review.

    4) Typo in subject: s/Cannonical/canonical/

    > +#define PCI_DEVICE_ID_MICROSEMI_PFX24XG3 0x8531
    > +#define PCI_DEVICE_ID_MICROSEMI_PFX32XG3 0x8532

    \
     
     \ /
      Last update: 2018-05-17 15:26    [W:2.264 / U:0.088 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site