On 06/06/2023 10:55, Stephan Gerhold wrote:
> <On Tue, Jun 06, 2023 at 08:36:10AM +0200, Krzysztof Kozlowski wrote:
>> On 05/06/2023 09:08, Stephan Gerhold wrote:
>>> On Qualcomm platforms, most subsystems (e.g. audio/modem DSP) are
>>> described as remote processors in the device tree, with a dedicated
>>> node where properties and services related to them can be described.
>>>
>>> The Resource Power Manager (RPM) is also such a subsystem, with a
>>> remote processor that is running a special firmware. Unfortunately,
>>> the RPM never got a dedicated node representing it properly in the
>>> device tree. Most of the RPM services are described below a top-level
>>> /smd or /rpm-glink node.
>>
>> Then what is rpm-requests? This is the true RPM. It looks like you now
>> duplicate half of it in a node above. Unless you want here to describe
>> ways to communicate with the RPM, not the RPM itself.
>>
>
> I think you're confusing hardware and firmware here. The rpm-proc node
> represents the RPM hardware block (or perhaps the RPM firmware overall),
> while rpm-requests is just *one* communication interface provided by the
> RPM firmware. Here is a rough picture of the RPM "subsystem" I'm trying
> to describe:
>
> +--------------------------------------------+
> | RPM subsystem (qcom,rpm-proc) |
> | |
> reset | +---------------+ +-----+ +-----+ |
> --------->| | | MPM | | CPR | ... |
> IPC interrupts | | ARM Cortex-M3 | +-----+ +-----+ |
> ----------------->| |--- | | |
> | +---------------+ |---------------------- |
> | +---------------+ | |
> | | Code RAM |--| +------------------+ |
> | +---------------+ | | | |
> | +---------------+ | | Message RAM | |
> | | Data RAM |--|--| | |
> | +---------------+ | +------------------+ |
> +--------------------|-----------------------+
> v
> NoC
>
> (Somewhat adapted from Figure 8-1 RPM overview in the APQ8016E Technical
> Reference Manual, but I added some extra stuff.)
>
> As you can see neither "SMD" nor "rpm-requests" exist in hardware.
> Again, it's just one communication protocol implemented by the RPM
> firmware running on the Cortex-M3 processor, much like a webserver
> running on a PC.
>
> When the SoC is started only the hardware block exists. Usually a
> component in the boot chain loads firmware into the code/data RAM and
> then de-asserts the reset line to start the Cortex-M3 processor.
>
> This is not guaranteed to happen. For example, I have a special firmware
> version where the firmware is only loaded but not brought out of reset.
> In this case Linux is responsible to de-assert the reset line.
> In follow-up patches I add that to the outer qcom,rpm-proc node:
>
> remoteproc {
> compatible = "qcom,msm8916-rpm-proc", "qcom,rpm-proc";
> resets = <&gcc GCC_RPM_RESET>;
> iommus = <&apps_smmu 0x0040>;
>
> smd-edge {
> /* ... */
> rpm-requests {
> qcom,smd-channels = "rpm_requests";
> };
> };
> };
>
> This reset line cannot be described on the rpm-requests node. Until the
> firmware is started there is no such thing as "SMD" or "rpm-requests".
OK, that makes sense, thank you for clarifying. It would be nice to
include it in the binding description (especially that you already wrote
it for me in the email :) ).
>
> When the RPM firmware is started it sets up some data structures in the
> message RAM and then begins serving requests, perhaps like this:
>
> +----------------------------------+
> | ARM Cortex-M3 |
> | +------------------------------+ | +--------------------------------+
> | | RPM Firmware | | | Message RAM |
> IPC Interrupt | | +----------------------+ | | | +----------------------------+ |
> ------------------>| SMD Server | | | | | SMD Data Structures & FIFO | |
> | | | +--------------+ | | | | | +--------------+ | |
> | | | | rpm_requests | ... | | | | | | rpm_requests | ... | |
> | | | +--------------+ | ... | | | | +--------------+ | |
> | | +----------------------+ | | | +----------------------------+ |
> | +------------------------------+ | | ... |
> +----------------------------------+ +--------------------------------+
>
> The "smd-edge" node represents the SMD part and "rpm_requests" is one
> of the communication channels that can be used to talk to the RPM firmware.
>
> Everything below rpm-requests: clocks, regulators, power domains are
> pure firmware constructions. They do not exist in the RPM hardware block,
> the firmware just acts as a proxy to collect votes from different
> clients and then configures the actual hardware.
OK
>
>>
>>> However, SMD/GLINK is just one of the communication channels to the RPM
>>> firmware. For example, the MPM interrupt functionality provided by the
>>> RPM does not use SMD/GLINK but writes directly to a special memory
>>> region allocated by the RPM firmware in combination with a mailbox.
>>> Currently there is no good place in the device tree to describe this
>>> functionality. It doesn't belong below SMD/GLINK but it's not an
>>> independent top-level device either.
>>>
>>> Introduce a new "qcom,rpm-proc" compatible that allows describing the
>>> RPM as a remote processor/subsystem like all others. The SMD/GLINK node
>>> is moved to a "smd-edge"/"glink-edge" subnode consistent with other
>>> existing bindings. Additional subnodes (e.g. interrupt-controller for
>>> MPM, rpm-master-stats) can be also added there.
>>
>> If this was about to stay, you should also update the qcom,smd.yaml, so
>> there will be no duplication.
>>
>
> qcom,smd.yaml is deprecated in the next patch (PATCH 07/14).
Please squash it, because it is logically one change - you rework one
solution and deprecate other.
>
>>>
>>> Signed-off-by: Stephan Gerhold <stephan@xxxxxxxxxxx>
>>> ---
>>> .../bindings/remoteproc/qcom,rpm-proc.yaml | 125 +++++++++++++++++++++
>>> 1 file changed, 125 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,rpm-proc.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,rpm-proc.yaml
>>> new file mode 100644
>>> index 000000000000..c06dd4f66503
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,rpm-proc.yaml
>>> @@ -0,0 +1,125 @@
>>> [...]
>>> + interrupt-controller:
>>> + type: object
>>> + $ref: /schemas/interrupt-controller/qcom,mpm.yaml#
>>> + description:
>>> + MSM Power Manager (MPM) interrupt controller that monitors interrupts
>>> + when the system is asleep.
>>
>> Isn't this a service of RPM?
>>
>>> +
>>> + master-stats:
>>> + $ref: /schemas/soc/qcom/qcom,rpm-master-stats.yaml#
>>> + description:
>>> + Subsystem-level low-power mode statistics provided by RPM.
>>
>> The same question.
>>
>
> Yes, they are services of the RPM firmware. But they have no relation to
> the SMD/GLINK channel.
>
> To clarify this I extend my picture from above with MPM:
>
> +----------------------------------+
> | ARM Cortex-M3 |
> | +------------------------------+ | +--------------------------------+
> | | RPM Firmware | | | Message RAM |
> IPC Interrupt 0 | | +----------------------+ | | | +----------------------------+ |
> ------------------->| SMD Server | | | | | SMD Data Structures & FIFO | |
> | | | +--------------+ | | | | | +--------------+ | |
> | | | | rpm_requests | ... | | | | | | rpm_requests | ... | |
> | | | +--------------+ | ... | | | | +--------------+ | |
> | | +----------------------+ | | | +----------------------------+ |
> IPC Interrupt 1 | | +----------------------+ | | | +----------------------------+ |
> ------------------->| MPM virtualization |<-----------| MPM register copy (vMPM) | |
> | | +----------------------+ | | | +----------------------------+ |
> | +-----------|------------------+ | | ... |
> +-------------v--------------------+ +--------------------------------+
> +--------------+
> | MPM Hardware |
> +--------------+
>
> As you can see, SMD and MPM are siblings. The MPM functionality
> implemented by the RPM firmware is not a child of the SMD server.
OK, also please include it in the description.
>
> It's a bit like a webserver and emailserver running on the same PC.
> Two separate services accessible via different ports and protocols.
>
> Thanks,
> Stephan
>
> NOTE: All of this is just my interpretation based on the public hints
> that exist. I have no access to internal documentation or source code
> that would prove that all of this is correct.
Sounds good enough for me :). Thank you.
Best regards,
Krzysztof
