From: holler@ahsoftware.de (Alexander Holler)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 16/16] phy: phy-core: fix initcall level
Date: Fri, 18 Sep 2015 08:59:44 +0200 [thread overview]
Message-ID: <55FBB660.4020302@ahsoftware.de> (raw)
In-Reply-To: <55FBAC2D.1020301@ti.com>
Am 18.09.2015 um 08:16 schrieb Kishon Vijay Abraham I:
> Hi,
>
> On Wednesday 26 August 2015 05:58 PM, Alexander Holler wrote:
>> The phy-core has to be initialized before other dependent usb-drivers,
>> otherwise a crash might occur.
>>
>> Currently phy_core_init() is called in the initcall-level device, which is
>> the same level where most usb-drivers will end up. By luck this seemed to
>> have been called most of the time before other usb-drivers without having
>> been explicitly enforced. But if phy_core_init() is not called before a
>> dependent driver, a null-pointer exception might occur (e.g. because the
>> phy device class isn't registered).
>
> Did you actually face a problem? IIUC the modules get loaded based on
> the drivers/Makefile order (unless the other modules are in a different
> initcall table).
I had a problem while playing with a modified init-system (based on
dependencies). So not an actual problem.
> IMHO the fix should be in the module that caused the crash. Change it to
> use module_init?
The problem arises if the init-system ignores the link order and assumes
all drivers in the same initcall level can be called without any special
ordering.
The problem might also appear if a driver changes its name, directory or
position in file system. E.g. how to you make sure that a driver in
staging will be linked after the phy-core? Actually this happens, but I
would assume its by luck. I assume if staging would be renamed to
'beta-quality' a lot of stuff would actually fail because of the problem
with the implicit link order.
Anyway, nothing which really has to be fixed. It's just a notice that
maybe another initcall level of 'subsys' or something else before
'device' might be a better place for phy-core. I've chosen fs_sync
instead of subsys because otherwise I would have had to look up if
phy-core depends on another subsystem and therefore has to be
initialized after subsys.
Regards,
Alexander Holler
>
> Thanks
> Kishon
>
>>
>> To fix this, phy_core_init() is moved to the initcall-level fs (right
>> before the standard initcall level device).
>>
>> Signed-off-by: Alexander Holler <holler@ahsoftware.de>
>> ---
>> drivers/phy/phy-core.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
>> index fc48fac..4945029 100644
>> --- a/drivers/phy/phy-core.c
>> +++ b/drivers/phy/phy-core.c
>> @@ -930,7 +930,7 @@ static int __init phy_core_init(void)
>>
>> return 0;
>> }
>> -module_init(phy_core_init);
>> +fs_initcall_sync(phy_core_init);
>>
>> static void __exit phy_core_exit(void)
>> {
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
WARNING: multiple messages have this Message-ID (diff)
From: Alexander Holler <holler-SXC+2es9fhnfWeYVQQPykw@public.gmane.org>
To: Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Greg KH
<gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>,
Russel King <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>,
Andrew Morton
<akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>,
Grant Likely
<grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
Tomeu Vizoso
<tomeu.vizoso-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
Subject: Re: [PATCH 16/16] phy: phy-core: fix initcall level
Date: Fri, 18 Sep 2015 08:59:44 +0200 [thread overview]
Message-ID: <55FBB660.4020302@ahsoftware.de> (raw)
In-Reply-To: <55FBAC2D.1020301-l0cyMroinI0@public.gmane.org>
Am 18.09.2015 um 08:16 schrieb Kishon Vijay Abraham I:
> Hi,
>
> On Wednesday 26 August 2015 05:58 PM, Alexander Holler wrote:
>> The phy-core has to be initialized before other dependent usb-drivers,
>> otherwise a crash might occur.
>>
>> Currently phy_core_init() is called in the initcall-level device, which is
>> the same level where most usb-drivers will end up. By luck this seemed to
>> have been called most of the time before other usb-drivers without having
>> been explicitly enforced. But if phy_core_init() is not called before a
>> dependent driver, a null-pointer exception might occur (e.g. because the
>> phy device class isn't registered).
>
> Did you actually face a problem? IIUC the modules get loaded based on
> the drivers/Makefile order (unless the other modules are in a different
> initcall table).
I had a problem while playing with a modified init-system (based on
dependencies). So not an actual problem.
> IMHO the fix should be in the module that caused the crash. Change it to
> use module_init?
The problem arises if the init-system ignores the link order and assumes
all drivers in the same initcall level can be called without any special
ordering.
The problem might also appear if a driver changes its name, directory or
position in file system. E.g. how to you make sure that a driver in
staging will be linked after the phy-core? Actually this happens, but I
would assume its by luck. I assume if staging would be renamed to
'beta-quality' a lot of stuff would actually fail because of the problem
with the implicit link order.
Anyway, nothing which really has to be fixed. It's just a notice that
maybe another initcall level of 'subsys' or something else before
'device' might be a better place for phy-core. I've chosen fs_sync
instead of subsys because otherwise I would have had to look up if
phy-core depends on another subsystem and therefore has to be
initialized after subsys.
Regards,
Alexander Holler
>
> Thanks
> Kishon
>
>>
>> To fix this, phy_core_init() is moved to the initcall-level fs (right
>> before the standard initcall level device).
>>
>> Signed-off-by: Alexander Holler <holler-SXC+2es9fhnfWeYVQQPykw@public.gmane.org>
>> ---
>> drivers/phy/phy-core.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
>> index fc48fac..4945029 100644
>> --- a/drivers/phy/phy-core.c
>> +++ b/drivers/phy/phy-core.c
>> @@ -930,7 +930,7 @@ static int __init phy_core_init(void)
>>
>> return 0;
>> }
>> -module_init(phy_core_init);
>> +fs_initcall_sync(phy_core_init);
>>
>> static void __exit phy_core_exit(void)
>> {
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
WARNING: multiple messages have this Message-ID (diff)
From: Alexander Holler <holler@ahsoftware.de>
To: Kishon Vijay Abraham I <kishon@ti.com>, linux-kernel@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
Greg KH <gregkh@linuxfoundation.org>,
Russel King <linux@arm.linux.org.uk>,
Andrew Morton <akpm@linux-foundation.org>,
Grant Likely <grant.likely@linaro.org>,
Tomeu Vizoso <tomeu.vizoso@collabora.com>
Subject: Re: [PATCH 16/16] phy: phy-core: fix initcall level
Date: Fri, 18 Sep 2015 08:59:44 +0200 [thread overview]
Message-ID: <55FBB660.4020302@ahsoftware.de> (raw)
In-Reply-To: <55FBAC2D.1020301@ti.com>
Am 18.09.2015 um 08:16 schrieb Kishon Vijay Abraham I:
> Hi,
>
> On Wednesday 26 August 2015 05:58 PM, Alexander Holler wrote:
>> The phy-core has to be initialized before other dependent usb-drivers,
>> otherwise a crash might occur.
>>
>> Currently phy_core_init() is called in the initcall-level device, which is
>> the same level where most usb-drivers will end up. By luck this seemed to
>> have been called most of the time before other usb-drivers without having
>> been explicitly enforced. But if phy_core_init() is not called before a
>> dependent driver, a null-pointer exception might occur (e.g. because the
>> phy device class isn't registered).
>
> Did you actually face a problem? IIUC the modules get loaded based on
> the drivers/Makefile order (unless the other modules are in a different
> initcall table).
I had a problem while playing with a modified init-system (based on
dependencies). So not an actual problem.
> IMHO the fix should be in the module that caused the crash. Change it to
> use module_init?
The problem arises if the init-system ignores the link order and assumes
all drivers in the same initcall level can be called without any special
ordering.
The problem might also appear if a driver changes its name, directory or
position in file system. E.g. how to you make sure that a driver in
staging will be linked after the phy-core? Actually this happens, but I
would assume its by luck. I assume if staging would be renamed to
'beta-quality' a lot of stuff would actually fail because of the problem
with the implicit link order.
Anyway, nothing which really has to be fixed. It's just a notice that
maybe another initcall level of 'subsys' or something else before
'device' might be a better place for phy-core. I've chosen fs_sync
instead of subsys because otherwise I would have had to look up if
phy-core depends on another subsystem and therefore has to be
initialized after subsys.
Regards,
Alexander Holler
>
> Thanks
> Kishon
>
>>
>> To fix this, phy_core_init() is moved to the initcall-level fs (right
>> before the standard initcall level device).
>>
>> Signed-off-by: Alexander Holler <holler@ahsoftware.de>
>> ---
>> drivers/phy/phy-core.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
>> index fc48fac..4945029 100644
>> --- a/drivers/phy/phy-core.c
>> +++ b/drivers/phy/phy-core.c
>> @@ -930,7 +930,7 @@ static int __init phy_core_init(void)
>>
>> return 0;
>> }
>> -module_init(phy_core_init);
>> +fs_initcall_sync(phy_core_init);
>>
>> static void __exit phy_core_exit(void)
>> {
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
next prev parent reply other threads:[~2015-09-18 6:59 UTC|newest]
Thread overview: 70+ 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 ` Alexander Holler
2015-08-26 12:28 ` 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 ` 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 ` Alexander Holler
2015-08-26 12:28 ` 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 ` 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 ` 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 ` 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 ` Alexander Holler
2015-08-26 12:28 ` [PATCH 07/16] deps: add debug configuration options Alexander Holler
2015-08-26 12:28 ` 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 ` 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 ` Alexander Holler
2015-08-26 12:28 ` [PATCH 10/16] deps: dts: omap: beagle: " Alexander Holler
2015-08-26 12:28 ` Alexander Holler
2015-08-26 12:28 ` Alexander Holler
2015-08-26 12:28 ` [PATCH 11/16] deps: WIP: generic: annotate some initcalls Alexander Holler
2015-08-26 12:28 ` Alexander Holler
2015-08-26 12:28 ` [PATCH 12/16] deps: WIP: imx: " Alexander Holler
2015-08-26 12:28 ` Alexander Holler
2015-08-26 12:28 ` [PATCH 13/16] deps: WIP: omap: " Alexander Holler
2015-08-26 12:28 ` Alexander Holler
2015-08-26 12:28 ` [PATCH 14/16] deps: WIP: kirkwood: " Alexander Holler
2015-08-26 12:28 ` Alexander Holler
2015-08-26 12:28 ` [PATCH 15/16] mtd: mtdcore: fix initcall level Alexander Holler
2015-08-26 12:28 ` Alexander Holler
2015-09-01 21:19 ` Brian Norris
2015-09-01 21:19 ` Brian Norris
2015-09-01 21:19 ` Brian Norris
2015-09-02 5:34 ` Alexander Holler
2015-09-02 5:34 ` Alexander Holler
2015-09-04 4:00 ` Alexander Holler
2015-09-04 4:00 ` Alexander Holler
2015-09-04 4:00 ` Alexander Holler
2015-09-08 19:36 ` Alexander Holler
2015-09-08 19:36 ` Alexander Holler
2015-09-08 19:36 ` Alexander Holler
2015-08-26 12:28 ` [PATCH 16/16] phy: phy-core: " Alexander Holler
2015-08-26 12:28 ` Alexander Holler
2015-09-18 6:16 ` Kishon Vijay Abraham I
2015-09-18 6:16 ` Kishon Vijay Abraham I
2015-09-18 6:16 ` Kishon Vijay Abraham I
2015-09-18 6:59 ` Alexander Holler [this message]
2015-09-18 6:59 ` Alexander Holler
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:01 ` Alexander Holler
2015-08-27 19:15 ` Alexander Holler
2015-08-27 19:15 ` Alexander Holler
2015-08-27 19:15 ` Alexander Holler
[not found] ` <1440592108-3740-1-git-send-email-holler-SXC+2es9fhnfWeYVQQPykw@public.gmane.org>
2015-09-04 9:18 ` deps: update in regard to parallel initialization of static linked drivers Alexander Holler
2015-09-04 9:18 ` Alexander Holler
2015-09-09 18:35 ` [PATCH 0/2] deps: parallel initialization of (device-)drivers Alexander Holler
2015-09-09 18:35 ` Alexander Holler
2015-09-09 18:35 ` [PATCH 1/2] " Alexander Holler
2015-09-09 18:35 ` Alexander Holler
2015-09-09 18:35 ` [PATCH 2/2] deps: avoid multiple calls to memmove by just setting duplicates to 0 Alexander Holler
2015-09-09 18:35 ` Alexander Holler
2015-09-14 19:53 ` [PATCH 0/2] deps: parallel initialization of (device-)drivers Alexander Holler
2015-09-14 19:53 ` Alexander Holler
2015-09-14 19:53 ` 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=55FBB660.4020302@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 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.