Hi Tanxiaofei,
Some more comments inline.
>-----Original Message-----
>From: tanxiaofei <tanxiaofei(a)huawei.com>
>Sent: 09 July 2024 10:34
>To: Shiju Jose <shiju.jose(a)huawei.com>; Jonathan Cameron
><jonathan.cameron(a)huawei.com>; Mauro Carvalho Chehab
><M.Chehab(a)huawei.com>; Roberto Sassu <roberto.sassu(a)huawei.com>;
>Guohanjun (Hanjun Guo) <guohanjun(a)huawei.com>
>Cc: linuxarm(a)openeuler.org; tanxiaofei <tanxiaofei(a)huawei.com>
>Subject: [PATCH] soc: hisilicon: Support memory repair driver on Kunpeng SoC
>
>Kunpeng SoC support several memory repair capability. Normally, such repairs
>are done in firmware startup stage or triggered by BMC(Baseboard
>Management Controller) and done in firmware in runtime stage. OS can't
>perceive such capability.
>
>Some support online repair, while others can't. One reason of not supporting
>online repair is that the memory is in use and the hardware does not support
>backpressure on the primary access channel.
>
>In order to support more online memory repair, we seek solutions from OS. That
>is try to isolate the memory page first, and do repair, and de-isloate after repair
>done.
>
>This patch support online ACLS(Adapter Cache Line Sparing) repair, and may add
>other repair such as PPR in future.
>
>Signed-off-by: Xiaofei Tan <tanxiaofei(a)huawei.com>
>---
> drivers/soc/hisilicon/Kconfig | 10 +
> drivers/soc/hisilicon/Makefile | 1 +
> drivers/soc/hisilicon/hisi_mem_ras.c | 553 +++++++++++++++++++++++++++
>drivers/soc/hisilicon/hisi_mem_ras.h | 84 ++++
> 4 files changed, 648 insertions(+)
> create mode 100644 drivers/soc/hisilicon/hisi_mem_ras.c
> create mode 100644 drivers/soc/hisilicon/hisi_mem_ras.h
>
>diff --git a/drivers/soc/hisilicon/Kconfig b/drivers/soc/hisilicon/Kconfig index
>ae8e55a776e0..983cafc38e5c 100644
>--- a/drivers/soc/hisilicon/Kconfig
>+++ b/drivers/soc/hisilicon/Kconfig
>@@ -44,4 +44,14 @@ config HISI_HBMCACHE
> To compile the driver as a module, choose M here:
> the module will be called hisi_hbmcache.
>
>+config HISI_MEM_RAS
>+ tristate "Add support for HISI Memory repair"
>+ depends on ACPI
>+ help
>+ Add Memory repair support for Hisilicon device, which can be used to
>query
>+ memory repair cap and repair hardware error in memory devices. This
>feature
>+ need to work with hardware firmwares.
>+
>+ If not sure say no.
>+
> endmenu
>diff --git a/drivers/soc/hisilicon/Makefile b/drivers/soc/hisilicon/Makefile index
>4ef8823250e8..b339a67bcd77 100644
>--- a/drivers/soc/hisilicon/Makefile
>+++ b/drivers/soc/hisilicon/Makefile
>@@ -6,3 +6,4 @@ obj-$(CONFIG_KUNPENG_HCCS) += kunpeng_hccs.o
> obj-$(CONFIG_HISI_HBMDEV) += hisi_hbmdev.o
> obj-$(CONFIG_HISI_HBMCACHE) += hisi_hbmcache.o
> obj-$(CONFIG_ARM64_PBHA) += pbha.o
>+obj-$(CONFIG_HISI_MEM_RAS) += hisi_mem_ras.o
>diff --git a/drivers/soc/hisilicon/hisi_mem_ras.c
>b/drivers/soc/hisilicon/hisi_mem_ras.c
>new file mode 100644
>index 000000000000..a29c41ccdba1
>--- /dev/null
>+++ b/drivers/soc/hisilicon/hisi_mem_ras.c
>@@ -0,0 +1,553 @@
>+// SPDX-License-Identifier: GPL-2.0
>+/*
>+ * Copyright (C) Huawei Technologies Co., Ltd. 2024. All rights reserved.
>+ */
>+
>+#include <linux/kobject.h>
>+#include <linux/module.h>
>+#include <linux/acpi.h>
>+#include <linux/iopoll.h>
>+#include <linux/platform_device.h>
>+#include <linux/mm.h>
>+#include <acpi/pcc.h>
>+
>+#include "hisi_mem_ras.h"
>+#define DRV_NAME "hisi_mem_ras"
>+
>+#define HISI_MEM_PCC_CMD_WAIT_RETRIES_NUM 500ULL
>+#define HISI_MEM_POLL_STATUS_TIME_INTERVAL_US 3
>+
>+static struct hisi_mem_ras *mras;
>+
>+struct hisi_mem_register_ctx {
>+ struct device *dev;
>+ u8 chan_id;
>+ int err;
>+ u64 paddr_min;
>+ u64 paddr_max;
>+};
>+
>+static acpi_status hisi_mem_get_chan_id_cb(struct acpi_resource *ares,
>+ void *context)
>+{
>+ struct acpi_resource_generic_register *reg;
>+ struct hisi_mem_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;
Passing PCC channel ID in register access size may not be correct.
>+
>+ return AE_OK;
>+}
>+
>+static int hisi_mem_get_pcc_chan_id(struct hisi_mem_dev *hdev) {
>+ struct platform_device *pdev = hdev->pdev;
>+ struct acpi_device *adev = ACPI_COMPANION(&pdev->dev);
>+ struct hisi_mem_register_ctx ctx = {0};
>+ acpi_handle handle = adev->handle;
>+ acpi_status status;
>+
>+ if (!acpi_has_method(handle, METHOD_NAME__CRS)) {
>+ dev_err(&pdev->dev, "No _CRS method.\n");
>+ return -ENODEV;
>+ }
>+
>+ ctx.dev = &pdev->dev;
>+ status = acpi_walk_resources(handle, METHOD_NAME__CRS,
>+ hisi_mem_get_chan_id_cb, &ctx);
>+ if (ACPI_FAILURE(status))
>+ return ctx.err;
>+
Please add following here as hdev->chan_id is not set.
hdev->chan_id = ctx->chan_id;
>+ return 0;
>+}
>+
>+static acpi_status hisi_mem_get_paddr_range_cb(struct acpi_resource *ares,
>+ void *context)
>+{
>+ struct hisi_mem_register_ctx *ctx = context;
>+ struct acpi_resource_address64 *addr64;
>+
>+ if (ares->type != ACPI_RESOURCE_TYPE_ADDRESS64)
>+ return AE_OK;
>+
>+ addr64 = &ares->data.address64;
>+ ctx->paddr_min = addr64->address.minimum;
>+ ctx->paddr_max = addr64->address.maximum;
>+
>+ return AE_OK;
>+}
>+
>+static int hisi_mem_get_paddr_range(struct hisi_mem_dev *hdev) {
>+ struct platform_device *pdev = hdev->pdev;
>+ struct acpi_device *adev = ACPI_COMPANION(&pdev->dev);
>+ struct hisi_mem_register_ctx ctx = {0};
>+ acpi_handle handle = adev->handle;
>+ acpi_status status;
>+
>+ if (!acpi_has_method(handle, METHOD_NAME__CRS)) {
>+ dev_err(&pdev->dev, "No _CRS method.\n");
>+ return -ENODEV;
>+ }
>+
>+ ctx.dev = &pdev->dev;
>+ status = acpi_walk_resources(handle, METHOD_NAME__CRS,
>+ hisi_mem_get_paddr_range_cb, &ctx);
>+ if (ACPI_FAILURE(status))
>+ return ctx.err;
>+
>+ hdev->paddr_min = ctx.paddr_min;
>+ hdev->paddr_max = ctx.paddr_max;
>+ return 0;
>+}
>+
>+static void hisi_mem_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 hisi_mem_pcc_rx_callback(struct mbox_client *cl, void
>+*mssg) {
>+ struct hisi_mem_mbox_client_info *cl_info =
>+ container_of(cl, struct hisi_mem_mbox_client_info,
>client);
>+
>+ complete(&cl_info->done);
>+}
>+
>+static void hisi_mem_unregister_pcc_channel(struct hisi_mem_dev *hdev)
>+{
>+ struct hisi_mem_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 hisi_mem_register_pcc_channel(struct hisi_mem_dev *hdev) {
>+ struct hisi_mem_mbox_client_info *cl_info = &hdev->cl_info;
>+ struct mbox_client *cl = &cl_info->client;
>+ struct pcc_mbox_chan *pcc_chan;
>+ struct platform_device *pdev = hdev->pdev;
>+ int rc;
>+
>+ cl->dev = &pdev->dev;
>+ cl->tx_block = false;
>+ cl->knows_txdone = true;
>+ cl->tx_done = hisi_mem_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);
Where the hdev->chan_id is being set? I think it is the same ctx->chan_id set in the
hisi_mem_get_chan_id_cb() ?
>+ if (IS_ERR(pcc_chan)) {
>+ dev_err(&pdev->dev, "PPC channel request failed.\n");
PPC -> PCC
>+ 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 =
>+ HISI_MEM_PCC_CMD_WAIT_RETRIES_NUM *
>pcc_chan->latency;
>+ if (!hdev->verspec_data->has_txdone_irq &&
>+ cl_info->mbox_chan->mbox->txdone_irq) {
>+ dev_err(&pdev->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(&pdev->dev, "PCC IRQ in PCCT isn't supported.\n");
>+ rc = -EINVAL;
>+ goto err_mbx_channel_free;
>+ }
>+
>+ if (pcc_chan->shmem_base_addr) {
>+ cl_info->pcc_comm_addr = ioremap(pcc_chan-
>>shmem_base_addr,
>+ pcc_chan->shmem_size);
>+ if (!cl_info->pcc_comm_addr) {
>+ dev_err(&pdev->dev, "Failed to ioremap PCC
>communication region for channel-%u.\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 hisi_mem_wait_cmd_complete_by_poll(struct hisi_mem_dev
>+*hdev) {
>+ struct hisi_mem_mbox_client_info *cl_info = &hdev->cl_info;
>+ struct acpi_pcct_shared_memory __iomem *comm_base =
>+ cl_info-
>>pcc_comm_addr;
>+ struct platform_device *pdev = hdev->pdev;
>+ 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,
>+ HISI_MEM_POLL_STATUS_TIME_INTERVAL_US,
>+ cl_info->deadline_us);
>+ if (unlikely(ret))
>+ dev_err(&pdev->dev, "poll PCC status failed, ret = %d.\n", ret);
>+
>+ return ret;
>+}
>+
>+static int hisi_mem_wait_cmd_complete_by_irq(struct hisi_mem_dev *hdev)
>+{
>+ struct hisi_mem_mbox_client_info *cl_info = &hdev->cl_info;
>+ struct platform_device *pdev = hdev->pdev;
>+
>+ if (!wait_for_completion_timeout(&cl_info->done,
>+ usecs_to_jiffies(cl_info->deadline_us))) {
>+ dev_err(&pdev->dev, "PCC command executed timeout!\n");
>+ return -ETIMEDOUT;
>+ }
>+
>+ return 0;
>+}
>+
>+static inline void hisi_mem_fill_pcc_shared_mem_region(struct hisi_mem_dev
>*hdev,
>+ u8 cmd,
>+ struct hisi_mem_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 hisi_mem_fill_ext_pcc_shared_mem_region(struct
>hisi_mem_dev *hdev,
>+ u8 cmd,
>+ struct hisi_mem_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 = HISI_MEM_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 const struct hisi_mem_verspecific_data hisi04b1_verspec_data = {
>+ .rx_callback = NULL,
>+ .wait_cmd_complete = hisi_mem_wait_cmd_complete_by_poll,
>+ .fill_pcc_shared_mem = hisi_mem_fill_pcc_shared_mem_region,
>+ .shared_mem_size = sizeof(struct acpi_pcct_shared_memory),
>+ .has_txdone_irq = false,
>+};
>+
>+static const struct hisi_mem_verspecific_data hisi04b2_verspec_data = {
>+ .rx_callback = hisi_mem_pcc_rx_callback,
>+ .wait_cmd_complete = hisi_mem_wait_cmd_complete_by_irq,
>+ .fill_pcc_shared_mem = hisi_mem_fill_ext_pcc_shared_mem_region,
>+ .shared_mem_size = sizeof(struct acpi_pcct_ext_pcc_shared_memory),
>+ .has_txdone_irq = true,
>+};
>+
>+static int hisi_mem_pcc_cmd_send(struct hisi_mem_dev *hdev, u8 cmd,
>+ struct hisi_mem_desc *desc)
>+{
>+ const struct hisi_mem_verspecific_data *verspec_data = hdev-
>>verspec_data;
>+ struct hisi_mem_mbox_client_info *cl_info = &hdev->cl_info;
>+ struct platform_device *pdev = hdev->pdev;
>+ void __iomem *comm_space;
>+ u16 space_size;
>+ int ret;
>+
>+ comm_space = cl_info->pcc_comm_addr + verspec_data-
>>shared_mem_size;
>+ space_size = HISI_MEM_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);
>+ if (ret < 0) {
>+ dev_err(&pdev->dev, "Send PCC mbox message failed, ret =
>%d.\n",
>+ ret);
>+ goto end;
>+ }
>+
>+ ret = verspec_data->wait_cmd_complete(hdev);
>+ if (ret)
>+ goto end;
>+
>+ /* Copy response data */
>+ memcpy_fromio((void *)desc, comm_space, space_size);
>+
>+end:
>+ 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;
>+}
>+
>+int hisi_mem_do_acls_query(struct hisi_mem_dev *hdev, int *status, u64
>+paddr) {
>+ struct platform_device *pdev = hdev->pdev;
>+ struct hisi_mem_desc desc;
>+ int ret;
>+
>+ memset(&desc, 0, sizeof(desc));
>+ desc.req.paddr = paddr;
>+ desc.req.subcmd = HISI_MEM_RAS_QUERY_CAP;
>+ ret = hisi_mem_pcc_cmd_send(hdev, HISI_MEM_RAS_ACLS, &desc);
>+ if (ret) {
>+ dev_err(&pdev->dev, "do acls query failed, ret = %d.\n", ret);
>+ return ret;
>+ }
>+
>+ *status = desc.rsp.retStatus;
>+ return 0;
>+}
>+
>+static ssize_t acls_query_store(struct kobject *kobj,
>+ struct kobj_attribute *attr,
>+ const char *buf, size_t count)
>+{
>+ struct hisi_mem_dev *hdev, *tmp;
>+ int status, ret;
>+ u64 paddr;
>+
>+ if (kstrtoull(buf, 16, &paddr))
>+ return -EINVAL;
>+
>+ list_for_each_entry_safe(hdev, tmp, &mras->list, list) {
>+ struct platform_device *pdev = hdev->pdev;
>+
>+ if (paddr < hdev->paddr_min || paddr > hdev->paddr_max)
>+ continue;
>+
>+ ret = hisi_mem_do_acls_query(hdev, &status, paddr);
>+ if (ret)
>+ return ret;
>+
>+ if (status == HISI_MEM_ACLS_OK) {
>+ return count;
>+ } else if (status == HISI_MEM_ACLS_QUERY_NO_RES) {
>+ dev_info(&pdev->dev, "acls query no resource left.\n");
>+ return -ENOSPC;
>+ }
>+
>+ dev_err(&pdev->dev, "acls query error code %d.\n", status);
>+ return -EIO;
>+ }
>+
>+ return -ENODEV;
>+}
>+
>+static struct kobj_attribute acls_query_store_attribute =
>+ __ATTR(acls_query, 0200, NULL, acls_query_store);
>+
>+int hisi_mem_do_acls_repair(struct hisi_mem_dev *hdev, int *status, u64
>+paddr) {
>+ struct hisi_mem_desc desc;
>+ struct platform_device *pdev;
>+ int ret;
>+
>+ memset(&desc, 0, sizeof(desc));
>+ desc.req.paddr = paddr;
>+ desc.req.subcmd = HISI_MEM_RAS_DO_REPAIR;
>+ ret = hisi_mem_pcc_cmd_send(hdev, HISI_MEM_RAS_ACLS, &desc);
>+ if (ret) {
>+ dev_err(&pdev->dev, "do acls repair failed, ret = %d.\n", ret);
>+ return ret;
>+ }
>+
>+ *status = desc.rsp.retStatus;
>+ return 0;
>+}
>+
>+static ssize_t acls_repair_store(struct kobject *kobj,
>+ struct kobj_attribute *attr,
>+ const char *buf, size_t count)
>+{
>+ struct hisi_mem_dev *hdev, *tmp;
>+ u64 paddr, ret;
>+ int status;
>+
>+ if (kstrtoull(buf, 16, &paddr))
>+ return -EINVAL;
>+
>+ list_for_each_entry_safe(hdev, tmp, &mras->list, list) {
>+ struct platform_device *pdev = hdev->pdev;
>+
>+ if (paddr < hdev->paddr_min || paddr > hdev->paddr_max)
>+ continue;
>+
>+ ret = hisi_mem_do_acls_repair(hdev, &status, paddr);
>+ if (ret)
>+ return ret;
>+
>+ if (status == HISI_MEM_ACLS_OK)
>+ return count;
>+
>+ dev_err(&pdev->dev, "acls repair error code %d.\n", status);
>+ return -EIO;
>+ }
>+
>+ return -ENODEV;
>+}
>+static struct kobj_attribute acls_repair_store_attribute =
>+ __ATTR(acls_repair, 0200, NULL, acls_repair_store);
>+
>+static struct attribute *acls_attrs[] = {
>+ &acls_query_store_attribute.attr,
>+ &acls_repair_store_attribute.attr,
>+ NULL,
>+};
>+
>+static struct attribute_group acls_attr_group = {
>+ .attrs = acls_attrs,
>+};
>+
>+static int hisi_mem_ras_probe(struct platform_device *pdev) {
>+ struct hisi_mem_dev *hdev;
>+ int ret = -ENOMEM;
>+
>+ hdev = kzalloc(sizeof(struct hisi_mem_dev), GFP_KERNEL);
>+ if (!hdev)
>+ return -ENOMEM;
>+
>+ hdev->pdev = pdev;
>+ ret = hisi_mem_get_pcc_chan_id(hdev);
>+ if (ret)
>+ goto free_hdev;
>+
>+ ret = hisi_mem_register_pcc_channel(hdev);
>+ if (ret)
>+ goto free_hdev;
>+
>+ platform_set_drvdata(pdev, hdev);
>+ list_add_tail(&hdev->list, &mras->list);
>+ hdev->verspec_data = &hisi04b2_verspec_data;
1. hdev->verspec_data is being accessed in the above hisi_mem_register_pcc_channel() function.
Need to set hdev->verspec_data before calling hisi_mem_register_pcc_channel()?
2. Both hisi04b1_verspec_data and hisi04b2_verspec_data are defined, seems hisi04b1_verspec_data not used.
>+ return 0;
>+
>+free_hdev:
>+ kfree(hdev);
>+ return ret;
>+}
>+
>+static int hisi_mem_ras_remove(struct platform_device *pdev) {
>+ struct hisi_mem_dev *hdev = platform_get_drvdata(pdev);
>+
>+ platform_set_drvdata(pdev, NULL);
>+ list_del(&hdev->list);
>+
>+ return 0;
>+}
>+
>+static const struct acpi_device_id hisi_mem_acpi_match[] = {
>+ { "HISI0FF1", 0},
>+ { },
>+};
>+MODULE_DEVICE_TABLE(acpi, hisi_mem_acpi_match);
>+
>+static struct platform_driver hisi_mem_ras_driver = {
>+ .probe = hisi_mem_ras_probe,
>+ .remove = hisi_mem_ras_remove,
>+ .driver = {
>+ .name = "hisi_mem_ras",
>+ .acpi_match_table = hisi_mem_acpi_match,
>+ },
>+};
>+
>+static int __init hisi_mem_ras_init(void) {
>+ int ret;
>+
>+ mras = kzalloc(sizeof(struct hisi_mem_ras), GFP_KERNEL);
>+ if (!mras)
>+ return -ENOMEM;
>+
>+ mras->acls_kobj = kobject_create_and_add("acls", kernel_kobj);
>+ if (!mras->acls_kobj) {
>+ kfree(mras);
>+ return -ENOMEM;
>+ }
>+
>+ ret = sysfs_create_group(mras->acls_kobj, &acls_attr_group);
>+ if (ret) {
>+ mras->acls_kobj = NULL;
This should be done after below kobject_put.
>+ kobject_put(mras->acls_kobj);
>+ return ret;
>+ }
>+
>+ ret = platform_driver_register(&hisi_mem_ras_driver);
>+ if (ret) {
>+ sysfs_remove_group(mras->acls_kobj, &acls_attr_group);
>+ kobject_put(mras->acls_kobj);
>+ kfree(mras);
>+ return ret;
>+ }
>+
>+ INIT_LIST_HEAD(&mras->list);
>+
>+ return 0;
>+}
>+module_init(hisi_mem_ras_init);
>+
>+static void __exit hisi_mem_ras_exit(void) {
>+ platform_driver_unregister(&hisi_mem_ras_driver);
>+ sysfs_remove_group(mras->acls_kobj, &acls_attr_group);
>+ kobject_put(mras->acls_kobj);
>+ kfree(mras);
>+}
>+module_exit(hisi_mem_ras_exit);
>+
>+MODULE_LICENSE("GPL v2");
>+MODULE_AUTHOR("Xiaofei Tan <tanxiaofei(a)huawei.com>");
>+MODULE_DESCRIPTION("HISILICON Memory RAS driver");
>+MODULE_ALIAS("platform:" DRV_NAME);
>diff --git a/drivers/soc/hisilicon/hisi_mem_ras.h
>b/drivers/soc/hisilicon/hisi_mem_ras.h
>new file mode 100644
>index 000000000000..a9acbb0c1012
>--- /dev/null
>+++ b/drivers/soc/hisilicon/hisi_mem_ras.h
>@@ -0,0 +1,84 @@
>+/* SPDX-License-Identifier: GPL-2.0 */
>+/*
>+ * Copyright (C) Huawei Technologies Co., Ltd. 2023. All rights reserved.
>+ */
>+
>+#ifndef _HISI_MEM_RAS_H
>+#define _HISI_MEM_RAS_H
>+
>+struct hisi_mem_ras {
>+ struct list_head list;
>+ struct kobject *acls_kobj;
>+};
>+
>+struct hisi_mem_mbox_client_info {
>+ struct mbox_client client;
>+ struct mbox_chan *mbox_chan;
>+ struct pcc_mbox_chan *pcc_chan;
>+ u64 deadline_us;
>+ void __iomem *pcc_comm_addr;
>+ struct completion done;
>+};
>+
>+struct hisi_mem_desc;
>+struct hisi_mem_dev;
>+
>+struct hisi_mem_verspecific_data {
>+ void (*rx_callback)(struct mbox_client *cl, void *mssg);
>+ int (*wait_cmd_complete)(struct hisi_mem_dev *hdev);
>+ void (*fill_pcc_shared_mem)(struct hisi_mem_dev *hdev,
>+ u8 cmd, struct hisi_mem_desc *desc,
>+ void __iomem *comm_space,
>+ u16 space_size);
>+ u16 shared_mem_size;
>+ bool has_txdone_irq;
>+};
>+
>+struct hisi_mem_dev {
>+ struct list_head list;
>+ struct platform_device *pdev;
>+ //struct acpi_device *acpi_dev;
>+ const struct hisi_mem_verspecific_data *verspec_data;
>+ /* device capabilities from firmware, like hisi_mem_CAPS_xxx. */
>+ u64 caps;
>+ u8 chan_id;
>+ u64 paddr_min;
>+ u64 paddr_max;
>+ struct mutex lock;
>+ struct hisi_mem_mbox_client_info cl_info; };
>+
>+#define HISI_MEM_PCC_SHARE_MEM_BYTES 128
>+enum hisi_mem_ras_cmd_type {
>+ HISI_MEM_RAS_ACLS = 0x4,
>+ HISI_MEM_RAS_PPR = 0X5,
>+};
>+
>+enum hisi_mem_ras_subcmd_type {
>+ HISI_MEM_RAS_QUERY_CAP = 0x0,
>+ HISI_MEM_RAS_DO_REPAIR = 0x1,
>+};
>+
>+struct hisi_mem_req_desc {
>+ u64 paddr;
>+ u8 subcmd;
>+ u8 rsv[3];
>+};
>+
>+#define HISI_MEM_ACLS_OK 0
>+#define HISI_MEM_ACLS_QUERY_NO_RES 1
>+#define HISI_MEM_ACLS_REPAIR_FAIL 2
>+#define HISI_MEM_ACLS_EINVAL 3
>+struct hisi_mem_rsp_desc {
>+ u8 retStatus; /* 0: success, other: failure */
>+ u8 rsv[7];
>+};
>+
>+struct hisi_mem_desc {
>+ union {
>+ struct hisi_mem_req_desc req;
>+ struct hisi_mem_rsp_desc rsp;
>+ };
>+};
>+
>+#endif
>--
>2.33.0
Thanks,
Shiju