[PATCH V4 8/8] arm64: dts: qcom: sm8650: Add video and camera clock controllers

Jagadeesh Kona posted 8 patches 1 year, 6 months ago
[PATCH V4 8/8] arm64: dts: qcom: sm8650: Add video and camera clock controllers
Posted by Jagadeesh Kona 1 year, 6 months ago
Add device nodes for video and camera clock controllers on Qualcomm
SM8650 platform.

Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8650.dtsi | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
index 336c54242778..d964762b0532 100644
--- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
@@ -4,10 +4,12 @@
  */
 
 #include <dt-bindings/clock/qcom,rpmh.h>
+#include <dt-bindings/clock/qcom,sm8650-camcc.h>
 #include <dt-bindings/clock/qcom,sm8650-dispcc.h>
 #include <dt-bindings/clock/qcom,sm8650-gcc.h>
 #include <dt-bindings/clock/qcom,sm8650-gpucc.h>
 #include <dt-bindings/clock/qcom,sm8650-tcsr.h>
+#include <dt-bindings/clock/qcom,sm8650-videocc.h>
 #include <dt-bindings/dma/qcom-gpi.h>
 #include <dt-bindings/firmware/qcom,scm.h>
 #include <dt-bindings/gpio/gpio.h>
@@ -3315,6 +3317,30 @@ opp-202000000 {
 			};
 		};
 
+		videocc: clock-controller@aaf0000 {
+			compatible = "qcom,sm8650-videocc";
+			reg = <0 0x0aaf0000 0 0x10000>;
+			clocks = <&bi_tcxo_div2>,
+				 <&gcc GCC_VIDEO_AHB_CLK>;
+			power-domains = <&rpmhpd RPMHPD_MMCX>;
+			#clock-cells = <1>;
+			#reset-cells = <1>;
+			#power-domain-cells = <1>;
+		};
+
+		camcc: clock-controller@ade0000 {
+			compatible = "qcom,sm8650-camcc";
+			reg = <0 0x0ade0000 0 0x20000>;
+			clocks = <&gcc GCC_CAMERA_AHB_CLK>,
+				 <&bi_tcxo_div2>,
+				 <&bi_tcxo_ao_div2>,
+				 <&sleep_clk>;
+			power-domains = <&rpmhpd RPMHPD_MMCX>;
+			#clock-cells = <1>;
+			#reset-cells = <1>;
+			#power-domain-cells = <1>;
+		};
+
 		mdss: display-subsystem@ae00000 {
 			compatible = "qcom,sm8650-mdss";
 			reg = <0 0x0ae00000 0 0x1000>;
-- 
2.43.0
Re: [PATCH V4 8/8] arm64: dts: qcom: sm8650: Add video and camera clock controllers
Posted by neil.armstrong@linaro.org 1 year, 6 months ago
On 02/06/2024 13:44, Jagadeesh Kona wrote:
> Add device nodes for video and camera clock controllers on Qualcomm
> SM8650 platform.
> 
> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
> Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---
>   arch/arm64/boot/dts/qcom/sm8650.dtsi | 26 ++++++++++++++++++++++++++
>   1 file changed, 26 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> index 336c54242778..d964762b0532 100644
> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> @@ -4,10 +4,12 @@
>    */
>   
>   #include <dt-bindings/clock/qcom,rpmh.h>
> +#include <dt-bindings/clock/qcom,sm8650-camcc.h>
>   #include <dt-bindings/clock/qcom,sm8650-dispcc.h>
>   #include <dt-bindings/clock/qcom,sm8650-gcc.h>
>   #include <dt-bindings/clock/qcom,sm8650-gpucc.h>
>   #include <dt-bindings/clock/qcom,sm8650-tcsr.h>
> +#include <dt-bindings/clock/qcom,sm8650-videocc.h>
>   #include <dt-bindings/dma/qcom-gpi.h>
>   #include <dt-bindings/firmware/qcom,scm.h>
>   #include <dt-bindings/gpio/gpio.h>
> @@ -3315,6 +3317,30 @@ opp-202000000 {
>   			};
>   		};
>   
> +		videocc: clock-controller@aaf0000 {
> +			compatible = "qcom,sm8650-videocc";
> +			reg = <0 0x0aaf0000 0 0x10000>;
> +			clocks = <&bi_tcxo_div2>,
> +				 <&gcc GCC_VIDEO_AHB_CLK>;
> +			power-domains = <&rpmhpd RPMHPD_MMCX>;
> +			#clock-cells = <1>;
> +			#reset-cells = <1>;
> +			#power-domain-cells = <1>;
> +		};
> +
> +		camcc: clock-controller@ade0000 {
> +			compatible = "qcom,sm8650-camcc";
> +			reg = <0 0x0ade0000 0 0x20000>;
> +			clocks = <&gcc GCC_CAMERA_AHB_CLK>,
> +				 <&bi_tcxo_div2>,
> +				 <&bi_tcxo_ao_div2>,
> +				 <&sleep_clk>;
> +			power-domains = <&rpmhpd RPMHPD_MMCX>;
> +			#clock-cells = <1>;
> +			#reset-cells = <1>;
> +			#power-domain-cells = <1>;
> +		};

