On 02.01.2014 19:03, Arnd Bergmann wrote: > On Thursday 02 January 2014 18:49:23 Florian Meier wrote: >> Add support for DMA controller of BCM2835 as used in the Raspberry Pi. >> Currently it only supports cyclic DMA. > > Looks very nice. Just a few details I noticed: > >> +#if defined(CONFIG_OF) >> +static const struct of_device_id bcm2835_dma_of_match[] = { >> + { .compatible = "brcm,bcm2835-dma", }, >> + {}, >> +}; >> +MODULE_DEVICE_TABLE(of, bcm2835_dma_of_match); >> +#endif > > I doubt we are going to see non-DT versions of this driver, so the #ifdef > can just get removed here. As already explained in previous versions of this patch thread, there is a non-DT version in a downstream kernel and the more I make this patch incompatible with non-DT, the harder it gets to upstream the remaining stuff. I hope this is not something that blocks this driver from getting accepted. > [...] > This can now be simplified using the dma_get_any_slave_channel() interface > taht Stephen Warren introduced. > [...] > dma_set_mask_and_coherent() Sigh, the API is developing faster than I can keep track with updating this patch. I hope some day I will be faster.... When Russell told me about the second one before, it hoped that I can avoid merging different trees on my own, but it seems that you want me to do that ;-) >> + if (pdev->dev.of_node) { >> + /* Device-tree DMA controller registration */ >> + rc = of_dma_controller_register(pdev->dev.of_node, >> + bcm2835_dma_xlate, od); >> + if (rc) { >> + dev_err(&pdev->dev, "Failed to register DMA controller\n"); >> + goto err_no_dma; >> + } >> + } > > If pdev->dev.of_node isn't set, you didn't get here, so the if() can be removed. Why not? (In the case of non-DT initialization) Greetings, Florian -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html