linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] OMAP: fix boot sequence
@ 2013-04-23 13:19 Grygorii Strashko
  2013-04-23 13:19 ` [PATCH 1/2] i2c: omap: convert to module_platform_driver() Grygorii Strashko
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Grygorii Strashko @ 2013-04-23 13:19 UTC (permalink / raw)
  To: Tony Lindgren, Samuel Ortiz
  Cc: Grygorii Strashko, Wolfram Sang, Ben Dooks (embedded platforms),
	Santosh Shilimkar, linux-omap, linux-i2c, linux-kernel

Hi

There are two public discussions now related to OMAP boot and drivers
initialization issues:
"Multiple issues with omap4 panda es in linux next"
  http://www.spinics.net/lists/linux-omap/msg90241.html
"[BUG] omap: mfd/regulator: twl/core: init order"
  http://www.spinics.net/lists/linux-omap/msg89980.html

In both cases there are pinctrl-single/I2C/MFD/Regulators initailization issue:
- regulators are not initialized because of twl,
- twl is not initialized because of I2C,
- I2C is not initialized because of pinctrl-single,
- pinctrl-single is initialized at mudule/device init time.
So, most everything will be shifted at late_initcall time. 

This may cause boot delay (more over, it can broken initialization of drivers
which are not ready to use deferred probe mechanism yet, for example DSS).

Introduced pathes shift I2C and TWL iniialization to module/device init layer
instead of subsys init layer where initialization dependencies resolved
indirectly in drivers/Makefile now.

Grygorii Strashko (2):
  i2c: omap: convert to module_platform_driver()
  mfd: twl-core: convert to module_i2c_driver()

 drivers/i2c/busses/i2c-omap.c |   14 +-------------
 drivers/mfd/twl-core.c        |   12 +-----------
 2 files changed, 2 insertions(+), 24 deletions(-)

Regards,
-grygorii

Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: "Ben Dooks (embedded platforms)" <ben-linux@fluff.org>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: linux-omap@vger.kernel.org
Cc: linux-i2c@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

-- 
1.7.9.5


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/2] i2c: omap: convert to module_platform_driver()
  2013-04-23 13:19 [PATCH 0/2] OMAP: fix boot sequence Grygorii Strashko
@ 2013-04-23 13:19 ` Grygorii Strashko
  2013-06-03 20:59   ` Kevin Hilman
  2013-04-23 13:19 ` [PATCH 2/2] mfd: twl-core: convert to module_i2c_driver() Grygorii Strashko
  2013-04-23 17:34 ` [PATCH 0/2] OMAP: fix boot sequence Tony Lindgren
  2 siblings, 1 reply; 14+ messages in thread
From: Grygorii Strashko @ 2013-04-23 13:19 UTC (permalink / raw)
  To: Tony Lindgren, Samuel Ortiz
  Cc: Grygorii Strashko, Wolfram Sang, Ben Dooks (embedded platforms),
	Santosh Shilimkar, linux-omap, linux-i2c, linux-kernel

The OMAP I2C driver has a relation to pinctrl-single driver. As result,
its probe will be deferred during system boot until late init time,
because the pinctrl-single is initizalized as moudle/device init time.
This, in turn, will delay initialization of all I2C devices (like mfd,
I2C regulators and etc.) and cause boot delay (more over, it can broken
initialization of drivers which are not ready to use deferred probe
mechanism yet, for example DSS).

There are no sense to keep OMAP I2C initialization on subsys init layer
any more, hence shift it to module/device layer where the i2c <-->
pinctrl-single dependency is resolved in drivers/Makefile now.

Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: "Ben Dooks (embedded platforms)" <ben-linux@fluff.org>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: linux-omap@vger.kernel.org
Cc: linux-i2c@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/i2c/busses/i2c-omap.c |   14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 4cc2f05..70d3fed 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -1342,19 +1342,7 @@ static struct platform_driver omap_i2c_driver = {
 	},
 };
 
-/* I2C may be needed to bring up other drivers */
-static int __init
-omap_i2c_init_driver(void)
-{
-	return platform_driver_register(&omap_i2c_driver);
-}
-subsys_initcall(omap_i2c_init_driver);
-
-static void __exit omap_i2c_exit_driver(void)
-{
-	platform_driver_unregister(&omap_i2c_driver);
-}
-module_exit(omap_i2c_exit_driver);
+module_platform_driver(omap_i2c_driver);
 
 MODULE_AUTHOR("MontaVista Software, Inc. (and others)");
 MODULE_DESCRIPTION("TI OMAP I2C bus adapter");
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/2] mfd: twl-core: convert to module_i2c_driver()
  2013-04-23 13:19 [PATCH 0/2] OMAP: fix boot sequence Grygorii Strashko
  2013-04-23 13:19 ` [PATCH 1/2] i2c: omap: convert to module_platform_driver() Grygorii Strashko
