Messages in this thread Patch in this message | | | Date | Wed, 2 Dec 2015 12:11:55 -0700 | From | Jason Gunthorpe <> | Subject | Re: [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
| |