On Wed, Feb 23, 2011 at 9:54 AM, Stephen Warren <swarren@xxxxxxxxxx> wrote: > Colin Cross wrote at Wednesday, February 23, 2011 10:38 AM: >> >> On Wed, Feb 23, 2011 at 9:29 AM, Stephen Warren <swarren@xxxxxxxxxx> wrote: >> > tegra_dma_init currently simply bails out early if any initialization fails. >> > This skips various data-structure initialization. In turn, this means that >> > tegra_dma_allocate_channel can still hand out channels. In this case, when >> > tegra_dma_free_channel is called, which calls tegra_dma_cancel, the walking >> > on ch->list will OOPS since the list's next/prev pointers may still be >> > NULL. >> > >> > To solve this: >> > * Mark all possible channels as in-use before doing anything else in init. >> > * Only mark a channel as free once all channel-related initialization has >> > completed. >> > >> > This prevents allocate_channel from handing out uninitialized channels. >> > >> > There is still one small hole; allocate_channel can't check the usage array >> > for the shared channel, since this channel is permanently marked in-use. >> > This could be solved using an explicit "init OK" flag that allocate_channel >> > could check. >> >> If we still need an init complete flag, why not skip this patch and >> just add that? > > I tend to like the cleanup in this patch; it simplifies the channel_usage > initialization. I'll let you make the call on whether to take this or not. Ok, it seems like a reasonable cleanup. Can I request a few changes? Get rid of the redundant memsets Replace the initial for loop to set all the bits with bitmap_set Might as well add the init complete flag to this patch as well. You could also do it by checking the channel_usage bit for the shared channel, and never setting it in tegra_dma_allocate_channel. > I can certainly throw together a second patch, or modify this patch, to also > use an init_ok flag to completely solve the shared channel case too though. > > -- > nvpublic > > -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html