[PATCH v2 00/29] Add OF support to the sh-sci serial port driver

[PATCH v2 00/29] Add OF support to the sh-sci serial port driver

[Laurent Pinchart]

Hello,

This is the second version of the patch set adds device tree bindings for the
sh sci serial port devices and adds OF parsing to the sh-sci driver.

The bindings are based on Bastian Hecht's proposal (see
http://www.spinics.net/lists/arm-kernel/msg228129.html). The approach taken
here is more minimalistic: instead of describing all hardware characteristics
that vary between the SCI device revisions in DT (such as registers layout),
that information is stored in the driver and selected based on the compatible
property value. Only SCI revisions used on ARM devices are supported through
DT, as DT support for SuperH is nowhere down the line.

Patches 01/29 to 06/29 clean up the sh-sci driver. Patches 07/29 to 27/29
replace memory and interrupt resources passed through platform data with
platform resources. Beside replacing a custom mechanism with a standard one,
it will also make the DT parsing code simpler as resource allocation will be
shared between DT and non-DT code paths. Finally, patches 28/29 to 29/29 add
OF parsing to the sh-sci driver and create DT bindings documentation.

The patches have been test on a Lager board (r8a7790-based). Support for other
SoCs will be added as needed. Note that all current Renesas ARM SoCs seem to
be compatible with the generic (H)SCI(F)(AB) devices, but the plan is for
their DT bindings to list the SoC-specific version in case incompatibilities
are found later.

