devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v11 04/19] sh-sci: Add h8300 SCI
       [not found] ` <1431097479-21101-1-git-send-email-ysato-Rn4VEauK+AKRv+LV9MX5uooqe+aC9MnS@public.gmane.org>
@ 2015-05-08 15:04   ` Yoshinori Sato
  0 siblings, 0 replies; 4+ messages in thread
From: Yoshinori Sato @ 2015-05-08 15:04 UTC (permalink / raw)
  To: linux-sh-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arch-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: Yoshinori Sato

Signed-off-by: Yoshinori Sato <ysato-Rn4VEauK+AKRv+LV9MX5uooqe+aC9MnS@public.gmane.org>
Acked-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
---
 Documentation/devicetree/bindings/serial/renesas,sci-serial.txt | 1 +
 drivers/tty/serial/Kconfig                                      | 2 +-
 drivers/tty/serial/sh-sci.c                                     | 6 ++++++
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
index ae73bb0..7534d46 100644
--- a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
+++ b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
@@ -29,6 +29,7 @@ Required properties:
     - "renesas,scifa" for generic SCIFA compatible UART.
     - "renesas,scifb" for generic SCIFB compatible UART.
     - "renesas,hscif" for generic HSCIF compatible UART.
+    - "renesas,sci" for generic SCI compatible UART.
 
     When compatible with the generic version, nodes must list the
     SoC-specific version corresponding to the platform first followed by the
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index f8120c1..dea1eff 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -728,7 +728,7 @@ config SERIAL_IP22_ZILOG_CONSOLE
 
 config SERIAL_SH_SCI
 	tristate "SuperH SCI(F) serial port support"
-	depends on SUPERH || ARCH_SHMOBILE || COMPILE_TEST
+	depends on SUPERH || ARCH_SHMOBILE || H8300 || COMPILE_TEST
 	select SERIAL_CORE
 
 config SERIAL_SH_SCI_NR_UARTS
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index e7d6566..1468ec5 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -2536,6 +2536,12 @@ static const struct of_device_id of_sci_match[] = {
 			.regtype = SCIx_HSCIF_REGTYPE,
 		},
 	}, {
+		.compatible = "renesas,sci",
+		.data = &(const struct sci_port_info) {
+			.type = PORT_SCI,
+			.regtype = SCIx_SCI_REGTYPE,
+		},
+	}, {
 		/* Terminator */
 	},
 };
-- 
2.1.4

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

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

