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

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

 





On 4/25/2024 7:02 PM, Vladimir Zapolskiy wrote:
Hi Jagadeesh,

On 4/22/24 14:00, Jagadeesh Kona wrote:

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?

Well, I didn't get any new information to abandon my suggestion, what is
wrong with it or why is it less preferable?

Even if you add a comment in the header file, it means that for SM8450
platforms you'll begin to define inapplicable/unrelated macro for the
platform, which opens a small risk of the misusage, and which can be
easily avoided. I believe that the clarity is better for maintenance.


Yes, I agree. Will check and move these new clocks to a separate header file in next series. Thanks!

Thanks,
Jagadeesh




[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