Skip to content

S
Simple PCIe FMC carrier SPEC - Software
Commit 4d8935ed authored by Alessandro Rubini's avatar Alessandro Rubini
Browse files
Options

kernel/spec: make MSI optional, disabled by default 

We haven't been able to run Message Signalled Interrupt
reliably, for a quirck in the Gennum chip and the kernel
insisting to configure it according to the standard. So this
adds "use_msi" as a module parameter, disabled by default.
See documentation for details.
Signed-off-by: Alessandro Rubini's avatarAlessandro Rubini <rubini@gnudd.com>
parent 190258e4
Hide whitespace changes
Inline Side-by-side
Showing with 108 additions and 33 deletions
doc/spec-sw.in
View file @ 4d8935ed
......@@ -185,11 +185,26 @@ The module can receive the following parameters to customize its operation:
@table @code
@item fw_name
This string parameter can be used to override the default name
(@code{fmc/spec-init.bin}) for the initialization binary file.
@item use_msi
This forces the driver to use @i{message signalled interrupts}.
MSI interrupts have some advantages over conventional wire-level
interrupts, but on some systems I've not been able to make them work.
By default the driver uses the old-fashioned wire-level
signalling method, unless you pass @code{use_msi=1} .
@item test_irq
If not zero, this parameter requests to self-test interrupt
generation, using the Gennum registers. This usually does not
work on my host, for yet unknown reasons (and that's why it is
generation, using the software-driven interrupt source in
Gennum registers. This usually does not
work on my host if I use MSI,
for yet unknown reasons (and that's why it is
disabled by default).
@item i2c_dump
......@@ -197,11 +212,6 @@ The module can receive the following parameters to customize its operation:
If not zero, this parameter requests to @i{printk} the content
of the FMC eeprom, for diagnostic purposes.
@item fw_name
This string parameter can be used to override the default name
(@code{fmc/spec-init.bin}) for the initialization binary file.
@end table
Any mezzanine-specific action must be performed by the driver for the
......@@ -277,6 +287,49 @@ the PCI identifiers:
Please note that not all sub-modules support all of those parameters.
You can use @i{modinfo} to check what is supported by each module.
@c ==========================================================================
@node Interrupts in spec.ko
@section Interrupts in spec.ko
Up to version 2.0 of this package, I enabled MSI (@i{message signalled
interrupts}) in the Gennum chip. Now the default is using normal
@i{wire-level} PCI interrupts, with a module parameter called
@code{use_msi}.
While MSI should perform better than conventional interrupts, we
didn't manage to make them work on all systems. It's still unclear to
us whether it depends on the kernel version or some chipset-specific
(mis)behavior. Actually, MSI work on both of my development systems,
but failed to work for three of my mates.
If you want to help with taming the MSI problem, you should load the
@i{spec} driver with @code{use_msi=1} and run the @i{wr-nic}
driver. If you are unable to exchange frames, please grep for @code{wr-nic}
in @file{/proc/interrupts} to see if the counters are moving or not. If not,
the problem is most likely in register 0x4a (@code{MSI_CONTROL})
that lost its correct value of @code{0xa5}. You can check the
high 16 bits in the output of @code{specmem -g 48} to verify.
The code is well commented about this MSI issue, please look
for @code{use_msi} in the source code.
With non-msi interrupts, the lines are shared with other peripherals,
and you'll something like this (this computer has two SPEC cards plugged):
@smalleexample
spusa.root# grep nic /proc/interrupts
16: 236 0 0 805 IO-APIC-fasteoi snd_hda_intel, wr-nic
18: 0 14721 16 33 IO-APIC-fasteoi ahci, ohci_hcd:usb4, wr-nic
@end smallexample
If you enable MSI, interrupts have higher numbers, allocated specifically
for the peripheral:
@smallexample
spusa.root# grep wr-nic /proc/interrupts
47: 0 0 0 124 PCI-MSI-edge wr-nic
48: 70470 0 0 26 PCI-MSI-edge wr-nic
@end smallexample
@c ##########################################################################
@node fmc-trivial.ko
@chapter fmc-trivial.ko
......
kernel/spec-fmc.c
View file @ 4d8935ed
......@@ -96,14 +96,18 @@ static int spec_irq_request(struct fmc_device *fmc, irq_handler_t handler,
u32 value;
ret = request_irq(fmc->irq, handler, flags, name, fmc);
if (ret) return ret;
value = gennum_readl(spec, GNPPCI_MSI_CONTROL);
if ((value & 0x810000) != 0x810000)
dev_err(&spec->pdev->dev, "invalid msi control: 0x%08x\n",
value);
value = 0xa50000 | (value & 0xffff);
gennum_writel(spec, value, GNPPCI_MSI_CONTROL);
if (ret)
return ret;
if (spec_use_msi) {
/* A check and a hack, but doesn't work on all computers */
value = gennum_readl(spec, GNPPCI_MSI_CONTROL);
if ((value & 0x810000) != 0x810000)
dev_err(&spec->pdev->dev, "invalid msi control: "
"0x%04x\n", value >> 16);
value = 0xa50000 | (value & 0xffff);
gennum_writel(spec, value, GNPPCI_MSI_CONTROL);
}
/* Enable gpio interrupts:
* gpio6: tp8: output low
......@@ -203,16 +207,18 @@ static int spec_irq_init(struct fmc_device *fmc)
uint32_t value;
int i;
/*
* Enable multiple-msi to work around a chip design bug.
* See http://blog.tftechpages.com/?p=595
*/
value = gennum_readl(spec, GNPPCI_MSI_CONTROL);
if ((value & 0x810000) != 0x810000)
dev_err(&spec->pdev->dev, "invalid msi control: 0x%08x\n",
value);
value = 0xa50000 | (value & 0xffff);
gennum_writel(spec, value, GNPPCI_MSI_CONTROL);
if (spec_use_msi) {
/*
* Enable multiple-msi to work around a chip design bug.
* See http://blog.tftechpages.com/?p=595
*/
value = gennum_readl(spec, GNPPCI_MSI_CONTROL);
if ((value & 0x810000) != 0x810000)
dev_err(&spec->pdev->dev, "invalid msi control: "
"0x%04x\n", value >> 16);
value = 0xa50000 | (value & 0xffff);
gennum_writel(spec, value, GNPPCI_MSI_CONTROL);
}
/*
* Now check the two least-significant bits of the msi-data register,
......@@ -221,14 +227,17 @@ static int spec_irq_init(struct fmc_device *fmc)
value = gennum_readl(spec, GNPPCI_MSI_DATA);
for (i = 0; i < 7; i++)
gennum_writel(spec, 0, GNINT_CFG(i));
gennum_writel(spec, 0x800c, GNINT_CFG(value & 0x03));
if (spec_use_msi)
gennum_writel(spec, 0x800c, GNINT_CFG(value & 0x03));
else
gennum_writel(spec, 0x800c, GNINT_CFG(0 /* first one */ ));
/* Finally, ensure we are able to receive it -- if the user asked to */
if (spec_test_irq == 0)
return 0;
spec->irq_count = 0;
init_completion(&spec->compl);
fmc->op->irq_request(fmc, spec_test_handler, "spec-test", 0);
fmc->op->irq_request(fmc, spec_test_handler, "spec-test", IRQF_SHARED);
gennum_writel(spec, 8, GNINT_STAT);
gennum_writel(spec, 0, GNINT_STAT);
wait_for_completion_timeout(&spec->compl, msecs_to_jiffies(50));
......
kernel/spec-pci.c
View file @ 4d8935ed
......@@ -27,6 +27,9 @@
char *spec_fw_name = "fmc/spec-init.bin";
module_param_named(fw_name, spec_fw_name, charp, 0444);
int spec_use_msi = 0;
module_param_named(use_msi, spec_use_msi, int, 0444);
/* Load the FPGA. This bases on loader-ll.c, a kernel/user space thing */
int spec_load_fpga(struct spec_dev *spec, const void *data, int size)
{
......@@ -97,8 +100,16 @@ static int __devinit spec_probe(struct pci_dev *pdev,
return -ENOMEM;
spec->pdev = pdev;
if ( (ret = pci_enable_msi_block(pdev, 1)) < 0)
dev_err(&pdev->dev, "enable msi block: error %i\n", ret);
if (spec_use_msi) {
/*
* This should be "4" but arch/x86/kernel/apic/io_apic.c
* says "x86 doesn't support multiple MSI yet".
*/
ret = pci_enable_msi_block(pdev, 1);
if (ret < 0)
dev_err(&pdev->dev, "%s: enable msi block: error %i\n",
__func__, ret);
}
/* Remap our 3 bars */
for (i = ret = 0; i < 3; i++) {
......@@ -138,7 +149,8 @@ out_unmap:
spec->area[i] = NULL;
}
pci_set_drvdata(pdev, NULL);
pci_disable_msi(pdev);
if (spec_use_msi)
pci_disable_msi(pdev);
pci_disable_device(pdev);
kfree(spec);
return ret;
......@@ -160,7 +172,7 @@ static void __devexit spec_remove(struct pci_dev *pdev)
}
pci_set_drvdata(pdev, NULL);
kfree(spec);
pci_disable_msi(pdev);
//pci_disable_msi(pdev);
pci_disable_device(pdev);
}
......
kernel/spec.h
View file @ 4d8935ed
......@@ -117,10 +117,11 @@ static inline void gennum_mask_val(struct spec_dev *spec,
gennum_writel(spec, v, reg);
}
/* Functions in spec-pci.c */
/* Functions and data in spec-pci.c */
extern int spec_load_fpga(struct spec_dev *spec, const void *data, int size);
extern int spec_load_fpga_file(struct spec_dev *spec, char *name);
extern char *spec_fw_name;
extern int spec_use_msi;
/* Functions in spec-fmc.c, used by spec-pci.c */
extern int spec_fmc_create(struct spec_dev *spec);
......
kernel/wr-nic-eth.c
View file @ 4d8935ed
......@@ -155,7 +155,7 @@ int wrn_eth_init(struct fmc_device *fmc)
unsigned long size;
int i, ret;
ret = fmc->op->irq_request(fmc, wrn_handler, "wr-nic", 0);
ret = fmc->op->irq_request(fmc, wrn_handler, "wr-nic", IRQF_SHARED);
if (ret < 0) {
dev_err(dev, "Can't request interrupt\n");
return ret;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment