Re: [U-Boot] [PATCH 5/6] apalis_imx6: Avoid calling setup_display() from SPL code

2017-09-25 Thread Stefano Babic 
On 24/09/2017 13:27, Max Krummenacher wrote:
> Hi
> 
> 
> I propose to drop my patches in favor of  Fabio's solution.
> 
> 

Agree, I will merge them.

Regards,
Stefano

> Reviewed-by: Max Krummenacher 
> 
> 
> Max
> 
> 
> *Von:* Anatolij Gustschin 
> *Gesendet:* Samstag, 23. September 2017 21:06:57
> *An:* Stefano Babic
> *Cc:* Fabio Estevam; u-boot@lists.denx.de; Max Krummenacher; Fabio Estevam
> *Betreff:* Re: [PATCH 5/6] apalis_imx6: Avoid calling setup_display()
> from SPL code
>  
> Hi Stefano,
> 
> On Sat, 23 Sep 2017 10:43:55 +0200
> Stefano Babic sba...@denx.de wrote:
> ...
>> Max has already fixed apalis / colibri, see for example
>> http://patchwork.ozlabs.org/patch/817053/. I will apply Max's patches
>> and drop 5/6 from your series. Anyway, thanks for having thought to
>> these two boards, too.
> 
> I would prefer patch from Fabio, so I applied all them to a build test
> branch and building with [1] passed now. There is a pull request for them.
> 
> Thanks,
> Anatolij
> 
> [1] http://patchwork.ozlabs.org/patch/806755


-- 
=
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 5/6] apalis_imx6: Avoid calling setup_display() from SPL code

2017-09-25 Thread Stefano Babic 
Hi Fabio,