@ 2013-04-23 13:19 ` Grygorii Strashko
  2013-05-16 22:17   ` Samuel Ortiz
  2013-04-23 17:34 ` [PATCH 0/2] OMAP: fix boot sequence Tony Lindgren
  2 siblings, 1 reply; 14+ messages in thread
From: Grygorii Strashko @ 2013-04-23 13:19 UTC (permalink / raw)
  To: Tony Lindgren, Samuel Ortiz
  Cc: Grygorii Strashko, Santosh Shilimkar, linux-omap, linux-kernel

Shift TWL initialization to module/device init layer, because I2C now is
not initialized on subsys init layer and shifted to module/device init
layer instead.

The I2C <--> TWL dependency should be resolved in drivers/Makefile now.

Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Cc: linux-omap@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/mfd/twl-core.c |   12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index 4f3baad..f01340d 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -1347,17 +1347,7 @@ static struct i2c_driver twl_driver = {
 	.remove		= twl_remove,
 };
 
-static int __init twl_init(void)
-{
-	return i2c_add_driver(&twl_driver);
-}
-subsys_initcall(twl_init);
-
-static void __exit twl_exit(void)
-{
-	i2c_del_driver(&twl_driver);
-}
-module_exit(twl_exit);
+module_i2c_driver(twl_driver);
 
 MODULE_AUTHOR("Texas Instruments, Inc.");
 MODULE_DESCRIPTION("I2C Core interface for TWL");
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/2] OMAP: fix boot sequence
  2013-04-23 13:19 [PATCH 0/2] OMAP: fix boot sequence Grygorii Strashko
  2013-04-23 13:19 ` [PATCH 1/2] i2c: omap: convert to module_platform_driver() Grygorii Strashko
  2013-04-23 13:19 ` [PATCH 2/2] mfd: twl-core: convert to module_i2c_driver() Grygorii Strashko
@ 2013-04-23 17:34 ` Tony Lindgren
  2013-04-23 17:50   ` Grygorii Strashko
  2 siblings, 1 reply; 14+ messages in thread
From: Tony Lindgren @ 2013-04-23 17:34 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: Samuel Ortiz, Wolfram Sang, Ben Dooks (embedded platforms),
	Santosh Shilimkar, linux-omap, linux-i2c, linux-kernel

* Grygorii Strashko <grygorii.strashko@ti.com> [130423 06:25]:
> Hi
> 
> There are two public discussions now related to OMAP boot and drivers
> initialization issues:
> "Multiple issues with omap4 panda es in linux next"
>   http://www.spinics.net/lists/linux-omap/msg90241.html
> "[BUG] omap: mfd/regulator: twl/core: init order"
>   http://www.spinics.net/lists/linux-omap/msg89980.html
> 
> In both cases there are pinctrl-single/I2C/MFD/Regulators initailization issue:
> - regulators are not initialized because of twl,
> - twl is not initialized because of I2C,
> - I2C is not initialized because of pinctrl-single,
> - pinctrl-single is initialized at mudule/device init time.
> So, most everything will be shifted at late_initcall time. 
> 
> This may cause boot delay (more over, it can broken initialization of drivers
> which are not ready to use deferred probe mechanism yet, for example DSS).
> 
> Introduced pathes shift I2C and TWL iniialization to module/device init layer
> instead of subsys init layer where initialization dependencies resolved
> indirectly in drivers/Makefile now.
> 
> Grygorii Strashko (2):
>   i2c: omap: convert to module_platform_driver()
>   mfd: twl-core: convert to module_i2c_driver()
> 
>  drivers/i2c/busses/i2c-omap.c |   14 +-------------
>  drivers/mfd/twl-core.c        |   12 +-----------
>  2 files changed, 2 insertions(+), 24 deletions(-)

Thanks, I have few questions:

1. It seems that the real fix to the issues we're seeing
   is to make the broken drivers to support -EPROBE_DEFER?

2. If so, can these be merged later on as clean-up?

3. Can these two patches be merged separately without
   breaking things?

Regards,

Tony

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/2] OMAP: fix boot sequence
  2013-04-23 17:34 ` [PATCH 0/2] OMAP: fix boot sequence Tony Lindgren
