All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] Patchset v4 Poplar 96Boards EE
@ 2017-05-08 16:36 Jorge Ramirez-Ortiz
  2017-05-08 16:36 ` [U-Boot] [PATCHv4 1/3] ARM64: dts: hi3798cv200-poplar: add device tree bindings Jorge Ramirez-Ortiz
                   ` (2 more replies)
  0 siblings, 3 replies; 53+ messages in thread
From: Jorge Ramirez-Ortiz @ 2017-05-08 16:36 UTC (permalink / raw)
  To: u-boot

[PATCHv4 1/3] ARM64: dts: hi3798cv200-poplar: add device tree bindings
[PATCHv4 2/3] driver: mmc: update debug info
[PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar

The following changes with respect to v3:

- Remove modifications to the linux kernel device tree
- Introduce board specific uboot device tree config
  * specify the clock for the console
  * define the ehci parameters to allow using the ehci-generic driver

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 1/3] ARM64: dts: hi3798cv200-poplar: add device tree bindings
  2017-05-08 16:36 [U-Boot] Patchset v4 Poplar 96Boards EE Jorge Ramirez-Ortiz
@ 2017-05-08 16:36 ` Jorge Ramirez-Ortiz
  2017-05-08 16:36 ` [U-Boot] [PATCHv4 2/3] driver: mmc: update debug info Jorge Ramirez-Ortiz
  2017-05-08 16:36 ` [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards Jorge Ramirez-Ortiz
  2 siblings, 0 replies; 53+ messages in thread
From: Jorge Ramirez-Ortiz @ 2017-05-08 16:36 UTC (permalink / raw)
  To: u-boot

pulled from linux-next tag 20170505

Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
---
 arch/arm/dts/hi3798cv200-poplar.dts     | 162 +++++++++++++
 arch/arm/dts/hi3798cv200.dtsi           | 411 ++++++++++++++++++++++++++++++++
 include/dt-bindings/clock/histb-clock.h |  66 +++++
 include/dt-bindings/reset/ti-syscon.h   |  38 +++
 4 files changed, 677 insertions(+)
 create mode 100644 arch/arm/dts/hi3798cv200-poplar.dts
 create mode 100644 arch/arm/dts/hi3798cv200.dtsi
 create mode 100644 include/dt-bindings/clock/histb-clock.h
 create mode 100644 include/dt-bindings/reset/ti-syscon.h

diff --git a/arch/arm/dts/hi3798cv200-poplar.dts b/arch/arm/dts/hi3798cv200-poplar.dts
new file mode 100644
index 0000000..b914287
--- /dev/null
+++ b/arch/arm/dts/hi3798cv200-poplar.dts
@@ -0,0 +1,162 @@
+/*
+ * DTS File for HiSilicon Poplar Development Board
+ *
+ * Copyright (c) 2016-2017 HiSilicon Technologies Co., Ltd.
+ *
+ * Released under the GPLv2 only.
+ * SPDX-License-Identifier: GPL-2.0
+ */
+
+/dts-v1/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include "hi3798cv200.dtsi"
+
+/ {
+	model = "HiSilicon Poplar Development Board";
+	compatible = "hisilicon,hi3798cv200-poplar", "hisilicon,hi3798cv200";
+
+	aliases {
+		serial0 = &uart0;
+		serial2 = &uart2;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+
+	memory at 0 {
+		device_type = "memory";
+		reg = <0x0 0x0 0x0 0x80000000>;
+	};
+
+	leds {
+		compatible = "gpio-leds";
+
+		user-led0 {
+			label = "USER-LED0";
+			gpios = <&gpio6 3 GPIO_ACTIVE_LOW>;
+			linux,default-trigger = "heartbeat";
+			default-state = "off";
+		};
+
+		user-led1 {
+			label = "USER-LED1";
+			gpios = <&gpio5 1 GPIO_ACTIVE_LOW>;
+			linux,default-trigger = "mmc0";
+			default-state = "off";
+		};
+
+		user-led2 {
+			label = "USER-LED2";
+			gpios = <&gpio5 2 GPIO_ACTIVE_LOW>;
+			linux,default-trigger = "none";
+			default-state = "off";
+		};
+
+		user-led3 {
+			label = "USER-LED3";
+			gpios = <&gpio10 6 GPIO_ACTIVE_LOW>;
+			linux,default-trigger = "cpu0";
+			default-state = "off";
+		};
+	};
+};
+
+&gmac1 {
+	status = "okay";
+	#address-cells = <1>;
+	#size-cells = <0>;
+	phy-handle = <&eth_phy1>;
+	phy-mode = "rgmii";
+	hisilicon,phy-reset-delays-us = <10000 10000 30000>;
+
+	eth_phy1: phy at 3 {
+		reg = <3>;
+	};
+};
+
+&gpio1 {
+	status = "okay";
+	gpio-line-names = "LS-GPIO-E",	"",
+			  "",		"",
+			  "",		"LS-GPIO-F",
+			  "",		"LS-GPIO-J";
+};
+
+&gpio2 {
+	status = "okay";
+	gpio-line-names = "LS-GPIO-H",	"LS-GPIO-I",
+			  "LS-GPIO-L",	"LS-GPIO-G",
+			  "LS-GPIO-K",	"",
+			  "",		"";
+};
+
+&gpio3 {
+	status = "okay";
+	gpio-line-names = "",		"",
+			  "",		"",
+			  "LS-GPIO-C",	"",
+			  "",		"LS-GPIO-B";
+};
+
+&gpio4 {
+	status = "okay";
+	gpio-line-names = "",		"",
+			  "",		"",
+			  "",		"LS-GPIO-D",
+			  "",		"";
+};
+
+&gpio5 {
+	status = "okay";
+	gpio-line-names = "",		"USER-LED-1",
+			  "USER-LED-2",	"",
+			  "",		"LS-GPIO-A",
+			  "",		"";
+};
+
+&gpio6 {
+	status = "okay";
+	gpio-line-names = "",		"",
+			  "",		"USER-LED-0",
+			  "",		"",
+			  "",		"";
+};
+
+&gpio10 {
+	status = "okay";
+	gpio-line-names = "",		"",
+			  "",		"",
+			  "",		"",
+			  "USER-LED-3",	"";
+};
+
+&i2c0 {
+	status = "okay";
+	label = "LS-I2C0";
+};
+
+&i2c2 {
+	status = "okay";
+	label = "LS-I2C1";
+};
+
+&ir {
+	status = "okay";
+};
+
+&spi0 {
+	status = "okay";
+	label = "LS-SPI0";
+};
+
+&uart0 {
+	status = "okay";
+};
+
+&uart2 {
+	status = "okay";
+	label = "LS-UART0";
+};
+/* No optional LS-UART1 on Low Speed Expansion Connector. */
diff --git a/arch/arm/dts/hi3798cv200.dtsi b/arch/arm/dts/hi3798cv200.dtsi
new file mode 100644
index 0000000..75865f8
--- /dev/null
+++ b/arch/arm/dts/hi3798cv200.dtsi
@@ -0,0 +1,411 @@
+/*
+ * DTS File for HiSilicon Hi3798cv200 SoC.
+ *
+ * Copyright (c) 2016-2017 HiSilicon Technologies Co., Ltd.
+ *
+ * Released under the GPLv2 only.
+ * SPDX-License-Identifier: GPL-2.0
+ */
+
+#include <dt-bindings/clock/histb-clock.h>
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/reset/ti-syscon.h>
+
+/ {
+	compatible = "hisilicon,hi3798cv200";
+	interrupt-parent = <&gic>;
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	psci {
+		compatible = "arm,psci-0.2";
+		method = "smc";
+	};
+
+	cpus {
+		#address-cells = <2>;
+		#size-cells = <0>;
+
+		cpu at 0 {
+			compatible = "arm,cortex-a53";
+			device_type = "cpu";
+			reg = <0x0 0x0>;
+			enable-method = "psci";
+		};
+
+		cpu at 1 {
+			compatible = "arm,cortex-a53";
+			device_type = "cpu";
+			reg = <0x0 0x1>;
+			enable-method = "psci";
+		};
+
+		cpu at 2 {
+			compatible = "arm,cortex-a53";
+			device_type = "cpu";
+			reg = <0x0 0x2>;
+			enable-method = "psci";
+		};
+
+		cpu at 3 {
+			compatible = "arm,cortex-a53";
+			device_type = "cpu";
+			reg = <0x0 0x3>;
+			enable-method = "psci";
+		};
+	};
+
+	gic: interrupt-controller at f1001000 {
+		compatible = "arm,gic-400";
+		reg = <0x0 0xf1001000 0x0 0x1000>,  /* GICD */
+		      <0x0 0xf1002000 0x0 0x100>;   /* GICC */
+		#address-cells = <0>;
+		#interrupt-cells = <3>;
+		interrupt-controller;
+	};
+
+	timer {
+		compatible = "arm,armv8-timer";
+		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) |
+			      IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) |
+			      IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) |
+			      IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) |
+			      IRQ_TYPE_LEVEL_LOW)>;
+	};
+
+	soc: soc at f0000000 {
+		compatible = "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges = <0x0 0x0 0xf0000000 0x10000000>;
+
+		crg: clock-reset-controller at 8a22000 {
+			compatible = "hisilicon,hi3798cv200-crg", "syscon", "simple-mfd";
+			reg = <0x8a22000 0x1000>;
+			#clock-cells = <1>;
+			#reset-cells = <2>;
+
+			gmacphyrst: reset-controller {
+				compatible = "ti,syscon-reset";
+				#reset-cells = <1>;
+				ti,reset-bits =
+					<0xcc 12 0xcc 12 0 0 (ASSERT_CLEAR |
+					 DEASSERT_SET|STATUS_NONE)>,
+					<0xcc 13 0xcc 13 0 0 (ASSERT_CLEAR |
+					 DEASSERT_SET|STATUS_NONE)>;
+			};
+		};
+
+		sysctrl: system-controller at 8000000 {
+			compatible = "hisilicon,hi3798cv200-sysctrl", "syscon";
+			reg = <0x8000000 0x1000>;
+			#clock-cells = <1>;
+			#reset-cells = <2>;
+		};
+
+		uart0: serial at 8b00000 {
+			compatible = "arm,pl011", "arm,primecell";
+			reg = <0x8b00000 0x1000>;
+			interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&sysctrl HISTB_UART0_CLK>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		uart2: serial at 8b02000 {
+			compatible = "arm,pl011", "arm,primecell";
+			reg = <0x8b02000 0x1000>;
+			interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&crg HISTB_UART2_CLK>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		i2c0: i2c at 8b10000 {
+			compatible = "hisilicon,hix5hd2-i2c";
+			reg = <0x8b10000 0x1000>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			interrupts = <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>;
+			clock-frequency = <400000>;
+			clocks = <&crg HISTB_I2C0_CLK>;
+			status = "disabled";
+		};
+
+		i2c1: i2c at 8b11000 {
+			compatible = "hisilicon,hix5hd2-i2c";
+			reg = <0x8b11000 0x1000>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			interrupts = <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
+			clock-frequency = <400000>;
+			clocks = <&crg HISTB_I2C1_CLK>;
+			status = "disabled";
+		};
+
+		i2c2: i2c at 8b12000 {
+			compatible = "hisilicon,hix5hd2-i2c";
+			reg = <0x8b12000 0x1000>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>;
+			clock-frequency = <400000>;
+			clocks = <&crg HISTB_I2C2_CLK>;
+			status = "disabled";
+		};
+
+		i2c3: i2c at 8b13000 {
+			compatible = "hisilicon,hix5hd2-i2c";
+			reg = <0x8b13000 0x1000>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			interrupts = <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
+			clock-frequency = <400000>;
+			clocks = <&crg HISTB_I2C3_CLK>;
+			status = "disabled";
+		};
+
+		i2c4: i2c at 8b14000 {
+			compatible = "hisilicon,hix5hd2-i2c";
+			reg = <0x8b14000 0x1000>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			interrupts = <GIC_SPI 42 IRQ_TYPE_LEVEL_HIGH>;
+			clock-frequency = <400000>;
+			clocks = <&crg HISTB_I2C4_CLK>;
+			status = "disabled";
+		};
+
+		spi0: spi at 8b1a000 {
+			compatible = "arm,pl022", "arm,primecell";
+			reg = <0x8b1a000 0x1000>;
+			interrupts = <GIC_SPI 45 IRQ_TYPE_LEVEL_HIGH>;
+			num-cs = <1>;
+			cs-gpios = <&gpio7 1 0>;
+			clocks = <&crg HISTB_SPI0_CLK>;
+			clock-names = "apb_pclk";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		emmc: mmc at 9830000 {
+			compatible = "snps,dw-mshc";
+			reg = <0x9830000 0x10000>;
+			interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&crg HISTB_MMC_CIU_CLK>,
+				 <&crg HISTB_MMC_BIU_CLK>;
+			clock-names = "ciu", "biu";
+		};
+
+		gpio0: gpio at 8b20000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x8b20000 0x1000>;
+			interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&crg HISTB_APB_CLK>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		gpio1: gpio at 8b21000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x8b21000 0x1000>;
+			interrupts = <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&crg HISTB_APB_CLK>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		gpio2: gpio at 8b22000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x8b22000 0x1000>;
+			interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&crg HISTB_APB_CLK>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		gpio3: gpio at 8b23000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x8b23000 0x1000>;
+			interrupts = <GIC_SPI 111 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&crg HISTB_APB_CLK>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		gpio4: gpio at 8b24000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x8b24000 0x1000>;
+			interrupts = <GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&crg HISTB_APB_CLK>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		gpio5: gpio at 8004000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x8004000 0x1000>;
+			interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&crg HISTB_APB_CLK>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		gpio6: gpio at 8b26000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x8b26000 0x1000>;
+			interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&crg HISTB_APB_CLK>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		gpio7: gpio at 8b27000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x8b27000 0x1000>;
+			interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&crg HISTB_APB_CLK>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		gpio8: gpio at 8b28000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x8b28000 0x1000>;
+			interrupts = <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&crg HISTB_APB_CLK>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		gpio9: gpio at 8b29000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x8b29000 0x1000>;
+			interrupts = <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&crg HISTB_APB_CLK>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		gpio10: gpio at 8b2a000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x8b2a000 0x1000>;
+			interrupts = <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&crg HISTB_APB_CLK>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		gpio11: gpio at 8b2b000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x8b2b000 0x1000>;
+			interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&crg HISTB_APB_CLK>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		gpio12: gpio at 8b2c000 {
+			compatible = "arm,pl061", "arm,primecell";
+			reg = <0x8b2c000 0x1000>;
+			interrupts = <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			clocks = <&crg HISTB_APB_CLK>;
+			clock-names = "apb_pclk";
+			status = "disabled";
+		};
+
+		gmac0: ethernet at 9840000 {
+			compatible = "hisilicon,hi3798cv200-gmac", "hisilicon,hisi-gmac-v2";
+			reg = <0x9840000 0x1000>,
+			      <0x984300c 0x4>;
+			interrupts = <GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&crg HISTB_ETH0_MAC_CLK>,
+				 <&crg HISTB_ETH0_MACIF_CLK>;
+			clock-names = "mac_core", "mac_ifc";
+			resets = <&crg 0xcc 8>,
+				 <&crg 0xcc 10>,
+				 <&gmacphyrst 0>;
+			reset-names = "mac_core", "mac_ifc", "phy";
+			status = "disabled";
+		};
+
+		gmac1: ethernet at 9841000 {
+			compatible = "hisilicon,hi3798cv200-gmac", "hisilicon,hisi-gmac-v2";
+			reg = <0x9841000 0x1000>,
+			      <0x9843010 0x4>;
+			interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&crg HISTB_ETH1_MAC_CLK>,
+				 <&crg HISTB_ETH1_MACIF_CLK>;
+			clock-names = "mac_core", "mac_ifc";
+			resets = <&crg 0xcc 9>,
+				 <&crg 0xcc 11>,
+				 <&gmacphyrst 1>;
+			reset-names = "mac_core", "mac_ifc", "phy";
+			status = "disabled";
+		};
+
+		ir: ir at 8001000 {
+			compatible = "hisilicon,hix5hd2-ir";
+			reg = <0x8001000 0x1000>;
+			interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&sysctrl HISTB_IR_CLK>;
+			status = "disabled";
+		};
+	};
+};
diff --git a/include/dt-bindings/clock/histb-clock.h b/include/dt-bindings/clock/histb-clock.h
new file mode 100644
index 0000000..181c0f0
--- /dev/null
+++ b/include/dt-bindings/clock/histb-clock.h
@@ -0,0 +1,66 @@
+/*
+ * Copyright (c) 2016 HiSilicon Technologies Co., Ltd.
+ *
+ * This program 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 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __DTS_HISTB_CLOCK_H
+#define __DTS_HISTB_CLOCK_H
+
+/* clocks provided by core CRG */
+#define HISTB_OSC_CLK			0
+#define HISTB_APB_CLK			1
+#define HISTB_AHB_CLK			2
+#define HISTB_UART1_CLK		3
+#define HISTB_UART2_CLK		4
+#define HISTB_UART3_CLK		5
+#define HISTB_I2C0_CLK		6
+#define HISTB_I2C1_CLK		7
+#define HISTB_I2C2_CLK		8
+#define HISTB_I2C3_CLK		9
+#define HISTB_I2C4_CLK		10
+#define HISTB_I2C5_CLK		11
+#define HISTB_SPI0_CLK		12
+#define HISTB_SPI1_CLK		13
+#define HISTB_SPI2_CLK		14
+#define HISTB_SCI_CLK			15
+#define HISTB_FMC_CLK			16
+#define HISTB_MMC_BIU_CLK		17
+#define HISTB_MMC_CIU_CLK		18
+#define HISTB_MMC_DRV_CLK		19
+#define HISTB_MMC_SAMPLE_CLK		20
+#define HISTB_SDIO0_BIU_CLK		21
+#define HISTB_SDIO0_CIU_CLK		22
+#define HISTB_SDIO0_DRV_CLK		23
+#define HISTB_SDIO0_SAMPLE_CLK	24
+#define HISTB_PCIE_AUX_CLK		25
+#define HISTB_PCIE_PIPE_CLK		26
+#define HISTB_PCIE_SYS_CLK		27
+#define HISTB_PCIE_BUS_CLK		28
+#define HISTB_ETH0_MAC_CLK		29
+#define HISTB_ETH0_MACIF_CLK		30
+#define HISTB_ETH1_MAC_CLK		31
+#define HISTB_ETH1_MACIF_CLK		32
+#define HISTB_COMBPHY1_CLK		33
+
+
+/* clocks provided by mcu CRG */
+#define HISTB_MCE_CLK	1
+#define HISTB_IR_CLK	2
+#define HISTB_TIMER01_CLK	3
+#define HISTB_LEDC_CLK	4
+#define HISTB_UART0_CLK	5
+#define HISTB_LSADC_CLK	6
+
+#endif	/* __DTS_HISTB_CLOCK_H */
diff --git a/include/dt-bindings/reset/ti-syscon.h b/include/dt-bindings/reset/ti-syscon.h
new file mode 100644
index 0000000..884fd91
--- /dev/null
+++ b/include/dt-bindings/reset/ti-syscon.h
@@ -0,0 +1,38 @@
+/*
+ * TI Syscon Reset definitions
+ *
+ * Copyright (C) 2015-2016 Texas Instruments Incorporated - http://www.ti.com/
+ *
+ * This program 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 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.
+ */
+
+#ifndef __DT_BINDINGS_RESET_TI_SYSCON_H__
+#define __DT_BINDINGS_RESET_TI_SYSCON_H__
+
+/*
+ * The reset does not support the feature and corresponding
+ * values are not valid
+ */
+#define ASSERT_NONE	(1 << 0)
+#define DEASSERT_NONE	(1 << 1)
+#define STATUS_NONE	(1 << 2)
+
+/* When set this function is activated by setting(vs clearing) this bit */
+#define ASSERT_SET	(1 << 3)
+#define DEASSERT_SET	(1 << 4)
+#define STATUS_SET	(1 << 5)
+
+/* The following are the inverse of the above and are added for consistency */
+#define ASSERT_CLEAR	(0 << 3)
+#define DEASSERT_CLEAR	(0 << 4)
+#define STATUS_CLEAR	(0 << 5)
+
+#endif
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 2/3] driver: mmc: update debug info
  2017-05-08 16:36 [U-Boot] Patchset v4 Poplar 96Boards EE Jorge Ramirez-Ortiz
  2017-05-08 16:36 ` [U-Boot] [PATCHv4 1/3] ARM64: dts: hi3798cv200-poplar: add device tree bindings Jorge Ramirez-Ortiz
