Re: [U-Boot] [PATCH v3 1/6] dm: i2c: Add u8 read/write i2c functions

[Simon Glass]

Hi,

On 20 September 2016 at 06:42, Keerthy  wrote:
>
>
> On Tuesday 20 September 2016 05:23 PM, Simon Glass wrote:
>>
>> Hi,
>>
>> On 19 September 2016 at 00:17, Keerthy  wrote:
>>>
>>> Add u8 i2c read/write hooks.
>>>
>>> Signed-off-by: Keerthy 
>>> ---
>>>  drivers/i2c/i2c-uclass.c | 10 ++
>>>  include/i2c.h| 24 
>>>  2 files changed, 34 insertions(+)
>>>
>>> diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c
>>> index dbd3789..6ce5d9a 100644
>>> --- a/drivers/i2c/i2c-uclass.c
>>> +++ b/drivers/i2c/i2c-uclass.c
>>> @@ -231,6 +231,16 @@ int dm_i2c_reg_write(struct udevice *dev, uint
>>> offset, uint value)
>>> return dm_i2c_write(dev, offset, , 1);
>>>  }
>>>
>>> +int dm_i2c_u8_read(struct udevice *dev, uint offset, u8 *val)
>>> +{
>>> +   return dm_i2c_read(dev, offset, val, 1);
>>> +}
>>> +
>>> +int dm_i2c_u8_write(struct udevice *dev, uint offset, u8 *val)
>>> +{
>>> +   return dm_i2c_write(dev, offset, val, 1);
>>> +}
>>
>>
>> These look almost the same as dm_i2c_reg_read/write(), but IMO those
>> are easier to use since they don't require a pointer to be passed. How
>> do you intend to use these two new functions?
>
>
> Simon,
>
> I see a kind of issue in the current implementation of
> int dm_i2c_reg_read(struct udevice *dev, uint offset)
> int dm_i2c_reg_write(struct udevice *dev, uint offset, uint value)
>
> In the current set up my need is to read and then write a 1 byte I2C
> register. It is best to have consistency with read and write.
> In the current case(dm_i2c_reg_read/write) i read an integer and now when i
> want to write i need to convert it to an unsigned integer.
>
> Instead of all this i made a patch which keeps u8 across read and write:
>
> int dm_i2c_reg_read(struct udevice *dev, uint offset, u8 *val)
> returns error/success with the return value and value is passed by reference
> in val variable.
>
> int dm_i2c_reg_write(struct udevice *dev, uint offset, u8 *val)
> returns error/success using the return value and the value to be written is
> sent in val.

Well I really don't see why you can use the other functions. But if
you prefer these, then OK.

>
> Regards,
> Keerthy
>
>
[...]

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v3 1/6] dm: i2c: Add u8 read/write i2c functions

[Keerthy]

On Tuesday 20 September 2016 05:23 PM, Simon Glass wrote:

Hi,

On 19 September 2016 at 00:17, Keerthy  wrote:

Add u8 i2c read/write hooks.

Signed-off-by: Keerthy 
---
 drivers/i2c/i2c-uclass.c | 10 ++
 include/i2c.h| 24 
 2 files changed, 34 insertions(+)

diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c
index dbd3789..6ce5d9a 100644
--- a/drivers/i2c/i2c-uclass.c
+++ b/drivers/i2c/i2c-uclass.c
@@ -231,6 +231,16 @@ int dm_i2c_reg_write(struct udevice *dev, uint offset, 
uint value)
return dm_i2c_write(dev, offset, , 1);
 }

+int dm_i2c_u8_read(struct udevice *dev, uint offset, u8 *val)
+{
+   return dm_i2c_read(dev, offset, val, 1);
+}
+
+int dm_i2c_u8_write(struct udevice *dev, uint offset, u8 *val)
+{
+   return dm_i2c_write(dev, offset, val, 1);
+}


These look almost the same as dm_i2c_reg_read/write(), but IMO those
are easier to use since they don't require a pointer to be passed. How
do you intend to use these two new functions?


Simon,

