2023-09-27 16:41:18

by Sudeep Holla

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

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 <[email protected]>
---
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/[email protected]

---
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


2023-09-27 16:41:25

by Sudeep Holla

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

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 <[email protected]>
Signed-off-by: Sudeep Holla <[email protected]>
---
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

2023-09-27 16:41:47

by Sudeep Holla

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

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 <[email protected]>
Signed-off-by: Sudeep Holla <[email protected]>
---
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

2023-09-28 11:42:51

by Huisong Li

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

lgtm,
Reviewed-by: Huisong Li <[email protected]>


在 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 <[email protected]>
> Signed-off-by: Sudeep Holla <[email protected]>
> ---
> 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 */
>

2023-09-29 11:56:46

by Sudeep Holla

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

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

2023-10-03 13:29:39

by Rafael J. Wysocki

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

On Wed, Sep 27, 2023 at 6:32 PM Sudeep Holla <[email protected]> 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 <[email protected]>
> Signed-off-by: Sudeep Holla <[email protected]>
> ---
> 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?

2023-10-03 14:30:48

by Sudeep Holla

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

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 <[email protected]> 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 <[email protected]>
> > Signed-off-by: Sudeep Holla <[email protected]>
> > ---
> > 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

2023-10-03 15:06:35

by Rafael J. Wysocki

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

On Tue, Oct 3, 2023 at 4:29 PM Sudeep Holla <[email protected]> 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 <[email protected]> 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 <[email protected]>
> > > Signed-off-by: Sudeep Holla <[email protected]>
> > > ---
> > > 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!