| Message ID | 1370274671-23812-1-git-send-email-pranavkumar@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On 3 June 2013 21:21, Pranavkumar Sawargaonkar <pranavkumar@linaro.org> wrote: > This patch adds support for defining and passing earlyprintk > related information i.e. device and address information via > device tree by adding it inside "chosen" node. > > This will help user to just specify "earlyprintk" from bootargs > without actually knowing the address and device to enable > earlyprintk. > > Mechanism: > > One can just append earlyprintk=device-type,address (same as we pass > through command line) in "/chosen" node to notify kernel which is the > earlyprintk device and what is its address. > > Backward Compatibility: > > This patch also allows existing method of specifying earlyprintk > parameter via bootargs. > > Existing method i.e. passing via bootargs will still have precedence > over device tree i.e. if one specifies earlyprintk=device-type,address > in bootargs then kernel will use information from bootargs instead of > device tree. > > If user just specifies earlyprintk (without =...) then kernel will > look for device tree earlyprintk parameter. > > Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org> > Signed-off-by: Anup Patel <anup.patel@linaro.org> > --- > arch/arm64/kernel/early_printk.c | 7 +++++++ > arch/arm64/kernel/setup.c | 21 ++++++++++++++++++++- > 2 files changed, 27 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/kernel/early_printk.c b/arch/arm64/kernel/early_printk.c > index fbb6e18..4e6f845 100644 > --- a/arch/arm64/kernel/early_printk.c > +++ b/arch/arm64/kernel/early_printk.c > @@ -29,6 +29,8 @@ > static void __iomem *early_base; > static void (*printch)(char ch); > > +extern char *earlyprintk_dt_args; > + > /* > * PL011 single character TX. > */ > @@ -116,6 +118,11 @@ static int __init setup_early_printk(char *buf) > phys_addr_t paddr = 0; > > if (!buf) { > + /* Try to check if Device Tree has this argument or not ? */ > + buf = earlyprintk_dt_args; > + } > + > + if (!buf) { > pr_warning("No earlyprintk arguments passed.\n"); > return 0; > } > diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c > index 6a9a532..fb2d56f 100644 > --- a/arch/arm64/kernel/setup.c > +++ b/arch/arm64/kernel/setup.c > @@ -60,6 +60,8 @@ EXPORT_SYMBOL(processor_id); > unsigned int elf_hwcap __read_mostly; > EXPORT_SYMBOL_GPL(elf_hwcap); > > +char *earlyprintk_dt_args; > + > static const char *cpu_name; > static const char *machine_name; > phys_addr_t __fdt_pointer __initdata; > @@ -122,6 +124,23 @@ static void __init setup_processor(void) > elf_hwcap = 0; > } > > +int __init early_init_dt_scan_chosen_arm64(unsigned long node, > + const char *uname, > + int depth, void *data) > +{ > + char *prop; > + > + /* Check if this is chosen node */ > + if (early_init_dt_scan_chosen(node, uname, depth, data) == 0) > + return 0; > + > + prop = of_get_flat_dt_prop(node, "earlyprintk", NULL); > + if (prop) > + earlyprintk_dt_args = prop; > + > + return 1; > +} > + > static void __init setup_machine_fdt(phys_addr_t dt_phys) > { > struct boot_param_header *devtree; > @@ -165,7 +184,7 @@ static void __init setup_machine_fdt(phys_addr_t dt_phys) > pr_info("Machine: %s\n", machine_name); > > /* Retrieve various information from the /chosen node */ > - of_scan_flat_dt(early_init_dt_scan_chosen, boot_command_line); > + of_scan_flat_dt(early_init_dt_scan_chosen_arm64, boot_command_line); > /* Initialize {size,address}-cells info */ > of_scan_flat_dt(early_init_dt_scan_root, NULL); > /* Setup memory, calling early_init_dt_add_memory_arch */ > -- > 1.7.9.5 > Ccing to devicetree-discuss@lists.ozlabs.org for comments. Original RFC posted for this patch is: https://patchwork.kernel.org/patch/2601411/ Thanks, Pranav
On Mon, 3 Jun 2013 21:21:11 +0530, Pranavkumar Sawargaonkar <pranavkumar@linaro.org> wrote: > This patch adds support for defining and passing earlyprintk > related information i.e. device and address information via > device tree by adding it inside "chosen" node. > > This will help user to just specify "earlyprintk" from bootargs > without actually knowing the address and device to enable > earlyprintk. > > Mechanism: > > One can just append earlyprintk=device-type,address (same as we pass > through command line) in "/chosen" node to notify kernel which is the > earlyprintk device and what is its address. > > Backward Compatibility: > > This patch also allows existing method of specifying earlyprintk > parameter via bootargs. > > Existing method i.e. passing via bootargs will still have precedence > over device tree i.e. if one specifies earlyprintk=device-type,address > in bootargs then kernel will use information from bootargs instead of > device tree. > > If user just specifies earlyprintk (without =...) then kernel will > look for device tree earlyprintk parameter. > > Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org> > Signed-off-by: Anup Patel <anup.patel@linaro.org> I'm not a big fan of this. It seems to be short-circuiting around existing properties. The kernel /should/ be able to use the linux,stdout-path property to determine what the earlyprintk device to use is. g. > --- > arch/arm64/kernel/early_printk.c | 7 +++++++ > arch/arm64/kernel/setup.c | 21 ++++++++++++++++++++- > 2 files changed, 27 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/kernel/early_printk.c b/arch/arm64/kernel/early_printk.c > index fbb6e18..4e6f845 100644 > --- a/arch/arm64/kernel/early_printk.c > +++ b/arch/arm64/kernel/early_printk.c > @@ -29,6 +29,8 @@ > static void __iomem *early_base; > static void (*printch)(char ch); > > +extern char *earlyprintk_dt_args; > + > /* > * PL011 single character TX. > */ > @@ -116,6 +118,11 @@ static int __init setup_early_printk(char *buf) > phys_addr_t paddr = 0; > > if (!buf) { > + /* Try to check if Device Tree has this argument or not ? */ > + buf = earlyprintk_dt_args; > + } > + > + if (!buf) { > pr_warning("No earlyprintk arguments passed.\n"); > return 0; > } > diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c > index 6a9a532..fb2d56f 100644 > --- a/arch/arm64/kernel/setup.c > +++ b/arch/arm64/kernel/setup.c > @@ -60,6 +60,8 @@ EXPORT_SYMBOL(processor_id); > unsigned int elf_hwcap __read_mostly; > EXPORT_SYMBOL_GPL(elf_hwcap); > > +char *earlyprintk_dt_args; > + > static const char *cpu_name; > static const char *machine_name; > phys_addr_t __fdt_pointer __initdata; > @@ -122,6 +124,23 @@ static void __init setup_processor(void) > elf_hwcap = 0; > } > > +int __init early_init_dt_scan_chosen_arm64(unsigned long node, > + const char *uname, > + int depth, void *data) > +{ > + char *prop; > + > + /* Check if this is chosen node */ > + if (early_init_dt_scan_chosen(node, uname, depth, data) == 0) > + return 0; > + > + prop = of_get_flat_dt_prop(node, "earlyprintk", NULL); > + if (prop) > + earlyprintk_dt_args = prop; > + > + return 1; > +} > + > static void __init setup_machine_fdt(phys_addr_t dt_phys) > { > struct boot_param_header *devtree; > @@ -165,7 +184,7 @@ static void __init setup_machine_fdt(phys_addr_t dt_phys) > pr_info("Machine: %s\n", machine_name); > > /* Retrieve various information from the /chosen node */ > - of_scan_flat_dt(early_init_dt_scan_chosen, boot_command_line); > + of_scan_flat_dt(early_init_dt_scan_chosen_arm64, boot_command_line); > /* Initialize {size,address}-cells info */ > of_scan_flat_dt(early_init_dt_scan_root, NULL); > /* Setup memory, calling early_init_dt_add_memory_arch */ > -- > 1.7.9.5 > > -- > 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/
Hi Grant, On 12 June 2013 18:58, Grant Likely <grant.likely@secretlab.ca> wrote: > On Mon, 3 Jun 2013 21:21:11 +0530, Pranavkumar Sawargaonkar <pranavkumar@linaro.org> wrote: >> This patch adds support for defining and passing earlyprintk >> related information i.e. device and address information via >> device tree by adding it inside "chosen" node. >> >> This will help user to just specify "earlyprintk" from bootargs >> without actually knowing the address and device to enable >> earlyprintk. >> >> Mechanism: >> >> One can just append earlyprintk=device-type,address (same as we pass >> through command line) in "/chosen" node to notify kernel which is the >> earlyprintk device and what is its address. >> >> Backward Compatibility: >> >> This patch also allows existing method of specifying earlyprintk >> parameter via bootargs. >> >> Existing method i.e. passing via bootargs will still have precedence >> over device tree i.e. if one specifies earlyprintk=device-type,address >> in bootargs then kernel will use information from bootargs instead of >> device tree. >> >> If user just specifies earlyprintk (without =...) then kernel will >> look for device tree earlyprintk parameter. >> >> Signed-off-by: Pranavkumar Sawargaonkar <pranavkumar@linaro.org> >> Signed-off-by: Anup Patel <anup.patel@linaro.org> > > I'm not a big fan of this. It seems to be short-circuiting around > existing properties. Agreed, but intention was to have a simple solution since it is just to be used for early printing/debugging. and also keep that in sync with existing method of specifying earlyprintk from command line. The kernel /should/ be able to use the > linux,stdout-path property to determine what the earlyprintk device to > use is. For this there are two problems: 1. Early printk code gets initialized before un-flattening of a device tree. Hence trying to find out node from stdout-path is tricky as we do not have of_find_node_by_path available. 2. Current compatible strings in arm64 early printk code are not in synced (or different) from actual compatible strings used in drivers - e.g. for PL011 In earlyprintk code match name is just pl011 but in dts it is specified as "arm,pl011" Hence we will need multiple changes to implement it. > > g. Thanks, Pranav > >> --- >> arch/arm64/kernel/early_printk.c | 7 +++++++ >> arch/arm64/kernel/setup.c | 21 ++++++++++++++++++++- >> 2 files changed, 27 insertions(+), 1 deletion(-) >> >> diff --git a/arch/arm64/kernel/early_printk.c b/arch/arm64/kernel/early_printk.c >> index fbb6e18..4e6f845 100644 >> --- a/arch/arm64/kernel/early_printk.c >> +++ b/arch/arm64/kernel/early_printk.c >> @@ -29,6 +29,8 @@ >> static void __iomem *early_base; >> static void (*printch)(char ch); >> >> +extern char *earlyprintk_dt_args; >> + >> /* >> * PL011 single character TX. >> */ >> @@ -116,6 +118,11 @@ static int __init setup_early_printk(char *buf) >> phys_addr_t paddr = 0; >> >> if (!buf) { >> + /* Try to check if Device Tree has this argument or not ? */ >> + buf = earlyprintk_dt_args; >> + } >> + >> + if (!buf) { >> pr_warning("No earlyprintk arguments passed.\n"); >> return 0; >> } >> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c >> index 6a9a532..fb2d56f 100644 >> --- a/arch/arm64/kernel/setup.c >> +++ b/arch/arm64/kernel/setup.c >> @@ -60,6 +60,8 @@ EXPORT_SYMBOL(processor_id); >> unsigned int elf_hwcap __read_mostly; >> EXPORT_SYMBOL_GPL(elf_hwcap); >> >> +char *earlyprintk_dt_args; >> + >> static const char *cpu_name; >> static const char *machine_name; >> phys_addr_t __fdt_pointer __initdata; >> @@ -122,6 +124,23 @@ static void __init setup_processor(void) >> elf_hwcap = 0; >> } >> >> +int __init early_init_dt_scan_chosen_arm64(unsigned long node, >> + const char *uname, >> + int depth, void *data) >> +{ >> + char *prop; >> + >> + /* Check if this is chosen node */ >> + if (early_init_dt_scan_chosen(node, uname, depth, data) == 0) >> + return 0; >> + >> + prop = of_get_flat_dt_prop(node, "earlyprintk", NULL); >> + if (prop) >> + earlyprintk_dt_args = prop; >> + >> + return 1; >> +} >> + >> static void __init setup_machine_fdt(phys_addr_t dt_phys) >> { >> struct boot_param_header *devtree; >> @@ -165,7 +184,7 @@ static void __init setup_machine_fdt(phys_addr_t dt_phys) >> pr_info("Machine: %s\n", machine_name); >> >> /* Retrieve various information from the /chosen node */ >> - of_scan_flat_dt(early_init_dt_scan_chosen, boot_command_line); >> + of_scan_flat_dt(early_init_dt_scan_chosen_arm64, boot_command_line); >> /* Initialize {size,address}-cells info */ >> of_scan_flat_dt(early_init_dt_scan_root, NULL); >> /* Setup memory, calling early_init_dt_add_memory_arch */ >> -- >> 1.7.9.5 >> >> -- >> 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/ > > -- > Grant Likely, B.Sc, P.Eng. > Secret Lab Technologies, Ltd.
On Thu, Jun 13, 2013 at 07:25:20AM +0100, Pranavkumar Sawargaonkar wrote: > On 12 June 2013 18:58, Grant Likely <grant.likely@secretlab.ca> wrote: > > On Mon, 3 Jun 2013 21:21:11 +0530, Pranavkumar Sawargaonkar > > <pranavkumar@linaro.org> wrote: > >> One can just append earlyprintk=device-type,address (same as we pass > >> through command line) in "/chosen" node to notify kernel which is the > >> earlyprintk device and what is its address. > > > > I'm not a big fan of this. It seems to be short-circuiting around > > existing properties. The kernel /should/ be able to use the > > linux,stdout-path property to determine what the earlyprintk device > > to use is. > > For this there are two problems: > > 1. Early printk code gets initialized before un-flattening of a device tree. > Hence trying to find out node from stdout-path is tricky as we do not have > of_find_node_by_path available. Looking at the existing uses of linux,stdout-path, it seems that it is pointed at an existing entry like &uart0, which cannot be parsed early enough. The base address is the main problem as it needs the DT to be unflattened (for example v2m_serial0 on arm64 would read as 0x90000 which is just an offset). If you pass the full path, of_find_node_by_path() wouldn't work either this early. Question for the DT guys - would it be feasible to pass a @<phys address> via the linux,stdout-path? Any other way to get the phys address of the device early during boot? > 2. Current compatible strings in arm64 early printk code are not in > synced (or different) from actual compatible strings used in drivers - > e.g. for PL011 In earlyprintk code match name is just pl011 but in dts > it is specified as "arm,pl011" Hence we will need multiple changes to > implement it. I think we can sort this out. The first point is more important.
diff --git a/arch/arm64/kernel/early_printk.c b/arch/arm64/kernel/early_printk.c index fbb6e18..4e6f845 100644 --- a/arch/arm64/kernel/early_printk.c +++ b/arch/arm64/kernel/early_printk.c @@ -29,6 +29,8 @@ static void __iomem *early_base; static void (*printch)(char ch); +extern char *earlyprintk_dt_args; + /* * PL011 single character TX. */ @@ -116,6 +118,11 @@ static int __init setup_early_printk(char *buf) phys_addr_t paddr = 0; if (!buf) { + /* Try to check if Device Tree has this argument or not ? */ + buf = earlyprintk_dt_args; + } + + if (!buf) { pr_warning("No earlyprintk arguments passed.\n"); return 0; } diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c index 6a9a532..fb2d56f 100644 --- a/arch/arm64/kernel/setup.c +++ b/arch/arm64/kernel/setup.c @@ -60,6 +60,8 @@ EXPORT_SYMBOL(processor_id); unsigned int elf_hwcap __read_mostly; EXPORT_SYMBOL_GPL(elf_hwcap); +char *earlyprintk_dt_args; + static const char *cpu_name; static const char *machine_name; phys_addr_t __fdt_pointer __initdata; @@ -122,6 +124,23 @@ static void __init setup_processor(void) elf_hwcap = 0; } +int __init early_init_dt_scan_chosen_arm64(unsigned long node, + const char *uname, + int depth, void *data) +{ + char *prop; + + /* Check if this is chosen node */ + if (early_init_dt_scan_chosen(node, uname, depth, data) == 0) + return 0; + + prop = of_get_flat_dt_prop(node, "earlyprintk", NULL); + if (prop) + earlyprintk_dt_args = prop; + + return 1; +} + static void __init setup_machine_fdt(phys_addr_t dt_phys) { struct boot_param_header *devtree; @@ -165,7 +184,7 @@ static void __init setup_machine_fdt(phys_addr_t dt_phys) pr_info("Machine: %s\n", machine_name); /* Retrieve various information from the /chosen node */ - of_scan_flat_dt(early_init_dt_scan_chosen, boot_command_line); + of_scan_flat_dt(early_init_dt_scan_chosen_arm64, boot_command_line); /* Initialize {size,address}-cells info */ of_scan_flat_dt(early_init_dt_scan_root, NULL); /* Setup memory, calling early_init_dt_add_memory_arch */