(2011/07/05 17:43), Kenji Kaneshige wrote:
> (2011/07/05 17:01), Naoki Yanagimoto wrote:
>> From: Naoki Yanagimoto<yanagimoto@xxxxxxxxxxxxxxxxxx>
>>
>> I got a problem that an abnormal value was returned from the configuration
>> space of some PCIe card at hotadd. The pciehp driver regarded the function of
>> the device as being unavailable, so the card did not work. The problem
>> disappeared when I simply added 1 second wait without using DLLLA.
>>
>> I think that it should wait for 1 second because "PCI Express Base
>> Specification Revision 3.0" says, "the software must wait for at least
>> 1 second to judge device is broken after Data Link Layer State Changed Event".
>>
>
> I think you are right.
>
>> Therefore, I send a patch that drops DLLLA checking and adds 1 second wait.
>
> But I think DLLLA checking is still required. I think we need the following things.
>
> - Wait for Data Link Layer State Changed event.
> - Wait for at least 1 second to judge device is broken after Data Link
> Layer State Changed Event
>
> Your patch does only the latter. I think we also need the former.
> What do you think about the following change? Could you try it?
> Please note that I've not tested it at all. Sorry...
I absolutely agree with you.
I tested it. The problem did not occur. It looks good!
Tested-by: Naoki Yanagimoto <yanagimoto@xxxxxxxxxxxxxxxxxx>
Thanks,
Naoki Yanagimoto
>
> Regards,
> Kenji Kaneshige
>
> ---
> drivers/pci/hotplug/pciehp_ctrl.c | 3 +++
> drivers/pci/hotplug/pciehp_hpc.c | 11 ++---------
> 2 files changed, 5 insertions(+), 9 deletions(-)
>
> Index: 20110705/drivers/pci/hotplug/pciehp_ctrl.c
> ===================================================================
> --- 20110705.orig/drivers/pci/hotplug/pciehp_ctrl.c
> +++ 20110705/drivers/pci/hotplug/pciehp_ctrl.c
> @@ -213,6 +213,9 @@ static int board_added(struct slot *p_sl
> goto err_exit;
> }
>
> + /* Wait for 1 second after checking link training status */
> + msleep(1000);
> +
> /* Check for a power fault */
> if (ctrl->power_fault_detected || pciehp_query_power_fault(p_slot)) {
> ctrl_err(ctrl, "Power fault on slot %s\n", slot_name(p_slot));
> Index: 20110705/drivers/pci/hotplug/pciehp_hpc.c
> ===================================================================
> --- 20110705.orig/drivers/pci/hotplug/pciehp_hpc.c
> +++ 20110705/drivers/pci/hotplug/pciehp_hpc.c
> @@ -275,16 +275,9 @@ int pciehp_check_link_status(struct cont
> * hot-plug capable downstream port. But old controller might
> * not implement it. In this case, we wait for 1000 ms.
> */
> - if (ctrl->link_active_reporting){
> - /* Wait for Data Link Layer Link Active bit to be set */
> + if (ctrl->link_active_reporting)
> pcie_wait_link_active(ctrl);
> - /*
> - * We must wait for 100 ms after the Data Link Layer
> - * Link Active bit reads 1b before initiating a
> - * configuration access to the hot added device.
> - */
> - msleep(100);
> - } else
> + else
> msleep(1000);
>
> retval = pciehp_readw(ctrl, PCI_EXP_LNKSTA,&lnk_status);
>
>
--
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