@ 2013-04-23 17:50   ` Grygorii Strashko
  0 siblings, 0 replies; 14+ messages in thread
From: Grygorii Strashko @ 2013-04-23 17:50 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Samuel Ortiz, Wolfram Sang, Ben Dooks (embedded platforms),
	Santosh Shilimkar, linux-omap, linux-i2c, linux-kernel,
	Tomi Valkeinen

On 04/23/2013 08:34 PM, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [130423 06:25]:
>> Hi
>>
>> There are two public discussions now related to OMAP boot and drivers
>> initialization issues:
>> "Multiple issues with omap4 panda es in linux next"
>>    http://www.spinics.net/lists/linux-omap/msg90241.html
>> "[BUG] omap: mfd/regulator: twl/core: init order"
>>    http://www.spinics.net/lists/linux-omap/msg89980.html
>>
>> In both cases there are pinctrl-single/I2C/MFD/Regulators initailization issue:
>> - regulators are not initialized because of twl,
>> - twl is not initialized because of I2C,
>> - I2C is not initialized because of pinctrl-single,
>> - pinctrl-single is initialized at mudule/device init time.
>> So, most everything will be shifted at late_initcall time.
>>
>> This may cause boot delay (more over, it can broken initialization of drivers
>> which are not ready to use deferred probe mechanism yet, for example DSS).
>>
>> Introduced pathes shift I2C and TWL iniialization to module/device init layer
>> instead of subsys init layer where initialization dependencies resolved
>> indirectly in drivers/Makefile now.
>>
>> Grygorii Strashko (2):
>>    i2c: omap: convert to module_platform_driver()
>>    mfd: twl-core: convert to module_i2c_driver()
>>
>>   drivers/i2c/busses/i2c-omap.c |   14 +-------------
>>   drivers/mfd/twl-core.c        |   12 +-----------
>>   2 files changed, 2 insertions(+), 24 deletions(-)
> Thanks, I have few questions:
>
> 1. It seems that the real fix to the issues we're seeing
>     is to make the broken drivers to support -EPROBE_DEFER?
yes.
>
> 2. If so, can these be merged later on as clean-up?
if i understand Tomi correctly, he has problems with DSS updating to use
  -EPROBE_DEFER, so this is rather fix than clean-up.
But need confirmation that it's true from him.
>
> 3. Can these two patches be merged separately without
>     breaking things?
I think, yes.
>
> Regards,
>
> Tony


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] mfd: twl-core: convert to module_i2c_driver()
  2013-04-23 13:19 ` [PATCH 2/2] mfd: twl-core: convert to module_i2c_driver() Grygorii Strashko
@ 2013-05-16 22:17   ` Samuel Ortiz
  2013-05-16 22:58     ` Tony Lindgren
  0 siblings, 1 reply; 14+ messages in thread
From: Samuel Ortiz @ 2013-05-16 22:17 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: Tony Lindgren, Santosh Shilimkar, linux-omap, linux-kernel

Hi Grygorii,

On Tue, Apr 23, 2013 at 04:19:10PM +0300, Grygorii Strashko wrote:
> Shift TWL initialization to module/device init layer, because I2C now is
> not initialized on subsys init layer and shifted to module/device init
> layer instead.
> 
> The I2C <--> TWL dependency should be resolved in drivers/Makefile now.
>
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Cc: linux-omap@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> 
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
>  drivers/mfd/twl-core.c |   12 +-----------
>  1 file changed, 1 insertion(+), 11 deletions(-)
I applied this one to mfd-next for now, and will move it to mfd-fixes if
someone confirms that this is indeed a fix.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] mfd: twl-core: convert to module_i2c_driver()
  2013-05-16 22:17   ` Samuel Ortiz
@ 2013-05-16 22:58     ` Tony Lindgren
  2013-05-17 10:46       ` Grygorii Strashko
  0 siblings, 1 reply; 14+ messages in thread
From: Tony Lindgren @ 2013-05-16 22:58 UTC (permalink / raw)
  To: Samuel Ortiz
  Cc: Grygorii Strashko, Santosh Shilimkar, linux-omap, linux-kernel

* Samuel Ortiz <sameo@linux.intel.com> [130516 15:22]:
> Hi Grygorii,
> 
> On Tue, Apr 23, 2013 at 04:19:10PM +0300, Grygorii Strashko wrote:
> > Shift TWL initialization to module/device init layer, because I2C now is
> > not initialized on subsys init layer and shifted to module/device init
> > layer instead.
> > 
> > The I2C <--> TWL dependency should be resolved in drivers/Makefile now.
> >
> > Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> > Cc: linux-omap@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > 
> > Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> > ---
> >  drivers/mfd/twl-core.c |   12 +-----------
> >  1 file changed, 1 insertion(+), 11 deletions(-)
> I applied this one to mfd-next for now, and will move it to mfd-fixes if
> someone confirms that this is indeed a fix.

Thanks, AFAIK it can wait until v3.11 merge window.

Regards,

Tony

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/2] mfd: twl-core: convert to module_i2c_driver()
  2013-05-16 22:58     ` Tony Lindgren
