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

[Manikanta Mylavarapu]

On 7/1/2023 4:21 PM, Krzysztof Kozlowski wrote:
> On 30/06/2023 09:12, Manikanta Mylavarapu wrote:
> > On 6/24/2023 12:49 PM, Krzysztof Kozlowski wrote:
> > > 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?
> >
> > 	Yeah i got your point. Indeed the requirement here is to
> > 	have device specific compatibles, so will have just two
> > 	compatible one for Q6-MPD and another for WCSS-MPD device's
> >
> >
> > 	"qcom,q6-mpd" --> For Q6-MPD device
> > 	"qcom,wcss-mpd" --> For WCSS-MPD device
> >
> > 	Is this approach fine ?
>
> Can you fix your reply style, so it is like on every mailing list? Some
> weird indentation does no help to read it.

Sure, i will change my reply style and don't use any indentation.
Sorry for inconvenience.

> I was proposing to drop compatibles entirely. Making compatibles generic
> is not solving any of my concerns.
>
> I don't understand what do you want to achieve here and very limited
> description of the hardware in the binding does not help.

Sure, i remove user pd compatibles. In root pd probe itself, user pd
remoteproc's are taken care. Also I updated diagram in cover page, it
gives enough information.

> > > 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...
> > >
> > >
> > > 	Yes, as i mentioned above the requirement is to have device
>
> What requirement? You did not describe anything. Binding describes
> hardware, not your requirements.
>
> Binding said nothing about devices.

Yeah got your point. So we removed userpd compatibles from dt-bindings.


> > 	specific bindings. We will remove "PD-X" from soc dtsi and
> > 	add it in board dts file.
>
> Why? How is it related to the bindings? What does it solve? Instead of
> doing some changes you should explain why.
>
> > 	So soc dts would have "Q6-MPD" master node & board dts have
> > 	"WCSS-MPD" child nodes based on number of radio's connected
> > 	on board.
> >
> > 	Is this fine ?
>
> Why?

"PD-X" controls user pd WCSS bring up. WCSS blocks may vary based on
number of radio's connected on board but QDSP6 is always present.
So i will keep Q6 node in soc dtsi file and move 'PD-X' node to board
dts file.

Thanks & Regards,
Manikanta.

> Best regards,
> Krzysztof