RE: [PATCH 1/1] drivers: misc: Add support for nCipher HSM devices

[Kim, David]

> > +/**
> > + * PCI device ID table.  We use the driver_data field to hold an
> > +index into
> > + * nfp_drvlist, so bear than in mind when editing either.
> > + */
> > +static struct pci_device_id nfp_pci_tbl[] = {
> > +{
> > +PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_21555,
> > +PCI_VENDOR_ID_NCIPHER,
> PCI_SUBSYSTEM_ID_NFAST_REV1, 0,
> > +0, /* Ignore class */
> > +0 /* Index into nfp_drvlist */
>
> Please use the correct PCI_DEVICE() macros, that should work here, right?
>
> And you are grabbing an Intel PCI device id?

We've removed our redefinition of PCI_VENDOR_ID_INTEL and are now using the PCI_DEVICE_SUB macro to define these structs.

>
> And kerneldoc everywhere, your commenting format for all of the code is a
> bit "odd".
>

All the commenting format should be better now.

Thanks,
Dave


David Kim
Senior Software Engineer
Tel: +44 1223 703449

nCipher Security
One Station Square
Cambridge CB1 2GA
United Kingdom