@ 2013-05-17 10:46       ` Grygorii Strashko
  0 siblings, 0 replies; 14+ messages in thread
From: Grygorii Strashko @ 2013-05-17 10:46 UTC (permalink / raw)
  To: Tony Lindgren; +Cc: Samuel Ortiz, Santosh Shilimkar, linux-omap, linux-kernel

On 05/17/2013 01:58 AM, Tony Lindgren wrote:
> * Samuel Ortiz <sameo@linux.intel.com> [130516 15:22]:
>> Hi Grygorii,
>>
>> On Tue, Apr 23, 2013 at 04:19:10PM +0300, Grygorii Strashko wrote:
>>> Shift TWL initialization to module/device init layer, because I2C now is
>>> not initialized on subsys init layer and shifted to module/device init
>>> layer instead.
>>>
>>> The I2C <--> TWL dependency should be resolved in drivers/Makefile now.
>>>
>>> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
>>> Cc: linux-omap@vger.kernel.org
>>> Cc: linux-kernel@vger.kernel.org
>>>
>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>>> ---
>>>   drivers/mfd/twl-core.c |   12 +-----------
>>>   1 file changed, 1 insertion(+), 11 deletions(-)
>> I applied this one to mfd-next for now, and will move it to mfd-fixes if
>> someone confirms that this is indeed a fix.
> Thanks, AFAIK it can wait until v3.11 merge window.
>
> Regards,
>
> Tony
Thanks, it's optimization which reduces the number of deferred probes at 
boot

Regards,
-grygorii



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] i2c: omap: convert to module_platform_driver()
  2013-04-23 13:19 ` [PATCH 1/2] i2c: omap: convert to module_platform_driver() Grygorii Strashko
@ 2013-06-03 20:59   ` Kevin Hilman
  2013-06-04 11:32     ` Grygorii Strashko
  0 siblings, 1 reply; 14+ messages in thread
From: Kevin Hilman @ 2013-06-03 20:59 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: Tony Lindgren, Samuel Ortiz, Wolfram Sang,
	Ben Dooks (embedded platforms),
	Santosh Shilimkar, linux-omap, linux-i2c, linux-kernel

Grygorii Strashko <grygorii.strashko@ti.com> writes:

> The OMAP I2C driver has a relation to pinctrl-single driver. As result,
> its probe will be deferred during system boot until late init time,
> because the pinctrl-single is initizalized as moudle/device init time.
> This, in turn, will delay initialization of all I2C devices (like mfd,
> I2C regulators and etc.) and cause boot delay (more over, it can broken
> initialization of drivers which are not ready to use deferred probe
> mechanism yet, for example DSS).
>
> There are no sense to keep OMAP I2C initialization on subsys init layer
> any more, hence shift it to module/device layer where the i2c <-->
> pinctrl-single dependency is resolved in drivers/Makefile now.
>
> Cc: Wolfram Sang <wsa@the-dreams.de>
> Cc: "Ben Dooks (embedded platforms)" <ben-linux@fluff.org>
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Cc: linux-omap@vger.kernel.org
> Cc: linux-i2c@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>

Testing this patch with PATCH 1/2, the twl_rtc driver fails to correctly
initialize on OMAP3:

         twl_rtc rtc.22: hctosys: invalid date/time

instead of the expected result:

          twl_rtc rtc.22: setting system clock to 2000-01-01 00:00:00 UTC (946684800)