Patch 08/29 ("serial: sh-sci: Support resources passed through platform
resources") is the only dependency of the arch/arm and arch/sh patches present
in this series. As it's too late to get it in v3.13, Simon will need a stable
branch of the linux serial tree with that patch included.

Changes compared to v1:

- Patches 04/29 to 27/29 have been added
- Multi-interrupt support has been removed from the DT bindings as they don't
  support any SoC requiring multi-interrupt yet

Cc: devicetree@vger.kernel.org

Bastian Hecht (1):
  serial: sh-sci: Add OF support

Laurent Pinchart (28):
  serial: sh-sci: Sort headers alphabetically
  serial: sh-sci: Remove baud rate calculation algorithm 5
  serial: sh-sci: Simplify baud rate calculation algorithms
  serial: sh-sci: Remove duplicate interrupt check in verify port op
  serial: sh-sci: Set the UPF_FIXED_PORT flag
  serial: sh-sci: Don't check IRQ in verify port operation
  serial: sh-sci: Support resources passed through platform resources
  ARM: shmobile: sh7372: Use macros to declare SCIF devices
  ARM: shmobile: sh73a0: Use macros to declare SCIF devices
  ARM: shmobile: r8a7740: Use macros to declare SCIF devices
  ARM: shmobile: r8a7779: Use macros to declare SCIF devices
  ARM: shmobile: r7s72100: Don't define SCIF platform data in an array
  ARM: shmobile: r8a73a4: Don't define SCIF platform data in an array
  ARM: shmobile: r8a7778: Don't define SCIF platform data in an array
  ARM: shmobile: r8a7790: Don't define SCIF platform data in an array
  ARM: shmobile: r8a7791: Don't define SCIF platform data in an array
  ARM: shmobile: sh7372: Declare SCIF register base and IRQ as resources
  ARM: shmobile: sh73a0: Declare SCIF register base and IRQ as resources
  ARM: shmobile: r7s72100: Declare SCIF register base and IRQ as
    resources
  ARM: shmobile: r8a73a4: Declare SCIF register base and IRQ as
    resources
  ARM: shmobile: r8a7740: Declare SCIF register base and IRQ as
    resources
  ARM: shmobile: r8a7778: Declare SCIF register base and IRQ as
    resources
  ARM: shmobile: r8a7779: Declare SCIF register base and IRQ as
    resources
  ARM: shmobile: r8a7790: Declare SCIF register base and IRQ as
    resources
  ARM: shmobile: r8a7791: Declare SCIF register base and IRQ as
    resources
  sh: Declare SCIF register base and IRQ as resources
  serial: sh-sci: Remove platform data mapbase and irqs fields
  serial: sh-sci: Add device tree bindings documentation

 .../bindings/serial/renesas,sci-serial.txt         |  42 ++++
 arch/arm/mach-shmobile/setup-r7s72100.c            |  63 +++---
 arch/arm/mach-shmobile/setup-r8a73a4.c             |  65 +++---
 arch/arm/mach-shmobile/setup-r8a7740.c             | 196 +++--------------
 arch/arm/mach-shmobile/setup-r8a7778.c             |  45 ++--
 arch/arm/mach-shmobile/setup-r8a7779.c             | 129 +++--------
 arch/arm/mach-shmobile/setup-r8a7790.c             | 117 +++++-----
 arch/arm/mach-shmobile/setup-r8a7791.c             | 130 +++++------
 arch/arm/mach-shmobile/setup-sh7372.c              | 161 +++-----------
 arch/arm/mach-shmobile/setup-sh73a0.c              | 192 +++--------------
 arch/sh/kernel/cpu/sh2/setup-sh7619.c              |  27 ++-
 arch/sh/kernel/cpu/sh2a/setup-mxg.c                |   9 +-
 arch/sh/kernel/cpu/sh2a/setup-sh7201.c             |  72 +++++--
 arch/sh/kernel/cpu/sh2a/setup-sh7203.c             |  36 +++-
 arch/sh/kernel/cpu/sh2a/setup-sh7206.c             |  36 +++-
 arch/sh/kernel/cpu/sh2a/setup-sh7264.c             |  96 +++++++--
 arch/sh/kernel/cpu/sh2a/setup-sh7269.c             |  96 +++++++--
 arch/sh/kernel/cpu/sh3/setup-sh7705.c              |  18 +-
 arch/sh/kernel/cpu/sh3/setup-sh770x.c              |  27 ++-
 arch/sh/kernel/cpu/sh3/setup-sh7710.c              |  18 +-
 arch/sh/kernel/cpu/sh3/setup-sh7720.c              |  18 +-
 arch/sh/kernel/cpu/sh4/setup-sh4-202.c             |  15 +-
 arch/sh/kernel/cpu/sh4/setup-sh7750.c              |  18 +-
 arch/sh/kernel/cpu/sh4/setup-sh7760.c              |  58 +++--
 arch/sh/kernel/cpu/sh4a/setup-sh7343.c             |  36 +++-
 arch/sh/kernel/cpu/sh4a/setup-sh7366.c             |   9 +-
 arch/sh/kernel/cpu/sh4a/setup-sh7722.c             |  27 ++-
 arch/sh/kernel/cpu/sh4a/setup-sh7723.c             |  54 +++--
 arch/sh/kernel/cpu/sh4a/setup-sh7724.c             |  54 +++--
 arch/sh/kernel/cpu/sh4a/setup-sh7734.c             |  66 ++++--
 arch/sh/kernel/cpu/sh4a/setup-sh7757.c             |  27 ++-
 arch/sh/kernel/cpu/sh4a/setup-sh7763.c             |  27 ++-
 arch/sh/kernel/cpu/sh4a/setup-sh7770.c             |  90 ++++++--
 arch/sh/kernel/cpu/sh4a/setup-sh7780.c             |  18 +-
 arch/sh/kernel/cpu/sh4a/setup-sh7785.c             |  54 +++--
 arch/sh/kernel/cpu/sh4a/setup-sh7786.c             |  82 +++++--
 arch/sh/kernel/cpu/sh4a/setup-shx3.c               |  45 ++--
 arch/sh/kernel/cpu/sh5/setup-sh5.c                 |  11 +-
 drivers/tty/serial/sh-sci.c                        | 238 ++++++++++++++++-----
 include/linux/serial_sci.h                         |  35 +--
 40 files changed, 1443 insertions(+), 1114 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/serial/renesas,sci-serial.txt

-- 
Regards,

Laurent Pinchart

[PATCH v2 28/29] serial: sh-sci: Add device tree bindings documentation

[Laurent Pinchart]

Document the device tree bindings for the sci serial port devices.

Cc: devicetree@vger.kernel.org
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Acked-by: Simon Horman <horms+renesas@verge.net.au>
---
 .../bindings/serial/renesas,sci-serial.txt         | 42 ++++++++++++++++++++++
 1 file changed, 42 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serial/renesas,sci-serial.txt

diff --git a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
new file mode 100644
index 0000000..66d3bca
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
@@ -0,0 +1,42 @@
+* Renesas SH-Mobile Serial Communication Interface
+
+Required properties:
+
+  - compatible: one of the following types (scif, scifa, scifb, hscif).
+
+    - "renesas,scif-r8a7790" for R8A7790 (R-Car H2) SCIF compatible UART.
+    - "renesas,scifa-r8a7790" for R8A7790 (R-Car H2) SCIFA compatible UART.
+    - "renesas,scifb-r8a7790" for R8A7790 (R-Car H2) SCIFB compatible UART.
+    - "renesas,hscif-r8a7790" for R8A7790 (R-Car H2) HSCIF compatible UART.
+    - "renesas,scif-generic" for generic SCIF compatible UART.
+    - "renesas,scifa-generic" for generic SCIFA compatible UART.
+    - "renesas,scifb-generic" for generic SCIFB compatible UART.
+    - "renesas,hscif-generic" for generic HSCIF compatible UART.
+
+    When compatible with the generic version, nodes must also list the
+    SoC-specific version corresponding to the platform.
+
+  - reg: Base address and length of the memory resource used by the UART.
+
+  - interrupt-parent: Reference to the parent interrupt controller.
+  - interrupts: Interrupt number.
+
+  - clocks: Reference to the SCIx UART interface clock.
+  - clock-names: Should be "sci_ick".
+
+Note: Each enabled SCIx UART should have an alias correctly numbered in the
+"aliases" node.
+
+Example:
+	aliases {
+		serial0 = &scifa0;
+	};
+
+	scifa0: serial@e6c40000 {
+		compatible = "renesas,scifa-r8a7790", "renesas,scifa-generic";
+		reg = <0 0xe6c40000 0 64>;
+		interrupt-parent = <&gic>;
+		interrupts = <0 144 4>;
+		clocks = <&mstp2_clks 4>;
+		clock-names = "sci_ick";
+	};
-- 
1.8.1.5

Re: [PATCH v2 28/29] serial: sh-sci: Add device tree bindings documentation

[Sergei Shtylyov]

On 10-11-2013 7:33, Laurent Pinchart wrote:

> Document the device tree bindings for the sci serial port devices.

> Cc: devicetree@vger.kernel.org

    Hm, why are you not CCing a bunch of people responsible for the DT 
bindings maintenance that scripts/get_maintainer.pl should yield?

> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Acked-by: Simon Horman <horms+renesas@verge.net.au>
> ---
>   .../bindings/serial/renesas,sci-serial.txt         | 42 ++++++++++++++++++++++
>   1 file changed, 42 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/serial/renesas,sci-serial.txt

> diff --git a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> new file mode 100644
> index 0000000..66d3bca
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> @@ -0,0 +1,42 @@
> +* Renesas SH-Mobile Serial Communication Interface
> +
> +Required properties:
> +
> +  - compatible: one of the following types (scif, scifa, scifb, hscif).
> +
> +    - "renesas,scif-r8a7790" for R8A7790 (R-Car H2) SCIF compatible UART.
> +    - "renesas,scifa-r8a7790" for R8A7790 (R-Car H2) SCIFA compatible UART.
> +    - "renesas,scifb-r8a7790" for R8A7790 (R-Car H2) SCIFB compatible UART.
> +    - "renesas,hscif-r8a7790" for R8A7790 (R-Car H2) HSCIF compatible UART.
> +    - "renesas,scif-generic" for generic SCIF compatible UART.
> +    - "renesas,scifa-generic" for generic SCIFA compatible UART.
> +    - "renesas,scifb-generic" for generic SCIFB compatible UART.
> +    - "renesas,hscif-generic" for generic HSCIF compatible UART.
> +
> +    When compatible with the generic version, nodes must also list the
> +    SoC-specific version corresponding to the platform.

    Which are not all specified here?

> +
> +  - reg: Base address and length of the memory resource used by the UART.
> +
> +  - interrupt-parent: Reference to the parent interrupt controller.

    This is not a required property. It can be specified in the parent node.

> +  - interrupts: Interrupt number.

    s/number/specifier/

> +
> +  - clocks: Reference to the SCIx UART interface clock.
> +  - clock-names: Should be "sci_ick".
> +
> +Note: Each enabled SCIx UART should have an alias correctly numbered in the
> +"aliases" node.
> +
> +Example:
> +	aliases {
> +		serial0 = &scifa0;
> +	};
> +
> +	scifa0: serial@e6c40000 {
> +		compatible = "renesas,scifa-r8a7790", "renesas,scifa-generic";
> +		reg = <0 0xe6c40000 0 64>;
> +		interrupt-parent = <&gic>;
> +		interrupts = <0 144 4>;
> +		clocks = <&mstp2_clks 4>;
> +		clock-names = "sci_ick";
> +	};

WBR, Sergei

Re: [PATCH v2 28/29] serial: sh-sci: Add device tree bindings documentation

[Laurent Pinchart]

Hi Sergei,

On Sunday 10 November 2013 22:07:16 Sergei Shtylyov wrote:
> On 10-11-2013 7:33, Laurent Pinchart wrote:
> > Document the device tree bindings for the sci serial port devices.
> > 
> > Cc: devicetree@vger.kernel.org
> 
> Hm, why are you not CCing a bunch of people responsible for the DT bindings
> maintenance that scripts/get_maintainer.pl should yield?

Because they're subscribed to the devicetree list, and they're already 
overwhelmed with e-mails.

> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > Acked-by: Simon Horman <horms+renesas@verge.net.au>
> > ---
> > 
> >   .../bindings/serial/renesas,sci-serial.txt         | 42 ++++++++++++++++
> >   1 file changed, 42 insertions(+)
> >   create mode 100644
> >   Documentation/devicetree/bindings/serial/renesas,sci-serial.txt> 
> > diff --git
> > a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> > b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt new
> > file mode 100644
> > index 0000000..66d3bca
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> > @@ -0,0 +1,42 @@
> > +* Renesas SH-Mobile Serial Communication Interface
> > +
> > +Required properties:
> > +
> > +  - compatible: one of the following types (scif, scifa, scifb, hscif).
> > +
> > +    - "renesas,scif-r8a7790" for R8A7790 (R-Car H2) SCIF compatible UART.
> > +    - "renesas,scifa-r8a7790" for R8A7790 (R-Car H2) SCIFA compatible
> > UART.
> > +    - "renesas,scifb-r8a7790" for R8A7790 (R-Car H2) SCIFB compatible
> > UART.
> > +    - "renesas,hscif-r8a7790" for R8A7790 (R-Car H2) HSCIF compatible
> > UART.
> > +    - "renesas,scif-generic" for generic SCIF compatible UART.
> > +    - "renesas,scifa-generic" for generic SCIFA compatible UART.
> > +    - "renesas,scifb-generic" for generic SCIFB compatible UART.
> > +    - "renesas,hscif-generic" for generic HSCIF compatible UART.
> > +
> > +    When compatible with the generic version, nodes must also list the
> > +    SoC-specific version corresponding to the platform.
> 
> Which are not all specified here?

The r8a7790-specific versions are. Other SoCs will be added later when ported 
to SCIF DT.

> > +
> > +  - reg: Base address and length of the memory resource used by the UART.
> > +
> > +  - interrupt-parent: Reference to the parent interrupt controller.
> 
>     This is not a required property. It can be specified in the parent node.

But in practice it isn't for the SCIF devices.

Given that we'll soon get a new way to specify interrupts, I think I'll just 
reference the corresponding DT bindings document when it will be updated 
instead of documenting all possibilities here.

> > +  - interrupts: Interrupt number.
> 
>     s/number/specifier/

I'll fix that.

> 
> > +
> > +  - clocks: Reference to the SCIx UART interface clock.
> > +  - clock-names: Should be "sci_ick".
> > +
> > +Note: Each enabled SCIx UART should have an alias correctly numbered in
> > the +"aliases" node.
> > +
> > +Example:
> > +	aliases {
> > +		serial0 = &scifa0;
> > +	};
> > +
> > +	scifa0: serial@e6c40000 {
> > +		compatible = "renesas,scifa-r8a7790", "renesas,scifa-generic";
> > +		reg = <0 0xe6c40000 0 64>;
> > +		interrupt-parent = <&gic>;
> > +		interrupts = <0 144 4>;
> > +		clocks = <&mstp2_clks 4>;
> > +		clock-names = "sci_ick";
> > +	};

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 28/29] serial: sh-sci: Add device tree bindings documentation

