On Wed 12 Jan 13:40 CST 2022, Luca Weiss wrote:
> From: Vladimir Lypak <vladimir.lypak@xxxxxxxxx>
>
> Add a base DT for MSM8953 SoC.
>
> Co-developed-by: Luca Weiss <luca@xxxxxxxxx>
> Signed-off-by: Vladimir Lypak <vladimir.lypak@xxxxxxxxx>
> Signed-off-by: Luca Weiss <luca@xxxxxxxxx>
> ---
> arch/arm64/boot/dts/qcom/msm8953.dtsi | 1337 +++++++++++++++++++++++++
> 1 file changed, 1337 insertions(+)
> create mode 100644 arch/arm64/boot/dts/qcom/msm8953.dtsi
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8953.dtsi b/arch/arm64/boot/dts/qcom/msm8953.dtsi
> new file mode 100644
> index 000000000000..59918b527750
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/msm8953.dtsi
> @@ -0,0 +1,1337 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/* Copyright (c) 2022, The Linux Foundation. All rights reserved. */
> +
> +#include <dt-bindings/clock/qcom,gcc-msm8953.h>
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/power/qcom-rpmpd.h>
> +#include <dt-bindings/thermal/thermal.h>
> +
> +/ {
> + interrupt-parent = <&intc>;
> +
> + #address-cells = <2>;
> + #size-cells = <2>;
Why do you have address/size-cells of 2, and then limit them to 1 in
/soc?
> +
> + aliases {
> + i2c1 = &i2c_1;
> + i2c2 = &i2c_2;
> + i2c3 = &i2c_3;
> + i2c4 = &i2c_4;
> + i2c5 = &i2c_5;
> + i2c6 = &i2c_6;
> + i2c7 = &i2c_7;
> + i2c8 = &i2c_8;
It was expressed a while back that you should specify alias only for the
things that you have enabled in your .dts.
> + };
[..]
> + tcsr_mutex: hwlock@1905000 {
> + compatible = "qcom,tcsr-mutex";
> + reg = <0x1905000 0x20000>;
> + #hwlock-cells = <1>;
> + };
> +
> + tcsr: syscon@1937000 {
> + compatible = "qcom,tcsr-msm8953", "syscon";
> + reg = <0x1937000 0x30000>;
> + };
> +
> + tcsr_phy_clk_scheme_sel: syscon@193f044 {
I don't fancy exposing a single word from the middle of &tcsr using a
syscon. The tcsr node should express the TCSR region and if we need to
reference bits of information within that we should do that in some
structured way.
Wouldn't nvmem be a good candidate for this?
> + compatible = "syscon";
> + reg = <0x193f044 0x4>;
> + };
> +
Regards,
Bjorn
