On 04/06/2017 04:40 AM, Simon Glass wrote: > Hi Marek, > > On 5 April 2017 at 19:32, Marek Vasut <[email protected]> wrote: >> On 04/06/2017 03:24 AM, Simon Glass wrote: >>> Hi Marek, >>> >>> On 5 April 2017 at 15:34, Marek Vasut <[email protected]> wrote: >>>> On 04/05/2017 05:03 PM, Simon Glass wrote: >>>>> +Tom >>>>> >>>>> Hi Marek, >>>>> >>>>> On 5 April 2017 at 04:21, Marek Vasut <[email protected]> wrote: >>>>>> On 04/05/2017 12:08 PM, Simon Glass wrote: >>>>>>> Hi Marek, >>>>>>> >>>>>>> On 5 April 2017 at 03:35, Marek Vasut <[email protected]> wrote: >>>>>>>> On 04/05/2017 04:21 AM, Simon Glass wrote: >>>>>>>>> Hi, >>>>>>>>> >>>>>>>>> On 4 April 2017 at 19:26, Kever Yang <[email protected]> >>>>>>>>> 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. -- Best regards, Marek Vasut _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
