2023-11-30 13:46:14

by Huisong Li

[permalink] [raw]
Subject: [PATCH v2 0/4] soc: hisilicon: kunpeng_hccs: Support the platform with PCC type3 and interrupt ack

The main purpose of this series is to support the platform with PCC type3
and interrupt ack. At the same time, this series also fix some incorrect
format strings, add failure log for no _CRS method and remove an unused
blank line.

---
v2:
- using a version specific structure to replace device version according
to Jonathan's advice.
- add a new patch that remove an unused blank line.

Huisong Li (4):
soc: hisilicon: kunpeng_hccs: Fix some incorrect format strings
soc: hisilicon: kunpeng_hccs: Add failure log for no _CRS method
soc: hisilicon: kunpeng_hccs: remove an unused blank line
soc: hisilicon: kunpeng_hccs: Support the platform with PCC type3 and
interrupt ack

drivers/soc/hisilicon/kunpeng_hccs.c | 153 +++++++++++++++++++++------
drivers/soc/hisilicon/kunpeng_hccs.h | 15 +++
2 files changed, 135 insertions(+), 33 deletions(-)

--
2.33.0


2023-11-30 13:46:23

by Huisong Li

[permalink] [raw]
Subject: [PATCH v2 2/4] soc: hisilicon: kunpeng_hccs: Add failure log for no _CRS method

Driver gets the PCC channel id by using the PCC GAS in _CRS.
But, currently, if the firmware has no _CRS method on platform, there
is not any failure log. So this patch adds the log for this.

Signed-off-by: Huisong Li <[email protected]>
Reviewed-by: Jonathan Cameron <[email protected]>
---
drivers/soc/hisilicon/kunpeng_hccs.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/hisilicon/kunpeng_hccs.c b/drivers/soc/hisilicon/kunpeng_hccs.c
index dad6235dbf1a..fd3ca0eb8175 100644
--- a/drivers/soc/hisilicon/kunpeng_hccs.c
+++ b/drivers/soc/hisilicon/kunpeng_hccs.c
@@ -85,8 +85,10 @@ static int hccs_get_pcc_chan_id(struct hccs_dev *hdev)
struct hccs_register_ctx ctx = {0};
acpi_status status;

- if (!acpi_has_method(handle, METHOD_NAME__CRS))
+ if (!acpi_has_method(handle, METHOD_NAME__CRS)) {
+ dev_err(hdev->dev, "No _CRS method.\n");
return -ENODEV;
+ }

ctx.dev = hdev->dev;
status = acpi_walk_resources(handle, METHOD_NAME__CRS,
--
2.33.0

2023-11-30 13:46:25

by Huisong Li

[permalink] [raw]
Subject: [PATCH v2 4/4] soc: hisilicon: kunpeng_hccs: Support the platform with PCC type3 and interrupt ack

Support the platform with PCC type3 and interrupt ack. And a version
specific structure is introduced to handle the difference between the
device in the code.

Signed-off-by: Huisong Li <[email protected]>
---
drivers/soc/hisilicon/kunpeng_hccs.c | 136 ++++++++++++++++++++++-----
drivers/soc/hisilicon/kunpeng_hccs.h | 15 +++
2 files changed, 126 insertions(+), 25 deletions(-)

diff --git a/drivers/soc/hisilicon/kunpeng_hccs.c b/drivers/soc/hisilicon/kunpeng_hccs.c
index 15125f1e0f3e..d2302ff8c0e9 100644
--- a/drivers/soc/hisilicon/kunpeng_hccs.c
+++ b/drivers/soc/hisilicon/kunpeng_hccs.c
@@ -110,6 +110,14 @@ static void hccs_chan_tx_done(struct mbox_client *cl, void *msg, int ret)
*(u8 *)msg, ret);
}

