| Message ID | 1397689950-1568-5-git-send-email-dianders@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Apr 16, 2014 at 04:12:28PM -0700, Doug Anderson wrote: > 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). Applied, thanks. > +- 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. What I was driving at by asking if this was the raw register value was that the binding should make this clearer ideally.
> 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 v3: > - Fixed kernel-doc notation for return > > 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 | 56 ++++++++++++++++++++++ > include/linux/mfd/tps65090.h | 5 ++ > 3 files changed, 65 insertions(+) Applied, thanks.
Lee, On Wed, Apr 23, 2014 at 4:51 AM, Lee Jones <lee.jones@linaro.org> wrote: >> 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 v3: >> - Fixed kernel-doc notation for return >> >> 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 | 56 ++++++++++++++++++++++ >> include/linux/mfd/tps65090.h | 5 ++ >> 3 files changed, 65 insertions(+) > > Applied, thanks. Ummmm, Mark said that he had already applied this patch to his tree (I mentioned it in my recent summary and you can see it in this thread too). I don't see it on git.kernel.org though <https://git.kernel.org/cgit/linux/kernel/git/broonie/regulator.git/log/?h=for-next> I'm worried this will cause a merge conflict if you both apply it. -Doug -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Lee, Doug,
I've bisected a boot failure on Tegra Dalmore (which has a tps65090)
down to this patch. It started in -next 0501, so I guess Lee might
have pushed some patches out now even though the commit date is a
little while back?
The commit is:
commit 60e91b51b515b20f85697fcd397911fdb97bbdca
Author: Doug Anderson <dianders@chromium.org>
AuthorDate: Wed Apr 16 16:12:28 2014 -0700
Commit: Lee Jones <lee.jones@linaro.org>
CommitDate: Wed Apr 23 12:34:01 2014 +0100
regulator: tps65090: Allow setting the overcurrent wait time
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>
Acked-by: Simon Glass <sjg@chromium.org>
Acked-by: Michael Spang <spang@chromium.org>
Acked-by: Sean Paul <seanpaul@chromium.org>
Acked-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
Panic is line 309:
303 for (num = 0; num < TPS65090_REGULATOR_MAX; num++) {
304 tps_pdata = tps65090_pdata->reg_pdata[num];
305
306 ri = &pmic[num];
307 ri->dev = &pdev->dev;
308 ri->desc = &tps65090_regulator_desc[num];
309 ri->overcurrent_wait_valid =
tps_pdata->overcurrent_wait_valid;
310 ri->overcurrent_wait = tps_pdata->overcurrent_wait;
so it looks like tps_pdata is NULL. Should likely be a check for it?
-Olof
On Wed, Apr 23, 2014 at 8:48 AM, Doug Anderson <dianders@chromium.org> wrote:
> Lee,
>
> On Wed, Apr 23, 2014 at 4:51 AM, Lee Jones <lee.jones@linaro.org> wrote:
>>> 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 v3:
>>> - Fixed kernel-doc notation for return
>>>
>>> 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 | 56 ++++++++++++++++++++++
>>> include/linux/mfd/tps65090.h | 5 ++
>>> 3 files changed, 65 insertions(+)
>>
>> Applied, thanks.
>
> Ummmm, Mark said that he had already applied this patch to his tree (I
> mentioned it in my recent summary and you can see it in this thread
> too). I don't see it on git.kernel.org though
> <https://git.kernel.org/cgit/linux/kernel/git/broonie/regulator.git/log/?h=for-next>
>
> I'm worried this will cause a merge conflict if you both apply it.
>
> -Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, May 01, 2014 at 11:17:58AM -0700, Olof Johansson wrote:
> so it looks like tps_pdata is NULL. Should likely be a check for it?
Yes, just about to post a fix.
Mark, On Thu, May 1, 2014 at 11:49 AM, Mark Brown <broonie@kernel.org> wrote: > On Thu, May 01, 2014 at 11:17:58AM -0700, Olof Johansson wrote: > >> so it looks like tps_pdata is NULL. Should likely be a check for it? > > Yes, just about to post a fix. Doh, was working on it at the same time. https://patchwork.kernel.org/patch/4099981/ Sorry for the bug. :( Thanks for the report! -Doug -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, May 01, 2014 at 11:52:28AM -0700, Doug Anderson wrote: > > Yes, just about to post a fix. > Doh, was working on it at the same time. No worries, I dropped my patch in favour of yours now (though they look to be identical apart from the metadata!).
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..ca04e9f 100644 --- a/drivers/regulator/tps65090-regulator.c +++ b/drivers/regulator/tps65090-regulator.c @@ -28,15 +28,58 @@ #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 +252,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 +306,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 +349,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 {