[PATCH 0/3] ACPI: PCC: Define and use the common PCC shared memory regions related macros

[PATCH 0/3] ACPI: PCC: Define and use the common PCC shared memory regions related macros

[Sudeep Holla]

This set of 3 small patches intend to consolidate and replace the existing
locally defined macros within couple of PCC client drivers when accessing
the command and status bitfields.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
Sudeep Holla (3):
      ACPI: PCC: Add PCC shared memory region command and status bitfields
      i2c: xgene-slimpro: Migrate to use generic PCC shmem related macros
      hwmon: (xgene) Migrate to use generic PCC shmem related macros

 drivers/hwmon/xgene-hwmon.c            | 16 +++++-----------
 drivers/i2c/busses/i2c-xgene-slimpro.c | 16 ++++------------
 include/acpi/pcc.h                     | 11 +++++++++++
 3 files changed, 20 insertions(+), 23 deletions(-)
---
base-commit: 6465e260f48790807eef06b583b38ca9789b6072
change-id: 20230926-pcc_defines-24be5e33b6f3

Best regards,
-- 
Regards,
Sudeep

[PATCH 1/3] ACPI: PCC: Add PCC shared memory region command and status bitfields

[Sudeep Holla]

Define the common macros to use when referring to various bitfields in
the PCC generic communications channel command and status fields.

Currently different drivers that need to use these bitfields have defined
these locally. This common macro is intended to consolidate and replace
those.

Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 include/acpi/pcc.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/include/acpi/pcc.h b/include/acpi/pcc.h
index 73e806fe7ce7..66d9934c2ee4 100644
--- a/include/acpi/pcc.h
+++ b/include/acpi/pcc.h
@@ -18,7 +18,18 @@ struct pcc_mbox_chan {
 	u16 min_turnaround_time;
 };
 
+/* Generic Communications Channel Shared Memory Region */
+#define PCC_SIGNATURE			0x50424300
+/* Generic Communications Channel Command Field */
+#define PCC_CMD_GENERATE_DB_INTR	BIT(15)
+/* Generic Communications Channel Status Field */
+#define PCC_STATUS_CMD_COMPLETE		BIT(0)
+#define PCC_STATUS_SCI_DOORBELL		BIT(1)
+#define PCC_STATUS_ERROR		BIT(2)
+#define PCC_STATUS_PLATFORM_NOTIFY	BIT(3)
+
 #define MAX_PCC_SUBSPACES	256
+
 #ifdef CONFIG_PCC
 extern struct pcc_mbox_chan *
 pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id);

-- 
2.42.0

[PATCH 2/3] i2c: xgene-slimpro: Migrate to use generic PCC shmem related macros

[Sudeep Holla]

Use the newly defined common and generic PCC shared memory region
related macros in this driver to replace the locally defined ones.

Cc: Andi Shyti <andi.shyti@kernel.org>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/i2c/busses/i2c-xgene-slimpro.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/i2c/busses/i2c-xgene-slimpro.c b/drivers/i2c/busses/i2c-xgene-slimpro.c
index fbc1ffbd2fa7..658396c9eeab 100644
--- a/drivers/i2c/busses/i2c-xgene-slimpro.c
+++ b/drivers/i2c/busses/i2c-xgene-slimpro.c
@@ -91,14 +91,6 @@
 
 #define SLIMPRO_IIC_MSG_DWORD_COUNT			3
 
