Re: [PATCH V2 01/13] dt-bindings: remoteproc: qcom: Add support for multipd model

From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
To: Manikanta Mylavarapu <quic_mmanikan@quicinc.com>,
	Kalle Valo <kvalo@kernel.org>
Cc: agross@kernel.org, andersson@kernel.org,
	konrad.dybcio@linaro.org, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
	jassisinghbrar@gmail.com, mathieu.poirier@linaro.org,
	mturquette@baylibre.com, sboyd@kernel.org,
	quic_eberman@quicinc.com, quic_mojha@quicinc.com,
	loic.poulain@linaro.org, linux-arm-msm@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-remoteproc@vger.kernel.org, linux-clk@vger.kernel.org,
	quic_srichara@quicinc.com, quic_sjaganat@quicinc.com,
	quic_kathirav@quicinc.com, quic_anusha@quicinc.com,
	quic_poovendh@quicinc.com, quic_varada@quicinc.com,
	quic_devipriy@quicinc.com
Subject: Re: [PATCH V2 01/13] dt-bindings: remoteproc: qcom: Add support for multipd model
Date: Sat, 24 Jun 2023 09:19:35 +0200	[thread overview]
Message-ID: <8dbd8330-de83-b663-8105-e33c16687a88@linaro.org> (raw)
In-Reply-To: <59d92b83-f759-50de-9b8b-834c3c0bec5a@quicinc.com>

On 21/06/2023 13:39, Manikanta Mylavarapu wrote:
>>> on number of wcss radios connected on that board and only one instance
>>> of 'qcom,ipq5018-q6-mpd'.
>>>
>>
>> I don't understand why the user protection domains need a specific
>> compatible. Why do they need compatible at all?
>>
>> Not mentioning that amount of your domains on Q6 is actually fixed per
>> SoC and probably should not be in DT at all.
>>
>    root domain is fixed per soc (One Q6 DSP, one per soc)
>    user domain(s) are variable (based on number of wcss radios attached)
> 
>    The sequence to initialize, bring up, tear down the Q6 and wcss radios
>    are completely different. So in order to differentiate them, we will
>    need different compatibles. So this is a new rproc driver/architecture
>    which has a parent/child topology (Q6 DSP -> Master/parent controls
>    the WCSS (child)).

I understand you need different properties, but I don't see yet the
benefit of creating here artificial compatibles. Look at your ipq9574
DTSI change - it does not use even ipq9574 compatibles for 66% of its
children.

Maybe you should have there just property describing type of device or
bringup?

Given SoC cannot come with different amount of children (PD) and
different amount of radios. You even fix the firmware name, so
boards/customers cannot use anything else. It's fixed and the only
variable element here is disabling some of the blocks per board, if they
do not have physical interface (e.g. radio).

You even hard-code the number of PD by using "pd-[123]", without unit
address, so you do not expect it will grow.

Unless you want to say that these are devices? But your binding does not
really suggest it...

Best regards,
Krzysztof