Re: [PATCH v10 04/19] sh-sci: Add H8/300 SCI.

devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* Re: [PATCH v10 04/19] sh-sci: Add H8/300 SCI.
       [not found] ` <1430736122-20929-6-git-send-email-ysato@users.sourceforge.jp>
@ 2015-05-04 13:58   ` Geert Uytterhoeven
  0 siblings, 0 replies; 4+ messages in thread
From: Geert Uytterhoeven @ 2015-05-04 13:58 UTC (permalink / raw)
  To: Yoshinori Sato
  Cc: linux-kernel@vger.kernel.org, Linux-Arch, Linux-sh list,
	devicetree@vger.kernel.org

CC devicetree

On Mon, May 4, 2015 at 12:41 PM, Yoshinori Sato
<ysato@users.sourceforge.jp> wrote:
> H8/300 internal serial controller of same as sh-sci.
> So h8300 use SH_SCI.
> And add sci bindings.
>
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>

Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

> ---
>  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 linux-arch" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[parent not found: <1430736122-20929-21-git-send-email-ysato@users.sourceforge.jp>]

[parent not found: <1430736122-20929-21-git-send-email-ysato-Rn4VEauK+AKRv+LV9MX5uooqe+aC9MnS@public.gmane.org>]

* Re: [PATCH v10 19/19] h8300: devicetree source
       [not found]   ` <1430736122-20929-21-git-send-email-ysato-Rn4VEauK+AKRv+LV9MX5uooqe+aC9MnS@public.gmane.org>
@ 2015-05-04 12:40     ` Geert Uytterhoeven
  0 siblings, 0 replies; 4+ messages in thread
From: Geert Uytterhoeven @ 2015-05-04 12:40 UTC (permalink / raw)
  To: Yoshinori Sato
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux-Arch,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

CC devicetree

