On 01.10.2013 18:22, Sebastian Andrzej Siewior wrote: > On 10/01/2013 03:09 PM, Daniel Mack wrote: >> 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, 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). I still disagree, but maybe that doesn't matter any more, because ... > 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? That sound's like a good idea. Everything that make the driver smaller and easier to understand is certainly a good thing :) So we can drop this patch favor of your cleanup. However, I appreciate if you did it on top of the second round of patches I sent today please, because my resume() implementation uses DESCS_AREAS as well. Thanks, Daniel -- 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
