| Message ID | 1474265858-25753-2-git-send-email-j-keerthy@ti.com |
|---|---|
| State | Superseded |
| Delegated to: | Przemyslaw Marczak |
| Headers | show |
On Mon, Sep 19, 2016 at 11:47:33AM +0530, Keerthy wrote: > Add u8 i2c read/write hooks. > > Signed-off-by: Keerthy <j-keerthy@ti.com> Reviewed-by: Tom Rini <trini@konsulko.com>
Hello, Am 19.09.2016 um 08:17 schrieb Keerthy: > Add u8 i2c read/write hooks. > > Signed-off-by: Keerthy <j-keerthy@ti.com> > --- > drivers/i2c/i2c-uclass.c | 10 ++++++++++ > include/i2c.h | 24 ++++++++++++++++++++++++ > 2 files changed, 34 insertions(+) Reviewed-by: Heiko Schocher <hs@denx.de> 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, &val, 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() >
Hi, On 19 September 2016 at 00:17, Keerthy <j-keerthy@ti.com> wrote: > Add u8 i2c read/write hooks. > > Signed-off-by: Keerthy <j-keerthy@ti.com> > --- > 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, &val, 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
On Tuesday 20 September 2016 05:23 PM, Simon Glass wrote: > Hi, > > On 19 September 2016 at 00:17, Keerthy <j-keerthy@ti.com> wrote: >> Add u8 i2c read/write hooks. >> >> Signed-off-by: Keerthy <j-keerthy@ti.com> >> --- >> 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, &val, 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 >
Hi, On 20 September 2016 at 06:42, Keerthy <a0393675@ti.com> wrote: > > > On Tuesday 20 September 2016 05:23 PM, Simon Glass wrote: >> >> Hi, >> >> On 19 September 2016 at 00:17, Keerthy <j-keerthy@ti.com> wrote: >>> >>> Add u8 i2c read/write hooks. >>> >>> Signed-off-by: Keerthy <j-keerthy@ti.com> >>> --- >>> 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, &val, 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
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, &val, 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()
Add u8 i2c read/write hooks. Signed-off-by: Keerthy <j-keerthy@ti.com> --- drivers/i2c/i2c-uclass.c | 10 ++++++++++ include/i2c.h | 24 ++++++++++++++++++++++++ 2 files changed, 34 insertions(+)