On Mon, 2011-09-26 at 09:40 -0500, Jon Mason wrote:
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 4e84fd4..d369316 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -3203,8 +3203,6 @@ int pcie_set_readrq(struct pci_dev *dev, int rq)
> > if (rq < 128 || rq > 4096 || !is_power_of_2(rq))
> > goto out;
> >
> > - v = (ffs(rq) - 8) << 12;
> > -
> > cap = pci_pcie_cap(dev);
> > if (!cap)
> > goto out;
> > @@ -3212,6 +3210,22 @@ int pcie_set_readrq(struct pci_dev *dev, int rq)
> > err = pci_read_config_word(dev, cap + PCI_EXP_DEVCTL, &ctl);
> > if (err)
> > goto out;
> > + /*
> > + * If using the "performance" PCIe config, we clamp the
> > + * read rq size to the max packet size to prevent the
> > + * host bridge generating requests larger than we can
> > + * cope with
> > + */
> > + if (pcie_bus_config == PCIE_BUS_PERFORMANCE) {
> > + int mps = pcie_get_mps(dev);
> > +
> > + if (mps < 0)
> > + return mps;
>
> Shouldn't this return an error, not the mps? "goto out" is what I was using.
There is no lock to release or anything like that so goto isn't useful,
return is fine. As for mps, well, mps < 0 -is- an error :-) We just pass
up the error code we got from pcie_get_mps()
> > + if (mps < rq)
> > + rq = mps;
>
> I had a dev_warn in there to notify the user.
I'm not sure we really need it but I can add it. Maybe a dev_dbg() tho ?
The "user" in 99% of the time has no idea and there's nothing actually
wrong here.
Cheers,
Ben.
> > + }
> > +
> > + v = (ffs(rq) - 8) << 12;
> >
> > if ((ctl & PCI_EXP_DEVCTL_READRQ) != v) {
> > ctl &= ~PCI_EXP_DEVCTL_READRQ;
> >
> >
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