@ 2017-05-08 16:36 ` Jorge Ramirez-Ortiz
  2017-05-08 16:36 ` [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards Jorge Ramirez-Ortiz
  2 siblings, 0 replies; 53+ messages in thread
From: Jorge Ramirez-Ortiz @ 2017-05-08 16:36 UTC (permalink / raw)
  To: u-boot

This driver is used in another board; remove board information from
the driver debug log.

Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
---
 drivers/mmc/hi6220_dw_mmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/hi6220_dw_mmc.c b/drivers/mmc/hi6220_dw_mmc.c
index fdaf1e4..d795198 100644
--- a/drivers/mmc/hi6220_dw_mmc.c
+++ b/drivers/mmc/hi6220_dw_mmc.c
@@ -20,7 +20,7 @@
 
 static int hi6220_dwmci_core_init(struct dwmci_host *host, int index)
 {
-	host->name = "HiKey DWMMC";
+	host->name = "Hisilicon DWMMC";
 
 	host->dev_index = index;
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-08 16:36 [U-Boot] Patchset v4 Poplar 96Boards EE Jorge Ramirez-Ortiz
  2017-05-08 16:36 ` [U-Boot] [PATCHv4 1/3] ARM64: dts: hi3798cv200-poplar: add device tree bindings Jorge Ramirez-Ortiz
  2017-05-08 16:36 ` [U-Boot] [PATCHv4 2/3] driver: mmc: update debug info Jorge Ramirez-Ortiz
@ 2017-05-08 16:36 ` Jorge Ramirez-Ortiz
  2017-05-08 17:29   ` Tom Rini
  2017-05-25 16:08   ` Andreas Färber
  2 siblings, 2 replies; 53+ messages in thread
From: Jorge Ramirez-Ortiz @ 2017-05-08 16:36 UTC (permalink / raw)
  To: u-boot

This port adds support for:
        1) Serial
        2) eMMC
        3) USB

It has been tested with ARM TRUSTED FIRMWARE running u-boot as the
BL33 executable [see board's README]

eMMC has been tested for reading and booting the loader[1] and linux
kernels as well as saving the u-boot environment.

USB has been tested with ASIX networking adapter and SanDisk 7.4GB
drive.

PSCI has been tested via the reset call.

The firwmare upgrade process has been tested via TFTP and USB FAT
filesystem containing the fastboot.bin image in one of the partitions.

Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
---
 arch/arm/Kconfig                                   |  12 ++
 arch/arm/dts/hi3798cv200.dtsi                      |   3 +
 arch/arm/dts/poplar-uboot.dtsi                     |  24 +++
 arch/arm/include/asm/arch-hi3798cv200/dwmmc.h      |  13 ++
 .../arm/include/asm/arch-hi3798cv200/hi3798cv200.h |  50 +++++
 board/hisilicon/poplar/Kconfig                     |  15 ++
 board/hisilicon/poplar/MAINTAINERS                 |   6 +
 board/hisilicon/poplar/Makefile                    |   7 +
 board/hisilicon/poplar/README                      | 232 +++++++++++++++++++++
 board/hisilicon/poplar/poplar.c                    | 155 ++++++++++++++
 configs/poplar_defconfig                           |  26 +++
 include/configs/poplar.h                           |  86 ++++++++
 12 files changed, 629 insertions(+)
 create mode 100644 arch/arm/dts/poplar-uboot.dtsi
 create mode 100644 arch/arm/include/asm/arch-hi3798cv200/dwmmc.h
 create mode 100644 arch/arm/include/asm/arch-hi3798cv200/hi3798cv200.h
 create mode 100644 board/hisilicon/poplar/Kconfig
 create mode 100644 board/hisilicon/poplar/MAINTAINERS
 create mode 100644 board/hisilicon/poplar/Makefile
 create mode 100644 board/hisilicon/poplar/README
 create mode 100644 board/hisilicon/poplar/poplar.c
 create mode 100644 configs/poplar_defconfig
 create mode 100644 include/configs/poplar.h

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 1df6b36..d41e047 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -816,6 +816,17 @@ config TARGET_HIKEY
 	  Support for HiKey 96boards platform. It features a HI6220
 	  SoC, with 8xA53 CPU, mali450 gpu, and 1GB RAM.
 
+config TARGET_POPLAR
+	bool "Support Poplar 96boards Enterprise Edition Platform"
+	select ARM64
+	select DM
+	select OF_CONTROL
+	select DM_SERIAL
+	select DM_USB
+	  help
+	  Support for Poplar 96boards EE platform. It features a HI3798cv200
+	  SoC, with 4xA53 CPU, MaliT720 GPU, and 1GB RAM.
+
 config TARGET_LS1012AQDS
 	bool "Support ls1012aqds"
 	select ARCH_LS1012A
@@ -1145,6 +1156,7 @@ source "board/grinn/chiliboard/Kconfig"
 source "board/gumstix/pepper/Kconfig"
 source "board/h2200/Kconfig"
 source "board/hisilicon/hikey/Kconfig"
+source "board/hisilicon/poplar/Kconfig"
 source "board/imx31_phycore/Kconfig"
 source "board/isee/igep0033/Kconfig"
 source "board/olimex/mx23_olinuxino/Kconfig"
diff --git a/arch/arm/dts/hi3798cv200.dtsi b/arch/arm/dts/hi3798cv200.dtsi
index 75865f8..caa17de 100644
--- a/arch/arm/dts/hi3798cv200.dtsi
+++ b/arch/arm/dts/hi3798cv200.dtsi
@@ -409,3 +409,6 @@
 		};
 	};
 };
+
+#include "poplar-uboot.dtsi"
+
diff --git a/arch/arm/dts/poplar-uboot.dtsi b/arch/arm/dts/poplar-uboot.dtsi
new file mode 100644
index 0000000..8a9668a
--- /dev/null
+++ b/arch/arm/dts/poplar-uboot.dtsi
@@ -0,0 +1,24 @@
+/*
+ * U-Boot addition to:
+ *  1) initialize the console clock rate.
+ *  2) provide support for the generic-ehci USB driver currently not available
+ *     in the linux kernel (8/May/2017).
+ *
+ * (C) Copyright 2017 Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
+ *
+ * SPDX-License-Identifier:     GPL-2.0+
+ */
+
+&soc {
+	usb2: ehci at 9890000 {
+		compatible = "generic-ehci";
+		reg = <0x9890000 0x100>;
+		status = "okay";
+	};
+};
+
+&uart0 {
+	clock = <75000000>;
+	status = "okay";
+};
+
diff --git a/arch/arm/include/asm/arch-hi3798cv200/dwmmc.h b/arch/arm/include/asm/arch-hi3798cv200/dwmmc.h
new file mode 100644
index 0000000..1060d94
--- /dev/null
+++ b/arch/arm/include/asm/arch-hi3798cv200/dwmmc.h
@@ -0,0 +1,13 @@
+/*
+ * (C) Copyright 2017 Linaro
+ * Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef _HI3798cv200_DWMMC_H_
+#define _HI3798cv200_DWMMC_H_
+
+int hi6220_dwmci_add_port(int index, u32 regbase, int bus_width);
+
+#endif /* _HI3798cv200_DWMMC_H_ */
diff --git a/arch/arm/include/asm/arch-hi3798cv200/hi3798cv200.h b/arch/arm/include/asm/arch-hi3798cv200/hi3798cv200.h
new file mode 100644
index 0000000..1bd0d78
--- /dev/null
+++ b/arch/arm/include/asm/arch-hi3798cv200/hi3798cv200.h
@@ -0,0 +1,50 @@
+/*
+ * (C) Copyright 2017 Linaro
+ * Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef __HI3798cv200_H__
+#define __HI3798cv200_H__
+
+#define REG_BASE_PERI_CTRL		0xF8A20000
+#define REG_BASE_CRG			0xF8A22000
+
+/* DEVICES */
+#define REG_BASE_EHCI			0XF9890000
+#define REG_BASE_MCI			0xF9830000
+
+/* PERI control registers (4KB) */
+	/* USB2 PHY01 configuration register */
+#define PERI_CTRL_USB0			(REG_BASE_PERI_CTRL + 0x120)
+
+/* PERI CRG registers (4KB) */
+	/* USB2 CTRL0 clock and soft reset */
+#define PERI_CRG46			(REG_BASE_CRG + 0xb8)
+#define USB2_BUS_CKEN			(1<<0)
+#define USB2_OHCI48M_CKEN		(1<<1)
+#define USB2_OHCI12M_CKEN		(1<<2)
+#define USB2_OTG_UTMI_CKEN		(1<<3)
+#define USB2_HST_PHY_CKEN		(1<<4)
+#define USB2_UTMI0_CKEN			(1<<5)
+#define USB2_BUS_SRST_REQ		(1<<12)
+#define USB2_UTMI0_SRST_REQ		(1<<13)
+#define USB2_HST_PHY_SYST_REQ		(1<<16)
+#define USB2_OTG_PHY_SYST_REQ		(1<<17)
+#define USB2_CLK48_SEL			(1<<20)
+
+	/* USB2 PHY clock and soft reset */
+#define PERI_CRG47			(REG_BASE_CRG + 0xbc)
+#define USB2_PHY01_REF_CKEN		(1 << 0)
+#define USB2_PHY2_REF_CKEN		(1 << 2)
+#define USB2_PHY01_SRST_REQ		(1 << 4)
+#define USB2_PHY2_SRST_REQ		(1 << 6)
+#define USB2_PHY01_SRST_TREQ0		(1 << 8)
+#define USB2_PHY01_SRST_TREQ1		(1 << 9)
+#define USB2_PHY2_SRST_TREQ		(1 << 10)
+#define USB2_PHY01_REFCLK_SEL		(1 << 12)
+#define USB2_PHY2_REFCLK_SEL		(1 << 14)
+
+
+#endif
diff --git a/board/hisilicon/poplar/Kconfig b/board/hisilicon/poplar/Kconfig
new file mode 100644
index 0000000..3397295
--- /dev/null
+++ b/board/hisilicon/poplar/Kconfig
@@ -0,0 +1,15 @@
+if TARGET_POPLAR
+
+config SYS_BOARD
+	default "poplar"
+
+config SYS_VENDOR
+	default "hisilicon"
+
+config SYS_SOC
+	default "hi3798cv200"
+
+config SYS_CONFIG_NAME
+	default "poplar"
+
+endif
diff --git a/board/hisilicon/poplar/MAINTAINERS b/board/hisilicon/poplar/MAINTAINERS
new file mode 100644
index 0000000..0cc01c8
--- /dev/null
+++ b/board/hisilicon/poplar/MAINTAINERS
@@ -0,0 +1,6 @@
+Poplar BOARD
+M:     Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
+S:     Maintained
+F:     board/hisilicon/poplar
+F:     include/configs/poplar.h
+F:     configs/poplar_defconfig
diff --git a/board/hisilicon/poplar/Makefile b/board/hisilicon/poplar/Makefile
new file mode 100644
index 0000000..101545d
--- /dev/null
+++ b/board/hisilicon/poplar/Makefile
@@ -0,0 +1,7 @@
+#
+# (C) Copyright 2017 Linaro
+# Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
+#
+# SPDX-License-Identifier:	GPL-2.0+
+#
+obj-y	:= poplar.o
diff --git a/board/hisilicon/poplar/README b/board/hisilicon/poplar/README
new file mode 100644
index 0000000..b35382b
--- /dev/null
+++ b/board/hisilicon/poplar/README
@@ -0,0 +1,232 @@
+================================================================================
+			Board Information
+================================================================================
+
+Developed by HiSilicon, the board features the Hi3798C V200 with an
+integrated quad-core 64-bit ARM Cortex A53 processor and high
+performance Mali T720 GPU, making it capable of running any commercial
+set-top solution based on Linux or Android. Its high performance
+specification also supports a premium user experience with up to H.265
+HEVC decoding of 4K video at 60 frames per second.
+
+SOC  Hisilicon Hi3798CV200
+CPU  Quad-core ARM Cortex-A53 64 bit
+DRAM DDR3/3L/4 SDRAM interface, maximum 32-bit data width 2 GB 
+USB  Two USB 2.0 ports One USB 3.0 ports
+CONSOLE  USB-micro port for console support
+ETHERNET  1 GBe Ethernet
+PCIE  One PCIe 2.0 interfaces
+JTAG  8-Pin JTAG
+EXPANSION INTERFACE  Linaro 96Boards Low Speed Expansion slot
+DIMENSION Standard 160×120 mm 96Boards Enterprice Edition form factor
+WIFI  802.11AC 2*2 with Bluetooth
+CONNECTORS  One connector for Smart Card One connector for TSI
+
+
+================================================================================
+			BUILD INSTRUCTIONS
+================================================================================
+
+Compile from source:
+====================
+
+Get all the sources
+
+  > mkdir -p ~/poplar/src ~/poplar/bin
+  > cd ~/poplar/src
+  > git clone https://github.com/Linaro/poplar-l-loader.git l-loader
+  > git clone https://github.com/Linaro/poplar-arm-trusted-firmware.git atf
+  > git clone https://github.com/Linaro/poplar-u-boot.git u-boot
+
+
+Compile U-Boot:
+===============
+
+  Prerequisite:
+  # sudo apt-get install device-tree-compiler
+
+  > cd ~/poplar/src/u-boot
+  > make CROSS_COMPILE=aarch64-linux-gnu- poplar_defconfig
+  > make CROSS_COMPILE=aarch64-linux-gnu-
+  > cp u-boot.bin ~/poplar/bin
+
+Compile ARM Trusted Firmware (ATF):
+===================================
+
+  > cd ~/poplar/src/atf
+  > make CROSS_COMPILE=aarch64-linux-gnu- all fip \
+		SPD=none BL33=~/poplar/bin/u-boot.bin DEBUG=1 PLAT=poplar
+
+Copy resulting binaries
+  > cp build/hi3798cv200/debug/bl1.bin ~/poplar/src/l-loader/atf/
+  > cp build/hi3798cv200/debug/fip.bin ~/poplar/src/l-loader/atf/
+
+Compile l-loader:
+=================
+
+  > cd ~/poplar/src/l-loader
+  > make clean
+  > make CROSS_COMPILE=arm-linux-gnueabi-
+
+   Due to BootROM requiremets, rename l-loader.bin to fastboot.bin:
+  > cp l-loader.bin ~/poplar/bin/fastboot.bin
+
+
+================================================================================
+			FLASH INSTRUCTIONS
+================================================================================
+
+Two methods:
+
+Using USB debrick support:
+	Copy fastboot.bin to a FAT partition on the USB drive and reboot the
+       poplar board while pressing S3(usb_boot).
+
+       The system will execute the new u-boot and boot into a shell which you
+       can then use to write to eMMC.
+
+Using U-BOOT from shell:
+	1) using AXIS usb ethernet dongle and tftp
+	2) using FAT formated USB drive
+
+
+1. TFTP (USB ethernet dongle)
+=============================
+
+Plug a USB AXIS ethernet dongle on any of the USB2 ports on the Poplar board.
+Copy fastboot.bin to your tftp server.
+In u-boot make sure your network is properly setup.
+
+Then
+
+=> tftp 0x30000000 fastboot.bin
+starting USB...
+USB0:   USB EHCI 1.00
+scanning bus 0 for devices... 1 USB Device(s) found
+USB1:   USB EHCI 1.00
+scanning bus 1 for devices... 3 USB Device(s) found
+       scanning usb for storage devices... 0 Storage Device(s) found
+       scanning usb for ethernet devices... 1 Ethernet Device(s) found
+Waiting for Ethernet connection... done.
+Using asx0 device
+TFTP from server 192.168.1.4; our IP address is 192.168.1.10
+Filename 'poplar/fastboot.bin'.
+Load address: 0x30000000
+Loading: #################################################################
+	 #################################################################
+	 ###############################################################
+	 2 MiB/s
+done
+Bytes transferred = 983040 (f0000 hex)
+
+=> mmc write 0x30000000 0 0x780
+
+MMC write: dev # 0, block # 0, count 1920 ... 1920 blocks written: OK
+=> reset
+
+
+2. USING USB FAT DRIVE
+=======================
+
+Copy fastboot.bin to any partition on a FAT32 formated usb flash drive.
+Enter the uboot prompt
+
+=> fatls usb 0:2
+   983040   fastboot.bin
+
+1 file(s), 0 dir(s)
+
+=> fatload usb 0:2 0x30000000 fastboot.bin
+reading fastboot.bin
+983040 bytes read in 44 ms (21.3 MiB/s)
+
+=> mmc write 0x30000000 0 0x780
+
+MMC write: dev # 0, block # 0, count 1920 ... 1920 blocks written: OK
+
+
+================================================================================
+				BOOT TRACE
+================================================================================
+
+Bootrom start
+Boot Media: eMMC
+Decrypt auxiliary code ...OK
+
+lsadc voltage min: 000000FE, max: 000000FF, aver: 000000FE, index: 00000000
+
+Entry boot auxiliary code
+
+Auxiliary code - v1.00
+DDR code - V1.1.2 20160205
+Build: Mar 24 2016 - 17:09:44
+Reg Version:  v134
+Reg Time:     2016/03/18 09:44:55
+Reg Name:     hi3798cv2dmb_hi3798cv200_ddr3_2gbyte_8bitx4_4layers.reg
+
+Boot auxiliary code success
+Bootrom success
+
+LOADER:  Switched to aarch64 mode
+LOADER:  Entering ARM TRUSTED FIRMWARE
+LOADER:  CPU0 executes at 0x000ce000
+
+INFO:    BL1: 0xe1000 - 0xe7000 [size = 24576]
+NOTICE:  Booting Trusted Firmware
+NOTICE:  BL1: v1.3(debug):v1.3-372-g1ba9c60
+NOTICE:  BL1: Built : 17:51:33, Apr 30 2017
+INFO:    BL1: RAM 0xe1000 - 0xe7000
+INFO:    BL1: Loading BL2
+INFO:    Loading image id=1 at address 0xe9000
+INFO:    Image id=1 loaded at address 0xe9000, size = 0x5008
+NOTICE:  BL1: Booting BL2
+INFO:    Entry point address = 0xe9000
+INFO:    SPSR = 0x3c5
+NOTICE:  BL2: v1.3(debug):v1.3-372-g1ba9c60
+NOTICE:  BL2: Built : 17:51:33, Apr 30 2017
+INFO:    BL2: Loading BL31
+INFO:    Loading image id=3 at address 0x129000
+INFO:    Image id=3 loaded at address 0x129000, size = 0x8038
+INFO:    BL2: Loading BL33
+INFO:    Loading image id=5 at address 0x37000000
+INFO:    Image id=5 loaded at address 0x37000000, size = 0x58f17
+NOTICE:  BL1: Booting BL31
+INFO:    Entry point address = 0x129000
+INFO:    SPSR = 0x3cd
+INFO:    Boot bl33 from 0x37000000 for 364311 Bytes
+NOTICE:  BL31: v1.3(debug):v1.3-372-g1ba9c60
+NOTICE:  BL31: Built : 17:51:33, Apr 30 2017
+INFO:    BL31: Initializing runtime services
+INFO:    BL31: Preparing for EL3 exit to normal world
+INFO:    Entry point address = 0x37000000
+INFO:    SPSR = 0x3c9
+
+
+U-Boot 2017.05-rc2-00130-gd2255b0 (Apr 30 2017 - 17:51:28 +0200)poplar
+
+Model: HiSilicon Poplar Development Board
+BOARD: Hisilicon HI3798cv200 Poplar
+DRAM:  1 GiB
+MMC:   Hisilicon DWMMC: 0
+In:    serial at f8b00000
+Out:   serial at f8b00000
+Err:   serial at f8b00000
+Net:   Net Initialization Skipped
+No ethernet found.
+
+Hit any key to stop autoboot:  0
+starting USB...
+USB0:   USB EHCI 1.00
+scanning bus 0 for devices... 1 USB Device(s) found
+USB1:   USB EHCI 1.00
+scanning bus 1 for devices... 4 USB Device(s) found
+       scanning usb for storage devices... 1 Storage Device(s) found
+       scanning usb for ethernet devices... 1 Ethernet Device(s) found
+
+USB device 0:
+    Device 0: Vendor: SanDisk Rev: 1.00 Prod: Cruzer Blade
+	    Type: Removable Hard Disk
+	    Capacity: 7632.0 MB = 7.4 GB (15630336 x 512)
+... is now current device
+Scanning usb 0:1...
+=>
diff --git a/board/hisilicon/poplar/poplar.c b/board/hisilicon/poplar/poplar.c
new file mode 100644
index 0000000..4569187
--- /dev/null
+++ b/board/hisilicon/poplar/poplar.c
@@ -0,0 +1,155 @@
+/*
+ * (C) Copyright 2017 Linaro
+ * Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <asm/arch/hi3798cv200.h>
+#include <linux/arm-smccc.h>
+#include <asm/arch/dwmmc.h>
+#include <asm/armv8/mmu.h>
+#include <asm/io.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+static struct mm_region poplar_mem_map[] = {
+	{
+		.virt = 0x0UL,
+		.phys = 0x0UL,
+		.size = 0x80000000UL,
+		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
+			 PTE_BLOCK_INNER_SHARE
+	}, {
+		.virt = 0x80000000UL,
+		.phys = 0x80000000UL,
+		.size = 0x80000000UL,
+		.attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+			 PTE_BLOCK_NON_SHARE |
+			 PTE_BLOCK_PXN | PTE_BLOCK_UXN
+	}, {
+		0,
+	}
+};
+
+struct mm_region *mem_map = poplar_mem_map;
+
+int checkboard(void)
+{
+	puts("BOARD: Hisilicon HI3798cv200 Poplar\n");
+
+	return 0;
+}
+
+void reset_cpu(ulong addr)
+{
+	psci_system_reset();
+}
+
+int dram_init(void)
+{
+	gd->ram_size = get_ram_size(NULL, 0x80000000);
+
+	return 0;
+}
+
+int dram_init_banksize(void)
+{
+	int i;
+
+	for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
+		gd->bd->bi_dram[i].start = i * gd->bd->bi_dram[i - 1].size;
+		gd->bd->bi_dram[i].size = get_ram_size( (void *)
+			gd->bd->bi_dram[i].start, 0x10000000);
+	}
+
+	return 0;
+}
+
+static void usb2_phy_config(void)
+{
+	const u32 config[] = {
+		/* close EOP pre-emphasis. open data pre-emphasis */
+		0xa1001c,
+		/* Rcomp = 150mW, increase DC level */
+		0xa00607,
+		/* keep Rcomp working */
+		0xa10700,
+		/* Icomp = 212mW, increase current drive */
+		0xa00aab,
+		/* EMI fix: rx_active not stay 1 when error packets received */
+		0xa11140,
+		/* Comp mode select */
+		0xa11041,
+		/* adjust eye diagram */
+		0xa0098c,
+		/* adjust eye diagram */
+		0xa10a0a,
+	};
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(config); i++) {
+		writel(config[i], PERI_CTRL_USB0);
+		clrsetbits_le32(PERI_CTRL_USB0, BIT(21), BIT(20) | BIT(22));
+		udelay(20);
+	}
+}
+
+static void usb2_phy_init(void)
+{
+	/* reset usb2 controller bus/utmi/roothub */
+	setbits_le32(PERI_CRG46,
+		USB2_BUS_SRST_REQ | USB2_UTMI0_SRST_REQ |
+		USB2_HST_PHY_SYST_REQ | USB2_OTG_PHY_SYST_REQ);
+	udelay(200);
+
+	/* reset usb2 phy por/utmi */
+	setbits_le32(PERI_CRG47, USB2_PHY01_SRST_REQ | USB2_PHY01_SRST_TREQ1);
+	udelay(200);
+
+	/* open usb2 ref clk */
+	setbits_le32(PERI_CRG47, USB2_PHY01_REF_CKEN);
+	udelay(300);
+
+	/* cancel usb2 power on reset */
+	clrbits_le32(PERI_CRG47, USB2_PHY01_SRST_REQ);
+	udelay(500);
+
+	usb2_phy_config();
+
+	/* cancel usb2 port reset, wait comp circuit stable */
+	clrbits_le32(PERI_CRG47, USB2_PHY01_SRST_TREQ1);
+	mdelay(10);
+
+	/* open usb2 controller clk */
+	setbits_le32(PERI_CRG46,
+		USB2_BUS_CKEN | USB2_OHCI48M_CKEN | USB2_OHCI12M_CKEN |
+		USB2_OTG_UTMI_CKEN | USB2_HST_PHY_CKEN | USB2_UTMI0_CKEN);
+	udelay(200);
+
+	/* cancel usb2 control reset */
+	clrbits_le32(PERI_CRG46,
+		USB2_BUS_SRST_REQ | USB2_UTMI0_SRST_REQ |
+		USB2_HST_PHY_SYST_REQ | USB2_OTG_PHY_SYST_REQ);
+	udelay(200);
+}
+
+int board_mmc_init(bd_t *bis)
+{
+	int ret;
+
+	ret = hi6220_dwmci_add_port(0, REG_BASE_MCI, 8);
+	if (ret)
+		printf("mmc init error (%d)\n", ret);
+
+	return ret;
+}
+
+int board_init(void)
+{
+	usb2_phy_init();
+
+	return 0;
+}
+
diff --git a/configs/poplar_defconfig b/configs/poplar_defconfig
new file mode 100644
index 0000000..8f9f40f
--- /dev/null
+++ b/configs/poplar_defconfig
@@ -0,0 +1,26 @@
+CONFIG_ARM=y
+CONFIG_TARGET_POPLAR=y
+CONFIG_IDENT_STRING="poplar"
+CONFIG_DEFAULT_DEVICE_TREE="hi3798cv200-poplar"
+CONFIG_SYS_PROMPT="poplar# "
+CONFIG_DISTRO_DEFAULTS=y
+CONFIG_DISPLAY_CPUINFO=n
+CONFIG_DISPLAY_BOARDINFO=y
+CONFIG_ISO_PARTITION=n
+CONFIG_MMC_DW=y
+CONFIG_MMC_DW_K3=y
+CONFIG_PL011_SERIAL=y
+CONFIG_PSCI_RESET=y
+CONFIG_USB=y
+CONFIG_USB_EHCI=y
+CONFIG_USB_EHCI_HCD=y
+CONFIG_USB_EHCI_GENERIC=y
+CONFIG_USB_STORAGE=y
+CONFIG_NET=y
+# CONFIG_CMD_IMLS is not set
+# CONFIG_DM_GPIO is not set
+CONFIG_LIB_RAND=y
+CONFIG_CMD_UNZIP=y
+CONFIG_CMD_MMC=y
+CONFIG_CMD_USB=y
+
diff --git a/include/configs/poplar.h b/include/configs/poplar.h
new file mode 100644
index 0000000..db208c1
--- /dev/null
+++ b/include/configs/poplar.h
@@ -0,0 +1,86 @@
+/*
+ * (C) Copyright 2017 Linaro
+ *
+ * Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
+ *
+ * Configuration for Poplar 96boards CE. Parts were derived from other ARM
+ * configurations.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef _POPLAR_H_
+#define _POPLAR_H_
+
+#include <linux/sizes.h>
+
+/* DRAM banks */
+#define CONFIG_NR_DRAM_BANKS			4
+
+/* SYS */
+#define CONFIG_SYS_BOOTM_LEN			0x1400000
+#define CONFIG_SYS_INIT_SP_ADDR			0x200000
+#define CONFIG_SYS_LOAD_ADDR			0x800000
+#define CONFIG_SYS_MALLOC_LEN			SZ_32M
+
+/* ATF bl33.bin load address (must match) */
+#define CONFIG_SYS_TEXT_BASE			0x37000000
+
+/* PL010/PL011 */
+#define CONFIG_PL01X_SERIAL
+
+/* USB configuration */
+#define CONFIG_SYS_USB_EHCI_MAX_ROOT_PORTS	3
+#define CONFIG_USB_MAX_CONTROLLER_COUNT		2
+#define CONFIG_SYS_USB_EVENT_POLL
+#define CONFIG_USB_HOST_ETHER
+#define CONFIG_USB_ETHER_ASIX
+
+/* SD/MMC */
+#define CONFIG_BOUNCE_BUFFER
+
+/*****************************************************************************
+ *  Initial environment variables
+ *****************************************************************************/
+
+#define BOOT_TARGET_DEVICES(func) 					\
+					func(USB, usb, 0)		\
+					func(MMC, mmc, 0) 		\
+					func(DHCP, dhcp, na)
+#ifndef CONFIG_SPL_BUILD
+#include <config_distro_defaults.h>
+#include <config_distro_bootcmd.h>
+#endif
+
+#define CONFIG_EXTRA_ENV_SETTINGS					\
+					"loader_mmc_blknum=0x0\0"	\
+					"loader_mmc_nblks=0x780\0"	\
+					"env_mmc_blknum=0xF0000\0"	\
+					"env_mmc_nblks=0x80\0"		\
+					"kernel_addr_r=0x30000000\0"	\
+					"pxefile_addr_r=0x32000000\0"	\
+					"scriptaddr=0x32000000\0"	\
+					"fdt_addr_r=0x32200000\0"	\
+					"ramdisk_addr_r=0x32400000\0"	\
+	BOOTENV
+
+
+/* Command line configuration */
+#define CONFIG_ENV_IS_IN_MMC		1
+#define CONFIG_SYS_MMC_ENV_DEV		0
+#define CONFIG_ENV_OFFSET		0xF0000  /* env_mmc_blknum */
+#define CONFIG_ENV_SIZE			0x10000  /* env_mmc_nblks bytes */
+#define CONFIG_CMD_ENV
+#define CONFIG_FAT_WRITE
+#define CONFIG_ENV_VARS_UBOOT_CONFIG
+
+/* Monitor Command Prompt */
+#define CONFIG_CMDLINE_EDITING
+#define CONFIG_SYS_LONGHELP
+#define CONFIG_SYS_CBSIZE		512
+#define CONFIG_SYS_MAXARGS		64
+#define CONFIG_SYS_PBSIZE		(CONFIG_SYS_CBSIZE + \
+					sizeof(CONFIG_SYS_PROMPT) + 16)
+#define CONFIG_SYS_BARGSIZE		CONFIG_SYS_CBSIZE
+
+#endif /* _POPLAR_H_ */
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-08 16:36 ` [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards Jorge Ramirez-Ortiz
@ 2017-05-08 17:29   ` Tom Rini
  2017-05-08 18:36     ` Jorge Ramirez
  2017-05-09 15:27     ` Jorge Ramirez
  2017-05-25 16:08   ` Andreas Färber
  1 sibling, 2 replies; 53+ messages in thread
From: Tom Rini @ 2017-05-08 17:29 UTC (permalink / raw)
  To: u-boot

On Mon, May 08, 2017 at 06:36:43PM +0200, Jorge Ramirez-Ortiz wrote:

> This port adds support for:
>         1) Serial
>         2) eMMC
>         3) USB
[snip]
>  arch/arm/dts/hi3798cv200.dtsi                      |   3 +
>  arch/arm/dts/poplar-uboot.dtsi                     |  24 +++
[snip]
> diff --git a/arch/arm/dts/hi3798cv200.dtsi b/arch/arm/dts/hi3798cv200.dtsi
> index 75865f8..caa17de 100644
> --- a/arch/arm/dts/hi3798cv200.dtsi
> +++ b/arch/arm/dts/hi3798cv200.dtsi
> @@ -409,3 +409,6 @@
>  		};
>  	};
>  };
> +
> +#include "poplar-uboot.dtsi"

NAK, that's not the mechanism, we have one that will automatically
include the right file.  IF it's needed.

> diff --git a/arch/arm/dts/poplar-uboot.dtsi b/arch/arm/dts/poplar-uboot.dtsi
> new file mode 100644
> index 0000000..8a9668a
> --- /dev/null
> +++ b/arch/arm/dts/poplar-uboot.dtsi
> @@ -0,0 +1,24 @@
> +/*
> + * U-Boot addition to:
> + *  1) initialize the console clock rate.
> + *  2) provide support for the generic-ehci USB driver currently not available
> + *     in the linux kernel (8/May/2017).
> + *
> + * (C) Copyright 2017 Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
> + *
> + * SPDX-License-Identifier:     GPL-2.0+
> + */
> +
> +&soc {
> +	usb2: ehci at 9890000 {
> +		compatible = "generic-ehci";
> +		reg = <0x9890000 0x100>;
> +		status = "okay";
> +	};
> +};
> +
> +&uart0 {
> +	clock = <75000000>;
> +	status = "okay";
> +};

These are NOT U-Boot specific properties, they should be in the generic
board dts file.  Lets wait for Alex to chime in on the status of getting
the dts file / etc merged into Linux before doing v5.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170508/2ce647aa/attachment.sig>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-08 17:29   ` Tom Rini
@ 2017-05-08 18:36     ` Jorge Ramirez
  2017-05-08 21:37       ` Tom Rini
  2017-05-09 15:27     ` Jorge Ramirez
  1 sibling, 1 reply; 53+ messages in thread
From: Jorge Ramirez @ 2017-05-08 18:36 UTC (permalink / raw)
  To: u-boot

On 05/08/2017 07:29 PM, Tom Rini wrote:
> On Mon, May 08, 2017 at 06:36:43PM +0200, Jorge Ramirez-Ortiz wrote:
>
>> This port adds support for:
>>          1) Serial
>>          2) eMMC
>>          3) USB
> [snip]
>>   arch/arm/dts/hi3798cv200.dtsi                      |   3 +
>>   arch/arm/dts/poplar-uboot.dtsi                     |  24 +++
> [snip]
>> diff --git a/arch/arm/dts/hi3798cv200.dtsi b/arch/arm/dts/hi3798cv200.dtsi
>> index 75865f8..caa17de 100644
>> --- a/arch/arm/dts/hi3798cv200.dtsi
>> +++ b/arch/arm/dts/hi3798cv200.dtsi
>> @@ -409,3 +409,6 @@
>>   		};
>>   	};
>>   };
>> +
>> +#include "poplar-uboot.dtsi"
> NAK, that's not the mechanism, we have one that will automatically
> include the right file.  IF it's needed.

yeah I thought so...still what about what is being done for the 
dragonboard? (that is what misled me really)
seems to me that board needs fixing as well...


>
>> diff --git a/arch/arm/dts/poplar-uboot.dtsi b/arch/arm/dts/poplar-uboot.dtsi
>> new file mode 100644
>> index 0000000..8a9668a
>> --- /dev/null
>> +++ b/arch/arm/dts/poplar-uboot.dtsi
>> @@ -0,0 +1,24 @@
>> +/*
>> + * U-Boot addition to:
>> + *  1) initialize the console clock rate.
>> + *  2) provide support for the generic-ehci USB driver currently not available
>> + *     in the linux kernel (8/May/2017).
>> + *
>> + * (C) Copyright 2017 Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
>> + *
>> + * SPDX-License-Identifier:     GPL-2.0+
>> + */
>> +
>> +&soc {
>> +	usb2: ehci at 9890000 {
>> +		compatible = "generic-ehci";
>> +		reg = <0x9890000 0x100>;
>> +		status = "okay";
>> +	};
>> +};
>> +
>> +&uart0 {
>> +	clock = <75000000>;
>> +	status = "okay";
>> +};
> These are NOT U-Boot specific properties, they should be in the generic
> board dts file.


why did you ask me to remove them from the generic board dts file?


> Lets wait for Alex to chime in on the status of getting
> the dts file / etc merged into Linux before doing v5.  Thanks!
>
sure, no pb. thanks for the quick responses :)

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-08 18:36     ` Jorge Ramirez
@ 2017-05-08 21:37       ` Tom Rini
  0 siblings, 0 replies; 53+ messages in thread
From: Tom Rini @ 2017-05-08 21:37 UTC (permalink / raw)
  To: u-boot

On Mon, May 08, 2017 at 08:36:28PM +0200, Jorge Ramirez wrote:
> On 05/08/2017 07:29 PM, Tom Rini wrote:
> >On Mon, May 08, 2017 at 06:36:43PM +0200, Jorge Ramirez-Ortiz wrote:
> >
> >>This port adds support for:
> >>         1) Serial
> >>         2) eMMC
> >>         3) USB
> >[snip]
> >>  arch/arm/dts/hi3798cv200.dtsi                      |   3 +
> >>  arch/arm/dts/poplar-uboot.dtsi                     |  24 +++
> >[snip]
> >>diff --git a/arch/arm/dts/hi3798cv200.dtsi b/arch/arm/dts/hi3798cv200.dtsi
> >>index 75865f8..caa17de 100644
> >>--- a/arch/arm/dts/hi3798cv200.dtsi
> >>+++ b/arch/arm/dts/hi3798cv200.dtsi
> >>@@ -409,3 +409,6 @@
> >>  		};
> >>  	};
> >>  };
> >>+
> >>+#include "poplar-uboot.dtsi"
> >NAK, that's not the mechanism, we have one that will automatically
> >include the right file.  IF it's needed.
> 
> yeah I thought so...still what about what is being done for the
> dragonboard? (that is what misled me really)
> seems to me that board needs fixing as well...