On 23/09/2017 17:11, Fabio Estevam wrote:
> On Sat, Sep 23, 2017 at 10:01 AM, Fabio Estevam  wrote:
>> Hi Stefano,
>>
>> On Sat, Sep 23, 2017 at 5:43 AM, Stefano Babic  wrote:
>>
>>> Max has already fixed apalis / colibri, see for example
>>> http://patchwork.ozlabs.org/patch/817053/. I will apply Max's patches
>>> and drop 5/6 from your series. Anyway, thanks for having thought to
>>> these two boards, too.
>>
>> Yes, I saw Max's patches too. When I tried the same approach on
>> cgtqmx6eval I got build failures:
>>
>> board/congatec/cgtqmx6eval/built-in.o: In function `setup_display':
>> /home/fabio/u-boot-imx/board/congatec/cgtqmx6eval/cgtqmx6eval.c:632:
>> undefined reference to `enable_ipu_clock'
>> /home/fabio/u-boot-imx/board/congatec/cgtqmx6eval/cgtqmx6eval.c:633:
>> undefined reference to `imx_setup_hdmi'
>>
>> It is true that Max's patches fix the warnings for Toradex boards, but
>> if in future someone rework the IPU code like it is done in
>> cgtqmx6eval, then failure would happen.
>>
>> So I thought that a more robust solution was to simply prevent making
>> explicit calls to setup_display() from SPL like I did here.
>>
>> This also has the benefit to have the same solution across several mx6 
>> boards.
> 
> One more advantage of my proposal is that it does not rely on adding
> logic to include/configs/.

You're right, I agree.

> 
> This is particularly important when someone moves the IPU related
> config symbols into Kconfig.
> 

+1

Best regards,
Stefano



-- 
=
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 5/6] apalis_imx6: Avoid calling setup_display() from SPL code

2017-09-24 Thread Max Krummenacher 
Hi


I propose to drop my patches in favor of  Fabio's solution.


Reviewed-by: Max Krummenacher 


Max


Von: Anatolij Gustschin 
Gesendet: Samstag, 23. September 2017 21:06:57
An: Stefano Babic
Cc: Fabio Estevam; u-boot@lists.denx.de; Max Krummenacher; Fabio Estevam
Betreff: Re: [PATCH 5/6] apalis_imx6: Avoid calling setup_display() from SPL 
code

Hi Stefano,

On Sat, 23 Sep 2017 10:43:55 +0200
Stefano Babic sba...@denx.de wrote:
...
> Max has already fixed apalis / colibri, see for example
> http://patchwork.ozlabs.org/patch/817053/. I will apply Max's patches
> and drop 5/6 from your series. Anyway, thanks for having thought to
> these two boards, too.

I would prefer patch from Fabio, so I applied all them to a build test
branch and building with [1] passed now. There is a pull request for them.

Thanks,
Anatolij

[1] http://patchwork.ozlabs.org/patch/806755
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 5/6] apalis_imx6: Avoid calling setup_display() from SPL code

2017-09-23 Thread Anatolij Gustschin 
Hi Stefano,

On Sat, 23 Sep 2017 10:43:55 +0200
Stefano Babic sba...@denx.de wrote:
... 
> Max has already fixed apalis / colibri, see for example
> http://patchwork.ozlabs.org/patch/817053/. I will apply Max's patches
> and drop 5/6 from your series. Anyway, thanks for having thought to
> these two boards, too.

I would prefer patch from Fabio, so I applied all them to a build test
branch and building with [1] passed now. There is a pull request for them.

Thanks,
Anatolij

[1] http://patchwork.ozlabs.org/patch/806755
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 5/6] apalis_imx6: Avoid calling setup_display() from SPL code

2017-09-23 Thread Fabio Estevam 
On Sat, Sep 23, 2017 at 10:01 AM, Fabio Estevam  wrote:
> Hi Stefano,
>
> On Sat, Sep 23, 2017 at 5:43 AM, Stefano Babic  wrote:
>
>> Max has already fixed apalis / colibri, see for example
>> http://patchwork.ozlabs.org/patch/817053/. I will apply Max's patches
>> and drop 5/6 from your series. Anyway, thanks for having thought to
>> these two boards, too.
>
> Yes, I saw Max's patches too. When I tried the same approach on
> cgtqmx6eval I got build failures:
>
> board/congatec/cgtqmx6eval/built-in.o: In function `setup_display':
> /home/fabio/u-boot-imx/board/congatec/cgtqmx6eval/cgtqmx6eval.c:632:
> undefined reference to `enable_ipu_clock'
> /home/fabio/u-boot-imx/board/congatec/cgtqmx6eval/cgtqmx6eval.c:633:
> undefined reference to `imx_setup_hdmi'
>
> It is true that Max's patches fix the warnings for Toradex boards, but
> if in future someone rework the IPU code like it is done in
> cgtqmx6eval, then failure would happen.
>
> So I thought that a more robust solution was to simply prevent making
> explicit calls to setup_display() from SPL like I did here.
>
> This also has the benefit to have the same solution across several mx6 boards.

One more advantage of my proposal is that it does not rely on adding
logic to include/configs/.

This is particularly important when someone moves the IPU related
config symbols into Kconfig.

Regards,

Fabio Estevam
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 5/6] apalis_imx6: Avoid calling setup_display() from SPL code

2017-09-23 Thread Fabio Estevam 
Hi Stefano,

On Sat, Sep 23, 2017 at 5:43 AM, Stefano Babic  wrote:

> Max has already fixed apalis / colibri, see for example
> http://patchwork.ozlabs.org/patch/817053/. I will apply Max's patches
> and drop 5/6 from your series. Anyway, thanks for having thought to
> these two boards, too.

Yes, I saw Max's patches too. When I tried the same approach on
cgtqmx6eval I got build failures:

board/congatec/cgtqmx6eval/built-in.o: In function `setup_display':
/home/fabio/u-boot-imx/board/congatec/cgtqmx6eval/cgtqmx6eval.c:632:
undefined reference to `enable_ipu_clock'
/home/fabio/u-boot-imx/board/congatec/cgtqmx6eval/cgtqmx6eval.c:633:
undefined reference to `imx_setup_hdmi'

It is true that Max's patches fix the warnings for Toradex boards, but
if in future someone rework the IPU code like it is done in
cgtqmx6eval, then failure would happen.

So I thought that a more robust solution was to simply prevent making
explicit calls to setup_display() from SPL like I did here.

This also has the benefit to have the same solution across several mx6 boards.

I am OK with Max's patches, but just wanted to provide a rationale for
my proposal.

Thanks,

Fabio Estevam
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 5/6] apalis_imx6: Avoid calling setup_display() from SPL code

2017-09-23 Thread Stefano Babic 
Hi Fabio,

On 23/09/2017 04:45, Fabio Estevam wrote:
> From: Fabio Estevam 
> 
> There is no need call setup_display() from SPL code, so move it to
> board_init(), which executes only in U-Boot proper.
> 
> Reported-by: Stefano Babic 
> Signed-off-by: Fabio Estevam 
> ---
>  board/toradex/apalis_imx6/apalis_imx6.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/board/toradex/apalis_imx6/apalis_imx6.c 
> b/board/toradex/apalis_imx6/apalis_imx6.c
> index 628a61d..b86dde8 100644
> --- a/board/toradex/apalis_imx6/apalis_imx6.c
> +++ b/board/toradex/apalis_imx6/apalis_imx6.c
> @@ -756,10 +756,6 @@ int board_early_init_f(void)
>  #else
>   setup_iomux_dce_uart();
>  #endif
> -
> -#if defined(CONFIG_VIDEO_IPUV3)
> - setup_display();
> -#endif
>   return 0;
>  }
>  
> @@ -781,6 +777,10 @@ int board_init(void)
>   setup_i2c(1, CONFIG_SYS_I2C_SPEED, 0x7f, _pad_info_loc);
>   setup_i2c(2, CONFIG_SYS_I2C_SPEED, 0x7f, _pad_info3);
>  
> +#if defined(CONFIG_VIDEO_IPUV3)
> + setup_display();
> +#endif
> +
>  #ifdef CONFIG_TDX_CMD_IMX_MFGR
>   (void) pmic_init();
>  #endif
> 

Max has already fixed apalis / colibri, see for example
http://patchwork.ozlabs.org/patch/817053/. I will apply Max's patches
and drop 5/6 from your series. Anyway, thanks for having thought to
these two boards, too.

Regards,
Stefano

-- 
=
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH 5/6] apalis_imx6: Avoid calling setup_display() from SPL code

2017-09-22 Thread Fabio Estevam 
From: Fabio Estevam 

There is no need call setup_display() from SPL code, so move it to
board_init(), which executes only in U-Boot proper.

Reported-by: Stefano Babic 
Signed-off-by: Fabio Estevam 
---
 board/toradex/apalis_imx6/apalis_imx6.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/board/toradex/apalis_imx6/apalis_imx6.c 
b/board/toradex/apalis_imx6/apalis_imx6.c
index 628a61d..b86dde8 100644
--- a/board/toradex/apalis_imx6/apalis_imx6.c
+++ b/board/toradex/apalis_imx6/apalis_imx6.c
@@ -756,10 +756,6 @@ int board_early_init_f(void)
 #else
setup_iomux_dce_uart();
 #endif
-
-#if defined(CONFIG_VIDEO_IPUV3)
-   setup_display();
-#endif
return 0;
 }
 
@@ -781,6 +777,10 @@ int board_init(void)
setup_i2c(1, CONFIG_SYS_I2C_SPEED, 0x7f, _pad_info_loc);
setup_i2c(2, CONFIG_SYS_I2C_SPEED, 0x7f, _pad_info3);
 
+#if defined(CONFIG_VIDEO_IPUV3)
+   setup_display();
+#endif
+
 #ifdef CONFIG_TDX_CMD_IMX_MFGR
(void) pmic_init();
 #endif
-- 
2.7.4

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot