Re: [PATCH v2 4/5] arm64: dts: exynos2200: add serial_0/1 nodes

[Ivaylo Ivanov <ivo.ivanov.ivanov1@gmail.com>]

On 8/2/25 02:12, Sam Protsenko wrote:
> On Wed, Jul 30, 2025 at 2:43 AM Ivaylo Ivanov
> <ivo.ivanov.ivanov1@gmail.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@gmail.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@10730000 {
>>                         reg = <0x10730000 0x1000>;
>>                 };
>>
>> +               serial_1: serial@10840000 {
>> +                       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@10a00000 {
>>                         compatible = "samsung,exynos2200-cmu-hsi0";
>>                         reg = <0x10a00000 0x8000>;
>> @@ -458,6 +471,19 @@ pinctrl_peric2: pinctrl@11c30000 {
>>                         reg = <0x11c30000 0x1000>;
>>                 };
>>
>> +               serial_0: serial@11c40000 {
>> +                       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?

Which :P

>  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].

Unless that does nothing and Samsung did not care enough to strip the
code for it.

>  If that's correct, it
> means there should exist a USI block, which should be modeled like so:
>
> 8<------------------------------------------------------------------------->8
>     usi_...: usi@11c400c0 {
>         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@11c40000 {
>             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?

Not really. Perhaps you could __verify__ to what USI instance it belongs?
And is there a sysreg register for controlling the behavior of that?

Again, I know nothing more than what downstream says.

Best regards,
Ivaylo

>
> 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@14e00000 {
>>                         compatible = "samsung,exynos2200-cmu-cmgp";
>>                         reg = <0x14e00000 0x8000>;
>> --
>> 2.43.0
>>
>>