diff mbox

[1/2] ARM: ux500: define serial port aliases

Message ID 1436520735-23692-1-git-send-email-linus.walleij@linaro.org
State Accepted
Commit 109978dea44e8416524e69f0e8888810fdf6e72c
Headers show

Commit Message

Linus Walleij July 10, 2015, 9:32 a.m. UTC
This enumerates the PL011 serial ports on the Ux500. This is
necessary to do if we want to remove one of the serial ports,
since userspace depends on console to be present on ttyAMA2
and we must not break userspace.

Cc: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Moved the alias definitions from the core SoC file
  to the toplevel DTS file, as requested by Arnd.

ARM SoC guys: if you find this patch OK, please apply this
directly for fixes, as a prerequisite for patch 2/2, then
apply 2/2.
---
 arch/arm/boot/dts/ste-ccu8540.dts           | 7 +++++++
 arch/arm/boot/dts/ste-ccu9540.dts           | 7 +++++++
 arch/arm/boot/dts/ste-dbx5x0.dtsi           | 6 +++---
 arch/arm/boot/dts/ste-hrefprev60-stuib.dts  | 7 +++++++
 arch/arm/boot/dts/ste-hrefprev60-tvk.dts    | 7 +++++++
 arch/arm/boot/dts/ste-hrefv60plus-stuib.dts | 7 +++++++
 arch/arm/boot/dts/ste-hrefv60plus-tvk.dts   | 7 +++++++
 arch/arm/boot/dts/ste-snowball.dts          | 7 +++++++
 8 files changed, 52 insertions(+), 3 deletions(-)

Comments

Linus Walleij July 13, 2015, 8:16 a.m. UTC | #1
On Sun, Jul 12, 2015 at 11:02 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday 10 July 2015 11:32:15 Linus Walleij wrote:
>> This enumerates the PL011 serial ports on the Ux500. This is
>> necessary to do if we want to remove one of the serial ports,
>> since userspace depends on console to be present on ttyAMA2
>> and we must not break userspace.
>>
>> Cc: Ulf Hansson <ulf.hansson@linaro.org>
>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>> ---
>> ChangeLog v1->v2:
>> - Moved the alias definitions from the core SoC file
>>   to the toplevel DTS file, as requested by Arnd.
>>
>>
>
> Do all of the boards expose all three uarts in the same order?

Yes and that is the reason the patch series needs this one
to come first.

What happened historically was that userspace was hard-coded
to use ttyAMA2 as console. So for this reason, all boards had
to define ttyAMA0, 1, 2 in their board file (later device tree) so they
would be enumerated in the right order, relying on the DT to
be parsed top to bottom of course.

So when I realized (in patch 2/2) that ttyAMA1 was not really
used on some boards, it was impossible to remove it
as userspace relied on the enumeration.

However I have to remove ttyAMA1 because it is fundamentally
wrong (the RC pin is used for MMC!). Incidentally it worked
to steal

But that breaks userspace since
Linus Walleij July 13, 2015, 8:21 a.m. UTC | #2
On Mon, Jul 13, 2015 at 10:16 AM, Linus Walleij
<linus.walleij@linaro.org> wrote:
> On Sun, Jul 12, 2015 at 11:02 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Friday 10 July 2015 11:32:15 Linus Walleij wrote:
>>> This enumerates the PL011 serial ports on the Ux500. This is
>>> necessary to do if we want to remove one of the serial ports,
>>> since userspace depends on console to be present on ttyAMA2
>>> and we must not break userspace.
>>>
>>> Cc: Ulf Hansson <ulf.hansson@linaro.org>
>>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>>> ---
>>> ChangeLog v1->v2:
>>> - Moved the alias definitions from the core SoC file
>>>   to the toplevel DTS file, as requested by Arnd.
>>>
>>>
>>
>> Do all of the boards expose all three uarts in the same order?
>
> Yes and that is the reason the patch series needs this one
> to come first.
>
> What happened historically was that userspace was hard-coded
> to use ttyAMA2 as console. So for this reason, all boards had
> to define ttyAMA0, 1, 2 in their board file (later device tree) so they
> would be enumerated in the right order, relying on the DT to
> be parsed top to bottom of course.
>
> So when I realized (in patch 2/2) that ttyAMA1 was not really
> used on some boards, it was impossible to remove it
> as userspace relied on the enumeration.
>
> However I have to remove ttyAMA1 because it is fundamentally
> wrong (the RC pin is used for MMC!). Incidentally it worked
> to steal
>
> But that breaks userspace since

Keyboard slip.

Incidentally it worked to steal the pin from ttyAMA1 even if it
was connected to the UART, but it makes things pretty
confused and we tightened the stringence of the pin control
framework and it exploded.

It turns out that ttyAMA1 was only enabled on these boards
(all boards) to get the enumeration right for userspace.

However the right way to get serial port enumeration right for
userspace is to use alias. And that is what the patch does.

Since the original code just defined all three serial ports
for all boards by them including ste-dbx500.dtsi I first made
the patch to that file, and then as you said it had to be done
on a per-toplevel file, I did it in the individual DTS files instead.

Yours,
Linus Walleij
Linus Walleij July 16, 2015, 2:03 p.m. UTC | #3
On Tue, Jul 14, 2015 at 10:53 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday 13 July 2015 10:21:51 Linus Walleij wrote:
>>
>> Since the original code just defined all three serial ports
>> for all boards by them including ste-dbx500.dtsi I first made
>> the patch to that file, and then as you said it had to be done
>> on a per-toplevel file, I did it in the individual DTS files instead.
>
> I'm not following here. If you have boards that want port 1 to be
> disabled, shouldn't you remove that alias?

No, that is done by not changing status = "disabled"; to
status = "okay"; on a per-board basis.

This is what the second patch is doing.

The aliases are set also to disabled ports to ensure enumeration
for the cases where a board file enabled a previously disabled
UART.

Yours,
Linus Walleij
Linus Walleij July 16, 2015, 7:57 p.m. UTC | #4
On Thu, Jul 16, 2015 at 4:09 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday 16 July 2015 16:03:18 Linus Walleij wrote:

>> The aliases are set also to disabled ports to ensure enumeration
>> for the cases where a board file enabled a previously disabled
>> UART.
>>
>
> Well, both really. The status property tells the kernel to not
> ever touch a device, while the alias is what traditionally gets
> used to assign an OS visible device name to a device in the tree.
> This is more true for MacOS and AIX than Linux, which sometimes
> does its own naming, but we really should not have an alias pointing
> at a disabled device node.

OK ... I will make a patch that remove the aliases for the disabled
nodes.

But this makes me think of whether the DT compiler should allow
it or at least warn about it?

Yours,
Linus Walleij
diff mbox

Patch

diff --git a/arch/arm/boot/dts/ste-ccu8540.dts b/arch/arm/boot/dts/ste-ccu8540.dts
index 32dd55e5f4e6..6eaaf638e52e 100644
--- a/arch/arm/boot/dts/ste-ccu8540.dts
+++ b/arch/arm/boot/dts/ste-ccu8540.dts
@@ -17,6 +17,13 @@ 
 	model = "ST-Ericsson U8540 platform with Device Tree";
 	compatible = "st-ericsson,ccu8540", "st-ericsson,u8540";
 
+	/* This stablilizes the serial port enumeration */
+	aliases {
+		serial0 = &ux500_serial0;
+		serial1 = &ux500_serial1;
+		serial2 = &ux500_serial2;
+	};
+
 	memory@0 {
 		device_type = "memory";
 		reg = <0x20000000 0x1f000000>, <0xc0000000 0x3f000000>;
diff --git a/arch/arm/boot/dts/ste-ccu9540.dts b/arch/arm/boot/dts/ste-ccu9540.dts
index 651c56d400a4..c8b815819cfe 100644
--- a/arch/arm/boot/dts/ste-ccu9540.dts
+++ b/arch/arm/boot/dts/ste-ccu9540.dts
@@ -16,6 +16,13 @@ 
 	model = "ST-Ericsson CCU9540 platform with Device Tree";
 	compatible = "st-ericsson,ccu9540", "st-ericsson,u9540";
 
+	/* This stablilizes the serial port enumeration */
+	aliases {
+		serial0 = &ux500_serial0;
+		serial1 = &ux500_serial1;
+		serial2 = &ux500_serial2;
+	};
+
 	memory {
 		reg = <0x00000000 0x20000000>;
 	};
diff --git a/arch/arm/boot/dts/ste-dbx5x0.dtsi b/arch/arm/boot/dts/ste-dbx5x0.dtsi
index 853684ad7773..a75f3289e653 100644
--- a/arch/arm/boot/dts/ste-dbx5x0.dtsi
+++ b/arch/arm/boot/dts/ste-dbx5x0.dtsi
@@ -971,7 +971,7 @@ 
 			power-domains = <&pm_domains DOMAIN_VAPE>;
 		};
 
