* [PATCH 0/3] Fixes for tps65090 for Samsung ARM Chromebook @ 2014-04-15 20:14 Doug Anderson 2014-04-15 20:14 ` [PATCH 3/3] regulator: tps65090: Make FETs more reliable Doug Anderson 2014-04-16 18:25 ` [PATCH v2 0/5] Fixes for tps65090 for Samsung ARM Chromebook Doug Anderson 0 siblings, 2 replies; 8+ messages in thread From: Doug Anderson @ 2014-04-15 20:14 UTC (permalink / raw) To: Anton Vorontsov Cc: Olof Johansson, Sachin Kamat, ajaykumar.rs, linux-samsung-soc, Doug Anderson, devicetree, Randy Dunlap, Mark Brown, Simon Glass, linux-doc, Liam Girdwood, Samuel Ortiz, linux-kernel, Kumar Gala, Ian Campbell, Dmitry Eremin-Solenikov, Rob Herring, David Woodhouse, Pawel Moll, Lee Jones, Mark Rutland, Sean Paul, Michael Spang These three patches bring tps65090 up to speed with what's currently in the Chromium OS kernel 3.8 tree and running on the Samsung ARM Chromebook. Changes were tested atop the current linux tree (v3.15-rc1). FET retries were tested on a machine with a known flaky tps65090. Since display isn't working on pure upstream, I augmented the code to turn FET1 (vcd_led) on/off 500 times at bootup. When testing I included <https://patchwork.kernel.org/patch/3980731/> to make sure tps65090 was in exynos5250-snow's device tree. Doug Anderson (3): mfd: tps65090: Allow charger module to be used when no irq mfd: tps65090: Stop caching registers regulator: tps65090: Make FETs more reliable .../devicetree/bindings/regulator/tps65090.txt | 4 + drivers/mfd/tps65090.c | 24 ++- drivers/power/tps65090-charger.c | 76 ++++++-- drivers/regulator/tps65090-regulator.c | 197 +++++++++++++++++++-- include/linux/mfd/tps65090.h | 5 + 5 files changed, 264 insertions(+), 42 deletions(-) -- 1.9.1.423.g4596e3a ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/3] regulator: tps65090: Make FETs more reliable 2014-04-15 20:14 [PATCH 0/3] Fixes for tps65090 for Samsung ARM Chromebook Doug Anderson @ 2014-04-15 20:14 ` Doug Anderson [not found] ` <1397592876-5741-4-git-send-email-dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> 2014-04-16 18:25 ` [PATCH v2 0/5] Fixes for tps65090 for Samsung ARM Chromebook Doug Anderson 1 sibling, 1 reply; 8+ messages in thread From: Doug Anderson @ 2014-04-15 20:14 UTC (permalink / raw) To: Anton Vorontsov Cc: Olof Johansson, Sachin Kamat, ajaykumar.rs, linux-samsung-soc, Doug Anderson, Simon Glass, Michael Spang, Sean Paul, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Randy Dunlap, Liam Girdwood, Mark Brown, Samuel Ortiz, Lee Jones, devicetree, linux-doc, linux-kernel An issue was discovered with tps65090 where sometimes the FETs wouldn't actually turn on when requested. The most problematic FET was the one use for the LCD backlight on the Samsung ARM Chromebook (FET1). Problems were especially prevalent when the device was plugged in to AC power, making the backlight voltage higher. Mitigate the problem by: * Allow setting the overcurrent wait time so devices with this problem can set it to the max. * Add retry logic on enables of FETs. Signed-off-by: Doug Anderson <dianders@chromium.org> Signed-off-by: Simon Glass <sjg@chromium.org> Signed-off-by: Michael Spang <spang@chromium.org> Signed-off-by: Sean Paul <seanpaul@chromium.org> --- .../devicetree/bindings/regulator/tps65090.txt | 4 + drivers/regulator/tps65090-regulator.c | 197 +++++++++++++++++++-- include/linux/mfd/tps65090.h | 5 + 3 files changed, 194 insertions(+), 12 deletions(-) diff --git a/Documentation/devicetree/bindings/regulator/tps65090.txt b/Documentation/devicetree/bindings/regulator/tps65090.txt index 313a60b..34098023 100644 --- a/Documentation/devicetree/bindings/regulator/tps65090.txt +++ b/Documentation/devicetree/bindings/regulator/tps65090.txt @@ -21,6 +21,10 @@ Optional properties: number should be provided. If it is externally controlled and no GPIO entry then driver will just configure this rails as external control and will not provide any enable/disable APIs. +- ti,overcurrent-wait: This is applicable to FET registers, which have a + poorly defined "overcurrent wait" field. If this property is present it + should be between 0 - 3. If this property isn't present we won't touch the + "overcurrent wait" field and we'll leave it to the BIOS/EC to deal with. Each regulator is defined using the standard binding for regulators. diff --git a/drivers/regulator/tps65090-regulator.c b/drivers/regulator/tps65090-regulator.c index 2e92ef6..e8d1c62 100644 --- a/drivers/regulator/tps65090-regulator.c +++ b/drivers/regulator/tps65090-regulator.c @@ -17,6 +17,7 @@ */ #include <linux/module.h> +#include <linux/delay.h> #include <linux/init.h> #include <linux/gpio.h> #include <linux/of_gpio.h> @@ -28,21 +29,186 @@ #include <linux/regulator/of_regulator.h> #include <linux/mfd/tps65090.h> +#define MAX_CTRL_READ_TRIES 5 +#define MAX_FET_ENABLE_TRIES 1000 + +#define CTRL_EN_BIT 0 /* Regulator enable bit, active high */ +#define CTRL_WT_BIT 2 /* Regulator wait time 0 bit */ +#define CTRL_PG_BIT 4 /* Regulator power good bit, 1=good */ +#define CTRL_TO_BIT 7 /* Regulator timeout bit, 1=wait */ + +#define MAX_OVERCURRENT_WAIT 3 /* Overcurrent wait must be less than this */ + +/** + * struct tps65090_regulator - Per-regulator data for a tps65090 regulator + * + * @dev: Pointer to our device. + * @desc: The struct regulator_desc for the regulator. + * @rdev: The struct regulator_dev for the regulator. + * @overcurrent_wait_valid: True if overcurrent_wait is valid. + * @overcurrent_wait: For FETs, the value to put in the WTFET bitfield. + */ + struct tps65090_regulator { struct device *dev; struct regulator_desc *desc; struct regulator_dev *rdev; + bool overcurrent_wait_valid; + int overcurrent_wait; }; static struct regulator_ops tps65090_ext_control_ops = { }; -static struct regulator_ops tps65090_reg_contol_ops = { +/** + * tps65090_fet_is_enabled - Tell if a fet is enabled + * + * @rdev: Regulator device + * @return true if the fet is enabled and its power is good; false otherwise. + */ +static int tps65090_fet_is_enabled(struct regulator_dev *rdev) +{ + unsigned int control; + unsigned int expected = rdev->desc->enable_mask | BIT(CTRL_PG_BIT); + int ret; + + ret = regmap_read(rdev->regmap, rdev->desc->enable_reg, &control); + if (ret != 0) + return ret; + + return (control & expected) == expected; +} + +/** + * tps65090_reg_set_overcurrent_wait - Setup overcurrent wait + * + * This will set the overcurrent wait time based on what's in the regulator + * info. + * + * @rdev: Regulator device + * @return 0 if no error, non-zero if there was an error writing the register. + */ +static int tps65090_reg_set_overcurrent_wait(struct regulator_dev *rdev) +{ + struct tps65090_regulator *ri = rdev_get_drvdata(rdev); + int ret; + + if (!ri->overcurrent_wait_valid) + return 0; + + ret = regmap_update_bits(rdev->regmap, rdev->desc->enable_reg, + MAX_OVERCURRENT_WAIT << CTRL_WT_BIT, + ri->overcurrent_wait << CTRL_WT_BIT); + if (ret) { + dev_err(&rdev->dev, "Error updating overcurrent wait %#x\n", + rdev->desc->enable_reg); + } + + return ret; +} + +/** + * tps6090_try_enable_fet - Try to enable a FET + * + * @rdev: Regulator device + * @return 0 if ok, -ENOTRECOVERABLE if the FET power good bit did not get set, + * or some other -ve value if another error occurred (e.g. i2c error) + */ +static int tps6090_try_enable_fet(struct regulator_dev *rdev) +{ + unsigned int control; + int ret, i; + + ret = regmap_update_bits(rdev->regmap, rdev->desc->enable_reg, + rdev->desc->enable_mask, + rdev->desc->enable_mask); + if (ret < 0) { + dev_err(&rdev->dev, "Error in updating reg %#x\n", + rdev->desc->enable_reg); + return ret; + } + + for (i = 0; i < MAX_CTRL_READ_TRIES; i++) { + ret = regmap_read(rdev->regmap, rdev->desc->enable_reg, + &control); + if (ret < 0) + return ret; + + if (!(control & BIT(CTRL_TO_BIT))) + break; + + usleep_range(1000, 1500); + } + if (!(control & BIT(CTRL_PG_BIT))) + return -ENOTRECOVERABLE; + + return 0; +} + +/** + * tps65090_fet_enable - Enable a FET, trying a few times if it fails + * + * Some versions of the tps65090 have issues when turning on the FETs. + * This function goes through several steps to ensure the best chance of the + * FET going on. Specifically: + * - We'll make sure that we bump the "overcurrent wait" to the maximum, which + * increases the chances that we'll turn on properly. + * - We'll retry turning the FET on multiple times (turning off in between). + * + * @rdev: Regulator device + * @return 0 if ok, non-zero if it fails. + */ +static int tps65090_fet_enable(struct regulator_dev *rdev) +{ + int ret, tries; + + ret = tps65090_reg_set_overcurrent_wait(rdev); + if (ret) + goto err; + + /* + * Try enabling multiple times until we succeed since sometimes the + * first try times out. + */ + for (tries = 0; ; tries++) { + ret = tps6090_try_enable_fet(rdev); + if (!ret) + break; + if (ret != -ENOTRECOVERABLE || tries == MAX_FET_ENABLE_TRIES) + goto err; + + /* Try turning the FET off (and then on again) */ + ret = regmap_update_bits(rdev->regmap, rdev->desc->enable_reg, + rdev->desc->enable_mask, 0); + if (ret) + goto err; + } + + if (tries) { + dev_warn(&rdev->dev, "reg %#x enable ok after %d tries\n", + rdev->desc->enable_reg, tries); + } + + return 0; +err: + dev_warn(&rdev->dev, "reg %#x enable failed\n", rdev->desc->enable_reg); + WARN_ON(1); + + return ret; +} + +static struct regulator_ops tps65090_reg_control_ops = { .enable = regulator_enable_regmap, .disable = regulator_disable_regmap, .is_enabled = regulator_is_enabled_regmap, }; +static struct regulator_ops tps65090_fet_control_ops = { + .enable = tps65090_fet_enable, + .disable = regulator_disable_regmap, + .is_enabled = tps65090_fet_is_enabled, +}; + static struct regulator_ops tps65090_ldo_ops = { }; @@ -53,22 +219,22 @@ static struct regulator_ops tps65090_ldo_ops = { .id = TPS65090_REGULATOR_##_id, \ .ops = &_ops, \ .enable_reg = _en_reg, \ - .enable_mask = BIT(0), \ + .enable_mask = BIT(CTRL_EN_BIT), \ .type = REGULATOR_VOLTAGE, \ .owner = THIS_MODULE, \ } static struct regulator_desc tps65090_regulator_desc[] = { - tps65090_REG_DESC(DCDC1, "vsys1", 0x0C, tps65090_reg_contol_ops), - tps65090_REG_DESC(DCDC2, "vsys2", 0x0D, tps65090_reg_contol_ops), - tps65090_REG_DESC(DCDC3, "vsys3", 0x0E, tps65090_reg_contol_ops), - tps65090_REG_DESC(FET1, "infet1", 0x0F, tps65090_reg_contol_ops), - tps65090_REG_DESC(FET2, "infet2", 0x10, tps65090_reg_contol_ops), - tps65090_REG_DESC(FET3, "infet3", 0x11, tps65090_reg_contol_ops), - tps65090_REG_DESC(FET4, "infet4", 0x12, tps65090_reg_contol_ops), - tps65090_REG_DESC(FET5, "infet5", 0x13, tps65090_reg_contol_ops), - tps65090_REG_DESC(FET6, "infet6", 0x14, tps65090_reg_contol_ops), - tps65090_REG_DESC(FET7, "infet7", 0x15, tps65090_reg_contol_ops), + tps65090_REG_DESC(DCDC1, "vsys1", 0x0C, tps65090_reg_control_ops), + tps65090_REG_DESC(DCDC2, "vsys2", 0x0D, tps65090_reg_control_ops), + tps65090_REG_DESC(DCDC3, "vsys3", 0x0E, tps65090_reg_control_ops), + tps65090_REG_DESC(FET1, "infet1", 0x0F, tps65090_fet_control_ops), + tps65090_REG_DESC(FET2, "infet2", 0x10, tps65090_fet_control_ops), + tps65090_REG_DESC(FET3, "infet3", 0x11, tps65090_fet_control_ops), + tps65090_REG_DESC(FET4, "infet4", 0x12, tps65090_fet_control_ops), + tps65090_REG_DESC(FET5, "infet5", 0x13, tps65090_fet_control_ops), + tps65090_REG_DESC(FET6, "infet6", 0x14, tps65090_fet_control_ops), + tps65090_REG_DESC(FET7, "infet7", 0x15, tps65090_fet_control_ops), tps65090_REG_DESC(LDO1, "vsys-l1", 0, tps65090_ldo_ops), tps65090_REG_DESC(LDO2, "vsys-l2", 0, tps65090_ldo_ops), }; @@ -209,6 +375,11 @@ static struct tps65090_platform_data *tps65090_parse_dt_reg_data( rpdata->gpio = of_get_named_gpio(np, "dcdc-ext-control-gpios", 0); + if (of_property_read_u32(tps65090_matches[idx].of_node, + "ti,overcurrent-wait", + &rpdata->overcurrent_wait) == 0) + rpdata->overcurrent_wait_valid = true; + tps65090_pdata->reg_pdata[idx] = rpdata; } return tps65090_pdata; @@ -258,6 +429,8 @@ static int tps65090_regulator_probe(struct platform_device *pdev) ri = &pmic[num]; ri->dev = &pdev->dev; ri->desc = &tps65090_regulator_desc[num]; + ri->overcurrent_wait_valid = tps_pdata->overcurrent_wait_valid; + ri->overcurrent_wait = tps_pdata->overcurrent_wait; /* * TPS5090 DCDC support the control from external digital input. diff --git a/include/linux/mfd/tps65090.h b/include/linux/mfd/tps65090.h index 3f43069..f25adfa 100644 --- a/include/linux/mfd/tps65090.h +++ b/include/linux/mfd/tps65090.h @@ -78,11 +78,16 @@ struct tps65090 { * DCDC1, DCDC2 and DCDC3. * @gpio: Gpio number if external control is enabled and controlled through * gpio. + * @overcurrent_wait_valid: True if the overcurrent_wait should be applied. + * @overcurrent_wait: Value to set as the overcurrent wait time. This is the + * actual bitfield value, not a time in ms (valid value are 0 - 3). */ struct tps65090_regulator_plat_data { struct regulator_init_data *reg_init_data; bool enable_ext_control; int gpio; + bool overcurrent_wait_valid; + int overcurrent_wait; }; struct tps65090_platform_data { -- 1.9.1.423.g4596e3a ^ permalink raw reply related [flat|nested] 8+ messages in thread
[parent not found: <1397592876-5741-4-git-send-email-dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>]
* Re: [PATCH 3/3] regulator: tps65090: Make FETs more reliable [not found] ` <1397592876-5741-4-git-send-email-dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> @ 2014-04-15 22:52 ` Mark Brown 2014-04-16 18:28 ` Doug Anderson 0 siblings, 1 reply; 8+ messages in thread From: Mark Brown @ 2014-04-15 22:52 UTC (permalink / raw) To: Doug Anderson Cc: Anton Vorontsov, Olof Johansson, Sachin Kamat, ajaykumar.rs-Sze3O3UU22JBDgjK7y7TUQ, linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA, Simon Glass, Michael Spang, Sean Paul, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Randy Dunlap, Liam Girdwood, Samuel Ortiz, Lee Jones, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-doc-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA [-- Attachment #1: Type: text/plain, Size: 1933 bytes --] On Tue, Apr 15, 2014 at 01:14:36PM -0700, Doug Anderson wrote: > Mitigate the problem by: > * Allow setting the overcurrent wait time so devices with this problem > can set it to the max. > * Add retry logic on enables of FETs. This is two changes, should really be two patches. > +- ti,overcurrent-wait: This is applicable to FET registers, which have a > + poorly defined "overcurrent wait" field. If this property is present it > + should be between 0 - 3. If this property isn't present we won't touch the > + "overcurrent wait" field and we'll leave it to the BIOS/EC to deal with. I take it this is the raw value to write to the register? > +static int tps65090_fet_is_enabled(struct regulator_dev *rdev) > +{ > + unsigned int control; > + unsigned int expected = rdev->desc->enable_mask | BIT(CTRL_PG_BIT); > + int ret; > + > + ret = regmap_read(rdev->regmap, rdev->desc->enable_reg, &control); > + if (ret != 0) > + return ret; > + > + return (control & expected) == expected; > +} No need to open code this, regulator_is_enabled_regmap() can check for any value in a bitfield. > +static int tps6090_try_enable_fet(struct regulator_dev *rdev) Why is this called tps6090_try_enable_fet(), looks like a missing 5? > + /* > + * Try enabling multiple times until we succeed since sometimes the > + * first try times out. > + */ > + for (tries = 0; ; tries++) { > + ret = tps6090_try_enable_fet(rdev); > + if (!ret) > + break; > + if (ret != -ENOTRECOVERABLE || tries == MAX_FET_ENABLE_TRIES) > + goto err; Make this a do { } while so we don't have the exit condition missing in the for loop please, it's doing the right thing but it's not as obvious as it could be. > + if (tries) { > + dev_warn(&rdev->dev, "reg %#x enable ok after %d tries\n", > + rdev->desc->enable_reg, tries); > + } No need for braces here, and I guess that ought to be retries rather than tries (though that is pedantry). [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] regulator: tps65090: Make FETs more reliable 2014-04-15 22:52 ` Mark Brown @ 2014-04-16 18:28 ` Doug Anderson 0 siblings, 0 replies; 8+ messages in thread From: Doug Anderson @ 2014-04-16 18:28 UTC (permalink / raw) To: Mark Brown Cc: Anton Vorontsov, Olof Johansson, Sachin Kamat, AJAY KUMAR RAMAKRISHNA SHYMALAMMA, linux-samsung-soc, Simon Glass, Michael Spang, Sean Paul, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Randy Dunlap, Liam Girdwood, Samuel Ortiz, Lee Jones, devicetree@vger.kernel.org, linux-doc, linux-kernel@vger.kernel.org Mark, On Tue, Apr 15, 2014 at 3:52 PM, Mark Brown <broonie@kernel.org> wrote: > On Tue, Apr 15, 2014 at 01:14:36PM -0700, Doug Anderson wrote: > >> Mitigate the problem by: >> * Allow setting the overcurrent wait time so devices with this problem >> can set it to the max. >> * Add retry logic on enables of FETs. > > This is two changes, should really be two patches. OK, sure. >> +- ti,overcurrent-wait: This is applicable to FET registers, which have a >> + poorly defined "overcurrent wait" field. If this property is present it >> + should be between 0 - 3. If this property isn't present we won't touch the >> + "overcurrent wait" field and we'll leave it to the BIOS/EC to deal with. > > I take it this is the raw value to write to the register? Yes. >> +static int tps65090_fet_is_enabled(struct regulator_dev *rdev) >> +{ >> + unsigned int control; >> + unsigned int expected = rdev->desc->enable_mask | BIT(CTRL_PG_BIT); >> + int ret; >> + >> + ret = regmap_read(rdev->regmap, rdev->desc->enable_reg, &control); >> + if (ret != 0) >> + return ret; >> + >> + return (control & expected) == expected; >> +} > > No need to open code this, regulator_is_enabled_regmap() can check for > any value in a bitfield. The overall problem was that we were checking a different bit than we were setting. We set "enabled" to turn things on and then we check "power good". I can avoid the open coding by doing something that's slightly a hack. I'll put that in V2 and you can tell me if you like it better. I'll set "enable_mask" and "enable_val" to include both bits. The "power good" is read only so it won't hurt to set it. ...and it doesn't hurt to check the enabled bit in addition to the power good bit. >> +static int tps6090_try_enable_fet(struct regulator_dev *rdev) > > Why is this called tps6090_try_enable_fet(), looks like a missing 5? typo. fixed. > >> + /* >> + * Try enabling multiple times until we succeed since sometimes the >> + * first try times out. >> + */ >> + for (tries = 0; ; tries++) { >> + ret = tps6090_try_enable_fet(rdev); >> + if (!ret) >> + break; >> + if (ret != -ENOTRECOVERABLE || tries == MAX_FET_ENABLE_TRIES) >> + goto err; > > Make this a do { } while so we don't have the exit condition missing in > the for loop please, it's doing the right thing but it's not as obvious > as it could be. It's not quite a "do { } while" since the break is in the middle, but happy to change to a "while (true)". Hope that's OK. > >> + if (tries) { >> + dev_warn(&rdev->dev, "reg %#x enable ok after %d tries\n", >> + rdev->desc->enable_reg, tries); >> + } > > No need for braces here, and I guess that ought to be retries rather > than tries (though that is pedantry). LOL. I've been yelled at for the opposite. ;) There's at least someone out there who thinks that we should have braces if you've got a single statement like this that wraps to two lines, but I can't remember who. In any case, fixed. -Doug ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 0/5] Fixes for tps65090 for Samsung ARM Chromebook 2014-04-15 20:14 [PATCH 0/3] Fixes for tps65090 for Samsung ARM Chromebook Doug Anderson 2014-04-15 20:14 ` [PATCH 3/3] regulator: tps65090: Make FETs more reliable Doug Anderson @ 2014-04-16 18:25 ` Doug Anderson 2014-04-16 18:25 ` [PATCH v2 4/5] regulator: tps65090: Allow setting the overcurrent wait time Doug Anderson 1 sibling, 1 reply; 8+ messages in thread From: Doug Anderson @ 2014-04-16 18:25 UTC (permalink / raw) To: Anton Vorontsov Cc: Olof Johansson, Sachin Kamat, ajaykumar.rs, linux-samsung-soc, Doug Anderson, devicetree, Randy Dunlap, Mark Brown, Simon Glass, linux-doc, Liam Girdwood, Samuel Ortiz, linux-kernel, Kumar Gala, Ian Campbell, Dmitry Eremin-Solenikov, Rob Herring, David Woodhouse, Pawel Moll, Lee Jones, Mark Rutland, Sean Paul, Michael Spang These five patches bring tps65090 up to speed with what's currently in the Chromium OS kernel 3.8 tree and running on the Samsung ARM Chromebook. Changes were tested atop the current linux tree (v3.15-rc1). FET retries were tested on a machine with a known flaky tps65090. Since display isn't working on pure upstream, I augmented the code to turn FET1 (vcd_led) on/off 500 times at bootup. When testing I included <https://patchwork.kernel.org/patch/3980731/> to make sure tps65090 was in exynos5250-snow's device tree. Dependencies: - Patch #1 (mfd no irq) has no dependencies, though patch #2 won't work without it. - Patch #2 (charger polling) can be applied without patch #1 but won't actually make charger polling work without it. - Patch #3 (caching) can be applied before retries patch but not after. - Patch #4 (overcurrent wait time) can be applied before retries patch but not after (just due to merge conflicts, no other reason). - Patch #5 (retries) absolutely requires patch #3 (caching). Changes in v2: - Split noirq (polling mode) changes into MFD and charger - Leave cache on for the registers that can be cached. - Move register offsets to mfd header file. - Separated the overcurrent and retries changes into two patches. - Now set overcurrent at probe time since it doesn't change. - Separated the overcurrent and retries changes into two patches. - No longer open code fet_is_enabled(). - Fixed tps6090 typo. - For loop => "while true". - Removed a set of braces. Doug Anderson (5): mfd: tps65090: Don't tell child devices we have an IRQ if we don't charger: tps65090: Allow charger module to be used when no irq mfd: tps65090: Stop caching most registers regulator: tps65090: Allow setting the overcurrent wait time regulator: tps65090: Make FETs more reliable by adding retries .../devicetree/bindings/regulator/tps65090.txt | 4 + drivers/mfd/tps65090.c | 41 ++-- drivers/power/tps65090-charger.c | 87 ++++++--- drivers/regulator/tps65090-regulator.c | 208 +++++++++++++++++++-- include/linux/mfd/tps65090.h | 19 ++ 5 files changed, 300 insertions(+), 59 deletions(-) -- 1.9.1.423.g4596e3a ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 4/5] regulator: tps65090: Allow setting the overcurrent wait time 2014-04-16 18:25 ` [PATCH v2 0/5] Fixes for tps65090 for Samsung ARM Chromebook Doug Anderson @ 2014-04-16 18:25 ` Doug Anderson 2014-04-16 20:33 ` Randy Dunlap 0 siblings, 1 reply; 8+ messages in thread From: Doug Anderson @ 2014-04-16 18:25 UTC (permalink / raw) To: Anton Vorontsov Cc: Olof Johansson, Sachin Kamat, ajaykumar.rs, linux-samsung-soc, Doug Anderson, Simon Glass, Michael Spang, Sean Paul, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Randy Dunlap, Liam Girdwood, Mark Brown, Samuel Ortiz, Lee Jones, devicetree, linux-doc, linux-kernel The tps65090 regulator allows you to specify how long you want it to wait before detecting an overcurrent condition. Allow specifying that through the device tree (or through platform data). Signed-off-by: Doug Anderson <dianders@chromium.org> Signed-off-by: Simon Glass <sjg@chromium.org> Signed-off-by: Michael Spang <spang@chromium.org> Signed-off-by: Sean Paul <seanpaul@chromium.org> --- Changes in v2: - Separated the overcurrent and retries changes into two patches. - Now set overcurrent at probe time since it doesn't change. .../devicetree/bindings/regulator/tps65090.txt | 4 ++ drivers/regulator/tps65090-regulator.c | 55 ++++++++++++++++++++++ include/linux/mfd/tps65090.h | 5 ++ 3 files changed, 64 insertions(+) diff --git a/Documentation/devicetree/bindings/regulator/tps65090.txt b/Documentation/devicetree/bindings/regulator/tps65090.txt index 313a60b..34098023 100644 --- a/Documentation/devicetree/bindings/regulator/tps65090.txt +++ b/Documentation/devicetree/bindings/regulator/tps65090.txt @@ -21,6 +21,10 @@ Optional properties: number should be provided. If it is externally controlled and no GPIO entry then driver will just configure this rails as external control and will not provide any enable/disable APIs. +- ti,overcurrent-wait: This is applicable to FET registers, which have a + poorly defined "overcurrent wait" field. If this property is present it + should be between 0 - 3. If this property isn't present we won't touch the + "overcurrent wait" field and we'll leave it to the BIOS/EC to deal with. Each regulator is defined using the standard binding for regulators. diff --git a/drivers/regulator/tps65090-regulator.c b/drivers/regulator/tps65090-regulator.c index 2e92ef6..ca13a1a 100644 --- a/drivers/regulator/tps65090-regulator.c +++ b/drivers/regulator/tps65090-regulator.c @@ -28,15 +28,57 @@ #include <linux/regulator/of_regulator.h> #include <linux/mfd/tps65090.h> +#define CTRL_WT_BIT 2 /* Regulator wait time 0 bit */ + +#define MAX_OVERCURRENT_WAIT 3 /* Overcurrent wait must be <= this */ + +/** + * struct tps65090_regulator - Per-regulator data for a tps65090 regulator + * + * @dev: Pointer to our device. + * @desc: The struct regulator_desc for the regulator. + * @rdev: The struct regulator_dev for the regulator. + * @overcurrent_wait_valid: True if overcurrent_wait is valid. + * @overcurrent_wait: For FETs, the value to put in the WTFET bitfield. + */ + struct tps65090_regulator { struct device *dev; struct regulator_desc *desc; struct regulator_dev *rdev; + bool overcurrent_wait_valid; + int overcurrent_wait; }; static struct regulator_ops tps65090_ext_control_ops = { }; +/** + * tps65090_reg_set_overcurrent_wait - Setup overcurrent wait + * + * This will set the overcurrent wait time based on what's in the regulator + * info. + * + * @ri: Overall regulator data + * @rdev: Regulator device + * @return 0 if no error, non-zero if there was an error writing the register. + */ +static int tps65090_reg_set_overcurrent_wait(struct tps65090_regulator *ri, + struct regulator_dev *rdev) +{ + int ret; + + ret = regmap_update_bits(rdev->regmap, rdev->desc->enable_reg, + MAX_OVERCURRENT_WAIT << CTRL_WT_BIT, + ri->overcurrent_wait << CTRL_WT_BIT); + if (ret) { + dev_err(&rdev->dev, "Error updating overcurrent wait %#x\n", + rdev->desc->enable_reg); + } + + return ret; +} + static struct regulator_ops tps65090_reg_contol_ops = { .enable = regulator_enable_regmap, .disable = regulator_disable_regmap, @@ -209,6 +251,11 @@ static struct tps65090_platform_data *tps65090_parse_dt_reg_data( rpdata->gpio = of_get_named_gpio(np, "dcdc-ext-control-gpios", 0); + if (of_property_read_u32(tps65090_matches[idx].of_node, + "ti,overcurrent-wait", + &rpdata->overcurrent_wait) == 0) + rpdata->overcurrent_wait_valid = true; + tps65090_pdata->reg_pdata[idx] = rpdata; } return tps65090_pdata; @@ -258,6 +305,8 @@ static int tps65090_regulator_probe(struct platform_device *pdev) ri = &pmic[num]; ri->dev = &pdev->dev; ri->desc = &tps65090_regulator_desc[num]; + ri->overcurrent_wait_valid = tps_pdata->overcurrent_wait_valid; + ri->overcurrent_wait = tps_pdata->overcurrent_wait; /* * TPS5090 DCDC support the control from external digital input. @@ -299,6 +348,12 @@ static int tps65090_regulator_probe(struct platform_device *pdev) } ri->rdev = rdev; + if (ri->overcurrent_wait_valid) { + ret = tps65090_reg_set_overcurrent_wait(ri, rdev); + if (ret < 0) + return ret; + } + /* Enable external control if it is require */ if (tps_pdata && is_dcdc(num) && tps_pdata->reg_init_data && tps_pdata->enable_ext_control) { diff --git a/include/linux/mfd/tps65090.h b/include/linux/mfd/tps65090.h index 45f0f9d..0bf2708 100644 --- a/include/linux/mfd/tps65090.h +++ b/include/linux/mfd/tps65090.h @@ -92,11 +92,16 @@ struct tps65090 { * DCDC1, DCDC2 and DCDC3. * @gpio: Gpio number if external control is enabled and controlled through * gpio. + * @overcurrent_wait_valid: True if the overcurrent_wait should be applied. + * @overcurrent_wait: Value to set as the overcurrent wait time. This is the + * actual bitfield value, not a time in ms (valid value are 0 - 3). */ struct tps65090_regulator_plat_data { struct regulator_init_data *reg_init_data; bool enable_ext_control; int gpio; + bool overcurrent_wait_valid; + int overcurrent_wait; }; struct tps65090_platform_data { -- 1.9.1.423.g4596e3a ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 4/5] regulator: tps65090: Allow setting the overcurrent wait time 2014-04-16 18:25 ` [PATCH v2 4/5] regulator: tps65090: Allow setting the overcurrent wait time Doug Anderson @ 2014-04-16 20:33 ` Randy Dunlap 2014-04-16 23:12 ` Doug Anderson 0 siblings, 1 reply; 8+ messages in thread From: Randy Dunlap @ 2014-04-16 20:33 UTC (permalink / raw) To: Doug Anderson, Anton Vorontsov Cc: Olof Johansson, Sachin Kamat, ajaykumar.rs, linux-samsung-soc, Simon Glass, Michael Spang, Sean Paul, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Liam Girdwood, Mark Brown, Samuel Ortiz, Lee Jones, devicetree, linux-doc, linux-kernel On 04/16/2014 11:25 AM, Doug Anderson wrote: > diff --git a/drivers/regulator/tps65090-regulator.c b/drivers/regulator/tps65090-regulator.c > index 2e92ef6..ca13a1a 100644 > --- a/drivers/regulator/tps65090-regulator.c > +++ b/drivers/regulator/tps65090-regulator.c > @@ -28,15 +28,57 @@ > +/** > + * tps65090_reg_set_overcurrent_wait - Setup overcurrent wait > + * > + * This will set the overcurrent wait time based on what's in the regulator > + * info. > + * > + * @ri: Overall regulator data > + * @rdev: Regulator device > + * @return 0 if no error, non-zero if there was an error writing the register. kernel-doc notation here should be: * Return: 0 if no error, non-zero if there was an error writing the register. > + */ > +static int tps65090_reg_set_overcurrent_wait(struct tps65090_regulator *ri, > + struct regulator_dev *rdev) > +{ -- ~Randy ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 4/5] regulator: tps65090: Allow setting the overcurrent wait time 2014-04-16 20:33 ` Randy Dunlap @ 2014-04-16 23:12 ` Doug Anderson 0 siblings, 0 replies; 8+ messages in thread From: Doug Anderson @ 2014-04-16 23:12 UTC (permalink / raw) To: Randy Dunlap Cc: Anton Vorontsov, Olof Johansson, Sachin Kamat, AJAY KUMAR RAMAKRISHNA SHYMALAMMA, linux-samsung-soc, Simon Glass, Michael Spang, Sean Paul, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Liam Girdwood, Mark Brown, Samuel Ortiz, Lee Jones, devicetree@vger.kernel.org, linux-doc, linux-kernel@vger.kernel.org Randy, On Wed, Apr 16, 2014 at 1:33 PM, Randy Dunlap <rdunlap@infradead.org> wrote: > On 04/16/2014 11:25 AM, Doug Anderson wrote: >> diff --git a/drivers/regulator/tps65090-regulator.c b/drivers/regulator/tps65090-regulator.c >> index 2e92ef6..ca13a1a 100644 >> --- a/drivers/regulator/tps65090-regulator.c >> +++ b/drivers/regulator/tps65090-regulator.c >> @@ -28,15 +28,57 @@ > >> +/** >> + * tps65090_reg_set_overcurrent_wait - Setup overcurrent wait >> + * >> + * This will set the overcurrent wait time based on what's in the regulator >> + * info. >> + * >> + * @ri: Overall regulator data >> + * @rdev: Regulator device >> + * @return 0 if no error, non-zero if there was an error writing the register. > > kernel-doc notation here should be: > > * Return: 0 if no error, non-zero if there was an error writing the register. Done in v3. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-04-16 23:12 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-04-15 20:14 [PATCH 0/3] Fixes for tps65090 for Samsung ARM Chromebook Doug Anderson 2014-04-15 20:14 ` [PATCH 3/3] regulator: tps65090: Make FETs more reliable Doug Anderson [not found] ` <1397592876-5741-4-git-send-email-dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> 2014-04-15 22:52 ` Mark Brown 2014-04-16 18:28 ` Doug Anderson 2014-04-16 18:25 ` [PATCH v2 0/5] Fixes for tps65090 for Samsung ARM Chromebook Doug Anderson 2014-04-16 18:25 ` [PATCH v2 4/5] regulator: tps65090: Allow setting the overcurrent wait time Doug Anderson 2014-04-16 20:33 ` Randy Dunlap 2014-04-16 23:12 ` Doug Anderson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).