Re: [PATCH] arm64: dts: rockchip: Add cif test clocks for rk3399

[Brian Norris <briannorris-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>]

Hi Shunqian,

On Mon, Jan 08, 2018 at 01:48:51PM +0800, Shunqian Zheng wrote:
> There are three pins can act as cif test clock for rk3399.
> They're sourced from 24M and output 24M by default and some boards
> may use them as camera 24M xvclk.
> 
> Signed-off-by: Shunqian Zheng <zhengsq-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> ---
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> index 7aa2144..daad42f 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -2293,6 +2293,23 @@
>  			};
>  		};
>  
> +		test {

Doesn't really matter much, but 'test' is a weird group name to me. I
think 'testclk' or 'test-clk' might be a more descriptive name?

> +			test_clkout0: test-clkout0 {
> +				rockchip,pins =
> +					<0 0 RK_FUNC_1 &pcfg_pull_none>;
> +			};
> +
> +			test_clkout1: test-clkout1 {
> +				rockchip,pins =
> +					<2 25 RK_FUNC_2 &pcfg_pull_none>;
> +			};
> +
> +			test_clkout2: test-clkout2 {
> +				rockchip,pins =
> +					<0 8 RK_FUNC_3 &pcfg_pull_none>;

Your function indexing is a little confusing to me, but one or more of
your datasheet, TRM, or patch are incorrect here. The datasheet says
"Func 3" (which is 1-indexed, so really means RK_FUNC_2) should be
TEST_CLKOUT2, but your TRM agrees with the patch, saying
2'b11=test_clkout2. So I think your patch is correct, but the datasheet
needs updated?

All in all, I think the patch looks good though:

Reviewed-by: Brian Norris <briannorris-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

> +			};
> +		};
> +
>  		tsadc {
>  			otp_gpio: otp-gpio {
>  				rockchip,pins = <1 6 RK_FUNC_GPIO &pcfg_pull_none>;
> -- 
> 1.9.1
>