On Tue, Dec 19, 2017 at 03:39:44PM +0000, Laurentiu Tudor wrote: > On 12/19/2017 05:29 PM, Greg KH wrote: > > On Tue, Dec 19, 2017 at 03:21:19PM +0000, Laurentiu Tudor wrote: > >> > >> > >> On 12/19/2017 04:48 PM, Greg KH wrote: > >>> On Wed, Nov 29, 2017 at 12:08:44PM +0200, laurentiu.tudor@xxxxxxx wrote: > >>>> From: Stuart Yoder <stuart.yoder@xxxxxxx> > >>>> > >>>> Move the source files out of staging into their final locations: > >>>> -include files in drivers/staging/fsl-mc/include go to include/linux/fsl > >>>> -irq-gic-v3-its-fsl-mc-msi.c goes to drivers/irqchip > >>>> -source in drivers/staging/fsl-mc/bus goes to drivers/bus/fsl-mc > >>>> -README.txt, providing and overview of DPAA goes to > >>>> Documentation/dpaa2/overview.txt > >>>> > >>>> Update or delete other remaining staging files-- Makefile, Kconfig, TODO. > >>>> Update dpaa2_eth and dpio staging drivers. > >>>> > >>>> Signed-off-by: Stuart Yoder <stuyoder@xxxxxxxxx> > >>>> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@xxxxxxx> > >>>> [Laurentiu: rebased, add dpaa2_eth and dpio #include updates] > >>>> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx> > >>>> Cc: Jason Cooper <jason@xxxxxxxxxxxxxx> > >>>> Cc: Marc Zyngier <marc.zyngier@xxxxxxx> > >>>> --- > >>>> Notes: > >>>> -v4: > >>>> - regenerated patch with renames detection disabled (Andrew Lunn) > >>>> -v3: > >>>> - rebased > >>> > >>> Ok, meta-comments on the structure of the code. > >>> > >>> You have 8 .h files that are "private" to your bus logic. That's 7 too > >>> many, some of them have a bigger license header than actual content :) > >>> > >>> Please consolidate into 1. > >>> > >>> Also, the headers should be moved to SPDX format to get rid of the > >>> boilerplate. I _think_ it's BSD/GPL, right? Hard to tell :( > >> > >> It's 3-clause BSD and GPLv2. Will make it clear when moving to SPDX. > > > > Thanks. > > > >>> Your "public" .h file does not need to go into a subdirectory, just name > >>> it fsl-mc.h and put it in include/linux/. > >> > >> There's already a "fsl" subdirectory in include/linux/ so it seemed to > >> make sense to use it. > > > > Ah, missed that. Ok, nevermind :)` > > > >>> One comment on the fields in your .h file, all of the user/kernel > >>> crossing boundry structures need to use the "__" variant of types, like > >>> "__u8" and the like. You mix and match them for some reason, you need > >>> to be consistent. > >>> > >>> Also, what's up with the .h files in drivers/staging/fsl-bus/include? > >>> You didn't touch those with this movement, right? Why? > >> > >> Those are not part of the bus "core". Some of them are part of the DPBP > >> and DPCON device types APIs and are used by drivers probing on this bus > >> and the rest are part of the DPIO driver which is also used by other > >> drivers. Since these devices (DPBP, DPCON, DPIO) are interfaces used by > >> all the other drivers it made sense to group them together with the bus. > > > > But all of these .h files are only used by the code in this specific > > directory, no where else. > > They are also used by our ethernet driver, see: > drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.h Ick, really? Then they should not be buried in a bus-specific location, but rather be in include/linux/SOMEWHERE, right? thanks, greg k-h _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