-/* PCC related defines */
-#define PCC_SIGNATURE			0x50424300
-#define PCC_STS_CMD_COMPLETE		BIT(0)
-#define PCC_STS_SCI_DOORBELL		BIT(1)
-#define PCC_STS_ERR			BIT(2)
-#define PCC_STS_PLAT_NOTIFY		BIT(3)
-#define PCC_CMD_GENERATE_DB_INT		BIT(15)
-
 struct slimpro_i2c_dev {
 	struct i2c_adapter adapter;
 	struct device *dev;
@@ -160,11 +152,11 @@ static void slimpro_i2c_pcc_rx_cb(struct mbox_client *cl, void *msg)
 
 	/* Check if platform sends interrupt */
 	if (!xgene_word_tst_and_clr(&generic_comm_base->status,
-				    PCC_STS_SCI_DOORBELL))
+				    PCC_STATUS_SCI_DOORBELL))
 		return;
 
 	if (xgene_word_tst_and_clr(&generic_comm_base->status,
-				   PCC_STS_CMD_COMPLETE)) {
+				   PCC_STATUS_CMD_COMPLETE)) {
 		msg = generic_comm_base + 1;
 
 		/* Response message msg[1] contains the return value. */
@@ -186,10 +178,10 @@ static void slimpro_i2c_pcc_tx_prepare(struct slimpro_i2c_dev *ctx, u32 *msg)
 		   cpu_to_le32(PCC_SIGNATURE | ctx->mbox_idx));
 
 	WRITE_ONCE(generic_comm_base->command,
-		   cpu_to_le16(SLIMPRO_MSG_TYPE(msg[0]) | PCC_CMD_GENERATE_DB_INT));
+		   cpu_to_le16(SLIMPRO_MSG_TYPE(msg[0]) | PCC_CMD_GENERATE_DB_INTR));
 
 	status = le16_to_cpu(READ_ONCE(generic_comm_base->status));
-	status &= ~PCC_STS_CMD_COMPLETE;
+	status &= ~PCC_STATUS_CMD_COMPLETE;
 	WRITE_ONCE(generic_comm_base->status, cpu_to_le16(status));
 
 	/* Copy the message to the PCC comm space */

-- 
2.42.0

[PATCH 3/3] hwmon: (xgene) Migrate to use generic PCC shmem related macros

[Sudeep Holla]

Use the newly defined common and generic PCC shared memory region
related macros in this driver to replace the locally defined ones.

Cc: Jean Delvare <jdelvare@suse.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/hwmon/xgene-hwmon.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/hwmon/xgene-hwmon.c b/drivers/hwmon/xgene-hwmon.c
index 78d9f52e2a71..1ccdd61b6d13 100644
--- a/drivers/hwmon/xgene-hwmon.c
+++ b/drivers/hwmon/xgene-hwmon.c
@@ -57,12 +57,6 @@
 	(MSG_TYPE_SET(MSG_TYPE_PWRMGMT) | \
 	MSG_SUBTYPE_SET(hndl) | TPC_CMD_SET(cmd) | type)
 
-/* PCC defines */
-#define PCC_SIGNATURE_MASK		0x50424300
-#define PCCC_GENERATE_DB_INT		BIT(15)
-#define PCCS_CMD_COMPLETE		BIT(0)
-#define PCCS_SCI_DOORBEL		BIT(1)
-#define PCCS_PLATFORM_NOTIFICATION	BIT(3)
 /*
  * Arbitrary retries in case the remote processor is slow to respond
  * to PCC commands
@@ -142,15 +136,15 @@ static int xgene_hwmon_pcc_rd(struct xgene_hwmon_dev *ctx, u32 *msg)
 
 	/* Write signature for subspace */
 	WRITE_ONCE(generic_comm_base->signature,
-		   cpu_to_le32(PCC_SIGNATURE_MASK | ctx->mbox_idx));
+		   cpu_to_le32(PCC_SIGNATURE | ctx->mbox_idx));
 
 	/* Write to the shared command region */
 	WRITE_ONCE(generic_comm_base->command,
-		   cpu_to_le16(MSG_TYPE(msg[0]) | PCCC_GENERATE_DB_INT));
+		   cpu_to_le16(MSG_TYPE(msg[0]) | PCC_CMD_GENERATE_DB_INTR));
 
 	/* Flip CMD COMPLETE bit */
 	val = le16_to_cpu(READ_ONCE(generic_comm_base->status));
-	val &= ~PCCS_CMD_COMPLETE;
+	val &= ~PCC_STATUS_CMD_COMPLETE;
 	WRITE_ONCE(generic_comm_base->status, cpu_to_le16(val));
 
 	/* Copy the message to the PCC comm space */