On Mon, May 4, 2015 at 12:42 PM, Yoshinori Sato
<ysato-Rn4VEauK+AKRv+LV9MX5uooqe+aC9MnS@public.gmane.org> wrote:
> Signed-off-by: Yoshinori Sato <ysato-Rn4VEauK+AKRv+LV9MX5uooqe+aC9MnS@public.gmane.org>
> ---
>  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/dt-bindings                    |   1 +
>  arch/h8300/boot/dts/edosk2674.dts                  | 111 +++++++++++++++++++++
>  arch/h8300/boot/dts/h8300h_sim.dts                 |  97 ++++++++++++++++++
>  arch/h8300/boot/dts/h8s_sim.dts                    | 103 +++++++++++++++++++
>  8 files changed, 380 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 120000 arch/h8300/boot/dts/dt-bindings
>  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/dt-bindings b/arch/h8300/boot/dts/dt-bindings
> new file mode 120000
> index 0000000..0cecb3d
> --- /dev/null
> +++ b/arch/h8300/boot/dts/dt-bindings
> @@ -0,0 +1 @@
> +../../../../include/dt-bindings
> \ No newline at end of file
> diff --git a/arch/h8300/boot/dts/edosk2674.dts b/arch/h8300/boot/dts/edosk2674.dts
> new file mode 100644
> index 0000000..fe3c334
> --- /dev/null
> +++ b/arch/h8300/boot/dts/edosk2674.dts
> @@ -0,0 +1,111 @@
> +#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: xclk {
> +                       #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>;
> +               };
> +               pclk: pclk {
> +                       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: intc@0 {
> +               compatible = "renesas,h8s-intc", "renesas,h8300-intc";
> +               #interrupt-cells = <1>;
> +               interrupt-controller;
> +       };
> +       tpu: tpu@ffffe0 {
> +               compatible = "renesas,tpu";
> +               reg = <0xffffe0 16>, <0xfffff0 12>;
> +               clocks = <&pclk>;
> +               clock-names = "peripheral_clk";
> +       };
> +
> +       timer8: timer@ffffb0 {
> +               compatible = "renesas,8bit-timer";
> +               reg = <0xffff80 10>;
> +               interrupts = <72 75>;
> +               clocks = <&pclk>;
> +               clock-names = "peripheral_clk";
> +               renesas,mode = <CLOCKEVENTDEVICE>;
> +               renesas,div = <DIV_8>;
> +       };
> +
> +       sci0: serial@ffff78 {
> +               compatible = "renesas,sci";
> +               reg = <0xffff78 8>;
> +               interrupts = <88 89 90 91>;
> +               clocks = <&pclk>;
> +               clock-names = "peripheral_clk";
> +       };
> +       sci1: serial@ffff80 {
> +               compatible = "renesas,sci";
> +               reg = <0xffff80 8>;
> +               interrupts = <92 93 94 95>;
> +               clocks = <&pclk>;
> +               clock-names = "peripheral_clk";
> +       };
> +       sci2: serial@ffff88 {
> +               compatible = "renesas,sci";
> +               reg = <0xffff88 8>;
> +               interrupts = <96 97 98 99>;
> +               clocks = <&pclk>;
> +               clock-names = "peripheral_clk";
> +       };
> +};
> diff --git a/arch/h8300/boot/dts/h8300h_sim.dts b/arch/h8300/boot/dts/h8300h_sim.dts
> new file mode 100644
> index 0000000..bd150fb
> --- /dev/null
> +++ b/arch/h8300/boot/dts/h8300h_sim.dts
> @@ -0,0 +1,97 @@
> +#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: xclk {
> +                       #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>;
> +               };
> +               pclk: pclk {
> +                       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: intc@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 39>;
> +               clocks = <&pclk>;
> +               clock-names = "peripheral_clk";
> +               renesas,mode = <CLOCKSOURCE>;
> +               renesas,div = <DIV_8>;
> +       };
> +       timer16: timer@ffff68 {
> +               compatible = "renesas,16bit-timer";
> +               reg = <0xffff68 8>, <0xffff60 8>;
> +               interrupts = <24>;
> +               renesas,channel = <0>;
> +               clocks = <&pclk>;
> +               clock-names = "peripheral_clk";
> +       };
> +
> +       sci0: serial@ffffb0 {
> +               compatible = "renesas,sci";
> +               reg = <0xffffb0 8>;
> +               interrupts = <52 53 54 55>;
> +               clocks = <&pclk>;
> +               clock-names = "peripheral_clk";
> +       };
> +       sci1: serial@ffffb8 {
> +               compatible = "renesas,sci";
> +               reg = <0xffffb8 8>;
> +               interrupts = <56 57 58 59>;
> +               clocks = <&pclk>;
> +               clock-names = "peripheral_clk";
> +       };
> +};
> diff --git a/arch/h8300/boot/dts/h8s_sim.dts b/arch/h8300/boot/dts/h8s_sim.dts
> new file mode 100644
> index 0000000..26b653d
> --- /dev/null
> +++ b/arch/h8300/boot/dts/h8s_sim.dts
> @@ -0,0 +1,103 @@
> +#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: xclk {
> +                       #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>;
> +               };
> +               pclk: pclk {
> +                       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: intc@0 {
> +               compatible = "renesas,h8s-intc", "renesas,h8300-intc";
> +               #interrupt-cells = <1>;
> +               interrupt-controller;
> +       };
> +       tpu: tpu@ffffe0 {
> +               compatible = "renesas,tpu";
> +               reg = <0xffffe0 16>, <0xfffff0 12>;
> +               clocks = <&pclk>;
> +               clock-names = "peripheral_clk";
> +       };
> +
> +       timer8: timer@ffffb0 {
> +               compatible = "renesas,8bit-timer";
> +               reg = <0xffff80 10>;
> +               interrupts = <72 75>;
> +               clocks = <&pclk>;
> +               clock-names = "peripheral_clk";
> +               renesas,mode = <CLOCKEVENTDEVICE>;
> +               renesas,div = <DIV_8>;
> +       };
> +
> +       sci0: serial@ffff78 {
> +               compatible = "renesas,sci";
> +               reg = <0xffff78 8>;
> +               interrupts = <88 89 90 91>;
> +               clocks = <&pclk>;
> +               clock-names = "peripheral_clk";
> +       };
> +       sci1: serial@ffff80 {
> +               compatible = "renesas,sci";
> +               reg = <0xffff80 8>;
> +               interrupts = <92 93 94 95>;
> +               clocks = <&pclk>;
> +               clock-names = "peripheral_clk";
> +       };
> +};
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arch" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
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	[flat|nested] 4+ messages in thread

