On 11 May 2016 at 14:15, Hoan Tran <hotran@xxxxxxx> wrote: > On Wed, May 11, 2016 at 4:57 AM, Ashwin Chaugule > <ashwin.chaugule@xxxxxxxxxx> wrote: >> On 11 May 2016 at 00:21, Hoan Tran <hotran@xxxxxxx> wrote: >>> On Tue, May 10, 2016 at 5:00 AM, Ashwin Chaugule >>>> On 6 May 2016 at 14:38, Hoan Tran <hotran@xxxxxxx> 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 >>>>> >>>>> 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(-) [..] >>>>> @@ -357,24 +534,44 @@ static int __init acpi_pcc_probe(void) >>>>> pcct_entry = (struct acpi_subtable_header *) ( >>>>> (unsigned long) pcct_tbl + sizeof(struct acpi_table_pcct)); >>>>> >>>>> + acpi_pcct_tbl = (struct acpi_table_pcct *) pcct_tbl; >>>>> + if (acpi_pcct_tbl->flags & ACPI_PCCT_DOORBELL) >>>>> + pcc_mbox_ctx.mbox_ctrl.txdone_irq = true; >>>>> + >>>>> for (i = 0; i < count; i++) { >>>>> struct acpi_generic_address *db_reg; >>>>> struct acpi_pcct_hw_reduced *pcct_ss; >>>>> - pcc_mbox_channels[i].con_priv = pcct_entry; >>>>> + >>>>> + pcc_mbox_ctx.chans[i].con_priv = pcct_entry; >>>>> + pcc_mbox_ctx.chans[i].mbox = &pcc_mbox_ctx.mbox_ctrl; >>>>> + >>>>> + pcct_ss = (struct acpi_pcct_hw_reduced *) pcct_entry; >>>>> + >>>>> + pcc_mbox_ctx.mbox_chans[i].chan = &pcc_mbox_ctx.chans[i]; >>>>> + if (pcc_mbox_ctx.mbox_ctrl.txdone_irq) { >>>>> + rc = pcc_parse_subspace_irq(&pcc_mbox_ctx.mbox_chans[i], >>>>> + pcct_ss); >>>>> + if (rc < 0) >>>>> + return rc; >>>>> + } >>>> >>>> I think we should parse the remaining entries and register them if >>>> they are sane. Some other PCC clients can then continue to function. >>> I think, it could be an error of PCCT table and need to be returned. >>>> >> >> Well, that could be dealt with a pr_err/warn() for that specific entry >> ? > But what happens if the PCC client requests this failed PCC subspace. > "pcc_parse_subspace_irq" function does "pr_err" for error cases. > >> IIRC not all subspaces require IRQ's. Its optional, isnt it? > Maybe I misunderstood: if "doorbell interrupt" bit of "platform > communications channel global flags" is set, the platform is capable > of generating an interrupt to indicate completion of a command. And if > this bit is set, "doorbell interrupt" and "doorbell interrupt flags" > fields of each subspace are active > Could you correct me, thanks >> You're right. My concern is addressed by your check for txdone_irq. Are you able to test this on a system which doesn't have Type 2 support? Thanks, Ashwin. -- 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
