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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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@xxxxxxxxxxx/

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@xxxxxxxxxxx>
+Cc Mani,

Thanks, Krzysztof!

This will conflict with:
https://lore.kernel.org/all/20230314080443.64635-3-manivannan.sadhasivam@xxxxxxxxxx/

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





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux