Skip to contentSkip to conversation

ARM: BCMRING: Fix UART declaration and clk dev name mismatch

PATCH New Jiandong Zheng 2 participants 5 messages May 7, 2012

Subsystems (2)
Arm (32-bit) Arm sub-architectures
lore
Share & tools
lore

Jiandong Zheng

author
#0 lore raw

PATCH 1/1 ARM: BCMRING: Fix UART declaration and clk dev name mismatch

Fix bcmring UART declaration and clk dev name mismatch

With 888073d41680e8244232f3d850e0424a4e9de60f (ARM:
bcmring: fix UART declarations), it introduced clk dev name mismatch and leads to UART initialization failure during boot.

Signed-off-by Jiandong Zheng<[email protected] <http://lists.infradead.org/mailman/listinfo/linux-arm-kernel>>
---
   arch/arm/mach-bcmring/core.c |    4 ++--
   1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm/mach-bcmring/core.c +2 -2
index adbfb19..5559857 100644
@@ -110,10 +110,10 @@ static struct clk_lookup lookups[] = {
           .con_id = "apb_pclk",
           .clk =&dummy_apb_pclk,
       }, {            /* UART0 */
-        .dev_id = "uarta",
+        .dev_id = "uartA",
           .clk =&uart_clk,
       }, {            /* UART1 */
-        .dev_id = "uartb",
+        .dev_id = "uartB",
           .clk =&uart_clk,
       }, {            /* SP804 timer 0 */
           .dev_id = "sp804",
Signature · 1 line
1.7.9.5
#1 lore raw

PATCH 1/1 ARM: BCMRING: Fix UART declaration and clk dev name mismatch

Quoted 5 lines · Jiandong Zheng

Fix bcmring UART declaration and clk dev name mismatch

With 888073d41680e8244232f3d850e0424a4e9de60f (ARM:
bcmring: fix UART declarations), it introduced clk dev name mismatch and leads to UART initialization failure during boot.

Do things work if you simply do a partial revert of 888073d41 ?

Meaning:

-static AMBA_APB_DEVICE(uartA, "uartA", 0, MM_ADDR_IO_UARTA, {IRQ_UARTA}, NULL); -static AMBA_APB_DEVICE(uartB, "uartB", 0, MM_ADDR_IO_UARTB, {IRQ_UARTB}, NULL); +static AMBA_APB_DEVICE(uartA, "uarta", 0, MM_ADDR_IO_UARTA, {IRQ_UARTA}, NULL); +static AMBA_APB_DEVICE(uartB, "uartb", 0, MM_ADDR_IO_UARTB, {IRQ_UARTB}, NULL);

The original error messages led me to believe it was two errors; i.e. (1) the wrong # of args error, and (2) the "a" vs. "A" error. But I think Olof was right on his 1st instinct, that it was just the single error, and the a <---> A thing was a side effect.

In which case the above should just work, and it will also keep the naming consistent with what it was before any of these commits.

Paul.
--

Quoted 36 lines
Signed-off-by Jiandong Zheng<[email protected]

?<http://lists.infradead.org/mailman/listinfo/linux-arm-kernel>>
---
?arch/arm/mach-bcmring/core.c | ? ?4 ++--
?1 file changed, 2 insertions(+), 2 deletions(-)

arch/arm/mach-bcmring/core.c +0 -0
index adbfb19..5559857 100644
@@ -110,10 +110,10 @@ static struct clk_lookup lookups[] = {
 ? ? ? ? ?.con_id = "apb_pclk",
 ? ? ? ? ?.clk =&dummy_apb_pclk,

? ? ?}, { ? ? ? ? ? ?/* UART0 */
- ? ? ? ?.dev_id = "uarta",
+ ? ? ? ?.dev_id = "uartA",
? ? ? ? ?.clk =&uart_clk,
? ? ?}, { ? ? ? ? ? ?/* UART1 */
- ? ? ? ?.dev_id = "uartb",
+ ? ? ? ?.dev_id = "uartB",
? ? ? ? ?.clk =&uart_clk,
? ? ?}, { ? ? ? ? ? ?/* SP804 timer 0 */
? ? ? ? ?.dev_id = "sp804",
--
1.7.9.5

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

author
#2 lore raw

PATCH 1/1 ARM: BCMRING: Fix UART declaration and clk dev name mismatch

Quoted 10 lines · Paul Gortmaker

Do things work if you simply do a partial revert of 888073d41 ?

Meaning:

(quoted diff) +2 -2
-static AMBA_APB_DEVICE(uartA, "uartA", 0, MM_ADDR_IO_UARTA, {IRQ_UARTA}, NULL);
-static AMBA_APB_DEVICE(uartB, "uartB", 0, MM_ADDR_IO_UARTB, {IRQ_UARTB}, NULL);
+static AMBA_APB_DEVICE(uartA, "uarta", 0, MM_ADDR_IO_UARTA, {IRQ_UARTA}, NULL);
+static AMBA_APB_DEVICE(uartB, "uartb", 0, MM_ADDR_IO_UARTB, {IRQ_UARTB}, NULL);
Quoted 8 lines

The original error messages led me to believe it was two errors; i.e. (1) the wrong # of args error, and (2) the "a" vs. "A" error. But I think Olof was right on his 1st instinct, that it was just the single error, and the a<---> A thing was a side effect.

In which case the above should just work, and it will also keep the naming consistent with what it was before any of these commits.

Yes. Either way works. Just in original code, the name uartA and the name string "uarta" looks confusing and easy to get it wrong.

Thanks,
JD

Quoted 4 lines

Paul.
--

#3 lore raw

PATCH 1/1 ARM: BCMRING: Fix UART declaration and clk dev name mismatch

Quoted 16 lines · [Re: [PATCH] ARM: BCMRING: Fix UART declaration and clk dev name mismatch] On…
Quoted 15 lines · Paul Gortmaker

Do things work if you simply do a partial revert of 888073d41 ?

Meaning:

(quoted diff) +2 -2
-static AMBA_APB_DEVICE(uartA, "uartA", 0, MM_ADDR_IO_UARTA, {IRQ_UARTA}, NULL);
-static AMBA_APB_DEVICE(uartB, "uartB", 0, MM_ADDR_IO_UARTB, {IRQ_UARTB}, NULL);
+static AMBA_APB_DEVICE(uartA, "uarta", 0, MM_ADDR_IO_UARTA, {IRQ_UARTA}, NULL);
+static AMBA_APB_DEVICE(uartB, "uartb", 0, MM_ADDR_IO_UARTB, {IRQ_UARTB}, NULL);

The original error messages led me to believe it was two errors; i.e. (1) the wrong # of args error, and (2) the "a" vs. "A" error. But I think Olof was right on his 1st instinct, that it was just the single error, and the a<---> A thing was a side effect.

Quoted 5 lines
Quoted 3 lines

In which case the above should just work, and it will also keep the naming consistent with what it was before any of these commits.

Yes. Either way works. Just in original code, the name uartA and the name string "uarta" looks confusing and easy to get it wrong.

Agreed, as it confused me as well. But I don't want to break any existing userspace. So if people expect the name "uarta" and the name "uartb" then we really should go with that. But I have no insight into the use case or userspace. So I leave that part of the input up to you.

Thanks,
Paul.

Quoted 9 lines

Thanks,
JD

Quoted 4 lines

Paul.
--

author
#4 lore raw

PATCH 1/1 ARM: BCMRING: Fix UART declaration and clk dev name mismatch

Quoted 22 lines · Paul Gortmaker
Quoted 15 lines · [Re: [PATCH] ARM: BCMRING: Fix UART declaration and clk dev name mismatch] On…
Quoted 14 lines · Paul Gortmaker

Do things work if you simply do a partial revert of 888073d41 ?

Meaning:

(quoted diff) +2 -2
-static AMBA_APB_DEVICE(uartA, "uartA", 0, MM_ADDR_IO_UARTA, {IRQ_UARTA}, NULL);
-static AMBA_APB_DEVICE(uartB, "uartB", 0, MM_ADDR_IO_UARTB, {IRQ_UARTB}, NULL);
+static AMBA_APB_DEVICE(uartA, "uarta", 0, MM_ADDR_IO_UARTA, {IRQ_UARTA}, NULL);
+static AMBA_APB_DEVICE(uartB, "uartb", 0, MM_ADDR_IO_UARTB, {IRQ_UARTB}, NULL);

The original error messages led me to believe it was two errors; i.e. (1) the wrong # of args error, and (2) the "a" vs. "A" error. But I think Olof was right on his 1st instinct, that it was just the
single error, and the a<---> A thing was a side effect.

Quoted 5 lines
Quoted 3 lines

In which case the above should just work, and it will also keep the naming consistent with what it was before any of these commits.

Yes. Either way works. Just in original code, the name uartA and the name string "uarta" looks confusing and easy to get it wrong.

Quoted 5 lines

Agreed, as it confused me as well. But I don't want to break any existing userspace. So if people expect the name "uarta" and the name "uartb" then we really should go with that. But I have no insight into the use case or userspace. So I leave that part of the input up to you.

I don't see user space has direct usage of these strings and the console runs fine so that it should be safe.

Thanks,
JD

Quoted 10 lines

Thanks,
Paul.

Quoted 6 lines

Thanks,
JD

Quoted 3 lines

Paul.
--