Re: [PATCH v4 2/6] peci: controller: Add StarFive JHB100 PECI driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] peci: starfive: missing reg_stride in starfive_peci_regmap_config
--

commit 2951f13eb5d64c12334f96bb237dfd0078d838b2
Author: Changhuang Liang <changhuang.liang@starfivetech.com>

peci: controller: Add StarFive JHB100 PECI driver

This commit adds a PECI controller driver for the StarFive JHB100 SoC to
support PECI protocol communication for CPU thermal management.

> diff --git a/drivers/peci/controller/peci-starfive.c b/drivers/peci/controller/peci-starfive.c
> new file mode 100644
> index 0000000000000..9f59721b891df
> --- /dev/null
> +++ b/drivers/peci/controller/peci-starfive.c
> @@ -0,0 +1,367 @@

[ ... ]

> +static const struct regmap_config starfive_peci_regmap_config = {
> +	.reg_bits = 32,
> +	.val_bits = 32,
> +	.max_register = STARFIVE_PECI_MAX_REG,
> +	.fast_io = true,
> +};

[Severity: Medium]
Should starfive_peci_regmap_config also set .reg_stride = 4?

When starfive_peci_probe() calls devm_regmap_init_mmio() with
config->reg_stride left at 0, __regmap_init() defaults map->reg_stride to 1:

drivers/base/regmap/regmap.c:__regmap_init() {
	...
	if (config->reg_stride)
		map->reg_stride = config->reg_stride;
	else
		map->reg_stride = 1;
	...
}

Because .max_register is set to STARFIVE_PECI_MAX_REG (0x100), reading the
regmap debugfs registers file (/sys/kernel/debug/regmap/<dev>/registers) will
cause regmap_debugfs_get_dump_start() and regmap_read_debugfs() to iterate
across registers in 1-byte increments and issue unaligned 32-bit readl() MMIO
accesses via regmap_mmio_read32le(), which can trigger misaligned MMIO access
traps or bus errors on RISC-V and return corrupted dump data.

[ ... ]

> +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);

[ ... ]

> -- 
> 2.34.1

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019114502.55307-1-changhuang.liang@starfivetech.com?part=2