RE: [PATCH] [linux-devel]arm64: Add DTS support for FSL's LS1012A SoC

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Stuart Yoder
> Sent: Friday, July 22, 2016 9:03 PM
> To: Bhaskar U <bhaskar.upadhaya@xxxxxxx>; devicetree@xxxxxxxxxxxxxxx;
> shawnguo@xxxxxxxxxx
> Cc: oss@xxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; Bhaskar U
> <bhaskar.upadhaya@xxxxxxx>; Prabhakar Kushwaha
> <prabhakar.kushwaha@xxxxxxx>; Calvin Johnson
> <calvin.johnson@xxxxxxx>; Makarand Pawagi <makarand.pawagi@xxxxxxx>;
> Pratiyush Srivastava <pratiyush.srivastava@xxxxxxx>; Yunhui Cui
> <yunhui.cui@xxxxxxx>; Rajesh Bhagat <rajesh.bhagat@xxxxxxx>; Huan
> Wang <alison.wang@xxxxxxx>; Hongtao Jia <hongtao.jia@xxxxxxx>; Anji J
> <anji.jagarlmudi@xxxxxxxxxxxxx>; Rajan Srivastava
> <rajan.srivastava@xxxxxxx>
> Subject: RE: [PATCH] [linux-devel]arm64: Add DTS support for FSL's LS1012A
> SoC
> 
> 
> > -----Original Message-----
> > From: Bhaskar Upadhaya [mailto:Bhaskar.Upadhaya@xxxxxxx]
> > Sent: Friday, July 22, 2016 12:35 AM
> > To: devicetree@xxxxxxxxxxxxxxx; shawnguo@xxxxxxxxxx
> > Cc: Stuart Yoder <stuart.yoder@xxxxxxx>; oss@xxxxxxxxxxxx;
> > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; Bhaskar U
> > <bhaskar.upadhaya@xxxxxxx>; Prabhakar Kushwaha
> > <prabhakar.kushwaha@xxxxxxx>; Calvin Johnson
> <calvin.johnson@xxxxxxx>;
> > Makarand Pawagi <makarand.pawagi@xxxxxxx>; Pratiyush Srivastava
> > <pratiyush.srivastava@xxxxxxx>; Yunhui Cui <yunhui.cui@xxxxxxx>;
> > Rajesh Bhagat <rajesh.bhagat@xxxxxxx>; Huan Wang
> > <alison.wang@xxxxxxx>; Hongtao Jia <hongtao.jia@xxxxxxx>; Anji J
> > <anji.jagarlmudi@xxxxxxxxxxxxx>
> > Subject: [PATCH] [linux-devel]arm64: Add DTS support for FSL's LS1012A
> > SoC
> >
> > The QorIQ LS1012A processor is a new Freescale' SoC optimized for
> > battery-backed or USB-powered, integrates a single ARM
> > Cortex-A53 core with a hardware packet forwarding engine and
> > high-speed interfaces to deliver line-rate networking performance.
> > LS1012AQDS, LS1012ARDB, LS1012AFRDM are a high-performance
> development
> > platform using LS1012A SoC.
> 
> A patch description should not read like a marketing brochure.  Delete the
> above text.
> 
> > diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts
> > b/arch/arm64/boot/dts/freescale/fsl-
> > ls1012a-frdm.dts
> > new file mode 100644
> > index 0000000..5db6133
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts
> > @@ -0,0 +1,186 @@
> > +/*
> > + * Device Tree Include file for Freescale Layerscape-1012A family SoC.
> 
> Why are you calling this an "Include file" when it is not?
> 
> > + * Copyright 2016, Freescale Semiconductor Inc.
> > +
> > + * Redistribution and use in source and binary forms, with or without
> > + * modification, are permitted provided that the following conditions are
> met:
> > + *     * Redistributions of source code must retain the above copyright
> > + *       notice, this list of conditions and the following disclaimer.
> > + *     * Redistributions in binary form must reproduce the above copyright
> > + *       notice, this list of conditions and the following disclaimer in the
> > + *       documentation and/or other materials provided with the distribution.
> > + *     * Neither the name of Freescale Semiconductor nor the
> > + *       names of its contributors may be used to endorse or promote
> products
> > + *       derived from this software without specific prior written permission.
> > + *
> > + *
> > + * ALTERNATIVELY, this software may be distributed under the terms of
> > + the
> > + * GNU General Public License ("GPL") as published by the Free
> > + Software
> > + * Foundation, either version 2 of that License or (at your option)
> > + any
> > + * later version.
> > + *
> > + * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor ``AS IS'' AND
> > + ANY
> > + * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
> THE
> > + IMPLIED
> > + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
> PURPOSE
> > + ARE
> > + * DISCLAIMED. IN NO EVENT SHALL Freescale Semiconductor BE LIABLE
> > + FOR ANY
> > + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> CONSEQUENTIAL
> > + DAMAGES
> > + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE
> GOODS OR
> > + SERVICES;
> > + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> HOWEVER
> > + CAUSED AND
> > + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
> > + OR TORT
> > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
> THE
> > + USE OF THIS
> > + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> > + */
> 
> The above license text is not consistent with other NXP/Freescale dts files.
> 
> > +/dts-v1/;
> > +
> > +#include "fsl-ls1012a.dtsi"
> > +
> > +/ {
> > +	model = "LS1012A FREEDOM Board";
> 
> Why is FREEDOM all caps?  In the marketing information on this board it is
> simpley "Freedom".
> 
> > diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1012a-qds.dts
> > b/arch/arm64/boot/dts/freescale/fsl-
> > ls1012a-qds.dts
> > new file mode 100644
> > index 0000000..40c85b7
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/fsl-ls1012a-qds.dts
> > @@ -0,0 +1,218 @@
> > +/*
> > + * Device Tree Include file for Freescale Layerscape-1012A family SoC.
> 
> This is not an include file.
> 
> Also, make the license text consistent with other NXP/Freescale dts files.
> 
> > diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1012a-rdb.dts
> > b/arch/arm64/boot/dts/freescale/fsl-
> > ls1012a-rdb.dts
> > new file mode 100644
> > index 0000000..eadcb63
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/fsl-ls1012a-rdb.dts
> > @@ -0,0 +1,115 @@
> > +/*
> > + * Device Tree Include file for Freescale Layerscape-1012A family SoC.
> 
> This is not an include file.
> 
> Also, make the license text consistent with other NXP/Freescale dts files.
> 
> > diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
> > b/arch/arm64/boot/dts/freescale/fsl-
> > ls1012a.dtsi
> > new file mode 100644
> > index 0000000..c5fce10
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/freescale/fsl-ls1012a.dtsi
> > @@ -0,0 +1,512 @@
> > +/*
> > + * Device Tree Include file for Freescale Layerscape-1043A family SoC.
> 
> This is not an LS1043A.
> 
> > + * Copyright 2016, Freescale Semiconductor
> > + *
> > + * This file is dual-licensed: you can use it either under the terms
> > + * of the GPLv2 or the X11 license, at your option. Note that this
> > + dual
> > + * licensing only applies to this file, and not this project as a
> > + * whole.
> > + *
> > + *  a) This library is free software; you can redistribute it and/or
> > + *     modify it under the terms of the GNU General Public License as
> > + *     published by the Free Software Foundation; either version 2 of the
> > + *     License, or (at your option) any later version.
> > + *
> > + *     This library 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.
> > + *
> > + * Or, alternatively,
> > + *
> > + *  b) Permission is hereby granted, free of charge, to any person
> > + *     obtaining a copy of this software and associated documentation
> > + *     files (the "Software"), to deal in the Software without
> > + *     restriction, including without limitation the rights to use,
> > + *     copy, modify, merge, publish, distribute, sublicense, and/or
> > + *     sell copies of the Software, and to permit persons to whom the
> > + *     Software is furnished to do so, subject to the following
> > + *     conditions:
> > + *
> > + *     The above copyright notice and this permission notice shall be
> > + *     included in all copies or substantial portions of the Software.
> > + *
> > + *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY
> KIND,
> > + *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE
> WARRANTIES
> > + *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> > + *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> > + *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> > + *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> > + *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE
> OR
> > + *     OTHER DEALINGS IN THE SOFTWARE.
> > + */
> >
> > +#include <dt-bindings/thermal/thermal.h>
> > +
> > +/ {
> > +	compatible = "fsl,ls1012a";
> > +	interrupt-parent = <&gic>;
> > +	#address-cells = <2>;
> > +	#size-cells = <2>;
> > +
> > +	cpus {
> > +		#address-cells = <2>;
> > +		#size-cells = <0>;
> > +
> > +		/*
> > +		 * We expect the enable-method for cpu's to be "psci", but this
> > +		 * is dependent on the SoC FW, which will fill this in.
> > +		 *
> > +		 * Currently supported enable-method is psci v0.2
> > +		 */
> > +		cpu0: cpu@0 {
> > +			device_type = "cpu";
> > +			compatible = "arm,cortex-a53";
> > +			reg = <0x0 0x0>;
> > +			clocks = <&clockgen 1 0>;
> > +			#cooling-cells = <2>;
> > +		};
> > +
> > +	};
> > +
> > +
> > +	sysclk: sysclk {
> > +		compatible = "fixed-clock";
> > +		#clock-cells = <0>;
> > +		clock-frequency = <100000000>;
> > +		clock-output-names = "sysclk";
> > +	};
> > +
> > +	timer {
> > +		compatible = "arm,armv8-timer";
> > +		interrupts = <1 13 0x1>, /* Physical Secure PPI */
> > +			     <1 14 0x1>, /* Physical Non-Secure PPI */
> > +			     <1 11 0x1>, /* Virtual PPI */
> > +			     <1 10 0x1>; /* Hypervisor PPI */
> > +		arm,reread-timer;
> > +	};
> > +
> > +	pmu {
> > +		compatible = "arm,armv8-pmuv3";
> > +		interrupts = <0 106 0x4>;
> > +	};
> > +
> > +	gic: interrupt-controller@1400000 {
> > +		compatible = "arm,gic-400";
> > +		#interrupt-cells = <3>;
> > +		interrupt-controller;
> > +		reg = <0x0 0x1401000 0 0x1000>, /* GICD */
> > +		      <0x0 0x1402000 0 0x2000>, /* GICC */
> > +		      <0x0 0x1404000 0 0x2000>, /* GICH */
> > +		      <0x0 0x1406000 0 0x2000>; /* GICV */
> > +		interrupts = <1 9 0xf08>;
> > +	};
> > +
> > +	soc {
> > +		compatible = "simple-bus";
> > +		#address-cells = <2>;
> > +		#size-cells = <2>;
> > +		ranges;
> > +
> > +		clockgen: clocking@1ee1000 {
> > +			compatible = "fsl,ls1012a-clockgen","fsl,ls1043a-
> clockgen";
> > +			reg = <0x0 0x1ee1000 0x0 0x1000>;
> > +			#clock-cells = <2>;
> > +			clocks = <&sysclk>;
> > +		};
> > +
> > +		scfg: scfg@1570000 {
> > +			compatible = "fsl,ls1012a-scfg", "fsl,ls1043a-scfg",
> "syscon";
> > +			reg = <0x0 0x1570000 0x0 0x10000>;
> > +			big-endian;
> > +		};
> > +
> > +		crypto: crypto@1700000 {
> > +			compatible = "fsl,sec-v5.4", "fsl,sec-v5.0",
> > +				     "fsl,sec-v4.0";
> > +			fsl,sec-era = <8>;
> > +			#address-cells = <1>;
> > +			#size-cells = <1>;
> > +			ranges = <0x0 0x00 0x1700000 0x100000>;
> > +			reg = <0x00 0x1700000 0x0 0x100000>;
> > +			interrupts = <0 75 0x4>;
> > +
> > +			sec_jr0: jr@10000 {
> > +				compatible = "fsl,sec-v5.4-job-ring",
> > +					     "fsl,sec-v5.0-job-ring",
> > +					     "fsl,sec-v4.0-job-ring";
> > +				reg	   = <0x10000 0x10000>;
> > +				interrupts = <0 71 0x4>;
> > +			};
> > +
> > +			sec_jr1: jr@20000 {
> > +				compatible = "fsl,sec-v5.4-job-ring",
> > +					     "fsl,sec-v5.0-job-ring",
> > +					     "fsl,sec-v4.0-job-ring";
> > +				reg	   = <0x20000 0x10000>;
> > +				interrupts = <0 72 0x4>;
> > +			};
> > +
> > +			sec_jr2: jr@30000 {
> > +				compatible = "fsl,sec-v5.4-job-ring",
> > +					     "fsl,sec-v5.0-job-ring",
> > +					     "fsl,sec-v4.0-job-ring";
> > +				reg	   = <0x30000 0x10000>;
> > +				interrupts = <0 73 0x4>;
> > +			};
> > +
> > +			sec_jr3: jr@40000 {
> > +				compatible = "fsl,sec-v5.4-job-ring",
> > +					     "fsl,sec-v5.0-job-ring",
> > +					     "fsl,sec-v4.0-job-ring";
> > +				reg	   = <0x40000 0x10000>;
> > +				interrupts = <0 74 0x4>;
> > +			};
> > +		};
> > +
> > +
> > +		dcfg: dcfg@1ee0000 {
> > +			compatible = "fsl,ls1012a-dcfg", "fsl,ls1043a-dcfg",
> "syscon";
> > +			reg = <0x0 0x1ee0000 0x0 0x10000>;
> > +		};
> > +
> > +		reset: reset@1EE00B0 {
> > +			compatible = "fsl,ls-reset";
> > +			reg = <0x0 0x1EE00B0 0x0 0x4>;
> > +			big-endian;
> > +		};
> 
> Where is the binding for the above node?  Other arm64 SoCs are using "syscon-
> reboot".  Can that be used?
> 
> 
> > +	reserved-memory {
> > +		#address-cells = <2>;
> > +		#size-cells = <2>;
> > +		ranges;
> > +
> > +		pfe_reserved: packetbuffer@83400000 {
> > +			reg = <0 0x83400000 0 0xc00000>;
> > +		};
> > +	};
> > +
> > +	pfe: pfe@04000000 {
> > +		compatible = "fsl,pfe";
> > +		ranges = <0x0 0x00 0x04000000 0xc00000
> > +			  0x1 0x00 0x83400000 0xc00000>;
> > +		reg =   <0x0 0x90500000 0x0 0x10000>,	/* APB 64K */
> > +			<0x0 0x04000000 0x0 0xc00000>,	/* AXI 16M */
> > +			<0x0 0x83400000 0x0 0xc00000>,    /* PFE DDR 12M */
> > +			<0x0 0x10000000 0x0 0x2000>;	/* OCRAM 8K  */
> > +		fsl,pfe-num-interfaces = < 0x2 >;
> > +		interrupts = <0 172 0x4>;
> > +		#interrupt-names = "hifirq";
> > +		memory-region = <&pfe_reserved>;
> > +		fsl,pfe-scfg = <&scfg 0>;
> > +	};
> 
> I understand the PFE driver needs some memory, but I think the way you are
> doing it needs to be completely re-thought through.  Where is the magic
> 0x83400000 coming from?  Why does that address show up in 2 places-- both
> the pfe reg property and the reserved-memory node?

IMO, it will be better to remove from here and submit a separate patch for pfe node. This can be done while submitting the pfe driver.
It will give more clarity. Also, pfe dt bindings documentation entry is required, isnt' it?

Calvin



--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux