Re: [PATCH v5 4/7] riscv: dts: eswin: add hsp bus node

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] dts: unclocked syscon MMIO access due to missing clocks property
--

commit 411f5ceff131ac2449926a00c5f169d3aa665b6d
Author: Pinkesh Vaghela <pinkesh.vaghela@einfochips.com>

riscv: dts: eswin: add hsp bus node

This commit adds an HSP bus node to manage the HSP CFG clock used by HSP
peripherals. This clock is required for accessing both the HSP CSR registers
and the register spaces of HSP peripheral devices.

> diff --git a/arch/riscv/boot/dts/eswin/eic7700.dtsi b/arch/riscv/boot/dts/eswin/eic7700.dtsi
> --- a/arch/riscv/boot/dts/eswin/eic7700.dtsi
> +++ b/arch/riscv/boot/dts/eswin/eic7700.dtsi
> @@ -252,6 +252,19 @@ plic: interrupt-controller@c000000 {
>  			#interrupt-cells = <1>;
>  		};
>  
> +		hsp: bus@50400000 {
> +			compatible = "simple-pm-bus";
> +			ranges = <0x0 0x50400000 0x0 0x50400000 0x0 0xa0000>;
> +			clocks = <&clk EIC7700_CLK_GATE_HSP_CFG_CLK>;
> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +
> +			syscon: syscon@50440000 {
> +				compatible = "eswin,eic7700-syscfg", "syscon";
> +				reg = <0x0 0x50440000 0x0 0x2000>;
> +			};

[Severity: High]
Will accessing this syscon node result in an unclocked MMIO fault?

The syscon node lacks a clocks property and seems to rely on the parent
simple-pm-bus for clocks. However, generic syscon does not trigger parent
runtime PM.

When looking at of_syscon_register() in drivers/mfd/syscon.c, it only attaches
a clock if the clocks property is present on the syscon node itself:

drivers/mfd/syscon.c:of_syscon_register() {
        ...
        if (check_res) {
                clk = of_clk_get(np, 0);
                if (IS_ERR(clk)) {
                        ret = PTR_ERR(clk);
                        /* clock is optional */
                        if (ret != -ENOENT)
                                goto err_clk;
                } else {
                        ret = regmap_mmio_attach_clk(regmap, clk);
        ...
}

Meanwhile, looking at simple_pm_bus_probe() in drivers/bus/simple-pm-bus.c,
the simple-pm-bus only enables runtime PM:

drivers/bus/simple-pm-bus.c:simple_pm_bus_probe() {
        ...
        pm_runtime_enable(&pdev->dev);

        if (np)
                of_platform_populate(np, NULL, lookup, &pdev->dev);
        ...
}

Since simple-pm-bus strictly relies on children to wake it up, and syscon
bypasses platform_device PM entirely, the bus is never resumed. Does this
mean any driver or debugfs interface accessing the eswin,eic7700-syscfg
regmap will perform MMIO without enabling the HSP_CFG_CLK?

> +		};
> +
>  		uart0: serial@50900000 {
>  			compatible = "snps,dw-apb-uart";
>  			reg = <0x0 0x50900000 0x0 0x10000>;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015125454.1381941-1-pinkesh.vaghela@einfochips.com?part=4