* [PATCH] ARM: integrator: fix OF-related regression
@ 2014-06-24 12:08 Linus Walleij
2014-06-25 13:06 ` Rob Herring
0 siblings, 1 reply; 9+ messages in thread
From: Linus Walleij @ 2014-06-24 12:08 UTC (permalink / raw)
To: linux-arm-kernel
Commit 07e461cd7e73a84f0e3757932b93cc80976fd749
"of: Ensure unique names without sacrificing determinism"
caused a boot failure regression on the Integrator machines.
The problem is probably caused by fiddling too much with
the device tree population in the OF init function, such
as passing the SoC bus device as parent when populating
the device tree.
This patch fixes the problem by:
- Avoiding to explicitly look up the tree root
- Look up devices needed before device population from
the match only, passing NULL as root
- Passing NULL as root and parent when calling
of_platform_populate()
After this the Integrators boot again. Tested on
Integrator/AP and Integrator/CP.
Cc: Grant Likely <grant.likely@linaro.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ARM SoC maintainers: if you're happy with this fix, please
apply it directly for fixes in the ARM SoC tree, thanks.
---
arch/arm/mach-integrator/integrator_ap.c | 26 +++++++-------------------
arch/arm/mach-integrator/integrator_cp.c | 23 ++++++-----------------
2 files changed, 13 insertions(+), 36 deletions(-)
diff --git a/arch/arm/mach-integrator/integrator_ap.c b/arch/arm/mach-integrator/integrator_ap.c
index dd0cc677d596..660ca6feff40 100644
--- a/arch/arm/mach-integrator/integrator_ap.c
+++ b/arch/arm/mach-integrator/integrator_ap.c
@@ -480,25 +480,18 @@ static const struct of_device_id ebi_match[] = {
static void __init ap_init_of(void)
{
unsigned long sc_dec;
- struct device_node *root;
struct device_node *syscon;
struct device_node *ebi;
struct device *parent;
struct soc_device *soc_dev;
struct soc_device_attribute *soc_dev_attr;
u32 ap_sc_id;
- int err;
int i;
- /* Here we create an SoC device for the root node */
- root = of_find_node_by_path("/");
- if (!root)
- return;
-
- syscon = of_find_matching_node(root, ap_syscon_match);
+ syscon = of_find_matching_node(NULL, ap_syscon_match);
if (!syscon)
return;
- ebi = of_find_matching_node(root, ebi_match);
+ ebi = of_find_matching_node(NULL, ebi_match);
if (!ebi)
return;
@@ -509,19 +502,17 @@ static void __init ap_init_of(void)
if (!ebi_base)
return;
+ of_platform_populate(NULL, of_default_bus_match_table,
+ ap_auxdata_lookup, NULL);
+
ap_sc_id = readl(ap_syscon_base);
soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
if (!soc_dev_attr)
return;
- err = of_property_read_string(root, "compatible",
- &soc_dev_attr->soc_id);
- if (err)
- return;
- err = of_property_read_string(root, "model", &soc_dev_attr->machine);
- if (err)
- return;
+ soc_dev_attr->soc_id = "XVC";
+ soc_dev_attr->machine = "Integrator/AP";
soc_dev_attr->family = "Integrator";
soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%c",
'A' + (ap_sc_id & 0x0f));
@@ -536,9 +527,6 @@ static void __init ap_init_of(void)
parent = soc_device_to_device(soc_dev);
integrator_init_sysfs(parent, ap_sc_id);
- of_platform_populate(root, of_default_bus_match_table,
- ap_auxdata_lookup, parent);
-
sc_dec = readl(ap_syscon_base + INTEGRATOR_SC_DEC_OFFSET);
for (i = 0; i < 4; i++) {
struct lm_device *lmdev;
diff --git a/arch/arm/mach-integrator/integrator_cp.c b/arch/arm/mach-integrator/integrator_cp.c
index a938242b0c95..0e57f8f820a5 100644
--- a/arch/arm/mach-integrator/integrator_cp.c
+++ b/arch/arm/mach-integrator/integrator_cp.c
@@ -279,20 +279,13 @@ static const struct of_device_id intcp_syscon_match[] = {
static void __init intcp_init_of(void)
{
- struct device_node *root;
struct device_node *cpcon;
struct device *parent;
struct soc_device *soc_dev;
struct soc_device_attribute *soc_dev_attr;
u32 intcp_sc_id;
- int err;
- /* Here we create an SoC device for the root node */
- root = of_find_node_by_path("/");
- if (!root)
- return;
-
- cpcon = of_find_matching_node(root, intcp_syscon_match);
+ cpcon = of_find_matching_node(NULL, intcp_syscon_match);
if (!cpcon)
return;
@@ -300,19 +293,17 @@ static void __init intcp_init_of(void)
if (!intcp_con_base)
return;
+ of_platform_populate(NULL, of_default_bus_match_table,
+ intcp_auxdata_lookup, NULL);
+
intcp_sc_id = readl(intcp_con_base);
soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
if (!soc_dev_attr)
return;
- err = of_property_read_string(root, "compatible",
- &soc_dev_attr->soc_id);
- if (err)
- return;
- err = of_property_read_string(root, "model", &soc_dev_attr->machine);
- if (err)
- return;
+ soc_dev_attr->soc_id = "XCV";
+ soc_dev_attr->machine = "Integrator/CP";
soc_dev_attr->family = "Integrator";
soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%c",
'A' + (intcp_sc_id & 0x0f));
@@ -326,8 +317,6 @@ static void __init intcp_init_of(void)
parent = soc_device_to_device(soc_dev);
integrator_init_sysfs(parent, intcp_sc_id);
- of_platform_populate(root, of_default_bus_match_table,
- intcp_auxdata_lookup, parent);
}
static const char * intcp_dt_board_compat[] = {
--
1.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH] ARM: integrator: fix OF-related regression
2014-06-24 12:08 [PATCH] ARM: integrator: fix OF-related regression Linus Walleij
@ 2014-06-25 13:06 ` Rob Herring
2014-06-26 12:15 ` Linus Walleij
0 siblings, 1 reply; 9+ messages in thread
From: Rob Herring @ 2014-06-25 13:06 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jun 24, 2014 at 7:08 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> Commit 07e461cd7e73a84f0e3757932b93cc80976fd749
> "of: Ensure unique names without sacrificing determinism"
> caused a boot failure regression on the Integrator machines.
>
> The problem is probably caused by fiddling too much with
> the device tree population in the OF init function, such
> as passing the SoC bus device as parent when populating
> the device tree.
>
> This patch fixes the problem by:
>
> - Avoiding to explicitly look up the tree root
> - Look up devices needed before device population from
> the match only, passing NULL as root
> - Passing NULL as root and parent when calling
> of_platform_populate()
Just curious, I don't see how this fixes booting for Integrator. Where
exactly does boot fail?
Rob
>
> After this the Integrators boot again. Tested on
> Integrator/AP and Integrator/CP.
>
> Cc: Grant Likely <grant.likely@linaro.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ARM SoC maintainers: if you're happy with this fix, please
> apply it directly for fixes in the ARM SoC tree, thanks.
> ---
> arch/arm/mach-integrator/integrator_ap.c | 26 +++++++-------------------
> arch/arm/mach-integrator/integrator_cp.c | 23 ++++++-----------------
> 2 files changed, 13 insertions(+), 36 deletions(-)
>
> diff --git a/arch/arm/mach-integrator/integrator_ap.c b/arch/arm/mach-integrator/integrator_ap.c
> index dd0cc677d596..660ca6feff40 100644
> --- a/arch/arm/mach-integrator/integrator_ap.c
> +++ b/arch/arm/mach-integrator/integrator_ap.c
> @@ -480,25 +480,18 @@ static const struct of_device_id ebi_match[] = {
> static void __init ap_init_of(void)
> {
> unsigned long sc_dec;
> - struct device_node *root;
> struct device_node *syscon;
> struct device_node *ebi;
> struct device *parent;
> struct soc_device *soc_dev;
> struct soc_device_attribute *soc_dev_attr;
> u32 ap_sc_id;
> - int err;
> int i;
>
> - /* Here we create an SoC device for the root node */
> - root = of_find_node_by_path("/");
> - if (!root)
> - return;
> -
> - syscon = of_find_matching_node(root, ap_syscon_match);
> + syscon = of_find_matching_node(NULL, ap_syscon_match);
> if (!syscon)
> return;
> - ebi = of_find_matching_node(root, ebi_match);
> + ebi = of_find_matching_node(NULL, ebi_match);
> if (!ebi)
> return;
>
> @@ -509,19 +502,17 @@ static void __init ap_init_of(void)
> if (!ebi_base)
> return;
>
> + of_platform_populate(NULL, of_default_bus_match_table,
> + ap_auxdata_lookup, NULL);
> +
> ap_sc_id = readl(ap_syscon_base);
>
> soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
> if (!soc_dev_attr)
> return;
>
> - err = of_property_read_string(root, "compatible",
> - &soc_dev_attr->soc_id);
> - if (err)
> - return;
> - err = of_property_read_string(root, "model", &soc_dev_attr->machine);
> - if (err)
> - return;
> + soc_dev_attr->soc_id = "XVC";
> + soc_dev_attr->machine = "Integrator/AP";
> soc_dev_attr->family = "Integrator";
> soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%c",
> 'A' + (ap_sc_id & 0x0f));
> @@ -536,9 +527,6 @@ static void __init ap_init_of(void)
> parent = soc_device_to_device(soc_dev);
> integrator_init_sysfs(parent, ap_sc_id);
>
> - of_platform_populate(root, of_default_bus_match_table,
> - ap_auxdata_lookup, parent);
> -
> sc_dec = readl(ap_syscon_base + INTEGRATOR_SC_DEC_OFFSET);
> for (i = 0; i < 4; i++) {
> struct lm_device *lmdev;
> diff --git a/arch/arm/mach-integrator/integrator_cp.c b/arch/arm/mach-integrator/integrator_cp.c
> index a938242b0c95..0e57f8f820a5 100644
> --- a/arch/arm/mach-integrator/integrator_cp.c
> +++ b/arch/arm/mach-integrator/integrator_cp.c
> @@ -279,20 +279,13 @@ static const struct of_device_id intcp_syscon_match[] = {
>
> static void __init intcp_init_of(void)
> {
> - struct device_node *root;
> struct device_node *cpcon;
> struct device *parent;
> struct soc_device *soc_dev;
> struct soc_device_attribute *soc_dev_attr;
> u32 intcp_sc_id;
> - int err;
>
> - /* Here we create an SoC device for the root node */
> - root = of_find_node_by_path("/");
> - if (!root)
> - return;
> -
> - cpcon = of_find_matching_node(root, intcp_syscon_match);
> + cpcon = of_find_matching_node(NULL, intcp_syscon_match);
> if (!cpcon)
> return;
>
> @@ -300,19 +293,17 @@ static void __init intcp_init_of(void)
> if (!intcp_con_base)
> return;
>
> + of_platform_populate(NULL, of_default_bus_match_table,
> + intcp_auxdata_lookup, NULL);
> +
> intcp_sc_id = readl(intcp_con_base);
>
> soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
> if (!soc_dev_attr)
> return;
>
> - err = of_property_read_string(root, "compatible",
> - &soc_dev_attr->soc_id);
> - if (err)
> - return;
> - err = of_property_read_string(root, "model", &soc_dev_attr->machine);
> - if (err)
> - return;
> + soc_dev_attr->soc_id = "XCV";
> + soc_dev_attr->machine = "Integrator/CP";
> soc_dev_attr->family = "Integrator";
> soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%c",
> 'A' + (intcp_sc_id & 0x0f));
> @@ -326,8 +317,6 @@ static void __init intcp_init_of(void)
>
> parent = soc_device_to_device(soc_dev);
> integrator_init_sysfs(parent, intcp_sc_id);
> - of_platform_populate(root, of_default_bus_match_table,
> - intcp_auxdata_lookup, parent);
> }
>
> static const char * intcp_dt_board_compat[] = {
> --
> 1.9.3
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH] ARM: integrator: fix OF-related regression
2014-06-25 13:06 ` Rob Herring
@ 2014-06-26 12:15 ` Linus Walleij
2014-06-26 19:39 ` Rabin Vincent
0 siblings, 1 reply; 9+ messages in thread
From: Linus Walleij @ 2014-06-26 12:15 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Jun 25, 2014 at 3:06 PM, Rob Herring <robherring2@gmail.com> wrote:
> On Tue, Jun 24, 2014 at 7:08 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
>> Commit 07e461cd7e73a84f0e3757932b93cc80976fd749
>> "of: Ensure unique names without sacrificing determinism"
>> caused a boot failure regression on the Integrator machines.
>>
>> The problem is probably caused by fiddling too much with
>> the device tree population in the OF init function, such
>> as passing the SoC bus device as parent when populating
>> the device tree.
>>
>> This patch fixes the problem by:
>>
>> - Avoiding to explicitly look up the tree root
>> - Look up devices needed before device population from
>> the match only, passing NULL as root
>> - Passing NULL as root and parent when calling
>> of_platform_populate()
>
> Just curious, I don't see how this fixes booting for Integrator. Where
> exactly does boot fail?
It fails by failing to populate the devicetree somehow. I don't
have the proper debugging needs, I just trial-and-horror-fixed
it by making the board init similar to other platforms :-/
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] ARM: integrator: fix OF-related regression
2014-06-26 12:15 ` Linus Walleij
@ 2014-06-26 19:39 ` Rabin Vincent
2014-06-26 21:31 ` Rob Herring
2014-06-30 16:52 ` Linus Walleij
0 siblings, 2 replies; 9+ messages in thread
From: Rabin Vincent @ 2014-06-26 19:39 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jun 26, 2014 at 02:15:42PM +0200, Linus Walleij wrote:
> On Wed, Jun 25, 2014 at 3:06 PM, Rob Herring <robherring2@gmail.com> wrote:
> > On Tue, Jun 24, 2014 at 7:08 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> >> Commit 07e461cd7e73a84f0e3757932b93cc80976fd749
> >> "of: Ensure unique names without sacrificing determinism"
> >> caused a boot failure regression on the Integrator machines.
> >>
> >> The problem is probably caused by fiddling too much with
> >> the device tree population in the OF init function, such
> >> as passing the SoC bus device as parent when populating
> >> the device tree.
> >>
> >> This patch fixes the problem by:
> >>
> >> - Avoiding to explicitly look up the tree root
> >> - Look up devices needed before device population from
> >> the match only, passing NULL as root
> >> - Passing NULL as root and parent when calling
> >> of_platform_populate()
> >
> > Just curious, I don't see how this fixes booting for Integrator. Where
> > exactly does boot fail?
>
> It fails by failing to populate the devicetree somehow. I don't
> have the proper debugging needs, I just trial-and-horror-fixed
> it by making the board init similar to other platforms :-/
earlyprintk doesn't work on Integrator hardware? It does on QEMU's
integratorcp so it's trivial to debug there.
The problem is that integrator_init_sysfs() adds a device attribute
called "fpga" to the soc0 bus, and the soc0 bus device is also used as
the root of the device tree, and the device tree has a node called "fpga".
Before 07e461cd7e73a84f0e3757932b93cc80976fd749 this node directory and
attribute file had unique paths:
sysfs: creating file '/devices/soc0/fpga'
sysfs: creating dir '/devices/soc0/fpga.0'
07e461cd7e73a84f0e3757932b93cc80976fd749 removed the .0 from the fpga node's
directory, leading to that node (and its subnodes) failing to register due to a
filename conflict with the fpga attribute's file:
------------[ cut here ]------------
WARNING: CPU: 0 PID: 1 at /home/rabin/dev/linux/fs/sysfs/dir.c:31 sysfs_warn_dup+0x54/0x74()
sysfs: cannot create duplicate filename '/devices/soc0/fpga'
Modules linked in:
CPU: 0 PID: 1 Comm: swapper Not tainted 3.15.0-rc5-00030-g244fb0a #17
[<c000ea34>] (unwind_backtrace) from [<c000c18c>] (show_stack+0x10/0x14)
[<c000c18c>] (show_stack) from [<c00193f0>] (warn_slowpath_common+0x64/0x84)
[<c00193f0>] (warn_slowpath_common) from [<c0019440>] (warn_slowpath_fmt+0x30/0x40)
[<c0019440>] (warn_slowpath_fmt) from [<c0104008>] (sysfs_warn_dup+0x54/0x74)
[<c0104008>] (sysfs_warn_dup) from [<c01040b0>] (sysfs_create_dir_ns+0x88/0x98)
[<c01040b0>] (sysfs_create_dir_ns) from [<c0188410>] (kobject_add_internal+0xac/0x2fc)
[<c0188410>] (kobject_add_internal) from [<c0188820>] (kobject_add+0x4c/0x98)
[<c0188820>] (kobject_add) from [<c01edacc>] (device_add+0xe0/0x4f0)
[<c01edacc>] (device_add) from [<c02559b0>] (of_platform_device_create_pdata+0x6c/0x8c)
[<c02559b0>] (of_platform_device_create_pdata) from [<c0255aac>] (of_platform_bus_create+0xd0/0x2b4)
[<c0255aac>] (of_platform_bus_create) from [<c0255db8>] (of_platform_populate+0x5c/0xa0)
[<c0255db8>] (of_platform_populate) from [<c0415fdc>] (customize_machine+0x20/0x44)
[<c0415fdc>] (customize_machine) from [<c0008b6c>] (do_one_initcall+0xfc/0x160)
[<c0008b6c>] (do_one_initcall) from [<c0413b40>] (kernel_init_freeable+0xf4/0x1ac)
[<c0413b40>] (kernel_init_freeable) from [<c0312b60>] (kernel_init+0x8/0xec)
[<c0312b60>] (kernel_init) from [<c0009830>] (ret_from_fork+0x14/0x24)
---[ end trace b6f453d64d825b4b ]---
------------[ cut here ]------------
WARNING: CPU: 0 PID: 1 at /home/rabin/dev/linux/lib/kobject.c:240 kobject_add_internal+0x274/0x2fc()
kobject_add_internal failed for fpga with -EEXIST, don't try to register things with the same name in the same directory.
Modules linked in:
CPU: 0 PID: 1 Comm: swapper Tainted: G W 3.15.0-rc5-00030-g244fb0a #17
[<c000ea34>] (unwind_backtrace) from [<c000c18c>] (show_stack+0x10/0x14)
[<c000c18c>] (show_stack) from [<c00193f0>] (warn_slowpath_common+0x64/0x84)
[<c00193f0>] (warn_slowpath_common) from [<c0019440>] (warn_slowpath_fmt+0x30/0x40)
[<c0019440>] (warn_slowpath_fmt) from [<c01885d8>] (kobject_add_internal+0x274/0x2fc)
[<c01885d8>] (kobject_add_internal) from [<c0188820>] (kobject_add+0x4c/0x98)
[<c0188820>] (kobject_add) from [<c01edacc>] (device_add+0xe0/0x4f0)
[<c01edacc>] (device_add) from [<c02559b0>] (of_platform_device_create_pdata+0x6c/0x8c)
[<c02559b0>] (of_platform_device_create_pdata) from [<c0255aac>] (of_platform_bus_create+0xd0/0x2b4)
[<c0255aac>] (of_platform_bus_create) from [<c0255db8>] (of_platform_populate+0x5c/0xa0)
[<c0255db8>] (of_platform_populate) from [<c0415fdc>] (customize_machine+0x20/0x44)
[<c0415fdc>] (customize_machine) from [<c0008b6c>] (do_one_initcall+0xfc/0x160)
[<c0008b6c>] (do_one_initcall) from [<c0413b40>] (kernel_init_freeable+0xf4/0x1ac)
[<c0413b40>] (kernel_init_freeable) from [<c0312b60>] (kernel_init+0x8/0xec)
[<c0312b60>] (kernel_init) from [<c0009830>] (ret_from_fork+0x14/0x24)
---[ end trace b6f453d64d825b4c ]---
After Linus' patch, the root is no longer soc0 so the fpga node's directory
becomes /devices/fpga while the attribute file remains at /devices/soc0/fpga.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] ARM: integrator: fix OF-related regression
2014-06-26 19:39 ` Rabin Vincent
@ 2014-06-26 21:31 ` Rob Herring
2014-06-27 12:14 ` Grant Likely
2014-06-30 16:52 ` Linus Walleij
1 sibling, 1 reply; 9+ messages in thread
From: Rob Herring @ 2014-06-26 21:31 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jun 26, 2014 at 2:39 PM, Rabin Vincent <rabin@rab.in> wrote:
> On Thu, Jun 26, 2014 at 02:15:42PM +0200, Linus Walleij wrote:
>> On Wed, Jun 25, 2014 at 3:06 PM, Rob Herring <robherring2@gmail.com> wrote:
>> > On Tue, Jun 24, 2014 at 7:08 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
>> >> Commit 07e461cd7e73a84f0e3757932b93cc80976fd749
>> >> "of: Ensure unique names without sacrificing determinism"
>> >> caused a boot failure regression on the Integrator machines.
>> >>
>> >> The problem is probably caused by fiddling too much with
>> >> the device tree population in the OF init function, such
>> >> as passing the SoC bus device as parent when populating
>> >> the device tree.
>> >>
>> >> This patch fixes the problem by:
>> >>
>> >> - Avoiding to explicitly look up the tree root
>> >> - Look up devices needed before device population from
>> >> the match only, passing NULL as root
>> >> - Passing NULL as root and parent when calling
>> >> of_platform_populate()
>> >
>> > Just curious, I don't see how this fixes booting for Integrator. Where
>> > exactly does boot fail?
>>
>> It fails by failing to populate the devicetree somehow. I don't
>> have the proper debugging needs, I just trial-and-horror-fixed
>> it by making the board init similar to other platforms :-/
>
> earlyprintk doesn't work on Integrator hardware? It does on QEMU's
> integratorcp so it's trivial to debug there.
I got as far as building a kernel to do that before getting distracted...
> The problem is that integrator_init_sysfs() adds a device attribute
> called "fpga" to the soc0 bus, and the soc0 bus device is also used as
> the root of the device tree, and the device tree has a node called "fpga".
>
> Before 07e461cd7e73a84f0e3757932b93cc80976fd749 this node directory and
> attribute file had unique paths:
>
> sysfs: creating file '/devices/soc0/fpga'
> sysfs: creating dir '/devices/soc0/fpga.0'
>
> 07e461cd7e73a84f0e3757932b93cc80976fd749 removed the .0 from the fpga node's
> directory, leading to that node (and its subnodes) failing to register due to a
> filename conflict with the fpga attribute's file:
Thanks for debugging. Now this makes sense as to why this is specific
to Integrator. I think this is the right direction and we should not
put devices under soc0/. That way the structure is consistent across
SOCs and regardless of whether you decide to create an soc_device or
not.
Rob
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] ARM: integrator: fix OF-related regression
2014-06-26 21:31 ` Rob Herring
@ 2014-06-27 12:14 ` Grant Likely
0 siblings, 0 replies; 9+ messages in thread
From: Grant Likely @ 2014-06-27 12:14 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 26 Jun 2014 16:31:28 -0500, Rob Herring <robherring2@gmail.com> wrote:
> On Thu, Jun 26, 2014 at 2:39 PM, Rabin Vincent <rabin@rab.in> wrote:
> > On Thu, Jun 26, 2014 at 02:15:42PM +0200, Linus Walleij wrote:
> >> On Wed, Jun 25, 2014 at 3:06 PM, Rob Herring <robherring2@gmail.com> wrote:
> >> > On Tue, Jun 24, 2014 at 7:08 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> >> >> Commit 07e461cd7e73a84f0e3757932b93cc80976fd749
> >> >> "of: Ensure unique names without sacrificing determinism"
> >> >> caused a boot failure regression on the Integrator machines.
> >> >>
> >> >> The problem is probably caused by fiddling too much with
> >> >> the device tree population in the OF init function, such
> >> >> as passing the SoC bus device as parent when populating
> >> >> the device tree.
> >> >>
> >> >> This patch fixes the problem by:
> >> >>
> >> >> - Avoiding to explicitly look up the tree root
> >> >> - Look up devices needed before device population from
> >> >> the match only, passing NULL as root
> >> >> - Passing NULL as root and parent when calling
> >> >> of_platform_populate()
> >> >
> >> > Just curious, I don't see how this fixes booting for Integrator. Where
> >> > exactly does boot fail?
> >>
> >> It fails by failing to populate the devicetree somehow. I don't
> >> have the proper debugging needs, I just trial-and-horror-fixed
> >> it by making the board init similar to other platforms :-/
> >
> > earlyprintk doesn't work on Integrator hardware? It does on QEMU's
> > integratorcp so it's trivial to debug there.
>
> I got as far as building a kernel to do that before getting distracted...
>
> > The problem is that integrator_init_sysfs() adds a device attribute
> > called "fpga" to the soc0 bus, and the soc0 bus device is also used as
> > the root of the device tree, and the device tree has a node called "fpga".
> >
> > Before 07e461cd7e73a84f0e3757932b93cc80976fd749 this node directory and
> > attribute file had unique paths:
> >
> > sysfs: creating file '/devices/soc0/fpga'
> > sysfs: creating dir '/devices/soc0/fpga.0'
> >
> > 07e461cd7e73a84f0e3757932b93cc80976fd749 removed the .0 from the fpga node's
> > directory, leading to that node (and its subnodes) failing to register due to a
> > filename conflict with the fpga attribute's file:
>
> Thanks for debugging. Now this makes sense as to why this is specific
> to Integrator. I think this is the right direction and we should not
> put devices under soc0/. That way the structure is consistent across
> SOCs and regardless of whether you decide to create an soc_device or
> not.
Agreed.
g.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] ARM: integrator: fix OF-related regression
2014-06-26 19:39 ` Rabin Vincent
2014-06-26 21:31 ` Rob Herring
@ 2014-06-30 16:52 ` Linus Walleij
2014-07-07 0:51 ` Olof Johansson
1 sibling, 1 reply; 9+ messages in thread
From: Linus Walleij @ 2014-06-30 16:52 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jun 26, 2014 at 9:39 PM, Rabin Vincent <rabin@rab.in> wrote:
> earlyprintk doesn't work on Integrator hardware?
It does, but loading the kernel is done over serial line 115200 bps
so iterative tests take "some time" :-)
> It does on QEMU's
> integratorcp so it's trivial to debug there.
Yeah I should be using QEMU for things like this ... poke self.
> The problem is that integrator_init_sysfs() adds a device attribute
> called "fpga" to the soc0 bus, and the soc0 bus device is also used as
> the root of the device tree, and the device tree has a node called "fpga".
(...)
> 07e461cd7e73a84f0e3757932b93cc80976fd749 removed the .0 from the fpga node's
> directory, leading to that node (and its subnodes) failing to register due to a
> filename conflict with the fpga attribute's file:
Aha! Thanks for root-causing this Rabin. Awesome.
> After Linus' patch, the root is no longer soc0 so the fpga node's directory
> becomes /devices/fpga while the attribute file remains at /devices/soc0/fpga.
It appears the patch is doing the right thing, though I had very vague
ideas as to why.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] ARM: integrator: fix OF-related regression
2014-06-30 16:52 ` Linus Walleij
@ 2014-07-07 0:51 ` Olof Johansson
2014-07-07 0:53 ` Olof Johansson
0 siblings, 1 reply; 9+ messages in thread
From: Olof Johansson @ 2014-07-07 0:51 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Jun 30, 2014 at 06:52:58PM +0200, Linus Walleij wrote:
> On Thu, Jun 26, 2014 at 9:39 PM, Rabin Vincent <rabin@rab.in> wrote:
>
> > earlyprintk doesn't work on Integrator hardware?
>
> It does, but loading the kernel is done over serial line 115200 bps
> so iterative tests take "some time" :-)
>
> > It does on QEMU's
> > integratorcp so it's trivial to debug there.
>
> Yeah I should be using QEMU for things like this ... poke self.
>
> > The problem is that integrator_init_sysfs() adds a device attribute
> > called "fpga" to the soc0 bus, and the soc0 bus device is also used as
> > the root of the device tree, and the device tree has a node called "fpga".
> (...)
> > 07e461cd7e73a84f0e3757932b93cc80976fd749 removed the .0 from the fpga node's
> > directory, leading to that node (and its subnodes) failing to register due to a
> > filename conflict with the fpga attribute's file:
>
> Aha! Thanks for root-causing this Rabin. Awesome.
>
> > After Linus' patch, the root is no longer soc0 so the fpga node's directory
> > becomes /devices/fpga while the attribute file remains at /devices/soc0/fpga.
>
> It appears the patch is doing the right thing, though I had very vague
> ideas as to why.
Ok, sounds like this is good to go. I'll apply it to the fixes branch, we can
fairly easily take it out right now if it needs to bake more so please speak up
quickly if that is the case.
-Olof
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] ARM: integrator: fix OF-related regression
2014-07-07 0:51 ` Olof Johansson
@ 2014-07-07 0:53 ` Olof Johansson
0 siblings, 0 replies; 9+ messages in thread
From: Olof Johansson @ 2014-07-07 0:53 UTC (permalink / raw)
To: linux-arm-kernel
On Sun, Jul 6, 2014 at 5:51 PM, Olof Johansson <olof@lixom.net> wrote:
> On Mon, Jun 30, 2014 at 06:52:58PM +0200, Linus Walleij wrote:
>> On Thu, Jun 26, 2014 at 9:39 PM, Rabin Vincent <rabin@rab.in> wrote:
>>
>> > earlyprintk doesn't work on Integrator hardware?
>>
>> It does, but loading the kernel is done over serial line 115200 bps
>> so iterative tests take "some time" :-)
>>
>> > It does on QEMU's
>> > integratorcp so it's trivial to debug there.
>>
>> Yeah I should be using QEMU for things like this ... poke self.
>>
>> > The problem is that integrator_init_sysfs() adds a device attribute
>> > called "fpga" to the soc0 bus, and the soc0 bus device is also used as
>> > the root of the device tree, and the device tree has a node called "fpga".
>> (...)
>> > 07e461cd7e73a84f0e3757932b93cc80976fd749 removed the .0 from the fpga node's
>> > directory, leading to that node (and its subnodes) failing to register due to a
>> > filename conflict with the fpga attribute's file:
>>
>> Aha! Thanks for root-causing this Rabin. Awesome.
>>
>> > After Linus' patch, the root is no longer soc0 so the fpga node's directory
>> > becomes /devices/fpga while the attribute file remains at /devices/soc0/fpga.
>>
>> It appears the patch is doing the right thing, though I had very vague
>> ideas as to why.
>
> Ok, sounds like this is good to go. I'll apply it to the fixes branch, we can
> fairly easily take it out right now if it needs to bake more so please speak up
> quickly if that is the case.
Hm, nevermind. Looks like Arnd silently applied it already, and it's
been sent up. Nothing left to do. :)
-Olof
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2014-07-07 0:53 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-06-24 12:08 [PATCH] ARM: integrator: fix OF-related regression Linus Walleij
2014-06-25 13:06 ` Rob Herring
2014-06-26 12:15 ` Linus Walleij
2014-06-26 19:39 ` Rabin Vincent
2014-06-26 21:31 ` Rob Herring
2014-06-27 12:14 ` Grant Likely
2014-06-30 16:52 ` Linus Walleij
2014-07-07 0:51 ` Olof Johansson
2014-07-07 0:53 ` Olof Johansson
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).