From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
To: Tudor Ambarus <tudor.ambarus@linaro.org>,
robh+dt@kernel.org, conor+dt@kernel.org, alim.akhtar@samsung.com
Cc: devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org,
semen.protsenko@linaro.org, peter.griffin@linaro.org
Subject: Re: [PATCH] arm64: dts: exynos: set USI mode in board dts
Date: Fri, 6 Oct 2023 11:11:36 +0200 [thread overview]
Message-ID: <5dbbbfd7-d4ea-4aa2-b1f5-7207e2767069@linaro.org> (raw)
In-Reply-To: <20231006090258.278369-1-tudor.ambarus@linaro.org>
On 06/10/2023 11:02, Tudor Ambarus wrote:
> The Universal Serial Interface (USI) provides selectable serial protocol
> (UART, SPI, I2C). Only one function can be used at a time. The SoC
> provides flexibility for boards to choose the protocol desired. Instead
> of selecting the USI protocol mode in the SoC dtsi file, select the mode
> in the board dts file as the USI IP can work in either of the 3 modes,
> but the board uses just one. Where the USI node was not enabled in the
> board dts file, just remove the samsung,mode specified in dtsi.
>
> Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
> ---
> .../boot/dts/exynos/exynos850-e850-96.dts | 2 ++
> arch/arm64/boot/dts/exynos/exynos850.dtsi | 8 ------
> .../boot/dts/exynos/exynosautov9-sadk.dts | 2 ++
> arch/arm64/boot/dts/exynos/exynosautov9.dtsi | 25 -------------------
> 4 files changed, 4 insertions(+), 33 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/exynos/exynos850-e850-96.dts b/arch/arm64/boot/dts/exynos/exynos850-e850-96.dts
> index 6ed38912507f..615c1d6647ea 100644
> --- a/arch/arm64/boot/dts/exynos/exynos850-e850-96.dts
> +++ b/arch/arm64/boot/dts/exynos/exynos850-e850-96.dts
> @@ -15,6 +15,7 @@
> #include <dt-bindings/gpio/gpio.h>
> #include <dt-bindings/input/input.h>
> #include <dt-bindings/leds/common.h>
> +#include <dt-bindings/soc/samsung,exynos-usi.h>
>
> / {
> model = "WinLink E850-96 board";
> @@ -187,6 +188,7 @@ &serial_0 {
> };
>
> &usi_uart {
> + samsung,mode = <USI_V2_UART>;
> samsung,clkreq-on; /* needed for UART mode */
> status = "okay";
> };
> diff --git a/arch/arm64/boot/dts/exynos/exynos850.dtsi b/arch/arm64/boot/dts/exynos/exynos850.dtsi
> index aa077008b3be..db35ee742a27 100644
> --- a/arch/arm64/boot/dts/exynos/exynos850.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos850.dtsi
> @@ -12,7 +12,6 @@
>
> #include <dt-bindings/clock/exynos850.h>
> #include <dt-bindings/interrupt-controller/arm-gic.h>
> -#include <dt-bindings/soc/samsung,exynos-usi.h>
>
> / {
> /* Also known under engineering name Exynos3830 */
> @@ -574,7 +573,6 @@ usi_uart: usi@138200c0 {
> compatible = "samsung,exynos850-usi";
> reg = <0x138200c0 0x20>;
> samsung,sysreg = <&sysreg_peri 0x1010>;
> - samsung,mode = <USI_V2_UART>;
This would have sense if the node was complete - had something else than
UART. However node has only UART currently, so mode is reasonable here.
I understand that choice of mode is DTS dependent, but now it looks like
the board could actually choose it which is no true. Board DTS canno
select something different than UART.
This should be also fixed with bringing missing USI children.
It makes sense though for usi_cmgp1 and others which have more children.
> #address-cells = <1>;
> #size-cells = <1>;
> ranges;
> @@ -600,7 +598,6 @@ usi_hsi2c_0: usi@138a00c0 {
> compatible = "samsung,exynos850-usi";
> reg = <0x138a00c0 0x20>;
> samsung,sysreg = <&sysreg_peri 0x1020>;
> - samsung,mode = <USI_V2_I2C>;
> #address-cells = <1>;
> #size-cells = <1>;
> ranges;
> @@ -628,7 +625,6 @@ usi_hsi2c_1: usi@138b00c0 {
> compatible = "samsung,exynos850-usi";
> reg = <0x138b00c0 0x20>;
> samsung,sysreg = <&sysreg_peri 0x1030>;
> - samsung,mode = <USI_V2_I2C>;
> #address-cells = <1>;
> #size-cells = <1>;
> ranges;
> @@ -656,7 +652,6 @@ usi_hsi2c_2: usi@138c00c0 {
> compatible = "samsung,exynos850-usi";
> reg = <0x138c00c0 0x20>;
> samsung,sysreg = <&sysreg_peri 0x1040>;
> - samsung,mode = <USI_V2_I2C>;
> #address-cells = <1>;
> #size-cells = <1>;
> ranges;
> @@ -684,7 +679,6 @@ usi_spi_0: usi@139400c0 {
> compatible = "samsung,exynos850-usi";
> reg = <0x139400c0 0x20>;
> samsung,sysreg = <&sysreg_peri 0x1050>;
> - samsung,mode = <USI_V2_SPI>;
> #address-cells = <1>;
> #size-cells = <1>;
> ranges;
> @@ -698,7 +692,6 @@ usi_cmgp0: usi@11d000c0 {
> compatible = "samsung,exynos850-usi";
> reg = <0x11d000c0 0x20>;
> samsung,sysreg = <&sysreg_cmgp 0x2000>;
> - samsung,mode = <USI_V2_I2C>;
> #address-cells = <1>;
> #size-cells = <1>;
> ranges;
> @@ -738,7 +731,6 @@ usi_cmgp1: usi@11d200c0 {
> compatible = "samsung,exynos850-usi";
> reg = <0x11d200c0 0x20>;
> samsung,sysreg = <&sysreg_cmgp 0x2010>;
> - samsung,mode = <USI_V2_I2C>;
> #address-cells = <1>;
> #size-cells = <1>;
> ranges;
> diff --git a/arch/arm64/boot/dts/exynos/exynosautov9-sadk.dts b/arch/arm64/boot/dts/exynos/exynosautov9-sadk.dts
> index bc1815f6ada2..91d302703366 100644
> --- a/arch/arm64/boot/dts/exynos/exynosautov9-sadk.dts
> +++ b/arch/arm64/boot/dts/exynos/exynosautov9-sadk.dts
> @@ -9,6 +9,7 @@
> /dts-v1/;
> #include "exynosautov9.dtsi"
> #include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/soc/samsung,exynos-usi.h>
>
> / {
> model = "Samsung ExynosAuto v9 SADK board";
> @@ -79,6 +80,7 @@ &ufs_1 {
> };
>
> &usi_0 {
> + samsung,mode = <USI_V2_UART>;
> samsung,clkreq-on; /* needed for UART mode */
> status = "okay";
> };
> diff --git a/arch/arm64/boot/dts/exynos/exynosautov9.dtsi b/arch/arm64/boot/dts/exynos/exynosautov9.dtsi
> index b228cd7e351e..92f4b738834a 100644
> --- a/arch/arm64/boot/dts/exynos/exynosautov9.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynosautov9.dtsi
> @@ -9,7 +9,6 @@
> #include <dt-bindings/clock/samsung,exynosautov9.h>
> #include <dt-bindings/interrupt-controller/arm-gic.h>
> #include <dt-bindings/soc/samsung,boot-mode.h>
> -#include <dt-bindings/soc/samsung,exynos-usi.h>
>
> / {
> compatible = "samsung,exynosautov9";
> @@ -392,7 +391,6 @@ usi_0: usi@103000c0 {
> "samsung,exynos850-usi";
> reg = <0x103000c0 0x20>;
> samsung,sysreg = <&syscon_peric0 0x1000>;
> - samsung,mode = <USI_V2_UART>;
> #address-cells = <1>;
> #size-cells = <1>;
> ranges;
> @@ -454,7 +452,6 @@ usi_i2c_0: usi@103100c0 {
Here as well. This node has only I2C, so mode is reasonable.
Best regards,
Krzysztof
next prev parent reply other threads:[~2023-10-06 9:11 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-06 9:02 Tudor Ambarus
2023-10-06 9:11 ` Krzysztof Kozlowski [this message]
2023-10-07 1:08 ` Sam Protsenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5dbbbfd7-d4ea-4aa2-b1f5-7207e2767069@linaro.org \
--to=krzysztof.kozlowski@linaro.org \
--cc=alim.akhtar@samsung.com \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=peter.griffin@linaro.org \
--cc=robh+dt@kernel.org \
--cc=semen.protsenko@linaro.org \
--cc=tudor.ambarus@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®