@@ -544,7 +538,7 @@ static void xgene_hwmon_pcc_rx_cb(struct mbox_client *cl, void *msg)
 	msg = generic_comm_base + 1;
 	/* Check if platform sends interrupt */
 	if (!xgene_word_tst_and_clr(&generic_comm_base->status,
-				    PCCS_SCI_DOORBEL))
+				    PCC_STATUS_SCI_DOORBELL))
 		return;
 
 	/*
@@ -566,7 +560,7 @@ static void xgene_hwmon_pcc_rx_cb(struct mbox_client *cl, void *msg)
 	      TPC_CMD(((u32 *)msg)[0]) == TPC_ALARM))) {
 		/* Check if platform completes command */
 		if (xgene_word_tst_and_clr(&generic_comm_base->status,
-					   PCCS_CMD_COMPLETE)) {
+					   PCC_STATUS_CMD_COMPLETE)) {
 			ctx->sync_msg.msg = ((u32 *)msg)[0];
 			ctx->sync_msg.param1 = ((u32 *)msg)[1];
 			ctx->sync_msg.param2 = ((u32 *)msg)[2];

-- 
2.42.0

Re: [PATCH 3/3] hwmon: (xgene) Migrate to use generic PCC shmem related macros

[Guenter Roeck]

On Tue, Sep 26, 2023 at 01:28:02PM +0100, Sudeep Holla wrote:
> Use the newly defined common and generic PCC shared memory region
> related macros in this driver to replace the locally defined ones.
> 
> Cc: Jean Delvare <jdelvare@suse.com>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

Acked-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/hwmon/xgene-hwmon.c | 16 +++++-----------
>  1 file changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/hwmon/xgene-hwmon.c b/drivers/hwmon/xgene-hwmon.c
> index 78d9f52e2a71..1ccdd61b6d13 100644
> --- a/drivers/hwmon/xgene-hwmon.c
> +++ b/drivers/hwmon/xgene-hwmon.c
> @@ -57,12 +57,6 @@
>  	(MSG_TYPE_SET(MSG_TYPE_PWRMGMT) | \
>  	MSG_SUBTYPE_SET(hndl) | TPC_CMD_SET(cmd) | type)
>  
> -/* PCC defines */
> -#define PCC_SIGNATURE_MASK		0x50424300
> -#define PCCC_GENERATE_DB_INT		BIT(15)
> -#define PCCS_CMD_COMPLETE		BIT(0)
> -#define PCCS_SCI_DOORBEL		BIT(1)
> -#define PCCS_PLATFORM_NOTIFICATION	BIT(3)
>  /*
>   * Arbitrary retries in case the remote processor is slow to respond
>   * to PCC commands
> @@ -142,15 +136,15 @@ static int xgene_hwmon_pcc_rd(struct xgene_hwmon_dev *ctx, u32 *msg)
>  
>  	/* Write signature for subspace */
>  	WRITE_ONCE(generic_comm_base->signature,
> -		   cpu_to_le32(PCC_SIGNATURE_MASK | ctx->mbox_idx));
> +		   cpu_to_le32(PCC_SIGNATURE | ctx->mbox_idx));
>  
>  	/* Write to the shared command region */
>  	WRITE_ONCE(generic_comm_base->command,
> -		   cpu_to_le16(MSG_TYPE(msg[0]) | PCCC_GENERATE_DB_INT));
> +		   cpu_to_le16(MSG_TYPE(msg[0]) | PCC_CMD_GENERATE_DB_INTR));
>  
>  	/* Flip CMD COMPLETE bit */
>  	val = le16_to_cpu(READ_ONCE(generic_comm_base->status));
> -	val &= ~PCCS_CMD_COMPLETE;
> +	val &= ~PCC_STATUS_CMD_COMPLETE;
>  	WRITE_ONCE(generic_comm_base->status, cpu_to_le16(val));
>  
>  	/* Copy the message to the PCC comm space */
> @@ -544,7 +538,7 @@ static void xgene_hwmon_pcc_rx_cb(struct mbox_client *cl, void *msg)
>  	msg = generic_comm_base + 1;
>  	/* Check if platform sends interrupt */
>  	if (!xgene_word_tst_and_clr(&generic_comm_base->status,
> -				    PCCS_SCI_DOORBEL))
> +				    PCC_STATUS_SCI_DOORBELL))
>  		return;
>  
>  	/*
> @@ -566,7 +560,7 @@ static void xgene_hwmon_pcc_rx_cb(struct mbox_client *cl, void *msg)
>  	      TPC_CMD(((u32 *)msg)[0]) == TPC_ALARM))) {
>  		/* Check if platform completes command */
>  		if (xgene_word_tst_and_clr(&generic_comm_base->status,
> -					   PCCS_CMD_COMPLETE)) {
> +					   PCC_STATUS_CMD_COMPLETE)) {
>  			ctx->sync_msg.msg = ((u32 *)msg)[0];
>  			ctx->sync_msg.param1 = ((u32 *)msg)[1];
>  			ctx->sync_msg.param2 = ((u32 *)msg)[2];
> 
> -- 
> 2.42.0
> 

Re: [PATCH 2/3] i2c: xgene-slimpro: Migrate to use generic PCC shmem related macros

[Andi Shyti]

Hi Sudeep,

> Use the newly defined common and generic PCC shared memory region
> related macros in this driver to replace the locally defined ones.
> 
> Cc: Andi Shyti <andi.shyti@kernel.org>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

Reviewed-by: Andi Shyti <andi.shyti@kernel.org> 

Thanks,
Andi

Re: [PATCH 1/3] ACPI: PCC: Add PCC shared memory region command and status bitfields

[lihuisong (C)]

Hi Sudeep,

在 2023/9/26 20:28, Sudeep Holla 写道:
> Define the common macros to use when referring to various bitfields in
> the PCC generic communications channel command and status fields.
Can you define the bit0 macros in the "flags" for Extended PCC Subspace 
Shared Memory Region?
>
> Currently different drivers that need to use these bitfields have defined
> these locally. This common macro is intended to consolidate and replace
> those.
>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>   include/acpi/pcc.h | 11 +++++++++++
>   1 file changed, 11 insertions(+)
>
> diff --git a/include/acpi/pcc.h b/include/acpi/pcc.h
> index 73e806fe7ce7..66d9934c2ee4 100644
> --- a/include/acpi/pcc.h
> +++ b/include/acpi/pcc.h
> @@ -18,7 +18,18 @@ struct pcc_mbox_chan {
>   	u16 min_turnaround_time;
>   };
>   
> +/* Generic Communications Channel Shared Memory Region */
> +#define PCC_SIGNATURE			0x50424300
Why is this signature 0x50424300?
In ACPI spec, this signature is all 0x50434303.
> +/* Generic Communications Channel Command Field */
> +#define PCC_CMD_GENERATE_DB_INTR	BIT(15)
> +/* Generic Communications Channel Status Field */
> +#define PCC_STATUS_CMD_COMPLETE		BIT(0)
> +#define PCC_STATUS_SCI_DOORBELL		BIT(1)
> +#define PCC_STATUS_ERROR		BIT(2)
> +#define PCC_STATUS_PLATFORM_NOTIFY	BIT(3)
> +
>   #define MAX_PCC_SUBSPACES	256
> +
>   #ifdef CONFIG_PCC
>   extern struct pcc_mbox_chan *
>   pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id);
>

