Patchwork [U-Boot,02/16] pmic:i2c: Add I2C byte order to PMIC framework

login
register
mail settings
Submitter Łukasz Majewski
Date Sept. 14, 2012, 3:40 p.m.
Message ID <1347637215-4830-3-git-send-email-l.majewski@samsung.com>
Download mbox | patch
Permalink /patch/183947/
State Superseded
Delegated to: Minkyu Kang
Headers show

Comments

Łukasz Majewski - Sept. 14, 2012, 3:40 p.m.
Since the pmic_reg_read is the u32 value, the order in which bytes
are placed to form u32 value is important.

This commit adds the reverse (which is default) and normal byte order.

Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/misc/pmic_i2c.c |   31 ++++++++++++++++++++++++-------
 include/pmic.h          |    3 +++
 2 files changed, 27 insertions(+), 7 deletions(-)
Stefano Babic - Sept. 17, 2012, 9:33 a.m.
On 14/09/2012 17:40, Lukasz Majewski wrote:
> Since the pmic_reg_read is the u32 value, the order in which bytes
> are placed to form u32 value is important.
> 
> This commit adds the reverse (which is default) and normal byte order.
> 
> Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---

Hi,

>  drivers/misc/pmic_i2c.c |   31 ++++++++++++++++++++++++-------
>  include/pmic.h          |    3 +++
>  2 files changed, 27 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/misc/pmic_i2c.c b/drivers/misc/pmic_i2c.c
> index e74c372..1aa5b7b 100644
> --- a/drivers/misc/pmic_i2c.c
> +++ b/drivers/misc/pmic_i2c.c
> @@ -40,13 +40,24 @@ int pmic_reg_write(struct pmic *p, u32 reg, u32 val)
>  
>  	switch (pmic_i2c_tx_num) {
>  	case 3:
> -		buf[0] = (val >> 16) & 0xff;
> -		buf[1] = (val >> 8) & 0xff;
> -		buf[2] = val & 0xff;
> +		if (pmic_i2c_byte_order == PMIC_BYTE_ORDER_NORMAL) {
> +			buf[2] = (val >> 16) & 0xff;
> +			buf[1] = (val >> 8) & 0xff;
> +			buf[0] = val & 0xff;
> +		} else {
> +			buf[0] = (val >> 16) & 0xff;
> +			buf[1] = (val >> 8) & 0xff;
> +			buf[2] = val & 0xff;
> +		}
>  		break;
>  	case 2:
> -		buf[0] = (val >> 8) & 0xff;
> -		buf[1] = val & 0xff;
> +		if (pmic_i2c_byte_order == PMIC_BYTE_ORDER_NORMAL) {
> +			buf[1] = (val >> 8) & 0xff;
> +			buf[0] = val & 0xff;
> +		} else {
> +			buf[0] = (val >> 8) & 0xff;
> +			buf[1] = val & 0xff;
> +		}
>  		break;
>  	case 1:
>  		buf[0] = val & 0xff;
> @@ -75,10 +86,16 @@ int pmic_reg_read(struct pmic *p, u32 reg, u32 *val)
>  
>  	switch (pmic_i2c_tx_num) {
>  	case 3:
> -		ret_val = buf[0] << 16 | buf[1] << 8 | buf[2];
> +		if (pmic_i2c_byte_order == PMIC_BYTE_ORDER_NORMAL)
> +			ret_val = buf[2] << 16 | buf[1] << 8 | buf[0];
> +		else
> +			ret_val = buf[0] << 16 | buf[1] << 8 | buf[2];
>  		break;
>  	case 2:
> -		ret_val = buf[0] << 8 | buf[1];
> +		if (pmic_i2c_byte_order == PMIC_BYTE_ORDER_NORMAL)
> +			ret_val = buf[1] << 8 | buf[0];
> +		else
> +			ret_val = buf[0] << 8 | buf[1];
>  		break;
>  	case 1:
>  		ret_val = buf[0];
> diff --git a/include/pmic.h b/include/pmic.h
> index 6a05b40..2166f73 100644
> --- a/include/pmic.h
> +++ b/include/pmic.h
> @@ -27,11 +27,13 @@
>  enum { PMIC_I2C, PMIC_SPI, };
>  enum { I2C_PMIC, I2C_NUM, };
>  enum { PMIC_READ, PMIC_WRITE, };
> +enum { PMIC_BYTE_ORDER_REVERSED, PMIC_BYTE_ORDER_NORMAL, };
>  
>  struct p_i2c {
>  	unsigned char addr;
>  	unsigned char *buf;
>  	unsigned char tx_num;
> +	unsigned char byte_order;

I can imagine we could have the same issue with SPI and not only with
I2C. The byte order is not strictly related to the interface type, and I
think this should add to the "struct pmic" instead of the "struct p_i2c".

Regards,
Stefano Babic
Lukasz Majewski - Sept. 17, 2012, 8:41 p.m.
Hi Stefano,

> On 14/09/2012 17:40, Lukasz Majewski wrote:
> > Since the pmic_reg_read is the u32 value, the order in which bytes
> > are placed to form u32 value is important.
> > 
> > This commit adds the reverse (which is default) and normal byte
> > order.
> > 
> > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > ---
> 
> Hi,
> 
> >  drivers/misc/pmic_i2c.c |   31 ++++++++++++++++++++++++-------
> >  include/pmic.h          |    3 +++
> >  2 files changed, 27 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/misc/pmic_i2c.c b/drivers/misc/pmic_i2c.c
> > index e74c372..1aa5b7b 100644
> > --- a/drivers/misc/pmic_i2c.c
> > +++ b/drivers/misc/pmic_i2c.c
> > @@ -40,13 +40,24 @@ int pmic_reg_write(struct pmic *p, u32 reg, u32
> > val) 
> >  	switch (pmic_i2c_tx_num) {
> >  	case 3:
> > -		buf[0] = (val >> 16) & 0xff;
> > -		buf[1] = (val >> 8) & 0xff;
> > -		buf[2] = val & 0xff;
> > +		if (pmic_i2c_byte_order == PMIC_BYTE_ORDER_NORMAL)
> > {
> > +			buf[2] = (val >> 16) & 0xff;
> > +			buf[1] = (val >> 8) & 0xff;
> > +			buf[0] = val & 0xff;
> > +		} else {
> > +			buf[0] = (val >> 16) & 0xff;
> > +			buf[1] = (val >> 8) & 0xff;
> > +			buf[2] = val & 0xff;
> > +		}
> >  		break;
> >  	case 2:
> > -		buf[0] = (val >> 8) & 0xff;
> > -		buf[1] = val & 0xff;
> > +		if (pmic_i2c_byte_order == PMIC_BYTE_ORDER_NORMAL)
> > {
> > +			buf[1] = (val >> 8) & 0xff;
> > +			buf[0] = val & 0xff;
> > +		} else {
> > +			buf[0] = (val >> 8) & 0xff;
> > +			buf[1] = val & 0xff;
> > +		}
> >  		break;
> >  	case 1:
> >  		buf[0] = val & 0xff;
> > @@ -75,10 +86,16 @@ int pmic_reg_read(struct pmic *p, u32 reg, u32
> > *val) 
> >  	switch (pmic_i2c_tx_num) {
> >  	case 3:
> > -		ret_val = buf[0] << 16 | buf[1] << 8 | buf[2];
> > +		if (pmic_i2c_byte_order == PMIC_BYTE_ORDER_NORMAL)
> > +			ret_val = buf[2] << 16 | buf[1] << 8 |
> > buf[0];
> > +		else
> > +			ret_val = buf[0] << 16 | buf[1] << 8 |
> > buf[2]; break;
> >  	case 2:
> > -		ret_val = buf[0] << 8 | buf[1];
> > +		if (pmic_i2c_byte_order == PMIC_BYTE_ORDER_NORMAL)
> > +			ret_val = buf[1] << 8 | buf[0];
> > +		else
> > +			ret_val = buf[0] << 8 | buf[1];
> >  		break;
> >  	case 1:
> >  		ret_val = buf[0];
> > diff --git a/include/pmic.h b/include/pmic.h
> > index 6a05b40..2166f73 100644
> > --- a/include/pmic.h
> > +++ b/include/pmic.h
> > @@ -27,11 +27,13 @@
> >  enum { PMIC_I2C, PMIC_SPI, };
> >  enum { I2C_PMIC, I2C_NUM, };
> >  enum { PMIC_READ, PMIC_WRITE, };
> > +enum { PMIC_BYTE_ORDER_REVERSED, PMIC_BYTE_ORDER_NORMAL, };
> >  
> >  struct p_i2c {
> >  	unsigned char addr;
> >  	unsigned char *buf;
> >  	unsigned char tx_num;
> > +	unsigned char byte_order;
> 
> I can imagine we could have the same issue with SPI and not only with
> I2C. The byte order is not strictly related to the interface type,
> and I think this should add to the "struct pmic" instead of the
> "struct p_i2c".
> 

Thanks for reply. 

Good point. I will move this to struct pmic.

Best regards,
Lukasz Majewski

Patch

diff --git a/drivers/misc/pmic_i2c.c b/drivers/misc/pmic_i2c.c
index e74c372..1aa5b7b 100644
--- a/drivers/misc/pmic_i2c.c
+++ b/drivers/misc/pmic_i2c.c
@@ -40,13 +40,24 @@  int pmic_reg_write(struct pmic *p, u32 reg, u32 val)
 
 	switch (pmic_i2c_tx_num) {
 	case 3:
-		buf[0] = (val >> 16) & 0xff;
-		buf[1] = (val >> 8) & 0xff;
-		buf[2] = val & 0xff;
+		if (pmic_i2c_byte_order == PMIC_BYTE_ORDER_NORMAL) {
+			buf[2] = (val >> 16) & 0xff;
+			buf[1] = (val >> 8) & 0xff;
+			buf[0] = val & 0xff;
+		} else {
+			buf[0] = (val >> 16) & 0xff;
+			buf[1] = (val >> 8) & 0xff;
+			buf[2] = val & 0xff;
+		}
 		break;
 	case 2:
-		buf[0] = (val >> 8) & 0xff;
-		buf[1] = val & 0xff;
+		if (pmic_i2c_byte_order == PMIC_BYTE_ORDER_NORMAL) {
+			buf[1] = (val >> 8) & 0xff;
+			buf[0] = val & 0xff;
+		} else {
+			buf[0] = (val >> 8) & 0xff;
+			buf[1] = val & 0xff;
+		}
 		break;
 	case 1:
 		buf[0] = val & 0xff;
@@ -75,10 +86,16 @@  int pmic_reg_read(struct pmic *p, u32 reg, u32 *val)
 
 	switch (pmic_i2c_tx_num) {
 	case 3:
-		ret_val = buf[0] << 16 | buf[1] << 8 | buf[2];
+		if (pmic_i2c_byte_order == PMIC_BYTE_ORDER_NORMAL)
+			ret_val = buf[2] << 16 | buf[1] << 8 | buf[0];
+		else
+			ret_val = buf[0] << 16 | buf[1] << 8 | buf[2];
 		break;
 	case 2:
-		ret_val = buf[0] << 8 | buf[1];
+		if (pmic_i2c_byte_order == PMIC_BYTE_ORDER_NORMAL)
+			ret_val = buf[1] << 8 | buf[0];
+		else
+			ret_val = buf[0] << 8 | buf[1];
 		break;
 	case 1:
 		ret_val = buf[0];
diff --git a/include/pmic.h b/include/pmic.h
index 6a05b40..2166f73 100644
--- a/include/pmic.h
+++ b/include/pmic.h
@@ -27,11 +27,13 @@ 
 enum { PMIC_I2C, PMIC_SPI, };
 enum { I2C_PMIC, I2C_NUM, };
 enum { PMIC_READ, PMIC_WRITE, };
+enum { PMIC_BYTE_ORDER_REVERSED, PMIC_BYTE_ORDER_NORMAL, };
 
 struct p_i2c {
 	unsigned char addr;
 	unsigned char *buf;
 	unsigned char tx_num;
+	unsigned char byte_order;
 };
 
 struct p_spi {
@@ -65,6 +67,7 @@  int pmic_set_output(struct pmic *p, u32 reg, int ldo, int on);
 
 #define pmic_i2c_addr (p->hw.i2c.addr)
 #define pmic_i2c_tx_num (p->hw.i2c.tx_num)
+#define pmic_i2c_byte_order (p->hw.i2c.byte_order)
 
 #define pmic_spi_bitlen (p->hw.spi.bitlen)
 #define pmic_spi_flags (p->hw.spi.flags)