| Message ID | 20240602114439.1611-9-quic_jkona@quicinc.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Add support for videocc and camcc on SM8650 | expand |
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>
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
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
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
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>;
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>; >
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
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>;