[U-Boot] [PATCH 1/2] fdt: Allow indicating a node is for U-Boot proper only
This add missing parts for previous commit 06f94461a9f4 ("fdt: Allow indicating a node is for U-Boot proper only")
At present it is not possible to specify that a node should be used before relocation (in U-Boot proper) without it also ending up in SPL and TPL device trees. Add a new "u-boot,dm-pre-proper" boolean property for this.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com --- Hi,
I create this patch, after testing the new key "u-boot,dm-pre-proper" for ltdc VIDEO driver on stm32mp1 board. I try to replace the previous used "u-boot,dm-pre-reloc".
/* pre-reloc probe = reserve video frame buffer in video_reserve() */ <dc { - u-boot,dm-pre-reloc; + u-boot,dm-pre-proper; };
And it is failing.
NB: preloc support is only needed in video uclass to reserved the frame buffer(called by board_f), so the node for video u-class driver is only used in U-BOOT pre-relocation and not in SPL.
After a grep in code, I see that the initial patch don't add the support of the new "u-boot,dm-pre-proper" in all location where "u-boot,dm-pre-reloc" was tested.
When I add the needed test in util.c, it is working.
drivers/core/util.c | 4 ++++ drivers/video/video-uclass.c | 4 +++- include/dm/ofnode.h | 6 ++++-- include/dm/util.h | 12 ++++++++---- 4 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/core/util.c b/drivers/core/util.c index 27a6848..6bedc8a 100644 --- a/drivers/core/util.c +++ b/drivers/core/util.c @@ -35,6 +35,8 @@ bool dm_fdt_pre_reloc(const void *blob, int offset) { if (fdt_getprop(blob, offset, "u-boot,dm-pre-reloc", NULL)) return true; + if (fdt_getprop(blob, offset, "u-boot,dm-pre-proper", NULL)) + return true;
#ifdef CONFIG_TPL_BUILD if (fdt_getprop(blob, offset, "u-boot,dm-tpl", NULL)) @@ -59,6 +61,8 @@ bool dm_ofnode_pre_reloc(ofnode node) { if (ofnode_read_bool(node, "u-boot,dm-pre-reloc")) return true; + if (ofnode_read_bool(node, "u-boot,dm-pre-proper")) + return true;
#ifdef CONFIG_TPL_BUILD if (ofnode_read_bool(node, "u-boot,dm-tpl")) diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c index f307cf2..a9d40fe 100644 --- a/drivers/video/video-uclass.c +++ b/drivers/video/video-uclass.c @@ -290,7 +290,9 @@ static int video_post_bind(struct udevice *dev) return 0; size = alloc_fb(dev, &addr); if (addr < gd->video_bottom) { - /* Device tree node may need the 'u-boot,dm-pre-reloc' tag */ + /* Device tree node may need the 'u-boot,dm-pre-reloc' or + * 'u-boot,dm-pre-proper' tag + */ printf("Video device '%s' cannot allocate frame buffer memory -ensure the device is set up before relocation\n", dev->name); return -ENOSPC; diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h index d206ee2..b45da5e 100644 --- a/include/dm/ofnode.h +++ b/include/dm/ofnode.h @@ -662,12 +662,14 @@ int ofnode_read_simple_size_cells(ofnode node); * After relocation and jumping into the real U-Boot binary it is possible to * determine if a node was bound in one of SPL/TPL stages. * - * There are 3 settings currently in use - * - + * There are 4 settings currently in use + * - u-boot,dm-pre-proper: U-Boot proper pre-relocation only * - u-boot,dm-pre-reloc: legacy and indicates any of TPL or SPL * Existing platforms only use it to indicate nodes needed in * SPL. Should probably be replaced by u-boot,dm-spl for * new platforms. + * - u-boot,dm-spl: SPL and U-Boot pre-relocation + * - u-boot,dm-tpl: TPL and U-Boot pre-relocation * * @node: node to check * @return true if node is needed in SPL/TL, false otherwise diff --git a/include/dm/util.h b/include/dm/util.h index 9ff6531..cbc209d 100644 --- a/include/dm/util.h +++ b/include/dm/util.h @@ -52,12 +52,14 @@ static inline void dm_dump_devres(void) * it is possible to determine if a node was bound in one of * SPL/TPL stages. * - * There are 3 settings currently in use - * - + * There are 4 settings currently in use + * - u-boot,dm-pre-proper: U-Boot proper pre-relocation only * - u-boot,dm-pre-reloc: legacy and indicates any of TPL or SPL * Existing platforms only use it to indicate nodes needed in * SPL. Should probably be replaced by u-boot,dm-spl for * existing platforms. + * - u-boot,dm-spl: SPL and U-Boot pre-relocation + * - u-boot,dm-tpl: TPL and U-Boot pre-relocation * @blob: devicetree * @offset: node offset * @@ -78,12 +80,14 @@ bool dm_fdt_pre_reloc(const void *blob, int offset); * it is possible to determine if a node was bound in one of * SPL/TPL stages. * - * There are 3 settings currently in use - * - + * There are 4 settings currently in use + * - u-boot,dm-pre-proper: U-Boot proper pre-relocation only * - u-boot,dm-pre-reloc: legacy and indicates any of TPL or SPL * Existing platforms only use it to indicate nodes needed in * SPL. Should probably be replaced by u-boot,dm-spl for * existing platforms. + * - u-boot,dm-spl: SPL and U-Boot pre-relocation + * - u-boot,dm-tpl: TPL and U-Boot pre-relocation * @node: of node * * Returns true if node is needed in SPL/TL, false otherwise.
The content dm_ofnode_pre_reloc() is identical with ofnode_pre_reloc() defined in drivers/core/ofnode.c and used only one time in drivers/core/lists.c:lists_bind_fdt().
So the function can be removed and directly call ofnode_pre_reloc.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---
drivers/core/lists.c | 2 +- drivers/core/util.c | 26 -------------------------- include/dm/util.h | 27 --------------------------- 3 files changed, 1 insertion(+), 54 deletions(-)
diff --git a/drivers/core/lists.c b/drivers/core/lists.c index a1f8284..67edf0f 100644 --- a/drivers/core/lists.c +++ b/drivers/core/lists.c @@ -173,7 +173,7 @@ int lists_bind_fdt(struct udevice *parent, ofnode node, struct udevice **devp, continue;
if (pre_reloc_only) { - if (!dm_ofnode_pre_reloc(node) && + if (!ofnode_pre_reloc(node) && !(entry->flags & DM_FLAG_PRE_RELOC)) return 0; } diff --git a/drivers/core/util.c b/drivers/core/util.c index 6bedc8a..fa9ccbd 100644 --- a/drivers/core/util.c +++ b/drivers/core/util.c @@ -56,29 +56,3 @@ bool dm_fdt_pre_reloc(const void *blob, int offset)
return false; } - -bool dm_ofnode_pre_reloc(ofnode node) -{ - if (ofnode_read_bool(node, "u-boot,dm-pre-reloc")) - return true; - if (ofnode_read_bool(node, "u-boot,dm-pre-proper")) - return true; - -#ifdef CONFIG_TPL_BUILD - if (ofnode_read_bool(node, "u-boot,dm-tpl")) - return true; -#elif defined(CONFIG_SPL_BUILD) - if (ofnode_read_bool(node, "u-boot,dm-spl")) - return true; -#else - /* - * In regular builds individual spl and tpl handling both - * count as handled pre-relocation for later second init. - */ - if (ofnode_read_bool(node, "u-boot,dm-spl") || - ofnode_read_bool(node, "u-boot,dm-tpl")) - return true; -#endif - - return false; -} diff --git a/include/dm/util.h b/include/dm/util.h index cbc209d..9b1128f 100644 --- a/include/dm/util.h +++ b/include/dm/util.h @@ -67,31 +67,4 @@ static inline void dm_dump_devres(void) */ bool dm_fdt_pre_reloc(const void *blob, int offset);
-/** - * Check if an of node should be or was bound before relocation. - * - * Devicetree nodes can be marked as needed to be bound - * in the loader stages via special devicetree properties. - * - * Before relocation this function can be used to check if nodes - * are required in either SPL or TPL stages. - * - * After relocation and jumping into the real U-Boot binary - * it is possible to determine if a node was bound in one of - * SPL/TPL stages. - * - * There are 4 settings currently in use - * - u-boot,dm-pre-proper: U-Boot proper pre-relocation only - * - u-boot,dm-pre-reloc: legacy and indicates any of TPL or SPL - * Existing platforms only use it to indicate nodes needed in - * SPL. Should probably be replaced by u-boot,dm-spl for - * existing platforms. - * - u-boot,dm-spl: SPL and U-Boot pre-relocation - * - u-boot,dm-tpl: TPL and U-Boot pre-relocation - * @node: of node - * - * Returns true if node is needed in SPL/TL, false otherwise. - */ -bool dm_ofnode_pre_reloc(ofnode node); - #endif
Hi Patrick,
On Mon, 4 Feb 2019 at 03:15, Patrick Delaunay patrick.delaunay@st.com wrote:
The content dm_ofnode_pre_reloc() is identical with ofnode_pre_reloc() defined in drivers/core/ofnode.c and used only one time in drivers/core/lists.c:lists_bind_fdt().
So the function can be removed and directly call ofnode_pre_reloc.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
drivers/core/lists.c | 2 +- drivers/core/util.c | 26 -------------------------- include/dm/util.h | 27 --------------------------- 3 files changed, 1 insertion(+), 54 deletions(-)
I think the docs for ofnode_pre_reloc() need updating.
Regards, Simon
Hi Simon,
From: Simon Glass sjg@chromium.org Sent: lundi 4 février 2019 15:41
Hi Patrick,
On Mon, 4 Feb 2019 at 03:15, Patrick Delaunay patrick.delaunay@st.com wrote:
The content dm_ofnode_pre_reloc() is identical with ofnode_pre_reloc() defined in drivers/core/ofnode.c and used only one time in drivers/core/lists.c:lists_bind_fdt().
So the function can be removed and directly call ofnode_pre_reloc.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
drivers/core/lists.c | 2 +- drivers/core/util.c | 26 -------------------------- include/dm/util.h | 27 --------------------------- 3 files changed, 1 insertion(+), 54 deletions(-)
I think the docs for ofnode_pre_reloc() need updating.
You think about which docs ?
With previous patch in the serie, in ofnode.h, I have:
/** * ofnode_pre_reloc() - check if a node should be bound before relocation * * Device tree nodes can be marked as needing-to-be-bound in the loader stages * via special device tree properties. * * Before relocation this function can be used to check if nodes are required * in either SPL or TPL stages. * * After relocation and jumping into the real U-Boot binary it is possible to * determine if a node was bound in one of SPL/TPL stages. * * There are 4 settings currently in use * - u-boot,dm-pre-proper: U-Boot proper pre-relocation only * - u-boot,dm-pre-reloc: legacy and indicates any of TPL or SPL * Existing platforms only use it to indicate nodes needed in * SPL. Should probably be replaced by u-boot,dm-spl for * new platforms. * - u-boot,dm-spl: SPL and U-Boot pre-relocation * - u-boot,dm-tpl: TPL and U-Boot pre-relocation * * @node: node to check * @return true if node is needed in SPL/TL, false otherwise */ bool ofnode_pre_reloc(ofnode node);
What do you expect ?
Regards, Simon
Regards Patrick
On Mon, 4 Feb 2019 at 03:15, Patrick Delaunay patrick.delaunay@st.com wrote:
This add missing parts for previous commit 06f94461a9f4 ("fdt: Allow indicating a node is for U-Boot proper only")
At present it is not possible to specify that a node should be used before relocation (in U-Boot proper) without it also ending up in SPL and TPL device trees. Add a new "u-boot,dm-pre-proper" boolean property for this.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
Reviewed-by: Simon Glass sjg@chromium.org
There was discussion some time ago about using a property instead:
chosen { u-boot,dm-spl = <&node1 &node2>; u-boot,dm-tpl = <&node1>; };
At the time I decided that this was more painful since it separates out the tag from its node.
I wonder if that is still true? We do now in fact generally use a u-boot.dtsi file to hold these tags.
I'm not suggesting a change, just raising the question.
Regards, Simon
On 2/4/19 3:40 PM, Simon Glass wrote:
On Mon, 4 Feb 2019 at 03:15, Patrick Delaunay patrick.delaunay@st.com wrote:
This add missing parts for previous commit 06f94461a9f4 ("fdt: Allow indicating a node is for U-Boot proper only")
At present it is not possible to specify that a node should be used before relocation (in U-Boot proper) without it also ending up in SPL and TPL device trees. Add a new "u-boot,dm-pre-proper" boolean property for this.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
Reviewed-by: Simon Glass sjg@chromium.org
There was discussion some time ago about using a property instead:
chosen { u-boot,dm-spl = <&node1 &node2>; u-boot,dm-tpl = <&node1>; };
At the time I decided that this was more painful since it separates out the tag from its node.
I wonder if that is still true? We do now in fact generally use a u-boot.dtsi file to hold these tags.
I'm not suggesting a change, just raising the question.
It's a good suggestion. In fact, you can then use some tool to walk back up the tree and only retain the branches which are referenced by phandle from u-boot,dm-spl/tpl node for U-Boot SPL/TPL DTs. I think that could save a bit of space too. I wonder if we can even use /omit-if-no-ref/ DTC syntax somehow.
Hi Marek,
From: Marek Vasut marex@denx.de Sent: mardi 5 février 2019 09:55
On 2/4/19 3:40 PM, Simon Glass wrote:
On Mon, 4 Feb 2019 at 03:15, Patrick Delaunay patrick.delaunay@st.com
wrote:
This add missing parts for previous commit 06f94461a9f4 ("fdt: Allow indicating a node is for U-Boot proper only")
At present it is not possible to specify that a node should be used before relocation (in U-Boot proper) without it also ending up in SPL and TPL device trees. Add a new "u-boot,dm-pre-proper" boolean property
for this.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
Reviewed-by: Simon Glass sjg@chromium.org
There was discussion some time ago about using a property instead:
chosen { u-boot,dm-spl = <&node1 &node2>; u-boot,dm-tpl = <&node1>; };
At the time I decided that this was more painful since it separates out the tag from its node.
I wonder if that is still true? We do now in fact generally use a u-boot.dtsi file to hold these tags.
I'm not suggesting a change, just raising the question.
It's a good suggestion. In fact, you can then use some tool to walk back up the tree and only retain the branches which are referenced by phandle from u- boot,dm-spl/tpl node for U-Boot SPL/TPL DTs. I think that could save a bit of space too. I wonder if we can even use /omit-if-no-ref/ DTC syntax somehow.
If I correctly understood the new feature of dtc "/omit-if-no-ref/", in kernel dtc since 4.18, dtc remove node when nobody use reference on this node, so it is useful for U-Boot nodes when they are used as reference : for example pincontrol nodes. cf: https://elinux.org/Device_Tree_Source_Undocumented
U-Boot dtb use dtc to compile dts files , so it can be used here.
But for SPL / TPL the parsing in done in fdtgrep, on dtb And the tags "/omit-if-no-ref/" are already removed in dtb.
NB: we could also remove the tags u-boot,dm-pre-reloc/u-boot,dm-spl : we can gain place in spl dtb These tags are not needed as binding is mandatory in SPL build for ALL node present in SPL device tree others node are cleaned by fdtgrep (but impact in SPL code)
PS: a other boring point: if we need to tag all the tree to have one sub-node in SPL For me spl tag on the children should be enough ... But is perhaps difficult in ftdgrep (need to parse all the fathers node)
For example to have pins1 and pins2, we have to also tag qspi-bk2-0 and pin-controller
pin-controller@50002000 { u-boot,dm-pre-reloc; qspi-bk2-0 { u-boot,dm-spl; pins1 { u-boot,dm-spl; }; pins2 { u-boot,dm-spl; }; }; };
-- Best regards, Marek Vasut
On 2/7/19 6:40 PM, Patrick DELAUNAY wrote:
Hi Marek,
From: Marek Vasut marex@denx.de Sent: mardi 5 février 2019 09:55
On 2/4/19 3:40 PM, Simon Glass wrote:
On Mon, 4 Feb 2019 at 03:15, Patrick Delaunay patrick.delaunay@st.com
wrote:
This add missing parts for previous commit 06f94461a9f4 ("fdt: Allow indicating a node is for U-Boot proper only")
At present it is not possible to specify that a node should be used before relocation (in U-Boot proper) without it also ending up in SPL and TPL device trees. Add a new "u-boot,dm-pre-proper" boolean property
for this.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
Reviewed-by: Simon Glass sjg@chromium.org
There was discussion some time ago about using a property instead:
chosen { u-boot,dm-spl = <&node1 &node2>; u-boot,dm-tpl = <&node1>; };
At the time I decided that this was more painful since it separates out the tag from its node.
I wonder if that is still true? We do now in fact generally use a u-boot.dtsi file to hold these tags.
I'm not suggesting a change, just raising the question.
It's a good suggestion. In fact, you can then use some tool to walk back up the tree and only retain the branches which are referenced by phandle from u- boot,dm-spl/tpl node for U-Boot SPL/TPL DTs. I think that could save a bit of space too. I wonder if we can even use /omit-if-no-ref/ DTC syntax somehow.
If I correctly understood the new feature of dtc "/omit-if-no-ref/", in kernel dtc since 4.18, dtc remove node when nobody use reference on this node, so it is useful for U-Boot nodes when they are used as reference : for example pincontrol nodes. cf: https://elinux.org/Device_Tree_Source_Undocumented
U-Boot dtb use dtc to compile dts files , so it can be used here.
But for SPL / TPL the parsing in done in fdtgrep, on dtb And the tags "/omit-if-no-ref/" are already removed in dtb.
Maybe we need feature which is similar, not the same.
NB: we could also remove the tags u-boot,dm-pre-reloc/u-boot,dm-spl : we can gain place in spl dtb These tags are not needed as binding is mandatory in SPL build for ALL node present in SPL device tree others node are cleaned by fdtgrep (but impact in SPL code)
PS: a other boring point: if we need to tag all the tree to have one sub-node in SPL For me spl tag on the children should be enough ...
Indeed, referencing the child node with a phandle from some chosen node would be much better than adding u-boot,dm-... throughout the tree.
But is perhaps difficult in ftdgrep (need to parse all the fathers node) For example to have pins1 and pins2, we have to also tag qspi-bk2-0 and pin-controller pin-controller@50002000 { u-boot,dm-pre-reloc; qspi-bk2-0 { u-boot,dm-spl; pins1 { u-boot,dm-spl; }; pins2 { u-boot,dm-spl; }; }; };
I still like using the phandle better than the u-boot,dm-spl all over the place.
Hi Marek,
From: Marek Vasut marex@denx.de Sent: jeudi 7 février 2019 18:48
On 2/7/19 6:40 PM, Patrick DELAUNAY wrote:
Hi Marek,
From: Marek Vasut marex@denx.de Sent: mardi 5 février 2019 09:55
On 2/4/19 3:40 PM, Simon Glass wrote:
On Mon, 4 Feb 2019 at 03:15, Patrick Delaunay patrick.delaunay@st.com
wrote:
This add missing parts for previous commit 06f94461a9f4 ("fdt: Allow indicating a node is for U-Boot proper only")
At present it is not possible to specify that a node should be used before relocation (in U-Boot proper) without it also ending up in SPL and TPL device trees. Add a new "u-boot,dm-pre-proper" boolean property
for this.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
Reviewed-by: Simon Glass sjg@chromium.org
There was discussion some time ago about using a property instead:
chosen { u-boot,dm-spl = <&node1 &node2>; u-boot,dm-tpl = <&node1>; };
At the time I decided that this was more painful since it separates out the tag from its node.
I wonder if that is still true? We do now in fact generally use a u-boot.dtsi file to hold these tags.
I'm not suggesting a change, just raising the question.
It's a good suggestion. In fact, you can then use some tool to walk back up the tree and only retain the branches which are referenced by phandle from u- boot,dm-spl/tpl node for U-Boot SPL/TPL DTs. I think that could save a bit of space too. I wonder if we can even use /omit-if-no-
ref/ DTC syntax somehow.
If I correctly understood the new feature of dtc "/omit-if-no-ref/", in kernel dtc since 4.18, dtc remove node when nobody use reference on this node, so it is useful for U-Boot nodes when they are used as reference :
for example pincontrol nodes.
cf: https://elinux.org/Device_Tree_Source_Undocumented
U-Boot dtb use dtc to compile dts files , so it can be used here.
But for SPL / TPL the parsing in done in fdtgrep, on dtb And the tags "/omit-if-no-ref/" are already removed in dtb.
Maybe we need feature which is similar, not the same.
Ok.
I take some time to check how the "omit is no ref" is done in dtc and fdtgrep manage the dtb. And I think a similar feature (count the number of reference for the resulting dtb) are difficult to do in ftdgrep, and so in SPL because dtc replace the reference by phandle in DTB.
ftdgrep can know if one node have phandle= referenced at least one time ... in U-boot device tree. But how know is referenced (directly or inderclty , for pincotnrol fro exemple)
For example
ddr@0x5A003000 { u-boot,dm-pre-reloc; compatible = "st,stm32mp1-ddr"; reg = <0x5a003000 0x00000550 0x5a004000 0x00000234>; clocks = <0x00000002 0x000000e4 0x00000002 0x000000dc 0x00000002 0x000000de 0x00000002 0x000000e0 0x00000002 0x000000e2 0x00000002 0x000000e5>; PHDANDLE PHANDLE PHANDLE
NB: we could also remove the tags u-boot,dm-pre-reloc/u-boot,dm-spl : we
can gain place in spl dtb
These tags are not needed as binding is mandatory in SPL build for ALLnode present in SPL device tree
others node are cleaned by fdtgrep (but impact in SPL code)
Finally I will sent a patch for this proposal (today I hope) => On my board stm32mp1-ev1, the SPL device tree is reduced by 790 bytes NB: side effect on the patch, we only need to TAG the children (all node are bounded in SPL) That improve the next point.
PS: a other boring point: if we need to tag all the tree to have one sub-node in
SPL
For me spl tag on the children should be enough ...Indeed, referencing the child node with a phandle from some chosen node would be much better than adding u-boot,dm-... throughout the tree.
PS: We can use the option -e of fdtgrep ... but with side effect (some unwanted node can be added) Example SPL should be see only the first flash on SPI controller, not need to see the second one.
But is perhaps difficult in ftdgrep (need to parse all thefathers node)
For example to have pins1 and pins2, we have to also tag qspi-bk2-0 and pin-controller pin-controller@50002000 { u-boot,dm-pre-reloc; qspi-bk2-0 { u-boot,dm-spl; pins1 { u-boot,dm-spl; }; pins2 { u-boot,dm-spl; }; }; };I still like using the phandle better than the u-boot,dm-spl all over the place.
I agree that it is a way for SPL improvement.
But still difficult : need to reference all the needed node (including pincontrol ones) and need to add this reference (when then don't exist yet in kernel DT)
for example, you need to have
chosen { u-boot,dm-spl = <&node1 &node2 &{/soc/pin-controller@0 /pin1_a/pins} &{/soc/pin-controller@0 /pin2_a/pins1} &{/soc/pin-controller@0 /pin2_a/pins2} >; u-boot,dm-tpl = <&node1 &{/soc/pin-controller@0 /pin1_a/pins}>; };
Kernel device tree is :
soc { node1: ip@0 { pinctrl-names = "default", "sleep", "standby"; pinctrl-0 = <&pin1_a &pin1_b &pin1_c >; }
node2: ip@0 { pinctrl-names = "default", "sleep", "standby"; pinctrl-0 = <&pin2_a &pin2_b &pin2_c>; }
pinctrl: pin-controller@0 { pin1_a: node1-0 { pins { .... }; }; pin1_b: node1-1 { pins1 { .... }; pins2 { .... }; }; pin1_c: node1-2 { pins1 { .... }; pins1 { .... }; }; pint2_a: node2-0 { pins1 { .... }; pins1 { .... }; }; pin2_b: node2-1 { pins1 { .... }; pins1 { .... }; }; pin2_c: node2-2 { pins1 { .... }; pins1 { .... }; };
Today I prefer the current way (kernel device tree in <board>.dts and added when necessary tag can be done in <board>-u-boot.dtsi), because we can use include (for commun feature for several boards)
see for example ./arch/arm/dts/stm32mp157-u-boot.dtsi
-- Best regards, Marek Vasut
Regards Patrick
Hi Marek and Simon
From: Patrick DELAUNAY Sent: vendredi 8 février 2019 13:47
Hi Marek,
From: Marek Vasut marex@denx.de Sent: jeudi 7 février 2019 18:48
On 2/7/19 6:40 PM, Patrick DELAUNAY wrote:
Hi Marek,
From: Marek Vasut marex@denx.de Sent: mardi 5 février 2019 09:55
On 2/4/19 3:40 PM, Simon Glass wrote:
On Mon, 4 Feb 2019 at 03:15, Patrick Delaunay patrick.delaunay@st.com
wrote:
This add missing parts for previous commit 06f94461a9f4 ("fdt: Allow indicating a node is for U-Boot proper only")
At present it is not possible to specify that a node should be used before relocation (in U-Boot proper) without it also ending up in SPL and TPL device trees. Add a new "u-boot,dm-pre-proper" boolean property
for this.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
...
NB: we could also remove the tags u-boot,dm-pre-reloc/u-boot,dm-spl : we
can gain place in spl dtb
These tags are not needed as binding is mandatory in SPLbuild for ALL
node present in SPL device tree
others node are cleaned by fdtgrep (but impact in SPL code)Finally I will sent a patch for this proposal (today I hope) => On my board stm32mp1-ev1, the SPL device tree is reduced by 790 bytes
FYI: patch sent with http://patchwork.ozlabs.org/patch/1039756/
NB: side effect on the patch, we only need to TAG the children (all node are bounded in SPL) That improve the next point.
After test, the phandle of the parent node is remove when the tag preloc or spl is not present.... So the need to tag all the DT tree is not solved.
Regards Patrick
On 2/11/19 12:56 PM, Patrick DELAUNAY wrote:
Hi Marek and Simon
From: Patrick DELAUNAY Sent: vendredi 8 février 2019 13:47
Hi Marek,
From: Marek Vasut marex@denx.de Sent: jeudi 7 février 2019 18:48
On 2/7/19 6:40 PM, Patrick DELAUNAY wrote:
Hi Marek,
From: Marek Vasut marex@denx.de Sent: mardi 5 février 2019 09:55
On 2/4/19 3:40 PM, Simon Glass wrote:
On Mon, 4 Feb 2019 at 03:15, Patrick Delaunay patrick.delaunay@st.com
wrote:
> > This add missing parts for previous commit 06f94461a9f4 > ("fdt: Allow indicating a node is for U-Boot proper only") > > At present it is not possible to specify that a node should be > used before relocation (in U-Boot proper) without it also ending > up in SPL and TPL device trees. Add a new "u-boot,dm-pre-proper" > boolean property
for this.
> > > Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
...
NB: we could also remove the tags u-boot,dm-pre-reloc/u-boot,dm-spl : we
can gain place in spl dtb
These tags are not needed as binding is mandatory in SPLbuild for ALL
node present in SPL device tree
others node are cleaned by fdtgrep (but impact in SPL code)Finally I will sent a patch for this proposal (today I hope) => On my board stm32mp1-ev1, the SPL device tree is reduced by 790 bytes
FYI: patch sent with http://patchwork.ozlabs.org/patch/1039756/
NB: side effect on the patch, we only need to TAG the children (all node are bounded in SPL) That improve the next point.
After test, the phandle of the parent node is remove when the tag preloc or spl is not present.... So the need to tag all the DT tree is not solved.
I'm not sure I quite understand this, but if you reference the leaf node in the tree, you should get references to all the parent nodes for free and they cannot be removed, right ?
Hi Marek,
From: Marek Vasut marex@denx.de Sent: lundi 11 février 2019 12:58
On 2/11/19 12:56 PM, Patrick DELAUNAY wrote:
Hi Marek and Simon
From: Patrick DELAUNAY Sent: vendredi 8 février 2019 13:47
Hi Marek,
From: Marek Vasut marex@denx.de Sent: jeudi 7 février 2019 18:48
On 2/7/19 6:40 PM, Patrick DELAUNAY wrote:
Hi Marek,
From: Marek Vasut marex@denx.de Sent: mardi 5 février 2019 09:55
On 2/4/19 3:40 PM, Simon Glass wrote: > On Mon, 4 Feb 2019 at 03:15, Patrick Delaunay > patrick.delaunay@st.com wrote: >> >> This add missing parts for previous commit 06f94461a9f4 >> ("fdt: Allow indicating a node is for U-Boot proper only") >> >> At present it is not possible to specify that a node should be >> used before relocation (in U-Boot proper) without it also ending >> up in SPL and TPL device trees. Add a new "u-boot,dm-pre-proper" >> boolean property for this. >> >> >> Signed-off-by: Patrick Delaunay patrick.delaunay@st.com >
...
NB: we could also remove the tags u-boot,dm-pre-reloc/u-boot,dm-spl : we
can gain place in spl dtb
These tags are not needed as binding is mandatory in SPLbuild for ALL
node present in SPL device tree
others node are cleaned by fdtgrep (but impact in SPL code)Finally I will sent a patch for this proposal (today I hope) => On my board stm32mp1-ev1, the SPL device tree is reduced by 790 bytes
FYI: patch sent with http://patchwork.ozlabs.org/patch/1039756/
NB: side effect on the patch, we only need to TAG the children (all node are bounded in SPL) That improve the next point.
After test, the phandle of the parent node is remove when the tag preloc or spl
is not present....
So the need to tag all the DT tree is not solved.
I'm not sure I quite understand this, but if you reference the leaf node in the tree, you should get references to all the parent nodes for free and they cannot be removed, right ?
Yes, I think the same before my test, it is why I indicate this point, even if it not clear.
But I don't known ftdgrep can be used fro this point.
To explain more in details:
I try to remove u-boot,dm-spl for some nodes in ED1/EV1 board :
------------------- arch/arm/dts/stm32mp157c-ed1-u-boot.dtsi ------------------- index 4898483..574b251 100644 @@ -165,14 +165,12 @@ /* SPL part **************************************/ /* MMC1 boot */ &sdmmc1_b4_pins_a { - u-boot,dm-spl; pins { u-boot,dm-spl; }; };
&sdmmc1_dir_pins_a { - u-boot,dm-spl; pins { u-boot,dm-spl; }; @@ -184,14 +182,12 @@
/* MMC2 boot */ &sdmmc2_b4_pins_a { - u-boot,dm-spl; pins { u-boot,dm-spl; }; };
&sdmmc2_d47_pins_a { - u-boot,dm-spl; pins { u-boot,dm-spl; };
------------------- arch/arm/dts/stm32mp157c-ev1-u-boot.dtsi ------------------- index 30b1734..eab7334 100644 @@ -35,14 +35,12 @@ };
&qspi_clk_pins_a { - u-boot,dm-spl; pins { u-boot,dm-spl; }; };
&qspi_bk1_pins_a { - u-boot,dm-spl; pins1 { u-boot,dm-spl; }; @@ -52,7 +50,6 @@ };
&qspi_bk2_pins_a { - u-boot,dm-spl; pins1 { u-boot,dm-spl; };
The result seems correct for a first look, but in fact some many phandle are missing and the device becomes invalid :
*************** *** 351,353 **** qspi-clk-0 { - phandle = <0x00000013>; pins { --- 351,352 ---- *************** *** 360,362 **** qspi-bk1-0 { - phandle = <0x00000014>; pins1 { --- 359,360 ---- *************** *** 375,377 **** qspi-bk2-0 { - phandle = <0x00000015>; pins1 { --- 373,374 ---- *************** *** 390,392 **** sdmmc1-b4@0 { - phandle = <0x00000016>; pins { --- 387,388 ---- *************** *** 399,401 **** sdmmc1-dir@0 { - phandle = <0x00000017>; pins { --- 395,396 ---- *************** *** 408,410 **** sdmmc2-b4@0 { - phandle = <0x0000001a>; pins { --- 403,404 ---- *************** *** 417,419 **** sdmmc2-d47@0 { - phandle = <0x0000001b>; pins { --- 411,412 ----
I think the fdtgrep correctly adds the supernode (the parent node) but not their properties , it is managed by the code:
FDT_REG_SUPERNODES is used by default in fdtgrep => tools/fdtgrep.c:1195: disp.flags = FDT_REG_SUPERNODES; /* Default flags */
But properties are not added except if h_include return true : => lib/libfdt/fdt_region.c:513: if ((flags & FDT_REG_SUPERNODES) && val && !p.want) p.want = WANT_NODES_ONLY;
So to have also properties, I think fdtgrep.c:h_include() should be change to return 1 for some FDT_IS_PROP case..... but it should be difficult to know that the chil are include, before to parse them
For example trace in ftdgrep for
sdmmc1-b4@0 { phandle = <0x00000016>; pins { pinmux = <0x0000280d 0x00002c0d 0x0000036b 0x0000035b 0x00000002 0x63312d64>; slew-rate = <0x00000003>; drive-push-pull; bias-disable; }; };
type=1, data=/soc/pin-controller@50002000/sdmmc2-b4@0 - val->type=1, str='/config', match=0 - val->type=1, str='/chosen', match=0 - no match, types_inc=11, types_exc=0, none_match=1f - checking node 'sdmmc2-b4@0' type=10, data=phandle - val->type=10, str='u-boot,dm-spl', match=0 - val->type=10, str='u-boot,dm-pre-reloc', match=0 - no match, types_inc=11, types_exc=0, none_match=1f - returning 0
=> phandle NOT added -----------------------------------------------------
type=1, data=/soc/pin-controller@50002000/sdmmc2-b4@0 - val->type=1, str='/config', match=0 - val->type=1, str='/chosen', match=0 - no match, types_inc=11, types_exc=0, none_match=1f - checking node 'sdmmc2-b4@0' type=10, data=phandle - val->type=10, str='u-boot,dm-spl', match=0 - val->type=10, str='u-boot,dm-pre-reloc', match=0 - no match, types_inc=11, types_exc=0, none_match=1f - returning 0 type=2, data=phandle - not in any condition - returning -1 type=1, data=/soc/pin-controller@50002000/sdmmc2-b4@0/pins - val->type=1, str='/config', match=0 - val->type=1, str='/chosen', match=0 - no match, types_inc=11, types_exc=0, none_match=1f - checking node 'pins' type=10, data=pinmux - val->type=10, str='u-boot,dm-spl', match=0 - val->type=10, str='u-boot,dm-pre-reloc', match=0 - no match, types_inc=11, types_exc=0, none_match=1f type=10, data=slew-rate - val->type=10, str='u-boot,dm-spl', match=0 - val->type=10, str='u-boot,dm-pre-reloc', match=0 - no match, types_inc=11, types_exc=0, none_match=1f type=10, data=drive-push-pull - val->type=10, str='u-boot,dm-spl', match=0 - val->type=10, str='u-boot,dm-pre-reloc', match=0 - no match, types_inc=11, types_exc=0, none_match=1f type=10, data=bias-pull-up - val->type=10, str='u-boot,dm-spl', match=0 - val->type=10, str='u-boot,dm-pre-reloc', match=0 - no match, types_inc=11, types_exc=0, none_match=1f type=10, data=u-boot,dm-spl - val->type=10, str='u-boot,dm-spl', match=1 - match inc u-boot,dm-spl - returning 1
------------ including sub node pins ------------------------------
type=2, data=pinmux - not in any condition - returning -1 type=2, data=slew-rate - not in any condition - returning -1 type=2, data=drive-push-pull - not in any condition - returning -1 type=2, data=bias-pull-up - not in any condition - returning -1 type=2, data=u-boot,dm-spl - not in any condition - returning -1
------- Next node --------
type=1, data=/soc/pin-controller@50002000/sdmmc2-d47@0 - val->type=1, str='/config', match=0 - val->type=1, str='/chosen', match=0 - no match, types_inc=11, types_exc=0, none_match=1f - checking node 'sdmmc2-d47@0'
-- Best regards, Marek Vasut
Sorry for the long exmplaination.
Regards Patrick
Hi Patrick,
On Mon, 11 Feb 2019 at 07:34, Patrick DELAUNAY patrick.delaunay@st.com wrote:
Hi Marek,
From: Marek Vasut marex@denx.de Sent: lundi 11 février 2019 12:58
On 2/11/19 12:56 PM, Patrick DELAUNAY wrote:
Hi Marek and Simon
From: Patrick DELAUNAY Sent: vendredi 8 février 2019 13:47
Hi Marek,
From: Marek Vasut marex@denx.de Sent: jeudi 7 février 2019 18:48
On 2/7/19 6:40 PM, Patrick DELAUNAY wrote:
Hi Marek,
> From: Marek Vasut marex@denx.de > Sent: mardi 5 février 2019 09:55 > > On 2/4/19 3:40 PM, Simon Glass wrote: >> On Mon, 4 Feb 2019 at 03:15, Patrick Delaunay >> patrick.delaunay@st.com > wrote: >>> >>> This add missing parts for previous commit 06f94461a9f4 >>> ("fdt: Allow indicating a node is for U-Boot proper only") >>> >>> At present it is not possible to specify that a node should be >>> used before relocation (in U-Boot proper) without it also ending >>> up in SPL and TPL device trees. Add a new "u-boot,dm-pre-proper" >>> boolean property > for this. >>> >>> >>> Signed-off-by: Patrick Delaunay patrick.delaunay@st.com >>
...
NB: we could also remove the tags u-boot,dm-pre-reloc/u-boot,dm-spl : we
can gain place in spl dtb
These tags are not needed as binding is mandatory in SPLbuild for ALL
node present in SPL device tree
others node are cleaned by fdtgrep (but impact in SPL code)Finally I will sent a patch for this proposal (today I hope) => On my board stm32mp1-ev1, the SPL device tree is reduced by 790 bytes
FYI: patch sent with http://patchwork.ozlabs.org/patch/1039756/
That patch was applied.
But what is the status of this patch, and the second one in this series?
If these are still needed can you please resend them, rebased?
Regards, Simon
Hi Simon,
Hi Patrick,
On Mon, 11 Feb 2019 at 07:34, Patrick DELAUNAY patrick.delaunay@st.com wrote:
Hi Marek,
From: Marek Vasut marex@denx.de Sent: lundi 11 février 2019 12:58
On 2/11/19 12:56 PM, Patrick DELAUNAY wrote:
Hi Marek and Simon
From: Patrick DELAUNAY Sent: vendredi 8 février 2019 13:47
Hi Marek,
From: Marek Vasut marex@denx.de Sent: jeudi 7 février 2019 18:48
On 2/7/19 6:40 PM, Patrick DELAUNAY wrote: > Hi Marek, > >> From: Marek Vasut marex@denx.de >> Sent: mardi 5 février 2019 09:55 >> >> On 2/4/19 3:40 PM, Simon Glass wrote: >>> On Mon, 4 Feb 2019 at 03:15, Patrick Delaunay >>> patrick.delaunay@st.com >> wrote: >>>> >>>> This add missing parts for previous commit 06f94461a9f4 >>>> ("fdt: Allow indicating a node is for U-Boot proper only") >>>> >>>> At present it is not possible to specify that a node should >>>> be used before relocation (in U-Boot proper) without it also >>>> ending up in SPL and TPL device trees. Add a new "u-boot,dm-pre-
proper"
>>>> boolean property >> for this. >>>> >>>> >>>> Signed-off-by: Patrick Delaunay patrick.delaunay@st.com >>>
...
> NB: we could also remove the tags > u-boot,dm-pre-reloc/u-boot,dm-spl > : we can gain place in spl dtb > These tags are not needed as binding is mandatory in SPL > build for ALL node present in SPL device tree > others node are cleaned by fdtgrep (but impact in SPL > code)
Finally I will sent a patch for this proposal (today I hope) => On my board stm32mp1-ev1, the SPL device tree is reduced by 790 bytes
FYI: patch sent with http://patchwork.ozlabs.org/patch/1039756/
That patch was applied.
But what is the status of this patch, and the second one in this series?
If these are still needed can you please resend them, rebased?
Yes , I need to sent a update. I finish just it (include tests) Friday, I will sent it today
Regards, Simon
Regards Patrick
participants (4)
-
Marek Vasut -
Patrick DELAUNAY -
Patrick Delaunay
-
Simon Glass