[v11,6/8] arm64: renesas: add Salvator-X board support on DTS

Commit Message

Simon Horman Oct. 15, 2015, 6:24 a.m. UTC

From: Kuninori Morimoto <[e-mail]>

Add initial board support for r8a7795 Salvator-X. At this point
only DEBUG0 and DEBUG1 serial ports are supported.

Signed-off-by: Kuninori Morimoto <[e-mail]>
Signed-off-by: Gaku Inami <[e-mail]>
Signed-off-by: Takeshi Kihara <[e-mail]>
Signed-off-by: Geert Uytterhoeven <[e-mail]>
Signed-off-by: Magnus Damm <[e-mail]>
Signed-off-by: Simon Horman <[e-mail]>
---
Changes since v10:
- None

Changes since V9: (Magnus Damm <[e-mail]>)
 - Added board specific EXTAL information by folding in:
  [PATCH][RFC] arm64: renesas: Add EXTAL configuration to Salvator-X

Changes since V8: (Magnus Damm <[e-mail]>)
- None

Changes since V7: (Magnus Damm <[e-mail]>)
- Added changelog

Based on:
 [PATCH 15/25] arm64: renesas: add Salvator-X board support on DTS
 [PATCH 5/5] arm64: renesas: salvator-x: Update SCIF2 pin group
 [PATCH 5/6] arm64: renesas: salvator-x: Enable SCIF1 on serial1
---
 Documentation/devicetree/bindings/arm/shmobile.txt |  2 +
 arch/arm64/boot/dts/renesas/Makefile               |  2 +
 arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts | 62 ++++++++++++++++++++++
 3 files changed, 66 insertions(+)
 create mode 100644 arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts

Comments

Mark Rutland Oct. 15, 2015, 11:01 a.m. UTC | #1

