Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755360AbbHYILA (ORCPT ); Tue, 25 Aug 2015 04:11:00 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:54184 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751763AbbHYIKz (ORCPT ); Tue, 25 Aug 2015 04:10:55 -0400 Date: Tue, 25 Aug 2015 13:40:42 +0530 From: Varadarajan Narayanan To: Stephen Boyd Cc: Mark Rutland , devicetree@vger.kernel.org, Russell King , Pawel Moll , Ian Campbell , linux-arm-msm@vger.kernel.org, Andy Gross , linux-kernel@vger.kernel.org, Rob Herring , Georgi Djakov , Kumar Gala , David Brown , linux-soc@vger.kernel.org, Lina Iyer , linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v3] qcom: ipq40xx: Add basic board/dts support for IPQ40XX SoC Message-ID: <20150825081042.GA14028@codeaurora.org> References: <20150824092813.GA15581@codeaurora.org> <20150824224928.GE14330@codeaurora.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150824224928.GE14330@codeaurora.org> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7953 Lines: 276 On Mon, Aug 24, 2015 at 03:49:28PM -0700, Stephen Boyd wrote: > On 08/24, Varadarajan Narayanan wrote: > > Add initial dts files and SoC support for IPQ40XX > > > > So far we haven't added any Xs in the model names for our SoC > support. Even for IPQ806X, we have it as IPQ8064 as the config > name with IPQ806x in the help text because there's IPQ8062 out > there. So I guess it should be IPQ4019 or something? Will rename it as IPQ4019 > > Signed-off-by: Varadarajan Narayanan > > --- > > diff --git a/Documentation/devicetree/bindings/qcom.txt b/Documentation/devicetree/bindings/qcom.txt > > new file mode 100644 > > index 0000000..7d56bd0 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/qcom.txt > > @@ -0,0 +1,16 @@ > > +Qualcomm IPQ device tree bindings > > +--------------------------------- > > + > > +System on Chip > > + > > +Device tree must specify which SoC it uses, with one of the > > +following compatible strings > > + > > + "qcom,ipq40xx" > > + > > +Platform > > + > > +Device tree must specify which Platform it uses, with one of the > > +following compatible strings > > + > > + "qcom,ipq40xx-r3pc" > > This file is not complete and I don't see any other silicon > vendors doing this, so please drop the entire file. Ok. > > diff --git a/arch/arm/boot/dts/qcom-ipq40xx-r3pc.dts b/arch/arm/boot/dts/qcom-ipq40xx-r3pc.dts > > new file mode 100644 > > index 0000000..7e4e629 > > --- /dev/null > > +++ b/arch/arm/boot/dts/qcom-ipq40xx-r3pc.dts > > @@ -0,0 +1,33 @@ > > +/* Copyright (c) 2014, The Linux Foundation. All rights reserved. > > + * > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License version 2 and > > + * only version 2 as published by the Free Software Foundation. > > + * > > + * This program is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > > + */ > > + > > +#include "qcom-ipq40xx.dtsi" > > + > > +/ { > > + model = "Qualcomm Technologies, Inc. IPQ40XX R3PC"; > > Same here, we should know what model number is on the board. Ok. > > + compatible = "qcom,ipq40xx-r3pc", "qcom,ipq40xx"; > > + > > + memory { > > + device_type = "memory"; > > + reg = <0x80000000 0x20000000>; /* 512MB */ > > + }; > > Doesn't the bootloader fill the memory node for us? Why do we > need this? > > > + > > + chosen { > > + bootargs = "root=/dev/ram rw init=/init console=ttyMSM0,115200n8 initrd=0x82000000,0x000E2246"; > > + }; > > Please don't add bootargs. Use stdout-path for the console part > and everything else should be done by the bootloader or is the > defaults. Since this is for the emulation platform, we don't have the bootloader. > > + > > + soc { > > + serial@78b0000 { > > + status = "ok"; > > + }; > > + }; > > +}; > > diff --git a/arch/arm/boot/dts/qcom-ipq40xx.dtsi b/arch/arm/boot/dts/qcom-ipq40xx.dtsi > > new file mode 100644 > > index 0000000..f572f38 > > --- /dev/null > > +++ b/arch/arm/boot/dts/qcom-ipq40xx.dtsi > > @@ -0,0 +1,81 @@ > > +/* > > + * Copyright (c) 2014, The Linux Foundation. All rights reserved. > > + * > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License version 2 and > > + * only version 2 as published by the Free Software Foundation. > > + * > > + * This program is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > > + */ > > + > > +/dts-v1/; > > + > > +#include "skeleton.dtsi" > > + > > +/ { > > + model = "Qualcomm Technologies, Inc. IPQ40XX"; > > + compatible = "qcom,ipq40xx"; > > These two should also be specific and drop the Xs. Same goes for > the file name. Ok. > > + interrupt-parent = <&intc>; > > + > > + cpus { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + cpu@0 { > > + device_type = "cpu"; > > + compatible = "arm,cortex-a7"; > > + reg = <0x0>; > > + }; > > + > > + cpu@1 { > > + device_type = "cpu"; > > + compatible = "arm,cortex-a7"; > > + reg = <0x1>; > > + }; > > + > > + cpu@2 { > > + device_type = "cpu"; > > + compatible = "arm,cortex-a7"; > > + reg = <0x2>; > > + }; > > + > > + cpu@3 { > > + device_type = "cpu"; > > + compatible = "arm,cortex-a7"; > > + reg = <0x3>; > > + }; > > + }; > > + > > + soc { > > + #address-cells = <1>; > > + #size-cells = <1>; > > + ranges; > > + compatible = "simple-bus"; > > + > > + intc: interrupt-controller@b000000 { > > + compatible = "qcom,msm-qgic2"; > > + interrupt-controller; > > + #interrupt-cells = <3>; > > + reg = <0x0b000000 0x1000>, > > + <0x0b002000 0x1000>; > > + }; > > + > > + timer { > > This node is not a child of the SoC node. It should be at the > root level. Ok. > > + compatible = "arm,armv7-timer"; > > + interrupts = <1 2 0xf08>, > > + <1 3 0xf08>, > > + <1 4 0xf08>, > > + <1 1 0xf08>; > > + clock-frequency = <20833333>; > > Drop this clock-frequency part if you can. The hardware should > properly report the frequency. Cannot drop this. This is the ref-clock frequency. For the chip it is 48MHz, for emulation is ~20MHz. Similar to https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/qcom-msm8974.dtsi#n89 > > + }; > > + > > + serial@78b0000 { > > + compatible = "qcom,msm-uartdm-v1.4", "qcom,msm-uartdm"; > > + reg = <0x78b0000 0x200>; > > + interrupts = <0 108 0>; > > + status = "disabled"; > > Don't you need some clocks here? Yes, will get updated in subsequent patches. > > + }; > > + }; > > +}; > > diff --git a/arch/arm/configs/ipq_defconfig b/arch/arm/configs/ipq_defconfig > > index 1cabd8b..89bf8ad 100644 > > --- a/arch/arm/configs/ipq_defconfig > > +++ b/arch/arm/configs/ipq_defconfig > > @@ -21,6 +21,7 @@ CONFIG_ARCH_QCOM=y > > CONFIG_ARCH_MSM8X60=y > > CONFIG_ARCH_MSM8960=y > > CONFIG_ARCH_MSM8974=y > > +CONFIG_ARCH_IPQ40XX=y > > CONFIG_ARCH_IPQ8064=y > > CONFIG_SMP=y > > CONFIG_PREEMPT=y > > This hunk should be in a separate patch. Ok. > > diff --git a/arch/arm/mach-qcom/Kconfig b/arch/arm/mach-qcom/Kconfig > > index fab49a2..70812aa 100644 > > --- a/arch/arm/mach-qcom/Kconfig > > +++ b/arch/arm/mach-qcom/Kconfig > > @@ -22,6 +22,10 @@ config ARCH_MSM8974 > > bool "Enable support for MSM8974" > > select HAVE_ARM_ARCH_TIMER > > > > +config ARCH_IPQ40XX > > + bool "Enable support for IPQ40XX" > > + select HAVE_ARM_ARCH_TIMER > > + > > config ARCH_IPQ8064 > > bool "Enable support for IPQ806x" > > select CLKSRC_QCOM > > diff --git a/arch/arm/mach-qcom/board.c b/arch/arm/mach-qcom/board.c > > index 6d8bbf7..af9e247 100644 > > --- a/arch/arm/mach-qcom/board.c > > +++ b/arch/arm/mach-qcom/board.c > > @@ -18,6 +18,7 @@ static const char * const qcom_dt_match[] __initconst = { > > "qcom,apq8064", > > "qcom,apq8074-dragonboard", > > "qcom,apq8084", > > + "qcom,ipq40xx", > > "qcom,ipq8062", > > "qcom,ipq8064", > > "qcom,msm8660-surf", > > > > And these two should also go as a separate patch. Ok. Thanks Varada > > -- > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > a Linux Foundation Collaborative Project > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/