* Re: [PATCH v10 19/19] h8300: devicetree source
       [not found] ` <1430736122-20929-21-git-send-email-ysato@users.sourceforge.jp>
       [not found]   ` <1430736122-20929-21-git-send-email-ysato-Rn4VEauK+AKRv+LV9MX5uooqe+aC9MnS@public.gmane.org>
@ 2015-05-04 15:09   ` Arnd Bergmann
  2015-05-07  4:47     ` Yoshinori Sato
  1 sibling, 1 reply; 4+ messages in thread
From: Arnd Bergmann @ 2015-05-04 15:09 UTC (permalink / raw)
  To: Yoshinori Sato; +Cc: linux-kernel, linux-arch, devicetree

On Monday 04 May 2015 19:42:02 Yoshinori Sato wrote:
> +
> +	h8intc: intc@0 {
> +		compatible = "renesas,h8s-intc", "renesas,h8300-intc";
> +		#interrupt-cells = <1>;
> +		interrupt-controller;
> +	};

The node name should be "interrupt-controller@0", not "intc@0", to follow
the common conventions.

> +	tpu: tpu@ffffe0 {
> +		compatible = "renesas,tpu";
> +		reg = <0xffffe0 16>, <0xfffff0 12>;
> +		clocks = <&pclk>;
> +		clock-names = "peripheral_clk";
> +	};
> +
> +	timer8: timer@ffffb0 {
> +		compatible = "renesas,8bit-timer";
> +		reg = <0xffff80 10>;
> +		interrupts = <72 75>;
> +		clocks = <&pclk>;
> +		clock-names = "peripheral_clk";
> +		renesas,mode = <CLOCKEVENTDEVICE>;
> +		renesas,div = <DIV_8>;
> +	};
> +

The renesas,div property seems odd here. How about defining a
"clock-frequency" property and figuring out the divider from
the parent clock in the driver?

Your new binding makes it mandatory to have a "fclk" clock, which
seems better suited than "peripheral_clk", so I'd suggest you
change the code to match the documentation (rather than the other
way round). Alternatively, you could make this an anonymous
clock and not specify the name at all.

The renesas,mode property seems odd. Why is that needed? It sounds
like you are encoding how you expect the device to be used by
Linux, rather than what it can do in hardware. If you have multiple
variants of the 8bit-timer hardware that have different features,
better use separate compatible strings for them, or a boolean
flag that announces the presence or absence of a feature.

If however, this is just a hint for Linux, maybe you can find a way
for the driver to take a guess itself, e.g. using the first
device it finds as a clockevent device, and only use a clocksource
device if there is more than one?

> +	sci0: serial@ffff78 {
> +		compatible = "renesas,sci";
> +		reg = <0xffff78 8>;
> +		interrupts = <88 89 90 91>;
> +		clocks = <&pclk>;
> +		clock-names = "peripheral_clk";
> +	};
> +	sci1: serial@ffff80 {
> +		compatible = "renesas,sci";
> +		reg = <0xffff80 8>;
> +		interrupts = <92 93 94 95>;
> +		clocks = <&pclk>;
> +		clock-names = "peripheral_clk";
> +	};
> +	sci2: serial@ffff88 {
> +		compatible = "renesas,sci";
> +		reg = <0xffff88 8>;
> +		interrupts = <96 97 98 99>;
> +		clocks = <&pclk>;
> +		clock-names = "peripheral_clk";
> +	};
> +};

The binding for sci requires the clock to be named "sci_ick",  so please
use that instead of "peripheral_clk". The driver can handle both.

	Arnd

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

* Re: [PATCH v10 19/19] h8300: devicetree source
  2015-05-04 15:09   ` Arnd Bergmann
@ 2015-05-07  4:47     ` Yoshinori Sato
  0 siblings, 0 replies; 4+ messages in thread
From: Yoshinori Sato @ 2015-05-07  4:47 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: linux-kernel, linux-arch, devicetree

