[PATCH 0/2] Fix omap serial early crash during hwmod _setup()

[PATCH 0/2] Fix omap serial early crash during hwmod _setup()

[Rajendra Nayak]

This series should fix the issue with the early serial crash seen
during hwmod _setup() as reported by [1]

The issue was reported on a am33xx device and was not seen on omap4
or omap5 devices as the hwmod data for both omap4 and omap5 had
statically defined HWMOD_INIT_NO_IDLE and HWMOD_INIT_NO_RESET for
uart3 which happens to be the debug console used on omap4 panda and
omap5 uevm boards.

The series reverts the patch which stubbed out omap_serial_early_init()
for DT boot and gets rid of the static flags in omap4 and omap5 hwmod
data.

This series along with the patch posted by Felipe [2] should fix the
issue reported by [1] completely and boot to prompt on a am33xx device
with LL_DEBUG enabled.

I have boot tested this series (along with [2]) on a omap4 panda es and
a am335x boneblack.

[1] http://comments.gmane.org/gmane.linux.ports.arm.omap/100628

[2] http://marc.info/?l=linux-omap&m=137353783912736&w=2

Rajendra Nayak (2):
  Revert "ARM: OMAP2+: Fix serial init for device tree based booting"
  ARM: OMAP4+: Get rid of the HWMOD_INIT_NO_IDLE and
    HWMOD_INIT_NO_RESET flags for uart

 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |    3 +--
 arch/arm/mach-omap2/omap_hwmod_54xx_data.c |    1 -
 arch/arm/mach-omap2/serial.c               |    3 ---
 3 files changed, 1 insertion(+), 6 deletions(-)

-- 
1.7.9.5

[PATCH 0/2] Fix omap serial early crash during hwmod _setup()

[Rajendra Nayak]

This series should fix the issue with the early serial crash seen
during hwmod _setup() as reported by [1]

The issue was reported on a am33xx device and was not seen on omap4
or omap5 devices as the hwmod data for both omap4 and omap5 had
statically defined HWMOD_INIT_NO_IDLE and HWMOD_INIT_NO_RESET for
uart3 which happens to be the debug console used on omap4 panda and
omap5 uevm boards.

The series reverts the patch which stubbed out omap_serial_early_init()
for DT boot and gets rid of the static flags in omap4 and omap5 hwmod
data.

This series along with the patch posted by Felipe [2] should fix the
issue reported by [1] completely and boot to prompt on a am33xx device
with LL_DEBUG enabled.

I have boot tested this series (along with [2]) on a omap4 panda es and
a am335x boneblack.

[1] http://comments.gmane.org/gmane.linux.ports.arm.omap/100628

[2] http://marc.info/?l=linux-omap&m=137353783912736&w=2

Rajendra Nayak (2):
  Revert "ARM: OMAP2+: Fix serial init for device tree based booting"
  ARM: OMAP4+: Get rid of the HWMOD_INIT_NO_IDLE and
    HWMOD_INIT_NO_RESET flags for uart

 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |    3 +--
 arch/arm/mach-omap2/omap_hwmod_54xx_data.c |    1 -
 arch/arm/mach-omap2/serial.c               |    3 ---
 3 files changed, 1 insertion(+), 6 deletions(-)

-- 
1.7.9.5

[PATCH 1/2] Revert "ARM: OMAP2+: Fix serial init for device tree based booting"

[Rajendra Nayak]

This reverts commit 82702ea11ddfe0e43382e1fa5b66d807d8114916.

The above commit stubbed out omap_serial_early_init() in case of
DT build thinking it was doing the serial port initializations.

omap_serial_early_init() however does not do the serial port
inits (its instead done by omap_serial_init_port()) instead
it sets the HWMOD_INIT_NO_IDLE and HWMOD_INIT_NO_RESET flags
for the console uart which causes hwmod to avoid doing a reset
followed by the idling of the console uart.

This is still needed even in the DT case.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
Reported-by: Mark Jackson <mpfj-list@newflow.co.uk>
Reported-by: Vaibhav Bedia <vaibhav.bedia@ti.com>
---
 arch/arm/mach-omap2/serial.c |    3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
