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

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

[Lu Baolu]

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 <qiuxu.zhuo@intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 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

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

[David Cohen]

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 <qiuxu.zhuo@intel.com>
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
>  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
> 

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

[Lu, Baolu]

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 <qiuxu.zhuo@intel.com>
>> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
>> ---
>>   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
>>
>

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

[Heikki Krogerus]

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 <qiuxu.zhuo@intel.com>
> > Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>

Adding Felipe. FWIW, my ACK in any case:

Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>


Thanks,

-- 
heikki

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

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 974 bytes --]

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Lu, Baolu]

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

>