Re: [PATCH 0/3] ACPI: PCC: Define and use the common PCC shared memory regions related macros

[lihuisong (C)]

Hi Sudeep,

Could you please use these new common macros for kunpeng_hccs?

在 2023/9/26 20:27, Sudeep Holla 写道:
> This set of 3 small patches intend to consolidate and replace the existing
> locally defined macros within couple of PCC client drivers when accessing
> the command and status bitfields.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> Sudeep Holla (3):
>        ACPI: PCC: Add PCC shared memory region command and status bitfields
>        i2c: xgene-slimpro: Migrate to use generic PCC shmem related macros
>        hwmon: (xgene) Migrate to use generic PCC shmem related macros
>
>   drivers/hwmon/xgene-hwmon.c            | 16 +++++-----------
>   drivers/i2c/busses/i2c-xgene-slimpro.c | 16 ++++------------
>   include/acpi/pcc.h                     | 11 +++++++++++
>   3 files changed, 20 insertions(+), 23 deletions(-)
> ---
> base-commit: 6465e260f48790807eef06b583b38ca9789b6072
> change-id: 20230926-pcc_defines-24be5e33b6f3
>
> Best regards,

Re: [PATCH 1/3] ACPI: PCC: Add PCC shared memory region command and status bitfields

[Sudeep Holla]

