diff mbox series

mailbox: pcc: export the PCC subspace type

Message ID 20230914115753.9064-1-lihuisong@huawei.com (mailing list archive)
State Handled Elsewhere, archived
Headers show
Series mailbox: pcc: export the PCC subspace type | expand

Commit Message

lihuisong (C) Sept. 14, 2023, 11:57 a.m. UTC
As stated in APCI spec, the size of the subspace shared memory region may
be different for different types. So it is useful for driver to fill PCC
communication space. But the driver used this PCC channel doesn't know what
is the subspace type of the channel.

So export the PCC subspace type by requesting PCC channel.

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 drivers/mailbox/pcc.c | 10 ++++------
 include/acpi/pcc.h    |  1 +
 2 files changed, 5 insertions(+), 6 deletions(-)

Comments

Sudeep Holla Sept. 14, 2023, 4:13 p.m. UTC | #1
On Thu, Sep 14, 2023 at 07:57:53PM +0800, Huisong Li wrote:
> As stated in APCI spec, the size of the subspace shared memory region may
> be different for different types. So it is useful for driver to fill PCC
> communication space. But the driver used this PCC channel doesn't know what
> is the subspace type of the channel.
> 
> So export the PCC subspace type by requesting PCC channel.
>

I would prefer to avoid this, but please post it with user to get better
understanding of it.
lihuisong (C) Sept. 15, 2023, 7:04 a.m. UTC | #2
在 2023/9/15 0:13, Sudeep Holla 写道:
> On Thu, Sep 14, 2023 at 07:57:53PM +0800, Huisong Li wrote:
>> As stated in APCI spec, the size of the subspace shared memory region may
>> be different for different types. So it is useful for driver to fill PCC
>> communication space. But the driver used this PCC channel doesn't know what
>> is the subspace type of the channel.
>>
>> So export the PCC subspace type by requesting PCC channel.
>>
> I would prefer to avoid this, but please post it with user to get better
> understanding of it.
Hi Sudeep,

Thanks for your fast review.
Sorry, I cannot fully understand what you mean. What should I do for it?
Currently, it seems that the subspace type in all drivers used PCC is 
fixed and not obtained from their platform.
IMO, PCCT is a best natural way to export it.

/Huisong
Sudeep Holla Sept. 18, 2023, 8:43 a.m. UTC | #3
On Fri, Sep 15, 2023 at 03:04:57PM +0800, lihuisong (C) wrote:
> 
> 在 2023/9/15 0:13, Sudeep Holla 写道:
> > On Thu, Sep 14, 2023 at 07:57:53PM +0800, Huisong Li wrote:
> > > As stated in APCI spec, the size of the subspace shared memory region may
> > > be different for different types. So it is useful for driver to fill PCC
> > > communication space. But the driver used this PCC channel doesn't know what
> > > is the subspace type of the channel.
> > > 
> > > So export the PCC subspace type by requesting PCC channel.
> > > 
> > I would prefer to avoid this, but please post it with user to get better
> > understanding of it.
> Hi Sudeep,
> 
> Thanks for your fast review.
> Sorry, I cannot fully understand what you mean. What should I do for it?
> Currently, it seems that the subspace type in all drivers used PCC is fixed
> and not obtained from their platform.
> IMO, PCCT is a best natural way to export it.
> 

OK, but please post the changes in the *driver* you ought to use this channel
type. Your change just allows the drivers to use it but it doesn't contain
any driver making use of it. So please post that change as well to understand
the issue better. It helps in the review.
lihuisong (C) Sept. 19, 2023, 11:51 a.m. UTC | #4
在 2023/9/18 16:43, Sudeep Holla 写道:
> On Fri, Sep 15, 2023 at 03:04:57PM +0800, lihuisong (C) wrote:
>> 在 2023/9/15 0:13, Sudeep Holla 写道:
>>> On Thu, Sep 14, 2023 at 07:57:53PM +0800, Huisong Li wrote:
>>>> As stated in APCI spec, the size of the subspace shared memory region may
>>>> be different for different types. So it is useful for driver to fill PCC
>>>> communication space. But the driver used this PCC channel doesn't know what
>>>> is the subspace type of the channel.
>>>>
>>>> So export the PCC subspace type by requesting PCC channel.
>>>>
>>> I would prefer to avoid this, but please post it with user to get better
>>> understanding of it.
>> Hi Sudeep,
>>
>> Thanks for your fast review.
>> Sorry, I cannot fully understand what you mean. What should I do for it?
>> Currently, it seems that the subspace type in all drivers used PCC is fixed
>> and not obtained from their platform.
>> IMO, PCCT is a best natural way to export it.
>>
> OK, but please post the changes in the *driver* you ought to use this channel
> type. Your change just allows the drivers to use it but it doesn't contain
> any driver making use of it. So please post that change as well to understand
> the issue better. It helps in the review.
All right, I will post it ASAP.
>
diff mbox series

Patch

diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index 94885e411085..9742cc7837bd 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -91,7 +91,6 @@  struct pcc_chan_reg {
  * @cmd_update: PCC register bundle for the command complete update register
  * @error: PCC register bundle for the error status register
  * @plat_irq: platform interrupt
- * @type: PCC subspace type
  * @plat_irq_flags: platform interrupt flags
  * @chan_in_use: this flag is used just to check if the interrupt needs
  *		handling when it is shared. Since only one transfer can occur
@@ -108,7 +107,6 @@  struct pcc_chan_info {
 	struct pcc_chan_reg cmd_update;
 	struct pcc_chan_reg error;
 	int plat_irq;
-	u8 type;
 	unsigned int plat_irq_flags;
 	bool chan_in_use;
 };
@@ -263,7 +261,7 @@  static bool pcc_mbox_cmd_complete_check(struct pcc_chan_info *pchan)
 	 * bit 0 indicates that Platform is sending a notification and OSPM
 	 * needs to respond this interrupt to process this command.
 	 */
-	if (pchan->type == ACPI_PCCT_TYPE_EXT_PCC_SLAVE_SUBSPACE)
+	if (pchan->chan.type == ACPI_PCCT_TYPE_EXT_PCC_SLAVE_SUBSPACE)
 		return !val;
 
 	return !!val;
@@ -284,7 +282,7 @@  static irqreturn_t pcc_mbox_irq(int irq, void *p)
 	int ret;
 
 	pchan = chan->con_priv;
-	if (pchan->type == ACPI_PCCT_TYPE_EXT_PCC_MASTER_SUBSPACE &&
+	if (pchan->chan.type == ACPI_PCCT_TYPE_EXT_PCC_MASTER_SUBSPACE &&
 	    !pchan->chan_in_use)
 		return IRQ_NONE;
 
@@ -312,7 +310,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->chan.type == ACPI_PCCT_TYPE_EXT_PCC_SLAVE_SUBSPACE)
 		pcc_send_data(chan, NULL);
 	pchan->chan_in_use = false;
 
@@ -766,7 +764,7 @@  static int pcc_mbox_probe(struct platform_device *pdev)
 
 		pcc_parse_subspace_shmem(pchan, pcct_entry);
 
-		pchan->type = pcct_entry->type;
+		pchan->chan.type = pcct_entry->type;
 		pcct_entry = (struct acpi_subtable_header *)
 			((unsigned long) pcct_entry + pcct_entry->length);
 	}
diff --git a/include/acpi/pcc.h b/include/acpi/pcc.h
index 73e806fe7ce7..eddf9f5dfba3 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;
+	u8 type;
 };
 
 #define MAX_PCC_SUBSPACES	256