[Mark Rutland]

Hi Laurent,

I have a few comments.

On Sun, Nov 10, 2013 at 03:33:40AM +0000, Laurent Pinchart wrote:
> Document the device tree bindings for the sci serial port devices.
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Acked-by: Simon Horman <horms+renesas@verge.net.au>
> ---
>  .../bindings/serial/renesas,sci-serial.txt         | 42 ++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> 
> diff --git a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> new file mode 100644
> index 0000000..66d3bca
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> @@ -0,0 +1,42 @@
> +* Renesas SH-Mobile Serial Communication Interface
> +
> +Required properties:
> +
> +  - compatible: one of the following types (scif, scifa, scifb, hscif).

Minor nit, but would be nice to have "Should contain one of the
following:". We might have future variants whereby the compatible string
will actually be a string list.

> +
> +    - "renesas,scif-r8a7790" for R8A7790 (R-Car H2) SCIF compatible UART.
> +    - "renesas,scifa-r8a7790" for R8A7790 (R-Car H2) SCIFA compatible UART.
> +    - "renesas,scifb-r8a7790" for R8A7790 (R-Car H2) SCIFB compatible UART.
> +    - "renesas,hscif-r8a7790" for R8A7790 (R-Car H2) HSCIF compatible UART.
> +    - "renesas,scif-generic" for generic SCIF compatible UART.
> +    - "renesas,scifa-generic" for generic SCIFA compatible UART.
> +    - "renesas,scifb-generic" for generic SCIFB compatible UART.
> +    - "renesas,hscif-generic" for generic HSCIF compatible UART.

Is the "-generic" suffix necessary? Why not just "renesas,scif" etc?

> +
> +    When compatible with the generic version, nodes must also list the
> +    SoC-specific version corresponding to the platform.

Presumably the SoC-specific version should come first; it would be nice
to note that.

> +
> +  - reg: Base address and length of the memory resource used by the UART.

I assume by memory resource you mean the memory-mapped registers?
Resource sounds like Linux-internal nomenclature leaking.

> +
> +  - interrupt-parent: Reference to the parent interrupt controller.

I don't think this is strictly necessary. It's implicit by default and
can be added optionally when a system is wired in a complex way. As it's
a completely standard optional property I'm not even sure it needs to be
documented.

> +  - interrupts: Interrupt number.

Minor nit: "Should contain an interrupt-specifier for ..."

I saw the cover mentioned multiple interrupts. Which logical interrupt
output of the device are you expecting here?

> +
> +  - clocks: Reference to the SCIx UART interface clock.

Minor nit: s/Reference to/A phandle + clock-specifier pair for/

> +  - clock-names: Should be "sci_ick".

As we're using clock-names, it would be nicer still to define clocks in
terms of clock-names so as to make it easier to update the document in
future and make the expected use of clock-names clearer:

- clocks: Should contain a phandle + clock-specifier pair for each entry
  in clock-names.

- clock-names: Should contain "sci_ick".

> +
> +Note: Each enabled SCIx UART should have an alias correctly numbered in the
> +"aliases" node.
> +
> +Example:
> +	aliases {
> +		serial0 = &scifa0;
> +	};
> +
> +	scifa0: serial@e6c40000 {
> +		compatible = "renesas,scifa-r8a7790", "renesas,scifa-generic";
> +		reg = <0 0xe6c40000 0 64>;
> +		interrupt-parent = <&gic>;
> +		interrupts = <0 144 4>;
> +		clocks = <&mstp2_clks 4>;
> +		clock-names = "sci_ick";
> +	};

Otherwise this looks good to me.

Thanks,
Mark.

Re: [PATCH v2 28/29] serial: sh-sci: Add device tree bindings documentation

[Laurent Pinchart]

Hi Mark,

Thank you for the quick and detailed review.

On Monday 11 November 2013 09:40:39 Mark Rutland wrote:
> Hi Laurent,
> 
> I have a few comments.
> 
> On Sun, Nov 10, 2013 at 03:33:40AM +0000, Laurent Pinchart wrote:
> > Document the device tree bindings for the sci serial port devices.
> > 
> > Cc: devicetree@vger.kernel.org
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > Acked-by: Simon Horman <horms+renesas@verge.net.au>
> > ---
> > 
> >  .../bindings/serial/renesas,sci-serial.txt         | 42 +++++++++++++++++
> >  1 file changed, 42 insertions(+)
> >  create mode 100644
> >  Documentation/devicetree/bindings/serial/renesas,sci-serial.txt> 
> > diff --git
> > a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> > b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt new
> > file mode 100644
> > index 0000000..66d3bca
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> > @@ -0,0 +1,42 @@
> > +* Renesas SH-Mobile Serial Communication Interface
> > +
> > +Required properties:
> > +
> > +  - compatible: one of the following types (scif, scifa, scifb, hscif).
> 
> Minor nit, but would be nice to have "Should contain one of the
> following:". We might have future variants whereby the compatible string
> will actually be a string list.

