| 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
| ||
|
Message-ID: <cf613367-dba6-4014-9be6-ff40de16a3a6@gmail.com>
Date: Wed, 6 Aug 2025 17:03:49 +0300
From: Ivaylo Ivanov <ivo.ivanov.ivanov1@...il.com>
To: Sam Protsenko <semen.protsenko@...aro.org>
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 8/2/25 02:12, Sam Protsenko wrote:
> 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?
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@...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?
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@...00000 {
>> compatible = "samsung,exynos2200-cmu-cmgp";
>> reg = <0x14e00000 0x8000>;
>> --
>> 2.43.0
>>
>>
Powered by blists - more mailing lists