Re: [PATCH v3 03/16] dt-bindings: remoteproc: k3-dsp: Allow referencing the timers' power domains

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dt-bindings: remoteproc: k3-dsp: misuse of power-domains for external timers
--

commit aa9adcac4a1bc9840a117c81a03e4ac0dade71bd
Author: Tomeu Vizoso <tomeu.vizoso@gmail.com>

dt-bindings: remoteproc: k3-dsp: Allow referencing the timers' power domains

This commit adds optional power-domains and power-domain-names properties
to the TI K3 DSP remoteproc binding so a client driver can hold the power
domains of two SoC timers used by the running firmware.

> diff --git a/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml b/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml
> index d6319c8e96f8c..80eaef0a19b8d 100644
> --- a/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/ti,k3-dsp-rproc.yaml
> @@ -76,6 +76,23 @@ properties:
>  # Optional properties:
>  # --------------------
>  
> +  power-domains:
> +    description: |
> +      Power domains of the SoC timers the running firmware ticks on and
> +      times its work out with. The firmware programs these timers itself
> +      and makes no power request for them, and the timers' own nodes are
> +      reserved for it, so nothing else requests their power; a client
> +      driver of the firmware holds them on while it drives the processor.
> +      The processor's own power is managed through ti,sci-dev-id, not here.
> +    items:
> +      - description: power domain of the first timer
> +      - description: power domain of the second timer
> +
> +  power-domain-names:
> +    items:
> +      - const: timer1
> +      - const: timer2

[Severity: Medium]
Should the standard power-domains property on the DSP node be used to
describe the power domains of external SoC timer peripherals rather than
the DSP device's own power domain?

In device tree bindings, power-domains on a device node specifies the power
domain(s) to which that device itself belongs. On TI K3 SoCs, the timer
peripherals already have their own device tree nodes (such as main_timer1
and main_timer2 in arch/arm64/boot/dts/ti/k3-am62p-j722s-common-main.dtsi)
which declare their own power-domains, clocks, and assigned-clocks.
Duplicating the timers' power-domains phandles onto the DSP node prevents
the binding from using power-domains for the processor's own power domain
(as done in ti,k3-r5f-rproc.yaml) and encodes a firmware-specific
workaround instead of referencing the timer nodes via phandles (similar to
ti,timers in ti,omap-remoteproc.yaml).

Additionally, requiring two fixed entries (timer1 and timer2) per DSP node
even when a single core only drives one timer appears to be shaped around
Linux's genpd_dev_pm_attach() behavior: as noted in the device tree patch
(arch/arm64/boot/dts/ti/k3-j722s-ti-ipc-firmware.dtsi), specifying two
entries keeps the platform bus from automatically attaching a single domain
to the K3 remoteproc device during platform_probe(), which would otherwise
set core->dev->pm_domain and cause dev_pm_domain_attach_list() in
thames_core_get_timers() (drivers/accel/thames/thames_core.c) to fail with
-EEXIST.

Could the binding reference the timer nodes via a phandle property instead
of repurposing power-domains on the DSP node around Linux genpd attachment
semantics?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023-thames-v3-0-3ab40aa17157@tomeuvizoso.net?part=3