[U-Boot] [PATCH v2 01/11] fdt: Allow ft_board_setup() to report failure

[Simon Glass]

On 24 October 2014 21:56, Simon Glass <sjg at chromium.org> wrote:
> Hi Tom,
>
> On 24 October 2014 12:04, Tom Rini <trini at ti.com> wrote:
>> On Thu, Oct 23, 2014 at 06:58:47PM -0600, Simon Glass wrote:
>>
>>> This function can fail if the device tree runs out of space. Rather than
>>> silently booting with an incomplete device tree, allow the failure to be
>>> detected.
>>>
>>> Unfortunately this involves changing a lot of places in the code. I have
>>> not changed behvaiour to return an error where one is not currently
>>> returned, to avoid unexpected breakage.
>>>
>>> Eventually it would be nice to allow boards to register functions to be
>>> called to update the device tree. This would avoid all the many functions
>>> to do this. However it's not clear yet if this should be done using driver
>>> model or with a linker list. This work is left for later.
>> [snip]
>>> diff --git a/board/freescale/p1_p2_rdb/p1_p2_rdb.c b/board/freescale/p1_p2_rdb/p1_p2_rdb.c
>>> index aba4f53..19fef50 100644
>>> --- a/board/freescale/p1_p2_rdb/p1_p2_rdb.c
>>> +++ b/board/freescale/p1_p2_rdb/p1_p2_rdb.c
>>> @@ -6,6 +6,7 @@
>>>
>>>  #include <common.h>
>>>  #include <command.h>
>>> +#include <errno.h>
>>>  #include <asm/processor.h>
>>>  #include <asm/mmu.h>
>>>  #include <asm/cache.h>
>>> @@ -234,7 +235,7 @@ int board_eth_init(bd_t *bis)
>>>  #if defined(CONFIG_OF_BOARD_SETUP)
>>>  extern void ft_pci_board_setup(void *blob);
>>>
>>> -void ft_board_setup(void *blob, bd_t *bd)
>>> +int ft_board_setup(void *blob, bd_t *bd)
>>>  {
>>>       const char *soc_usb_compat = "fsl-usb2-dr";
>>>       int err, usb1_off, usb2_off;
>>> @@ -263,39 +264,41 @@ void ft_board_setup(void *blob, bd_t *bd)
>>>               int off = fdt_node_offset_by_compatible(blob, -1,
>>>                       soc_elbc_compat);
>>>               if (off < 0) {
>>> -                     printf("WARNING: could not find compatible node"
>>> -                             " %s: %s.\n", soc_elbc_compat,
>>> -                             fdt_strerror(off));
>>> -                             return;
>>> +                     printf("WARNING: could not find compatible node %s\n",
>>> +                            soc_elbc_compat);
>>> +                     return off;
>>>               }
>>>               err = fdt_del_node(blob, off);
>>>               if (err < 0) {
>>> -                     printf("WARNING: could not remove %s: %s.\n",
>>> -                             soc_elbc_compat, fdt_strerror(err));
>>> +                     printf("WARNING: could not remove %s\n",
>>> +                            soc_elbc_compat);
>>> +                     return err;
>>>               }
>>> -             return;
>>> +             return 0;
>>>       }
>>>  #endif
>>>       /* Delete USB2 node as it is muxed with eLBC */
>>>       usb1_off = fdt_node_offset_by_compatible(blob, -1,
>>>               soc_usb_compat);
>>>       if (usb1_off < 0) {
>>> -             printf("WARNING: could not find compatible node"
>>> -                     " %s: %s.\n", soc_usb_compat,
>>> -                     fdt_strerror(usb1_off));
>>> -             return;
>>> +             printf("WARNING: could not find compatible node %s\n",
>>> +                    soc_usb_compat);
>>> +             return usb1_off;
>>>       }
>>>       usb2_off = fdt_node_offset_by_compatible(blob, usb1_off,
>>>                       soc_usb_compat);
>>>       if (usb2_off < 0) {
>>> -             printf("WARNING: could not find compatible node"
>>> -                     " %s: %s.\n", soc_usb_compat,
>>> -                     fdt_strerror(usb2_off));
>>> -             return;
>>> +             printf("WARNING: could not find compatible node %s\n",
>>> +                    soc_usb_compat);
>>> +             return usb2_off;
>>>       }
>>>       err = fdt_del_node(blob, usb2_off);
>>> -     if (err < 0)
>>> -             printf("WARNING: could not remove %s: %s.\n",
>>> -                     soc_usb_compat, fdt_strerror(err));
>>> +     if (err < 0) {
>>> +             printf("WARNING: could not remove %s\n", soc_usb_compat);
>>> +             return err;
>>> +     }
>>> +
>>> +     return 0;
>>>  }
>>> +
>>>  #endif
>>> diff --git a/board/freescale/p1_p2_rdb_pc/p1_p2_rdb_pc.c b/board/freescale/p1_p2_rdb_pc/p1_p2_rdb_pc.c
>>> index a6756c6..66bc041 100644
>>> --- a/board/freescale/p1_p2_rdb_pc/p1_p2_rdb_pc.c
>>> +++ b/board/freescale/p1_p2_rdb_pc/p1_p2_rdb_pc.c
>>> @@ -6,6 +6,7 @@
>>>
>>>  #include <common.h>
>>>  #include <command.h>
>>> +#include <errno.h>
>>>  #include <hwconfig.h>
>>>  #include <pci.h>
>>>  #include <i2c.h>
>>> @@ -424,7 +425,7 @@ static void fdt_board_fixup_qe_pins(void *blob)
>>>  #endif
>>>
>>>  #ifdef CONFIG_OF_BOARD_SETUP
>>> -void ft_board_setup(void *blob, bd_t *bd)
>>> +int ft_board_setup(void *blob, bd_t *bd)
>>>  {
>>>       phys_addr_t base;
>>>       phys_size_t size;
>>> @@ -459,17 +460,17 @@ void ft_board_setup(void *blob, bd_t *bd)
>>>               int off = fdt_node_offset_by_compatible(blob, -1,
>>>                               soc_elbc_compat);
>>>               if (off < 0) {
>>> -                     printf("WARNING: could not find compatible node %s: %s.\n",
>>> -                            soc_elbc_compat,
>>> -                            fdt_strerror(off));
>>> -                             return;
>>> +                     printf("WARNING: could not find compatible node %s\n",
>>> +                            soc_elbc_compat);
>>> +                     return off;
>>>               }
>>>               err = fdt_del_node(blob, off);
>>>               if (err < 0) {
>>> -                     printf("WARNING: could not remove %s: %s.\n",
>>> -                            soc_elbc_compat, fdt_strerror(err));
>>> +                     printf("WARNING: could not remove %s\n",
>>> +                            soc_elbc_compat);
>>> +                     return err;
>>>               }
>>> -             return;
>>> +             return 0;
>>>       }
>>>  #endif
>>>
>>> @@ -477,24 +478,23 @@ void ft_board_setup(void *blob, bd_t *bd)
>>>       usb1_off = fdt_node_offset_by_compatible(blob, -1,
>>>               soc_usb_compat);
>>>       if (usb1_off < 0) {
>>> -             printf("WARNING: could not find compatible node %s: %s.\n",
>>> -                    soc_usb_compat,
>>> -                    fdt_strerror(usb1_off));
>>> -             return;
>>> +             printf("WARNING: could not find compatible node %s\n",
>>> +                    soc_usb_compat);
>>> +             return usb1_off;
>>>       }
>>>       usb2_off = fdt_node_offset_by_compatible(blob, usb1_off,
>>>                       soc_usb_compat);
>>>       if (usb2_off < 0) {
>>> -             printf("WARNING: could not find compatible node %s: %s.\n",
>>> -                    soc_usb_compat,
>>> -                    fdt_strerror(usb2_off));
>>> -             return;
>>> +             printf("WARNING: could not find compatible node %s\n",
>>> +                    soc_usb_compat);
>>> +             return usb2_off;
>>>       }
>>>       err = fdt_del_node(blob, usb2_off);
>>>       if (err < 0) {
>>> -             printf("WARNING: could not remove %s: %s.\n",
>>> -                    soc_usb_compat, fdt_strerror(err));
>>> +             printf("WARNING: could not remove %s\n", soc_usb_compat);
>>> +             return err;
>>>       }
>>>
>>> +     return 0;
>>>  }
>>>  #endif
>>
>> In both of these boards you add <errno.h> but don't make use of it, did
>> you intend to make something return -FDT_ERR_NOTFOUND and then not need
>> to after all?  Thanks!
>
> Actually -ENOENT I think, but it was a v1 mistake and I removed it. I
> can remove these headers also.

Fixed, and applied to u-boot-fdt.