On Friday 07 March 2014, Santosh Shilimkar wrote:
> >> +
> >> + ret = dma_set_mask(dev, dma_mask);
> >> + if (ret < 0) {
> >> + dev_err(dev, "failed to set DMA mask %pad\n", &dma_mask);
> >> + dev->dma_mask = NULL;
> >> + return;
> >> + }
> >> +
> >> + dev_dbg(dev, "dma_pfn_offset(%#08lx) dma_mask(%pad)\n",
> >> + dev->dma_pfn_offset, dev->dma_mask);
> >> +
> >> + ret = dma_set_coherent_mask(dev, dma_mask);
> >
> > I think these 2 calls belong in the drivers, not here.
> >
> I also had same initial thought but Arnd mentioned that its a
> shared responsibility between ARCH and drivers. Driver which
> could be common between arches not always have the correct
> mask information and it can change based on which arch it
> is running.
>
> With some discussion back and forth, we thought updating
> the dma_mask while the device getting created, would be
> better place since we can find the arch capability at
> this centralise code and update it.
>
> Ofcourse its bit debatable as the question you asked is
> bit obvious as well. I let Arnd give his view here.
If we set the mask *here*, we probably don't want to call 'dma_set_mask', but
write to the mask directly, or we could call dma_coerce_mask_and_coherent(),
which is really for overriding the mask pointer and value at once in cases
where you absolutely know what it should be.
We do need to decide what interface we want to use in platform device drivers,
and I'm hoping that Russell has some idea which one he prefers:
a) Follow what we do for PCI devices: assume that we can do DMA_MASK(32)
on any device, and have drivers call dma_set_mask(DMA_MASK(64)) on devices
that would like to do more than that, or call e.g. dma_set_mask(DMA_MASK(28))
for devices that can do less than 32 bit, as given in the argument. This
approach would be most consistent with the way PCI works, but it doesn't
really work well for the case where the mask is less than 32-bit and the
device driver doesn't know that.
b) Never have to call dma_set_mask() for platform devices and assume that the
platform code sets it up correctly. This would probably be the simpler
solution, and I can't think of any downsides at the moment.
In either case we probably want to call something like dt_dma_configure()
from dma_set_mask() again to make sure that we stay within the limits
imposed by the bus structure.
Arnd
--
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
