On Fri, Jul 22, 2016 at 02:10:22PM +0900, Masahiro Yamada wrote: > Hi. > > > 2016-07-21 21:29 GMT+09:00 B, Ravi <ravib...@ti.com>: > > Hi Marek > > > >>> The crash at dwc3 driver observed due to offset misalignment of > >>> structure members across files causing wrong code generation and leads > >>> to crash, the issue is found during dfu test. > >>> > >>> For instance, ther is is mismatch in code generation to access the > >>> address of structure member dwc->dep[0] in gadget.c and ep0.c. This > >>> leads to NULL pointer reference casuing the crash. The inclusion of > >>> common.h fixes the issue. > > > >>Please explain why this patch fixes the issue. > > > > Ok I will explain, due to the commit[1] the resource_size_t size has > > increased to 8 bytes (64 bit), compared to earlier 32 bit (4bytes) and the > > definition is moved to includes/linux/types.h from asm.h. Due to this > > change the code generated in gadget.c is correct, due to inclusion of right > > header file (common.h, which includes linux/types.h). Whereas, the ep0.c > > does not includes common.h, hence size of resources_size_t is 4 bytes, > > causing wrong offset code generated for structure members which includes > > resource_size_t, which leads to pointing to wrong offset location causing > > the crash. > > > I will explain it more precisely. > > > Both gadget.c and ep.c include <linux/types.h> > > See, > include/linux/types.h is included > from include/linux/kernel.h included > from drivers/usb/dwc3/ep0.c > > > So, <linux/types.h> is not a problem. > > > The root cause of problem is: > gadget.c include <config.h>, but ep0.c does not. > > > If <config.h> is not included, any CONFIGs > from the board header are defined. > > > The size of phys_addr_t depends on CONFIG_PHYS_64BIT > as you see in: > > > #ifdef CONFIG_PHYS_64BIT > typedef unsigned long long phys_addr_t; > typedef unsigned long long phys_size_t; > #else > /* DMA addresses are 32-bits wide */ > typedef unsigned long phys_addr_t; > typedef unsigned long phys_size_t; > #endif > > > So, phys_addr_t is 8 byte in gadget.c > and phys_addr_t is 4 byte in ep0.c > > > My commit changed resource_size_t > based on phys_addr_t, so it triggered > the problem for DWC3, which had already potentially existed. > > > CONFIGs in Kconfig are guaranteed to be defined for all files, > but CONFIGs in board headers are not. > > So we need to make sure to add > #include <common.h> (or #include <config.h>) > in each source file. > > > So, your patch is doing a right thing. > > I will issue my Reviewed-by when you update the git-log. > > > (Moving CONFIG_PHYS_64BIT is a right thing as well)
Can we move PHYS_64BIT to Kconfig instead here please? This is the kind of thing we should be able to select based on SoC / board. Thanks! -- Tom
signature.asc
Description: Digital signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