Yes, patches welcome ;)  Seriously tho, yes, dragonboard is a bad
example here, along with some exynos and broadcom parts, and should be
corrected.  I'd appreciate a patch there.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170508/dae76b0f/attachment.sig>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-08 17:29   ` Tom Rini
  2017-05-08 18:36     ` Jorge Ramirez
@ 2017-05-09 15:27     ` Jorge Ramirez
  2017-05-10  2:30       ` Tom Rini
  1 sibling, 1 reply; 53+ messages in thread
From: Jorge Ramirez @ 2017-05-09 15:27 UTC (permalink / raw)
  To: u-boot

On 05/08/2017 07:29 PM, Tom Rini wrote:
> On Mon, May 08, 2017 at 06:36:43PM +0200, Jorge Ramirez-Ortiz wrote:
>
>> This port adds support for:
>>          1) Serial
>>          2) eMMC
>>          3) USB
> [snip]
>>   arch/arm/dts/hi3798cv200.dtsi                      |   3 +
>>   arch/arm/dts/poplar-uboot.dtsi                     |  24 +++
> [snip]
>> diff --git a/arch/arm/dts/hi3798cv200.dtsi b/arch/arm/dts/hi3798cv200.dtsi
>> index 75865f8..caa17de 100644
>> --- a/arch/arm/dts/hi3798cv200.dtsi
>> +++ b/arch/arm/dts/hi3798cv200.dtsi
>> @@ -409,3 +409,6 @@
>>   		};
>>   	};
>>   };
>> +
>> +#include "poplar-uboot.dtsi"
> NAK, that's not the mechanism, we have one that will automatically
> include the right file.  IF it's needed.
>
>> diff --git a/arch/arm/dts/poplar-uboot.dtsi b/arch/arm/dts/poplar-uboot.dtsi
>> new file mode 100644
>> index 0000000..8a9668a
>> --- /dev/null
>> +++ b/arch/arm/dts/poplar-uboot.dtsi
>> @@ -0,0 +1,24 @@
>> +/*
>> + * U-Boot addition to:
>> + *  1) initialize the console clock rate.
>> + *  2) provide support for the generic-ehci USB driver currently not available
>> + *     in the linux kernel (8/May/2017).
>> + *
>> + * (C) Copyright 2017 Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
>> + *
>> + * SPDX-License-Identifier:     GPL-2.0+
>> + */
>> +
>> +&soc {
>> +	usb2: ehci at 9890000 {
>> +		compatible = "generic-ehci";
>> +		reg = <0x9890000 0x100>;
>> +		status = "okay";
>> +	};
>> +};
>> +
>> +&uart0 {
>> +	clock = <75000000>;
>> +	status = "okay";
>> +};
> These are NOT U-Boot specific properties, they should be in the generic
> board dts file.  Lets wait for Alex to chime in on the status of getting
> the dts file / etc merged into Linux before doing v5.  Thanks!
>

hey Tom, I am not sure how to move this forward really so let me clarify 
where I think we stand:

1. The linux kernel does not need the clock property in the uart nodes 
(only u-boot does: serial_pl01x.c needs fixing).
2. ehci is not present in the linux kernel poplar dts yet but it will be 
eventually.

with this in mind, what is blocking the acceptance of the patchset?

I can do v5 using the linux kernel dts as is and creating a 
hi3798cv200-u-boot.dtsi that simply adds the nodes above (this time no 
#include required :) )

Then when ehci is added to the kernel, the ehci node can be removed from 
u-boot.dtsi
And when uboot updates its pl01x.c serial driver,  the uart0 node can be 
removed and the u-boot.dtsi filed completely deleted.

would this be acceptable? if not, what would you see us doing?

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-09 15:27     ` Jorge Ramirez
@ 2017-05-10  2:30       ` Tom Rini
  2017-05-10  9:33         ` Jorge Ramirez
  0 siblings, 1 reply; 53+ messages in thread
From: Tom Rini @ 2017-05-10  2:30 UTC (permalink / raw)
  To: u-boot