I see a kind of issue in the current implementation of
int dm_i2c_reg_read(struct udevice *dev, uint offset)
int dm_i2c_reg_write(struct udevice *dev, uint offset, uint value)

In the current set up my need is to read and then write a 1 byte I2C 
register. It is best to have consistency with read and write.
In the current case(dm_i2c_reg_read/write) i read an integer and now 
when i want to write i need to convert it to an unsigned integer.


Instead of all this i made a patch which keeps u8 across read and write:

int dm_i2c_reg_read(struct udevice *dev, uint offset, u8 *val)
returns error/success with the return value and value is passed by 
reference in val variable.


int dm_i2c_reg_write(struct udevice *dev, uint offset, u8 *val)
returns error/success using the return value and the value to be written 
is sent in val.


Regards,
Keerthy





+
 /**
  * i2c_probe_chip() - probe for a chip on a bus
  *
diff --git a/include/i2c.h b/include/i2c.h
index d500445..c3059ad 100644
--- a/include/i2c.h
+++ b/include/i2c.h
@@ -191,6 +191,30 @@ int dm_i2c_reg_read(struct udevice *dev, uint offset);
 int dm_i2c_reg_write(struct udevice *dev, uint offset, unsigned int val);

 /**
+ * dm_i2c_u8_read() - Read a byte value from an I2C register
+ *
+ * This reads a single value from the given address in an I2C chip
+ *
+ * @dev:Device to use for transfer
+ * @addr:   Address to read from
+ * @val:   Value read is stored in val
+ * @return 0 for success, -ve on error
+ */
+int dm_i2c_u8_read(struct udevice *dev, uint offset, u8 *val);
+
+/**
+ * dm_i2c_u8_write() - Write a byte value to an I2C register
+ *
+ * This writes a single value to the given address in an I2C chip
+ *
+ * @dev:Device to use for transfer
+ * @addr:   Address to write to
+ * @val:Value to write
+ * @return 0 on success, -ve on error
+ */
+int dm_i2c_u8_write(struct udevice *dev, uint offset, u8 *val);
+
+/**
  * dm_i2c_xfer() - Transfer messages over I2C
  *
  * This transfers a raw message. It is best to use dm_i2c_reg_read/write()
--
1.9.1


Regards,
Simon


___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v3 1/6] dm: i2c: Add u8 read/write i2c functions

[Simon Glass]

Hi,

On 19 September 2016 at 00:17, Keerthy  wrote:
> Add u8 i2c read/write hooks.
>
> Signed-off-by: Keerthy 
> ---
>  drivers/i2c/i2c-uclass.c | 10 ++
>  include/i2c.h| 24 
>  2 files changed, 34 insertions(+)
>
> diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c
> index dbd3789..6ce5d9a 100644
> --- a/drivers/i2c/i2c-uclass.c
> +++ b/drivers/i2c/i2c-uclass.c
> @@ -231,6 +231,16 @@ int dm_i2c_reg_write(struct udevice *dev, uint offset, 
> uint value)
> return dm_i2c_write(dev, offset, , 1);
>  }
>
> +int dm_i2c_u8_read(struct udevice *dev, uint offset, u8 *val)
> +{
> +   return dm_i2c_read(dev, offset, val, 1);
> +}
> +
> +int dm_i2c_u8_write(struct udevice *dev, uint offset, u8 *val)
> +{
> +   return dm_i2c_write(dev, offset, val, 1);
> +}

These look almost the same as dm_i2c_reg_read/write(), but IMO those
are easier to use since they don't require a pointer to be passed. How
do you intend to use these two new functions?

> +
>  /**
>   * i2c_probe_chip() - probe for a chip on a bus
>   *
> diff --git a/include/i2c.h b/include/i2c.h
> index d500445..c3059ad 100644
> --- a/include/i2c.h
> +++ b/include/i2c.h
> @@ -191,6 +191,30 @@ int dm_i2c_reg_read(struct udevice *dev, uint offset);
>  int dm_i2c_reg_write(struct udevice *dev, uint offset, unsigned int val);
>
>  /**
> + * dm_i2c_u8_read() - Read a byte value from an I2C register
> + *
> + * This reads a single value from the given address in an I2C chip
> + *
> + * @dev:Device to use for transfer
> + * @addr:   Address to read from
> + * @val:   Value read is stored in val
> + * @return 0 for success, -ve on error
> + */
> +int dm_i2c_u8_read(struct udevice *dev, uint offset, u8 *val);
> +
> +/**
> + * dm_i2c_u8_write() - Write a byte value to an I2C register
> + *
> + * This writes a single value to the given address in an I2C chip
> + *
> + * @dev:Device to use for transfer
> + * @addr:   Address to write to
> + * @val:Value to write
> + * @return 0 on success, -ve on error
> + */
> +int dm_i2c_u8_write(struct udevice *dev, uint offset, u8 *val);
> +
> +/**
>   * dm_i2c_xfer() - Transfer messages over I2C
>   *
>   * This transfers a raw message. It is best to use dm_i2c_reg_read/write()
> --
> 1.9.1

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v3 1/6] dm: i2c: Add u8 read/write i2c functions

[Heiko Schocher]

Hello,

Am 19.09.2016 um 08:17 schrieb Keerthy:

Add u8 i2c read/write hooks.

Signed-off-by: Keerthy 
---
  drivers/i2c/i2c-uclass.c | 10 ++
  include/i2c.h| 24 
  2 files changed, 34 insertions(+)


Reviewed-by: Heiko Schocher 

bye,
Heiko


diff --git a/drivers/i2c/i2c-uclass.c b/drivers/i2c/i2c-uclass.c
index dbd3789..6ce5d9a 100644
--- a/drivers/i2c/i2c-uclass.c
+++ b/drivers/i2c/i2c-uclass.c
@@ -231,6 +231,16 @@ int dm_i2c_reg_write(struct udevice *dev, uint offset, 
uint value)
return dm_i2c_write(dev, offset, , 1);
  }

+int dm_i2c_u8_read(struct udevice *dev, uint offset, u8 *val)
+{
+   return dm_i2c_read(dev, offset, val, 1);
+}
+
+int dm_i2c_u8_write(struct udevice *dev, uint offset, u8 *val)
+{
+   return dm_i2c_write(dev, offset, val, 1);
+}
+
  /**
   * i2c_probe_chip() - probe for a chip on a bus
   *
diff --git a/include/i2c.h b/include/i2c.h
index d500445..c3059ad 100644
--- a/include/i2c.h
+++ b/include/i2c.h
@@ -191,6 +191,30 @@ int dm_i2c_reg_read(struct udevice *dev, uint offset);
  int dm_i2c_reg_write(struct udevice *dev, uint offset, unsigned int val);

  /**
+ * dm_i2c_u8_read() - Read a byte value from an I2C register
+ *
+ * This reads a single value from the given address in an I2C chip
+ *
+ * @dev:Device to use for transfer
+ * @addr:   Address to read from
+ * @val:   Value read is stored in val
+ * @return 0 for success, -ve on error
+ */
+int dm_i2c_u8_read(struct udevice *dev, uint offset, u8 *val);
+
+/**
+ * dm_i2c_u8_write() - Write a byte value to an I2C register
+ *
+ * This writes a single value to the given address in an I2C chip
+ *
+ * @dev:Device to use for transfer
+ * @addr:   Address to write to
+ * @val:Value to write
+ * @return 0 on success, -ve on error
+ */
+int dm_i2c_u8_write(struct udevice *dev, uint offset, u8 *val);
+
+/**
   * dm_i2c_xfer() - Transfer messages over I2C
   *
   * This transfers a raw message. It is best to use dm_i2c_reg_read/write()



--
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH v3 1/6] dm: i2c: Add u8 read/write i2c functions

[Tom Rini]

On Mon, Sep 19, 2016 at 11:47:33AM +0530, Keerthy wrote:

> Add u8 i2c read/write hooks.
> 
> Signed-off-by: Keerthy 

Reviewed-by: Tom Rini 

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot