[PATCH] commands: oftree: kill oftree -f

[PATCH] commands: oftree: kill oftree -f

[Sascha Hauer]

Being able to free the live device tree seemed like a good idea in the
early days of device tree support when the first boards were brought up.
It turned out to be illusory that noone stores pointers to the live tree
which become invalid when oftree -f is executed. Enough people stumbled
upon crashed boards with this option, so remove it finally.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 commands/oftree.c | 17 +----------------
 1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/commands/oftree.c b/commands/oftree.c
index 8a47c0be58..26a47bb581 100644
--- a/commands/oftree.c
+++ b/commands/oftree.c
@@ -48,7 +48,6 @@ static int do_oftree(int argc, char *argv[])
 	int probe = 0;
 	char *load = NULL;
 	char *save = NULL;
-	int free_of = 0;
 	int ret;
 	struct device_node *root;
 
@@ -65,25 +64,12 @@ static int do_oftree(int argc, char *argv[])
 				return COMMAND_ERROR_USAGE;
 			}
 			break;
-		case 'f':
-			free_of = 1;
-			break;
 		case 's':
 			save = optarg;
 			break;
 		}
 	}
 
-	if (free_of) {
-		struct device_node *root = of_get_root_node();
-
-		if (root)
-			of_delete_node(root);
-
-		if (!load)
-			return 0;
-	}
-
 	if (!probe && !load && !save)
 		return COMMAND_ERROR_USAGE;
 
@@ -140,13 +126,12 @@ BAREBOX_CMD_HELP_TEXT("Options:")
 BAREBOX_CMD_HELP_OPT ("-l <DTB>",  "Load <DTB> to internal devicetree\n")
 BAREBOX_CMD_HELP_OPT ("-s <DTB>",  "save internal devicetree to <DTB>\n")
 BAREBOX_CMD_HELP_OPT ("-p",  "probe devices from stored device tree")
-BAREBOX_CMD_HELP_OPT ("-f",  "free stored device tree")
 BAREBOX_CMD_HELP_END
 
 BAREBOX_CMD_START(oftree)
 	.cmd		= do_oftree,
 	BAREBOX_CMD_DESC("handle device trees")
-	BAREBOX_CMD_OPTS("[-lspf]")
+	BAREBOX_CMD_OPTS("[-lsp]")
 	BAREBOX_CMD_GROUP(CMD_GRP_MISC)
 	BAREBOX_CMD_HELP(cmd_oftree_help)
 BAREBOX_CMD_END
-- 
2.17.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] commands: oftree: kill oftree -f

[Alexander Kurz]

Hi Sascha,
On Mon, 11 Jun 2018, Sascha Hauer wrote:

> Being able to free the live device tree seemed like a good idea in the
> early days of device tree support when the first boards were brought up.
> It turned out to be illusory that noone stores pointers to the live tree
> which become invalid when oftree -f is executed. Enough people stumbled
> upon crashed boards with this option, so remove it finally.

oftree -f is still in use in boot scripts, e.g. for the kindle-mx50,
and at least one other board.
The usecase for the kindle-mx50 is
* DT based barebox
* ATAGs dependent antiquated, non upgradeable kernel

start_linux() (arch/arm/lib32/armlinux.c) wont do setup_tags() if there
still exists an oftree.
If there is an alternative way to make boot issue 
ATAGs, removing the -f option would be fine.

May I mention an old (rejected) proposal about this issue
http://lists.infradead.org/pipermail/barebox/2016-December/028633.html

Also note: it's still in Documentation/user/devicetree.rst

Regards, Alexander

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] commands: oftree: kill oftree -f

[Sascha Hauer]

On Mon, Jun 11, 2018 at 06:35:48PM +0000, Alexander Kurz wrote:
> Hi Sascha,
> On Mon, 11 Jun 2018, Sascha Hauer wrote:
> 
> > Being able to free the live device tree seemed like a good idea in the
> > early days of device tree support when the first boards were brought up.
> > It turned out to be illusory that noone stores pointers to the live tree
> > which become invalid when oftree -f is executed. Enough people stumbled
> > upon crashed boards with this option, so remove it finally.
> 
> oftree -f is still in use in boot scripts, e.g. for the kindle-mx50,
> and at least one other board.
> The usecase for the kindle-mx50 is
> * DT based barebox
> * ATAGs dependent antiquated, non upgradeable kernel
> 
> start_linux() (arch/arm/lib32/armlinux.c) wont do setup_tags() if there
> still exists an oftree.
> If there is an alternative way to make boot issue 
> ATAGs, removing the -f option would be fine.
> 
> May I mention an old (rejected) proposal about this issue
> http://lists.infradead.org/pipermail/barebox/2016-December/028633.html
> 
> Also note: it's still in Documentation/user/devicetree.rst

Thanks for noting. I was a bit enthusiastic that I finally found the
energy to kill this option and forgot why I haven't done it earlier ;)
I just sent an updated series which should solve the issue of being able
to do a ATAG boot.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox