From: jassi brar on
On Thu, Mar 25, 2010 at 12:17 PM, Joonyoung Shim
<jy0922.shim(a)samsung.com> wrote:
> The PL330 is currently the dma controller using at the S5PC1XX arm SoC.
> This supports DMA_MEMCPY and DMA_SLAVE.
>
> The datasheet for the PL330 can find below url:
> http://infocenter.arm.com/help/topic/com.arm.doc.ddi0424a/DDI0424A_dmac_pl330_r0p0_trm.pdf
>
> Signed-off-by: Joonyoung Shim <jy0922.shim(a)samsung.com>
> ---
> Change log:
>
> v2: Convert into an amba_device driver.
>    Code clean and update from v1 patch review.

[CC'ing Russell and Ben as stakeholders]

Dear Maintainers

I too have been writing a driver for PL330 after taking into account the
suggestions of Russell, Ben and other participants of the thread
http://lists.infradead.org/pipermail/linux-arm-kernel/2010-February/009856.html

If you don't think this driver conflicts with the theme of the thread,
may I ask you
to please put this driver on hold until you checkout my implementation
of solution
to the issue... which should be soon.

Regards,
Jaswinder Singh
Solution-1 Group
Linux Kernel Team
System LSI Division
Samsung Electronics Co., Ltd.
--
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: Marc Zyngier on
On Thu, 25 Mar 2010 12:17:15 +0900
Joonyoung Shim <jy0922.shim(a)samsung.com> wrote:

> +/* instruction set functions */
> +static inline int pl330_dmaaddh(u8 *desc_pool_virt, u16 imm, bool ra)
> +{
> + u8 opcode = DMAADDH | (ra << 1);
> +
> + writeb(opcode, desc_pool_virt++);

desc_pool_virt is a virtual address (from dma_alloc_coherent). In such
case, write[bwl] seems to be the wrong interface. I suggest the
following code:

*desc_pool_virt++ = opcode;

> + writew(imm, desc_pool_virt);

Does anything ensure that this won't generate an unaligned access?

> + return 3;
> +}

M.
--
I'm the slime oozin' out from your TV set...
--
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 Joonyoung Shim <jy0922.shim(a)samsung.com>:

> The PL330 is currently the dma controller using at the S5PC1XX arm SoC.
> This supports DMA_MEMCPY and DMA_SLAVE.

Looks good to me now so:
Acked-by: Linus Walleij <linus.walleij(a)stericsson.com>

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: Linus Walleij on
2010/3/25 jassi brar <jassisinghbrar(a)gmail.com>:

> I too have been writing a driver for PL330 after taking into account the
> suggestions of Russell, Ben and other participants of the thread
> http://lists.infradead.org/pipermail/linux-arm-kernel/2010-February/009856.html
>
> If you don't think this driver conflicts with the theme of the thread,
> may I ask you to please put this driver on hold until you checkout my implementation
> of solution to the issue... which should be soon.

Please post the code as it looks today even if it's not compiling
instead of asking others
to hold their patches back. It will be obvious from what you have if
there is some special
use you're covering. Perhaps Joonyoung can simply port over the stuff
you need to
this driver if you show your code.

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: Joonyoung Shim on
On 3/25/2010 2:44 PM, Marc Zyngier wrote:
> On Thu, 25 Mar 2010 12:17:15 +0900
> Joonyoung Shim <jy0922.shim(a)samsung.com> wrote:
>
>> +/* instruction set functions */
>> +static inline int pl330_dmaaddh(u8 *desc_pool_virt, u16 imm, bool ra)
>> +{
>> + u8 opcode = DMAADDH | (ra << 1);
>> +
>> + writeb(opcode, desc_pool_virt++);
>
> desc_pool_virt is a virtual address (from dma_alloc_coherent). In such
> case, write[bwl] seems to be the wrong interface. I suggest the
> following code:
>
> *desc_pool_virt++ = opcode;
>
>> + writew(imm, desc_pool_virt);
>

Right. The write[bwl] is api for address ioremapped of io device. I will
change these.

> Does anything ensure that this won't generate an unaligned access?
>

PL330 DMA controller fetches variable length instructions that consist of
one to six bytes, so i think unaligned access is no problem.

>> + return 3;
>> +}
>
> M.

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