From: Grant Grundler [mailto:grundler@google.com]=20
Sent: Monday, November 16, 2009 11:08 PM

quoted

+static int sata_sil24_msi; =A0 =A0/* Disable MSI */=20
+module_param_named(msi, sata_sil24_msi, bool, S_IRUGO);=20
+MODULE_PARM_DESC(msi, "Enable MSI (Default: false)");

=20
Vivek,
Do we even still need the parameter? I'm thinking either MSI=20
works with a chipset or it doesn't. The kernel has globals to=20
"know" which state is true.

Sometimes even in a platform, some PCIe endpoints do very=20
well with MSI while others may have to resort to legacy ints.=20
Should we let the endpoints make the final call.

=20
If the parameter is needed, when this driver is compiled into=20
the kernel, how is "msi" parameter specified?
I think the parameter needs to be documented and fit in with=20
other "msi" parameters.
See "nomsi" in Documentation/kernel-parameters.txt.

In this case "msi" is supposed to be passed via insmod and=20
not via kernel cmdline. If the driver is built-in the kernel,=20
then force sata_sil24_msi =3D 1 in the driver to enable it.

=20
If you want to keep this module parameter, can I suggest=20
calling the exported parameter "sata_sil24_msi"?
=20

Will do it in the subsequent patch.

pci_intx() isn't documented in MSI-HOWTO.txt  - because it's=20
already called:
    pci_msi_enable() -> pci_msi_enable_block() ->=20
msi_capability_init()
       -> pci_intx_for_msi(dev, 0) -> pci_intx(pdev, 0);
=20
(thanks to willy (Mathew Wilcox) for pointing me at
msi_capability_init() - I overlooked it)
=20
Please add "Reviewed-by: Grant Grundler=20
[off-list ref]" once the variable name is changed and=20
the pci_intx() call is removed.

Will take care in the upcoming patch

=20
cheers,
grant

Thanks,
Vivek
=20