index 3a674de..58d5b56 100644
--- a/arch/arm/mach-omap2/serial.c
+++ b/arch/arm/mach-omap2/serial.c
@@ -175,9 +175,6 @@ static char *cmdline_find_option(char *str)
 
 static int __init omap_serial_early_init(void)
 {
-	if (of_have_populated_dt())
-		return -ENODEV;
-
 	do {
 		char oh_name[MAX_UART_HWMOD_NAME_LEN];
 		struct omap_hwmod *oh;
-- 
1.7.9.5

[PATCH 1/2] Revert "ARM: OMAP2+: Fix serial init for device tree based booting"

[Rajendra Nayak]

This reverts commit 82702ea11ddfe0e43382e1fa5b66d807d8114916.

The above commit stubbed out omap_serial_early_init() in case of
DT build thinking it was doing the serial port initializations.

omap_serial_early_init() however does not do the serial port
inits (its instead done by omap_serial_init_port()) instead
it sets the HWMOD_INIT_NO_IDLE and HWMOD_INIT_NO_RESET flags
for the console uart which causes hwmod to avoid doing a reset
followed by the idling of the console uart.

This is still needed even in the DT case.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
Reported-by: Mark Jackson <mpfj-list@newflow.co.uk>
Reported-by: Vaibhav Bedia <vaibhav.bedia@ti.com>
---
 arch/arm/mach-omap2/serial.c |    3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
index 3a674de..58d5b56 100644
--- a/arch/arm/mach-omap2/serial.c
+++ b/arch/arm/mach-omap2/serial.c
@@ -175,9 +175,6 @@ static char *cmdline_find_option(char *str)
 
 static int __init omap_serial_early_init(void)
 {
-	if (of_have_populated_dt())
-		return -ENODEV;
-
 	do {
 		char oh_name[MAX_UART_HWMOD_NAME_LEN];
 		struct omap_hwmod *oh;
-- 
1.7.9.5

[PATCH 2/2] ARM: OMAP4+: Get rid of the HWMOD_INIT_NO_IDLE and HWMOD_INIT_NO_RESET flags for uart

[Rajendra Nayak]

The HWMOD_INIT_NO_IDLE and HWMOD_INIT_NO_RESET flags don't belong in the
SoC data but instead need to be set runtime based on the uart thats used
for debug console, so hwmod avoids doing a reset followed by a idling of
the device. This is already happening as part of the omap_serial_early_init()
so remove these flags from the static SoC data.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |    3 +--
 arch/arm/mach-omap2/omap_hwmod_54xx_data.c |    1 -
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index d04b5e6..f80f120 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -2858,8 +2858,7 @@ static struct omap_hwmod omap44xx_uart3_hwmod = {
 	.name		= "uart3",
 	.class		= &omap44xx_uart_hwmod_class,
 	.clkdm_name	= "l4_per_clkdm",
-	.flags		= HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET |
-				HWMOD_SWSUP_SIDLE_ACT,
+	.flags		= HWMOD_SWSUP_SIDLE_ACT,
 	.main_clk	= "func_48m_fclk",
 	.prcm = {
 		.omap4 = {
diff --git a/arch/arm/mach-omap2/omap_hwmod_54xx_data.c b/arch/arm/mach-omap2/omap_hwmod_54xx_data.c
index f37ae96..e816edd 100644
--- a/arch/arm/mach-omap2/omap_hwmod_54xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_54xx_data.c
@@ -1375,7 +1375,6 @@ static struct omap_hwmod omap54xx_uart3_hwmod = {
 	.name		= "uart3",
 	.class		= &omap54xx_uart_hwmod_class,
 	.clkdm_name	= "l4per_clkdm",
-	.flags		= HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET,
 	.main_clk	= "func_48m_fclk",
 	.prcm = {
 		.omap4 = {
-- 
1.7.9.5

[PATCH 2/2] ARM: OMAP4+: Get rid of the HWMOD_INIT_NO_IDLE and HWMOD_INIT_NO_RESET flags for uart

[Rajendra Nayak]

The HWMOD_INIT_NO_IDLE and HWMOD_INIT_NO_RESET flags don't belong in the
SoC data but instead need to be set runtime based on the uart thats used
for debug console, so hwmod avoids doing a reset followed by a idling of
the device. This is already happening as part of the omap_serial_early_init()
so remove these flags from the static SoC data.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |    3 +--
 arch/arm/mach-omap2/omap_hwmod_54xx_data.c |    1 -
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index d04b5e6..f80f120 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -2858,8 +2858,7 @@ static struct omap_hwmod omap44xx_uart3_hwmod = {
 	.name		= "uart3",
 	.class		= &omap44xx_uart_hwmod_class,
 	.clkdm_name	= "l4_per_clkdm",
-	.flags		= HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET |
-				HWMOD_SWSUP_SIDLE_ACT,
+	.flags		= HWMOD_SWSUP_SIDLE_ACT,
 	.main_clk	= "func_48m_fclk",
 	.prcm = {
 		.omap4 = {
diff --git a/arch/arm/mach-omap2/omap_hwmod_54xx_data.c b/arch/arm/mach-omap2/omap_hwmod_54xx_data.c
index f37ae96..e816edd 100644
--- a/arch/arm/mach-omap2/omap_hwmod_54xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_54xx_data.c
@@ -1375,7 +1375,6 @@ static struct omap_hwmod omap54xx_uart3_hwmod = {
 	.name		= "uart3",
 	.class		= &omap54xx_uart_hwmod_class,
 	.clkdm_name	= "l4per_clkdm",
-	.flags		= HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET,
 	.main_clk	= "func_48m_fclk",
 	.prcm = {
 		.omap4 = {
-- 
1.7.9.5

Re: [PATCH 0/2] Fix omap serial early crash during hwmod _setup()

[Felipe Balbi]

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

On Thu, Jul 11, 2013 at 04:23:03PM +0530, Rajendra Nayak wrote:
> This series should fix the issue with the early serial crash seen
> during hwmod _setup() as reported by [1]
> 
> The issue was reported on a am33xx device and was not seen on omap4
> or omap5 devices as the hwmod data for both omap4 and omap5 had
> statically defined HWMOD_INIT_NO_IDLE and HWMOD_INIT_NO_RESET for
> uart3 which happens to be the debug console used on omap4 panda and
> omap5 uevm boards.
> 
> The series reverts the patch which stubbed out omap_serial_early_init()
> for DT boot and gets rid of the static flags in omap4 and omap5 hwmod
> data.
> 
> This series along with the patch posted by Felipe [2] should fix the
> issue reported by [1] completely and boot to prompt on a am33xx device
> with LL_DEBUG enabled.
> 
> I have boot tested this series (along with [2]) on a omap4 panda es and
> a am335x boneblack.
> 
> [1] http://comments.gmane.org/gmane.linux.ports.arm.omap/100628
> 
> [2] http://marc.info/?l=linux-omap&m=137353783912736&w=2
> 
> Rajendra Nayak (2):
>   Revert "ARM: OMAP2+: Fix serial init for device tree based booting"
>   ARM: OMAP4+: Get rid of the HWMOD_INIT_NO_IDLE and
>     HWMOD_INIT_NO_RESET flags for uart

boot tested on omap4-panda rev a3. Console still works and it suspends
and resumes fine after setting a proper timeout.

Tested-by: Felipe Balbi <balbi@ti.com>

-- 
balbi

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

[PATCH 0/2] Fix omap serial early crash during hwmod _setup()

[Felipe Balbi]

On Thu, Jul 11, 2013 at 04:23:03PM +0530, Rajendra Nayak wrote:
> This series should fix the issue with the early serial crash seen
> during hwmod _setup() as reported by [1]
> 
> The issue was reported on a am33xx device and was not seen on omap4
> or omap5 devices as the hwmod data for both omap4 and omap5 had
> statically defined HWMOD_INIT_NO_IDLE and HWMOD_INIT_NO_RESET for
> uart3 which happens to be the debug console used on omap4 panda and
> omap5 uevm boards.
> 
> The series reverts the patch which stubbed out omap_serial_early_init()
> for DT boot and gets rid of the static flags in omap4 and omap5 hwmod
> data.
> 
> This series along with the patch posted by Felipe [2] should fix the
> issue reported by [1] completely and boot to prompt on a am33xx device
> with LL_DEBUG enabled.
> 
> I have boot tested this series (along with [2]) on a omap4 panda es and
> a am335x boneblack.
> 
> [1] http://comments.gmane.org/gmane.linux.ports.arm.omap/100628
> 
> [2] http://marc.info/?l=linux-omap&m=137353783912736&w=2
> 
> Rajendra Nayak (2):
>   Revert "ARM: OMAP2+: Fix serial init for device tree based booting"
>   ARM: OMAP4+: Get rid of the HWMOD_INIT_NO_IDLE and
>     HWMOD_INIT_NO_RESET flags for uart

boot tested on omap4-panda rev a3. Console still works and it suspends
and resumes fine after setting a proper timeout.

Tested-by: Felipe Balbi <balbi@ti.com>

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130711/b7de8f7f/attachment.sig>

Re: [PATCH 1/2] Revert "ARM: OMAP2+: Fix serial init for device tree based booting"

[Tony Lindgren]

* Rajendra Nayak <rnayak@ti.com> [130711 03:59]:
> This reverts commit 82702ea11ddfe0e43382e1fa5b66d807d8114916.
> 
> The above commit stubbed out omap_serial_early_init() in case of
> DT build thinking it was doing the serial port initializations.

Well not really. It was done to cut dependency between device
tree initialized drivers and pdata initialized drivers.
 
> omap_serial_early_init() however does not do the serial port
> inits (its instead done by omap_serial_init_port()) instead
> it sets the HWMOD_INIT_NO_IDLE and HWMOD_INIT_NO_RESET flags
> for the console uart which causes hwmod to avoid doing a reset
> followed by the idling of the console uart.
> 
> This is still needed even in the DT case.

This fix is going the wrong way.

The console is working fine with DT based booting for me,
except for the earlyprintk fix needed.

And there should not be any need to parse cmdline for console
as omap-serial.c sets it up and already knows about it.

Care to state what exactly this attempts to fix and for which
omaps? If this is only needed for am33xx, then why?

Regards,

Tony
 
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> Reported-by: Mark Jackson <mpfj-list@newflow.co.uk>
> Reported-by: Vaibhav Bedia <vaibhav.bedia@ti.com>
> ---
>  arch/arm/mach-omap2/serial.c |    3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
> index 3a674de..58d5b56 100644
> --- a/arch/arm/mach-omap2/serial.c
> +++ b/arch/arm/mach-omap2/serial.c
> @@ -175,9 +175,6 @@ static char *cmdline_find_option(char *str)
>  
>  static int __init omap_serial_early_init(void)
>  {
> -	if (of_have_populated_dt())
> -		return -ENODEV;
> -
>  	do {
>  		char oh_name[MAX_UART_HWMOD_NAME_LEN];
>  		struct omap_hwmod *oh;
> -- 
> 1.7.9.5
> 

[PATCH 1/2] Revert "ARM: OMAP2+: Fix serial init for device tree based booting"

[Tony Lindgren]

* Rajendra Nayak <rnayak@ti.com> [130711 03:59]:
> This reverts commit 82702ea11ddfe0e43382e1fa5b66d807d8114916.
> 
> The above commit stubbed out omap_serial_early_init() in case of
> DT build thinking it was doing the serial port initializations.

Well not really. It was done to cut dependency between device
tree initialized drivers and pdata initialized drivers.
 
> omap_serial_early_init() however does not do the serial port
> inits (its instead done by omap_serial_init_port()) instead
> it sets the HWMOD_INIT_NO_IDLE and HWMOD_INIT_NO_RESET flags
> for the console uart which causes hwmod to avoid doing a reset
> followed by the idling of the console uart.
> 
> This is still needed even in the DT case.

This fix is going the wrong way.

The console is working fine with DT based booting for me,
except for the earlyprintk fix needed.

And there should not be any need to parse cmdline for console
as omap-serial.c sets it up and already knows about it.

Care to state what exactly this attempts to fix and for which
omaps? If this is only needed for am33xx, then why?

Regards,

Tony
 
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> Reported-by: Mark Jackson <mpfj-list@newflow.co.uk>
> Reported-by: Vaibhav Bedia <vaibhav.bedia@ti.com>
> ---
>  arch/arm/mach-omap2/serial.c |    3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
> index 3a674de..58d5b56 100644
> --- a/arch/arm/mach-omap2/serial.c
> +++ b/arch/arm/mach-omap2/serial.c
> @@ -175,9 +175,6 @@ static char *cmdline_find_option(char *str)
>  
>  static int __init omap_serial_early_init(void)
>  {
> -	if (of_have_populated_dt())
> -		return -ENODEV;
> -
>  	do {
>  		char oh_name[MAX_UART_HWMOD_NAME_LEN];
>  		struct omap_hwmod *oh;
> -- 
> 1.7.9.5
> 

Re: [PATCH 0/2] Fix omap serial early crash during hwmod _setup()

[Tony Lindgren]

* Rajendra Nayak <rnayak@ti.com> [130711 03:59]:
> This series should fix the issue with the early serial crash seen
> during hwmod _setup() as reported by [1]
> 
> The issue was reported on a am33xx device and was not seen on omap4
> or omap5 devices as the hwmod data for both omap4 and omap5 had
> statically defined HWMOD_INIT_NO_IDLE and HWMOD_INIT_NO_RESET for
> uart3 which happens to be the debug console used on omap4 panda and
> omap5 uevm boards.

The serial driver should be capable of setting this on it's own
as it sets up the console.

Regards,

Tony

[PATCH 0/2] Fix omap serial early crash during hwmod _setup()

[Tony Lindgren]

* Rajendra Nayak <rnayak@ti.com> [130711 03:59]:
> This series should fix the issue with the early serial crash seen
> during hwmod _setup() as reported by [1]
> 
> The issue was reported on a am33xx device and was not seen on omap4
> or omap5 devices as the hwmod data for both omap4 and omap5 had
> statically defined HWMOD_INIT_NO_IDLE and HWMOD_INIT_NO_RESET for
> uart3 which happens to be the debug console used on omap4 panda and
> omap5 uevm boards.

The serial driver should be capable of setting this on it's own
as it sets up the console.

Regards,

Tony

Re: [PATCH 1/2] Revert "ARM: OMAP2+: Fix serial init for device tree based booting"

[Rajendra Nayak]

On Friday 12 July 2013 12:50 PM, Tony Lindgren wrote:
> * Rajendra Nayak <rnayak@ti.com> [130711 03:59]:
>> This reverts commit 82702ea11ddfe0e43382e1fa5b66d807d8114916.
>>
>> The above commit stubbed out omap_serial_early_init() in case of
>> DT build thinking it was doing the serial port initializations.
> 
> Well not really. It was done to cut dependency between device
> tree initialized drivers and pdata initialized drivers.
>  
>> omap_serial_early_init() however does not do the serial port
>> inits (its instead done by omap_serial_init_port()) instead
>> it sets the HWMOD_INIT_NO_IDLE and HWMOD_INIT_NO_RESET flags
>> for the console uart which causes hwmod to avoid doing a reset
>> followed by the idling of the console uart.
>>
>> This is still needed even in the DT case.
> 
> This fix is going the wrong way.
> 
> The console is working fine with DT based booting for me,
> except for the earlyprintk fix needed.

It works on omap4 and omap5. It won't work on any
am33xx devices.

> 
> And there should not be any need to parse cmdline for console
> as omap-serial.c sets it up and already knows about it.
> 
> Care to state what exactly this attempts to fix and for which
> omaps? If this is only needed for am33xx, then why?

Yes, this is needed only for am33xx because am33xx hwmod rightly
had the hwmod flags for NO_IDLE and NO_RESET removed and omap4
and omap5 wrongly still carry them around.

Just try applying PATCH 2/2 of this series and it won't work on the
omap4 sdp anymore.

regards,
Rajendra

> 
> Regards,
> 
> Tony
>  
>> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
>> Reported-by: Mark Jackson <mpfj-list@newflow.co.uk>
>> Reported-by: Vaibhav Bedia <vaibhav.bedia@ti.com>
>> ---
>>  arch/arm/mach-omap2/serial.c |    3 ---
>>  1 file changed, 3 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
>> index 3a674de..58d5b56 100644
>> --- a/arch/arm/mach-omap2/serial.c
>> +++ b/arch/arm/mach-omap2/serial.c
>> @@ -175,9 +175,6 @@ static char *cmdline_find_option(char *str)
>>  
>>  static int __init omap_serial_early_init(void)
>>  {
>> -	if (of_have_populated_dt())
>> -		return -ENODEV;
>> -
>>  	do {
>>  		char oh_name[MAX_UART_HWMOD_NAME_LEN];
>>  		struct omap_hwmod *oh;
>> -- 
>> 1.7.9.5
>>

[PATCH 1/2] Revert "ARM: OMAP2+: Fix serial init for device tree based booting"

[Rajendra Nayak]

On Friday 12 July 2013 12:50 PM, Tony Lindgren wrote:
> * Rajendra Nayak <rnayak@ti.com> [130711 03:59]:
>> This reverts commit 82702ea11ddfe0e43382e1fa5b66d807d8114916.
>>
>> The above commit stubbed out omap_serial_early_init() in case of
>> DT build thinking it was doing the serial port initializations.
> 
> Well not really. It was done to cut dependency between device
> tree initialized drivers and pdata initialized drivers.
>  
>> omap_serial_early_init() however does not do the serial port
>> inits (its instead done by omap_serial_init_port()) instead
>> it sets the HWMOD_INIT_NO_IDLE and HWMOD_INIT_NO_RESET flags
>> for the console uart which causes hwmod to avoid doing a reset
>> followed by the idling of the console uart.
>>
>> This is still needed even in the DT case.
> 
> This fix is going the wrong way.
> 
> The console is working fine with DT based booting for me,
> except for the earlyprintk fix needed.

It works on omap4 and omap5. It won't work on any
am33xx devices.

> 
> And there should not be any need to parse cmdline for console
> as omap-serial.c sets it up and already knows about it.
> 
> Care to state what exactly this attempts to fix and for which
> omaps? If this is only needed for am33xx, then why?

Yes, this is needed only for am33xx because am33xx hwmod rightly
had the hwmod flags for NO_IDLE and NO_RESET removed and omap4
and omap5 wrongly still carry them around.

Just try applying PATCH 2/2 of this series and it won't work on the
omap4 sdp anymore.

regards,
Rajendra

> 
> Regards,
> 
> Tony
>  
>> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
>> Reported-by: Mark Jackson <mpfj-list@newflow.co.uk>
>> Reported-by: Vaibhav Bedia <vaibhav.bedia@ti.com>
>> ---
>>  arch/arm/mach-omap2/serial.c |    3 ---
>>  1 file changed, 3 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
>> index 3a674de..58d5b56 100644
>> --- a/arch/arm/mach-omap2/serial.c
>> +++ b/arch/arm/mach-omap2/serial.c
>> @@ -175,9 +175,6 @@ static char *cmdline_find_option(char *str)
>>  
>>  static int __init omap_serial_early_init(void)
>>  {
>> -	if (of_have_populated_dt())
>> -		return -ENODEV;
>> -
>>  	do {
>>  		char oh_name[MAX_UART_HWMOD_NAME_LEN];
>>  		struct omap_hwmod *oh;
>> -- 
>> 1.7.9.5
>>

Re: [PATCH 0/2] Fix omap serial early crash during hwmod _setup()

[Rajendra Nayak]

On Friday 12 July 2013 12:52 PM, Tony Lindgren wrote:
> * Rajendra Nayak <rnayak@ti.com> [130711 03:59]:
>> This series should fix the issue with the early serial crash seen
>> during hwmod _setup() as reported by [1]
>>
>> The issue was reported on a am33xx device and was not seen on omap4
>> or omap5 devices as the hwmod data for both omap4 and omap5 had
>> statically defined HWMOD_INIT_NO_IDLE and HWMOD_INIT_NO_RESET for
>> uart3 which happens to be the debug console used on omap4 panda and
>> omap5 uevm boards.
> 
> The serial driver should be capable of setting this on it's own
> as it sets up the console.

These flags are used to avoid an early reset that hwmod does to all devices
on the SoC and is done much before serial driver is initialized.

> 
> Regards,
> 
> Tony
> 

[PATCH 0/2] Fix omap serial early crash during hwmod _setup()

[Rajendra Nayak]

On Friday 12 July 2013 12:52 PM, Tony Lindgren wrote:
> * Rajendra Nayak <rnayak@ti.com> [130711 03:59]:
>> This series should fix the issue with the early serial crash seen
>> during hwmod _setup() as reported by [1]
>>
>> The issue was reported on a am33xx device and was not seen on omap4
>> or omap5 devices as the hwmod data for both omap4 and omap5 had
>> statically defined HWMOD_INIT_NO_IDLE and HWMOD_INIT_NO_RESET for
>> uart3 which happens to be the debug console used on omap4 panda and
>> omap5 uevm boards.
> 
> The serial driver should be capable of setting this on it's own
> as it sets up the console.

These flags are used to avoid an early reset that hwmod does to all devices
on the SoC and is done much before serial driver is initialized.

> 
> Regards,
> 
> Tony
> 

Re: [PATCH 1/2] Revert "ARM: OMAP2+: Fix serial init for device tree based booting"

[Tony Lindgren]

* Rajendra Nayak <rnayak@ti.com> [130712 00:38]:
> On Friday 12 July 2013 12:50 PM, Tony Lindgren wrote:
> > * Rajendra Nayak <rnayak@ti.com> [130711 03:59]:
> >> This reverts commit 82702ea11ddfe0e43382e1fa5b66d807d8114916.
> >>
> >> The above commit stubbed out omap_serial_early_init() in case of
> >> DT build thinking it was doing the serial port initializations.
> > 
> > Well not really. It was done to cut dependency between device
> > tree initialized drivers and pdata initialized drivers.
> >  
> >> omap_serial_early_init() however does not do the serial port
> >> inits (its instead done by omap_serial_init_port()) instead
> >> it sets the HWMOD_INIT_NO_IDLE and HWMOD_INIT_NO_RESET flags
> >> for the console uart which causes hwmod to avoid doing a reset
> >> followed by the idling of the console uart.
> >>
> >> This is still needed even in the DT case.
> > 
> > This fix is going the wrong way.
> > 
> > The console is working fine with DT based booting for me,
> > except for the earlyprintk fix needed.
> 
> It works on omap4 and omap5. It won't work on any
> am33xx devices.

OK. I assume the regular serial onsole works just fine, what does
not work is the earlyprintk for am33xx?

> > And there should not be any need to parse cmdline for console
> > as omap-serial.c sets it up and already knows about it.
> > 
> > Care to state what exactly this attempts to fix and for which
> > omaps? If this is only needed for am33xx, then why?
> 
> Yes, this is needed only for am33xx because am33xx hwmod rightly
> had the hwmod flags for NO_IDLE and NO_RESET removed and omap4
> and omap5 wrongly still carry them around.

OK.
 
> Just try applying PATCH 2/2 of this series and it won't work on the
> omap4 sdp anymore.

OK, so that's only for earlyprintk then?

If so, it seems the right fix is to set the NO_IDLE and NO_RESET
flags based on ifdef CONFIG_DEBUG_OMAP4UART3 etc as that is selected
in the Kconfig now.

The current code in mach-omap2/serial.c is wrong, and is a hack
needed for the pdata based booting. What's broken is that
omap_serial_early_init() parses the cmdline for console, which
itself is pretty nasty, and it won't work the right way as
there's nothing stopping from having the earlycon in a different
UART from the serial console. So we just want to get rid of the
whole mach-omap2/serial.c once we're all DT.

So to summarize, we have two bugs:

1. Omap hwmod code can reset UART while earlycon may be using
   it. The fix to this is to use NO_IDLE and NO_RESET flags in
   the hwmod code if CONFIG_DEBUG_OMAPxUARTy is specified.

2. A bug in drivers/tty/serial/omap-serial.c where the
   missing context loss count can cause NULL context to be
   initialized during driver probe causing port to hang for
   earlycon. The fix for that is what Felipe has suggested or
   fix it in the driver by removing the context loss count usage
   and detect the need for context restore based on the UART
   state.

Or am I still missing something?

Regards,

Tony

[PATCH 1/2] Revert "ARM: OMAP2+: Fix serial init for device tree based booting"

[Tony Lindgren]

* Rajendra Nayak <rnayak@ti.com> [130712 00:38]:
> On Friday 12 July 2013 12:50 PM, Tony Lindgren wrote:
> > * Rajendra Nayak <rnayak@ti.com> [130711 03:59]:
> >> This reverts commit 82702ea11ddfe0e43382e1fa5b66d807d8114916.
> >>
> >> The above commit stubbed out omap_serial_early_init() in case of
> >> DT build thinking it was doing the serial port initializations.
> > 
> > Well not really. It was done to cut dependency between device
> > tree initialized drivers and pdata initialized drivers.
> >  
> >> omap_serial_early_init() however does not do the serial port
> >> inits (its instead done by omap_serial_init_port()) instead
> >> it sets the HWMOD_INIT_NO_IDLE and HWMOD_INIT_NO_RESET flags
> >> for the console uart which causes hwmod to avoid doing a reset
> >> followed by the idling of the console uart.
> >>
> >> This is still needed even in the DT case.
> > 
> > This fix is going the wrong way.
> > 
> > The console is working fine with DT based booting for me,
> > except for the earlyprintk fix needed.
> 
> It works on omap4 and omap5. It won't work on any
> am33xx devices.

OK. I assume the regular serial onsole works just fine, what does
not work is the earlyprintk for am33xx?

> > And there should not be any need to parse cmdline for console
> > as omap-serial.c sets it up and already knows about it.
> > 
> > Care to state what exactly this attempts to fix and for which
> > omaps? If this is only needed for am33xx, then why?
> 
> Yes, this is needed only for am33xx because am33xx hwmod rightly
> had the hwmod flags for NO_IDLE and NO_RESET removed and omap4
> and omap5 wrongly still carry them around.

OK.
 
> Just try applying PATCH 2/2 of this series and it won't work on the
> omap4 sdp anymore.

OK, so that's only for earlyprintk then?

If so, it seems the right fix is to set the NO_IDLE and NO_RESET
flags based on ifdef CONFIG_DEBUG_OMAP4UART3 etc as that is selected
in the Kconfig now.

The current code in mach-omap2/serial.c is wrong, and is a hack
needed for the pdata based booting. What's broken is that
omap_serial_early_init() parses the cmdline for console, which
itself is pretty nasty, and it won't work the right way as
there's nothing stopping from having the earlycon in a different
UART from the serial console. So we just want to get rid of the
whole mach-omap2/serial.c once we're all DT.

So to summarize, we have two bugs:

1. Omap hwmod code can reset UART while earlycon may be using
   it. The fix to this is to use NO_IDLE and NO_RESET flags in
   the hwmod code if CONFIG_DEBUG_OMAPxUARTy is specified.

2. A bug in drivers/tty/serial/omap-serial.c where the
   missing context loss count can cause NULL context to be
   initialized during driver probe causing port to hang for
   earlycon. The fix for that is what Felipe has suggested or
   fix it in the driver by removing the context loss count usage
   and detect the need for context restore based on the UART
   state.

Or am I still missing something?

Regards,

Tony

Re: [PATCH 0/2] Fix omap serial early crash during hwmod _setup()

[Tony Lindgren]

* Rajendra Nayak <rnayak@ti.com> [130712 00:39]:
> On Friday 12 July 2013 12:52 PM, Tony Lindgren wrote:
> > * Rajendra Nayak <rnayak@ti.com> [130711 03:59]:
> >> This series should fix the issue with the early serial crash seen
> >> during hwmod _setup() as reported by [1]
> >>
> >> The issue was reported on a am33xx device and was not seen on omap4
> >> or omap5 devices as the hwmod data for both omap4 and omap5 had
> >> statically defined HWMOD_INIT_NO_IDLE and HWMOD_INIT_NO_RESET for
> >> uart3 which happens to be the debug console used on omap4 panda and
> >> omap5 uevm boards.
> > 
> > The serial driver should be capable of setting this on it's own
> > as it sets up the console.
> 
> These flags are used to avoid an early reset that hwmod does to all devices
> on the SoC and is done much before serial driver is initialized.

OK so that's only needed for earlycon then it seems.

Regards,

Tony

[PATCH 0/2] Fix omap serial early crash during hwmod _setup()

[Tony Lindgren]

* Rajendra Nayak <rnayak@ti.com> [130712 00:39]:
> On Friday 12 July 2013 12:52 PM, Tony Lindgren wrote:
> > * Rajendra Nayak <rnayak@ti.com> [130711 03:59]:
> >> This series should fix the issue with the early serial crash seen
> >> during hwmod _setup() as reported by [1]
> >>
> >> The issue was reported on a am33xx device and was not seen on omap4
> >> or omap5 devices as the hwmod data for both omap4 and omap5 had
> >> statically defined HWMOD_INIT_NO_IDLE and HWMOD_INIT_NO_RESET for
> >> uart3 which happens to be the debug console used on omap4 panda and
> >> omap5 uevm boards.
> > 
> > The serial driver should be capable of setting this on it's own
> > as it sets up the console.
> 
> These flags are used to avoid an early reset that hwmod does to all devices
> on the SoC and is done much before serial driver is initialized.

OK so that's only needed for earlycon then it seems.

Regards,

Tony

Re: [PATCH 1/2] Revert "ARM: OMAP2+: Fix serial init for device tree based booting"

[Rajendra Nayak]

On Friday 12 July 2013 01:33 PM, Tony Lindgren wrote:
> * Rajendra Nayak <rnayak@ti.com> [130712 00:38]:
>> On Friday 12 July 2013 12:50 PM, Tony Lindgren wrote:
>>> * Rajendra Nayak <rnayak@ti.com> [130711 03:59]:
>>>> This reverts commit 82702ea11ddfe0e43382e1fa5b66d807d8114916.
>>>>
>>>> The above commit stubbed out omap_serial_early_init() in case of
>>>> DT build thinking it was doing the serial port initializations.
>>>
>>> Well not really. It was done to cut dependency between device
>>> tree initialized drivers and pdata initialized drivers.
>>>  
>>>> omap_serial_early_init() however does not do the serial port
>>>> inits (its instead done by omap_serial_init_port()) instead
>>>> it sets the HWMOD_INIT_NO_IDLE and HWMOD_INIT_NO_RESET flags
>>>> for the console uart which causes hwmod to avoid doing a reset
>>>> followed by the idling of the console uart.
>>>>
>>>> This is still needed even in the DT case.
>>>
>>> This fix is going the wrong way.
>>>
>>> The console is working fine with DT based booting for me,
>>> except for the earlyprintk fix needed.
>>
>> It works on omap4 and omap5. It won't work on any
>> am33xx devices.
> 
> OK. I assume the regular serial onsole works just fine, what does
> not work is the earlyprintk for am33xx?

Yes, thats right.

> 
>>> And there should not be any need to parse cmdline for console
>>> as omap-serial.c sets it up and already knows about it.
>>>
>>> Care to state what exactly this attempts to fix and for which
>>> omaps? If this is only needed for am33xx, then why?
>>
>> Yes, this is needed only for am33xx because am33xx hwmod rightly
>> had the hwmod flags for NO_IDLE and NO_RESET removed and omap4
>> and omap5 wrongly still carry them around.
> 
> OK.
>  
>> Just try applying PATCH 2/2 of this series and it won't work on the
>> omap4 sdp anymore.
> 
> OK, so that's only for earlyprintk then?

yes,

> 
> If so, it seems the right fix is to set the NO_IDLE and NO_RESET
> flags based on ifdef CONFIG_DEBUG_OMAP4UART3 etc as that is selected
> in the Kconfig now.

ok makes sense. It seems like the static data in hwmod can be populated
based on these defines? something like

/* uart3 */
static struct omap_hwmod omap44xx_uart3_hwmod = {
        .name           = "uart3",
        .class          = &omap44xx_uart_hwmod_class,
        .clkdm_name     = "l4_per_clkdm",
#ifdef CONFIG_DEBUG_OMAP4UART3
        .flags          = HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET |
                                HWMOD_SWSUP_SIDLE_ACT,
#else
        .flags          = HWMOD_SWSUP_SIDLE_ACT,
#endif
        .main_clk       = "func_48m_fclk",
        .prcm = {
                .omap4 = {
                        .clkctrl_offs = OMAP4_CM_L4PER_UART3_CLKCTRL_OFFSET,
                        .context_offs = OMAP4_RM_L4PER_UART3_CONTEXT_OFFSET,
                        .modulemode   = MODULEMODE_SWCTRL,
                },
        },
};

And same way for others? That way the cmdline parsing can be done away
with even for the non-DT case.

> 
> The current code in mach-omap2/serial.c is wrong, and is a hack
> needed for the pdata based booting. What's broken is that
> omap_serial_early_init() parses the cmdline for console, which
> itself is pretty nasty, and it won't work the right way as
> there's nothing stopping from having the earlycon in a different
> UART from the serial console. So we just want to get rid of the
> whole mach-omap2/serial.c once we're all DT.
> 
> So to summarize, we have two bugs:
> 
> 1. Omap hwmod code can reset UART while earlycon may be using
>    it. The fix to this is to use NO_IDLE and NO_RESET flags in
>    the hwmod code if CONFIG_DEBUG_OMAPxUARTy is specified.
> 
> 2. A bug in drivers/tty/serial/omap-serial.c where the
>    missing context loss count can cause NULL context to be
>    initialized during driver probe causing port to hang for
>    earlycon. The fix for that is what Felipe has suggested or
>    fix it in the driver by removing the context loss count usage
>    and detect the need for context restore based on the UART
>    state.
> 
> Or am I still missing something?

No, thats pretty much the 2 issues we have.

regards,
Rajendra
> 
> Regards,
> 
> Tony
> 

[PATCH 1/2] Revert "ARM: OMAP2+: Fix serial init for device tree based booting"

[Rajendra Nayak]

On Friday 12 July 2013 01:33 PM, Tony Lindgren wrote:
> * Rajendra Nayak <rnayak@ti.com> [130712 00:38]:
>> On Friday 12 July 2013 12:50 PM, Tony Lindgren wrote:
>>> * Rajendra Nayak <rnayak@ti.com> [130711 03:59]:
>>>> This reverts commit 82702ea11ddfe0e43382e1fa5b66d807d8114916.
>>>>
>>>> The above commit stubbed out omap_serial_early_init() in case of
>>>> DT build thinking it was doing the serial port initializations.
>>>
>>> Well not really. It was done to cut dependency between device
>>> tree initialized drivers and pdata initialized drivers.
>>>  
>>>> omap_serial_early_init() however does not do the serial port
>>>> inits (its instead done by omap_serial_init_port()) instead
>>>> it sets the HWMOD_INIT_NO_IDLE and HWMOD_INIT_NO_RESET flags
>>>> for the console uart which causes hwmod to avoid doing a reset
>>>> followed by the idling of the console uart.
>>>>
>>>> This is still needed even in the DT case.
>>>
>>> This fix is going the wrong way.
>>>
>>> The console is working fine with DT based booting for me,
>>> except for the earlyprintk fix needed.
>>
>> It works on omap4 and omap5. It won't work on any
>> am33xx devices.
> 
> OK. I assume the regular serial onsole works just fine, what does
> not work is the earlyprintk for am33xx?

Yes, thats right.

> 
>>> And there should not be any need to parse cmdline for console
>>> as omap-serial.c sets it up and already knows about it.
>>>
>>> Care to state what exactly this attempts to fix and for which
>>> omaps? If this is only needed for am33xx, then why?
>>
>> Yes, this is needed only for am33xx because am33xx hwmod rightly
>> had the hwmod flags for NO_IDLE and NO_RESET removed and omap4
>> and omap5 wrongly still carry them around.
> 
> OK.
>  
>> Just try applying PATCH 2/2 of this series and it won't work on the
>> omap4 sdp anymore.
> 
> OK, so that's only for earlyprintk then?

yes,

> 
> If so, it seems the right fix is to set the NO_IDLE and NO_RESET
> flags based on ifdef CONFIG_DEBUG_OMAP4UART3 etc as that is selected
> in the Kconfig now.

ok makes sense. It seems like the static data in hwmod can be populated
based on these defines? something like

/* uart3 */
static struct omap_hwmod omap44xx_uart3_hwmod = {
        .name           = "uart3",
        .class          = &omap44xx_uart_hwmod_class,
        .clkdm_name     = "l4_per_clkdm",
#ifdef CONFIG_DEBUG_OMAP4UART3
        .flags          = HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET |
                                HWMOD_SWSUP_SIDLE_ACT,
#else
        .flags          = HWMOD_SWSUP_SIDLE_ACT,
#endif
        .main_clk       = "func_48m_fclk",
        .prcm = {
                .omap4 = {
                        .clkctrl_offs = OMAP4_CM_L4PER_UART3_CLKCTRL_OFFSET,
                        .context_offs = OMAP4_RM_L4PER_UART3_CONTEXT_OFFSET,
                        .modulemode   = MODULEMODE_SWCTRL,
                },
        },
};

And same way for others? That way the cmdline parsing can be done away
with even for the non-DT case.

> 
> The current code in mach-omap2/serial.c is wrong, and is a hack
> needed for the pdata based booting. What's broken is that
> omap_serial_early_init() parses the cmdline for console, which
> itself is pretty nasty, and it won't work the right way as
> there's nothing stopping from having the earlycon in a different
> UART from the serial console. So we just want to get rid of the
> whole mach-omap2/serial.c once we're all DT.
> 
> So to summarize, we have two bugs:
> 
> 1. Omap hwmod code can reset UART while earlycon may be using
>    it. The fix to this is to use NO_IDLE and NO_RESET flags in
>    the hwmod code if CONFIG_DEBUG_OMAPxUARTy is specified.
> 
> 2. A bug in drivers/tty/serial/omap-serial.c where the
>    missing context loss count can cause NULL context to be
>    initialized during driver probe causing port to hang for
>    earlycon. The fix for that is what Felipe has suggested or
>    fix it in the driver by removing the context loss count usage
>    and detect the need for context restore based on the UART
>    state.
> 
> Or am I still missing something?

No, thats pretty much the 2 issues we have.

regards,
Rajendra
> 
> Regards,
> 
> Tony
> 

Re: [PATCH 1/2] Revert "ARM: OMAP2+: Fix serial init for device tree based booting"

[Tony Lindgren]

* Rajendra Nayak <rnayak@ti.com> [130712 02:06]:
> On Friday 12 July 2013 01:33 PM, Tony Lindgren wrote:
> > 
> > OK, so that's only for earlyprintk then?
> 
> yes,
> 
> > 
> > If so, it seems the right fix is to set the NO_IDLE and NO_RESET
> > flags based on ifdef CONFIG_DEBUG_OMAP4UART3 etc as that is selected
> > in the Kconfig now.
> 
> ok makes sense. It seems like the static data in hwmod can be populated
> based on these defines? something like
> 
> /* uart3 */
> static struct omap_hwmod omap44xx_uart3_hwmod = {
>         .name           = "uart3",
>         .class          = &omap44xx_uart_hwmod_class,
>         .clkdm_name     = "l4_per_clkdm",
> #ifdef CONFIG_DEBUG_OMAP4UART3
>         .flags          = HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET |
>                                 HWMOD_SWSUP_SIDLE_ACT,
> #else
>         .flags          = HWMOD_SWSUP_SIDLE_ACT,
> #endif
>         .main_clk       = "func_48m_fclk",
>         .prcm = {
>                 .omap4 = {
>                         .clkctrl_offs = OMAP4_CM_L4PER_UART3_CLKCTRL_OFFSET,
>                         .context_offs = OMAP4_RM_L4PER_UART3_CONTEXT_OFFSET,
>                         .modulemode   = MODULEMODE_SWCTRL,
>                 },
>         },
> };
> 
> And same way for others? That way the cmdline parsing can be done away
> with even for the non-DT case.

Yes we can do it that way. How about add a common macro for it if
it's always the same? Then the .flags line would be just:

#define HWMOD_OMAP_UART_FLAGS(soc, port)
...

	.flags		= HWMOD_OMAP_UART_FLAGS(4, 3),

> > The current code in mach-omap2/serial.c is wrong, and is a hack
> > needed for the pdata based booting. What's broken is that
> > omap_serial_early_init() parses the cmdline for console, which
> > itself is pretty nasty, and it won't work the right way as
> > there's nothing stopping from having the earlycon in a different
> > UART from the serial console. So we just want to get rid of the
> > whole mach-omap2/serial.c once we're all DT.
> > 
> > So to summarize, we have two bugs:
> > 
> > 1. Omap hwmod code can reset UART while earlycon may be using
> >    it. The fix to this is to use NO_IDLE and NO_RESET flags in
> >    the hwmod code if CONFIG_DEBUG_OMAPxUARTy is specified.
> > 
> > 2. A bug in drivers/tty/serial/omap-serial.c where the
> >    missing context loss count can cause NULL context to be
> >    initialized during driver probe causing port to hang for
> >    earlycon. The fix for that is what Felipe has suggested or
> >    fix it in the driver by removing the context loss count usage
> >    and detect the need for context restore based on the UART
> >    state.
> > 
> > Or am I still missing something?
> 
> No, thats pretty much the 2 issues we have.

OK thanks good to hear it's limited to earlycon issues.

Regards,

Tony

[PATCH 1/2] Revert "ARM: OMAP2+: Fix serial init for device tree based booting"

[Tony Lindgren]

* Rajendra Nayak <rnayak@ti.com> [130712 02:06]:
> On Friday 12 July 2013 01:33 PM, Tony Lindgren wrote:
> > 
> > OK, so that's only for earlyprintk then?
> 
> yes,
> 
> > 
> > If so, it seems the right fix is to set the NO_IDLE and NO_RESET
> > flags based on ifdef CONFIG_DEBUG_OMAP4UART3 etc as that is selected
> > in the Kconfig now.
> 
> ok makes sense. It seems like the static data in hwmod can be populated
> based on these defines? something like
> 
> /* uart3 */
> static struct omap_hwmod omap44xx_uart3_hwmod = {
>         .name           = "uart3",
>         .class          = &omap44xx_uart_hwmod_class,
>         .clkdm_name     = "l4_per_clkdm",
> #ifdef CONFIG_DEBUG_OMAP4UART3
>         .flags          = HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET |
>                                 HWMOD_SWSUP_SIDLE_ACT,
> #else
>         .flags          = HWMOD_SWSUP_SIDLE_ACT,
> #endif
>         .main_clk       = "func_48m_fclk",
>         .prcm = {
>                 .omap4 = {
>                         .clkctrl_offs = OMAP4_CM_L4PER_UART3_CLKCTRL_OFFSET,
>                         .context_offs = OMAP4_RM_L4PER_UART3_CONTEXT_OFFSET,
>                         .modulemode   = MODULEMODE_SWCTRL,
>                 },
>         },
> };
> 
> And same way for others? That way the cmdline parsing can be done away
> with even for the non-DT case.

Yes we can do it that way. How about add a common macro for it if
it's always the same? Then the .flags line would be just:

#define HWMOD_OMAP_UART_FLAGS(soc, port)
...

	.flags		= HWMOD_OMAP_UART_FLAGS(4, 3),

> > The current code in mach-omap2/serial.c is wrong, and is a hack
> > needed for the pdata based booting. What's broken is that
> > omap_serial_early_init() parses the cmdline for console, which
> > itself is pretty nasty, and it won't work the right way as
> > there's nothing stopping from having the earlycon in a different
> > UART from the serial console. So we just want to get rid of the
> > whole mach-omap2/serial.c once we're all DT.
> > 
> > So to summarize, we have two bugs:
> > 
> > 1. Omap hwmod code can reset UART while earlycon may be using
> >    it. The fix to this is to use NO_IDLE and NO_RESET flags in
> >    the hwmod code if CONFIG_DEBUG_OMAPxUARTy is specified.
> > 
> > 2. A bug in drivers/tty/serial/omap-serial.c where the
> >    missing context loss count can cause NULL context to be
> >    initialized during driver probe causing port to hang for
> >    earlycon. The fix for that is what Felipe has suggested or
> >    fix it in the driver by removing the context loss count usage
> >    and detect the need for context restore based on the UART
> >    state.
> > 
> > Or am I still missing something?
> 
> No, thats pretty much the 2 issues we have.

OK thanks good to hear it's limited to earlycon issues.

Regards,

Tony

Re: [PATCH 1/2] Revert "ARM: OMAP2+: Fix serial init for device tree based booting"

[Rajendra Nayak]

On Friday 12 July 2013 02:48 PM, Tony Lindgren wrote:
> * Rajendra Nayak <rnayak@ti.com> [130712 02:06]:
>> On Friday 12 July 2013 01:33 PM, Tony Lindgren wrote:
>>>
>>> OK, so that's only for earlyprintk then?
>>
>> yes,
>>
>>>
>>> If so, it seems the right fix is to set the NO_IDLE and NO_RESET
>>> flags based on ifdef CONFIG_DEBUG_OMAP4UART3 etc as that is selected
>>> in the Kconfig now.
>>
>> ok makes sense. It seems like the static data in hwmod can be populated
>> based on these defines? something like
>>
>> /* uart3 */
>> static struct omap_hwmod omap44xx_uart3_hwmod = {
>>         .name           = "uart3",
>>         .class          = &omap44xx_uart_hwmod_class,
>>         .clkdm_name     = "l4_per_clkdm",
>> #ifdef CONFIG_DEBUG_OMAP4UART3
>>         .flags          = HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET |
>>                                 HWMOD_SWSUP_SIDLE_ACT,
>> #else
>>         .flags          = HWMOD_SWSUP_SIDLE_ACT,
>> #endif
>>         .main_clk       = "func_48m_fclk",
>>         .prcm = {
>>                 .omap4 = {
>>                         .clkctrl_offs = OMAP4_CM_L4PER_UART3_CLKCTRL_OFFSET,
>>                         .context_offs = OMAP4_RM_L4PER_UART3_CONTEXT_OFFSET,
>>                         .modulemode   = MODULEMODE_SWCTRL,
>>                 },
>>         },
>> };
>>
>> And same way for others? That way the cmdline parsing can be done away
>> with even for the non-DT case.
> 
> Yes we can do it that way. How about add a common macro for it if
> it's always the same? Then the .flags line would be just:
> 
> #define HWMOD_OMAP_UART_FLAGS(soc, port)
> ...
> 
> 	.flags		= HWMOD_OMAP_UART_FLAGS(4, 3),

Right, that seems much cleaner. I will send a patch out for it and get rid
of all the cmdline parsing in serial.c

regards
Rajendra
> 
>>> The current code in mach-omap2/serial.c is wrong, and is a hack
>>> needed for the pdata based booting. What's broken is that
>>> omap_serial_early_init() parses the cmdline for console, which
>>> itself is pretty nasty, and it won't work the right way as
>>> there's nothing stopping from having the earlycon in a different
>>> UART from the serial console. So we just want to get rid of the
>>> whole mach-omap2/serial.c once we're all DT.
>>>
>>> So to summarize, we have two bugs:
>>>
>>> 1. Omap hwmod code can reset UART while earlycon may be using
>>>    it. The fix to this is to use NO_IDLE and NO_RESET flags in
>>>    the hwmod code if CONFIG_DEBUG_OMAPxUARTy is specified.
>>>
>>> 2. A bug in drivers/tty/serial/omap-serial.c where the
>>>    missing context loss count can cause NULL context to be
>>>    initialized during driver probe causing port to hang for
>>>    earlycon. The fix for that is what Felipe has suggested or
>>>    fix it in the driver by removing the context loss count usage
>>>    and detect the need for context restore based on the UART
>>>    state.
>>>
>>> Or am I still missing something?
>>
>> No, thats pretty much the 2 issues we have.
> 
> OK thanks good to hear it's limited to earlycon issues.
> 
> Regards,
> 
> Tony
> 

[PATCH 1/2] Revert "ARM: OMAP2+: Fix serial init for device tree based booting"

[Rajendra Nayak]

On Friday 12 July 2013 02:48 PM, Tony Lindgren wrote:
> * Rajendra Nayak <rnayak@ti.com> [130712 02:06]:
>> On Friday 12 July 2013 01:33 PM, Tony Lindgren wrote:
>>>
>>> OK, so that's only for earlyprintk then?
>>
>> yes,
>>
>>>
>>> If so, it seems the right fix is to set the NO_IDLE and NO_RESET
>>> flags based on ifdef CONFIG_DEBUG_OMAP4UART3 etc as that is selected
>>> in the Kconfig now.
>>
>> ok makes sense. It seems like the static data in hwmod can be populated
>> based on these defines? something like
>>
>> /* uart3 */
>> static struct omap_hwmod omap44xx_uart3_hwmod = {
>>         .name           = "uart3",
>>         .class          = &omap44xx_uart_hwmod_class,
>>         .clkdm_name     = "l4_per_clkdm",
>> #ifdef CONFIG_DEBUG_OMAP4UART3
>>         .flags          = HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET |
>>                                 HWMOD_SWSUP_SIDLE_ACT,
>> #else
>>         .flags          = HWMOD_SWSUP_SIDLE_ACT,
>> #endif
>>         .main_clk       = "func_48m_fclk",
>>         .prcm = {
>>                 .omap4 = {
>>                         .clkctrl_offs = OMAP4_CM_L4PER_UART3_CLKCTRL_OFFSET,
>>                         .context_offs = OMAP4_RM_L4PER_UART3_CONTEXT_OFFSET,
>>                         .modulemode   = MODULEMODE_SWCTRL,
>>                 },
>>         },
>> };
>>
>> And same way for others? That way the cmdline parsing can be done away
>> with even for the non-DT case.
> 
> Yes we can do it that way. How about add a common macro for it if
> it's always the same? Then the .flags line would be just:
> 
> #define HWMOD_OMAP_UART_FLAGS(soc, port)
> ...
> 
> 	.flags		= HWMOD_OMAP_UART_FLAGS(4, 3),

Right, that seems much cleaner. I will send a patch out for it and get rid
of all the cmdline parsing in serial.c

regards
Rajendra
> 
>>> The current code in mach-omap2/serial.c is wrong, and is a hack
>>> needed for the pdata based booting. What's broken is that
>>> omap_serial_early_init() parses the cmdline for console, which
>>> itself is pretty nasty, and it won't work the right way as
>>> there's nothing stopping from having the earlycon in a different
>>> UART from the serial console. So we just want to get rid of the
>>> whole mach-omap2/serial.c once we're all DT.
>>>
>>> So to summarize, we have two bugs:
>>>
>>> 1. Omap hwmod code can reset UART while earlycon may be using
>>>    it. The fix to this is to use NO_IDLE and NO_RESET flags in
>>>    the hwmod code if CONFIG_DEBUG_OMAPxUARTy is specified.
>>>
>>> 2. A bug in drivers/tty/serial/omap-serial.c where the
>>>    missing context loss count can cause NULL context to be
>>>    initialized during driver probe causing port to hang for
>>>    earlycon. The fix for that is what Felipe has suggested or
>>>    fix it in the driver by removing the context loss count usage
>>>    and detect the need for context restore based on the UART
>>>    state.
>>>
>>> Or am I still missing something?
>>
>> No, thats pretty much the 2 issues we have.
> 
> OK thanks good to hear it's limited to earlycon issues.
> 
> Regards,
> 
> Tony
> 

Re: [PATCH 1/2] Revert "ARM: OMAP2+: Fix serial init for device tree based booting"

[Tony Lindgren]

* Rajendra Nayak <rnayak@ti.com> [130712 02:29]:
>
> Right, that seems much cleaner. I will send a patch out for it and get rid
> of all the cmdline parsing in serial.c

Oh that's right, then it should not be needed for legacy pdata based
booting either :)

Regards,

Tony

[PATCH 1/2] Revert "ARM: OMAP2+: Fix serial init for device tree based booting"

[Tony Lindgren]

* Rajendra Nayak <rnayak@ti.com> [130712 02:29]:
>
> Right, that seems much cleaner. I will send a patch out for it and get rid
> of all the cmdline parsing in serial.c

Oh that's right, then it should not be needed for legacy pdata based
booting either :)

Regards,

Tony

Re: [PATCH 1/2] Revert "ARM: OMAP2+: Fix serial init for device tree based booting"

[Rajendra Nayak]

On Friday 12 July 2013 02:48 PM, Tony Lindgren wrote:
> * Rajendra Nayak <rnayak@ti.com> [130712 02:06]:
>> On Friday 12 July 2013 01:33 PM, Tony Lindgren wrote:
>>>
>>> OK, so that's only for earlyprintk then?
>>
>> yes,
>>
>>>
>>> If so, it seems the right fix is to set the NO_IDLE and NO_RESET
>>> flags based on ifdef CONFIG_DEBUG_OMAP4UART3 etc as that is selected
>>> in the Kconfig now.
>>
>> ok makes sense. It seems like the static data in hwmod can be populated
>> based on these defines? something like
>>
>> /* uart3 */
>> static struct omap_hwmod omap44xx_uart3_hwmod = {
>>         .name           = "uart3",
>>         .class          = &omap44xx_uart_hwmod_class,
>>         .clkdm_name     = "l4_per_clkdm",
>> #ifdef CONFIG_DEBUG_OMAP4UART3
>>         .flags          = HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET |
>>                                 HWMOD_SWSUP_SIDLE_ACT,
>> #else
>>         .flags          = HWMOD_SWSUP_SIDLE_ACT,
>> #endif
>>         .main_clk       = "func_48m_fclk",
>>         .prcm = {
>>                 .omap4 = {
>>                         .clkctrl_offs = OMAP4_CM_L4PER_UART3_CLKCTRL_OFFSET,
>>                         .context_offs = OMAP4_RM_L4PER_UART3_CONTEXT_OFFSET,
>>                         .modulemode   = MODULEMODE_SWCTRL,
>>                 },
>>         },
>> };
>>
>> And same way for others? That way the cmdline parsing can be done away
>> with even for the non-DT case.
> 
> Yes we can do it that way. How about add a common macro for it if
> it's always the same? Then the .flags line would be just:
> 
> #define HWMOD_OMAP_UART_FLAGS(soc, port)
> ...
> 
> 	.flags		= HWMOD_OMAP_UART_FLAGS(4, 3),

I started doing this and ended up with equal number of #ifdefs in the
header :( Was the intention of doing this to reduce the #ifdefs? in which
case maybe I am doing something wrong.

> 
>>> The current code in mach-omap2/serial.c is wrong, and is a hack
>>> needed for the pdata based booting. What's broken is that
>>> omap_serial_early_init() parses the cmdline for console, which
>>> itself is pretty nasty, and it won't work the right way as
>>> there's nothing stopping from having the earlycon in a different
>>> UART from the serial console. So we just want to get rid of the
>>> whole mach-omap2/serial.c once we're all DT.
>>>
>>> So to summarize, we have two bugs:
>>>
>>> 1. Omap hwmod code can reset UART while earlycon may be using
>>>    it. The fix to this is to use NO_IDLE and NO_RESET flags in
>>>    the hwmod code if CONFIG_DEBUG_OMAPxUARTy is specified.
>>>
>>> 2. A bug in drivers/tty/serial/omap-serial.c where the
>>>    missing context loss count can cause NULL context to be
>>>    initialized during driver probe causing port to hang for
>>>    earlycon. The fix for that is what Felipe has suggested or
>>>    fix it in the driver by removing the context loss count usage
>>>    and detect the need for context restore based on the UART
>>>    state.
>>>
>>> Or am I still missing something?
>>
>> No, thats pretty much the 2 issues we have.
> 
> OK thanks good to hear it's limited to earlycon issues.
> 
> Regards,
> 
> Tony
> 

[PATCH 1/2] Revert "ARM: OMAP2+: Fix serial init for device tree based booting"

[Rajendra Nayak]

On Friday 12 July 2013 02:48 PM, Tony Lindgren wrote:
> * Rajendra Nayak <rnayak@ti.com> [130712 02:06]:
>> On Friday 12 July 2013 01:33 PM, Tony Lindgren wrote:
>>>
>>> OK, so that's only for earlyprintk then?
>>
>> yes,
>>
>>>
>>> If so, it seems the right fix is to set the NO_IDLE and NO_RESET
>>> flags based on ifdef CONFIG_DEBUG_OMAP4UART3 etc as that is selected
>>> in the Kconfig now.
>>
>> ok makes sense. It seems like the static data in hwmod can be populated
>> based on these defines? something like
>>
>> /* uart3 */
>> static struct omap_hwmod omap44xx_uart3_hwmod = {
>>         .name           = "uart3",
>>         .class          = &omap44xx_uart_hwmod_class,
>>         .clkdm_name     = "l4_per_clkdm",
>> #ifdef CONFIG_DEBUG_OMAP4UART3
>>         .flags          = HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET |
>>                                 HWMOD_SWSUP_SIDLE_ACT,
>> #else
>>         .flags          = HWMOD_SWSUP_SIDLE_ACT,
>> #endif
>>         .main_clk       = "func_48m_fclk",
>>         .prcm = {
>>                 .omap4 = {
>>                         .clkctrl_offs = OMAP4_CM_L4PER_UART3_CLKCTRL_OFFSET,
>>                         .context_offs = OMAP4_RM_L4PER_UART3_CONTEXT_OFFSET,
>>                         .modulemode   = MODULEMODE_SWCTRL,
>>                 },
>>         },
>> };
>>
>> And same way for others? That way the cmdline parsing can be done away
>> with even for the non-DT case.
> 
> Yes we can do it that way. How about add a common macro for it if
> it's always the same? Then the .flags line would be just:
> 
> #define HWMOD_OMAP_UART_FLAGS(soc, port)
> ...
> 
> 	.flags		= HWMOD_OMAP_UART_FLAGS(4, 3),

I started doing this and ended up with equal number of #ifdefs in the
header :( Was the intention of doing this to reduce the #ifdefs? in which
case maybe I am doing something wrong.

> 
>>> The current code in mach-omap2/serial.c is wrong, and is a hack
>>> needed for the pdata based booting. What's broken is that
>>> omap_serial_early_init() parses the cmdline for console, which
>>> itself is pretty nasty, and it won't work the right way as
>>> there's nothing stopping from having the earlycon in a different
>>> UART from the serial console. So we just want to get rid of the
>>> whole mach-omap2/serial.c once we're all DT.
>>>
>>> So to summarize, we have two bugs:
>>>
>>> 1. Omap hwmod code can reset UART while earlycon may be using
>>>    it. The fix to this is to use NO_IDLE and NO_RESET flags in
>>>    the hwmod code if CONFIG_DEBUG_OMAPxUARTy is specified.
>>>
>>> 2. A bug in drivers/tty/serial/omap-serial.c where the
>>>    missing context loss count can cause NULL context to be
>>>    initialized during driver probe causing port to hang for
>>>    earlycon. The fix for that is what Felipe has suggested or
>>>    fix it in the driver by removing the context loss count usage
>>>    and detect the need for context restore based on the UART
>>>    state.
>>>
>>> Or am I still missing something?
>>
>> No, thats pretty much the 2 issues we have.
> 
> OK thanks good to hear it's limited to earlycon issues.
> 
> Regards,
> 
> Tony
> 

Re: [PATCH 1/2] Revert "ARM: OMAP2+: Fix serial init for device tree based booting"

[Tony Lindgren]

* Rajendra Nayak <rnayak@ti.com> [130712 05:18]:
> On Friday 12 July 2013 02:48 PM, Tony Lindgren wrote:
> > * Rajendra Nayak <rnayak@ti.com> [130712 02:06]:
> >> On Friday 12 July 2013 01:33 PM, Tony Lindgren wrote:
> >>>
> >>> OK, so that's only for earlyprintk then?
> >>
> >> yes,
> >>
> >>>
> >>> If so, it seems the right fix is to set the NO_IDLE and NO_RESET
> >>> flags based on ifdef CONFIG_DEBUG_OMAP4UART3 etc as that is selected
> >>> in the Kconfig now.
> >>
> >> ok makes sense. It seems like the static data in hwmod can be populated
> >> based on these defines? something like
> >>
> >> /* uart3 */
> >> static struct omap_hwmod omap44xx_uart3_hwmod = {
> >>         .name           = "uart3",
> >>         .class          = &omap44xx_uart_hwmod_class,
> >>         .clkdm_name     = "l4_per_clkdm",
> >> #ifdef CONFIG_DEBUG_OMAP4UART3
> >>         .flags          = HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET |
> >>                                 HWMOD_SWSUP_SIDLE_ACT,
> >> #else
> >>         .flags          = HWMOD_SWSUP_SIDLE_ACT,
> >> #endif
> >>         .main_clk       = "func_48m_fclk",
> >>         .prcm = {
> >>                 .omap4 = {
> >>                         .clkctrl_offs = OMAP4_CM_L4PER_UART3_CLKCTRL_OFFSET,
> >>                         .context_offs = OMAP4_RM_L4PER_UART3_CONTEXT_OFFSET,
> >>                         .modulemode   = MODULEMODE_SWCTRL,
> >>                 },
> >>         },
> >> };
> >>
> >> And same way for others? That way the cmdline parsing can be done away
> >> with even for the non-DT case.
> > 
> > Yes we can do it that way. How about add a common macro for it if
> > it's always the same? Then the .flags line would be just:
> > 
> > #define HWMOD_OMAP_UART_FLAGS(soc, port)
> > ...
> > 
> > 	.flags		= HWMOD_OMAP_UART_FLAGS(4, 3),
> 
> I started doing this and ended up with equal number of #ifdefs in the
> header :( Was the intention of doing this to reduce the #ifdefs? in which
> case maybe I am doing something wrong.

Hmm can't you use ## for substition in the macro to reduce the ifdefs
and have just one macro instead one for every debug port?

Regards,

Tony

[PATCH 1/2] Revert "ARM: OMAP2+: Fix serial init for device tree based booting"

[Tony Lindgren]

* Rajendra Nayak <rnayak@ti.com> [130712 05:18]:
> On Friday 12 July 2013 02:48 PM, Tony Lindgren wrote:
> > * Rajendra Nayak <rnayak@ti.com> [130712 02:06]:
> >> On Friday 12 July 2013 01:33 PM, Tony Lindgren wrote:
> >>>
> >>> OK, so that's only for earlyprintk then?
> >>
> >> yes,
> >>
> >>>
> >>> If so, it seems the right fix is to set the NO_IDLE and NO_RESET
> >>> flags based on ifdef CONFIG_DEBUG_OMAP4UART3 etc as that is selected
> >>> in the Kconfig now.
> >>
> >> ok makes sense. It seems like the static data in hwmod can be populated
> >> based on these defines? something like
> >>
> >> /* uart3 */
> >> static struct omap_hwmod omap44xx_uart3_hwmod = {
> >>         .name           = "uart3",
> >>         .class          = &omap44xx_uart_hwmod_class,
> >>         .clkdm_name     = "l4_per_clkdm",
> >> #ifdef CONFIG_DEBUG_OMAP4UART3
> >>         .flags          = HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET |
> >>                                 HWMOD_SWSUP_SIDLE_ACT,
> >> #else
> >>         .flags          = HWMOD_SWSUP_SIDLE_ACT,
> >> #endif
> >>         .main_clk       = "func_48m_fclk",
> >>         .prcm = {
> >>                 .omap4 = {
> >>                         .clkctrl_offs = OMAP4_CM_L4PER_UART3_CLKCTRL_OFFSET,
> >>                         .context_offs = OMAP4_RM_L4PER_UART3_CONTEXT_OFFSET,
> >>                         .modulemode   = MODULEMODE_SWCTRL,
> >>                 },
> >>         },
> >> };
> >>
> >> And same way for others? That way the cmdline parsing can be done away
> >> with even for the non-DT case.
> > 
> > Yes we can do it that way. How about add a common macro for it if
> > it's always the same? Then the .flags line would be just:
> > 
> > #define HWMOD_OMAP_UART_FLAGS(soc, port)
> > ...
> > 
> > 	.flags		= HWMOD_OMAP_UART_FLAGS(4, 3),
> 
> I started doing this and ended up with equal number of #ifdefs in the
> header :( Was the intention of doing this to reduce the #ifdefs? in which
> case maybe I am doing something wrong.

Hmm can't you use ## for substition in the macro to reduce the ifdefs
and have just one macro instead one for every debug port?

Regards,

Tony