diff mbox series

serial: Make full device search optional

Message ID 20180129125720.32470-1-agraf@suse.de
State Accepted
Commit ae5326a6b34b34b1827edf2eee1a0e9e5363c5a2
Headers show
Series serial: Make full device search optional | expand

Commit Message

Alexander Graf Jan. 29, 2018, 12:57 p.m. UTC
Commit 608b0c4ad4e5ec0c ("serial: Use next serial device if probing fails")
added code to search for more serial devices if the default one was not
probed correctly.

Unfortunately, that breaks omap3_evm. So while investigating why that is
the case, let's disable the full search for everyone but bcm283x where it
is needed.

Fixes: 608b0c4ad4e5ec0c ("serial: Use next serial device if probing fails")
Reported-by: Derald D. Woods <woods.technical@gmail.com>
Signed-off-by: Alexander Graf <agraf@suse.de>

---

Derald, could you please test this patch and verify it does indeed unbreak
omap3_evm?

Thanks!
---
 arch/arm/Kconfig               |  1 +
 drivers/serial/Kconfig         | 12 ++++++++++++
 drivers/serial/serial-uclass.c | 13 +++++++++++++
 3 files changed, 26 insertions(+)

Comments

Derald Woods Jan. 29, 2018, 1:46 p.m. UTC | #1
On Jan 29, 2018 6:57 AM, "Alexander Graf" <agraf@suse.de> wrote:

Commit 608b0c4ad4e5ec0c ("serial: Use next serial device if probing fails")
added code to search for more serial devices if the default one was not
probed correctly.

Unfortunately, that breaks omap3_evm. So while investigating why that is
the case, let's disable the full search for everyone but bcm283x where it
is needed.

Fixes: 608b0c4ad4e5ec0c ("serial: Use next serial device if probing fails")
Reported-by: Derald D. Woods <woods.technical@gmail.com>
Signed-off-by: Alexander Graf <agraf@suse.de>

---

Derald, could you please test this patch and verify it does indeed unbreak
omap3_evm?



I will check later today. The board is not with me at the moment.

Derald



Thanks!
---
 arch/arm/Kconfig               |  1 +
 drivers/serial/Kconfig         | 12 ++++++++++++
 drivers/serial/serial-uclass.c | 13 +++++++++++++
 3 files changed, 26 insertions(+)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 30a6f6dc53..a423aa9629 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -550,6 +550,7 @@ config ARCH_BCM283X
        select DM_GPIO
        select OF_CONTROL
        select PL01X_SERIAL
+       select SERIAL_SEARCH_ALL
        imply FAT_WRITE

 config TARGET_VEXPRESS_CA15_TC2
diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
index 3ffedba525..93e602e0ee 100644
--- a/drivers/serial/Kconfig
+++ b/drivers/serial/Kconfig
@@ -79,6 +79,18 @@ config SERIAL_RX_BUFFER_SIZE
        help
          The size of the RX buffer (needs to be power of 2)

+config SERIAL_SEARCH_ALL
+       bool "Search for serial devices after default one failed"
+       depends on DM_SERIAL
+       help
+         The serial subsystem only searches for a single serial device
+         that was instantiated, but does not check whether it was probed
+         correctly. With this option set, we make successful probing
+         mandatory and search for fallback serial devices if the default
+         device does not work.
+
+         If unsure, say N.
+
 config SPL_DM_SERIAL
        bool "Enable Driver Model for serial drivers in SPL"
        depends on DM_SERIAL
diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
index 68ca2d09d1..9891c20656 100644
--- a/drivers/serial/serial-uclass.c
+++ b/drivers/serial/serial-uclass.c
@@ -74,7 +74,9 @@ static void serial_find_console_or_panic(void)
 {
        const void *blob = gd->fdt_blob;
        struct udevice *dev;
+#ifdef CONFIG_SERIAL_SEARCH_ALL
        int ret;
+#endif

        if (CONFIG_IS_ENABLED(OF_PLATDATA)) {
                uclass_first_device(UCLASS_SERIAL, &dev);
@@ -113,6 +115,8 @@ static void serial_find_console_or_panic(void)
 #else
 #define INDEX 0
 #endif
+
+#ifdef CONFIG_SERIAL_SEARCH_ALL
                if (!uclass_get_device_by_seq(UCLASS_SERIAL, INDEX, &dev) ||
                    !uclass_get_device(UCLASS_SERIAL, INDEX, &dev)) {
                        if (dev->flags & DM_FLAG_ACTIVATED) {
@@ -131,6 +135,15 @@ static void serial_find_console_or_panic(void)
                                return;
                        }
                }
+#else
+               if (!uclass_get_device_by_seq(UCLASS_SERIAL, INDEX, &dev) ||
+                   !uclass_get_device(UCLASS_SERIAL, INDEX, &dev) ||
+                   (!uclass_first_device(UCLASS_SERIAL, &dev) && dev)) {
+                       gd->cur_serial_dev = dev;
+                       return;
+               }
+#endif
+
 #undef INDEX
        }

--
2.12.3
Tom Rini Jan. 29, 2018, 4:55 p.m. UTC | #2
On Mon, Jan 29, 2018 at 01:57:20PM +0100, Alexander Graf wrote:

> Commit 608b0c4ad4e5ec0c ("serial: Use next serial device if probing fails")

> added code to search for more serial devices if the default one was not

> probed correctly.

> 

> Unfortunately, that breaks omap3_evm. So while investigating why that is

> the case, let's disable the full search for everyone but bcm283x where it

> is needed.

> 

> Fixes: 608b0c4ad4e5ec0c ("serial: Use next serial device if probing fails")

> Reported-by: Derald D. Woods <woods.technical@gmail.com>

> Signed-off-by: Alexander Graf <agraf@suse.de>

> 


Applied to u-boot/master, thanks!

-- 
Tom
Derald Woods Jan. 29, 2018, 11:41 p.m. UTC | #3
On Mon, Jan 29, 2018 at 07:46:09AM -0600, Derald Woods wrote:
> On Jan 29, 2018 6:57 AM, "Alexander Graf" <agraf@suse.de> wrote:
> 
> Commit 608b0c4ad4e5ec0c ("serial: Use next serial device if probing fails")
> added code to search for more serial devices if the default one was not
> probed correctly.
> 
> Unfortunately, that breaks omap3_evm. So while investigating why that is
> the case, let's disable the full search for everyone but bcm283x where it
> is needed.
> 
> Fixes: 608b0c4ad4e5ec0c ("serial: Use next serial device if probing fails")
> Reported-by: Derald D. Woods <woods.technical@gmail.com>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> 
> ---
> 
> Derald, could you please test this patch and verify it does indeed unbreak
> omap3_evm?
> 

The omap3_evm boots now with this patch applied on master. If I enable
SERIAL_SEARCH_ALL, it does not boot. I always build cleanly using the
default config only. On failure, there is no console input/output and
the board unresponsive.

Derald

> 
> 
> I will check later today. The board is not with me at the moment.
> 
> Derald
> 
> 
> 
> Thanks!
> ---
>  arch/arm/Kconfig               |  1 +
>  drivers/serial/Kconfig         | 12 ++++++++++++
>  drivers/serial/serial-uclass.c | 13 +++++++++++++
>  3 files changed, 26 insertions(+)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 30a6f6dc53..a423aa9629 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -550,6 +550,7 @@ config ARCH_BCM283X
>         select DM_GPIO
>         select OF_CONTROL
>         select PL01X_SERIAL
> +       select SERIAL_SEARCH_ALL
>         imply FAT_WRITE
> 
>  config TARGET_VEXPRESS_CA15_TC2
> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> index 3ffedba525..93e602e0ee 100644
> --- a/drivers/serial/Kconfig
> +++ b/drivers/serial/Kconfig
> @@ -79,6 +79,18 @@ config SERIAL_RX_BUFFER_SIZE
>         help
>           The size of the RX buffer (needs to be power of 2)
> 
> +config SERIAL_SEARCH_ALL
> +       bool "Search for serial devices after default one failed"
> +       depends on DM_SERIAL
> +       help
> +         The serial subsystem only searches for a single serial device
> +         that was instantiated, but does not check whether it was probed
> +         correctly. With this option set, we make successful probing
> +         mandatory and search for fallback serial devices if the default
> +         device does not work.
> +
> +         If unsure, say N.
> +
>  config SPL_DM_SERIAL
>         bool "Enable Driver Model for serial drivers in SPL"
>         depends on DM_SERIAL
> diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
> index 68ca2d09d1..9891c20656 100644
> --- a/drivers/serial/serial-uclass.c
> +++ b/drivers/serial/serial-uclass.c
> @@ -74,7 +74,9 @@ static void serial_find_console_or_panic(void)
>  {
>         const void *blob = gd->fdt_blob;
>         struct udevice *dev;
> +#ifdef CONFIG_SERIAL_SEARCH_ALL
>         int ret;
> +#endif
> 
>         if (CONFIG_IS_ENABLED(OF_PLATDATA)) {
>                 uclass_first_device(UCLASS_SERIAL, &dev);
> @@ -113,6 +115,8 @@ static void serial_find_console_or_panic(void)
>  #else
>  #define INDEX 0
>  #endif
> +
> +#ifdef CONFIG_SERIAL_SEARCH_ALL
>                 if (!uclass_get_device_by_seq(UCLASS_SERIAL, INDEX, &dev) ||
>                     !uclass_get_device(UCLASS_SERIAL, INDEX, &dev)) {
>                         if (dev->flags & DM_FLAG_ACTIVATED) {
> @@ -131,6 +135,15 @@ static void serial_find_console_or_panic(void)
>                                 return;
>                         }
>                 }
> +#else
> +               if (!uclass_get_device_by_seq(UCLASS_SERIAL, INDEX, &dev) ||
> +                   !uclass_get_device(UCLASS_SERIAL, INDEX, &dev) ||
> +                   (!uclass_first_device(UCLASS_SERIAL, &dev) && dev)) {
> +                       gd->cur_serial_dev = dev;
> +                       return;
> +               }
> +#endif
> +
>  #undef INDEX
>         }
> 
> --
> 2.12.3
Alexander Graf Jan. 30, 2018, 9:17 a.m. UTC | #4
On 01/30/2018 12:41 AM, Derald D. Woods wrote:
> On Mon, Jan 29, 2018 at 07:46:09AM -0600, Derald Woods wrote:
>> On Jan 29, 2018 6:57 AM, "Alexander Graf" <agraf@suse.de> wrote:
>>
>> Commit 608b0c4ad4e5ec0c ("serial: Use next serial device if probing fails")
>> added code to search for more serial devices if the default one was not
>> probed correctly.
>>
>> Unfortunately, that breaks omap3_evm. So while investigating why that is
>> the case, let's disable the full search for everyone but bcm283x where it
>> is needed.
>>
>> Fixes: 608b0c4ad4e5ec0c ("serial: Use next serial device if probing fails")
>> Reported-by: Derald D. Woods <woods.technical@gmail.com>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>
>> ---
>>
>> Derald, could you please test this patch and verify it does indeed unbreak
>> omap3_evm?
>>
> The omap3_evm boots now with this patch applied on master. If I enable
> SERIAL_SEARCH_ALL, it does not boot. I always build cleanly using the
> default config only. On failure, there is no console input/output and
> the board unresponsive.

So SPL is already broken? Can you try a known working SPL with 
SERIAL_SEARCH_ALL=y U-Boot payload on top? Does that work?


Alex
Derald Woods Jan. 30, 2018, 1:09 p.m. UTC | #5
On Jan 30, 2018 3:17 AM, "Alexander Graf" <agraf@suse.de> wrote:

On 01/30/2018 12:41 AM, Derald D. Woods wrote:

> On Mon, Jan 29, 2018 at 07:46:09AM -0600, Derald Woods wrote:
>
>> On Jan 29, 2018 6:57 AM, "Alexander Graf" <agraf@suse.de> wrote:
>>
>> Commit 608b0c4ad4e5ec0c ("serial: Use next serial device if probing
>> fails")
>> added code to search for more serial devices if the default one was not
>> probed correctly.
>>
>> Unfortunately, that breaks omap3_evm. So while investigating why that is
>> the case, let's disable the full search for everyone but bcm283x where it
>> is needed.
>>
>> Fixes: 608b0c4ad4e5ec0c ("serial: Use next serial device if probing
>> fails")
>> Reported-by: Derald D. Woods <woods.technical@gmail.com>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>
>> ---
>>
>> Derald, could you please test this patch and verify it does indeed unbreak
>> omap3_evm?
>>
>> The omap3_evm boots now with this patch applied on master. If I enable
> SERIAL_SEARCH_ALL, it does not boot. I always build cleanly using the
> default config only. On failure, there is no console input/output and
> the board unresponsive.
>

So SPL is already broken? Can you try a known working SPL with
SERIAL_SEARCH_ALL=y U-Boot payload on top? Does that work?


I will give that path a try and see what I can discover. Again, it will be
later today or tomorrow before I can get to this. This is why I asked what
should the board code actually look like. As the omap3_evm is ahead of some
other OMAP34XX boards currently, a good working example would be helpful.
If omap3_evm becomes the example, let's make it a good one.

Derald
Alexander Graf Jan. 30, 2018, 1:34 p.m. UTC | #6
On 01/30/2018 02:09 PM, Derald Woods wrote:
> On Jan 30, 2018 3:17 AM, "Alexander Graf" <agraf@suse.de 
> <mailto:agraf@suse.de>> wrote:
>
>     On 01/30/2018 12:41 AM, Derald D. Woods wrote:
>
>         On Mon, Jan 29, 2018 at 07:46:09AM -0600, Derald Woods wrote:
>
>             On Jan 29, 2018 6:57 AM, "Alexander Graf" <agraf@suse.de
>             <mailto:agraf@suse.de>> wrote:
>
>             Commit 608b0c4ad4e5ec0c ("serial: Use next serial device
>             if probing fails")
>             added code to search for more serial devices if the
>             default one was not
>             probed correctly.
>
>             Unfortunately, that breaks omap3_evm. So while
>             investigating why that is
>             the case, let's disable the full search for everyone but
>             bcm283x where it
>             is needed.
>
>             Fixes: 608b0c4ad4e5ec0c ("serial: Use next serial device
>             if probing fails")
>             Reported-by: Derald D. Woods <woods.technical@gmail.com
>             <mailto:woods.technical@gmail.com>>
>             Signed-off-by: Alexander Graf <agraf@suse.de
>             <mailto:agraf@suse.de>>
>
>             ---
>
>             Derald, could you please test this patch and verify it
>             does indeed unbreak
>             omap3_evm?
>
>         The omap3_evm boots now with this patch applied on master. If
>         I enable
>         SERIAL_SEARCH_ALL, it does not boot. I always build cleanly
>         using the
>         default config only. On failure, there is no console
>         input/output and
>         the board unresponsive.
>
>
>     So SPL is already broken? Can you try a known working SPL with
>     SERIAL_SEARCH_ALL=y U-Boot payload on top? Does that work?
>
>
> I will give that path a try and see what I can discover. Again, it 
> will be later today or tomorrow before I can get to this. This is why 
> I asked what should the board code actually look like. As the 
> omap3_evm is ahead of some other OMAP34XX boards currently, a good 
> working example would be helpful. If omap3_evm becomes the example, 
> let's make it a good one.

If you want to make it a good example, don't disable CONFIG_EFI_LOADER :).

But really, the only major difference I saw between beagle and evm was 
the fact that evm used DM in SPL. I patched that up locally (had to 
remove ext support as the binary became too big otherwise), but that 
didn't show the issue either. So we'll have to wait on your tests.


Alex
Derald Woods Feb. 5, 2018, 12:39 a.m. UTC | #7
On Tue, Jan 30, 2018 at 7:34 AM, Alexander Graf <agraf@suse.de> wrote:

> On 01/30/2018 02:09 PM, Derald Woods wrote:
>
>> On Jan 30, 2018 3:17 AM, "Alexander Graf" <agraf@suse.de <mailto:
>> agraf@suse.de>> wrote:
>>
>>     On 01/30/2018 12:41 AM, Derald D. Woods wrote:
>>
>>         On Mon, Jan 29, 2018 at 07:46:09AM -0600, Derald Woods wrote:
>>
>>             On Jan 29, 2018 6:57 AM, "Alexander Graf" <agraf@suse.de
>>             <mailto:agraf@suse.de>> wrote:
>>
>>             Commit 608b0c4ad4e5ec0c ("serial: Use next serial device
>>             if probing fails")
>>             added code to search for more serial devices if the
>>             default one was not
>>             probed correctly.
>>
>>             Unfortunately, that breaks omap3_evm. So while
>>             investigating why that is
>>             the case, let's disable the full search for everyone but
>>             bcm283x where it
>>             is needed.
>>
>>             Fixes: 608b0c4ad4e5ec0c ("serial: Use next serial device
>>             if probing fails")
>>             Reported-by: Derald D. Woods <woods.technical@gmail.com
>>             <mailto:woods.technical@gmail.com>>
>>             Signed-off-by: Alexander Graf <agraf@suse.de
>>             <mailto:agraf@suse.de>>
>>
>>             ---
>>
>>             Derald, could you please test this patch and verify it
>>             does indeed unbreak
>>             omap3_evm?
>>
>>         The omap3_evm boots now with this patch applied on master. If
>>         I enable
>>         SERIAL_SEARCH_ALL, it does not boot. I always build cleanly
>>         using the
>>         default config only. On failure, there is no console
>>         input/output and
>>         the board unresponsive.
>>
>>
>>     So SPL is already broken? Can you try a known working SPL with
>>     SERIAL_SEARCH_ALL=y U-Boot payload on top? Does that work?
>>
>>
>> I will give that path a try and see what I can discover. Again, it will
>> be later today or tomorrow before I can get to this. This is why I asked
>> what should the board code actually look like. As the omap3_evm is ahead of
>> some other OMAP34XX boards currently, a good working example would be
>> helpful. If omap3_evm becomes the example, let's make it a good one.
>>
>
> If you want to make it a good example, don't disable CONFIG_EFI_LOADER :).
>
> But really, the only major difference I saw between beagle and evm was the
> fact that evm used DM in SPL. I patched that up locally (had to remove ext
> support as the binary became too big otherwise), but that didn't show the
> issue either. So we'll have to wait on your test
> ​s.
>


​It looks like some compiler issue is causing the problem. I was using GCC
7.2.0. When I go back to GCC 6.4.0 the board boots with SERIAL_SEARCH_ALL=y.
I also verified this by enabling SPL_DM_SERIAL on 'omap3_beagle' and
booting with SERIAL_SEARCH_ALL=y. Both GCC versions compiled without error.
GCC 6.4.0 is the compiler version that is working for me now. The actual
offending generated code will take more time, for me, to sort through.

Derald
Alexander Graf Feb. 5, 2018, 9:42 a.m. UTC | #8
On 05.02.18 01:39, Derald Woods wrote:
> On Tue, Jan 30, 2018 at 7:34 AM, Alexander Graf <agraf@suse.de
> <mailto:agraf@suse.de>> wrote:
> 
>     On 01/30/2018 02:09 PM, Derald Woods wrote:
> 
>         On Jan 30, 2018 3:17 AM, "Alexander Graf" <agraf@suse.de
>         <mailto:agraf@suse.de> <mailto:agraf@suse.de
>         <mailto:agraf@suse.de>>> wrote:
> 
>             On 01/30/2018 12:41 AM, Derald D. Woods wrote:
> 
>                 On Mon, Jan 29, 2018 at 07:46:09AM -0600, Derald Woods
>         wrote:
> 
>                     On Jan 29, 2018 6:57 AM, "Alexander Graf"
>         <agraf@suse.de <mailto:agraf@suse.de>
>                     <mailto:agraf@suse.de <mailto:agraf@suse.de>>> wrote:
> 
>                     Commit 608b0c4ad4e5ec0c ("serial: Use next serial device
>                     if probing fails")
>                     added code to search for more serial devices if the
>                     default one was not
>                     probed correctly.
> 
>                     Unfortunately, that breaks omap3_evm. So while
>                     investigating why that is
>                     the case, let's disable the full search for everyone but
>                     bcm283x where it
>                     is needed.
> 
>                     Fixes: 608b0c4ad4e5ec0c ("serial: Use next serial device
>                     if probing fails")
>                     Reported-by: Derald D. Woods
>         <woods.technical@gmail.com <mailto:woods.technical@gmail.com>
>                     <mailto:woods.technical@gmail.com
>         <mailto:woods.technical@gmail.com>>>
>                     Signed-off-by: Alexander Graf <agraf@suse.de
>         <mailto:agraf@suse.de>
>                     <mailto:agraf@suse.de <mailto:agraf@suse.de>>>
> 
>                     ---
> 
>                     Derald, could you please test this patch and verify it
>                     does indeed unbreak
>                     omap3_evm?
> 
>                 The omap3_evm boots now with this patch applied on
>         master. If
>                 I enable
>                 SERIAL_SEARCH_ALL, it does not boot. I always build cleanly
>                 using the
>                 default config only. On failure, there is no console
>                 input/output and
>                 the board unresponsive.
> 
> 
>             So SPL is already broken? Can you try a known working SPL with
>             SERIAL_SEARCH_ALL=y U-Boot payload on top? Does that work?
> 
> 
>         I will give that path a try and see what I can discover. Again,
>         it will be later today or tomorrow before I can get to this.
>         This is why I asked what should the board code actually look
>         like. As the omap3_evm is ahead of some other OMAP34XX boards
>         currently, a good working example would be helpful. If omap3_evm
>         becomes the example, let's make it a good one.
> 
> 
>     If you want to make it a good example, don't disable
>     CONFIG_EFI_LOADER :).
> 
>     But really, the only major difference I saw between beagle and evm
>     was the fact that evm used DM in SPL. I patched that up locally (had
>     to remove ext support as the binary became too big otherwise), but
>     that didn't show the issue either. So we'll have to wait on your test
>     ​s.
> 
> 
>  
> ​It looks like some compiler issue is causing the problem. I was using
> GCC 7.2.0. When I go back to GCC 6.4.0 the board boots with
> SERIAL_SEARCH_ALL=y. I also verified this by enabling SPL_DM_SERIAL on
> 'omap3_beagle' and booting with SERIAL_SEARCH_ALL=y. Both GCC versions
> compiled without error. GCC 6.4.0 is the compiler version that is
> working for me now. The actual offending generated code will take more
> time, for me, to sort through.

Can you somehow make it break with omap3_beagle? I have one of those and
could then help debug.


Alex
Derald Woods Feb. 5, 2018, 1:13 p.m. UTC | #9
On Mon, Feb 5, 2018 at 3:42 AM, Alexander Graf <agraf@suse.de> wrote:

>
>
> On 05.02.18 01:39, Derald Woods wrote:
> > On Tue, Jan 30, 2018 at 7:34 AM, Alexander Graf <agraf@suse.de
> > <mailto:agraf@suse.de>> wrote:
> >
> >     On 01/30/2018 02:09 PM, Derald Woods wrote:
> >
> >         On Jan 30, 2018 3:17 AM, "Alexander Graf" <agraf@suse.de
> >         <mailto:agraf@suse.de> <mailto:agraf@suse.de
> >         <mailto:agraf@suse.de>>> wrote:
> >
> >             On 01/30/2018 12:41 AM, Derald D. Woods wrote:
> >
> >                 On Mon, Jan 29, 2018 at 07:46:09AM -0600, Derald Woods
> >         wrote:
> >
> >                     On Jan 29, 2018 6:57 AM, "Alexander Graf"
> >         <agraf@suse.de <mailto:agraf@suse.de>
> >                     <mailto:agraf@suse.de <mailto:agraf@suse.de>>>
> wrote:
> >
> >                     Commit 608b0c4ad4e5ec0c ("serial: Use next serial
> device
> >                     if probing fails")
> >                     added code to search for more serial devices if the
> >                     default one was not
> >                     probed correctly.
> >
> >                     Unfortunately, that breaks omap3_evm. So while
> >                     investigating why that is
> >                     the case, let's disable the full search for everyone
> but
> >                     bcm283x where it
> >                     is needed.
> >
> >                     Fixes: 608b0c4ad4e5ec0c ("serial: Use next serial
> device
> >                     if probing fails")
> >                     Reported-by: Derald D. Woods
> >         <woods.technical@gmail.com <mailto:woods.technical@gmail.com>
> >                     <mailto:woods.technical@gmail.com
> >         <mailto:woods.technical@gmail.com>>>
> >                     Signed-off-by: Alexander Graf <agraf@suse.de
> >         <mailto:agraf@suse.de>
> >                     <mailto:agraf@suse.de <mailto:agraf@suse.de>>>
> >
> >                     ---
> >
> >                     Derald, could you please test this patch and verify
> it
> >                     does indeed unbreak
> >                     omap3_evm?
> >
> >                 The omap3_evm boots now with this patch applied on
> >         master. If
> >                 I enable
> >                 SERIAL_SEARCH_ALL, it does not boot. I always build
> cleanly
> >                 using the
> >                 default config only. On failure, there is no console
> >                 input/output and
> >                 the board unresponsive.
> >
> >
> >             So SPL is already broken? Can you try a known working SPL
> with
> >             SERIAL_SEARCH_ALL=y U-Boot payload on top? Does that work?
> >
> >
> >         I will give that path a try and see what I can discover. Again,
> >         it will be later today or tomorrow before I can get to this.
> >         This is why I asked what should the board code actually look
> >         like. As the omap3_evm is ahead of some other OMAP34XX boards
> >         currently, a good working example would be helpful. If omap3_evm
> >         becomes the example, let's make it a good one.
> >
> >
> >     If you want to make it a good example, don't disable
> >     CONFIG_EFI_LOADER :).
> >
> >     But really, the only major difference I saw between beagle and evm
> >     was the fact that evm used DM in SPL. I patched that up locally (had
> >     to remove ext support as the binary became too big otherwise), but
> >     that didn't show the issue either. So we'll have to wait on your test
> >     ​s.
> >
> >
> >
> > ​It looks like some compiler issue is causing the problem. I was using
> > GCC 7.2.0. When I go back to GCC 6.4.0 the board boots with
> > SERIAL_SEARCH_ALL=y. I also verified this by enabling SPL_DM_SERIAL on
> > 'omap3_beagle' and booting with SERIAL_SEARCH_ALL=y. Both GCC versions
> > compiled without error. GCC 6.4.0 is the compiler version that is
> > working for me now. The actual offending generated code will take more
> > time, for me, to sort through.
>
> Can you somehow make it break with omap3_beagle? I have one of those and
> could then help debug.
>


​No. Not with the current default configurations. I have both the
beagleboard(3530) and beagleboard-xM(3730) at home. Their configuration
currently does not enable DM_SERIAL/SPL_DM_SERIAL. I just recently added
OF_CONTROL for them on U-Boot master. The code path is different for
without DM_SERIAL. Enabling DM_SERIAL will aid in comparison, but will
require dropping some config options to make it fit into SRAM. The '-xM'
does not have NAND. On the omap3_evm, I enable UBI in the default config.
So their are at least a couple of options that would not apply to both
beagle variants. There should probably be a separate default config file
for each variant. The 3530 beagle variant would/should be identical to the
omap3_evm. They are somewhat related from a historical perspective. I will
put together an update to the default configurations this week. Once that
is done, I will be able to make a true comparison. (omap3_evm has a 3730
variant also)

Derald
Derald Woods Feb. 13, 2018, midnight UTC | #10
On Mon, Feb 5, 2018 at 7:13 AM, Derald Woods <woods.technical@gmail.com>
wrote:

> On Mon, Feb 5, 2018 at 3:42 AM, Alexander Graf <agraf@suse.de> wrote:
>
>>
>>
>> On 05.02.18 01:39, Derald Woods wrote:
>> > On Tue, Jan 30, 2018 at 7:34 AM, Alexander Graf <agraf@suse.de
>> > <mailto:agraf@suse.de>> wrote:
>> >
>> >     On 01/30/2018 02:09 PM, Derald Woods wrote:
>> >
>> >         On Jan 30, 2018 3:17 AM, "Alexander Graf" <agraf@suse.de
>> >         <mailto:agraf@suse.de> <mailto:agraf@suse.de
>> >         <mailto:agraf@suse.de>>> wrote:
>> >
>> >             On 01/30/2018 12:41 AM, Derald D. Woods wrote:
>> >
>> >                 On Mon, Jan 29, 2018 at 07:46:09AM -0600, Derald Woods
>> >         wrote:
>> >
>> >                     On Jan 29, 2018 6:57 AM, "Alexander Graf"
>> >         <agraf@suse.de <mailto:agraf@suse.de>
>> >                     <mailto:agraf@suse.de <mailto:agraf@suse.de>>>
>> wrote:
>> >
>> >                     Commit 608b0c4ad4e5ec0c ("serial: Use next serial
>> device
>> >                     if probing fails")
>> >                     added code to search for more serial devices if the
>> >                     default one was not
>> >                     probed correctly.
>> >
>> >                     Unfortunately, that breaks omap3_evm. So while
>> >                     investigating why that is
>> >                     the case, let's disable the full search for
>> everyone but
>> >                     bcm283x where it
>> >                     is needed.
>> >
>> >                     Fixes: 608b0c4ad4e5ec0c ("serial: Use next serial
>> device
>> >                     if probing fails")
>> >                     Reported-by: Derald D. Woods
>> >         <woods.technical@gmail.com <mailto:woods.technical@gmail.com>
>> >                     <mailto:woods.technical@gmail.com
>> >         <mailto:woods.technical@gmail.com>>>
>> >                     Signed-off-by: Alexander Graf <agraf@suse.de
>> >         <mailto:agraf@suse.de>
>> >                     <mailto:agraf@suse.de <mailto:agraf@suse.de>>>
>> >
>> >                     ---
>> >
>> >                     Derald, could you please test this patch and verify
>> it
>> >                     does indeed unbreak
>> >                     omap3_evm?
>> >
>> >                 The omap3_evm boots now with this patch applied on
>> >         master. If
>> >                 I enable
>> >                 SERIAL_SEARCH_ALL, it does not boot. I always build
>> cleanly
>> >                 using the
>> >                 default config only. On failure, there is no console
>> >                 input/output and
>> >                 the board unresponsive.
>> >
>> >
>> >             So SPL is already broken? Can you try a known working SPL
>> with
>> >             SERIAL_SEARCH_ALL=y U-Boot payload on top? Does that work?
>> >
>> >
>> >         I will give that path a try and see what I can discover. Again,
>> >         it will be later today or tomorrow before I can get to this.
>> >         This is why I asked what should the board code actually look
>> >         like. As the omap3_evm is ahead of some other OMAP34XX boards
>> >         currently, a good working example would be helpful. If omap3_evm
>> >         becomes the example, let's make it a good one.
>> >
>> >
>> >     If you want to make it a good example, don't disable
>> >     CONFIG_EFI_LOADER :).
>> >
>> >     But really, the only major difference I saw between beagle and evm
>> >     was the fact that evm used DM in SPL. I patched that up locally (had
>> >     to remove ext support as the binary became too big otherwise), but
>> >     that didn't show the issue either. So we'll have to wait on your
>> test
>> >     ​s.
>> >
>> >
>> >
>> > ​It looks like some compiler issue is causing the problem. I was using
>> > GCC 7.2.0. When I go back to GCC 6.4.0 the board boots with
>> > SERIAL_SEARCH_ALL=y. I also verified this by enabling SPL_DM_SERIAL on
>> > 'omap3_beagle' and booting with SERIAL_SEARCH_ALL=y. Both GCC versions
>> > compiled without error. GCC 6.4.0 is the compiler version that is
>> > working for me now. The actual offending generated code will take more
>> > time, for me, to sort through.
>>
>> Can you somehow make it break with omap3_beagle? I have one of those and
>> could then help debug.
>>
>
>
> ​No. Not with the current default configurations. I have both the
> beagleboard(3530) and beagleboard-xM(3730) at home. Their configuration
> currently does not enable DM_SERIAL/SPL_DM_SERIAL. I just recently added
> OF_CONTROL for them on U-Boot master. The code path is different for
> without DM_SERIAL. Enabling DM_SERIAL will aid in comparison, but will
> require dropping some config options to make it fit into SRAM. The '-xM'
> does not have NAND. On the omap3_evm, I enable UBI in the default config.
> So their are at least a couple of options that would not apply to both
> beagle variants. There should probably be a separate default config file
> for each variant. The 3530 beagle variant would/should be identical to the
> omap3_evm. They are somewhat related from a historical perspective. I will
> put together an update to the default configurations this week. Once that
> is done, I will be able to make a true comparison. (omap3_evm has a 3730
> variant also)
>
> Derald
>


​The issue appears to have been related to the use of
CONFIG_SPL_SYS_MALLOC_SIMPLE and CONFIG_SYS_MALLOC_F_LEN in the default
config. The RFC patch series is given below:

https://lists.denx.de/pipermail/u-boot/2018-February/320152.html

​The BeagleBoard config never attempted to use CONFIG_SPL_SYS_MALLOC_SIMPLE.

​Derald​
Alexander Graf Feb. 15, 2018, 3 p.m. UTC | #11
On 13.02.18 01:00, Derald Woods wrote:
> On Mon, Feb 5, 2018 at 7:13 AM, Derald Woods <woods.technical@gmail.com
> <mailto:woods.technical@gmail.com>> wrote:
> 
>     On Mon, Feb 5, 2018 at 3:42 AM, Alexander Graf <agraf@suse.de
>     <mailto:agraf@suse.de>>wrote:
> 
> 
> 
>         On 05.02.18 01:39, Derald Woods wrote:
>         > On Tue, Jan 30, 2018 at 7:34 AM, Alexander Graf <agraf@suse.de <mailto:agraf@suse.de>
>         > <mailto:agraf@suse.de <mailto:agraf@suse.de>>> wrote:
>         >
>         >     On 01/30/2018 02:09 PM, Derald Woods wrote:
>         >
>         >         On Jan 30, 2018 3:17 AM, "Alexander Graf" <agraf@suse.de <mailto:agraf@suse.de>
>         >         <mailto:agraf@suse.de <mailto:agraf@suse.de>>
>         <mailto:agraf@suse.de <mailto:agraf@suse.de>
>         >         <mailto:agraf@suse.de <mailto:agraf@suse.de>>>> wrote:
>         >
>         >             On 01/30/2018 12:41 AM, Derald D. Woods wrote:
>         >
>         >                 On Mon, Jan 29, 2018 at 07:46:09AM -0600, Derald Woods
>         >         wrote:
>         >
>         >                     On Jan 29, 2018 6:57 AM, "Alexander Graf"
>         >         <agraf@suse.de <mailto:agraf@suse.de> <mailto:agraf@suse.de
>         <mailto:agraf@suse.de>>
>         >                     <mailto:agraf@suse.de <mailto:agraf@suse.de> <mailto:agraf@suse.de
>         <mailto:agraf@suse.de>>>> wrote:
>         >
>         >                     Commit 608b0c4ad4e5ec0c ("serial: Use next serial device
>         >                     if probing fails")
>         >                     added code to search for more serial devices if the
>         >                     default one was not
>         >                     probed correctly.
>         >
>         >                     Unfortunately, that breaks omap3_evm. So while
>         >                     investigating why that is
>         >                     the case, let's disable the full search for everyone but
>         >                     bcm283x where it
>         >                     is needed.
>         >
>         >                     Fixes: 608b0c4ad4e5ec0c ("serial: Use next serial device
>         >                     if probing fails")
>         >                     Reported-by: Derald D. Woods
>         >         <woods.technical@gmail.com <mailto:woods.technical@gmail.com>
>         <mailto:woods.technical@gmail.com
>         <mailto:woods.technical@gmail.com>>
>         >                     <mailto:woods.technical@gmail.com
>         <mailto:woods.technical@gmail.com>
>         >         <mailto:woods.technical@gmail.com <mailto:woods.technical@gmail.com>>>>
>         >                     Signed-off-by: Alexander Graf <agraf@suse.de <mailto:agraf@suse.de>
>         >         <mailto:agraf@suse.de <mailto:agraf@suse.de>>
>         >                     <mailto:agraf@suse.de
>         <mailto:agraf@suse.de> <mailto:agraf@suse.de
>         <mailto:agraf@suse.de>>>>
>         >
>         >                     ---
>         >
>         >                     Derald, could you please test this patch
>         and verify it
>         >                     does indeed unbreak
>         >                     omap3_evm?
>         >
>         >                 The omap3_evm boots now with this patch applied on
>         >         master. If
>         >                 I enable
>         >                 SERIAL_SEARCH_ALL, it does not boot. I always
>         build cleanly
>         >                 using the
>         >                 default config only. On failure, there is no
>         console
>         >                 input/output and
>         >                 the board unresponsive.
>         >
>         >
>         >             So SPL is already broken? Can you try a known
>         working SPL with
>         >             SERIAL_SEARCH_ALL=y U-Boot payload on top? Does
>         that work?
>         >
>         >
>         >         I will give that path a try and see what I can
>         discover. Again,
>         >         it will be later today or tomorrow before I can get to
>         this.
>         >         This is why I asked what should the board code
>         actually look
>         >         like. As the omap3_evm is ahead of some other OMAP34XX
>         boards
>         >         currently, a good working example would be helpful. If
>         omap3_evm
>         >         becomes the example, let's make it a good one.
>         >
>         >
>         >     If you want to make it a good example, don't disable
>         >     CONFIG_EFI_LOADER :).
>         >
>         >     But really, the only major difference I saw between beagle
>         and evm
>         >     was the fact that evm used DM in SPL. I patched that up
>         locally (had
>         >     to remove ext support as the binary became too big
>         otherwise), but
>         >     that didn't show the issue either. So we'll have to wait
>         on your test
>         >     ​s.
>         >
>         >
>         >  
>         > ​It looks like some compiler issue is causing the problem. I
>         was using
>         > GCC 7.2.0. When I go back to GCC 6.4.0 the board boots with
>         > SERIAL_SEARCH_ALL=y. I also verified this by enabling
>         SPL_DM_SERIAL on
>         > 'omap3_beagle' and booting with SERIAL_SEARCH_ALL=y. Both GCC
>         versions
>         > compiled without error. GCC 6.4.0 is the compiler version that is
>         > working for me now. The actual offending generated code will
>         take more
>         > time, for me, to sort through.
> 
>         Can you somehow make it break with omap3_beagle? I have one of
>         those and
>         could then help debug.
> 
> 
> 
>     ​No. Not with the current default configurations. I have both the
>     beagleboard(3530) and beagleboard-xM(3730) at home. Their
>     configuration currently does not enable DM_SERIAL/SPL_DM_SERIAL. I
>     just recently added OF_CONTROL for them on U-Boot master. The code
>     path is different for without DM_SERIAL. Enabling DM_SERIAL will aid
>     in comparison, but will require dropping some config options to make
>     it fit into SRAM. The '-xM' does not have NAND. On the omap3_evm, I
>     enable UBI in the default config. So their are at least a couple of
>     options that would not apply to both beagle variants. There should
>     probably be a separate default config file for each variant. The
>     3530 beagle variant would/should be identical to the omap3_evm. They
>     are somewhat related from a historical perspective. I will put
>     together an update to the default configurations this week. Once
>     that is done, I will be able to make a true comparison. (omap3_evm
>     has a 3730 variant also)
> 
>     Derald
> 
> 
> 
> ​The issue appears to have been related to the use of
> CONFIG_SPL_SYS_MALLOC_SIMPLE and CONFIG_SYS_MALLOC_F_LEN in the default
> config. The RFC patch series is given below:
> 
> https://lists.denx.de/pipermail/u-boot/2018-February/320152.html
> 
> ​The BeagleBoard config never attempted to use CONFIG_SPL_SYS_MALLOC_SIMPLE.

So you're saying it's probably just running out of memory?


Alex
Derald Woods Feb. 15, 2018, 5:02 p.m. UTC | #12
On Feb 15, 2018 9:00 AM, "Alexander Graf" <agraf@suse.de> wrote:



On 13.02.18 01:00, Derald Woods wrote:
> On Mon, Feb 5, 2018 at 7:13 AM, Derald Woods <woods.technical@gmail.com
> <mailto:woods.technical@gmail.com>> wrote:
>
>     On Mon, Feb 5, 2018 at 3:42 AM, Alexander Graf <agraf@suse.de
>     <mailto:agraf@suse.de>>wrote:
>
>
>
>         On 05.02.18 01:39, Derald Woods wrote:
>         > On Tue, Jan 30, 2018 at 7:34 AM, Alexander Graf <agraf@suse.de
<mailto:agraf@suse.de>
>         > <mailto:agraf@suse.de <mailto:agraf@suse.de>>> wrote:
>         >
>         >     On 01/30/2018 02:09 PM, Derald Woods wrote:
>         >
>         >         On Jan 30, 2018 3:17 AM, "Alexander Graf" <agraf@suse.de
<mailto:agraf@suse.de>
>         >         <mailto:agraf@suse.de <mailto:agraf@suse.de>>
>         <mailto:agraf@suse.de <mailto:agraf@suse.de>
>         >         <mailto:agraf@suse.de <mailto:agraf@suse.de>>>> wrote:
>         >
>         >             On 01/30/2018 12:41 AM, Derald D. Woods wrote:
>         >
>         >                 On Mon, Jan 29, 2018 at 07:46:09AM -0600,
Derald Woods
>         >         wrote:
>         >
>         >                     On Jan 29, 2018 6:57 AM, "Alexander Graf"
>         >         <agraf@suse.de <mailto:agraf@suse.de> <mailto:
agraf@suse.de
>         <mailto:agraf@suse.de>>
>         >                     <mailto:agraf@suse.de <mailto:agraf@suse.de>
<mailto:agraf@suse.de
>         <mailto:agraf@suse.de>>>> wrote:
>         >
>         >                     Commit 608b0c4ad4e5ec0c ("serial: Use next
serial device
>         >                     if probing fails")
>         >                     added code to search for more serial
devices if the
>         >                     default one was not
>         >                     probed correctly.
>         >
>         >                     Unfortunately, that breaks omap3_evm. So
while
>         >                     investigating why that is
>         >                     the case, let's disable the full search for
everyone but
>         >                     bcm283x where it
>         >                     is needed.
>         >
>         >                     Fixes: 608b0c4ad4e5ec0c ("serial: Use next
serial device
>         >                     if probing fails")
>         >                     Reported-by: Derald D. Woods
>         >         <woods.technical@gmail.com <mailto:
woods.technical@gmail.com>
>         <mailto:woods.technical@gmail.com
>         <mailto:woods.technical@gmail.com>>
>         >                     <mailto:woods.technical@gmail.com
>         <mailto:woods.technical@gmail.com>
>         >         <mailto:woods.technical@gmail.com <mailto:
woods.technical@gmail.com>>>>
>         >                     Signed-off-by: Alexander Graf <agraf@suse.de
<mailto:agraf@suse.de>
>         >         <mailto:agraf@suse.de <mailto:agraf@suse.de>>
>         >                     <mailto:agraf@suse.de
>         <mailto:agraf@suse.de> <mailto:agraf@suse.de
>         <mailto:agraf@suse.de>>>>
>         >
>         >                     ---
>         >
>         >                     Derald, could you please test this patch
>         and verify it
>         >                     does indeed unbreak
>         >                     omap3_evm?
>         >
>         >                 The omap3_evm boots now with this patch applied
on
>         >         master. If
>         >                 I enable
>         >                 SERIAL_SEARCH_ALL, it does not boot. I always
>         build cleanly
>         >                 using the
>         >                 default config only. On failure, there is no
>         console
>         >                 input/output and
>         >                 the board unresponsive.
>         >
>         >
>         >             So SPL is already broken? Can you try a known
>         working SPL with
>         >             SERIAL_SEARCH_ALL=y U-Boot payload on top? Does
>         that work?
>         >
>         >
>         >         I will give that path a try and see what I can
>         discover. Again,
>         >         it will be later today or tomorrow before I can get to
>         this.
>         >         This is why I asked what should the board code
>         actually look
>         >         like. As the omap3_evm is ahead of some other OMAP34XX
>         boards
>         >         currently, a good working example would be helpful. If
>         omap3_evm
>         >         becomes the example, let's make it a good one.
>         >
>         >
>         >     If you want to make it a good example, don't disable
>         >     CONFIG_EFI_LOADER :).
>         >
>         >     But really, the only major difference I saw between beagle
>         and evm
>         >     was the fact that evm used DM in SPL. I patched that up
>         locally (had
>         >     to remove ext support as the binary became too big
>         otherwise), but
>         >     that didn't show the issue either. So we'll have to wait
>         on your test
>         >     ​s.
>         >
>         >
>         >
>         > ​It looks like some compiler issue is causing the problem. I
>         was using
>         > GCC 7.2.0. When I go back to GCC 6.4.0 the board boots with
>         > SERIAL_SEARCH_ALL=y. I also verified this by enabling
>         SPL_DM_SERIAL on
>         > 'omap3_beagle' and booting with SERIAL_SEARCH_ALL=y. Both GCC
>         versions
>         > compiled without error. GCC 6.4.0 is the compiler version that
is
>         > working for me now. The actual offending generated code will
>         take more
>         > time, for me, to sort through.
>
>         Can you somehow make it break with omap3_beagle? I have one of
>         those and
>         could then help debug.
>
>
>
>     ​No. Not with the current default configurations. I have both the
>     beagleboard(3530) and beagleboard-xM(3730) at home. Their
>     configuration currently does not enable DM_SERIAL/SPL_DM_SERIAL. I
>     just recently added OF_CONTROL for them on U-Boot master. The code
>     path is different for without DM_SERIAL. Enabling DM_SERIAL will aid
>     in comparison, but will require dropping some config options to make
>     it fit into SRAM. The '-xM' does not have NAND. On the omap3_evm, I
>     enable UBI in the default config. So their are at least a couple of
>     options that would not apply to both beagle variants. There should
>     probably be a separate default config file for each variant. The
>     3530 beagle variant would/should be identical to the omap3_evm. They
>     are somewhat related from a historical perspective. I will put
>     together an update to the default configurations this week. Once
>     that is done, I will be able to make a true comparison. (omap3_evm
>     has a 3730 variant also)
>
>     Derald
>
>
>
> ​The issue appears to have been related to the use of
> CONFIG_SPL_SYS_MALLOC_SIMPLE and CONFIG_SYS_MALLOC_F_LEN in the default
> config. The RFC patch series is given below:
>
> https://lists.denx.de/pipermail/u-boot/2018-February/320152.html
>
> ​The BeagleBoard config never attempted to use
CONFIG_SPL_SYS_MALLOC_SIMPLE.

So you're saying it's probably just running out of memory?


Alex


The configuration mix was the key difference. The patch series has the
details. The default malloc length set by configuration is actually
smaller. So there are likely a mix of factors.

Derald
Alexander Graf Feb. 15, 2018, 7:37 p.m. UTC | #13
On 15.02.18 18:02, Derald Woods wrote:
> 
> 
> On Feb 15, 2018 9:00 AM, "Alexander Graf" <agraf@suse.de
> <mailto:agraf@suse.de>> wrote:
> 
> 
> 
>     On 13.02.18 01:00, Derald Woods wrote:
>     > On Mon, Feb 5, 2018 at 7:13 AM, Derald Woods
>     <woods.technical@gmail.com <mailto:woods.technical@gmail.com>
>     > <mailto:woods.technical@gmail.com
>     <mailto:woods.technical@gmail.com>>> wrote:
>     >
>     >     On Mon, Feb 5, 2018 at 3:42 AM, Alexander Graf <agraf@suse.de
>     <mailto:agraf@suse.de>
>     >     <mailto:agraf@suse.de <mailto:agraf@suse.de>>>wrote:
>     >
>     >
>     >
>     >         On 05.02.18 01:39, Derald Woods wrote:
>     >         > On Tue, Jan 30, 2018 at 7:34 AM, Alexander Graf
>     <agraf@suse.de <mailto:agraf@suse.de> <mailto:agraf@suse.de
>     <mailto:agraf@suse.de>>
>     >         > <mailto:agraf@suse.de <mailto:agraf@suse.de>
>     <mailto:agraf@suse.de <mailto:agraf@suse.de>>>> wrote:
>     >         >
>     >         >     On 01/30/2018 02:09 PM, Derald Woods wrote:
>     >         >
>     >         >         On Jan 30, 2018 3:17 AM, "Alexander Graf"
>     <agraf@suse.de <mailto:agraf@suse.de> <mailto:agraf@suse.de
>     <mailto:agraf@suse.de>>
>     >         >         <mailto:agraf@suse.de <mailto:agraf@suse.de>
>     <mailto:agraf@suse.de <mailto:agraf@suse.de>>>
>     >         <mailto:agraf@suse.de <mailto:agraf@suse.de>
>     <mailto:agraf@suse.de <mailto:agraf@suse.de>>
>     >         >         <mailto:agraf@suse.de <mailto:agraf@suse.de>
>     <mailto:agraf@suse.de <mailto:agraf@suse.de>>>>> wrote:
>     >         >
>     >         >             On 01/30/2018 12:41 AM, Derald D. Woods wrote:
>     >         >
>     >         >                 On Mon, Jan 29, 2018 at 07:46:09AM
>     -0600, Derald Woods
>     >         >         wrote:
>     >         >
>     >         >                     On Jan 29, 2018 6:57 AM, "Alexander
>     Graf"
>     >         >         <agraf@suse.de <mailto:agraf@suse.de>
>     <mailto:agraf@suse.de <mailto:agraf@suse.de>> <mailto:agraf@suse.de
>     <mailto:agraf@suse.de>
>     >         <mailto:agraf@suse.de <mailto:agraf@suse.de>>>
>     >         >                     <mailto:agraf@suse.de
>     <mailto:agraf@suse.de> <mailto:agraf@suse.de <mailto:agraf@suse.de>>
>     <mailto:agraf@suse.de <mailto:agraf@suse.de>
>     >         <mailto:agraf@suse.de <mailto:agraf@suse.de>>>>> wrote:
>     >         >
>     >         >                     Commit 608b0c4ad4e5ec0c ("serial:
>     Use next serial device
>     >         >                     if probing fails")
>     >         >                     added code to search for more serial
>     devices if the
>     >         >                     default one was not
>     >         >                     probed correctly.
>     >         >
>     >         >                     Unfortunately, that breaks
>     omap3_evm. So while
>     >         >                     investigating why that is
>     >         >                     the case, let's disable the full
>     search for everyone but
>     >         >                     bcm283x where it
>     >         >                     is needed.
>     >         >
>     >         >                     Fixes: 608b0c4ad4e5ec0c ("serial:
>     Use next serial device
>     >         >                     if probing fails")
>     >         >                     Reported-by: Derald D. Woods
>     >         >         <woods.technical@gmail.com
>     <mailto:woods.technical@gmail.com> <mailto:woods.technical@gmail.com
>     <mailto:woods.technical@gmail.com>>
>     >         <mailto:woods.technical@gmail.com
>     <mailto:woods.technical@gmail.com>
>     >         <mailto:woods.technical@gmail.com
>     <mailto:woods.technical@gmail.com>>>
>     >         >                     <mailto:woods.technical@gmail.com
>     <mailto:woods.technical@gmail.com>
>     >         <mailto:woods.technical@gmail.com
>     <mailto:woods.technical@gmail.com>>
>     >         >         <mailto:woods.technical@gmail.com
>     <mailto:woods.technical@gmail.com> <mailto:woods.technical@gmail.com
>     <mailto:woods.technical@gmail.com>>>>>
>     >         >                     Signed-off-by: Alexander Graf
>     <agraf@suse.de <mailto:agraf@suse.de> <mailto:agraf@suse.de
>     <mailto:agraf@suse.de>>
>     >         >         <mailto:agraf@suse.de <mailto:agraf@suse.de>
>     <mailto:agraf@suse.de <mailto:agraf@suse.de>>>
>     >         >                     <mailto:agraf@suse.de
>     <mailto:agraf@suse.de>
>     >         <mailto:agraf@suse.de <mailto:agraf@suse.de>>
>     <mailto:agraf@suse.de <mailto:agraf@suse.de>
>     >         <mailto:agraf@suse.de <mailto:agraf@suse.de>>>>>
>     >         >
>     >         >                     ---
>     >         >
>     >         >                     Derald, could you please test this patch
>     >         and verify it
>     >         >                     does indeed unbreak
>     >         >                     omap3_evm?
>     >         >
>     >         >                 The omap3_evm boots now with this patch
>     applied on
>     >         >         master. If
>     >         >                 I enable
>     >         >                 SERIAL_SEARCH_ALL, it does not boot. I
>     always
>     >         build cleanly
>     >         >                 using the
>     >         >                 default config only. On failure, there is no
>     >         console
>     >         >                 input/output and
>     >         >                 the board unresponsive.
>     >         >
>     >         >
>     >         >             So SPL is already broken? Can you try a known
>     >         working SPL with
>     >         >             SERIAL_SEARCH_ALL=y U-Boot payload on top? Does
>     >         that work?
>     >         >
>     >         >
>     >         >         I will give that path a try and see what I can
>     >         discover. Again,
>     >         >         it will be later today or tomorrow before I can
>     get to
>     >         this.
>     >         >         This is why I asked what should the board code
>     >         actually look
>     >         >         like. As the omap3_evm is ahead of some other
>     OMAP34XX
>     >         boards
>     >         >         currently, a good working example would be
>     helpful. If
>     >         omap3_evm
>     >         >         becomes the example, let's make it a good one.
>     >         >
>     >         >
>     >         >     If you want to make it a good example, don't disable
>     >         >     CONFIG_EFI_LOADER :).
>     >         >
>     >         >     But really, the only major difference I saw between
>     beagle
>     >         and evm
>     >         >     was the fact that evm used DM in SPL. I patched that up
>     >         locally (had
>     >         >     to remove ext support as the binary became too big
>     >         otherwise), but
>     >         >     that didn't show the issue either. So we'll have to wait
>     >         on your test
>     >         >     ​s.
>     >         >
>     >         >
>     >         >  
>     >         > ​It looks like some compiler issue is causing the problem. I
>     >         was using
>     >         > GCC 7.2.0. When I go back to GCC 6.4.0 the board boots with
>     >         > SERIAL_SEARCH_ALL=y. I also verified this by enabling
>     >         SPL_DM_SERIAL on
>     >         > 'omap3_beagle' and booting with SERIAL_SEARCH_ALL=y.
>     Both GCC
>     >         versions
>     >         > compiled without error. GCC 6.4.0 is the compiler
>     version that is
>     >         > working for me now. The actual offending generated code will
>     >         take more
>     >         > time, for me, to sort through.
>     >
>     >         Can you somehow make it break with omap3_beagle? I have one of
>     >         those and
>     >         could then help debug.
>     >
>     >
>     >
>     >     ​No. Not with the current default configurations. I have both the
>     >     beagleboard(3530) and beagleboard-xM(3730) at home. Their
>     >     configuration currently does not enable DM_SERIAL/SPL_DM_SERIAL. I
>     >     just recently added OF_CONTROL for them on U-Boot master. The code
>     >     path is different for without DM_SERIAL. Enabling DM_SERIAL
>     will aid
>     >     in comparison, but will require dropping some config options
>     to make
>     >     it fit into SRAM. The '-xM' does not have NAND. On the
>     omap3_evm, I
>     >     enable UBI in the default config. So their are at least a
>     couple of
>     >     options that would not apply to both beagle variants. There should
>     >     probably be a separate default config file for each variant. The
>     >     3530 beagle variant would/should be identical to the
>     omap3_evm. They
>     >     are somewhat related from a historical perspective. I will put
>     >     together an update to the default configurations this week. Once
>     >     that is done, I will be able to make a true comparison. (omap3_evm
>     >     has a 3730 variant also)
>     >
>     >     Derald
>     >
>     >
>     >
>     > ​The issue appears to have been related to the use of
>     > CONFIG_SPL_SYS_MALLOC_SIMPLE and CONFIG_SYS_MALLOC_F_LEN in the
>     default
>     > config. The RFC patch series is given below:
>     >
>     > https://lists.denx.de/pipermail/u-boot/2018-February/320152.html
>     <https://lists.denx.de/pipermail/u-boot/2018-February/320152.html>
>     >
>     > ​The BeagleBoard config never attempted to use
>     CONFIG_SPL_SYS_MALLOC_SIMPLE.
> 
>     So you're saying it's probably just running out of memory?
> 
> 
>     Alex
> 
> 
> The configuration mix was the key difference. The patch series has the
> details. The default malloc length set by configuration is actually
> smaller. So there are likely a mix of factors. 

Ah, so you're saying with your changes you can reproduce the problem on
the beagleboard?

I still fail to see the exact connection between the serial bug and the
patch set :)


Alex
diff mbox series

Patch

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 30a6f6dc53..a423aa9629 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -550,6 +550,7 @@  config ARCH_BCM283X
 	select DM_GPIO
 	select OF_CONTROL
 	select PL01X_SERIAL
+	select SERIAL_SEARCH_ALL
 	imply FAT_WRITE
 
 config TARGET_VEXPRESS_CA15_TC2
diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
index 3ffedba525..93e602e0ee 100644
--- a/drivers/serial/Kconfig
+++ b/drivers/serial/Kconfig
@@ -79,6 +79,18 @@  config SERIAL_RX_BUFFER_SIZE
 	help
 	  The size of the RX buffer (needs to be power of 2)
 
+config SERIAL_SEARCH_ALL
+	bool "Search for serial devices after default one failed"
+	depends on DM_SERIAL
+	help
+	  The serial subsystem only searches for a single serial device
+	  that was instantiated, but does not check whether it was probed
+	  correctly. With this option set, we make successful probing
+	  mandatory and search for fallback serial devices if the default
+	  device does not work.
+
+	  If unsure, say N.
+
 config SPL_DM_SERIAL
 	bool "Enable Driver Model for serial drivers in SPL"
 	depends on DM_SERIAL
diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
index 68ca2d09d1..9891c20656 100644
--- a/drivers/serial/serial-uclass.c
+++ b/drivers/serial/serial-uclass.c
@@ -74,7 +74,9 @@  static void serial_find_console_or_panic(void)
 {
 	const void *blob = gd->fdt_blob;
 	struct udevice *dev;
+#ifdef CONFIG_SERIAL_SEARCH_ALL
 	int ret;
+#endif
 
 	if (CONFIG_IS_ENABLED(OF_PLATDATA)) {
 		uclass_first_device(UCLASS_SERIAL, &dev);
@@ -113,6 +115,8 @@  static void serial_find_console_or_panic(void)
 #else
 #define INDEX 0
 #endif
+
+#ifdef CONFIG_SERIAL_SEARCH_ALL
 		if (!uclass_get_device_by_seq(UCLASS_SERIAL, INDEX, &dev) ||
 		    !uclass_get_device(UCLASS_SERIAL, INDEX, &dev)) {
 			if (dev->flags & DM_FLAG_ACTIVATED) {
@@ -131,6 +135,15 @@  static void serial_find_console_or_panic(void)
 				return;
 			}
 		}
+#else
+		if (!uclass_get_device_by_seq(UCLASS_SERIAL, INDEX, &dev) ||
+		    !uclass_get_device(UCLASS_SERIAL, INDEX, &dev) ||
+		    (!uclass_first_device(UCLASS_SERIAL, &dev) && dev)) {
+			gd->cur_serial_dev = dev;
+			return;
+		}
+#endif
+
 #undef INDEX
 	}