+static void hccs_pcc_rx_callback(struct mbox_client *cl, void *mssg)
+{
+ struct hccs_mbox_client_info *cl_info =
+ container_of(cl, struct hccs_mbox_client_info, client);
+
+ complete(&cl_info->done);
+}
+
static void hccs_unregister_pcc_channel(struct hccs_dev *hdev)
{
struct hccs_mbox_client_info *cl_info = &hdev->cl_info;
@@ -131,6 +139,9 @@ static int hccs_register_pcc_channel(struct hccs_dev *hdev)
cl->tx_block = false;
cl->knows_txdone = true;
cl->tx_done = hccs_chan_tx_done;
+ cl->rx_callback = hdev->verspec_data->rx_callback;
+ init_completion(&cl_info->done);
+
pcc_chan = pcc_mbox_request_channel(cl, hdev->chan_id);
if (IS_ERR(pcc_chan)) {
dev_err(dev, "PPC channel request failed.\n");
@@ -147,10 +158,16 @@ static int hccs_register_pcc_channel(struct hccs_dev *hdev)
*/
cl_info->deadline_us =
HCCS_PCC_CMD_WAIT_RETRIES_NUM * pcc_chan->latency;
- if (cl_info->mbox_chan->mbox->txdone_irq) {
+ if (!hdev->verspec_data->has_txdone_irq &&
+ cl_info->mbox_chan->mbox->txdone_irq) {
dev_err(dev, "PCC IRQ in PCCT is enabled.\n");
rc = -EINVAL;
goto err_mbx_channel_free;
+ } else if (hdev->verspec_data->has_txdone_irq &&
+ !cl_info->mbox_chan->mbox->txdone_irq) {
+ dev_err(dev, "PCC IRQ in PCCT isn't supported.\n");
+ rc = -EINVAL;
+ goto err_mbx_channel_free;
}

if (pcc_chan->shmem_base_addr) {
@@ -172,7 +189,7 @@ static int hccs_register_pcc_channel(struct hccs_dev *hdev)
return rc;
}

-static int hccs_check_chan_cmd_complete(struct hccs_dev *hdev)
+static inline int hccs_wait_cmd_complete_by_poll(struct hccs_dev *hdev)
{
struct hccs_mbox_client_info *cl_info = &hdev->cl_info;
struct acpi_pcct_shared_memory __iomem *comm_base =
@@ -194,30 +211,75 @@ static int hccs_check_chan_cmd_complete(struct hccs_dev *hdev)
return ret;
}

+static inline int hccs_wait_cmd_complete_by_irq(struct hccs_dev *hdev)
+{
+ struct hccs_mbox_client_info *cl_info = &hdev->cl_info;
+ int ret = 0;
+
+ if (!wait_for_completion_timeout(&cl_info->done,
+ usecs_to_jiffies(cl_info->deadline_us))) {
+ dev_err(hdev->dev, "PCC command executed timeout!\n");
+ ret = -ETIMEDOUT;
+ }
+
+ return ret;
+}
+
+static inline void hccs_fill_pcc_shared_mem_region(struct hccs_dev *hdev,
+ u8 cmd,
+ struct hccs_desc *desc,
+ void __iomem *comm_space,
+ u16 space_size)
+{
+ struct acpi_pcct_shared_memory tmp = {
+ .signature = PCC_SIGNATURE | hdev->chan_id,
+ .command = cmd,
+ .status = 0,
+ };
+
+ memcpy_toio(hdev->cl_info.pcc_comm_addr, (void *)&tmp,
+ sizeof(struct acpi_pcct_shared_memory));
+
+ /* Copy the message to the PCC comm space */
+ memcpy_toio(comm_space, (void *)desc, space_size);
+}
+
+static inline void hccs_fill_ext_pcc_shared_mem_region(struct hccs_dev *hdev,
+ u8 cmd,
+ struct hccs_desc *desc,
+ void __iomem *comm_space,
+ u16 space_size)
+{
+ struct acpi_pcct_ext_pcc_shared_memory tmp = {
+ .signature = PCC_SIGNATURE | hdev->chan_id,
+ .flags = PCC_CMD_COMPLETION_NOTIFY,
+ .length = HCCS_PCC_SHARE_MEM_BYTES,
+ .command = cmd,
+ };
+
+ memcpy_toio(hdev->cl_info.pcc_comm_addr, (void *)&tmp,
+ sizeof(struct acpi_pcct_ext_pcc_shared_memory));
+
+ /* Copy the message to the PCC comm space */
+ memcpy_toio(comm_space, (void *)desc, space_size);
+}
+
static int hccs_pcc_cmd_send(struct hccs_dev *hdev, u8 cmd,
struct hccs_desc *desc)
{
+ const struct hccs_verspecific_data *verspec_data = hdev->verspec_data;
struct hccs_mbox_client_info *cl_info = &hdev->cl_info;
- void __iomem *comm_space = cl_info->pcc_comm_addr +
- sizeof(struct acpi_pcct_shared_memory);
struct hccs_fw_inner_head *fw_inner_head;
- struct acpi_pcct_shared_memory tmp = {0};
- u16 comm_space_size;
+ void __iomem *comm_space;
+ u16 space_size;
int ret;

- /* Write signature for this subspace */
- tmp.signature = PCC_SIGNATURE | hdev->chan_id;
- /* Write to the shared command region */
- tmp.command = cmd;
- /* Clear cmd complete bit */
- tmp.status = 0;
- memcpy_toio(cl_info->pcc_comm_addr, (void *)&tmp,
- sizeof(struct acpi_pcct_shared_memory));
-
- /* Copy the message to the PCC comm space */
- comm_space_size = HCCS_PCC_SHARE_MEM_BYTES -
- sizeof(struct acpi_pcct_shared_memory);
- memcpy_toio(comm_space, (void *)desc, comm_space_size);
+ comm_space = cl_info->pcc_comm_addr + verspec_data->shared_mem_size;
+ space_size = HCCS_PCC_SHARE_MEM_BYTES - verspec_data->shared_mem_size;
+ verspec_data->fill_pcc_shared_mem(hdev, cmd, desc,
+ comm_space, space_size);
+ if (verspec_data->has_txdone_irq)
+ reinit_completion(&cl_info->done);

/* Ring doorbell */
ret = mbox_send_message(cl_info->mbox_chan, &cmd);
@@ -227,13 +289,12 @@ static int hccs_pcc_cmd_send(struct hccs_dev *hdev, u8 cmd,
goto end;
}

- /* Wait for completion */
- ret = hccs_check_chan_cmd_complete(hdev);
+ ret = verspec_data->wait_cmd_complete(hdev);
if (ret)
goto end;

/* Copy response data */
- memcpy_fromio((void *)desc, comm_space, comm_space_size);
+ memcpy_fromio((void *)desc, comm_space, space_size);
fw_inner_head = &desc->rsp.fw_inner_head;
if (fw_inner_head->retStatus) {
dev_err(hdev->dev, "Execute PCC command failed, error code = %u.\n",
@@ -242,7 +303,10 @@ static int hccs_pcc_cmd_send(struct hccs_dev *hdev, u8 cmd,
}

end:
- mbox_client_txdone(cl_info->mbox_chan, ret);
+ if (verspec_data->has_txdone_irq)
+ mbox_chan_txdone(cl_info->mbox_chan, ret);
+ else
+ mbox_client_txdone(cl_info->mbox_chan, ret);
return ret;
}

@@ -1213,6 +1277,11 @@ static int hccs_probe(struct platform_device *pdev)
hdev->dev = &pdev->dev;
platform_set_drvdata(pdev, hdev);

+ /*
+ * Here would never be failure as the driver and device has been matched.
+ */
+ hdev->verspec_data = acpi_device_get_match_data(hdev->dev);
+
mutex_init(&hdev->lock);
rc = hccs_get_pcc_chan_id(hdev);
if (rc)
@@ -1249,9 +1318,26 @@ static void hccs_remove(struct platform_device *pdev)
hccs_unregister_pcc_channel(hdev);
}

+static const struct hccs_verspecific_data hisi04b1_verspec_data = {
+ .rx_callback = NULL,
+ .wait_cmd_complete = hccs_wait_cmd_complete_by_poll,
+ .fill_pcc_shared_mem = hccs_fill_pcc_shared_mem_region,
+ .shared_mem_size = sizeof(struct acpi_pcct_shared_memory),
+ .has_txdone_irq = false,
+};
+
+static const struct hccs_verspecific_data hisi04b2_verspec_data = {
+ .rx_callback = hccs_pcc_rx_callback,
+ .wait_cmd_complete = hccs_wait_cmd_complete_by_irq,
+ .fill_pcc_shared_mem = hccs_fill_ext_pcc_shared_mem_region,
+ .shared_mem_size = sizeof(struct acpi_pcct_ext_pcc_shared_memory),
+ .has_txdone_irq = true,
+};
+
static const struct acpi_device_id hccs_acpi_match[] = {
- { "HISI04B1"},
- { ""},
+ { "HISI04B1", (unsigned long)&hisi04b1_verspec_data},
+ { "HISI04B2", (unsigned long)&hisi04b2_verspec_data},
+ { }
};
MODULE_DEVICE_TABLE(acpi, hccs_acpi_match);

diff --git a/drivers/soc/hisilicon/kunpeng_hccs.h b/drivers/soc/hisilicon/kunpeng_hccs.h
index 6012d2776028..c3adbc01b471 100644
--- a/drivers/soc/hisilicon/kunpeng_hccs.h
+++ b/drivers/soc/hisilicon/kunpeng_hccs.h
@@ -51,11 +51,26 @@ struct hccs_mbox_client_info {
struct pcc_mbox_chan *pcc_chan;
u64 deadline_us;
void __iomem *pcc_comm_addr;
+ struct completion done;
+};
+
+struct hccs_desc;
+
+struct hccs_verspecific_data {
+ void (*rx_callback)(struct mbox_client *cl, void *mssg);
+ int (*wait_cmd_complete)(struct hccs_dev *hdev);
+ void (*fill_pcc_shared_mem)(struct hccs_dev *hdev,
+ u8 cmd, struct hccs_desc *desc,
+ void __iomem *comm_space,
+ u16 space_size);
+ u16 shared_mem_size;
+ bool has_txdone_irq;
};

struct hccs_dev {
struct device *dev;
struct acpi_device *acpi_dev;
+ const struct hccs_verspecific_data *verspec_data;
u64 caps;
u8 chip_num;
struct hccs_chip_info *chips;
--
2.33.0

2023-11-30 13:46:26

by Huisong Li

[permalink] [raw]
Subject: [PATCH v2 3/4] soc: hisilicon: kunpeng_hccs: remove an unused blank line

Remove an unused blank line.

Signed-off-by: Huisong Li <[email protected]>
---
drivers/soc/hisilicon/kunpeng_hccs.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/soc/hisilicon/kunpeng_hccs.c b/drivers/soc/hisilicon/kunpeng_hccs.c
index fd3ca0eb8175..15125f1e0f3e 100644
--- a/drivers/soc/hisilicon/kunpeng_hccs.c
+++ b/drivers/soc/hisilicon/kunpeng_hccs.c
@@ -529,7 +529,6 @@ static int hccs_get_all_port_info_on_die(struct hccs_dev *hdev,

static int hccs_query_all_port_info_on_platform(struct hccs_dev *hdev)
{
-
struct device *dev = hdev->dev;
struct hccs_chip_info *chip;
struct hccs_die_info *die;
--
2.33.0

2023-11-30 13:46:54

by Huisong Li

[permalink] [raw]
Subject: [PATCH v2 1/4] soc: hisilicon: kunpeng_hccs: Fix some incorrect format strings

Fix some incorrect format strings.

Signed-off-by: Huisong Li <[email protected]>
Reviewed-by: Jonathan Cameron <[email protected]>
---
drivers/soc/hisilicon/kunpeng_hccs.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/soc/hisilicon/kunpeng_hccs.c b/drivers/soc/hisilicon/kunpeng_hccs.c
index e31791659560..dad6235dbf1a 100644
--- a/drivers/soc/hisilicon/kunpeng_hccs.c
+++ b/drivers/soc/hisilicon/kunpeng_hccs.c
@@ -155,7 +155,7 @@ static int hccs_register_pcc_channel(struct hccs_dev *hdev)
cl_info->pcc_comm_addr = ioremap(pcc_chan->shmem_base_addr,
pcc_chan->shmem_size);
if (!cl_info->pcc_comm_addr) {
- dev_err(dev, "Failed to ioremap PCC communication region for channel-%d.\n",
+ dev_err(dev, "Failed to ioremap PCC communication region for channel-%u.\n",
hdev->chan_id);
rc = -ENOMEM;
goto err_mbx_channel_free;
@@ -1097,7 +1097,7 @@ static int hccs_create_hccs_dir(struct hccs_dev *hdev,
int ret;

ret = kobject_init_and_add(&port->kobj, &hccs_port_type,
- &die->kobj, "hccs%d", port->port_id);
+ &die->kobj, "hccs%u", port->port_id);
if (ret) {
kobject_put(&port->kobj);
return ret;
@@ -1115,7 +1115,7 @@ static int hccs_create_die_dir(struct hccs_dev *hdev,
u16 i;

ret = kobject_init_and_add(&die->kobj, &hccs_die_type,
- &chip->kobj, "die%d", die->die_id);
+ &chip->kobj, "die%u", die->die_id);
if (ret) {
kobject_put(&die->kobj);
return ret;
@@ -1125,7 +1125,7 @@ static int hccs_create_die_dir(struct hccs_dev *hdev,
port = &die->ports[i];
ret = hccs_create_hccs_dir(hdev, die, port);
if (ret) {
- dev_err(hdev->dev, "create hccs%d dir failed.\n",
+ dev_err(hdev->dev, "create hccs%u dir failed.\n",
port->port_id);
goto err;
}
@@ -1147,7 +1147,7 @@ static int hccs_create_chip_dir(struct hccs_dev *hdev,
u16 id;

ret = kobject_init_and_add(&chip->kobj, &hccs_chip_type,
- &hdev->dev->kobj, "chip%d", chip->chip_id);
+ &hdev->dev->kobj, "chip%u", chip->chip_id);
if (ret) {
kobject_put(&chip->kobj);
return ret;
@@ -1178,7 +1178,7 @@ static int hccs_create_topo_dirs(struct hccs_dev *hdev)
chip = &hdev->chips[id];
ret = hccs_create_chip_dir(hdev, chip);
if (ret) {
- dev_err(hdev->dev, "init chip%d dir failed!\n", id);
+ dev_err(hdev->dev, "init chip%u dir failed!\n", id);
goto err;
}
}
--
2.33.0

2023-11-30 14:43:02

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 3/4] soc: hisilicon: kunpeng_hccs: remove an unused blank line

On Thu, 30 Nov 2023 21:45:49 +0800
Huisong Li <[email protected]> wrote:

> Remove an unused blank line.
>
> Signed-off-by: Huisong Li <[email protected]>
FWIW
Reviewed-by: Jonathan Cameron <[email protected]>

> ---
> drivers/soc/hisilicon/kunpeng_hccs.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/soc/hisilicon/kunpeng_hccs.c b/drivers/soc/hisilicon/kunpeng_hccs.c
> index fd3ca0eb8175..15125f1e0f3e 100644
> --- a/drivers/soc/hisilicon/kunpeng_hccs.c
> +++ b/drivers/soc/hisilicon/kunpeng_hccs.c
> @@ -529,7 +529,6 @@ static int hccs_get_all_port_info_on_die(struct hccs_dev *hdev,
>
> static int hccs_query_all_port_info_on_platform(struct hccs_dev *hdev)
> {
> -
> struct device *dev = hdev->dev;
> struct hccs_chip_info *chip;
> struct hccs_die_info *die;

2023-11-30 14:49:20

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] soc: hisilicon: kunpeng_hccs: Support the platform with PCC type3 and interrupt ack

On Thu, 30 Nov 2023 21:45:50 +0800
Huisong Li <[email protected]> wrote:

> Support the platform with PCC type3 and interrupt ack. And a version
> specific structure is introduced to handle the difference between the
> device in the code.
>
> Signed-off-by: Huisong Li <[email protected]>

Hi.

A few trivial things inline but now looks good to me!

Reviewed-by: Jonathan Cameron <[email protected]>

> ---
> drivers/soc/hisilicon/kunpeng_hccs.c | 136 ++++++++++++++++++++++-----
> drivers/soc/hisilicon/kunpeng_hccs.h | 15 +++
> 2 files changed, 126 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/soc/hisilicon/kunpeng_hccs.c b/drivers/soc/hisilicon/kunpeng_hccs.c
> index 15125f1e0f3e..d2302ff8c0e9 100644
> --- a/drivers/soc/hisilicon/kunpeng_hccs.c
> +++ b/drivers/soc/hisilicon/kunpeng_hccs.c

...

>
> -static int hccs_check_chan_cmd_complete(struct hccs_dev *hdev)
> +static inline int hccs_wait_cmd_complete_by_poll(struct hccs_dev *hdev)

Why inline? Do we have numbers to support this hint to the compiler being
useful?

> {
> struct hccs_mbox_client_info *cl_info = &hdev->cl_info;
> struct acpi_pcct_shared_memory __iomem *comm_base =
> @@ -194,30 +211,75 @@ static int hccs_check_chan_cmd_complete(struct hccs_dev *hdev)
> return ret;
> }
>
> +static inline int hccs_wait_cmd_complete_by_irq(struct hccs_dev *hdev)
> +{
> + struct hccs_mbox_client_info *cl_info = &hdev->cl_info;
> + int ret = 0;

Drop ret...

> +
> + if (!wait_for_completion_timeout(&cl_info->done,
> + usecs_to_jiffies(cl_info->deadline_us))) {
> + dev_err(hdev->dev, "PCC command executed timeout!\n");
> + ret = -ETIMEDOUT;
return -TIMEDOUT;
...
> + }
> +
> + return ret;
return 0;

> +}

> +static const struct hccs_verspecific_data hisi04b1_verspec_data = {
> + .rx_callback = NULL,

It's harmless but no need to set callback to NULL. Maybe it acts as documentation?
It's common practice to just let C spec guarantees initialize any not implemented callbacks
to 0 without them needing to be done explicitly.

> + .wait_cmd_complete = hccs_wait_cmd_complete_by_poll,
> + .fill_pcc_shared_mem = hccs_fill_pcc_shared_mem_region,
> + .shared_mem_size = sizeof(struct acpi_pcct_shared_memory),
> + .has_txdone_irq = false,
> +};
> +
> +static const struct hccs_verspecific_data hisi04b2_verspec_data = {
> + .rx_callback = hccs_pcc_rx_callback,
> + .wait_cmd_complete = hccs_wait_cmd_complete_by_irq,
> + .fill_pcc_shared_mem = hccs_fill_ext_pcc_shared_mem_region,
> + .shared_mem_size = sizeof(struct acpi_pcct_ext_pcc_shared_memory),
> + .has_txdone_irq = true,
> +};

2023-12-01 02:46:39

by Huisong Li

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] soc: hisilicon: kunpeng_hccs: Support the platform with PCC type3 and interrupt ack


在 2023/11/30 22:49, Jonathan Cameron 写道:
> On Thu, 30 Nov 2023 21:45:50 +0800
> Huisong Li <[email protected]> wrote:
>
>> Support the platform with PCC type3 and interrupt ack. And a version
>> specific structure is introduced to handle the difference between the
>> device in the code.
>>
>> Signed-off-by: Huisong Li <[email protected]>
> Hi.
>
> A few trivial things inline but now looks good to me!
>
> Reviewed-by: Jonathan Cameron <[email protected]>
>
>> ---
>> drivers/soc/hisilicon/kunpeng_hccs.c | 136 ++++++++++++++++++++++-----
>> drivers/soc/hisilicon/kunpeng_hccs.h | 15 +++
>> 2 files changed, 126 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/soc/hisilicon/kunpeng_hccs.c b/drivers/soc/hisilicon/kunpeng_hccs.c
>> index 15125f1e0f3e..d2302ff8c0e9 100644
>> --- a/drivers/soc/hisilicon/kunpeng_hccs.c
>> +++ b/drivers/soc/hisilicon/kunpeng_hccs.c
> ...
>
>>
>> -static int hccs_check_chan_cmd_complete(struct hccs_dev *hdev)
>> +static inline int hccs_wait_cmd_complete_by_poll(struct hccs_dev *hdev)
> Why inline? Do we have numbers to support this hint to the compiler being
> useful?
No testing for this, but here might not be really useful.
So will remove this inline.
>> {
>> struct hccs_mbox_client_info *cl_info = &hdev->cl_info;
>> struct acpi_pcct_shared_memory __iomem *comm_base =
>> @@ -194,30 +211,75 @@ static int hccs_check_chan_cmd_complete(struct hccs_dev *hdev)
>> return ret;
>> }
>>
>> +static inline int hccs_wait_cmd_complete_by_irq(struct hccs_dev *hdev)
>> +{
>> + struct hccs_mbox_client_info *cl_info = &hdev->cl_info;
>> + int ret = 0;
> Drop ret...
Ack
>
>> +
>> + if (!wait_for_completion_timeout(&cl_info->done,
>> + usecs_to_jiffies(cl_info->deadline_us))) {
>> + dev_err(hdev->dev, "PCC command executed timeout!\n");
>> + ret = -ETIMEDOUT;
> return -TIMEDOUT;
> ...
>> + }
>> +
>> + return ret;
> return 0;
Ack
>> +}
>> +static const struct hccs_verspecific_data hisi04b1_verspec_data = {
>> + .rx_callback = NULL,
> It's harmless but no need to set callback to NULL. Maybe it acts as documentation?
Just to explicitly assign value and show the difference between the devices.
> It's common practice to just let C spec guarantees initialize any not implemented callbacks
> to 0 without them needing to be done explicitly.
Correct, but It's harmless.
>
>> + .wait_cmd_complete = hccs_wait_cmd_complete_by_poll,
>> + .fill_pcc_shared_mem = hccs_fill_pcc_shared_mem_region,
>> + .shared_mem_size = sizeof(struct acpi_pcct_shared_memory),
>> + .has_txdone_irq = false,
>> +};
>> +
>> +static const struct hccs_verspecific_data hisi04b2_verspec_data = {
>> + .rx_callback = hccs_pcc_rx_callback,
>> + .wait_cmd_complete = hccs_wait_cmd_complete_by_irq,
>> + .fill_pcc_shared_mem = hccs_fill_ext_pcc_shared_mem_region,
>> + .shared_mem_size = sizeof(struct acpi_pcct_ext_pcc_shared_memory),
>> + .has_txdone_irq = true,
>> +};
> .