Daniel Mack <daniel@xxxxxxxxxx> writes: > Hi Robert, > > Thanks for pushing this topic :) > One minor nit: > >> +int mmp_pdma_toggle_reserved_channel(int legacy_channel) >> +{ >> + if (legacy_unavailable & (1 << legacy_channel)) >> + return -EBUSY; >> + legacy_reserved ^= 1 << legacy_channel; >> + return 0; >> +} >> +EXPORT_SYMBOL_GPL(mmp_pdma_toggle_reserved_channel); > > My concern is that if pxa_request_dma() is called more than once for > whatever reason by a legacy implementation, the toggled bit mask might > get out of sync. This is not possible. The first call to pxa_request_dma() sets dma_channels[i].name to a non-NULL value. The second call to pxa_request_dma() cannot take the same i as !dma_channels[i].name is not fullfilled. > As we know exactly on the caller site what we want to achieve, let's make the > API explicit with something like: > > int mmp_pdma_set_reserved_channel(int legacy_channel, bool reserved) Even if I have no strong opinion about it, I'll let the patch as it is, unless you really want me to add the reserved parameter, in which case I'll release a v3. Cheers. -- Robert -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html