> +/ {
> +	model = "Renesas Salvator-X board based on r8a7795";
> +	compatible = "renesas,salvator-x", "renesas,r8a7795";
> +
> +	aliases {
> +		serial0 = &scif2;
> +		serial1 = &scif1;
> +	};
> +
> +	chosen {
> +		bootargs = "ignore_loglevel";

Do we really need this kind of thing in the kernel DTs?

> +		stdout-path = &scif2;

No rate? It would be better to be explicit here; you should be able to
have:

	stdout-path = "serial0:115200n8"

Where "115200n8" is replaced with whatever configuration this board
actually has.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Simon Horman Oct. 23, 2015, 7 a.m. UTC | #2

On Thu, Oct 15, 2015 at 12:01:40PM +0100, Mark Rutland wrote:
> > +/ {
> > +	model = "Renesas Salvator-X board based on r8a7795";
> > +	compatible = "renesas,salvator-x", "renesas,r8a7795";
> > +
> > +	aliases {
> > +		serial0 = &scif2;
> > +		serial1 = &scif1;
> > +	};
> > +
> > +	chosen {
> > +		bootargs = "ignore_loglevel";
> 
> Do we really need this kind of thing in the kernel DTs?

I agree this looks a bit lonely by itself, and I'm not entirely
opposed to removing it. However, I do seek consistency between
the renesas ARM and ARM64 SoCs where possible.

Further work[1] on this file outside of this series adds
"rw root=/dev/nfs ip=dhcp" to bootargs, the result being consistent
with Renesas ARM32 SoCs.

[1] [PATCH v5 4/5] arm64: dts: r8a7795: enable nfs root on Salvator-X board
    http://www.spinics.net/lists/arm-kernel/msg452743.html

> > +		stdout-path = &scif2;
> 
> No rate? It would be better to be explicit here; you should be able to
> have:
> 
> 	stdout-path = "serial0:115200n8"
> 
> Where "115200n8" is replaced with whatever configuration this board
> actually has.

I think that we have had this discussion before in relation to
a different board for a different Renesas SoC but I could be mistaken.

The r8a7795 uses the sh-sci serial driver for which the default rate is
115200. It is hard for me to conceive of a situation where that would
change without due consideration being given to the implications for DT
files.

Not specifying the baud here is consistent with what we have
been doing for ARM32 Renesas SoCs for some time.
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Geert Uytterhoeven Oct. 29, 2015, 7:52 a.m. UTC | #3

Hi Simon,

On Fri, Oct 23, 2015 at 9:00 AM, Simon Horman <[e-mail]> wrote:
> On Thu, Oct 15, 2015 at 12:01:40PM +0100, Mark Rutland wrote:
>> > +           stdout-path = &scif2;
>>
>> No rate? It would be better to be explicit here; you should be able to
>> have:
>>
>>       stdout-path = "serial0:115200n8"
>>
>> Where "115200n8" is replaced with whatever configuration this board
>> actually has.
>
> I think that we have had this discussion before in relation to
> a different board for a different Renesas SoC but I could be mistaken.

IIRC, at that time the code to parse the options wasn't upstream yet, so
adding the options would have broken the serial console.

I can confirm it works fine on Salvator-X with

        stdout-path = "serial0:115200n8";

> The r8a7795 uses the sh-sci serial driver for which the default rate is
> 115200. It is hard for me to conceive of a situation where that would
> change without due consideration being given to the implications for DT
> files.
>
> Not specifying the baud here is consistent with what we have
> been doing for ARM32 Renesas SoCs for some time.

FWIW, I have updated all ARM32 Renesas DTSes locally:
  1. Use alias in and add serial options to stdout-path,
  2. Drop superfluous console= from bootargs (shmobile-legacy is gone,
     and it works fine on boards with both fbdev and serial consoles).

And everything looks fine on the boards I could try it on (sh73a0/kzm9g,
r8a73a4/ape6evm, r8a7740/armadillo, r8a7791/koelsch).

Hence if you're open for this change, I can submit patches (or a single patch,
up to your preference).

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [e-mail]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Simon Horman Oct. 30, 2015, 7:51 a.m. UTC | #4

On Thu, Oct 29, 2015 at 08:52:12AM +0100, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Fri, Oct 23, 2015 at 9:00 AM, Simon Horman <[e-mail]> wrote:
> > On Thu, Oct 15, 2015 at 12:01:40PM +0100, Mark Rutland wrote:
> >> > +           stdout-path = &scif2;
> >>
> >> No rate? It would be better to be explicit here; you should be able to
> >> have:
> >>
> >>       stdout-path = "serial0:115200n8"
> >>
> >> Where "115200n8" is replaced with whatever configuration this board
> >> actually has.
> >
> > I think that we have had this discussion before in relation to
> > a different board for a different Renesas SoC but I could be mistaken.
> 
> IIRC, at that time the code to parse the options wasn't upstream yet, so
> adding the options would have broken the serial console.
> 
> I can confirm it works fine on Salvator-X with
> 
>         stdout-path = "serial0:115200n8";

Thanks.

Is this considered to be best practice?
If so I am of a mind to update the patch.

> > The r8a7795 uses the sh-sci serial driver for which the default rate is
> > 115200. It is hard for me to conceive of a situation where that would
> > change without due consideration being given to the implications for DT
> > files.
> >
> > Not specifying the baud here is consistent with what we have
> > been doing for ARM32 Renesas SoCs for some time.
> 
> FWIW, I have updated all ARM32 Renesas DTSes locally:
>   1. Use alias in and add serial options to stdout-path,
>   2. Drop superfluous console= from bootargs (shmobile-legacy is gone,
>      and it works fine on boards with both fbdev and serial consoles).
> 
> And everything looks fine on the boards I could try it on (sh73a0/kzm9g,
> r8a73a4/ape6evm, r8a7740/armadillo, r8a7791/koelsch).
> 
> Hence if you're open for this change, I can submit patches (or a single patch,
> up to your preference).

Sure, I'm happy to consider that (I slightly prefer per-board patches) if:

* That is now considered best practice and;
* Due consideration is given to backwards compatibility
  (I suspect it is new DTs work with old kernels so its a non-issue)
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Geert Uytterhoeven Nov. 3, 2015, 2:24 p.m. UTC | #5

Hi Mark, Rob,

On Thu, Oct 29, 2015 at 8:52 AM, Geert Uytterhoeven
<[e-mail]> wrote:
> On Fri, Oct 23, 2015 at 9:00 AM, Simon Horman <[e-mail]> wrote:
>> On Thu, Oct 15, 2015 at 12:01:40PM +0100, Mark Rutland wrote:
>>> > +           stdout-path = &scif2;
>>>
>>> No rate? It would be better to be explicit here; you should be able to
>>> have:
>>>
>>>       stdout-path = "serial0:115200n8"
>>>
>>> Where "115200n8" is replaced with whatever configuration this board
>>> actually has.
>>
>> I think that we have had this discussion before in relation to
>> a different board for a different Renesas SoC but I could be mistaken.
>
> IIRC, at that time the code to parse the options wasn't upstream yet, so
> adding the options would have broken the serial console.
>
> I can confirm it works fine on Salvator-X with
>
>         stdout-path = "serial0:115200n8";

Unfortunately this breaks earlycon support:

        Malformed early option 'earlycon'

The call to fdt_getprop() in early_init_dt_scan_chosen_serial() fails.
I guess that unlike the standard console handling code, the fdt code can't
handle aliases yet?

P.S. SCIF earlycon support is not yet upstream, as it still doesn't work on
arm32 (I discovered today it does on arm64 ;-): it pretends to work, but
nothing is output to the serial port (until the normal serial console kicks
in).

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [e-mail]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Simon Horman Nov. 9, 2015, 2:19 a.m. UTC | #6

Hi,

On Tue, Nov 03, 2015 at 03:24:06PM +0100, Geert Uytterhoeven wrote:
> Hi Mark, Rob,
> 
> On Thu, Oct 29, 2015 at 8:52 AM, Geert Uytterhoeven
> <[e-mail]> wrote:
> > On Fri, Oct 23, 2015 at 9:00 AM, Simon Horman <[e-mail]> wrote:
> >> On Thu, Oct 15, 2015 at 12:01:40PM +0100, Mark Rutland wrote:
> >>> > +           stdout-path = &scif2;
> >>>
> >>> No rate? It would be better to be explicit here; you should be able to
> >>> have:
> >>>
> >>>       stdout-path = "serial0:115200n8"
> >>>
> >>> Where "115200n8" is replaced with whatever configuration this board
> >>> actually has.
> >>
> >> I think that we have had this discussion before in relation to
> >> a different board for a different Renesas SoC but I could be mistaken.
> >
> > IIRC, at that time the code to parse the options wasn't upstream yet, so
> > adding the options would have broken the serial console.
> >
> > I can confirm it works fine on Salvator-X with
> >
> >         stdout-path = "serial0:115200n8";
> 
> Unfortunately this breaks earlycon support:
> 
>         Malformed early option 'earlycon'
> 
> The call to fdt_getprop() in early_init_dt_scan_chosen_serial() fails.
> I guess that unlike the standard console handling code, the fdt code can't
> handle aliases yet?
> 
> P.S. SCIF earlycon support is not yet upstream, as it still doesn't work on
> arm32 (I discovered today it does on arm64 ;-): it pretends to work, but
> nothing is output to the serial port (until the normal serial console kicks
> in).

As discussed off-list.

I will update the stdout-path as Mark suggested.
The expectation being that early con will support that syntax.
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Geert Uytterhoeven Nov. 16, 2015, 9:53 a.m. UTC | #7

On Tue, Nov 3, 2015 at 3:24 PM, Geert Uytterhoeven <[e-mail]> wrote:
> On Thu, Oct 29, 2015 at 8:52 AM, Geert Uytterhoeven
> <[e-mail]> wrote:
>> On Fri, Oct 23, 2015 at 9:00 AM, Simon Horman <[e-mail]> wrote:
>>> On Thu, Oct 15, 2015 at 12:01:40PM +0100, Mark Rutland wrote:
>>>> > +           stdout-path = &scif2;
>>>>
>>>> No rate? It would be better to be explicit here; you should be able to
>>>> have:
>>>>
>>>>       stdout-path = "serial0:115200n8"
>>>>
>>>> Where "115200n8" is replaced with whatever configuration this board
>>>> actually has.
>>>
>>> I think that we have had this discussion before in relation to
>>> a different board for a different Renesas SoC but I could be mistaken.
>>
>> IIRC, at that time the code to parse the options wasn't upstream yet, so
>> adding the options would have broken the serial console.
>>
>> I can confirm it works fine on Salvator-X with
>>
>>         stdout-path = "serial0:115200n8";
>
> Unfortunately this breaks earlycon support:
>
>         Malformed early option 'earlycon'
>
> The call to fdt_getprop() in early_init_dt_scan_chosen_serial() fails.
> I guess that unlike the standard console handling code, the fdt code can't
> handle aliases yet?

Apparently this issue was fixed in v4.4-rc1. For the backport team:

commit 6296ad9e3375c6c1ddbb371f589ba6a145bb31df
Author: Stefan Agner <[e-mail]>
Date:   Sat Oct 10 01:29:30 2015 -0700

    of/fdt: fix aliases with baudrate in earlycon

> P.S. SCIF earlycon support is not yet upstream, as it still doesn't work on
> arm32 (I discovered today it does on arm64 ;-): it pretends to work, but
> nothing is output to the serial port (until the normal serial console kicks
> in).

And this issue is mostly resolved, too. So v4.5 will be perfect ;-)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [e-mail]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Simon Horman Nov. 17, 2015, 5:31 p.m. UTC | #8

On Mon, Nov 16, 2015 at 10:53:39AM +0100, Geert Uytterhoeven wrote:
> On Tue, Nov 3, 2015 at 3:24 PM, Geert Uytterhoeven <[e-mail]> wrote:
> > On Thu, Oct 29, 2015 at 8:52 AM, Geert Uytterhoeven
> > <[e-mail]> wrote:
> >> On Fri, Oct 23, 2015 at 9:00 AM, Simon Horman <[e-mail]> wrote:
> >>> On Thu, Oct 15, 2015 at 12:01:40PM +0100, Mark Rutland wrote:
> >>>> > +           stdout-path = &scif2;
> >>>>
> >>>> No rate? It would be better to be explicit here; you should be able to
> >>>> have:
> >>>>
> >>>>       stdout-path = "serial0:115200n8"
> >>>>
> >>>> Where "115200n8" is replaced with whatever configuration this board
> >>>> actually has.
> >>>
> >>> I think that we have had this discussion before in relation to
> >>> a different board for a different Renesas SoC but I could be mistaken.
> >>
> >> IIRC, at that time the code to parse the options wasn't upstream yet, so
> >> adding the options would have broken the serial console.
> >>
> >> I can confirm it works fine on Salvator-X with
> >>
> >>         stdout-path = "serial0:115200n8";
> >
> > Unfortunately this breaks earlycon support:
> >
> >         Malformed early option 'earlycon'
> >
> > The call to fdt_getprop() in early_init_dt_scan_chosen_serial() fails.
> > I guess that unlike the standard console handling code, the fdt code can't
> > handle aliases yet?
> 
> Apparently this issue was fixed in v4.4-rc1. For the backport team:
> 
> commit 6296ad9e3375c6c1ddbb371f589ba6a145bb31df
> Author: Stefan Agner <[e-mail]>
> Date:   Sat Oct 10 01:29:30 2015 -0700
> 
>     of/fdt: fix aliases with baudrate in earlycon
> 
> > P.S. SCIF earlycon support is not yet upstream, as it still doesn't work on
> > arm32 (I discovered today it does on arm64 ;-): it pretends to work, but
> > nothing is output to the serial port (until the normal serial console kicks
> > in).
> 
> And this issue is mostly resolved, too. So v4.5 will be perfect ;-)

