Re: [PATCH v4] staging: fsl-mc: move bus driver out of staging
On 12/20/2017 01:06 PM, Greg KH wrote: > On Wed, Dec 20, 2017 at 10:52:52AM +, Laurentiu Tudor wrote: >> >> >> On 12/20/2017 12:42 PM, Greg KH wrote: >>> On Wed, Dec 20, 2017 at 10:26:49AM +, Laurentiu Tudor wrote: On 12/19/2017 06:10 PM, Greg KH wrote: >>> 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? Right. The goal is that in the end, all headers be moved to the already existing include/linux/fsl/. For now I've left these in staging because they are not part of the bus "core" infrastructure. >>> >>> Then shouldn't they be in the drivers/staging/fsl-mc/include/ directory >>> now to show this? >> >> Not sure i get your comment. Aren't we talking about the headers in there? >> >> This was your original comment: >> >> > Also, what's up with the .h files in drivers/staging/fsl-bus/include? >> > You didn't touch those with this movement, right? Why? > > Ok, yeah, I'm getting confused now. Let's just see what you do with > your next set of patches and we can go from there :) Ok, I'll start working on it. Thanks for the review! --- Best Regards, Laurentiu
Re: [PATCH v4] staging: fsl-mc: move bus driver out of staging
On 12/20/2017 01:06 PM, Greg KH wrote: > On Wed, Dec 20, 2017 at 10:52:52AM +, Laurentiu Tudor wrote: >> >> >> On 12/20/2017 12:42 PM, Greg KH wrote: >>> On Wed, Dec 20, 2017 at 10:26:49AM +, Laurentiu Tudor wrote: On 12/19/2017 06:10 PM, Greg KH wrote: >>> 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? Right. The goal is that in the end, all headers be moved to the already existing include/linux/fsl/. For now I've left these in staging because they are not part of the bus "core" infrastructure. >>> >>> Then shouldn't they be in the drivers/staging/fsl-mc/include/ directory >>> now to show this? >> >> Not sure i get your comment. Aren't we talking about the headers in there? >> >> This was your original comment: >> >> > Also, what's up with the .h files in drivers/staging/fsl-bus/include? >> > You didn't touch those with this movement, right? Why? > > Ok, yeah, I'm getting confused now. Let's just see what you do with > your next set of patches and we can go from there :) Ok, I'll start working on it. Thanks for the review! --- Best Regards, Laurentiu
Re: [PATCH v4] staging: fsl-mc: move bus driver out of staging
On Wed, Dec 20, 2017 at 10:52:52AM +, Laurentiu Tudor wrote: > > > On 12/20/2017 12:42 PM, Greg KH wrote: > > On Wed, Dec 20, 2017 at 10:26:49AM +, Laurentiu Tudor wrote: > >> On 12/19/2017 06:10 PM, Greg KH wrote: > > 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? > >> > >> Right. The goal is that in the end, all headers be moved to the already > >> existing include/linux/fsl/. For now I've left these in staging because > >> they are not part of the bus "core" infrastructure. > > > > Then shouldn't they be in the drivers/staging/fsl-mc/include/ directory > > now to show this? > > Not sure i get your comment. Aren't we talking about the headers in there? > > This was your original comment: > > > Also, what's up with the .h files in drivers/staging/fsl-bus/include? > > You didn't touch those with this movement, right? Why? Ok, yeah, I'm getting confused now. Let's just see what you do with your next set of patches and we can go from there :) thanks, greg k-h
Re: [PATCH v4] staging: fsl-mc: move bus driver out of staging
On Wed, Dec 20, 2017 at 10:52:52AM +, Laurentiu Tudor wrote: > > > On 12/20/2017 12:42 PM, Greg KH wrote: > > On Wed, Dec 20, 2017 at 10:26:49AM +, Laurentiu Tudor wrote: > >> On 12/19/2017 06:10 PM, Greg KH wrote: > > 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? > >> > >> Right. The goal is that in the end, all headers be moved to the already > >> existing include/linux/fsl/. For now I've left these in staging because > >> they are not part of the bus "core" infrastructure. > > > > Then shouldn't they be in the drivers/staging/fsl-mc/include/ directory > > now to show this? > > Not sure i get your comment. Aren't we talking about the headers in there? > > This was your original comment: > > > Also, what's up with the .h files in drivers/staging/fsl-bus/include? > > You didn't touch those with this movement, right? Why? Ok, yeah, I'm getting confused now. Let's just see what you do with your next set of patches and we can go from there :) thanks, greg k-h
Re: [PATCH v4] staging: fsl-mc: move bus driver out of staging
On 12/20/2017 12:42 PM, Greg KH wrote: > On Wed, Dec 20, 2017 at 10:26:49AM +, Laurentiu Tudor wrote: >> On 12/19/2017 06:10 PM, Greg KH wrote: > 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? >> >> Right. The goal is that in the end, all headers be moved to the already >> existing include/linux/fsl/. For now I've left these in staging because >> they are not part of the bus "core" infrastructure. > > Then shouldn't they be in the drivers/staging/fsl-mc/include/ directory > now to show this? Not sure i get your comment. Aren't we talking about the headers in there? This was your original comment: > Also, what's up with the .h files in drivers/staging/fsl-bus/include? > You didn't touch those with this movement, right? Why? --- Best Regards, Laurentiu
Re: [PATCH v4] staging: fsl-mc: move bus driver out of staging
On 12/20/2017 12:42 PM, Greg KH wrote: > On Wed, Dec 20, 2017 at 10:26:49AM +, Laurentiu Tudor wrote: >> On 12/19/2017 06:10 PM, Greg KH wrote: > 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? >> >> Right. The goal is that in the end, all headers be moved to the already >> existing include/linux/fsl/. For now I've left these in staging because >> they are not part of the bus "core" infrastructure. > > Then shouldn't they be in the drivers/staging/fsl-mc/include/ directory > now to show this? Not sure i get your comment. Aren't we talking about the headers in there? This was your original comment: > Also, what's up with the .h files in drivers/staging/fsl-bus/include? > You didn't touch those with this movement, right? Why? --- Best Regards, Laurentiu
Re: [PATCH v4] staging: fsl-mc: move bus driver out of staging
On Wed, Dec 20, 2017 at 10:26:49AM +, Laurentiu Tudor wrote: > On 12/19/2017 06:10 PM, Greg KH wrote: > >>> 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? > > Right. The goal is that in the end, all headers be moved to the already > existing include/linux/fsl/. For now I've left these in staging because > they are not part of the bus "core" infrastructure. Then shouldn't they be in the drivers/staging/fsl-mc/include/ directory now to show this? thanks, greg k-h
Re: [PATCH v4] staging: fsl-mc: move bus driver out of staging
On Wed, Dec 20, 2017 at 10:26:49AM +, Laurentiu Tudor wrote: > On 12/19/2017 06:10 PM, Greg KH wrote: > >>> 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? > > Right. The goal is that in the end, all headers be moved to the already > existing include/linux/fsl/. For now I've left these in staging because > they are not part of the bus "core" infrastructure. Then shouldn't they be in the drivers/staging/fsl-mc/include/ directory now to show this? thanks, greg k-h
Re: [PATCH v4] staging: fsl-mc: move bus driver out of staging
On 12/19/2017 06:10 PM, Greg KH wrote: > On Tue, Dec 19, 2017 at 03:39:44PM +, Laurentiu Tudor wrote: >> On 12/19/2017 05:29 PM, Greg KH wrote: >>> On Tue, Dec 19, 2017 at 03:21:19PM +, Laurentiu Tudor wrote: On 12/19/2017 04:48 PM, Greg KH wrote: > On Wed, Nov 29, 2017 at 12:08:44PM +0200, laurentiu.tu...@nxp.com wrote: >> From: Stuart Yoder>> >> 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 >> Signed-off-by: Laurentiu Tudor >> [Laurentiu: rebased, add dpaa2_eth and dpio #include updates] >> Cc: Thomas Gleixner >> Cc: Jason Cooper >> Cc: Marc Zyngier >> --- >> 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? Right. The goal is that in the end, all headers be moved to the already existing include/linux/fsl/. For now I've left these in staging because they are not part of the bus "core" infrastructure. --- Best Regards, Laurentiu
Re: [PATCH v4] staging: fsl-mc: move bus driver out of staging
On 12/19/2017 06:10 PM, Greg KH wrote: > On Tue, Dec 19, 2017 at 03:39:44PM +, Laurentiu Tudor wrote: >> On 12/19/2017 05:29 PM, Greg KH wrote: >>> On Tue, Dec 19, 2017 at 03:21:19PM +, Laurentiu Tudor wrote: On 12/19/2017 04:48 PM, Greg KH wrote: > On Wed, Nov 29, 2017 at 12:08:44PM +0200, laurentiu.tu...@nxp.com wrote: >> From: Stuart Yoder >> >> 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 >> Signed-off-by: Laurentiu Tudor >> [Laurentiu: rebased, add dpaa2_eth and dpio #include updates] >> Cc: Thomas Gleixner >> Cc: Jason Cooper >> Cc: Marc Zyngier >> --- >> 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? Right. The goal is that in the end, all headers be moved to the already existing include/linux/fsl/. For now I've left these in staging because they are not part of the bus "core" infrastructure. --- Best Regards, Laurentiu
Re: [PATCH v4] staging: fsl-mc: move bus driver out of staging
On Tue, Dec 19, 2017 at 03:39:44PM +, Laurentiu Tudor wrote: > On 12/19/2017 05:29 PM, Greg KH wrote: > > On Tue, Dec 19, 2017 at 03:21:19PM +, Laurentiu Tudor wrote: > >> > >> > >> On 12/19/2017 04:48 PM, Greg KH wrote: > >>> On Wed, Nov 29, 2017 at 12:08:44PM +0200, laurentiu.tu...@nxp.com wrote: > From: Stuart Yoder> > 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 > Signed-off-by: Laurentiu Tudor > [Laurentiu: rebased, add dpaa2_eth and dpio #include updates] > Cc: Thomas Gleixner > Cc: Jason Cooper > Cc: Marc Zyngier > --- > 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
Re: [PATCH v4] staging: fsl-mc: move bus driver out of staging
On Tue, Dec 19, 2017 at 03:39:44PM +, Laurentiu Tudor wrote: > On 12/19/2017 05:29 PM, Greg KH wrote: > > On Tue, Dec 19, 2017 at 03:21:19PM +, Laurentiu Tudor wrote: > >> > >> > >> On 12/19/2017 04:48 PM, Greg KH wrote: > >>> On Wed, Nov 29, 2017 at 12:08:44PM +0200, laurentiu.tu...@nxp.com wrote: > From: Stuart Yoder > > 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 > Signed-off-by: Laurentiu Tudor > [Laurentiu: rebased, add dpaa2_eth and dpio #include updates] > Cc: Thomas Gleixner > Cc: Jason Cooper > Cc: Marc Zyngier > --- > 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
Re: [PATCH v4] staging: fsl-mc: move bus driver out of staging
On 12/19/2017 05:29 PM, Greg KH wrote: > On Tue, Dec 19, 2017 at 03:21:19PM +, Laurentiu Tudor wrote: >> >> >> On 12/19/2017 04:48 PM, Greg KH wrote: >>> On Wed, Nov 29, 2017 at 12:08:44PM +0200, laurentiu.tu...@nxp.com wrote: From: Stuart YoderMove 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 Signed-off-by: Laurentiu Tudor [Laurentiu: rebased, add dpaa2_eth and dpio #include updates] Cc: Thomas Gleixner Cc: Jason Cooper Cc: Marc Zyngier --- 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
Re: [PATCH v4] staging: fsl-mc: move bus driver out of staging
On 12/19/2017 05:29 PM, Greg KH wrote: > On Tue, Dec 19, 2017 at 03:21:19PM +, Laurentiu Tudor wrote: >> >> >> On 12/19/2017 04:48 PM, Greg KH wrote: >>> On Wed, Nov 29, 2017 at 12:08:44PM +0200, laurentiu.tu...@nxp.com wrote: From: Stuart Yoder 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 Signed-off-by: Laurentiu Tudor [Laurentiu: rebased, add dpaa2_eth and dpio #include updates] Cc: Thomas Gleixner Cc: Jason Cooper Cc: Marc Zyngier --- 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
Re: [PATCH v4] staging: fsl-mc: move bus driver out of staging
On Tue, Dec 19, 2017 at 03:21:19PM +, Laurentiu Tudor wrote: > > > On 12/19/2017 04:48 PM, Greg KH wrote: > > On Wed, Nov 29, 2017 at 12:08:44PM +0200, laurentiu.tu...@nxp.com wrote: > >> From: Stuart Yoder> >> > >> 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 > >> Signed-off-by: Laurentiu Tudor > >> [Laurentiu: rebased, add dpaa2_eth and dpio #include updates] > >> Cc: Thomas Gleixner > >> Cc: Jason Cooper > >> Cc: Marc Zyngier > >> --- > >> 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. 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 :) > > 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. > >> 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. thanks, greg k-h
Re: [PATCH v4] staging: fsl-mc: move bus driver out of staging
On Tue, Dec 19, 2017 at 03:21:19PM +, Laurentiu Tudor wrote: > > > On 12/19/2017 04:48 PM, Greg KH wrote: > > On Wed, Nov 29, 2017 at 12:08:44PM +0200, laurentiu.tu...@nxp.com wrote: > >> From: Stuart Yoder > >> > >> 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 > >> Signed-off-by: Laurentiu Tudor > >> [Laurentiu: rebased, add dpaa2_eth and dpio #include updates] > >> Cc: Thomas Gleixner > >> Cc: Jason Cooper > >> Cc: Marc Zyngier > >> --- > >> 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. 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 :) > > 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. > >> 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. thanks, greg k-h
Re: [PATCH v4] staging: fsl-mc: move bus driver out of staging
On 12/19/2017 04:48 PM, Greg KH wrote: > On Wed, Nov 29, 2017 at 12:08:44PM +0200, laurentiu.tu...@nxp.com wrote: >> From: Stuart Yoder>> >> 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 >> Signed-off-by: Laurentiu Tudor >> [Laurentiu: rebased, add dpaa2_eth and dpio #include updates] >> Cc: Thomas Gleixner >> Cc: Jason Cooper >> Cc: Marc Zyngier >> --- >> 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. > > 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. > 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. > 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. >> 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 >> 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? > And speaking of documentation, you have directories in sysfs, yet no > Documentation/ABI/ files describing them. Please fix that up. Hmm, I was under the impression that we did have sysfs documentation. Will look into it. > that's a good start :) Yep. :) --- Thanks & Best Regards, Laurentiu
Re: [PATCH v4] staging: fsl-mc: move bus driver out of staging
On 12/19/2017 04:48 PM, Greg KH wrote: > On Wed, Nov 29, 2017 at 12:08:44PM +0200, laurentiu.tu...@nxp.com wrote: >> From: Stuart Yoder >> >> 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 >> Signed-off-by: Laurentiu Tudor >> [Laurentiu: rebased, add dpaa2_eth and dpio #include updates] >> Cc: Thomas Gleixner >> Cc: Jason Cooper >> Cc: Marc Zyngier >> --- >> 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. > > 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. > 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. > 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. >> 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 >> 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? > And speaking of documentation, you have directories in sysfs, yet no > Documentation/ABI/ files describing them. Please fix that up. Hmm, I was under the impression that we did have sysfs documentation. Will look into it. > that's a good start :) Yep. :) --- Thanks & Best Regards, Laurentiu
Re: [PATCH v4] staging: fsl-mc: move bus driver out of staging
On Wed, Nov 29, 2017 at 12:08:44PM +0200, laurentiu.tu...@nxp.com wrote: > From: Stuart Yoder> > 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 > Signed-off-by: Laurentiu Tudor > [Laurentiu: rebased, add dpaa2_eth and dpio #include updates] > Cc: Thomas Gleixner > Cc: Jason Cooper > Cc: Marc Zyngier > --- > 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 :( Your "public" .h file does not need to go into a subdirectory, just name it fsl-mc.h and put it in include/linux/. 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? 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? > drivers/staging/fsl-dpaa2/ethernet/README | 2 +- Why does a README file for a different driver need to be touched? > 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. It doesn't need to have a separate directory either, right? And speaking of documentation, you have directories in sysfs, yet no Documentation/ABI/ files describing them. Please fix that up. that's a good start :) thanks, greg k-h
Re: [PATCH v4] staging: fsl-mc: move bus driver out of staging
On Wed, Nov 29, 2017 at 12:08:44PM +0200, laurentiu.tu...@nxp.com wrote: > From: Stuart Yoder > > 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 > Signed-off-by: Laurentiu Tudor > [Laurentiu: rebased, add dpaa2_eth and dpio #include updates] > Cc: Thomas Gleixner > Cc: Jason Cooper > Cc: Marc Zyngier > --- > 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 :( Your "public" .h file does not need to go into a subdirectory, just name it fsl-mc.h and put it in include/linux/. 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? 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? > drivers/staging/fsl-dpaa2/ethernet/README | 2 +- Why does a README file for a different driver need to be touched? > 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. It doesn't need to have a separate directory either, right? And speaking of documentation, you have directories in sysfs, yet no Documentation/ABI/ files describing them. Please fix that up. that's a good start :) thanks, greg k-h