On Mon, May 28, 2018 at 03:28:40PM +0200, Geert Uytterhoeven wrote:
> From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@xxxxxxxxxxx>
>
> This patch adds the pin control for HSCIF1, and supports connection with
> the Debug Serial-1 (CN26) on Salvator boards.
>
> SCIF1 and HSCI1 are sharing the pin connected to the Debug Serial-1
> (CN26) on Salvator boards, and it is necessary to use it by exclusion.
Perhaps, "... it is necessary to ensure that either SCIF1 or HSCIF1 is
enabled, not both".
>
> As for the default of this DeviceTree, SCIF1 is connected.
>
> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@xxxxxxxxxxx>
> Signed-off-by: Takeshi Kihara <takeshi.kihara.df@xxxxxxxxxxx>
> [geert: Add missing "uart-has-rtscts"]
> Signed-off-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> ---
> This depends on "arm64: dts: renesas: r8a77965: Add all HSCIF nodes",
> due to the &hscif1 reference.
>
> This doesn't really change anything, but serves as an example and
> documentation.
>
> Questions:
> 1. Do we want to have such examples/documentation?
This seems reasonable to me. It describes the hardware, though
unfortunately we are not able to describe "exclusion", other than in words.
>
> 2. Should we make HSCIF1 the default? IMHO, HSCIF is superior to SCIF.
> Both are served by the same driver, so increased kernel size is not
> a counter-argument.
> For comparison: r8a7790-lager.dts uses SCIFA1 instead of SCIF1 for
> the debug serial (not for the serial console, due to earlycon and
> U-Boot compatibility).
I lean towards yes. But I am curious to know why Yamasaki-san didn't do so.
> arch/arm64/boot/dts/renesas/salvator-common.dtsi | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> index 9256fbaaab7f3297..976fda67e202f054 100644
> --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> @@ -341,6 +341,15 @@
> clock-frequency = <32768>;
> };
>
> +&hscif1 {
> + pinctrl-0 = <&hscif1_pins>;
> + pinctrl-names = "default";
> +
> + uart-has-rtscts;
> + /* Please use exclusively to the scif1 node */
Perhaps "/* Please only enable hscif1 or scif1 */
> + /* status = "okay"; */
> +};
> +
> &hsusb {
> dr_mode = "otg";
> status = "okay";
> @@ -546,6 +555,11 @@
> function = "du";
> };
>
> + hscif1_pins: hscif1 {
> + groups = "hscif1_data_a", "hscif1_ctrl_a";
> + function = "hscif1";
> + };
> +
> i2c2_pins: i2c2 {
> groups = "i2c2_a";
> function = "i2c2";
> @@ -711,6 +725,7 @@
> pinctrl-names = "default";
>
> uart-has-rtscts;
> + /* Please use exclusively to the hscif1 node */
Ditto
> status = "okay";
> };
>
> --
> 2.7.4
>
