Roland Dreier wrote:
> > +#define myri10ge_pio_copy(to,from,size) __iowrite64_copy(to,from,size/8)
>
> Why do you need this wrapper? Why not just call __iowrite64_copy()
> without the obfuscation? Anyone reading the code will just have to
> search back to this define and mentally translate the size back and
> forth all the time.
>
Well, I know that abstraction layer is bad. But in this case I really
think that a name like myri10ge_pio_copy(size) is way less obfuscating
than __iowrite64_copy(size/8).
Will fix it if it really matters.
> > +int myri10ge_hyper_msi_cap_on(struct pci_dev *pdev)
> > +{
> > + uint8_t cap_off;
> > + int nbcap = 0;
> > +
> > + cap_off = PCI_CAPABILITY_LIST - 1;
> > + /* go through all caps looking for a hypertransport msi mapping */
>
> This looks like something that should be fixed up in the general PCI
> quirk handling rather than in every driver...
>
> > +static int
> > +myri10ge_use_msi(struct pci_dev *pdev)
> > +{
> > + if (myri10ge_msi == 1 || myri10ge_msi == 0)
> > + return myri10ge_msi;
> > +
> > + /* find root complex for our device */
> > + while (pdev->bus && pdev->bus->self) {
> > + pdev = pdev->bus->self;
> > + }
>
> Similarly looks like generic PCI code (if it's needed at all). If I
> understand correctly you're trying to check if MSI has a chance at
> working on the system, but a network device driver has no business
> walking up the PCI hierarchy.
>
Right, I will look at moving all this to the core PCI code.
Thanks for all the comments.
Brice
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
