From: Nikolay Dimitrov <picmaster+prvs=1343ce5590@mail.bg>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH V3] ARM: mx6: Add support for Kosagi Novena
Date: Tue, 23 Sep 2014 00:38:19 +0300 [thread overview]
Message-ID: <542096CB.8020209@mail.bg> (raw)
In-Reply-To: <1411307086-12105-1-git-send-email-marex@denx.de>
Hi Marek,
I checked the I2C stuff, didn't saw any major issues. Here are a couple
of comments for possible improvements:
On 09/21/2014 04:44 PM, Marek Vasut wrote:
> +/*
> + * I2C
> + */
> +/* I2C1, RAM */
> +struct i2c_pads_info i2c_pad_info0 = {
I can suggest to reword the comment as: "I2C1: DDR3 SO-DIMM RAM" or
something similar, to avoid confusion with I2C RAM.
> +/* I2C2 Camera, MIPI */
> +static struct i2c_pads_info i2c_pad_info1 = {
I didn't found any MIPI camera on the schematic, so I think the comment
is misleading. The connected devices on I2C2 are PMIC, HDMI DDC, FPGA,
and only the PMIC could be useful for booting. So I can suggest to
reword the comment like: "I2C2: PMIC".
> +/* I2C3, J15 - RGB connector */
> +static struct i2c_pads_info i2c_pad_info2 = {
I didn't found such connect J15 on my schematic (novena_pvt2.pdf).
There's a connector JP10L, which is for the LCD panel. Again the comment
could be reworded as: "I2C3: Utility EEPROM, LCD panel"
> +static void novena_spl_setup_iomux_i2c(void)
> +{
> + setup_i2c(0, CONFIG_SYS_I2C_SPEED, 0x7f, &i2c_pad_info0);
> + setup_i2c(1, CONFIG_SYS_I2C_SPEED, 0x7f, &i2c_pad_info1);
> + setup_i2c(2, CONFIG_SYS_I2C_SPEED, 0x7f, &i2c_pad_info2);
> +}
Is the I2C1 bus used anywhere? I found only usage of I2C2 for PMIC and
I2C3 for Utility EEPROM reading.
> +/* I2C EEPROM */
> +#ifdef CONFIG_CMD_EEPROM
> +#define CONFIG_SYS_I2C_MULTI_EEPROMS
> +#define CONFIG_SYS_I2C_EEPROM_ADDR_LEN 2
> +#define CONFIG_SYS_SPD_BUS_NUM 2
> +#endif
I'm puzzled by the value of CONFIG_SYS_SPD_BUS_NUM - the SPD MEM is
connected to I2C1, which should be bus 0, right?
These are only recommendations, please feel free to accept/reject them
as you wish. The only reason why I suggest them is because usually the
U-Boot source code is excellent documentation about the hardware boards,
and any effort spent to clarify the peripherals and their usage will be
greatly appreciated later by the U-Boot users.
Thanks again for your serious efforts to kick-off the Novena board support.
Kind regards,
Nikolay
next prev parent reply other threads:[~2014-09-22 21:38 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-09-21 13:44 [U-Boot] [PATCH V3] ARM: mx6: Add support for Kosagi Novena Marek Vasut
2014-09-22 13:44 ` Stefano Babic
2014-09-22 18:24 ` Nikolay Dimitrov
2014-09-23 9:47 ` Marek Vasut
2014-09-23 23:42 ` Nikolay Dimitrov
2014-09-24 5:48 ` Tim Harvey
2014-09-24 10:09 ` Nikolay Dimitrov
2014-09-22 21:38 ` Nikolay Dimitrov [this message]
2014-09-23 10:14 ` Marek Vasut
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=542096CB.8020209@mail.bg \
--to=picmaster+prvs=1343ce5590@mail.bg \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.