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

[Balaji T K <balajitk@ti.com>]

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 ?

>
> 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@ti.com>
> Cc: Sricharan R <r.sricharan@ti.com>
> Reported-by: Ruchika Kharwar <ruchika@ti.com>
> Signed-off-by: Nishanth Menon <nm@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.

Thanks and Regards
Balaji T K

> -{
> -	return i2c_write(chip_no, reg, 1, &val, 1);
> -}
> -
> -static inline int palmas_read_u8(u8 chip_no, u8 reg, u8 *val)
> -{
> -	return i2c_read(chip_no, reg, 1, val, 1);
> -}
> -
>   void twl6035_init_settings(void)
>   {
>   	return;
> @@ -57,7 +46,7 @@ int twl6035_mmc1_poweron_ldo(void)
>   	/* set LDO9 TWL6035 to 3V */
>   	val = 0x2b; /* (3 -.9)*28 +1 */
>
> -	if (palmas_write_u8(0x48, LDO9_VOLTAGE, val)) {
> +	if (twl6035_i2c_write_u8(0x48, val, LDO9_VOLTAGE)) {
>   		printf("twl6035: could not set LDO9 voltage.\n");
>   		return 1;
>   	}
> @@ -65,7 +54,7 @@ int twl6035_mmc1_poweron_ldo(void)
>   	/* TURN ON LDO9 */
>   	val = LDO_ON | LDO_MODE_SLEEP | LDO_MODE_ACTIVE;
>
> -	if (palmas_write_u8(0x48, LDO9_CTRL, val)) {
> +	if (twl6035_i2c_write_u8(0x48, val, LDO9_CTRL)) {
>   		printf("twl6035: could not turn on LDO9.\n");
>   		return 1;
>   	}
>