Excellent!
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/Documentation/devicetree/bindings/arm/shmobile.txt b/Documentation/devicetree/bindings/arm/shmobile.txt
index 8d696a0d62b3..95d0aea4c701 100644
--- a/Documentation/devicetree/bindings/arm/shmobile.txt
+++ b/Documentation/devicetree/bindings/arm/shmobile.txt
@@ -59,6 +59,8 @@  Boards:
     compatible = "renesas,lager", "renesas,r8a7790"
   - Marzen
     compatible = "renesas,marzen", "renesas,r8a7779"
+  - Salvator-X
+    compatible = "renesas,salvator-x", "renesas,r8a7795";
 
 Note: Reference Device Tree Implementations are temporary implementations
       to ease the migration from platform devices to Device Tree, and are
diff --git a/arch/arm64/boot/dts/renesas/Makefile b/arch/arm64/boot/dts/renesas/Makefile
index fec69f46d65b..9ce1890a650e 100644
--- a/arch/arm64/boot/dts/renesas/Makefile
+++ b/arch/arm64/boot/dts/renesas/Makefile
@@ -1,2 +1,4 @@ 
+dtb-$(CONFIG_ARCH_R8A7795) += r8a7795-salvator-x.dtb
+
 always		:= $(dtb-y)
 clean-files	:= *.dtb
