Patchwork [U-Boot,V2,04/11] I2C: mxc_i2c: get rid of __REG access

login
register
mail settings
Submitter Stefano Babic
Date Jan. 20, 2011, 8:46 a.m.
Message ID <1295513194-16158-5-git-send-email-sbabic@denx.de>
Download mbox | patch
Permalink /patch/79652/
State Superseded
Delegated to: Stefano Babic
Headers show

Comments

Stefano Babic - Jan. 20, 2011, 8:46 a.m.
This driver accesses to processor's register
via __REG macros, that are removed (or are planned
to be removed) and replaced by C structures.
This patches replaces all occurrencies of __REG macros.

Signed-off-by: Stefano Babic <sbabic@denx.de>
CC: Heiko Schocher <hs@denx.de>
---
 drivers/i2c/mxc_i2c.c |   68 ++++++++++++++++++++++++++++++------------------
 1 files changed, 42 insertions(+), 26 deletions(-)
Wolfgang Denk - Jan. 20, 2011, 9:27 a.m.
Dear Stefano Babic,

In message <1295513194-16158-5-git-send-email-sbabic@denx.de> you wrote:
> This driver accesses to processor's register
> via __REG macros, that are removed (or are planned
> to be removed) and replaced by C structures.
> This patches replaces all occurrencies of __REG macros.

Checkpatch says:

	total: 0 errors, 2 warnings, 169 lines checked

Please have a look.

