On Mon, Oct 24, 2011 at 09:58:39AM +0200, Grant Likely wrote: > On Mon, Oct 24, 2011 at 9:49 AM, Felipe Balbi <balbi@xxxxxx> wrote: > > On Mon, Oct 24, 2011 at 09:41:24AM +0200, Grant Likely wrote: > >> On Mon, Oct 24, 2011 at 09:42:28AM +0300, Felipe Balbi wrote: > >> > then I can drop the dwc3 platform_device allocation and all of that > >> > resource copying, etc. > >> > > >> > What do you think ? > >> > >> Looks reasonable to me. of_platform_populate() should be able to > >> handle the device generation for you here. > > > > Ok cool I looking into that and it handles everything I need. There are > > only three issues which I see: > > > > a) it hardcoded DMA mask to 32-bit. Right ? > > b) it's not using dma_set_coherent_mask() > > c) in case parent is a valid pointer, shouldn't it copy DMA mask from > > parent ? > > > > I mean (doesn't solve (a) above): > > > > diff --git a/drivers/of/platform.c b/drivers/of/platform.c > > index ed5a6d3..172d4a9 100644 > > --- a/drivers/of/platform.c > > +++ b/drivers/of/platform.c > > @@ -204,7 +204,12 @@ struct platform_device *of_platform_device_create_pdata( > > #if defined(CONFIG_MICROBLAZE) > > dev->archdata.dma_mask = 0xffffffffUL; > > #endif > > - dev->dev.coherent_dma_mask = DMA_BIT_MASK(32); > > + > > + if (parent) > > + dma_set_coherent_mask(&dev->dev, parent->coherent_dma_mask); > > + else > > + dma_set_coherent_mask(&dev->dev, DMA_BIT_MASK(32)); > > + > > Right, this does need to be fixed. The existing code just matched > what the historical powerpc code did, but it is certainly not correct. should I send patch above correctly ? Or do you want to also solve 32-bit coherent mask altogether ? What are your plans for that ? Add a separate property to pass coherent_mask size (32-bit, 64-bit, etc) ? -- balbi
Attachment:
signature.asc
Description: Digital signature