* [PATCH v11 19/19] h8300: devicetree source
       [not found] <1431097479-21101-1-git-send-email-ysato@users.sourceforge.jp>
       [not found] ` <1431097479-21101-1-git-send-email-ysato-Rn4VEauK+AKRv+LV9MX5uooqe+aC9MnS@public.gmane.org>
@ 2015-05-08 15:04 ` Yoshinori Sato
  2015-05-08 16:50   ` Mark Rutland
  1 sibling, 1 reply; 4+ messages in thread
From: Yoshinori Sato @ 2015-05-08 15:04 UTC (permalink / raw)
  To: linux-kernel, linux-arch, devicetree; +Cc: Yoshinori Sato

Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>
---
 Documentation/devicetree/bindings/h8300/cpu.txt    |  17 ++++
 .../interrupt-controller/renesas,h8300h-intc.txt   |  20 ++++
 .../interrupt-controller/renesas,h8s-intc.txt      |  20 ++++
 arch/h8300/boot/dts/Makefile                       |  11 +++
 arch/h8300/boot/dts/edosk2674.dts                  | 109 +++++++++++++++++++++
 arch/h8300/boot/dts/h8300h_sim.dts                 |  95 ++++++++++++++++++
 arch/h8300/boot/dts/h8s_sim.dts                    | 101 +++++++++++++++++++
 7 files changed, 373 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/h8300/cpu.txt
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,h8300h-intc.txt
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,h8s-intc.txt
 create mode 100644 arch/h8300/boot/dts/Makefile
 create mode 100644 arch/h8300/boot/dts/edosk2674.dts
 create mode 100644 arch/h8300/boot/dts/h8300h_sim.dts
 create mode 100644 arch/h8300/boot/dts/h8s_sim.dts

diff --git a/Documentation/devicetree/bindings/h8300/cpu.txt b/Documentation/devicetree/bindings/h8300/cpu.txt
new file mode 100644
index 0000000..f1287e0
--- /dev/null
+++ b/Documentation/devicetree/bindings/h8300/cpu.txt
@@ -0,0 +1,17 @@
+* H8/300 CPU bindings
+
+Required properties:
+
+- compatible: Compatible property value should be "renesas,h8300".
+- reg: Contains CPU index.
+- clock-frequency: Contains the clock frequency for CPU, in Hz.
+- renesas,bus-width: Contain the memory bus width.
+
+Example:
+
+		cpu@0 {
+			compatible = "renesas,h8300";
+			reg = <0>;
+			clock-frequency = <20000000>;
+			renesas,bus-width = <16>;
+		};
diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,h8300h-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/renesas,h8300h-intc.txt
new file mode 100644
index 0000000..79053dd
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,h8300h-intc.txt
@@ -0,0 +1,20 @@
+* H8/300H Interrupt controller
+
+Required properties:
+
+- compatible: has to be "renesas,h8300h-intc", "renesas,h8300-intc" as fallback.
+- #interrupt-cells: has to be <1>: an interrupt index and flags, as defined in
+  interrupts.txt in this directory
+
+Optional properties:
+
+- any properties, listed in interrupts.txt, and any standard resource allocation
+  properties
+
+Example:
+
+	h8intc: intc@0 {
+		compatible = "renesas,h8300h-intc", "renesas,h8300-intc";
+		#interrupt-cells = <1>;
+		interrupt-controller;
+	};
diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,h8s-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/renesas,h8s-intc.txt
new file mode 100644
index 0000000..1206191
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,h8s-intc.txt
@@ -0,0 +1,20 @@
+* H8S Interrupt controller
+
+Required properties:
+
+- compatible: has to be "renesas,h8s-intc", "renesas,h8300-intc" as fallback.
+- #interrupt-cells: has to be <1>: an interrupt index and flags, as defined in
+  interrupts.txt in this directory
+
+Optional properties:
+
+- any properties, listed in interrupts.txt, and any standard resource allocation
+  properties
+
+Example:
+
+	h8intc: intc@0 {
+		compatible = "renesas,h8s-intc", "renesas,h8300-intc";
+		#interrupt-cells = <1>;
+		interrupt-controller;
+	};
diff --git a/arch/h8300/boot/dts/Makefile b/arch/h8300/boot/dts/Makefile
new file mode 100644
index 0000000..bb123fa
--- /dev/null
+++ b/arch/h8300/boot/dts/Makefile
@@ -0,0 +1,11 @@
+ifneq '$(CONFIG_H8300_BUILTIN_DTB)' '""'
+BUILTIN_DTB := $(patsubst "%",%,$(CONFIG_H8300_BUILTIN_DTB)).dtb.o
+endif
+
+obj-y += $(BUILTIN_DTB)
+
+dtb-$(CONFIG_H8300H_SIM) := h8300h_sim.dtb
+dtb-$(CONFIG_H8S_SIM) := h8s_sim.dtb
+dtb-$(CONFIG_EDOSK2674) := edosk2674.dtb
+
+clean-files := *.dtb.S
diff --git a/arch/h8300/boot/dts/edosk2674.dts b/arch/h8300/boot/dts/edosk2674.dts
new file mode 100644
index 0000000..60e73b9
--- /dev/null
+++ b/arch/h8300/boot/dts/edosk2674.dts
@@ -0,0 +1,109 @@
+#include <dt-bindings/clock/renesas,8bit-timer.h>
+
+/dts-v1/;
+/ {
+	compatible = "renesas,edosk2674";
+	#address-cells = <1>;
+	#size-cells = <1>;
+	interrupt-parent = <&h8intc>;
+
+	chosen {
+		bootargs = "console=ttySC2,38400";
+	};
+	aliases {
+		serial0 = &sci0;
+		serial1 = &sci1;
+		serial2 = &sci2;
+	};
+
+	clocks {
+		ranges;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		xclk: oscillator {
+			#clock-cells = <0>;
+			compatible = "fixed-clock";
+			clock-frequency = <33333333>;
+			clock-output-names = "xtal";
+		};
+		pllclk: pllclk {
+			compatible = "renesas,h8s2678-pll-clock";
+			clocks = <&xclk>;
+			#clock-cells = <0>;
+			reg = <0xfee03b 2>, <0xfee045 2>;
+		};
+		cclk: cclk {
+			compatible = "renesas,h8300-div-clock";
+			clocks = <&pllclk>;
+			#clock-cells = <0>;
+			reg = <0xfee03b 2>;
+			renesas,width = <3>;
+		};
+		fclk: fclk {
+			compatible = "fixed-factor-clock";
+			clocks = <&cclk>;
+			#clock-cells = <0>;
+			clock-div = <1>;
+			clock-mult = <1>;
+		};
+	};
+
+	memory@0 {
+		device_type = "memory";
+		reg = <0x400000 0x800000>;
+	};
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		cpu@0 {
+			compatible = "renesas,h8300";
+			reg = <0>;
+			renesas,bus-width = <16>;
+			clock-frequency = <33333333>;
+			bus-width = <16>;
+		};
+	};
+
+	h8intc: interrupt-controller@0 {
+		compatible = "renesas,h8s-intc", "renesas,h8300-intc";
+		#interrupt-cells = <1>;
+		interrupt-controller;
+	};
+	tpu: timer@ffffe0 {
+		compatible = "renesas,tpu";
+		reg = <0xffffe0 16>, <0xfffff0 12>;
+		clocks = <&fclk>;
+		clock-names = "fck";
+	};
+
+	timer8: timer@ffffb0 {
+		compatible = "renesas,8bit-timer";
+		reg = <0xffff80 10>;
+		interrupts = <72 75>;
+		clocks = <&fclk>;
+		clock-names = "fck";
+	};
+
+	sci0: serial@ffff78 {
+		compatible = "renesas,sci";
+		reg = <0xffff78 8>;
+		interrupts = <88 89 90 91>;
+		clocks = <&fclk>;
+		clock-names = "sci_ick";
+	};
+	sci1: serial@ffff80 {
+		compatible = "renesas,sci";
+		reg = <0xffff80 8>;
+		interrupts = <92 93 94 95>;
+		clocks = <&fclk>;
+		clock-names = "sci_ick";
+	};
+	sci2: serial@ffff88 {
+		compatible = "renesas,sci";
+		reg = <0xffff88 8>;
+		interrupts = <96 97 98 99>;
+		clocks = <&fclk>;
+		clock-names = "sci_ick";
+	};
+};
diff --git a/arch/h8300/boot/dts/h8300h_sim.dts b/arch/h8300/boot/dts/h8300h_sim.dts
new file mode 100644
index 0000000..08c3e35
--- /dev/null
+++ b/arch/h8300/boot/dts/h8300h_sim.dts
@@ -0,0 +1,95 @@
+#include <dt-bindings/clock/renesas,8bit-timer.h>
+
+/dts-v1/;
+/ {
+	compatible = "gnu,gdbsim";
+	#address-cells = <1>;
+	#size-cells = <1>;
+	interrupt-parent = <&h8intc>;
+
+	chosen {
+		bootargs = "console=ttySC0";
+	};
+	aliases {
+		serial0 = &sci0;
+		serial1 = &sci1;
+	};
+
+	clocks {
+		ranges;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		xclk: oscillator {
+			#clock-cells = <0>;
+			compatible = "fixed-clock";
+			clock-frequency = <20000000>;
+			clock-output-names = "xtal";
+		};
+		cclk: cclk {
+			compatible = "renesas,h8300-div-clock";
+			clocks = <&xclk>;
+			#clock-cells = <0>;
+			reg = <0xfee01b 2>;
+			renesas,width = <2>;
+		};
+		fclk: fclk {
+			compatible = "fixed-factor-clock";
+			clocks = <&cclk>;
+			#clock-cells = <0>;
+			clock-div = <1>;
+			clock-mult = <1>;
+		};
+	};
+
+	memory@0 {
+		device_type = "memory";
+		reg = <0x400000 0x400000>;
+	};
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		cpu@0 {
+			compatible = "renesas,h8300";
+			reg = <0>;
+			clock-frequency = <20000000>;
+			renesas,bus-width = <16>;
+		};
+	};
+
+	h8intc: interrupt-controller@0 {
+		compatible = "renesas,h8300h-intc", "renesas,h8300-intc";
+		#interrupt-cells = <1>;
+		interrupt-controller;
+	};
+	timer8: timer@ffff80 {
+		compatible = "renesas,8bit-timer";
+		reg = <0xffff80 10>;
+		interrupts = <36>;
+		clocks = <&fclk>;
+		clock-names = "fck";
+	};
+	timer16: timer@ffff68 {
+		compatible = "renesas,16bit-timer";
+		reg = <0xffff68 8>, <0xffff60 8>;
+		interrupts = <24>;
+		renesas,channel = <0>;
+		clocks = <&fclk>;
+		clock-names = "fck";
+	};
+
+	sci0: serial@ffffb0 {
+		compatible = "renesas,sci";
+		reg = <0xffffb0 8>;
+		interrupts = <52 53 54 55>;
+		clocks = <&fclk>;
+		clock-names = "sci_ick";
+	};
+	sci1: serial@ffffb8 {
+		compatible = "renesas,sci";
+		reg = <0xffffb8 8>;
+		interrupts = <56 57 58 59>;
+		clocks = <&fclk>;
+		clock-names = "sci_ick";
+	};
+};
diff --git a/arch/h8300/boot/dts/h8s_sim.dts b/arch/h8300/boot/dts/h8s_sim.dts
new file mode 100644
index 0000000..4f93eb6
--- /dev/null
+++ b/arch/h8300/boot/dts/h8s_sim.dts
@@ -0,0 +1,101 @@
+#include <dt-bindings/clock/renesas,8bit-timer.h>
+
+/dts-v1/;
+/ {
+	compatible = "gnu,gdbsim";
+	#address-cells = <1>;
+	#size-cells = <1>;
+	interrupt-parent = <&h8intc>;
+
+	chosen {
+		bootargs = "earlyprintk=h8300-sim console=ttySC0";
+	};
+	aliases {
+		serial0 = &sci0;
+		serial1 = &sci1;
+	};
+
+	clocks {
+		ranges;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		xclk: oscillator {
+			#clock-cells = <0>;
+			compatible = "fixed-clock";
+			clock-frequency = <33333333>;
+			clock-output-names = "xtal";
+		};
+		pllclk: pllclk {
+			compatible = "renesas,h8s2678-pll-clock";
+			clocks = <&xclk>;
+			#clock-cells = <0>;
+			reg = <0xfee03b 2>, <0xfee045 2>;
+		};
+		cclk: cclk {
+			compatible = "renesas,h8300-div-clock";
+			clocks = <&pllclk>;
+			#clock-cells = <0>;
+			reg = <0xfee03b 2>;
+			renesas,width = <3>;
+		};
+		fclk: fclk {
+			compatible = "fixed-factor-clock";
+			clocks = <&cclk>;
+			#clock-cells = <0>;
+			clock-div = <1>;
+			clock-mult = <1>;
+		};
+	};
+
+	memory@0 {
+		device_type = "memory";
+		reg = <0x400000 0x800000>;
+	};
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		cpu@0 {
+			compatible = "renesas,h8300";
+			reg = <0>;
+			renesas,bus-width = <16>;
+			clock-frequency = <33333333>;
+			bus-width = <16>;
+		};
+	};
+
+	h8intc: interrupt_controler@0 {
+		compatible = "renesas,h8s-intc", "renesas,h8300-intc";
+		#interrupt-cells = <1>;
+		interrupt-controller;
+	};
+	tpu: timer@ffffe0 {
+		compatible = "renesas,tpu";
+		reg = <0xffffe0 16>, <0xfffff0 12>;
+		clocks = <&fclk>;
+		clock-names = "fck";
+	};
+
+	timer8: timer@ffffb0 {
+		compatible = "renesas,8bit-timer";
+		reg = <0xffff80 10>;
+		interrupts = <72 75>;
+		clocks = <&fclk>;
+		clock-names = "fck";
+	};
+
+	sci0: serial@ffff78 {
+		compatible = "renesas,sci";
+		reg = <0xffff78 8>;
+		interrupts = <88 89 90 91>;
+		clocks = <&fclk>;
+		clock-names = "sci_ick";
+	};
+	sci1: serial@ffff80 {
+		compatible = "renesas,sci";
+		reg = <0xffff80 8>;
+		interrupts = <92 93 94 95>;
+		clocks = <&fclk>;
+		clock-names = "sci_ick";
+	};
+};
-- 
2.1.4

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

* Re: [PATCH v11 19/19] h8300: devicetree source
  2015-05-08 15:04 ` [PATCH v11 19/19] h8300: devicetree source Yoshinori Sato
