Re: [PATCH] dmaengine: mcf-edma: add ColdFire mcf5441x eDMA support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, May 04, 2018 at 09:18:19PM +0200, Angelo Dureghello wrote:
Hi Vinod,

thanks for the review,

On Thu, May 03, 2018 at 10:18:30PM +0530, Vinod Koul wrote:
On Wed, Apr 25, 2018 at 10:08:17PM +0200, Angelo Dureghello wrote:
This patch adds dma support for NXP mcf5441x (ColdFire) family.

ColdFire mcf5441x implements an edma hw module similar to the

Is it similar to to edma ?


It is similar to Freescale "edma" but with a different number of
channels, a bit different register set, different interrupt
structure, no channel multiplexer.

ok

one implemented in Vybrid VFxxx controllers, but with a slightly
different register set, more dma channels (64 instead of 32),
a different interrupt mechanism and some other minor differences.

For the above reasons, modfying fsl-edma.c was too complex and
likely too ugly. From here, the decision to create a different
driver, but starting from fsl-edma.

can the common stuff be made into a lib and shared between then two rather
than having a same driver or different drivers?

It should be possible to collect some common code in a kind of
mcf_edma_core.c common module, but in this case i cannot then test
the Vybrid edma after the changes since i miss that hardware.

Sure you should send the patches and folks who care about fsl driver
would look it up and test

Would be maybe possible for you to diff fsl-edma and this mcf-edma,
just to confirm if i can still stay this way, or if moving to a
library becomes mandatory ?

well since you know the IP you would make a better guess on that, best is
to check register sets in drivers

+// SPDX-License-Identifier: GPL-2.0

Copyright info should be here in c99 style comments


Seems checkpatch.pl, for C files, does not like the C style
initial line comment:

WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
#87: FILE: drivers/dma/mcf-edma.c:1:
+/* SPDX-License-Identifier: GPL-2.0 */

While c++ type is accepted.

In contrary, in .h files it wants cpp // style and not C style.

SC99 comments style is
// SPDX-License-Identifier: GPL-2.0

Point is the copyright should be added is same formar i.e.,

// Copyright 20018 - foo bar

this line should follow the spdx line

+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

why do you need this, why not use dev_xxx


Well, pr_ style seems to simplify the call a bit, should be allowed
but if you prefer i can move all to dev_ format.

in hindsight dev_ makes better sense, been there done that :)

-- 
~Vinod
--
To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Video for Linux]     [Yosemite News]     [Linux S/390]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux