Hi Marek, On 5 April 2017 at 19:32, Marek Vasut <ma...@denx.de> wrote: > On 04/06/2017 03:24 AM, Simon Glass wrote: >> Hi Marek, >> >> On 5 April 2017 at 15:34, Marek Vasut <ma...@denx.de> wrote: >>> On 04/05/2017 05:03 PM, Simon Glass wrote: >>>> +Tom >>>> >>>> Hi Marek, >>>> >>>> On 5 April 2017 at 04:21, Marek Vasut <ma...@denx.de> wrote: >>>>> On 04/05/2017 12:08 PM, Simon Glass wrote: >>>>>> Hi Marek, >>>>>> >>>>>> On 5 April 2017 at 03:35, Marek Vasut <ma...@denx.de> wrote: >>>>>>> On 04/05/2017 04:21 AM, Simon Glass wrote: >>>>>>>> Hi, >>>>>>>> >>>>>>>> On 4 April 2017 at 19:26, Kever Yang <kever.y...@rock-chips.com> 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? > >>> >>> If this also needs to be fixed in DM, so be it. >> >> OK. >> >> Regards, >> Simon Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