@ 2015-05-08 16:50   ` Mark Rutland
  2015-05-09 11:19     ` Yoshinori Sato
  0 siblings, 1 reply; 4+ messages in thread
From: Mark Rutland @ 2015-05-08 16:50 UTC (permalink / raw)
  To: Yoshinori Sato
  Cc: linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org,
	devicetree@vger.kernel.org

Hi,

> +++ b/Documentation/devicetree/bindings/h8300/cpu.txt
> @@ -0,0 +1,17 @@
> +* H8/300 CPU bindings
> +
> +Required properties:
> +
> +- compatible: Compatible property value should be "renesas,h8300".
> +- reg: Contains CPU index.

What does the "CPU index" correspond to physically on the CPU?

Can h8300 support SMP?

> +- clock-frequency: Contains the clock frequency for CPU, in Hz.

Is this strictly necessary?

> +- renesas,bus-width: Contain the memory bus width.

What's this needed for?

[...]

> +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,h8300h-intc.txt
> @@ -0,0 +1,20 @@
> +* H8/300H Interrupt controller
> +
> +Required properties:
> +
> +- compatible: has to be "renesas,h8300h-intc", "renesas,h8300-intc" as fallback.
> +- #interrupt-cells: has to be <1>: an interrupt index and flags, as defined in
> +  interrupts.txt in this directory