On Wed, Sep 27, 2023 at 10:07:15AM +0800, lihuisong (C) wrote:
> Hi Sudeep,
> 
> 在 2023/9/26 20:28, Sudeep Holla 写道:
> > Define the common macros to use when referring to various bitfields in
> > the PCC generic communications channel command and status fields.
>
> Can you define the bit0 macros in the "flags" for Extended PCC Subspace
> Shared Memory Region?

Sure I will take a look and include it in v2 if applicable.

> > 
> > Currently different drivers that need to use these bitfields have defined
> > these locally. This common macro is intended to consolidate and replace
> > those.
> > 
> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > ---
> >   include/acpi/pcc.h | 11 +++++++++++
> >   1 file changed, 11 insertions(+)
> > 
> > diff --git a/include/acpi/pcc.h b/include/acpi/pcc.h
> > index 73e806fe7ce7..66d9934c2ee4 100644
> > --- a/include/acpi/pcc.h
> > +++ b/include/acpi/pcc.h
> > @@ -18,7 +18,18 @@ struct pcc_mbox_chan {
> >   	u16 min_turnaround_time;
> >   };
> > +/* Generic Communications Channel Shared Memory Region */
> > +#define PCC_SIGNATURE			0x50424300
> Why is this signature 0x50424300?

It is as per the specification.

> In ACPI spec, this signature is all 0x50434303.

No, not exactly. It is just an example.
The PCC signature - The signature of a subspace is computed by a bitwise-or
of the value 0x50434300 with the subspace ID. For example, subspace 3 has
signature 0x50434303

And I see the driver you mentioned(drivers/soc/hisilicon/kunpeng_hccs.c)
is doing the right thing. I am bit confused as why you being the author
of the driver are now confused.

-- 
Regards,
Sudeep

Re: [PATCH 0/3] ACPI: PCC: Define and use the common PCC shared memory regions related macros

[Sudeep Holla]

On Wed, Sep 27, 2023 at 10:11:21AM +0800, lihuisong (C) wrote:
> Hi Sudeep,
> 
> Could you please use these new common macros for kunpeng_hccs?

Sure, sorry for missing that. I had these changes in a branch for a while,
did check for new additions when I decided to post them.

-- 
Regards,
Sudeep

Re: [PATCH 0/3] ACPI: PCC: Define and use the common PCC shared memory regions related macros

[Rafael J. Wysocki]

On Tue, Sep 26, 2023 at 2:33 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> This set of 3 small patches intend to consolidate and replace the existing
> locally defined macros within couple of PCC client drivers when accessing
> the command and status bitfields.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> Sudeep Holla (3):
>       ACPI: PCC: Add PCC shared memory region command and status bitfields
>       i2c: xgene-slimpro: Migrate to use generic PCC shmem related macros
>       hwmon: (xgene) Migrate to use generic PCC shmem related macros
>
>  drivers/hwmon/xgene-hwmon.c            | 16 +++++-----------
>  drivers/i2c/busses/i2c-xgene-slimpro.c | 16 ++++------------
>  include/acpi/pcc.h                     | 11 +++++++++++
>  3 files changed, 20 insertions(+), 23 deletions(-)
> ---

This is fine with me.

How do you want to route it?

Re: [PATCH 0/3] ACPI: PCC: Define and use the common PCC shared memory regions related macros

[Sudeep Holla]

