On 07/22/2016 07:10 AM, 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) > > Thanks for the sensible and understandable explanation.
-- Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
