2013-06-03 15:51:33

by Pranavkumar Sawargaonkar

[permalink] [raw]
Subject: [PATCH] arm64: Add support to pass earlyprintk argument via device tree

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 <[email protected]>
Signed-off-by: Anup Patel <[email protected]>
---
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


2013-06-10 10:43:29

by Pranavkumar Sawargaonkar

[permalink] [raw]
Subject: Re: [PATCH] arm64: Add support to pass earlyprintk argument via device tree

Hi,

On 3 June 2013 21:21, Pranavkumar Sawargaonkar <[email protected]> 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 <[email protected]>
> Signed-off-by: Anup Patel <[email protected]>
> ---
> 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 [email protected] for comments.
Original RFC posted for this patch is:
https://patchwork.kernel.org/patch/2601411/

Thanks,
Pranav

2013-06-12 13:28:27

by Grant Likely

[permalink] [raw]
Subject: Re: [PATCH] arm64: Add support to pass earlyprintk argument via device tree

On Mon, 3 Jun 2013 21:21:11 +0530, Pranavkumar Sawargaonkar <[email protected]> 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 <[email protected]>
> Signed-off-by: Anup Patel <[email protected]>

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 [email protected]
> 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.

2013-06-13 06:25:23

by Pranavkumar Sawargaonkar

[permalink] [raw]
Subject: Re: [PATCH] arm64: Add support to pass earlyprintk argument via device tree

Hi Grant,

On 12 June 2013 18:58, Grant Likely <[email protected]> wrote:
> On Mon, 3 Jun 2013 21:21:11 +0530, Pranavkumar Sawargaonkar <[email protected]> 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 <[email protected]>
>> Signed-off-by: Anup Patel <[email protected]>
>
> 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 [email protected]
>> 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.

2013-06-21 14:24:22

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH] arm64: Add support to pass earlyprintk argument via device tree

On Thu, Jun 13, 2013 at 07:25:20AM +0100, Pranavkumar Sawargaonkar wrote:
> On 12 June 2013 18:58, Grant Likely <[email protected]> wrote:
> > On Mon, 3 Jun 2013 21:21:11 +0530, Pranavkumar Sawargaonkar
> > <[email protected]> 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.

--
Catalin