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

["Måns Rullgård" <mans@mansr.com>]

Andy Shevchenko <andriy.shevchenko@linux.intel.com> writes:

> On Sun, 2015-11-22 at 13:03 +0000, Måns Rullgård wrote:
>> Andy Shevchenko <andriy.shevchenko@linux.intel.com> 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).

It already uses the dmaengine API to do the transfers.  To make it
generic, two things need to change:

1. The DMA controller should get its own DT node instead of the SATA
   driver calling dw_dma_probe() directly.

2. The SATA driver can then use dma_request_slave_channel() and a "dmas"
   DT property instead of dma_request_channel() and filter function.

The changes are easy to make as such.  The only problem is that we need
to preserve compatibility with old device trees.

> 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.

See Documentation/devicetree/bindings/dma/dma.txt

>> 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.

What's your interest in this driver if you don't have the hardware?

-- 
Måns Rullgård
mans@mansr.com