linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: holler@ahsoftware.de (Alexander Holler)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 15/16] mtd: mtdcore: fix initcall level
Date: Wed, 2 Sep 2015 07:34:42 +0200	[thread overview]
Message-ID: <55E68A72.30909@ahsoftware.de> (raw)
In-Reply-To: <20150901211938.GJ81844@google.com>

Am 01.09.2015 um 23:19 schrieb Brian Norris:
> Hi Alexander,
>
> No judgment here for the rest of this series, but for this patch:
>
> On Wed, Aug 26, 2015 at 02:28:27PM +0200, Alexander Holler wrote:
>> The mtd-core has to be initialized before other dependent mtd-drivers,
>> otherwise a crash might occur.
>>
>> Currently mtd_init() is called in the initcall-level device, which is the
>> same level where most mtd-drivers will end up. By luck this seemed to have
>> been called most of the time before other mtd-drivers without having been
>> explicitly enforced.
>
> I can't really speak for the original authors, but it does not appear to
> be entirely "by luck." Link order was one of the de facto ways to get
> this ordering (though it's not really a great one), and mtdcore was
> always linked first within the drivers/mtd/ directory structure.
>
> But that's just background, I think this is worth fixing anyway. It
> could, for instance, become a problem if drivers are located outside
> drivers/mtd/; I see random board files in arch/ that register with MTD,
> and I'm actually not sure how they have never tripped on this.

I've already found at least a half a dozen other drivers with the same 
problem through my shuffling of the drivers which all end up in the 
standard initcall level device. I'm aware that this is based on the link 
order, which itself is based on linker behaviour (and maybe other things 
like make or other build tools). I'm just calling it luck, because this 
is nowhere explicitly stated, at least I've never seen such a statement, 
neither in any source, nor somewhere in Documentation. So I've choosen 
the term "by luck" in order to provoke a bit (or to stimulate a 
discussion about that widespread problem).

>
>> But if mtd_init() is not called before a dependent
>> driver, a null-pointer exception might occur (e.g. because the mtd device
>> class isn't registered).
>>
>> To fix this, mtd-init() is moved to the initcall-level fs (right before
>> the standard initcall level device).
>
> Is there a good reason we shouldn't just make this a subsys_initcall()?
> That would match the naming better, and it mirrors what, e.g.,
> block/genhd uses. It would also allow flexibility if there are other
> current/future use cases that might need to sit between the subsystem
> and the drivers.

No real reason here. The names for the initcall levels seem to be 
outdated since a long time anyway, and I think just speaking about the 
numbers 1-7 (or 0-14) would be better anyways. The only reason why I've 
used the fs (sync) level is because I was too lazy to check out if 
mtdcore might depend on something else in any other level. Therefor I've 
used the one most close to were it was before.

Also I've an idea about how to fix that in general for all drivers (by 
using the same algorithm I've now introduced just for DT-based drivers 
with a device description). Wouldn't be that hard to use that for all 
drivers, but as I've written in a follow up to the introductory mail, 
one step after another.

Regards,

Alexander Holler

  reply	other threads:[~2015-09-02  5:34 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-26 12:28 [PATCH 00/16] deps: deterministic driver initialization order Alexander Holler
2015-08-26 12:28 ` [PATCH 01/16] deps: dtc: Automatically add new property 'dependencies' which contains a list of referenced phandles Alexander Holler
2015-08-26 12:28 ` [PATCH 02/16] deps: dtc: Add option to print initialization order Alexander Holler
2015-08-26 12:28 ` [PATCH 03/16] deps: dtc: Add option to print dependency graph as dot (Graphviz) Alexander Holler
2015-08-26 12:28 ` [PATCH 04/16] deps: dtc: introduce new (virtual) property no-dependencies Alexander Holler
2015-08-26 12:28 ` [PATCH 05/16] deps: introduce initcalls annotated with a struct device_driver Alexander Holler
2015-08-26 12:28 ` [PATCH 06/16] deps: deterministic driver initialization sequence based on dependencies from the DT Alexander Holler
2015-08-26 12:28 ` [PATCH 07/16] deps: add debug configuration options Alexander Holler
2015-08-26 12:28 ` [PATCH 08/16] deps: dts: kirkwood: dockstar: add dependency ehci -> usb power regulator Alexander Holler
2015-08-26 12:28 ` [PATCH 09/16] deps: dts: imx6q: make some remote-endpoints non-dependencies Alexander Holler
2015-08-26 12:28 ` [PATCH 10/16] deps: dts: omap: beagle: " Alexander Holler
2015-08-26 12:28 ` [PATCH 11/16] deps: WIP: generic: annotate some initcalls Alexander Holler
2015-08-26 12:28 ` [PATCH 12/16] deps: WIP: imx: " Alexander Holler
2015-08-26 12:28 ` [PATCH 13/16] deps: WIP: omap: " Alexander Holler
2015-08-26 12:28 ` [PATCH 14/16] deps: WIP: kirkwood: " Alexander Holler
2015-08-26 12:28 ` [PATCH 15/16] mtd: mtdcore: fix initcall level Alexander Holler
2015-09-01 21:19   ` Brian Norris
2015-09-02  5:34     ` Alexander Holler [this message]
2015-09-04  4:00       ` Alexander Holler
2015-09-08 19:36         ` Alexander Holler
2015-08-26 12:28 ` [PATCH 16/16] phy: phy-core: " Alexander Holler
2015-09-18  6:16   ` Kishon Vijay Abraham I
2015-09-18  6:59     ` Alexander Holler
2015-08-27 19:01 ` [PATCH 00/16] deps: deterministic driver initialization order Alexander Holler
2015-08-27 19:15   ` Alexander Holler

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=55E68A72.30909@ahsoftware.de \
    --to=holler@ahsoftware.de \
    --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).