diff mbox series

arm64: dts: renesas: ulcb-kf: add HSCIF1 node

Message ID 20230524203520.1354-1-wsa+renesas@sang-engineering.com (mailing list archive)
State Superseded
Delegated to: Geert Uytterhoeven
Headers show
Series arm64: dts: renesas: ulcb-kf: add HSCIF1 node | expand

Commit Message

Wolfram Sang May 24, 2023, 8:35 p.m. UTC
Exposed on CN4. Tested by connecting it to a Renesas Ebisu board. Also,
remove flow control for SCIF1. The schematics are misleading, the flow
control is for HSCIF1. SCIF1 (for GPS) does not use flow control.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

I extracted the removal of SCIF1 flow control from the GPS patches
because I think that actually belongs here.

 arch/arm64/boot/dts/renesas/ulcb-kf.dtsi | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

Comments

Geert Uytterhoeven May 25, 2023, 8:24 a.m. UTC | #1
Hi Wolfram,

On Wed, May 24, 2023 at 10:37 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> Exposed on CN4. Tested by connecting it to a Renesas Ebisu board. Also,
> remove flow control for SCIF1. The schematics are misleading, the flow
> control is for HSCIF1. SCIF1 (for GPS) does not use flow control.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Thanks for your patch!

It's actually a bit more complicated ;-)
CN4 can be served by either SCIF1 or HSCIF1, including flow control
for both.  But the current pin control setting for SCIF1 is wrong for
that case, as it should use scif1_data_a instead of scif1_data_b.

However, as the only serial port that can be muxed to the GPS pins
is SCIF1, we have no choice but to use SCIF1 for the GPS, and HSCIF1
for CN4, like your patch does.

> ---
>
> I extracted the removal of SCIF1 flow control from the GPS patches
> because I think that actually belongs here.

Yes it does, thanks!

> --- a/arch/arm64/boot/dts/renesas/ulcb-kf.dtsi
> +++ b/arch/arm64/boot/dts/renesas/ulcb-kf.dtsi
> @@ -10,6 +10,7 @@ / {
>         aliases {
>                 serial1 = &hscif0;
>                 serial2 = &scif1;
> +               serial3 = &hscif1;
>                 mmc2 = &sdhi3;
>         };
>
> @@ -132,6 +133,14 @@ &hscif0 {
>         status = "okay";
>  };
>
> +&hscif1 {
> +       pinctrl-0 = <&hscif1_pins>;
> +       pinctrl-names = "default";
> +       uart-has-rtscts;
> +
> +       status = "okay";
> +};
> +
>  &hsusb {
>         dr_mode = "otg";
>         status = "okay";
> @@ -387,8 +396,13 @@ hscif0_pins: hscif0 {
>                 function = "hscif0";
>         };
>
> +       hscif1_pins: hscif1 {
> +               groups = "hscif1_data_a", "hscif1_ctrl_a";
> +               function = "hscif1";
> +       };
> +

The above LGTM, so
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

>         scif1_pins: scif1 {
> -               groups = "scif1_data_b", "scif1_ctrl";
> +               groups = "scif1_data_b";
>                 function = "scif1";
>         };
>
> @@ -418,7 +432,6 @@ &sound_clk_pins
>  &scif1 {
>         pinctrl-0 = <&scif1_pins>;
>         pinctrl-names = "default";
> -       uart-has-rtscts;
>
>         status = "okay";
>  };

This part also LGTM, so
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

However, I think the scif1 changes should be a separate patch, with
Fixes: c6c816e22bc89ea4 ("arm64: dts: ulcb-kf: enable SCIF1")
so please split it off.
Thanks!

Gr{oetje,eeting}s,

                        Geert
Wolfram Sang May 25, 2023, 8:35 a.m. UTC | #2
> However, as the only serial port that can be muxed to the GPS pins
> is SCIF1, we have no choice but to use SCIF1 for the GPS, and HSCIF1
> for CN4, like your patch does.

Ack. IMHO we should have the intended configuration upstream. Users can
pinmux from there as they want.
Wolfram Sang May 25, 2023, 8:36 a.m. UTC | #3
> However, I think the scif1 changes should be a separate patch, with
> Fixes: c6c816e22bc89ea4 ("arm64: dts: ulcb-kf: enable SCIF1")
> so please split it off.

OK, will do.
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/renesas/ulcb-kf.dtsi b/arch/arm64/boot/dts/renesas/ulcb-kf.dtsi
index ff3a9ab6e6b0..e62f5359f64b 100644
--- a/arch/arm64/boot/dts/renesas/ulcb-kf.dtsi
+++ b/arch/arm64/boot/dts/renesas/ulcb-kf.dtsi
@@ -10,6 +10,7 @@  / {
 	aliases {
 		serial1 = &hscif0;
 		serial2 = &scif1;
+		serial3 = &hscif1;
 		mmc2 = &sdhi3;
 	};
 
@@ -132,6 +133,14 @@  &hscif0 {
 	status = "okay";
 };
 
+&hscif1 {
+	pinctrl-0 = <&hscif1_pins>;
+	pinctrl-names = "default";
+	uart-has-rtscts;
+
+	status = "okay";
+};
+
 &hsusb {
 	dr_mode = "otg";
 	status = "okay";
@@ -387,8 +396,13 @@  hscif0_pins: hscif0 {
 		function = "hscif0";
 	};
 
+	hscif1_pins: hscif1 {
+		groups = "hscif1_data_a", "hscif1_ctrl_a";
+		function = "hscif1";
+	};
+
 	scif1_pins: scif1 {
-		groups = "scif1_data_b", "scif1_ctrl";
+		groups = "scif1_data_b";
 		function = "scif1";
 	};
 
@@ -418,7 +432,6 @@  &sound_clk_pins
 &scif1 {
 	pinctrl-0 = <&scif1_pins>;
 	pinctrl-names = "default";
-	uart-has-rtscts;
 
 	status = "okay";
 };