Subject: Re: [PATCH v1 1/2] sata_dwc_460ex: move to generic DMA driver

From: Andy Shevchenko <[email-masked]>
Date: Mon, 23 Nov 2015 12:10:41 +0200

On Sun, 2015-11-22 at 13:03 +0000, Måns Rullgård wrote:
> Andy Shevchenko <[email-masked]> writes:
> 
> > The SATA implementation based on two actually different devices,
> > i.e. SATA and
> > DMA controllers.
> > 
> > For Synopsys DesignWare DMA we have already a generic
> > implementation of the
> > driver. Thus, the patch converts the code to use DMAEngine
> > framework and
> > dw_dmac driver.
> > 
> > In future it will be better to split the devices inside DTS as well
> > like it's
> > done on other platforms.

> > @@ -1721,16 +1227,16 @@ static int sata_dwc_probe(struct
> > platform_device *ofdev)
> >  		   idr, ver[0], ver[1], ver[2]);
> > 
> >  	/* Get SATA DMA interrupt number */
> > -	irq = irq_of_parse_and_map(np, 1);
> > -	if (irq == NO_IRQ) {
> > +	hsdev->dma->irq = irq_of_parse_and_map(np, 1);
> 
> This doesn't look like it has been more than compile-tested.

Yes, that's true, my question [1] was a crying in the wilderness.

>   Nothing
> ever allocates hsdev->dma, so it can't possibly work.

You are right.

> 
> Also, has anyone given any thought to getting rid of the dependency
> on
> the DW DMA controller?

How? Before it was even more harder link to it (embedded routines).

On the other hand you may introduce dma_ops and use them like it's
done, for example, in spi-dw*.c

>   Presumably support for old device trees would
> need to be retained for compatibility.  Maybe checking for a "dmas"
> property and falling back on the current behaviour if it's missing.  

I didn't get how DT is related to DW or any other DMAC used with this
SATA controller.

> My
> goal is to get this driver working with another chip using the same
> SATA
> controller but a different DMA engine.

It would be nice to eventually bring this working with generic DMA
Engine API. Please, keep me in Cc list regarding this driver.

[1] https://lkml.org/lkml/2014/12/12/547

-- 
Andy Shevchenko <[email-masked]>
Intel Finland Oy