lkml.org 
[lkml]   [2013]   [Sep]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 30/51] DMA-API: dma: dw_dmac.c: convert to use dma_coerce_mask_and_coherent()
On Fri, Sep 20, 2013 at 05:26:46PM +0300, Andy Shevchenko wrote:
> On Thu, 2013-09-19 at 22:55 +0100, Russell King wrote:
> > This code sequence:
> > if (!pdev->dev.dma_mask) {
> > pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
> > pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
> > }
> > bypasses the architectures check on the DMA mask. It can be replaced
> > with dma_coerce_mask_and_coherent(), avoiding the direct initialization
> > of this mask.
> >
> > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> > ---
> > drivers/dma/dw/platform.c | 8 +++-----
> > 1 files changed, 3 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/dma/dw/platform.c b/drivers/dma/dw/platform.c
> > index e35d975..453822c 100644
> > --- a/drivers/dma/dw/platform.c
> > +++ b/drivers/dma/dw/platform.c
> > @@ -191,11 +191,9 @@ static int dw_probe(struct platform_device *pdev)
> > if (IS_ERR(chip->regs))
> > return PTR_ERR(chip->regs);
> >
> > - /* Apply default dma_mask if needed */
> > - if (!dev->dma_mask) {
> > - dev->dma_mask = &dev->coherent_dma_mask;
> > - dev->coherent_dma_mask = DMA_BIT_MASK(32);
> > - }
> > + err = dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> > + if (err)
> > + return err;
>
> I have at least one question.
>
> In case of new code you always assign dev->dma_mask.
>
> static inline int dma_coerce_mask_and_coherent(struct device *dev, u64
> mask)
> {
> dev->dma_mask = &dev->coherent_dma_mask;
> return dma_set_mask_and_coherent(dev, mask);
> }
>
> So, the question is how keep the initialized dma_mask (and should we do
> so by your opinion)?

Well, the way the DMA mask stuff is supposed to operate is:

- The device creator initializes the DMA mask to some default value.
- The driver then uses dma_set_mask() / dma_set_coherent_mask() /
dma_set_mask_and_coherent() to adjust the mask according to the
capabilities of the device, *even* if the mask is the same as the
default.

This is specified in the various DMA API documents.

So, in PCI land, it works like this:
- When a PCI device is created, it has its mask set to 32-bit.
- When a driver comes along
- if the device is capable of 64-bit addressing, it tries to set a
64-bit mask. If this fails, it tries to set a 32-bit mask and
turns off 64-bit DMA.
- if a device is not capable of 32-bit addressing but of a smaller
space (there are some PCI devices which can only do 31-bit) then
it tries to set that mask.
If the driver can't successfully set a mask, it should fail to
initialise.

This is where we should be headed with all drivers, and I would welcome
a patch for this driver to make it conform wrt the DMA API and DMA masks
in place of this patch.

Think of the coerse stuff as a middle-step to bring these types of issues
up to the fore.


\
 
 \ /
  Last update: 2013-09-20 17:01    [W:0.049 / U:34.796 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site