Hi Hoan, On Fri, May 06, 2016 at 11:38:34AM -0700, Hoan Tran wrote: > From: hotran <hotran@xxxxxxx> > > 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 <hotran@xxxxxxx> > --- > 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 <linux/delay.h> > #include <linux/io.h> > #include <linux/init.h> [...] > @@ -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 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
