[v5,2/2] dmaengine: fsl-edma: add ColdFire mcf5441x edma support

[Vinod Koul <vkoul@kernel.org>]

On 30-06-18, 15:42, Angelo Dureghello wrote:
> Hi Vinod,
> 
> fixed mostly all, but sorry, i have still two questions before
> proceeding,
> 
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +// Copyright (c) 2013-2014 Freescale Semiconductor, Inc
> > > +// Copyright (c) 2017 Sysam, Angelo Dureghello  <angelo@sysam.it>
> > > +/*
> > > + * drivers/dma/mcf-edma.c
> > > + *
> > > + * Driver for the Freescale ColdFire 64-ch eDMA implementation,
> > > + * derived from drivers/dma/fsl-edma.c.
> > > + *
> > > + * This program is free software; you can redistribute  it and/or modify it
> > > + * under  the terms of  the GNU General  Public License as published by the
> > > + * Free Software Foundation;  either version 2 of the  License, or (at your
> > > + * option) any later version.
> > > + */
> > 
> > again, no need for text
> > 
> 
> It is not clear to me now how the initial header should be (i guess for 
> all the 3 c files at this point).
> 
> Do you want just something as :
> 
> // SPDX-License-Identifier: GPL-2.0
> // Copyright (c) 2013-2014 Freescale Semiconductor, Inc
> // Copyright (c) 2017 Sysam, Angelo Dureghello  <angelo@sysam.it>

That would be nice. Text is no longer required as SPDX represents that.
Copyright needs to be retained.
> 
> And nothing else ?
> 
> Majority of the files in the dma folder has also generally a line with 
> the file name and path, a brief driver explaination and the reduced GPL
> licence text, and, as imx-sdma.c often copyrights at the end. So what is 
> the current rule ?

Above and if you would like add a line explaining the role of driver.
File name and path are not required and gets stale as people update
stuff in future.

> > > +static int __init mcf_edma_init(void)
> > > +{
> > > +	return platform_driver_register(&mcf_edma_driver);
> > > +}
> > > +subsys_initcall(mcf_edma_init);
> > 
> > why subsys_initcall?
> > 
> 
> I find subsys_initcall in several dma drivers, my understanding is that
> it initializes the driver before other drivers can use it.
> It also sets the driver as built in only.
> This seems ok for my case.

Yes that is the case, but in your case I would like to know why you
would want this. Others doing is not a good enough justification and you
need to find your reason :)