-		uart@80120000 {
+		ux500_serial0: uart@80120000 {
 			compatible = "arm,pl011", "arm,primecell";
 			reg = <0x80120000 0x1000>;
 			interrupts = <0 11 IRQ_TYPE_LEVEL_HIGH>;
@@ -986,7 +986,7 @@ 
 			status = "disabled";
 		};
 
-		uart@80121000 {
+		ux500_serial1: uart@80121000 {
 			compatible = "arm,pl011", "arm,primecell";
 			reg = <0x80121000 0x1000>;
 			interrupts = <0 19 IRQ_TYPE_LEVEL_HIGH>;
@@ -1001,7 +1001,7 @@ 
 			status = "disabled";
 		};
 
-		uart@80007000 {
+		ux500_serial2: uart@80007000 {
 			compatible = "arm,pl011", "arm,primecell";
 			reg = <0x80007000 0x1000>;
 			interrupts = <0 26 IRQ_TYPE_LEVEL_HIGH>;
diff --git a/arch/arm/boot/dts/ste-hrefprev60-stuib.dts b/arch/arm/boot/dts/ste-hrefprev60-stuib.dts
index 2b1cb5b584b6..18e9795a94f9 100644
--- a/arch/arm/boot/dts/ste-hrefprev60-stuib.dts
+++ b/arch/arm/boot/dts/ste-hrefprev60-stuib.dts
@@ -17,6 +17,13 @@ 
 	model = "ST-Ericsson HREF (pre-v60) and ST UIB";
 	compatible = "st-ericsson,mop500", "st-ericsson,u8500";
 
+	/* This stablilizes the serial port enumeration */
+	aliases {
+		serial0 = &ux500_serial0;
+		serial1 = &ux500_serial1;
+		serial2 = &ux500_serial2;
+	};
+
 	soc {
 		/* Reset line for the BU21013 touchscreen */
 		i2c@80110000 {
diff --git a/arch/arm/boot/dts/ste-hrefprev60-tvk.dts b/arch/arm/boot/dts/ste-hrefprev60-tvk.dts
index 59523f866812..24739914e689 100644
--- a/arch/arm/boot/dts/ste-hrefprev60-tvk.dts
+++ b/arch/arm/boot/dts/ste-hrefprev60-tvk.dts
@@ -16,4 +16,11 @@ 
 / {
 	model = "ST-Ericsson HREF (pre-v60) and TVK1281618 UIB";
 	compatible = "st-ericsson,mop500", "st-ericsson,u8500";
+
+	/* This stablilizes the serial port enumeration */
+	aliases {
+		serial0 = &ux500_serial0;
+		serial1 = &ux500_serial1;
+		serial2 = &ux500_serial2;
+	};
 };
diff --git a/arch/arm/boot/dts/ste-hrefv60plus-stuib.dts b/arch/arm/boot/dts/ste-hrefv60plus-stuib.dts
index 8c6a2de56cf1..c2e1ba019a2f 100644
--- a/arch/arm/boot/dts/ste-hrefv60plus-stuib.dts
+++ b/arch/arm/boot/dts/ste-hrefv60plus-stuib.dts
@@ -19,6 +19,13 @@ 
 	model = "ST-Ericsson HREF (v60+) and ST UIB";
 	compatible = "st-ericsson,hrefv60+", "st-ericsson,u8500";
 
+	/* This stablilizes the serial port enumeration */
+	aliases {
+		serial0 = &ux500_serial0;
+		serial1 = &ux500_serial1;
+		serial2 = &ux500_serial2;
+	};
+
 	soc {
 		/* Reset line for the BU21013 touchscreen */
 		i2c@80110000 {
diff --git a/arch/arm/boot/dts/ste-hrefv60plus-tvk.dts b/arch/arm/boot/dts/ste-hrefv60plus-tvk.dts
index d53cccdce776..ebd8547e98f1 100644
--- a/arch/arm/boot/dts/ste-hrefv60plus-tvk.dts
+++ b/arch/arm/boot/dts/ste-hrefv60plus-tvk.dts
@@ -18,4 +18,11 @@ 
 / {
 	model = "ST-Ericsson HREF (v60+) and TVK1281618 UIB";
 	compatible = "st-ericsson,hrefv60+", "st-ericsson,u8500";
+
+	/* This stablilizes the serial port enumeration */
+	aliases {
+		serial0 = &ux500_serial0;
+		serial1 = &ux500_serial1;
+		serial2 = &ux500_serial2;
+	};
 };
diff --git a/arch/arm/boot/dts/ste-snowball.dts b/arch/arm/boot/dts/ste-snowball.dts
index 9edadc37719f..acf14cebef81 100644
--- a/arch/arm/boot/dts/ste-snowball.dts
+++ b/arch/arm/boot/dts/ste-snowball.dts
@@ -18,6 +18,13 @@ 
 	model = "Calao Systems Snowball platform with device tree";
 	compatible = "calaosystems,snowball-a9500", "st-ericsson,u9500";
 
+	/* This stablilizes the serial port enumeration */
+	aliases {
+		serial0 = &ux500_serial0;
+		serial1 = &ux500_serial1;
+		serial2 = &ux500_serial2;
+	};
+
 	memory {
 		reg = <0x00000000 0x20000000>;
 	};