On Mon, May 28, 2018 at 03:28:40PM +0200, Geert Uytterhoeven wrote: > From: Hiromitsu Yamasaki <[email protected]> > > 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 <[email protected]> > Signed-off-by: Takeshi Kihara <[email protected]> > [geert: Add missing "uart-has-rtscts"] > Signed-off-by: Geert Uytterhoeven <[email protected]> > --- > 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 >
