Thread (6 messages) 6 messages, 4 authors, 2009-11-17

Re: [PATCH 1/1] ata/sata_sil24: MSI support, disabled by default

From: Grant Grundler <hidden>
Date: 2009-11-16 17:48:14
Also in: linux-ide

On Sun, Nov 15, 2009 at 10:19 PM, Vivek Mahajan
[off-list ref] wrote:
quoted hunk ↗ jump to hunk
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 <redacted>
---
=C2=A0drivers/ata/sata_sil24.c | =C2=A0 =C2=A09 +++++++++
=C2=A01 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 =3D {
=C2=A0#endif
=C2=A0};

+static int sata_sil24_msi; =C2=A0 =C2=A0/* 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, h=
ow
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). :(

quoted hunk ↗ jump to hunk
+
=C2=A0/*
=C2=A0* Use bits 30-31 of port_flags to encode available port numbers.
=C2=A0* Current maxium is 4.
@@ -1340,6 +1344,11 @@ static int sil24_init_one(struct pci_dev *pdev, co=
nst struct pci_device_id *ent)
=C2=A0 =C2=A0 =C2=A0 =C2=A0sil24_init_controller(host);

+ =C2=A0 =C2=A0 =C2=A0 if (sata_sil24_msi && !!pci_msi_enable(pdev)) {
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 dev_printk(KERN_INFO, =
&pdev->dev, "Using MSI\n");
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 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 [off-list ref]" once the
variable name is changed and the pci_intx() call is removed.

cheers,
grant
+ =C2=A0 =C2=A0 =C2=A0 }
+
=C2=A0 =C2=A0 =C2=A0 =C2=A0pci_set_master(pdev);
=C2=A0 =C2=A0 =C2=A0 =C2=A0return ata_host_activate(host, pdev->irq, sil2=
4_interrupt, IRQF_SHARED,
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 &sil24_sht);
--
1.5.6.5

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at =C2=A0http://vger.kernel.org/majordomo-info.html
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help