Sure, I can do that.

> > +
> > +    - "renesas,scif-r8a7790" for R8A7790 (R-Car H2) SCIF compatible UART.
> > +    - "renesas,scifa-r8a7790" for R8A7790 (R-Car H2) SCIFA compatible
> > UART. +    - "renesas,scifb-r8a7790" for R8A7790 (R-Car H2) SCIFB
> > compatible UART. +    - "renesas,hscif-r8a7790" for R8A7790 (R-Car H2)
> > HSCIF compatible UART. +    - "renesas,scif-generic" for generic SCIF
> > compatible UART.
> > +    - "renesas,scifa-generic" for generic SCIFA compatible UART.
> > +    - "renesas,scifb-generic" for generic SCIFB compatible UART.
> > +    - "renesas,hscif-generic" for generic HSCIF compatible UART.
> 
> Is the "-generic" suffix necessary? Why not just "renesas,scif" etc?

You're right, I'll remove the suffix.

> > +
> > +    When compatible with the generic version, nodes must also list the
> > +    SoC-specific version corresponding to the platform.
> 
> Presumably the SoC-specific version should come first; it would be nice
> to note that.

Indeed, I'll do so.

> > +
> > +  - reg: Base address and length of the memory resource used by the UART.
> 
> I assume by memory resource you mean the memory-mapped registers?
> Resource sounds like Linux-internal nomenclature leaking.

I'll fix it as well, although we could decide on a definition of "resource" 
that isn't Linux-specific :-)

> > +
> > +  - interrupt-parent: Reference to the parent interrupt controller.
> 
> I don't think this is strictly necessary. It's implicit by default and
> can be added optionally when a system is wired in a complex way. As it's
> a completely standard optional property I'm not even sure it needs to be
> documented.
> 
> > +  - interrupts: Interrupt number.
> 
> Minor nit: "Should contain an interrupt-specifier for ..."
> 
> I saw the cover mentioned multiple interrupts. Which logical interrupt
> output of the device are you expecting here?

SCI devices come in several flavours, with individual interrupt sources wired 
up to different interrupt lines, or multiplexed all together. As the DT 
bindings support the later only at the moment, there's only a single interrupt 
to handle.

Should we come up with a standard way of saying in the bindings that a device 
uses the interrupt bindings defined in interrupt-controller/interrupts.txt ? 
This will become even more important with the interrupts-extended property, we 
don't want every DT bindings document to detail all possible way to specify 
interrupts.

> > +
> > +  - clocks: Reference to the SCIx UART interface clock.
> 
> Minor nit: s/Reference to/A phandle + clock-specifier pair for/
> 
> > +  - clock-names: Should be "sci_ick".
> 
> As we're using clock-names, it would be nicer still to define clocks in
> terms of clock-names so as to make it easier to update the document in
> future and make the expected use of clock-names clearer:
> 
> - clocks: Should

"Should", or "must" ?

> contain a phandle + clock-specifier pair for each entry
>   in clock-names.
> 
> - clock-names: Should contain "sci_ick".

Shouldn't that be

"Should contain "sci_ick" for the SCIx UART interface clock."

? Otherwise the bindings wouldn't tell which clock the clocks property should 
reference.

> > +
> > +Note: Each enabled SCIx UART should have an alias correctly numbered in
> > the +"aliases" node.
> > +
> > +Example:
> > +	aliases {
> > +		serial0 = &scifa0;
> > +	};
> > +
> > +	scifa0: serial@e6c40000 {
> > +		compatible = "renesas,scifa-r8a7790", "renesas,scifa-generic";
> > +		reg = <0 0xe6c40000 0 64>;
> > +		interrupt-parent = <&gic>;
> > +		interrupts = <0 144 4>;
> > +		clocks = <&mstp2_clks 4>;
> > +		clock-names = "sci_ick";
> > +	};
> 
> Otherwise this looks good to me.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 28/29] serial: sh-sci: Add device tree bindings documentation

[Mark Rutland]

On Mon, Nov 11, 2013 at 01:26:11PM +0000, Laurent Pinchart wrote:
> Hi Mark,
> 
> Thank you for the quick and detailed review.
> 
> On Monday 11 November 2013 09:40:39 Mark Rutland wrote:
> > Hi Laurent,
> > 
> > I have a few comments.
> > 
> > On Sun, Nov 10, 2013 at 03:33:40AM +0000, Laurent Pinchart wrote:
> > > Document the device tree bindings for the sci serial port devices.
> > > 
> > > Cc: devicetree@vger.kernel.org
> > > Signed-off-by: Laurent Pinchart
> > > <laurent.pinchart+renesas@ideasonboard.com>
> > > Acked-by: Simon Horman <horms+renesas@verge.net.au>
> > > ---
> > > 
> > >  .../bindings/serial/renesas,sci-serial.txt         | 42 +++++++++++++++++
> > >  1 file changed, 42 insertions(+)
> > >  create mode 100644
> > >  Documentation/devicetree/bindings/serial/renesas,sci-serial.txt> 
> > > diff --git
> > > a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> > > b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt new
> > > file mode 100644
> > > index 0000000..66d3bca
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> > > @@ -0,0 +1,42 @@
> > > +* Renesas SH-Mobile Serial Communication Interface
> > > +
> > > +Required properties:
> > > +
> > > +  - compatible: one of the following types (scif, scifa, scifb, hscif).
> > 
> > Minor nit, but would be nice to have "Should contain one of the
> > following:". We might have future variants whereby the compatible string
> > will actually be a string list.
> 
> Sure, I can do that.
> 
> > > +
> > > +    - "renesas,scif-r8a7790" for R8A7790 (R-Car H2) SCIF compatible UART.
> > > +    - "renesas,scifa-r8a7790" for R8A7790 (R-Car H2) SCIFA compatible
> > > UART. +    - "renesas,scifb-r8a7790" for R8A7790 (R-Car H2) SCIFB
> > > compatible UART. +    - "renesas,hscif-r8a7790" for R8A7790 (R-Car H2)
> > > HSCIF compatible UART. +    - "renesas,scif-generic" for generic SCIF
> > > compatible UART.
> > > +    - "renesas,scifa-generic" for generic SCIFA compatible UART.
> > > +    - "renesas,scifb-generic" for generic SCIFB compatible UART.
> > > +    - "renesas,hscif-generic" for generic HSCIF compatible UART.
> > 
> > Is the "-generic" suffix necessary? Why not just "renesas,scif" etc?
> 
> You're right, I'll remove the suffix.
> 
> > > +
> > > +    When compatible with the generic version, nodes must also list the
> > > +    SoC-specific version corresponding to the platform.
> > 
> > Presumably the SoC-specific version should come first; it would be nice
> > to note that.
> 
> Indeed, I'll do so.
> 
> > > +
> > > +  - reg: Base address and length of the memory resource used by the UART.
> > 
> > I assume by memory resource you mean the memory-mapped registers?
> > Resource sounds like Linux-internal nomenclature leaking.
> 
> I'll fix it as well, although we could decide on a definition of "resource" 
> that isn't Linux-specific :-)

