| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20191022000833.GI20212@google.com>
Date: Mon, 21 Oct 2019 17:08:33 -0700
From: Matthias Kaehlcke <mka@...omium.org>
To: Rajendra Nayak <rnayak@...eaurora.org>
Cc: agross@...nel.org, robh+dt@...nel.org, bjorn.andersson@...aro.org,
linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, Taniya Das <tdas@...eaurora.org>
Subject: Re: [PATCH v2 02/13] arm64: dts: sc7180: Add minimal dts/dtsi files
for SC7180 soc
Hi Rajendra,
I don't have all the hardware documentation for a full review, but
find a few comments inline.
On Mon, Oct 21, 2019 at 12:25:11PM +0530, Rajendra Nayak wrote:
> Add skeletal sc7180 SoC dtsi and idp board dts files.
>
> Co-developed-by: Taniya Das <tdas@...eaurora.org>
> Signed-off-by: Taniya Das <tdas@...eaurora.org>
> Signed-off-by: Rajendra Nayak <rnayak@...eaurora.org>
> ---
> v2:
> * Sorted the nodes in alphabetical order
> * Dropped the bi_tcxo/bi_tcxo_ao nodes
>
> arch/arm64/boot/dts/qcom/Makefile | 1 +
> arch/arm64/boot/dts/qcom/sc7180-idp.dts | 47 ++++
> arch/arm64/boot/dts/qcom/sc7180.dtsi | 300 ++++++++++++++++++++++++
> 3 files changed, 348 insertions(+)
> create mode 100644 arch/arm64/boot/dts/qcom/sc7180-idp.dts
> create mode 100644 arch/arm64/boot/dts/qcom/sc7180.dtsi
>
> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> index 6498a1ec893f..7a5c2f7fe37f 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -13,6 +13,7 @@ dtb-$(CONFIG_ARCH_QCOM) += msm8998-asus-novago-tp370ql.dtb
> dtb-$(CONFIG_ARCH_QCOM) += msm8998-hp-envy-x2.dtb
> dtb-$(CONFIG_ARCH_QCOM) += msm8998-lenovo-miix-630.dtb
> dtb-$(CONFIG_ARCH_QCOM) += msm8998-mtp.dtb
> +dtb-$(CONFIG_ARCH_QCOM) += sc7180-idp.dtb
> dtb-$(CONFIG_ARCH_QCOM) += sdm845-cheza-r1.dtb
> dtb-$(CONFIG_ARCH_QCOM) += sdm845-cheza-r2.dtb
> dtb-$(CONFIG_ARCH_QCOM) += sdm845-cheza-r3.dtb
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> new file mode 100644
> index 000000000000..f8b7e098f5b4
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> @@ -0,0 +1,47 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * SC7180 IDP board device tree source
> + *
> + * Copyright (c) 2019, The Linux Foundation. All rights reserved.
> + */
> +
> +/dts-v1/;
> +
> +#include "sc7180.dtsi"
> +
> +/ {
> + model = "Qualcomm Technologies, Inc. SC7180 IDP";
> + compatible = "qcom,sc7180-idp";
> +
> + aliases {
> + serial0 = &uart2;
> + };
> +
> + chosen {
> + stdout-path = "serial0:115200n8";
> + };
> +};
> +
> +&qupv3_id_0 {
> + status = "okay";
> +};
> +
> +&uart2 {
> + status = "okay";
> +};
> +
> +/* PINCTRL - additions to nodes defined in sc7180.dtsi */
> +
> +&qup_uart2_default {
> + pinconf-tx {
> + pins = "gpio44";
> + drive-strength = <2>;
> + bias-disable;
> + };
> +
> + pinconf-rx {
> + pins = "gpio45";
> + drive-strength = <2>;
> + bias-pull-up;
> + };
> +};
This config seems reasonable as default for a UART in general.
Would it make sense to configure these in the SoC .dtsi?
> diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> new file mode 100644
> index 000000000000..82bf7cdce6b8
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
> @@ -0,0 +1,300 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * SC7180 SoC device tree source
> + *
> + * Copyright (c) 2019, The Linux Foundation. All rights reserved.
> + */
> +
> +#include <dt-bindings/clock/qcom,gcc-sc7180.h>
Note: depends on "Add Global Clock controller (GCC) driver for SC7180"
(https://patchwork.kernel.org/project/linux-arm-msm/list/?submitter=179717)
which isn't merged yet.
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +/ {
> + interrupt-parent = <&intc>;
> +
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + chosen { };
> +
> + clocks {
> + xo_board: xo-board {
> + compatible = "fixed-clock";
> + clock-frequency = <38400000>;
> + clock-output-names = "xo_board";
> + #clock-cells = <0>;
> + };
> +
> + sleep_clk: sleep-clk {
> + compatible = "fixed-clock";
> + clock-frequency = <32764>;
> + clock-output-names = "sleep_clk";
> + #clock-cells = <0>;
> + };
> + };
> +
> + cpus {
> + #address-cells = <2>;
> + #size-cells = <0>;
> +
> + CPU0: cpu@0 {
> + device_type = "cpu";
> + compatible = "arm,armv8";
> + reg = <0x0 0x0>;
> + enable-method = "psci";
> + next-level-cache = <&L2_0>;
> + L2_0: l2-cache {
> + compatible = "cache";
> + next-level-cache = <&L3_0>;
> + L3_0: l3-cache {
> + compatible = "cache";
> + };
> + };
> + };
> +
> + CPU1: cpu@100 {
> + device_type = "cpu";
> + compatible = "arm,armv8";
> + reg = <0x0 0x100>;
> + enable-method = "psci";
> + next-level-cache = <&L2_100>;
> + L2_100: l2-cache {
> + compatible = "cache";
> + next-level-cache = <&L3_0>;
> + };
> + };
> +
> + CPU2: cpu@200 {
> + device_type = "cpu";
> + compatible = "arm,armv8";
> + reg = <0x0 0x200>;
> + enable-method = "psci";
> + next-level-cache = <&L2_200>;
> + L2_200: l2-cache {
> + compatible = "cache";
> + next-level-cache = <&L3_0>;
> + };
> + };
> +
> + CPU3: cpu@300 {
> + device_type = "cpu";
> + compatible = "arm,armv8";
> + reg = <0x0 0x300>;
> + enable-method = "psci";
> + next-level-cache = <&L2_300>;
> + L2_300: l2-cache {
> + compatible = "cache";
> + next-level-cache = <&L3_0>;
> + };
> + };
> +
> + CPU4: cpu@400 {
> + device_type = "cpu";
> + compatible = "arm,armv8";
> + reg = <0x0 0x400>;
> + enable-method = "psci";
> + next-level-cache = <&L2_400>;
> + L2_400: l2-cache {
> + compatible = "cache";
> + next-level-cache = <&L3_0>;
> + };
> + };
> +
> + CPU5: cpu@500 {
> + device_type = "cpu";
> + compatible = "arm,armv8";
> + reg = <0x0 0x500>;
> + enable-method = "psci";
> + next-level-cache = <&L2_500>;
> + L2_500: l2-cache {
> + compatible = "cache";
> + next-level-cache = <&L3_0>;
> + };
> + };
> +
> + CPU6: cpu@600 {
> + device_type = "cpu";
> + compatible = "arm,armv8";
> + reg = <0x0 0x600>;
> + enable-method = "psci";
> + next-level-cache = <&L2_600>;
> + L2_600: l2-cache {
> + compatible = "cache";
> + next-level-cache = <&L3_0>;
> + };
> + };
> +
> + CPU7: cpu@700 {
> + device_type = "cpu";
> + compatible = "arm,armv8";
> + reg = <0x0 0x700>;
> + enable-method = "psci";
> + next-level-cache = <&L2_700>;
> + L2_700: l2-cache {
> + compatible = "cache";
> + next-level-cache = <&L3_0>;
> + };
> + };
> + };
> +
> + memory@...00000 {
> + device_type = "memory";
> + /* We expect the bootloader to fill in the size */
> + reg = <0 0x80000000 0 0>;
> + };
> +
> + pmu {
> + compatible = "arm,armv8-pmuv3";
> + interrupts = <GIC_PPI 5 IRQ_TYPE_LEVEL_HIGH>;
> + };
> +
> + psci {
> + compatible = "arm,psci-1.0";
> + method = "smc";
> + };
> +
> + soc: soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges = <0 0 0 0 0x10 0>;
> + dma-ranges = <0 0 0 0 0x10 0>;
> + compatible = "simple-bus";
> +
> + gcc: clock-controller@...000 {
> + compatible = "qcom,gcc-sc7180";
> + reg = <0 0x00100000 0 0x1f0000>;
> + #clock-cells = <1>;
> + #reset-cells = <1>;
> + #power-domain-cells = <1>;
> + };
> +
> + qupv3_id_0: geniqup@...000 {
The QUP enumeration is a bit confusing. The Hardware Register
Description has QUPV3_0_QUPV3_ID_0 at 0x00800000 and
QUPV3_1_QUPV3_ID_0 at 0x00a00000. This QUP apparently is
the latter. In the SDM845 DT the QUP @ac0000 has the label
'qupv3_id_1', I guess this should be the same here.
> + compatible = "qcom,geni-se-qup";
> + reg = <0 0x00ac0000 0 0x6000>;
> + clock-names = "m-ahb", "s-ahb";
> + clocks = <&gcc GCC_QUPV3_WRAP_1_M_AHB_CLK>,
> + <&gcc GCC_QUPV3_WRAP_1_S_AHB_CLK>;
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> + status = "disabled";
> +
> + uart2: serial@...000 {
> + compatible = "qcom,geni-debug-uart";
> + reg = <0 0x00a88000 0 0x4000>;
Related to the comment above: on SDM845 this UART has the label
'uart10'. I understand these are different SoCs, but could you
please clarify the enumeration of the SC7180 QUPs and their ports?
> + clock-names = "se";
> + clocks = <&gcc GCC_QUPV3_WRAP1_S2_CLK>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&qup_uart2_default>;
> + interrupts = <GIC_SPI 355 IRQ_TYPE_LEVEL_HIGH>;
> + status = "disabled";
> + };
> + };
> +
> + tlmm: pinctrl@...0000 {
> + compatible = "qcom,sc7180-pinctrl";
> + reg = <0 0x03500000 0 0x300000>,
> + <0 0x03900000 0 0x300000>,
> + <0 0x03d00000 0 0x300000>;
> + reg-names = "west", "north", "south";
> + interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + interrupt-controller;
> + #interrupt-cells = <2>;
> + gpio-ranges = <&tlmm 0 0 120>;
> +
> + qup_uart2_default: qup-uart2-default {
> + pinmux {
> + pins = "gpio44", "gpio45";
> + function = "qup12";
> + };
> + };
> + };
> +
> + intc: interrupt-controller@...00000 {
> + compatible = "arm,gic-v3";
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> + #interrupt-cells = <3>;
> + interrupt-controller;
> + reg = <0 0x17a00000 0 0x10000>, /* GICD */
> + <0 0x17a60000 0 0x100000>; /* GICR * 8 */
> + interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
> +
> + gic-its@...40000 {
> + compatible = "arm,gic-v3-its";
> + msi-controller;
> + #msi-cells = <1>;
> + reg = <0 0x17a40000 0 0x20000>;
> + status = "disabled";
> + };
> + };
> +
> + timer@...20000{
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> + compatible = "arm,armv7-timer-mem";
> + reg = <0 0x17c20000 0 0x1000>;
> +
> + frame@...21000 {
> + frame-number = <0>;
> + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>;
> + reg = <0 0x17c21000 0 0x1000>,
> + <0 0x17c22000 0 0x1000>;
> + };
> +
> + frame@...23000 {
> + frame-number = <1>;
> + interrupts = <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>;
> + reg = <0 0x17c23000 0 0x1000>;
> + status = "disabled";
> + };
> +
> + frame@...25000 {
> + frame-number = <2>;
> + interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>;
> + reg = <0 0x17c25000 0 0x1000>;
> + status = "disabled";
> + };
> +
> + frame@...27000 {
> + frame-number = <3>;
> + interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
> + reg = <0 0x17c27000 0 0x1000>;
> + status = "disabled";
> + };
> +
> + frame@...29000 {
> + frame-number = <4>;
> + interrupts = <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>;
> + reg = <0 0x17c29000 0 0x1000>;
> + status = "disabled";
> + };
> +
> + frame@...2b000 {
> + frame-number = <5>;
> + interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>;
> + reg = <0 0x17c2b000 0 0x1000>;
> + status = "disabled";
> + };
> +
> + frame@...2d000 {
> + frame-number = <6>;
> + interrupts = <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>;
> + reg = <0 0x17c2d000 0 0x1000>;
> + status = "disabled";
> + };
> + };
> + };
> +
> + timer {
> + compatible = "arm,armv8-timer";
> + interrupts = <GIC_PPI 1 IRQ_TYPE_LEVEL_LOW>,
> + <GIC_PPI 2 IRQ_TYPE_LEVEL_LOW>,
> + <GIC_PPI 3 IRQ_TYPE_LEVEL_LOW>,
> + <GIC_PPI 0 IRQ_TYPE_LEVEL_LOW>;
> + };
> +};
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
>
Powered by blists - more mailing lists