Messages in this thread | | | Date | Tue, 23 May 2023 10:39:22 +0100 | From | Sudeep Holla <> | Subject | Re: [PATCH v2 1/2] soc: hisilicon: Support HCCS driver on Kunpeng SoC |
| |
On Mon, May 22, 2023 at 03:22:10PM +0800, Huisong Li wrote: > The Huawei Cache-Coherent System (HCCS) is a bus protocol standard > for ensuring cache coherent on HiSilicon SoC. The performance of > the application may be affected if some hccs ports are in non-full > lane status, have a large number of CRC errors and so on. > > This driver provides the query interface of the health status and > port information of HCCS on Kunpeng SoC. > > Signed-off-by: Huisong Li <lihuisong@huawei.com> > --- > MAINTAINERS | 6 + > drivers/soc/Kconfig | 1 + > drivers/soc/Makefile | 1 + > drivers/soc/hisilicon/Kconfig | 19 + > drivers/soc/hisilicon/Makefile | 2 + > drivers/soc/hisilicon/kunpeng_hccs.c | 1287 ++++++++++++++++++++++++++ > drivers/soc/hisilicon/kunpeng_hccs.h | 196 ++++ > 7 files changed, 1512 insertions(+) > create mode 100644 drivers/soc/hisilicon/Kconfig > create mode 100644 drivers/soc/hisilicon/Makefile > create mode 100644 drivers/soc/hisilicon/kunpeng_hccs.c > create mode 100644 drivers/soc/hisilicon/kunpeng_hccs.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index eddbc48c61e9..fe0e796e8445 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -9399,6 +9399,12 @@ S: Maintained > W: http://www.hisilicon.com > F: drivers/spi/spi-hisi-sfc-v3xx.c > > +HISILICON KUNPENG SOC HCCS DRIVER
s/HCCS/HCCS INFO or QUERY/ ?
> +M: Huisong Li <lihuisong@huawei.com> > +S: Maintained > +F: drivers/soc/hisilicon/kunpeng_hccs.c > +F: drivers/soc/hisilicon/kunpeng_hccs.h > + > HMM - Heterogeneous Memory Management > M: Jérôme Glisse <jglisse@redhat.com> > L: linux-mm@kvack.org > diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig > index 4e176280113a..d21e75d69294 100644 > --- a/drivers/soc/Kconfig > +++ b/drivers/soc/Kconfig > @@ -10,6 +10,7 @@ source "drivers/soc/bcm/Kconfig" > source "drivers/soc/canaan/Kconfig" > source "drivers/soc/fsl/Kconfig" > source "drivers/soc/fujitsu/Kconfig" > +source "drivers/soc/hisilicon/Kconfig" > source "drivers/soc/imx/Kconfig" > source "drivers/soc/ixp4xx/Kconfig" > source "drivers/soc/litex/Kconfig" > diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile > index 3b0f9fb3b5c8..531f46f3ad98 100644 > --- a/drivers/soc/Makefile > +++ b/drivers/soc/Makefile > @@ -14,6 +14,7 @@ obj-$(CONFIG_MACH_DOVE) += dove/ > obj-y += fsl/ > obj-y += fujitsu/ > obj-$(CONFIG_ARCH_GEMINI) += gemini/ > +obj-y += hisilicon/ > obj-y += imx/ > obj-y += ixp4xx/ > obj-$(CONFIG_SOC_XWAY) += lantiq/ > diff --git a/drivers/soc/hisilicon/Kconfig b/drivers/soc/hisilicon/Kconfig > new file mode 100644 > index 000000000000..87a1f15cbedb > --- /dev/null > +++ b/drivers/soc/hisilicon/Kconfig > @@ -0,0 +1,19 @@ > +# SPDX-License-Identifier: GPL-2.0-only > + > +menu "Hisilicon SoC drivers" > + depends on ARCH_HISI || COMPILE_TEST > + > +config KUNPENG_HCCS
Ditto, add INFO or QUERY to the name as HCCS alone suggests it is some driver to manage the above bus protocol which is not correct.
> + tristate "HCCS driver on Kunpeng SoC" > + depends on ACPI > + depends on ARM64 || COMPILE_TEST > + help > + The Huawei Cache-Coherent System (HCCS) is a bus protocol standard > + for ensuring cache coherent on HiSilicon SoC. The performance of > + the application may be affected if some hccs ports are in non-full > + lane status, have a large number of CRC errors and so on. > + > + Say M here if you want to include support for querying the health > + status and port information of HCCS on Kunpeng SoC. > + > +endmenu > diff --git a/drivers/soc/hisilicon/Makefile b/drivers/soc/hisilicon/Makefile > new file mode 100644 > index 000000000000..226e747e70d6 > --- /dev/null > +++ b/drivers/soc/hisilicon/Makefile > @@ -0,0 +1,2 @@ > +# SPDX-License-Identifier: GPL-2.0-only > +obj-$(CONFIG_KUNPENG_HCCS) += kunpeng_hccs.o > diff --git a/drivers/soc/hisilicon/kunpeng_hccs.c b/drivers/soc/hisilicon/kunpeng_hccs.c > new file mode 100644 > index 000000000000..20a506a04bb7 > --- /dev/null > +++ b/drivers/soc/hisilicon/kunpeng_hccs.c > @@ -0,0 +1,1287 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * The Huawei Cache-Coherent System (HCCS) is a bus protocol standard for > + * ensuring cache coherent on HiSilicon SoC. > + * > + * Copyright (c) 2023 Hisilicon Limited. > + * Author: Huisong Li <lihuisong@huawei.com> > + * > + * HCCS driver for Kunpeng SoC provides the following features: > + * - Retrieve info as belows each port:
Can we have something like: "Retrieve the following information about each port:"
> + * - port type > + * - lane mode > + * - using status
perhaps just status, "using status" doesn't sound correct to me.
> + * - current lane mode > + * - link state machine > + * - lane mask > + * - CRC error count > + * > + * - Retrieve info as belows all ports on die or chip:
Similarly: "Retrieve the following information about all the ports on the chip or the die:"
> + * - if all used ports are in linked > + * - if all linked ports are in full lane > + * - CRC error count sum > + */ > +#include <linux/sysfs.h> > +#include <linux/acpi.h> > +#include <linux/io.h> > +#include <linux/kobject.h> > +#include <linux/iopoll.h> > +#include <linux/platform_device.h> > +#include <acpi/pcc.h> > + > +#include "kunpeng_hccs.h" > + > +/* PCC defines */ > +#define HCCS_PCC_SIGNATURE_MASK 0x50434300 > +#define HCCS_PCC_STATUS_CMD_COMPLETE BIT(0)
I am not really sure if we keep duplicating this. I will try to cook up a patch consolidating these.
> + > +/* > + * Arbitrary retries in case the remote processor is slow to respond > + * to PCC commands > + */ > +#define HCCS_PCC_CMD_WAIT_RETRIES_NUM 500ULL > +#define HCCS_POLL_STATUS_TIME_INTERVAL_US 3 > + > +static struct hccs_port_info *kobj_to_port_info(struct kobject *k) > +{ > + return container_of(k, struct hccs_port_info, kobj); > +} > + > +static struct hccs_die_info *kobj_to_die_info(struct kobject *k) > +{ > + return container_of(k, struct hccs_die_info, kobj); > +} > + > +static struct hccs_chip_info *kobj_to_chip_info(struct kobject *k) > +{ > + return container_of(k, struct hccs_chip_info, kobj); > +} > + > +struct hccs_register_ctx { > + struct device *dev; > + u8 chan_id; > + int err; > +}; > + > +static acpi_status hccs_get_register_cb(struct acpi_resource *ares, > + void *context) > +{ > + struct acpi_resource_generic_register *reg; > + struct hccs_register_ctx *ctx = context; > + > + if (ares->type != ACPI_RESOURCE_TYPE_GENERIC_REGISTER) > + return AE_OK; > + > + reg = &ares->data.generic_reg; > + if (reg->space_id != ACPI_ADR_SPACE_PLATFORM_COMM) { > + dev_err(ctx->dev, "Bad register resource.\n"); > + ctx->err = -EINVAL; > + return AE_ERROR; > + } > + ctx->chan_id = reg->access_size; > + > + return AE_OK; > +} > + > +static int hccs_get_pcc_chan_id(struct hccs_dev *hdev) > +{ > + acpi_handle handle = ACPI_HANDLE(hdev->dev); > + struct hccs_register_ctx ctx = {0}; > + acpi_status status; > + > + if (!acpi_has_method(handle, METHOD_NAME__CRS)) > + return -ENODEV; > + > + ctx.dev = hdev->dev; > + status = acpi_walk_resources(handle, METHOD_NAME__CRS, > + hccs_get_register_cb, &ctx); > + if (ACPI_FAILURE(status)) > + return ctx.err; > + hdev->chan_id = ctx.chan_id; > + > + return 0; > +} > + > +static void hccs_chan_tx_done(struct mbox_client *cl, void *msg, int ret) > +{ > + if (ret < 0) > + pr_debug("TX did not complete: CMD sent:0x%x, ret:%d\n", > + *(u8 *)msg, ret); > + else > + pr_debug("TX completed. CMD sent:0x%x, ret:%d\n", > + *(u8 *)msg, ret); > +} > + > +static void hccs_unregister_pcc_channel(struct hccs_dev *hdev) > +{ > + struct hccs_mbox_client_info *cl_info = &hdev->cl_info; > + > + if (cl_info->pcc_comm_addr) > + iounmap(cl_info->pcc_comm_addr); > + pcc_mbox_free_channel(hdev->cl_info.pcc_chan); > +} > + > +static int hccs_register_pcc_channel(struct hccs_dev *hdev) > +{ > + struct hccs_mbox_client_info *cl_info = &hdev->cl_info; > + struct mbox_client *cl = &cl_info->client; > + struct pcc_mbox_chan *pcc_chan; > + struct device *dev = hdev->dev; > + int rc; > + > + cl->dev = dev; > + cl->tx_block = false; > + cl->knows_txdone = true; > + cl->tx_done = hccs_chan_tx_done; > + pcc_chan = pcc_mbox_request_channel(cl, hdev->chan_id); > + if (IS_ERR(pcc_chan)) { > + dev_err(dev, "PPC channel request failed.\n"); > + rc = -ENODEV; > + goto out; > + } > + cl_info->pcc_chan = pcc_chan; > + cl_info->mbox_chan = pcc_chan->mchan; > + > + /* > + * pcc_chan->latency is just a nominal value. In reality the remote > + * processor could be much slower to reply. So add an arbitrary amount > + * of wait on top of nominal. > + */ > + cl_info->deadline_us = > + HCCS_PCC_CMD_WAIT_RETRIES_NUM * pcc_chan->latency; > + if (cl_info->mbox_chan->mbox->txdone_irq) { > + dev_err(dev, "PCC IRQ in PCCT is enabled.\n"); > + rc = -EINVAL; > + goto err_mbx_channel_free; > + } > + > + if (pcc_chan->shmem_base_addr) { > + cl_info->pcc_comm_addr = (void __force *)ioremap(
I would prefer to use acpi_os_ioremap as it has addition checks to ensure it is not in any EFI mappings.
> + 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", > + hdev->chan_id); > + rc = -ENOMEM; > + goto err_mbx_channel_free; > + } > + } > + > + return 0; > + > +err_mbx_channel_free: > + pcc_mbox_free_channel(cl_info->pcc_chan); > +out: > + return rc; > +} > + > +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 *comm_base = cl_info->pcc_comm_addr; > + 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 & HCCS_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); > + > + return ret; > +} > + > +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; > + struct acpi_pcct_shared_memory *comm_base = cl_info->pcc_comm_addr; > + void *comm_space = (void *)(comm_base + 1); > + struct hccs_fw_inner_head *fw_inner_head; > + struct acpi_pcct_shared_memory tmp = {0}; > + u16 comm_space_size; > + int ret; > + > + /* Write signature for this subspace */ > + tmp.signature = HCCS_PCC_SIGNATURE_MASK | hdev->chan_id;
Why do we need to do this every single time ? This is never changed as it is fixed for a channel, so wondering if it can be done once in probe or so ?
-- Regards, Sudeep
| |