| Message ID | 1389797898-10254-1-git-send-email-clsee@altera.com |
|---|---|
| State | Superseded |
| Headers | show |
On Wed, 2014-01-15 at 08:58 -0600, Chin Liang See wrote: > Enhance the DesignWare I2C driver to support address length more > than 1 byte. This enhancement is required as some I2C slave > device such as EEPROM chip might have 16 bit address byte. > diff --git a/drivers/i2c/designware_i2c.c b/drivers/i2c/designware_i2c.c > index cb2ac04..c0ac5f7 100644 > --- a/drivers/i2c/designware_i2c.c > +++ b/drivers/i2c/designware_i2c.c > @@ -205,27 +205,21 @@ static int check_params(uint addr, int alen, uchar *buffer, int len) > return 1; > } > > - if (alen > 1) { > - printf("addr len %d not supported\n", alen); > - return 1; > - } > - > - if (addr + len > 256) { > - printf("address out of range\n"); > - return 1; > - } > - > return 0; > } Hi Chin, if you strip down functionality of "check_params()" to one single check I would recommend you to remove "check_params()" at all and do in-place check for "buffer" existence. Moreover you may just use "assert" for this check because this buffer is passed by u-boot (no need to check every parameter passed to any function in run-time) so in production/release build it won't exist at all. Regards, Alexey
Hi Alexey, On Wed, 2014-01-15 at 15:30 +0000, Alexey Brodkin wrote: > On Wed, 2014-01-15 at 08:58 -0600, Chin Liang See wrote: > > Enhance the DesignWare I2C driver to support address length more > > than 1 byte. This enhancement is required as some I2C slave > > device such as EEPROM chip might have 16 bit address byte. > > diff --git a/drivers/i2c/designware_i2c.c b/drivers/i2c/designware_i2c.c > > index cb2ac04..c0ac5f7 100644 > > --- a/drivers/i2c/designware_i2c.c > > +++ b/drivers/i2c/designware_i2c.c > > @@ -205,27 +205,21 @@ static int check_params(uint addr, int alen, uchar *buffer, int len) > > return 1; > > } > > > > - if (alen > 1) { > > - printf("addr len %d not supported\n", alen); > > - return 1; > > - } > > - > > - if (addr + len > 256) { > > - printf("address out of range\n"); > > - return 1; > > - } > > - > > return 0; > > } > > Hi Chin, > > if you strip down functionality of "check_params()" to one single check > I would recommend you to remove "check_params()" at all and do in-place > check for "buffer" existence. > > Moreover you may just use "assert" for this check because this buffer is > passed by u-boot (no need to check every parameter passed to any > function in run-time) so in production/release build it won't exist at > all. Good suggestion. I agreed that we can strip off the check_params. Let me send the v2 patch. Thanks and have a nice day! Chin Liang > > Regards, > Alexey > >
diff --git a/drivers/i2c/designware_i2c.c b/drivers/i2c/designware_i2c.c index cb2ac04..c0ac5f7 100644 --- a/drivers/i2c/designware_i2c.c +++ b/drivers/i2c/designware_i2c.c @@ -205,27 +205,21 @@ static int check_params(uint addr, int alen, uchar *buffer, int len) return 1; } - if (alen > 1) { - printf("addr len %d not supported\n", alen); - return 1; - } - - if (addr + len > 256) { - printf("address out of range\n"); - return 1; - } - return 0; } -static int i2c_xfer_init(uchar chip, uint addr) +static int i2c_xfer_init(uchar chip, uint addr, int alen) { if (i2c_wait_for_bb()) return 1; i2c_setaddress(chip); - writel(addr, &i2c_regs_p->ic_cmd_data); - + while (alen) { + alen--; + /* high byte address going out first */ + writel((addr >> (alen * 8)) & 0xff, + &i2c_regs_p->ic_cmd_data); + } return 0; } @@ -269,7 +263,7 @@ int i2c_read(uchar chip, uint addr, int alen, uchar *buffer, int len) if (check_params(addr, alen, buffer, len)) return 1; - if (i2c_xfer_init(chip, addr)) + if (i2c_xfer_init(chip, addr, alen)) return 1; start_time_rx = get_timer(0); @@ -310,7 +304,7 @@ int i2c_write(uchar chip, uint addr, int alen, uchar *buffer, int len) if (check_params(addr, alen, buffer, len)) return 1; - if (i2c_xfer_init(chip, addr)) + if (i2c_xfer_init(chip, addr, alen)) return 1; start_time_tx = get_timer(0);
Enhance the DesignWare I2C driver to support address length more than 1 byte. This enhancement is required as some I2C slave device such as EEPROM chip might have 16 bit address byte. Signed-off-by: Chin Liang See <clsee@altera.com> Cc: Alexey Brodkin <Alexey.Brodkin@synopsys.com> Cc: Tom Rini <trini@ti.com> cc: Armando Visconti <armando.visconti@st.com> Cc: Stefan Roese <sr@denx.de> Cc: Albert ARIBAUD <albert.u.boot@aribaud.net> Cc: Heiko Schocher <hs@denx.de> Cc: Vipin KUMAR <vipin.kumar@st.com> Cc: Tom Rix <Tom.Rix@windriver.com> Cc: Mischa Jonker <mjonker@synopsys.com> --- drivers/i2c/designware_i2c.c | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-)