Hello,
I tested this today, and it works fine. Apparently It even solves the
problem my systems were facing, thus I do not need my patch anymore!
:-)
Thanks,
Rajat
On Thu, Mar 20, 2014 at 6:30 AM, Rajat Jain <rajatxjain@xxxxxxxxx> wrote:
> Thanks, I'll test it out on my systems today.
>
> On Wed, Mar 19, 2014 at 11:47 PM, Yinghai Lu <yinghai@xxxxxxxxxx> wrote:
>> On Wed, Mar 19, 2014 at 8:16 PM, Yinghai Lu <yinghai@xxxxxxxxxx> wrote:
>>> On Wed, Mar 19, 2014 at 3:01 PM, Bjorn Helgaas <bhelgaas@xxxxxxxxxx> wrote:
>>>> On Wed, Mar 19, 2014 at 12:18 PM, Yinghai Lu <yinghai@xxxxxxxxxx> wrote:
>>>>
>>>> I think the current pciehp design is a bit broken. Today we perform
>>>> commands very synchronously:
>>>>
>>>> pcie_write_cmd() {
>>>> write Slot Control
>>>> if (Command Complete supported) {
>>>> wait for Command Complete
>>>> }
>>>> }
>>>>
>>>> The typical driver pattern would be more asynchronous, like this:
>>>>
>>>> pcie_write_cmd() {
>>>> read Slot Status
>>>> if (!Command Completed) {
>>>> wait for Command Complete
>>>> }
>>>> write Slot Control
>>>> }
>>>>
>>>> With the asynchronous pattern, we would probably never wait at all
>>>> unless we performed two commands in immediate succession. I wouldn't
>>>> be surprised if doing this asynchronously would effectively cover up
>>>> these hardware differences.
>>>
>>> I like this idea. Will give it a try.
>>
>> It works. Please check the patch that check CC before wait/write command.
>>
>> Subject: [PATCH v2] pciehp: only wait command complete for real hotplug control
>>
>> On system with 16 PCI express hotplug slots, customer complain every slot
>> will report "Command not completed in 1000 msec" during initialization.
>>
>> Intel says that we should only wait command complete only for
>> Electromechanical Interlock Control
>> Power Controller Control
>> Power Indicator Control
>> Attention Indicator Control
>>
>> System with AMD/Nvidia chipset have same problem.
>>
>> Two ways to address the problem:
>> 1. change to only wait when (EIC, PCC, PIC, AIC) bits get changed.
>> 2. or check CMD_COMPLETE and wait before write command. Only wait
>> when CC is set and cmd_busy is true. For chipset from intel
>> will only have CC set for real hotplug control, so we could
>> skip the wait for others.
>>
>> This patch is using second way.
>>
>> With that we could save 16 seconds during booting, later with 32 sockets system
>> with 64 pcie hotplug slots we could save 64 seconds.
>>
>> -v2: use second way that is suggested by Bjorn.
>>
>> Signed-off-by: Yinghai Lu <yinghai@xxxxxxxxxx>
>>
>> ---
>> drivers/pci/hotplug/pciehp_hpc.c | 26 ++++++++++----------------
>> 1 file changed, 10 insertions(+), 16 deletions(-)
>>
>> Index: linux-2.6/drivers/pci/hotplug/pciehp_hpc.c
>> ===================================================================
>> --- linux-2.6.orig/drivers/pci/hotplug/pciehp_hpc.c
>> +++ linux-2.6/drivers/pci/hotplug/pciehp_hpc.c
>> @@ -158,15 +158,8 @@ static void pcie_write_cmd(struct contro
>> mutex_lock(&ctrl->ctrl_lock);
>>
>> pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &slot_status);
>> - if (slot_status & PCI_EXP_SLTSTA_CC) {
>> - if (!ctrl->no_cmd_complete) {
>> - /*
>> - * After 1 sec and CMD_COMPLETED still not set, just
>> - * proceed forward to issue the next command according
>> - * to spec. Just print out the error message.
>> - */
>> - ctrl_dbg(ctrl, "CMD_COMPLETED not clear after 1 sec\n");
>> - } else if (!NO_CMD_CMPL(ctrl)) {
>> + if (ctrl->no_cmd_complete && slot_status & PCI_EXP_SLTSTA_CC) {
>> + if (!NO_CMD_CMPL(ctrl)) {
>> /*
>> * This controller seems to notify of command completed
>> * event even though it supports none of power
>> @@ -182,16 +175,11 @@ static void pcie_write_cmd(struct contro
>> }
>>
>> pcie_capability_read_word(pdev, PCI_EXP_SLTCTL, &slot_ctrl);
>> - slot_ctrl &= ~mask;
>> - slot_ctrl |= (cmd & mask);
>> - ctrl->cmd_busy = 1;
>> - smp_mb();
>> - pcie_capability_write_word(pdev, PCI_EXP_SLTCTL, slot_ctrl);
>> -
>> /*
>> * Wait for command completion.
>> */
>> - if (!ctrl->no_cmd_complete) {
>> + if (!ctrl->no_cmd_complete && (slot_status & PCI_EXP_SLTSTA_CC) &&
>> + ctrl->cmd_busy) {
>> int poll = 0;
>> /*
>> * if hotplug interrupt is not enabled or command
>> @@ -203,6 +191,12 @@ static void pcie_write_cmd(struct contro
>> poll = 1;
>> pcie_wait_cmd(ctrl, poll);
>> }
>> +
>> + slot_ctrl &= ~mask;
>> + slot_ctrl |= (cmd & mask);
>> + ctrl->cmd_busy = 1;
>> + smp_mb();
>> + pcie_capability_write_word(pdev, PCI_EXP_SLTCTL, slot_ctrl);
>> mutex_unlock(&ctrl->ctrl_lock);
>> }
--
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
