RE: [PATCH] pci/pciehp: Allow polling/irq mode to be decided on a per-port basis

[Rajat Jain]

Hello Bjorn,

Was wanting to check your interest on up streaming this one. This one is fairly self-contained and simple, and allows to use polling on a per-port basis.

Please let me know you have any comments.

Thanks,

Rajat

> -----Original Message-----
> From: Rajat Jain [mailto:rajatxjain@xxxxxxxxx]
> Sent: Monday, March 31, 2014 4:52 PM
> To: Bjorn Helgaas; linux-pci@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Cc: Rajat Jain; Guenter Roeck
> Subject: [PATCH] pci/pciehp: Allow polling/irq mode to be decided on a per-
> port basis
> 
> Today, there is a global pciehp_poll_mode module parameter using which
> either _all_ the hot-pluggable ports are to use polling, or _all_ the ports are
> to use interrupts.
> 
> In a system where a certain port has IRQ issues, today the only option is to
> use the parameter that converts ALL the ports to use polling mode.
> This is not good, and hence this patch intruduces a bit field that can be set
> using a PCI quirk that indicates that polling should always be used for this
> particular PCIe port. The remaining ports can still hoose to continue to
> operate in whatever mode they wish to.
> 
> Signed-off-by: Rajat Jain <rajatxjain@xxxxxxxxx>
> Signed-off-by: Rajat Jain <rajatjain@xxxxxxxxxxx>
> Signed-off-by: Guenter Roeck <groeck@xxxxxxxxxxx>
> ---
>  drivers/pci/hotplug/pciehp.h     |    1 +
>  drivers/pci/hotplug/pciehp_hpc.c |   16 ++++++++++------
>  include/linux/pci.h              |    1 +
>  3 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/hotplug/pciehp.h b/drivers/pci/hotplug/pciehp.h
> index d208791..753a3b4 100644
> --- a/drivers/pci/hotplug/pciehp.h
> +++ b/drivers/pci/hotplug/pciehp.h
> @@ -98,6 +98,7 @@ struct controller {
>  	unsigned int no_cmd_complete:1;
>  	unsigned int link_active_reporting:1;
>  	unsigned int notification_enabled:1;
> +	unsigned int use_polling:1;	/* Always uses polling for this slot */
>  	unsigned int power_fault_detected;
>  };
> 
> diff --git a/drivers/pci/hotplug/pciehp_hpc.c
> b/drivers/pci/hotplug/pciehp_hpc.c
> index d7d058f..d210d23 100644
> --- a/drivers/pci/hotplug/pciehp_hpc.c
> +++ b/drivers/pci/hotplug/pciehp_hpc.c
> @@ -82,7 +82,7 @@ static inline int pciehp_request_irq(struct controller
> *ctrl)
>  	int retval, irq = ctrl->pcie->irq;
> 
>  	/* Install interrupt polling timer. Start with 10 sec delay */
> -	if (pciehp_poll_mode) {
> +	if (ctrl->use_polling) {
>  		init_timer(&ctrl->poll_timer);
>  		start_int_poll_timer(ctrl, 10);
>  		return 0;
> @@ -98,7 +98,7 @@ static inline int pciehp_request_irq(struct controller
> *ctrl)
> 
>  static inline void pciehp_free_irq(struct controller *ctrl)  {
> -	if (pciehp_poll_mode)
> +	if (ctrl->use_polling)
>  		del_timer_sync(&ctrl->poll_timer);
>  	else
>  		free_irq(ctrl->pcie->irq, ctrl);
> @@ -131,7 +131,7 @@ static int pcie_poll_cmd(struct controller *ctrl)
> 
>  static void pcie_wait_cmd(struct controller *ctrl, int poll)  {
> -	unsigned int msecs = pciehp_poll_mode ? 2500 : 1000;
> +	unsigned int msecs = ctrl->use_polling ? 2500 : 1000;
>  	unsigned long timeout = msecs_to_jiffies(msecs);
>  	int rc;
> 
> @@ -595,7 +595,7 @@ void pcie_enable_notification(struct controller *ctrl)
>  		cmd |= PCI_EXP_SLTCTL_PDCE;
>  	if (MRL_SENS(ctrl))
>  		cmd |= PCI_EXP_SLTCTL_MRLSCE;
> -	if (!pciehp_poll_mode)
> +	if (!ctrl->use_polling)
>  		cmd |= PCI_EXP_SLTCTL_HPIE | PCI_EXP_SLTCTL_CCIE;
> 
>  	mask = (PCI_EXP_SLTCTL_PDCE | PCI_EXP_SLTCTL_ABPE | @@ -
> 642,14 +642,14 @@ int pciehp_reset_slot(struct slot *slot, int probe)
>  	stat_mask |= PCI_EXP_SLTSTA_DLLSC;
> 
>  	pcie_write_cmd(ctrl, 0, ctrl_mask);
> -	if (pciehp_poll_mode)
> +	if (ctrl->use_polling)
>  		del_timer_sync(&ctrl->poll_timer);
> 
>  	pci_reset_bridge_secondary_bus(ctrl->pcie->port);
> 
>  	pcie_capability_write_word(pdev, PCI_EXP_SLTSTA, stat_mask);
>  	pcie_write_cmd(ctrl, ctrl_mask, ctrl_mask);
> -	if (pciehp_poll_mode)
> +	if (ctrl->use_polling)
>  		int_poll_timeout(ctrl->poll_timer.data);
> 
>  	return 0;
> @@ -789,6 +789,10 @@ struct controller *pcie_init(struct pcie_device *dev)
>                  ctrl_dbg(ctrl, "Link Active Reporting supported\n");
>                  ctrl->link_active_reporting = 1;
>          }
> +	if (pciehp_poll_mode || dev->port->hotplug_polling) {
> +		ctrl_info(ctrl, "will use polling\n");
> +		ctrl->use_polling = 1;
> +	}
> 
>  	/* Clear all remaining event bits in Slot Status register */
>  	pcie_capability_write_word(pdev, PCI_EXP_SLTSTA, diff --git
> a/include/linux/pci.h b/include/linux/pci.h index a13d682..b2ec72e 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -336,6 +336,7 @@ struct pci_dev {
>  	unsigned int	is_virtfn:1;
>  	unsigned int	reset_fn:1;
>  	unsigned int    is_hotplug_bridge:1;
> +	unsigned int    hotplug_polling:1; /* Port uses polling for hotplug */
>  	unsigned int    __aer_firmware_first_valid:1;
>  	unsigned int	__aer_firmware_first:1;
>  	unsigned int	broken_intx_masking:1;
> --
> 1.7.9.5
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/