...
> @@ -204,7 +219,8 @@ int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
>  int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len)
>  {
>  	int timeout = 10000;
> -	DPRINTF("%s chip: 0x%02x addr: 0x%04x alen: %d len: %d\n",__FUNCTION__, chip, addr, alen, len);
> +	DPRINTF("%s chip: 0x%02x addr: 0x%04x alen: %d len: %d\n",
> +		__FUNCTION__, chip, addr, alen, len);

While touching this anyway, can we please convert the DPRINTF() into
debug() ?

Best regards,

Wolfgang Denk
Stefano Babic - Jan. 20, 2011, 10:23 a.m.
On 01/20/2011 10:27 AM, Wolfgang Denk wrote:
> Dear Stefano Babic,
> 
> In message <1295513194-16158-5-git-send-email-sbabic@denx.de> you wrote:
>> This driver accesses to processor's register
>> via __REG macros, that are removed (or are planned
>> to be removed) and replaced by C structures.
>> This patches replaces all occurrencies of __REG macros.
> 
> Checkpatch says:
> 
> 	total: 0 errors, 2 warnings, 169 lines checked

I have seen, but because __FUNCTION__is generally used, I have not
replaced the occurrencies with the suggested __func__. At least, not in
this patch. I think it is should be done in a separate patch (I will do
 in next post).

>> @@ -204,7 +219,8 @@ int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
>>  int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len)
>>  {
>>  	int timeout = 10000;
>> -	DPRINTF("%s chip: 0x%02x addr: 0x%04x alen: %d len: %d\n",__FUNCTION__, chip, addr, alen, len);
>> +	DPRINTF("%s chip: 0x%02x addr: 0x%04x alen: %d len: %d\n",
>> +		__FUNCTION__, chip, addr, alen, len);
> 
> While touching this anyway, can we please convert the DPRINTF() into
> debug() ?

You missed the following patch that removes the ugly private debug
functions..

Best regards,
Stefano Babic

Patch

diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c
index 7f669ff..eead502 100755
--- a/drivers/i2c/mxc_i2c.c
+++ b/drivers/i2c/mxc_i2c.c
@@ -23,6 +23,7 @@ 
  */
 
 #include <common.h>
+#include <asm/io.h>
 
 #if defined(CONFIG_HARD_I2C)
 
@@ -81,15 +82,26 @@  static u16 div[] = { 30, 32, 36, 42, 48, 52, 60, 72, 80, 88, 104, 128, 144,
 	             160, 192, 240, 288, 320, 384, 480, 576, 640, 768, 960,
 	             1152, 1280, 1536, 1920, 2304, 2560, 3072, 3840};
 
+static inline void i2c_reset(void)
+{
+	writew(0, I2C_BASE + I2CR);	/* Reset module */
+	writew(0, I2C_BASE + I2SR);
+	writew(I2CR_IEN, I2C_BASE + I2CR);
+}
+
 void i2c_init(int speed, int unused)
 {
 	int freq;
 	int i;
 
 #if defined(CONFIG_MX31)
+	struct clock_control_regs *sc_regs =
+		(struct clock_control_regs *)CCM_BASE;
+
 	freq = mx31_get_ipg_clk();
 	/* start the required I2C clock */
-	__REG(CCM_CGR0) = __REG(CCM_CGR0) | (3 << I2C_CLK_OFFSET);
+	writel(readl(&sc_regs->cgr0) | (3 << I2C_CLK_OFFSET),
+		&sc_regs->cgr0);
 #else
 	freq = mxc_get_clock(MXC_IPG_PERCLK);
 #endif
@@ -100,28 +112,26 @@  void i2c_init(int speed, int unused)
 
 	DPRINTF("%s: speed: %d\n",__FUNCTION__, speed);
 
-	__REG16(I2C_BASE + I2CR) = 0; /* Reset module */
-	__REG16(I2C_BASE + IFDR) = i;
-	__REG16(I2C_BASE + I2CR) = I2CR_IEN;
-	__REG16(I2C_BASE + I2SR) = 0;
+	writew(i, I2C_BASE + IFDR);
+	i2c_reset();
 }
 
 static int wait_busy(void)
 {
 	int timeout = 10000;
 
-	while (!(__REG16(I2C_BASE + I2SR) & I2SR_IIF) && --timeout)
+	while (!(readw(I2C_BASE + I2SR) & I2SR_IIF) && --timeout)
 		udelay(1);
-	__REG16(I2C_BASE + I2SR) = 0; /* clear interrupt */
+	writew(0, I2C_BASE + I2SR); /* clear interrupt */
 
 	return timeout;
 }
 
 static int tx_byte(u8 byte)
 {
-	__REG16(I2C_BASE + I2DR) = byte;
+	writew(byte, I2C_BASE + I2DR);
 
-	if (!wait_busy() || __REG16(I2C_BASE + I2SR) & I2SR_RX_NO_AK)
+	if (!wait_busy() || readw(I2C_BASE + I2SR) & I2SR_RX_NO_AK)
 		return -1;
 	return 0;
 }
@@ -131,27 +141,27 @@  static int rx_byte(void)
 	if (!wait_busy())
 		return -1;
 
-	return __REG16(I2C_BASE + I2DR);
+	return readw(I2C_BASE + I2DR);
 }
 
 int i2c_probe(uchar chip)
 {
 	int ret;
 
-	__REG16(I2C_BASE + I2CR) = 0; /* Reset module */
-	__REG16(I2C_BASE + I2CR) = I2CR_IEN;
+	writew(0, I2C_BASE + I2CR); /* Reset module */
+	writew(I2CR_IEN, I2C_BASE + I2CR);
 
-	__REG16(I2C_BASE + I2CR) = I2CR_IEN |  I2CR_MSTA | I2CR_MTX;
+	writew(I2CR_IEN |  I2CR_MSTA | I2CR_MTX, I2C_BASE + I2CR);
 	ret = tx_byte(chip << 1);
-	__REG16(I2C_BASE + I2CR) = I2CR_IEN | I2CR_MTX;
+	writew(I2CR_IEN | I2CR_MTX, I2C_BASE + I2CR);
 
 	return ret;
 }
 
 static int i2c_addr(uchar chip, uint addr, int alen)
 {
-	__REG16(I2C_BASE + I2SR) = 0; /* clear interrupt */
-	__REG16(I2C_BASE + I2CR) = I2CR_IEN |  I2CR_MSTA | I2CR_MTX;
+	writew(0, I2C_BASE + I2SR);
+	writew(I2CR_IEN |  I2CR_MSTA | I2CR_MTX, I2C_BASE + I2CR);
 
 	if (tx_byte(chip << 1))
 		return -1;
@@ -167,35 +177,40 @@  int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
 	int timeout = 10000;
 	int ret;
 
-	DPRINTF("%s chip: 0x%02x addr: 0x%04x alen: %d len: %d\n",__FUNCTION__, chip, addr, alen, len);
+	DPRINTF("%s chip: 0x%02x addr: 0x%04x alen: %d len: %d\n",
+		__FUNCTION__, chip, addr, alen, len);
 
 	if (i2c_addr(chip, addr, alen)) {
 		printf("i2c_addr failed\n");
 		return -1;
 	}
 
-	__REG16(I2C_BASE + I2CR) = I2CR_IEN |  I2CR_MSTA | I2CR_MTX | I2CR_RSTA;
+	writew(I2CR_IEN | I2CR_MSTA | I2CR_MTX | I2CR_RSTA, I2C_BASE + I2CR);
 
 	if (tx_byte(chip << 1 | 1))
 		return -1;
 
-	__REG16(I2C_BASE + I2CR) = I2CR_IEN |  I2CR_MSTA | ((len == 1) ? I2CR_TX_NO_AK : 0);
+	writew(I2CR_IEN | I2CR_MSTA |
+		((len == 1) ? I2CR_TX_NO_AK : 0),
+		I2C_BASE + I2CR);
 
-	ret = __REG16(I2C_BASE + I2DR);
+	ret = readw(I2C_BASE + I2DR);
 
 	while (len--) {
 		if ((ret = rx_byte()) < 0)
 			return -1;
 		*buf++ = ret;
 		if (len <= 1)
-			__REG16(I2C_BASE + I2CR) = I2CR_IEN |  I2CR_MSTA | I2CR_TX_NO_AK;
+			writew(I2CR_IEN | I2CR_MSTA |
+				I2CR_TX_NO_AK,
+				I2C_BASE + I2CR);
 	}
 
 	wait_busy();
 
-	__REG16(I2C_BASE + I2CR) = I2CR_IEN;
+	writew(I2CR_IEN, I2C_BASE + I2CR);
 
-	while (__REG16(I2C_BASE + I2SR) & I2SR_IBB && --timeout)
+	while (readw(I2C_BASE + I2SR) & I2SR_IBB && --timeout)
 		udelay(1);
 
 	return 0;
@@ -204,7 +219,8 @@  int i2c_read(uchar chip, uint addr, int alen, uchar *buf, int len)
 int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len)
 {
 	int timeout = 10000;
-	DPRINTF("%s chip: 0x%02x addr: 0x%04x alen: %d len: %d\n",__FUNCTION__, chip, addr, alen, len);
+	DPRINTF("%s chip: 0x%02x addr: 0x%04x alen: %d len: %d\n",
+		__FUNCTION__, chip, addr, alen, len);
 
 	if (i2c_addr(chip, addr, alen))
 		return -1;
@@ -213,9 +229,9 @@  int i2c_write(uchar chip, uint addr, int alen, uchar *buf, int len)
 		if (tx_byte(*buf++))
 			return -1;
 
-	__REG16(I2C_BASE + I2CR) = I2CR_IEN;
+	writew(I2CR_IEN, I2C_BASE + I2CR);
 
-	while (__REG16(I2C_BASE + I2SR) & I2SR_IBB && --timeout)
+	while (readw(I2C_BASE + I2SR) & I2SR_IBB && --timeout)
 		udelay(1);
 
 	return 0;