Re: [PATCH v2 2/5] dt-bindings: arm: msm: Add bindings for multi channel DDR in LLCC

[Mukesh Ojha]

On 4/6/2023 3:04 PM, Krzysztof Kozlowski wrote:
> On 06/04/2023 11:19, Komal Bajaj wrote:
> > > > >    
> > > > >      interrupts:
> > > > >        maxItems: 1
> > > > >    
> > > > > +  multi-ch-bit-off:
> > > > > +    items:
> > > > > +      - description: Specifies the offset in bits into the multi_channel_register
> > > > > +                     and the number of bits used to decide which LLCC configuration
> > > > > +                     to use
> > > > There are here few issues.
> > > > First, I don't fully understand the property. What is an LLCC
> > > > configuration? Like some fused values?
> >
> > There are different configuration for LLCC based on the number of
> > DDR channel it uses. Here, we are basically trying to get information
> > about the same.
> >
> > > > Second, don't make it a register specific, it will not scale easily to
> > > > any new version of this interface. Although how this should look like
> > > > depends on what is it.
> >
> > LLCC driver can only get DDR channel information from the register.

Actually, any one can read this property there is no boundation to that.
However, some of the bits information of this register only matters to 
LLCC to make decision.

> And why would that exactly matter to DT? How does it solve my concern
> that your approach does not scale?

I understand your concern about not making it register specific, however 
this register is a secure fuse register where the interested bits are
known to us and should only be used by llcc to select right slice 
configuration table to apply.

How about something like this,

Add another property like qcom,tcsr_feature_config under qcom,scm
and read this property under qcom_scm driver and expose
read exported symbol for LLCC driver. LLCC driver can use API and
apply bit offset/bit size to get right value inside the target driver 
data(e.g .data = &XXXX_cfg }) or maintain it inside same 
tcsr_feature_config DT (this can be discussed)
Also, we can have a yaml file for tcsr_feature_config.

e.g..


scm: scm {
        compatible = "qcom,scm-sm8450", "qcom,scm";
        qcom,dload-mode = <&tcsr 0x13000>;
        ...
	qcom,tcsr_feature_config = <&tcsr_feature_config>
};

tcsr_feature_config: syscon@fd484000 {
	compatible = "qcom, XXXX", qcom,tcsr-featureconfig";
	reg = <0xXXXXXXXX 0xYYYY>;
};


-- Mukesh

> Best regards,
> Krzysztof