[U-Boot] [PATCH] power: twl6035: cleanup register access API

[Nishanth Menon]

On 12:24-20130313, Balaji T K wrote:
> Hi,
> 
> On Wednesday 13 March 2013 02:09 AM, Nishanth Menon wrote:
> >commit 21144298 (power: twl6035: add palmas PMIC support)
> >introduced twl6035_i2c_[read|write]_u8
> >Then, commit dd23e59d (omap5: pbias ldo9 turn on)
> >introduced palmas_[read|write]_u8
> >for precisely the same access function. TWL6035 belongs to
> >the palmas family, so instead of having an palmas API,
> >we could use twl6035 API instead (which is used elsewhere
> >as well).
> 
> can you provide reference where twl6035 API is used ?
This is based on commit 21144298 - the exposed API for the rest is what
the intent was supposed to be.
> 
> >
> >Account for the parameter change while doing the change and
> >remove palmas register accessors.
> 
> That is the reason ("parameter change") for introducing
> palmas_write/read_u8 :-)
> 
> I think twl6035_i2c_read/write_u8 should have parameter
> reg address and value interchanged or should get completely removed.
> twl6035_i2c_read/write_u8 is based on legacy implementation
> of twl6030/twl4030.
> 
> 
> >
> >Cc: Balaji T K <balajitk at ti.com>
> >Cc: Sricharan R <r.sricharan at ti.com>
> >Reported-by: Ruchika Kharwar <ruchika at ti.com>
> >Signed-off-by: Nishanth Menon <nm at ti.com>
> >---
> >  drivers/power/twl6035.c |   15 ++-------------
> >  1 file changed, 2 insertions(+), 13 deletions(-)
> >
> >diff --git a/drivers/power/twl6035.c b/drivers/power/twl6035.c
> >index d3de698..b0b2406 100644
> >--- a/drivers/power/twl6035.c
> >+++ b/drivers/power/twl6035.c
> >@@ -34,17 +34,6 @@ int twl6035_i2c_read_u8(u8 chip_no, u8 *val, u8 reg)
> >  	return i2c_read(chip_no, reg, 1, val, 1);
> >  }
> >
> >-/* To align with i2c mw/mr address, reg, val command syntax */
> >-static inline int palmas_write_u8(u8 chip_no, u8 reg, u8 val)
> 
> Please note the reg address, val order with i2c_write API.
> It aligns with i2c mw/mr command @ u-boot cmd line,
> with kernel APIs, i2cget, i2cset utilities.
> 
> It don't see a reason why twl6035_i2c_read/write_u8
> should have parameters interchanged.
Then we should rather *fix* function usage *NOT* introduce an new
function for the same job!

But wait an min, twl4030_i2c_[read|write]_u8 follows the same proto as
twl6035_i2c_read/write_u8!
You are welcome to fix both while at it. But please *DONOT* introduce
duplicate functions on style need!


-- 
Regards,
Nishanth Menon