Surely you need two cells to encode index and flags?

> +
> +Optional properties:
> +
> +- any properties, listed in interrupts.txt, and any standard resource allocation
> +  properties
> +
> +Example:
> +
> +	h8intc: intc@0 {

Nit: call this "interrupt-controller" rather than "ntc".

Without a reg you shouldn't have a unit-address (the '@0' part).

How does the CPU communicate with this controller? I assume it's not
MMIO.

These comments also apply to "renesas,h8s-intc".

> diff --git a/arch/h8300/boot/dts/edosk2674.dts b/arch/h8300/boot/dts/edosk2674.dts
> new file mode 100644
> index 0000000..60e73b9
> --- /dev/null
> +++ b/arch/h8300/boot/dts/edosk2674.dts
> @@ -0,0 +1,109 @@
> +#include <dt-bindings/clock/renesas,8bit-timer.h>
> +
> +/dts-v1/;
> +/ {
> +	compatible = "renesas,edosk2674";
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +	interrupt-parent = <&h8intc>;
> +
> +	chosen {
> +		bootargs = "console=ttySC2,38400";
> +	};

It would be great if you could use stdout-path from the start rather
than passing console= in bootargs. That makes things far less fragile
w.r.t. physical vs logical naming, and keeps console and earlycon in
sync.

It also means that a user can replace bootargs and still expect the
console to work by default (unless overridden explicitly).

> +	aliases {
> +		serial0 = &sci0;
> +		serial1 = &sci1;
> +		serial2 = &sci2;
> +	};
> +
> +	clocks {
> +		ranges;
> +		#address-cells = <1>;
> +		#size-cells = <1>;

Please get rid of the clocks container node and place the clocks
directly under the root node. There's nothing magic about a /clocks
node, and it's not been listed as a bus of any sort.

> +		pllclk: pllclk {
> +			compatible = "renesas,h8s2678-pll-clock";
> +			clocks = <&xclk>;
> +			#clock-cells = <0>;
> +			reg = <0xfee03b 2>, <0xfee045 2>;
> +		};
> +		cclk: cclk {
> +			compatible = "renesas,h8300-div-clock";
> +			clocks = <&pllclk>;
> +			#clock-cells = <0>;
> +			reg = <0xfee03b 2>;
> +			renesas,width = <3>;
> +		};

Are there existing bindings for these? I didn't see any as part of the
portion of the series I was Cc'd for.

> +	memory@0 {

Nit: the unit-address should math the address in the reg entry (here it
should be 400000 rather than 0).

[...]

> +	chosen {
> +		bootargs = "earlyprintk=h8300-sim console=ttySC0";
> +	};

If you implement earlycon you'd only need a single stdout-path entry
here, which would make this much nicer.

Thanks,
Mark.

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

* Re: [PATCH v11 19/19] h8300: devicetree source
  2015-05-08 16:50   ` Mark Rutland
@ 2015-05-09 11:19     ` Yoshinori Sato
  0 siblings, 0 replies; 4+ messages in thread
From: Yoshinori Sato @ 2015-05-09 11:19 UTC (permalink / raw)
  To: Mark Rutland
  Cc: linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org,
	devicetree@vger.kernel.org

At Fri, 8 May 2015 17:50:00 +0100,
Mark Rutland wrote:
> 
> Hi,
> 
> > +++ b/Documentation/devicetree/bindings/h8300/cpu.txt
> > @@ -0,0 +1,17 @@
> > +* H8/300 CPU bindings
> > +
> > +Required properties:
> > +
> > +- compatible: Compatible property value should be "renesas,h8300".
> > +- reg: Contains CPU index.
> 
> What does the "CPU index" correspond to physically on the CPU?
> 
> Can h8300 support SMP?

No.
It's unnecessary.

> > +- clock-frequency: Contains the clock frequency for CPU, in Hz.
> 
> Is this strictly necessary?

Yes.
There are no ways to calculate.

> > +- renesas,bus-width: Contain the memory bus width.
> 
> What's this needed for?

Hmm...
This value can get bus controller setting.
It's considered.

> 
> [...]
> 
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,h8300h-intc.txt
> > @@ -0,0 +1,20 @@
> > +* H8/300H Interrupt controller
> > +
> > +Required properties:
> > +
> > +- compatible: has to be "renesas,h8300h-intc", "renesas,h8300-intc" as fallback.
> > +- #interrupt-cells: has to be <1>: an interrupt index and flags, as defined in
> > +  interrupts.txt in this directory
> 
> Surely you need two cells to encode index and flags?

That's right.
flag field needed.

> 
> > +
> > +Optional properties:
> > +
> > +- any properties, listed in interrupts.txt, and any standard resource allocation
> > +  properties
> > +
> > +Example:
> > +
> > +	h8intc: intc@0 {
> 
> Nit: call this "interrupt-controller" rather than "ntc".

OK.
Updated.

> Without a reg you shouldn't have a unit-address (the '@0' part).
> 
> How does the CPU communicate with this controller? I assume it's not
> MMIO.

Interrupt controller is MMIO.
It set base address.

> These comments also apply to "renesas,h8s-intc".

OK.

> > diff --git a/arch/h8300/boot/dts/edosk2674.dts b/arch/h8300/boot/dts/edosk2674.dts
> > new file mode 100644
> > index 0000000..60e73b9
> > --- /dev/null
> > +++ b/arch/h8300/boot/dts/edosk2674.dts
> > @@ -0,0 +1,109 @@
> > +#include <dt-bindings/clock/renesas,8bit-timer.h>
> > +
> > +/dts-v1/;
> > +/ {
> > +	compatible = "renesas,edosk2674";
> > +	#address-cells = <1>;
> > +	#size-cells = <1>;
> > +	interrupt-parent = <&h8intc>;
> > +
> > +	chosen {
> > +		bootargs = "console=ttySC2,38400";
> > +	};
> 
> It would be great if you could use stdout-path from the start rather
> than passing console= in bootargs. That makes things far less fragile
> w.r.t. physical vs logical naming, and keeps console and earlycon in
> sync.
> 
> It also means that a user can replace bootargs and still expect the
> console to work by default (unless overridden explicitly).

OK.

> > +	aliases {
> > +		serial0 = &sci0;
> > +		serial1 = &sci1;
> > +		serial2 = &sci2;
> > +	};
> > +
> > +	clocks {
> > +		ranges;
> > +		#address-cells = <1>;
> > +		#size-cells = <1>;
> 
> Please get rid of the clocks container node and place the clocks
> directly under the root node. There's nothing magic about a /clocks
> node, and it's not been listed as a bus of any sort.

OK.

> > +		pllclk: pllclk {
> > +			compatible = "renesas,h8s2678-pll-clock";
> > +			clocks = <&xclk>;
> > +			#clock-cells = <0>;
> > +			reg = <0xfee03b 2>, <0xfee045 2>;
> > +		};
> > +		cclk: cclk {
> > +			compatible = "renesas,h8300-div-clock";
> > +			clocks = <&pllclk>;
> > +			#clock-cells = <0>;
> > +			reg = <0xfee03b 2>;
> > +			renesas,width = <3>;
> > +		};
> 
> Are there existing bindings for these? I didn't see any as part of the
> portion of the series I was Cc'd for.

No.
It new clock bindings.
Please refer Message-Id: <1431097479-21101-18-git-send-email-ysato@users.sourceforge.jp>

> > +	memory@0 {
> 
> Nit: the unit-address should math the address in the reg entry (here it
> should be 400000 rather than 0).

OK.

> [...]
> 
> > +	chosen {
> > +		bootargs = "earlyprintk=h8300-sim console=ttySC0";
> > +	};
> 
> If you implement earlycon you'd only need a single stdout-path entry
> here, which would make this much nicer.

OK.

> Thanks,
> Mark.

Thanks comment.
It was helpful.

-- 
Yoshinori Sato
<ysato@users.sourceforge.jp>

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

end of thread, other threads:[~2015-05-09 11:19 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <1431097479-21101-1-git-send-email-ysato@users.sourceforge.jp>
     [not found] ` <1431097479-21101-1-git-send-email-ysato-Rn4VEauK+AKRv+LV9MX5uooqe+aC9MnS@public.gmane.org>
2015-05-08 15:04   ` [PATCH v11 04/19] sh-sci: Add h8300 SCI Yoshinori Sato
2015-05-08 15:04 ` [PATCH v11 19/19] h8300: devicetree source Yoshinori Sato
2015-05-08 16:50   ` Mark Rutland
2015-05-09 11:19     ` Yoshinori Sato

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).