Re: [PATCH V2 RESEND 1/6] dt-bindings: clock: qcom: Add SM8650 video clock controller

From: Jagadeesh Kona
Date: Mon Apr 22 2024 - 07:00:57 EST




On 4/19/2024 2:31 AM, Vladimir Zapolskiy wrote:
Hello Jagadeesh,

On 3/25/24 08:07, Jagadeesh Kona wrote:


On 3/21/2024 6:42 PM, Dmitry Baryshkov wrote:
On Thu, 21 Mar 2024 at 11:26, Jagadeesh Kona <quic_jkona@xxxxxxxxxxx> wrote:

Extend device tree bindings of SM8450 videocc to add support
for SM8650 videocc. While it at, fix the incorrect header
include in sm8450 videocc yaml documentation.

Signed-off-by: Jagadeesh Kona <quic_jkona@xxxxxxxxxxx>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
---
   .../devicetree/bindings/clock/qcom,sm8450-videocc.yaml    | 4 +++-
   include/dt-bindings/clock/qcom,sm8450-videocc.h           | 8 +++++++-
   2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml b/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml
index bad8f019a8d3..79f55620eb70 100644
--- a/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,sm8450-videocc.yaml
@@ -8,18 +8,20 @@ title: Qualcomm Video Clock & Reset Controller on SM8450

   maintainers:
     - Taniya Das <quic_tdas@xxxxxxxxxxx>
+  - Jagadeesh Kona <quic_jkona@xxxxxxxxxxx>

   description: |
     Qualcomm video clock control module provides the clocks, resets and power
     domains on SM8450.

-  See also:: include/dt-bindings/clock/qcom,videocc-sm8450.h
+  See also:: include/dt-bindings/clock/qcom,sm8450-videocc.h

This almost pleads to go to a separate patch. Fixes generally should
be separated from the rest of the changes.


Thanks Dmitry for your review.

Sure, will separate this into a separate patch in next series.


   properties:
     compatible:
       enum:
         - qcom,sm8450-videocc
         - qcom,sm8550-videocc
+      - qcom,sm8650-videocc

     reg:
       maxItems: 1
diff --git a/include/dt-bindings/clock/qcom,sm8450-videocc.h b/include/dt-bindings/clock/qcom,sm8450-videocc.h
index 9d795adfe4eb..ecfebe52e4bb 100644
--- a/include/dt-bindings/clock/qcom,sm8450-videocc.h
+++ b/include/dt-bindings/clock/qcom,sm8450-videocc.h
@@ -1,6 +1,6 @@
   /* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
   /*
- * Copyright (c) 2023, Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2023-2024, Qualcomm Innovation Center, Inc. All rights reserved.
    */

   #ifndef _DT_BINDINGS_CLK_QCOM_VIDEO_CC_SM8450_H
@@ -19,6 +19,11 @@
   #define VIDEO_CC_MVS1C_DIV2_DIV_CLK_SRC                                9
   #define VIDEO_CC_PLL0                                          10
   #define VIDEO_CC_PLL1                                          11
+#define VIDEO_CC_MVS0_SHIFT_CLK                                        12
+#define VIDEO_CC_MVS0C_SHIFT_CLK                               13
+#define VIDEO_CC_MVS1_SHIFT_CLK                                        14
+#define VIDEO_CC_MVS1C_SHIFT_CLK                               15
+#define VIDEO_CC_XO_CLK_SRC                                    16

Are these values applicable to sm8450?


No, the shift clocks above are part of SM8650 only. To reuse the
existing SM8550 videocc driver for SM8650 and to register these shift
clocks for SM8650, I added them here.


In such case I'd strongly suggest to add a new qcom,sm8650-videocc.h file,
and do #include qcom,sm8450-videocc.h in it, thus the new header will be
really a short one.

This will add pristine clarity.


Thanks Vladimir for your suggestion. I believe adding a comment for these set of clocks should be sufficient to indicate these clocks are applicable only for SM8650, I can add the required comment and post the next series. Please let me know if this works?

Thanks,
Jagadeesh