Re: [PATCH/RFC] arm64: dts: renesas: salvator-common: Add HSCIF1 device support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
> 



[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux