diff mbox series

[07/17] MIPS: loongson32: Convert UART platform device to DT

Message ID 20230729134318.1694467-8-keguang.zhang@gmail.com (mailing list archive)
State New
Headers show
Series MIPS: loongson32: Convert all platform devices to DT | expand

Commit Message

Keguang Zhang July 29, 2023, 1:43 p.m. UTC
Add UART device nodes for Loongson-1 boards,
and drop the legacy platform devices and data accordingly.

Signed-off-by: Keguang Zhang <keguang.zhang@gmail.com>
---
 arch/mips/boot/dts/loongson/loongson1.dtsi    | 54 +++++++++++++++++++
 arch/mips/boot/dts/loongson/loongson1b.dtsi   | 12 +++++
 arch/mips/boot/dts/loongson/loongson1c.dtsi   | 12 +++++
 arch/mips/boot/dts/loongson/lsgz_1b_dev.dts   | 20 +++++++
 arch/mips/boot/dts/loongson/smartloong_1c.dts | 20 +++++++
 arch/mips/loongson32/common/platform.c        | 44 ---------------
 arch/mips/loongson32/ls1b/board.c             |  3 --
 arch/mips/loongson32/ls1c/board.c             |  3 --
 8 files changed, 118 insertions(+), 50 deletions(-)

Comments

Krzysztof Kozlowski July 30, 2023, 8:26 a.m. UTC | #1
On 29/07/2023 15:43, Keguang Zhang wrote:
> Add UART device nodes for Loongson-1 boards,
> and drop the legacy platform devices and data accordingly.
> 
> Signed-off-by: Keguang Zhang <keguang.zhang@gmail.com>
> ---
>  arch/mips/boot/dts/loongson/loongson1.dtsi    | 54 +++++++++++++++++++

Same problem - DTS is always separate. It seems you made this mistake
everywhere, so entire patchset needs to be fixed. Keep all DTS - your
base board and extending it - at the end of the patchset and squash it.
There is little point to add new DTS in steps (e.g. first add incomplete
broken DTS and then immediately fix it... no, instead just add correct
and complete DTS).


Best regards,
Krzysztof
Keguang Zhang July 31, 2023, 3:04 a.m. UTC | #2
On Sun, Jul 30, 2023 at 4:26 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 29/07/2023 15:43, Keguang Zhang wrote:
> > Add UART device nodes for Loongson-1 boards,
> > and drop the legacy platform devices and data accordingly.
> >
> > Signed-off-by: Keguang Zhang <keguang.zhang@gmail.com>
> > ---
> >  arch/mips/boot/dts/loongson/loongson1.dtsi    | 54 +++++++++++++++++++
>
> Same problem - DTS is always separate. It seems you made this mistake
> everywhere, so entire patchset needs to be fixed. Keep all DTS - your
> base board and extending it - at the end of the patchset and squash it.
> There is little point to add new DTS in steps (e.g. first add incomplete
> broken DTS and then immediately fix it... no, instead just add correct
> and complete DTS).
>
Sorry. I thought it would be easier to review for split patches.
Thanks for the explanation.
Will send v2 with one complete DTS.
>
> Best regards,
> Krzysztof
>
Keguang Zhang July 31, 2023, 3:32 a.m. UTC | #3
On Mon, Jul 31, 2023 at 11:04 AM Keguang Zhang <keguang.zhang@gmail.com> wrote:
>
> On Sun, Jul 30, 2023 at 4:26 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
> >
> > On 29/07/2023 15:43, Keguang Zhang wrote:
> > > Add UART device nodes for Loongson-1 boards,
> > > and drop the legacy platform devices and data accordingly.
> > >
> > > Signed-off-by: Keguang Zhang <keguang.zhang@gmail.com>
> > > ---
> > >  arch/mips/boot/dts/loongson/loongson1.dtsi    | 54 +++++++++++++++++++
> >
> > Same problem - DTS is always separate. It seems you made this mistake
> > everywhere, so entire patchset needs to be fixed. Keep all DTS - your
> > base board and extending it - at the end of the patchset and squash it.
> > There is little point to add new DTS in steps (e.g. first add incomplete
> > broken DTS and then immediately fix it... no, instead just add correct
> > and complete DTS).
> >
> Sorry. I thought it would be easier to review for split patches.
> Thanks for the explanation.
> Will send v2 with one complete DTS.

Hello Thomas,
May I ask your opinion about the way to delete the obsolete platform devices?
Should I delete them in one patch? Or in separated patches?
Thanks!

> >
> > Best regards,
> > Krzysztof
> >
>
>
> --
> Best regards,
>
> Keguang Zhang
Krzysztof Kozlowski July 31, 2023, 6:56 a.m. UTC | #4
On 31/07/2023 05:32, Keguang Zhang wrote:
> On Mon, Jul 31, 2023 at 11:04 AM Keguang Zhang <keguang.zhang@gmail.com> wrote:
>>
>> On Sun, Jul 30, 2023 at 4:26 PM Krzysztof Kozlowski
>> <krzysztof.kozlowski@linaro.org> wrote:
>>>
>>> On 29/07/2023 15:43, Keguang Zhang wrote:
>>>> Add UART device nodes for Loongson-1 boards,
>>>> and drop the legacy platform devices and data accordingly.
>>>>
>>>> Signed-off-by: Keguang Zhang <keguang.zhang@gmail.com>
>>>> ---
>>>>  arch/mips/boot/dts/loongson/loongson1.dtsi    | 54 +++++++++++++++++++
>>>
>>> Same problem - DTS is always separate. It seems you made this mistake
>>> everywhere, so entire patchset needs to be fixed. Keep all DTS - your
>>> base board and extending it - at the end of the patchset and squash it.
>>> There is little point to add new DTS in steps (e.g. first add incomplete
>>> broken DTS and then immediately fix it... no, instead just add correct
>>> and complete DTS).
>>>
>> Sorry. I thought it would be easier to review for split patches.
>> Thanks for the explanation.
>> Will send v2 with one complete DTS.
> 
> Hello Thomas,
> May I ask your opinion about the way to delete the obsolete platform devices?
> Should I delete them in one patch? Or in separated patches?

subsystem patches are split per subsystem. arch-code can be either
together or also split per type of driver. Removal of code is
incremental. Just add extending it, but your patch was not extending
DTS, but adding new one.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/arch/mips/boot/dts/loongson/loongson1.dtsi b/arch/mips/boot/dts/loongson/loongson1.dtsi
index 711c88fd2781..c77aa2d0f66c 100644
--- a/arch/mips/boot/dts/loongson/loongson1.dtsi
+++ b/arch/mips/boot/dts/loongson/loongson1.dtsi
@@ -72,4 +72,58 @@  intc3: interrupt-controller@1fd01088 {
 			interrupts = <5>;
 		};
 	};
+
+	apb: bus@1fe40000 {
+		compatible = "simple-bus";
+		reg = <0x1fe40000 0x40000>;
+		ranges;
+
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		uart0: serial@1fe40000 {
+			compatible = "ns16550a";
+			reg = <0x1fe40000 0x8>;
+
+			interrupt-parent = <&intc0>;
+			interrupts = <2 IRQ_TYPE_LEVEL_HIGH>;
+
+			clocks = <&clkc LS1X_CLKID_APB>;
+
+			status = "disabled";
+		};
+
+		uart1: serial@1fe44000 {
+			compatible = "ns16550a";
+			reg = <0x1fe44000 0x8>;
+
+			interrupt-parent = <&intc0>;
+
+			clocks = <&clkc LS1X_CLKID_APB>;
+
+			status = "disabled";
+		};
+
+		uart2: serial@1fe48000 {
+			compatible = "ns16550a";
+			reg = <0x1fe48000 0x8>;
+
+			interrupt-parent = <&intc0>;
+
+			clocks = <&clkc LS1X_CLKID_APB>;
+
+			status = "disabled";
+		};
+
+		uart3: serial@1fe4c000 {
+			compatible = "ns16550a";
+			reg = <0x1fe4c000 0x8>;
+
+			interrupt-parent = <&intc0>;
+
+			clocks = <&clkc LS1X_CLKID_APB>;
+
+			status = "disabled";
+		};
+	};
 };
