From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752268AbcEIJnV (ORCPT ); Mon, 9 May 2016 05:43:21 -0400 Received: from foss.arm.com ([217.140.101.70]:36530 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750961AbcEIJnT (ORCPT ); Mon, 9 May 2016 05:43:19 -0400 Date: Mon, 9 May 2016 10:43:10 +0100 From: Alexey Klimov To: Hoan Tran Cc: Jassi Brar , Ashwin Chaugule , "Rafael J. Wysocki" , Robert Moore , linux-kernel@vger.kernel.org, linux-acpi@vger.kernel.org, lho@apm.com, dhdang@apm.com Subject: Re: [PATCH v2] mailbox: pcc: Support HW-Reduced Communication Subspace Type 2 Message-ID: <20160509094309.GA14006@arm.com> References: <1462559914-28363-1-git-send-email-hotran@apm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1462559914-28363-1-git-send-email-hotran@apm.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Hoan, On Fri, May 06, 2016 at 11:38:34AM -0700, Hoan Tran wrote: > From: hotran > > ACPI 6.1 has a PCC HW-Reduced Communication Subspace Type 2 intended for > use on HW-Reduce ACPI Platform, which requires read-modify-write sequence > to acknowledge doorbell interrupt. This patch provides the implementation > for the Communication Subspace Type 2. > > This patch depends on patch [1] which supports PCC subspace type 2 header > [1] https://lkml.org/lkml/2016/5/5/14 > - [PATCH v2 03/13] ACPICA: ACPI 6.1: Support for new PCCT subtable So you finally decided to use separate structure declaration for type 2. Good. > v2 > * Remove changes inside "actbl3.h". This file is taken care by ACPICA. > * Parse both subspace type 1 and subspace type 2 > * Remove unnecessary variable initialization > * ISR returns IRQ_NONE in case of error > > v1 > * Initial > > Signed-off-by: Hoan Tran > --- > drivers/mailbox/pcc.c | 395 +++++++++++++++++++++++++++++++++++++------------- > 1 file changed, 296 insertions(+), 99 deletions(-) > > diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c > index 043828d..58c9a67 100644 > --- a/drivers/mailbox/pcc.c > +++ b/drivers/mailbox/pcc.c > @@ -59,6 +59,7 @@ > #include > #include > #include [...] > @@ -307,6 +440,43 @@ static int parse_pcc_subspace(struct acpi_subtable_header *header, > } > > /** > + * pcc_parse_subspace_irq - Parse the PCC IRQ and PCC ACK register > + * There should be one entry per PCC client. > + * @mbox_chans: Pointer to the PCC mailbox channel data > + * @pcct_ss: Pointer to the ACPI subtable header under the PCCT. > + * > + * Return: 0 for Success, else errno. > + * > + * This gets called for each entry in the PCC table. > + */ > +static int pcc_parse_subspace_irq(struct pcc_mbox_chan *mbox_chans, > + struct acpi_pcct_hw_reduced *pcct_ss) > +{ > + mbox_chans->irq = pcc_map_interrupt(pcct_ss->doorbell_interrupt, > + (u32)pcct_ss->flags); > + if (mbox_chans->irq <= 0) { > + pr_err("PCC GSI %d not registered\n", > + pcct_ss->doorbell_interrupt); > + return -EINVAL; > + } > + > + if (pcct_ss->header.type > + == ACPI_PCCT_TYPE_HW_REDUCED_SUBSPACE_TYPE2) { > + struct acpi_pcct_hw_reduced_type2 *pcct2_ss = (void *)pcct_ss; > + > + mbox_chans->pcc_doorbell_ack_vaddr = acpi_os_ioremap( > + pcct2_ss->doorbell_ack_register.address, > + pcct2_ss->doorbell_ack_register.bit_width / 8); > + if (!mbox_chans->pcc_doorbell_ack_vaddr) { > + pr_err("Failed to ioremap PCC ACK register\n"); > + return -ENOMEM; > + } > + } > + > + return 0; > +} > + > +/** > * acpi_pcc_probe - Parse the ACPI tree for the PCCT. > * > * Return: 0 for Success, else errno. > @@ -316,7 +486,8 @@ static int __init acpi_pcc_probe(void) > acpi_size pcct_tbl_header_size; > struct acpi_table_header *pcct_tbl; > struct acpi_subtable_header *pcct_entry; > - int count, i; > + struct acpi_table_pcct *acpi_pcct_tbl; > + int count, i, rc; > acpi_status status = AE_OK; > > /* Search for PCCT */ > @@ -334,22 +505,28 @@ static int __init acpi_pcc_probe(void) > ACPI_PCCT_TYPE_HW_REDUCED_SUBSPACE, > parse_pcc_subspace, MAX_PCC_SUBSPACES); > > + count += acpi_table_parse_entries(ACPI_SIG_PCCT, > + sizeof(struct acpi_table_pcct), > + ACPI_PCCT_TYPE_HW_REDUCED_SUBSPACE_TYPE2, > + parse_pcc_subspace, MAX_PCC_SUBSPACES); > + > if (count <= 0) { > pr_err("Error parsing PCC subspaces from PCCT\n"); > return -EINVAL; > } Looks like after first call to acpi_table_parse_entries() you may have negative number in count. And then you add counted number of type 2 subtables to count variable. I am not aware how pedantic this all should be but you may have more than MAX_PCC_SUBSPACES subspaces or don't probe any subspaces at all with such approach. Or other side effects. Best regards, Alexey Klimov