We could, I guess. For the moment I'd prefer to point out that these are
MMIO registers :)

> 
> > > +
> > > +  - interrupt-parent: Reference to the parent interrupt controller.
> > 
> > I don't think this is strictly necessary. It's implicit by default and
> > can be added optionally when a system is wired in a complex way. As it's
> > a completely standard optional property I'm not even sure it needs to be
> > documented.
> > 
> > > +  - interrupts: Interrupt number.
> > 
> > Minor nit: "Should contain an interrupt-specifier for ..."
> > 
> > I saw the cover mentioned multiple interrupts. Which logical interrupt
> > output of the device are you expecting here?
> 
> SCI devices come in several flavours, with individual interrupt sources wired 
> up to different interrupt lines, or multiplexed all together. As the DT 
> bindings support the later only at the moment, there's only a single interrupt 
> to handle.

Ok. If the binding just covers the muxed interrupt for now that's fine.
We can add interrupt-names later when we support multiple interrupts
(and fall back to assuming muxed when no interrupt-names property is
present).

> 
> Should we come up with a standard way of saying in the bindings that a device 
> uses the interrupt bindings defined in interrupt-controller/interrupts.txt ? 
> This will become even more important with the interrupts-extended property, we 
> don't want every DT bindings document to detail all possible way to specify 
> interrupts.

It would certainly be nice to have a standard way of doing just that.
I'd hope that with the schema work going on that would fall into place
automatically, so perhaps it's not worth doing now.

> 
> > > +
> > > +  - clocks: Reference to the SCIx UART interface clock.
> > 
> > Minor nit: s/Reference to/A phandle + clock-specifier pair for/
> > 
> > > +  - clock-names: Should be "sci_ick".
> > 
> > As we're using clock-names, it would be nicer still to define clocks in
> > terms of clock-names so as to make it easier to update the document in
> > future and make the expected use of clock-names clearer:
> > 
> > - clocks: Should
> 
> "Should", or "must" ?

I don't expect the format of clocks and the relationship to clock-names
to change, so "must" is likely more appropriate. I'm just too used to
writing "should"...

> 
> > contain a phandle + clock-specifier pair for each entry
> >   in clock-names.
> > 
> > - clock-names: Should contain "sci_ick".
> 
> Shouldn't that be
> 
> "Should contain "sci_ick" for the SCIx UART interface clock."
> 
> ? Otherwise the bindings wouldn't tell which clock the clocks property should 
> reference.

Yes, that sounds much better.

Cheers,
Mark.

Re: [PATCH v2 28/29] serial: sh-sci: Add device tree bindings documentation

[Laurent Pinchart]

Hi Mark,

On Monday 11 November 2013 15:48:49 Mark Rutland wrote:
> On Mon, Nov 11, 2013 at 01:26:11PM +0000, Laurent Pinchart wrote:
> > Hi Mark,
> > 
> > Thank you for the quick and detailed review.
> > 
> > On Monday 11 November 2013 09:40:39 Mark Rutland wrote:
> > > Hi Laurent,
> > > 
> > > I have a few comments.
> > > 
> > > On Sun, Nov 10, 2013 at 03:33:40AM +0000, Laurent Pinchart wrote:
> > > > Document the device tree bindings for the sci serial port devices.
> > > > 
> > > > Cc: devicetree@vger.kernel.org
> > > > Signed-off-by: Laurent Pinchart
> > > > <laurent.pinchart+renesas@ideasonboard.com>
> > > > Acked-by: Simon Horman <horms+renesas@verge.net.au>
> > > > ---
> > > > 
> > > >  .../bindings/serial/renesas,sci-serial.txt         | 42 +++++++++++++
> > > >  1 file changed, 42 insertions(+)
> > > >  create mode 100644
> > > >  Documentation/devicetree/bindings/serial/renesas,sci-serial.txt>
> > > > 
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> > > > b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt new
> > > > file mode 100644
> > > > index 0000000..66d3bca
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> > > > @@ -0,0 +1,42 @@
> > > > +* Renesas SH-Mobile Serial Communication Interface
> > > > +
> > > > +Required properties:
> > > > +
> > > > +  - compatible: one of the following types (scif, scifa, scifb,
> > > > hscif).
> > > 
> > > Minor nit, but would be nice to have "Should contain one of the
> > > following:". We might have future variants whereby the compatible string
> > > will actually be a string list.
> > 
> > Sure, I can do that.
> > 
> > > > +
> > > > +    - "renesas,scif-r8a7790" for R8A7790 (R-Car H2) SCIF compatible
> > > > UART.
> > > > +    - "renesas,scifa-r8a7790" for R8A7790 (R-Car H2) SCIFA compatible
> > > > UART. +    - "renesas,scifb-r8a7790" for R8A7790 (R-Car H2) SCIFB
> > > > compatible UART. +    - "renesas,hscif-r8a7790" for R8A7790 (R-Car H2)
> > > > HSCIF compatible UART. +    - "renesas,scif-generic" for generic SCIF
> > > > compatible UART.
> > > > +    - "renesas,scifa-generic" for generic SCIFA compatible UART.
> > > > +    - "renesas,scifb-generic" for generic SCIFB compatible UART.
> > > > +    - "renesas,hscif-generic" for generic HSCIF compatible UART.
> > > 
> > > Is the "-generic" suffix necessary? Why not just "renesas,scif" etc?
> > 
> > You're right, I'll remove the suffix.
> > 
> > > > +
> > > > +    When compatible with the generic version, nodes must also list
> > > > the
> > > > +    SoC-specific version corresponding to the platform.
> > > 
> > > Presumably the SoC-specific version should come first; it would be nice
> > > to note that.
> > 
> > Indeed, I'll do so.
> > 
> > > > +
> > > > +  - reg: Base address and length of the memory resource used by the
> > > > UART.
> > > 
> > > I assume by memory resource you mean the memory-mapped registers?
> > > Resource sounds like Linux-internal nomenclature leaking.
> > 
> > I'll fix it as well, although we could decide on a definition of
> > "resource" that isn't Linux-specific :-)
> 
> We could, I guess. For the moment I'd prefer to point out that these are
> MMIO registers :)
> 
> > > > +
> > > > +  - interrupt-parent: Reference to the parent interrupt controller.
> > > 
> > > I don't think this is strictly necessary. It's implicit by default and
> > > can be added optionally when a system is wired in a complex way. As it's
> > > a completely standard optional property I'm not even sure it needs to be
> > > documented.
> > > 
> > > > +  - interrupts: Interrupt number.
> > > 
> > > Minor nit: "Should contain an interrupt-specifier for ..."
> > > 
> > > I saw the cover mentioned multiple interrupts. Which logical interrupt
> > > output of the device are you expecting here?
> > 
> > SCI devices come in several flavours, with individual interrupt sources
> > wired up to different interrupt lines, or multiplexed all together. As
> > the DT bindings support the later only at the moment, there's only a
> > single interrupt to handle.
> 
> Ok. If the binding just covers the muxed interrupt for now that's fine.
> We can add interrupt-names later when we support multiple interrupts
> (and fall back to assuming muxed when no interrupt-names property is
> present).

That was my plan, good.