At Mon, 04 May 2015 17:09:21 +0200,
Arnd Bergmann wrote:
> 
> On Monday 04 May 2015 19:42:02 Yoshinori Sato wrote:
> > +
> > +	h8intc: intc@0 {
> > +		compatible = "renesas,h8s-intc", "renesas,h8300-intc";
> > +		#interrupt-cells = <1>;
> > +		interrupt-controller;
> > +	};
> 
> The node name should be "interrupt-controller@0", not "intc@0", to follow
> the common conventions.

OK.

> > +	tpu: tpu@ffffe0 {
> > +		compatible = "renesas,tpu";
> > +		reg = <0xffffe0 16>, <0xfffff0 12>;
> > +		clocks = <&pclk>;
> > +		clock-names = "peripheral_clk";
> > +	};
> > +
> > +	timer8: timer@ffffb0 {
> > +		compatible = "renesas,8bit-timer";
> > +		reg = <0xffff80 10>;
> > +		interrupts = <72 75>;
> > +		clocks = <&pclk>;
> > +		clock-names = "peripheral_clk";
> > +		renesas,mode = <CLOCKEVENTDEVICE>;
> > +		renesas,div = <DIV_8>;
> > +	};
> > +
> 
> The renesas,div property seems odd here. How about defining a
> "clock-frequency" property and figuring out the divider from
> the parent clock in the driver?

Hmm...
It more better idea.
I will fix.

> Your new binding makes it mandatory to have a "fclk" clock, which
> seems better suited than "peripheral_clk", so I'd suggest you
> change the code to match the documentation (rather than the other
> way round). Alternatively, you could make this an anonymous
> clock and not specify the name at all.

OK.

> The renesas,mode property seems odd. Why is that needed? It sounds
> like you are encoding how you expect the device to be used by
> Linux, rather than what it can do in hardware. If you have multiple
> variants of the 8bit-timer hardware that have different features,
> better use separate compatible strings for them, or a boolean
> flag that announces the presence or absence of a feature.

This timer have some mode. But driver using only one mode.
I think it isn't necessary.

> If however, this is just a hint for Linux, maybe you can find a way
> for the driver to take a guess itself, e.g. using the first
> device it finds as a clockevent device, and only use a clocksource
> device if there is more than one?

Many clocks are put in order.

> > +	sci0: serial@ffff78 {
> > +		compatible = "renesas,sci";
> > +		reg = <0xffff78 8>;
> > +		interrupts = <88 89 90 91>;
> > +		clocks = <&pclk>;
> > +		clock-names = "peripheral_clk";
> > +	};
> > +	sci1: serial@ffff80 {
> > +		compatible = "renesas,sci";
> > +		reg = <0xffff80 8>;
> > +		interrupts = <92 93 94 95>;
> > +		clocks = <&pclk>;
> > +		clock-names = "peripheral_clk";
> > +	};
> > +	sci2: serial@ffff88 {
> > +		compatible = "renesas,sci";
> > +		reg = <0xffff88 8>;
> > +		interrupts = <96 97 98 99>;
> > +		clocks = <&pclk>;
> > +		clock-names = "peripheral_clk";
> > +	};
> > +};
> 
> The binding for sci requires the clock to be named "sci_ick",  so please
> use that instead of "peripheral_clk". The driver can handle both.
>

OK.

> 	Arnd

Thanks.

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

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

end of thread, other threads:[~2015-05-07  4:47 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <1430736122-20929-2-git-send-email-ysato@users.sourceforge.jp>
     [not found] ` <1430736122-20929-6-git-send-email-ysato@users.sourceforge.jp>
2015-05-04 13:58   ` [PATCH v10 04/19] sh-sci: Add H8/300 SCI Geert Uytterhoeven
     [not found] ` <1430736122-20929-21-git-send-email-ysato@users.sourceforge.jp>
     [not found]   ` <1430736122-20929-21-git-send-email-ysato-Rn4VEauK+AKRv+LV9MX5uooqe+aC9MnS@public.gmane.org>
2015-05-04 12:40     ` [PATCH v10 19/19] h8300: devicetree source Geert Uytterhoeven
2015-05-04 15:09   ` Arnd Bergmann
2015-05-07  4:47     ` 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).