| Message ID | 4E12CEBE.2090608@jp.fujitsu.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
(2011/07/05 17:43), Kenji Kaneshige wrote: > (2011/07/05 17:01), Naoki Yanagimoto wrote: >> From: Naoki Yanagimoto<yanagimoto@np.css.fujitsu.com> >> >> 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@np.css.fujitsu.com> 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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);