Re: [PATCH v3] Add pcie_hp=nomsi to disable MSI/MSI-X for pciehp driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



(2012/01/10 6:10), MUNEDA Takahiro wrote:

(snip.)

--- linux-3.2.orig/drivers/pci/hotplug/pciehp_core.c
+++ linux-3.2/drivers/pci/hotplug/pciehp_core.c
@@ -42,6 +42,7 @@ int pciehp_debug;
  int pciehp_poll_mode;
  int pciehp_poll_time;
  int pciehp_force;
+int pciehp_msi_disabled;

I think pciehp_msi_disabled needs to be defined in port driver
side because pciehp can be built as a module.

  struct workqueue_struct *pciehp_wq;
  struct workqueue_struct *pciehp_ordered_wq;

@@ -57,10 +58,12 @@ module_param(pciehp_debug, bool, 0644);
  module_param(pciehp_poll_mode, bool, 0644);
  module_param(pciehp_poll_time, int, 0644);
  module_param(pciehp_force, bool, 0644);
+module_param(pciehp_msi_disabled, bool, 0644);

When pciehp is build as a module, the pciehp driver is loaded
after the MSI(-X) of the port is initialized. So I don't think
this module parameter works. When pciehp is build as built-in,
pciehp.pciehp_msi_disabled would work, but I think it is
meaningless because you also provide "pcie_hp=" boot parameter
(and I don't know what will happen if user specify different
values for pciehp.pciehp_msi_disabled and pcie_hp). As a result,
I think this module parameter should be removed.

  MODULE_PARM_DESC(pciehp_debug, "Debugging mode enabled or not");
  MODULE_PARM_DESC(pciehp_poll_mode, "Using polling mechanism for hot-plug events or not");
  MODULE_PARM_DESC(pciehp_poll_time, "Polling mechanism frequency, in seconds");
  MODULE_PARM_DESC(pciehp_force, "Force pciehp, even if OSHP is missing");
+MODULE_PARM_DESC(pciehp_msi, "MSI/MSI-X enabled or not");

Maybe it should be "pciehp_msi_disabled" instead of "pciehp_msi"?
Anyway, I think we should remove this for the same reason above.



  #define PCIE_MODULE_NAME "pciehp"

@@ -72,6 +75,15 @@ static int get_attention_status	(struct
  static int get_latch_status	(struct hotplug_slot *slot, u8 *value);
  static int get_adapter_status	(struct hotplug_slot *slot, u8 *value);

+static int __init pciehp_setup(char *str)
+{
+	if (!strncmp(str, "nomsi", 5))
+		pciehp_msi_disabled = true;
+
+	return 1;
+}
+__setup("pcie_hp=", pciehp_setup);
+

I think this needs to be in port driver side, otherwise pcie_hp=
boot parameter doesn't work when building pciehp as a module.
Additionally, I got the the following warning message when building
pciehp as a module.

drivers/pci/hotplug/pciehp_core.c:77: warning: 'pciehp_setup' defined but not used

(snip.)


-	/* We have to use INTx if MSI cannot be used for PCIe PME. */
-	if ((mask&  PCIE_PORT_SERVICE_PME)&&  pcie_pme_no_msi()) {
+	/* We have to use INTx if MSI cannot be used for PCIe PME or pciehp. */
+	if (((mask&  PCIE_PORT_SERVICE_PME)&&  pcie_pme_no_msi()) ||
+	    ((mask&  PCIE_PORT_SERVICE_HP)&&  pciehp_no_msi()) ||
+	    ((mask&  PCIE_PORT_SERVICE_HP)&&
+	     (dev->vendor == PCI_VENDOR_ID_IDT&&  dev->device == 0x807f))) {

How about implementing vendor/device check as a quirk?

Regards,
Kenji Kaneshige
--
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


[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux