lkml.org 
[lkml]   [2015]   [Dec]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module parameter
On Wed, Dec 02, 2015 at 11:27:27AM -0700, Jason Gunthorpe wrote:

> I'm guessing that if the driver probe order is tpm_crb,tpm_tis then
> things work because tpm_crb will claim the device first? Otherwise
> tpm_tis claims these things unconditionally? If the probe order is
> reversed things become broken?

Okay, I didn't find the is_fifo before, so that make sense

But this:

> What is the address tpm_tis should be using? I see two things, it
> either uses the x86 default address or it expects the ACPI to have a
> MEM resource. AFAIK ACPI should never rely on hard wired addresses, so
> I removed that code in this series. Perhaps tpm_tis should be using
> control_area_pa ? Will ACPI ever present a struct resource? (if yes,
> why isn't tpm_crb using one?)

Is then still a problem. On Martin's system the MSFT0101 device does
not have a struct resource attached to it. Does any system, or is this
just dead code?

Should the control_area_pa be used?

Martin: could you try this (along with the other hunk to prevent the
oops):

diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index 8a3509cb10da..6824a00ba513 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -138,6 +138,8 @@ static inline int is_fifo(struct acpi_device *dev)
if (le32_to_cpu(tbl->start_method) != TPM2_START_FIFO)
return 0;

+ dev_err(&dev->dev, "control area pa is %x\n", tbl->control_area_pa);
+
/* TPM 2.0 FIFO */
return 1;
}
Hoping to see it print 0xFED40000
> There is also something wrong with the endianness in the acpi
> stuff. I don't see endianness conversions in other acpi places, so I
> wonder if the ones in tpm_crb are correct. If they are correct then
> the struct needs le/be notations and there are some missing
> conversions.

I've made a patch to take care of this and move every thing to the
include/acpi/actbl2.h definitions, which is why I didn't notice
is_fifo in the first place...

Jason


\
 
 \ /
  Last update: 2015-12-02 20:21    [W:0.141 / U:0.136 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site