| Message ID | 1396022885-6102-6-git-send-email-grant.likely@linaro.org |
|---|---|
| State | Accepted |
| Commit | a752ee56ad84bf9a35b8323af1ad22b03c1df2c4 |
| Headers | show |
Hi, On Fri, Mar 28, 2014 at 09:08:05AM -0700, Grant Likely wrote: > The evh_bytechan, hvc_opal and hvc_vio drivers all open code the parsing > of the stdout node in the device tree. This patch simplifies the driver > by removing the duplicated functionality. > > Signed-off-by: Grant Likely <grant.likely@linaro.org> > --- > drivers/of/base.c | 5 ++++- > drivers/tty/ehv_bytechan.c | 43 ++++--------------------------------------- > drivers/tty/hvc/hvc_opal.c | 14 +++----------- > drivers/tty/hvc/hvc_vio.c | 29 ++++++++++------------------- > include/linux/of.h | 1 + > 5 files changed, 22 insertions(+), 70 deletions(-) > > diff --git a/drivers/of/base.c b/drivers/of/base.c > index a5643badeb1d..f845fd05d750 100644 > --- a/drivers/of/base.c > +++ b/drivers/of/base.c > @@ -35,7 +35,7 @@ struct device_node *of_allnodes; > EXPORT_SYMBOL(of_allnodes); > struct device_node *of_chosen; > struct device_node *of_aliases; > -static struct device_node *of_stdout; > +struct device_node *of_stdout; > > static struct kset *of_kset; > > @@ -1956,9 +1956,12 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64 align)) > of_chosen = of_find_node_by_path("/chosen@0"); > > if (of_chosen) { > + /* linux,stdout-path and /aliases/stdout are for legacy compatibility */ > const char *name = of_get_property(of_chosen, "stdout-path", NULL); > if (!name) > name = of_get_property(of_chosen, "linux,stdout-path", NULL); > + if (!name) > + name = of_get_property(of_aliases, "stdout", NULL); > if (name) > of_stdout = of_find_node_by_path(name); > } > diff --git a/drivers/tty/ehv_bytechan.c b/drivers/tty/ehv_bytechan.c > index 0419b69e270f..4f485e88f60c 100644 > --- a/drivers/tty/ehv_bytechan.c > +++ b/drivers/tty/ehv_bytechan.c > @@ -108,55 +108,23 @@ static void disable_tx_interrupt(struct ehv_bc_data *bc) > * > * The byte channel to be used for the console is specified via a "stdout" > * property in the /chosen node. > - * > - * For compatible with legacy device trees, we also look for a "stdout" alias. > */ Did you mean to remove this /aliases/stdout checking? Based on the comment it seems that some older device trees / platforms might rely on it? > static int find_console_handle(void) > { > - struct device_node *np, *np2; > + struct device_node *np = of_stdout; > const char *sprop = NULL; > const uint32_t *iprop; > > - np = of_find_node_by_path("/chosen"); > - if (np) > - sprop = of_get_property(np, "stdout-path", NULL); > - > - if (!np || !sprop) { > - of_node_put(np); > - np = of_find_node_by_name(NULL, "aliases"); > - if (np) > - sprop = of_get_property(np, "stdout", NULL); > - } > - > - if (!sprop) { > - of_node_put(np); > - return 0; > - } > - > /* We don't care what the aliased node is actually called. We only > * care if it's compatible with "epapr,hv-byte-channel", because that > - * indicates that it's a byte channel node. We use a temporary > - * variable, 'np2', because we can't release 'np' until we're done with > - * 'sprop'. > + * indicates that it's a byte channel node. > */ > - np2 = of_find_node_by_path(sprop); > - of_node_put(np); > - np = np2; > - if (!np) { > - pr_warning("ehv-bc: stdout node '%s' does not exist\n", sprop); > - return 0; > - } > - > - /* Is it a byte channel? */ > - if (!of_device_is_compatible(np, "epapr,hv-byte-channel")) { > - of_node_put(np); > + if (!np || !of_device_is_compatible(np, "epapr,hv-byte-channel")) > return 0; > - } > > stdout_irq = irq_of_parse_and_map(np, 0); > if (stdout_irq == NO_IRQ) { > - pr_err("ehv-bc: no 'interrupts' property in %s node\n", sprop); > - of_node_put(np); > + pr_err("ehv-bc: no 'interrupts' property in %s node\n", np->full_name); > return 0; > } > > @@ -167,12 +135,9 @@ static int find_console_handle(void) > if (!iprop) { > pr_err("ehv-bc: no 'hv-handle' property in %s node\n", > np->name); > - of_node_put(np); > return 0; > } > stdout_bc = be32_to_cpu(*iprop); > - > - of_node_put(np); > return 1; > } > > diff --git a/drivers/tty/hvc/hvc_opal.c b/drivers/tty/hvc/hvc_opal.c > index b01659bd4f7c..0ec1a80dd6a2 100644 > --- a/drivers/tty/hvc/hvc_opal.c > +++ b/drivers/tty/hvc/hvc_opal.c > @@ -322,22 +322,14 @@ static void udbg_init_opal_common(void) > > void __init hvc_opal_init_early(void) > { > - struct device_node *stdout_node = NULL; > + struct device_node *stdout_node = of_node_get(of_stdout); > const __be32 *termno; > const char *name = NULL; Name is no longer used, should be removed. > const struct hv_ops *ops; > u32 index; > > - /* find the boot console from /chosen/stdout */ > - if (of_chosen) > - name = of_get_property(of_chosen, "linux,stdout-path", NULL); > - if (name) { > - stdout_node = of_find_node_by_path(name); > - if (!stdout_node) { > - pr_err("hvc_opal: Failed to locate default console!\n"); > - return; > - } > - } else { > + /* If the console wasn't in /chosen, try /ibm,opal */ > + if (!stdout_node) { > struct device_node *opal, *np; > > /* Current OPAL takeover doesn't provide the stdout -Olof -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On Fri, 28 Mar 2014 11:54:43 -0700, Olof Johansson <olof@lixom.net> wrote: > On Fri, Mar 28, 2014 at 09:08:05AM -0700, Grant Likely wrote: > > diff --git a/drivers/tty/ehv_bytechan.c b/drivers/tty/ehv_bytechan.c > > index 0419b69e270f..4f485e88f60c 100644 > > --- a/drivers/tty/ehv_bytechan.c > > +++ b/drivers/tty/ehv_bytechan.c > > @@ -108,55 +108,23 @@ static void disable_tx_interrupt(struct ehv_bc_data *bc) > > * > > * The byte channel to be used for the console is specified via a "stdout" > > * property in the /chosen node. > > - * > > - * For compatible with legacy device trees, we also look for a "stdout" alias. > > */ > > Did you mean to remove this /aliases/stdout checking? Based on the comment it > seems that some older device trees / platforms might rely on it? I moved it to the common code. > > diff --git a/drivers/tty/hvc/hvc_opal.c b/drivers/tty/hvc/hvc_opal.c > > index b01659bd4f7c..0ec1a80dd6a2 100644 > > --- a/drivers/tty/hvc/hvc_opal.c > > +++ b/drivers/tty/hvc/hvc_opal.c > > @@ -322,22 +322,14 @@ static void udbg_init_opal_common(void) > > > > void __init hvc_opal_init_early(void) > > { > > - struct device_node *stdout_node = NULL; > > + struct device_node *stdout_node = of_node_get(of_stdout); > > const __be32 *termno; > > const char *name = NULL; > > Name is no longer used, should be removed. Fixed, thanks. g. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/of/base.c b/drivers/of/base.c index a5643badeb1d..f845fd05d750 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -35,7 +35,7 @@ struct device_node *of_allnodes; EXPORT_SYMBOL(of_allnodes); struct device_node *of_chosen; struct device_node *of_aliases; -static struct device_node *of_stdout; +struct device_node *of_stdout; static struct kset *of_kset; @@ -1956,9 +1956,12 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64 align)) of_chosen = of_find_node_by_path("/chosen@0"); if (of_chosen) { + /* linux,stdout-path and /aliases/stdout are for legacy compatibility */ const char *name = of_get_property(of_chosen, "stdout-path", NULL); if (!name) name = of_get_property(of_chosen, "linux,stdout-path", NULL); + if (!name) + name = of_get_property(of_aliases, "stdout", NULL); if (name) of_stdout = of_find_node_by_path(name); } diff --git a/drivers/tty/ehv_bytechan.c b/drivers/tty/ehv_bytechan.c index 0419b69e270f..4f485e88f60c 100644 --- a/drivers/tty/ehv_bytechan.c +++ b/drivers/tty/ehv_bytechan.c @@ -108,55 +108,23 @@ static void disable_tx_interrupt(struct ehv_bc_data *bc) * * The byte channel to be used for the console is specified via a "stdout" * property in the /chosen node. - * - * For compatible with legacy device trees, we also look for a "stdout" alias. */ static int find_console_handle(void) { - struct device_node *np, *np2; + struct device_node *np = of_stdout; const char *sprop = NULL; const uint32_t *iprop; - np = of_find_node_by_path("/chosen"); - if (np) - sprop = of_get_property(np, "stdout-path", NULL); - - if (!np || !sprop) { - of_node_put(np); - np = of_find_node_by_name(NULL, "aliases"); - if (np) - sprop = of_get_property(np, "stdout", NULL); - } - - if (!sprop) { - of_node_put(np); - return 0; - } - /* We don't care what the aliased node is actually called. We only * care if it's compatible with "epapr,hv-byte-channel", because that - * indicates that it's a byte channel node. We use a temporary - * variable, 'np2', because we can't release 'np' until we're done with - * 'sprop'. + * indicates that it's a byte channel node. */ - np2 = of_find_node_by_path(sprop); - of_node_put(np); - np = np2; - if (!np) { - pr_warning("ehv-bc: stdout node '%s' does not exist\n", sprop); - return 0; - } - - /* Is it a byte channel? */ - if (!of_device_is_compatible(np, "epapr,hv-byte-channel")) { - of_node_put(np); + if (!np || !of_device_is_compatible(np, "epapr,hv-byte-channel")) return 0; - } stdout_irq = irq_of_parse_and_map(np, 0); if (stdout_irq == NO_IRQ) { - pr_err("ehv-bc: no 'interrupts' property in %s node\n", sprop); - of_node_put(np); + pr_err("ehv-bc: no 'interrupts' property in %s node\n", np->full_name); return 0; } @@ -167,12 +135,9 @@ static int find_console_handle(void) if (!iprop) { pr_err("ehv-bc: no 'hv-handle' property in %s node\n", np->name); - of_node_put(np); return 0; } stdout_bc = be32_to_cpu(*iprop); - - of_node_put(np); return 1; } diff --git a/drivers/tty/hvc/hvc_opal.c b/drivers/tty/hvc/hvc_opal.c index b01659bd4f7c..0ec1a80dd6a2 100644 --- a/drivers/tty/hvc/hvc_opal.c +++ b/drivers/tty/hvc/hvc_opal.c @@ -322,22 +322,14 @@ static void udbg_init_opal_common(void) void __init hvc_opal_init_early(void) { - struct device_node *stdout_node = NULL; + struct device_node *stdout_node = of_node_get(of_stdout); const __be32 *termno; const char *name = NULL; const struct hv_ops *ops; u32 index; - /* find the boot console from /chosen/stdout */ - if (of_chosen) - name = of_get_property(of_chosen, "linux,stdout-path", NULL); - if (name) { - stdout_node = of_find_node_by_path(name); - if (!stdout_node) { - pr_err("hvc_opal: Failed to locate default console!\n"); - return; - } - } else { + /* If the console wasn't in /chosen, try /ibm,opal */ + if (!stdout_node) { struct device_node *opal, *np; /* Current OPAL takeover doesn't provide the stdout diff --git a/drivers/tty/hvc/hvc_vio.c b/drivers/tty/hvc/hvc_vio.c index b594abfbf21e..5618b5fc7500 100644 --- a/drivers/tty/hvc/hvc_vio.c +++ b/drivers/tty/hvc/hvc_vio.c @@ -404,42 +404,35 @@ module_exit(hvc_vio_exit); void __init hvc_vio_init_early(void) { - struct device_node *stdout_node; const __be32 *termno; const char *name; const struct hv_ops *ops; /* find the boot console from /chosen/stdout */ - if (!of_chosen) + if (!of_stdout) return; - name = of_get_property(of_chosen, "linux,stdout-path", NULL); - if (name == NULL) - return; - stdout_node = of_find_node_by_path(name); - if (!stdout_node) - return; - name = of_get_property(stdout_node, "name", NULL); + name = of_get_property(of_stdout, "name", NULL); if (!name) { printk(KERN_WARNING "stdout node missing 'name' property!\n"); - goto out; + return; } /* Check if it's a virtual terminal */ if (strncmp(name, "vty", 3) != 0) - goto out; - termno = of_get_property(stdout_node, "reg", NULL); + return; + termno = of_get_property(of_stdout, "reg", NULL); if (termno == NULL) - goto out; + return; hvterm_priv0.termno = of_read_number(termno, 1); spin_lock_init(&hvterm_priv0.buf_lock); hvterm_privs[0] = &hvterm_priv0; /* Check the protocol */ - if (of_device_is_compatible(stdout_node, "hvterm1")) { + if (of_device_is_compatible(of_stdout, "hvterm1")) { hvterm_priv0.proto = HV_PROTOCOL_RAW; ops = &hvterm_raw_ops; } - else if (of_device_is_compatible(stdout_node, "hvterm-protocol")) { + else if (of_device_is_compatible(of_stdout, "hvterm-protocol")) { hvterm_priv0.proto = HV_PROTOCOL_HVSI; ops = &hvterm_hvsi_ops; hvsilib_init(&hvterm_priv0.hvsi, hvc_get_chars, hvc_put_chars, @@ -447,7 +440,7 @@ void __init hvc_vio_init_early(void) /* HVSI, perform the handshake now */ hvsilib_establish(&hvterm_priv0.hvsi); } else - goto out; + return; udbg_putc = udbg_hvc_putc; udbg_getc = udbg_hvc_getc; udbg_getc_poll = udbg_hvc_getc_poll; @@ -456,14 +449,12 @@ void __init hvc_vio_init_early(void) * backend for HVSI, only do udbg */ if (hvterm_priv0.proto == HV_PROTOCOL_HVSI) - goto out; + return; #endif /* Check whether the user has requested a different console. */ if (!strstr(cmd_line, "console=")) add_preferred_console("hvc", 0, NULL); hvc_instantiate(0, 0, ops); -out: - of_node_put(stdout_node); } /* call this from early_init() for a working debug console on diff --git a/include/linux/of.h b/include/linux/of.h index 417945ebd8e1..1df918ee87d0 100644 --- a/include/linux/of.h +++ b/include/linux/of.h @@ -113,6 +113,7 @@ static inline void of_node_put(struct device_node *node) { } extern struct device_node *of_allnodes; extern struct device_node *of_chosen; extern struct device_node *of_aliases; +extern struct device_node *of_stdout; extern raw_spinlock_t devtree_lock; static inline bool of_have_populated_dt(void)
The evh_bytechan, hvc_opal and hvc_vio drivers all open code the parsing of the stdout node in the device tree. This patch simplifies the driver by removing the duplicated functionality. Signed-off-by: Grant Likely <grant.likely@linaro.org> --- drivers/of/base.c | 5 ++++- drivers/tty/ehv_bytechan.c | 43 ++++--------------------------------------- drivers/tty/hvc/hvc_opal.c | 14 +++----------- drivers/tty/hvc/hvc_vio.c | 29 ++++++++++------------------- include/linux/of.h | 1 + 5 files changed, 22 insertions(+), 70 deletions(-)