On Tue, Aug 6, 2013 at 1:09 AM, Yijing Wang <wangyijing@xxxxxxxxxx> wrote: > v2->v3: Update CC stable tag suggested by Li Zefan. > v1->v2: Update patch log, remove Joe's reported-by, because his problem > was mainly caused by BIOS incorrect setting. But this patch mainly > to fix the bug caused by device hot add. Conservatively, this > version only update the mps problem when hot add. When the device > mps < parent mps found, this patch try to update device mps. > It seems unlikely device mps > parent mps after hot add device. > So we don't care that situation. > > Currently we don't update device's mps vaule when doing > pci device hot-add. The hot-added device's mps will be set > to default value (128B). But the upstream port device's mps > may be larger than 128B which was set by firmware during > system bootup. In this case the new added device may not > work normally. This patch try to update the hot added device > mps euqal to its parent mps, if device mpss < parent mps, > print warning. > > References: https://bugzilla.kernel.org/show_bug.cgi?id=60671 > Reported-by: Yijing Wang <wangyijing@xxxxxxxxxx> > Signed-off-by: Yijing Wang <wangyijing@xxxxxxxxxx> > Cc: Jon Mason <jdmason@xxxxxxxx> > Cc: stable@xxxxxxxxxxxxxxx # 3.4+ > --- > drivers/pci/probe.c | 43 +++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 43 insertions(+), 0 deletions(-) > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index cf57fe7..9b0e634 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -1603,6 +1603,40 @@ static int pcie_bus_configure_set(struct pci_dev *dev, void *data) > return 0; > } > > +static int pcie_bus_update_set(struct pci_dev *dev, void *data) > +{ > + int mps, p_mps; > + > + if (!pci_is_pcie(dev) || !dev->bus->self) > + return 0; > + > + mps = pcie_get_mps(dev); > + p_mps = pcie_get_mps(dev->bus->self); > + > + if (mps < p_mps) > + goto update; It would be more clear to have it return 0 if mps >= p_mps, and not have the goto statement. > + > + return 0; > + > +update: > + /* If current mpss is lager than upstream, use upstream mps to update > + * current mps, otherwise print warning info. > + */ > + if ((128 << dev->pcie_mpss) >= p_mps) > + pcie_write_mps(dev, p_mps); > + else > + dev_warn(&dev->dev, "MPS %d MPSS %d both smaller than upstream MPS %d\n" > + "If necessary, use \"pci=pcie_bus_peer2peer\" boot parameter to avoid this problem\n", > + mps, 128 << dev->pcie_mpss, p_mps); > + return 0; > +} > + > +static void pcie_bus_update_setting(struct pci_bus *bus) > +{ > + if (bus->self->is_hotplug_bridge) > + pci_walk_bus(bus, pcie_bus_update_set, NULL); Why not just call pcie_bus_configure_set(bus->self, &smpss); with smpss of 0? > +} > + > /* pcie_bus_configure_settings requires that pci_walk_bus work in a top-down, > * parents then children fashion. If this changes, then this code will not > * work as designed. > @@ -1614,6 +1648,15 @@ void pcie_bus_configure_settings(struct pci_bus *bus, u8 mpss) > if (!pci_is_pcie(bus->self)) > return; > > + /* Sometimes we should update device mps here, > + * eg. after hot add, device mps value will be > + * set to default(128B), but the upstream port > + * mps value may be larger than 128B, if we do > + * not update the device mps, it maybe can not > + * work normally. This is slightly confusing to me. It would be more clear to say: There are situations (i.e., hot add) where the upstream port might have a larger MPS than the device. In these situations, the port MPS needs to be reconfigured to the lower value or the device will not operate properly. > + */ > + pcie_bus_update_setting(bus); This only seems to be necessary in the "TUNE_OFF" case. It would be best to move it under that, just 2 lines down. > + > if (pcie_bus_config == PCIE_BUS_TUNE_OFF) Perhaps it is time to make "SAFE" the default option, per the discussion at last years PCI mini-summit. Bjorn, thoughts? Thanks, Jon > return; > > -- > 1.7.1 > > -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html