lkml.org 
[lkml]   [2009]   [Sep]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 5/6] tpm_tis: convert from pnp_driver to acpi_driver
    From
    Date
    On Mon, 2009-07-20 at 11:27 -0700, Andy Isaacson wrote:
    > On Wed, Jul 01, 2009 at 10:45:19AM -0300, Rajiv Andrade wrote:
    > > On Wed, 2009-07-01 at 11:01 +0100, Alan Cox wrote:
    > > > On Tue, 30 Jun 2009 18:04:14 -0700
    > > > Andy Isaacson <adi@vmware.com> wrote:
    > > >
    > > > > Not all TIS-compatible TPM chips have a _HID method in their ACPI entry,
    > > > > and the TPM spec says that the _CID method should be used to enumerate
    > > > > the TPM chip.
    > > >
    > > > There are a number of systems with TPMs (older laptops) that don't work
    > > > very well if you enable ACPI.
    > > >
    > > > This is therefore a regression - NAK
    > > >
    > > > Probably the best thing to do is to provide both ACPI and PnP
    > > > registration according to what is configured into the kernel. (And I
    > > > guess spot duplicates although the resource should be busy anyway)
    > > > --
    > > David sent this earlier when I said that PNP didn't work with this chip:
    > >
    > > <quote>
    > > The problem here is acpi pnp but the fix is really simple. The current
    > > pnpacpi/core.c routine that looks for isapnp devices enumerated in acpi
    > > enforces that the acpi hid be a valid isapnp id (the formats are
    > > slightly different). But that's broken: it shoudl be enforcing that
    > > either the acpi hid or any acpi cids be valid isapnp ids. It's a
    > > one-line change to do this, see patch 2.
    > >
    > > commit 7a553b4e7439ad0733da7da8663d32aa4865aa9e
    > > Author: David Smith <dds@google.com>
    > > Date: Tue Apr 28 18:52:02 2009 +0900
    > >
    > > Update ACPI PNP to support devices with EISA PNP CIDs but non-PNP HIDs
    > >
    > > Signed-off-by: David Smith <dds@google.com>
    > >
    > > diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c
    > > index 9496494..8bfddfb 100644
    > > --- a/drivers/pnp/pnpacpi/core.c
    > > +++ b/drivers/pnp/pnpacpi/core.c
    > > @@ -159,8 +159,8 @@ static int __init pnpacpi_add_device(struct acpi_device *device)
    > > * driver should not be loaded.
    > > */
    > > status = acpi_get_handle(device->handle, "_CRS", &temp);
    > > - if (ACPI_FAILURE(status) || !ispnpidacpi(acpi_device_hid(device)) ||
    > > - is_exclusive_device(device) || (!device->status.present))
    > > + if (ACPI_FAILURE(status) || is_exclusive_device(device) ||
    > > + (!device->status.present))
    > > return 0;
    > >
    > > dev = pnp_alloc_dev(&pnpacpi_protocol, num, acpi_device_hid(device));
    > >
    > > </quote>
    >
    > Len,
    >
    > Is this an acceptable change to pnpacpi? It resolves an issue with
    > tpm_tis but I'm concerned that it might have far-reaching impact.
    >
    > I've pasted in the problematic DSDT (manually fixing up whitespace to
    > make it more readable), and then a normal TPM simply has a _HID which
    > is matched by a pnp_device_id table in the driver
    > (drivers/char/tpm/tpm_tis.c).
    >
    > T400:
    > Device (TPM)
    > {
    > Method (_HID, 0, NotSerialized)
    > {
    > TPHY (0x00)
    > If (LEqual (TPMV, 0x01)) { Return (0x0201D824) }
    > If (LEqual (TPMV, 0x02)) { Return (0x0435CF4D) }
    > If (LEqual (TPMV, 0x03)) { Return (0x02016D08) }
    > If (LEqual (TPMV, 0x04)) { Return (0x01016D08) }
    > If (LOr (LEqual (TPMV, 0x05), LEqual (TPMV, 0x06))) {
    > Return (0x0010A35C)
    > }
    > If (LEqual (TPMV, 0x08)) { Return (0x00128D06) }
    > If (LEqual (TPMV, 0x09)) { Return ("INTC0102") }
    > Return (0x310CD041)
    > }
    >
    > Name (_CID, EisaId ("PNP0C31"))
    >
    > standard TPM:
    > Device (TPM)
    > {
    > Name (_HID, EisaId ("BCM0102"))
    > Name (_CID, EisaId ("PNP0C31"))
    >
    > The full thread is at
    > http://lkml.org/lkml/2009/7/1/265
    >
    > Thanks for any insight.
    >

    We've already waited too much on this, is it acceptable to make the
    workaround depend on (and only on) the module parameter you've set in
    patch 6/6? Therefore no need to check the vendor ID.

    <snip>
    +MODULE_PARM_DESC(itpm, "Force iTPM workarounds (found on some Lenovo laptops)");
    </snip>

    It already mentions _Force_, which in many cases maps to "it's all your
    responsibility"...

    And yes, still without PNP, but at least, working.

    Rajiv





    \
     
     \ /
      Last update: 2009-09-10 21:11    [W:0.041 / U:58.516 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site