powernv: Search for new flash DT node location
PATCH 1/1 powernv: Search for new flash DT node location
Skiboot will place the flash device tree node at ibm,opal/flash/flash@0 on P9 and later systems, so Linux needs to search for it there as well as ibm,opal/flash@0 for backwards compatibility.
--- arch/powerpc/platforms/powernv/opal.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
arch/powerpc/platforms/powernv/opal.c +6 -1
@@ -755,9 +755,14 @@ static int __init opal_init(void)/* Initialize platform devices: IPMI backend, PRD & flash interface */ opal_pdev_init (opal_node,"ibm,opal-ipmi" ); -opal_pdev_init (opal_node,"ibm,opal-flash" ); +opal_pdev_init (opal_node,"ibm,opal-flash" );// old <= P8 flash location opal_pdev_init (opal_node,"ibm,opal-prd" ); +/* New >= P9 flash location */ + np =of_get_child_by_name (opal_node,"flash" ); +if (np) +opal_pdev_init (np,"ibm,opal-flash" ); +/* Initialise OPAL kmsg dumper for flushing console on panic */ opal_kmsg_init ();
Signature · 1 line
2.9.2
For reference, here's a link to the skiboot patch on the list
https://lists.ozlabs.org/pipermail/skiboot/2016-August/004274.html
Michael Ellerman
reviewerQuoting Jack Miller (2016-08-02 06:50:35)
arch/powerpc/platfor=
Quoted 12 lines
Skiboot will place the flash device tree node at ibm,opal/flash/flash@0 on P9 and later systems, so Linux needs to search for it there as well as ibm,opal/flash@0 for backwards compatibility.
=Signed-off-by Jack Miller <[email protected]>--- arch/powerpc/platforms/powernv/opal.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)=
arch/powerpc/platfor= renamed from arch/powerpc/platforms/powernv/opal.c +0
-0
ms/powernv/opal.c
arch/powerpc/platforms/powernv/opal.c static int __init opal_init(void) · line 757
Quoted 7 lines
index ae29eaf..2847cb0 100644
arch/powerpc/platforms/powernv/opal.c +0
-0@@ -755,9 +755,14 @@ static int __init opal_init(void) =/* Initialize platform devices: IPMI backend, PRD & flash interfa=
ce */
arch/powerpc/platforms/powernv/opal.c static int __init opal_init(void)
Quoted 3 lines
arch/powerpc/platforms/powernv/opal.c +1
-1opal_pdev_init (opal_node,"ibm,opal-ipmi" ); -opal_pdev_init (opal_node,"ibm,opal-flash" ); +opal_pdev_init (opal_node,"ibm,opal-flash" );// old <=3D P8 flash=
location
arch/powerpc/platforms/powernv/opal.c static int __init opal_init(void) · line 764
Quoted 7 lines
arch/powerpc/platforms/powernv/opal.c +4
-0opal_pdev_init (opal_node,"ibm,opal-prd" ); = +/* New >=3D P9 flash location */ + np =3D of_get_child_by_name(opal_node, "flash"); +if (np) +opal_pdev_init (np,"ibm,opal-flash" );
We could instead just search for all nodes that are compatible with "ibm,opal-flash". We do that for i2c, see opal_i2c_create_devs().
Is there a particular reason not to do that?
cheers
Quoted 4 lines · Michael Ellerman
We could instead just search for all nodes that are compatible with "ibm,opal-flash". We do that for i2c, see
opal_i2c_create_devs().Is there a particular reason not to do that?
I'm actually surprised that this is preferred. Jeremy mentioned something similar, but I guess I just don't like the idea of finding devices in weird places in the tree. Then again, if we can't trust the DT we're in bigger trouble than erroneous flash nodes =).
If we really just want to find compatible nodes anywhere, let's simplify i2c and pdev_init into one function and make that behavior consistent with this new patch.
- Jack
PATCH 1/1 powernv: Simplify searching for compatible device nodes
This condenses the opal node searching into a single function that finds all compatible nodes, instead of just searching the ibm,opal children, for ipmi, flash, and prd similar to how opal-i2c nodes are found.
--- arch/powerpc/platforms/powernv/opal.c | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-)
arch/powerpc/platforms/powernv/opal.c +6 -16
@@ -640,21 +640,11 @@ static void __init opal_dump_region_init(void) "rc = %d\n", rc); } -static void opal_pdev_init (struct device_node *opal_node, -const char *compatible) +static void opal_pdev_init (const char *compatible) {struct device_node *np; -for_each_child_of_node (opal_node, np) -if (of_device_is_compatible (np, compatible)) -of_platform_device_create (np,NULL ,NULL ); -} - -static void opal_i2c_create_devs (void ) -{ -struct device_node *np; - -for_each_compatible_node (np,NULL ,"ibm,opal-i2c" ) +for_each_compatible_node (np,NULL , compatible)of_platform_device_create (np,NULL ,NULL ); }
@@ -722,7 +712,7 @@ static int __init opal_init(void)opal_hmi_handler_init ();/* Create i2c platform devices */ -opal_i2c_create_devs (); +opal_pdev_init ("ibm,opal-i2c" );/* Setup a heatbeat thread if requested by OPAL */ opal_init_heartbeat ();
@@ -754,9 +744,9 @@ static int __init opal_init(void) }/* Initialize platform devices: IPMI backend, PRD & flash interface */ -opal_pdev_init (opal_node,"ibm,opal-ipmi" ); -opal_pdev_init (opal_node,"ibm,opal-flash" ); -opal_pdev_init (opal_node,"ibm,opal-prd" ); +opal_pdev_init ("ibm,opal-ipmi" ); +opal_pdev_init ("ibm,opal-flash" ); +opal_pdev_init ("ibm,opal-prd" );/* Initialise OPAL kmsg dumper for flushing console on panic */ opal_kmsg_init ();
Signature · 1 line
2.9.2
PATCH v2 1/1 powernv: Simplify searching for compatible device nodes
In reply to Jack Miller(rebased on powerpc/next)
This condenses the opal node searching into a single function that finds all compatible nodes, instead of just searching the ibm,opal children, for ipmi, flash, and prd similar to how opal-i2c nodes are found.
--- arch/powerpc/platforms/powernv/opal.c | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-)
arch/powerpc/platforms/powernv/opal.c +7 -17
@@ -631,21 +631,11 @@ static void __init opal_dump_region_init(void) "rc = %d\n", rc); } -static void opal_pdev_init (struct device_node *opal_node, -const char *compatible) +static void opal_pdev_init (const char *compatible) {struct device_node *np; -for_each_child_of_node (opal_node, np) -if (of_device_is_compatible (np, compatible)) -of_platform_device_create (np,NULL ,NULL ); -} - -static void opal_i2c_create_devs (void ) -{ -struct device_node *np; - -for_each_compatible_node (np,NULL ,"ibm,opal-i2c" ) +for_each_compatible_node (np,NULL , compatible)of_platform_device_create (np,NULL ,NULL ); }
@@ -717,7 +707,7 @@ static int __init opal_init(void)opal_hmi_handler_init ();/* Create i2c platform devices */ -opal_i2c_create_devs (); +opal_pdev_init ("ibm,opal-i2c" );/* Setup a heatbeat thread if requested by OPAL */ opal_init_heartbeat ();
@@ -752,12 +742,12 @@ static int __init opal_init(void) }/* Initialize platform devices: IPMI backend, PRD & flash interface */ -opal_pdev_init (opal_node,"ibm,opal-ipmi" ); -opal_pdev_init (opal_node,"ibm,opal-flash" ); -opal_pdev_init (opal_node,"ibm,opal-prd" ); +opal_pdev_init ("ibm,opal-ipmi" ); +opal_pdev_init ("ibm,opal-flash" ); +opal_pdev_init ("ibm,opal-prd" );/* Initialise platform device: oppanel interface */ -opal_pdev_init (opal_node,"ibm,opal-oppanel" ); +opal_pdev_init ("ibm,opal-oppanel" );/* Initialise OPAL kmsg dumper for flushing console on panic */ opal_kmsg_init ();
Signature · 1 line
2.9.2
Re: [PATCH v2] powernv: Simplify searching for compatible device nodes
Quoted 7 lines · On Wed, 3 Aug 2016 12:18:00 -0500 Jack Miller <[email protected]> wrote:
(rebased on powerpc/next)
This condenses the opal node searching into a single function that finds all compatible nodes, instead of just searching the ibm,opal children, for ipmi, flash, and prd similar to how opal-i2c nodes are found.
Signed-off-by Jack Miller <[email protected]>
Using a version of the related skiboot patch that may not be the final one:
Quoted 58 lines
--- arch/powerpc/platforms/powernv/opal.c | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-)arch/powerpc/platforms/powernv/opal.c +7
-17@@ -631,21 +631,11 @@ static void __init opal_dump_region_init(void) "rc = %d\n", rc); } -static void opal_pdev_init (struct device_node *opal_node, -const char *compatible) +static void opal_pdev_init (const char *compatible) {struct device_node *np; -for_each_child_of_node (opal_node, np) -if (of_device_is_compatible (np, compatible)) -of_platform_device_create (np,NULL ,NULL ); -} - -static void opal_i2c_create_devs (void ) -{ -struct device_node *np; - -for_each_compatible_node (np,NULL ,"ibm,opal-i2c" ) +for_each_compatible_node (np,NULL , compatible)of_platform_device_create (np,NULL ,NULL ); }@@ -717,7 +707,7 @@ static int __init opal_init(void)opal_hmi_handler_init ();/* Create i2c platform devices */ -opal_i2c_create_devs (); +opal_pdev_init ("ibm,opal-i2c" );/* Setup a heatbeat thread if requested by OPAL */ opal_init_heartbeat ();@@ -752,12 +742,12 @@ static int __init opal_init(void) }/* Initialize platform devices: IPMI backend, PRD & flash interface */ -opal_pdev_init (opal_node,"ibm,opal-ipmi" ); -opal_pdev_init (opal_node,"ibm,opal-flash" ); -opal_pdev_init (opal_node,"ibm,opal-prd" ); +opal_pdev_init ("ibm,opal-ipmi" ); +opal_pdev_init ("ibm,opal-flash" ); +opal_pdev_init ("ibm,opal-prd" );/* Initialise platform device: oppanel interface */ -opal_pdev_init (opal_node,"ibm,opal-oppanel" ); +opal_pdev_init ("ibm,opal-oppanel" );/* Initialise OPAL kmsg dumper for flushing console on panic */ opal_kmsg_init ();
Michael Ellerman
reviewerRe: [PATCH v2] powernv: Simplify searching for compatible device nodes
Quoted 5 lines · Cyril Bur
Quoted 2 lines · On Wed, 3 Aug 2016 12:18:00 -0500 Jack Miller <[email protected]> wrote:
(rebased on powerpc/next)
Quoted 8 lines
Quoted 5 lines
This condenses the opal node searching into a single function that finds all compatible nodes, instead of just searching the ibm,opal children, for ipmi, flash, and prd similar to how opal-i2c nodes are found.
Signed-off-by Jack Miller <[email protected]>Using a version of the related skiboot patch that may not be the final one:
Tested-by Cyril Bur <[email protected]>
Thanks. The part I'm still not clear on is *why* we're moving them in skiboot?
cheers
Re: [PATCH v2] powernv: Simplify searching for compatible device nodes
Quoted 11 lines · Michael Ellerman
Quoted 9 lines · Cyril Bur
Quoted 6 lines · On Wed, 3 Aug 2016 12:18:00 -0500 Jack Miller <[email protected]> wrote:
(rebased on powerpc/next)
This condenses the opal node searching into a single function that finds all compatible nodes, instead of just searching the ibm,opal children, for ipmi, flash, and prd similar to how opal-i2c nodes are found.
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
Signed-off-by Jack Miller <[email protected]>Using a version of the related skiboot patch that may not be the final one:
Tested-by Cyril Bur <[email protected]>Thanks. The part I'm still not clear on is *why* we're moving them in skiboot?
Ostensibly so the actual flash device nodes can inherit the #size-cells / #address-cells properties properly set in the flash parent node instead of the ibm,opal node (which has them set to 0). This would be more correct if anything actually started to honor these settings.
The only concrete effect though is stopping dtc (and thus fwts) from whinging when you run skiboot's output DT through it.
- Jack
Michael Ellerman
reviewerQuoted 9 lines · Jack Miller
Quoted 4 lines · Michael Ellerman
We could instead just search for all nodes that are compatible with "ibm,opal-flash". We do that for i2c, see
opal_i2c_create_devs().Is there a particular reason not to do that?
I'm actually surprised that this is preferred. Jeremy mentioned something similar, but I guess I just don't like the idea of finding devices in weird places in the tree.
But where is "weird". Arguably "/opal/flash" is weird. What does it mean? There's a bus called "opal" and a device on it called "flash"? No.
Point being the structure is fairly arbitrary, or at least debatable, so tying the code 100% to the structure is inflexible. As we have discovered.
Our other option is to tell skiboot to get stuffed, and leave the flash node where it was on P8.
Quoted 2 lines
Then again, if we can't trust the DT we're in bigger trouble than erroneous flash nodes =).
Quite :)
Quoted 3 lines
If we really just want to find compatible nodes anywhere, let's simplify i2c and
pdev_initinto one function and make that behavior consistent with this new patch.
That seems OK to me.
We should get an ack from Stewart though for the other node types.
cheers
Stewart Smith
Acked-byQuoted 26 lines · Michael Ellerman
Quoted 9 lines · Jack Miller
Quoted 4 lines · Michael Ellerman
We could instead just search for all nodes that are compatible with "ibm,opal-flash". We do that for i2c, see
opal_i2c_create_devs().Is there a particular reason not to do that?
I'm actually surprised that this is preferred. Jeremy mentioned something similar, but I guess I just don't like the idea of finding devices in weird places in the tree.
But where is "weird". Arguably "/opal/flash" is weird. What does it mean? There's a bus called "opal" and a device on it called "flash"? No.
Point being the structure is fairly arbitrary, or at least debatable, so tying the code 100% to the structure is inflexible. As we have discovered.
Our other option is to tell skiboot to get stuffed, and leave the flash node where it was on P8.
Quoted 2 lines
Then again, if we can't trust the DT we're in bigger trouble than erroneous flash nodes =).
Quite :)
Quoted 7 lines
Quoted 3 lines
If we really just want to find compatible nodes anywhere, let's simplify i2c and
pdev_initinto one function and make that behavior consistent with this new patch.That seems OK to me.
We should get an ack from Stewart though for the other node types.
For finding nodes based on compatible no matter where they are in the tree,
(and yes, includes other nodes too)
The exact location then isn't too important, and having a /flash that's ibm,opal-flash and allows for some other driver to bind to it I think is also something we shouldn't rule out.
Signature · 2 lines
Stewart Smith OPAL Architect, IBM.