All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mika Westerberg <mika.westerberg@linux.intel.com>
To: Lukas Wunner <lukas@wunner.de>
Cc: linux-kernel@vger.kernel.org,
	Andreas Noever <andreas.noever@gmail.com>,
	Michael Jamet <michael.jamet@intel.com>,
	Yehezkel Bernat <YehezkelShB@gmail.com>,
	Lu Baolu <baolu.lu@linux.intel.com>
Subject: Re: [PATCH 2/2] thunderbolt: Initialize after IOMMUs
Date: Thu, 6 Sep 2018 14:46:52 +0300	[thread overview]
Message-ID: <20180906114652.GK2283@lahna.fi.intel.com> (raw)
In-Reply-To: <20180906112101.qmhkcqx73zmmko3q@wunner.de>

On Thu, Sep 06, 2018 at 01:21:01PM +0200, Lukas Wunner wrote:
> On Thu, Sep 06, 2018 at 02:07:56PM +0300, Mika Westerberg wrote:
> > On Thu, Sep 06, 2018 at 01:00:49PM +0200, Lukas Wunner wrote:
> > > On Thu, Sep 06, 2018 at 01:36:02PM +0300, Mika Westerberg wrote:
> > > > On Thu, Sep 06, 2018 at 10:13:37AM +0200, Lukas Wunner wrote:
> > > > > So with this patch, you rely on the linker ordering nhi_init() after
> > > > > ir_dev_scope_init(), however to the best of my knowledge the link
> > > > > order is not guaranteed.
> > > > 
> > > > What says that?
> > > 
> > > Within the same initcall level, the ordering is determined by the Makefile
> > > AFAIK.  Someone changes the Makefile, your dependency scheme falls apart.
> > 
> > There are other drivers doing the same so they would fail as well. It is
> > common practice AFAIK.
> 
> That doesn't make it a *good* practice.

It is good enough for our case.

> > > > > Looking at commit acb40d841257, which started this, I'm wondering
> > > > > why you did not simply export tbnet_init() and call it from the
> > > > > thunderbolt driver after the property stuff has been fully set up?
> > > > > After all, thunderbolt-net is useless without thunderbolt or am I
> > > > > missing something?  Then you could revert back to module_init().
> > > > 
> > > > The same reason you don't call PCI driver functions from PCI core. It
> > > > makes absolutely zero sense.
> > > > 
> > > > Thunderbolt is bus and provides driver API to drivers. We hopefully are
> > > > getting other service drivers (say SCSI over TBT) that are going to be
> > > > use the same interfaces.
> > > 
> > > Then add a blocking notifier chain into which these service drivers can
> > > hook.  Other buses have that as well.
> > 
> > It is really too complex to add notifier just for that. This works fine
> > and is not against any kernel principles I am aware of.
> 
> Well, there's a difference between "it works and gets the job done,
> let's move on" and "let's try to find a solution that fixes not just
> this use case but potentially benefits others as well".
> 
> FWIW, what I had in mind is a blocking notifier chain that gets called
> when a bus registers or unregisters.  TB service drivers would then check
> if it's tb_bus_type and start initialization.

Like I said, I think it is too complex.

If we ever need to change the initcall level third time (which I doubt)
we can start thinking about more complex solutions.

  reply	other threads:[~2018-09-06 11:47 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-03 13:20 [PATCH 1/2] thunderbolt: Do not handle ICM events after domain is stopped Mika Westerberg
2018-09-03 13:20 ` [PATCH 2/2] thunderbolt: Initialize after IOMMUs Mika Westerberg
2018-09-05  8:47   ` Lukas Wunner
2018-09-05  9:46     ` Mika Westerberg
2018-09-06  8:13       ` Lukas Wunner
2018-09-06 10:36         ` Mika Westerberg
2018-09-06 11:00           ` Lukas Wunner
2018-09-06 11:07             ` Mika Westerberg
2018-09-06 11:21               ` Lukas Wunner
2018-09-06 11:46                 ` Mika Westerberg [this message]
2018-09-14  7:52   ` Mika Westerberg
2018-09-14  7:52 ` [PATCH 1/2] thunderbolt: Do not handle ICM events after domain is stopped Mika Westerberg
  -- strict thread matches above, loose matches on Subject: below --
2018-09-24 10:20 [PATCH 0/2] thunderbolt: Fixes for v4.19-rc6 Mika Westerberg
2018-09-24 10:20 ` [PATCH 2/2] thunderbolt: Initialize after IOMMUs Mika Westerberg

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=20180906114652.GK2283@lahna.fi.intel.com \
    --to=mika.westerberg@linux.intel.com \
    --cc=YehezkelShB@gmail.com \
    --cc=andreas.noever@gmail.com \
    --cc=baolu.lu@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lukas@wunner.de \
    --cc=michael.jamet@intel.com \
    /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.