From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek Vasut Date: Thu, 6 Apr 2017 17:33:09 +0200 Subject: [U-Boot] usb: dwc2: invalidate the dcache before starting the DMA In-Reply-To: References: <20170401065139.12084-1-eddie.cai.linux@gmail.com> <7475b51e-54a9-9180-6044-36dbd4f84b41@denx.de> <85c37627-137a-ff50-42da-9961a15e2cd3@denx.de> <64c3f2df-631e-ba35-c764-8993d9467062@denx.de> <8a824d16-86d7-2515-c2a4-6f5d32679e6e@denx.de> Message-ID: <9eb8c73a-d2f4-e792-9036-309a77322f58@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 04/06/2017 04:06 PM, Simon Glass wrote: > Hi Marek, > > On 6 April 2017 at 04:22, Marek Vasut wrote: >> On 04/06/2017 04:40 AM, Simon Glass wrote: >>> Hi Marek, >>> >>> On 5 April 2017 at 19:32, Marek Vasut wrote: >>>> On 04/06/2017 03:24 AM, Simon Glass wrote: >>>>> Hi Marek, >>>>> >>>>> On 5 April 2017 at 15:34, Marek Vasut wrote: >>>>>> On 04/05/2017 05:03 PM, Simon Glass wrote: >>>>>>> +Tom >>>>>>> >>>>>>> Hi Marek, >>>>>>> >>>>>>> On 5 April 2017 at 04:21, Marek Vasut wrote: >>>>>>>> On 04/05/2017 12:08 PM, Simon Glass wrote: >>>>>>>>> Hi Marek, >>>>>>>>> >>>>>>>>> On 5 April 2017 at 03:35, Marek Vasut wrote: >>>>>>>>>> On 04/05/2017 04:21 AM, Simon Glass wrote: >>>>>>>>>>> Hi, >>>>>>>>>>> >>>>>>>>>>> On 4 April 2017 at 19:26, Kever Yang wrote: >>>>>>>>>>>> Hi Eddie, >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> We should only need to do only one time cache operation for a buffer >>>>>>>>>>>> >>>>>>>>>>>> ready to do DMA transfer, so you need to remove another cache invalidate >>>>>>>>>>>> >>>>>>>>>>>> operation for the same buffer in the same function. >>>>>>>>>>> >>>>>>>>>>> I think this is a more general problem and might cause issues with >>>>>>>>>>> other drivers. So I have sent this patch: >>>>>>>>>>> >>>>>>>>>>> http://patchwork.ozlabs.org/patch/746917/ >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> This feels like papering over a problem though ... which will bite you >>>>>>>>>> later anyway. >>>>>>>>> >>>>>>>>> I believe the problem only happens because we have cached zero bytes >>>>>>>>> caused by this function. If the driver does the right thing (as dwc2.c >>>>>>>>> already does) then everything should be find from then on. >>>>>>>> >>>>>>>> And I think the driver is where this should be fixed ? That is, the >>>>>>>> driver should do the right thing and flush/invalidate caches correctly. >>>>>>>> >>>>>>>>> Notice that the problem does not happen without driver model, since >>>>>>>>> non-DM code in dwc2.c uses BSS for the buffers, which is zeroed with >>>>>>>>> the cache off. >>>>>>> >>>>>>> I'm not sure if you read the long and windy thread with Stefan B but >>>>>>> the upshot is that the driver is doing the right thing. >>>>>>> >>>>>>> If the driver were doing the memset() then you could make a case that >>>>>>> we should change the driver, but since DM is doing it and DM is >>>>>>> promising that DMA can be used on the buffer, I think the best place >>>>>>> for the fix is in DM. The driver should not need to change and neither >>>>>>> should any other driver when we convert it to DM. >>>>>>> >>>>>>> On that last point I really want to avoid having to change the caching >>>>>>> behaviour of drivers just to work with DM! >>>>>> >>>>>> So will the driver work with your patch and without DM ? I don't think >>>>>> so, so I think what Eddie's patch does is correct. I'd really like him >>>>>> to send a new version and apply that. >>>>> >>>>> Yes the driver work fine without DM and the code is correct. The >>>>> buffer is in BSS in that case and we don't have the cache problem. I >>>>> think we would have seen this problem before :-) >>>> >>>> I am seeing problems around this code and this patch makes sense to me, >>>> so I think this patch should go in as well ... >>> >>> OK, well up to you. What sort of problems? >> >> Some sticks are not detected for me and adding a small delay here >> magically fixes it. I suspect I'm hitting this cache issue. > > Is this with CONFIG_DM_USB or without? With. > Also does your platform have a write-through or write-back cache? I think WT, but I'm not 100% sure . I can check when I have access to the board . It's SoCFPGA, C-A9 . -- Best regards, Marek Vasut