From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from szxga02-in.huawei.com ([119.145.14.65]:2466 "EHLO szxga02-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750852AbaG3IPj (ORCPT ); Wed, 30 Jul 2014 04:15:39 -0400 Message-ID: <53D8A942.80008@huawei.com> Date: Wed, 30 Jul 2014 16:13:54 +0800 From: Yijing Wang MIME-Version: 1.0 To: Ethan Zhao CC: Bjorn Helgaas , linux-pci , Jordan Hargrave , "" , Jon Mason Subject: Re: [PATCH] PCI: update device mps when doing pci hotplug References: <1406621877-12022-1-git-send-email-wangyijing@huawei.com> <53D89754.3010704@huawei.com> In-Reply-To: Content-Type: text/plain; charset="UTF-8" Sender: linux-pci-owner@vger.kernel.org List-ID: >> Yes, that issue is BIOS bug, the mps setting is wrong after system boot up. >> But that issue is not same as this one, Keith and Jordan found the issue >> after hot-plug. And my patch only modify the hotplug slot connected device. >> >> In my idea, make the device work is important, because these platforms with windows >> can run happy, why linux leave this issue to BIOS. > > That is a reason to make it works with Linux, but does your > platform have _HPX from ACPI for those hot-added back devices ? if it > has, maybe windows could apply _HPX to configure those devices and > work well. > I checked DSDT table exported from my server, but no "_HPX" found. Further more, kernel use pciehp first to support pcie hotplug device. And in pciehp, driver won't touch ACPI methods like "_HPX". Thanks! Yijing. >> >>> >>>> Reported-by: Keith Busch >>>> Reported-by: Jordan_Hargrave@Dell.com >>>> Reported-by: Yijing Wang >>>> Signed-off-by: Yijing Wang >>>> Cc: Jon Mason >>>> --- >>>> drivers/pci/probe.c | 39 +++++++++++++++++++++++++++++++++++++++ >>>> 1 files changed, 39 insertions(+), 0 deletions(-) >>>> >>>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c >>>> index e3cf8a2..583ca52 100644 >>>> --- a/drivers/pci/probe.c >>>> +++ b/drivers/pci/probe.c >>>> @@ -1613,6 +1613,44 @@ static void pcie_write_mrrs(struct pci_dev *dev) >>>> dev_err(&dev->dev, "MRRS was unable to be configured with a safe value. If problems are experienced, try running with pci=pcie_bus_safe\n"); >>>> } >>>> >>>> +/** >>>> + * pcie_bus_update_set - update device mps when device doing hot-add >>>> + * @dev: PCI device to set >>>> + * >>>> + * After device hot add, mps will be set to default(128B), But the >>>> + * upstream port device's mps may be larger than 128B which was set >>>> + * by firmware during system bootup. Then we should update the device >>>> + * mps to equal to its parent mps, Or the device can not work normally. >>>> + */ >>>> +static void pcie_bus_update_set(struct pci_dev *dev) >>>> +{ >>>> + int mps, p_mps, mpss; >>>> + struct pci_dev *parent; >>>> + >>>> + if (!pci_is_pcie(dev) || !dev->bus->self >>>> + || !dev->bus->self->is_hotplug_bridge) >>>> + return; >>>> + >>>> + parent = dev->bus->self; >>>> + mps = pcie_get_mps(dev); >>>> + p_mps = pcie_get_mps(parent); >>>> + >>>> + if (mps >= p_mps) >>>> + return; >>>> + >>>> + mpss = 128 << dev->pcie_mpss; >>>> + if (mpss < p_mps) { >>>> + dev_warn(&dev->dev, "MPSS %d smaller than upstream MPS %d\n" >>>> + "If necessary, use \"pci=pcie_bus_safe\" boot parameter to avoid this problem\n", >>>> + mpss, p_mps); >>>> + return; >>>> + } >>>> + >>>> + pcie_write_mps(dev, p_mps); >>>> + dev_info(&dev->dev, "Max Payload Size set to %4d/%4d (was %4d)\n", >>>> + pcie_get_mps(dev), 128 << dev->pcie_mpss, mps); >>>> +} >>>> + >>>> static void pcie_bus_detect_mps(struct pci_dev *dev) >>>> { >>>> struct pci_dev *bridge = dev->bus->self; >>>> @@ -1637,6 +1675,7 @@ static int pcie_bus_configure_set(struct pci_dev *dev, void *data) >>>> return 0; >>>> >>>> if (pcie_bus_config == PCIE_BUS_TUNE_OFF) { >>>> + pcie_bus_update_set(dev); >>>> pcie_bus_detect_mps(dev); >>>> return 0; >>>> } >>>> -- >>>> 1.7.1 >>>> >>>> -- >>>> To unsubscribe from this list: send the line "unsubscribe linux-pci" in >>>> the body of a message to majordomo@vger.kernel.org >>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> >>> . >>> >> >> >> -- >> Thanks! >> Yijing >> > > . > -- Thanks! Yijing