* [PATCH] powerpc: legacy_serial loop cleanup
@ 2006-03-24 4:17 Michael Neuling
2006-03-24 18:26 ` Hollis Blanchard
2006-03-25 12:44 ` Benjamin Herrenschmidt
0 siblings, 2 replies; 6+ messages in thread
From: Michael Neuling @ 2006-03-24 4:17 UTC (permalink / raw)
To: linuxppc-dev; +Cc: michael
We only ever execute the loop once, so let's move it to a function
making it more readable. Cleanup patch, no functional change.
Signed-off-by: Michael Neuling <mikey@neuling.org>
---
arch/powerpc/kernel/legacy_serial.c | 39 +++++++++++++++++++-----------------
1 files changed, 21 insertions(+), 18 deletions(-)
Index: linux-2.6-linus/arch/powerpc/kernel/legacy_serial.c
===================================================================
--- linux-2.6-linus.orig/arch/powerpc/kernel/legacy_serial.c
+++ linux-2.6-linus/arch/powerpc/kernel/legacy_serial.c
@@ -236,6 +236,26 @@ static int __init add_legacy_pci_port(st
}
#endif
+static void __init setup_legacy_serial_console(int console)
+{
+ if (console >= 0) {
+ struct legacy_serial_info *info =
+ &legacy_serial_infos[legacy_serial_console];
+ void __iomem *addr;
+
+ if (info->taddr == 0)
+ return;
+ addr = ioremap(info->taddr, 0x1000);
+ if (addr == NULL)
+ return;
+ if (info->speed == 0)
+ info->speed = udbg_probe_uart_speed(addr, info->clock);
+ DBG("default console speed = %d\n", info->speed);
+ udbg_init_uart(addr, info->speed, info->clock);
+ }
+ return;
+}
+
/*
* This is called very early, as part of setup_system() or eventually
* setup_arch(), basically before anything else in this file. This function
@@ -319,24 +339,7 @@ void __init find_legacy_serial_ports(voi
DBG("legacy_serial_console = %d\n", legacy_serial_console);
- /* udbg is 64 bits only for now, that will change soon though ... */
- while (legacy_serial_console >= 0) {
- struct legacy_serial_info *info =
- &legacy_serial_infos[legacy_serial_console];
- void __iomem *addr;
-
- if (info->taddr == 0)
- break;
- addr = ioremap(info->taddr, 0x1000);
- if (addr == NULL)
- break;
- if (info->speed == 0)
- info->speed = udbg_probe_uart_speed(addr, info->clock);
- DBG("default console speed = %d\n", info->speed);
- udbg_init_uart(addr, info->speed, info->clock);
- break;
- }
-
+ setup_legacy_serial_console(legacy_serial_console);
DBG(" <- find_legacy_serial_port()\n");
}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc: legacy_serial loop cleanup
2006-03-24 4:17 [PATCH] powerpc: legacy_serial loop cleanup Michael Neuling
@ 2006-03-24 18:26 ` Hollis Blanchard
2006-03-25 4:45 ` Michael Neuling
2006-03-25 12:44 ` Benjamin Herrenschmidt
1 sibling, 1 reply; 6+ messages in thread
From: Hollis Blanchard @ 2006-03-24 18:26 UTC (permalink / raw)
To: linuxppc-dev, mikey; +Cc: michael
On Thursday 23 March 2006 22:17, Michael Neuling wrote:
> We only ever execute the loop once, so let's move it to a function
> making it more readable. Cleanup patch, no functional change.
I don't understand: it's only used once, so make it a function? Why not just
change the "while" to an "if"?
Regardless, two style issues:
- remove the plain "return"
- reduce indenting like so:
if (console < 0)
return;
struct legacy_serial_info *info = ...
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> ---
> arch/powerpc/kernel/legacy_serial.c | 39
+++++++++++++++++++-----------------
> 1 files changed, 21 insertions(+), 18 deletions(-)
>
> Index: linux-2.6-linus/arch/powerpc/kernel/legacy_serial.c
> ===================================================================
> --- linux-2.6-linus.orig/arch/powerpc/kernel/legacy_serial.c
> +++ linux-2.6-linus/arch/powerpc/kernel/legacy_serial.c
> @@ -236,6 +236,26 @@ static int __init add_legacy_pci_port(st
> }
> #endif
>
> +static void __init setup_legacy_serial_console(int console)
> +{
> + if (console >= 0) {
> + struct legacy_serial_info *info =
> + &legacy_serial_infos[legacy_serial_console];
> + void __iomem *addr;
> +
> + if (info->taddr == 0)
> + return;
> + addr = ioremap(info->taddr, 0x1000);
> + if (addr == NULL)
> + return;
> + if (info->speed == 0)
> + info->speed = udbg_probe_uart_speed(addr, info->clock);
> + DBG("default console speed = %d\n", info->speed);
> + udbg_init_uart(addr, info->speed, info->clock);
> + }
> + return;
> +}
> +
> /*
> * This is called very early, as part of setup_system() or eventually
> * setup_arch(), basically before anything else in this file. This function
> @@ -319,24 +339,7 @@ void __init find_legacy_serial_ports(voi
>
> DBG("legacy_serial_console = %d\n", legacy_serial_console);
>
> - /* udbg is 64 bits only for now, that will change soon though ... */
> - while (legacy_serial_console >= 0) {
> - struct legacy_serial_info *info =
> - &legacy_serial_infos[legacy_serial_console];
> - void __iomem *addr;
> -
> - if (info->taddr == 0)
> - break;
> - addr = ioremap(info->taddr, 0x1000);
> - if (addr == NULL)
> - break;
> - if (info->speed == 0)
> - info->speed = udbg_probe_uart_speed(addr, info->clock);
> - DBG("default console speed = %d\n", info->speed);
> - udbg_init_uart(addr, info->speed, info->clock);
> - break;
> - }
> -
> + setup_legacy_serial_console(legacy_serial_console);
> DBG(" <- find_legacy_serial_port()\n");
> }
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc: legacy_serial loop cleanup
2006-03-24 18:26 ` Hollis Blanchard
@ 2006-03-25 4:45 ` Michael Neuling
2006-03-25 7:05 ` Stephen Rothwell
0 siblings, 1 reply; 6+ messages in thread
From: Michael Neuling @ 2006-03-25 4:45 UTC (permalink / raw)
To: Hollis Blanchard; +Cc: michael, linuxppc-dev
> I don't understand: it's only used once, so make it a function? Why not just
> change the "while" to an "if"?
Because you can't use a break statements inside an if. We could you
gotos, but the idea was to increase read ability :-)
> Regardless, two style issues:
> - remove the plain "return"
> - reduce indenting like so:
> if (console < 0)
> return;
> struct legacy_serial_info *info = ...
Agreed. Updated patch below.
Mikey
We only ever execute the loop once, so let's move it to a function
making it more readable. Cleanup patch, no functional change.
Signed-off-by: Michael Neuling <mikey@neuling.org>
---
arch/powerpc/kernel/legacy_serial.c | 38 ++++++++++++++++++------------------
1 files changed, 20 insertions(+), 18 deletions(-)
Index: linux-2.6-powerpc-merge/arch/powerpc/kernel/legacy_serial.c
===================================================================
--- linux-2.6-powerpc-merge.orig/arch/powerpc/kernel/legacy_serial.c
+++ linux-2.6-powerpc-merge/arch/powerpc/kernel/legacy_serial.c
@@ -236,6 +236,25 @@ static int __init add_legacy_pci_port(st
}
#endif
+static void __init setup_legacy_serial_console(int console)
+{
+ struct legacy_serial_info *info =
+ &legacy_serial_infos[legacy_serial_console];
+ void __iomem *addr;
+
+ if (console < 0)
+ return;
+ if (info->taddr == 0)
+ return;
+ addr = ioremap(info->taddr, 0x1000);
+ if (addr == NULL)
+ return;
+ if (info->speed == 0)
+ info->speed = udbg_probe_uart_speed(addr, info->clock);
+ DBG("default console speed = %d\n", info->speed);
+ udbg_init_uart(addr, info->speed, info->clock);
+}
+
/*
* This is called very early, as part of setup_system() or eventually
* setup_arch(), basically before anything else in this file. This function
@@ -319,24 +338,7 @@ void __init find_legacy_serial_ports(voi
DBG("legacy_serial_console = %d\n", legacy_serial_console);
- /* udbg is 64 bits only for now, that will change soon though ... */
- while (legacy_serial_console >= 0) {
- struct legacy_serial_info *info =
- &legacy_serial_infos[legacy_serial_console];
- void __iomem *addr;
-
- if (info->taddr == 0)
- break;
- addr = ioremap(info->taddr, 0x1000);
- if (addr == NULL)
- break;
- if (info->speed == 0)
- info->speed = udbg_probe_uart_speed(addr, info->clock);
- DBG("default console speed = %d\n", info->speed);
- udbg_init_uart(addr, info->speed, info->clock);
- break;
- }
-
+ setup_legacy_serial_console(legacy_serial_console);
DBG(" <- find_legacy_serial_port()\n");
}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc: legacy_serial loop cleanup
2006-03-25 4:45 ` Michael Neuling
@ 2006-03-25 7:05 ` Stephen Rothwell
2006-03-26 0:07 ` Michael Neuling
0 siblings, 1 reply; 6+ messages in thread
From: Stephen Rothwell @ 2006-03-25 7:05 UTC (permalink / raw)
To: Michael Neuling; +Cc: michael, linuxppc-dev
[-- Attachment #1: Type: text/plain, Size: 564 bytes --]
On Sat, 25 Mar 2006 15:45:11 +1100 Michael Neuling <mikey@neuling.org> wrote:
>
> Agreed. Updated patch below.
> .
> .
> +static void __init setup_legacy_serial_console(int console)
> +{
> + struct legacy_serial_info *info =
> + &legacy_serial_infos[legacy_serial_console];
Except that you don't want to do that ^ (assuming you meant "console")
> + void __iomem *addr;
> +
> + if (console < 0)
> + return;
before this ^ ... :-)
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
[-- Attachment #2: Type: application/pgp-signature, Size: 191 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc: legacy_serial loop cleanup
2006-03-24 4:17 [PATCH] powerpc: legacy_serial loop cleanup Michael Neuling
2006-03-24 18:26 ` Hollis Blanchard
@ 2006-03-25 12:44 ` Benjamin Herrenschmidt
1 sibling, 0 replies; 6+ messages in thread
From: Benjamin Herrenschmidt @ 2006-03-25 12:44 UTC (permalink / raw)
To: mikey; +Cc: michael, linuxppc-dev
> +static void __init setup_legacy_serial_console(int console)
> +{
> + if (console >= 0) {
> + struct legacy_serial_info *info =
> + &legacy_serial_infos[legacy_serial_console];
> + void __iomem *addr;
> +
> + if (info->taddr == 0)
> + return;
> + addr = ioremap(info->taddr, 0x1000);
> + if (addr == NULL)
> + return;
> + if (info->speed == 0)
> + info->speed = udbg_probe_uart_speed(addr, info->clock);
> + DBG("default console speed = %d\n", info->speed);
> + udbg_init_uart(addr, info->speed, info->clock);
> + }
> + return;
Hrm... What is the point of having a function ending with return; ?
What about, instead, something like:
if (consoles < 0)
return;
do shit ...
Ben.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc: legacy_serial loop cleanup
2006-03-25 7:05 ` Stephen Rothwell
@ 2006-03-26 0:07 ` Michael Neuling
0 siblings, 0 replies; 6+ messages in thread
From: Michael Neuling @ 2006-03-26 0:07 UTC (permalink / raw)
To: Stephen Rothwell; +Cc: michael, linuxppc-dev
> > +static void __init setup_legacy_serial_console(int console)
> > +{
> > + struct legacy_serial_info *info =3D
> > + &legacy_serial_infos[legacy_serial_console];
>
> Except that you don't want to do that ^ (assuming you meant "console")
>
> > + void __iomem *addr;
> > +
> > + if (console < 0)
> > + return;
>
> before this ^ ... :-)
Indeed, thanks. Updated patch below. This time for sure! :-)
Mikey
We only ever execute the loop once, so let's move it to a function
making it more readable. Cleanup patch, no functional change.
Signed-off-by: Michael Neuling <mikey@neuling.org>
---
arch/powerpc/kernel/legacy_serial.c | 38 ++++++++++++++++++------------------
1 files changed, 19 insertions(+), 19 deletions(-)
Index: linux-2.6-powerpc-merge/arch/powerpc/kernel/legacy_serial.c
===================================================================
--- linux-2.6-powerpc-merge.orig/arch/powerpc/kernel/legacy_serial.c
+++ linux-2.6-powerpc-merge/arch/powerpc/kernel/legacy_serial.c
@@ -236,6 +236,23 @@ static int __init add_legacy_pci_port(st
}
#endif
+static void __init setup_legacy_serial_console(int console)
+{
+ struct legacy_serial_info *info =
+ &legacy_serial_infos[console];
+ void __iomem *addr;
+
+ if (info->taddr == 0)
+ return;
+ addr = ioremap(info->taddr, 0x1000);
+ if (addr == NULL)
+ return;
+ if (info->speed == 0)
+ info->speed = udbg_probe_uart_speed(addr, info->clock);
+ DBG("default console speed = %d\n", info->speed);
+ udbg_init_uart(addr, info->speed, info->clock);
+}
+
/*
* This is called very early, as part of setup_system() or eventually
* setup_arch(), basically before anything else in this file. This function
@@ -318,25 +335,8 @@ void __init find_legacy_serial_ports(voi
#endif
DBG("legacy_serial_console = %d\n", legacy_serial_console);
-
- /* udbg is 64 bits only for now, that will change soon though ... */
- while (legacy_serial_console >= 0) {
- struct legacy_serial_info *info =
- &legacy_serial_infos[legacy_serial_console];
- void __iomem *addr;
-
- if (info->taddr == 0)
- break;
- addr = ioremap(info->taddr, 0x1000);
- if (addr == NULL)
- break;
- if (info->speed == 0)
- info->speed = udbg_probe_uart_speed(addr, info->clock);
- DBG("default console speed = %d\n", info->speed);
- udbg_init_uart(addr, info->speed, info->clock);
- break;
- }
-
+ if (legacy_serial_console >= 0)
+ setup_legacy_serial_console(legacy_serial_console);
DBG(" <- find_legacy_serial_port()\n");
}
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-03-26 0:07 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-03-24 4:17 [PATCH] powerpc: legacy_serial loop cleanup Michael Neuling
2006-03-24 18:26 ` Hollis Blanchard
2006-03-25 4:45 ` Michael Neuling
2006-03-25 7:05 ` Stephen Rothwell
2006-03-26 0:07 ` Michael Neuling
2006-03-25 12:44 ` Benjamin Herrenschmidt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).