From: gregkh@linuxfoundation.org (Greg KH)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2] staging: fsl-mc: move bus driver out of staging
Date: Tue, 28 Nov 2017 13:59:03 +0100 [thread overview]
Message-ID: <20171128125903.GA13019@kroah.com> (raw)
In-Reply-To: <5A1C2FEB.2080006@nxp.com>
On Mon, Nov 27, 2017 at 03:32:28PM +0000, Laurentiu Tudor wrote:
>
>
> On 11/03/2017 05:17 PM, Greg KH wrote:
> > On Thu, Aug 31, 2017 at 06:04:30PM +0200, Greg KH wrote:
> >> On Mon, Aug 28, 2017 at 01:54:05PM +0300, 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>
> >>
> >> This is going to have to wait until I get a chunk of time to do the
> >> review. Probably after 4.13-final is out.
> >
> > Ok, review comments as I go through the code:
> > mc-sys.c 323 EXPORT_SYMBOL(mc_send_command);
> >
> > should be EXPORT_SYMBOL_GPL(fsl_mc_send_command); to match up with your
> > other exports and global namespace, right?
> >
> > You export a lot of dpcon_* symbols that no one uses, please do not do
> > that. Verify that all of them are actually used right now, if not,
> > remove them. If you think you are going to use them in the future,
> > wonderful, add them in then.
> >
> > Same for your dpaa2_* exported symbols, most are not used from what I
> > can see.
> >
> > struct dpaa2_io {
> > atomic_t refs;
> >
> > That's a kref, please use it instead of trying to roll your own.
> >
> > And even for this, your locking is not correct (i.e. you do not have
> > any), that needs to be fixed so that teardown works correctly.
> >
> > You have a lot of WARN_ON() calls, that's going to be ignored and should
> > all not be needed now that the code is debugged and working properly.
> > Please remove them, or turn them into dev_err() calls with a real if ()
> > check instead.
> >
> > You are checking "strings" for the type of device in a lot of places,
> > like this:
> > if (strcmp(obj_desc->type, "dprc") == 0) {
> > why are you not just using the built-in driver model .type field and
> > comparing that to the different type structures? It's much easier, and
> > you don't have to again, "roll your own". See the USB or Greybus code
> > for examples of busses that have different "types" of devices on them at
> > the same time.
> >
> > Ok, that's enough for now, please work on this, and we can go from
> > there...
> >
>
> What would the next steps be, now that the patches are in staging-next?
> Are there plans for a new round of review?
Send a patch that moves the files you think should be moved at this
point in time, as I'm not quite sure which ones exactly you feel are
ready to go.
thanks,
greg k-h
next prev parent reply other threads:[~2017-11-28 12:59 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-28 10:54 [PATCH v2] staging: fsl-mc: move bus driver out of staging laurentiu.tudor at nxp.com
2017-08-31 16:04 ` Greg KH
2017-09-11 11:55 ` Laurentiu Tudor
2017-09-11 13:03 ` Greg KH
2017-09-11 13:08 ` Laurentiu Tudor
2017-11-03 15:17 ` Greg KH
2017-11-03 15:38 ` Laurentiu Tudor
2017-11-06 14:20 ` Laurentiu Tudor
2017-11-06 19:40 ` Roy Pledge
2017-11-27 15:32 ` Laurentiu Tudor
2017-11-28 12:59 ` Greg KH [this message]
2017-11-28 13:35 ` 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=20171128125903.GA13019@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).