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

[PATCH v2 0/4] 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>
---
Changes in v2:
- Added review/ack tags from Andi Shyti(I2C) and Guenter Roeck(hwmon)
- Added bitfields for Initiator Responder Communications Channel flags as well
- Migrated kunpeng_hccs soc driver to use generic PCC shmem related macros
- Link to v1: https://lore.kernel.org/r/20230926-pcc_defines-v1-0-0f925a1658fd@arm.com

---
Sudeep Holla (4):
      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
      soc: kunpeng_hccs: Migrate to use generic PCC shmem related macros

 drivers/hwmon/xgene-hwmon.c            | 16 +++++-----------
 drivers/i2c/busses/i2c-xgene-slimpro.c | 16 ++++------------
 drivers/soc/hisilicon/kunpeng_hccs.c   |  8 ++------
 include/acpi/pcc.h                     | 13 +++++++++++++
 4 files changed, 24 insertions(+), 29 deletions(-)
---
base-commit: 6465e260f48790807eef06b583b38ca9789b6072
change-id: 20230926-pcc_defines-24be5e33b6f3

Best regards,
-- 
Regards,
Sudeep

[PATCH v2 1/4] 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 | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/include/acpi/pcc.h b/include/acpi/pcc.h
index 73e806fe7ce7..021891a7434f 100644
--- a/include/acpi/pcc.h
+++ b/include/acpi/pcc.h
@@ -18,7 +18,20 @@ 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)
+/* Initiator Responder Communications Channel Flags */
+#define PCC_CMD_COMPLETION_NOTIFY	BIT(0)
+
 #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

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

[Rafael J. Wysocki]

On Wed, Sep 27, 2023 at 6:32 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> 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 | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/include/acpi/pcc.h b/include/acpi/pcc.h
> index 73e806fe7ce7..021891a7434f 100644
> --- a/include/acpi/pcc.h
> +++ b/include/acpi/pcc.h
> @@ -18,7 +18,20 @@ 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)
> +/* Initiator Responder Communications Channel Flags */
> +#define PCC_CMD_COMPLETION_NOTIFY      BIT(0)
> +
>  #define MAX_PCC_SUBSPACES      256
> +
>  #ifdef CONFIG_PCC
>  extern struct pcc_mbox_chan *
>  pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id);
>
> --

Do you want me to pick up this lot?

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

[Sudeep Holla]

On Tue, Oct 03, 2023 at 03:29:16PM +0200, Rafael J. Wysocki wrote:
> On Wed, Sep 27, 2023 at 6:32 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > 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 | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> >
> > diff --git a/include/acpi/pcc.h b/include/acpi/pcc.h
> > index 73e806fe7ce7..021891a7434f 100644
> > --- a/include/acpi/pcc.h
> > +++ b/include/acpi/pcc.h
> > @@ -18,7 +18,20 @@ 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)
> > +/* Initiator Responder Communications Channel Flags */
> > +#define PCC_CMD_COMPLETION_NOTIFY      BIT(0)
> > +
> >  #define MAX_PCC_SUBSPACES      256
> > +
> >  #ifdef CONFIG_PCC
> >  extern struct pcc_mbox_chan *
> >  pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id);
> >
> > --
> 
> Do you want me to pick up this lot?

I have applied this to me branch [1]. It also has long pending PCC driver
changes. I will send the pull request by end of this week.

-- 
Regards,
Sudeep

[1] https://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git/log/?h=for-next/pcc/updates

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

[Rafael J. Wysocki]

On Tue, Oct 3, 2023 at 4:29 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> On Tue, Oct 03, 2023 at 03:29:16PM +0200, Rafael J. Wysocki wrote:
> > On Wed, Sep 27, 2023 at 6:32 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
> > >
> > > 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 | 13 +++++++++++++
> > >  1 file changed, 13 insertions(+)
> > >
> > > diff --git a/include/acpi/pcc.h b/include/acpi/pcc.h
> > > index 73e806fe7ce7..021891a7434f 100644
> > > --- a/include/acpi/pcc.h
> > > +++ b/include/acpi/pcc.h
> > > @@ -18,7 +18,20 @@ 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)
> > > +/* Initiator Responder Communications Channel Flags */
> > > +#define PCC_CMD_COMPLETION_NOTIFY      BIT(0)
> > > +
> > >  #define MAX_PCC_SUBSPACES      256
> > > +
> > >  #ifdef CONFIG_PCC
> > >  extern struct pcc_mbox_chan *
> > >  pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id);
> > >
> > > --
> >
> > Do you want me to pick up this lot?
>
> I have applied this to me branch [1]. It also has long pending PCC driver
> changes. I will send the pull request by end of this week.

Sounds good, thanks!

[PATCH v2 2/4] 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.

Reviewed-by: 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

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

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 356 bytes --]

On Wed, Sep 27, 2023 at 05:26:11PM +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.
> 
> Reviewed-by: Andi Shyti <andi.shyti@kernel.org>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

Acked-by: Wolfram Sang <wsa@kernel.org>


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2 3/4] 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>
Acked-by: 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

