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 dmaengine" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
