From: Dan Williams on
jassi brar wrote:
>> Perhaps Joonyoung can simply port over the stuff
>> you need to this driver if you show your code.
> Having worked on Samsung SoCs(with PL330 DMAC) based products, I would be
> _very_ surprised if any user found this implementation useful.
> Let alone testing, this implementation can't even explain usability
> for fast peripherals
> with shallow FIFOs. I didn't give feedback for this patch because I am
> not sure if this
> is the right way to go at all.

This is the wrong attitude. If it were not for a simple oversight
Joonyoung's driver would already be upstream for the past two kernel
releases. So you need to work together to improve that driver to
incorporate what you need.

It sounds like you just need to add an extension for the arch specific
dma api. At first glance this can mimic the approach taken by
Nobuhiro-san with the shdma driver.

--
Dan

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Linus Walleij on
2010/3/25 jassi brar <jassisinghbrar(a)gmail.com>:

> My approach is to write a separate PL330 core driver as the backend which
> can be reused by any DMA API implementer driver. That will avoid
> having two copies of the PL330 driver, among other benefits.

Seems like a rather good approach.

> And if this patch is accepted, there
> _will_ exist two copies of the PL330 driver -- one in drivers/dma/pl330_dmac.c
> and another in arch/arm/plat-samsung/. Only the former will be lying unused
> until some other SoC vendor decided to use PL330, because S3C has come too
> long a way to change its drivers to driver/dma/ API and modify DMA
> drivers for every SoC.

What's wrong with merging them later then? Refactoring FTW.

> I have the pl330-core part almost ready, but i need time to implement
> some _testable_
> implementation of the scheme. If maintainers want to see structure of
> my code, I can
> share it too, but I think I pretty much made it clear.

Why not just post it on the list? I'm curious! Since I'm working on a PrimeCell
DMA API I would love to look at PrimeCell DMA engine drivers.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: jassi brar on
On Fri, Mar 26, 2010 at 12:13 AM, Dan Williams <dan.j.williams(a)intel.com> wrote:
> jassi brar wrote:
>>>
>>> Perhaps Joonyoung can simply port over the stuff
>>> you need to this driver if you show your code.
>>
>> Having worked on Samsung SoCs(with PL330 DMAC) based products, I would be
>> _very_ surprised if any user found this implementation useful.
>> Let alone testing, this implementation can't even explain usability
>> for fast peripherals
>> with shallow FIFOs. I didn't give feedback for this patch because I am
>> not sure if this
>> is the right way to go at all.
>
> This is the wrong attitude.  If it were not for a simple oversight
> Joonyoung's driver would already be upstream for the past two kernel
> releases.  So you need to work together to improve that driver to
> incorporate what you need.
Nothing wrong in attitude here.
Giving feedback on the code only comes after one is convinced with the
overall approach taken. The last time I raised the PL330 driver issue,
most people were not enthusiastic with this drivers/dma/ approach.
I wasn't active mainline discussions when the driver was originally
submitted a few months ago.
And now my replies are not very 'polite' because theres a lot going on
in the background that people in public threads don't know about.


> It sounds like you just need to add an extension for the arch specific dma
> api.
I actually plan more than that.
Apart from inefficient design, JoonYoung's driver has made some fatal
assumptions
about PL330, which will result in DMA aborts if used with SoCs that implement
configuration of PL330 that is very different from Samsung SoCs'
Of course, I address all such issues that I can think of, in my implementation.

regards.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: jassi brar on
On Fri, Mar 26, 2010 at 12:20 AM, Linus Walleij
<linus.ml.walleij(a)gmail.com> wrote:
> 2010/3/25 jassi brar <jassisinghbrar(a)gmail.com>:
>> And if this patch is accepted, there
>> _will_ exist two copies of the PL330 driver -- one in drivers/dma/pl330_dmac.c
>> and another in arch/arm/plat-samsung/. Only the former will be lying unused
>> until some other SoC vendor decided to use PL330, because S3C has come too
>> long a way to change its drivers to driver/dma/ API and modify DMA
>> drivers for every SoC.
>
> What's wrong with merging them later then? Refactoring FTW.
The amount of code that will be modified or taken out of drivers/dma/pl330_dmac
will so much that I will be left only with constrained data structures in
the file to do tricks to make it work with the PL330 engine driver.
I am not very keen on authoring the driver/dma/ driver but neither am I
interested in having to cleanup someone else' code.

>> I have the pl330-core part almost ready, but i need time to implement
>> some _testable_
>> implementation of the scheme. If maintainers want to see structure of
>> my code, I can
>> share it too, but I think I pretty much made it clear.
>
> Why not just post it on the list? I'm curious! Since I'm working on a PrimeCell
> DMA API I would love to look at PrimeCell DMA engine drivers.
I'll post in a day or two when the PL330 core driver takes come shape
closer to what it is supposed to look.
That will help me getting suggestions for improvement, i hope.

regards.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Dan Williams on
On Thu, Mar 25, 2010 at 3:27 PM, jassi brar <jassisinghbrar(a)gmail.com> wrote:
> On Fri, Mar 26, 2010 at 12:13 AM, Dan Williams <dan.j.williams(a)intel.com> wrote:
>> jassi brar wrote:
>>>>
>>>> Perhaps Joonyoung can simply port over the stuff
>>>> you need to this driver if you show your code.
>>>
>>> Having worked on Samsung SoCs(with PL330 DMAC) based products, I would be
>>> _very_ surprised if any user found this implementation useful.
>>> Let alone testing, this implementation can't even explain usability
>>> for fast peripherals
>>> with shallow FIFOs. I didn't give feedback for this patch because I am
>>> not sure if this
>>> is the right way to go at all.
>>
>> This is the wrong attitude. �If it were not for a simple oversight
>> Joonyoung's driver would already be upstream for the past two kernel
>> releases. �So you need to work together to improve that driver to
>> incorporate what you need.
> Nothing wrong in attitude here.
> Giving feedback on the code only comes after one is convinced with the
> overall approach taken. The last time I raised the PL330 driver issue,
> most people were not enthusiastic with this drivers/dma/ approach.
> I wasn't active mainline discussions when the driver was originally
> submitted a few months ago.
> And now my replies are not very 'polite' because theres a lot going on
> in the background that people in public threads don't know about.

Thanks for clarifying.

>
>
>> It sounds like you just need to add an extension for the arch specific dma
>> api.
> I actually plan more than that.
> Apart from inefficient design, JoonYoung's driver has made some fatal
> assumptions
> about PL330, which will result in DMA aborts if used with SoCs that implement
> configuration of PL330 that is very different from Samsung SoCs'
> Of course, I address all such issues that I can think of, in my implementation.

Ok, I'll rely on acked-by's from the interested parties once your
driver is out as I do not have a vested interest in this hardware,
just the dmaengine framework issues.

--
Dan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/