If you resend, please respect the style of the SM8650 DT
with blank lines to separate different properties groups:

+		videocc: clock-controller@aaf0000 {
+			compatible = "qcom,sm8650-videocc";
+			reg = <0 0x0aaf0000 0 0x10000>;

+			clocks = <&bi_tcxo_div2>,
+				 <&gcc GCC_VIDEO_AHB_CLK>;

+			power-domains = <&rpmhpd RPMHPD_MMCX>;

+			#clock-cells = <1>;
+			#reset-cells = <1>;
+			#power-domain-cells = <1>;
+		};
+
+		camcc: clock-controller@ade0000 {
+			compatible = "qcom,sm8650-camcc";
+			reg = <0 0x0ade0000 0 0x20000>;

+			clocks = <&gcc GCC_CAMERA_AHB_CLK>,
+				 <&bi_tcxo_div2>,
+				 <&bi_tcxo_ao_div2>,
+				 <&sleep_clk>;

+			power-domains = <&rpmhpd RPMHPD_MMCX>;

+			#clock-cells = <1>;
+			#reset-cells = <1>;
+			#power-domain-cells = <1>;
+		};

And add the missing required-opps for the clock controllers like
dispcc.

Thanks,
Neil


> +
>   		mdss: display-subsystem@ae00000 {
>   			compatible = "qcom,sm8650-mdss";
>   			reg = <0 0x0ae00000 0 0x1000>;
Re: [PATCH V4 8/8] arm64: dts: qcom: sm8650: Add video and camera clock controllers
Posted by Dmitry Baryshkov 1 year, 6 months ago
On Tue, Jun 18, 2024 at 02:17:23PM GMT, neil.armstrong@linaro.org wrote:
> On 02/06/2024 13:44, Jagadeesh Kona wrote:
> > Add device nodes for video and camera clock controllers on Qualcomm
> > SM8650 platform.
> > 
> > Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
> > Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> > ---
> >   arch/arm64/boot/dts/qcom/sm8650.dtsi | 26 ++++++++++++++++++++++++++
> >   1 file changed, 26 insertions(+)
> > 

[...]

> 
> And add the missing required-opps for the clock controllers like
> dispcc.

Unless the opps is required because cmd-db has lower level than
required for the functioning of the device, there should be no need to
add the required-opps.

> 
> Thanks,
> Neil
> 
> 
> > +
> >   		mdss: display-subsystem@ae00000 {
> >   			compatible = "qcom,sm8650-mdss";
> >   			reg = <0 0x0ae00000 0 0x1000>;
> 

-- 
With best wishes
Dmitry
Re: [PATCH V4 8/8] arm64: dts: qcom: sm8650: Add video and camera clock controllers
Posted by Vladimir Zapolskiy 1 year, 6 months ago
Hi Dmitry,

On 6/18/24 17:33, Dmitry Baryshkov wrote:
> On Tue, Jun 18, 2024 at 02:17:23PM GMT, neil.armstrong@linaro.org wrote:
>> On 02/06/2024 13:44, Jagadeesh Kona wrote:
>>> Add device nodes for video and camera clock controllers on Qualcomm
>>> SM8650 platform.
>>>
>>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
>>> Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>>> ---
>>>    arch/arm64/boot/dts/qcom/sm8650.dtsi | 26 ++++++++++++++++++++++++++
>>>    1 file changed, 26 insertions(+)
>>>
> 
> [...]
> 
>>
>> And add the missing required-opps for the clock controllers like
>> dispcc.
> 
> Unless the opps is required because cmd-db has lower level than
> required for the functioning of the device, there should be no need to
> add the required-opps.
> 

this is totally fine, but then 'required-opps' property shall be removed
from the list of required properties in device tree bindings description.

--
Best wishes,
Vladimir
Re: [PATCH V4 8/8] arm64: dts: qcom: sm8650: Add video and camera clock controllers
Posted by Vladimir Zapolskiy 1 year, 6 months ago
Hi Jagadeesh.

On 6/2/24 14:44, Jagadeesh Kona wrote:
> Add device nodes for video and camera clock controllers on Qualcomm
> SM8650 platform.
> 
> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
> Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---
>   arch/arm64/boot/dts/qcom/sm8650.dtsi | 26 ++++++++++++++++++++++++++
>   1 file changed, 26 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> index 336c54242778..d964762b0532 100644
> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> @@ -4,10 +4,12 @@
>    */
>   
>   #include <dt-bindings/clock/qcom,rpmh.h>
> +#include <dt-bindings/clock/qcom,sm8650-camcc.h>
>   #include <dt-bindings/clock/qcom,sm8650-dispcc.h>
>   #include <dt-bindings/clock/qcom,sm8650-gcc.h>
>   #include <dt-bindings/clock/qcom,sm8650-gpucc.h>
>   #include <dt-bindings/clock/qcom,sm8650-tcsr.h>
> +#include <dt-bindings/clock/qcom,sm8650-videocc.h>
>   #include <dt-bindings/dma/qcom-gpi.h>
>   #include <dt-bindings/firmware/qcom,scm.h>
>   #include <dt-bindings/gpio/gpio.h>
> @@ -3315,6 +3317,30 @@ opp-202000000 {
>   			};
>   		};
>   
> +		videocc: clock-controller@aaf0000 {
> +			compatible = "qcom,sm8650-videocc";
> +			reg = <0 0x0aaf0000 0 0x10000>;
> +			clocks = <&bi_tcxo_div2>,
> +				 <&gcc GCC_VIDEO_AHB_CLK>;
> +			power-domains = <&rpmhpd RPMHPD_MMCX>;
> +			#clock-cells = <1>;
> +			#reset-cells = <1>;
> +			#power-domain-cells = <1>;
> +		};
> +
> +		camcc: clock-controller@ade0000 {
> +			compatible = "qcom,sm8650-camcc";
> +			reg = <0 0x0ade0000 0 0x20000>;
> +			clocks = <&gcc GCC_CAMERA_AHB_CLK>,
> +				 <&bi_tcxo_div2>,
> +				 <&bi_tcxo_ao_div2>,
> +				 <&sleep_clk>;
> +			power-domains = <&rpmhpd RPMHPD_MMCX>;

When you test the change on a particular board, do you get here any build
time warning messages like this one?

   clock-controller@ade0000: 'required-opps' is a required property
	  from schema $id: http://devicetree.org/schemas/clock/qcom,sm8450-camcc.yaml#

I believe it's a valid warning, which has to be fixes, and as it says it
corresponds to the required property exactly.

> +			#clock-cells = <1>;
> +			#reset-cells = <1>;
> +			#power-domain-cells = <1>;
> +		};
> +
>   		mdss: display-subsystem@ae00000 {
>   			compatible = "qcom,sm8650-mdss";
>   			reg = <0 0x0ae00000 0 0x1000>;

--
Best wishes,
Vladimir
Re: [PATCH V4 8/8] arm64: dts: qcom: sm8650: Add video and camera clock controllers
Posted by Jagadeesh Kona 1 year, 6 months ago

On 6/13/2024 2:52 AM, Vladimir Zapolskiy wrote:
> Hi Jagadeesh.
> 
> On 6/2/24 14:44, Jagadeesh Kona wrote:
>> Add device nodes for video and camera clock controllers on Qualcomm
>> SM8650 platform.
>>
>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
>> Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
>> ---
>>   arch/arm64/boot/dts/qcom/sm8650.dtsi | 26 ++++++++++++++++++++++++++
>>   1 file changed, 26 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi 
>> b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>> index 336c54242778..d964762b0532 100644
>> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
>> @@ -4,10 +4,12 @@
>>    */
>>   #include <dt-bindings/clock/qcom,rpmh.h>
>> +#include <dt-bindings/clock/qcom,sm8650-camcc.h>
>>   #include <dt-bindings/clock/qcom,sm8650-dispcc.h>
>>   #include <dt-bindings/clock/qcom,sm8650-gcc.h>
>>   #include <dt-bindings/clock/qcom,sm8650-gpucc.h>
>>   #include <dt-bindings/clock/qcom,sm8650-tcsr.h>
>> +#include <dt-bindings/clock/qcom,sm8650-videocc.h>
>>   #include <dt-bindings/dma/qcom-gpi.h>
>>   #include <dt-bindings/firmware/qcom,scm.h>
>>   #include <dt-bindings/gpio/gpio.h>
>> @@ -3315,6 +3317,30 @@ opp-202000000 {
>>               };
>>           };
>> +        videocc: clock-controller@aaf0000 {
>> +            compatible = "qcom,sm8650-videocc";
>> +            reg = <0 0x0aaf0000 0 0x10000>;
>> +            clocks = <&bi_tcxo_div2>,
>> +                 <&gcc GCC_VIDEO_AHB_CLK>;
>> +            power-domains = <&rpmhpd RPMHPD_MMCX>;
>> +            #clock-cells = <1>;
>> +            #reset-cells = <1>;
>> +            #power-domain-cells = <1>;
>> +        };
>> +
>> +        camcc: clock-controller@ade0000 {
>> +            compatible = "qcom,sm8650-camcc";
>> +            reg = <0 0x0ade0000 0 0x20000>;
>> +            clocks = <&gcc GCC_CAMERA_AHB_CLK>,
>> +                 <&bi_tcxo_div2>,
>> +                 <&bi_tcxo_ao_div2>,
>> +                 <&sleep_clk>;
>> +            power-domains = <&rpmhpd RPMHPD_MMCX>;
> 
> When you test the change on a particular board, do you get here any build
> time warning messages like this one?
> 
>    clock-controller@ade0000: 'required-opps' is a required property
>        from schema $id: 
> http://devicetree.org/schemas/clock/qcom,sm8450-camcc.yaml#
> 
> I believe it's a valid warning, which has to be fixes, and as it says it
> corresponds to the required property exactly.
> 

Thanks Vladimir for pointing this issue. I will check about this warning 
and will fix this.

Thanks,
Jagadeesh
Re: [PATCH V4 8/8] arm64: dts: qcom: sm8650: Add video and camera clock controllers
Posted by Konrad Dybcio 1 year, 6 months ago
On 2.06.2024 1:44 PM, Jagadeesh Kona wrote:
> Add device nodes for video and camera clock controllers on Qualcomm
> SM8650 platform.
> 
> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
> Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad
Re: [PATCH V4 8/8] arm64: dts: qcom: sm8650: Add video and camera clock controllers
Posted by Dmitry Baryshkov 1 year, 6 months ago
On Sun, Jun 02, 2024 at 05:14:39PM +0530, Jagadeesh Kona wrote:
> Add device nodes for video and camera clock controllers on Qualcomm
> SM8650 platform.
> 
> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
> Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/sm8650.dtsi | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>


-- 
With best wishes
Dmitry