Re: [PATCH v1 1/2] sata_dwc_460ex: move to generic DMA driver
Andy Shevchenko writes:
> On Sun, 2015-11-22 at 13:03 +, Måns Rullgård wrote:
>> Andy Shevchenko 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
m...@mansr.com
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v1 1/2] sata_dwc_460ex: move to generic DMA driver
On Sun, 2015-11-22 at 13:03 +, Måns Rullgård wrote:
> Andy Shevchenko 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
Intel Finland Oy
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v1 1/2] sata_dwc_460ex: move to generic DMA driver
Andy Shevchenko 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.
>
> Signed-off-by: Andy Shevchenko
> ---
> drivers/ata/sata_dwc_460ex.c | 736
> +++
> 1 file changed, 122 insertions(+), 614 deletions(-)
>
[...]
> @@ -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. Nothing
ever allocates hsdev->dma, so it can't possibly work.
Also, has anyone given any thought to getting rid of the dependency on
the DW DMA controller? 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. My
goal is to get this driver working with another chip using the same SATA
controller but a different DMA engine.
--
Måns Rullgård
m...@mansr.com
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
