Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <d7d03e81-03d6-b643-a2e6-0867bab7e0ef@quicinc.com>
Date:   Thu, 6 Apr 2023 15:22:39 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: [PATCH v2 2/5] dt-bindings: arm: msm: Add bindings for multi
 channel DDR in LLCC
Content-Language: en-US
To:     Komal Bajaj <quic_kbajaj@quicinc.com>,
        Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
CC:     Rob Herring <robh+dt@kernel.org>,
        Bjorn Andersson <andersson@kernel.org>,
        Konrad Dybcio <konrad.dybcio@linaro.org>,
        Abel Vesa <abel.vesa@linaro.org>,
        Rishabh Bhatnagar <rishabhb@codeaurora.org>,
        Prakash Ranjan <saiprakash.ranjan@codeaurora.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Andy Gross <agross@kernel.org>, <devicetree@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <linux-arm-msm@vger.kernel.org>
References: <20230313124040.9463-1-quic_kbajaj@quicinc.com>
 <20230313124040.9463-3-quic_kbajaj@quicinc.com>
 <2b3e39b9-ea70-db9b-89f7-09054df363c3@linaro.org>
 <20230315134814.GA98488@thinkpad>
 <c8f3499f-d927-6657-c7c6-732ed2222525@quicinc.com>
From:   Mukesh Ojha <quic_mojha@quicinc.com>
In-Reply-To: <c8f3499f-d927-6657-c7c6-732ed2222525@quicinc.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 4/6/2023 2:49 PM, Komal Bajaj wrote:
> Didn't see my reply on the list, so sending it again.
> And also I see that the dt patch is already applied.

The reason why you are not seeing your replies at

https://lore.kernel.org/lkml/20230313124040.9463-1-quic_kbajaj@quicinc.com/

is because your reply cc-list contain some invalid domain 
(codeaurora.org) email id's and any list/email mentioned
after that would not be getting your emails.

-- Mukesh

> 
> Thanks Krzysztof and Manivannan for reviewing the patch.
> 
> 
> On 3/15/2023 7:18 PM, Manivannan Sadhasivam wrote:
>> On Wed, Mar 15, 2023 at 08:41:21AM +0100, Krzysztof Kozlowski wrote:
>>> On 13/03/2023 13:40, Komal Bajaj wrote:
>>>> Add description for additional nodes needed to support
>>>> mulitple channel DDR configurations in LLCC.
>>>>
>>>> Signed-off-by: Komal Bajaj<quic_kbajaj@quicinc.com>
>>> +Cc Mani,
>>>
>> Thanks, Krzysztof!
>>
>>> This will conflict with:
>>> https://lore.kernel.org/all/20230314080443.64635-3-manivannan.sadhasivam@linaro.org/
>>>
>>> Please rebase on top of Mani's patches (assuming they are not
>>> conflicting in principle)
>>>
>>>> ---
>>>>   Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml | 9 +++++++++
>>>>   1 file changed, 9 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml b/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml
>>>> index 38efcad56dbd..9a4a76caf490 100644
>>>> --- a/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml
>>>> +++ b/Documentation/devicetree/bindings/arm/msm/qcom,llcc.yaml
>>>> @@ -37,15 +37,24 @@ properties:
>>>>       items:
>>> minItems: 2
>>>
>>>>         - description: LLCC base register region
>>>>         - description: LLCC broadcast base register region
>>>> +      - description: Feature register to decide which LLCC configuration
>>>> +                     to use, this is optional
>>>>   
>>>>     reg-names:
>>> minItems: 2
>>>
>>>>       items:
>>>>         - const: llcc_base
>>>>         - const: llcc_broadcast_base
>>>> +      - const: multi_channel_register
>> Is this the actual register region or a specific register offset? We generally
>> try to pass the base address of the region along with the size and use the
>> offset inside the driver to access any specific registers.
>>
>> Thanks,
>> Mani
> 
> This is a specific register offset outside the LLCC register region which has the
> information of number of DDR channel.
> 
>>>>   
>>>>     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.
> 
>>> Third, you need vendor prefix and type (unless this is a generic
>>> property, but does not look like). Then "items" is probably wrong. Line
>>> break after "description: "
> 
> Noted, will take care of this in the next patchset.
> 
> Thanks
> Komal
> 
>>> Best regards,
>>> Krzysztof
>>>
>