From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michal Simek Date: Thu, 21 Apr 2016 06:47:28 +0200 Subject: [U-Boot] [PATCH] dm: fdtdec: Check full path for alias In-Reply-To: References: <8e099aeaccd18d6dd51fb6252a4f12031f140e89.1460638938.git.michal.simek@xilinx.com> Message-ID: <57185B60.4020403@xilinx.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Simon, On 20.4.2016 16:41, Simon Glass wrote: > Hi Michal, > > On 14 April 2016 at 07:02, Michal Simek 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 >> --- >> >> 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