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 > So just mush them all together, having > individual .h files doesn't really help anyone here, right? It's just > more files for no good reason. And, it might help you see if you really > need all of the info in those files :) Ok, I'll try to come up with something that reduces the number of header files. >>> For this initial move, only move the bus "core" code out, not the other >>> stuff like: >>> >>>> drivers/irqchip/Makefile | 1 + >>>> drivers/irqchip/irq-gic-v3-its-fsl-mc-msi.c | 119 +++ >>> >>> these should be a separate file move, right? >> >> This bus uses msi interrupts and this file contains glue code needed to >> enable interrupts in the GICv3 irqchip. Without this I don't think the >> bus driver can work because itself makes use of interrupts. > > How is this all working today? Just leave the non-bus code alone, and > move the irqchip code as a separate patch. Ok, i can do this. >>>> drivers/staging/fsl-dpaa2/ethernet/README | 2 +- >>> >>> Why does a README file for a different driver need to be touched? >> >> It mentions a file in the old location of the bus. This is how the diff >> looks: >> >> - drivers/staging/fsl-mc/README.txt >> + Documentation/dpaa2/overview.txt > > Ah. > >>>> drivers/staging/fsl-dpaa2/ethernet/dpaa2-eth.c | 2 +- >>>> drivers/staging/fsl-dpaa2/ethernet/dpni.c | 2 +- >>>> drivers/staging/fsl-mc/README.txt | 386 --------- >>> >>> This file gets moved to the Documentation directory, yet it is not tied >>> into the documentation build process, that's not good. >> >> Will look into that. >> >>> It doesn't need to have a separate directory either, right? >> >> Agreed, maybe the destination directory isn't the best choice. Maybe >> bus-devices/fsl-mc.txt makes more sense? Can you please suggest? > > It should be in .rst format, and tied into the documentation build > somehow, I don't really know where new stuff is going, you should look > at recent changes in that directory for more examples. Ok, will do. --- Thanks & Best Regards, Laurentiu _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