On Wed, Sep 27, 2023 at 04:19:21PM +0200, Rafael J. Wysocki wrote:
> On Tue, Sep 26, 2023 at 2:33 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > This set of 3 small patches intend to consolidate and replace the existing
> > locally defined macros within couple of PCC client drivers when accessing
> > the command and status bitfields.
> >
> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > ---
> > Sudeep Holla (3):
> >       ACPI: PCC: Add PCC shared memory region command and status bitfields
> >       i2c: xgene-slimpro: Migrate to use generic PCC shmem related macros
> >       hwmon: (xgene) Migrate to use generic PCC shmem related macros
> >
> >  drivers/hwmon/xgene-hwmon.c            | 16 +++++-----------
> >  drivers/i2c/busses/i2c-xgene-slimpro.c | 16 ++++------------
> >  include/acpi/pcc.h                     | 11 +++++++++++
> >  3 files changed, 20 insertions(+), 23 deletions(-)
> > ---
> 
> This is fine with me.
> 
> How do you want to route it?

I have to respin this to include another driver.

I also have 2 PCC mailbox driver changes that I wanted to send a pull request
to you. I can make this part of that PR or you can take it directly. Both
hwmon and i2c maintainers have acked now.

--
Regards,
Sudeep

Re: [PATCH 0/3] ACPI: PCC: Define and use the common PCC shared memory regions related macros

[Rafael J. Wysocki]

On Wed, Sep 27, 2023 at 4:47 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> On Wed, Sep 27, 2023 at 04:19:21PM +0200, Rafael J. Wysocki wrote:
> > On Tue, Sep 26, 2023 at 2:33 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
> > >
> > > This set of 3 small patches intend to consolidate and replace the existing
> > > locally defined macros within couple of PCC client drivers when accessing
> > > the command and status bitfields.
> > >
> > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > > ---
> > > Sudeep Holla (3):
> > >       ACPI: PCC: Add PCC shared memory region command and status bitfields
> > >       i2c: xgene-slimpro: Migrate to use generic PCC shmem related macros
> > >       hwmon: (xgene) Migrate to use generic PCC shmem related macros
> > >
> > >  drivers/hwmon/xgene-hwmon.c            | 16 +++++-----------
> > >  drivers/i2c/busses/i2c-xgene-slimpro.c | 16 ++++------------
> > >  include/acpi/pcc.h                     | 11 +++++++++++
> > >  3 files changed, 20 insertions(+), 23 deletions(-)
> > > ---
> >
> > This is fine with me.
> >
> > How do you want to route it?
>
> I have to respin this to include another driver.
>
> I also have 2 PCC mailbox driver changes that I wanted to send a pull request
> to you. I can make this part of that PR or you can take it directly. Both
> hwmon and i2c maintainers have acked now.

A PR would be convenient. :-)

Re: [PATCH 1/3] ACPI: PCC: Add PCC shared memory region command and status bitfields

[lihuisong (C)]

在 2023/9/27 21:59, Sudeep Holla 写道:
> On Wed, Sep 27, 2023 at 10:07:15AM +0800, lihuisong (C) wrote:
>> Hi Sudeep,
>>
>> 在 2023/9/26 20:28, Sudeep Holla 写道:
>>> Define the common macros to use when referring to various bitfields in
>>> the PCC generic communications channel command and status fields.
>> Can you define the bit0 macros in the "flags" for Extended PCC Subspace
>> Shared Memory Region?
> Sure I will take a look and include it in v2 if applicable.
Thanks
>
>>> Currently different drivers that need to use these bitfields have defined
>>> these locally. This common macro is intended to consolidate and replace
>>> those.
>>>
>>> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>>> ---
>>>    include/acpi/pcc.h | 11 +++++++++++
>>>    1 file changed, 11 insertions(+)
>>>
>>> diff --git a/include/acpi/pcc.h b/include/acpi/pcc.h
>>> index 73e806fe7ce7..66d9934c2ee4 100644
>>> --- a/include/acpi/pcc.h
>>> +++ b/include/acpi/pcc.h
>>> @@ -18,7 +18,18 @@ struct pcc_mbox_chan {
>>>    	u16 min_turnaround_time;
>>>    };
>>> +/* Generic Communications Channel Shared Memory Region */
>>> +#define PCC_SIGNATURE			0x50424300
>> Why is this signature 0x50424300?
> It is as per the specification.
>
>> In ACPI spec, this signature is all 0x50434303.
> No, not exactly. It is just an example.
> The PCC signature - The signature of a subspace is computed by a bitwise-or
> of the value 0x50434300 with the subspace ID. For example, subspace 3 has
> signature 0x50434303
Sorry for my mistake. I know this.
I mean, why doesn't the following macro follow spec and define this 
signature as 0x504*3*430.
"#define PCC_SIGNATURE **0x504*2*4300*"*
Because it seems that all version of ACPI spec is 0x5043430.
>
> And I see the driver you mentioned(drivers/soc/hisilicon/kunpeng_hccs.c)
> is doing the right thing. I am bit confused as why you being the author
> of the driver are now confused.
I used 0x50424300 instead of 0x50424300 according to the spec.
>

