Hi Wolfram,
On Wed, May 24, 2023 at 10:37 PM Wolfram Sang
<wsa+renesas@xxxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxx>
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@xxxxxxxxx>
> 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@xxxxxxxxx>
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
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
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