On Tue, May 09, 2017 at 05:27:12PM +0200, Jorge Ramirez wrote:
> On 05/08/2017 07:29 PM, Tom Rini wrote:
> >On Mon, May 08, 2017 at 06:36:43PM +0200, Jorge Ramirez-Ortiz wrote:
> >
> >>This port adds support for:
> >>         1) Serial
> >>         2) eMMC
> >>         3) USB
> >[snip]
> >>  arch/arm/dts/hi3798cv200.dtsi                      |   3 +
> >>  arch/arm/dts/poplar-uboot.dtsi                     |  24 +++
> >[snip]
> >>diff --git a/arch/arm/dts/hi3798cv200.dtsi b/arch/arm/dts/hi3798cv200.dtsi
> >>index 75865f8..caa17de 100644
> >>--- a/arch/arm/dts/hi3798cv200.dtsi
> >>+++ b/arch/arm/dts/hi3798cv200.dtsi
> >>@@ -409,3 +409,6 @@
> >>  		};
> >>  	};
> >>  };
> >>+
> >>+#include "poplar-uboot.dtsi"
> >NAK, that's not the mechanism, we have one that will automatically
> >include the right file.  IF it's needed.
> >
> >>diff --git a/arch/arm/dts/poplar-uboot.dtsi b/arch/arm/dts/poplar-uboot.dtsi
> >>new file mode 100644
> >>index 0000000..8a9668a
> >>--- /dev/null
> >>+++ b/arch/arm/dts/poplar-uboot.dtsi
> >>@@ -0,0 +1,24 @@
> >>+/*
> >>+ * U-Boot addition to:
> >>+ *  1) initialize the console clock rate.
> >>+ *  2) provide support for the generic-ehci USB driver currently not available
> >>+ *     in the linux kernel (8/May/2017).
> >>+ *
> >>+ * (C) Copyright 2017 Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
> >>+ *
> >>+ * SPDX-License-Identifier:     GPL-2.0+
> >>+ */
> >>+
> >>+&soc {
> >>+	usb2: ehci at 9890000 {
> >>+		compatible = "generic-ehci";
> >>+		reg = <0x9890000 0x100>;
> >>+		status = "okay";
> >>+	};
> >>+};
> >>+
> >>+&uart0 {
> >>+	clock = <75000000>;
> >>+	status = "okay";
> >>+};
> >These are NOT U-Boot specific properties, they should be in the generic
> >board dts file.  Lets wait for Alex to chime in on the status of getting
> >the dts file / etc merged into Linux before doing v5.  Thanks!
> >
> 
> hey Tom, I am not sure how to move this forward really so let me
> clarify where I think we stand:
> 
> 1. The linux kernel does not need the clock property in the uart
> nodes (only u-boot does: serial_pl01x.c needs fixing).
> 2. ehci is not present in the linux kernel poplar dts yet but it
> will be eventually.
> 
> with this in mind, what is blocking the acceptance of the patchset?
> 
> I can do v5 using the linux kernel dts as is and creating a
> hi3798cv200-u-boot.dtsi that simply adds the nodes above (this time
> no #include required :) )
> 
> Then when ehci is added to the kernel, the ehci node can be removed
> from u-boot.dtsi
> And when uboot updates its pl01x.c serial driver,  the uart0 node
> can be removed and the u-boot.dtsi filed completely deleted.

Can you take a stab at updating the pl01x driver?  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170509/34a88d09/attachment.sig>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-10  2:30       ` Tom Rini
@ 2017-05-10  9:33         ` Jorge Ramirez
  2017-05-10 14:49           ` Tom Rini
  0 siblings, 1 reply; 53+ messages in thread
From: Jorge Ramirez @ 2017-05-10  9:33 UTC (permalink / raw)
  To: u-boot

On 05/10/2017 04:30 AM, Tom Rini wrote:
>> hey Tom, I am not sure how to move this forward really so let me
>> clarify where I think we stand:
>>
>> 1. The linux kernel does not need the clock property in the uart
>> nodes (only u-boot does: serial_pl01x.c needs fixing).
>> 2. ehci is not present in the linux kernel poplar dts yet but it
>> will be eventually.
>>
>> with this in mind, what is blocking the acceptance of the patchset?
>>
>> I can do v5 using the linux kernel dts as is and creating a
>> hi3798cv200-u-boot.dtsi that simply adds the nodes above (this time
>> no #include required:)  )
>>
>> Then when ehci is added to the kernel, the ehci node can be removed
>> from u-boot.dtsi
>> And when uboot updates its pl01x.c serial driver,  the uart0 node
>> can be removed and the u-boot.dtsi filed completely deleted.
> Can you take a stab at updating the pl01x driver?  Thanks!

updating pl01x is not a big deal I dont think; however this will mean 
requiring a platform specific clock driver to just use the pl01x - which 
will take me some time to get into uboot for my platform (and the same 
might happen to other users).

- if the issue for accepting the Poplar patchset is with the dts 
modification I can revert to just not using OF for pl01x like other 
platforms do.
- if the issue is that we wish to enforce each new platform to commit a 
clock driver then I just need to plan for it (to be honest I didn't 
envision this to be the case when all I need is to enable a uart)

maintaining full compatibility with the kernel's dts comes at a price so 
just making sure that this is the direction we want to take (not just a 
one off where I am the lucky one :) )

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-10  9:33         ` Jorge Ramirez
@ 2017-05-10 14:49           ` Tom Rini
  2017-05-10 16:33             ` Rob Herring
  0 siblings, 1 reply; 53+ messages in thread
From: Tom Rini @ 2017-05-10 14:49 UTC (permalink / raw)
  To: u-boot

On Wed, May 10, 2017 at 11:33:35AM +0200, Jorge Ramirez wrote:
> On 05/10/2017 04:30 AM, Tom Rini wrote:
> >>hey Tom, I am not sure how to move this forward really so let me
> >>clarify where I think we stand:
> >>
> >>1. The linux kernel does not need the clock property in the uart
> >>nodes (only u-boot does: serial_pl01x.c needs fixing).
> >>2. ehci is not present in the linux kernel poplar dts yet but it
> >>will be eventually.
> >>
> >>with this in mind, what is blocking the acceptance of the patchset?
> >>
> >>I can do v5 using the linux kernel dts as is and creating a
> >>hi3798cv200-u-boot.dtsi that simply adds the nodes above (this time
> >>no #include required:)  )
> >>
> >>Then when ehci is added to the kernel, the ehci node can be removed
> >>from u-boot.dtsi
> >>And when uboot updates its pl01x.c serial driver,  the uart0 node
> >>can be removed and the u-boot.dtsi filed completely deleted.
> >Can you take a stab at updating the pl01x driver?  Thanks!
> 
> updating pl01x is not a big deal I dont think; however this will
> mean requiring a platform specific clock driver to just use the
> pl01x - which will take me some time to get into uboot for my
> platform (and the same might happen to other users).

Ah right.  So the flip side here, is one not allowed to have the clock
property anymore?  Yes, it may not be used in the kernel, but has
someone argued that it's not part of the hardware description?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170510/e46a6043/attachment.sig>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-10 14:49           ` Tom Rini
@ 2017-05-10 16:33             ` Rob Herring
  2017-05-10 17:45               ` Tom Rini
  2017-05-10 17:49               ` Jorge Ramirez
  0 siblings, 2 replies; 53+ messages in thread
From: Rob Herring @ 2017-05-10 16:33 UTC (permalink / raw)
  To: u-boot

On Wed, May 10, 2017 at 9:49 AM, Tom Rini <trini@konsulko.com> wrote:
> On Wed, May 10, 2017 at 11:33:35AM +0200, Jorge Ramirez wrote:
>> On 05/10/2017 04:30 AM, Tom Rini wrote:
>> >>hey Tom, I am not sure how to move this forward really so let me
>> >>clarify where I think we stand:
>> >>
>> >>1. The linux kernel does not need the clock property in the uart
>> >>nodes (only u-boot does: serial_pl01x.c needs fixing).
>> >>2. ehci is not present in the linux kernel poplar dts yet but it
>> >>will be eventually.
>> >>
>> >>with this in mind, what is blocking the acceptance of the patchset?
>> >>
>> >>I can do v5 using the linux kernel dts as is and creating a
>> >>hi3798cv200-u-boot.dtsi that simply adds the nodes above (this time
>> >>no #include required:)  )
>> >>
>> >>Then when ehci is added to the kernel, the ehci node can be removed
>> >>from u-boot.dtsi
>> >>And when uboot updates its pl01x.c serial driver,  the uart0 node
>> >>can be removed and the u-boot.dtsi filed completely deleted.
>> >Can you take a stab at updating the pl01x driver?  Thanks!
>>
>> updating pl01x is not a big deal I dont think; however this will
>> mean requiring a platform specific clock driver to just use the
>> pl01x - which will take me some time to get into uboot for my
>> platform (and the same might happen to other users).
>
> Ah right.  So the flip side here, is one not allowed to have the clock
> property anymore?  Yes, it may not be used in the kernel, but has
> someone argued that it's not part of the hardware description?

First I've ever seen a "clock" property. We have "clocks" from the
clock binding which is a phandle plus #clock-cells number of args. We
also have "clock-frequency" which is just the frequency value and has
been around forever. Why u-boot went off and did something different i
don't know. Sigh. What I can say is a 3rd way is not going to be
accepted.

Generally, the clock binding replaces clock-frequency, but there are
some cases where clock binding would be overkill or too complicated
for early boot and using clock-frequency would be okay. But I don't
think "I haven't written my platform clock controller driver yet" is a
reason to use clock-frequency as generally most platforms are going to
have to have one. Providing a stub driver that just returns a fixed
frequency shouldn't be too hard IMO.

Rob

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-10 16:33             ` Rob Herring
@ 2017-05-10 17:45               ` Tom Rini
  2017-05-10 18:09                 ` Simon Glass
  2017-05-10 19:09                 ` Rob Herring
  2017-05-10 17:49               ` Jorge Ramirez
  1 sibling, 2 replies; 53+ messages in thread
From: Tom Rini @ 2017-05-10 17:45 UTC (permalink / raw)
  To: u-boot

On Wed, May 10, 2017 at 11:33:12AM -0500, Rob Herring wrote:
> On Wed, May 10, 2017 at 9:49 AM, Tom Rini <trini@konsulko.com> wrote:
> > On Wed, May 10, 2017 at 11:33:35AM +0200, Jorge Ramirez wrote:
> >> On 05/10/2017 04:30 AM, Tom Rini wrote:
> >> >>hey Tom, I am not sure how to move this forward really so let me
> >> >>clarify where I think we stand:
> >> >>
> >> >>1. The linux kernel does not need the clock property in the uart
> >> >>nodes (only u-boot does: serial_pl01x.c needs fixing).
> >> >>2. ehci is not present in the linux kernel poplar dts yet but it
> >> >>will be eventually.
> >> >>
> >> >>with this in mind, what is blocking the acceptance of the patchset?
> >> >>
> >> >>I can do v5 using the linux kernel dts as is and creating a
> >> >>hi3798cv200-u-boot.dtsi that simply adds the nodes above (this time
> >> >>no #include required:)  )
> >> >>
> >> >>Then when ehci is added to the kernel, the ehci node can be removed
> >> >>from u-boot.dtsi
> >> >>And when uboot updates its pl01x.c serial driver,  the uart0 node
> >> >>can be removed and the u-boot.dtsi filed completely deleted.
> >> >Can you take a stab at updating the pl01x driver?  Thanks!
> >>
> >> updating pl01x is not a big deal I dont think; however this will
> >> mean requiring a platform specific clock driver to just use the
> >> pl01x - which will take me some time to get into uboot for my
> >> platform (and the same might happen to other users).
> >
> > Ah right.  So the flip side here, is one not allowed to have the clock
> > property anymore?  Yes, it may not be used in the kernel, but has
> > someone argued that it's not part of the hardware description?
> 
> First I've ever seen a "clock" property. We have "clocks" from the
> clock binding which is a phandle plus #clock-cells number of args. We
> also have "clock-frequency" which is just the frequency value and has
> been around forever. Why u-boot went off and did something different i
> don't know. Sigh. What I can say is a 3rd way is not going to be
> accepted.

Aw crap, I'm in the wrong.  I was thinking this was "clock-frequency"
and not that we had invented our own property here.

> Generally, the clock binding replaces clock-frequency, but there are
> some cases where clock binding would be overkill or too complicated
> for early boot and using clock-frequency would be okay. But I don't
> think "I haven't written my platform clock controller driver yet" is a
> reason to use clock-frequency as generally most platforms are going to
> have to have one. Providing a stub driver that just returns a fixed
> frequency shouldn't be too hard IMO.

So, trying to dig out of the hole we made here.  The generic serial
binding (bindings/serial/serial.txt) documents clock-frequency.  The
pl011 binding (and primecell which it also references) does not.  Would
adding clock-frequency to a pl011 node be valid or invalid?  If valid,
would it also be acceptable to include in dts files that also fill in
clocks/clock-names from that binding?  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170510/5df0e0d0/attachment.sig>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-10 16:33             ` Rob Herring
  2017-05-10 17:45               ` Tom Rini
@ 2017-05-10 17:49               ` Jorge Ramirez
  2017-05-10 18:21                 ` Rob Herring
  1 sibling, 1 reply; 53+ messages in thread
From: Jorge Ramirez @ 2017-05-10 17:49 UTC (permalink / raw)
  To: u-boot

On 05/10/2017 06:33 PM, Rob Herring wrote:
> On Wed, May 10, 2017 at 9:49 AM, Tom Rini <trini@konsulko.com> wrote:
>> On Wed, May 10, 2017 at 11:33:35AM +0200, Jorge Ramirez wrote:
>>> On 05/10/2017 04:30 AM, Tom Rini wrote:
>>>>> hey Tom, I am not sure how to move this forward really so let me
>>>>> clarify where I think we stand:
>>>>>
>>>>> 1. The linux kernel does not need the clock property in the uart
>>>>> nodes (only u-boot does: serial_pl01x.c needs fixing).
>>>>> 2. ehci is not present in the linux kernel poplar dts yet but it
>>>>> will be eventually.
>>>>>
>>>>> with this in mind, what is blocking the acceptance of the patchset?
>>>>>
>>>>> I can do v5 using the linux kernel dts as is and creating a
>>>>> hi3798cv200-u-boot.dtsi that simply adds the nodes above (this time
>>>>> no #include required:)  )
>>>>>
>>>>> Then when ehci is added to the kernel, the ehci node can be removed
>>>> >from u-boot.dtsi
>>>>> And when uboot updates its pl01x.c serial driver,  the uart0 node
>>>>> can be removed and the u-boot.dtsi filed completely deleted.
>>>> Can you take a stab at updating the pl01x driver?  Thanks!
>>> updating pl01x is not a big deal I dont think; however this will
>>> mean requiring a platform specific clock driver to just use the
>>> pl01x - which will take me some time to get into uboot for my
>>> platform (and the same might happen to other users).
>> Ah right.  So the flip side here, is one not allowed to have the clock
>> property anymore?  Yes, it may not be used in the kernel, but has
>> someone argued that it's not part of the hardware description?
> First I've ever seen a "clock" property. We have "clocks" from the
> clock binding which is a phandle plus #clock-cells number of args. We
> also have "clock-frequency" which is just the frequency value and has
> been around forever. Why u-boot went off and did something different i
> don't know. Sigh. What I can say is a 3rd way is not going to be
> accepted.
>
> Generally, the clock binding replaces clock-frequency, but there are
> some cases where clock binding would be overkill or too complicated
> for early boot and using clock-frequency would be okay.

agreed

> But I don't
> think "I haven't written my platform clock controller driver yet" is a
> reason to use clock-frequency as generally most platforms are going to
> have to have one. Providing a stub driver that just returns a fixed
> frequency shouldn't be too hard IMO.

I also agree but please do notice that this was not quite what I was saying.
what I am saying is that writing a stub driver to only provide a single 
UART clock rate and nothing else is also an overkill (this platform has 
no need for other clocks in u-boot)

Incidentally the value that I need to retrieve is itself hard-coded in 
an array in the kernel source code and set up via 
clk_register_fixed_rate instead of using a fixed-clock node in the 
device tree.
So there is not much value that I can see in providing such a stub 
driver really...



>
> Rob

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-10 17:45               ` Tom Rini
@ 2017-05-10 18:09                 ` Simon Glass
  2017-05-10 19:09                 ` Rob Herring
  1 sibling, 0 replies; 53+ messages in thread
From: Simon Glass @ 2017-05-10 18:09 UTC (permalink / raw)
  To: u-boot

Hi,

On 10 May 2017 at 11:45, Tom Rini <trini@konsulko.com> wrote:
> On Wed, May 10, 2017 at 11:33:12AM -0500, Rob Herring wrote:
>> On Wed, May 10, 2017 at 9:49 AM, Tom Rini <trini@konsulko.com> wrote:
>> > On Wed, May 10, 2017 at 11:33:35AM +0200, Jorge Ramirez wrote:
>> >> On 05/10/2017 04:30 AM, Tom Rini wrote:
>> >> >>hey Tom, I am not sure how to move this forward really so let me
>> >> >>clarify where I think we stand:
>> >> >>
>> >> >>1. The linux kernel does not need the clock property in the uart
>> >> >>nodes (only u-boot does: serial_pl01x.c needs fixing).
>> >> >>2. ehci is not present in the linux kernel poplar dts yet but it
>> >> >>will be eventually.
>> >> >>
>> >> >>with this in mind, what is blocking the acceptance of the patchset?
>> >> >>
>> >> >>I can do v5 using the linux kernel dts as is and creating a
>> >> >>hi3798cv200-u-boot.dtsi that simply adds the nodes above (this time
>> >> >>no #include required:)  )
>> >> >>
>> >> >>Then when ehci is added to the kernel, the ehci node can be removed
>> >> >>from u-boot.dtsi
>> >> >>And when uboot updates its pl01x.c serial driver,  the uart0 node
>> >> >>can be removed and the u-boot.dtsi filed completely deleted.
>> >> >Can you take a stab at updating the pl01x driver?  Thanks!
>> >>
>> >> updating pl01x is not a big deal I dont think; however this will
>> >> mean requiring a platform specific clock driver to just use the
>> >> pl01x - which will take me some time to get into uboot for my
>> >> platform (and the same might happen to other users).
>> >
>> > Ah right.  So the flip side here, is one not allowed to have the clock
>> > property anymore?  Yes, it may not be used in the kernel, but has
>> > someone argued that it's not part of the hardware description?
>>
>> First I've ever seen a "clock" property. We have "clocks" from the
>> clock binding which is a phandle plus #clock-cells number of args. We
>> also have "clock-frequency" which is just the frequency value and has
>> been around forever. Why u-boot went off and did something different i
>> don't know. Sigh. What I can say is a 3rd way is not going to be
>> accepted.
>
> Aw crap, I'm in the wrong.  I was thinking this was "clock-frequency"
> and not that we had invented our own property here.
>
>> Generally, the clock binding replaces clock-frequency, but there are
>> some cases where clock binding would be overkill or too complicated
>> for early boot and using clock-frequency would be okay. But I don't
>> think "I haven't written my platform clock controller driver yet" is a
>> reason to use clock-frequency as generally most platforms are going to
>> have to have one. Providing a stub driver that just returns a fixed
>> frequency shouldn't be too hard IMO.
>
> So, trying to dig out of the hole we made here.  The generic serial
> binding (bindings/serial/serial.txt) documents clock-frequency.  The
> pl011 binding (and primecell which it also references) does not.  Would
> adding clock-frequency to a pl011 node be valid or invalid?  If valid,
> would it also be acceptable to include in dts files that also fill in
> clocks/clock-names from that binding?  Thanks!

clock-frequency should be OK and supported for boards which don't yet
have a clock driver. I don't think we need to explicitly update the
pl011 binding though.

Regards,
Simon

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-10 17:49               ` Jorge Ramirez
@ 2017-05-10 18:21                 ` Rob Herring
  2017-05-10 18:37                   ` Jorge Ramirez
  0 siblings, 1 reply; 53+ messages in thread
From: Rob Herring @ 2017-05-10 18:21 UTC (permalink / raw)
  To: u-boot

On Wed, May 10, 2017 at 12:49 PM, Jorge Ramirez
<jorge.ramirez-ortiz@linaro.org> wrote:
> On 05/10/2017 06:33 PM, Rob Herring wrote:
>>
>> On Wed, May 10, 2017 at 9:49 AM, Tom Rini <trini@konsulko.com> wrote:
>>>
>>> On Wed, May 10, 2017 at 11:33:35AM +0200, Jorge Ramirez wrote:
>>>>
>>>> On 05/10/2017 04:30 AM, Tom Rini wrote:
>>>>>>
>>>>>> hey Tom, I am not sure how to move this forward really so let me
>>>>>> clarify where I think we stand:
>>>>>>
>>>>>> 1. The linux kernel does not need the clock property in the uart
>>>>>> nodes (only u-boot does: serial_pl01x.c needs fixing).
>>>>>> 2. ehci is not present in the linux kernel poplar dts yet but it
>>>>>> will be eventually.
>>>>>>
>>>>>> with this in mind, what is blocking the acceptance of the patchset?
>>>>>>
>>>>>> I can do v5 using the linux kernel dts as is and creating a
>>>>>> hi3798cv200-u-boot.dtsi that simply adds the nodes above (this time
>>>>>> no #include required:)  )
>>>>>>
>>>>>> Then when ehci is added to the kernel, the ehci node can be removed
>>>>>
>>>>> >from u-boot.dtsi
>>>>>>
>>>>>> And when uboot updates its pl01x.c serial driver,  the uart0 node
>>>>>> can be removed and the u-boot.dtsi filed completely deleted.
>>>>>
>>>>> Can you take a stab at updating the pl01x driver?  Thanks!
>>>>
>>>> updating pl01x is not a big deal I dont think; however this will
>>>> mean requiring a platform specific clock driver to just use the
>>>> pl01x - which will take me some time to get into uboot for my
>>>> platform (and the same might happen to other users).
>>>
>>> Ah right.  So the flip side here, is one not allowed to have the clock
>>> property anymore?  Yes, it may not be used in the kernel, but has
>>> someone argued that it's not part of the hardware description?
>>
>> First I've ever seen a "clock" property. We have "clocks" from the
>> clock binding which is a phandle plus #clock-cells number of args. We
>> also have "clock-frequency" which is just the frequency value and has
>> been around forever. Why u-boot went off and did something different i
>> don't know. Sigh. What I can say is a 3rd way is not going to be
>> accepted.
>>
>> Generally, the clock binding replaces clock-frequency, but there are
>> some cases where clock binding would be overkill or too complicated
>> for early boot and using clock-frequency would be okay.
>
>
> agreed
>
>> But I don't
>> think "I haven't written my platform clock controller driver yet" is a
>> reason to use clock-frequency as generally most platforms are going to
>> have to have one. Providing a stub driver that just returns a fixed
>> frequency shouldn't be too hard IMO.
>
>
> I also agree but please do notice that this was not quite what I was saying.
> what I am saying is that writing a stub driver to only provide a single UART
> clock rate and nothing else is also an overkill (this platform has no need
> for other clocks in u-boot)

Sorry, I find that hard to believe. There's no SD card, display, SPI,
I2C? Those all need clocks at some point.

> Incidentally the value that I need to retrieve is itself hard-coded in an
> array in the kernel source code and set up via clk_register_fixed_rate
> instead of using a fixed-clock node in the device tree.
> So there is not much value that I can see in providing such a stub driver
> really...

If you don't need clock properties in Linux, then you shouldn't need
them in u-boot either. But that's not what I see in the dts under
review:

+               uart0: serial at 8b00000 {
+                       compatible = "arm,pl011", "arm,primecell";
+                       reg = <0x8b00000 0x1000>;
+                       interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>;
+                       clocks = <&sysctrl HISTB_UART0_CLK>;
+                       clock-names = "apb_pclk";
+                       status = "disabled";
+               };
+
+               uart2: serial at 8b02000 {
+                       compatible = "arm,pl011", "arm,primecell";
+                       reg = <0x8b02000 0x1000>;
+                       interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH>;
+                       clocks = <&crg HISTB_UART2_CLK>;
+                       clock-names = "apb_pclk";
+                       status = "disabled";
+               };
+

Which BTW is also wrong as a single clock is deprecated. There should
be 2 clocks.

Rob

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-10 18:21                 ` Rob Herring
@ 2017-05-10 18:37                   ` Jorge Ramirez
  0 siblings, 0 replies; 53+ messages in thread
From: Jorge Ramirez @ 2017-05-10 18:37 UTC (permalink / raw)
  To: u-boot

On 05/10/2017 08:21 PM, Rob Herring wrote:
> On Wed, May 10, 2017 at 12:49 PM, Jorge Ramirez
> <jorge.ramirez-ortiz@linaro.org> wrote:
>> On 05/10/2017 06:33 PM, Rob Herring wrote:
>>> On Wed, May 10, 2017 at 9:49 AM, Tom Rini <trini@konsulko.com> wrote:
>>>> On Wed, May 10, 2017 at 11:33:35AM +0200, Jorge Ramirez wrote:
>>>>> On 05/10/2017 04:30 AM, Tom Rini wrote:
>>>>>>> hey Tom, I am not sure how to move this forward really so let me
>>>>>>> clarify where I think we stand:
>>>>>>>
>>>>>>> 1. The linux kernel does not need the clock property in the uart
>>>>>>> nodes (only u-boot does: serial_pl01x.c needs fixing).
>>>>>>> 2. ehci is not present in the linux kernel poplar dts yet but it
>>>>>>> will be eventually.
>>>>>>>
>>>>>>> with this in mind, what is blocking the acceptance of the patchset?
>>>>>>>
>>>>>>> I can do v5 using the linux kernel dts as is and creating a
>>>>>>> hi3798cv200-u-boot.dtsi that simply adds the nodes above (this time
>>>>>>> no #include required:)  )
>>>>>>>
>>>>>>> Then when ehci is added to the kernel, the ehci node can be removed
>>>>>> >from u-boot.dtsi
>>>>>>> And when uboot updates its pl01x.c serial driver,  the uart0 node
>>>>>>> can be removed and the u-boot.dtsi filed completely deleted.
>>>>>> Can you take a stab at updating the pl01x driver?  Thanks!
>>>>> updating pl01x is not a big deal I dont think; however this will
>>>>> mean requiring a platform specific clock driver to just use the
>>>>> pl01x - which will take me some time to get into uboot for my
>>>>> platform (and the same might happen to other users).
>>>> Ah right.  So the flip side here, is one not allowed to have the clock
>>>> property anymore?  Yes, it may not be used in the kernel, but has
>>>> someone argued that it's not part of the hardware description?
>>> First I've ever seen a "clock" property. We have "clocks" from the
>>> clock binding which is a phandle plus #clock-cells number of args. We
>>> also have "clock-frequency" which is just the frequency value and has
>>> been around forever. Why u-boot went off and did something different i
>>> don't know. Sigh. What I can say is a 3rd way is not going to be
>>> accepted.
>>>
>>> Generally, the clock binding replaces clock-frequency, but there are
>>> some cases where clock binding would be overkill or too complicated
>>> for early boot and using clock-frequency would be okay.
>>
>> agreed
>>
>>> But I don't
>>> think "I haven't written my platform clock controller driver yet" is a
>>> reason to use clock-frequency as generally most platforms are going to
>>> have to have one. Providing a stub driver that just returns a fixed
>>> frequency shouldn't be too hard IMO.
>>
>> I also agree but please do notice that this was not quite what I was saying.
>> what I am saying is that writing a stub driver to only provide a single UART
>> clock rate and nothing else is also an overkill (this platform has no need
>> for other clocks in u-boot)
> Sorry, I find that hard to believe. There's no SD card, display, SPI,
> I2C? Those all need clocks at some point.

No, the BOOT ROM takes care of them in this platform (pinux, clocks, DDR 
initialization, etc).
in uboot we are using the console (for which we only need the 
clock-frequency), eMMC and USB ehci (for storage and networking) and 
there are no clocks that need to be set in BL33 (uboot runs as BL33)


>
>> Incidentally the value that I need to retrieve is itself hard-coded in an
>> array in the kernel source code and set up via clk_register_fixed_rate
>> instead of using a fixed-clock node in the device tree.
>> So there is not much value that I can see in providing such a stub driver
>> really...
> If you don't need clock properties in Linux, then you shouldn't need
> them in u-boot either. But that's not what I see in the dts under
> review:

sorry I dont understand your point. is this a question about uboot?

>
> +               uart0: serial at 8b00000 {
> +                       compatible = "arm,pl011", "arm,primecell";
> +                       reg = <0x8b00000 0x1000>;
> +                       interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>;
> +                       clocks = <&sysctrl HISTB_UART0_CLK>;
> +                       clock-names = "apb_pclk";
> +                       status = "disabled";
> +               };
> +
> +               uart2: serial at 8b02000 {
> +                       compatible = "arm,pl011", "arm,primecell";
> +                       reg = <0x8b02000 0x1000>;
> +                       interrupts = <GIC_SPI 51 IRQ_TYPE_LEVEL_HIGH>;
> +                       clocks = <&crg HISTB_UART2_CLK>;
> +                       clock-names = "apb_pclk";
> +                       status = "disabled";
> +               };
> +
>
> Which BTW is also wrong as a single clock is deprecated. There should
> be 2 clocks.

yes I noticed that as well while reading the bindings
>
> Rob

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-10 17:45               ` Tom Rini
  2017-05-10 18:09                 ` Simon Glass
@ 2017-05-10 19:09                 ` Rob Herring
  2017-05-10 19:42                   ` Simon Glass
  1 sibling, 1 reply; 53+ messages in thread
From: Rob Herring @ 2017-05-10 19:09 UTC (permalink / raw)
  To: u-boot

On Wed, May 10, 2017 at 12:45 PM, Tom Rini <trini@konsulko.com> wrote:
> On Wed, May 10, 2017 at 11:33:12AM -0500, Rob Herring wrote:
>> On Wed, May 10, 2017 at 9:49 AM, Tom Rini <trini@konsulko.com> wrote:
>> > On Wed, May 10, 2017 at 11:33:35AM +0200, Jorge Ramirez wrote:
>> >> On 05/10/2017 04:30 AM, Tom Rini wrote:
>> >> >>hey Tom, I am not sure how to move this forward really so let me
>> >> >>clarify where I think we stand:
>> >> >>
>> >> >>1. The linux kernel does not need the clock property in the uart
>> >> >>nodes (only u-boot does: serial_pl01x.c needs fixing).
>> >> >>2. ehci is not present in the linux kernel poplar dts yet but it
>> >> >>will be eventually.
>> >> >>
>> >> >>with this in mind, what is blocking the acceptance of the patchset?
>> >> >>
>> >> >>I can do v5 using the linux kernel dts as is and creating a
>> >> >>hi3798cv200-u-boot.dtsi that simply adds the nodes above (this time
>> >> >>no #include required:)  )
>> >> >>
>> >> >>Then when ehci is added to the kernel, the ehci node can be removed
>> >> >>from u-boot.dtsi
>> >> >>And when uboot updates its pl01x.c serial driver,  the uart0 node
>> >> >>can be removed and the u-boot.dtsi filed completely deleted.
>> >> >Can you take a stab at updating the pl01x driver?  Thanks!
>> >>
>> >> updating pl01x is not a big deal I dont think; however this will
>> >> mean requiring a platform specific clock driver to just use the
>> >> pl01x - which will take me some time to get into uboot for my
>> >> platform (and the same might happen to other users).
>> >
>> > Ah right.  So the flip side here, is one not allowed to have the clock
>> > property anymore?  Yes, it may not be used in the kernel, but has
>> > someone argued that it's not part of the hardware description?
>>
>> First I've ever seen a "clock" property. We have "clocks" from the
>> clock binding which is a phandle plus #clock-cells number of args. We
>> also have "clock-frequency" which is just the frequency value and has
>> been around forever. Why u-boot went off and did something different i
>> don't know. Sigh. What I can say is a 3rd way is not going to be
>> accepted.
>
> Aw crap, I'm in the wrong.  I was thinking this was "clock-frequency"
> and not that we had invented our own property here.
>
>> Generally, the clock binding replaces clock-frequency, but there are
>> some cases where clock binding would be overkill or too complicated
>> for early boot and using clock-frequency would be okay. But I don't
>> think "I haven't written my platform clock controller driver yet" is a
>> reason to use clock-frequency as generally most platforms are going to
>> have to have one. Providing a stub driver that just returns a fixed
>> frequency shouldn't be too hard IMO.
>
> So, trying to dig out of the hole we made here.  The generic serial
> binding (bindings/serial/serial.txt) documents clock-frequency.  The
> pl011 binding (and primecell which it also references) does not.  Would
> adding clock-frequency to a pl011 node be valid or invalid?

Valid in general. It's a common property in the DT spec. Though, it
should be listed in the pl011 binding doc as used just like we list
reg or interrupts.

>  If valid,
> would it also be acceptable to include in dts files that also fill in
> clocks/clock-names from that binding?

Generally we treat that as not valid as they are mutually exclusive.

There's 2 better options. You can define fixed clocks with
"fixed-clock" compatible and you already have infrastructure in u-boot
to use that. However, the upstream dts file already defines clocks, so
that doesn't really work here. The 2nd option is have a table of clock
ids and frequencies and register that list of clocks based on matching
the clock controller. You'd need a little bit of infrastructure to
support that, but otherwise a platform would just need to define a
table.

Rob

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-10 19:09                 ` Rob Herring
@ 2017-05-10 19:42                   ` Simon Glass
  2017-05-10 21:19                     ` Jorge Ramirez
  2017-05-11 12:35                     ` Tom Rini
  0 siblings, 2 replies; 53+ messages in thread
From: Simon Glass @ 2017-05-10 19:42 UTC (permalink / raw)
  To: u-boot

Hi,

On 10 May 2017 at 13:09, Rob Herring <robh@kernel.org> wrote:
> On Wed, May 10, 2017 at 12:45 PM, Tom Rini <trini@konsulko.com> wrote:
>> On Wed, May 10, 2017 at 11:33:12AM -0500, Rob Herring wrote:
>>> On Wed, May 10, 2017 at 9:49 AM, Tom Rini <trini@konsulko.com> wrote:
>>> > On Wed, May 10, 2017 at 11:33:35AM +0200, Jorge Ramirez wrote:
>>> >> On 05/10/2017 04:30 AM, Tom Rini wrote:
>>> >> >>hey Tom, I am not sure how to move this forward really so let me
>>> >> >>clarify where I think we stand:
>>> >> >>
>>> >> >>1. The linux kernel does not need the clock property in the uart
>>> >> >>nodes (only u-boot does: serial_pl01x.c needs fixing).
>>> >> >>2. ehci is not present in the linux kernel poplar dts yet but it
>>> >> >>will be eventually.
>>> >> >>
>>> >> >>with this in mind, what is blocking the acceptance of the patchset?
>>> >> >>
>>> >> >>I can do v5 using the linux kernel dts as is and creating a
>>> >> >>hi3798cv200-u-boot.dtsi that simply adds the nodes above (this time
>>> >> >>no #include required:)  )
>>> >> >>
>>> >> >>Then when ehci is added to the kernel, the ehci node can be removed
>>> >> >>from u-boot.dtsi
>>> >> >>And when uboot updates its pl01x.c serial driver,  the uart0 node
>>> >> >>can be removed and the u-boot.dtsi filed completely deleted.
>>> >> >Can you take a stab at updating the pl01x driver?  Thanks!
>>> >>
>>> >> updating pl01x is not a big deal I dont think; however this will
>>> >> mean requiring a platform specific clock driver to just use the
>>> >> pl01x - which will take me some time to get into uboot for my
>>> >> platform (and the same might happen to other users).
>>> >
>>> > Ah right.  So the flip side here, is one not allowed to have the clock
>>> > property anymore?  Yes, it may not be used in the kernel, but has
>>> > someone argued that it's not part of the hardware description?
>>>
>>> First I've ever seen a "clock" property. We have "clocks" from the
>>> clock binding which is a phandle plus #clock-cells number of args. We
>>> also have "clock-frequency" which is just the frequency value and has
>>> been around forever. Why u-boot went off and did something different i
>>> don't know. Sigh. What I can say is a 3rd way is not going to be
>>> accepted.
>>
>> Aw crap, I'm in the wrong.  I was thinking this was "clock-frequency"
>> and not that we had invented our own property here.
>>
>>> Generally, the clock binding replaces clock-frequency, but there are
>>> some cases where clock binding would be overkill or too complicated
>>> for early boot and using clock-frequency would be okay. But I don't
>>> think "I haven't written my platform clock controller driver yet" is a
>>> reason to use clock-frequency as generally most platforms are going to
>>> have to have one. Providing a stub driver that just returns a fixed
>>> frequency shouldn't be too hard IMO.
>>
>> So, trying to dig out of the hole we made here.  The generic serial
>> binding (bindings/serial/serial.txt) documents clock-frequency.  The
>> pl011 binding (and primecell which it also references) does not.  Would
>> adding clock-frequency to a pl011 node be valid or invalid?
>
> Valid in general. It's a common property in the DT spec. Though, it
> should be listed in the pl011 binding doc as used just like we list
> reg or interrupts.
>
>>  If valid,
>> would it also be acceptable to include in dts files that also fill in
>> clocks/clock-names from that binding?
>
> Generally we treat that as not valid as they are mutually exclusive.
>
> There's 2 better options. You can define fixed clocks with
> "fixed-clock" compatible and you already have infrastructure in u-boot
> to use that. However, the upstream dts file already defines clocks, so
> that doesn't really work here. The 2nd option is have a table of clock
> ids and frequencies and register that list of clocks based on matching
> the clock controller. You'd need a little bit of infrastructure to
> support that, but otherwise a platform would just need to define a
> table.

It sounds like you are saying the first option isn't an option. The
second option adds another layer of pain - we are trying to avoid
having platform data.

I'd prefer (in this order):

1. A clock driver
2. Use the existing clock-frequency property

Regards,
SImon

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-10 19:42                   ` Simon Glass
@ 2017-05-10 21:19                     ` Jorge Ramirez
  2017-05-10 21:31                       ` Simon Glass
  2017-05-11 12:35                     ` Tom Rini
  1 sibling, 1 reply; 53+ messages in thread
From: Jorge Ramirez @ 2017-05-10 21:19 UTC (permalink / raw)
  To: u-boot

On 05/10/2017 09:42 PM, Simon Glass wrote:
>>>>>> updating pl01x is not a big deal I dont think; however this will
>>>>>> mean requiring a platform specific clock driver to just use the
>>>>>> pl01x - which will take me some time to get into uboot for my
>>>>>> platform (and the same might happen to other users).
>>>>> Ah right.  So the flip side here, is one not allowed to have the clock
>>>>> property anymore?  Yes, it may not be used in the kernel, but has
>>>>> someone argued that it's not part of the hardware description?
>>>> First I've ever seen a "clock" property. We have "clocks" from the
>>>> clock binding which is a phandle plus #clock-cells number of args. We
>>>> also have "clock-frequency" which is just the frequency value and has
>>>> been around forever. Why u-boot went off and did something different i
>>>> don't know. Sigh. What I can say is a 3rd way is not going to be
>>>> accepted.
>>> Aw crap, I'm in the wrong.  I was thinking this was "clock-frequency"
>>> and not that we had invented our own property here.
>>>
>>>> Generally, the clock binding replaces clock-frequency, but there are
>>>> some cases where clock binding would be overkill or too complicated
>>>> for early boot and using clock-frequency would be okay. But I don't
>>>> think "I haven't written my platform clock controller driver yet" is a
>>>> reason to use clock-frequency as generally most platforms are going to
>>>> have to have one. Providing a stub driver that just returns a fixed
>>>> frequency shouldn't be too hard IMO.
>>> So, trying to dig out of the hole we made here.  The generic serial
>>> binding (bindings/serial/serial.txt) documents clock-frequency.  The
>>> pl011 binding (and primecell which it also references) does not.  Would
>>> adding clock-frequency to a pl011 node be valid or invalid?
>> Valid in general. It's a common property in the DT spec. Though, it
>> should be listed in the pl011 binding doc as used just like we list
>> reg or interrupts.
>>
>>>   If valid,
>>> would it also be acceptable to include in dts files that also fill in
>>> clocks/clock-names from that binding?
>> Generally we treat that as not valid as they are mutually exclusive.
>>
>> There's 2 better options. You can define fixed clocks with
>> "fixed-clock" compatible and you already have infrastructure in u-boot
>> to use that. However, the upstream dts file already defines clocks, so
>> that doesn't really work here. The 2nd option is have a table of clock
>> ids and frequencies and register that list of clocks based on matching
>> the clock controller. You'd need a little bit of infrastructure to
>> support that, but otherwise a platform would just need to define a
>> table.
> It sounds like you are saying the first option isn't an option. The
> second option adds another layer of pain - we are trying to avoid
> having platform data.
>
> I'd prefer (in this order):
>
> 1. A clock driver

please could you explain the rationale for this request on this 
particular platform?

And I mean for a platform where a clock driver will:

1. NOT access any hardware
2. *only* provide single hard-coded value (a compiled in frequency) for 
the pl01x driver.

Consider also (another option) that the pl01x driver can be compiled 
without OF support and that said frequency can be provided by CONFIG.

It is just that before adding layers of stub code I would like to 
understand the technical need when there is only one consumer (I don't 
want to pollute U-Boot's code base with code that is not providing value).

What do we want to achieve by writing a SoC clock driver that hard-codes 
the frequency rate for the console?
what is the benefit of having such a driver and why is this not an 
overkill on this platform?


> 2. Use the existing clock-frequency property

yes, this I could understand.
And it doesn't even need to add a single line to the linux kernel dts 
files which would be imported from the linux kernel tree and keep 
unmodified.

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-10 21:19                     ` Jorge Ramirez
@ 2017-05-10 21:31                       ` Simon Glass
  2017-05-11 11:45                         ` Jorge Ramirez
  0 siblings, 1 reply; 53+ messages in thread
From: Simon Glass @ 2017-05-10 21:31 UTC (permalink / raw)
  To: u-boot

Hi,

On 10 May 2017 at 15:19, Jorge Ramirez <jorge.ramirez-ortiz@linaro.org> wrote:
>
> On 05/10/2017 09:42 PM, Simon Glass wrote:
>
> updating pl01x is not a big deal I dont think; however this will
> mean requiring a platform specific clock driver to just use the
> pl01x - which will take me some time to get into uboot for my
> platform (and the same might happen to other users).
>
> Ah right.  So the flip side here, is one not allowed to have the clock
> property anymore?  Yes, it may not be used in the kernel, but has
> someone argued that it's not part of the hardware description?
>
> First I've ever seen a "clock" property. We have "clocks" from the
> clock binding which is a phandle plus #clock-cells number of args. We
> also have "clock-frequency" which is just the frequency value and has
> been around forever. Why u-boot went off and did something different i
> don't know. Sigh. What I can say is a 3rd way is not going to be
> accepted.
>
> Aw crap, I'm in the wrong.  I was thinking this was "clock-frequency"
> and not that we had invented our own property here.
>
> Generally, the clock binding replaces clock-frequency, but there are
> some cases where clock binding would be overkill or too complicated
> for early boot and using clock-frequency would be okay. But I don't
> think "I haven't written my platform clock controller driver yet" is a
> reason to use clock-frequency as generally most platforms are going to
> have to have one. Providing a stub driver that just returns a fixed
> frequency shouldn't be too hard IMO.
>
> So, trying to dig out of the hole we made here.  The generic serial
> binding (bindings/serial/serial.txt) documents clock-frequency.  The
> pl011 binding (and primecell which it also references) does not.  Would
> adding clock-frequency to a pl011 node be valid or invalid?
>
> Valid in general. It's a common property in the DT spec. Though, it
> should be listed in the pl011 binding doc as used just like we list
> reg or interrupts.
>
>  If valid,
> would it also be acceptable to include in dts files that also fill in
> clocks/clock-names from that binding?
>
> Generally we treat that as not valid as they are mutually exclusive.
>
> There's 2 better options. You can define fixed clocks with
> "fixed-clock" compatible and you already have infrastructure in u-boot
> to use that. However, the upstream dts file already defines clocks, so
> that doesn't really work here. The 2nd option is have a table of clock
> ids and frequencies and register that list of clocks based on matching
> the clock controller. You'd need a little bit of infrastructure to
> support that, but otherwise a platform would just need to define a
> table.
>
> It sounds like you are saying the first option isn't an option. The
> second option adds another layer of pain - we are trying to avoid
> having platform data.
>
> I'd prefer (in this order):
>
> 1. A clock driver
>
>
> please could you explain the rationale for this request on this particular platform?
>
> And I mean for a platform where a clock driver will:
>
> 1. NOT access any hardware
> 2. *only* provide single hard-coded value (a compiled in frequency) for the pl01x driver.
>
> Consider also (another option) that the pl01x driver can be compiled without OF support and that said frequency can be provided by CONFIG.
>
> It is just that before adding layers of stub code I would like to understand the technical need when there is only one consumer (I don't want to pollute U-Boot's code base with code that is not providing value).
>
> What do we want to achieve by writing a SoC clock driver that hard-codes the frequency rate for the console?
> what is the benefit of having such a driver and why is this not an overkill on this platform?

For just one device I can accept that it is overkill. Once you have
another device, or (e.g.) the ability to change clocks in U-Boot at
run time, a clock driver makes sense.

>
>
> 2. Use the existing clock-frequency property
>
>
> yes, this I could understand.
> And it doesn't even need to add a single line to the linux kernel dts files which would be imported from the linux kernel tree and keep unmodified.

Then it sounds like this approach works for you?

Regards,
Simon

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-10 21:31                       ` Simon Glass
@ 2017-05-11 11:45                         ` Jorge Ramirez
  0 siblings, 0 replies; 53+ messages in thread
From: Jorge Ramirez @ 2017-05-11 11:45 UTC (permalink / raw)
  To: u-boot

On 05/10/2017 11:31 PM, Simon Glass wrote:
>> There's 2 better options. You can define fixed clocks with
>> "fixed-clock" compatible and you already have infrastructure in u-boot
>> to use that. However, the upstream dts file already defines clocks, so
>> that doesn't really work here. The 2nd option is have a table of clock
>> ids and frequencies and register that list of clocks based on matching
>> the clock controller. You'd need a little bit of infrastructure to
>> support that, but otherwise a platform would just need to define a
>> table.
>>
>> It sounds like you are saying the first option isn't an option. The
>> second option adds another layer of pain - we are trying to avoid
>> having platform data.
>>
>> I'd prefer (in this order):
>>
>> 1. A clock driver
>>
>>
>> please could you explain the rationale for this request on this particular platform?
>>
>> And I mean for a platform where a clock driver will:
>>
>> 1. NOT access any hardware
>> 2.*only*  provide single hard-coded value (a compiled in frequency) for the pl01x driver.
>>
>> Consider also (another option) that the pl01x driver can be compiled without OF support and that said frequency can be provided by CONFIG.
>>
>> It is just that before adding layers of stub code I would like to understand the technical need when there is only one consumer (I don't want to pollute U-Boot's code base with code that is not providing value).
>>
>> What do we want to achieve by writing a SoC clock driver that hard-codes the frequency rate for the console?
>> what is the benefit of having such a driver and why is this not an overkill on this platform?
> For just one device I can accept that it is overkill. Once you have
> another device, or (e.g.) the ability to change clocks in U-Boot at
> run time, a clock driver makes sense.
>
>> 2. Use the existing clock-frequency property
>>
>>
>> yes, this I could understand.
>> And it doesn't even need to add a single line to the linux kernel dts files which would be imported from the linux kernel tree and keep unmodified.
> Then it sounds like this approach works for you?

ACK: this works for this platform (eMMC, USB host (net/storage) and UART).
if this were to change in the future extending with a clock driver as 
you said would make sense but I don't see this happening.

But to be clear, using "clock-frequency" would mean that I will have to 
modify serial_pl01x.c (replace "clock" with "clock-frequency") and also 
fix the device trees of all the current users of this driver - all of 
them modified their device trees to add u-boot's "clock" property.

do you concur with this?

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-10 19:42                   ` Simon Glass
  2017-05-10 21:19                     ` Jorge Ramirez
@ 2017-05-11 12:35                     ` Tom Rini
  2017-05-11 14:34                       ` Jorge Ramirez
  1 sibling, 1 reply; 53+ messages in thread
From: Tom Rini @ 2017-05-11 12:35 UTC (permalink / raw)
  To: u-boot

On Wed, May 10, 2017 at 01:42:22PM -0600, Simon Glass wrote:
> Hi,
> 
> On 10 May 2017 at 13:09, Rob Herring <robh@kernel.org> wrote:
> > On Wed, May 10, 2017 at 12:45 PM, Tom Rini <trini@konsulko.com> wrote:
> >> On Wed, May 10, 2017 at 11:33:12AM -0500, Rob Herring wrote:
> >>> On Wed, May 10, 2017 at 9:49 AM, Tom Rini <trini@konsulko.com> wrote:
> >>> > On Wed, May 10, 2017 at 11:33:35AM +0200, Jorge Ramirez wrote:
> >>> >> On 05/10/2017 04:30 AM, Tom Rini wrote:
> >>> >> >>hey Tom, I am not sure how to move this forward really so let me
> >>> >> >>clarify where I think we stand:
> >>> >> >>
> >>> >> >>1. The linux kernel does not need the clock property in the uart
> >>> >> >>nodes (only u-boot does: serial_pl01x.c needs fixing).
> >>> >> >>2. ehci is not present in the linux kernel poplar dts yet but it
> >>> >> >>will be eventually.
> >>> >> >>
> >>> >> >>with this in mind, what is blocking the acceptance of the patchset?
> >>> >> >>
> >>> >> >>I can do v5 using the linux kernel dts as is and creating a
> >>> >> >>hi3798cv200-u-boot.dtsi that simply adds the nodes above (this time
> >>> >> >>no #include required:)  )
> >>> >> >>
> >>> >> >>Then when ehci is added to the kernel, the ehci node can be removed
> >>> >> >>from u-boot.dtsi
> >>> >> >>And when uboot updates its pl01x.c serial driver,  the uart0 node
> >>> >> >>can be removed and the u-boot.dtsi filed completely deleted.
> >>> >> >Can you take a stab at updating the pl01x driver?  Thanks!
> >>> >>
> >>> >> updating pl01x is not a big deal I dont think; however this will
> >>> >> mean requiring a platform specific clock driver to just use the
> >>> >> pl01x - which will take me some time to get into uboot for my
> >>> >> platform (and the same might happen to other users).
> >>> >
> >>> > Ah right.  So the flip side here, is one not allowed to have the clock
> >>> > property anymore?  Yes, it may not be used in the kernel, but has
> >>> > someone argued that it's not part of the hardware description?
> >>>
> >>> First I've ever seen a "clock" property. We have "clocks" from the
> >>> clock binding which is a phandle plus #clock-cells number of args. We
> >>> also have "clock-frequency" which is just the frequency value and has
> >>> been around forever. Why u-boot went off and did something different i
> >>> don't know. Sigh. What I can say is a 3rd way is not going to be
> >>> accepted.
> >>
> >> Aw crap, I'm in the wrong.  I was thinking this was "clock-frequency"
> >> and not that we had invented our own property here.
> >>
> >>> Generally, the clock binding replaces clock-frequency, but there are
> >>> some cases where clock binding would be overkill or too complicated
> >>> for early boot and using clock-frequency would be okay. But I don't
> >>> think "I haven't written my platform clock controller driver yet" is a
> >>> reason to use clock-frequency as generally most platforms are going to
> >>> have to have one. Providing a stub driver that just returns a fixed
> >>> frequency shouldn't be too hard IMO.
> >>
> >> So, trying to dig out of the hole we made here.  The generic serial
> >> binding (bindings/serial/serial.txt) documents clock-frequency.  The
> >> pl011 binding (and primecell which it also references) does not.  Would
> >> adding clock-frequency to a pl011 node be valid or invalid?
> >
> > Valid in general. It's a common property in the DT spec. Though, it
> > should be listed in the pl011 binding doc as used just like we list
> > reg or interrupts.
> >
> >>  If valid,
> >> would it also be acceptable to include in dts files that also fill in
> >> clocks/clock-names from that binding?
> >
> > Generally we treat that as not valid as they are mutually exclusive.
> >
> > There's 2 better options. You can define fixed clocks with
> > "fixed-clock" compatible and you already have infrastructure in u-boot
> > to use that. However, the upstream dts file already defines clocks, so
> > that doesn't really work here. The 2nd option is have a table of clock
> > ids and frequencies and register that list of clocks based on matching
> > the clock controller. You'd need a little bit of infrastructure to
> > support that, but otherwise a platform would just need to define a
> > table.
> 
> It sounds like you are saying the first option isn't an option. The
> second option adds another layer of pain - we are trying to avoid
> having platform data.

No, I think we're going for overkill here by not doing serial_pl01x.c as
platform data.  ns16550 does platform data for this already.  This
sounds like the lowest overhead way to get the clock populated and not
have some DT data that's not going to be accepted upstream.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170511/f5ac1336/attachment.sig>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-11 12:35                     ` Tom Rini
@ 2017-05-11 14:34                       ` Jorge Ramirez
  2017-05-11 22:32                         ` Tom Rini
  2017-05-17 22:06                         ` Tom Rini
  0 siblings, 2 replies; 53+ messages in thread
From: Jorge Ramirez @ 2017-05-11 14:34 UTC (permalink / raw)
  To: u-boot

On 05/11/2017 02:35 PM, Tom Rini wrote:
> On Wed, May 10, 2017 at 01:42:22PM -0600, Simon Glass wrote:
>> Hi,
>>
>> On 10 May 2017 at 13:09, Rob Herring <robh@kernel.org> wrote:
>>> On Wed, May 10, 2017 at 12:45 PM, Tom Rini <trini@konsulko.com> wrote:
>>>> On Wed, May 10, 2017 at 11:33:12AM -0500, Rob Herring wrote:
>>>>> On Wed, May 10, 2017 at 9:49 AM, Tom Rini <trini@konsulko.com> wrote:
>>>>>> On Wed, May 10, 2017 at 11:33:35AM +0200, Jorge Ramirez wrote:
>>>>>>> On 05/10/2017 04:30 AM, Tom Rini wrote:
>>>>>>>>> hey Tom, I am not sure how to move this forward really so let me
>>>>>>>>> clarify where I think we stand:
>>>>>>>>>
>>>>>>>>> 1. The linux kernel does not need the clock property in the uart
>>>>>>>>> nodes (only u-boot does: serial_pl01x.c needs fixing).
>>>>>>>>> 2. ehci is not present in the linux kernel poplar dts yet but it
>>>>>>>>> will be eventually.
>>>>>>>>>
>>>>>>>>> with this in mind, what is blocking the acceptance of the patchset?
>>>>>>>>>
>>>>>>>>> I can do v5 using the linux kernel dts as is and creating a
>>>>>>>>> hi3798cv200-u-boot.dtsi that simply adds the nodes above (this time
>>>>>>>>> no #include required:)  )
>>>>>>>>>
>>>>>>>>> Then when ehci is added to the kernel, the ehci node can be removed
>>>>>>>> >from u-boot.dtsi
>>>>>>>>> And when uboot updates its pl01x.c serial driver,  the uart0 node
>>>>>>>>> can be removed and the u-boot.dtsi filed completely deleted.
>>>>>>>> Can you take a stab at updating the pl01x driver?  Thanks!
>>>>>>> updating pl01x is not a big deal I dont think; however this will
>>>>>>> mean requiring a platform specific clock driver to just use the
>>>>>>> pl01x - which will take me some time to get into uboot for my
>>>>>>> platform (and the same might happen to other users).
>>>>>> Ah right.  So the flip side here, is one not allowed to have the clock
>>>>>> property anymore?  Yes, it may not be used in the kernel, but has
>>>>>> someone argued that it's not part of the hardware description?
>>>>> First I've ever seen a "clock" property. We have "clocks" from the
>>>>> clock binding which is a phandle plus #clock-cells number of args. We
>>>>> also have "clock-frequency" which is just the frequency value and has
>>>>> been around forever. Why u-boot went off and did something different i
>>>>> don't know. Sigh. What I can say is a 3rd way is not going to be
>>>>> accepted.
>>>> Aw crap, I'm in the wrong.  I was thinking this was "clock-frequency"
>>>> and not that we had invented our own property here.
>>>>
>>>>> Generally, the clock binding replaces clock-frequency, but there are
>>>>> some cases where clock binding would be overkill or too complicated
>>>>> for early boot and using clock-frequency would be okay. But I don't
>>>>> think "I haven't written my platform clock controller driver yet" is a
>>>>> reason to use clock-frequency as generally most platforms are going to
>>>>> have to have one. Providing a stub driver that just returns a fixed
>>>>> frequency shouldn't be too hard IMO.
>>>> So, trying to dig out of the hole we made here.  The generic serial
>>>> binding (bindings/serial/serial.txt) documents clock-frequency.  The
>>>> pl011 binding (and primecell which it also references) does not.  Would
>>>> adding clock-frequency to a pl011 node be valid or invalid?
>>> Valid in general. It's a common property in the DT spec. Though, it
>>> should be listed in the pl011 binding doc as used just like we list
>>> reg or interrupts.
>>>
>>>>   If valid,
>>>> would it also be acceptable to include in dts files that also fill in
>>>> clocks/clock-names from that binding?
>>> Generally we treat that as not valid as they are mutually exclusive.
>>>
>>> There's 2 better options. You can define fixed clocks with
>>> "fixed-clock" compatible and you already have infrastructure in u-boot
>>> to use that. However, the upstream dts file already defines clocks, so
>>> that doesn't really work here. The 2nd option is have a table of clock
>>> ids and frequencies and register that list of clocks based on matching
>>> the clock controller. You'd need a little bit of infrastructure to
>>> support that, but otherwise a platform would just need to define a
>>> table.
>> It sounds like you are saying the first option isn't an option. The
>> second option adds another layer of pain - we are trying to avoid
>> having platform data.
> No, I think we're going for overkill here by not doing serial_pl01x.c as
> platform data.  ns16550 does platform data for this already.  This
> sounds like the lowest overhead way to get the clock populated and not
> have some DT data that's not going to be accepted upstream.
>


ummmm I am a bit lost at this point, could we recap please?

let's see: I need to use the pl01x uart on an aarch64 platform and I 
dont need to enable any clocks for uboot in my SoC. Not now, unlikely ever.

Doing what other boards have done to this date is no longer acceptable 
(ie platform data for the pl01x or using uboots "clock" property 
embedded in the hacked device trees)

so, what is the recommended way of configuring this uart?

1. write a dummy clock driver to provide the number of choice for the 
board (in my case 75m)
2. using platform data for the pl01x
3. using u-boot's very own "clock" property in a separate -u-boot.dtsi
4. using a proper clock-frequency property and fixing pl01x and all 
those dts files that incorrectly have coded uboot properties...


TIA

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-11 14:34                       ` Jorge Ramirez
@ 2017-05-11 22:32                         ` Tom Rini
  2017-05-12  8:16                           ` Jorge Ramirez
  2017-05-17 22:06                         ` Tom Rini
  1 sibling, 1 reply; 53+ messages in thread
From: Tom Rini @ 2017-05-11 22:32 UTC (permalink / raw)
  To: u-boot

On Thu, May 11, 2017 at 04:34:18PM +0200, Jorge Ramirez wrote:
> On 05/11/2017 02:35 PM, Tom Rini wrote:
> >On Wed, May 10, 2017 at 01:42:22PM -0600, Simon Glass wrote:
> >>Hi,
> >>
> >>On 10 May 2017 at 13:09, Rob Herring <robh@kernel.org> wrote:
> >>>On Wed, May 10, 2017 at 12:45 PM, Tom Rini <trini@konsulko.com> wrote:
> >>>>On Wed, May 10, 2017 at 11:33:12AM -0500, Rob Herring wrote:
> >>>>>On Wed, May 10, 2017 at 9:49 AM, Tom Rini <trini@konsulko.com> wrote:
> >>>>>>On Wed, May 10, 2017 at 11:33:35AM +0200, Jorge Ramirez wrote:
> >>>>>>>On 05/10/2017 04:30 AM, Tom Rini wrote:
> >>>>>>>>>hey Tom, I am not sure how to move this forward really so let me
> >>>>>>>>>clarify where I think we stand:
> >>>>>>>>>
> >>>>>>>>>1. The linux kernel does not need the clock property in the uart
> >>>>>>>>>nodes (only u-boot does: serial_pl01x.c needs fixing).
> >>>>>>>>>2. ehci is not present in the linux kernel poplar dts yet but it
> >>>>>>>>>will be eventually.
> >>>>>>>>>
> >>>>>>>>>with this in mind, what is blocking the acceptance of the patchset?
> >>>>>>>>>
> >>>>>>>>>I can do v5 using the linux kernel dts as is and creating a
> >>>>>>>>>hi3798cv200-u-boot.dtsi that simply adds the nodes above (this time
> >>>>>>>>>no #include required:)  )
> >>>>>>>>>
> >>>>>>>>>Then when ehci is added to the kernel, the ehci node can be removed
> >>>>>>>>>from u-boot.dtsi
> >>>>>>>>>And when uboot updates its pl01x.c serial driver,  the uart0 node
> >>>>>>>>>can be removed and the u-boot.dtsi filed completely deleted.
> >>>>>>>>Can you take a stab at updating the pl01x driver?  Thanks!
> >>>>>>>updating pl01x is not a big deal I dont think; however this will
> >>>>>>>mean requiring a platform specific clock driver to just use the
> >>>>>>>pl01x - which will take me some time to get into uboot for my
> >>>>>>>platform (and the same might happen to other users).
> >>>>>>Ah right.  So the flip side here, is one not allowed to have the clock
> >>>>>>property anymore?  Yes, it may not be used in the kernel, but has
> >>>>>>someone argued that it's not part of the hardware description?
> >>>>>First I've ever seen a "clock" property. We have "clocks" from the
> >>>>>clock binding which is a phandle plus #clock-cells number of args. We
> >>>>>also have "clock-frequency" which is just the frequency value and has
> >>>>>been around forever. Why u-boot went off and did something different i
> >>>>>don't know. Sigh. What I can say is a 3rd way is not going to be
> >>>>>accepted.
> >>>>Aw crap, I'm in the wrong.  I was thinking this was "clock-frequency"
> >>>>and not that we had invented our own property here.
> >>>>
> >>>>>Generally, the clock binding replaces clock-frequency, but there are
> >>>>>some cases where clock binding would be overkill or too complicated
> >>>>>for early boot and using clock-frequency would be okay. But I don't
> >>>>>think "I haven't written my platform clock controller driver yet" is a
> >>>>>reason to use clock-frequency as generally most platforms are going to
> >>>>>have to have one. Providing a stub driver that just returns a fixed
> >>>>>frequency shouldn't be too hard IMO.
> >>>>So, trying to dig out of the hole we made here.  The generic serial
> >>>>binding (bindings/serial/serial.txt) documents clock-frequency.  The
> >>>>pl011 binding (and primecell which it also references) does not.  Would
> >>>>adding clock-frequency to a pl011 node be valid or invalid?
> >>>Valid in general. It's a common property in the DT spec. Though, it
> >>>should be listed in the pl011 binding doc as used just like we list
> >>>reg or interrupts.
> >>>
> >>>>  If valid,
> >>>>would it also be acceptable to include in dts files that also fill in
> >>>>clocks/clock-names from that binding?
> >>>Generally we treat that as not valid as they are mutually exclusive.
> >>>
> >>>There's 2 better options. You can define fixed clocks with
> >>>"fixed-clock" compatible and you already have infrastructure in u-boot
> >>>to use that. However, the upstream dts file already defines clocks, so
> >>>that doesn't really work here. The 2nd option is have a table of clock
> >>>ids and frequencies and register that list of clocks based on matching
> >>>the clock controller. You'd need a little bit of infrastructure to
> >>>support that, but otherwise a platform would just need to define a
> >>>table.
> >>It sounds like you are saying the first option isn't an option. The
> >>second option adds another layer of pain - we are trying to avoid
> >>having platform data.
> >No, I think we're going for overkill here by not doing serial_pl01x.c as
> >platform data.  ns16550 does platform data for this already.  This
> >sounds like the lowest overhead way to get the clock populated and not
> >have some DT data that's not going to be accepted upstream.
> >
> 
> 
> ummmm I am a bit lost at this point, could we recap please?

Sure.

> let's see: I need to use the pl01x uart on an aarch64 platform and I
> dont need to enable any clocks for uboot in my SoC. Not now,
> unlikely ever.
> 
> Doing what other boards have done to this date is no longer
> acceptable (ie platform data for the pl01x or using uboots "clock"
> property embedded in the hacked device trees)

The only thing we all agree on right now is that "clock" is wrong and
must be replaced.  I've decided we need to discuss bringing in platform
data for pl01x.  Once we resolve this, then you can re-spin the series
(and hopefully have the USB nodes be submitted to Linux too, since
they're the standard ones and, uh, should just enable USB on your board
in the kernel too..)  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170511/3ad7c255/attachment.sig>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-11 22:32                         ` Tom Rini
@ 2017-05-12  8:16                           ` Jorge Ramirez
  2017-05-12 12:35                             ` Tom Rini
  0 siblings, 1 reply; 53+ messages in thread
From: Jorge Ramirez @ 2017-05-12  8:16 UTC (permalink / raw)
  To: u-boot

On 05/12/2017 12:32 AM, Tom Rini wrote:
>> ummmm I am a bit lost at this point, could we recap please?
> Sure.
>
>> let's see: I need to use the pl01x uart on an aarch64 platform and I
>> dont need to enable any clocks for uboot in my SoC. Not now,
>> unlikely ever.
>>
>> Doing what other boards have done to this date is no longer
>> acceptable (ie platform data for the pl01x or using uboots "clock"
>> property embedded in the hacked device trees)
> The only thing we all agree on right now is that "clock" is wrong and
> must be replaced.  I've decided we need to discuss bringing in platform
> data for pl01x.  Once we resolve this, then you can re-spin the series
> (and hopefully have the USB nodes be submitted to Linux too, since
> they're the standard ones and, uh, should just enable USB on your board
> in the kernel too..)  Thanks!

cool, that sounds great, thanks.

yeah the usb nodes should be ready pretty soon, I have seen them 
circulating already.

btw, what was it that triggered our discussion?  it is not like any of 
the dts files for armv8 boards are verbatim copies of what you find in 
the kernel.

Is there a new rule -explicit somewhere?- enforcing that this has to be 
the case then?

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-12  8:16                           ` Jorge Ramirez
@ 2017-05-12 12:35                             ` Tom Rini
  2017-05-15 21:38                               ` Rob Herring
  0 siblings, 1 reply; 53+ messages in thread
From: Tom Rini @ 2017-05-12 12:35 UTC (permalink / raw)
  To: u-boot

On Fri, May 12, 2017 at 10:16:52AM +0200, Jorge Ramirez wrote:
> On 05/12/2017 12:32 AM, Tom Rini wrote:
> >>ummmm I am a bit lost at this point, could we recap please?
> >Sure.
> >
> >>let's see: I need to use the pl01x uart on an aarch64 platform and I
> >>dont need to enable any clocks for uboot in my SoC. Not now,
> >>unlikely ever.
> >>
> >>Doing what other boards have done to this date is no longer
> >>acceptable (ie platform data for the pl01x or using uboots "clock"
> >>property embedded in the hacked device trees)
> >The only thing we all agree on right now is that "clock" is wrong and
> >must be replaced.  I've decided we need to discuss bringing in platform
> >data for pl01x.  Once we resolve this, then you can re-spin the series
> >(and hopefully have the USB nodes be submitted to Linux too, since
> >they're the standard ones and, uh, should just enable USB on your board
> >in the kernel too..)  Thanks!
> 
> cool, that sounds great, thanks.
> 
> yeah the usb nodes should be ready pretty soon, I have seen them
> circulating already.
> 
> btw, what was it that triggered our discussion?  it is not like any
> of the dts files for armv8 boards are verbatim copies of what you
> find in the kernel.

They've gotten out of sync? Sigh..  I suppose this starts to push me
from the "keep them in the kernel" camp to "push them to a separate
authoritative repository" camp.

I do know some of the early boards were out of sync, but that shouldn't
be the case moving forward, and isn't the case for ARMv7 things.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170512/b931ca5d/attachment.sig>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-12 12:35                             ` Tom Rini
@ 2017-05-15 21:38                               ` Rob Herring
  2017-05-17 13:33                                 ` Tom Rini
  0 siblings, 1 reply; 53+ messages in thread
From: Rob Herring @ 2017-05-15 21:38 UTC (permalink / raw)
  To: u-boot

On Fri, May 12, 2017 at 7:35 AM, Tom Rini <trini@konsulko.com> wrote:
> On Fri, May 12, 2017 at 10:16:52AM +0200, Jorge Ramirez wrote:
>> On 05/12/2017 12:32 AM, Tom Rini wrote:
>> >>ummmm I am a bit lost at this point, could we recap please?
>> >Sure.
>> >
>> >>let's see: I need to use the pl01x uart on an aarch64 platform and I
>> >>dont need to enable any clocks for uboot in my SoC. Not now,
>> >>unlikely ever.
>> >>
>> >>Doing what other boards have done to this date is no longer
>> >>acceptable (ie platform data for the pl01x or using uboots "clock"
>> >>property embedded in the hacked device trees)
>> >The only thing we all agree on right now is that "clock" is wrong and
>> >must be replaced.  I've decided we need to discuss bringing in platform
>> >data for pl01x.  Once we resolve this, then you can re-spin the series
>> >(and hopefully have the USB nodes be submitted to Linux too, since
>> >they're the standard ones and, uh, should just enable USB on your board
>> >in the kernel too..)  Thanks!
>>
>> cool, that sounds great, thanks.
>>
>> yeah the usb nodes should be ready pretty soon, I have seen them
>> circulating already.
>>
>> btw, what was it that triggered our discussion?  it is not like any
>> of the dts files for armv8 boards are verbatim copies of what you
>> find in the kernel.
>
> They've gotten out of sync? Sigh..  I suppose this starts to push me
> from the "keep them in the kernel" camp to "push them to a separate
> authoritative repository" camp.

What's wrong with the standalone DT tree[1] and importing that to
u-boot periodically?

I know folks would like a completely separate tree that's not "the
Linux DT tree", but I don't see that happening any time soon. Do we
have some Linuxisms in bindings, yes, but in general I think they are
more the exception than rule and were things that went in with little
review. These days I'm reviewing pretty much all bindings (not all dts
files though), so I think it's less of a problem. Logistically, we
could probably work out how to move bindings and dts files to a
standalone repository as I could apply bindings and most dts files go
thru arm-soc maintainers. My biggest concern with a separate
repository is review because we would quickly loose any review that
Linux subsystem maintainers do, and no one is beating down my door to
help be a DT maintainer.

Rob

[1] git://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-15 21:38                               ` Rob Herring
@ 2017-05-17 13:33                                 ` Tom Rini
  2017-05-17 14:38                                   ` Rob Herring
  0 siblings, 1 reply; 53+ messages in thread
From: Tom Rini @ 2017-05-17 13:33 UTC (permalink / raw)
  To: u-boot

On Mon, May 15, 2017 at 04:38:14PM -0500, Rob Herring wrote:
> On Fri, May 12, 2017 at 7:35 AM, Tom Rini <trini@konsulko.com> wrote:
> > On Fri, May 12, 2017 at 10:16:52AM +0200, Jorge Ramirez wrote:
> >> On 05/12/2017 12:32 AM, Tom Rini wrote:
> >> >>ummmm I am a bit lost at this point, could we recap please?
> >> >Sure.
> >> >
> >> >>let's see: I need to use the pl01x uart on an aarch64 platform and I
> >> >>dont need to enable any clocks for uboot in my SoC. Not now,
> >> >>unlikely ever.
> >> >>
> >> >>Doing what other boards have done to this date is no longer
> >> >>acceptable (ie platform data for the pl01x or using uboots "clock"
> >> >>property embedded in the hacked device trees)
> >> >The only thing we all agree on right now is that "clock" is wrong and
> >> >must be replaced.  I've decided we need to discuss bringing in platform
> >> >data for pl01x.  Once we resolve this, then you can re-spin the series
> >> >(and hopefully have the USB nodes be submitted to Linux too, since
> >> >they're the standard ones and, uh, should just enable USB on your board
> >> >in the kernel too..)  Thanks!
> >>
> >> cool, that sounds great, thanks.
> >>
> >> yeah the usb nodes should be ready pretty soon, I have seen them
> >> circulating already.
> >>
> >> btw, what was it that triggered our discussion?  it is not like any
> >> of the dts files for armv8 boards are verbatim copies of what you
> >> find in the kernel.
> >
> > They've gotten out of sync? Sigh..  I suppose this starts to push me
> > from the "keep them in the kernel" camp to "push them to a separate
> > authoritative repository" camp.
> 
> What's wrong with the standalone DT tree[1] and importing that to
> u-boot periodically?
> 
> I know folks would like a completely separate tree that's not "the
> Linux DT tree", but I don't see that happening any time soon. Do we
> have some Linuxisms in bindings, yes, but in general I think they are
> more the exception than rule and were things that went in with little
> review. These days I'm reviewing pretty much all bindings (not all dts
> files though), so I think it's less of a problem. Logistically, we
> could probably work out how to move bindings and dts files to a
> standalone repository as I could apply bindings and most dts files go
> thru arm-soc maintainers. My biggest concern with a separate
> repository is review because we would quickly loose any review that
> Linux subsystem maintainers do, and no one is beating down my door to
> help be a DT maintainer.

Thinking about this, the key word is authoritative.  Right now we say
(every time I spot a new dts patch) "is this in the kernel yet?" and the
answers break down to:
- Yes, see v4.x
- Yes, see linux-next $tag (or Yes, see maintainer-tree-$X)
- No, we're working on it.

To me, the first is great, the second is OK only in that we're probably
not relying on anything bleeding edge and likely to change between
linux-next $tag and when it finally goes upstream.  The third is where
we're at with this board.  And a problem is that even with the short
kernel release cycle, there is a lot of not wanting to wait until things
get into the next upstream release.

Making a big and possibly wrong assumption, for something like this
board, that doesn't introduce any new bindings, shouldn't this dts be
able to go in quickly, once it of course is otherwise correct?  And
U-Boot (and barebox and the kernel and anyone else that cares) doesn't
want the tree until it was correct either.  And once a tree is in this
upstream, it's just a matter of saying import dts files for $X, taken
from $hash of the device tree repository (or even just included as I
might make myself get in the habit of syncing the tree in post release,
as it'd be on our release cycle, but honestly, I could / should just do
that, even if it's a -rc).

> [1] git://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git

... also, so it rebases and isn't stable?  That makes life less fun for
tracing back when we synced $X last.


-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170517/f0e73622/attachment.sig>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-17 13:33                                 ` Tom Rini
@ 2017-05-17 14:38                                   ` Rob Herring
  2017-05-17 22:13                                     ` Tom Rini
  0 siblings, 1 reply; 53+ messages in thread
From: Rob Herring @ 2017-05-17 14:38 UTC (permalink / raw)
  To: u-boot

On Wed, May 17, 2017 at 8:33 AM, Tom Rini <trini@konsulko.com> wrote:
> On Mon, May 15, 2017 at 04:38:14PM -0500, Rob Herring wrote:
>> On Fri, May 12, 2017 at 7:35 AM, Tom Rini <trini@konsulko.com> wrote:
>> > On Fri, May 12, 2017 at 10:16:52AM +0200, Jorge Ramirez wrote:
>> >> On 05/12/2017 12:32 AM, Tom Rini wrote:
>> >> >>ummmm I am a bit lost at this point, could we recap please?
>> >> >Sure.
>> >> >
>> >> >>let's see: I need to use the pl01x uart on an aarch64 platform and I
>> >> >>dont need to enable any clocks for uboot in my SoC. Not now,
>> >> >>unlikely ever.
>> >> >>
>> >> >>Doing what other boards have done to this date is no longer
>> >> >>acceptable (ie platform data for the pl01x or using uboots "clock"
>> >> >>property embedded in the hacked device trees)
>> >> >The only thing we all agree on right now is that "clock" is wrong and
>> >> >must be replaced.  I've decided we need to discuss bringing in platform
>> >> >data for pl01x.  Once we resolve this, then you can re-spin the series
>> >> >(and hopefully have the USB nodes be submitted to Linux too, since
>> >> >they're the standard ones and, uh, should just enable USB on your board
>> >> >in the kernel too..)  Thanks!
>> >>
>> >> cool, that sounds great, thanks.
>> >>
>> >> yeah the usb nodes should be ready pretty soon, I have seen them
>> >> circulating already.
>> >>
>> >> btw, what was it that triggered our discussion?  it is not like any
>> >> of the dts files for armv8 boards are verbatim copies of what you
>> >> find in the kernel.
>> >
>> > They've gotten out of sync? Sigh..  I suppose this starts to push me
>> > from the "keep them in the kernel" camp to "push them to a separate
>> > authoritative repository" camp.
>>
>> What's wrong with the standalone DT tree[1] and importing that to
>> u-boot periodically?
>>
>> I know folks would like a completely separate tree that's not "the
>> Linux DT tree", but I don't see that happening any time soon. Do we
>> have some Linuxisms in bindings, yes, but in general I think they are
>> more the exception than rule and were things that went in with little
>> review. These days I'm reviewing pretty much all bindings (not all dts
>> files though), so I think it's less of a problem. Logistically, we
>> could probably work out how to move bindings and dts files to a
>> standalone repository as I could apply bindings and most dts files go
>> thru arm-soc maintainers. My biggest concern with a separate
>> repository is review because we would quickly loose any review that
>> Linux subsystem maintainers do, and no one is beating down my door to
>> help be a DT maintainer.
>
> Thinking about this, the key word is authoritative.  Right now we say
> (every time I spot a new dts patch) "is this in the kernel yet?" and the
> answers break down to:
> - Yes, see v4.x
> - Yes, see linux-next $tag (or Yes, see maintainer-tree-$X)
> - No, we're working on it.
>
> To me, the first is great, the second is OK only in that we're probably
> not relying on anything bleeding edge and likely to change between
> linux-next $tag and when it finally goes upstream.  The third is where
> we're at with this board.  And a problem is that even with the short
> kernel release cycle, there is a lot of not wanting to wait until things
> get into the next upstream release.

Maybe it was the 3rd case at the start of this, but it is now in v4.12-rc1.

Generally, commits in -next are not rebased and should match what ends
up in mainline. But that's not guaranteed and syncing with -next would
probably not be the best policy.

> Making a big and possibly wrong assumption, for something like this
> board, that doesn't introduce any new bindings, shouldn't this dts be
> able to go in quickly, once it of course is otherwise correct?

New SoC too, so probably a bit more than just a new assembly of
existing bindings. Worst case is someone submits this just before the
merge window, it's pulled in just after N-rc1, and doesn't get tagged
until O-rc1. In this case, the dts was committed on Apr 6 and rc1 was
tagged May 13. We could look at doing a filtered tree based off of
-next perhaps. Perhaps we should wait to see if the latency is really
a problem.

>  And
> U-Boot (and barebox and the kernel and anyone else that cares) doesn't
> want the tree until it was correct either.

Exactly.

>  And once a tree is in this
> upstream, it's just a matter of saying import dts files for $X, taken
> from $hash of the device tree repository (or even just included as I
> might make myself get in the habit of syncing the tree in post release,
> as it'd be on our release cycle, but honestly, I could / should just do
> that, even if it's a -rc).

Usually an -rcX is safe to take, but sometimes bindings do get changed
during -rc cycle.

>
>> [1] git://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git
>
> ... also, so it rebases and isn't stable?  That makes life less fun for
> tracing back when we synced $X last.

It is generated with git-filter-branch. So it may be regenerated if
the generation scripts change. As long as you are tracking kernel tags
as to what you've imported, then it should not matter. I'm not sure if
we've rebased recently. It was named that somewhat as a CYA. Ian can
better comment on this.

Rob

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-11 14:34                       ` Jorge Ramirez
  2017-05-11 22:32                         ` Tom Rini
@ 2017-05-17 22:06                         ` Tom Rini
  2017-05-25 18:38                           ` Jorge Ramirez
  1 sibling, 1 reply; 53+ messages in thread
From: Tom Rini @ 2017-05-17 22:06 UTC (permalink / raw)
  To: u-boot

On Thu, May 11, 2017 at 04:34:18PM +0200, Jorge Ramirez wrote:
> On 05/11/2017 02:35 PM, Tom Rini wrote:
> >On Wed, May 10, 2017 at 01:42:22PM -0600, Simon Glass wrote:
> >>Hi,
> >>
> >>On 10 May 2017 at 13:09, Rob Herring <robh@kernel.org> wrote:
> >>>On Wed, May 10, 2017 at 12:45 PM, Tom Rini <trini@konsulko.com> wrote:
> >>>>On Wed, May 10, 2017 at 11:33:12AM -0500, Rob Herring wrote:
> >>>>>On Wed, May 10, 2017 at 9:49 AM, Tom Rini <trini@konsulko.com> wrote:
> >>>>>>On Wed, May 10, 2017 at 11:33:35AM +0200, Jorge Ramirez wrote:
> >>>>>>>On 05/10/2017 04:30 AM, Tom Rini wrote:
> >>>>>>>>>hey Tom, I am not sure how to move this forward really so let me
> >>>>>>>>>clarify where I think we stand:
> >>>>>>>>>
> >>>>>>>>>1. The linux kernel does not need the clock property in the uart
> >>>>>>>>>nodes (only u-boot does: serial_pl01x.c needs fixing).
> >>>>>>>>>2. ehci is not present in the linux kernel poplar dts yet but it
> >>>>>>>>>will be eventually.
> >>>>>>>>>
> >>>>>>>>>with this in mind, what is blocking the acceptance of the patchset?
> >>>>>>>>>
> >>>>>>>>>I can do v5 using the linux kernel dts as is and creating a
> >>>>>>>>>hi3798cv200-u-boot.dtsi that simply adds the nodes above (this time
> >>>>>>>>>no #include required:)  )
> >>>>>>>>>
> >>>>>>>>>Then when ehci is added to the kernel, the ehci node can be removed
> >>>>>>>>>from u-boot.dtsi
> >>>>>>>>>And when uboot updates its pl01x.c serial driver,  the uart0 node
> >>>>>>>>>can be removed and the u-boot.dtsi filed completely deleted.
> >>>>>>>>Can you take a stab at updating the pl01x driver?  Thanks!
> >>>>>>>updating pl01x is not a big deal I dont think; however this will
> >>>>>>>mean requiring a platform specific clock driver to just use the
> >>>>>>>pl01x - which will take me some time to get into uboot for my
> >>>>>>>platform (and the same might happen to other users).
> >>>>>>Ah right.  So the flip side here, is one not allowed to have the clock
> >>>>>>property anymore?  Yes, it may not be used in the kernel, but has
> >>>>>>someone argued that it's not part of the hardware description?
> >>>>>First I've ever seen a "clock" property. We have "clocks" from the
> >>>>>clock binding which is a phandle plus #clock-cells number of args. We
> >>>>>also have "clock-frequency" which is just the frequency value and has
> >>>>>been around forever. Why u-boot went off and did something different i
> >>>>>don't know. Sigh. What I can say is a 3rd way is not going to be
> >>>>>accepted.
> >>>>Aw crap, I'm in the wrong.  I was thinking this was "clock-frequency"
> >>>>and not that we had invented our own property here.
> >>>>
> >>>>>Generally, the clock binding replaces clock-frequency, but there are
> >>>>>some cases where clock binding would be overkill or too complicated
> >>>>>for early boot and using clock-frequency would be okay. But I don't
> >>>>>think "I haven't written my platform clock controller driver yet" is a
> >>>>>reason to use clock-frequency as generally most platforms are going to
> >>>>>have to have one. Providing a stub driver that just returns a fixed
> >>>>>frequency shouldn't be too hard IMO.
> >>>>So, trying to dig out of the hole we made here.  The generic serial
> >>>>binding (bindings/serial/serial.txt) documents clock-frequency.  The
> >>>>pl011 binding (and primecell which it also references) does not.  Would
> >>>>adding clock-frequency to a pl011 node be valid or invalid?
> >>>Valid in general. It's a common property in the DT spec. Though, it
> >>>should be listed in the pl011 binding doc as used just like we list
> >>>reg or interrupts.
> >>>
> >>>>  If valid,
> >>>>would it also be acceptable to include in dts files that also fill in
> >>>>clocks/clock-names from that binding?
> >>>Generally we treat that as not valid as they are mutually exclusive.
> >>>
> >>>There's 2 better options. You can define fixed clocks with
> >>>"fixed-clock" compatible and you already have infrastructure in u-boot
> >>>to use that. However, the upstream dts file already defines clocks, so
> >>>that doesn't really work here. The 2nd option is have a table of clock
> >>>ids and frequencies and register that list of clocks based on matching
> >>>the clock controller. You'd need a little bit of infrastructure to
> >>>support that, but otherwise a platform would just need to define a
> >>>table.
> >>It sounds like you are saying the first option isn't an option. The
> >>second option adds another layer of pain - we are trying to avoid
> >>having platform data.
> >No, I think we're going for overkill here by not doing serial_pl01x.c as
> >platform data.  ns16550 does platform data for this already.  This
> >sounds like the lowest overhead way to get the clock populated and not
> >have some DT data that's not going to be accepted upstream.
> >
> 
> 
> ummmm I am a bit lost at this point, could we recap please?

Lets update the recap:
- Please re-submit the dts file, now with whatever form is in v4.12-rc1,
  saying as such in the commit (if it's just the commit message that
  changes, that's fine and great).
- Please update serial_pl01x.c to be able to get the clock via platform
  data, update and test your board to confirm it works.
- It'd be awesome if you do, but it won't block your series if you
  don't, update the rest of the platforms that had been using the
  "clock" platform to instead use the platform data method.

Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170517/70b111e3/attachment.sig>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-17 14:38                                   ` Rob Herring
@ 2017-05-17 22:13                                     ` Tom Rini
  0 siblings, 0 replies; 53+ messages in thread
From: Tom Rini @ 2017-05-17 22:13 UTC (permalink / raw)
  To: u-boot

On Wed, May 17, 2017 at 09:38:06AM -0500, Rob Herring wrote:
> On Wed, May 17, 2017 at 8:33 AM, Tom Rini <trini@konsulko.com> wrote:
> > On Mon, May 15, 2017 at 04:38:14PM -0500, Rob Herring wrote:
> >> On Fri, May 12, 2017 at 7:35 AM, Tom Rini <trini@konsulko.com> wrote:
> >> > On Fri, May 12, 2017 at 10:16:52AM +0200, Jorge Ramirez wrote:
> >> >> On 05/12/2017 12:32 AM, Tom Rini wrote:
> >> >> >>ummmm I am a bit lost at this point, could we recap please?
> >> >> >Sure.
> >> >> >
> >> >> >>let's see: I need to use the pl01x uart on an aarch64 platform and I
> >> >> >>dont need to enable any clocks for uboot in my SoC. Not now,
> >> >> >>unlikely ever.
> >> >> >>
> >> >> >>Doing what other boards have done to this date is no longer
> >> >> >>acceptable (ie platform data for the pl01x or using uboots "clock"
> >> >> >>property embedded in the hacked device trees)
> >> >> >The only thing we all agree on right now is that "clock" is wrong and
> >> >> >must be replaced.  I've decided we need to discuss bringing in platform
> >> >> >data for pl01x.  Once we resolve this, then you can re-spin the series
> >> >> >(and hopefully have the USB nodes be submitted to Linux too, since
> >> >> >they're the standard ones and, uh, should just enable USB on your board
> >> >> >in the kernel too..)  Thanks!
> >> >>
> >> >> cool, that sounds great, thanks.
> >> >>
> >> >> yeah the usb nodes should be ready pretty soon, I have seen them
> >> >> circulating already.
> >> >>
> >> >> btw, what was it that triggered our discussion?  it is not like any
> >> >> of the dts files for armv8 boards are verbatim copies of what you
> >> >> find in the kernel.
> >> >
> >> > They've gotten out of sync? Sigh..  I suppose this starts to push me
> >> > from the "keep them in the kernel" camp to "push them to a separate
> >> > authoritative repository" camp.
> >>
> >> What's wrong with the standalone DT tree[1] and importing that to
> >> u-boot periodically?
> >>
> >> I know folks would like a completely separate tree that's not "the
> >> Linux DT tree", but I don't see that happening any time soon. Do we
> >> have some Linuxisms in bindings, yes, but in general I think they are
> >> more the exception than rule and were things that went in with little
> >> review. These days I'm reviewing pretty much all bindings (not all dts
> >> files though), so I think it's less of a problem. Logistically, we
> >> could probably work out how to move bindings and dts files to a
> >> standalone repository as I could apply bindings and most dts files go
> >> thru arm-soc maintainers. My biggest concern with a separate
> >> repository is review because we would quickly loose any review that
> >> Linux subsystem maintainers do, and no one is beating down my door to
> >> help be a DT maintainer.
> >
> > Thinking about this, the key word is authoritative.  Right now we say
> > (every time I spot a new dts patch) "is this in the kernel yet?" and the
> > answers break down to:
> > - Yes, see v4.x
> > - Yes, see linux-next $tag (or Yes, see maintainer-tree-$X)
> > - No, we're working on it.
> >
> > To me, the first is great, the second is OK only in that we're probably
> > not relying on anything bleeding edge and likely to change between
> > linux-next $tag and when it finally goes upstream.  The third is where
> > we're at with this board.  And a problem is that even with the short
> > kernel release cycle, there is a lot of not wanting to wait until things
> > get into the next upstream release.
> 
> Maybe it was the 3rd case at the start of this, but it is now in v4.12-rc1.
> 
> Generally, commits in -next are not rebased and should match what ends
> up in mainline. But that's not guaranteed and syncing with -next would
> probably not be the best policy.

Right.  I don't like to take the "it's in -next", but the judgement call
is that it's often going to be fine anyhow.

> > Making a big and possibly wrong assumption, for something like this
> > board, that doesn't introduce any new bindings, shouldn't this dts be
> > able to go in quickly, once it of course is otherwise correct?
> 
> New SoC too, so probably a bit more than just a new assembly of
> existing bindings. Worst case is someone submits this just before the
> merge window, it's pulled in just after N-rc1, and doesn't get tagged
> until O-rc1. In this case, the dts was committed on Apr 6 and rc1 was
> tagged May 13. We could look at doing a filtered tree based off of
> -next perhaps. Perhaps we should wait to see if the latency is really
> a problem.
> 
> >  And
> > U-Boot (and barebox and the kernel and anyone else that cares) doesn't
> > want the tree until it was correct either.
> 
> Exactly.
> 
> >  And once a tree is in this
> > upstream, it's just a matter of saying import dts files for $X, taken
> > from $hash of the device tree repository (or even just included as I
> > might make myself get in the habit of syncing the tree in post release,
> > as it'd be on our release cycle, but honestly, I could / should just do
> > that, even if it's a -rc).
> 
> Usually an -rcX is safe to take, but sometimes bindings do get changed
> during -rc cycle.
> 
> >
> >> [1] git://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git
> >
> > ... also, so it rebases and isn't stable?  That makes life less fun for
> > tracing back when we synced $X last.
> 
> It is generated with git-filter-branch. So it may be regenerated if
> the generation scripts change. As long as you are tracking kernel tags
> as to what you've imported, then it should not matter. I'm not sure if
> we've rebased recently. It was named that somewhat as a CYA. Ian can
> better comment on this.

Well, I suppose the thing here now is that I'm the squeaky wheel, and
other projects seem to be fine.  I'll give things a harder try on my end
and see where we get from there, wrt problems.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170517/f1dce583/attachment.sig>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-08 16:36 ` [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards Jorge Ramirez-Ortiz
  2017-05-08 17:29   ` Tom Rini
@ 2017-05-25 16:08   ` Andreas Färber
  1 sibling, 0 replies; 53+ messages in thread
From: Andreas Färber @ 2017-05-25 16:08 UTC (permalink / raw)
  To: u-boot

Am 08.05.2017 um 18:36 schrieb Jorge Ramirez-Ortiz:
> This port adds support for:
>         1) Serial
>         2) eMMC
>         3) USB
> 
> It has been tested with ARM TRUSTED FIRMWARE running u-boot as the
> BL33 executable [see board's README]
> 
> eMMC has been tested for reading and booting the loader[1] and linux
> kernels as well as saving the u-boot environment.
> 
> USB has been tested with ASIX networking adapter and SanDisk 7.4GB
> drive.
> 
> PSCI has been tested via the reset call.
> 
> The firwmare upgrade process has been tested via TFTP and USB FAT
> filesystem containing the fastboot.bin image in one of the partitions.
> 
> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
> ---
>  arch/arm/Kconfig                                   |  12 ++
>  arch/arm/dts/hi3798cv200.dtsi                      |   3 +
>  arch/arm/dts/poplar-uboot.dtsi                     |  24 +++
>  arch/arm/include/asm/arch-hi3798cv200/dwmmc.h      |  13 ++
>  .../arm/include/asm/arch-hi3798cv200/hi3798cv200.h |  50 +++++
>  board/hisilicon/poplar/Kconfig                     |  15 ++
>  board/hisilicon/poplar/MAINTAINERS                 |   6 +
>  board/hisilicon/poplar/Makefile                    |   7 +
>  board/hisilicon/poplar/README                      | 232 +++++++++++++++++++++
>  board/hisilicon/poplar/poplar.c                    | 155 ++++++++++++++
>  configs/poplar_defconfig                           |  26 +++
>  include/configs/poplar.h                           |  86 ++++++++
>  12 files changed, 629 insertions(+)
>  create mode 100644 arch/arm/dts/poplar-uboot.dtsi
>  create mode 100644 arch/arm/include/asm/arch-hi3798cv200/dwmmc.h
>  create mode 100644 arch/arm/include/asm/arch-hi3798cv200/hi3798cv200.h
>  create mode 100644 board/hisilicon/poplar/Kconfig
>  create mode 100644 board/hisilicon/poplar/MAINTAINERS
>  create mode 100644 board/hisilicon/poplar/Makefile
>  create mode 100644 board/hisilicon/poplar/README
>  create mode 100644 board/hisilicon/poplar/poplar.c
>  create mode 100644 configs/poplar_defconfig
>  create mode 100644 include/configs/poplar.h

Wende an: ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
.git/rebase-apply/patch:237: trailing whitespace.
DRAM DDR3/3L/4 SDRAM interface, maximum 32-bit data width 2 GB
.git/rebase-apply/patch:66: new blank line at EOF.
+
.git/rebase-apply/patch:96: new blank line at EOF.
+
.git/rebase-apply/patch:616: new blank line at EOF.
+
.git/rebase-apply/patch:648: new blank line at EOF.
+
warning: 5 Zeilen fügen Whitespace-Fehler hinzu.

Please address the whitespace warnings.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-17 22:06                         ` Tom Rini
@ 2017-05-25 18:38                           ` Jorge Ramirez
  2017-05-25 20:31                             ` Tom Rini
  0 siblings, 1 reply; 53+ messages in thread
From: Jorge Ramirez @ 2017-05-25 18:38 UTC (permalink / raw)
  To: u-boot

On 05/18/2017 12:06 AM, Tom Rini wrote:
>>>> having platform data.
>>> No, I think we're going for overkill here by not doing serial_pl01x.c as
>>> platform data.  ns16550 does platform data for this already.  This
>>> sounds like the lowest overhead way to get the clock populated and not
>>> have some DT data that's not going to be accepted upstream.
>>>
>> ummmm I am a bit lost at this point, could we recap please?
> Lets update the recap:
> - Please re-submit the dts file, now with whatever form is in v4.12-rc1,
>    saying as such in the commit (if it's just the commit message that
>    changes, that's fine and great).

The DTS file in v4.12-rc2 still does NOT contain the usb node.

==> Should I then not use the DM on USB so I can avoid DTS changes?


> - Please update serial_pl01x.c to be able to get the clock via platform
>    data, update and test your board to confirm it works.

um, It gets tricky;
I can not use platform_data since I can not use SERIAL_DM because the 
device tree does have a UART node which gets picked up.

I will have to disable DM_SERIAL and add some configs in 
include/configs/poplar.h

+#define CONFIG_PL011_SERIAL  1
+#define CONFIG_PL011_CLOCK   75000000
+#define CONFIG_PL01x_PORTS   {(void *) 0xF8B00000,}
+#define CONFIG_CONS_INDEX    0

==> is this acceptable? if not, then what should I do?

> - It'd be awesome if you do, but it won't block your series if you
>    don't, update the rest of the platforms that had been using the
>    "clock" platform to instead use the platform data method.
>
> Thanks!

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-25 18:38                           ` Jorge Ramirez
@ 2017-05-25 20:31                             ` Tom Rini
  2017-05-25 20:55                               ` Jorge Ramirez
  0 siblings, 1 reply; 53+ messages in thread
From: Tom Rini @ 2017-05-25 20:31 UTC (permalink / raw)
  To: u-boot

On Thu, May 25, 2017 at 08:38:47PM +0200, Jorge Ramirez wrote:
> On 05/18/2017 12:06 AM, Tom Rini wrote:
> >>>>having platform data.
> >>>No, I think we're going for overkill here by not doing serial_pl01x.c as
> >>>platform data.  ns16550 does platform data for this already.  This
> >>>sounds like the lowest overhead way to get the clock populated and not
> >>>have some DT data that's not going to be accepted upstream.
> >>>
> >>ummmm I am a bit lost at this point, could we recap please?
> >Lets update the recap:
> >- Please re-submit the dts file, now with whatever form is in v4.12-rc1,
> >   saying as such in the commit (if it's just the commit message that
> >   changes, that's fine and great).
> 
> The DTS file in v4.12-rc2 still does NOT contain the usb node.
> 
> ==> Should I then not use the DM on USB so I can avoid DTS changes?

Well, you can either put it in the -u-boot.dtsi file for the board, and
remove that later once it's upstream.

> >- Please update serial_pl01x.c to be able to get the clock via platform
> >   data, update and test your board to confirm it works.
> 
> um, It gets tricky;
> I can not use platform_data since I can not use SERIAL_DM because
> the device tree does have a UART node which gets picked up.

How about disabling the node in -u-boot.dtsi for the board and then you
can use platform data, I think...  But Rob, this loops us back around to
the first problem too, kind of.  Can we really just not make use of
clock-frequency and the kernel just not use it?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170525/648f9d72/attachment.sig>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-25 20:31                             ` Tom Rini
@ 2017-05-25 20:55                               ` Jorge Ramirez
  2017-05-25 20:58                                 ` Jorge Ramirez
  0 siblings, 1 reply; 53+ messages in thread
From: Jorge Ramirez @ 2017-05-25 20:55 UTC (permalink / raw)
  To: u-boot

On 05/25/2017 10:31 PM, Tom Rini wrote:
> On Thu, May 25, 2017 at 08:38:47PM +0200, Jorge Ramirez wrote:
>> On 05/18/2017 12:06 AM, Tom Rini wrote:
>>>>>> having platform data.
>>>>> No, I think we're going for overkill here by not doing serial_pl01x.c as
>>>>> platform data.  ns16550 does platform data for this already.  This
>>>>> sounds like the lowest overhead way to get the clock populated and not
>>>>> have some DT data that's not going to be accepted upstream.
>>>>>
>>>> ummmm I am a bit lost at this point, could we recap please?
>>> Lets update the recap:
>>> - Please re-submit the dts file, now with whatever form is in v4.12-rc1,
>>>    saying as such in the commit (if it's just the commit message that
>>>    changes, that's fine and great).
>> The DTS file in v4.12-rc2 still does NOT contain the usb node.
>>
>> ==> Should I then not use the DM on USB so I can avoid DTS changes?
> Well, you can either put it in the -u-boot.dtsi file for the board, and
> remove that later once it's upstream.

yes I'll do that. thanks.

>
>>> - Please update serial_pl01x.c to be able to get the clock via platform
>>>    data, update and test your board to confirm it works.
>> um, It gets tricky;
>> I can not use platform_data since I can not use SERIAL_DM because
>> the device tree does have a UART node which gets picked up.
> How about disabling the node in -u-boot.dtsi for the board and then you
> can use platform data,

I dont think that would because CONFIG_OF is enabled for USB
So the kernel dtsi (not the u-boot.dtsi) that contains the uart node 
(without the clock!) will be picked by uboot and the uart will not be 
initialized....


I still think that the simplest solution is to let me merge with the 
kernel's device tree plus this u-boot.dtsi [1] and then just get rid of 
the file when possible (and the source code not the configs) would not 
need to change

https://github.com/ldts/poplar-u-boot/blob/upstream/arch/arm/dts/hi3798cv200-u-boot.dtsi 


> I think...  But Rob, this loops us back around to
> the first problem too, kind of.  Can we really just not make use of
> clock-frequency and the kernel just not use it?
>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-25 20:55                               ` Jorge Ramirez
@ 2017-05-25 20:58                                 ` Jorge Ramirez
  2017-05-25 21:12                                   ` Tom Rini
  0 siblings, 1 reply; 53+ messages in thread
From: Jorge Ramirez @ 2017-05-25 20:58 UTC (permalink / raw)
  To: u-boot

On 05/25/2017 10:55 PM, Jorge Ramirez wrote:
> On 05/25/2017 10:31 PM, Tom Rini wrote:
>> On Thu, May 25, 2017 at 08:38:47PM +0200, Jorge Ramirez wrote:
>>> On 05/18/2017 12:06 AM, Tom Rini wrote:
>>>>>>> having platform data.
>>>>>> No, I think we're going for overkill here by not doing 
>>>>>> serial_pl01x.c as
>>>>>> platform data.  ns16550 does platform data for this already.  This
>>>>>> sounds like the lowest overhead way to get the clock populated 
>>>>>> and not
>>>>>> have some DT data that's not going to be accepted upstream.
>>>>>>
>>>>> ummmm I am a bit lost at this point, could we recap please?
>>>> Lets update the recap:
>>>> - Please re-submit the dts file, now with whatever form is in 
>>>> v4.12-rc1,
>>>>    saying as such in the commit (if it's just the commit message that
>>>>    changes, that's fine and great).
>>> The DTS file in v4.12-rc2 still does NOT contain the usb node.
>>>
>>> ==> Should I then not use the DM on USB so I can avoid DTS changes?
>> Well, you can either put it in the -u-boot.dtsi file for the board, and
>> remove that later once it's upstream. 


yes I'll do that. thanks.

>
>>
>>>> - Please update serial_pl01x.c to be able to get the clock via 
>>>> platform
>>>>    data, update and test your board to confirm it works.
>>> um, It gets tricky;
>>> I can not use platform_data since I can not use SERIAL_DM because
>>> the device tree does have a UART node which gets picked up.
>> How about disabling the node in -u-boot.dtsi for the board and then you
>> can use platform data,
>

I dont think that would because CONFIG_OF is enabled for USB; so the 
kernel dtsi that contains the uart node (without the clock!) will be 
picked by u-boot and the uart will not be initialized properly.
I still think that the simplest solution is to let me merge with the 
kernel's device tree plus this u-boot.dtsi [1];
then just get rid of the file when possible (and NEIHER the source code 
NOR the configs) would need to change

[1] 
https://github.com/ldts/poplar-u-boot/blob/upstream/arch/arm/dts/hi3798cv200-u-boot.dtsi 


>
>> I think...  But Rob, this loops us back around to
>> the first problem too, kind of.  Can we really just not make use of
>> clock-frequency and the kernel just not use it?
>>
>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-25 20:58                                 ` Jorge Ramirez
@ 2017-05-25 21:12                                   ` Tom Rini
  2017-05-25 21:16                                     ` Jorge Ramirez
  2017-05-25 21:21                                     ` Simon Glass
  0 siblings, 2 replies; 53+ messages in thread
From: Tom Rini @ 2017-05-25 21:12 UTC (permalink / raw)
  To: u-boot

On Thu, May 25, 2017 at 10:58:20PM +0200, Jorge Ramirez wrote:
> On 05/25/2017 10:55 PM, Jorge Ramirez wrote:
> >On 05/25/2017 10:31 PM, Tom Rini wrote:
> >>On Thu, May 25, 2017 at 08:38:47PM +0200, Jorge Ramirez wrote:
> >>>On 05/18/2017 12:06 AM, Tom Rini wrote:
> >>>>>>>having platform data.
> >>>>>>No, I think we're going for overkill here by not doing
> >>>>>>serial_pl01x.c as
> >>>>>>platform data.  ns16550 does platform data for this already.  This
> >>>>>>sounds like the lowest overhead way to get the clock
> >>>>>>populated and not
> >>>>>>have some DT data that's not going to be accepted upstream.
> >>>>>>
> >>>>>ummmm I am a bit lost at this point, could we recap please?
> >>>>Lets update the recap:
> >>>>- Please re-submit the dts file, now with whatever form is
> >>>>in v4.12-rc1,
> >>>>   saying as such in the commit (if it's just the commit message that
> >>>>   changes, that's fine and great).
> >>>The DTS file in v4.12-rc2 still does NOT contain the usb node.
> >>>
> >>>==> Should I then not use the DM on USB so I can avoid DTS changes?
> >>Well, you can either put it in the -u-boot.dtsi file for the board, and
> >>remove that later once it's upstream.
> 
> 
> yes I'll do that. thanks.
> 
> >
> >>
> >>>>- Please update serial_pl01x.c to be able to get the clock
> >>>>via platform
> >>>>   data, update and test your board to confirm it works.
> >>>um, It gets tricky;
> >>>I can not use platform_data since I can not use SERIAL_DM because
> >>>the device tree does have a UART node which gets picked up.
> >>How about disabling the node in -u-boot.dtsi for the board and then you
> >>can use platform data,
> >
> 
> I dont think that would because CONFIG_OF is enabled for USB; so the
> kernel dtsi that contains the uart node (without the clock!) will be
> picked by u-boot and the uart will not be initialized properly.
> I still think that the simplest solution is to let me merge with the
> kernel's device tree plus this u-boot.dtsi [1];
> then just get rid of the file when possible (and NEIHER the source
> code NOR the configs) would need to change
> 
> [1] https://github.com/ldts/poplar-u-boot/blob/upstream/arch/arm/dts/hi3798cv200-u-boot.dtsi

Yes, sorry.  [1] needs to be updated to disable uart0 so that you can
use platform data, at least for now.  I do want to talk more with Rob
about the general problem this exposes.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170525/9f96a00f/attachment.sig>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-25 21:12                                   ` Tom Rini
@ 2017-05-25 21:16                                     ` Jorge Ramirez
  2017-05-25 22:08                                       ` Tom Rini
  2017-05-25 21:21                                     ` Simon Glass
  1 sibling, 1 reply; 53+ messages in thread
From: Jorge Ramirez @ 2017-05-25 21:16 UTC (permalink / raw)
  To: u-boot

On 05/25/2017 11:12 PM, Tom Rini wrote:
> On Thu, May 25, 2017 at 10:58:20PM +0200, Jorge Ramirez wrote:
>> On 05/25/2017 10:55 PM, Jorge Ramirez wrote:
>>> On 05/25/2017 10:31 PM, Tom Rini wrote:
>>>> On Thu, May 25, 2017 at 08:38:47PM +0200, Jorge Ramirez wrote:
>>>>> On 05/18/2017 12:06 AM, Tom Rini wrote:
>>>>>>>>> having platform data.
>>>>>>>> No, I think we're going for overkill here by not doing
>>>>>>>> serial_pl01x.c as
>>>>>>>> platform data.  ns16550 does platform data for this already.  This
>>>>>>>> sounds like the lowest overhead way to get the clock
>>>>>>>> populated and not
>>>>>>>> have some DT data that's not going to be accepted upstream.
>>>>>>>>
>>>>>>> ummmm I am a bit lost at this point, could we recap please?
>>>>>> Lets update the recap:
>>>>>> - Please re-submit the dts file, now with whatever form is
>>>>>> in v4.12-rc1,
>>>>>>    saying as such in the commit (if it's just the commit message that
>>>>>>    changes, that's fine and great).
>>>>> The DTS file in v4.12-rc2 still does NOT contain the usb node.
>>>>>
>>>>> ==> Should I then not use the DM on USB so I can avoid DTS changes?
>>>> Well, you can either put it in the -u-boot.dtsi file for the board, and
>>>> remove that later once it's upstream.
>>
>> yes I'll do that. thanks.
>>
>>>>>> - Please update serial_pl01x.c to be able to get the clock
>>>>>> via platform
>>>>>>    data, update and test your board to confirm it works.
>>>>> um, It gets tricky;
>>>>> I can not use platform_data since I can not use SERIAL_DM because
>>>>> the device tree does have a UART node which gets picked up.
>>>> How about disabling the node in -u-boot.dtsi for the board and then you
>>>> can use platform data,
>> I dont think that would because CONFIG_OF is enabled for USB; so the
>> kernel dtsi that contains the uart node (without the clock!) will be
>> picked by u-boot and the uart will not be initialized properly.
>> I still think that the simplest solution is to let me merge with the
>> kernel's device tree plus this u-boot.dtsi [1];
>> then just get rid of the file when possible (and NEIHER the source
>> code NOR the configs) would need to change
>>
>> [1] https://github.com/ldts/poplar-u-boot/blob/upstream/arch/arm/dts/hi3798cv200-u-boot.dtsi
> Yes, sorry.  [1] needs to be updated to disable uart0 so that you can
> use platform data, at least for now.  I do want to talk more with Rob
> about the general problem this exposes.

so you want me to

1) keep the node in 
https://github.com/ldts/poplar-u-boot/blob/upstream/arch/arm/dts/hi3798cv200-u-boot.dtsi 

2) add status=disable
3) then add the platform_data

BUT for the pl011 driver to take the platform_data dont I also have to 
disable CONFIG_OF?

but  if I disable CONFIG_OF then I lose USB_DM

am I wrong?




>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-25 21:12                                   ` Tom Rini
  2017-05-25 21:16                                     ` Jorge Ramirez
@ 2017-05-25 21:21                                     ` Simon Glass
  2017-05-25 22:09                                       ` Tom Rini
  1 sibling, 1 reply; 53+ messages in thread
From: Simon Glass @ 2017-05-25 21:21 UTC (permalink / raw)
  To: u-boot

Hi Tom,

On 25 May 2017 at 15:12, Tom Rini <trini@konsulko.com> wrote:
> On Thu, May 25, 2017 at 10:58:20PM +0200, Jorge Ramirez wrote:
>> On 05/25/2017 10:55 PM, Jorge Ramirez wrote:
>> >On 05/25/2017 10:31 PM, Tom Rini wrote:
>> >>On Thu, May 25, 2017 at 08:38:47PM +0200, Jorge Ramirez wrote:
>> >>>On 05/18/2017 12:06 AM, Tom Rini wrote:
>> >>>>>>>having platform data.
>> >>>>>>No, I think we're going for overkill here by not doing
>> >>>>>>serial_pl01x.c as
>> >>>>>>platform data.  ns16550 does platform data for this already.  This
>> >>>>>>sounds like the lowest overhead way to get the clock
>> >>>>>>populated and not
>> >>>>>>have some DT data that's not going to be accepted upstream.
>> >>>>>>
>> >>>>>ummmm I am a bit lost at this point, could we recap please?
>> >>>>Lets update the recap:
>> >>>>- Please re-submit the dts file, now with whatever form is
>> >>>>in v4.12-rc1,
>> >>>>   saying as such in the commit (if it's just the commit message that
>> >>>>   changes, that's fine and great).
>> >>>The DTS file in v4.12-rc2 still does NOT contain the usb node.
>> >>>
>> >>>==> Should I then not use the DM on USB so I can avoid DTS changes?
>> >>Well, you can either put it in the -u-boot.dtsi file for the board, and
>> >>remove that later once it's upstream.
>>
>>
>> yes I'll do that. thanks.
>>
>> >
>> >>
>> >>>>- Please update serial_pl01x.c to be able to get the clock
>> >>>>via platform
>> >>>>   data, update and test your board to confirm it works.
>> >>>um, It gets tricky;
>> >>>I can not use platform_data since I can not use SERIAL_DM because
>> >>>the device tree does have a UART node which gets picked up.
>> >>How about disabling the node in -u-boot.dtsi for the board and then you
>> >>can use platform data,
>> >
>>
>> I dont think that would because CONFIG_OF is enabled for USB; so the
>> kernel dtsi that contains the uart node (without the clock!) will be
>> picked by u-boot and the uart will not be initialized properly.
>> I still think that the simplest solution is to let me merge with the
>> kernel's device tree plus this u-boot.dtsi [1];
>> then just get rid of the file when possible (and NEIHER the source
>> code NOR the configs) would need to change
>>
>> [1] https://github.com/ldts/poplar-u-boot/blob/upstream/arch/arm/dts/hi3798cv200-u-boot.dtsi
>
> Yes, sorry.  [1] needs to be updated to disable uart0 so that you can
> use platform data, at least for now.  I do want to talk more with Rob
> about the general problem this exposes.

Using platform data because we cannot put a clock frequency in the DT
node seems unfortunate to me. With a little flexibility, DT can be
made to work. But IMO the sort of pedantry makes great the enemy of
good.

Regards,
Simon

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-25 21:16                                     ` Jorge Ramirez
@ 2017-05-25 22:08                                       ` Tom Rini
  2017-05-26  7:28                                         ` Jorge Ramirez
  0 siblings, 1 reply; 53+ messages in thread
From: Tom Rini @ 2017-05-25 22:08 UTC (permalink / raw)
  To: u-boot

On Thu, May 25, 2017 at 11:16:42PM +0200, Jorge Ramirez wrote:
> On 05/25/2017 11:12 PM, Tom Rini wrote:
> >On Thu, May 25, 2017 at 10:58:20PM +0200, Jorge Ramirez wrote:
> >>On 05/25/2017 10:55 PM, Jorge Ramirez wrote:
> >>>On 05/25/2017 10:31 PM, Tom Rini wrote:
> >>>>On Thu, May 25, 2017 at 08:38:47PM +0200, Jorge Ramirez wrote:
> >>>>>On 05/18/2017 12:06 AM, Tom Rini wrote:
> >>>>>>>>>having platform data.
> >>>>>>>>No, I think we're going for overkill here by not doing
> >>>>>>>>serial_pl01x.c as
> >>>>>>>>platform data.  ns16550 does platform data for this already.  This
> >>>>>>>>sounds like the lowest overhead way to get the clock
> >>>>>>>>populated and not
> >>>>>>>>have some DT data that's not going to be accepted upstream.
> >>>>>>>>
> >>>>>>>ummmm I am a bit lost at this point, could we recap please?
> >>>>>>Lets update the recap:
> >>>>>>- Please re-submit the dts file, now with whatever form is
> >>>>>>in v4.12-rc1,
> >>>>>>   saying as such in the commit (if it's just the commit message that
> >>>>>>   changes, that's fine and great).
> >>>>>The DTS file in v4.12-rc2 still does NOT contain the usb node.
> >>>>>
> >>>>>==> Should I then not use the DM on USB so I can avoid DTS changes?
> >>>>Well, you can either put it in the -u-boot.dtsi file for the board, and
> >>>>remove that later once it's upstream.
> >>
> >>yes I'll do that. thanks.
> >>
> >>>>>>- Please update serial_pl01x.c to be able to get the clock
> >>>>>>via platform
> >>>>>>   data, update and test your board to confirm it works.
> >>>>>um, It gets tricky;
> >>>>>I can not use platform_data since I can not use SERIAL_DM because
> >>>>>the device tree does have a UART node which gets picked up.
> >>>>How about disabling the node in -u-boot.dtsi for the board and then you
> >>>>can use platform data,
> >>I dont think that would because CONFIG_OF is enabled for USB; so the
> >>kernel dtsi that contains the uart node (without the clock!) will be
> >>picked by u-boot and the uart will not be initialized properly.
> >>I still think that the simplest solution is to let me merge with the
> >>kernel's device tree plus this u-boot.dtsi [1];
> >>then just get rid of the file when possible (and NEIHER the source
> >>code NOR the configs) would need to change
> >>
> >>[1] https://github.com/ldts/poplar-u-boot/blob/upstream/arch/arm/dts/hi3798cv200-u-boot.dtsi
> >Yes, sorry.  [1] needs to be updated to disable uart0 so that you can
> >use platform data, at least for now.  I do want to talk more with Rob
> >about the general problem this exposes.
> 
> so you want me to
> 
> 1) keep the node in https://github.com/ldts/poplar-u-boot/blob/upstream/arch/arm/dts/hi3798cv200-u-boot.dtsi

Well, a uart0 node, but no "clock" property as that just needs to go
away.

> 2) add status=disable
> 3) then add the platform_data
> 
> BUT for the pl011 driver to take the platform_data dont I also have
> to disable CONFIG_OF?
> 
> but  if I disable CONFIG_OF then I lose USB_DM

No, the status = "disable" on uart0 should remove it from the dtb, or at
least we should see it and go "Oh, no, we don't have uart0 via DT".

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170525/6f8a0334/attachment.sig>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-25 21:21                                     ` Simon Glass
@ 2017-05-25 22:09                                       ` Tom Rini
  0 siblings, 0 replies; 53+ messages in thread
From: Tom Rini @ 2017-05-25 22:09 UTC (permalink / raw)
  To: u-boot

On Thu, May 25, 2017 at 03:21:04PM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On 25 May 2017 at 15:12, Tom Rini <trini@konsulko.com> wrote:
> > On Thu, May 25, 2017 at 10:58:20PM +0200, Jorge Ramirez wrote:
> >> On 05/25/2017 10:55 PM, Jorge Ramirez wrote:
> >> >On 05/25/2017 10:31 PM, Tom Rini wrote:
> >> >>On Thu, May 25, 2017 at 08:38:47PM +0200, Jorge Ramirez wrote:
> >> >>>On 05/18/2017 12:06 AM, Tom Rini wrote:
> >> >>>>>>>having platform data.
> >> >>>>>>No, I think we're going for overkill here by not doing
> >> >>>>>>serial_pl01x.c as
> >> >>>>>>platform data.  ns16550 does platform data for this already.  This
> >> >>>>>>sounds like the lowest overhead way to get the clock
> >> >>>>>>populated and not
> >> >>>>>>have some DT data that's not going to be accepted upstream.
> >> >>>>>>
> >> >>>>>ummmm I am a bit lost at this point, could we recap please?
> >> >>>>Lets update the recap:
> >> >>>>- Please re-submit the dts file, now with whatever form is
> >> >>>>in v4.12-rc1,
> >> >>>>   saying as such in the commit (if it's just the commit message that
> >> >>>>   changes, that's fine and great).
> >> >>>The DTS file in v4.12-rc2 still does NOT contain the usb node.
> >> >>>
> >> >>>==> Should I then not use the DM on USB so I can avoid DTS changes?
> >> >>Well, you can either put it in the -u-boot.dtsi file for the board, and
> >> >>remove that later once it's upstream.
> >>
> >>
> >> yes I'll do that. thanks.
> >>
> >> >
> >> >>
> >> >>>>- Please update serial_pl01x.c to be able to get the clock
> >> >>>>via platform
> >> >>>>   data, update and test your board to confirm it works.
> >> >>>um, It gets tricky;
> >> >>>I can not use platform_data since I can not use SERIAL_DM because
> >> >>>the device tree does have a UART node which gets picked up.
> >> >>How about disabling the node in -u-boot.dtsi for the board and then you
> >> >>can use platform data,
> >> >
> >>
> >> I dont think that would because CONFIG_OF is enabled for USB; so the
> >> kernel dtsi that contains the uart node (without the clock!) will be
> >> picked by u-boot and the uart will not be initialized properly.
> >> I still think that the simplest solution is to let me merge with the
> >> kernel's device tree plus this u-boot.dtsi [1];
> >> then just get rid of the file when possible (and NEIHER the source
> >> code NOR the configs) would need to change
> >>
> >> [1] https://github.com/ldts/poplar-u-boot/blob/upstream/arch/arm/dts/hi3798cv200-u-boot.dtsi
> >
> > Yes, sorry.  [1] needs to be updated to disable uart0 so that you can
> > use platform data, at least for now.  I do want to talk more with Rob
> > about the general problem this exposes.
> 
> Using platform data because we cannot put a clock frequency in the DT
> node seems unfortunate to me. With a little flexibility, DT can be
> made to work. But IMO the sort of pedantry makes great the enemy of
> good.

This, and the alias issue we talked about the other day (wrt mmc) do
highlight what I see as problems of the dts being too kernel-centric.
But I also really want to wait for Rob to chime in before we get too far
here.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170525/46d18186/attachment.sig>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-25 22:08                                       ` Tom Rini
@ 2017-05-26  7:28                                         ` Jorge Ramirez
  2017-05-26  7:46                                           ` Jorge Ramirez
  2017-05-26 12:46                                           ` Tom Rini
  0 siblings, 2 replies; 53+ messages in thread
From: Jorge Ramirez @ 2017-05-26  7:28 UTC (permalink / raw)
  To: u-boot

On 05/26/2017 12:08 AM, Tom Rini wrote:
> On Thu, May 25, 2017 at 11:16:42PM +0200, Jorge Ramirez wrote:
>> On 05/25/2017 11:12 PM, Tom Rini wrote:
>>> On Thu, May 25, 2017 at 10:58:20PM +0200, Jorge Ramirez wrote:
>>>> On 05/25/2017 10:55 PM, Jorge Ramirez wrote:
>>>>> On 05/25/2017 10:31 PM, Tom Rini wrote:
>>>>>> On Thu, May 25, 2017 at 08:38:47PM +0200, Jorge Ramirez wrote:
>>>>>>> On 05/18/2017 12:06 AM, Tom Rini wrote:
>>>>>>>>>>> having platform data.
>>>>>>>>>> No, I think we're going for overkill here by not doing
>>>>>>>>>> serial_pl01x.c as
>>>>>>>>>> platform data.  ns16550 does platform data for this already.  This
>>>>>>>>>> sounds like the lowest overhead way to get the clock
>>>>>>>>>> populated and not
>>>>>>>>>> have some DT data that's not going to be accepted upstream.
>>>>>>>>>>
>>>>>>>>> ummmm I am a bit lost at this point, could we recap please?
>>>>>>>> Lets update the recap:
>>>>>>>> - Please re-submit the dts file, now with whatever form is
>>>>>>>> in v4.12-rc1,
>>>>>>>>    saying as such in the commit (if it's just the commit message that
>>>>>>>>    changes, that's fine and great).
>>>>>>> The DTS file in v4.12-rc2 still does NOT contain the usb node.
>>>>>>>
>>>>>>> ==> Should I then not use the DM on USB so I can avoid DTS changes?
>>>>>> Well, you can either put it in the -u-boot.dtsi file for the board, and
>>>>>> remove that later once it's upstream.
>>>> yes I'll do that. thanks.
>>>>
>>>>>>>> - Please update serial_pl01x.c to be able to get the clock
>>>>>>>> via platform
>>>>>>>>    data, update and test your board to confirm it works.
>>>>>>> um, It gets tricky;
>>>>>>> I can not use platform_data since I can not use SERIAL_DM because
>>>>>>> the device tree does have a UART node which gets picked up.
>>>>>> How about disabling the node in -u-boot.dtsi for the board and then you
>>>>>> can use platform data,
>>>> I dont think that would because CONFIG_OF is enabled for USB; so the
>>>> kernel dtsi that contains the uart node (without the clock!) will be
>>>> picked by u-boot and the uart will not be initialized properly.
>>>> I still think that the simplest solution is to let me merge with the
>>>> kernel's device tree plus this u-boot.dtsi [1];
>>>> then just get rid of the file when possible (and NEIHER the source
>>>> code NOR the configs) would need to change
>>>>
>>>> [1] https://github.com/ldts/poplar-u-boot/blob/upstream/arch/arm/dts/hi3798cv200-u-boot.dtsi
>>> Yes, sorry.  [1] needs to be updated to disable uart0 so that you can
>>> use platform data, at least for now.  I do want to talk more with Rob
>>> about the general problem this exposes.
>> so you want me to
>>
>> 1) keep the node in https://github.com/ldts/poplar-u-boot/blob/upstream/arch/arm/dts/hi3798cv200-u-boot.dtsi
> Well, a uart0 node, but no "clock" property as that just needs to go
> away.

Sounds good to me. now see below

>
>> 2) add status=disable
>> 3) then add the platform_data
>>
>> BUT for the pl011 driver to take the platform_data dont I also have
>> to disable CONFIG_OF?
>>
>> but  if I disable CONFIG_OF then I lose USB_DM
> No, the status = "disable" on uart0 should remove it from the dtb, or at
> least we should see it and go "Oh, no, we don't have uart0 via DT".
>


1) Since the UART0 is enabled in the kernel's DTS I will have to modify 
the device tree that I am importing from kernel.org to disable it.

2) But even doing this is not enough: I have to completely remove the 
uart0 node from the tree.


So to sum up:

In order to get the platform data for pl01x I have to either disable OF 
(so I lose the USB node as I said earlier) or *completely* remove the 
UART0 node from from the kernel dts.
I personally would rather not modify the kernel's DTS trees that I am 
importing into uboot but I am really confused about the policy now.

please could you clarify?

I still think what I proposed when we started is the better way to go: a 
uboot specific hi3798cv200-u-boot.dtsifile that contains the two nodes 
that are giving trouble.

The timeline then goes:
- the usb node will disappear as soon as it lands in korg
- the uart node and the whole file will be removed during the cleanup of 
all the pl01x uart offenders.

but if you think modifying the kernels dts is better I can do that as well.

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-26  7:28                                         ` Jorge Ramirez
@ 2017-05-26  7:46                                           ` Jorge Ramirez
  2017-05-26 12:46                                           ` Tom Rini
  1 sibling, 0 replies; 53+ messages in thread
From: Jorge Ramirez @ 2017-05-26  7:46 UTC (permalink / raw)
  To: u-boot

On 05/26/2017 09:28 AM, Jorge Ramirez wrote:
>>>>>
>>>>> [1] 
>>>>> https://github.com/ldts/poplar-u-boot/blob/upstream/arch/arm/dts/hi3798cv200-u-boot.dtsi 
>>>>>
>>>> Yes, sorry.  [1] needs to be updated to disable uart0 so that you can
>>>> use platform data, at least for now.  I do want to talk more with Rob
>>>> about the general problem this exposes.
>>> so you want me to
>>>
>>> 1) keep the node in 
>>> https://github.com/ldts/poplar-u-boot/blob/upstream/arch/arm/dts/hi3798cv200-u-boot.dtsi 
>>>
>> Well, a uart0 node, but no "clock" property as that just needs to go
>> away.
>
> Sounds good to me. now see below
>
>>
>>> 2) add status=disable
>>> 3) then add the platform_data
>>>
>>> BUT for the pl011 driver to take the platform_data dont I also have
>>> to disable CONFIG_OF?
>>>
>>> but  if I disable CONFIG_OF then I lose USB_DM
>> No, the status = "disable" on uart0 should remove it from the dtb, or at
>> least we should see it and go "Oh, no, we don't have uart0 via DT".
>>
>
>
> 1) Since the UART0 is enabled in the kernel's DTS I will have to 
> modify the device tree that I am importing from kernel.org to disable it.
>
> 2) But even doing this is not enough: I have to completely remove the 
> uart0 node from the tree.
>
>
> So to sum up:
>
> In order to get the platform data for pl01x I have to either disable 
> OF (so I lose the USB node as I said earlier) or *completely* remove 
> the UART0 node from from the kernel dts.
> I personally would rather not modify the kernel's DTS trees that I am 
> importing into uboot but I am really confused about the policy now.

ie: to be clear from my side, doing the following is not enough:

diff --git a/arch/arm/dts/hi3798cv200-poplar.dts 
b/arch/arm/dts/hi3798cv200-poplar.dts
index b914287..d4ce16d 100644
--- a/arch/arm/dts/hi3798cv200-poplar.dts
+++ b/arch/arm/dts/hi3798cv200-poplar.dts
@@ -152,7 +152,7 @@
  };

  &uart0 {
-   status = "okay";
+ status = "disabled";
  };


I have to actually do

diff --git a/arch/arm/dts/hi3798cv200-poplar.dts 
b/arch/arm/dts/hi3798cv200-poplar.dts
index b914287..028013f 100644
--- a/arch/arm/dts/hi3798cv200-poplar.dts
+++ b/arch/arm/dts/hi3798cv200-poplar.dts
@@ -17,7 +17,6 @@
         compatible = "hisilicon,hi3798cv200-poplar", 
"hisilicon,hi3798cv200";

         aliases {
-           serial0 = &uart0;
                 serial2 = &uart2;
         };

@@ -151,12 +150,9 @@
         label = "LS-SPI0";
  };

-&uart0 {
-   status = "okay";
-};

  &uart2 {
         status = "okay";
         label = "LS-UART0";
  };


OR

[jramirez at titan git.u-boot (upstream *$)]$ git diff
diff --git a/arch/arm/dts/hi3798cv200-poplar.dts 
b/arch/arm/dts/hi3798cv200-poplar.dts
index b914287..5d909b83 100644
--- a/arch/arm/dts/hi3798cv200-poplar.dts
+++ b/arch/arm/dts/hi3798cv200-poplar.dts
@@ -21,10 +21,6 @@
                 serial2 = &uart2;
         };

-   chosen {
-           stdout-path = "serial0:115200n8";
-   };
-
         memory at 0 {
                 device_type = "memory";
                 reg = <0x0 0x0 0x0 0x80000000>;
@@ -152,7 +148,7 @@
  };

  &uart0 {
-   status = "okay";
+ status = "disabled";
  };

  &uart2 {



Is this the right way to go then? alter the kernel DTS when merging into 
uboot?

>
> please could you clarify?
>
> I still think what I proposed when we started is the better way to go: 
> a uboot specific hi3798cv200-u-boot.dtsifile that contains the two 
> nodes that are giving trouble.
>
> The timeline then goes:
> - the usb node will disappear as soon as it lands in korg
> - the uart node and the whole file will be removed during the cleanup 
> of all the pl01x uart offenders.
>
> but if you think modifying the kernels dts is better I can do that as 
> well. 

^ permalink raw reply related	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-26  7:28                                         ` Jorge Ramirez
  2017-05-26  7:46                                           ` Jorge Ramirez
@ 2017-05-26 12:46                                           ` Tom Rini
  2017-05-26 12:58                                             ` Jorge Ramirez Ortiz
  1 sibling, 1 reply; 53+ messages in thread
From: Tom Rini @ 2017-05-26 12:46 UTC (permalink / raw)
  To: u-boot

On Fri, May 26, 2017 at 09:28:28AM +0200, Jorge Ramirez wrote:
> On 05/26/2017 12:08 AM, Tom Rini wrote:
> >On Thu, May 25, 2017 at 11:16:42PM +0200, Jorge Ramirez wrote:
> >>On 05/25/2017 11:12 PM, Tom Rini wrote:
> >>>On Thu, May 25, 2017 at 10:58:20PM +0200, Jorge Ramirez wrote:
> >>>>On 05/25/2017 10:55 PM, Jorge Ramirez wrote:
> >>>>>On 05/25/2017 10:31 PM, Tom Rini wrote:
> >>>>>>On Thu, May 25, 2017 at 08:38:47PM +0200, Jorge Ramirez wrote:
> >>>>>>>On 05/18/2017 12:06 AM, Tom Rini wrote:
> >>>>>>>>>>>having platform data.
> >>>>>>>>>>No, I think we're going for overkill here by not doing
> >>>>>>>>>>serial_pl01x.c as
> >>>>>>>>>>platform data.  ns16550 does platform data for this already.  This
> >>>>>>>>>>sounds like the lowest overhead way to get the clock
> >>>>>>>>>>populated and not
> >>>>>>>>>>have some DT data that's not going to be accepted upstream.
> >>>>>>>>>>
> >>>>>>>>>ummmm I am a bit lost at this point, could we recap please?
> >>>>>>>>Lets update the recap:
> >>>>>>>>- Please re-submit the dts file, now with whatever form is
> >>>>>>>>in v4.12-rc1,
> >>>>>>>>   saying as such in the commit (if it's just the commit message that
> >>>>>>>>   changes, that's fine and great).
> >>>>>>>The DTS file in v4.12-rc2 still does NOT contain the usb node.
> >>>>>>>
> >>>>>>>==> Should I then not use the DM on USB so I can avoid DTS changes?
> >>>>>>Well, you can either put it in the -u-boot.dtsi file for the board, and
> >>>>>>remove that later once it's upstream.
> >>>>yes I'll do that. thanks.
> >>>>
> >>>>>>>>- Please update serial_pl01x.c to be able to get the clock
> >>>>>>>>via platform
> >>>>>>>>   data, update and test your board to confirm it works.
> >>>>>>>um, It gets tricky;
> >>>>>>>I can not use platform_data since I can not use SERIAL_DM because
> >>>>>>>the device tree does have a UART node which gets picked up.
> >>>>>>How about disabling the node in -u-boot.dtsi for the board and then you
> >>>>>>can use platform data,
> >>>>I dont think that would because CONFIG_OF is enabled for USB; so the
> >>>>kernel dtsi that contains the uart node (without the clock!) will be
> >>>>picked by u-boot and the uart will not be initialized properly.
> >>>>I still think that the simplest solution is to let me merge with the
> >>>>kernel's device tree plus this u-boot.dtsi [1];
> >>>>then just get rid of the file when possible (and NEIHER the source
> >>>>code NOR the configs) would need to change
> >>>>
> >>>>[1] https://github.com/ldts/poplar-u-boot/blob/upstream/arch/arm/dts/hi3798cv200-u-boot.dtsi
> >>>Yes, sorry.  [1] needs to be updated to disable uart0 so that you can
> >>>use platform data, at least for now.  I do want to talk more with Rob
> >>>about the general problem this exposes.
> >>so you want me to
> >>
> >>1) keep the node in https://github.com/ldts/poplar-u-boot/blob/upstream/arch/arm/dts/hi3798cv200-u-boot.dtsi
> >Well, a uart0 node, but no "clock" property as that just needs to go
> >away.
> 
> Sounds good to me. now see below
> 
> >>2) add status=disable
> >>3) then add the platform_data
> >>
> >>BUT for the pl011 driver to take the platform_data dont I also have
> >>to disable CONFIG_OF?
> >>
> >>but  if I disable CONFIG_OF then I lose USB_DM
> >No, the status = "disable" on uart0 should remove it from the dtb, or at
> >least we should see it and go "Oh, no, we don't have uart0 via DT".
> 
> 1) Since the UART0 is enabled in the kernel's DTS I will have to
> modify the device tree that I am importing from kernel.org to
> disable it.

