diff mbox series

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

Message ID 20240321092529.13362-2-quic_jkona@quicinc.com (mailing list archive)
State Superseded
Headers show
Series Add support for videocc and camcc on SM8650 | expand

Commit Message

Jagadeesh Kona March 21, 2024, 9:25 a.m. UTC
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@quicinc.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../devicetree/bindings/clock/qcom,sm8450-videocc.yaml    | 4 +++-
 include/dt-bindings/clock/qcom,sm8450-videocc.h           | 8 +++++++-
 2 files changed, 10 insertions(+), 2 deletions(-)

Comments

Dmitry Baryshkov March 21, 2024, 1:12 p.m. UTC | #1
On Thu, 21 Mar 2024 at 11:26, Jagadeesh Kona <quic_jkona@quicinc.com> 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@quicinc.com>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  .../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@quicinc.com>
> +  - Jagadeesh Kona <quic_jkona@quicinc.com>
>
>  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.

>
>  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?

>
>  /* VIDEO_CC power domains */
>  #define VIDEO_CC_MVS0C_GDSC                                    0
> @@ -34,5 +39,6 @@
>  #define CVP_VIDEO_CC_MVS1C_BCR                                 4
>  #define VIDEO_CC_MVS0C_CLK_ARES                                        5
>  #define VIDEO_CC_MVS1C_CLK_ARES                                        6
> +#define VIDEO_CC_XO_CLK_ARES                                   7
>
>  #endif
> --
> 2.43.0
>
>
Jagadeesh Kona March 25, 2024, 6:07 a.m. UTC | #2
On 3/21/2024 6:42 PM, Dmitry Baryshkov wrote:
> On Thu, 21 Mar 2024 at 11:26, Jagadeesh Kona <quic_jkona@quicinc.com> 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@quicinc.com>
>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> ---
>>   .../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@quicinc.com>
>> +  - Jagadeesh Kona <quic_jkona@quicinc.com>
>>
>>   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.

Thanks,
Jagadeesh

>>
>>   /* VIDEO_CC power domains */
>>   #define VIDEO_CC_MVS0C_GDSC                                    0
>> @@ -34,5 +39,6 @@
>>   #define CVP_VIDEO_CC_MVS1C_BCR                                 4
>>   #define VIDEO_CC_MVS0C_CLK_ARES                                        5
>>   #define VIDEO_CC_MVS1C_CLK_ARES                                        6
>> +#define VIDEO_CC_XO_CLK_ARES                                   7
>>
>>   #endif
>> --
>> 2.43.0
>>
>>
> 
>
Dmitry Baryshkov March 25, 2024, 6:38 a.m. UTC | #3
On Mon, 25 Mar 2024 at 08:08, Jagadeesh Kona <quic_jkona@quicinc.com> wrote:
>
>
>
> On 3/21/2024 6:42 PM, Dmitry Baryshkov wrote:
> > On Thu, 21 Mar 2024 at 11:26, Jagadeesh Kona <quic_jkona@quicinc.com> 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@quicinc.com>
> >> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >> ---
> >>   .../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@quicinc.com>
> >> +  - Jagadeesh Kona <quic_jkona@quicinc.com>
> >>
> >>   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.

At least it deserves a comment.

>
> Thanks,
> Jagadeesh
>
> >>
> >>   /* VIDEO_CC power domains */
> >>   #define VIDEO_CC_MVS0C_GDSC                                    0
> >> @@ -34,5 +39,6 @@
> >>   #define CVP_VIDEO_CC_MVS1C_BCR                                 4
> >>   #define VIDEO_CC_MVS0C_CLK_ARES                                        5
> >>   #define VIDEO_CC_MVS1C_CLK_ARES                                        6
> >> +#define VIDEO_CC_XO_CLK_ARES                                   7
> >>
> >>   #endif
> >> --
> >> 2.43.0
> >>
> >>
> >
> >
Jagadeesh Kona March 25, 2024, 7 a.m. UTC | #4
On 3/25/2024 12:08 PM, Dmitry Baryshkov wrote:
> On Mon, 25 Mar 2024 at 08:08, Jagadeesh Kona <quic_jkona@quicinc.com> wrote:
>>
>>
>>
>> On 3/21/2024 6:42 PM, Dmitry Baryshkov wrote:
>>> On Thu, 21 Mar 2024 at 11:26, Jagadeesh Kona <quic_jkona@quicinc.com> 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@quicinc.com>
>>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>> ---
>>>>    .../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@quicinc.com>
>>>> +  - Jagadeesh Kona <quic_jkona@quicinc.com>
>>>>
>>>>    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.
> 
> At least it deserves a comment.
> 

Yes, will add the comment in next series.

Thanks,
Jagadeesh

>>
>>>>
>>>>    /* VIDEO_CC power domains */
>>>>    #define VIDEO_CC_MVS0C_GDSC                                    0
>>>> @@ -34,5 +39,6 @@
>>>>    #define CVP_VIDEO_CC_MVS1C_BCR                                 4
>>>>    #define VIDEO_CC_MVS0C_CLK_ARES                                        5
>>>>    #define VIDEO_CC_MVS1C_CLK_ARES                                        6
>>>> +#define VIDEO_CC_XO_CLK_ARES                                   7
>>>>
>>>>    #endif
>>>> --
>>>> 2.43.0
>>>>
>>>>
>>>
>>>
> 
> 
>
Vladimir Zapolskiy April 18, 2024, 9:01 p.m. UTC | #5
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@quicinc.com> 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@quicinc.com>
>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>> ---
>>>    .../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@quicinc.com>
>>> +  - Jagadeesh Kona <quic_jkona@quicinc.com>
>>>
>>>    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.

--
Best wishes,
Vladimir
Jagadeesh Kona April 22, 2024, 11 a.m. UTC | #6
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@quicinc.com> 
>>> 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@quicinc.com>
>>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>> ---
>>>>    .../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@quicinc.com>
>>>> +  - Jagadeesh Kona <quic_jkona@quicinc.com>
>>>>
>>>>    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
Vladimir Zapolskiy April 25, 2024, 1:32 p.m. UTC | #7
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@quicinc.com>
>>>> 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@quicinc.com>
>>>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>>> ---
>>>>>     .../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@quicinc.com>
>>>>> +  - Jagadeesh Kona <quic_jkona@quicinc.com>
>>>>>
>>>>>     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.

--
Best wishes,
Vladimir
Jagadeesh Kona April 26, 2024, 2:26 p.m. UTC | #8
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@quicinc.com>
>>>>> 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@quicinc.com>
>>>>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>>>> ---
>>>>>>     .../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@quicinc.com>
>>>>>> +  - Jagadeesh Kona <quic_jkona@quicinc.com>
>>>>>>
>>>>>>     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
diff mbox series

Patch

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@quicinc.com>
+  - Jagadeesh Kona <quic_jkona@quicinc.com>
 
 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
 
 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
 
 /* VIDEO_CC power domains */
 #define VIDEO_CC_MVS0C_GDSC					0
@@ -34,5 +39,6 @@ 
 #define CVP_VIDEO_CC_MVS1C_BCR					4
 #define VIDEO_CC_MVS0C_CLK_ARES					5
 #define VIDEO_CC_MVS1C_CLK_ARES					6
+#define VIDEO_CC_XO_CLK_ARES					7
 
 #endif