[alsa-devel] [RFC 0/7] MFD: twl6040: Conversion to i2c driver
Hello,
This series will convert the twl6040 MFD driver to an i2c driver. Compared to older twl4030/5030/TPS the twl6040 is a standalone audio IC. It is better if the twl6040-core (and all of it's child devices) does not depend on the twl-core since it has nothing to with it.
With this conversion the dependency on twl can be dropped from the twl6040 driver stack (core, vibra, audio).
Between the first and second patch the audio will not probe on OMAP4, but I felt it is better this way at least for the first RFC series to not have too big change within one patch. The two patch can be squashed together later if no objections.
Regards, Peter --- Peter Ujfalusi (7): MFD: twl-core: Detach twl6040 from the pmic mfd driver MFD: twl6040: Convert to i2c driver, and separate it from twl core ASoC: twl6040: Remove dependency on twl4030 from Kconfig OMAP: 4430sdp: Correct fixed regulator device ID OMAP: sdp4430: Add fixed regulator for twl6040 needs OMAP: omap4panda: Add fixed regulator for twl6040 needs MFD: TWL6040: Add regulator support for VIO, V2V1 supplies
arch/arm/mach-omap2/board-4430sdp.c | 77 +++++++++++++-- arch/arm/mach-omap2/board-generic.c | 2 +- arch/arm/mach-omap2/board-omap4panda.c | 75 +++++++++++++-- arch/arm/mach-omap2/twl-common.c | 37 ++++++- arch/arm/mach-omap2/twl-common.h | 10 +- drivers/input/misc/Kconfig | 1 - drivers/input/misc/twl6040-vibra.c | 4 +- drivers/mfd/Kconfig | 2 +- drivers/mfd/twl-core.c | 58 +++++++----- drivers/mfd/twl6040-core.c | 165 ++++++++++++++++++++++++++------ include/linux/i2c/twl.h | 12 --- include/linux/mfd/twl6040.h | 27 +++++ sound/soc/codecs/Kconfig | 2 +- sound/soc/codecs/twl6040.c | 3 +- 14 files changed, 375 insertions(+), 100 deletions(-)
On OMAP4 platform audio has separate IC, it is no longer part of the pmic chip. Prevent twl-core to claim the 0x4b address, which belongs to the twl6040 audio IC.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- drivers/mfd/twl-core.c | 58 +++++++++++++++++++++++++++-------------------- 1 files changed, 33 insertions(+), 25 deletions(-)
diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c index e04e04d..ac6fdd93 100644 --- a/drivers/mfd/twl-core.c +++ b/drivers/mfd/twl-core.c @@ -115,8 +115,8 @@ #define twl_has_watchdog() false #endif
-#if defined(CONFIG_MFD_TWL4030_AUDIO) || defined(CONFIG_MFD_TWL4030_AUDIO_MODULE) ||\ - defined(CONFIG_TWL6040_CORE) || defined(CONFIG_TWL6040_CORE_MODULE) +#if defined(CONFIG_MFD_TWL4030_AUDIO) || \ + defined(CONFIG_MFD_TWL4030_AUDIO_MODULE) #define twl_has_codec() true #else #define twl_has_codec() false @@ -146,6 +146,7 @@ #define SUB_CHIP_ID1 1 #define SUB_CHIP_ID2 2 #define SUB_CHIP_ID3 3 +#define SUB_CHIP_ID_INVAL 0xff
#define TWL_MODULE_LAST TWL4030_MODULE_LAST
@@ -315,7 +316,7 @@ static struct twl_mapping twl6030_map[] = { * so they continue to match the order in this table. */ { SUB_CHIP_ID1, TWL6030_BASEADD_USB }, - { SUB_CHIP_ID3, TWL6030_BASEADD_AUDIO }, + { SUB_CHIP_ID_INVAL, TWL6030_BASEADD_AUDIO }, { SUB_CHIP_ID2, TWL6030_BASEADD_DIEID }, { SUB_CHIP_ID2, TWL6030_BASEADD_RSV }, { SUB_CHIP_ID1, TWL6030_BASEADD_PIH }, @@ -377,6 +378,11 @@ int twl_i2c_write(u8 mod_no, u8 *value, u8 reg, unsigned num_bytes) return -EPERM; } sid = twl_map[mod_no].sid; + if (unlikely(sid == SUB_CHIP_ID_INVAL)) { + pr_err("%s: module %d is not part of the pmic\n", + DRIVER_NAME, mod_no); + return -EINVAL; + } twl = &twl_modules[sid];
mutex_lock(&twl->xfer_lock); @@ -434,6 +440,11 @@ int twl_i2c_read(u8 mod_no, u8 *value, u8 reg, unsigned num_bytes) return -EPERM; } sid = twl_map[mod_no].sid; + if (unlikely(sid == SUB_CHIP_ID_INVAL)) { + pr_err("%s: module %d is not part of the pmic\n", + DRIVER_NAME, mod_no); + return -EINVAL; + } twl = &twl_modules[sid];
mutex_lock(&twl->xfer_lock); @@ -834,15 +845,6 @@ add_children(struct twl4030_platform_data *pdata, unsigned long features) return PTR_ERR(child); }
- if (twl_has_codec() && pdata->audio && twl_class_is_6030()) { - sub_chip_id = twl_map[TWL_MODULE_AUDIO_VOICE].sid; - child = add_child(sub_chip_id, "twl6040", - pdata->audio, sizeof(*pdata->audio), - false, 0, 0); - if (IS_ERR(child)) - return PTR_ERR(child); - } - /* twl4030 regulators */ if (twl_has_regulator() && twl_class_is_4030()) { child = add_regulator(TWL4030_REG_VPLL1, pdata->vpll1, @@ -1163,18 +1165,21 @@ int twl6030_exit_irq(void);
static int twl_remove(struct i2c_client *client) { - unsigned i; + unsigned i, num_slaves; int status;
- if (twl_class_is_4030()) + if (twl_class_is_4030()) { status = twl4030_exit_irq(); - else + num_slaves = TWL_NUM_SLAVES; + } else { status = twl6030_exit_irq(); + num_slaves = TWL_NUM_SLAVES - 1; + }
if (status < 0) return status;
- for (i = 0; i < TWL_NUM_SLAVES; i++) { + for (i = 0; i < num_slaves; i++) { struct twl_client *twl = &twl_modules[i];
if (twl->client && twl->client != client) @@ -1190,7 +1195,7 @@ static int __devinit twl_probe(struct i2c_client *client, const struct i2c_device_id *id) { int status; - unsigned i; + unsigned i, num_slaves; struct twl4030_platform_data *pdata = client->dev.platform_data; struct device_node *node = client->dev.of_node; u8 temp; @@ -1244,7 +1249,17 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id) return -EBUSY; }
- for (i = 0; i < TWL_NUM_SLAVES; i++) { + if ((id->driver_data) & TWL6030_CLASS) { + twl_id = TWL6030_CLASS_ID; + twl_map = &twl6030_map[0]; + num_slaves = TWL_NUM_SLAVES - 1; + } else { + twl_id = TWL4030_CLASS_ID; + twl_map = &twl4030_map[0]; + num_slaves = TWL_NUM_SLAVES; + } + + for (i = 0; i < num_slaves; i++) { struct twl_client *twl = &twl_modules[i];
twl->address = client->addr + i; @@ -1263,13 +1278,6 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id) mutex_init(&twl->xfer_lock); } inuse = true; - if ((id->driver_data) & TWL6030_CLASS) { - twl_id = TWL6030_CLASS_ID; - twl_map = &twl6030_map[0]; - } else { - twl_id = TWL4030_CLASS_ID; - twl_map = &twl4030_map[0]; - }
/* setup clock framework */ clocks_init(&client->dev, pdata->clock);
twl6040 is a separate chip, it is not part of the twl6030 PMIC. Complete the separation of the twl6040 from the twl core.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- arch/arm/mach-omap2/board-4430sdp.c | 12 ++-- arch/arm/mach-omap2/board-generic.c | 2 +- arch/arm/mach-omap2/board-omap4panda.c | 13 ++-- arch/arm/mach-omap2/twl-common.c | 37 ++++++++-- arch/arm/mach-omap2/twl-common.h | 10 +-- drivers/input/misc/Kconfig | 1 - drivers/input/misc/twl6040-vibra.c | 4 +- drivers/mfd/Kconfig | 2 +- drivers/mfd/twl6040-core.c | 124 ++++++++++++++++++++++++------- include/linux/i2c/twl.h | 12 --- include/linux/mfd/twl6040.h | 25 +++++++ sound/soc/codecs/twl6040.c | 3 +- 12 files changed, 174 insertions(+), 71 deletions(-)
diff --git a/arch/arm/mach-omap2/board-4430sdp.c b/arch/arm/mach-omap2/board-4430sdp.c index 7eaeb08..f4ac553 100644 --- a/arch/arm/mach-omap2/board-4430sdp.c +++ b/arch/arm/mach-omap2/board-4430sdp.c @@ -20,6 +20,7 @@ #include <linux/usb/otg.h> #include <linux/spi/spi.h> #include <linux/i2c/twl.h> +#include <linux/mfd/twl6040.h> #include <linux/gpio_keys.h> #include <linux/regulator/machine.h> #include <linux/regulator/fixed.h> @@ -554,7 +555,7 @@ static struct regulator_init_data sdp4430_vusim = { }, };
-static struct twl4030_codec_data twl6040_codec = { +static struct twl6040_codec_data twl6040_codec = { /* single-step ramp for headset and handsfree */ .hs_left_step = 0x0f, .hs_right_step = 0x0f, @@ -562,7 +563,7 @@ static struct twl4030_codec_data twl6040_codec = { .hf_right_step = 0x1d, };
-static struct twl4030_vibra_data twl6040_vibra = { +static struct twl6040_vibra_data twl6040_vibra = { .vibldrv_res = 8, .vibrdrv_res = 3, .viblmotor_res = 10, @@ -571,16 +572,14 @@ static struct twl4030_vibra_data twl6040_vibra = { .vddvibr_uV = 0, /* fixed volt supply - VBAT */ };
-static struct twl4030_audio_data twl6040_audio = { +static struct twl6040_platform_data twl6040_data = { .codec = &twl6040_codec, .vibra = &twl6040_vibra, .audpwron_gpio = 127, - .naudint_irq = OMAP44XX_IRQ_SYS_2N, .irq_base = TWL6040_CODEC_IRQ_BASE, };
static struct twl4030_platform_data sdp4430_twldata = { - .audio = &twl6040_audio, /* Regulators */ .vusim = &sdp4430_vusim, .vaux1 = &sdp4430_vaux1, @@ -611,7 +610,8 @@ static int __init omap4_i2c_init(void) TWL_COMMON_REGULATOR_VCXIO | TWL_COMMON_REGULATOR_VUSB | TWL_COMMON_REGULATOR_CLK32KG); - omap4_pmic_init("twl6030", &sdp4430_twldata); + omap4_pmic_init("twl6030", &sdp4430_twldata, + &twl6040_data, OMAP44XX_IRQ_SYS_2N); omap_register_i2c_bus(2, 400, NULL, 0); omap_register_i2c_bus(3, 400, sdp4430_i2c_3_boardinfo, ARRAY_SIZE(sdp4430_i2c_3_boardinfo)); diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c index d587560..68b9d8a 100644 --- a/arch/arm/mach-omap2/board-generic.c +++ b/arch/arm/mach-omap2/board-generic.c @@ -35,7 +35,7 @@ static struct twl4030_platform_data sdp4430_twldata = {
static void __init omap4_i2c_init(void) { - omap4_pmic_init("twl6030", &sdp4430_twldata); + omap4_pmic_init("twl6030", &sdp4430_twldata, NULL, 0); } #endif
diff --git a/arch/arm/mach-omap2/board-omap4panda.c b/arch/arm/mach-omap2/board-omap4panda.c index eada955..da31a72 100644 --- a/arch/arm/mach-omap2/board-omap4panda.c +++ b/arch/arm/mach-omap2/board-omap4panda.c @@ -25,6 +25,7 @@ #include <linux/gpio.h> #include <linux/usb/otg.h> #include <linux/i2c/twl.h> +#include <linux/mfd/twl6040.h> #include <linux/regulator/machine.h> #include <linux/regulator/fixed.h> #include <linux/wl12xx.h> @@ -277,7 +278,7 @@ static int __init omap4_twl6030_hsmmc_init(struct omap2_hsmmc_info *controllers) return 0; }
-static struct twl4030_codec_data twl6040_codec = { +static struct twl6040_codec_data twl6040_codec = { /* single-step ramp for headset and handsfree */ .hs_left_step = 0x0f, .hs_right_step = 0x0f, @@ -285,17 +286,14 @@ static struct twl4030_codec_data twl6040_codec = { .hf_right_step = 0x1d, };
-static struct twl4030_audio_data twl6040_audio = { +static struct twl6040_platform_data twl6040_data = { .codec = &twl6040_codec, .audpwron_gpio = 127, - .naudint_irq = OMAP44XX_IRQ_SYS_2N, .irq_base = TWL6040_CODEC_IRQ_BASE, };
/* Panda board uses the common PMIC configuration */ -static struct twl4030_platform_data omap4_panda_twldata = { - .audio = &twl6040_audio, -}; +static struct twl4030_platform_data omap4_panda_twldata;
/* * Display monitor features are burnt in their EEPROM as EDID data. The EEPROM @@ -319,7 +317,8 @@ static int __init omap4_panda_i2c_init(void) TWL_COMMON_REGULATOR_VCXIO | TWL_COMMON_REGULATOR_VUSB | TWL_COMMON_REGULATOR_CLK32KG); - omap4_pmic_init("twl6030", &omap4_panda_twldata); + omap4_pmic_init("twl6030", &omap4_panda_twldata, + &twl6040_data, OMAP44XX_IRQ_SYS_2N); omap_register_i2c_bus(2, 400, NULL, 0); /* * Bus 3 is attached to the DVI port where devices like the pico DLP diff --git a/arch/arm/mach-omap2/twl-common.c b/arch/arm/mach-omap2/twl-common.c index 10b20c6..bc03154 100644 --- a/arch/arm/mach-omap2/twl-common.c +++ b/arch/arm/mach-omap2/twl-common.c @@ -37,6 +37,16 @@ static struct i2c_board_info __initdata pmic_i2c_board_info = { .flags = I2C_CLIENT_WAKE, };
+static struct i2c_board_info __initdata omap4_i2c1_board_info[] = { + { + .addr = 0x48, + .flags = I2C_CLIENT_WAKE, + }, + { + I2C_BOARD_INFO("twl6040", 0x4b), + }, +}; + void __init omap_pmic_init(int bus, u32 clkrate, const char *pmic_type, int pmic_irq, struct twl4030_platform_data *pmic_data) @@ -49,14 +59,31 @@ void __init omap_pmic_init(int bus, u32 clkrate, omap_register_i2c_bus(bus, clkrate, &pmic_i2c_board_info, 1); }
+void omap4_pmic_init(const char *pmic_type, + struct twl4030_platform_data *pmic_data, + struct twl6040_platform_data *twl6040_data, int twl6040_irq) +{ + /* PMIC part*/ + strncpy(omap4_i2c1_board_info[0].type, pmic_type, + sizeof(omap4_i2c1_board_info[0].type)); + omap4_i2c1_board_info[0].irq = OMAP44XX_IRQ_SYS_1N; + omap4_i2c1_board_info[0].platform_data = pmic_data; + + /* TWL6040 audio IC part */ + omap4_i2c1_board_info[1].irq = twl6040_irq; + omap4_i2c1_board_info[1].platform_data = twl6040_data; + + omap_register_i2c_bus(1, 400, omap4_i2c1_board_info, 2); + +} + void __init omap_pmic_late_init(void) { /* Init the OMAP TWL parameters (if PMIC has been registerd) */ - if (!pmic_i2c_board_info.irq) - return; - - omap3_twl_init(); - omap4_twl_init(); + if (pmic_i2c_board_info.irq) + omap3_twl_init(); + if (omap4_i2c1_board_info[0].irq) + omap4_twl_init(); }
#if defined(CONFIG_ARCH_OMAP3) diff --git a/arch/arm/mach-omap2/twl-common.h b/arch/arm/mach-omap2/twl-common.h index 275dde8..0962748 100644 --- a/arch/arm/mach-omap2/twl-common.h +++ b/arch/arm/mach-omap2/twl-common.h @@ -29,6 +29,7 @@
struct twl4030_platform_data; +struct twl6040_platform_data;
void omap_pmic_init(int bus, u32 clkrate, const char *pmic_type, int pmic_irq, struct twl4030_platform_data *pmic_data); @@ -46,12 +47,9 @@ static inline void omap3_pmic_init(const char *pmic_type, omap_pmic_init(1, 2600, pmic_type, INT_34XX_SYS_NIRQ, pmic_data); }
-static inline void omap4_pmic_init(const char *pmic_type, - struct twl4030_platform_data *pmic_data) -{ - /* Phoenix Audio IC needs I2C1 to start with 400 KHz or less */ - omap_pmic_init(1, 400, pmic_type, OMAP44XX_IRQ_SYS_1N, pmic_data); -} +void omap4_pmic_init(const char *pmic_type, + struct twl4030_platform_data *pmic_data, + struct twl6040_platform_data *audio_data, int twl6040_irq);
void omap3_pmic_get_config(struct twl4030_platform_data *pmic_data, u32 pdata_flags, u32 regulators_flags); diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig index 7b46781..6a9ee24 100644 --- a/drivers/input/misc/Kconfig +++ b/drivers/input/misc/Kconfig @@ -368,7 +368,6 @@ config INPUT_TWL4030_VIBRA
config INPUT_TWL6040_VIBRA tristate "Support for TWL6040 Vibrator" - depends on TWL4030_CORE select TWL6040_CORE select INPUT_FF_MEMLESS help diff --git a/drivers/input/misc/twl6040-vibra.c b/drivers/input/misc/twl6040-vibra.c index 45874fe..14e94f5 100644 --- a/drivers/input/misc/twl6040-vibra.c +++ b/drivers/input/misc/twl6040-vibra.c @@ -28,7 +28,7 @@ #include <linux/module.h> #include <linux/platform_device.h> #include <linux/workqueue.h> -#include <linux/i2c/twl.h> +#include <linux/input.h> #include <linux/mfd/twl6040.h> #include <linux/slab.h> #include <linux/delay.h> @@ -257,7 +257,7 @@ static SIMPLE_DEV_PM_OPS(twl6040_vibra_pm_ops, twl6040_vibra_suspend, NULL);
static int __devinit twl6040_vibra_probe(struct platform_device *pdev) { - struct twl4030_vibra_data *pdata = pdev->dev.platform_data; + struct twl6040_vibra_data *pdata = pdev->dev.platform_data; struct vibra_info *info; int ret;
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index cd13e9f..203e9bc 100644 --- a/drivers/mfd/Kconfig +++ b/drivers/mfd/Kconfig @@ -252,7 +252,7 @@ config TWL6030_PWM
config TWL6040_CORE bool - depends on TWL4030_CORE && GENERIC_HARDIRQS + depends on I2C && GENERIC_HARDIRQS select MFD_CORE default n
diff --git a/drivers/mfd/twl6040-core.c b/drivers/mfd/twl6040-core.c index dda8629..21d6a99 100644 --- a/drivers/mfd/twl6040-core.c +++ b/drivers/mfd/twl6040-core.c @@ -30,12 +30,60 @@ #include <linux/platform_device.h> #include <linux/gpio.h> #include <linux/delay.h> -#include <linux/i2c/twl.h> +#include <linux/i2c.h> #include <linux/mfd/core.h> #include <linux/mfd/twl6040.h>
#define VIBRACTRL_MEMBER(reg) ((reg == TWL6040_REG_VIBCTLL) ? 0 : 1)
+static int twl6040_i2c_read(struct i2c_client *i2c, u8 *value, u8 reg) +{ + struct i2c_msg msg[2]; + int ret; + + msg[0].addr = i2c->addr; + msg[0].len = 1; + msg[0].flags = 0; + msg[0].buf = ® + + msg[1].addr = i2c->addr; + msg[1].flags = I2C_M_RD; + msg[1].len = 1; + msg[1].buf = value; + ret = i2c_transfer(i2c->adapter, msg, 2); + + if (ret < 0) + return ret; + if (ret != 2) + return -EIO; + + return 0; +} + +static int twl6040_i2c_write(struct i2c_client *i2c, u8 value, u8 reg) +{ + struct i2c_msg msg; + u8 data[2]; + int ret; + + data[0] = reg; + data[1] = value; + + msg.addr = i2c->addr; + msg.len = 2; + msg.flags = 0; + msg.buf = data; + + ret = i2c_transfer(i2c->adapter, &msg, 1); + + if (ret < 0) + return ret; + if (ret != 1) + return -EIO; + + return 0; +} + int twl6040_reg_read(struct twl6040 *twl6040, unsigned int reg) { int ret; @@ -47,7 +95,7 @@ int twl6040_reg_read(struct twl6040 *twl6040, unsigned int reg) reg == TWL6040_REG_VIBCTLR)) { val = twl6040->vibra_ctrl_cache[VIBRACTRL_MEMBER(reg)]; } else { - ret = twl_i2c_read_u8(TWL_MODULE_AUDIO_VOICE, &val, reg); + ret = twl6040_i2c_read(twl6040->control_data, &val, reg); if (ret < 0) { mutex_unlock(&twl6040->io_mutex); return ret; @@ -64,7 +112,7 @@ int twl6040_reg_write(struct twl6040 *twl6040, unsigned int reg, u8 val) int ret;
mutex_lock(&twl6040->io_mutex); - ret = twl_i2c_write_u8(TWL_MODULE_AUDIO_VOICE, val, reg); + ret = twl6040_i2c_write(twl6040->control_data, val, reg); /* Cache the vibra control registers */ if (reg == TWL6040_REG_VIBCTLL || reg == TWL6040_REG_VIBCTLR) twl6040->vibra_ctrl_cache[VIBRACTRL_MEMBER(reg)] = val; @@ -80,12 +128,12 @@ int twl6040_set_bits(struct twl6040 *twl6040, unsigned int reg, u8 mask) u8 val;
mutex_lock(&twl6040->io_mutex); - ret = twl_i2c_read_u8(TWL_MODULE_AUDIO_VOICE, &val, reg); + ret = twl6040_i2c_read(twl6040->control_data, &val, reg); if (ret) goto out;
val |= mask; - ret = twl_i2c_write_u8(TWL_MODULE_AUDIO_VOICE, val, reg); + ret = twl6040_i2c_write(twl6040->control_data, val, reg); out: mutex_unlock(&twl6040->io_mutex); return ret; @@ -98,12 +146,12 @@ int twl6040_clear_bits(struct twl6040 *twl6040, unsigned int reg, u8 mask) u8 val;
mutex_lock(&twl6040->io_mutex); - ret = twl_i2c_read_u8(TWL_MODULE_AUDIO_VOICE, &val, reg); + ret = twl6040_i2c_read(twl6040->control_data, &val, reg); if (ret) goto out;
val &= ~mask; - ret = twl_i2c_write_u8(TWL_MODULE_AUDIO_VOICE, val, reg); + ret = twl6040_i2c_write(twl6040->control_data, val, reg); out: mutex_unlock(&twl6040->io_mutex); return ret; @@ -468,21 +516,22 @@ static struct resource twl6040_codec_rsrc[] = { }, };
-static int __devinit twl6040_probe(struct platform_device *pdev) +static int __devinit twl6040_probe(struct i2c_client *client, + const struct i2c_device_id *id) { - struct twl4030_audio_data *pdata = pdev->dev.platform_data; + struct twl6040_platform_data *pdata = client->dev.platform_data; struct twl6040 *twl6040; struct mfd_cell *cell = NULL; int ret, children = 0;
if (!pdata) { - dev_err(&pdev->dev, "Platform data is missing\n"); + dev_err(&client->dev, "Platform data is missing\n"); return -EINVAL; }
/* In order to operate correctly we need valid interrupt config */ - if (!pdata->naudint_irq || !pdata->irq_base) { - dev_err(&pdev->dev, "Invalid IRQ configuration\n"); + if (!client->irq || !pdata->irq_base) { + dev_err(&client->dev, "Invalid IRQ configuration\n"); return -EINVAL; }
@@ -490,10 +539,11 @@ static int __devinit twl6040_probe(struct platform_device *pdev) if (!twl6040) return -ENOMEM;
- platform_set_drvdata(pdev, twl6040); + i2c_set_clientdata(client, twl6040);
- twl6040->dev = &pdev->dev; - twl6040->irq = pdata->naudint_irq; + twl6040->dev = &client->dev; + twl6040->control_data = client; + twl6040->irq = client->irq; twl6040->irq_base = pdata->irq_base;
mutex_init(&twl6040->mutex); @@ -562,12 +612,12 @@ static int __devinit twl6040_probe(struct platform_device *pdev) }
if (children) { - ret = mfd_add_devices(&pdev->dev, pdev->id, twl6040->cells, + ret = mfd_add_devices(&client->dev, -1, twl6040->cells, children, NULL, 0); if (ret) goto mfd_err; } else { - dev_err(&pdev->dev, "No platform data found for children\n"); + dev_err(&client->dev, "No platform data found for children\n"); ret = -ENODEV; goto mfd_err; } @@ -582,14 +632,14 @@ gpio2_err: if (gpio_is_valid(twl6040->audpwron)) gpio_free(twl6040->audpwron); gpio1_err: - platform_set_drvdata(pdev, NULL); + i2c_set_clientdata(client, NULL); kfree(twl6040); return ret; }
-static int __devexit twl6040_remove(struct platform_device *pdev) +static int __devexit twl6040_remove(struct i2c_client *client) { - struct twl6040 *twl6040 = platform_get_drvdata(pdev); + struct twl6040 *twl6040 = i2c_get_clientdata(client);
if (twl6040->power_count) twl6040_power(twl6040, 0); @@ -600,23 +650,41 @@ static int __devexit twl6040_remove(struct platform_device *pdev) free_irq(twl6040->irq_base + TWL6040_IRQ_READY, twl6040); twl6040_irq_exit(twl6040);
- mfd_remove_devices(&pdev->dev); - platform_set_drvdata(pdev, NULL); + mfd_remove_devices(&client->dev); + i2c_set_clientdata(client, NULL); kfree(twl6040);
return 0; }
-static struct platform_driver twl6040_driver = { +static const struct i2c_device_id twl6040_i2c_id[] = { + { "twl6040", 0, }, + { }, +}; +MODULE_DEVICE_TABLE(i2c, twl6040_i2c_id); + +static struct i2c_driver twl6040_driver = { + .driver = { + .name = "twl6040", + .owner = THIS_MODULE, + }, .probe = twl6040_probe, .remove = __devexit_p(twl6040_remove), - .driver = { - .owner = THIS_MODULE, - .name = "twl6040", - }, + .id_table = twl6040_i2c_id, };
-module_platform_driver(twl6040_driver); +static int __devinit twl6040_init(void) +{ + return i2c_add_driver(&twl6040_driver); +} +module_init(twl6040_init); + +static void __devexit twl6040_exit(void) +{ + i2c_del_driver(&twl6040_driver); +} + +module_exit(twl6040_exit);
MODULE_DESCRIPTION("TWL6040 MFD"); MODULE_AUTHOR("Misael Lopez Cruz misael.lopez@ti.com"); diff --git a/include/linux/i2c/twl.h b/include/linux/i2c/twl.h index 78d3465..3826a53 100644 --- a/include/linux/i2c/twl.h +++ b/include/linux/i2c/twl.h @@ -666,23 +666,11 @@ struct twl4030_codec_data { unsigned int check_defaults:1; unsigned int reset_registers:1; unsigned int hs_extmute:1; - u16 hs_left_step; - u16 hs_right_step; - u16 hf_left_step; - u16 hf_right_step; void (*set_hs_extmute)(int mute); };
struct twl4030_vibra_data { unsigned int coexist; - - /* twl6040 */ - unsigned int vibldrv_res; /* left driver resistance */ - unsigned int vibrdrv_res; /* right driver resistance */ - unsigned int viblmotor_res; /* left motor resistance */ - unsigned int vibrmotor_res; /* right motor resistance */ - int vddvibl_uV; /* VDDVIBL volt, set 0 for fixed reg */ - int vddvibr_uV; /* VDDVIBR volt, set 0 for fixed reg */ };
struct twl4030_audio_data { diff --git a/include/linux/mfd/twl6040.h b/include/linux/mfd/twl6040.h index 2463c261..72f530f 100644 --- a/include/linux/mfd/twl6040.h +++ b/include/linux/mfd/twl6040.h @@ -174,7 +174,32 @@ #define TWL6040_SYSCLK_SEL_LPPLL 0 #define TWL6040_SYSCLK_SEL_HPPLL 1
+struct twl6040_codec_data { + u16 hs_left_step; + u16 hs_right_step; + u16 hf_left_step; + u16 hf_right_step; +}; + +struct twl6040_vibra_data { + unsigned int vibldrv_res; /* left driver resistance */ + unsigned int vibrdrv_res; /* right driver resistance */ + unsigned int viblmotor_res; /* left motor resistance */ + unsigned int vibrmotor_res; /* right motor resistance */ + int vddvibl_uV; /* VDDVIBL volt, set 0 for fixed reg */ + int vddvibr_uV; /* VDDVIBR volt, set 0 for fixed reg */ +}; + +struct twl6040_platform_data { + int audpwron_gpio; /* audio power-on gpio */ + unsigned int irq_base; + + struct twl6040_codec_data *codec; + struct twl6040_vibra_data *vibra; +}; + struct twl6040 { + struct i2c_client *control_data; struct device *dev; struct mutex mutex; struct mutex io_mutex; diff --git a/sound/soc/codecs/twl6040.c b/sound/soc/codecs/twl6040.c index ece47ed..3694e9f 100644 --- a/sound/soc/codecs/twl6040.c +++ b/sound/soc/codecs/twl6040.c @@ -26,7 +26,6 @@ #include <linux/pm.h> #include <linux/platform_device.h> #include <linux/slab.h> -#include <linux/i2c/twl.h> #include <linux/mfd/twl6040.h>
#include <sound/core.h> @@ -1515,7 +1514,7 @@ static int twl6040_resume(struct snd_soc_codec *codec) static int twl6040_probe(struct snd_soc_codec *codec) { struct twl6040_data *priv; - struct twl4030_codec_data *pdata = dev_get_platdata(codec->dev); + struct twl6040_codec_data *pdata = dev_get_platdata(codec->dev); struct platform_device *pdev = container_of(codec->dev, struct platform_device, dev); int ret = 0;
On Thu, Feb 02, 2012 at 02:16:54PM +0200, Peter Ujfalusi wrote:
+static int twl6040_i2c_read(struct i2c_client *i2c, u8 *value, u8 reg) +{
- struct i2c_msg msg[2];
- int ret;
May as well convert to regmap while you're at it, saves some code and will get you access to the regmap features - you have to make updates in all the relevant places anyway. We should be pushing to remove use of the ASoC level code for register cache and whatnot to cut down on code duplication (especially for MFDs where it has a few issues interacting with the MFD) and this seems like a good opportunity.
The calling convention here seems a bit weird too, you've got value then register but normally we have register then value for I2C/SPI devices. Except when we don't :(
-module_platform_driver(twl6040_driver); +static int __devinit twl6040_init(void) +{
- return i2c_add_driver(&twl6040_driver);
+} +module_init(twl6040_init);
+static void __devexit twl6040_exit(void) +{
- i2c_del_driver(&twl6040_driver);
+}
+module_exit(twl6040_exit);
There's module_i2c_driver() in mainline as of the last merge window.
On 02/02/2012 02:48 PM, Mark Brown wrote:
On Thu, Feb 02, 2012 at 02:16:54PM +0200, Peter Ujfalusi wrote:
+static int twl6040_i2c_read(struct i2c_client *i2c, u8 *value, u8 reg) +{
- struct i2c_msg msg[2];
- int ret;
May as well convert to regmap while you're at it, saves some code and will get you access to the regmap features - you have to make updates in all the relevant places anyway. We should be pushing to remove use of the ASoC level code for register cache and whatnot to cut down on code duplication (especially for MFDs where it has a few issues interacting with the MFD) and this seems like a good opportunity.
True. I have done the bulk of this set back in June, 2011 and thought that I will do the move to regmap as increment, but you are right. I'm anyway touching the relevant places, so why not do it properly? I'll take the regmap into use for v2.
The calling convention here seems a bit weird too, you've got value then register but normally we have register then value for I2C/SPI devices.
I have based these part on the twl-core's implementation. I should not have done that, I know.. Let's see how it will look like with regmap.
Except when we don't :(
-module_platform_driver(twl6040_driver); +static int __devinit twl6040_init(void) +{
- return i2c_add_driver(&twl6040_driver);
+} +module_init(twl6040_init);
+static void __devexit twl6040_exit(void) +{
- i2c_del_driver(&twl6040_driver);
+}
+module_exit(twl6040_exit);
There's module_i2c_driver() in mainline as of the last merge window.
Oh, good to know. Thanks
On Thu, Feb 02, 2012 at 03:01:06PM +0200, Peter Ujfalusi wrote:
On 02/02/2012 02:48 PM, Mark Brown wrote:
The calling convention here seems a bit weird too, you've got value then register but normally we have register then value for I2C/SPI devices.
I have based these part on the twl-core's implementation. I should not have done that, I know.. Let's see how it will look like with regmap.
Yeah, like I say...
Except when we don't :(
...it's not like we've been consistent so far. I'm not that fussed either way, I probably wouldn't have mentioned it if I hadn't otherwise been commenting.
twl6040 no longer needs twl4030.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/codecs/Kconfig | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index 7c205e7..9462d50 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -56,7 +56,7 @@ config SND_SOC_ALL_CODECS select SND_SOC_TPA6130A2 if I2C select SND_SOC_TLV320DAC33 if I2C select SND_SOC_TWL4030 if TWL4030_CORE - select SND_SOC_TWL6040 if TWL4030_CORE + select SND_SOC_TWL6040 select SND_SOC_UDA134X select SND_SOC_UDA1380 if I2C select SND_SOC_WL1273 if MFD_WL1273_CORE
On Thu, Feb 02, 2012 at 02:16:55PM +0200, Peter Ujfalusi wrote:
twl6040 no longer needs twl4030.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com
Reviewed-by: Mark Brown broonie@opensource.wolfsonmicro.com
Not applying since this needs to follow on from (or be integrated with) the patch splitting the twl6040 to avoid build breaks.
The board has two fixed voltage regulator. Correct the device ID for the vbat, which used -1. Create defines for the fixed regulator IDs so when adding new regulators we know the next available ID to use for them.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- arch/arm/mach-omap2/board-4430sdp.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-omap2/board-4430sdp.c b/arch/arm/mach-omap2/board-4430sdp.c index f4ac553..d02172d 100644 --- a/arch/arm/mach-omap2/board-4430sdp.c +++ b/arch/arm/mach-omap2/board-4430sdp.c @@ -62,6 +62,9 @@ #define GPIO_WIFI_PMENA 54 #define GPIO_WIFI_IRQ 53
+#define FIXED_REG_VBAT_ID 0 +#define FIXED_REG_VWLAN_ID 1 + static const int sdp4430_keymap[] = { KEY(0, 0, KEY_E), KEY(0, 1, KEY_R), @@ -368,7 +371,7 @@ static struct fixed_voltage_config sdp4430_vbat_pdata = {
static struct platform_device sdp4430_vbat = { .name = "reg-fixed-voltage", - .id = -1, + .id = FIXED_REG_VBAT_ID, .dev = { .platform_data = &sdp4430_vbat_pdata, }, @@ -478,7 +481,7 @@ static struct fixed_voltage_config sdp4430_vwlan = {
static struct platform_device omap_vwlan_device = { .name = "reg-fixed-voltage", - .id = 1, + .id = FIXED_REG_VWLAN_ID, .dev = { .platform_data = &sdp4430_vwlan, },
On Thu, Feb 02, 2012 at 02:16:56PM +0200, Peter Ujfalusi wrote:
The board has two fixed voltage regulator. Correct the device ID for the vbat, which used -1. Create defines for the fixed regulator IDs so when adding new regulators we know the next available ID to use for them.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com
Reviewed-by: Mark Brown broonie@opensource.wolfsonmicro.com
The twl6040 VIO power is connected to SMPS V1V8, the V2V1 power is coming from SMPS V2V1 of twl6030. These are fixed, always on regulators. Create fixed voltage regulators for these supplies.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- arch/arm/mach-omap2/board-4430sdp.c | 58 +++++++++++++++++++++++++++++++++++ 1 files changed, 58 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-omap2/board-4430sdp.c b/arch/arm/mach-omap2/board-4430sdp.c index d02172d..5de5753 100644 --- a/arch/arm/mach-omap2/board-4430sdp.c +++ b/arch/arm/mach-omap2/board-4430sdp.c @@ -64,6 +64,8 @@
#define FIXED_REG_VBAT_ID 0 #define FIXED_REG_VWLAN_ID 1 +#define FIXED_REG_V2V1_ID 2 +#define FIXED_REG_V1V8_ID 3
static const int sdp4430_keymap[] = { KEY(0, 0, KEY_E), @@ -349,6 +351,60 @@ static int __init omap_ethernet_init(void) return status; }
+static struct regulator_consumer_supply sdp4430_vpmic_v2v1_supply[] = { + REGULATOR_SUPPLY("v2v1", "1-004b"), +}; + +static struct regulator_init_data sdp4430_v2v1smps = { + .constraints = { + .always_on = true, + }, + .num_consumer_supplies = ARRAY_SIZE(sdp4430_vpmic_v2v1_supply), + .consumer_supplies = sdp4430_vpmic_v2v1_supply, +}; + +static struct fixed_voltage_config sdp4430_v2v1_pdata = { + .supply_name = "VPMIC-V2V1", + .microvolts = 2100000, + .init_data = &sdp4430_v2v1smps, + .gpio = -EINVAL, +}; + +static struct platform_device sdp4430_v2v1 = { + .name = "reg-fixed-voltage", + .id = FIXED_REG_V2V1_ID, + .dev = { + .platform_data = &sdp4430_v2v1_pdata, + }, +}; + +static struct regulator_consumer_supply sdp4430_vpmic_v1v8_supply[] = { + REGULATOR_SUPPLY("vio", "1-004b"), +}; + +static struct regulator_init_data sdp4430_v1v8smps = { + .constraints = { + .always_on = true, + }, + .num_consumer_supplies = ARRAY_SIZE(sdp4430_vpmic_v1v8_supply), + .consumer_supplies = sdp4430_vpmic_v1v8_supply, +}; + +static struct fixed_voltage_config sdp4430_v1v8_pdata = { + .supply_name = "VPMIC-V1V8", + .microvolts = 1800000, + .init_data = &sdp4430_v1v8smps, + .gpio = -EINVAL, +}; + +static struct platform_device sdp4430_v1v8 = { + .name = "reg-fixed-voltage", + .id = FIXED_REG_V1V8_ID, + .dev = { + .platform_data = &sdp4430_v1v8_pdata, + }, +}; + static struct regulator_consumer_supply sdp4430_vbat_supply[] = { REGULATOR_SUPPLY("vddvibl", "twl6040-vibra"), REGULATOR_SUPPLY("vddvibr", "twl6040-vibra"), @@ -416,6 +472,8 @@ static struct platform_device *sdp4430_devices[] __initdata = { &sdp4430_vbat, &sdp4430_dmic_codec, &sdp4430_abe_audio, + &sdp4430_v1v8, + &sdp4430_v2v1, };
static struct omap_musb_board_data musb_board_data = {
On Thu, Feb 02, 2012 at 02:16:57PM +0200, Peter Ujfalusi wrote:
The twl6040 VIO power is connected to SMPS V1V8, the V2V1 power is coming from SMPS V2V1 of twl6030. These are fixed, always on regulators. Create fixed voltage regulators for these supplies.
Shouldn't the PMIC be creating the relevant fixed voltage regulators? It's a bit clearer given that the supplies actually come from the PMIC and it would cut down a bit on the boilerplate in boards (though much less so with device tree, sadly).
On 02/02/2012 02:40 PM, Mark Brown wrote:
On Thu, Feb 02, 2012 at 02:16:57PM +0200, Peter Ujfalusi wrote:
The twl6040 VIO power is connected to SMPS V1V8, the V2V1 power is coming from SMPS V2V1 of twl6030. These are fixed, always on regulators. Create fixed voltage regulators for these supplies.
Shouldn't the PMIC be creating the relevant fixed voltage regulators? It's a bit clearer given that the supplies actually come from the PMIC and it would cut down a bit on the boilerplate in boards (though much less so with device tree, sadly).
Yes these are coming from twl6030, so it might be better to create the fixed regulators in the PMIC driver. I'll take a look.
Thanks, Péter
The twl6040 VIO power is connected to SMPS V1V8, the V2V1 power is coming from SMPS V2V1 of twl6030. These are fixed, always on regulators. Create fixed voltage regulators for these supplies.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- arch/arm/mach-omap2/board-omap4panda.c | 62 +++++++++++++++++++++++++++++++- 1 files changed, 61 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-omap2/board-omap4panda.c b/arch/arm/mach-omap2/board-omap4panda.c index da31a72..4d66c60 100644 --- a/arch/arm/mach-omap2/board-omap4panda.c +++ b/arch/arm/mach-omap2/board-omap4panda.c @@ -56,6 +56,10 @@ #define HDMI_GPIO_HPD 60 /* Hot plug pin for HDMI */ #define HDMI_GPIO_LS_OE 41 /* Level shifter for HDMI */
+#define FIXED_REG_VWLAN_ID 0 +#define FIXED_REG_V2V1_ID 1 +#define FIXED_REG_V1V8_ID 2 + /* wl127x BT, FM, GPS connectivity chip */ static int wl1271_gpios[] = {46, -1, -1}; static struct platform_device wl1271_device = { @@ -116,10 +120,66 @@ static struct platform_device panda_abe_audio = { }, };
+static struct regulator_consumer_supply panda_vdd_v2v1_supply[] = { + REGULATOR_SUPPLY("v2v1", "1-004b"), +}; + +static struct regulator_init_data panda_v2v1smps = { + .constraints = { + .always_on = true, + }, + .num_consumer_supplies = ARRAY_SIZE(panda_vdd_v2v1_supply), + .consumer_supplies = panda_vdd_v2v1_supply, +}; + +static struct fixed_voltage_config panda_v2v1_pdata = { + .supply_name = "VDD-V2V1", + .microvolts = 2100000, + .init_data = &panda_v2v1smps, + .gpio = -EINVAL, +}; + +static struct platform_device panda_v2v1 = { + .name = "reg-fixed-voltage", + .id = FIXED_REG_V2V1_ID, + .dev = { + .platform_data = &panda_v2v1_pdata, + }, +}; + +static struct regulator_consumer_supply panda_vio_v1v8_supply[] = { + REGULATOR_SUPPLY("vio", "1-004b"), +}; + +static struct regulator_init_data panda_v1v8smps = { + .constraints = { + .always_on = true, + }, + .num_consumer_supplies = ARRAY_SIZE(panda_vio_v1v8_supply), + .consumer_supplies = panda_vio_v1v8_supply, +}; + +static struct fixed_voltage_config panda_v1v8_pdata = { + .supply_name = "VIO-V1V8", + .microvolts = 1800000, + .init_data = &panda_v1v8smps, + .gpio = -EINVAL, +}; + +static struct platform_device panda_v1v8 = { + .name = "reg-fixed-voltage", + .id = FIXED_REG_V1V8_ID, + .dev = { + .platform_data = &panda_v1v8_pdata, + }, +}; + static struct platform_device *panda_devices[] __initdata = { &leds_gpio, &wl1271_device, &panda_abe_audio, + &panda_v1v8, + &panda_v2v1, };
static const struct usbhs_omap_board_data usbhs_bdata __initconst = { @@ -218,7 +278,7 @@ static struct fixed_voltage_config panda_vwlan = {
static struct platform_device omap_vwlan_device = { .name = "reg-fixed-voltage", - .id = 1, + .id = FIXED_REG_VWLAN_ID, .dev = { .platform_data = &panda_vwlan, },
On Thu, Feb 02, 2012 at 02:16:58PM +0200, Peter Ujfalusi wrote:
The twl6040 VIO power is connected to SMPS V1V8, the V2V1 power is coming from SMPS V2V1 of twl6030. These are fixed, always on regulators. Create fixed voltage regulators for these supplies.
Same issue as previously, shouldn't the PMIC create the regulators?
twl6040 has three power supply source: VBAT needs to be connected to VBAT VIO, and V2V1. Add regulator support for the VIO, V2V1 supplies.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- drivers/mfd/twl6040-core.c | 41 +++++++++++++++++++++++++++++++++++++++++ include/linux/mfd/twl6040.h | 2 ++ 2 files changed, 43 insertions(+), 0 deletions(-)
diff --git a/drivers/mfd/twl6040-core.c b/drivers/mfd/twl6040-core.c index 21d6a99..04568ea 100644 --- a/drivers/mfd/twl6040-core.c +++ b/drivers/mfd/twl6040-core.c @@ -27,12 +27,14 @@ #include <linux/types.h> #include <linux/slab.h> #include <linux/kernel.h> +#include <linux/err.h> #include <linux/platform_device.h> #include <linux/gpio.h> #include <linux/delay.h> #include <linux/i2c.h> #include <linux/mfd/core.h> #include <linux/mfd/twl6040.h> +#include <linux/regulator/consumer.h>
#define VIBRACTRL_MEMBER(reg) ((reg == TWL6040_REG_VIBCTLL) ? 0 : 1)
@@ -541,6 +543,33 @@ static int __devinit twl6040_probe(struct i2c_client *client,
i2c_set_clientdata(client, twl6040);
+ twl6040->vio = regulator_get(&client->dev, "vio"); + if (IS_ERR(twl6040->vio)) { + ret = PTR_ERR(twl6040->vio); + dev_err(&client->dev, "Failed to request VIO supply: %d\n", + ret); + goto regulator_get_err; + } + twl6040->v2v1 = regulator_get(&client->dev, "v2v1"); + if (IS_ERR(twl6040->v2v1)) { + ret = PTR_ERR(twl6040->v2v1); + dev_err(&client->dev, "Failed to request V2V1 supply: %d\n", + ret); + regulator_put(twl6040->vio); + goto regulator_get_err; + } + ret = regulator_enable(twl6040->vio); + if (ret != 0) { + dev_err(&client->dev, "Failed to enable VIO: %d\n", ret); + goto power_err; + } + ret = regulator_enable(twl6040->v2v1); + if (ret != 0) { + dev_err(&client->dev, "Failed to enable V2V1: %d\n", ret); + regulator_disable(twl6040->vio); + goto power_err; + } + twl6040->dev = &client->dev; twl6040->control_data = client; twl6040->irq = client->irq; @@ -632,6 +661,12 @@ gpio2_err: if (gpio_is_valid(twl6040->audpwron)) gpio_free(twl6040->audpwron); gpio1_err: + regulator_disable(twl6040->v2v1); + regulator_disable(twl6040->vio); +power_err: + regulator_put(twl6040->vio); + regulator_put(twl6040->v2v1); +regulator_get_err: i2c_set_clientdata(client, NULL); kfree(twl6040); return ret; @@ -652,6 +687,12 @@ static int __devexit twl6040_remove(struct i2c_client *client)
mfd_remove_devices(&client->dev); i2c_set_clientdata(client, NULL); + + regulator_disable(twl6040->v2v1); + regulator_disable(twl6040->vio); + regulator_put(twl6040->vio); + regulator_put(twl6040->v2v1); + kfree(twl6040);
return 0; diff --git a/include/linux/mfd/twl6040.h b/include/linux/mfd/twl6040.h index 72f530f..146de4e 100644 --- a/include/linux/mfd/twl6040.h +++ b/include/linux/mfd/twl6040.h @@ -201,6 +201,8 @@ struct twl6040_platform_data { struct twl6040 { struct i2c_client *control_data; struct device *dev; + struct regulator *vio; + struct regulator *v2v1; struct mutex mutex; struct mutex io_mutex; struct mutex irq_mutex;
On Thu, Feb 02, 2012 at 02:16:59PM +0200, Peter Ujfalusi wrote:
- twl6040->vio = regulator_get(&client->dev, "vio");
- if (IS_ERR(twl6040->vio)) {
ret = PTR_ERR(twl6040->vio);dev_err(&client->dev, "Failed to request VIO supply: %d\n",ret);goto regulator_get_err;- }
- twl6040->v2v1 = regulator_get(&client->dev, "v2v1");
- if (IS_ERR(twl6040->v2v1)) {
ret = PTR_ERR(twl6040->v2v1);dev_err(&client->dev, "Failed to request V2V1 supply: %d\n",ret);regulator_put(twl6040->vio);goto regulator_get_err;- }
Looks like you want regulator_bulk_get() here. Or (better yet though it'd be a potential issue for merge via MFD and the benefits aren't that exciting since you still need to disable) devm_regulator_bulk_get().
- ret = regulator_enable(twl6040->vio);
- if (ret != 0) {
dev_err(&client->dev, "Failed to enable VIO: %d\n", ret);goto power_err;- }
- ret = regulator_enable(twl6040->v2v1);
- if (ret != 0) {
dev_err(&client->dev, "Failed to enable V2V1: %d\n", ret);regulator_disable(twl6040->vio);goto power_err;- }
Similarly regulator_bulk_enable() here, and it'll fix...
gpio1_err:
- regulator_disable(twl6040->v2v1);
- regulator_disable(twl6040->vio);
...the fact that if you fail to enable the v2.1 regulator you don't disable vio.
On 02/02/2012 02:52 PM, Mark Brown wrote:
On Thu, Feb 02, 2012 at 02:16:59PM +0200, Peter Ujfalusi wrote:
- twl6040->vio = regulator_get(&client->dev, "vio");
- if (IS_ERR(twl6040->vio)) {
ret = PTR_ERR(twl6040->vio);dev_err(&client->dev, "Failed to request VIO supply: %d\n",ret);goto regulator_get_err;- }
- twl6040->v2v1 = regulator_get(&client->dev, "v2v1");
- if (IS_ERR(twl6040->v2v1)) {
ret = PTR_ERR(twl6040->v2v1);dev_err(&client->dev, "Failed to request V2V1 supply: %d\n",ret);regulator_put(twl6040->vio);goto regulator_get_err;- }
Looks like you want regulator_bulk_get() here. Or (better yet though it'd be a potential issue for merge via MFD and the benefits aren't that exciting since you still need to disable) devm_regulator_bulk_get().
I need separate control for the two power source since we can hit different power levels depending on which is powered/not powered: power down: vio, v2v1 is OFF deep sleep: vio is ON, v2v1 is OFF sleep/power on: vio, v2v1 is ON
And we have certain sequence to move between power states.
- ret = regulator_enable(twl6040->vio);
- if (ret != 0) {
dev_err(&client->dev, "Failed to enable VIO: %d\n", ret);goto power_err;- }
- ret = regulator_enable(twl6040->v2v1);
- if (ret != 0) {
dev_err(&client->dev, "Failed to enable V2V1: %d\n", ret);regulator_disable(twl6040->vio);
I disable the vio here, if the v2v1 enable fails.
goto power_err;- }
Similarly regulator_bulk_enable() here, and it'll fix...
gpio1_err:
- regulator_disable(twl6040->v2v1);
- regulator_disable(twl6040->vio);
...the fact that if you fail to enable the v2.1 regulator you don't disable vio.
It is handled within the if(){} With the devm_regulator this will be much nicer for sure.
On Thu, Feb 02, 2012 at 03:18:09PM +0200, Peter Ujfalusi wrote:
On 02/02/2012 02:52 PM, Mark Brown wrote:
Looks like you want regulator_bulk_get() here. Or (better yet though it'd be a potential issue for merge via MFD and the benefits aren't that exciting since you still need to disable) devm_regulator_bulk_get().
I need separate control for the two power source since we can hit different power levels depending on which is powered/not powered: power down: vio, v2v1 is OFF deep sleep: vio is ON, v2v1 is OFF sleep/power on: vio, v2v1 is ON
And we have certain sequence to move between power states.
That's not a problem for using the bulk get - the array is part of the API so you can use regulator_bulk_get() and still look at individual supplies within the array later on when enabling and disabling them.
- ret = regulator_enable(twl6040->vio);
- if (ret != 0) {
dev_err(&client->dev, "Failed to enable VIO: %d\n", ret);goto power_err;- }
- ret = regulator_enable(twl6040->v2v1);
- if (ret != 0) {
dev_err(&client->dev, "Failed to enable V2V1: %d\n", ret);regulator_disable(twl6040->vio);
I disable the vio here, if the v2v1 enable fails.
Oh, that's quite confusing when mixed in with the goto/unwind - it'd be clearer to have the extra lable to jump to.
On 02/02/2012 03:32 PM, Mark Brown wrote:
That's not a problem for using the bulk get - the array is part of the API so you can use regulator_bulk_get() and still look at individual supplies within the array later on when enabling and disabling them.
For some reason I have associated the use of regulator_bulk_get with the use of regulator_bulk_enable/disable. It did not even crossed my mind that I can still use regulator_enable on the individual regulators. Will convert the regulator_get/put to bulk operations. It will make the code a bit cleaner. Thanks
- ret = regulator_enable(twl6040->vio);
- if (ret != 0) {
dev_err(&client->dev, "Failed to enable VIO: %d\n", ret);goto power_err;- }
- ret = regulator_enable(twl6040->v2v1);
- if (ret != 0) {
dev_err(&client->dev, "Failed to enable V2V1: %d\n", ret);regulator_disable(twl6040->vio);I disable the vio here, if the v2v1 enable fails.
Oh, that's quite confusing when mixed in with the goto/unwind - it'd be clearer to have the extra lable to jump to.
I guess this is a matter of taste. However I have found some inconsistent naming with the exit labels, which I'm going to fix for the next series.
On Thu, Feb 02, 2012 at 03:59:20PM +0200, Peter Ujfalusi wrote:
On 02/02/2012 03:32 PM, Mark Brown wrote:
That's not a problem for using the bulk get - the array is part of the API so you can use regulator_bulk_get() and still look at individual supplies within the array later on when enabling and disabling them.
For some reason I have associated the use of regulator_bulk_get with the use of regulator_bulk_enable/disable. It did not even crossed my mind that I can still use regulator_enable on the individual regulators.
They do normally end up going together since a very large proportion of devices need all their supplies on when active but don't care about sequencing so you end up with a common pattern. It's not required, though.
Will convert the regulator_get/put to bulk operations. It will make the code a bit cleaner.
Good stuff.
participants (2)
-
Mark Brown -
Peter Ujfalusi