> > Should we come up with a standard way of saying in the bindings that a
> > device uses the interrupt bindings defined in
> > interrupt-controller/interrupts.txt ? This will become even more
> > important with the interrupts-extended property, we don't want every DT
> > bindings document to detail all possible way to specify interrupts.
> 
> It would certainly be nice to have a standard way of doing just that.
> I'd hope that with the schema work going on that would fall into place
> automatically, so perhaps it's not worth doing now.

Fine with me.

> > > > +
> > > > +  - clocks: Reference to the SCIx UART interface clock.
> > > 
> > > Minor nit: s/Reference to/A phandle + clock-specifier pair for/
> > > 
> > > > +  - clock-names: Should be "sci_ick".
> > > 
> > > As we're using clock-names, it would be nicer still to define clocks in
> > > terms of clock-names so as to make it easier to update the document in
> > > future and make the expected use of clock-names clearer:
> > > 
> > > - clocks: Should
> > 
> > "Should", or "must" ?
> 
> I don't expect the format of clocks and the relationship to clock-names
> to change, so "must" is likely more appropriate. I'm just too used to
> writing "should"...

Are "Should" and "Must" to be interpreted as described in 
http://tools.ietf.org/html/rfc2119 ?

1. MUST   This word, or the terms "REQUIRED" or "SHALL", mean that the
   definition is an absolute requirement of the specification.

3. SHOULD   This word, or the adjective "RECOMMENDED", mean that there
   may exist valid reasons in particular circumstances to ignore a
   particular item, but the full implications must be understood and
   carefully weighed before choosing a different course.

If so, shouldn't we use "Must" in most places ?

> > > contain a phandle + clock-specifier pair for each entry
> > > 
> > >   in clock-names.
> > > 
> > > - clock-names: Should contain "sci_ick".
> > 
> > Shouldn't that be
> > 
> > "Should contain "sci_ick" for the SCIx UART interface clock."
> > 
> > ? Otherwise the bindings wouldn't tell which clock the clocks property
> > should reference.
> 
> Yes, that sounds much better.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 00/29] Add OF support to the sh-sci serial port driver

[Simon Horman]

On Sun, Nov 10, 2013 at 04:33:12AM +0100, Laurent Pinchart wrote:
> Hello,
> 
> This is the second version of the patch set adds device tree bindings for the
> sh sci serial port devices and adds OF parsing to the sh-sci driver.
> 
> The bindings are based on Bastian Hecht's proposal (see
> http://www.spinics.net/lists/arm-kernel/msg228129.html). The approach taken
> here is more minimalistic: instead of describing all hardware characteristics
> that vary between the SCI device revisions in DT (such as registers layout),
> that information is stored in the driver and selected based on the compatible
> property value. Only SCI revisions used on ARM devices are supported through
> DT, as DT support for SuperH is nowhere down the line.
> 
> Patches 01/29 to 06/29 clean up the sh-sci driver.

It seems to me that these could be queued up by Greg.
Let me know if you would like me to pass them on to him.
I'm quite happy for you to do that yourself. Or to wait.
Its entirely up to you.

In any case for those patches:

Acked-by: Simon Horman <horms+renesas@verge.net.au>

> Patches 07/29 to 27/29
> replace memory and interrupt resources passed through platform data with
> platform resources. Beside replacing a custom mechanism with a standard one,
> it will also make the DT parsing code simpler as resource allocation will be
> shared between DT and non-DT code paths.
> Finally, patches 28/29 to 29/29 add
> OF parsing to the sh-sci driver and create DT bindings documentation.

29 is awaiting devicetree review and 28 makes little sense
before that happens. Right?

> The patches have been test on a Lager board (r8a7790-based). Support for other
> SoCs will be added as needed. Note that all current Renesas ARM SoCs seem to
> be compatible with the generic (H)SCI(F)(AB) devices, but the plan is for
> their DT bindings to list the SoC-specific version in case incompatibilities
> are found later.
> 
> Patch 08/29 ("serial: sh-sci: Support resources passed through platform
> resources") is the only dependency of the arch/arm and arch/sh patches present
> in this series. As it's too late to get it in v3.13, Simon will need a stable
> branch of the linux serial tree with that patch included.

That is patch 7 not 8, right?

Do all of the shmobile patches depends on that patch?
I would be happy to take some earlier if practical.

> 
> Changes compared to v1:
> 
> - Patches 04/29 to 27/29 have been added
> - Multi-interrupt support has been removed from the DT bindings as they don't
>   support any SoC requiring multi-interrupt yet
> 
> Cc: devicetree@vger.kernel.org
> 
> Bastian Hecht (1):
>   serial: sh-sci: Add OF support
> 
> Laurent Pinchart (28):
>   serial: sh-sci: Sort headers alphabetically
>   serial: sh-sci: Remove baud rate calculation algorithm 5
>   serial: sh-sci: Simplify baud rate calculation algorithms
>   serial: sh-sci: Remove duplicate interrupt check in verify port op
>   serial: sh-sci: Set the UPF_FIXED_PORT flag
>   serial: sh-sci: Don't check IRQ in verify port operation
>   serial: sh-sci: Support resources passed through platform resources
>   ARM: shmobile: sh7372: Use macros to declare SCIF devices
>   ARM: shmobile: sh73a0: Use macros to declare SCIF devices
>   ARM: shmobile: r8a7740: Use macros to declare SCIF devices
>   ARM: shmobile: r8a7779: Use macros to declare SCIF devices
>   ARM: shmobile: r7s72100: Don't define SCIF platform data in an array
>   ARM: shmobile: r8a73a4: Don't define SCIF platform data in an array
>   ARM: shmobile: r8a7778: Don't define SCIF platform data in an array
>   ARM: shmobile: r8a7790: Don't define SCIF platform data in an array
>   ARM: shmobile: r8a7791: Don't define SCIF platform data in an array
>   ARM: shmobile: sh7372: Declare SCIF register base and IRQ as resources
>   ARM: shmobile: sh73a0: Declare SCIF register base and IRQ as resources
>   ARM: shmobile: r7s72100: Declare SCIF register base and IRQ as
>     resources
>   ARM: shmobile: r8a73a4: Declare SCIF register base and IRQ as
>     resources
>   ARM: shmobile: r8a7740: Declare SCIF register base and IRQ as
>     resources
>   ARM: shmobile: r8a7778: Declare SCIF register base and IRQ as
>     resources
>   ARM: shmobile: r8a7779: Declare SCIF register base and IRQ as
>     resources
>   ARM: shmobile: r8a7790: Declare SCIF register base and IRQ as
>     resources
>   ARM: shmobile: r8a7791: Declare SCIF register base and IRQ as
>     resources
>   sh: Declare SCIF register base and IRQ as resources
>   serial: sh-sci: Remove platform data mapbase and irqs fields
>   serial: sh-sci: Add device tree bindings documentation
> 
>  .../bindings/serial/renesas,sci-serial.txt         |  42 ++++
>  arch/arm/mach-shmobile/setup-r7s72100.c            |  63 +++---
>  arch/arm/mach-shmobile/setup-r8a73a4.c             |  65 +++---
>  arch/arm/mach-shmobile/setup-r8a7740.c             | 196 +++--------------
>  arch/arm/mach-shmobile/setup-r8a7778.c             |  45 ++--
>  arch/arm/mach-shmobile/setup-r8a7779.c             | 129 +++--------
>  arch/arm/mach-shmobile/setup-r8a7790.c             | 117 +++++-----
>  arch/arm/mach-shmobile/setup-r8a7791.c             | 130 +++++------
>  arch/arm/mach-shmobile/setup-sh7372.c              | 161 +++-----------
>  arch/arm/mach-shmobile/setup-sh73a0.c              | 192 +++--------------
>  arch/sh/kernel/cpu/sh2/setup-sh7619.c              |  27 ++-
>  arch/sh/kernel/cpu/sh2a/setup-mxg.c                |   9 +-
>  arch/sh/kernel/cpu/sh2a/setup-sh7201.c             |  72 +++++--
>  arch/sh/kernel/cpu/sh2a/setup-sh7203.c             |  36 +++-
>  arch/sh/kernel/cpu/sh2a/setup-sh7206.c             |  36 +++-
>  arch/sh/kernel/cpu/sh2a/setup-sh7264.c             |  96 +++++++--
>  arch/sh/kernel/cpu/sh2a/setup-sh7269.c             |  96 +++++++--
>  arch/sh/kernel/cpu/sh3/setup-sh7705.c              |  18 +-
>  arch/sh/kernel/cpu/sh3/setup-sh770x.c              |  27 ++-
>  arch/sh/kernel/cpu/sh3/setup-sh7710.c              |  18 +-
>  arch/sh/kernel/cpu/sh3/setup-sh7720.c              |  18 +-
>  arch/sh/kernel/cpu/sh4/setup-sh4-202.c             |  15 +-
>  arch/sh/kernel/cpu/sh4/setup-sh7750.c              |  18 +-
>  arch/sh/kernel/cpu/sh4/setup-sh7760.c              |  58 +++--
>  arch/sh/kernel/cpu/sh4a/setup-sh7343.c             |  36 +++-
>  arch/sh/kernel/cpu/sh4a/setup-sh7366.c             |   9 +-
>  arch/sh/kernel/cpu/sh4a/setup-sh7722.c             |  27 ++-
>  arch/sh/kernel/cpu/sh4a/setup-sh7723.c             |  54 +++--
>  arch/sh/kernel/cpu/sh4a/setup-sh7724.c             |  54 +++--
>  arch/sh/kernel/cpu/sh4a/setup-sh7734.c             |  66 ++++--
>  arch/sh/kernel/cpu/sh4a/setup-sh7757.c             |  27 ++-
>  arch/sh/kernel/cpu/sh4a/setup-sh7763.c             |  27 ++-
>  arch/sh/kernel/cpu/sh4a/setup-sh7770.c             |  90 ++++++--
>  arch/sh/kernel/cpu/sh4a/setup-sh7780.c             |  18 +-
>  arch/sh/kernel/cpu/sh4a/setup-sh7785.c             |  54 +++--
>  arch/sh/kernel/cpu/sh4a/setup-sh7786.c             |  82 +++++--
>  arch/sh/kernel/cpu/sh4a/setup-shx3.c               |  45 ++--
>  arch/sh/kernel/cpu/sh5/setup-sh5.c                 |  11 +-
>  drivers/tty/serial/sh-sci.c                        | 238 ++++++++++++++++-----
>  include/linux/serial_sci.h                         |  35 +--
>  40 files changed, 1443 insertions(+), 1114 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH v2 00/29] Add OF support to the sh-sci serial port driver