so something is still not right for the init sequence.

Kevin

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] i2c: omap: convert to module_platform_driver()
  2013-06-03 20:59   ` Kevin Hilman
@ 2013-06-04 11:32     ` Grygorii Strashko
  2013-06-04 18:29       ` Kevin Hilman
  0 siblings, 1 reply; 14+ messages in thread
From: Grygorii Strashko @ 2013-06-04 11:32 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: Tony Lindgren, Samuel Ortiz, Wolfram Sang,
	Ben Dooks (embedded platforms),
	Santosh Shilimkar, linux-omap, linux-i2c, linux-kernel

Hi Kevin,
On 06/03/2013 11:59 PM, Kevin Hilman wrote:
> Grygorii Strashko <grygorii.strashko@ti.com> writes:
>
>> The OMAP I2C driver has a relation to pinctrl-single driver. As result,
>> its probe will be deferred during system boot until late init time,
>> because the pinctrl-single is initizalized as moudle/device init time.
>> This, in turn, will delay initialization of all I2C devices (like mfd,
>> I2C regulators and etc.) and cause boot delay (more over, it can broken
>> initialization of drivers which are not ready to use deferred probe
>> mechanism yet, for example DSS).
>>
>> There are no sense to keep OMAP I2C initialization on subsys init layer
>> any more, hence shift it to module/device layer where the i2c <-->
>> pinctrl-single dependency is resolved in drivers/Makefile now.
>>
>> Cc: Wolfram Sang <wsa@the-dreams.de>
>> Cc: "Ben Dooks (embedded platforms)" <ben-linux@fluff.org>
>> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
>> Cc: linux-omap@vger.kernel.org
>> Cc: linux-i2c@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> Testing this patch with PATCH 1/2, the twl_rtc driver fails to correctly
> initialize on OMAP3:
>
>           twl_rtc rtc.22: hctosys: invalid date/time
>
> instead of the expected result:
>
>            twl_rtc rtc.22: setting system clock to 2000-01-01 00:00:00 UTC (946684800)
>
> so something is still not right for the init sequence.
>
> Kevin
I think, the root cause of the problem isn't this patch - it's just yet 
another side effect
of using deferred probes :). I've taken a look on twl-rtc code and found 
possible error place
static int __init twl_rtc_init(void)
{
     if (twl_class_is_4030()) <------ here,
         rtc_reg_map = (u8 *) twl4030_rtc_reg_map;
     else
         rtc_reg_map = (u8 *) twl6030_rtc_reg_map;

     return platform_driver_register(&twl4030rtc_driver);
}
In drivers/Makefile:
obj-$(CONFIG_RTC_LIB)        += rtc/  <------ RTC is placed before I2C
obj-y                += i2c/ media/ <----- and only here I2C bus 
instantiates TWL-core device
and configures twl_priv->twl_id

Thats why it's working on my OMAP4/twl6030 board. Could you check if 
below fix will work on OMAP3:
diff --git a/drivers/rtc/rtc-twl.c b/drivers/rtc/rtc-twl.c
index 8751a52..aaa5015 100644
--- a/drivers/rtc/rtc-twl.c
+++ b/drivers/rtc/rtc-twl.c
@@ -469,6 +469,11 @@ static int twl_rtc_probe(struct platform_device *pdev)
         if (irq <= 0)
                 goto out1;

+       if (twl_class_is_4030())
+               rtc_reg_map = (u8 *) twl4030_rtc_reg_map;
+       else
+               rtc_reg_map = (u8 *) twl6030_rtc_reg_map;
+
         ret = twl_rtc_read_u8(&rd_reg, REG_RTC_STATUS_REG);
         if (ret < 0)
                 goto out1;
@@ -610,11 +615,6 @@ static struct platform_driver twl4030rtc_driver = {

  static int __init twl_rtc_init(void)
  {
-       if (twl_class_is_4030())
-               rtc_reg_map = (u8 *) twl4030_rtc_reg_map;
-       else
-               rtc_reg_map = (u8 *) twl6030_rtc_reg_map;
-
         return platform_driver_register(&twl4030rtc_driver);
  }
  module_init(twl_rtc_init);

Unfortunately, I have no OMAP3 HW and can't check it.

Regards,
-grygorii



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] i2c: omap: convert to module_platform_driver()
  2013-06-04 11:32     ` Grygorii Strashko
