Messages in this thread Patch in this message | | | Date | Fri, 4 Nov 2022 11:39:52 -0400 | Subject | Re: [RFC] ACPI: PCC: Support shared interrupt for multiple subspaces | From | Robbie King <> |
| |
On 11/4/2022 11:15 AM, Sudeep Holla wrote: > On Fri, Nov 04, 2022 at 11:04:22AM -0400, Robbie King wrote: >> Hello Huisong, your raising of the shared interrupt issue is very timely, I >> am working to implement "Extended PCC subspaces (types 3 and 4)" using PCC >> on the ARM RDN2 reference platform as a proof of concept, and encountered >> this issue as well. FWIW, I am currently testing using Sudeep's patch with >> the "chan_in_use" flag removed, and so far have not encountered any issues. >> > > Interesting, do you mean the patch I post in this thread but without the > whole chan_in_use flag ?
That's right, diff I'm running with is attached to end of message.
> >> I think the RDN2 may provide an example of a write only interrupt >> acknowledge mechanism mentioned by Sudeep. >> > > Yes. > >> The RDN2 reference design uses the MHUv2 IP for the doorbell mechanism. If >> my implementation is correct (and it quite possibly is not), acknowledging >> the DB interrupt from the platform is accomplished by writing a 1 to the >> appropriate bit in the receiver channel window CH_CLR register, which is >> documented as: >> >> Channel flag clear. >> Write 0b1 to a bit clears the corresponding bit in the CH_ST and CH_ST_MSK. >> Writing 0b0 has no effect. >> Each bit always reads as 0b0. >> > > Correct, on this MHUv[1-2], it is write only register and it reads zero. > So basically you will ignore the interrupt if we apply the logic Huisong > proposed initially. > >> in the "Arm Corstone SSE-700 Subsystem Technical Reference Manual". >> >> Apologies if I am off in the weeds here as I have only been working with >> PCC/SCMI for a very short period of time. > > Good to know info :). >
It helps that your linux / firmware code is easy to follow! :)
One other minor issue I encountered was that a NULL GAS (all zeros) doesn't seem to be supported by pcc_chan_reg_init, may be a good opportunity for me to submit my first RFC...
diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c index ed18936b8ce6..3fa7335d15b0 100644 --- a/drivers/mailbox/pcc.c +++ b/drivers/mailbox/pcc.c @@ -100,6 +100,7 @@ struct pcc_chan_info { struct pcc_chan_reg cmd_update; struct pcc_chan_reg error; int plat_irq; + unsigned int plat_irq_flags; };
diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c index ed18936b8ce6..3fa7335d15b0 100644 --- a/drivers/mailbox/pcc.c +++ b/drivers/mailbox/pcc.c @@ -100,6 +100,7 @@ struct pcc_chan_info { struct pcc_chan_reg cmd_update; struct pcc_chan_reg error; int plat_irq; + unsigned int plat_irq_flags; };
#define to_pcc_chan_info(c) container_of(c, struct pcc_chan_info, chan) @@ -221,6 +222,12 @@ static int pcc_map_interrupt(u32 interrupt, u32 flags) return acpi_register_gsi(NULL, interrupt, trigger, polarity); }
+static bool pcc_chan_plat_irq_can_be_shared(struct pcc_chan_info *pchan) +{ + return (pchan->plat_irq_flags & ACPI_PCCT_INTERRUPT_MODE) == + ACPI_LEVEL_SENSITIVE; +} + /** * pcc_mbox_irq - PCC mailbox interrupt handler * @irq: interrupt number @@ -310,9 +317,12 @@ pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id)
if (pchan->plat_irq > 0) { int rc; + unsigned long irqflags;
- rc = devm_request_irq(dev, pchan->plat_irq, pcc_mbox_irq, 0, - MBOX_IRQ_NAME, chan); + irqflags = pcc_chan_plat_irq_can_be_shared(pchan) ? + IRQF_SHARED | IRQF_ONESHOT : 0; + rc = devm_request_irq(dev, pchan->plat_irq, pcc_mbox_irq, + irqflags, MBOX_IRQ_NAME, chan); if (unlikely(rc)) { dev_err(dev, "failed to register PCC interrupt %d\n", pchan->plat_irq); @@ -458,6 +468,8 @@ static int pcc_parse_subspace_irq(struct pcc_chan_info *pchan, return -EINVAL; }
+ pchan->plat_irq_flags = pcct_ss->flags; + if (pcct_ss->header.type == ACPI_PCCT_TYPE_HW_REDUCED_SUBSPACE_TYPE2) { struct acpi_pcct_hw_reduced_type2 *pcct2_ss = (void *)pcct_ss;
| |