* [PATCH v1 0/3] soc: hisilicon: kunpeng_hccs: Support the platform with PCC type3 and interrupt ack
@ 2023-11-09 5:45 Huisong Li
2023-11-09 5:45 ` [PATCH v1 1/3] soc: hisilicon: kunpeng_hccs: Fix some incorrect format strings Huisong Li
` (4 more replies)
0 siblings, 5 replies; 23+ messages in thread
From: Huisong Li @ 2023-11-09 5:45 UTC (permalink / raw)
To: xuwei5
Cc: linux-kernel, soc, linux-arm-kernel, arnd, krzk, sudeep.holla,
liuyonglong, lihuisong
Fix some incorrect format strings and add failure log for no _CRS method.
Add support for the platform with PCC type3 and interrupt ack.
Huisong Li (3):
soc: hisilicon: kunpeng_hccs: Fix some incorrect format strings
soc: hisilicon: kunpeng_hccs: Add failure log for no _CRS method
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 | 2 +
2 files changed, 119 insertions(+), 36 deletions(-)
--
2.33.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 23+ messages in thread* [PATCH v1 1/3] soc: hisilicon: kunpeng_hccs: Fix some incorrect format strings 2023-11-09 5:45 [PATCH v1 0/3] soc: hisilicon: kunpeng_hccs: Support the platform with PCC type3 and interrupt ack Huisong Li @ 2023-11-09 5:45 ` Huisong Li 2023-11-28 15:25 ` Jonathan Cameron 2023-11-09 5:45 ` [PATCH v1 2/3] soc: hisilicon: kunpeng_hccs: Add failure log for no _CRS method Huisong Li ` (3 subsequent siblings) 4 siblings, 1 reply; 23+ messages in thread From: Huisong Li @ 2023-11-09 5:45 UTC (permalink / raw) To: xuwei5 Cc: linux-kernel, soc, linux-arm-kernel, arnd, krzk, sudeep.holla, liuyonglong, lihuisong Fix some incorrect format strings. Signed-off-by: Huisong Li <lihuisong@huawei.com> --- 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v1 1/3] soc: hisilicon: kunpeng_hccs: Fix some incorrect format strings 2023-11-09 5:45 ` [PATCH v1 1/3] soc: hisilicon: kunpeng_hccs: Fix some incorrect format strings Huisong Li @ 2023-11-28 15:25 ` Jonathan Cameron 0 siblings, 0 replies; 23+ messages in thread From: Jonathan Cameron @ 2023-11-28 15:25 UTC (permalink / raw) To: Huisong Li Cc: xuwei5, linux-kernel, soc, linux-arm-kernel, arnd, krzk, sudeep.holla, liuyonglong On Thu, 9 Nov 2023 13:45:24 +0800 Huisong Li <lihuisong@huawei.com> wrote: > Fix some incorrect format strings. > > Signed-off-by: Huisong Li <lihuisong@huawei.com> These are indeed all unsigned. Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v1 2/3] soc: hisilicon: kunpeng_hccs: Add failure log for no _CRS method 2023-11-09 5:45 [PATCH v1 0/3] soc: hisilicon: kunpeng_hccs: Support the platform with PCC type3 and interrupt ack Huisong Li 2023-11-09 5:45 ` [PATCH v1 1/3] soc: hisilicon: kunpeng_hccs: Fix some incorrect format strings Huisong Li @ 2023-11-09 5:45 ` Huisong Li 2023-11-28 15:22 ` Jonathan Cameron 2023-11-09 5:45 ` [PATCH v1 3/3] soc: hisilicon: kunpeng_hccs: Support the platform with PCC type3 and interrupt ack Huisong Li ` (2 subsequent siblings) 4 siblings, 1 reply; 23+ messages in thread From: Huisong Li @ 2023-11-09 5:45 UTC (permalink / raw) To: xuwei5 Cc: linux-kernel, soc, linux-arm-kernel, arnd, krzk, sudeep.holla, liuyonglong, lihuisong 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 <lihuisong@huawei.com> --- 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v1 2/3] soc: hisilicon: kunpeng_hccs: Add failure log for no _CRS method 2023-11-09 5:45 ` [PATCH v1 2/3] soc: hisilicon: kunpeng_hccs: Add failure log for no _CRS method Huisong Li @ 2023-11-28 15:22 ` Jonathan Cameron 0 siblings, 0 replies; 23+ messages in thread From: Jonathan Cameron @ 2023-11-28 15:22 UTC (permalink / raw) To: Huisong Li Cc: xuwei5, linux-kernel, soc, linux-arm-kernel, arnd, krzk, sudeep.holla, liuyonglong On Thu, 9 Nov 2023 13:45:25 +0800 Huisong Li <lihuisong@huawei.com> wrote: > 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 <lihuisong@huawei.com> Make sense to me. Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v1 3/3] soc: hisilicon: kunpeng_hccs: Support the platform with PCC type3 and interrupt ack 2023-11-09 5:45 [PATCH v1 0/3] soc: hisilicon: kunpeng_hccs: Support the platform with PCC type3 and interrupt ack Huisong Li 2023-11-09 5:45 ` [PATCH v1 1/3] soc: hisilicon: kunpeng_hccs: Fix some incorrect format strings Huisong Li 2023-11-09 5:45 ` [PATCH v1 2/3] soc: hisilicon: kunpeng_hccs: Add failure log for no _CRS method Huisong Li @ 2023-11-09 5:45 ` Huisong Li 2023-11-28 15:44 ` Jonathan Cameron 2023-11-30 13:45 ` [PATCH v2 0/4] " Huisong Li 2023-12-01 3:45 ` [PATCH v3 0/5] " Huisong Li 4 siblings, 1 reply; 23+ messages in thread From: Huisong Li @ 2023-11-09 5:45 UTC (permalink / raw) To: xuwei5 Cc: linux-kernel, soc, linux-arm-kernel, arnd, krzk, sudeep.holla, liuyonglong, lihuisong Support the platform with PCC type3 and interrupt ack. Signed-off-by: Huisong Li <lihuisong@huawei.com> --- drivers/soc/hisilicon/kunpeng_hccs.c | 137 +++++++++++++++++++++------ drivers/soc/hisilicon/kunpeng_hccs.h | 2 + 2 files changed, 110 insertions(+), 29 deletions(-) diff --git a/drivers/soc/hisilicon/kunpeng_hccs.c b/drivers/soc/hisilicon/kunpeng_hccs.c index fd3ca0eb8175..96cdac7be244 100644 --- a/drivers/soc/hisilicon/kunpeng_hccs.c +++ b/drivers/soc/hisilicon/kunpeng_hccs.c @@ -38,6 +38,11 @@ #define HCCS_PCC_CMD_WAIT_RETRIES_NUM 500ULL #define HCCS_POLL_STATUS_TIME_INTERVAL_US 3 +enum hccs_hw_device_version { + HCCS_HW_DEVICE_V1 = 0, + HCCS_HW_DEVICE_V2 = 1, +}; + static struct hccs_port_info *kobj_to_port_info(struct kobject *k) { return container_of(k, struct hccs_port_info, kobj); @@ -100,6 +105,21 @@ static int hccs_get_pcc_chan_id(struct hccs_dev *hdev) return 0; } +static int hccs_get_device_version(struct hccs_dev *hdev) +{ + const struct acpi_device_id *acpi_id; + + acpi_id = acpi_match_device(hdev->dev->driver->acpi_match_table, + hdev->dev); + if (!acpi_id) { + dev_err(hdev->dev, "get device version failed."); + return -EINVAL; + } + + hdev->dev_ver = (u8)acpi_id->driver_data; + return 0; +} + static void hccs_chan_tx_done(struct mbox_client *cl, void *msg, int ret) { if (ret < 0) @@ -110,6 +130,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 *m) +{ + 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 +159,11 @@ 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; + if (hdev->dev_ver == HCCS_HW_DEVICE_V2) { + cl->rx_callback = hccs_pcc_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 +180,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->dev_ver == HCCS_HW_DEVICE_V1 && + 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->dev_ver == HCCS_HW_DEVICE_V2 && + !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) { @@ -175,49 +214,81 @@ static int hccs_register_pcc_channel(struct hccs_dev *hdev) static int hccs_check_chan_cmd_complete(struct hccs_dev *hdev) { struct hccs_mbox_client_info *cl_info = &hdev->cl_info; - struct acpi_pcct_shared_memory __iomem *comm_base = - cl_info->pcc_comm_addr; + struct acpi_pcct_shared_memory __iomem *comm_base; + int ret = 0; u16 status; - int ret; /* * Poll PCC status register every 3us(delay_us) for maximum of * deadline_us(timeout_us) until PCC command complete bit is set(cond) */ - ret = readw_poll_timeout(&comm_base->status, status, - status & PCC_STATUS_CMD_COMPLETE, - HCCS_POLL_STATUS_TIME_INTERVAL_US, - cl_info->deadline_us); - if (unlikely(ret)) - dev_err(hdev->dev, "poll PCC status failed, ret = %d.\n", ret); + if (hdev->dev_ver == HCCS_HW_DEVICE_V1) { + comm_base = cl_info->pcc_comm_addr; + ret = readw_poll_timeout(&comm_base->status, status, + status & PCC_STATUS_CMD_COMPLETE, + HCCS_POLL_STATUS_TIME_INTERVAL_US, + cl_info->deadline_us); + if (unlikely(ret)) + dev_err(hdev->dev, "poll PCC status failed, ret = %d.\n", ret); + } else { + 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 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 hccs_mbox_client_info *cl_info = &hdev->cl_info; + struct acpi_pcct_ext_pcc_shared_memory tmp1 = {0}; + struct acpi_pcct_shared_memory tmp2 = {0}; + + if (hdev->dev_ver == HCCS_HW_DEVICE_V1) { + tmp2.signature = PCC_SIGNATURE | hdev->chan_id; + tmp2.command = cmd; + tmp2.status = 0; + memcpy_toio(cl_info->pcc_comm_addr, (void *)&tmp2, + sizeof(struct acpi_pcct_shared_memory)); + } else { + tmp1.signature = PCC_SIGNATURE | hdev->chan_id; + tmp1.command = cmd; + tmp1.flags = PCC_CMD_COMPLETION_NOTIFY; + tmp1.length = HCCS_PCC_SHARE_MEM_BYTES; + memcpy_toio(cl_info->pcc_comm_addr, (void *)&tmp1, + 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) { 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 shared_mem_size; + 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)); + shared_mem_size = hdev->dev_ver == HCCS_HW_DEVICE_V1 ? + sizeof(struct acpi_pcct_shared_memory) : + sizeof(struct acpi_pcct_ext_pcc_shared_memory); + comm_space = cl_info->pcc_comm_addr + shared_mem_size; + space_size = HCCS_PCC_SHARE_MEM_BYTES - shared_mem_size; - /* 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); + hccs_fill_pcc_shared_mem_region(hdev, cmd, desc, + comm_space, space_size); + if (cl_info->mbox_chan->mbox->txdone_irq) + reinit_completion(&cl_info->done); /* Ring doorbell */ ret = mbox_send_message(cl_info->mbox_chan, &cmd); @@ -233,7 +304,7 @@ static int hccs_pcc_cmd_send(struct hccs_dev *hdev, u8 cmd, 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 +313,10 @@ static int hccs_pcc_cmd_send(struct hccs_dev *hdev, u8 cmd, } end: - mbox_client_txdone(cl_info->mbox_chan, ret); + if (cl_info->mbox_chan->mbox->txdone_irq) + mbox_chan_txdone(cl_info->mbox_chan, ret); + else + mbox_client_txdone(cl_info->mbox_chan, ret); return ret; } @@ -1214,6 +1288,10 @@ static int hccs_probe(struct platform_device *pdev) hdev->dev = &pdev->dev; platform_set_drvdata(pdev, hdev); + rc = hccs_get_device_version(hdev); + if (rc) + return rc; + mutex_init(&hdev->lock); rc = hccs_get_pcc_chan_id(hdev); if (rc) @@ -1251,7 +1329,8 @@ static void hccs_remove(struct platform_device *pdev) } static const struct acpi_device_id hccs_acpi_match[] = { - { "HISI04B1"}, + { "HISI04B1", HCCS_HW_DEVICE_V1}, + { "HISI04B2", HCCS_HW_DEVICE_V2}, { ""}, }; 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..bbb1aada0c6c 100644 --- a/drivers/soc/hisilicon/kunpeng_hccs.h +++ b/drivers/soc/hisilicon/kunpeng_hccs.h @@ -51,12 +51,14 @@ struct hccs_mbox_client_info { struct pcc_mbox_chan *pcc_chan; u64 deadline_us; void __iomem *pcc_comm_addr; + struct completion done; }; struct hccs_dev { struct device *dev; struct acpi_device *acpi_dev; u64 caps; + u8 dev_ver; u8 chip_num; struct hccs_chip_info *chips; u8 chan_id; -- 2.33.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v1 3/3] soc: hisilicon: kunpeng_hccs: Support the platform with PCC type3 and interrupt ack 2023-11-09 5:45 ` [PATCH v1 3/3] soc: hisilicon: kunpeng_hccs: Support the platform with PCC type3 and interrupt ack Huisong Li @ 2023-11-28 15:44 ` Jonathan Cameron 2023-11-30 1:16 ` lihuisong (C) 0 siblings, 1 reply; 23+ messages in thread From: Jonathan Cameron @ 2023-11-28 15:44 UTC (permalink / raw) To: Huisong Li Cc: xuwei5, linux-kernel, soc, linux-arm-kernel, arnd, krzk, sudeep.holla, liuyonglong On Thu, 9 Nov 2023 13:45:26 +0800 Huisong Li <lihuisong@huawei.com> wrote: > Support the platform with PCC type3 and interrupt ack. Probably mention this is version 2 as that's what you call it in the code. > > Signed-off-by: Huisong Li <lihuisong@huawei.com> Hi. Main comment in here is that it is almost always better to use a version specific structure with callbacks / data etc rather than have instances of if (version1) do_x; else if (version2) do_y; It ends up pulling all the differences into one place + allows a great deal more flexibility. See inline for details. Otherwise looks fine to me Jonathan > --- > drivers/soc/hisilicon/kunpeng_hccs.c | 137 +++++++++++++++++++++------ > drivers/soc/hisilicon/kunpeng_hccs.h | 2 + > 2 files changed, 110 insertions(+), 29 deletions(-) > > diff --git a/drivers/soc/hisilicon/kunpeng_hccs.c b/drivers/soc/hisilicon/kunpeng_hccs.c > index fd3ca0eb8175..96cdac7be244 100644 > --- a/drivers/soc/hisilicon/kunpeng_hccs.c > +++ b/drivers/soc/hisilicon/kunpeng_hccs.c > @@ -38,6 +38,11 @@ ... > > +static int hccs_get_device_version(struct hccs_dev *hdev) > +{ > + const struct acpi_device_id *acpi_id; > + > + acpi_id = acpi_match_device(hdev->dev->driver->acpi_match_table, > + hdev->dev); Why not just have hdev->dev_ver = (u8)acpi_device_get_match_data(&hdev->dev); inline where this is called? You probably don't even need the error check as you can't get here without an appropriate match as the driver would never be matched. > + if (!acpi_id) { > + dev_err(hdev->dev, "get device version failed."); > + return -EINVAL; > + } > + > + hdev->dev_ver = (u8)acpi_id->driver_data; > + return 0; > +} > + ... > static void hccs_unregister_pcc_channel(struct hccs_dev *hdev) > { > struct hccs_mbox_client_info *cl_info = &hdev->cl_info; > @@ -131,6 +159,11 @@ 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; > + if (hdev->dev_ver == HCCS_HW_DEVICE_V2) { I'd prefer to see this done as data. That is, instead of having an enum used in the ACPI match data, have a pointer to a struct with the callback. Then this will become something like hcc_info = acpi_device_get_match_data(hdev); cl->rx_callback = hcc_info->rx_callback; init_completion(&cl_info->done); Initializing the completion is harmless if it's not used, so just do it unconditionally. > + cl->rx_callback = hccs_pcc_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 +180,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->dev_ver == HCCS_HW_DEVICE_V1 && > + cl_info->mbox_chan->mbox->txdone_irq) { Also data in hcc_info would be better than version number based code flow. if (hcc_info->has_txdone_irq && cl_info->mbox_chan->mbox->rx_done_irq) { .... } else if (!hcc_info->has_txdone_irq && !cl_info->mbox_chan->mbox->tx_done_irq) { ... } > dev_err(dev, "PCC IRQ in PCCT is enabled.\n"); > rc = -EINVAL; > goto err_mbx_channel_free; > + } else if (hdev->dev_ver == HCCS_HW_DEVICE_V2 && > + !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) { > @@ -175,49 +214,81 @@ static int hccs_register_pcc_channel(struct hccs_dev *hdev) > static int hccs_check_chan_cmd_complete(struct hccs_dev *hdev) > { > struct hccs_mbox_client_info *cl_info = &hdev->cl_info; > - struct acpi_pcct_shared_memory __iomem *comm_base = > - cl_info->pcc_comm_addr; > + struct acpi_pcct_shared_memory __iomem *comm_base; > + int ret = 0; > u16 status; > - int ret; > > /* > * Poll PCC status register every 3us(delay_us) for maximum of > * deadline_us(timeout_us) until PCC command complete bit is set(cond) > */ > - ret = readw_poll_timeout(&comm_base->status, status, > - status & PCC_STATUS_CMD_COMPLETE, > - HCCS_POLL_STATUS_TIME_INTERVAL_US, > - cl_info->deadline_us); > - if (unlikely(ret)) > - dev_err(hdev->dev, "poll PCC status failed, ret = %d.\n", ret); > + if (hdev->dev_ver == HCCS_HW_DEVICE_V1) { As above. I'd prefer to see this as a call back in an info structure rather than code here. > + comm_base = cl_info->pcc_comm_addr; > + ret = readw_poll_timeout(&comm_base->status, status, > + status & PCC_STATUS_CMD_COMPLETE, > + HCCS_POLL_STATUS_TIME_INTERVAL_US, > + cl_info->deadline_us); > + if (unlikely(ret)) > + dev_err(hdev->dev, "poll PCC status failed, ret = %d.\n", ret); > + } else { > + 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 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 hccs_mbox_client_info *cl_info = &hdev->cl_info; > + struct acpi_pcct_ext_pcc_shared_memory tmp1 = {0}; > + struct acpi_pcct_shared_memory tmp2 = {0}; > + > + if (hdev->dev_ver == HCCS_HW_DEVICE_V1) { 1) tmp1 and temp2 are local to the two forks of this statement so pull them down here. 2) Use c99 style struct init to make this cleaner. struct acpi_pcct_shared_memory = { .signature = x, .command = cmd, }; I'd also like this to be a callback in the version specific info structure rather than done as an if / else here that really doesn't extend well if we get a lot more versions doing it differently. > + tmp2.signature = PCC_SIGNATURE | hdev->chan_id; > + tmp2.command = cmd; > + tmp2.status = 0; > + memcpy_toio(cl_info->pcc_comm_addr, (void *)&tmp2, > + sizeof(struct acpi_pcct_shared_memory)); > + } else { > + tmp1.signature = PCC_SIGNATURE | hdev->chan_id; > + tmp1.command = cmd; > + tmp1.flags = PCC_CMD_COMPLETION_NOTIFY; > + tmp1.length = HCCS_PCC_SHARE_MEM_BYTES; > + memcpy_toio(cl_info->pcc_comm_addr, (void *)&tmp1, > + sizeof(struct acpi_pcct_ext_pcc_shared_memory)); > + } > + > + /* Copy the message to the PCC comm space */ > + memcpy_toio(comm_space, (void *)desc, space_size); > +} ... > @@ -1214,6 +1288,10 @@ static int hccs_probe(struct platform_device *pdev) > hdev->dev = &pdev->dev; > platform_set_drvdata(pdev, hdev); > > + rc = hccs_get_device_version(hdev); > + if (rc) > + return rc; > + > mutex_init(&hdev->lock); > rc = hccs_get_pcc_chan_id(hdev); > if (rc) > @@ -1251,7 +1329,8 @@ static void hccs_remove(struct platform_device *pdev) > } > > static const struct acpi_device_id hccs_acpi_match[] = { > - { "HISI04B1"}, > + { "HISI04B1", HCCS_HW_DEVICE_V1}, > + { "HISI04B2", HCCS_HW_DEVICE_V2}, > { ""}, Side comment, but {} So no content (rely on c initializing it to be zero filled anyway and no comma as we don't want anything after this point. > }; > 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..bbb1aada0c6c 100644 > --- a/drivers/soc/hisilicon/kunpeng_hccs.h > +++ b/drivers/soc/hisilicon/kunpeng_hccs.h > @@ -51,12 +51,14 @@ struct hccs_mbox_client_info { > struct pcc_mbox_chan *pcc_chan; > u64 deadline_us; > void __iomem *pcc_comm_addr; > + struct completion done; > }; > > struct hccs_dev { > struct device *dev; > struct acpi_device *acpi_dev; > u64 caps; > + u8 dev_ver; See above, but I'd rather see const struct hcc_verspecific_info *info; here that encodes the differences as callbacks and data and rather than code. > u8 chip_num; > struct hccs_chip_info *chips; > u8 chan_id; _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v1 3/3] soc: hisilicon: kunpeng_hccs: Support the platform with PCC type3 and interrupt ack 2023-11-28 15:44 ` Jonathan Cameron @ 2023-11-30 1:16 ` lihuisong (C) 0 siblings, 0 replies; 23+ messages in thread From: lihuisong (C) @ 2023-11-30 1:16 UTC (permalink / raw) To: Jonathan Cameron Cc: xuwei5, linux-kernel, soc, linux-arm-kernel, arnd, krzk, sudeep.holla, liuyonglong 在 2023/11/28 23:44, Jonathan Cameron 写道: > On Thu, 9 Nov 2023 13:45:26 +0800 > Huisong Li <lihuisong@huawei.com> wrote: > >> Support the platform with PCC type3 and interrupt ack. > Probably mention this is version 2 as that's what you call it > in the code. >> Signed-off-by: Huisong Li <lihuisong@huawei.com> > Hi. > > Main comment in here is that it is almost always better to use > a version specific structure with callbacks / data etc rather than > have instances of if (version1) do_x; else if (version2) do_y; > > It ends up pulling all the differences into one place + allows a > great deal more flexibility. See inline for details. Hi Jonathan, Thanks for reviewing this patch. It is a good idea to use a version specific structure as driver data. will be fixed in next version. Please take a look at it later.😁 Thanks for your advice. BR, Huisong >> --- >> drivers/soc/hisilicon/kunpeng_hccs.c | 137 +++++++++++++++++++++------ >> drivers/soc/hisilicon/kunpeng_hccs.h | 2 + >> 2 files changed, 110 insertions(+), 29 deletions(-) >> >> diff --git a/drivers/soc/hisilicon/kunpeng_hccs.c b/drivers/soc/hisilicon/kunpeng_hccs.c >> index fd3ca0eb8175..96cdac7be244 100644 >> --- a/drivers/soc/hisilicon/kunpeng_hccs.c >> +++ b/drivers/soc/hisilicon/kunpeng_hccs.c >> @@ -38,6 +38,11 @@ > ... > >> >> +static int hccs_get_device_version(struct hccs_dev *hdev) >> +{ >> + const struct acpi_device_id *acpi_id; >> + >> + acpi_id = acpi_match_device(hdev->dev->driver->acpi_match_table, >> + hdev->dev); > > Why not just have > hdev->dev_ver = (u8)acpi_device_get_match_data(&hdev->dev); > inline where this is called? > > You probably don't even need the error check as you can't get here > without an appropriate match as the driver would never be matched. Yes, it could be ok. This will be replaced by a version sepcific info according to your advice. > >> + if (!acpi_id) { >> + dev_err(hdev->dev, "get device version failed."); >> + return -EINVAL; >> + } >> + >> + hdev->dev_ver = (u8)acpi_id->driver_data; >> + return 0; >> +} >> + > ... > >> static void hccs_unregister_pcc_channel(struct hccs_dev *hdev) >> { >> struct hccs_mbox_client_info *cl_info = &hdev->cl_info; >> @@ -131,6 +159,11 @@ 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; >> + if (hdev->dev_ver == HCCS_HW_DEVICE_V2) { > I'd prefer to see this done as data. That is, instead of having an enum > used in the ACPI match data, have a pointer to a struct with the callback. > Then this will become something like > > hcc_info = acpi_device_get_match_data(hdev); > cl->rx_callback = hcc_info->rx_callback; > init_completion(&cl_info->done); > > Initializing the completion is harmless if it's not used, so just do it > unconditionally. Ack > >> + cl->rx_callback = hccs_pcc_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 +180,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->dev_ver == HCCS_HW_DEVICE_V1 && >> + cl_info->mbox_chan->mbox->txdone_irq) { > Also data in hcc_info would be better than version number based > code flow. > if (hcc_info->has_txdone_irq && > cl_info->mbox_chan->mbox->rx_done_irq) { > .... > } else if (!hcc_info->has_txdone_irq && > !cl_info->mbox_chan->mbox->tx_done_irq) { > ... > } Thanks for your example. >> dev_err(dev, "PCC IRQ in PCCT is enabled.\n"); >> rc = -EINVAL; >> goto err_mbx_channel_free; >> + } else if (hdev->dev_ver == HCCS_HW_DEVICE_V2 && >> + !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) { >> @@ -175,49 +214,81 @@ static int hccs_register_pcc_channel(struct hccs_dev *hdev) >> static int hccs_check_chan_cmd_complete(struct hccs_dev *hdev) >> { >> struct hccs_mbox_client_info *cl_info = &hdev->cl_info; >> - struct acpi_pcct_shared_memory __iomem *comm_base = >> - cl_info->pcc_comm_addr; >> + struct acpi_pcct_shared_memory __iomem *comm_base; >> + int ret = 0; >> u16 status; >> - int ret; >> >> /* >> * Poll PCC status register every 3us(delay_us) for maximum of >> * deadline_us(timeout_us) until PCC command complete bit is set(cond) >> */ >> - ret = readw_poll_timeout(&comm_base->status, status, >> - status & PCC_STATUS_CMD_COMPLETE, >> - HCCS_POLL_STATUS_TIME_INTERVAL_US, >> - cl_info->deadline_us); >> - if (unlikely(ret)) >> - dev_err(hdev->dev, "poll PCC status failed, ret = %d.\n", ret); >> + if (hdev->dev_ver == HCCS_HW_DEVICE_V1) { > As above. I'd prefer to see this as a call back in an info structure rather > than code here. Ack >> + comm_base = cl_info->pcc_comm_addr; >> + ret = readw_poll_timeout(&comm_base->status, status, >> + status & PCC_STATUS_CMD_COMPLETE, >> + HCCS_POLL_STATUS_TIME_INTERVAL_US, >> + cl_info->deadline_us); >> + if (unlikely(ret)) >> + dev_err(hdev->dev, "poll PCC status failed, ret = %d.\n", ret); >> + } else { >> + 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 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 hccs_mbox_client_info *cl_info = &hdev->cl_info; >> + struct acpi_pcct_ext_pcc_shared_memory tmp1 = {0}; >> + struct acpi_pcct_shared_memory tmp2 = {0}; >> + >> + if (hdev->dev_ver == HCCS_HW_DEVICE_V1) { > 1) tmp1 and temp2 are local to the two forks of this statement > so pull them down here. > 2) Use c99 style struct init to make this cleaner. > struct acpi_pcct_shared_memory = { > .signature = x, > .command = cmd, > }; > > I'd also like this to be a callback in the version specific info > structure rather than done as an if / else here that really doesn't > extend well if we get a lot more versions doing it differently. It is both be ok. but will fixed. > >> + tmp2.signature = PCC_SIGNATURE | hdev->chan_id; >> + tmp2.command = cmd; >> + tmp2.status = 0; >> + memcpy_toio(cl_info->pcc_comm_addr, (void *)&tmp2, >> + sizeof(struct acpi_pcct_shared_memory)); >> + } else { >> + tmp1.signature = PCC_SIGNATURE | hdev->chan_id; >> + tmp1.command = cmd; >> + tmp1.flags = PCC_CMD_COMPLETION_NOTIFY; >> + tmp1.length = HCCS_PCC_SHARE_MEM_BYTES; >> + memcpy_toio(cl_info->pcc_comm_addr, (void *)&tmp1, >> + sizeof(struct acpi_pcct_ext_pcc_shared_memory)); >> + } >> + >> + /* Copy the message to the PCC comm space */ >> + memcpy_toio(comm_space, (void *)desc, space_size); >> +} > ... > > >> @@ -1214,6 +1288,10 @@ static int hccs_probe(struct platform_device *pdev) >> hdev->dev = &pdev->dev; >> platform_set_drvdata(pdev, hdev); >> >> + rc = hccs_get_device_version(hdev); >> + if (rc) >> + return rc; >> + >> mutex_init(&hdev->lock); >> rc = hccs_get_pcc_chan_id(hdev); >> if (rc) >> @@ -1251,7 +1329,8 @@ static void hccs_remove(struct platform_device *pdev) >> } >> >> static const struct acpi_device_id hccs_acpi_match[] = { >> - { "HISI04B1"}, >> + { "HISI04B1", HCCS_HW_DEVICE_V1}, >> + { "HISI04B2", HCCS_HW_DEVICE_V2}, >> { ""}, > Side comment, but > {} > So no content (rely on c initializing it to be zero filled anyway and no comma > as we don't want anything after this point. correct. > >> }; >> 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..bbb1aada0c6c 100644 >> --- a/drivers/soc/hisilicon/kunpeng_hccs.h >> +++ b/drivers/soc/hisilicon/kunpeng_hccs.h >> @@ -51,12 +51,14 @@ struct hccs_mbox_client_info { >> struct pcc_mbox_chan *pcc_chan; >> u64 deadline_us; >> void __iomem *pcc_comm_addr; >> + struct completion done; >> }; >> >> struct hccs_dev { >> struct device *dev; >> struct acpi_device *acpi_dev; >> u64 caps; >> + u8 dev_ver; > See above, but I'd rather see > const struct hcc_verspecific_info *info; > here that encodes the differences as callbacks and data and rather than code. Good idea, so ack. >> u8 chip_num; >> struct hccs_chip_info *chips; >> u8 chan_id; > . _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 0/4] soc: hisilicon: kunpeng_hccs: Support the platform with PCC type3 and interrupt ack 2023-11-09 5:45 [PATCH v1 0/3] soc: hisilicon: kunpeng_hccs: Support the platform with PCC type3 and interrupt ack Huisong Li ` (2 preceding siblings ...) 2023-11-09 5:45 ` [PATCH v1 3/3] soc: hisilicon: kunpeng_hccs: Support the platform with PCC type3 and interrupt ack Huisong Li @ 2023-11-30 13:45 ` Huisong Li 2023-11-30 13:45 ` [PATCH v2 1/4] soc: hisilicon: kunpeng_hccs: Fix some incorrect format strings Huisong Li ` (3 more replies) 2023-12-01 3:45 ` [PATCH v3 0/5] " Huisong Li 4 siblings, 4 replies; 23+ messages in thread From: Huisong Li @ 2023-11-30 13:45 UTC (permalink / raw) To: xuwei5 Cc: linux-kernel, soc, linux-arm-kernel, Jonathan.Cameron, arnd, krzk, sudeep.holla, liuyonglong, lihuisong 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 1/4] soc: hisilicon: kunpeng_hccs: Fix some incorrect format strings 2023-11-30 13:45 ` [PATCH v2 0/4] " Huisong Li @ 2023-11-30 13:45 ` Huisong Li 2023-11-30 13:45 ` [PATCH v2 2/4] soc: hisilicon: kunpeng_hccs: Add failure log for no _CRS method Huisong Li ` (2 subsequent siblings) 3 siblings, 0 replies; 23+ messages in thread From: Huisong Li @ 2023-11-30 13:45 UTC (permalink / raw) To: xuwei5 Cc: linux-kernel, soc, linux-arm-kernel, Jonathan.Cameron, arnd, krzk, sudeep.holla, liuyonglong, lihuisong Fix some incorrect format strings. Signed-off-by: Huisong Li <lihuisong@huawei.com> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> --- 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v2 2/4] soc: hisilicon: kunpeng_hccs: Add failure log for no _CRS method 2023-11-30 13:45 ` [PATCH v2 0/4] " Huisong Li 2023-11-30 13:45 ` [PATCH v2 1/4] soc: hisilicon: kunpeng_hccs: Fix some incorrect format strings Huisong Li @ 2023-11-30 13:45 ` Huisong Li 2023-11-30 13:45 ` [PATCH v2 3/4] soc: hisilicon: kunpeng_hccs: remove an unused blank line Huisong Li 2023-11-30 13:45 ` [PATCH v2 4/4] soc: hisilicon: kunpeng_hccs: Support the platform with PCC type3 and interrupt ack Huisong Li 3 siblings, 0 replies; 23+ messages in thread From: Huisong Li @ 2023-11-30 13:45 UTC (permalink / raw) To: xuwei5 Cc: linux-kernel, soc, linux-arm-kernel, Jonathan.Cameron, arnd, krzk, sudeep.holla, liuyonglong, lihuisong 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 <lihuisong@huawei.com> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> --- 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v2 3/4] soc: hisilicon: kunpeng_hccs: remove an unused blank line 2023-11-30 13:45 ` [PATCH v2 0/4] " Huisong Li 2023-11-30 13:45 ` [PATCH v2 1/4] soc: hisilicon: kunpeng_hccs: Fix some incorrect format strings Huisong Li 2023-11-30 13:45 ` [PATCH v2 2/4] soc: hisilicon: kunpeng_hccs: Add failure log for no _CRS method Huisong Li @ 2023-11-30 13:45 ` Huisong Li 2023-11-30 14:42 ` Jonathan Cameron 2023-11-30 13:45 ` [PATCH v2 4/4] soc: hisilicon: kunpeng_hccs: Support the platform with PCC type3 and interrupt ack Huisong Li 3 siblings, 1 reply; 23+ messages in thread From: Huisong Li @ 2023-11-30 13:45 UTC (permalink / raw) To: xuwei5 Cc: linux-kernel, soc, linux-arm-kernel, Jonathan.Cameron, arnd, krzk, sudeep.holla, liuyonglong, lihuisong Remove an unused blank line. Signed-off-by: Huisong Li <lihuisong@huawei.com> --- 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v2 3/4] soc: hisilicon: kunpeng_hccs: remove an unused blank line 2023-11-30 13:45 ` [PATCH v2 3/4] soc: hisilicon: kunpeng_hccs: remove an unused blank line Huisong Li @ 2023-11-30 14:42 ` Jonathan Cameron 0 siblings, 0 replies; 23+ messages in thread From: Jonathan Cameron @ 2023-11-30 14:42 UTC (permalink / raw) To: Huisong Li Cc: xuwei5, linux-kernel, soc, linux-arm-kernel, arnd, krzk, sudeep.holla, liuyonglong On Thu, 30 Nov 2023 21:45:49 +0800 Huisong Li <lihuisong@huawei.com> wrote: > Remove an unused blank line. > > Signed-off-by: Huisong Li <lihuisong@huawei.com> FWIW Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > 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; _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 4/4] soc: hisilicon: kunpeng_hccs: Support the platform with PCC type3 and interrupt ack 2023-11-30 13:45 ` [PATCH v2 0/4] " Huisong Li ` (2 preceding siblings ...) 2023-11-30 13:45 ` [PATCH v2 3/4] soc: hisilicon: kunpeng_hccs: remove an unused blank line Huisong Li @ 2023-11-30 13:45 ` Huisong Li 2023-11-30 14:49 ` Jonathan Cameron 3 siblings, 1 reply; 23+ messages in thread From: Huisong Li @ 2023-11-30 13:45 UTC (permalink / raw) To: xuwei5 Cc: linux-kernel, soc, linux-arm-kernel, Jonathan.Cameron, arnd, krzk, sudeep.holla, liuyonglong, lihuisong 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 <lihuisong@huawei.com> --- 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v2 4/4] soc: hisilicon: kunpeng_hccs: Support the platform with PCC type3 and interrupt ack 2023-11-30 13:45 ` [PATCH v2 4/4] soc: hisilicon: kunpeng_hccs: Support the platform with PCC type3 and interrupt ack Huisong Li @ 2023-11-30 14:49 ` Jonathan Cameron 2023-12-01 2:45 ` lihuisong (C) 0 siblings, 1 reply; 23+ messages in thread From: Jonathan Cameron @ 2023-11-30 14:49 UTC (permalink / raw) To: Huisong Li Cc: xuwei5, linux-kernel, soc, linux-arm-kernel, arnd, krzk, sudeep.holla, liuyonglong On Thu, 30 Nov 2023 21:45:50 +0800 Huisong Li <lihuisong@huawei.com> 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 <lihuisong@huawei.com> Hi. A few trivial things inline but now looks good to me! Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > 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, > +}; _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 4/4] soc: hisilicon: kunpeng_hccs: Support the platform with PCC type3 and interrupt ack 2023-11-30 14:49 ` Jonathan Cameron @ 2023-12-01 2:45 ` lihuisong (C) 0 siblings, 0 replies; 23+ messages in thread From: lihuisong (C) @ 2023-12-01 2:45 UTC (permalink / raw) To: Jonathan Cameron Cc: xuwei5, linux-kernel, soc, linux-arm-kernel, arnd, krzk, sudeep.holla, liuyonglong 在 2023/11/30 22:49, Jonathan Cameron 写道: > On Thu, 30 Nov 2023 21:45:50 +0800 > Huisong Li <lihuisong@huawei.com> 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 <lihuisong@huawei.com> > Hi. > > A few trivial things inline but now looks good to me! > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > >> --- >> 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, >> +}; > . _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3 0/5] soc: hisilicon: kunpeng_hccs: Support the platform with PCC type3 and interrupt ack 2023-11-09 5:45 [PATCH v1 0/3] soc: hisilicon: kunpeng_hccs: Support the platform with PCC type3 and interrupt ack Huisong Li ` (3 preceding siblings ...) 2023-11-30 13:45 ` [PATCH v2 0/4] " Huisong Li @ 2023-12-01 3:45 ` Huisong Li 2023-12-01 3:45 ` [PATCH v3 1/5] soc: hisilicon: kunpeng_hccs: Fix some incorrect format strings Huisong Li ` (5 more replies) 4 siblings, 6 replies; 23+ messages in thread From: Huisong Li @ 2023-12-01 3:45 UTC (permalink / raw) To: xuwei5 Cc: linux-kernel, soc, linux-arm-kernel, Jonathan.Cameron, arnd, krzk, sudeep.holla, liuyonglong, lihuisong 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 clean codes and modify the incorrect email domain name. --- v3: - remove inline tag of two wait_cmd_complete function. - delete variable 'ret' in hccs_wait_cmd_complete_by_irq() - add a new patch which fix incorrect email domain name in document. 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 (5): 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 doc: kunpeng_hccs: Fix incorrect email domain name soc: hisilicon: kunpeng_hccs: Support the platform with PCC type3 and interrupt ack .../sysfs-devices-platform-kunpeng_hccs | 6 +- drivers/soc/hisilicon/kunpeng_hccs.c | 152 ++++++++++++++---- drivers/soc/hisilicon/kunpeng_hccs.h | 15 ++ 3 files changed, 137 insertions(+), 36 deletions(-) -- 2.33.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v3 1/5] soc: hisilicon: kunpeng_hccs: Fix some incorrect format strings 2023-12-01 3:45 ` [PATCH v3 0/5] " Huisong Li @ 2023-12-01 3:45 ` Huisong Li 2023-12-01 3:45 ` [PATCH v3 2/5] soc: hisilicon: kunpeng_hccs: Add failure log for no _CRS method Huisong Li ` (4 subsequent siblings) 5 siblings, 0 replies; 23+ messages in thread From: Huisong Li @ 2023-12-01 3:45 UTC (permalink / raw) To: xuwei5 Cc: linux-kernel, soc, linux-arm-kernel, Jonathan.Cameron, arnd, krzk, sudeep.holla, liuyonglong, lihuisong Fix some incorrect format strings. Signed-off-by: Huisong Li <lihuisong@huawei.com> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> --- 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 2/5] soc: hisilicon: kunpeng_hccs: Add failure log for no _CRS method 2023-12-01 3:45 ` [PATCH v3 0/5] " Huisong Li 2023-12-01 3:45 ` [PATCH v3 1/5] soc: hisilicon: kunpeng_hccs: Fix some incorrect format strings Huisong Li @ 2023-12-01 3:45 ` Huisong Li 2023-12-01 3:45 ` [PATCH v3 3/5] soc: hisilicon: kunpeng_hccs: Remove an unused blank line Huisong Li ` (3 subsequent siblings) 5 siblings, 0 replies; 23+ messages in thread From: Huisong Li @ 2023-12-01 3:45 UTC (permalink / raw) To: xuwei5 Cc: linux-kernel, soc, linux-arm-kernel, Jonathan.Cameron, arnd, krzk, sudeep.holla, liuyonglong, lihuisong 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 <lihuisong@huawei.com> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> --- 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 3/5] soc: hisilicon: kunpeng_hccs: Remove an unused blank line 2023-12-01 3:45 ` [PATCH v3 0/5] " Huisong Li 2023-12-01 3:45 ` [PATCH v3 1/5] soc: hisilicon: kunpeng_hccs: Fix some incorrect format strings Huisong Li 2023-12-01 3:45 ` [PATCH v3 2/5] soc: hisilicon: kunpeng_hccs: Add failure log for no _CRS method Huisong Li @ 2023-12-01 3:45 ` Huisong Li 2023-12-01 3:45 ` [PATCH v3 4/5] doc: kunpeng_hccs: Fix incorrect email domain name Huisong Li ` (2 subsequent siblings) 5 siblings, 0 replies; 23+ messages in thread From: Huisong Li @ 2023-12-01 3:45 UTC (permalink / raw) To: xuwei5 Cc: linux-kernel, soc, linux-arm-kernel, Jonathan.Cameron, arnd, krzk, sudeep.holla, liuyonglong, lihuisong Remove an unused blank line. Signed-off-by: Huisong Li <lihuisong@huawei.com> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> --- 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 4/5] doc: kunpeng_hccs: Fix incorrect email domain name 2023-12-01 3:45 ` [PATCH v3 0/5] " Huisong Li ` (2 preceding siblings ...) 2023-12-01 3:45 ` [PATCH v3 3/5] soc: hisilicon: kunpeng_hccs: Remove an unused blank line Huisong Li @ 2023-12-01 3:45 ` Huisong Li 2023-12-01 3:45 ` [PATCH v3 5/5] soc: hisilicon: kunpeng_hccs: Support the platform with PCC type3 and interrupt ack Huisong Li 2023-12-08 7:18 ` [PATCH v3 0/5] " Wei Xu 5 siblings, 0 replies; 23+ messages in thread From: Huisong Li @ 2023-12-01 3:45 UTC (permalink / raw) To: xuwei5 Cc: linux-kernel, soc, linux-arm-kernel, Jonathan.Cameron, arnd, krzk, sudeep.holla, liuyonglong, lihuisong The email domain name in Contact is wrong. So this patch has to modify it. Signed-off-by: Huisong Li <lihuisong@huawei.com> --- .../ABI/testing/sysfs-devices-platform-kunpeng_hccs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/ABI/testing/sysfs-devices-platform-kunpeng_hccs b/Documentation/ABI/testing/sysfs-devices-platform-kunpeng_hccs index fdb4e36310fb..1666340820f7 100644 --- a/Documentation/ABI/testing/sysfs-devices-platform-kunpeng_hccs +++ b/Documentation/ABI/testing/sysfs-devices-platform-kunpeng_hccs @@ -3,7 +3,7 @@ What: /sys/devices/platform/HISI04Bx:00/chipX/linked_full_lane What: /sys/devices/platform/HISI04Bx:00/chipX/crc_err_cnt Date: November 2023 KernelVersion: 6.6 -Contact: Huisong Li <lihuisong@huawei.org> +Contact: Huisong Li <lihuisong@huawei.com> Description: The /sys/devices/platform/HISI04Bx:00/chipX/ directory contains read-only attributes exposing some summarization @@ -26,7 +26,7 @@ What: /sys/devices/platform/HISI04Bx:00/chipX/dieY/linked_full_lane What: /sys/devices/platform/HISI04Bx:00/chipX/dieY/crc_err_cnt Date: November 2023 KernelVersion: 6.6 -Contact: Huisong Li <lihuisong@huawei.org> +Contact: Huisong Li <lihuisong@huawei.com> Description: The /sys/devices/platform/HISI04Bx:00/chipX/dieY/ directory contains read-only attributes exposing some summarization @@ -54,7 +54,7 @@ What: /sys/devices/platform/HISI04Bx:00/chipX/dieY/hccsN/lane_mask What: /sys/devices/platform/HISI04Bx:00/chipX/dieY/hccsN/crc_err_cnt Date: November 2023 KernelVersion: 6.6 -Contact: Huisong Li <lihuisong@huawei.org> +Contact: Huisong Li <lihuisong@huawei.com> Description: The /sys/devices/platform/HISI04Bx/chipX/dieX/hccsN/ directory contains read-only attributes exposing information about -- 2.33.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 5/5] soc: hisilicon: kunpeng_hccs: Support the platform with PCC type3 and interrupt ack 2023-12-01 3:45 ` [PATCH v3 0/5] " Huisong Li ` (3 preceding siblings ...) 2023-12-01 3:45 ` [PATCH v3 4/5] doc: kunpeng_hccs: Fix incorrect email domain name Huisong Li @ 2023-12-01 3:45 ` Huisong Li 2023-12-08 7:18 ` [PATCH v3 0/5] " Wei Xu 5 siblings, 0 replies; 23+ messages in thread From: Huisong Li @ 2023-12-01 3:45 UTC (permalink / raw) To: xuwei5 Cc: linux-kernel, soc, linux-arm-kernel, Jonathan.Cameron, arnd, krzk, sudeep.holla, liuyonglong, lihuisong 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 <lihuisong@huawei.com> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> --- drivers/soc/hisilicon/kunpeng_hccs.c | 135 ++++++++++++++++++++++----- drivers/soc/hisilicon/kunpeng_hccs.h | 15 +++ 2 files changed, 125 insertions(+), 25 deletions(-) diff --git a/drivers/soc/hisilicon/kunpeng_hccs.c b/drivers/soc/hisilicon/kunpeng_hccs.c index 15125f1e0f3e..9ff70b38e5e9 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 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,74 @@ static int hccs_check_chan_cmd_complete(struct hccs_dev *hdev) return ret; } +static int hccs_wait_cmd_complete_by_irq(struct hccs_dev *hdev) +{ + struct hccs_mbox_client_info *cl_info = &hdev->cl_info; + + if (!wait_for_completion_timeout(&cl_info->done, + usecs_to_jiffies(cl_info->deadline_us))) { + dev_err(hdev->dev, "PCC command executed timeout!\n"); + return -ETIMEDOUT; + } + + return 0; +} + +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 +288,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 +302,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 +1276,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 +1317,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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v3 0/5] soc: hisilicon: kunpeng_hccs: Support the platform with PCC type3 and interrupt ack 2023-12-01 3:45 ` [PATCH v3 0/5] " Huisong Li ` (4 preceding siblings ...) 2023-12-01 3:45 ` [PATCH v3 5/5] soc: hisilicon: kunpeng_hccs: Support the platform with PCC type3 and interrupt ack Huisong Li @ 2023-12-08 7:18 ` Wei Xu 5 siblings, 0 replies; 23+ messages in thread From: Wei Xu @ 2023-12-08 7:18 UTC (permalink / raw) To: Huisong Li Cc: linux-kernel, soc, linux-arm-kernel, Jonathan.Cameron, arnd, krzk, sudeep.holla, liuyonglong, xuwei5 Hi Huisong, On 2023/12/1 11:45, Huisong Li wrote: > 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 clean codes > and modify the incorrect email domain name. > > --- > v3: > - remove inline tag of two wait_cmd_complete function. > - delete variable 'ret' in hccs_wait_cmd_complete_by_irq() > - add a new patch which fix incorrect email domain name in document. > > 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 (5): > 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 > doc: kunpeng_hccs: Fix incorrect email domain name > soc: hisilicon: kunpeng_hccs: Support the platform with PCC type3 and > interrupt ack > > .../sysfs-devices-platform-kunpeng_hccs | 6 +- > drivers/soc/hisilicon/kunpeng_hccs.c | 152 ++++++++++++++---- > drivers/soc/hisilicon/kunpeng_hccs.h | 15 ++ > 3 files changed, 137 insertions(+), 36 deletions(-) > Thanks! Series applied to the HiSilicon driver tree. Best Regards, Wei _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2023-12-08 7:19 UTC | newest] Thread overview: 23+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-11-09 5:45 [PATCH v1 0/3] soc: hisilicon: kunpeng_hccs: Support the platform with PCC type3 and interrupt ack Huisong Li 2023-11-09 5:45 ` [PATCH v1 1/3] soc: hisilicon: kunpeng_hccs: Fix some incorrect format strings Huisong Li 2023-11-28 15:25 ` Jonathan Cameron 2023-11-09 5:45 ` [PATCH v1 2/3] soc: hisilicon: kunpeng_hccs: Add failure log for no _CRS method Huisong Li 2023-11-28 15:22 ` Jonathan Cameron 2023-11-09 5:45 ` [PATCH v1 3/3] soc: hisilicon: kunpeng_hccs: Support the platform with PCC type3 and interrupt ack Huisong Li 2023-11-28 15:44 ` Jonathan Cameron 2023-11-30 1:16 ` lihuisong (C) 2023-11-30 13:45 ` [PATCH v2 0/4] " Huisong Li 2023-11-30 13:45 ` [PATCH v2 1/4] soc: hisilicon: kunpeng_hccs: Fix some incorrect format strings Huisong Li 2023-11-30 13:45 ` [PATCH v2 2/4] soc: hisilicon: kunpeng_hccs: Add failure log for no _CRS method Huisong Li 2023-11-30 13:45 ` [PATCH v2 3/4] soc: hisilicon: kunpeng_hccs: remove an unused blank line Huisong Li 2023-11-30 14:42 ` Jonathan Cameron 2023-11-30 13:45 ` [PATCH v2 4/4] soc: hisilicon: kunpeng_hccs: Support the platform with PCC type3 and interrupt ack Huisong Li 2023-11-30 14:49 ` Jonathan Cameron 2023-12-01 2:45 ` lihuisong (C) 2023-12-01 3:45 ` [PATCH v3 0/5] " Huisong Li 2023-12-01 3:45 ` [PATCH v3 1/5] soc: hisilicon: kunpeng_hccs: Fix some incorrect format strings Huisong Li 2023-12-01 3:45 ` [PATCH v3 2/5] soc: hisilicon: kunpeng_hccs: Add failure log for no _CRS method Huisong Li 2023-12-01 3:45 ` [PATCH v3 3/5] soc: hisilicon: kunpeng_hccs: Remove an unused blank line Huisong Li 2023-12-01 3:45 ` [PATCH v3 4/5] doc: kunpeng_hccs: Fix incorrect email domain name Huisong Li 2023-12-01 3:45 ` [PATCH v3 5/5] soc: hisilicon: kunpeng_hccs: Support the platform with PCC type3 and interrupt ack Huisong Li 2023-12-08 7:18 ` [PATCH v3 0/5] " Wei Xu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).