RE: [PATCH 1/1] ata/sata_sil24: MSI support, disabled by default
From: Jeff Garzik [mailto:j...@garzik.org] Sent: Tuesday, November 17, 2009 1:12 PM In this case msi is supposed to be passed via insmod and not via kernel cmdline. If the driver is built-in the kernel, then force sata_sil24_msi = 1 in the driver to enable it. First, the original patch was just fine, and it was applied. You should have received email confirmation of this already. Yes, I did. Second, all module options are available on the kernel command line, when a module is built into the kernel. You supply a module name prefix to each module option, on the kernel command line. Correct, sata_sil24.msi enables it. Jeff Thanks, Vivek ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/1] ata/sata_sil24: MSI support, disabled by default
On Sun, Nov 15, 2009 at 10:19 PM, Vivek Mahajan
vivek.maha...@freescale.com wrote:
The following patch adds MSI support. Some platforms
may have broken MSI, so those are defaulted to use
legacy PCI interrupts.
Signed-off-by: Vivek Mahajan vivek.maha...@freescale.com
---
drivers/ata/sata_sil24.c | 9 +
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/drivers/ata/sata_sil24.c b/drivers/ata/sata_sil24.c
index e6946fc..1370df6 100644
--- a/drivers/ata/sata_sil24.c
+++ b/drivers/ata/sata_sil24.c
@@ -417,6 +417,10 @@ static struct ata_port_operations sil24_ops = {
#endif
};
+static int sata_sil24_msi; /* Disable MSI */
+module_param_named(msi, sata_sil24_msi, bool, S_IRUGO);
+MODULE_PARM_DESC(msi, Enable MSI (Default: false));
Vivek,
Do we even still need the parameter? I'm thinking either MSI works
with a chipset
or it doesn't. The kernel has globals to know which state is true.
If the parameter is needed, when this driver is compiled into the kernel, how
is msi parameter specified?
I think the parameter needs to be documented and fit in with other
msi parameters.
See nomsi in Documentation/kernel-parameters.txt.
If you want to keep this module parameter, can I suggest calling the
exported parameter sata_sil24_msi?
I'm not able to test this since the chipset I have sata_sil24 devices
plugged into don't support MSI (older AMD/Nvidia chipset). :(
+
/*
* Use bits 30-31 of port_flags to encode available port numbers.
* Current maxium is 4.
@@ -1340,6 +1344,11 @@ static int sil24_init_one(struct pci_dev *pdev, const
struct pci_device_id *ent)
sil24_init_controller(host);
+ if (sata_sil24_msi !!pci_msi_enable(pdev)) {
+ dev_printk(KERN_INFO, pdev-dev, Using MSI\n);
+ pci_intx(pdev, 0);
pci_intx() isn't documented in MSI-HOWTO.txt - because it's already called:
pci_msi_enable() - pci_msi_enable_block() - msi_capability_init()
- pci_intx_for_msi(dev, 0) - pci_intx(pdev, 0);
(thanks to willy (Mathew Wilcox) for pointing me at
msi_capability_init() - I overlooked it)
Please add Reviewed-by: Grant Grundler grund...@google.com once the
variable name is changed and the pci_intx() call is removed.
cheers,
grant
+ }
+
pci_set_master(pdev);
return ata_host_activate(host, pdev-irq, sil24_interrupt, IRQF_SHARED,
sil24_sht);
--
1.5.6.5
--
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/1] ata/sata_sil24: MSI support, disabled by default
On 11/16/2009 01:19 AM, Vivek Mahajan wrote: The following patch adds MSI support. Some platforms may have broken MSI, so those are defaulted to use legacy PCI interrupts. Signed-off-by: Vivek Mahajanvivek.maha...@freescale.com --- drivers/ata/sata_sil24.c |9 + 1 files changed, 9 insertions(+), 0 deletions(-) applied ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH 1/1] ata/sata_sil24: MSI support, disabled by default
From: Grant Grundler [mailto:grund...@google.com] Sent: Monday, November 16, 2009 11:08 PM +static int sata_sil24_msi; /* Disable MSI */ +module_param_named(msi, sata_sil24_msi, bool, S_IRUGO); +MODULE_PARM_DESC(msi, Enable MSI (Default: false)); Vivek, Do we even still need the parameter? I'm thinking either MSI works with a chipset or it doesn't. The kernel has globals to know which state is true. Sometimes even in a platform, some PCIe endpoints do very well with MSI while others may have to resort to legacy ints. Should we let the endpoints make the final call. If the parameter is needed, when this driver is compiled into the kernel, how is msi parameter specified? I think the parameter needs to be documented and fit in with other msi parameters. See nomsi in Documentation/kernel-parameters.txt. In this case msi is supposed to be passed via insmod and not via kernel cmdline. If the driver is built-in the kernel, then force sata_sil24_msi = 1 in the driver to enable it. If you want to keep this module parameter, can I suggest calling the exported parameter sata_sil24_msi? Will do it in the subsequent patch. pci_intx() isn't documented in MSI-HOWTO.txt - because it's already called: pci_msi_enable() - pci_msi_enable_block() - msi_capability_init() - pci_intx_for_msi(dev, 0) - pci_intx(pdev, 0); (thanks to willy (Mathew Wilcox) for pointing me at msi_capability_init() - I overlooked it) Please add Reviewed-by: Grant Grundler grund...@google.com once the variable name is changed and the pci_intx() call is removed. Will take care in the upcoming patch cheers, grant Thanks, Vivek ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/1] ata/sata_sil24: MSI support, disabled by default
On 11/17/2009 01:59 AM, Mahajan Vivek-B08308 wrote: From: Grant Grundler [mailto:grund...@google.com] Sent: Monday, November 16, 2009 11:08 PM +static int sata_sil24_msi;/* Disable MSI */ +module_param_named(msi, sata_sil24_msi, bool, S_IRUGO); +MODULE_PARM_DESC(msi, Enable MSI (Default: false)); Vivek, Do we even still need the parameter? I'm thinking either MSI works with a chipset or it doesn't. The kernel has globals to know which state is true. Sometimes even in a platform, some PCIe endpoints do very well with MSI while others may have to resort to legacy ints. Should we let the endpoints make the final call. If the parameter is needed, when this driver is compiled into the kernel, how is msi parameter specified? I think the parameter needs to be documented and fit in with other msi parameters. See nomsi in Documentation/kernel-parameters.txt. In this case msi is supposed to be passed via insmod and not via kernel cmdline. If the driver is built-in the kernel, then force sata_sil24_msi = 1 in the driver to enable it. First, the original patch was just fine, and it was applied. You should have received email confirmation of this already. Second, all module options are available on the kernel command line, when a module is built into the kernel. You supply a module name prefix to each module option, on the kernel command line. Jeff ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
