lkml.org 
[lkml]   [2007]   [Jul]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    SubjectRe: 2.6.23-rc1-mm1 - seems OK on Dell Latitude D820, except for tpm_tis
    Date
    On Friday 27 July 2007 04:43:13 pm Bjorn Helgaas wrote:
    > On Friday 27 July 2007 07:28:09 am Valdis.Kletnieks@vt.edu wrote:
    > > Looks like the problematic code is in tpm_tis.c tpm_tis_init() near here:
    > >
    > > for (i = 3; i < 16 && chip->vendor.irq == 0; i++) {
    > > iowrite8(i, chip->vendor.iobase +
    > > TPM_INT_VECTOR(chip->vendor.locality));
    > > if (request_irq
    > > (i, tis_int_probe, IRQF_SHARED,
    > > chip->vendor.miscdev.name, chip) != 0) {
    > > dev_info(chip->dev,
    > > "Unable to request irq: %d for probe\n"
    > > ,
    > > i);
    > > continue;
    > > }
    > >
    > > This seems to be misbehaving differently for the two different DEBUG_SHIRQ
    > > cases.
    > >
    > > With DEBUG_SHIRQ=n, it starts at IRQ3, gets to at least 8 (where it complains
    > > it can't request it for probing), and possibly all the way to 15, without ever
    > > actually selecting and assigning an IRQ (to refresh memories, in that range
    > > /proc/interrupts only lists:
    > >
    > > 8: 0 0 IO-APIC-edge rtc
    > > 9: 3 0 IO-APIC-fasteoi acpi
    > > 12: 94 0 IO-APIC-edge i8042
    > > 14: 148166 0 IO-APIC-edge libata
    > > 15: 94 0 IO-APIC-edge libata
    > >
    > > So there's certainly IRQ's available. No idea why it doesn't choose one. But
    > > since it never chose one, it never gets into the "wait for the IRQ" protected
    > > by 'if (chip->vendor.irq)' at the end of tpm_tis_send.
    > >
    > > With DEBUG_SHIRQ=y, It starts at IRQ3, and assigns it (which seems a good thing).
    > > Unfortunately, this then hits the timeouts in tpm_tis_send.
    > >
    > > Anybody got an idea what *should* be happening here?
    >
    > I don't know why tpm_tis_init() is messing around trying different
    > IRQs between 3 and 16. That looks suspiciously x86-dependent.
    >
    > Maybe if you don't have PNP (though I doubt TPMs exist on any
    > pre-PNPBIOS machines) the "check-IRQ" loop would be necessary.
    >
    > But you're using the PNP probe, and PNP should just tell you what
    > IRQ the device is configured for (and whether the IRQ can be
    > shared -- see 8250_pnp.c for an example).

    I think tpm_tis should do something like the patch below to discover
    its interrupt. If ACPI tells us the device's IRQ, we should just use
    it rather than blindly trying a few possibilities. So the patch below
    might make it work, because it should skip the problematic code you
    identified above.

    But there could still be an issue with DEBUG_SHIRQ and the IRQ probe
    loop. Even if we used the attached patch, we'd probably still want
    the IRQ probe to work when you specify tpm_tis.force=1.

    Index: w/drivers/char/tpm/tpm_tis.c
    ===================================================================
    --- w.orig/drivers/char/tpm/tpm_tis.c 2007-07-30 11:29:31.000000000 -0600
    +++ w/drivers/char/tpm/tpm_tis.c 2007-07-30 11:35:20.000000000 -0600
    @@ -433,17 +433,12 @@
    MODULE_PARM_DESC(interrupts, "Enable interrupts");

    static int tpm_tis_init(struct device *dev, resource_size_t start,
    - resource_size_t len)
    + resource_size_t len, unsigned int irq)
    {
    u32 vendor, intfcaps, intmask;
    int rc, i;
    struct tpm_chip *chip;

    - if (!start)
    - start = TIS_MEM_BASE;
    - if (!len)
    - len = TIS_MEM_LEN;
    -
    if (!(chip = tpm_register_hardware(dev, &tpm_tis)))
    return -ENODEV;

    @@ -510,7 +505,9 @@
    iowrite32(intmask,
    chip->vendor.iobase +
    TPM_INT_ENABLE(chip->vendor.locality));
    - if (interrupts) {
    + if (interrupts)
    + chip->vendor.irq = irq;
    + if (interrupts && !chip->vendor.irq) {
    chip->vendor.irq =
    ioread8(chip->vendor.iobase +
    TPM_INT_VECTOR(chip->vendor.locality));
    @@ -595,10 +592,13 @@
    const struct pnp_device_id *pnp_id)
    {
    resource_size_t start, len;
    + unsigned int irq;
    +
    start = pnp_mem_start(pnp_dev, 0);
    len = pnp_mem_len(pnp_dev, 0);
    + irq = pnp_irq(pnp_dev, 0);

    - return tpm_tis_init(&pnp_dev->dev, start, len);
    + return tpm_tis_init(&pnp_dev->dev, start, len, irq);
    }

    static int tpm_tis_pnp_suspend(struct pnp_dev *dev, pm_message_t msg)
    @@ -658,7 +658,7 @@
    return rc;
    if (IS_ERR(pdev=platform_device_register_simple("tpm_tis", -1, NULL, 0)))
    return PTR_ERR(pdev);
    - if((rc=tpm_tis_init(&pdev->dev, 0, 0)) != 0) {
    + if((rc=tpm_tis_init(&pdev->dev, TIS_MEM_BASE, TIS_MEM_LEN, 0)) != 0) {
    platform_device_unregister(pdev);
    driver_unregister(&tis_drv);
    }
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/
    \
     
     \ /
      Last update: 2007-07-30 20:13    [W:4.189 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site