From: Rajat Jain <rajatjain@juniper.net>
To: Takashi Iwai <tiwai@suse.de>, Bjorn Helgaas <bhelgaas@google.com>
CC: Alex Williamson <alex.williamson@redhat.com>,
        "linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH] PCI: pciehp: Remove surprise bit checks
Thread-Topic: [PATCH] PCI: pciehp: Remove surprise bit checks
Thread-Index: AQHPJx3aPSx88dSndk+D3PuOxMYjFJqwZLPg
Date: Tue, 11 Feb 2014 18:52:30 +0000
Message-ID: <1117e550ec6248e3a1834f887cbfdfad@DM2PR05MB671.namprd05.prod.outlook.com>
References: <s5hmwhx98vx.wl%tiwai@suse.de>
In-Reply-To: <s5hmwhx98vx.wl%tiwai@suse.de>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
Content-Transfer-Encoding: 8bit

Hello Takashi,

> -----Original Message-----
> From: linux-pci-owner@vger.kernel.org [mailto:linux-pci-
> owner@vger.kernel.org] On Behalf Of Takashi Iwai
> Sent: Tuesday, February 11, 2014 3:39 AM
> To: Bjorn Helgaas
> Cc: Alex Williamson; linux-pci@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: [PATCH] PCI: pciehp: Remove surprise bit checks
> 
> Currently pciehp driver checks the surprise removal bit and handles
> the hotplug event only when the bit is set.  But there are devices
> that don't set that bit but yet expect the hotplug working, e.g. the
> PCI card reader on HP ProBook 445 and 455 laptops appears only when
> you insert a card, and it needs the hotplug event handling.
> 
> For fixing this, basically we may ignore the surprise bit and always
> handle the event.

Some of this has been taken care of in the recent changes to pciehcp (last night).

Please see

https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?h=pci/pciehp&id=a3eba3887960d00abb912fe3703cd3a058c721b8

Also, link state hotplug has been added as part of this patch set. Can you please try with Bjorn's latest pci tree?

With that, I think you should not see the issue you are describing, because if the PCIe Link comes up when you insert the card, it shall be added (now) irrespective of whether or not you have the surprise bit set.

Thanks,

Rajat


> The only big concern in the past was the KVM device
> assignment, but this has been fixed in KVM side (ignoring hotplug
> events during secondary bus resets), there should be no obstacle
> ahead.
> 
> The earlier discussion thread is found at:
>     https://lkml.org/lkml/2013/3/20/274
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=70261
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  drivers/pci/hotplug/pciehp_ctrl.c | 11 -----------
>  drivers/pci/hotplug/pciehp_hpc.c  | 19 +++++++------------
>  2 files changed, 7 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/pci/hotplug/pciehp_ctrl.c
> b/drivers/pci/hotplug/pciehp_ctrl.c
> index 50628487597d..8f6c626b4e21 100644
> --- a/drivers/pci/hotplug/pciehp_ctrl.c
> +++ b/drivers/pci/hotplug/pciehp_ctrl.c
> @@ -434,8 +434,6 @@ static void interrupt_event_handler(struct
> work_struct *work)
>  		break;
>  	case INT_PRESENCE_ON:
>  	case INT_PRESENCE_OFF:
> -		if (!HP_SUPR_RM(ctrl))
> -			break;
>  		ctrl_dbg(ctrl, "Surprise Removal\n");
>  		handle_surprise_event(p_slot);
>  		break;
> @@ -494,15 +492,6 @@ int pciehp_disable_slot(struct slot *p_slot)
>  	if (!p_slot->ctrl)
>  		return 1;
> 
> -	if (!HP_SUPR_RM(p_slot->ctrl)) {
> -		pciehp_get_adapter_status(p_slot, &getstatus);
> -		if (!getstatus) {
> -			ctrl_info(ctrl, "No adapter on slot(%s)\n",
> -				  slot_name(p_slot));
> -			return -ENODEV;
> -		}
> -	}
> -
>  	if (MRL_SENS(p_slot->ctrl)) {
>  		pciehp_get_latch_status(p_slot, &getstatus);
>  		if (getstatus) {
> diff --git a/drivers/pci/hotplug/pciehp_hpc.c
> b/drivers/pci/hotplug/pciehp_hpc.c
> index 14acfccb7670..5f9196a85a53 100644
> --- a/drivers/pci/hotplug/pciehp_hpc.c
> +++ b/drivers/pci/hotplug/pciehp_hpc.c
> @@ -636,21 +636,16 @@ int pciehp_reset_slot(struct slot *slot, int
> probe)
>  	if (probe)
>  		return 0;
> 
> -	if (HP_SUPR_RM(ctrl)) {
> -		pcie_write_cmd(ctrl, 0, PCI_EXP_SLTCTL_PDCE);
> -		if (pciehp_poll_mode)
> -			del_timer_sync(&ctrl->poll_timer);
> -	}
> +	pcie_write_cmd(ctrl, 0, PCI_EXP_SLTCTL_PDCE);
> +	if (pciehp_poll_mode)
> +		del_timer_sync(&ctrl->poll_timer);
> 
>  	pci_reset_bridge_secondary_bus(ctrl->pcie->port);
> 
> -	if (HP_SUPR_RM(ctrl)) {
> -		pcie_capability_write_word(pdev, PCI_EXP_SLTSTA,
> -					   PCI_EXP_SLTSTA_PDC);
> -		pcie_write_cmd(ctrl, PCI_EXP_SLTCTL_PDCE,
> PCI_EXP_SLTCTL_PDCE);
> -		if (pciehp_poll_mode)
> -			int_poll_timeout(ctrl->poll_timer.data);
> -	}
> +	pcie_capability_write_word(pdev, PCI_EXP_SLTSTA,
> PCI_EXP_SLTSTA_PDC);
> +	pcie_write_cmd(ctrl, PCI_EXP_SLTCTL_PDCE, PCI_EXP_SLTCTL_PDCE);
> +	if (pciehp_poll_mode)
> +		int_poll_timeout(ctrl->poll_timer.data);
> 
>  	return 0;
>  }
> --
> 1.8.5.2
> 
> --
> 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
> 

????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m????????????I?