在 2022/11/4 23:39, 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.
Hello Robbie, In multiple subspaces scenario, there is a problem
that OS doesn't know which channel should respond to the interrupt
if no this chan_in_use flag. If you have not not encountered any
issues in this case, it may be related to your register settings.
@Sudeep, what shoud we do next?
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;
.
