On 10/01/2013 03:09 PM, Daniel Mack wrote: > Hi Sebastian, Hi Daniel, > sorry for the long delay, I got distracted by other things. No problem. > Well, I didn't merge the descriptors. Look again at my changes please. > > A simplified version of the code as it stands is: > > for (i = 0; i < DESCS_AREAS; i++) > cdd->cd = dma_alloc_coherent(dev, ..., &cdd->descs_phys, GFP_KERNEL); > > for (i = 0; i < DESCS_AREAS; i++) > dma_free_coherent(dev, mem_decs, cdd->cd, cdd->descs_phys); > > So you're effectively allocating and freeing the same pointer > DESCS_AREAS times, which is certainly not what you wanted. Well, no but since DESCS_AREAS is 1 it does not matter. > > And this just doesn't hit you because DESCS_AREAS is always 1: exactly. > > BUILD_BUG_ON(DESCS_AREAS != 1); > > > So, after all, my patch doesn't really change any of the runtime > behaviour. Consider it a cosmetic cleanup if you wish :) But it looks strange (in my opinion at least). But now, that I look again at it yes you moved the alloc out of the loop and replaced mem_decs with the computation. So nothing changed but you moved it outside. Right I think one desc area should be enough so I would just remove the for loop and DESCS_AREAS as well. How does this sound to you? > > > Thanks, > Daniel > Sebastian -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
