[U-Boot] [PATCH] dm: fdtdec: Check full path for alias

[Simon Glass]

Hi Michal,

On 14 April 2016 at 07:02, Michal Simek <michal.simek at xilinx.com> wrote:
> Fix fdtdec_get_alias_seq() which is not checking full path to certain
> node and it incorrectly provides incorrect seq number.
> Checking full path ensure that if alias is present correct seq number is
> return.
> This problem was found on ZynqMP zcu102 where are several I2C muxes
> where the first bus name is i2c at 0 and for following muxes incorrect seq
> numbers are return.
>
> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
> ---
>
> I am not sure if there is any macro for max alias length which I can use
> instead of new macro.
>
> Before:
> ZynqMP> i2c bus
> Bus 0:  i2c at ff020000
>    20: gpio at 20, offset len 1, flags 0
>    21: gpio at 21, offset len 1, flags 0
>    75: i2cswitch at 75, offset len 1, flags 0
> Bus 750:        i2c at 0
> Bus 751:        i2c at 1
> Bus 752:        i2c at 2
> Bus 1:  i2c at ff030000
>    74: i2cswitch at 74, offset len 1, flags 0
>    75: i2cswitch at 75, offset len 1, flags 0
> Bus 750:        i2c at 0

Sorry I'm a bit confused by this. Do you have two i2c at 0 buses?

> Bus 751:        i2c at 1
> Bus 752:        i2c at 2
> Bus 1743:       i2c at 3
> Bus 1744:       i2c at 4
> Bus 750:        i2c at 0
> Bus 751:        i2c at 1
> Bus 752:        i2c at 2
> Bus 1743:       i2c at 3
> Bus 1744:       i2c at 4
> Bus 1755:       i2c at 5
> Bus 1756:       i2c at 6
> Bus 1757:       i2c at 7
>
> After:
> ZynqMP> i2c bus
> Bus 0:  i2c at ff020000
>    20: gpio at 20, offset len 1, flags 0
>    21: gpio at 21, offset len 1, flags 0
>    75: i2cswitch at 75, offset len 1, flags 0
> Bus 750:        i2c at 0
> Bus 751:        i2c at 1
> Bus 752:        i2c at 2
> Bus 1:  i2c at ff030000
>    74: i2cswitch at 74, offset len 1, flags 0
>    75: i2cswitch at 75, offset len 1, flags 0
> Bus 1740:       i2c at 0
> Bus 1741:       i2c at 1
> Bus 1742:       i2c at 2
> Bus 1743:       i2c at 3
> Bus 1744:       i2c at 4
> Bus 1750:       i2c at 0
> Bus 1751:       i2c at 1
> Bus 1752:       i2c at 2
> Bus 1753:       i2c at 3
> Bus 1754:       i2c at 4
> Bus 1755:       i2c at 5
> Bus 1756:       i2c at 6
> Bus 1757:       i2c at 7
>
> Which reflects my aliases
> i2c0 = &i2c0;
> i2c750 = &i2c0_75_0;
> i2c751 = &i2c0_75_1;
> i2c752 = &i2c0_75_2;
> i2c1 = &i2c1;
> i2c1740 = &i2c1_74_0;
> i2c1741 = &i2c1_74_1;
> i2c1742 = &i2c1_74_2;
> i2c1743 = &i2c1_74_3;
> i2c1744 = &i2c1_74_4;
> i2c1750 = &i2c1_75_0;
> i2c1751 = &i2c1_75_1;
> i2c1752 = &i2c1_75_2;
> i2c1753 = &i2c1_75_3;
> i2c1754 = &i2c1_75_4;
> i2c1755 = &i2c1_75_5;
> i2c1756 = &i2c1_75_6;
> i2c1757 = &i2c1_75_7;

I only see one in you aliases above.

>
> ---
>  lib/fdtdec.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/lib/fdtdec.c b/lib/fdtdec.c
> index 70acc29c924d..79efcf0cd6b0 100644
> --- a/lib/fdtdec.c
> +++ b/lib/fdtdec.c
> @@ -506,6 +506,8 @@ int fdtdec_add_aliases_for_id(const void *blob, const char *name,
>         return num_found;
>  }
>
> +#define FDT_ALIAS_PATH_LEN     64
> +
>  int fdtdec_get_alias_seq(const void *blob, const char *base, int offset,
>                          int *seqp)
>  {
> @@ -514,9 +516,13 @@ int fdtdec_get_alias_seq(const void *blob, const char *base, int offset,
>         int find_namelen;
>         int prop_offset;
>         int aliases;
> +       char buf[FDT_ALIAS_PATH_LEN];
> +
> +       fdt_get_path(blob, offset, buf, sizeof(buf));

Eek! That is pretty expensive.

>
>         find_name = fdt_get_name(blob, offset, &find_namelen);
> -       debug("Looking for '%s' at %d, name %s\n", base, offset, find_name);
> +       debug("Looking for '%s' at %d, name %s, buf %s\n",
> +             base, offset, find_name, buf);
>
>         aliases = fdt_path_offset(blob, "/aliases");
>         for (prop_offset = fdt_first_property_offset(blob, aliases);
> @@ -533,6 +539,13 @@ int fdtdec_get_alias_seq(const void *blob, const char *base, int offset,
>                     strncmp(name, base, base_len))
>                         continue;
>
> +               if (len >= FDT_ALIAS_PATH_LEN)
> +                       printf("Too long path in aliases node\n");

Can you return an error and make it debug() here?

> +
> +               /* Check full path first not to point to incorrect alias */
> +               if (strncmp(prop, buf, min(len, FDT_ALIAS_PATH_LEN)))
> +                       continue;
> +
>                 slash = strrchr(prop, '/');
>                 if (strcmp(slash + 1, find_name))
>                         continue;
> --
> 1.9.1
>

Regards,
Simon