Re: [PATCH 1/3] ACPI: PCC: Add PCC shared memory region command and status bitfields

[Sudeep Holla]

On Thu, Sep 28, 2023 at 11:49:25AM +0800, lihuisong (C) wrote:
> 
> 在 2023/9/27 21:59, Sudeep Holla 写道:
> > On Wed, Sep 27, 2023 at 10:07:15AM +0800, lihuisong (C) wrote:

[...]

> > > > +/* Generic Communications Channel Shared Memory Region */
> > > > +#define PCC_SIGNATURE			0x50424300
> > > Why is this signature 0x50424300?
> > It is as per the specification.
> > 
> > > In ACPI spec, this signature is all 0x50434303.
> > No, not exactly. It is just an example.
> > The PCC signature - The signature of a subspace is computed by a bitwise-or
> > of the value 0x50434300 with the subspace ID. For example, subspace 3 has
> > signature 0x50434303
> Sorry for my mistake. I know this.
> I mean, why doesn't the following macro follow spec and define this
> signature as 0x504*3*430.
> "#define PCC_SIGNATURE **0x504*2*4300*"*
> Because it seems that all version of ACPI spec is 0x5043430.

Sorry my mistake. Stupidly the existing drivers have it wrong and I just
copied them without paying much attention. I will fix it up. It must be
0x50434300 instead of 0x50424300. If you have no other comments, can you
please ack v2 patch 4/4 changing soc kunpeng_hccs driver. I will fixup
the PCC_SIGNATURE and send it as part of my PR to Rafael.

Refer [1] for the change in PCC_SIGNATURE, sorry for missing the point. I
was confident that the existing code is correct :), but I am wrong.

-- 
Regards,
Sudeep

[1] https://git.kernel.org/sudeep.holla/l/pcc_defines

Re: [PATCH 1/3] ACPI: PCC: Add PCC shared memory region command and status bitfields

[lihuisong (C)]

在 2023/9/28 19:11, Sudeep Holla 写道:
> On Thu, Sep 28, 2023 at 11:49:25AM +0800, lihuisong (C) wrote:
>> 在 2023/9/27 21:59, Sudeep Holla 写道:
>>> On Wed, Sep 27, 2023 at 10:07:15AM +0800, lihuisong (C) wrote:
> [...]
>
>>>>> +/* Generic Communications Channel Shared Memory Region */
>>>>> +#define PCC_SIGNATURE			0x50424300
>>>> Why is this signature 0x50424300?
>>> It is as per the specification.
>>>
>>>> In ACPI spec, this signature is all 0x50434303.
>>> No, not exactly. It is just an example.
>>> The PCC signature - The signature of a subspace is computed by a bitwise-or
>>> of the value 0x50434300 with the subspace ID. For example, subspace 3 has
>>> signature 0x50434303
>> Sorry for my mistake. I know this.
>> I mean, why doesn't the following macro follow spec and define this
>> signature as 0x504*3*430.
>> "#define PCC_SIGNATURE **0x504*2*4300*"*
>> Because it seems that all version of ACPI spec is 0x5043430.
> Sorry my mistake. Stupidly the existing drivers have it wrong and I just
> copied them without paying much attention. I will fix it up. It must be
> 0x50434300 instead of 0x50424300. If you have no other comments, can you
They are very similar.😁
> please ack v2 patch 4/4 changing soc kunpeng_hccs driver. I will fixup
> the PCC_SIGNATURE and send it as part of my PR to Rafael.
ok
>
> Refer [1] for the change in PCC_SIGNATURE, sorry for missing the point. I
> was confident that the existing code is correct :), but I am wrong.
>