On 29/06/2022 11:47, Chanho Park wrote:
>> On 29/06/2022 03:56, Chanho Park wrote:
>>> Universal Serial Interface (USI) supports three types of serial
>>> interface such as Universal Asynchronous Receiver and Transmitter
>>> (UART), Serial Peripheral Interface (SPI), and Inter-Integrated Circuit
>> (I2C).
>>> Each protocols can be working independently and configured as one of
>>> those using external configuration inputs.
>>> Exynos Auto v9 SoC support 12 USIs. When a USI uses two pins such as
>>> i2c and 3 wire uarts(RX/TX only), we can use remain two pins as i2c mode.
>>> So, we can define one USI node that includes serial/spi and hsi2c.
>>> usi_i2c nodes can be used only for i2c mode.
>>>
>>> We can have below combinations for one USI.
>>> 1) The usi node is used either 4 pin uart or 4 pin spi -> No usi_i2c
>>> can be used
>>> 2) The usi node is used 2 pin uart(RX/TX) and i2c(SDA/SCL) -> usi_i2c
>>> should be enabled to use the latter i2c
>>> 3) The usi node is used i2c(SDA/SCL) and i2c(SDA/SCL) -> usi_i2c
>>> should be enabled to use the latter i2c
>>>
>>> By default, all USIs are initially set to uart mode by below setting.
>>> samsung,mode = <USI_V2_UART>;
>>> You can change it either USI_V2_SPI or USI_V2_I2C.
>>>
>>> Signed-off-by: Chanho Park <chanho61.park@xxxxxxxxxxx>
>>> ---
>>> .../boot/dts/exynos/exynosautov9-usi.dtsi | 1127 +++++++++++++++++
>>> 1 file changed, 1127 insertions(+)
>>> create mode 100644 arch/arm64/boot/dts/exynos/exynosautov9-usi.dtsi
>>
>> Put all this directly in exynosautov9.dtsi, because this is not a re-
>> usable piece among different DTSI.
>
> Okay. I'll move them in the exynosautov9.dtsi. I thought they're too long to put in the exynosautov9.dtsi and I also found a similar case such as exynos5433-bus.dtsi and exynos5433-tmu.dtsi.
Indeed we did like that... The tmu maybe was meant to be re-used,
although it references specific clusters. But the split of bus I don't
understand - it does not help.
I don't think it improved readability.
>
>>
>>>
>>> diff --git a/arch/arm64/boot/dts/exynos/exynosautov9-usi.dtsi
>>> b/arch/arm64/boot/dts/exynos/exynosautov9-usi.dtsi
>>> new file mode 100644
>>> index 000000000000..0e4c6332770b
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/exynos/exynosautov9-usi.dtsi
>>> @@ -0,0 +1,1127 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Samsung's ExynosAutov9 SoC USI device tree source
>>> + *
>>> + * Copyright (c) 2022 Samsung Electronics Co., Ltd.
>>> + *
>>> + * Samsung's ExynosAutov9 SoC USI(Universal Serial Interface,
>>> +uart/spi/i2c)
>>> + * are listed as device tree nodes in this file.
>>> + */
>>> +
>>> +/* PERIC0 USIs */
>>> +&soc {
>>> + syscon_peric0: syscon@10220000 {
>>> + compatible = "samsung,exynosautov9-sysreg", "syscon";
>>> + reg = <0x10220000 0x2000>;
>>> + };
>>> +
>>> + usi_0: usi@103000c0 {
>>> + compatible = "samsung,exynos850-usi";
>>
>> We should start adding dedicated compatible, so:
>> "samsung,exynosautov9-usi", "samsung,exynos850-usi"
>
> So, I need to add the compatible to the Documentation/devicetree/bindings/soc/samsung/exynos-usi.yaml, right?
>
> - samsung,exynos850-usi # for USIv2 (Exynos850, ExynosAutoV9)
>
> To be>
> - samsung,exynos850-usi
> - samsung,exynosautov9-usi
compatible:
oneOf:
- items:
- const: samsung,exynosautov9-usi
- const: samsung,exynos850-usi
- enum:
- samsung,exynos850-usi # for USIv2 (Exynos850, ExynosAutoV9)
>
>>
>>> + reg = <0x103000c0 0x20>;
>>> + samsung,sysreg = <&syscon_peric0 0x1000>;
>>> + samsung,mode = <USI_V2_UART>;
>>> + #address-cells = <1>;
>>> + #size-cells = <1>;
>>> + ranges;
>>> + clocks = <&cmu_peric0 CLK_GOUT_PERIC0_PCLK_0>,
>>> + <&cmu_peric0 CLK_GOUT_PERIC0_IPCLK_0>;
>>> + clock-names = "pclk", "ipclk";
>>> + status = "disabled";
>>> +
>>> + /* USI: UART */
>>
>> Skip the comments, they are obvious from device node name. Long time ago I
>> was not advocating this, but I see it's benefits - much easier to
>> introduce changes to DTS or binding in case of some differences.
Eh, I think my reply got mixed up. The last sentence was about
compatible, so it should be:
We should start adding dedicated compatible, so:
"samsung,exynosautov9-usi", "samsung,exynos850-usi".
Long time ago I was not advocating this, but I see it's benefits - much
easier to introduce changes to DTS or binding in case of some differences.
and here only about the comment.
> I'll drop them.
Yes, please.
>
>>
>>> + serial_0: serial@10300000 {
>>> + compatible = "samsung,exynos850-uart";
>>
>> Here as well.
>
> I'll add "samsung,exynosautov9-uart" to the yaml file.
>
>>
Best regards,
Krzysztof