@ 2013-06-04 18:29       ` Kevin Hilman
  2013-06-05 11:28         ` Grygorii Strashko
  0 siblings, 1 reply; 14+ messages in thread
From: Kevin Hilman @ 2013-06-04 18:29 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: Tony Lindgren, Samuel Ortiz, Wolfram Sang,
	Ben Dooks (embedded platforms),
	Santosh Shilimkar, linux-omap, linux-i2c, linux-kernel

Grygorii Strashko <grygorii.strashko@ti.com> writes:

> Hi Kevin,
> On 06/03/2013 11:59 PM, Kevin Hilman wrote:
>> Grygorii Strashko <grygorii.strashko@ti.com> writes:
>>
>>> The OMAP I2C driver has a relation to pinctrl-single driver. As result,
>>> its probe will be deferred during system boot until late init time,
>>> because the pinctrl-single is initizalized as moudle/device init time.
>>> This, in turn, will delay initialization of all I2C devices (like mfd,
>>> I2C regulators and etc.) and cause boot delay (more over, it can broken
>>> initialization of drivers which are not ready to use deferred probe
>>> mechanism yet, for example DSS).
>>>
>>> There are no sense to keep OMAP I2C initialization on subsys init layer
>>> any more, hence shift it to module/device layer where the i2c <-->
>>> pinctrl-single dependency is resolved in drivers/Makefile now.
>>>
>>> Cc: Wolfram Sang <wsa@the-dreams.de>
>>> Cc: "Ben Dooks (embedded platforms)" <ben-linux@fluff.org>
>>> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
>>> Cc: linux-omap@vger.kernel.org
>>> Cc: linux-i2c@vger.kernel.org
>>> Cc: linux-kernel@vger.kernel.org
>>>
>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>> Testing this patch with PATCH 1/2, the twl_rtc driver fails to correctly
>> initialize on OMAP3:
>>
>>           twl_rtc rtc.22: hctosys: invalid date/time
>>
>> instead of the expected result:
>>
>>            twl_rtc rtc.22: setting system clock to 2000-01-01 00:00:00 UTC (946684800)
>>
>> so something is still not right for the init sequence.
>>
>> Kevin
> I think, the root cause of the problem isn't this patch - it's just
> yet another side effect
> of using deferred probes :). I've taken a look on twl-rtc code and
> found possible error place
> static int __init twl_rtc_init(void)
> {
>     if (twl_class_is_4030()) <------ here,
>         rtc_reg_map = (u8 *) twl4030_rtc_reg_map;
>     else
>         rtc_reg_map = (u8 *) twl6030_rtc_reg_map;
>
>     return platform_driver_register(&twl4030rtc_driver);
> }
> In drivers/Makefile:
> obj-$(CONFIG_RTC_LIB)        += rtc/  <------ RTC is placed before I2C
> obj-y                += i2c/ media/ <----- and only here I2C bus
> instantiates TWL-core device
> and configures twl_priv->twl_id
>
> Thats why it's working on my OMAP4/twl6030 board. Could you check if
> below fix will work on OMAP3:

Yes, your fix works.  Thanks for digging into it.

Care to send a proper patch?  Please be sure to send to Andrew Morton
also, since he'll be the one to queue the RTC patch.

