Hi, On Wed, May 9, 2018 at 1:55 AM, wlf <wulf@xxxxxxxxxxxxxx> wrote: >>>>> + } else if (hsotg->params.host_dma) { >>>> >>>> Are you sure this is "else if"? Can't you have descriptor DMA enabled >>>> in the controller and still need to do a normal DMA transfer if you >>>> plug in a hub? Seems like this should be just "if". >>> >>> Sorry, I don't understand the case "have descriptor DMA enabled in the >>> controller and still need to do a normal DMA transfer". But maybe it >>> still >>> has another problem if just use "if" here, because it will create kmem >>> caches for Slave mode which actually doesn't need aligned DMA buf. >> >> So right now your code looks like: >> >> if (hsotg->params.dma_desc_enable || >> hsotg->params.dma_desc_fs_enable) { >> ... >> ... >> } else if (hsotg->params.host_dma) { >> ... >> } >> >> >> I've never played much with "descriptor DMA" on dwc2 because every >> time I enabled it (last I tried was years ago) a whole bunch of >> peripherals stopped working and I didn't dig (I just left it off). >> Thus, I'm no expert on descriptor DMA. ...that being said, my >> understanding is that if you enable descriptor DMA in the controller >> then it will enable a smarter DMA mode for some of the transfers. >> IIRC Descriptor DMA mode specifically can't handle splits. Is my >> memory correct there? Presumably that means that when you enable >> descriptor DMA in the controller the driver will automatically fall >> back to normal Address DMA mode if things get too complicated. When >> it falls back to Address DMA mode, presumably dwc2_hcd_init() won't >> get called again. That means that any data structures that are needed >> for Address DMA need to be allocated in dwc2_hcd_init() even if >> descriptor DMA is enabled because we might need to fall back to >> Address DMA. >> >> Thus, I'm suggesting changing the code to: >> >> if (hsotg->params.dma_desc_enable || >> hsotg->params.dma_desc_fs_enable) { >> ... >> ... >> } >> >> if (hsotg->params.host_dma) { >> ... >> } >> >> >> ...as I said, I'm no expert and I could just be confused. If >> something I say seems wrong please correct me. > > Ah, I got it. Thanks for your detailed explanation. Although I don't know if > it's possible that dwc2 driver automatically fall back to normal Address DMA > mode when desc DMA work abnormally with split, I agree with your suggestion. > I'll fix it in V4 patch. Hmm, I guess you're right. I did a quick search and I can't find any evidence of a fallback like this. Maybe I dreamed that. I found some old comment in the commit history that said: /* * Disable descriptor dma mode by default as the HW can support * it, but does not support it for SPLIT transactions. * Disable it for FS devices as well. */ ...and it looks there's currently nobody using descriptor DMA anyway (assuming I read the code correctly). It does seem like people were ever going to turn it on then it would have to be dynamic as I was dreaming it was... -Doug -- 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