public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
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

  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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox