Re: [PATCH v2 1/3] arm64: dts: qcom: sm8250: Add camcc DT node

[Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>]

Hi Bryan,

On 4/14/22 02:17, Bryan O'Donoghue wrote:
> Add the camcc DT node for the Camera Clock Controller on sm8250.
> 
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>   arch/arm64/boot/dts/qcom/sm8250.dtsi | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> index af8f22636436..906bc8ed25b7 100644
> --- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
> @@ -19,6 +19,7 @@
>   #include <dt-bindings/sound/qcom,q6afe.h>
>   #include <dt-bindings/thermal/thermal.h>
>   #include <dt-bindings/clock/qcom,videocc-sm8250.h>
> +#include <dt-bindings/clock/qcom,camcc-sm8250.h>

please swap the two last lines to get a bit better alphanumeric order, it
might be a good idea to sort out all the include directives.

>   / {
>   	interrupt-parent = <&intc>;
> @@ -3149,6 +3150,20 @@ videocc: clock-controller@abf0000 {
>   			#power-domain-cells = <1>;
>   		};
>   
> +		camcc: clock-controller@ad00000 {
> +			compatible = "qcom,sm8250-camcc";
> +			reg = <0 0x0ad00000 0 0x10000>;
> +			clocks = <&gcc GCC_VIDEO_AHB_CLK>,

I believe it should be GCC_CAMERA_AHB_CLK.

> +				 <&rpmhcc RPMH_CXO_CLK>,
> +				 <&rpmhcc RPMH_CXO_CLK_A>;
> +			clock-names = "iface", "bi_tcxo", "bi_tcxo_ao";

I see that the driver references to "sleep_clk", which won't be found
automatically, since it's name is "sleep-clk", so, it might be reasonable
to add it to the list.

> +			power-domains = <&rpmhpd SM8250_MMCX>;
> +			required-opps = <&rpmhpd_opp_low_svs>;
> +			#clock-cells = <1>;
> +			#reset-cells = <1>;
> +			#power-domain-cells = <1>;
> +		};
> +
>   		mdss: mdss@ae00000 {
>   			compatible = "qcom,sm8250-mdss";
>   			reg = <0 0x0ae00000 0 0x1000>;

--
Best wishes,
Vladimir