Hi Markus,
some minor points. I cannot find the original patch in my mailbox, that
is the reason my answer can look weird. ;-)
On 10/07/2014 09:24, Stefano Babic wrote:
> Hi Markus,
>
> On 10/07/2014 09:11, Markus Niebel wrote:
>> Am 23.06.2014 17:56, wrote Markus Niebel:
>>> From: Markus Niebel <markus.nie...@tq-group.com>
>>>
>>> This patch adds the changes to boards.cfg and the board directory
>>> under board/tqc.
>>>
>>> TQMa6 is a family of modules based on Freescale i.MX6. It consists of
>>> TQMa6Q (i.MX6 Quad), TQMa6D (i.MX6 Dual) featuring eMMC, and 1 GiB DDR3
>>> TQMa6S (i.MX6 Solo) featuring eMMC and 512 MiB DDR3
>>>
>>> The modules need a baseboard. Initially the MBa6x starterkit mainboard is
>>> supported. To easy support for other mainboards the functionality is
>>> splitted
>>> in one file for the module (tqma6.c) and one file for the baseboard (tqma6_
>>> mba6).
>>>
>>> The modules can be boot from eMMC (on USDHC3) and SPI flash.
>>>
>>> The following features are supported:
>>> - MMC: eMMC on module (on USDHC3) and SD-card (on MBa6x mainboard)
>>> - Ethernet: RGMII using micrel KSZ9031 phy on MBa6x mainboard for TQMa6<x>
>>> module.
>>> The phy needs special configurations for the pad skew registers to adjust
>>> for
>>> the signal routing.
>>> Also support for standard ethernet commands and uppdate via tftp.
>>> - SPI: ECSPI1 with bootable serial flash on module and two additional
>>> chip selects on MBa6x
>>> - I2C: This patch adds support for the I2C busses on the TQMa6<x> modules
>>> (I2C3)
>>> and MBa6x baseboards (I2C1). The LM75 temperature sensors on TQMa6<x> and
>>> MBa6x
>>> are also configured.
>>> - USB: high speed host 1 on MBa6x and support for USB storage
>>> - PMIC: support for pfuze 100 on TQMa6<x>
>>>
>>> Signed-off-by: Markus Niebel <markus.nie...@tq-group.com>
>>> ---
>>
>> Ping ... any comments or shall I rebase after 2014.07 will be released?
>
> No, please wait...I am reviewing your patch for merging into -next, even
> before 2014.07 is released. If I still see open issues, I will inform you.
>
There is a ton of warning by checkpatch because line exceeds 80 chars.
They should be fixed.
Then:
+ printf("Warning: failed to initialize eMMC dev\n");
Use puts instead of printf for static strings. I think there are also a
couple of other identical cases.
You set the variable "board" in your code. "board" is quite generic and
you use it to set the name. We have already a case in U-Boot doing this
with am335x boards. Use "board_name" (even "board_rev" if required) to
set target's name to be consistent with other boards.
I have not understood why you put board_mmc_getcd() and
board_mmc_getwp() in the module file (tqma6.c). They refer then to board
specific code, for example tqma6_bb_board_mmc_getcd(). Should they not
belong to the baseboard file ?
+ power_pfuze100_init(2);
Understood, but maybe better to have a constant for the bus number
+#if defined(CONFIG_MX6Q)
+#define MBA6X_KSZ9031_CTRL_SKEW 0x0032
+#define MBA6X_KSZ9031_CLK_SKEW 0x03ff
+#define MBA6X_KSZ9031_RX_SKEW 0x3333
+#define MBA6X_KSZ9031_TX_SKEW 0x2036
+#elif defined(CONFIG_MX6S)
+#define MBA6X_KSZ9031_CTRL_SKEW 0x0030
Does the skew depend on SOC type or maybe on the different baseboard ?
Is it correct to make the #ifdef dependig on the SOC ?
There are several new undocumented CONFIG_ in your configuration.
A CONFIG_ should be documented, but it seems you use only locally in
your tqma6.h. Then I will suggest to change their name to better
understand they are not general configuration switches for U-Boot.
Best regards,
Stefano Babic
--
=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
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
http://lists.denx.de/mailman/listinfo/u-boot
