Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932330AbbFIJOO (ORCPT ); Tue, 9 Jun 2015 05:14:14 -0400 Received: from mga01.intel.com ([192.55.52.88]:9919 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752483AbbFIJOI (ORCPT ); Tue, 9 Jun 2015 05:14:08 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.13,579,1427785200"; d="scan'208";a="584596470" Date: Tue, 9 Jun 2015 12:14:00 +0300 From: Jarkko Sakkinen To: Peter Huewe Cc: "moderated list:TPM DEVICE DRIVER" , open list Subject: Re: [tpmdd-devel] [PATCH] tpm, tpm_crb: migrate to struct acpi_table_tpm2 and acpi_tpm2_control Message-ID: <20150609091400.GA4968@jsakkine-mobl1> References: <1432915058-5598-1-git-send-email-jarkko.sakkinen@linux.intel.com> <20150608115435.GA19358@jsakkine-mobl1> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150608115435.GA19358@jsakkine-mobl1> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2342 Lines: 74 On Mon, Jun 08, 2015 at 02:54:35PM +0300, Jarkko Sakkinen wrote: > Hi > > I somehow missed your reply to this last week. > > On Tue, Jun 02, 2015 at 04:00:37PM +0200, Peter Huewe wrote: > > Hi > > >Betreff: [PATCH] tpm, tpm_crb: migrate to struct acpi_table_tpm2 and acpi_tpm2_control > > > Migrate to struct acpi_table_tpm2 and struct acpi_tpm2_control defined > > > in include/acpi/actbl3.h from the internal structures. > > > > I definitely do like the idea! Thanks for spotting this! > > > > However one small remark > > > -struct crb_control_area { > > > - u32 req; > > > - u32 sts; > > > - u32 cancel; > > > - u32 start; > > > - u32 int_enable; > > > - u32 int_sts; > > > - u32 cmd_size; > > > - u64 cmd_pa; > > > - u32 rsp_size; > > > - u64 rsp_pa; > > > -} __packed; > > > - > > > > > > - if (le32_to_cpu(ioread32(&priv->cca->sts)) & CRB_CA_STS_ERROR) > > > + if (le32_to_cpu(ioread32(&priv->ctl->error)) & CRB_CA_STS_ERROR) > > > return -EIO; > > > > I know the fields are described in include/acpi/actbl3.h as > > +struct acpi_tpm2_control { > > + u32 reserved; > > + u32 error; > > + u32 cancel; > > + u32 start; > > + u64 interrupt_control; > > + u32 command_size; > > + u64 command_address; > > + u32 response_size; > > + u64 response_address; > > +}; > > > > but are the names there still correct? Isn't this information outdated? > > The acpi spec refers to the MS spec which is not present anymore, and MS refers to the TCG -- and in the PTP your names are used. > > > > ---> We should update the ACPI header? > > At least the naming for reserved and error. > > What do you think? > > I think you are right. It does not make sense to degrade here. I'll > prepare "CRB fixes" patch set and also include a workaround for this > bug: > > https://bugzilla.kernel.org/show_bug.cgi?id=98181 > > See my last comment. Some rethinking after sending this email. I implement and submit the bug as a separate item as these are unrelated issues. I think the control area should not be in the ACPI headers in the first place as it is not defined TCG ACPI Specification. /Jarkko -- 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/