diff --git a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
new file mode 100644
index 000000000000..f522fda7843a
--- /dev/null
+++ b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts
@@ -0,0 +1,62 @@ 
+/*
+ * Device Tree Source for the Salvator-X board
+ *
+ * Copyright (C) 2015 Renesas Electronics Corp.
+ *
+ * This file is licensed under the terms of the GNU General Public License
+ * version 2.  This program is licensed "as is" without any warranty of any
+ * kind, whether express or implied.
+ */
+
+/dts-v1/;
+#include "r8a7795.dtsi"
+
+/ {
+	model = "Renesas Salvator-X board based on r8a7795";
+	compatible = "renesas,salvator-x", "renesas,r8a7795";
+
+	aliases {
+		serial0 = &scif2;
+		serial1 = &scif1;
+	};
+
+	chosen {
+		bootargs = "ignore_loglevel";
+		stdout-path = &scif2;
+	};
+
+	memory@48000000 {
+		device_type = "memory";
+		/* first 128MB is reserved for secure area. */
+		reg = <0x0 0x48000000 0x0 0x38000000>;
+	};
+};
+
+&extal_clk {
+	clock-frequency = <16666666>;
+};
+
+&pfc {
+	scif1_pins: scif1 {
+		renesas,groups = "scif1_data_a", "scif1_ctrl";
+		renesas,function = "scif1";
+	};
+	scif2_pins: scif2 {
+		renesas,groups = "scif2_data_a";
+		renesas,function = "scif2";
+	};
+};
+
+&scif1 {
+	pinctrl-0 = <&scif1_pins>;
+	pinctrl-names = "default";
+
+	status = "okay";
+};
+
+&scif2 {
+	pinctrl-0 = <&scif2_pins>;
+	pinctrl-names = "default";
+
+	status = "okay";
+};