[U-Boot] [PATCH V2 0/9] OMAP3-5: TWL[46]03[05]: cleanup register access and misc minimal cleanups
V1: http://patchwork.ozlabs.org/patch/227112/
This series helps standardize register parameters for TWL4030, 6030 and 6035 used in various OMAP3,4,5 based platforms. For historical reasons, we have been following val, reg as the order of parameters while we have reg, val in every other i2c apis including i2c mw/mr command @ u-boot cmd line, with kernel APIs, i2cget, i2cset utilities.
Instead of maintaining this forked implementation, it is never too late to fix them.
Build tested (MAKEALL) platforms-at least these seem to be be impacted ones: cm_t35 devkit8000 dig297 igep0020 igep0020_nand igep0030 igep0030_nand nokia_rx51 omap3_beagle omap3_evm omap3_evm_quick_mmc omap3_evm_quick_nand omap3_logic omap3_mvblx omap3_overo omap3_pandora omap3_sdp3430 omap3_zoom1 omap3_zoom2 omap4_panda omap4_sdp4430 omap5_evm tricorder
Boot tested platforms (upto kernel+shell with dtb): omap3_beagle - tested on beagle XM (C1), beagle(C1D) - TWL4030 omap4_panda - tested on PandaBoard(A3) and PandaBoard-ES(EB3) - TWL6030 omap5_evm - OMAP5 uEVM - TWL6035
twl4030 changes are little wider in scope, so I have split them into two patches to help review
Series is based on u-boot master: master 8b906a9 Merge branch 'spi' of git://git.denx.de/u-boot-x86 (rationale being the changes if done on v2013.04-rc1 have much changes to allow this series to apply cleanly on the latest)
NOTE: the series tries to maintain existing indentation style to allow the code to stay in sync with legacy code.
Nishanth Menon (9): twl4030: make twl4030_i2c_write_u8 prototype consistent twl4030: make twl4030_i2c_read_u8 prototype consistent twl6030: twl6030_i2c_[read|write]_u8 prototype consistent twl6030: move twl6030 register access functions to common header file twl6030: add header guard twl6035: make twl6030_i2c_[read|write]_u8 static inline twl6035: twl6035_i2c_[read|write]_u8 prototype consistent twl6035: remove redundant palmas_[read|write]_u8 twl6035: add header guard
board/cm_t35/cm_t35.c | 24 +++++++------- board/nokia/rx51/rx51.c | 52 +++++++++++++++--------------- board/pandora/pandora.c | 3 +- drivers/misc/twl4030_led.c | 4 +-- drivers/power/twl4030.c | 16 +++++----- drivers/power/twl6030.c | 75 +++++++++++++++++++------------------------- drivers/power/twl6035.c | 26 ++------------- drivers/usb/phy/twl4030.c | 48 ++++++++++++++-------------- include/twl4030.h | 4 +-- include/twl6030.h | 16 ++++++++++ include/twl6035.h | 18 +++++++++-- 11 files changed, 142 insertions(+), 144 deletions(-)
Regards, Nishanth Menon
u-boot standard i2c register write prototype is i2c_reg_write(u8 addr, u8 reg, u8 val)
twl4030_i2c_write_u8(u8 addr, u8 val, u8 reg) does not provide consistency, so switch the prototype to be consistent with rest of u-boot i2c operations: twl4030_i2c_write_u8(u8 addr, u8 reg, u8 val)
Signed-off-by: Nishanth Menon nm@ti.com --- board/cm_t35/cm_t35.c | 20 +++++++++---------- board/nokia/rx51/rx51.c | 36 +++++++++++++++++----------------- board/pandora/pandora.c | 3 ++- drivers/misc/twl4030_led.c | 4 ++-- drivers/power/twl4030.c | 12 ++++++------ drivers/usb/phy/twl4030.c | 46 ++++++++++++++++++++++---------------------- include/twl4030.h | 2 +- 7 files changed, 62 insertions(+), 61 deletions(-)
diff --git a/board/cm_t35/cm_t35.c b/board/cm_t35/cm_t35.c index e0e8235..a14f89f 100644 --- a/board/cm_t35/cm_t35.c +++ b/board/cm_t35/cm_t35.c @@ -430,17 +430,17 @@ static void setup_net_chip_gmpc(void) static void reset_net_chip(void) { /* Set GPIO1 of TPS65930 as output */ - twl4030_i2c_write_u8(TWL4030_CHIP_GPIO, 0x02, - TWL4030_BASEADD_GPIO + 0x03); + twl4030_i2c_write_u8(TWL4030_CHIP_GPIO, TWL4030_BASEADD_GPIO + 0x03, + 0x02); /* Send a pulse on the GPIO pin */ - twl4030_i2c_write_u8(TWL4030_CHIP_GPIO, 0x02, - TWL4030_BASEADD_GPIO + 0x0C); + twl4030_i2c_write_u8(TWL4030_CHIP_GPIO, TWL4030_BASEADD_GPIO + 0x0C, + 0x02); udelay(1); - twl4030_i2c_write_u8(TWL4030_CHIP_GPIO, 0x02, - TWL4030_BASEADD_GPIO + 0x09); + twl4030_i2c_write_u8(TWL4030_CHIP_GPIO, TWL4030_BASEADD_GPIO + 0x09, + 0x02); mdelay(40); - twl4030_i2c_write_u8(TWL4030_CHIP_GPIO, 0x02, - TWL4030_BASEADD_GPIO + 0x0C); + twl4030_i2c_write_u8(TWL4030_CHIP_GPIO, TWL4030_BASEADD_GPIO + 0x0C, + 0x02); mdelay(1); } #else @@ -537,10 +537,10 @@ int ehci_hcd_init(int index, struct ehci_hccr **hccr, struct ehci_hcor **hcor) twl4030_i2c_read_u8(TWL4030_CHIP_GPIO, &val, offset); /* Set GPIO6 and GPIO7 of TPS65930 as output */ val |= 0xC0; - twl4030_i2c_write_u8(TWL4030_CHIP_GPIO, val, offset); + twl4030_i2c_write_u8(TWL4030_CHIP_GPIO, offset, val); offset = TWL4030_BASEADD_GPIO + TWL4030_GPIO_SETGPIODATAOUT1; /* Take both PHYs out of reset */ - twl4030_i2c_write_u8(TWL4030_CHIP_GPIO, 0xC0, offset); + twl4030_i2c_write_u8(TWL4030_CHIP_GPIO, offset, 0xC0); udelay(1);
return omap_ehci_hcd_init(&usbhs_bdata, hccr, hcor); diff --git a/board/nokia/rx51/rx51.c b/board/nokia/rx51/rx51.c index 48eb65f..464302b 100644 --- a/board/nokia/rx51/rx51.c +++ b/board/nokia/rx51/rx51.c @@ -332,10 +332,10 @@ void *video_hw_init(void) static void twl4030_regulator_set_mode(u8 id, u8 mode) { u16 msg = MSG_SINGULAR(DEV_GRP_P1, id, mode); - twl4030_i2c_write_u8(TWL4030_CHIP_PM_MASTER, msg >> 8, - TWL4030_PM_MASTER_PB_WORD_MSB); - twl4030_i2c_write_u8(TWL4030_CHIP_PM_MASTER, msg & 0xff, - TWL4030_PM_MASTER_PB_WORD_LSB); + twl4030_i2c_write_u8(TWL4030_CHIP_PM_MASTER, + TWL4030_PM_MASTER_PB_WORD_MSB, msg >> 8); + twl4030_i2c_write_u8(TWL4030_CHIP_PM_MASTER, + TWL4030_PM_MASTER_PB_WORD_LSB, msg & 0xff); }
static void omap3_emu_romcode_call(u32 service_id, u32 *parameters) @@ -410,8 +410,8 @@ int misc_init_r(void) TWL4030_PM_MASTER_PB_CFG);
/* enable I2C access to powerbus (needed for twl4030 regulator) */ - twl4030_i2c_write_u8(TWL4030_CHIP_PM_MASTER, 0x02, - TWL4030_PM_MASTER_PB_CFG); + twl4030_i2c_write_u8(TWL4030_CHIP_PM_MASTER, TWL4030_PM_MASTER_PB_CFG, + 0x02);
/* set VAUX3, VSIM and VMMC1 state to active - enable eMMC memory */ twl4030_regulator_set_mode(RES_VAUX3, RES_STATE_ACTIVE); @@ -419,8 +419,8 @@ int misc_init_r(void) twl4030_regulator_set_mode(RES_VMMC1, RES_STATE_ACTIVE);
/* restore I2C access state */ - twl4030_i2c_write_u8(TWL4030_CHIP_PM_MASTER, state, - TWL4030_PM_MASTER_PB_CFG); + twl4030_i2c_write_u8(TWL4030_CHIP_PM_MASTER, TWL4030_PM_MASTER_PB_CFG, + state);
/* set env variable attkernaddr for relocated kernel */ sprintf(buf, "%#x", KERNEL_ADDRESS); @@ -481,8 +481,8 @@ void hw_watchdog_reset(void) /* timeout 0 means watchdog is disabled */ /* reset watchdog timeout to 31s (maximum) */ if (timeout != 0) - twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, 31, - TWL4030_PM_RECEIVER_WATCHDOG_CFG); + twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, + TWL4030_PM_RECEIVER_WATCHDOG_CFG, 31);
/* store last watchdog reset time */ twl_wd_time = get_timer(0); @@ -541,18 +541,18 @@ int rx51_kp_init(void) ctrl |= TWL4030_KEYPAD_CTRL_KBD_ON; ctrl |= TWL4030_KEYPAD_CTRL_SOFT_NRST; ctrl |= TWL4030_KEYPAD_CTRL_SOFTMODEN; - ret |= twl4030_i2c_write_u8(TWL4030_CHIP_KEYPAD, ctrl, - TWL4030_KEYPAD_KEYP_CTRL_REG); + ret |= twl4030_i2c_write_u8(TWL4030_CHIP_KEYPAD, + TWL4030_KEYPAD_KEYP_CTRL_REG, ctrl); /* enable key event status */ - ret |= twl4030_i2c_write_u8(TWL4030_CHIP_KEYPAD, 0xfe, - TWL4030_KEYPAD_KEYP_IMR1); + ret |= twl4030_i2c_write_u8(TWL4030_CHIP_KEYPAD, + TWL4030_KEYPAD_KEYP_IMR1, 0xfe); /* enable interrupt generation on rising and falling */ /* this is a workaround for qemu twl4030 emulation */ - ret |= twl4030_i2c_write_u8(TWL4030_CHIP_KEYPAD, 0x57, - TWL4030_KEYPAD_KEYP_EDR); + ret |= twl4030_i2c_write_u8(TWL4030_CHIP_KEYPAD, + TWL4030_KEYPAD_KEYP_EDR, 0x57); /* enable ISR clear on read */ - ret |= twl4030_i2c_write_u8(TWL4030_CHIP_KEYPAD, 0x05, - TWL4030_KEYPAD_KEYP_SIH_CTRL); + ret |= twl4030_i2c_write_u8(TWL4030_CHIP_KEYPAD, + TWL4030_KEYPAD_KEYP_SIH_CTRL, 0x05); return 0; }
diff --git a/board/pandora/pandora.c b/board/pandora/pandora.c index 9ff5dd7..5f0c58d 100644 --- a/board/pandora/pandora.c +++ b/board/pandora/pandora.c @@ -114,8 +114,9 @@ int misc_init_r(void)
/* Enable battery backup capacitor (3.2V, 0.5mA charge current) */ twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, + TWL4030_PM_RECEIVER_BB_CFG, TWL4030_BB_CFG_BBCHEN | TWL4030_BB_CFG_BBSEL_3200MV | - TWL4030_BB_CFG_BBISEL_500UA, TWL4030_PM_RECEIVER_BB_CFG); + TWL4030_BB_CFG_BBISEL_500UA);
dieid_num_r();
diff --git a/drivers/misc/twl4030_led.c b/drivers/misc/twl4030_led.c index 33cea11..e150d8f 100644 --- a/drivers/misc/twl4030_led.c +++ b/drivers/misc/twl4030_led.c @@ -42,7 +42,7 @@ void twl4030_led_init(unsigned char ledon_mask) if (ledon_mask & TWL4030_LED_LEDEN_LEDBON) ledon_mask |= TWL4030_LED_LEDEN_LEDBPWM;
- twl4030_i2c_write_u8(TWL4030_CHIP_LED, ledon_mask, - TWL4030_LED_LEDEN); + twl4030_i2c_write_u8(TWL4030_CHIP_LED, TWL4030_LED_LEDEN, + ledon_mask);
} diff --git a/drivers/power/twl4030.c b/drivers/power/twl4030.c index e7d5f13..3e933e9 100644 --- a/drivers/power/twl4030.c +++ b/drivers/power/twl4030.c @@ -51,8 +51,8 @@ void twl4030_power_reset_init(void) printf("Could not initialize hardware reset\n"); } else { val |= TWL4030_PM_MASTER_SW_EVENTS_STOPON_PWRON; - if (twl4030_i2c_write_u8(TWL4030_CHIP_PM_MASTER, val, - TWL4030_PM_MASTER_P1_SW_EVENTS)) { + if (twl4030_i2c_write_u8(TWL4030_CHIP_PM_MASTER, + TWL4030_PM_MASTER_P1_SW_EVENTS, val)) { printf("Error:TWL4030: failed to write the power register\n"); printf("Could not initialize hardware reset\n"); } @@ -68,8 +68,8 @@ void twl4030_pmrecv_vsel_cfg(u8 vsel_reg, u8 vsel_val, int ret;
/* Select the Voltage */ - ret = twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, vsel_val, - vsel_reg); + ret = twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, vsel_reg, + vsel_val); if (ret != 0) { printf("Could not write vsel to reg %02x (%d)\n", vsel_reg, ret); @@ -77,8 +77,8 @@ void twl4030_pmrecv_vsel_cfg(u8 vsel_reg, u8 vsel_val, }
/* Select the Device Group (enable the supply if dev_grp_sel != 0) */ - ret = twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, dev_grp_sel, - dev_grp); + ret = twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, dev_grp, + dev_grp_sel); if (ret != 0) printf("Could not write grp_sel to reg %02x (%d)\n", dev_grp, ret); diff --git a/drivers/usb/phy/twl4030.c b/drivers/usb/phy/twl4030.c index 54d2e61..f41cc07 100644 --- a/drivers/usb/phy/twl4030.c +++ b/drivers/usb/phy/twl4030.c @@ -54,7 +54,7 @@ static int twl4030_usb_write(u8 address, u8 data) { int ret;
- ret = twl4030_i2c_write_u8(TWL4030_CHIP_USB, data, address); + ret = twl4030_i2c_write_u8(TWL4030_CHIP_USB, address, data); if (ret != 0) printf("TWL4030:USB:Write[0x%x] Error %d\n", address, ret);
@@ -78,40 +78,40 @@ static int twl4030_usb_read(u8 address) static void twl4030_usb_ldo_init(void) { /* Enable writing to power configuration registers */ - twl4030_i2c_write_u8(TWL4030_CHIP_PM_MASTER, 0xC0, - TWL4030_PM_MASTER_PROTECT_KEY); - twl4030_i2c_write_u8(TWL4030_CHIP_PM_MASTER, 0x0C, - TWL4030_PM_MASTER_PROTECT_KEY); + twl4030_i2c_write_u8(TWL4030_CHIP_PM_MASTER, + TWL4030_PM_MASTER_PROTECT_KEY, 0xC0); + twl4030_i2c_write_u8(TWL4030_CHIP_PM_MASTER, + TWL4030_PM_MASTER_PROTECT_KEY, 0x0C);
/* put VUSB3V1 LDO in active state */ - twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, 0x00, - TWL4030_PM_RECEIVER_VUSB_DEDICATED2); + twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, + TWL4030_PM_RECEIVER_VUSB_DEDICATED2, 0x00);
/* input to VUSB3V1 LDO is from VBAT, not VBUS */ - twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, 0x14, - TWL4030_PM_RECEIVER_VUSB_DEDICATED1); + twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, + TWL4030_PM_RECEIVER_VUSB_DEDICATED1, 0x14);
/* turn on 3.1V regulator */ - twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, 0x20, - TWL4030_PM_RECEIVER_VUSB3V1_DEV_GRP); - twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, 0x00, - TWL4030_PM_RECEIVER_VUSB3V1_TYPE); + twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, + TWL4030_PM_RECEIVER_VUSB3V1_DEV_GRP, 0x20); + twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, + TWL4030_PM_RECEIVER_VUSB3V1_TYPE, 0x00);
/* turn on 1.5V regulator */ - twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, 0x20, - TWL4030_PM_RECEIVER_VUSB1V5_DEV_GRP); - twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, 0x00, - TWL4030_PM_RECEIVER_VUSB1V5_TYPE); + twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, + TWL4030_PM_RECEIVER_VUSB1V5_DEV_GRP, 0x20); + twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, + TWL4030_PM_RECEIVER_VUSB1V5_TYPE, 0x00);
/* turn on 1.8V regulator */ - twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, 0x20, - TWL4030_PM_RECEIVER_VUSB1V8_DEV_GRP); - twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, 0x00, - TWL4030_PM_RECEIVER_VUSB1V8_TYPE); + twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, + TWL4030_PM_RECEIVER_VUSB1V8_DEV_GRP, 0x20); + twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, + TWL4030_PM_RECEIVER_VUSB1V8_TYPE, 0x00);
/* disable access to power configuration registers */ - twl4030_i2c_write_u8(TWL4030_CHIP_PM_MASTER, 0x00, - TWL4030_PM_MASTER_PROTECT_KEY); + twl4030_i2c_write_u8(TWL4030_CHIP_PM_MASTER, + TWL4030_PM_MASTER_PROTECT_KEY, 0x00); }
static void twl4030_phy_power(void) diff --git a/include/twl4030.h b/include/twl4030.h index 5aa1841..aca695d 100644 --- a/include/twl4030.h +++ b/include/twl4030.h @@ -638,7 +638,7 @@ * examples are TWL4030_PM_RECEIVER_VMMC1_DEV_GRP and * TWL4030_LED_LEDEN. */ -static inline int twl4030_i2c_write_u8(u8 chip_no, u8 val, u8 reg) +static inline int twl4030_i2c_write_u8(u8 chip_no, u8 reg, u8 val) { return i2c_write(chip_no, reg, 1, &val, 1); }
u-boot standard i2c read prototype is i2c_read(addr, reg, 1, &buf, 1)
twl4030_i2c_read_u8(u8 addr, u8 *val, u8 reg) does not provide consistency, so switch the prototype to be consistent with rest of u-boot i2c operations: twl4030_i2c_read_u8(u8 addr, u8 reg, u8 *val)
Signed-off-by: Nishanth Menon nm@ti.com --- board/cm_t35/cm_t35.c | 4 ++-- board/nokia/rx51/rx51.c | 16 ++++++++-------- drivers/power/twl4030.c | 4 ++-- drivers/usb/phy/twl4030.c | 2 +- include/twl4030.h | 2 +- 5 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/board/cm_t35/cm_t35.c b/board/cm_t35/cm_t35.c index a14f89f..4562d50 100644 --- a/board/cm_t35/cm_t35.c +++ b/board/cm_t35/cm_t35.c @@ -385,7 +385,7 @@ int board_mmc_getcd(struct mmc *mmc) { u8 val;
- if (twl4030_i2c_read_u8(TWL4030_CHIP_GPIO, &val, TWL4030_BASEADD_GPIO)) + if (twl4030_i2c_read_u8(TWL4030_CHIP_GPIO, TWL4030_BASEADD_GPIO, &val)) return -1;
return !(val & 1); @@ -534,7 +534,7 @@ int ehci_hcd_init(int index, struct ehci_hccr **hccr, struct ehci_hcor **hcor) udelay(1000);
offset = TWL4030_BASEADD_GPIO + TWL4030_GPIO_GPIODATADIR1; - twl4030_i2c_read_u8(TWL4030_CHIP_GPIO, &val, offset); + twl4030_i2c_read_u8(TWL4030_CHIP_GPIO, offset, &val); /* Set GPIO6 and GPIO7 of TPS65930 as output */ val |= 0xC0; twl4030_i2c_write_u8(TWL4030_CHIP_GPIO, offset, val); diff --git a/board/nokia/rx51/rx51.c b/board/nokia/rx51/rx51.c index 464302b..f7a56c5 100644 --- a/board/nokia/rx51/rx51.c +++ b/board/nokia/rx51/rx51.c @@ -406,8 +406,8 @@ int misc_init_r(void) TWL4030_PM_RECEIVER_DEV_GRP_P1);
/* store I2C access state */ - twl4030_i2c_read_u8(TWL4030_CHIP_PM_MASTER, &state, - TWL4030_PM_MASTER_PB_CFG); + twl4030_i2c_read_u8(TWL4030_CHIP_PM_MASTER, TWL4030_PM_MASTER_PB_CFG, + &state);
/* enable I2C access to powerbus (needed for twl4030 regulator) */ twl4030_i2c_write_u8(TWL4030_CHIP_PM_MASTER, TWL4030_PM_MASTER_PB_CFG, @@ -475,8 +475,8 @@ void hw_watchdog_reset(void) return;
/* read actual watchdog timeout */ - twl4030_i2c_read_u8(TWL4030_CHIP_PM_RECEIVER, &timeout, - TWL4030_PM_RECEIVER_WATCHDOG_CFG); + twl4030_i2c_read_u8(TWL4030_CHIP_PM_RECEIVER, + TWL4030_PM_RECEIVER_WATCHDOG_CFG, &timeout);
/* timeout 0 means watchdog is disabled */ /* reset watchdog timeout to 31s (maximum) */ @@ -531,8 +531,8 @@ int rx51_kp_init(void) { int ret = 0; u8 ctrl; - ret = twl4030_i2c_read_u8(TWL4030_CHIP_KEYPAD, &ctrl, - TWL4030_KEYPAD_KEYP_CTRL_REG); + ret = twl4030_i2c_read_u8(TWL4030_CHIP_KEYPAD, + TWL4030_KEYPAD_KEYP_CTRL_REG, &ctrl);
if (ret) return ret; @@ -615,8 +615,8 @@ int rx51_kp_tstc(void) for (i = 0; i < 2; i++) {
/* check interrupt register for events */ - twl4030_i2c_read_u8(TWL4030_CHIP_KEYPAD, &intr, - TWL4030_KEYPAD_KEYP_ISR1+(2*i)); + twl4030_i2c_read_u8(TWL4030_CHIP_KEYPAD, + TWL4030_KEYPAD_KEYP_ISR1 + (2 * i), &intr);
/* no event */ if (!(intr&1)) diff --git a/drivers/power/twl4030.c b/drivers/power/twl4030.c index 3e933e9..a9b10fb 100644 --- a/drivers/power/twl4030.c +++ b/drivers/power/twl4030.c @@ -45,8 +45,8 @@ void twl4030_power_reset_init(void) { u8 val = 0; - if (twl4030_i2c_read_u8(TWL4030_CHIP_PM_MASTER, &val, - TWL4030_PM_MASTER_P1_SW_EVENTS)) { + if (twl4030_i2c_read_u8(TWL4030_CHIP_PM_MASTER, + TWL4030_PM_MASTER_P1_SW_EVENTS, &val)) { printf("Error:TWL4030: failed to read the power register\n"); printf("Could not initialize hardware reset\n"); } else { diff --git a/drivers/usb/phy/twl4030.c b/drivers/usb/phy/twl4030.c index f41cc07..74f1dcc 100644 --- a/drivers/usb/phy/twl4030.c +++ b/drivers/usb/phy/twl4030.c @@ -66,7 +66,7 @@ static int twl4030_usb_read(u8 address) u8 data; int ret;
- ret = twl4030_i2c_read_u8(TWL4030_CHIP_USB, &data, address); + ret = twl4030_i2c_read_u8(TWL4030_CHIP_USB, address, &data); if (ret == 0) ret = data; else diff --git a/include/twl4030.h b/include/twl4030.h index aca695d..569ad27 100644 --- a/include/twl4030.h +++ b/include/twl4030.h @@ -643,7 +643,7 @@ static inline int twl4030_i2c_write_u8(u8 chip_no, u8 reg, u8 val) return i2c_write(chip_no, reg, 1, &val, 1); }
-static inline int twl4030_i2c_read_u8(u8 chip_no, u8 *val, u8 reg) +static inline int twl4030_i2c_read_u8(u8 chip_no, u8 reg, u8 *val) { return i2c_read(chip_no, reg, 1, val, 1); }
u-boot standard i2c register access prototype is i2c_read(addr, reg, 1, &buf, 1) i2c_reg_write(u8 addr, u8 reg, u8 val)
twl6030_i2c_read_u8(u8 addr, u8 *val, u8 reg) twl6030_i2c_write_u8(u8 addr, u8 val, u8 reg) does not provide consistency, so switch the prototype to be consistent with rest of u-boot i2c operations: twl6030_i2c_read_u8(u8 addr, u8 reg, u8 *val) twl6030_i2c_write_u8(u8 addr, u8 reg, u8 val)
Signed-off-by: Nishanth Menon nm@ti.com --- drivers/power/twl6030.c | 68 +++++++++++++++++++++++------------------------ 1 file changed, 34 insertions(+), 34 deletions(-)
diff --git a/drivers/power/twl6030.c b/drivers/power/twl6030.c index c5a0038..af1114c 100644 --- a/drivers/power/twl6030.c +++ b/drivers/power/twl6030.c @@ -26,12 +26,12 @@ #include <twl6030.h>
/* Functions to read and write from TWL6030 */ -static inline int twl6030_i2c_write_u8(u8 chip_no, u8 val, u8 reg) +static inline int twl6030_i2c_write_u8(u8 chip_no, u8 reg, u8 val) { return i2c_write(chip_no, reg, 1, &val, 1); }
-static inline int twl6030_i2c_read_u8(u8 chip_no, u8 *val, u8 reg) +static inline int twl6030_i2c_read_u8(u8 chip_no, u8 reg, u8 *val) { return i2c_read(chip_no, reg, 1, val, 1); } @@ -42,13 +42,13 @@ static int twl6030_gpadc_read_channel(u8 channel_no) u8 msb = 0; int ret = 0;
- ret = twl6030_i2c_read_u8(TWL6030_CHIP_ADC, &lsb, - GPCH0_LSB + channel_no * 2); + ret = twl6030_i2c_read_u8(TWL6030_CHIP_ADC, + GPCH0_LSB + channel_no * 2, &lsb); if (ret) return ret;
- ret = twl6030_i2c_read_u8(TWL6030_CHIP_ADC, &msb, - GPCH0_MSB + channel_no * 2); + ret = twl6030_i2c_read_u8(TWL6030_CHIP_ADC, + GPCH0_MSB + channel_no * 2, &msb); if (ret) return ret;
@@ -60,7 +60,7 @@ static int twl6030_gpadc_sw2_trigger(void) u8 val; int ret = 0;
- ret = twl6030_i2c_write_u8(TWL6030_CHIP_ADC, CTRL_P2_SP2, CTRL_P2); + ret = twl6030_i2c_write_u8(TWL6030_CHIP_ADC, CTRL_P2, CTRL_P2_SP2); if (ret) return ret;
@@ -68,7 +68,7 @@ static int twl6030_gpadc_sw2_trigger(void) val = CTRL_P2_BUSY;
while (!((val & CTRL_P2_EOCP2) && (!(val & CTRL_P2_BUSY)))) { - ret = twl6030_i2c_read_u8(TWL6030_CHIP_ADC, &val, CTRL_P2); + ret = twl6030_i2c_read_u8(TWL6030_CHIP_ADC, CTRL_P2, &val); if (ret) return ret; udelay(1000); @@ -79,29 +79,29 @@ static int twl6030_gpadc_sw2_trigger(void)
void twl6030_stop_usb_charging(void) { - twl6030_i2c_write_u8(TWL6030_CHIP_CHARGER, 0, CONTROLLER_CTRL1); + twl6030_i2c_write_u8(TWL6030_CHIP_CHARGER, CONTROLLER_CTRL1, 0);
return; }
void twl6030_start_usb_charging(void) { - twl6030_i2c_write_u8(TWL6030_CHIP_CHARGER, CHARGERUSB_VICHRG_1500, - CHARGERUSB_VICHRG); - twl6030_i2c_write_u8(TWL6030_CHIP_CHARGER, CHARGERUSB_CIN_LIMIT_NONE, - CHARGERUSB_CINLIMIT); - twl6030_i2c_write_u8(TWL6030_CHIP_CHARGER, MBAT_TEMP, - CONTROLLER_INT_MASK); - twl6030_i2c_write_u8(TWL6030_CHIP_CHARGER, MASK_MCHARGERUSB_THMREG, - CHARGERUSB_INT_MASK); - twl6030_i2c_write_u8(TWL6030_CHIP_CHARGER, CHARGERUSB_VOREG_4P0, - CHARGERUSB_VOREG); - twl6030_i2c_write_u8(TWL6030_CHIP_CHARGER, CHARGERUSB_CTRL2_VITERM_400, - CHARGERUSB_CTRL2); - twl6030_i2c_write_u8(TWL6030_CHIP_CHARGER, TERM, CHARGERUSB_CTRL1); + twl6030_i2c_write_u8(TWL6030_CHIP_CHARGER, + CHARGERUSB_VICHRG, CHARGERUSB_VICHRG_1500); + twl6030_i2c_write_u8(TWL6030_CHIP_CHARGER, + CHARGERUSB_CINLIMIT, CHARGERUSB_CIN_LIMIT_NONE); + twl6030_i2c_write_u8(TWL6030_CHIP_CHARGER, + CONTROLLER_INT_MASK, MBAT_TEMP); + twl6030_i2c_write_u8(TWL6030_CHIP_CHARGER, + CHARGERUSB_INT_MASK, MASK_MCHARGERUSB_THMREG); + twl6030_i2c_write_u8(TWL6030_CHIP_CHARGER, + CHARGERUSB_VOREG, CHARGERUSB_VOREG_4P0); + twl6030_i2c_write_u8(TWL6030_CHIP_CHARGER, + CHARGERUSB_CTRL2, CHARGERUSB_CTRL2_VITERM_400); + twl6030_i2c_write_u8(TWL6030_CHIP_CHARGER, CHARGERUSB_CTRL1, TERM); /* Enable USB charging */ - twl6030_i2c_write_u8(TWL6030_CHIP_CHARGER, CONTROLLER_CTRL1_EN_CHARGER, - CONTROLLER_CTRL1); + twl6030_i2c_write_u8(TWL6030_CHIP_CHARGER, + CONTROLLER_CTRL1, CONTROLLER_CTRL1_EN_CHARGER); return; }
@@ -111,8 +111,8 @@ int twl6030_get_battery_current(void) u8 msb = 0; u8 lsb = 0;
- twl6030_i2c_read_u8(TWL6030_CHIP_CHARGER, &msb, FG_REG_11); - twl6030_i2c_read_u8(TWL6030_CHIP_CHARGER, &lsb, FG_REG_10); + twl6030_i2c_read_u8(TWL6030_CHIP_CHARGER, FG_REG_11, &msb); + twl6030_i2c_read_u8(TWL6030_CHIP_CHARGER, FG_REG_10, &lsb); battery_current = ((msb << 8) | lsb);
/* convert 10 bit signed number to 16 bit signed number */ @@ -156,10 +156,10 @@ void twl6030_init_battery_charging(void) int ret = 0;
/* Enable VBAT measurement */ - twl6030_i2c_write_u8(TWL6030_CHIP_PM, VBAT_MEAS, MISC1); + twl6030_i2c_write_u8(TWL6030_CHIP_PM, MISC1, VBAT_MEAS);
/* Enable GPADC module */ - ret = twl6030_i2c_write_u8(TWL6030_CHIP_CHARGER, FGS | GPADCS, TOGGLE1); + ret = twl6030_i2c_write_u8(TWL6030_CHIP_CHARGER, TOGGLE1, FGS | GPADCS); if (ret) { printf("Failed to enable GPADC\n"); return; @@ -173,7 +173,7 @@ void twl6030_init_battery_charging(void) printf("Main battery voltage too low!\n");
/* Check for the presence of USB charger */ - twl6030_i2c_read_u8(TWL6030_CHIP_CHARGER, &stat1, CONTROLLER_STAT1); + twl6030_i2c_read_u8(TWL6030_CHIP_CHARGER, CONTROLLER_STAT1, &stat1);
/* check for battery presence indirectly via Fuel gauge */ if ((stat1 & VBUS_DET) && (battery_volt < 3300)) @@ -185,8 +185,8 @@ void twl6030_init_battery_charging(void) void twl6030_power_mmc_init() { /* set voltage to 3.0 and turnon for APP */ - twl6030_i2c_write_u8(TWL6030_CHIP_PM, 0x15, VMMC_CFG_VOLTATE); - twl6030_i2c_write_u8(TWL6030_CHIP_PM, 0x21, VMMC_CFG_STATE); + twl6030_i2c_write_u8(TWL6030_CHIP_PM, VMMC_CFG_VOLTATE, 0x15); + twl6030_i2c_write_u8(TWL6030_CHIP_PM, VMMC_CFG_STATE, 0x21); }
void twl6030_usb_device_settings() @@ -194,12 +194,12 @@ void twl6030_usb_device_settings() u8 data = 0;
/* Select APP Group and set state to ON */ - twl6030_i2c_write_u8(TWL6030_CHIP_PM, 0x21, VUSB_CFG_STATE); + twl6030_i2c_write_u8(TWL6030_CHIP_PM, VUSB_CFG_STATE, 0x21);
- twl6030_i2c_read_u8(TWL6030_CHIP_PM, &data, MISC2); + twl6030_i2c_read_u8(TWL6030_CHIP_PM, MISC2, &data); data |= 0x10;
/* Select the input supply for VBUS regulator */ - twl6030_i2c_write_u8(TWL6030_CHIP_PM, data, MISC2); + twl6030_i2c_write_u8(TWL6030_CHIP_PM, MISC2, data); } #endif
twl6030_i2c_[read|write]_u8 can be used else where to access multi-function device such as twl6030, so move the register access functions to the common twl6030.h header file.
Signed-off-by: Nishanth Menon nm@ti.com --- drivers/power/twl6030.c | 11 ----------- include/twl6030.h | 11 +++++++++++ 2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/power/twl6030.c b/drivers/power/twl6030.c index af1114c..f984b95 100644 --- a/drivers/power/twl6030.c +++ b/drivers/power/twl6030.c @@ -25,17 +25,6 @@
#include <twl6030.h>
-/* Functions to read and write from TWL6030 */ -static inline int twl6030_i2c_write_u8(u8 chip_no, u8 reg, u8 val) -{ - return i2c_write(chip_no, reg, 1, &val, 1); -} - -static inline int twl6030_i2c_read_u8(u8 chip_no, u8 reg, u8 *val) -{ - return i2c_read(chip_no, reg, 1, val, 1); -} - static int twl6030_gpadc_read_channel(u8 channel_no) { u8 lsb = 0; diff --git a/include/twl6030.h b/include/twl6030.h index a9fcadb..f7ba3c7 100644 --- a/include/twl6030.h +++ b/include/twl6030.h @@ -126,6 +126,17 @@ #define GPCH0_LSB 0x57 #define GPCH0_MSB 0x58
+/* Functions to read and write from TWL6030 */ +static inline int twl6030_i2c_write_u8(u8 chip_no, u8 reg, u8 val) +{ + return i2c_write(chip_no, reg, 1, &val, 1); +} + +static inline int twl6030_i2c_read_u8(u8 chip_no, u8 reg, u8 *val) +{ + return i2c_read(chip_no, reg, 1, val, 1); +} + void twl6030_init_battery_charging(void); void twl6030_usb_device_settings(void); void twl6030_start_usb_charging(void);
Add an header guard to common header file to prevent multiple includes messing things up.
Signed-off-by: Nishanth Menon nm@ti.com --- include/twl6030.h | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/include/twl6030.h b/include/twl6030.h index f7ba3c7..029b21f 100644 --- a/include/twl6030.h +++ b/include/twl6030.h @@ -21,6 +21,9 @@ * MA 02111-1307 USA */
+#ifndef TWL6030_H +#define TWL6030_H + #include <common.h> #include <i2c.h>
@@ -144,3 +147,5 @@ void twl6030_stop_usb_charging(void); int twl6030_get_battery_voltage(void); int twl6030_get_battery_current(void); void twl6030_power_mmc_init(void); + +#endif /* TWL6030_H */
twl6030_i2c_[read|write]_u8 can be used else where to access multi-function device such as twl6035, we dont need to incurr an function call overhead, so use static inline.
Signed-off-by: Nishanth Menon nm@ti.com --- drivers/power/twl6035.c | 11 ----------- include/twl6035.h | 13 +++++++++++-- 2 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/drivers/power/twl6035.c b/drivers/power/twl6035.c index d3de698..0c2cdfe 100644 --- a/drivers/power/twl6035.c +++ b/drivers/power/twl6035.c @@ -23,17 +23,6 @@ #include <config.h> #include <twl6035.h>
-/* Functions to read and write from TWL6030 */ -int twl6035_i2c_write_u8(u8 chip_no, u8 val, u8 reg) -{ - return i2c_write(chip_no, reg, 1, &val, 1); -} - -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) { diff --git a/include/twl6035.h b/include/twl6035.h index ce74348..89c2ab5 100644 --- a/include/twl6035.h +++ b/include/twl6035.h @@ -36,7 +36,16 @@ #define LDO_MODE_SLEEP (1 << 2) #define LDO_MODE_ACTIVE (1 << 0)
-int twl6035_i2c_write_u8(u8 chip_no, u8 val, u8 reg); -int twl6035_i2c_read_u8(u8 chip_no, u8 *val, u8 reg); +/* Functions to read and write from TWL6030 */ +static inline int twl6035_i2c_write_u8(u8 chip_no, u8 val, u8 reg) +{ + return i2c_write(chip_no, reg, 1, &val, 1); +} + +static inline int twl6035_i2c_read_u8(u8 chip_no, u8 *val, u8 reg) +{ + return i2c_read(chip_no, reg, 1, val, 1); +} + void twl6035_init_settings(void); int twl6035_mmc1_poweron_ldo(void);
u-boot standard i2c register access prototype is i2c_read(addr, reg, 1, &buf, 1) i2c_reg_write(u8 addr, u8 reg, u8 val)
twl6035_i2c_read_u8(u8 addr, u8 *val, u8 reg) twl6035_i2c_write_u8(u8 addr, u8 val, u8 reg) does not provide consistency, so switch the prototype to be consistent with rest of u-boot i2c operations: twl6035_i2c_read_u8(u8 addr, u8 reg, u8 *val) twl6035_i2c_write_u8(u8 addr, u8 reg, u8 val)
Signed-off-by: Nishanth Menon nm@ti.com --- include/twl6035.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/twl6035.h b/include/twl6035.h index 89c2ab5..edc2db5 100644 --- a/include/twl6035.h +++ b/include/twl6035.h @@ -37,12 +37,12 @@ #define LDO_MODE_ACTIVE (1 << 0)
/* Functions to read and write from TWL6030 */ -static inline int twl6035_i2c_write_u8(u8 chip_no, u8 val, u8 reg) +static inline int twl6035_i2c_write_u8(u8 chip_no, u8 reg, u8 val) { return i2c_write(chip_no, reg, 1, &val, 1); }
-static inline int twl6035_i2c_read_u8(u8 chip_no, u8 *val, u8 reg) +static inline int twl6035_i2c_read_u8(u8 chip_no, u8 reg, u8 *val) { return i2c_read(chip_no, reg, 1, val, 1); }
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). The original intent of introducing palmas_[read|write]_u8 was to stay consistent with u-boot's i2c accessors.
Since we have cleaned up twl6035_i2c_[read|write]_u8 with the same objective, we can drop the redundant function.
Reported-by: Ruchika Kharwar ruchika@ti.com Signed-off-by: Nishanth Menon nm@ti.com --- V1: http://patchwork.ozlabs.org/patch/227112/ 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 0c2cdfe..8561db8 100644 --- a/drivers/power/twl6035.c +++ b/drivers/power/twl6035.c @@ -23,17 +23,6 @@ #include <config.h> #include <twl6035.h>
-/* To align with i2c mw/mr address, reg, val command syntax */ -static inline int palmas_write_u8(u8 chip_no, u8 reg, u8 val) -{ - 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; @@ -46,7 +35,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, LDO9_VOLTAGE, val)) { printf("twl6035: could not set LDO9 voltage.\n"); return 1; } @@ -54,7 +43,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, LDO9_CTRL, val)) { printf("twl6035: could not turn on LDO9.\n"); return 1; }
Add an header guard to common header file to prevent multiple includes messing things up.
Signed-off-by: Nishanth Menon nm@ti.com --- include/twl6035.h | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/include/twl6035.h b/include/twl6035.h index edc2db5..d9131c1 100644 --- a/include/twl6035.h +++ b/include/twl6035.h @@ -21,6 +21,9 @@ * MA 02111-1307 USA */
+#ifndef TWL6035_H +#define TWL6035_H + #include <common.h> #include <i2c.h>
@@ -49,3 +52,5 @@ static inline int twl6035_i2c_read_u8(u8 chip_no, u8 reg, u8 *val)
void twl6035_init_settings(void); int twl6035_mmc1_poweron_ldo(void); + +#endif /* TWL6035_H */
Hi Nishanth,
On Saturday 23 March 2013 03:03 AM, Nishanth Menon wrote:
V1: http://patchwork.ozlabs.org/patch/227112/
This series helps standardize register parameters for TWL4030, 6030 and 6035 used in various OMAP3,4,5 based platforms. For historical reasons, we have been following val, reg as the order of parameters while we have reg, val in every other i2c apis including i2c mw/mr command @ u-boot cmd line, with kernel APIs, i2cget, i2cset utilities.
Instead of maintaining this forked implementation, it is never too late to fix them.
Build tested (MAKEALL) platforms-at least these seem to be be impacted ones: cm_t35 devkit8000 dig297 igep0020 igep0020_nand igep0030 igep0030_nand nokia_rx51 omap3_beagle omap3_evm omap3_evm_quick_mmc omap3_evm_quick_nand omap3_logic omap3_mvblx omap3_overo omap3_pandora omap3_sdp3430 omap3_zoom1 omap3_zoom2 omap4_panda omap4_sdp4430 omap5_evm tricorder
Boot tested platforms (upto kernel+shell with dtb): omap3_beagle - tested on beagle XM (C1), beagle(C1D) - TWL4030 omap4_panda - tested on PandaBoard(A3) and PandaBoard-ES(EB3) - TWL6030 omap5_evm - OMAP5 uEVM - TWL6035
twl4030 changes are little wider in scope, so I have split them into two patches to help review
Series is based on u-boot master: master 8b906a9 Merge branch 'spi' of git://git.denx.de/u-boot-x86 (rationale being the changes if done on v2013.04-rc1 have much changes to allow this series to apply cleanly on the latest)
NOTE: the series tries to maintain existing indentation style to allow the code to stay in sync with legacy code.
Nishanth Menon (9): twl4030: make twl4030_i2c_write_u8 prototype consistent twl4030: make twl4030_i2c_read_u8 prototype consistent twl6030: twl6030_i2c_[read|write]_u8 prototype consistent twl6030: move twl6030 register access functions to common header file twl6030: add header guard twl6035: make twl6030_i2c_[read|write]_u8 static inline twl6035: twl6035_i2c_[read|write]_u8 prototype consistent twl6035: remove redundant palmas_[read|write]_u8 twl6035: add header guard
board/cm_t35/cm_t35.c | 24 +++++++------- board/nokia/rx51/rx51.c | 52 +++++++++++++++--------------- board/pandora/pandora.c | 3 +- drivers/misc/twl4030_led.c | 4 +-- drivers/power/twl4030.c | 16 +++++----- drivers/power/twl6030.c | 75 +++++++++++++++++++------------------------- drivers/power/twl6035.c | 26 ++------------- drivers/usb/phy/twl4030.c | 48 ++++++++++++++-------------- include/twl4030.h | 4 +-- include/twl6030.h | 16 ++++++++++ include/twl6035.h | 18 +++++++++-- 11 files changed, 142 insertions(+), 144 deletions(-)
Regards, Nishanth Menon
All of TWL[46]03[05]_i2c_[write/read]_u8 is doing the same. (ie) i2c_write(chip_no, reg, 1, &val, 1); i2c_read(chip_no, reg, 1, val, 1);
We always seem to use 1 byte addresses and length.
Then why can't we move to to twl_common.h and use just one function every where ?
Otherwise, this is a required cleanup.
Reviewed-by: R Sricharan r.sricharan@ti.com
Regards, Sricharan
Hi Sricharan,
On Mon, Mar 25, 2013 at 12:47 PM, Sricharan R r.sricharan@ti.com wrote:
All of TWL[46]03[05]_i2c_[write/read]_u8 is doing the same. (ie) i2c_write(chip_no, reg, 1, &val, 1); i2c_read(chip_no, reg, 1, val, 1);
We always seem to use 1 byte addresses and length. Then why can't we move to to twl_common.h and use just one function every where ? Otherwise, this is a required cleanup.
I had initially considered that, but then having twl6030, 6035, 4030 as API names help us to know from readability angle which register is being accessed and if it the right one. Further, the PMICs are drastically different that using a twl_read_write_u8 might end up confusing reviewer/readability. + the fact that they are inline allows us to have no overhead. --- Regards, Nishanth Menon
Hi Nishanth,
On Monday 25 March 2013 11:50 PM, Nishanth Menon wrote:
Hi Sricharan,
On Mon, Mar 25, 2013 at 12:47 PM, Sricharan R r.sricharan@ti.com wrote:
All of TWL[46]03[05]_i2c_[write/read]_u8 is doing the same. (ie) i2c_write(chip_no, reg, 1, &val, 1); i2c_read(chip_no, reg, 1, val, 1);
We always seem to use 1 byte addresses and length. Then why can't we move to to twl_common.h and use just one function every where ? Otherwise, this is a required cleanup.I had initially considered that, but then having twl6030, 6035, 4030 as API names help us to know from readability angle which register is being accessed and if it the right one. Further, the PMICs are drastically different that using a twl_read_write_u8 might end up confusing reviewer/readability.
- the fact that they are inline allows us to have no overhead.
Now, while adding support for VAYU which has TPS659038, in the current approach we will end up creating a new tps659038.h which does exactly the same thing. This does not feel correct. Can't we differentiate using register names that are passed instead ?
Regards, Sricharan
On 15:01-20130326, Sricharan R wrote:
approach we will end up creating a new tps659038.h which does exactly the same thing. This does not feel correct. Can't we differentiate using register names that are passed instead ?
tps659038/twl6035/twl6037 all belong to palmas family of PMICs. So, how about renaming the file to palmas.c and use palmas_i2c_read/write functions?
On Tuesday 26 March 2013 06:55 PM, Nishanth Menon wrote:
On 15:01-20130326, Sricharan R wrote:
approach we will end up creating a new tps659038.h which does exactly the same thing. This does not feel correct. Can't we differentiate using register names that are passed instead ?
tps659038/twl6035/twl6037 all belong to palmas family of PMICs. So, how about renaming the file to palmas.c and use palmas_i2c_read/write functions?
Yes, sounds good.
Regards, Sricharan
participants (2)
-
Nishanth Menon -
Sricharan R