diff --git a/arch/mips/boot/dts/loongson/loongson1b.dtsi b/arch/mips/boot/dts/loongson/loongson1b.dtsi
index 784ae9b6572d..437a77cee163 100644
--- a/arch/mips/boot/dts/loongson/loongson1b.dtsi
+++ b/arch/mips/boot/dts/loongson/loongson1b.dtsi
@@ -73,3 +73,15 @@  clkc: clock-controller@1fe78030 {
 		#clock-cells = <1>;
 	};
 };
+
+&uart1 {
+	interrupts = <3 IRQ_TYPE_LEVEL_HIGH>;
+};
+
+&uart2 {
+	interrupts = <4 IRQ_TYPE_LEVEL_HIGH>;
+};
+
+&uart3 {
+	interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
+};
diff --git a/arch/mips/boot/dts/loongson/loongson1c.dtsi b/arch/mips/boot/dts/loongson/loongson1c.dtsi
index 8570c4c72677..1dd575b7b2f9 100644
--- a/arch/mips/boot/dts/loongson/loongson1c.dtsi
+++ b/arch/mips/boot/dts/loongson/loongson1c.dtsi
@@ -40,3 +40,15 @@  intc4: interrupt-controller@1fd010a0 {
 		interrupts = <6>;
 	};
 };
