designware_i2c: Enhance DesignWare I2C driver address support

PATCH Superseded Chin Liang See 3 participants 5 messages Jan 15, 2014 – Feb 4, 2014 I2C

Share & tools

Chin Liang See

author

2014-01-15 15:45 UTC #0 lore raw

PATCH v2 1/1 designware_i2c: Enhance DesignWare I2C driver address support

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 <[email protected]>

Cc Alexey Brodkin <[email protected]>

Cc Tom Rini <[email protected]>

cc Armando Visconti <[email protected]>

Cc Stefan Roese <[email protected]>

Cc Albert ARIBAUD <[email protected]>

Cc Heiko Schocher <[email protected]>

Cc Vipin KUMAR <[email protected]>

Cc Mischa Jonker <[email protected]>

---
Changes for v2
- Removed the function check_params()

---
 drivers/i2c/designware_i2c.c |   41 +++++++++--------------------------------
 1 file changed, 9 insertions(+), 32 deletions(-)

drivers/i2c/designware_i2c.c +9 ~~-32~~

index cb2ac04..5b8482a 100644
@@ -197,35 +197,18 @@ static int i2c_wait_for_bb(void)
 	return 0;
 }
 
-/* check parameters for i2c_read and i2c_write */
-static int check_params(uint addr, int alen, uchar *buffer, int len)
-{
-	if (buffer == NULL) {
-		printf("Buffer is invalid\n");
-		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;
 }

@@ -266,10 +249,7 @@ int i2c_read(uchar chip, uint addr, int alen, uchar *buffer, int len)
 {
 	unsigned long start_time_rx;
 
-	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);

@@ -307,10 +287,7 @@ int i2c_write(uchar chip, uint addr, int alen, uchar *buffer, int len)
 	int nb = len;
 	unsigned long start_time_tx;
 
-	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);

Signature · 1 line

1.7.9.5

Alexey Brodkin

2014-01-15 15:51 UTC #1 lore raw

PATCH v2 1/1 designware_i2c: Enhance DesignWare I2C driver address support

Quoted 2 lines · Chin Liang See

Changes for v2
- Removed the function check_params()

Ok, so you decided to not add "assert" check instead.
I think it's ok - it's not a requirement. Others don't do it as well so let's leave it as it is.

Acked-by Alexey Brodkin <[email protected]>

Regards,
Alexey

Chin Liang See

author

2014-01-22 15:37 UTC #2 lore raw

PATCH v2 1/1 designware_i2c: Enhance DesignWare I2C driver address support

Thanks Alexey.

Hi Heiko,

I believe this patch should be good for apply. Would need your help then. :) Thanks

Chin Liang

Quoted 13 lines · Alexey Brodkin

Quoted 2 lines · Chin Liang See
Changes for v2
- Removed the function check_params()
Ok, so you decided to not add "assert" check instead.
I think it's ok - it's not a requirement. Others don't do it as well so let's leave it as it is.
Acked-by Alexey Brodkin <[email protected]>
Regards,
Alexey

Heiko Schocher

2014-01-30 05:20 UTC #3 lore raw

PATCH v2 1/1 designware_i2c: Enhance DesignWare I2C driver address support

Hello Chin,

Am 22.01.2014 16:37, schrieb Chin Liang See:

Quoted 4 lines

Thanks Alexey.
Hi Heiko,

Quoted 19 lines

I believe this patch should be good for apply. Would need your help then. :) Thanks
Chin Liang
Quoted 12 lines · Alexey Brodkin
Quoted 2 lines · Chin Liang See
Changes for v2
- Removed the function check_params()
Ok, so you decided to not add "assert" check instead.
I think it's ok - it's not a requirement. Others don't do it as well so let's leave it as it is.
Acked-by Alexey Brodkin<[email protected]>
Regards,
Alexey

Your patch apply not clean to current head f889cc81c1572f4af0be950fd49bb6b67bc580fb also checkpatch drops one warning:

CHECK: Alignment should match open parenthesis
#82: FILE: drivers/i2c/designware_i2c.c:210:
+ writel((addr >> (alen * 8)) & 0xff,
+ &i2c_regs_p->ic_cmd_data);

total: 0 errors, 0 warnings, 1 checks, 64 lines checked

Could you please fix this, thanks!

bye,
Heiko

Signature · 2 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Chin Liang See

author

2014-02-04 17:51 UTC #4 lore raw

PATCH v2 1/1 designware_i2c: Enhance DesignWare I2C driver address support

Hi Heiko,

Quoted 22 lines · Heiko Schocher

Hello Chin,
Am 22.01.2014 16:37, schrieb Chin Liang See:
Quoted 19 lines
Thanks Alexey.
Hi Heiko,
I believe this patch should be good for apply. Would need your help then. :) Thanks
Chin Liang
Quoted 8 lines · Alexey Brodkin
Quoted 2 lines · Chin Liang See
Changes for v2
- Removed the function check_params()
Ok, so you decided to not add "assert" check instead.
I think it's ok - it's not a requirement. Others don't do it as well so let's leave it as it is.

Quoted 8 lines

Quoted 4 lines
Quoted 4 lines
Acked-by Alexey Brodkin<[email protected]>
Regards,
Alexey
Your patch apply not clean to current head f889cc81c1572f4af0be950fd49bb6b67bc580fb also checkpatch drops one warning:

Yup, it caused by a later patch. Let me re-base my patch.

Quoted 6 lines

CHECK: Alignment should match open parenthesis
#82: FILE: drivers/i2c/designware_i2c.c:210:
drivers/i2c/designware_i2c.c +2 -0
+               writel((addr >> (alen * 8)) & 0xff,
+                       &i2c_regs_p->ic_cmd_data);
total: 0 errors, 0 warnings, 1 checks, 64 lines checked

I overlooked this as I saw no errors. Let me fix that too. Thanks again for your helps.

Chin Liang

Quoted 5 lines

Could you please fix this, thanks!
bye,
Heiko