Re: [PATCH v3 2/6] peci: controller: Add StarFive JHB100 PECI driver
From: Winiarska, Iwona
Date: Fri Oct 16 2026 - 16:10:31 EST
On Thu, 2026-10-08 at 03:06 -0700, Changhuang Liang wrote:
> Add PECI controller driver for StarFive JHB100 SoC. The driver supports
> PECI protocol communication for CPU thermal management.
>
> For this controller, the special clock and reset operation sequence is:
> probe: clk_prepare_enable() then reset_control_deassert()
> remove: clk_disable_unprepare() then reset_control_assert()
>
> Co-developed-by: Mason Huo <mason.huo@xxxxxxxxxxxxxxxx>
> Signed-off-by: Mason Huo <mason.huo@xxxxxxxxxxxxxxxx>
> Signed-off-by: Changhuang Liang <changhuang.liang@xxxxxxxxxxxxxxxx>
> ---
> MAINTAINERS | 8 +
> drivers/peci/controller/Kconfig | 17 ++
> drivers/peci/controller/Makefile | 1 +
> drivers/peci/controller/peci-starfive.c | 376 ++++++++++++++++++++++++
> 4 files changed, 402 insertions(+)
> create mode 100644 drivers/peci/controller/peci-starfive.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d3dc32c7a0f0..1fb3a3f03fce 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -26653,6 +26653,14 @@ S: Supported
> F: Documentation/devicetree/bindings/interrupt-
> controller/starfive,jhb100-intc.yaml
> F: drivers/irqchip/irq-starfive-jhb100-intc.c
>
> +STARFIVE JHB100 PECI DRIVER
> +M: Changhuang Liang <changhuang.liang@xxxxxxxxxxxxxxxx>
> +M: Mason Huo <mason.huo@xxxxxxxxxxxxxxxx>
> +L: openbmc@xxxxxxxxxxxxxxxx (moderated for non-subscribers)
> +S: Maintained
> +F: Documentation/devicetree/bindings/peci/starfive,jhb100-peci.yaml
> +F: drivers/peci/controller/peci-starfive.c
> +
> STARFIVE JHB100 PINCTRL DRIVERS
> M: Changhuang Liang <changhuang.liang@xxxxxxxxxxxxxxxx>
> M: Lianfeng Ouyang <lianfeng.ouyang@xxxxxxxxxxxxxxxx>
> diff --git a/drivers/peci/controller/Kconfig b/drivers/peci/controller/Kconfig
> index 4f9c245ad042..c0c35bc179ef 100644
> --- a/drivers/peci/controller/Kconfig
> +++ b/drivers/peci/controller/Kconfig
> @@ -32,3 +32,20 @@ config PECI_NPCM
>
> This support is also available as a module. If so, the module
> will be called peci-npcm.
> +
> +config PECI_STARFIVE
> + tristate "STARFIVE PECI support"
> + depends on ARCH_STARFIVE || COMPILE_TEST
> + depends on OF
> + depends on HAS_IOMEM
> + select REGMAP_MMIO
> + help
> + This option enables PECI controller driver for StarFive JHB100
> + SoC. It allows BMC to discover devices connected to it, and
> + communicate with them using PECI protocol.
> +
> + Say Y here if your system runs on StarFive JHB100 SoC and you are
> + using it as BMC for Intel platform.
> +
> + This driver can also be built as a module. If so, the module will
> + be called peci-starfive.
> diff --git a/drivers/peci/controller/Makefile
> b/drivers/peci/controller/Makefile
> index e247449bb423..935e356b058c 100644
> --- a/drivers/peci/controller/Makefile
> +++ b/drivers/peci/controller/Makefile
> @@ -2,3 +2,4 @@
>
> obj-$(CONFIG_PECI_ASPEED) += peci-aspeed.o
> obj-$(CONFIG_PECI_NPCM) += peci-npcm.o
> +obj-$(CONFIG_PECI_STARFIVE) += peci-starfive.o
> diff --git a/drivers/peci/controller/peci-starfive.c
> b/drivers/peci/controller/peci-starfive.c
> new file mode 100644
> index 000000000000..93aca2413f5c
> --- /dev/null
> +++ b/drivers/peci/controller/peci-starfive.c
> @@ -0,0 +1,376 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2024 StarFive Technology Co., Ltd.
> + */
> +
> +#include <linux/unaligned.h>
Order includes alphabetically.
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/completion.h>
> +#include <linux/interrupt.h>
> +#include <linux/jiffies.h>
> +#include <linux/minmax.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/peci.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +
> +/* Control register */
> +#define STARFIVE_PECI_CTRL 0x00
> +/* automatically clears after transfer started */
> +#define STARFIVE_PECI_CTRL_START BIT(0)
> +#define STARFIVE_PECI_CTRL_FCS_MODE BIT(2)
> +#define STARFIVE_PECI_CTRL_AW_FCS_EN BIT(3)
> +#define STARFIVE_PECI_CTRL_RDY BIT(4)
> +
> +/* Read/Write length register */
> +#define STARFIVE_PECI_HDR 0x04
> +#define STARFIVE_PECI_HDR_RD_LEN_MASK GENMASK(23, 16)
> +#define STARFIVE_PECI_HDR_WR_LEN_MASK GENMASK(15, 8)
> +#define STARFIVE_PECI_HDR_TARGET_ADDR_MASK GENMASK(7, 0)
> +
> +/* Feature control register */
> +#define STARFIVE_PECI_F_CTRL 0x0c
> +#define STARFIVE_PECI_F_CTRL_EN BIT(0)
> +
> +/* Interrupt enable register */
> +#define STARFIVE_PECI_INT_EN 0x10
> +#define STARFIVE_PECI_INT_XFER_DONE BIT(0)
> +#define STARFIVE_PECI_INT_CFG_ERR BIT(1)
> +#define STARFIVE_PECI_INT_TBIT_ERR BIT(2)
> +#define STARFIVE_PECI_INT_BAD_WR_FCS BIT(3)
> +#define STARFIVE_PECI_INT_ABORT_WR_FCS BIT(4)
> +#define STARFIVE_PECI_INT_BAD_RD_FCS BIT(5)
> +#define STARFIVE_PECI_INT_BUS_CONTENTION BIT(6)
> +#define STARFIVE_PECI_INT_TBIT_OVER_LIMIT BIT(7)
> +#define STARFIVE_PECI_INT_ALL GENMASK(7, 0)
> +
> +/* Interrupt status register */
> +#define STARFIVE_PECI_INT_STS 0x14
> +#define STARFIVE_PECI_STS_XFER_DONE BIT(0)
> +#define STARFIVE_PECI_STS_CFG_ERR BIT(1)
> +#define STARFIVE_PECI_STS_TBIT_ERR BIT(2)
> +#define STARFIVE_PECI_STS_BAD_WR_FCS BIT(3)
> +#define STARFIVE_PECI_STS_ABORT_WR_FCS BIT(4)
> +#define STARFIVE_PECI_STS_BAD_RD_FCS BIT(5)
> +#define STARFIVE_PECI_STS_BUS_CONTENTION BIT(6)
> +#define STARFIVE_PECI_STS_TBIT_OVER_LIMIT BIT(7)
> +#define STARFIVE_PECI_STS_MASK GENMASK(7, 0)
> +
> +/* Received FCS data register */
> +#define STARFIVE_PECI_FCS_RCVD 0x1C
> +#define STARFIVE_PECI_RCVD_WR_FCS_MASK GENMASK(7, 0)
> +#define STARFIVE_PECI_RCVD_RD_FCS_MASK GENMASK(15, 8)
> +
> +/* Rx/Tx Data Buffer Registers */
> +#define STARFIVE_PECI_WR_DATA(n) (0x70 + ((n) * 4))
> +#define STARFIVE_PECI_RD_DATA(n) (0xB0 + ((n) * 4))
> +
> +/* Hardware TX/RX data FIFOs are 64 bytes, but PECI core caps requests lower
> */
> +#define STARFIVE_PECI_DATA_BUF_SIZE_MAX min(64,
> PECI_REQUEST_MAX_BUF_SIZE)
> +#define STARFIVE_PECI_MAX_REG 0x100
> +
> +/* Timeout */
> +#define STARFIVE_PECI_IDLE_CHECK_TIMEOUT_US (50 * USEC_PER_MSEC)
> +#define STARFIVE_PECI_IDLE_CHECK_INTERVAL_US (10 * USEC_PER_MSEC)
> +#define STARFIVE_PECI_CMD_TIMEOUT_MS_DEFAULT 1000
> +#define STARFIVE_PECI_CMD_TIMEOUT_MS_MAX 1000
> +
> +struct starfive_peci {
> + u32 cmd_timeout_ms;
> + struct completion xfer_complete;
> + struct regmap *regmap;
> + u32 status;
> + spinlock_t lock; /* sync completion status */
> + struct peci_controller *controller;
> + struct device *dev;
> + struct clk *clk;
> + struct reset_control *rst;
> + int irq;
> +};
> +
> +static int starfive_peci_xfer(struct peci_controller *controller,
> + u8 addr, struct peci_request *req)
> +{
> + struct starfive_peci *priv = dev_get_drvdata(controller->dev.parent);
> + unsigned long timeout = msecs_to_jiffies(priv->cmd_timeout_ms);
> + u32 msg_rd;
> + u32 cmd_sts;
> + u32 peci_hdr;
> + int i, ret, j;
> +
> + if (req->tx.len > STARFIVE_PECI_DATA_BUF_SIZE_MAX ||
> + req->rx.len > STARFIVE_PECI_DATA_BUF_SIZE_MAX)
> + return -EINVAL;
> +
> + /* Check command sts and bus idle state */
> + ret = regmap_read_poll_timeout(priv->regmap,
> + STARFIVE_PECI_CTRL, cmd_sts,
> + cmd_sts & STARFIVE_PECI_CTRL_RDY,
> + STARFIVE_PECI_IDLE_CHECK_INTERVAL_US,
> + STARFIVE_PECI_IDLE_CHECK_TIMEOUT_US);
> + if (ret)
> + return ret;
> +
> + spin_lock_irq(&priv->lock);
> + reinit_completion(&priv->xfer_complete);
> +
> + peci_hdr = FIELD_PREP(STARFIVE_PECI_HDR_TARGET_ADDR_MASK, addr) |
> + FIELD_PREP(STARFIVE_PECI_HDR_WR_LEN_MASK, req->tx.len) |
> + FIELD_PREP(STARFIVE_PECI_HDR_RD_LEN_MASK, req->rx.len);
> + regmap_write(priv->regmap, STARFIVE_PECI_HDR, peci_hdr);
> +
> + /* Currently, we do not support write commands, force this bit to 0.
> */
> + regmap_write_bits(priv->regmap, STARFIVE_PECI_CTRL,
> + STARFIVE_PECI_CTRL_AW_FCS_EN, 0);
> +
> + for (i = 0; i < ALIGN(req->tx.len, 4) / 4; i++)
> + regmap_write(priv->regmap, STARFIVE_PECI_WR_DATA(i),
> + get_unaligned_le32(&req->tx.buf[4 * i]));
> +
> + dev_dbg(priv->dev, "addr : %#02x, tx.len : %#02x, rx.len : %#02x\n",
> + addr, req->tx.len, req->rx.len);
> + print_hex_dump_bytes("TX : ", DUMP_PREFIX_NONE, req->tx.buf,
> + req->tx.len);
> +
> + priv->status = 0;
> +
> + regmap_write_bits(priv->regmap, STARFIVE_PECI_CTRL,
> + STARFIVE_PECI_CTRL_START,
> STARFIVE_PECI_CTRL_START);
> +
> + spin_unlock_irq(&priv->lock);
> +
> + ret = wait_for_completion_interruptible_timeout(&priv->xfer_complete,
> + timeout);
> + if (ret < 0)
> + return ret;
> +
> + if (ret == 0) {
> + dev_dbg(priv->dev, "Timeout waiting for a response\n");
> + return -ETIMEDOUT;
> + }
> +
> + spin_lock_irq(&priv->lock);
> +
> + if (priv->status != STARFIVE_PECI_STS_XFER_DONE) {
> + spin_unlock_irq(&priv->lock);
> + dev_dbg(priv->dev, "No valid response, status: %#02x\n",
> + priv->status);
> + return -EIO;
> + }
> +
> + regmap_read(priv->regmap, STARFIVE_PECI_FCS_RCVD, &msg_rd);
> + dev_dbg(priv->dev, "write & read command FCS : %#02lx & %#02lx\n",
> + FIELD_GET(STARFIVE_PECI_RCVD_WR_FCS_MASK, msg_rd),
> + FIELD_GET(STARFIVE_PECI_RCVD_RD_FCS_MASK, msg_rd));
> +
> + for (i = 0; i < ALIGN(req->rx.len, 4) / 4; i++) {
> + regmap_read(priv->regmap, STARFIVE_PECI_RD_DATA(i), &msg_rd);
> +
> + if (req->rx.len - 4 * (i + 1) >= 0) {
> + put_unaligned_be32(msg_rd, &req->rx.buf[req->rx.len -
> 4 * (i + 1)]);
> + } else {
> + for (j = req->rx.len % 4 - 1; j >= 0; j--) {
> + req->rx.buf[j] = (u8)msg_rd;
> + msg_rd >>= 8;
> + }
> + }
> + }
> +
> + spin_unlock_irq(&priv->lock);
Do we need to keep the lock for copying data into rx.buf? Or is it just to
protect the completion status? If the latter, then please reduce its scope to
avoid misleading whoever is looking at this code into thinking that e.g. rx.buf
is also shared with the IRQ handler.
Thanks
-Iwona
> +
> + print_hex_dump_bytes("RX : ",
> + DUMP_PREFIX_NONE, req->rx.buf, req->rx.len);
> +
> + return 0;
> +}
> +
> +static irqreturn_t starfive_peci_irq_handler(int irq, void *arg)
> +{
> + struct starfive_peci *priv = arg;
> + u32 status;
> +
> + spin_lock(&priv->lock);
> + regmap_read(priv->regmap, STARFIVE_PECI_INT_STS, &status);
> + priv->status |= (status & STARFIVE_PECI_STS_MASK);Order
> + regmap_write(priv->regmap, STARFIVE_PECI_INT_STS,
> + status & STARFIVE_PECI_STS_MASK);
> +
> + /*
> + * All commands are terminated with the XFER_DONE bit set, even when
> + * an error condition is reported alongside it.
> + */
> + if (status & STARFIVE_PECI_STS_XFER_DONE)
> + complete(&priv->xfer_complete);
> +
> + spin_unlock(&priv->lock);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void starfive_peci_clk_reset_release(void *data)
> +{
> + struct starfive_peci *priv = data;
> +
> + clk_disable_unprepare(priv->clk);
> + reset_control_assert(priv->rst);
> +}
> +
> +static int starfive_peci_init_clk_rst(struct starfive_peci *priv)
> +{
> + int ret;
> +
> + priv->clk = devm_clk_get(priv->dev, NULL);
> + if (IS_ERR(priv->clk))
> + return dev_err_probe(priv->dev, PTR_ERR(priv->clk),
> + "Failed to get peci clock\n");
> +
> + priv->rst = devm_reset_control_get_exclusive(priv->dev, NULL);
> + if (IS_ERR(priv->rst))
> + return dev_err_probe(priv->dev, PTR_ERR(priv->rst),
> + "Failed to get reset control\n");
> +
> + ret = clk_prepare_enable(priv->clk);
> + if (ret)
> + return dev_err_probe(priv->dev, ret,
> + "Failed to enable peci clock\n");
> +
> + ret = reset_control_deassert(priv->rst);
> + if (ret) {
> + clk_disable_unprepare(priv->clk);
> + return dev_err_probe(priv->dev, ret,
> + "Failed to deassert reset control\n");
> + }
> +
> + return devm_add_action_or_reset(priv->dev,
> starfive_peci_clk_reset_release,
> + priv);
> +}
> +
> +static int starfive_peci_init_ctrl(struct starfive_peci *priv)
> +{
> + u32 cmd_sts;
> + int ret;
> +
> + ret = device_property_read_u32(priv->dev, "cmd-timeout-ms",
> + &priv->cmd_timeout_ms);
> + if (ret) {
> + priv->cmd_timeout_ms = STARFIVE_PECI_CMD_TIMEOUT_MS_DEFAULT;
> + } else if (priv->cmd_timeout_ms > STARFIVE_PECI_CMD_TIMEOUT_MS_MAX ||
> + priv->cmd_timeout_ms == 0) {
> + dev_warn(priv->dev,
> + "Invalid cmd-timeout-ms: %u, falling back to: %u\n",
> + priv->cmd_timeout_ms,
> + STARFIVE_PECI_CMD_TIMEOUT_MS_DEFAULT);
> +
> + priv->cmd_timeout_ms = STARFIVE_PECI_CMD_TIMEOUT_MS_DEFAULT;
> + }
> +
> + regmap_write_bits(priv->regmap, STARFIVE_PECI_F_CTRL,
> + STARFIVE_PECI_F_CTRL_EN, STARFIVE_PECI_F_CTRL_EN);
> + regmap_write_bits(priv->regmap, STARFIVE_PECI_CTRL,
> + STARFIVE_PECI_CTRL_FCS_MODE,
> STARFIVE_PECI_CTRL_FCS_MODE);
> +
> + ret = regmap_read_poll_timeout(priv->regmap,
> + STARFIVE_PECI_CTRL, cmd_sts,
> + cmd_sts & STARFIVE_PECI_CTRL_RDY,
> + STARFIVE_PECI_IDLE_CHECK_INTERVAL_US,
> + STARFIVE_PECI_IDLE_CHECK_TIMEOUT_US);
> + if (ret)
> + return ret;
> +
> + regmap_write(priv->regmap, STARFIVE_PECI_INT_STS,
> STARFIVE_PECI_STS_MASK);
> +
> + /* PECI interrupt enable */
> + regmap_write(priv->regmap, STARFIVE_PECI_INT_EN,
> STARFIVE_PECI_INT_ALL);
> +
> + return 0;
> +}
> +
> +static const struct regmap_config starfive_peci_regmap_config = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .max_register = STARFIVE_PECI_MAX_REG,
> + .fast_io = true,
> +};
> +
> +static const struct peci_controller_ops starfive_ops = {
> + .xfer = starfive_peci_xfer,
> +};
> +
> +static int starfive_peci_probe(struct platform_device *pdev)
> +{
> + struct peci_controller *controller;
> + struct starfive_peci *priv;
> + void __iomem *base;
> + int ret;
> +
> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + priv->dev = &pdev->dev;
> + dev_set_drvdata(&pdev->dev, priv);
> +
> + base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + priv->regmap = devm_regmap_init_mmio(&pdev->dev, base,
> + &starfive_peci_regmap_config);
> + if (IS_ERR(priv->regmap))
> + return dev_err_probe(priv->dev, PTR_ERR(priv->regmap),
> + "Failed to initialize regmap\n");
> +
> + priv->irq = platform_get_irq(pdev, 0);
> + if (priv->irq < 0)
> + return dev_err_probe(priv->dev, priv->irq, "Failed to get
> IRQ\n");
> +
> + init_completion(&priv->xfer_complete);
> + spin_lock_init(&priv->lock);
> +
> + ret = starfive_peci_init_clk_rst(priv);
> + if (ret)
> + return dev_err_probe(priv->dev, ret, "Failed to initialize
> clock and reset\n");
> +
> + ret = devm_request_irq(&pdev->dev,
> + priv->irq, starfive_peci_irq_handler,
> + 0, "peci-starfive", priv);
> + if (ret)
> + return dev_err_probe(priv->dev, ret, "Failed to request
> IRQ\n");
> +
> + ret = starfive_peci_init_ctrl(priv);
> + if (ret)
> + return dev_err_probe(priv->dev, ret, "Failed to initialize
> control\n");
> +
> + controller = devm_peci_controller_add(priv->dev, &starfive_ops);
> + if (IS_ERR(controller))
> + return dev_err_probe(priv->dev, PTR_ERR(controller),
> + "Failed to add peci controller\n");
> +
> + priv->controller = controller;
> +
> + return 0;
> +}
> +
> +static const struct of_device_id starfive_peci_of_table[] = {
> + { .compatible = "starfive,jhb100-peci", },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, starfive_peci_of_table);
> +
> +static struct platform_driver starfive_peci_driver = {
> + .probe = starfive_peci_probe,
> + .driver = {
> + .name = KBUILD_MODNAME,
> + .of_match_table = starfive_peci_of_table,
> + },
> +};
> +module_platform_driver(starfive_peci_driver);
> +
> +MODULE_AUTHOR("Changhuang Liang <changhuang.liang@xxxxxxxxxxxxxxxx>");
> +MODULE_AUTHOR("Mason Huo <mason.huo@xxxxxxxxxxxxxxxx>");
> +MODULE_DESCRIPTION("StarFive PECI driver");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS("PECI");