Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932976AbbLBTMD (ORCPT ); Wed, 2 Dec 2015 14:12:03 -0500 Received: from quartz.orcorp.ca ([184.70.90.242]:50245 "EHLO quartz.orcorp.ca" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932273AbbLBTMA (ORCPT ); Wed, 2 Dec 2015 14:12:00 -0500 Date: Wed, 2 Dec 2015 12:11:55 -0700 From: Jason Gunthorpe To: Jarkko Sakkinen Cc: Martin Wilck , tpmdd-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org, Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= Subject: Re: [tpmdd-devel] [PATCH v2 0/3] tpm_tis: Clean up force module parameter Message-ID: <20151202191155.GA2832@obsidianresearch.com> References: <1448996309-15220-1-git-send-email-jgunthorpe@obsidianresearch.com> <20151201213351.GC5071@intel.com> <20151202182726.GB30972@obsidianresearch.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20151202182726.GB30972@obsidianresearch.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-Broken-Reverse-DNS: no host name found for IP address 10.0.0.160 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2195 Lines: 59 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 -- 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/