2015-05-22 11:30:22

by Baolu Lu

[permalink] [raw]
Subject: [PATCH v3 1/1] usb: ulpi: ulpi_init should be executed in subsys_initcall

Phy drivers and the ulpi interface providers depend on the
registeration of the ulpi bus. Ulpi registers the bus in
module_init(). This could result in a load order issue, i.e.
ulpi phy drivers or the ulpi interface providers loading
before the bus registeration.

This patch fixes this load order issue by putting ulpi_init
in subsys_initcall().

Reported-by: Zhuo Qiuxu <[email protected]>
Signed-off-by: Lu Baolu <[email protected]>
---
drivers/usb/common/ulpi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
index 0e6f968..01c0c04 100644
--- a/drivers/usb/common/ulpi.c
+++ b/drivers/usb/common/ulpi.c
@@ -242,7 +242,7 @@ static int __init ulpi_init(void)
{
return bus_register(&ulpi_bus);
}
-module_init(ulpi_init);
+subsys_initcall(ulpi_init);

static void __exit ulpi_exit(void)
{
--
2.1.4


2015-05-22 16:06:35

by David Cohen

[permalink] [raw]
Subject: Re: [PATCH v3 1/1] usb: ulpi: ulpi_init should be executed in subsys_initcall

Hi,

On Fri, May 22, 2015 at 07:29:15PM +0800, Lu Baolu wrote:
> Phy drivers and the ulpi interface providers depend on the
> registeration of the ulpi bus. Ulpi registers the bus in
> module_init(). This could result in a load order issue, i.e.

It's still not an issue :(
I'd say "unnecessary probe delays".

But of cource it's Felipe's call :) Description looks better now.

BR, David

> ulpi phy drivers or the ulpi interface providers loading
> before the bus registeration.
>
> This patch fixes this load order issue by putting ulpi_init
> in subsys_initcall().
>
> Reported-by: Zhuo Qiuxu <[email protected]>
> Signed-off-by: Lu Baolu <[email protected]>
> ---
> drivers/usb/common/ulpi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
> index 0e6f968..01c0c04 100644
> --- a/drivers/usb/common/ulpi.c
> +++ b/drivers/usb/common/ulpi.c
> @@ -242,7 +242,7 @@ static int __init ulpi_init(void)
> {
> return bus_register(&ulpi_bus);
> }
> -module_init(ulpi_init);
> +subsys_initcall(ulpi_init);
>
> static void __exit ulpi_exit(void)
> {
> --
> 2.1.4
>

2015-05-25 06:24:05

by Baolu Lu

[permalink] [raw]
Subject: Re: [PATCH v3 1/1] usb: ulpi: ulpi_init should be executed in subsys_initcall



On 05/23/2015 12:08 AM, David Cohen wrote:
> Hi,
>
> On Fri, May 22, 2015 at 07:29:15PM +0800, Lu Baolu wrote:
>> Phy drivers and the ulpi interface providers depend on the
>> registeration of the ulpi bus. Ulpi registers the bus in
>> module_init(). This could result in a load order issue, i.e.
> It's still not an issue :(
> I'd say "unnecessary probe delays".

I managed to boot a kernel built from the top of Felipe's
remotes/origin/next branch under an Ubuntu environment
on Intel's Bay Trail tablet.

The same panic (as I found in the Android environment previously)
shows up as well. And if I replace module_init() with sys_initcall(),
the panic disappears.

Thanks,
-Baolu

>
> But of cource it's Felipe's call :) Description looks better now.
>
> BR, David
>
>> ulpi phy drivers or the ulpi interface providers loading
>> before the bus registeration.
>>
>> This patch fixes this load order issue by putting ulpi_init
>> in subsys_initcall().
>>
>> Reported-by: Zhuo Qiuxu <[email protected]>
>> Signed-off-by: Lu Baolu <[email protected]>
>> ---
>> drivers/usb/common/ulpi.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
>> index 0e6f968..01c0c04 100644
>> --- a/drivers/usb/common/ulpi.c
>> +++ b/drivers/usb/common/ulpi.c
>> @@ -242,7 +242,7 @@ static int __init ulpi_init(void)
>> {
>> return bus_register(&ulpi_bus);
>> }
>> -module_init(ulpi_init);
>> +subsys_initcall(ulpi_init);
>>
>> static void __exit ulpi_exit(void)
>> {
>> --
>> 2.1.4
>>
>

2015-05-25 12:09:08

by Heikki Krogerus

[permalink] [raw]
Subject: Re: [PATCH v3 1/1] usb: ulpi: ulpi_init should be executed in subsys_initcall

On Fri, May 22, 2015 at 09:08:45AM -0700, David Cohen wrote:
> Hi,
>
> On Fri, May 22, 2015 at 07:29:15PM +0800, Lu Baolu wrote:
> > Phy drivers and the ulpi interface providers depend on the
> > registeration of the ulpi bus. Ulpi registers the bus in
> > module_init(). This could result in a load order issue, i.e.
>
> It's still not an issue :(
> I'd say "unnecessary probe delays".
>
> But of cource it's Felipe's call :) Description looks better now.
>
> BR, David
>
> > ulpi phy drivers or the ulpi interface providers loading
> > before the bus registeration.
> >
> > This patch fixes this load order issue by putting ulpi_init
> > in subsys_initcall().
> >
> > Reported-by: Zhuo Qiuxu <[email protected]>
> > Signed-off-by: Lu Baolu <[email protected]>