> diff --git a/drivers/rtc/rtc-twl.c b/drivers/rtc/rtc-twl.c
> index 8751a52..aaa5015 100644
> --- a/drivers/rtc/rtc-twl.c
> +++ b/drivers/rtc/rtc-twl.c
> @@ -469,6 +469,11 @@ static int twl_rtc_probe(struct platform_device *pdev)
>         if (irq <= 0)
>                 goto out1;
>
> +       if (twl_class_is_4030())
> +               rtc_reg_map = (u8 *) twl4030_rtc_reg_map;
> +       else
> +               rtc_reg_map = (u8 *) twl6030_rtc_reg_map;
> +
>         ret = twl_rtc_read_u8(&rd_reg, REG_RTC_STATUS_REG);
>         if (ret < 0)
>                 goto out1;
> @@ -610,11 +615,6 @@ static struct platform_driver twl4030rtc_driver = {
>
>  static int __init twl_rtc_init(void)
>  {
> -       if (twl_class_is_4030())
> -               rtc_reg_map = (u8 *) twl4030_rtc_reg_map;
> -       else
> -               rtc_reg_map = (u8 *) twl6030_rtc_reg_map;
> -
>         return platform_driver_register(&twl4030rtc_driver);
>  }
>  module_init(twl_rtc_init);
>
> Unfortunately, I have no OMAP3 HW and can't check it.

I suggest you find a Beagle or Gumstix/Overo type board someplace.
There is an abundance of cheap OMAP3 hardware available.

Kevin


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] i2c: omap: convert to module_platform_driver()
  2013-06-04 18:29       ` Kevin Hilman
@ 2013-06-05 11:28         ` Grygorii Strashko
  2013-06-05 13:50           ` Wolfram Sang
  0 siblings, 1 reply; 14+ messages in thread
From: Grygorii Strashko @ 2013-06-05 11:28 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: Tony Lindgren, Samuel Ortiz, Wolfram Sang,
	Ben Dooks (embedded platforms),
	Santosh Shilimkar, linux-omap, linux-i2c, linux-kernel

On 06/04/2013 09:29 PM, Kevin Hilman wrote:
> Grygorii Strashko <grygorii.strashko@ti.com> writes:
>
>> Hi Kevin,
>> On 06/03/2013 11:59 PM, Kevin Hilman wrote:
>>> Grygorii Strashko <grygorii.strashko@ti.com> writes:
>>>
>>>> The OMAP I2C driver has a relation to pinctrl-single driver. As result,
>>>> its probe will be deferred during system boot until late init time,
>>>> because the pinctrl-single is initizalized as moudle/device init time.
>>>> This, in turn, will delay initialization of all I2C devices (like mfd,
>>>> I2C regulators and etc.) and cause boot delay (more over, it can broken
>>>> initialization of drivers which are not ready to use deferred probe
>>>> mechanism yet, for example DSS).
>>>>
>>>> There are no sense to keep OMAP I2C initialization on subsys init layer
>>>> any more, hence shift it to module/device layer where the i2c <-->
>>>> pinctrl-single dependency is resolved in drivers/Makefile now.
>>>>
>>>> Cc: Wolfram Sang <wsa@the-dreams.de>
>>>> Cc: "Ben Dooks (embedded platforms)" <ben-linux@fluff.org>
>>>> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
>>>> Cc: linux-omap@vger.kernel.org
>>>> Cc: linux-i2c@vger.kernel.org
>>>> Cc: linux-kernel@vger.kernel.org
>>>>
>>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>>> Testing this patch with PATCH 1/2, the twl_rtc driver fails to correctly
>>> initialize on OMAP3:
>>>
>>>            twl_rtc rtc.22: hctosys: invalid date/time
>>>
>>> instead of the expected result:
>>>
>>>             twl_rtc rtc.22: setting system clock to 2000-01-01 00:00:00 UTC (946684800)
>>>
>>> so something is still not right for the init sequence.
>>>
>>> Kevin
>> I think, the root cause of the problem isn't this patch - it's just
>> yet another side effect
>> of using deferred probes :). I've taken a look on twl-rtc code and
>> found possible error place
>> static int __init twl_rtc_init(void)
>> {
>>      if (twl_class_is_4030()) <------ here,
>>          rtc_reg_map = (u8 *) twl4030_rtc_reg_map;
>>      else
>>          rtc_reg_map = (u8 *) twl6030_rtc_reg_map;
>>
>>      return platform_driver_register(&twl4030rtc_driver);
>> }
>> In drivers/Makefile:
>> obj-$(CONFIG_RTC_LIB)        += rtc/  <------ RTC is placed before I2C
>> obj-y                += i2c/ media/ <----- and only here I2C bus
>> instantiates TWL-core device
>> and configures twl_priv->twl_id
>>
>> Thats why it's working on my OMAP4/twl6030 board. Could you check if
>> below fix will work on OMAP3:
> Yes, your fix works.  Thanks for digging into it.
>
> Care to send a proper patch?  Please be sure to send to Andrew Morton
> also, since he'll be the one to queue the RTC patch.
Hi Kevin,

The similar patch already exists:
https://patchwork.kernel.org/patch/2448251/ - [v2,1/2] RTC: rtc-twl: Fix 
rtc_reg_map initialization
from Peter Ujfalusi

-grygorii

>
>> diff --git a/drivers/rtc/rtc-twl.c b/drivers/rtc/rtc-twl.c
>> index 8751a52..aaa5015 100644
>> --- a/drivers/rtc/rtc-twl.c
>> +++ b/drivers/rtc/rtc-twl.c
>> @@ -469,6 +469,11 @@ static int twl_rtc_probe(struct platform_device *pdev)
>>          if (irq <= 0)
>>                  goto out1;
>>
>> +       if (twl_class_is_4030())
>> +               rtc_reg_map = (u8 *) twl4030_rtc_reg_map;
>> +       else
>> +               rtc_reg_map = (u8 *) twl6030_rtc_reg_map;
>> +
>>          ret = twl_rtc_read_u8(&rd_reg, REG_RTC_STATUS_REG);
>>          if (ret < 0)
>>                  goto out1;
>> @@ -610,11 +615,6 @@ static struct platform_driver twl4030rtc_driver = {
>>
>>   static int __init twl_rtc_init(void)
>>   {
>> -       if (twl_class_is_4030())
>> -               rtc_reg_map = (u8 *) twl4030_rtc_reg_map;
>> -       else
>> -               rtc_reg_map = (u8 *) twl6030_rtc_reg_map;
>> -
>>          return platform_driver_register(&twl4030rtc_driver);
>>   }
>>   module_init(twl_rtc_init);
>>
>> Unfortunately, I have no OMAP3 HW and can't check it.
> I suggest you find a Beagle or Gumstix/Overo type board someplace.
> There is an abundance of cheap OMAP3 hardware available.
>
> Kevin
>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] i2c: omap: convert to module_platform_driver()
  2013-06-05 11:28         ` Grygorii Strashko