[PATCH v2 4/4] soc: kunpeng_hccs: 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: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/soc/hisilicon/kunpeng_hccs.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/soc/hisilicon/kunpeng_hccs.c b/drivers/soc/hisilicon/kunpeng_hccs.c
index f3810d9d1caa..27a96cafd1ea 100644
--- a/drivers/soc/hisilicon/kunpeng_hccs.c
+++ b/drivers/soc/hisilicon/kunpeng_hccs.c
@@ -31,10 +31,6 @@
 
 #include "kunpeng_hccs.h"
 
-/* PCC defines */
-#define HCCS_PCC_SIGNATURE_MASK		0x50434300
-#define HCCS_PCC_STATUS_CMD_COMPLETE	BIT(0)
-
 /*
  * Arbitrary retries in case the remote processor is slow to respond
  * to PCC commands
@@ -187,7 +183,7 @@ static int hccs_check_chan_cmd_complete(struct hccs_dev *hdev)
 	 * deadline_us(timeout_us) until PCC command complete bit is set(cond)
 	 */
 	ret = readw_poll_timeout(&comm_base->status, status,
-				 status & HCCS_PCC_STATUS_CMD_COMPLETE,
+				 status & PCC_STATUS_CMD_COMPLETE,
 				 HCCS_POLL_STATUS_TIME_INTERVAL_US,
 				 cl_info->deadline_us);
 	if (unlikely(ret))
@@ -208,7 +204,7 @@ static int hccs_pcc_cmd_send(struct hccs_dev *hdev, u8 cmd,
 	int ret;
 
 	/* Write signature for this subspace */
-	tmp.signature = HCCS_PCC_SIGNATURE_MASK | hdev->chan_id;
+	tmp.signature = PCC_SIGNATURE | hdev->chan_id;
 	/* Write to the shared command region */
 	tmp.command = cmd;
 	/* Clear cmd complete bit */

-- 
2.42.0

Re: [PATCH v2 4/4] soc: kunpeng_hccs: Migrate to use generic PCC shmem related macros

[lihuisong (C)]

lgtm,
Reviewed-by: Huisong Li <lihuisong@huawei.com>


在 2023/9/28 0:26, 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: Huisong Li <lihuisong@huawei.com>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>   drivers/soc/hisilicon/kunpeng_hccs.c | 8 ++------
>   1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/soc/hisilicon/kunpeng_hccs.c b/drivers/soc/hisilicon/kunpeng_hccs.c
> index f3810d9d1caa..27a96cafd1ea 100644
> --- a/drivers/soc/hisilicon/kunpeng_hccs.c
> +++ b/drivers/soc/hisilicon/kunpeng_hccs.c
> @@ -31,10 +31,6 @@
>   
>   #include "kunpeng_hccs.h"
>   
> -/* PCC defines */
> -#define HCCS_PCC_SIGNATURE_MASK		0x50434300
> -#define HCCS_PCC_STATUS_CMD_COMPLETE	BIT(0)
> -
>   /*
>    * Arbitrary retries in case the remote processor is slow to respond
>    * to PCC commands
> @@ -187,7 +183,7 @@ static int hccs_check_chan_cmd_complete(struct hccs_dev *hdev)
>   	 * deadline_us(timeout_us) until PCC command complete bit is set(cond)
>   	 */
>   	ret = readw_poll_timeout(&comm_base->status, status,
> -				 status & HCCS_PCC_STATUS_CMD_COMPLETE,
> +				 status & PCC_STATUS_CMD_COMPLETE,
>   				 HCCS_POLL_STATUS_TIME_INTERVAL_US,
>   				 cl_info->deadline_us);
>   	if (unlikely(ret))
> @@ -208,7 +204,7 @@ static int hccs_pcc_cmd_send(struct hccs_dev *hdev, u8 cmd,
>   	int ret;
>   
>   	/* Write signature for this subspace */
> -	tmp.signature = HCCS_PCC_SIGNATURE_MASK | hdev->chan_id;
> +	tmp.signature = PCC_SIGNATURE | hdev->chan_id;
>   	/* Write to the shared command region */
>   	tmp.command = cmd;
>   	/* Clear cmd complete bit */
>

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

[Sudeep Holla]

On Wed, 27 Sep 2023 17:26:09 +0100, Sudeep Holla 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.
>

Applied to sudeep.holla/linux (for-next/pcc/updates), thanks!

[1/4] ACPI: PCC: Add PCC shared memory region command and status bitfields
      [fixed the signature value]
      https://git.kernel.org/sudeep.holla/c/55d235ebb684
[2/4] i2c: xgene-slimpro: Migrate to use generic PCC shmem related macros
      https://git.kernel.org/sudeep.holla/c/89a4ad1f437c
[3/4] hwmon: (xgene) Migrate to use generic PCC shmem related macros
      https://git.kernel.org/sudeep.holla/c/2cf39b806be7
[4/4] soc: kunpeng_hccs: Migrate to use generic PCC shmem related macros
      https://git.kernel.org/sudeep.holla/c/a46e42c09798
--
Regards,
Sudeep