Adding Felipe. FWIW, my ACK in any case:

Acked-by: Heikki Krogerus <[email protected]>


Thanks,

--
heikki

2015-05-26 14:52:49

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH v3 1/1] usb: ulpi: ulpi_init should be executed in subsys_initcall

Hi,

On Mon, May 25, 2015 at 02:24:00PM +0800, Lu, Baolu wrote:
>
>
> On 05/23/2015 12:08 AM, David Cohen wrote:
> >Hi,
> >
> >On Fri, May 22, 2015 at 07:29:15PM +0800, Lu Baolu wrote:
> >>Phy drivers and the ulpi interface providers depend on the
> >>registeration of the ulpi bus. Ulpi registers the bus in
> >>module_init(). This could result in a load order issue, i.e.
> >It's still not an issue :(
> >I'd say "unnecessary probe delays".
>
> I managed to boot a kernel built from the top of Felipe's
> remotes/origin/next branch under an Ubuntu environment
> on Intel's Bay Trail tablet.
>
> The same panic (as I found in the Android environment previously)
> shows up as well. And if I replace module_init() with sys_initcall(),
> the panic disappears.

the problem is something else... Moving things around in the init levels
is just a workaround for another issue. Seems like there's some missing
EPROBE_DEFER somewhere.

--
balbi


Attachments:
(No filename) (945.00 B)
signature.asc (819.00 B)
Digital signature
Download all attachments

2015-05-27 01:33:24

by Baolu Lu

[permalink] [raw]
Subject: Re: [PATCH v3 1/1] usb: ulpi: ulpi_init should be executed in subsys_initcall



On 05/26/2015 10:50 PM, Felipe Balbi wrote:
> Hi,
>
> On Mon, May 25, 2015 at 02:24:00PM +0800, Lu, Baolu wrote:
>>
>> On 05/23/2015 12:08 AM, David Cohen wrote:
>>> Hi,
>>>
>>> On Fri, May 22, 2015 at 07:29:15PM +0800, Lu Baolu wrote:
>>>> Phy drivers and the ulpi interface providers depend on the
>>>> registeration of the ulpi bus. Ulpi registers the bus in
>>>> module_init(). This could result in a load order issue, i.e.
>>> It's still not an issue :(
>>> I'd say "unnecessary probe delays".
>> I managed to boot a kernel built from the top of Felipe's
>> remotes/origin/next branch under an Ubuntu environment
>> on Intel's Bay Trail tablet.
>>
>> The same panic (as I found in the Android environment previously)
>> shows up as well. And if I replace module_init() with sys_initcall(),
>> the panic disappears.
> the problem is something else... Moving things around in the init levels
> is just a workaround for another issue. Seems like there's some missing
> EPROBE_DEFER somewhere.

Yes, I agree.

Heikki, I assume "missing EPROBE_DEFER issue" will be fixed by
Sasha's patch. I will resend the patch with a new commit
message.

Thanks,
Baolu

>