Re: [PATCH 10/14] soc: qcom: Add RPM processor/subsystem driver

From: Konrad Dybcio <konrad.dybcio@linaro.org>
To: Stephan Gerhold <stephan@gerhold.net>
Cc: Bjorn Andersson <andersson@kernel.org>,
	Andy Gross <agross@kernel.org>, Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Mathieu Poirier <mathieu.poirier@linaro.org>,
	linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-remoteproc@vger.kernel.org
Subject: Re: [PATCH 10/14] soc: qcom: Add RPM processor/subsystem driver
Date: Mon, 5 Jun 2023 21:55:59 +0200	[thread overview]
Message-ID: <ce80b713-3c42-9cd3-73b1-9dcfb194140a@linaro.org> (raw)
In-Reply-To: <ZH480vq8iNgkoK78@gerhold.net>


On 5.06.2023 21:51, Stephan Gerhold wrote:
> On Mon, Jun 05, 2023 at 09:06:54PM +0200, Konrad Dybcio wrote:
>>
>>
>> On 5.06.2023 09:08, Stephan Gerhold wrote:
>>> Add a simple driver for the qcom,rpm-proc compatible that registers the
>>> "smd-edge" and populates other children defined in the device tree.
>>>
>>> Note that the DT schema belongs to the remoteproc subsystem while this
>>> driver is added inside soc/qcom. I argue that the RPM *is* a remoteproc,
>>> but as an implementation detail in Linux it can currently not benefit
>>> from anything provided by the remoteproc subsystem. The RPM firmware is
>>> usually already loaded and started by earlier components in the boot
>>> chain and is not meant to be ever restarted.
>>>
>>> To avoid breaking existing kernel configurations the driver is always
>>> built when smd-rpm.c is also built. They belong closely together anyway.
>>>
>>> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
>>> ---
[...]

> SMEM is arch_initcall() so at least for the SMD case it can never
> succeed probing in core_initcall() and would likely just cause
> unnecessary probe deferrals. That's why I chose arch_initcall().
> 
> Are you sure anything will really benefit from core_initcall() here?
> 
> I'd just like to avoid making things worse by using a random way too
> early initcall level. We have some really weird examples in the tree
> currently, e.g.:
>   - rpmpd: core_initcall()
>   - smd-rpm: arch_initcall()
>   - glink-rpm: subsys_initcall()
> But they actually need to be loaded in opposite order...
Yes, we should make some sort of dep graph and clean it up..

Konrad
> 
> Thanks,
> Stephan