| Message ID | 20240528191823.17775-2-admiyo@os.amperecomputing.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | MCTP over PCC | expand |
On 2024-05-29 at 00:48:21, admiyo@os.amperecomputing.com (admiyo@os.amperecomputing.com) wrote: > From: Adam Young <admiyo@amperecomputing.com> > > diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c > index 94885e411085..774727b89693 100644 > --- a/drivers/mailbox/pcc.c > +++ b/drivers/mailbox/pcc.c > @@ -280,6 +280,7 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p) > { > struct pcc_chan_info *pchan; > struct mbox_chan *chan = p; > + struct pcc_mbox_chan *pmchan; Reverse xmas tree. > u64 val; > int ret; > >
On Tue, May 28, 2024 at 03:18:21PM -0400, admiyo@os.amperecomputing.com wrote: > From: Adam Young <admiyo@amperecomputing.com> > > Type 4 PCC channels have an option to send back a response > to the platform when they are done processing the request. > The flag to indicate whether or not to respond is inside > the message body, and thus is not available to the pcc > mailbox. Since only one message can be processed at once per > channel, the value of this flag is checked during message processing > and passed back via the channels global structure. > > Ideally, the mailbox callback function would return a value > indicating whether the message requires an ACK, but that > would be a change to the mailbox API. That would involve > some change to all (about 12) of the mailbox based drivers, > and the majority of them would not need to know about the > ACK call. > I don't have all the 3 patches. Is this sent by error or am I expected to just review this patch while other 2 are not mailbox related ? > Signed-off-by: Adam Young <admiyo@os.amperecomputing.com> > --- > drivers/mailbox/pcc.c | 5 ++++- > include/acpi/pcc.h | 1 + > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c > index 94885e411085..774727b89693 100644 > --- a/drivers/mailbox/pcc.c > +++ b/drivers/mailbox/pcc.c > @@ -280,6 +280,7 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p) > { > struct pcc_chan_info *pchan; > struct mbox_chan *chan = p; > + struct pcc_mbox_chan *pmchan; > u64 val; > int ret; > > @@ -304,6 +305,8 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p) > if (pcc_chan_reg_read_modify_write(&pchan->plat_irq_ack)) > return IRQ_NONE; > > + pmchan = &pchan->chan; > + pmchan->ack_rx = true; //TODO default to False We need to remove this and detect when it can be true if the default expected is false.
diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c index 94885e411085..774727b89693 100644 --- a/drivers/mailbox/pcc.c +++ b/drivers/mailbox/pcc.c @@ -280,6 +280,7 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p) { struct pcc_chan_info *pchan; struct mbox_chan *chan = p; + struct pcc_mbox_chan *pmchan; u64 val; int ret; @@ -304,6 +305,8 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p) if (pcc_chan_reg_read_modify_write(&pchan->plat_irq_ack)) return IRQ_NONE; + pmchan = &pchan->chan; + pmchan->ack_rx = true; //TODO default to False mbox_chan_received_data(chan, NULL); /* @@ -312,7 +315,7 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p) * * The PCC master subspace channel clears chan_in_use to free channel. */ - if (pchan->type == ACPI_PCCT_TYPE_EXT_PCC_SLAVE_SUBSPACE) + if ((pchan->type == ACPI_PCCT_TYPE_EXT_PCC_SLAVE_SUBSPACE) && pmchan->ack_rx) pcc_send_data(chan, NULL); pchan->chan_in_use = false; diff --git a/include/acpi/pcc.h b/include/acpi/pcc.h index 9b373d172a77..297913378c2b 100644 --- a/include/acpi/pcc.h +++ b/include/acpi/pcc.h @@ -16,6 +16,7 @@ struct pcc_mbox_chan { u32 latency; u32 max_access_rate; u16 min_turnaround_time; + bool ack_rx; }; /* Generic Communications Channel Shared Memory Region */