Hi Stefano!

On 10/22/14 10:36, Stefano Babic wrote:
Hi Soeren,

On 21/10/2014 19:54, Soeren Moch wrote:

This is also copied from sabresd. Can we factorize in some way ?

I can, and probably should, simplify this code. But in fact this code
is wrong, and in the same way for many imx6q boards (e.g. sabresd,
wandboard, nitrogen6x,...).

That makes things only worse. Duplicating the code, we have an
additional board with bad / wrong code.


This code configures the external video clock (LDB_DIx clock). This
clock is hardcoded to have 65MHz in drivers/video/ipu_common.c:ldb_clk.
But in fact the clock rate is configured to 75.42MHz (528MHz/7) on all
boards. So the display does not show 1024x768@60Hz as configured, but
something similar to 1024x768@70Hz (not VESA compliant), which much
monitors can handle, but on other monitors there are problems.
(one out of tree monitors works for me).

Ok, understood.


My intention was to get this initial support for tbs2910 merged with the
(wrong) code from sabresd used as template, and later to discuss how to
cleanup this code.

Do you prefer a simplified version of this code for the initial patch?

Maybe this is the best approach: you can at the beginning drop support
for video, an let your board be merged without wrong code. Then we can
discuss about fixing the wrong clock as shared code, letting that all
boards take advantage for that.

U-Boot without video support is useless for me. So I will try to
setup the video clock correctly in a new version of this patch.


+#endif /* CONFIG_VIDEO_IPUV3 */
+
+int board_eth_init(bd_t *bis)
+{
+    setup_iomux_enet();
+    setup_pcie();
+
+    return cpu_eth_init(bis);
+}
+
+int board_early_init_f(void)
+{
+    setup_iomux_uart();
+#ifdef CONFIG_VIDEO_IPUV3
+    setup_display();

I do not understand why setup_display() should be called at this point.
Generally, board_early_init_f() is called to setup iomux for peripherals
needed before relocation, as uart, letting the rest of the setup in
board_init(). Why do you need here ?

In fact this was also copied from sabresd/wandboard/nitrogen6x.
My assumption was, that the clocks must be configured before the
ipu is initialized.

That is correct, but ipu is initialized by video_init() after
board_init() is called. Generally, board_early_init() is responsible for
setup some initial peripherals, for example the iomux for uart or for
RAM controller. The common initialization is then put into board_init().
I am expecting that you have no issues by moving setup_display() in the
board_init() function.


You are right here, there is no problem with setup_display() in board_init().

So I wonder why all imx6q boards have setup_display() in board_early_init(),
you even committed such code for mx6qsabreauto yesterday.

Best regards,
Soeren Moch
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to