[U-Boot] [PATCH] dm: fdtdec: Check full path for alias
Michal Simek
michal.simek at xilinx.com
Thu Apr 21 06:47:28 CEST 2016
Hi Simon,
On 20.4.2016 16:41, Simon Glass wrote:
> 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?
Yes there are two i2c IP cores. It means 2 main i2c busses.
Here is full i2c description
arch/arm/dts/zynqmp-zcu102.dts
I have pushed my hacky/testing tree here with these changes.
u-boot-microblaze.git xnext/i2c
>
>> 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.
i2c0 and i2c1 are that main one.
>
>>
>> ---
>> 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.
How to do it better?
>
>>
>> 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?
Error code?
>
>> +
>> + /* 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
>>
Thanks,
Michal
More information about the U-Boot
mailing list