Re: [RFC PATCH powerpc] Fix a dma_mask issue of vio
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: 2013-11-21 00:22:43
On Thu, 2013-11-21 at 00:08 +0000, Russell King - ARM Linux wrote:
On Thu, Nov 21, 2013 at 11:01:42AM +1100, Benjamin Herrenschmidt wrote:quoted
On Wed, 2013-11-20 at 23:23 +0000, Russell King - ARM Linux wrote:quoted
Li Zong's patch works around the issue of a failing dma_set_mask(), but as I've already said elsewhere, the real fix is to get whatever created the struct device to initialise the dev->dma_mask with a bus default. Using dma_coerce_xxx() merely makes the problem "go away" papering over the issue - it's fine to do it this way, but someone should still fix the broken code creating these devices...Ok, they are created by the vio bus core, so it should be doing the job here of setting the dma_mask pointer to a proper value. Li, can you take care of that ? Look at other bus types we have in there such as the macio bus etc...Oh, hang on a moment, this is the "bus" code. In which case, the question becomes: do vio devices ever need to have a separate streaming DMA mask from a coherent DMA mask? If not, then something like the following is what's needed here, and I should've never have used dma_set_mask_and_coherent().
No, a single mask.
quoted hunk ↗ jump to hunk
dma_set_mask_and_coherent() (and the other dma_set_mask() functions) are really supposed to be used by drivers only. arch/powerpc/kernel/vio.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)diff --git a/arch/powerpc/kernel/vio.c b/arch/powerpc/kernel/vio.c index e7d0c88f621a..d771778f398e 100644 --- a/arch/powerpc/kernel/vio.c +++ b/arch/powerpc/kernel/vio.c@@ -1419,7 +1419,8 @@ struct vio_dev *vio_register_device_node(struct device_node *of_node) /* needed to ensure proper operation of coherent allocations * later, in case driver doesn't set it explicitly */ - dma_set_mask_and_coherent(&viodev->dev, DMA_BIT_MASK(64)); + viodev->dev.coherent_dma_mask = DMA_BIT_MASK(64); + viodev->dev.dma_mask = &viodev->dev.coherent_dma_mask; } /* register with generic device framework */
Right that's exactly what I had in mind. Li, can you test this please ? The previous "fix" using dma_coerce_mask_and_coherent() is already on its way to Linus, so we'll rework the above patch to undo it but for now please test. Cheers, Ben.