From: gregkh@linuxfoundation.org (Greg KH)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v4] staging: fsl-mc: move bus driver out of staging
Date: Tue, 19 Dec 2017 15:48:02 +0100 [thread overview]
Message-ID: <20171219144802.GA4534@kroah.com> (raw)
In-Reply-To: <20171129100844.19874-1-laurentiu.tudor@nxp.com>
On Wed, Nov 29, 2017 at 12:08:44PM +0200, laurentiu.tudor at nxp.com wrote:
> From: Stuart Yoder <stuart.yoder@nxp.com>
>
> 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@gmail.com>
> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> [Laurentiu: rebased, add dpaa2_eth and dpio #include updates]
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> ---
> 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
WARNING: multiple messages have this Message-ID (diff)
From: Greg KH <gregkh@linuxfoundation.org>
To: laurentiu.tudor@nxp.com
Cc: ruxandra.radulescu@nxp.com, devel@driverdev.osuosl.org,
linux-kernel@vger.kernel.org, bogdan.purcareata@nxp.com,
leoyang.li@nxp.com, stuyoder@gmail.com, roy.pledge@nxp.com,
andrew@lunn.ch, linux-arm-kernel@lists.infradead.org,
Stuart Yoder <stuart.yoder@nxp.com>,
Thomas Gleixner <tglx@linutronix.de>,
Jason Cooper <jason@lakedaemon.net>,
Marc Zyngier <marc.zyngier@arm.com>
Subject: Re: [PATCH v4] staging: fsl-mc: move bus driver out of staging
Date: Tue, 19 Dec 2017 15:48:02 +0100 [thread overview]
Message-ID: <20171219144802.GA4534@kroah.com> (raw)
In-Reply-To: <20171129100844.19874-1-laurentiu.tudor@nxp.com>
On Wed, Nov 29, 2017 at 12:08:44PM +0200, laurentiu.tudor@nxp.com wrote:
> From: Stuart Yoder <stuart.yoder@nxp.com>
>
> 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@gmail.com>
> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> [Laurentiu: rebased, add dpaa2_eth and dpio #include updates]
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> ---
> 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
next prev parent reply other threads:[~2017-12-19 14:48 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-29 10:08 [PATCH v4] staging: fsl-mc: move bus driver out of staging laurentiu.tudor
2017-12-19 14:48 ` Greg KH [this message]
2017-12-19 14:48 ` Greg KH
2017-12-19 15:21 ` Laurentiu Tudor
2017-12-19 15:21 ` Laurentiu Tudor
2017-12-19 15:29 ` Greg KH
2017-12-19 15:29 ` Greg KH
2017-12-19 15:39 ` Laurentiu Tudor
2017-12-19 15:39 ` Laurentiu Tudor
2017-12-19 16:10 ` Greg KH
2017-12-19 16:10 ` Greg KH
2017-12-20 10:26 ` Laurentiu Tudor
2017-12-20 10:26 ` Laurentiu Tudor
2017-12-20 10:42 ` Greg KH
2017-12-20 10:42 ` Greg KH
2017-12-20 10:52 ` Laurentiu Tudor
2017-12-20 10:52 ` Laurentiu Tudor
2017-12-20 11:06 ` Greg KH
2017-12-20 11:06 ` Greg KH
2017-12-20 11:52 ` Laurentiu Tudor
2017-12-20 11:52 ` Laurentiu Tudor
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171219144802.GA4534@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.