[Laurent Pinchart]

Hi Simon,

On Tuesday 19 November 2013 10:55:39 Simon Horman wrote:
> On Sun, Nov 10, 2013 at 04:33:12AM +0100, Laurent Pinchart wrote:
> > Hello,
> > 
> > This is the second version of the patch set adds device tree bindings for
> > the sh sci serial port devices and adds OF parsing to the sh-sci driver.
> > 
> > The bindings are based on Bastian Hecht's proposal (see
> > http://www.spinics.net/lists/arm-kernel/msg228129.html). The approach
> > taken here is more minimalistic: instead of describing all hardware
> > characteristics that vary between the SCI device revisions in DT (such as
> > registers layout), that information is stored in the driver and selected
> > based on the compatible property value. Only SCI revisions used on ARM
> > devices are supported through DT, as DT support for SuperH is nowhere
> > down the line.
> > 
> > Patches 01/29 to 06/29 clean up the sh-sci driver.
> 
> It seems to me that these could be queued up by Greg.
> Let me know if you would like me to pass them on to him.
> I'm quite happy for you to do that yourself. Or to wait.
> Its entirely up to you.
> 
> In any case for those patches:
> 
> Acked-by: Simon Horman <horms+renesas@verge.net.au>

Thank you. I'll resend the series with the DT bindings updated for r8a7791 and 
will ask Greg to pick the patches up and provide a stable branch with patches 
01/29 to 07/29.

> > Patches 07/29 to 27/29 replace memory and interrupt resources passed
> > through platform data with platform resources. Beside replacing a custom
> > mechanism with a standard one, it will also make the DT parsing code
> > simpler as resource allocation will be shared between DT and non-DT code
> > paths. Finally, patches 28/29 to 29/29 add OF parsing to the sh-sci driver
> > and create DT bindings documentation.
> 
> 29 is awaiting devicetree review and 28 makes little sense before that
> happens. Right?

Correct.