Yes, the dtsi file in [1] is what modifies it.

> 2) But even doing this is not enough: I have to completely remove
> the uart0 node from the tree.

Why?  Is this in theory, or in practice?  I ask since we just had to
disable the kernel-enabled mmc3 node on am335x-evm.dts in
am335x-evm-u-boot.dtsi in order to have it boot as probing mmc3 in
U-Boot fails (we don't enable the relevant clocks).  So disabling uart0
should cause the resulting dtb file to omit entirely, or just have
&uart0 {status="disabled"} or similar and U-Boot should not do anything,
so platform data should be used.  If this is not the case, there's a bug
we need to track down.

> 
> 
> So to sum up:
> 
> In order to get the platform data for pl01x I have to either disable
> OF (so I lose the USB node as I said earlier) or *completely* remove
> the UART0 node from from the kernel dts.
> I personally would rather not modify the kernel's DTS trees that I
> am importing into uboot but I am really confused about the policy
> now.
> 
> please could you clarify?
> 
> I still think what I proposed when we started is the better way to
> go: a uboot specific hi3798cv200-u-boot.dtsifile that contains the
> two nodes that are giving trouble.

I don't understand what we're not understanding, yes, you should be
using a -u-boot.dtsi file to mark uart0 as disabled and not have to
modify the kernel dts file at all.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170526/b3b44262/attachment.sig>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-26 12:46                                           ` Tom Rini
@ 2017-05-26 12:58                                             ` Jorge Ramirez Ortiz
  2017-05-26 16:09                                               ` Tom Rini
  0 siblings, 1 reply; 53+ messages in thread
From: Jorge Ramirez Ortiz @ 2017-05-26 12:58 UTC (permalink / raw)
  To: u-boot

On May 26, 2017 2:46 PM, "Tom Rini" <trini@konsulko.com> wrote:

On Fri, May 26, 2017 at 09:28:28AM +0200, Jorge Ramirez wrote:
> On 05/26/2017 12:08 AM, Tom Rini wrote:
> >On Thu, May 25, 2017 at 11:16:42PM +0200, Jorge Ramirez wrote:
> >>On 05/25/2017 11:12 PM, Tom Rini wrote:
> >>>On Thu, May 25, 2017 at 10:58:20PM +0200, Jorge Ramirez wrote:
> >>>>On 05/25/2017 10:55 PM, Jorge Ramirez wrote:
> >>>>>On 05/25/2017 10:31 PM, Tom Rini wrote:
> >>>>>>On Thu, May 25, 2017 at 08:38:47PM +0200, Jorge Ramirez wrote:
> >>>>>>>On 05/18/2017 12:06 AM, Tom Rini wrote:
> >>>>>>>>>>>having platform data.
> >>>>>>>>>>No, I think we're going for overkill here by not doing
> >>>>>>>>>>serial_pl01x.c as
> >>>>>>>>>>platform data.  ns16550 does platform data for this already.
This
> >>>>>>>>>>sounds like the lowest overhead way to get the clock
> >>>>>>>>>>populated and not
> >>>>>>>>>>have some DT data that's not going to be accepted upstream.
> >>>>>>>>>>
> >>>>>>>>>ummmm I am a bit lost at this point, could we recap please?
> >>>>>>>>Lets update the recap:
> >>>>>>>>- Please re-submit the dts file, now with whatever form is
> >>>>>>>>in v4.12-rc1,
> >>>>>>>>   saying as such in the commit (if it's just the commit message
that
> >>>>>>>>   changes, that's fine and great).
> >>>>>>>The DTS file in v4.12-rc2 still does NOT contain the usb node.
> >>>>>>>
> >>>>>>>==> Should I then not use the DM on USB so I can avoid DTS changes?
> >>>>>>Well, you can either put it in the -u-boot.dtsi file for the board,
and
> >>>>>>remove that later once it's upstream.
> >>>>yes I'll do that. thanks.
> >>>>
> >>>>>>>>- Please update serial_pl01x.c to be able to get the clock
> >>>>>>>>via platform
> >>>>>>>>   data, update and test your board to confirm it works.
> >>>>>>>um, It gets tricky;
> >>>>>>>I can not use platform_data since I can not use SERIAL_DM because
> >>>>>>>the device tree does have a UART node which gets picked up.
> >>>>>>How about disabling the node in -u-boot.dtsi for the board and then
you
> >>>>>>can use platform data,
> >>>>I dont think that would because CONFIG_OF is enabled for USB; so the
> >>>>kernel dtsi that contains the uart node (without the clock!) will be
> >>>>picked by u-boot and the uart will not be initialized properly.
> >>>>I still think that the simplest solution is to let me merge with the
> >>>>kernel's device tree plus this u-boot.dtsi [1];
> >>>>then just get rid of the file when possible (and NEIHER the source
> >>>>code NOR the configs) would need to change
> >>>>
> >>>>[1] https://github.com/ldts/poplar-u-boot/blob/upstream/
arch/arm/dts/hi3798cv200-u-boot.dtsi
> >>>Yes, sorry.  [1] needs to be updated to disable uart0 so that you can
> >>>use platform data, at least for now.  I do want to talk more with Rob
> >>>about the general problem this exposes.
> >>so you want me to
> >>
> >>1) keep the node in https://github.com/ldts/poplar-u-boot/blob/upstream/
arch/arm/dts/hi3798cv200-u-boot.dtsi
> >Well, a uart0 node, but no "clock" property as that just needs to go
> >away.
>
> Sounds good to me. now see below
>
> >>2) add status=disable
> >>3) then add the platform_data
> >>
> >>BUT for the pl011 driver to take the platform_data dont I also have
> >>to disable CONFIG_OF?
> >>
> >>but  if I disable CONFIG_OF then I lose USB_DM
> >No, the status = "disable" on uart0 should remove it from the dtb, or at
> >least we should see it and go "Oh, no, we don't have uart0 via DT".
>
> 1) Since the UART0 is enabled in the kernel's DTS I will have to
> modify the device tree that I am importing from kernel.org to
> disable it.

Yes, the dtsi file in [1] is what modifies it.

> 2) But even doing this is not enough: I have to completely remove
> the uart0 node from the tree.

Why?  Is this in theory, or in practice?  I ask since we just had to
disable the kernel-enabled mmc3 node on am335x-evm.dts in
am335x-evm-u-boot.dtsi in order to have it boot as probing mmc3 in
U-Boot fails (we don't enable the relevant clocks).  So disabling uart0
should cause the resulting dtb file to omit entirely, or just have
&uart0 {status="disabled"} or similar and U-Boot should not do anything,
so platform data should be used.  If this is not the case, there's a bug
we need to track down.

>
>
> So to sum up:
>
> In order to get the platform data for pl01x I have to either disable
> OF (so I lose the USB node as I said earlier) or *completely* remove
> the UART0 node from from the kernel dts.
> I personally would rather not modify the kernel's DTS trees that I
> am importing into uboot but I am really confused about the policy
> now.
>
> please could you clarify?
>
> I still think what I proposed when we started is the better way to
> go: a uboot specific hi3798cv200-u-boot.dtsifile that contains the
> two nodes that are giving trouble.

I don't understand what we're not understanding, yes, you should be
using a -u-boot.dtsi file to mark uart0 as disabled and not have to
modify the kernel dts file at all.



This the bit that is NOT possible. Doing that is not enough.

I will also have to modify the kernel dts.


--
Tom

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-26 12:58                                             ` Jorge Ramirez Ortiz
@ 2017-05-26 16:09                                               ` Tom Rini
  2017-05-29  9:00                                                 ` Jorge Ramirez
  0 siblings, 1 reply; 53+ messages in thread
From: Tom Rini @ 2017-05-26 16:09 UTC (permalink / raw)
  To: u-boot

On Fri, May 26, 2017 at 02:58:04PM +0200, Jorge Ramirez Ortiz wrote:
> On May 26, 2017 2:46 PM, "Tom Rini" <trini@konsulko.com> wrote:
> 
> On Fri, May 26, 2017 at 09:28:28AM +0200, Jorge Ramirez wrote:
> > On 05/26/2017 12:08 AM, Tom Rini wrote:
> > >On Thu, May 25, 2017 at 11:16:42PM +0200, Jorge Ramirez wrote:
> > >>On 05/25/2017 11:12 PM, Tom Rini wrote:
> > >>>On Thu, May 25, 2017 at 10:58:20PM +0200, Jorge Ramirez wrote:
> > >>>>On 05/25/2017 10:55 PM, Jorge Ramirez wrote:
> > >>>>>On 05/25/2017 10:31 PM, Tom Rini wrote:
> > >>>>>>On Thu, May 25, 2017 at 08:38:47PM +0200, Jorge Ramirez wrote:
> > >>>>>>>On 05/18/2017 12:06 AM, Tom Rini wrote:
> > >>>>>>>>>>>having platform data.
> > >>>>>>>>>>No, I think we're going for overkill here by not doing
> > >>>>>>>>>>serial_pl01x.c as
> > >>>>>>>>>>platform data.  ns16550 does platform data for this already.
> This
> > >>>>>>>>>>sounds like the lowest overhead way to get the clock
> > >>>>>>>>>>populated and not
> > >>>>>>>>>>have some DT data that's not going to be accepted upstream.
> > >>>>>>>>>>
> > >>>>>>>>>ummmm I am a bit lost at this point, could we recap please?
> > >>>>>>>>Lets update the recap:
> > >>>>>>>>- Please re-submit the dts file, now with whatever form is
> > >>>>>>>>in v4.12-rc1,
> > >>>>>>>>   saying as such in the commit (if it's just the commit message
> that
> > >>>>>>>>   changes, that's fine and great).
> > >>>>>>>The DTS file in v4.12-rc2 still does NOT contain the usb node.
> > >>>>>>>
> > >>>>>>>==> Should I then not use the DM on USB so I can avoid DTS changes?
> > >>>>>>Well, you can either put it in the -u-boot.dtsi file for the board,
> and
> > >>>>>>remove that later once it's upstream.
> > >>>>yes I'll do that. thanks.
> > >>>>
> > >>>>>>>>- Please update serial_pl01x.c to be able to get the clock
> > >>>>>>>>via platform
> > >>>>>>>>   data, update and test your board to confirm it works.
> > >>>>>>>um, It gets tricky;
> > >>>>>>>I can not use platform_data since I can not use SERIAL_DM because
> > >>>>>>>the device tree does have a UART node which gets picked up.
> > >>>>>>How about disabling the node in -u-boot.dtsi for the board and then
> you
> > >>>>>>can use platform data,
> > >>>>I dont think that would because CONFIG_OF is enabled for USB; so the
> > >>>>kernel dtsi that contains the uart node (without the clock!) will be
> > >>>>picked by u-boot and the uart will not be initialized properly.
> > >>>>I still think that the simplest solution is to let me merge with the
> > >>>>kernel's device tree plus this u-boot.dtsi [1];
> > >>>>then just get rid of the file when possible (and NEIHER the source
> > >>>>code NOR the configs) would need to change
> > >>>>
> > >>>>[1] https://github.com/ldts/poplar-u-boot/blob/upstream/
> arch/arm/dts/hi3798cv200-u-boot.dtsi
> > >>>Yes, sorry.  [1] needs to be updated to disable uart0 so that you can
> > >>>use platform data, at least for now.  I do want to talk more with Rob
> > >>>about the general problem this exposes.
> > >>so you want me to
> > >>
> > >>1) keep the node in https://github.com/ldts/poplar-u-boot/blob/upstream/
> arch/arm/dts/hi3798cv200-u-boot.dtsi
> > >Well, a uart0 node, but no "clock" property as that just needs to go
> > >away.
> >
> > Sounds good to me. now see below
> >
> > >>2) add status=disable
> > >>3) then add the platform_data
> > >>
> > >>BUT for the pl011 driver to take the platform_data dont I also have
> > >>to disable CONFIG_OF?
> > >>
> > >>but  if I disable CONFIG_OF then I lose USB_DM
> > >No, the status = "disable" on uart0 should remove it from the dtb, or at
> > >least we should see it and go "Oh, no, we don't have uart0 via DT".
> >
> > 1) Since the UART0 is enabled in the kernel's DTS I will have to
> > modify the device tree that I am importing from kernel.org to
> > disable it.
> 
> Yes, the dtsi file in [1] is what modifies it.
> 
> > 2) But even doing this is not enough: I have to completely remove
> > the uart0 node from the tree.
> 
> Why?  Is this in theory, or in practice?  I ask since we just had to
> disable the kernel-enabled mmc3 node on am335x-evm.dts in
> am335x-evm-u-boot.dtsi in order to have it boot as probing mmc3 in
> U-Boot fails (we don't enable the relevant clocks).  So disabling uart0
> should cause the resulting dtb file to omit entirely, or just have
> &uart0 {status="disabled"} or similar and U-Boot should not do anything,
> so platform data should be used.  If this is not the case, there's a bug
> we need to track down.
> 
> >
> >
> > So to sum up:
> >
> > In order to get the platform data for pl01x I have to either disable
> > OF (so I lose the USB node as I said earlier) or *completely* remove
> > the UART0 node from from the kernel dts.
> > I personally would rather not modify the kernel's DTS trees that I
> > am importing into uboot but I am really confused about the policy
> > now.
> >
> > please could you clarify?
> >
> > I still think what I proposed when we started is the better way to
> > go: a uboot specific hi3798cv200-u-boot.dtsifile that contains the
> > two nodes that are giving trouble.
> 
> I don't understand what we're not understanding, yes, you should be
> using a -u-boot.dtsi file to mark uart0 as disabled and not have to
> modify the kernel dts file at all.
> 
> 
> 
> This the bit that is NOT possible. Doing that is not enough.

To be clear, are you trying this on current mainline?  Simon reminded me
that if you don't have 7452946e7f37 in your tree, the -u-boot.dtsi file
cannot disable a node.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170526/da8c1b19/attachment.sig>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-26 16:09                                               ` Tom Rini
@ 2017-05-29  9:00                                                 ` Jorge Ramirez
  2017-05-29 11:57                                                   ` Tom Rini
  0 siblings, 1 reply; 53+ messages in thread
From: Jorge Ramirez @ 2017-05-29  9:00 UTC (permalink / raw)
  To: u-boot

On 05/26/2017 06:09 PM, Tom Rini wrote:
>>> So to sum up:
>>>
>>> In order to get the platform data for pl01x I have to either disable
>>> OF (so I lose the USB node as I said earlier) or*completely*  remove
>>> the UART0 node from from the kernel dts.
>>> I personally would rather not modify the kernel's DTS trees that I
>>> am importing into uboot but I am really confused about the policy
>>> now.
>>>
>>> please could you clarify?
>>>
>>> I still think what I proposed when we started is the better way to
>>> go: a uboot specific hi3798cv200-u-boot.dtsifile that contains the
>>> two nodes that are giving trouble.
>> I don't understand what we're not understanding, yes, you should be
>> using a -u-boot.dtsi file to mark uart0 as disabled and not have to
>> modify the kernel dts file at all.
>>
>>
>>
>> This the bit that is NOT possible. Doing that is not enough.
> To be clear, are you trying this on current mainline?  Simon reminded me
> that if you don't have 7452946e7f37 in your tree, the -u-boot.dtsi file
> cannot disable a node.

yes I have that commit (thanks Tom for checking this)

The issue is actually with serial-uclass.c when the kernel dts contains 
a chosen node that contains the stdout-path.
     chosen {
         stdout-path = "serial0:115200n8";
     };

Disabling uart0 (ie serial0) in u-boot.dtsi loses the console instead of 
probing the pl01x for the platform_data.

is there a pre-defined way to work around this?

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-29  9:00                                                 ` Jorge Ramirez
@ 2017-05-29 11:57                                                   ` Tom Rini
  2017-05-29 12:18                                                     ` Jorge Ramirez
  0 siblings, 1 reply; 53+ messages in thread
From: Tom Rini @ 2017-05-29 11:57 UTC (permalink / raw)
  To: u-boot

On Mon, May 29, 2017 at 11:00:32AM +0200, Jorge Ramirez wrote:
> On 05/26/2017 06:09 PM, Tom Rini wrote:
> >>>So to sum up:
> >>>
> >>>In order to get the platform data for pl01x I have to either disable
> >>>OF (so I lose the USB node as I said earlier) or*completely*  remove
> >>>the UART0 node from from the kernel dts.
> >>>I personally would rather not modify the kernel's DTS trees that I
> >>>am importing into uboot but I am really confused about the policy
> >>>now.
> >>>
> >>>please could you clarify?
> >>>
> >>>I still think what I proposed when we started is the better way to
> >>>go: a uboot specific hi3798cv200-u-boot.dtsifile that contains the
> >>>two nodes that are giving trouble.
> >>I don't understand what we're not understanding, yes, you should be
> >>using a -u-boot.dtsi file to mark uart0 as disabled and not have to
> >>modify the kernel dts file at all.
> >>
> >>
> >>
> >>This the bit that is NOT possible. Doing that is not enough.
> >To be clear, are you trying this on current mainline?  Simon reminded me
> >that if you don't have 7452946e7f37 in your tree, the -u-boot.dtsi file
> >cannot disable a node.
> 
> yes I have that commit (thanks Tom for checking this)
> 
> The issue is actually with serial-uclass.c when the kernel dts
> contains a chosen node that contains the stdout-path.
>     chosen {
>         stdout-path = "serial0:115200n8";
>     };
> 
> Disabling uart0 (ie serial0) in u-boot.dtsi loses the console
> instead of probing the pl01x for the platform_data.
> 
> is there a pre-defined way to work around this?

Provide a new stdout-path in the -u-boot.dtsi too?  Any changes you
could make to the main dts file so that this would work should be able
to be done in the -u-boot.dtsi too.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170529/fcbbf834/attachment.sig>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-29 11:57                                                   ` Tom Rini
@ 2017-05-29 12:18                                                     ` Jorge Ramirez
  2017-05-29 12:23                                                       ` Jorge Ramirez
  2017-05-29 12:26                                                       ` Tom Rini
  0 siblings, 2 replies; 53+ messages in thread
From: Jorge Ramirez @ 2017-05-29 12:18 UTC (permalink / raw)
  To: u-boot

On 05/29/2017 01:57 PM, Tom Rini wrote:
>> The issue is actually with serial-uclass.c when the kernel dts
>> contains a chosen node that contains the stdout-path.
>>      chosen {
>>          stdout-path = "serial0:115200n8";
>>      };
>>
>> Disabling uart0 (ie serial0) in u-boot.dtsi loses the console
>> instead of probing the pl01x for the platform_data.
>>
>> is there a pre-defined way to work around this?
> Provide a new stdout-path in the -u-boot.dtsi too?  Any changes you

yes I obviously tried that but are you sure that that is allowed with 
"chosen" it being kind of a special type of node?
the compiler just cant find the label when added to u-boot.dtsi hence 
why I was asking (sorry, I should have raised it upfront)
ie:

/*
  * U-Boot addition to:
  *  1) use platform data for the console
  *  2) provide support for the generic-ehci USB driver currently not 
available
  *     in the linux kernel (8/May/2017).
  *
  * (C) Copyright 2017 Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
  *
  * SPDX-License-Identifier:     GPL-2.0+
  */

&soc {
     usb2: ehci at 9890000 {
         compatible = "generic-ehci";
         reg = <0x9890000 0x100>;
         status = "okay";
     };
};

&uart0 {
     status = "disabled";
};

&chosen {
     stdout-path = &uart0;

};


leads to

   LD      u-boot
   OBJCOPY u-boot.srec
   OBJCOPY u-boot-nodtb.bin
   SYM     u-boot.sym
start=$(aarch64-linux-gnu-nm u-boot | grep __rel_dyn_start | cut -f 1 -d 
' '); end=$(aarch64-linux-gnu-nm u-boot | grep __rel_dyn_end | cut -f 1 
-d ' '); tools/relocate-rela u-boot-nodtb.bin 0x37000000 $start $end
   DTC     arch/arm/dts/hi3798cv200-poplar.dtb
Error: ./arch/arm/dts/hi3798cv200-u-boot.dtsi:27.2-3 label or path, 
'chosen', not found
FATAL ERROR: Syntax error parsing input tree
scripts/Makefile.lib:322: recipe for target 
'arch/arm/dts/hi3798cv200-poplar.dtb' failed
make[2]: *** [arch/arm/dts/hi3798cv200-poplar.dtb] Error 1
dts/Makefile:32: recipe for target 'arch/arm/dts/hi3798cv200-poplar.dtb' 
failed
make[1]: *** [arch/arm/dts/hi3798cv200-poplar.dtb] Error 2
Makefile:865: recipe for target 'dts/dt.dtb' failed
make: *** [dts/dt.dtb] Error 2

> could make to the main dts file so that this would work should be able
> to be done in the -u-boot.dtsi too.

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-29 12:18                                                     ` Jorge Ramirez
@ 2017-05-29 12:23                                                       ` Jorge Ramirez
  2017-05-29 12:26                                                       ` Tom Rini
  1 sibling, 0 replies; 53+ messages in thread
From: Jorge Ramirez @ 2017-05-29 12:23 UTC (permalink / raw)
  To: u-boot

On 05/29/2017 02:18 PM, Jorge Ramirez wrote:
> On 05/29/2017 01:57 PM, Tom Rini wrote:
>>> The issue is actually with serial-uclass.c when the kernel dts
>>> contains a chosen node that contains the stdout-path.
>>>      chosen {
>>>          stdout-path = "serial0:115200n8";
>>>      };
>>>
>>> Disabling uart0 (ie serial0) in u-boot.dtsi loses the console
>>> instead of probing the pl01x for the platform_data.
>>>
>>> is there a pre-defined way to work around this?
>> Provide a new stdout-path in the -u-boot.dtsi too?  Any changes you
>
> yes I obviously tried that but are you sure that that is allowed with 
> "chosen" it being kind of a special type of node?
> the compiler just cant find the label when added to u-boot.dtsi hence 
> why I was asking (sorry, I should have raised it upfront)
> ie:
>
> /*
>  * U-Boot addition to:
>  *  1) use platform data for the console
>  *  2) provide support for the generic-ehci USB driver currently not 
> available
>  *     in the linux kernel (8/May/2017).
>  *
>  * (C) Copyright 2017 Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
>  *
>  * SPDX-License-Identifier:     GPL-2.0+
>  */
>
> &soc {
>     usb2: ehci at 9890000 {
>         compatible = "generic-ehci";
>         reg = <0x9890000 0x100>;
>         status = "okay";
>     };
> };
>
> &uart0 {
>     status = "disabled";
> };
>
> &chosen {
>     stdout-path = &uart0;
>
> };


oops, wrong syntax. ok done.
will send v5 today

thanks!


>
>
> leads to
>
>   LD      u-boot
>   OBJCOPY u-boot.srec
>   OBJCOPY u-boot-nodtb.bin
>   SYM     u-boot.sym
> start=$(aarch64-linux-gnu-nm u-boot | grep __rel_dyn_start | cut -f 1 
> -d ' '); end=$(aarch64-linux-gnu-nm u-boot | grep __rel_dyn_end | cut 
> -f 1 -d ' '); tools/relocate-rela u-boot-nodtb.bin 0x37000000 $start $end
>   DTC     arch/arm/dts/hi3798cv200-poplar.dtb
> Error: ./arch/arm/dts/hi3798cv200-u-boot.dtsi:27.2-3 label or path, 
> 'chosen', not found
> FATAL ERROR: Syntax error parsing input tree
> scripts/Makefile.lib:322: recipe for target 
> 'arch/arm/dts/hi3798cv200-poplar.dtb' failed
> make[2]: *** [arch/arm/dts/hi3798cv200-poplar.dtb] Error 1
> dts/Makefile:32: recipe for target 
> 'arch/arm/dts/hi3798cv200-poplar.dtb' failed
> make[1]: *** [arch/arm/dts/hi3798cv200-poplar.dtb] Error 2
> Makefile:865: recipe for target 'dts/dt.dtb' failed
> make: *** [dts/dt.dtb] Error 2
>
>> could make to the main dts file so that this would work should be able
>> to be done in the -u-boot.dtsi too.
>
>
>
>
>
>
>
>
>
>
>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-29 12:18                                                     ` Jorge Ramirez
  2017-05-29 12:23                                                       ` Jorge Ramirez
@ 2017-05-29 12:26                                                       ` Tom Rini
  2017-05-29 12:28                                                         ` Jorge Ramirez
  1 sibling, 1 reply; 53+ messages in thread
From: Tom Rini @ 2017-05-29 12:26 UTC (permalink / raw)
  To: u-boot

On Mon, May 29, 2017 at 02:18:48PM +0200, Jorge Ramirez wrote:
> On 05/29/2017 01:57 PM, Tom Rini wrote:
> >>The issue is actually with serial-uclass.c when the kernel dts
> >>contains a chosen node that contains the stdout-path.
> >>     chosen {
> >>         stdout-path = "serial0:115200n8";
> >>     };
> >>
> >>Disabling uart0 (ie serial0) in u-boot.dtsi loses the console
> >>instead of probing the pl01x for the platform_data.
> >>
> >>is there a pre-defined way to work around this?
> >Provide a new stdout-path in the -u-boot.dtsi too?  Any changes you
> 
> yes I obviously tried that but are you sure that that is allowed
> with "chosen" it being kind of a special type of node?
> the compiler just cant find the label when added to u-boot.dtsi
> hence why I was asking (sorry, I should have raised it upfront)
> ie:
> 
> /*
>  * U-Boot addition to:
>  *  1) use platform data for the console
>  *  2) provide support for the generic-ehci USB driver currently not
> available
>  *     in the linux kernel (8/May/2017).
>  *
>  * (C) Copyright 2017 Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
>  *
>  * SPDX-License-Identifier:     GPL-2.0+
>  */
> 
> &soc {
>     usb2: ehci at 9890000 {
>         compatible = "generic-ehci";
>         reg = <0x9890000 0x100>;
>         status = "okay";
>     };
> };
> 
> &uart0 {
>     status = "disabled";
> };
> 
> &chosen {
>     stdout-path = &uart0;
> 
> };
> 
> 
> leads to
> 
>   LD      u-boot
>   OBJCOPY u-boot.srec
>   OBJCOPY u-boot-nodtb.bin
>   SYM     u-boot.sym
> start=$(aarch64-linux-gnu-nm u-boot | grep __rel_dyn_start | cut -f
> 1 -d ' '); end=$(aarch64-linux-gnu-nm u-boot | grep __rel_dyn_end |
> cut -f 1 -d ' '); tools/relocate-rela u-boot-nodtb.bin 0x37000000
> $start $end
>   DTC     arch/arm/dts/hi3798cv200-poplar.dtb
> Error: ./arch/arm/dts/hi3798cv200-u-boot.dtsi:27.2-3 label or path,
> 'chosen', not found
> FATAL ERROR: Syntax error parsing input tree
> scripts/Makefile.lib:322: recipe for target
> 'arch/arm/dts/hi3798cv200-poplar.dtb' failed
> make[2]: *** [arch/arm/dts/hi3798cv200-poplar.dtb] Error 1
> dts/Makefile:32: recipe for target
> 'arch/arm/dts/hi3798cv200-poplar.dtb' failed
> make[1]: *** [arch/arm/dts/hi3798cv200-poplar.dtb] Error 2
> Makefile:865: recipe for target 'dts/dt.dtb' failed
> make: *** [dts/dt.dtb] Error 2
> 

Well, lets see.  Pantelis has been telling me that what we're doing here
is going to lead to problems like what you see here in some cases.  Any
suggestions?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170529/4a795671/attachment.sig>

^ permalink raw reply	[flat|nested] 53+ messages in thread

* [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards
  2017-05-29 12:26                                                       ` Tom Rini
@ 2017-05-29 12:28                                                         ` Jorge Ramirez
  0 siblings, 0 replies; 53+ messages in thread
From: Jorge Ramirez @ 2017-05-29 12:28 UTC (permalink / raw)
  To: u-boot

On 05/29/2017 02:26 PM, Tom Rini wrote:
> On Mon, May 29, 2017 at 02:18:48PM +0200, Jorge Ramirez wrote:
>> On 05/29/2017 01:57 PM, Tom Rini wrote:
>>>> The issue is actually with serial-uclass.c when the kernel dts
>>>> contains a chosen node that contains the stdout-path.
>>>>      chosen {
>>>>          stdout-path = "serial0:115200n8";
>>>>      };
>>>>
>>>> Disabling uart0 (ie serial0) in u-boot.dtsi loses the console
>>>> instead of probing the pl01x for the platform_data.
>>>>
>>>> is there a pre-defined way to work around this?
>>> Provide a new stdout-path in the -u-boot.dtsi too?  Any changes you
>> yes I obviously tried that but are you sure that that is allowed
>> with "chosen" it being kind of a special type of node?
>> the compiler just cant find the label when added to u-boot.dtsi
>> hence why I was asking (sorry, I should have raised it upfront)
>> ie:
>>
>> /*
>>   * U-Boot addition to:
>>   *  1) use platform data for the console
>>   *  2) provide support for the generic-ehci USB driver currently not
>> available
>>   *     in the linux kernel (8/May/2017).
>>   *
>>   * (C) Copyright 2017 Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>
>>   *
>>   * SPDX-License-Identifier:     GPL-2.0+
>>   */
>>
>> &soc {
>>      usb2: ehci at 9890000 {
>>          compatible = "generic-ehci";
>>          reg = <0x9890000 0x100>;
>>          status = "okay";
>>      };
>> };
>>
>> &uart0 {
>>      status = "disabled";
>> };
>>
>> &chosen {
>>      stdout-path = &uart0;
>>
>> };
>>
>>
>> leads to
>>
>>    LD      u-boot
>>    OBJCOPY u-boot.srec
>>    OBJCOPY u-boot-nodtb.bin
>>    SYM     u-boot.sym
>> start=$(aarch64-linux-gnu-nm u-boot | grep __rel_dyn_start | cut -f
>> 1 -d ' '); end=$(aarch64-linux-gnu-nm u-boot | grep __rel_dyn_end |
>> cut -f 1 -d ' '); tools/relocate-rela u-boot-nodtb.bin 0x37000000
>> $start $end
>>    DTC     arch/arm/dts/hi3798cv200-poplar.dtb
>> Error: ./arch/arm/dts/hi3798cv200-u-boot.dtsi:27.2-3 label or path,
>> 'chosen', not found
>> FATAL ERROR: Syntax error parsing input tree
>> scripts/Makefile.lib:322: recipe for target
>> 'arch/arm/dts/hi3798cv200-poplar.dtb' failed
>> make[2]: *** [arch/arm/dts/hi3798cv200-poplar.dtb] Error 1
>> dts/Makefile:32: recipe for target
>> 'arch/arm/dts/hi3798cv200-poplar.dtb' failed
>> make[1]: *** [arch/arm/dts/hi3798cv200-poplar.dtb] Error 2
>> Makefile:865: recipe for target 'dts/dt.dtb' failed
>> make: *** [dts/dt.dtb] Error 2
>>
> Well, lets see.  Pantelis has been telling me that what we're doing here
> is going to lead to problems like what you see here in some cases.  Any
> suggestions?
>

sorry Tom, this is the right syntax for "chosen" hi3798cv200-u-boot.dtsi
uart0 is now initialized and functional via platform data

&soc {
     usb2: ehci at 9890000 {
         compatible = "generic-ehci";
         reg = <0x9890000 0x100>;
         status = "okay";
     };
};

&uart0 {
     status = "disabled";
};

/{
     chosen {
         stdout-path = "";
     };
};

^ permalink raw reply	[flat|nested] 53+ messages in thread

end of thread, other threads:[~2017-05-29 12:28 UTC | newest]

Thread overview: 53+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-08 16:36 [U-Boot] Patchset v4 Poplar 96Boards EE Jorge Ramirez-Ortiz
2017-05-08 16:36 ` [U-Boot] [PATCHv4 1/3] ARM64: dts: hi3798cv200-poplar: add device tree bindings Jorge Ramirez-Ortiz
2017-05-08 16:36 ` [U-Boot] [PATCHv4 2/3] driver: mmc: update debug info Jorge Ramirez-Ortiz
2017-05-08 16:36 ` [U-Boot] [PATCHv4 3/3] ARM64: poplar: hi3798cv200: u-boot support for Poplar 96Boards Jorge Ramirez-Ortiz
2017-05-08 17:29   ` Tom Rini
2017-05-08 18:36     ` Jorge Ramirez
2017-05-08 21:37       ` Tom Rini
2017-05-09 15:27     ` Jorge Ramirez
2017-05-10  2:30       ` Tom Rini
2017-05-10  9:33         ` Jorge Ramirez
2017-05-10 14:49           ` Tom Rini
2017-05-10 16:33             ` Rob Herring
2017-05-10 17:45               ` Tom Rini
2017-05-10 18:09                 ` Simon Glass
2017-05-10 19:09                 ` Rob Herring
2017-05-10 19:42                   ` Simon Glass
2017-05-10 21:19                     ` Jorge Ramirez
2017-05-10 21:31                       ` Simon Glass
2017-05-11 11:45                         ` Jorge Ramirez
2017-05-11 12:35                     ` Tom Rini
2017-05-11 14:34                       ` Jorge Ramirez
2017-05-11 22:32                         ` Tom Rini
2017-05-12  8:16                           ` Jorge Ramirez
2017-05-12 12:35                             ` Tom Rini
2017-05-15 21:38                               ` Rob Herring
2017-05-17 13:33                                 ` Tom Rini
2017-05-17 14:38                                   ` Rob Herring
2017-05-17 22:13                                     ` Tom Rini
2017-05-17 22:06                         ` Tom Rini
2017-05-25 18:38                           ` Jorge Ramirez
2017-05-25 20:31                             ` Tom Rini
2017-05-25 20:55                               ` Jorge Ramirez
2017-05-25 20:58                                 ` Jorge Ramirez
2017-05-25 21:12                                   ` Tom Rini
2017-05-25 21:16                                     ` Jorge Ramirez
2017-05-25 22:08                                       ` Tom Rini
2017-05-26  7:28                                         ` Jorge Ramirez
2017-05-26  7:46                                           ` Jorge Ramirez
2017-05-26 12:46                                           ` Tom Rini
2017-05-26 12:58                                             ` Jorge Ramirez Ortiz
2017-05-26 16:09                                               ` Tom Rini
2017-05-29  9:00                                                 ` Jorge Ramirez
2017-05-29 11:57                                                   ` Tom Rini
2017-05-29 12:18                                                     ` Jorge Ramirez
2017-05-29 12:23                                                       ` Jorge Ramirez
2017-05-29 12:26                                                       ` Tom Rini
2017-05-29 12:28                                                         ` Jorge Ramirez
2017-05-25 21:21                                     ` Simon Glass
2017-05-25 22:09                                       ` Tom Rini
2017-05-10 17:49               ` Jorge Ramirez
2017-05-10 18:21                 ` Rob Herring
2017-05-10 18:37                   ` Jorge Ramirez
2017-05-25 16:08   ` Andreas Färber

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.