+
+&uart1 {
+	interrupts = <4 IRQ_TYPE_LEVEL_HIGH>;
+};
+
+&uart2 {
+	interrupts = <5 IRQ_TYPE_LEVEL_HIGH>;
+};
+
+&uart3 {
+	interrupts = <29 IRQ_TYPE_LEVEL_HIGH>;
+};
diff --git a/arch/mips/boot/dts/loongson/lsgz_1b_dev.dts b/arch/mips/boot/dts/loongson/lsgz_1b_dev.dts
index d12c723b0a2b..89c3dfa574f7 100644
--- a/arch/mips/boot/dts/loongson/lsgz_1b_dev.dts
+++ b/arch/mips/boot/dts/loongson/lsgz_1b_dev.dts
@@ -11,6 +11,10 @@  / {
 	compatible = "loongson,lsgz-1b-dev", "loongson,ls1b";
 	model = "LSGZ_1B_DEV Board";
 
+	chosen {
+		bootargs = "console=ttyS2,115200";
+	};
+
 	memory@0 {
 		device_type = "memory";
 		reg = <0x0 0x4000000>;
@@ -23,3 +27,19 @@  xtal: xtal {
 		#clock-cells = <0>;
 	};
 };
+
+&uart0 {
+	status = "okay";
+};
+
+&uart1 {
+	status = "okay";
+};
+
+&uart2 {
+	status = "okay";
+};
+
+&uart3 {
+	status = "okay";
+};
diff --git a/arch/mips/boot/dts/loongson/smartloong_1c.dts b/arch/mips/boot/dts/loongson/smartloong_1c.dts
index 64e869acfd86..188aab9e3685 100644
--- a/arch/mips/boot/dts/loongson/smartloong_1c.dts
+++ b/arch/mips/boot/dts/loongson/smartloong_1c.dts
@@ -11,6 +11,10 @@  / {
 	compatible = "loongmasses,smartloong-1c", "loongson,ls1c";
 	model = "Smartloong_1C Board";
 
+	chosen {
+		bootargs = "console=ttyS2,115200";
+	};
+
 	memory@0 {
 		device_type = "memory";
 		reg = <0x0 0x2000000>;
@@ -23,3 +27,19 @@  xtal: xtal {
 		#clock-cells = <0>;
 	};
 };
+
+&uart0 {
+	status = "okay";
+};
+
+&uart1 {
+	status = "okay";
+};
+
+&uart2 {
+	status = "okay";
+};
+
+&uart3 {
+	status = "okay";
+};
diff --git a/arch/mips/loongson32/common/platform.c b/arch/mips/loongson32/common/platform.c
index 8075590a9f83..8272b4133e25 100644
--- a/arch/mips/loongson32/common/platform.c
+++ b/arch/mips/loongson32/common/platform.c
@@ -9,7 +9,6 @@ 
 #include <linux/mtd/partitions.h>
 #include <linux/sizes.h>
 #include <linux/phy.h>
-#include <linux/serial_8250.h>
 #include <linux/stmmac.h>
 #include <linux/usb/ehci_pdriver.h>
 
@@ -18,49 +17,6 @@ 
 #include <dma.h>
 #include <nand.h>
 
-/* 8250/16550 compatible UART */
-#define LS1X_UART(_id)						\
-	{							\
-		.mapbase	= LS1X_UART ## _id ## _BASE,	\
-		.irq		= LS1X_UART ## _id ## _IRQ,	\
-		.iotype		= UPIO_MEM,			\
-		.flags		= UPF_IOREMAP | UPF_FIXED_TYPE, \
-		.type		= PORT_16550A,			\
-	}
-
-static struct plat_serial8250_port ls1x_serial8250_pdata[] = {
-	LS1X_UART(0),
-	LS1X_UART(1),
-	LS1X_UART(2),
-	LS1X_UART(3),
-	{},
-};
-
-struct platform_device ls1x_uart_pdev = {
-	.name		= "serial8250",
-	.id		= PLAT8250_DEV_PLATFORM,
-	.dev		= {
-		.platform_data = ls1x_serial8250_pdata,
-	},
-};
-
-void __init ls1x_serial_set_uartclk(struct platform_device *pdev)
-{
-	struct clk *clk;
-	struct plat_serial8250_port *p;
-
-	clk = clk_get(&pdev->dev, pdev->name);
-	if (IS_ERR(clk)) {
-		pr_err("unable to get %s clock, err=%ld",
-		       pdev->name, PTR_ERR(clk));
-		return;
-	}
-	clk_prepare_enable(clk);
-
-	for (p = pdev->dev.platform_data; p->flags != 0; ++p)
-		p->uartclk = clk_get_rate(clk);
-}
-
 /* Synopsys Ethernet GMAC */
 static struct stmmac_mdio_bus_data ls1x_mdio_bus_data = {
 	.phy_mask	= 0,
diff --git a/arch/mips/loongson32/ls1b/board.c b/arch/mips/loongson32/ls1b/board.c
index fed8d432ef20..e8290f200096 100644
--- a/arch/mips/loongson32/ls1b/board.c
+++ b/arch/mips/loongson32/ls1b/board.c
@@ -34,7 +34,6 @@  static const struct gpio_led_platform_data ls1x_led_pdata __initconst = {
 };
 
 static struct platform_device *ls1b_platform_devices[] __initdata = {
-	&ls1x_uart_pdev,
 	&ls1x_eth0_pdev,
 	&ls1x_eth1_pdev,
 	&ls1x_ehci_pdev,
@@ -46,8 +45,6 @@  static struct platform_device *ls1b_platform_devices[] __initdata = {
 
 static int __init ls1b_platform_init(void)
 {
-	ls1x_serial_set_uartclk(&ls1x_uart_pdev);
-
 	gpio_led_register_device(-1, &ls1x_led_pdata);
 
 	return platform_add_devices(ls1b_platform_devices,
diff --git a/arch/mips/loongson32/ls1c/board.c b/arch/mips/loongson32/ls1c/board.c
index 9dcfe9de55b0..a7096964fb30 100644
--- a/arch/mips/loongson32/ls1c/board.c
+++ b/arch/mips/loongson32/ls1c/board.c
@@ -6,7 +6,6 @@ 
 #include <platform.h>
 
 static struct platform_device *ls1c_platform_devices[] __initdata = {
-	&ls1x_uart_pdev,
 	&ls1x_eth0_pdev,
 	&ls1x_rtc_pdev,
 	&ls1x_wdt_pdev,
@@ -14,8 +13,6 @@  static struct platform_device *ls1c_platform_devices[] __initdata = {
 
 static int __init ls1c_platform_init(void)
 {
-	ls1x_serial_set_uartclk(&ls1x_uart_pdev);
-
 	return platform_add_devices(ls1c_platform_devices,
 				   ARRAY_SIZE(ls1c_platform_devices));
 }