> > The patches have been test on a Lager board (r8a7790-based). Support for
> > other SoCs will be added as needed. Note that all current Renesas ARM
> > SoCs seem to be compatible with the generic (H)SCI(F)(AB) devices, but
> > the plan is for their DT bindings to list the SoC-specific version in
> > case incompatibilities are found later.
> > 
> > Patch 08/29 ("serial: sh-sci: Support resources passed through platform
> > resources") is the only dependency of the arch/arm and arch/sh patches
> > present in this series. As it's too late to get it in v3.13, Simon will
> > need a stable branch of the linux serial tree with that patch included.
> 
> That is patch 7 not 8, right?

Yes, sorry.

> Do all of the shmobile patches depends on that patch?

Yes they do.

> I would be happy to take some earlier if practical.
> 
> > Changes compared to v1:
> > 
> > - Patches 04/29 to 27/29 have been added
> > - Multi-interrupt support has been removed from the DT bindings as they
> >   don't support any SoC requiring multi-interrupt yet
> > 
> > Cc: devicetree@vger.kernel.org
> > 
> > Bastian Hecht (1):
> >   serial: sh-sci: Add OF support
> > 
> > Laurent Pinchart (28):
> >   serial: sh-sci: Sort headers alphabetically
> >   serial: sh-sci: Remove baud rate calculation algorithm 5
> >   serial: sh-sci: Simplify baud rate calculation algorithms
> >   serial: sh-sci: Remove duplicate interrupt check in verify port op
> >   serial: sh-sci: Set the UPF_FIXED_PORT flag
> >   serial: sh-sci: Don't check IRQ in verify port operation
> >   serial: sh-sci: Support resources passed through platform resources
> >   ARM: shmobile: sh7372: Use macros to declare SCIF devices
> >   ARM: shmobile: sh73a0: Use macros to declare SCIF devices
> >   ARM: shmobile: r8a7740: Use macros to declare SCIF devices
> >   ARM: shmobile: r8a7779: Use macros to declare SCIF devices
> >   ARM: shmobile: r7s72100: Don't define SCIF platform data in an array
> >   ARM: shmobile: r8a73a4: Don't define SCIF platform data in an array
> >   ARM: shmobile: r8a7778: Don't define SCIF platform data in an array
> >   ARM: shmobile: r8a7790: Don't define SCIF platform data in an array
> >   ARM: shmobile: r8a7791: Don't define SCIF platform data in an array
> >   ARM: shmobile: sh7372: Declare SCIF register base and IRQ as resources
> >   ARM: shmobile: sh73a0: Declare SCIF register base and IRQ as resources
> >   ARM: shmobile: r7s72100: Declare SCIF register base and IRQ as
> >     resources
> >   ARM: shmobile: r8a73a4: Declare SCIF register base and IRQ as
> >     resources
> >   ARM: shmobile: r8a7740: Declare SCIF register base and IRQ as
> >     resources
> >   ARM: shmobile: r8a7778: Declare SCIF register base and IRQ as
> >     resources
> >   ARM: shmobile: r8a7779: Declare SCIF register base and IRQ as
> >     resources
> >   ARM: shmobile: r8a7790: Declare SCIF register base and IRQ as
> >     resources
> >   ARM: shmobile: r8a7791: Declare SCIF register base and IRQ as
> >     resources
> >   
> >   sh: Declare SCIF register base and IRQ as resources
> >   serial: sh-sci: Remove platform data mapbase and irqs fields
> >   serial: sh-sci: Add device tree bindings documentation
> >  
> >  .../bindings/serial/renesas,sci-serial.txt         |  42 ++++
> >  arch/arm/mach-shmobile/setup-r7s72100.c            |  63 +++---
> >  arch/arm/mach-shmobile/setup-r8a73a4.c             |  65 +++---
> >  arch/arm/mach-shmobile/setup-r8a7740.c             | 196 +++-------------
> >  arch/arm/mach-shmobile/setup-r8a7778.c             |  45 ++--
> >  arch/arm/mach-shmobile/setup-r8a7779.c             | 129 +++--------
> >  arch/arm/mach-shmobile/setup-r8a7790.c             | 117 +++++-----
> >  arch/arm/mach-shmobile/setup-r8a7791.c             | 130 +++++------
> >  arch/arm/mach-shmobile/setup-sh7372.c              | 161 +++-----------
> >  arch/arm/mach-shmobile/setup-sh73a0.c              | 192 +++-------------
> >  arch/sh/kernel/cpu/sh2/setup-sh7619.c              |  27 ++-
> >  arch/sh/kernel/cpu/sh2a/setup-mxg.c                |   9 +-
> >  arch/sh/kernel/cpu/sh2a/setup-sh7201.c             |  72 +++++--
> >  arch/sh/kernel/cpu/sh2a/setup-sh7203.c             |  36 +++-
> >  arch/sh/kernel/cpu/sh2a/setup-sh7206.c             |  36 +++-
> >  arch/sh/kernel/cpu/sh2a/setup-sh7264.c             |  96 +++++++--
> >  arch/sh/kernel/cpu/sh2a/setup-sh7269.c             |  96 +++++++--
> >  arch/sh/kernel/cpu/sh3/setup-sh7705.c              |  18 +-
> >  arch/sh/kernel/cpu/sh3/setup-sh770x.c              |  27 ++-
> >  arch/sh/kernel/cpu/sh3/setup-sh7710.c              |  18 +-
> >  arch/sh/kernel/cpu/sh3/setup-sh7720.c              |  18 +-
> >  arch/sh/kernel/cpu/sh4/setup-sh4-202.c             |  15 +-
> >  arch/sh/kernel/cpu/sh4/setup-sh7750.c              |  18 +-
> >  arch/sh/kernel/cpu/sh4/setup-sh7760.c              |  58 +++--
> >  arch/sh/kernel/cpu/sh4a/setup-sh7343.c             |  36 +++-
> >  arch/sh/kernel/cpu/sh4a/setup-sh7366.c             |   9 +-
> >  arch/sh/kernel/cpu/sh4a/setup-sh7722.c             |  27 ++-
> >  arch/sh/kernel/cpu/sh4a/setup-sh7723.c             |  54 +++--
> >  arch/sh/kernel/cpu/sh4a/setup-sh7724.c             |  54 +++--
> >  arch/sh/kernel/cpu/sh4a/setup-sh7734.c             |  66 ++++--
> >  arch/sh/kernel/cpu/sh4a/setup-sh7757.c             |  27 ++-
> >  arch/sh/kernel/cpu/sh4a/setup-sh7763.c             |  27 ++-
> >  arch/sh/kernel/cpu/sh4a/setup-sh7770.c             |  90 ++++++--
> >  arch/sh/kernel/cpu/sh4a/setup-sh7780.c             |  18 +-
> >  arch/sh/kernel/cpu/sh4a/setup-sh7785.c             |  54 +++--
> >  arch/sh/kernel/cpu/sh4a/setup-sh7786.c             |  82 +++++--
> >  arch/sh/kernel/cpu/sh4a/setup-shx3.c               |  45 ++--
> >  arch/sh/kernel/cpu/sh5/setup-sh5.c                 |  11 +-
> >  drivers/tty/serial/sh-sci.c                        | 238 +++++++++++++---
> >  include/linux/serial_sci.h                         |  35 +--
> >  40 files changed, 1443 insertions(+), 1114 deletions(-)
> >  create mode 100644
> >  Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 00/29] Add OF support to the sh-sci serial port driver

[Simon Horman]

On Tue, Nov 19, 2013 at 02:12:43PM +0100, Laurent Pinchart wrote:
> Hi Simon,
> 
> On Tuesday 19 November 2013 10:55:39 Simon Horman wrote:
> > On Sun, Nov 10, 2013 at 04:33:12AM +0100, Laurent Pinchart wrote:
> > > Hello,
> > > 
> > > This is the second version of the patch set adds device tree bindings for
> > > the sh sci serial port devices and adds OF parsing to the sh-sci driver.
> > > 
> > > The bindings are based on Bastian Hecht's proposal (see
> > > http://www.spinics.net/lists/arm-kernel/msg228129.html). The approach
> > > taken here is more minimalistic: instead of describing all hardware
> > > characteristics that vary between the SCI device revisions in DT (such as
> > > registers layout), that information is stored in the driver and selected
> > > based on the compatible property value. Only SCI revisions used on ARM
> > > devices are supported through DT, as DT support for SuperH is nowhere
> > > down the line.
> > > 
> > > Patches 01/29 to 06/29 clean up the sh-sci driver.
> > 
> > It seems to me that these could be queued up by Greg.
> > Let me know if you would like me to pass them on to him.
> > I'm quite happy for you to do that yourself. Or to wait.
> > Its entirely up to you.
> > 
> > In any case for those patches:
> > 
> > Acked-by: Simon Horman <horms+renesas@verge.net.au>
> 
> Thank you. I'll resend the series with the DT bindings updated for r8a7791 and 
> will ask Greg to pick the patches up and provide a stable branch with patches 
> 01/29 to 07/29.

Thanks, that sounds great.