Some USI blocks can be configured as SPI controllers. Add corresponding
SPI nodes to Exynos850 SoC device tree.
Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
---
Changes in v2:
- Sorted pinctrl properties properly
arch/arm64/boot/dts/exynos/exynos850.dtsi | 54 +++++++++++++++++++++++
1 file changed, 54 insertions(+)
diff --git a/arch/arm64/boot/dts/exynos/exynos850.dtsi b/arch/arm64/boot/dts/exynos/exynos850.dtsi
index 618bc674896e..ca257da74b50 100644
--- a/arch/arm64/boot/dts/exynos/exynos850.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos850.dtsi
@@ -738,6 +738,24 @@ usi_spi_0: usi@139400c0 {
<&cmu_peri CLK_GOUT_SPI0_IPCLK>;
clock-names = "pclk", "ipclk";
status = "disabled";
+
+ spi_0: spi@13940000 {
+ compatible = "samsung,exynos850-spi";
+ reg = <0x13940000 0x30>;
+ interrupts = <GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>;
+ pinctrl-0 = <&spi0_pins>;
+ pinctrl-names = "default";
+ clocks = <&cmu_peri CLK_GOUT_SPI0_IPCLK>,
+ <&cmu_peri CLK_GOUT_SPI0_PCLK>;
+ clock-names = "spi_busclk0", "spi";
+ samsung,spi-src-clk = <0>;
+ dmas = <&pdma0 5>, <&pdma0 4>;
+ dma-names = "tx", "rx";
+ num-cs = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ };
};
usi_cmgp0: usi@11d000c0 {
@@ -779,6 +797,24 @@ serial_1: serial@11d00000 {
clock-names = "uart", "clk_uart_baud0";
status = "disabled";
};
+
+ spi_1: spi@11d00000 {
+ compatible = "samsung,exynos850-spi";
+ reg = <0x11d00000 0x30>;
+ interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>;
+ pinctrl-0 = <&spi1_pins>;
+ pinctrl-names = "default";
+ clocks = <&cmu_cmgp CLK_GOUT_CMGP_USI0_IPCLK>,
+ <&cmu_cmgp CLK_GOUT_CMGP_USI0_PCLK>;
+ clock-names = "spi_busclk0", "spi";
+ samsung,spi-src-clk = <0>;
+ dmas = <&pdma0 12>, <&pdma0 13>;
+ dma-names = "tx", "rx";
+ num-cs = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ };
};
usi_cmgp1: usi@11d200c0 {
@@ -820,6 +856,24 @@ serial_2: serial@11d20000 {
clock-names = "uart", "clk_uart_baud0";
status = "disabled";
};
+
+ spi_2: spi@11d20000 {
+ compatible = "samsung,exynos850-spi";
+ reg = <0x11d20000 0x30>;
+ interrupts = <GIC_SPI 63 IRQ_TYPE_LEVEL_HIGH>;
+ pinctrl-0 = <&spi2_pins>;
+ pinctrl-names = "default";
+ clocks = <&cmu_cmgp CLK_GOUT_CMGP_USI1_IPCLK>,
+ <&cmu_cmgp CLK_GOUT_CMGP_USI1_PCLK>;
+ clock-names = "spi_busclk0", "spi";
+ samsung,spi-src-clk = <0>;
+ dmas = <&pdma0 14>, <&pdma0 15>;
+ dma-names = "tx", "rx";
+ num-cs = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "disabled";
+ };
};
};
};
--
2.39.2On 25/01/2024 02:38, Sam Protsenko wrote:
> Some USI blocks can be configured as SPI controllers. Add corresponding
> SPI nodes to Exynos850 SoC device tree.
>
> Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> ---
> Changes in v2:
> - Sorted pinctrl properties properly
>
> arch/arm64/boot/dts/exynos/exynos850.dtsi | 54 +++++++++++++++++++++++
> 1 file changed, 54 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/exynos/exynos850.dtsi b/arch/arm64/boot/dts/exynos/exynos850.dtsi
> index 618bc674896e..ca257da74b50 100644
> --- a/arch/arm64/boot/dts/exynos/exynos850.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos850.dtsi
> @@ -738,6 +738,24 @@ usi_spi_0: usi@139400c0 {
> <&cmu_peri CLK_GOUT_SPI0_IPCLK>;
> clock-names = "pclk", "ipclk";
> status = "disabled";
> +
> + spi_0: spi@13940000 {
> + compatible = "samsung,exynos850-spi";
> + reg = <0x13940000 0x30>;
> + interrupts = <GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>;
> + pinctrl-0 = <&spi0_pins>;
> + pinctrl-names = "default";
> + clocks = <&cmu_peri CLK_GOUT_SPI0_IPCLK>,
> + <&cmu_peri CLK_GOUT_SPI0_PCLK>;
> + clock-names = "spi_busclk0", "spi";
> + samsung,spi-src-clk = <0>;
> + dmas = <&pdma0 5>, <&pdma0 4>;
> + dma-names = "tx", "rx";
> + num-cs = <1>;
For the future: please keep properties sorted by name, so clocks+name,
dmas+name, interrupts, pinctrl+name, more-or-less matching DTS coding
style. address/size cells can go to the end.
Best regards,
Krzysztof
On Thu, Feb 1, 2024 at 4:31 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 25/01/2024 02:38, Sam Protsenko wrote:
> > Some USI blocks can be configured as SPI controllers. Add corresponding
> > SPI nodes to Exynos850 SoC device tree.
> >
> > Signed-off-by: Sam Protsenko <semen.protsenko@linaro.org>
> > ---
> > Changes in v2:
> > - Sorted pinctrl properties properly
> >
> > arch/arm64/boot/dts/exynos/exynos850.dtsi | 54 +++++++++++++++++++++++
> > 1 file changed, 54 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/exynos/exynos850.dtsi b/arch/arm64/boot/dts/exynos/exynos850.dtsi
> > index 618bc674896e..ca257da74b50 100644
> > --- a/arch/arm64/boot/dts/exynos/exynos850.dtsi
> > +++ b/arch/arm64/boot/dts/exynos/exynos850.dtsi
> > @@ -738,6 +738,24 @@ usi_spi_0: usi@139400c0 {
> > <&cmu_peri CLK_GOUT_SPI0_IPCLK>;
> > clock-names = "pclk", "ipclk";
> > status = "disabled";
> > +
> > + spi_0: spi@13940000 {
> > + compatible = "samsung,exynos850-spi";
> > + reg = <0x13940000 0x30>;
> > + interrupts = <GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>;
> > + pinctrl-0 = <&spi0_pins>;
> > + pinctrl-names = "default";
> > + clocks = <&cmu_peri CLK_GOUT_SPI0_IPCLK>,
> > + <&cmu_peri CLK_GOUT_SPI0_PCLK>;
> > + clock-names = "spi_busclk0", "spi";
> > + samsung,spi-src-clk = <0>;
> > + dmas = <&pdma0 5>, <&pdma0 4>;
> > + dma-names = "tx", "rx";
> > + num-cs = <1>;
>
> For the future: please keep properties sorted by name, so clocks+name,
> dmas+name, interrupts, pinctrl+name, more-or-less matching DTS coding
> style. address/size cells can go to the end.
>
Noted, thanks! So IIUC, basically follow the order of properties
described at [1], but keep the standard/common properties block
sorted, and then keep vendor properties sorted, right?
[1] Documentation/devicetree/bindings/dts-coding-style.rst
> Best regards,
> Krzysztof
>
On 1/25/24 01:38, Sam Protsenko wrote:
> diff --git a/arch/arm64/boot/dts/exynos/exynos850.dtsi b/arch/arm64/boot/dts/exynos/exynos850.dtsi
> index 618bc674896e..ca257da74b50 100644
> --- a/arch/arm64/boot/dts/exynos/exynos850.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynos850.dtsi
> @@ -738,6 +738,24 @@ usi_spi_0: usi@139400c0 {
> <&cmu_peri CLK_GOUT_SPI0_IPCLK>;
> clock-names = "pclk", "ipclk";
> status = "disabled";
> +
> + spi_0: spi@13940000 {
> + compatible = "samsung,exynos850-spi";
> + reg = <0x13940000 0x30>;
> + interrupts = <GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>;
> + pinctrl-0 = <&spi0_pins>;
> + pinctrl-names = "default";
> + clocks = <&cmu_peri CLK_GOUT_SPI0_IPCLK>,
> + <&cmu_peri CLK_GOUT_SPI0_PCLK>;
> + clock-names = "spi_busclk0", "spi";
> + samsung,spi-src-clk = <0>;
this optional property
> + dmas = <&pdma0 5>, <&pdma0 4>;
> + dma-names = "tx", "rx";
> + num-cs = <1>;
and this one, are already defaults in the driver. Shall you remove them?
> + #address-cells = <1>;
> + #size-cells = <0>;
> + status = "disabled";
> + };
> };
On Mon, Jan 29, 2024 at 11:51 AM Tudor Ambarus <tudor.ambarus@linaro.org> wrote:
>
>
>
> On 1/25/24 01:38, Sam Protsenko wrote:
> > diff --git a/arch/arm64/boot/dts/exynos/exynos850.dtsi b/arch/arm64/boot/dts/exynos/exynos850.dtsi
> > index 618bc674896e..ca257da74b50 100644
> > --- a/arch/arm64/boot/dts/exynos/exynos850.dtsi
> > +++ b/arch/arm64/boot/dts/exynos/exynos850.dtsi
> > @@ -738,6 +738,24 @@ usi_spi_0: usi@139400c0 {
> > <&cmu_peri CLK_GOUT_SPI0_IPCLK>;
> > clock-names = "pclk", "ipclk";
> > status = "disabled";
> > +
> > + spi_0: spi@13940000 {
> > + compatible = "samsung,exynos850-spi";
> > + reg = <0x13940000 0x30>;
> > + interrupts = <GIC_SPI 221 IRQ_TYPE_LEVEL_HIGH>;
> > + pinctrl-0 = <&spi0_pins>;
> > + pinctrl-names = "default";
> > + clocks = <&cmu_peri CLK_GOUT_SPI0_IPCLK>,
> > + <&cmu_peri CLK_GOUT_SPI0_PCLK>;
> > + clock-names = "spi_busclk0", "spi";
> > + samsung,spi-src-clk = <0>;
>
> this optional property
>
The reason this property is provided here despite being optional, is
to avoid corresponding dev_warn() message from spi-s3c64xx.c driver:
if (of_property_read_u32(dev->of_node, "samsung,spi-src-clk", &temp)) {
dev_warn(dev, "spi bus clock parent not specified, using
clock at index 0 as parent\n");
The same usage (samsung,spi-src-clk = <0>) can be encountered in
multiple other Exynos dts in arch/arm/ and arch/arm64/, and it's also
used in bindings example. Probably for the same reason explained
above. Even if dev_warn() is removed in the driver, I guess the older
kernels will still print it if spi-src-clk is omitted. So I'd like to
keep it here.
> > + dmas = <&pdma0 5>, <&pdma0 4>;
> > + dma-names = "tx", "rx";
> > + num-cs = <1>;
>
> and this one, are already defaults in the driver. Shall you remove them?
>
For exactly the same reasoning as stated above, I'd like to keep this
here to keep dmesg clean and tidy. Otherwise it prints this warning:
if (of_property_read_u32(dev->of_node, "num-cs", &temp)) {
dev_warn(dev, "number of chip select lines not specified,
assuming 1 chip select line\n");
And even if the warning is removed in the driver, older kernels will
still print it.
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + status = "disabled";
> > + };
> > };
On 1/29/24 19:39, Sam Protsenko wrote:
>>> + samsung,spi-src-clk = <0>;
>> this optional property
>>
> The reason this property is provided here despite being optional, is
> to avoid corresponding dev_warn() message from spi-s3c64xx.c driver:
>
> if (of_property_read_u32(dev->of_node, "samsung,spi-src-clk", &temp)) {
> dev_warn(dev, "spi bus clock parent not specified, using
> clock at index 0 as parent\n");
>
> The same usage (samsung,spi-src-clk = <0>) can be encountered in
> multiple other Exynos dts in arch/arm/ and arch/arm64/, and it's also
> used in bindings example. Probably for the same reason explained
> above. Even if dev_warn() is removed in the driver, I guess the older
> kernels will still print it if spi-src-clk is omitted. So I'd like to
> keep it here.
Yeah, I know. I proposed a patch switching to dev_dbg. If it's so
annoying and implies adding superfluous properties to DT, maybe it is
worth to add a fixes tag to the dev_dbg patch and backport it to stable
kernels?
Your patch looks fine. I guess the vendor specific properties shall be
last if you keep them, see:
https://www.kernel.org/doc/html/latest/devicetree/bindings/dts-coding-style.html#order-of-properties-in-device-node
If you remove the vendor properties or reorder them, one can add:
Reviewed-by: Tudor Ambarus <tudor.ambarus@linaro.org>
On Wed, 24 Jan 2024 19:38:58 -0600, Sam Protsenko wrote:
> Some USI blocks can be configured as SPI controllers. Add corresponding
> SPI nodes to Exynos850 SoC device tree.
>
>
Applied, thanks!
[3/3] arm64: dts: exynos: Add SPI nodes for Exynos850
https://git.kernel.org/krzk/linux/c/98473b0d78caa5502b7eee05553ee168f0b0b424
Best regards,
--
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
On 01/02/2024 11:36, Krzysztof Kozlowski wrote: > > On Wed, 24 Jan 2024 19:38:58 -0600, Sam Protsenko wrote: >> Some USI blocks can be configured as SPI controllers. Add corresponding >> SPI nodes to Exynos850 SoC device tree. >> >> > > Applied, thanks! > > [3/3] arm64: dts: exynos: Add SPI nodes for Exynos850 > https://git.kernel.org/krzk/linux/c/98473b0d78caa5502b7eee05553ee168f0b0b424 And dropped. You did not test it. For some time, all Samsung SoCs and its variants are expected not to introduce any new `dtbs_check W=1` warnings. Several platforms, like all ARM64 Samsung SoCs, have already zero warnings, thus for such platforms it is extra easy for the submitter to validate DTS before posting a patch. The patch briefly looks like it is not conforming to this rule. Please confirm that you tested your patch and it does not introduce any new warnings (linux-next is decisive here). Best regards, Krzysztof
On Thu, Feb 1, 2024 at 5:56 AM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 01/02/2024 11:36, Krzysztof Kozlowski wrote: > > > > On Wed, 24 Jan 2024 19:38:58 -0600, Sam Protsenko wrote: > >> Some USI blocks can be configured as SPI controllers. Add corresponding > >> SPI nodes to Exynos850 SoC device tree. > >> > >> > > > > Applied, thanks! > > > > [3/3] arm64: dts: exynos: Add SPI nodes for Exynos850 > > https://git.kernel.org/krzk/linux/c/98473b0d78caa5502b7eee05553ee168f0b0b424 > > And dropped. You did not test it. > Right. Forgot to re-test it after re-shuffling the clocks. Sorry for the hustle, I'll send v3 shortly. > For some time, all Samsung SoCs and its variants are expected not to > introduce any new `dtbs_check W=1` warnings. Several platforms, like all > ARM64 Samsung SoCs, have already zero warnings, thus for such platforms > it is extra easy for the submitter to validate DTS before posting a > patch. The patch briefly looks like it is not conforming to this rule. > Please confirm that you tested your patch and it does not introduce any > new warnings (linux-next is decisive here). > > Best regards, > Krzysztof >
© 2016 - 2025 Red Hat, Inc.