lkml.org 
[lkml]   [2023]   [Apr]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH] soc: hisilicon: Support HCCS driver on Kunpeng SoC
    From
    Hi Krzysztof,
    Thanks for your review. My reply is as follows.

    在 2023/4/24 16:42, Krzysztof Kozlowski 写道:
    > On 24/04/2023 09:30, 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 | 18 +
    >> drivers/soc/hisilicon/Makefile | 2 +
    >> drivers/soc/hisilicon/kunpeng_hccs.c | 1296 ++++++++++++++++++++++++++
    >> drivers/soc/hisilicon/kunpeng_hccs.h | 204 ++++
    >> 7 files changed, 1528 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
    >> +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..81768d47f572
    >> --- /dev/null
    >> +++ b/drivers/soc/hisilicon/Kconfig
    >> @@ -0,0 +1,18 @@
    >> +# SPDX-License-Identifier: GPL-2.0-only
    >> +
    >> +menu "Hisilicon SoC drivers"
    >> + depends on ARCH_HISI
    >> +
    >> +config KUNPENG_HCCS
    >> + tristate "HCCS driver on Kunpeng SoC"
    >> + depends on ARM64 && ACPI
    >> + 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..2b52f7dedc78
    >> --- /dev/null
    >> +++ b/drivers/soc/hisilicon/kunpeng_hccs.c
    >> @@ -0,0 +1,1296 @@
    >> +// 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:
    >> + * - port type
    >> + * - lane mode
    >> + * - using status
    >> + * - current lane mode
    >> + * - link state machine
    >> + * - lane mask
    >> + * - CRC error count
    >> + *
    >> + * - Retrieve info as belows all ports on die or chip:
    >> + * - 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)
    >> +
    >> +/*
    >> + * 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);
    >> +}
    >> +
    >> +static bool hccs_dev_property_supported(struct hccs_dev *hdev)
    >> +{
    >> + struct device *dev = hdev->dev;
    >> +
    >> + if (hdev->intr_mode) {
    >> + dev_err(dev, "interrupt mode is unsupported.\n");
    >> + return false;
    >> + }
    >> +
    >> + if (hdev->type >= ACPI_PCCT_TYPE_EXT_PCC_MASTER_SUBSPACE) {
    >> + dev_err(dev, "PCC type-%u is unsupported.\n", hdev->type);
    >> + return false;
    >> + }
    >> +
    >> + return true;
    >> +}
    >> +
    >> +static int hccs_get_device_property(struct hccs_dev *hdev)
    >> +{
    >> + struct device *dev = hdev->dev;
    >> +
    >> + if (device_property_read_u32(dev, "device-flags", &hdev->flags)) {
    >> + dev_err(hdev->dev, "no device-flags property.\n");
    >> + return -ENODEV;
    >> + }
    >> +
    >> + if (device_property_read_u8(dev, "pcc-type", &hdev->type)) {
    >> + dev_err(hdev->dev, "no pcc-type property.\n");
    >> + return -ENODEV;
    >> + }
    >> +
    >> + if (device_property_read_u32(dev, "pcc-chan-id", &hdev->chan_id)) {
    >> + dev_err(hdev->dev, "no pcc-channel property.\n");
    >> + return -ENODEV;
    >> + }
    >> +
    >> + hdev->intr_mode = hccs_get_bit(hdev->flags, HCCS_DEV_FLAGS_INTR_B);
    >> + if (!hccs_dev_property_supported(hdev))
    > Maybe leave a comment that all these are ACPI-only and are not allowed
    > in DT usage. Without bindings we are not going to review them. OTOH, I
    > think we don't have a process for such cases in general.
    Thanks for your suggestion. will add it.
    >
    >> + return -EOPNOTSUPP;
    >> +
    >> + return 0;
    >> +}
    >> +
    > (...)
    >
    >> +static struct attribute *hccs_chip_default_attrs[] = {
    >> + &all_linked_on_chip_attr.attr,
    >> + &linked_full_lane_on_chip_attr.attr,
    >> + &crc_err_cnt_sum_on_chip_attr.attr,
    >> + NULL,
    >> +};
    >> +ATTRIBUTE_GROUPS(hccs_chip_default);
    >> +
    >> +static const struct kobj_type hccs_chip_type = {
    >> + .sysfs_ops = &hccs_comm_ops,
    >> + .default_groups = hccs_chip_default_groups,
    > Missing sysfs documentation.
    I will add another patch to do this.
    >
    > This also looks like:
    > 1. Duplicating parts of socinfo.
    Where is the duplicating part. Sorry, I cannot get it.
    > 2. Open-coding coding sysfs API.
    I cannot understand it. Can you be more specific?
    >
    > Anyway, after describing this API it might turn out it is not valid for
    > upstream usage...
    >
    >> +};
    >> +
    >> +static void hccs_remove_die_dir(struct hccs_die_info *die)
    >> +{
    >> + struct hccs_port_info *port;
    >> + u8 i;
    >> +
    >> + for (i = 0; i < die->port_num; i++) {
    >> + port = &die->ports[i];
    >> + if (port->dir_created)
    >> + kobject_put(&port->kobj);
    >> + }
    >> +
    >> + kobject_put(&die->kobj);
    >> +}
    >> +
    >> +static void hccs_remove_chip_dir(struct hccs_chip_info *chip)
    >> +{
    >> + struct hccs_die_info *die;
    >> + u8 i;
    >> +
    >> + for (i = 0; i < chip->die_num; i++) {
    >> + die = &chip->dies[i];
    >> + if (die->dir_created)
    >> + hccs_remove_die_dir(die);
    >> + }
    >> +
    >> + kobject_put(&chip->kobj);
    >> +}
    >> +
    >> +static void hccs_remove_topo_dirs(struct hccs_dev *hdev)
    >> +{
    >> + u8 i;
    >> +
    >> + for (i = 0; i < hdev->chip_num; i++)
    >> + hccs_remove_chip_dir(&hdev->chips[i]);
    >> +}
    >> +
    >> +static int hccs_create_hccs_dir(struct hccs_dev *hdev,
    >> + struct hccs_die_info *die,
    >> + struct hccs_port_info *port)
    >> +{
    >> + int ret;
    >> +
    >> + ret = kobject_init_and_add(&port->kobj, &hccs_port_type,
    >> + &die->kobj, "hccs%d", port->port_id);
    >> + if (ret) {
    >> + kobject_put(&port->kobj);
    >> + return ret;
    >> + }
    >> +
    >> + return 0;
    >> +}
    >> +
    >> +static int hccs_create_die_dir(struct hccs_dev *hdev,
    >> + struct hccs_chip_info *chip,
    >> + struct hccs_die_info *die)
    >> +{
    >> + struct hccs_port_info *port;
    >> + int ret;
    >> + u16 i;
    >> +
    >> + ret = kobject_init_and_add(&die->kobj, &hccs_die_type,
    >> + &chip->kobj, "die%d", die->die_id);
    >> + if (ret) {
    >> + kobject_put(&die->kobj);
    >> + return ret;
    >> + }
    >> +
    >> + for (i = 0; i < die->port_num; i++) {
    >> + port = &die->ports[i];
    >> + ret = hccs_create_hccs_dir(hdev, die, port);
    >> + if (ret) {
    >> + dev_err(hdev->dev, "create hccs%d dir failed.\n",
    >> + port->port_id);
    >> + goto err;
    >> + }
    >> + port->dir_created = true;
    >> + }
    >> +
    >> + return 0;
    >> +err:
    >> + hccs_remove_die_dir(die);
    >> +
    >> + return ret;
    >> +}
    >> +
    >> +static int hccs_create_chip_dir(struct hccs_dev *hdev,
    >> + struct hccs_chip_info *chip)
    >> +{
    >> + struct hccs_die_info *die;
    >> + int ret;
    >> + u16 id;
    >> +
    >> + ret = kobject_init_and_add(&chip->kobj, &hccs_chip_type,
    >> + &hdev->dev->kobj, "chip%d", chip->chip_id);
    >> + if (ret) {
    >> + kobject_put(&chip->kobj);
    >> + return ret;
    >> + }
    >> +
    >> + for (id = 0; id < chip->die_num; id++) {
    >> + die = &chip->dies[id];
    >> + ret = hccs_create_die_dir(hdev, chip, die);
    >> + if (ret)
    >> + goto err;
    >> + die->dir_created = true;
    >> + }
    >> +
    >> + return 0;
    >> +err:
    >> + hccs_remove_chip_dir(chip);
    >> +
    >> + return ret;
    >> +}
    >> +
    >> +static int hccs_create_topo_dirs(struct hccs_dev *hdev)
    >> +{
    >> + struct hccs_chip_info *chip;
    >> + u8 id, k;
    >> + int ret;
    >> +
    >> + for (id = 0; id < hdev->chip_num; id++) {
    >> + chip = &hdev->chips[id];
    >> + ret = hccs_create_chip_dir(hdev, chip);
    >> + if (ret) {
    >> + dev_err(hdev->dev, "init chip%d dir failed!\n", id);
    >> + goto err;
    >> + }
    >> + }
    >> +
    >> + return 0;
    >> +err:
    >> + for (k = 0; k < id; k++)
    >> + hccs_remove_chip_dir(&hdev->chips[k]);
    >> +
    >> + return ret;
    >> +}
    >> +
    >> +static int hccs_probe(struct platform_device *pdev)
    >> +{
    >> + struct acpi_device *acpi_dev;
    >> + struct hccs_dev *hdev;
    >> + int rc;
    >> +
    >> + pr_info("kunpeng_hccs is initializing.\n");
    > Drop.
    Ack.
    >
    >> +
    >> + if (acpi_disabled) {
    >> + dev_err(&pdev->dev, "acpi is disabled.\n");
    > Is it possible for your ACPI-only driver?
    Yes
    >
    >
    >> + return -ENODEV;
    >> + }
    >> + acpi_dev = ACPI_COMPANION(&pdev->dev);
    >> + if (!acpi_dev)
    >> + return -ENODEV;
    >> +
    >> + hdev = devm_kzalloc(&pdev->dev, sizeof(*hdev), GFP_KERNEL);
    >> + if (!hdev)
    >> + return -ENOMEM;
    >> + hdev->acpi_dev = acpi_dev;
    >> + hdev->dev = &pdev->dev;
    >> + platform_set_drvdata(pdev, hdev);
    >> +
    >> + rc = hccs_get_device_property(hdev);
    >> + if (rc)
    >> + return rc;
    >> +
    >> + mutex_init(&hdev->lock);
    >> +
    >> + rc = hccs_register_pcc_channel(hdev);
    >> + if (rc)
    >> + return rc;
    >> +
    >> + rc = hccs_get_dev_caps(hdev);
    >> + if (rc)
    >> + goto err_uninit_mbox_client;
    >> +
    >> + rc = hccs_get_hw_info(hdev);
    >> + if (rc)
    >> + goto err_uninit_mbox_client;
    >> +
    >> + rc = hccs_create_topo_dirs(hdev);
    >> + if (rc)
    >> + goto err_uninit_mbox_client;
    >> +
    >> + dev_info(&pdev->dev, "HISI HCCS driver registered!\n");
    > Drop simple probe success msgs. There's already kernel infrastructure
    > giving you this information.
    All right, drop it.
    >
    >> +
    >> + return 0;
    >> +
    >> +err_uninit_mbox_client:
    >> + hccs_unregister_pcc_channel(hdev);
    >> +
    >> + return rc;
    >> +}
    >> +
    >> +static int hccs_remove(struct platform_device *pdev)
    >> +{
    >> + struct hccs_dev *hdev = platform_get_drvdata(pdev);
    >> +
    >> + hccs_remove_topo_dirs(hdev);
    >> + hccs_unregister_pcc_channel(hdev);
    >> +
    >> + return 0;
    >> +}
    >> +
    >> +static const struct acpi_device_id hccs_acpi_match[] = {
    >> + { "HISI04B1", },
    >> +};
    >> +
    >> +static struct platform_driver hccs_driver = {
    >> + .probe = hccs_probe,
    >> + .remove = hccs_remove,
    >> + .driver = {
    >> + .name = "kunpeng_hccs",
    >> + .acpi_match_table = ACPI_PTR(hccs_acpi_match),
    > Drop ACPI_PTR as it is not balanced with __maybe_unused.
    ok, will drop it in v2.

    \
     
     \ /
      Last update: 2023-04-25 05:16    [W:8.466 / U:0.460 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site