Re: [PATCH v3 5/8] riscv: dts: eswin: add hsp bus node

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Generic `syscon` node under `simple-pm-bus` will be accessed with its clock disabled, causing a bus fault.
--

--- Patch [5]: [PATCH v3 5/8] riscv: dts: eswin: add hsp bus node ---
commit 4e57b37c3aadc988b1aed52b079b78ca92be234c
Author: Pinkesh Vaghela <[email protected]>

riscv: dts: eswin: add hsp bus node

Add 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.

> --- a/arch/riscv/boot/dts/eswin/eic7700.dtsi
> +++ b/arch/riscv/boot/dts/eswin/eic7700.dtsi
> @@ -252,6 +252,19 @@ plic: interrupt-controller@c000000 {
[ ... ]
> +		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 cause a bus fault due to the clock being
disabled?

Because the child syscon@50440000 uses the generic syscon compatible string, it
does not bind to a standard platform driver that would invoke
pm_runtime_get_sync(). As a result, the parent simple-pm-bus will remain
suspended and the clock EIC7700_CLK_GATE_HSP_CFG_CLK will stay disabled.

Additionally, since the clocks property is on the parent bus rather than
directly on the syscon node, of_syscon_register() cannot fetch the clock to
attach it to the underlying regmap_mmio.

Does this clock need to be placed directly on the syscon node so the generic
regmap core can manage it during register accesses?

> +			};
> +		};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5