@ 2013-06-05 13:50           ` Wolfram Sang
  2013-06-06 14:58             ` Grygorii Strashko
  0 siblings, 1 reply; 14+ messages in thread
From: Wolfram Sang @ 2013-06-05 13:50 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: Kevin Hilman, Tony Lindgren, Samuel Ortiz,
	Ben Dooks (embedded platforms),
	Santosh Shilimkar, linux-omap, linux-i2c, linux-kernel

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


> The similar patch already exists:
> https://patchwork.kernel.org/patch/2448251/ - [v2,1/2] RTC: rtc-twl:
> Fix rtc_reg_map initialization
> from Peter Ujfalusi

So, I think it is best if you resend this patch after all the fixes it
needs are applied or you resend the series with all patches it depends
on. Which should then probably go via arm-soc. Or?


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] i2c: omap: convert to module_platform_driver()
  2013-06-05 13:50           ` Wolfram Sang
@ 2013-06-06 14:58             ` Grygorii Strashko
  0 siblings, 0 replies; 14+ messages in thread
From: Grygorii Strashko @ 2013-06-06 14:58 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Kevin Hilman, Tony Lindgren, Samuel Ortiz,
	Ben Dooks (embedded platforms),
	Santosh Shilimkar, linux-omap, linux-i2c, linux-kernel

On 06/05/2013 04:50 PM, Wolfram Sang wrote:
>> The similar patch already exists:
>> https://patchwork.kernel.org/patch/2448251/ - [v2,1/2] RTC: rtc-twl:
>> Fix rtc_reg_map initialization
>> from Peter Ujfalusi
> So, I think it is best if you resend this patch after all the fixes it
> needs are applied or you resend the series with all patches it depends
> on. Which should then probably go via arm-soc. Or?
>
It can wait - I'll track and resend after twl-rtc patches.

-grygorii


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2013-06-06 15:02 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-04-23 13:19 [PATCH 0/2] OMAP: fix boot sequence Grygorii Strashko
2013-04-23 13:19 ` [PATCH 1/2] i2c: omap: convert to module_platform_driver() Grygorii Strashko
2013-06-03 20:59   ` Kevin Hilman
2013-06-04 11:32     ` Grygorii Strashko
2013-06-04 18:29       ` Kevin Hilman
2013-06-05 11:28         ` Grygorii Strashko
2013-06-05 13:50           ` Wolfram Sang
2013-06-06 14:58             ` Grygorii Strashko
2013-04-23 13:19 ` [PATCH 2/2] mfd: twl-core: convert to module_i2c_driver() Grygorii Strashko
2013-05-16 22:17   ` Samuel Ortiz
2013-05-16 22:58     ` Tony Lindgren
2013-05-17 10:46       ` Grygorii Strashko
2013-04-23 17:34 ` [PATCH 0/2] OMAP: fix boot sequence Tony Lindgren
2013-04-23 17:50   ` Grygorii Strashko

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).