linux-kernel - Re: [PATCH v2 4/5] arm64: dts: exynos2200: add serial

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAPLW+4kOiBOZYQVA-ZiMEn+-_ZF1HD8QTm9AH0y34bcY+Mu92A@mail.gmail.com>
Date: Fri, 1 Aug 2025 18:12:47 -0500
From: Sam Protsenko <semen.protsenko@...aro.org>
To: Ivaylo Ivanov <ivo.ivanov.ivanov1@...il.com>
Cc: Krzysztof Kozlowski <krzk+dt@...nel.org>, Alim Akhtar <alim.akhtar@...sung.com>, 
	Rob Herring <robh@...nel.org>, Conor Dooley <conor+dt@...nel.org>, linux-samsung-soc@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 4/5] arm64: dts: exynos2200: add serial_0/1 nodes

On Wed, Jul 30, 2025 at 2:43 AM Ivaylo Ivanov
<ivo.ivanov.ivanov1@...il.com> wrote:
>
> Add nodes for serial_0 (UART_DBG) and serial_1 (UART_BT), which
> allows using them.
>
> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@...il.com>
> ---
>  arch/arm64/boot/dts/exynos/exynos2200.dtsi | 26 ++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/exynos/exynos2200.dtsi b/arch/arm64/boot/dts/exynos/exynos2200.dtsi
> index bab77b442..22c6da907 100644
> --- a/arch/arm64/boot/dts/exynos/exynos2200.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos2200.dtsi
> @@ -336,6 +336,19 @@ pinctrl_peric1: pinctrl@...30000 {
>                         reg = <0x10730000 0x1000>;
>                 };
>
> +               serial_1: serial@...40000 {
> +                       compatible = "samsung,exynos2200-uart", "google,gs101-uart";
> +                       reg = <0x10840000 0x100>;
> +                       clocks = <&cmu_peric1 CLK_MOUT_PERIC1_NOC_USER>,
> +                                <&cmu_peric1 CLK_DOUT_PERIC1_UART_BT>;
> +                       clock-names = "uart", "clk_uart_baud0";
> +                       interrupts = <GIC_SPI 674 IRQ_TYPE_LEVEL_HIGH 0>;
> +                       pinctrl-0 = <&uart1_bus>;
> +                       pinctrl-names = "default";
> +                       samsung,uart-fifosize = <256>;
> +                       status = "disabled";
> +               };
> +
>                 cmu_hsi0: clock-controller@...00000 {
>                         compatible = "samsung,exynos2200-cmu-hsi0";
>                         reg = <0x10a00000 0x8000>;
> @@ -458,6 +471,19 @@ pinctrl_peric2: pinctrl@...30000 {
>                         reg = <0x11c30000 0x1000>;
>                 };
>
> +               serial_0: serial@...40000 {
> +                       compatible = "samsung,exynos2200-uart", "google,gs101-uart";
> +                       reg = <0x11c40000 0x100>;
> +                       clocks = <&cmu_peric2 CLK_MOUT_PERIC2_NOC_USER>,
> +                                <&cmu_peric2 CLK_DOUT_PERIC2_UART_DBG>;
> +                       clock-names = "uart", "clk_uart_baud0";
> +                       interrupts = <GIC_SPI 687 IRQ_TYPE_LEVEL_HIGH 0>;
> +                       pinctrl-0 = <&uart0_bus_single>;
> +                       pinctrl-names = "default";
> +                       samsung,uart-fifosize = <256>;
> +                       status = "disabled";
> +               };
> +

Shouldn't these two serial nodes be children of some corresponding USI
nodes? IIUC, the downstream counterpart of this device tree is [1]? I
can see the corresponding USI node is missing there. And if you don't
have the TRM, it might get confusing. But, apart from my intuition
telling me that those UART blocks *should* be implemented as a part of
USI IP blocks in Exynos2200, there is also a fact that the downstream
driver is actually accessing USI registers in exynos_usi_init()
function, in the exynos-uart driver here: [2]. If that's correct, it
means there should exist a USI block, which should be modeled like so:

8<------------------------------------------------------------------------->8
    usi_...: usi@...400c0 {
        compatible = ...;
        reg = <0x138200c0 0x20>;
        samsung,sysreg = <...>;
        samsung,mode = <USI_MODE_UART>;
        #address-cells = <1>;
        #size-cells = <1>;
        ranges;
        clocks = <&cmu_peric2 CLK_MOUT_PERIC2_NOC_USER>,
                 <&cmu_peric2 CLK_DOUT_PERIC2_UART_DBG>;
        clock-names = "pclk", "ipclk";
        status = "disabled";

        serial_0: serial@...40000 {
            compatible = "samsung,exynos2200-uart", "google,gs101-uart";
            reg = <0x11c40000 0x100>;
            clocks = <&cmu_peric2 CLK_MOUT_PERIC2_NOC_USER>,
                     <&cmu_peric2 CLK_DOUT_PERIC2_UART_DBG>;
            clock-names = "uart", "clk_uart_baud0";
            interrupts = <GIC_SPI 687 IRQ_TYPE_LEVEL_HIGH 0>;
            pinctrl-0 = <&uart0_bus_single>;
            pinctrl-names = "default";
            samsung,uart-fifosize = <256>;
            status = "disabled";
        };
    };
8<------------------------------------------------------------------------->8

This way you can achieve the same behavior as in downstream kernel, by
making the (upstream) USI driver to initialize corresponding USI
registers for you.

Does that make any sense?

Thanks!

[1] https://github.com/jgudec/android_kernel_samsung_exynos2200/blob/CWAI/arch/arm64/boot/dts/exynos/s5e9925.dts#L4648
[2] https://github.com/jgudec/android_kernel_samsung_exynos2200/blob/CWAI/drivers/tty/serial/exynos_tty.c#L2181


>                 cmu_cmgp: clock-controller@...00000 {
>                         compatible = "samsung,exynos2200-cmu-cmgp";
>                         reg = <0x14e00000 0x8000>;
> --
> 2.43.0
>
>