* [U-Boot] [PATCH] dm: check OF_LIVE is enabled
@ 2019-02-26 9:26 Ibai Erkiaga
2019-03-10 21:51 ` Simon Glass
0 siblings, 1 reply; 4+ messages in thread
From: Ibai Erkiaga @ 2019-02-26 9:26 UTC (permalink / raw)
To: u-boot
Livetree implemented functions does not have conditional compilation so
check if CONFIG_IS_ENABLED prior using those functions.
The issue does not report any error in a normal build as the toolchain
optimize the code. Using -O0 triggers the error so the patch is intended
to fix issues on a ongoing effor to build U-Boot with -O0.
Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>
---
drivers/core/ofnode.c | 60 +++++++++++++++++++++---------------------
drivers/serial/serial-uclass.c | 2 +-
2 files changed, 31 insertions(+), 31 deletions(-)
diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index 0e584c1..caa7fa5 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -20,7 +20,7 @@ int ofnode_read_u32(ofnode node, const char *propname, u32 *outp)
assert(ofnode_valid(node));
debug("%s: %s: ", __func__, propname);
- if (ofnode_is_np(node)) {
+ if (ofnode_is_np(node) && CONFIG_IS_ENABLED(OF_LIVE)) {
return of_read_u32(ofnode_to_np(node), propname, outp);
} else {
const fdt32_t *cell;
@@ -63,7 +63,7 @@ int ofnode_read_u64(ofnode node, const char *propname, u64 *outp)
assert(ofnode_valid(node));
debug("%s: %s: ", __func__, propname);
- if (ofnode_is_np(node))
+ if (ofnode_is_np(node) && CONFIG_IS_ENABLED(OF_LIVE))
return of_read_u64(ofnode_to_np(node), propname, outp);
cell = fdt_getprop(gd->fdt_blob, ofnode_to_offset(node), propname,
@@ -109,7 +109,7 @@ const char *ofnode_read_string(ofnode node, const char *propname)
assert(ofnode_valid(node));
debug("%s: %s: ", __func__, propname);
- if (ofnode_is_np(node)) {
+ if (ofnode_is_np(node) && CONFIG_IS_ENABLED(OF_LIVE)) {
struct property *prop = of_find_property(
ofnode_to_np(node), propname, NULL);
@@ -141,7 +141,7 @@ ofnode ofnode_find_subnode(ofnode node, const char *subnode_name)
assert(ofnode_valid(node));
debug("%s: %s: ", __func__, subnode_name);
- if (ofnode_is_np(node)) {
+ if (ofnode_is_np(node) && CONFIG_IS_ENABLED(OF_LIVE)) {
const struct device_node *np = ofnode_to_np(node);
for (np = np->child; np; np = np->sibling) {
@@ -166,7 +166,7 @@ int ofnode_read_u32_array(ofnode node, const char *propname,
assert(ofnode_valid(node));
debug("%s: %s: ", __func__, propname);
- if (ofnode_is_np(node)) {
+ if (ofnode_is_np(node) && CONFIG_IS_ENABLED(OF_LIVE)) {
return of_read_u32_array(ofnode_to_np(node), propname,
out_values, sz);
} else {
@@ -179,7 +179,7 @@ int ofnode_read_u32_array(ofnode node, const char *propname,
ofnode ofnode_first_subnode(ofnode node)
{
assert(ofnode_valid(node));
- if (ofnode_is_np(node))
+ if (ofnode_is_np(node) && CONFIG_IS_ENABLED(OF_LIVE))
return np_to_ofnode(node.np->child);
return offset_to_ofnode(
@@ -189,7 +189,7 @@ ofnode ofnode_first_subnode(ofnode node)
ofnode ofnode_next_subnode(ofnode node)
{
assert(ofnode_valid(node));
- if (ofnode_is_np(node))
+ if (ofnode_is_np(node) && CONFIG_IS_ENABLED(OF_LIVE))
return np_to_ofnode(node.np->sibling);
return offset_to_ofnode(
@@ -201,7 +201,7 @@ ofnode ofnode_get_parent(ofnode node)
ofnode parent;
assert(ofnode_valid(node));
- if (ofnode_is_np(node))
+ if (ofnode_is_np(node) && CONFIG_IS_ENABLED(OF_LIVE))
parent = np_to_ofnode(of_get_parent(ofnode_to_np(node)));
else
parent.of_offset = fdt_parent_offset(gd->fdt_blob,
@@ -213,7 +213,7 @@ ofnode ofnode_get_parent(ofnode node)
const char *ofnode_get_name(ofnode node)
{
assert(ofnode_valid(node));
- if (ofnode_is_np(node))
+ if (ofnode_is_np(node) && CONFIG_IS_ENABLED(OF_LIVE))
return strrchr(node.np->full_name, '/') + 1;
return fdt_get_name(gd->fdt_blob, ofnode_to_offset(node), NULL);
@@ -223,7 +223,7 @@ ofnode ofnode_get_by_phandle(uint phandle)
{
ofnode node;
- if (of_live_active())
+ if (of_live_active() && CONFIG_IS_ENABLED(OF_LIVE))
node = np_to_ofnode(of_find_node_by_phandle(phandle));
else
node.of_offset = fdt_node_offset_by_phandle(gd->fdt_blob,
@@ -236,7 +236,7 @@ int ofnode_read_size(ofnode node, const char *propname)
{
int len;
- if (ofnode_is_np(node)) {
+ if (ofnode_is_np(node) && CONFIG_IS_ENABLED(OF_LIVE)) {
struct property *prop = of_find_property(
ofnode_to_np(node), propname, NULL);
@@ -256,7 +256,7 @@ fdt_addr_t ofnode_get_addr_index(ofnode node, int index)
int na, ns;
fdt_size_t size;
- if (ofnode_is_np(node)) {
+ if (ofnode_is_np(node) && CONFIG_IS_ENABLED(OF_LIVE)) {
const __be32 *prop_val;
uint flags;
@@ -267,7 +267,7 @@ fdt_addr_t ofnode_get_addr_index(ofnode node, int index)
ns = of_n_size_cells(ofnode_to_np(node));
- if (IS_ENABLED(CONFIG_OF_TRANSLATE) && ns > 0) {
+ if (CONFIG_IS_ENABLED(CONFIG_OF_TRANSLATE) && ns > 0) {
return of_translate_address(ofnode_to_np(node), prop_val);
} else {
na = of_n_addr_cells(ofnode_to_np(node));
@@ -292,7 +292,7 @@ fdt_addr_t ofnode_get_addr(ofnode node)
int ofnode_stringlist_search(ofnode node, const char *property,
const char *string)
{
- if (ofnode_is_np(node)) {
+ if (ofnode_is_np(node) && CONFIG_IS_ENABLED(OF_LIVE)) {
return of_property_match_string(ofnode_to_np(node),
property, string);
} else {
@@ -360,7 +360,7 @@ int ofnode_parse_phandle_with_args(ofnode node, const char *list_name,
int index,
struct ofnode_phandle_args *out_args)
{
- if (ofnode_is_np(node)) {
+ if (ofnode_is_np(node) && CONFIG_IS_ENABLED(OF_LIVE)) {
struct of_phandle_args args;
int ret;
@@ -389,7 +389,7 @@ int ofnode_parse_phandle_with_args(ofnode node, const char *list_name,
int ofnode_count_phandle_with_args(ofnode node, const char *list_name,
const char *cells_name)
{
- if (ofnode_is_np(node))
+ if (ofnode_is_np(node) && CONFIG_IS_ENABLED(OF_LIVE))
return of_count_phandle_with_args(ofnode_to_np(node),
list_name, cells_name);
else
@@ -400,8 +400,8 @@ int ofnode_count_phandle_with_args(ofnode node, const char *list_name,
ofnode ofnode_path(const char *path)
{
- if (of_live_active())
- return np_to_ofnode(of_find_node_by_path(path));
+ if (of_live_active() && CONFIG_IS_ENABLED(OF_LIVE))
+ return np_to_ofnode(of_find_node_opts_by_path(path, NULL));
else
return offset_to_ofnode(fdt_path_offset(gd->fdt_blob, path));
}
@@ -516,7 +516,7 @@ int ofnode_decode_display_timing(ofnode parent, int index,
const void *ofnode_get_property(ofnode node, const char *propname, int *lenp)
{
- if (ofnode_is_np(node))
+ if (ofnode_is_np(node) && CONFIG_IS_ENABLED(OF_LIVE))
return of_get_property(ofnode_to_np(node), propname, lenp);
else
return fdt_getprop(gd->fdt_blob, ofnode_to_offset(node),
@@ -525,7 +525,7 @@ const void *ofnode_get_property(ofnode node, const char *propname, int *lenp)
bool ofnode_is_available(ofnode node)
{
- if (ofnode_is_np(node))
+ if (ofnode_is_np(node) && CONFIG_IS_ENABLED(OF_LIVE))
return of_device_is_available(ofnode_to_np(node));
else
return fdtdec_get_is_enabled(gd->fdt_blob,
@@ -535,7 +535,7 @@ bool ofnode_is_available(ofnode node)
fdt_addr_t ofnode_get_addr_size(ofnode node, const char *property,
fdt_size_t *sizep)
{
- if (ofnode_is_np(node)) {
+ if (ofnode_is_np(node) && CONFIG_IS_ENABLED(OF_LIVE)) {
int na, ns;
int psize;
const struct device_node *np = ofnode_to_np(node);
@@ -547,7 +547,7 @@ fdt_addr_t ofnode_get_addr_size(ofnode node, const char *property,
ns = of_n_size_cells(np);
*sizep = of_read_number(prop + na, ns);
- if (IS_ENABLED(CONFIG_OF_TRANSLATE) && ns > 0)
+ if (CONFIG_IS_ENABLED(CONFIG_OF_TRANSLATE) && ns > 0)
return of_translate_address(np, prop);
else
return of_read_number(prop, na);
@@ -561,7 +561,7 @@ fdt_addr_t ofnode_get_addr_size(ofnode node, const char *property,
const uint8_t *ofnode_read_u8_array_ptr(ofnode node, const char *propname,
size_t sz)
{
- if (ofnode_is_np(node)) {
+ if (ofnode_is_np(node) && CONFIG_IS_ENABLED(OF_LIVE)) {
const struct device_node *np = ofnode_to_np(node);
int psize;
const __be32 *prop = of_get_property(np, propname, &psize);
@@ -668,7 +668,7 @@ int ofnode_read_pci_vendev(ofnode node, u16 *vendor, u16 *device)
int ofnode_read_addr_cells(ofnode node)
{
- if (ofnode_is_np(node))
+ if (ofnode_is_np(node) && CONFIG_IS_ENABLED(OF_LIVE))
return of_n_addr_cells(ofnode_to_np(node));
else /* NOTE: this call should walk up the parent stack */
return fdt_address_cells(gd->fdt_blob, ofnode_to_offset(node));
@@ -676,7 +676,7 @@ int ofnode_read_addr_cells(ofnode node)
int ofnode_read_size_cells(ofnode node)
{
- if (ofnode_is_np(node))
+ if (ofnode_is_np(node) && CONFIG_IS_ENABLED(OF_LIVE))
return of_n_size_cells(ofnode_to_np(node));
else /* NOTE: this call should walk up the parent stack */
return fdt_size_cells(gd->fdt_blob, ofnode_to_offset(node));
@@ -684,7 +684,7 @@ int ofnode_read_size_cells(ofnode node)
int ofnode_read_simple_addr_cells(ofnode node)
{
- if (ofnode_is_np(node))
+ if (ofnode_is_np(node) && CONFIG_IS_ENABLED(OF_LIVE))
return of_simple_addr_cells(ofnode_to_np(node));
else
return fdt_address_cells(gd->fdt_blob, ofnode_to_offset(node));
@@ -692,7 +692,7 @@ int ofnode_read_simple_addr_cells(ofnode node)
int ofnode_read_simple_size_cells(ofnode node)
{
- if (ofnode_is_np(node))
+ if (ofnode_is_np(node) && CONFIG_IS_ENABLED(OF_LIVE))
return of_simple_size_cells(ofnode_to_np(node));
else
return fdt_size_cells(gd->fdt_blob, ofnode_to_offset(node));
@@ -726,7 +726,7 @@ bool ofnode_pre_reloc(ofnode node)
int ofnode_read_resource(ofnode node, uint index, struct resource *res)
{
- if (ofnode_is_np(node)) {
+ if (ofnode_is_np(node) && CONFIG_IS_ENABLED(OF_LIVE)) {
return of_address_to_resource(ofnode_to_np(node), index, res);
} else {
struct fdt_resource fres;
@@ -758,7 +758,7 @@ int ofnode_read_resource_byname(ofnode node, const char *name,
u64 ofnode_translate_address(ofnode node, const fdt32_t *in_addr)
{
- if (ofnode_is_np(node))
+ if (ofnode_is_np(node) && CONFIG_IS_ENABLED(OF_LIVE))
return of_translate_address(ofnode_to_np(node), in_addr);
else
return fdt_translate_address(gd->fdt_blob, ofnode_to_offset(node), in_addr);
@@ -766,7 +766,7 @@ u64 ofnode_translate_address(ofnode node, const fdt32_t *in_addr)
int ofnode_device_is_compatible(ofnode node, const char *compat)
{
- if (ofnode_is_np(node))
+ if (ofnode_is_np(node) && CONFIG_IS_ENABLED(OF_LIVE))
return of_device_is_compatible(ofnode_to_np(node), compat,
NULL, NULL);
else
diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
index d4488a2..ee64616 100644
--- a/drivers/serial/serial-uclass.c
+++ b/drivers/serial/serial-uclass.c
@@ -86,7 +86,7 @@ static void serial_find_console_or_panic(void)
}
} else if (CONFIG_IS_ENABLED(OF_CONTROL) && blob) {
/* Live tree has support for stdout */
- if (of_live_active()) {
+ if (CONFIG_IS_ENABLED(OF_LIVE) && of_live_active()) {
struct device_node *np = of_get_stdout();
if (np && !uclass_get_device_by_ofnode(UCLASS_SERIAL,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH] dm: check OF_LIVE is enabled
2019-02-26 9:26 [U-Boot] [PATCH] dm: check OF_LIVE is enabled Ibai Erkiaga
@ 2019-03-10 21:51 ` Simon Glass
2019-03-12 7:19 ` Ibai Erkiaga Elorza
0 siblings, 1 reply; 4+ messages in thread
From: Simon Glass @ 2019-03-10 21:51 UTC (permalink / raw)
To: u-boot
Hi Ibai,
On Tue, 26 Feb 2019 at 02:26, Ibai Erkiaga
<ibai.erkiaga-elorza@xilinx.com> wrote:
>
> Livetree implemented functions does not have conditional compilation so
> check if CONFIG_IS_ENABLED prior using those functions.
>
> The issue does not report any error in a normal build as the toolchain
> optimize the code. Using -O0 triggers the error so the patch is intended
> to fix issues on a ongoing effor to build U-Boot with -O0.
>
> Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>
> ---
>
> drivers/core/ofnode.c | 60 +++++++++++++++++++++---------------------
> drivers/serial/serial-uclass.c | 2 +-
> 2 files changed, 31 insertions(+), 31 deletions(-)
>
This is supposed to work by using of_live_active(), which is called
from ofnode_is_np(). Instead of changing all this code, is it possible
to update of_live_active() somehow?
Regards,
Simon
^ permalink raw reply [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH] dm: check OF_LIVE is enabled
2019-03-10 21:51 ` Simon Glass
@ 2019-03-12 7:19 ` Ibai Erkiaga Elorza
2019-03-19 1:23 ` Simon Glass
0 siblings, 1 reply; 4+ messages in thread
From: Ibai Erkiaga Elorza @ 2019-03-12 7:19 UTC (permalink / raw)
To: u-boot
Hi Simon,
> -----Original Message-----
> From: Simon Glass <sjg@chromium.org>
> Sent: 10 March 2019 21:51
> To: Ibai Erkiaga Elorza <IBAIE@xilinx.com>
> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Patrick Delaunay
> <patrick.delaunay@st.com>; Andy Shevchenko
> <andriy.shevchenko@linux.intel.com>; Bin Meng <bmeng.cn@gmail.com>;
> Patrice Chotard <patrice.chotard@st.com>
> Subject: Re: [U-Boot][PATCH] dm: check OF_LIVE is enabled
>
> Hi Ibai,
>
> On Tue, 26 Feb 2019 at 02:26, Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>
> wrote:
> >
> > Livetree implemented functions does not have conditional compilation
> > so check if CONFIG_IS_ENABLED prior using those functions.
> >
> > The issue does not report any error in a normal build as the toolchain
> > optimize the code. Using -O0 triggers the error so the patch is
> > intended to fix issues on a ongoing effor to build U-Boot with -O0.
> >
> > Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>
> > ---
> >
> > drivers/core/ofnode.c | 60 +++++++++++++++++++++---------------------
> > drivers/serial/serial-uclass.c | 2 +-
> > 2 files changed, 31 insertions(+), 31 deletions(-)
> >
>
> This is supposed to work by using of_live_active(), which is called from
> ofnode_is_np(). Instead of changing all this code, is it possible to update
> of_live_active() somehow?
>
I've been trying to figure out it but I think there is no way just changing of_live_active as the compiler does not discard branch statements with nested fixed return values. I was able to make it work just changing ofnode_is_np using pre-processor macro to set as false when OF_LIVE is not enabled, but not sure if it the right approach. I will take a deeper look to the entire OF code and maybe suggest a code refactoring.
> Regards,
> Simon
Regards
Ibai
^ permalink raw reply [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH] dm: check OF_LIVE is enabled
2019-03-12 7:19 ` Ibai Erkiaga Elorza
@ 2019-03-19 1:23 ` Simon Glass
0 siblings, 0 replies; 4+ messages in thread
From: Simon Glass @ 2019-03-19 1:23 UTC (permalink / raw)
To: u-boot
Hi Ibai,
On Tue, 12 Mar 2019 at 15:19, Ibai Erkiaga Elorza <IBAIE@xilinx.com> wrote:
>
> Hi Simon,
>
> > -----Original Message-----
> > From: Simon Glass <sjg@chromium.org>
> > Sent: 10 March 2019 21:51
> > To: Ibai Erkiaga Elorza <IBAIE@xilinx.com>
> > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Patrick Delaunay
> > <patrick.delaunay@st.com>; Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com>; Bin Meng <bmeng.cn@gmail.com>;
> > Patrice Chotard <patrice.chotard@st.com>
> > Subject: Re: [U-Boot][PATCH] dm: check OF_LIVE is enabled
> >
> > Hi Ibai,
> >
> > On Tue, 26 Feb 2019 at 02:26, Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>
> > wrote:
> > >
> > > Livetree implemented functions does not have conditional compilation
> > > so check if CONFIG_IS_ENABLED prior using those functions.
> > >
> > > The issue does not report any error in a normal build as the toolchain
> > > optimize the code. Using -O0 triggers the error so the patch is
> > > intended to fix issues on a ongoing effor to build U-Boot with -O0.
> > >
> > > Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com>
> > > ---
> > >
> > > drivers/core/ofnode.c | 60 +++++++++++++++++++++---------------------
> > > drivers/serial/serial-uclass.c | 2 +-
> > > 2 files changed, 31 insertions(+), 31 deletions(-)
> > >
> >
> > This is supposed to work by using of_live_active(), which is called from
> > ofnode_is_np(). Instead of changing all this code, is it possible to update
> > of_live_active() somehow?
> >
>
> I've been trying to figure out it but I think there is no way just changing of_live_active as the compiler does not discard branch statements with nested fixed return values. I was able to make it work just changing ofnode_is_np using pre-processor macro to set as false when OF_LIVE is not enabled, but not sure if it the right approach. I will take a deeper look to the entire OF code and maybe suggest a code refactoring.
Just returning false when OF_LIVE is not enabled seems fine to me. You
can never have an ofnode if livetree is not enabled.
Regards,
Simon
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-03-19 1:23 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-26 9:26 [U-Boot] [PATCH] dm: check OF_LIVE is enabled Ibai Erkiaga
2019-03-10 21:51 ` Simon Glass
2019-03-12 7:19 ` Ibai Erkiaga Elorza
2019-03-19 1:23 ` Simon Glass
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.