2013-06-07 08:07:02

by Paul Walmsley

[permalink] [raw]
Subject: [PATCH] regulator: core: add regulator_get_linear_step()


Add regulator_get_linear_step(), which returns the voltage step size
between VSEL values for linear regulators. This is intended for use
by regulator consumers which build their own voltage-to-VSEL tables.

Signed-off-by: Paul Walmsley <[email protected]>
Reviewed-by: Andrew Chew <[email protected]>
Cc: Matthew Longnecker <[email protected]>
---

Applies on v3.10-rc4. Will be used by the upcoming Tegra DFLL clocksource
driver, which will build its own table of voltage-to-VSEL values by
querying the regulator framework.

drivers/regulator/core.c | 15 +++++++++++++++
include/linux/regulator/consumer.h | 1 +
2 files changed, 16 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 6e50178..f07d7ad 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2135,6 +2135,21 @@ int regulator_list_voltage(struct regulator *regulator, unsigned selector)
EXPORT_SYMBOL_GPL(regulator_list_voltage);

/**
+ * regulator_get_linear_step - return the voltage step size between VSEL values
+ * @regulator: regulator source
+ *
+ * Returns the voltage step size between VSEL values for linear
+ * regulators, or return 0 if the regulator isn't a linear regulator.
+ */
+unsigned int regulator_get_linear_step(struct regulator *regulator)
+{
+ struct regulator_dev *rdev = regulator->rdev;
+
+ return rdev->desc->uV_step;
+}
+EXPORT_SYMBOL_GPL(regulator_get_linear_step);
+
+/**
* regulator_is_supported_voltage - check if a voltage range can be supported
*
* @regulator: Regulator to check.
diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
index 145022a..3a76389c 100644
--- a/include/linux/regulator/consumer.h
+++ b/include/linux/regulator/consumer.h
@@ -165,6 +165,7 @@ int regulator_count_voltages(struct regulator *regulator);
int regulator_list_voltage(struct regulator *regulator, unsigned selector);
int regulator_is_supported_voltage(struct regulator *regulator,
int min_uV, int max_uV);
+unsigned int regulator_get_linear_step(struct regulator *regulator);
int regulator_set_voltage(struct regulator *regulator, int min_uV, int max_uV);
int regulator_set_voltage_time(struct regulator *regulator,
int old_uV, int new_uV);
--
1.7.10.4


2013-06-07 09:09:32

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH] regulator: core: add regulator_get_linear_step()

On Fri, Jun 07, 2013 at 08:06:56AM +0000, Paul Walmsley wrote:

> Applies on v3.10-rc4. Will be used by the upcoming Tegra DFLL clocksource
> driver, which will build its own table of voltage-to-VSEL values by
> querying the regulator framework.

Can you show the code please?


Attachments:
(No filename) (280.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2013-06-07 09:38:13

by Paul Walmsley

[permalink] [raw]
Subject: Re: [PATCH] regulator: core: add regulator_get_linear_step()

Hi Mark,

On Fri, 7 Jun 2013, Mark Brown wrote:

> On Fri, Jun 07, 2013 at 08:06:56AM +0000, Paul Walmsley wrote:
>
> > Applies on v3.10-rc4. Will be used by the upcoming Tegra DFLL clocksource
> > driver, which will build its own table of voltage-to-VSEL values by
> > querying the regulator framework.
>
> Can you show the code please?

The driver isn't 100% ready to post yet. Still wrapping up a few last
tweaks (in an unrelated part of the driver). But here are the parts of
the code that use that regulator function.

This is the immediate caller:

+/**
+ * tegra_dfll_init_pmic_data - initialize PMIC regulator data
+ * @pdev: DFLL instance
+ *
+ * Read the PMIC integration data, including regulator data, from DT
+ * and the the regulator framework. Build the voltage map from
+ * regulator data. Returns 0 upon success or -EINVAL upon error.
+ */
+static int __must_check tegra_dfll_init_pmic_data(struct platform_device *pdev)
+{
+ struct tegra_dfll *td = dev_get_drvdata(&pdev->dev);
+ int r;
+
+ td->vdd = regulator_get(&pdev->dev, "vdd");
+ if (IS_ERR(td->vdd)) {
+ dev_err(&pdev->dev, "couldn't locate regulator\n");
+ return -EINVAL;
+ }
+
+ td->vdd_step = regulator_get_linear_step(td->vdd);
+ if (!td->vdd_step) {
+ dev_err(&pdev->dev, "only linear map regulators supported\n");
+ goto tdipd_err;
+ }
+
+ r = parse_of_dfll_pmic_integration(pdev, pdev->dev.of_node);
+ if (r) {
+ dev_err(&pdev->dev, "DFLL PMIC integration parse error\n");
+ goto tdipd_err;
+ }
+
+ r = tegra_dfll_regulator_probe_voltages(pdev);
+ if (r) {
+ dev_err(&pdev->dev, "couldn't probe regulator voltages\n");
+ goto tdipd_err;
+ }
+
+ return 0;
+
+tdipd_err:
+ regulator_put(td->vdd);
+ return -EINVAL;
+}


... and here's the code that uses td->vdd_step:

+static int get_cvb_voltage(struct platform_device *pdev, int c0, int c1,
+ int c2)
+{
+ struct tegra_dfll *td = dev_get_drvdata(&pdev->dev);
+ /* apply only speedo scale: output mv = cvb_mv * v_scale */
+ int mv;
+
+ /* combined: apply voltage scale and round to cvb alignment step */
+ mv = DIV_ROUND_CLOSEST(c2 * td->speedo_value, td->cvb_speedo_scale);
+ mv = DIV_ROUND_CLOSEST((mv + c1) * td->speedo_value,
+ td->cvb_speedo_scale) + c0;
+
+ /* XXX Bring back cvb_alignment_uv; put it in the board dfll DT */
+ return DIV_ROUND_UP(mv * 1000,
+ td->cvb_voltage_scale * td->vdd_step) *
+ td->vdd_step / 1000;
+}


But, maybe you're more interested in the voltage probing code:


+/**
+ * tegra_dfll_regulator_probe_voltages - build vdd_map[] from the regulator
+ * @pdev: DFLL instance
+ *
+ * Build the vdd_map from regulator framework and DFLL DT data.
+ * Returns 0 upon success, or -ENOSPC on a memory allocation failure.
+ */
+static int tegra_dfll_regulator_probe_voltages(struct platform_device *pdev)
+{
+ struct tegra_dfll *td = dev_get_drvdata(&pdev->dev);
+ int c, i, j, vdd_uv, vdd_mv;
+ struct tegra_dfll_voltage_reg_map *vdd_map;
+
+ c = regulator_count_voltages(td->vdd);
+ if (c < 0)
+ return c;
+
+ vdd_map = kzalloc(sizeof(struct tegra_dfll_voltage_reg_map) * c,
+ GFP_KERNEL);
+ if (!vdd_map)
+ return -ENOMEM;
+
+ j = 0;
+ for (i = 0; i < c; i++) {
+ vdd_uv = regulator_list_voltage(td->vdd, i);
+ if (vdd_uv <= 0)
+ continue;
+
+ if (vdd_uv < td->dfll_min_microvolt)
+ continue;
+
+ if (vdd_uv > td->dfll_max_microvolt)
+ break;
+
+ vdd_mv = vdd_uv / 1000;
+
+ vdd_map[j].reg_value = i;
+ vdd_map[j].reg_uv = vdd_uv;
+ vdd_map[j].reg_mv = vdd_mv;
+ j++;
+ }
+
+ td->vdd_map_size = j;
+ td->vdd_map = vdd_map;
+
+ return 0;
+}


If you've got a different approach in mind, I'm happy to switch to it.


- Paul

2013-06-07 10:00:04

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH] regulator: core: add regulator_get_linear_step()

On Fri, Jun 07, 2013 at 09:38:10AM +0000, Paul Walmsley wrote:

> +static int get_cvb_voltage(struct platform_device *pdev, int c0, int c1,
> + int c2)
> +{
> + struct tegra_dfll *td = dev_get_drvdata(&pdev->dev);
> + /* apply only speedo scale: output mv = cvb_mv * v_scale */
> + int mv;
> +
> + /* combined: apply voltage scale and round to cvb alignment step */
> + mv = DIV_ROUND_CLOSEST(c2 * td->speedo_value, td->cvb_speedo_scale);
> + mv = DIV_ROUND_CLOSEST((mv + c1) * td->speedo_value,
> + td->cvb_speedo_scale) + c0;
> +
> + /* XXX Bring back cvb_alignment_uv; put it in the board dfll DT */
> + return DIV_ROUND_UP(mv * 1000,
> + td->cvb_voltage_scale * td->vdd_step) *
> + td->vdd_step / 1000;
> +}

Hrm, right. So I guess for this the question is why you need the steps
at all - can't the driver just ask for the voltage it wants with
whatever tolerance is appropriate (or just specify the maximum tolerable
voltage if that's better)? I'm not sure what cvb_voltage_scale is.

I guess the problem here is that you have to hit some fairly tight
tolerances on the voltage?


Attachments:
(No filename) (1.08 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments

2013-06-07 10:03:25

by Paul Walmsley

[permalink] [raw]
Subject: Re: [PATCH] regulator: core: add regulator_get_linear_step()

Hi Mark,

On Fri, 7 Jun 2013, Mark Brown wrote:

> On Fri, Jun 07, 2013 at 09:38:10AM +0000, Paul Walmsley wrote:
>
> > +static int get_cvb_voltage(struct platform_device *pdev, int c0, int c1,
> > + int c2)
> > +{
> > + struct tegra_dfll *td = dev_get_drvdata(&pdev->dev);
> > + /* apply only speedo scale: output mv = cvb_mv * v_scale */
> > + int mv;
> > +
> > + /* combined: apply voltage scale and round to cvb alignment step */
> > + mv = DIV_ROUND_CLOSEST(c2 * td->speedo_value, td->cvb_speedo_scale);
> > + mv = DIV_ROUND_CLOSEST((mv + c1) * td->speedo_value,
> > + td->cvb_speedo_scale) + c0;
> > +
> > + /* XXX Bring back cvb_alignment_uv; put it in the board dfll DT */
> > + return DIV_ROUND_UP(mv * 1000,
> > + td->cvb_voltage_scale * td->vdd_step) *
> > + td->vdd_step / 1000;
> > +}
>
> Hrm, right. So I guess for this the question is why you need the steps
> at all - can't the driver just ask for the voltage it wants with
> whatever tolerance is appropriate (or just specify the maximum tolerable
> voltage if that's better)? I'm not sure what cvb_voltage_scale is.
>
> I guess the problem here is that you have to hit some fairly tight
> tolerances on the voltage?

The IP block has an I2C controller embedded in it that autonomously sends
set-voltage commands (across a range of voltages) to the PMIC. To enable
that, the driver must first initialize the IP block with a
voltage-to-selector table.


- Paul

2013-06-07 10:11:54

by Paul Walmsley

[permalink] [raw]
Subject: Re: [PATCH] regulator: core: add regulator_get_linear_step()

On Fri, 7 Jun 2013, Paul Walmsley wrote:

> The IP block has an I2C controller embedded in it that autonomously sends
> set-voltage commands (across a range of voltages) to the PMIC. To enable
> that, the driver must first initialize the IP block with a
> voltage-to-selector table.

Here's the table loading code -

+/**
+ * _load_lut - load voltage lookup table into DFLL RAM
+ * @pdev: DFLL instance
+ *
+ * Load the voltage-to-PMIC register value lookup table into the DFLL
+ * IP block LUT memory. td->lut_min and td->lut_max are used to cap
+ * the minimum and maximum voltage requested. This function shouldn't
+ * be called directly by code other than dfll_load_lut(), since this
+ * function doesn't handle the necessary pre- and post-requisites. No
+ * return value.
+ */
+static void _load_lut(struct platform_device *pdev)
+{
+ struct tegra_dfll *td = dev_get_drvdata(&pdev->dev);
+ int i;
+ u32 val;
+
+ val = td->out_map[td->lut_min]->reg_value;
+ for (i = 0; i <= td->lut_min; i++)
+ dfll_writel(td, val, DFLL_OUTPUT_LUT + i * 4);
+
+ for (; i < td->lut_max; i++) {
+ val = td->out_map[i]->reg_value;
+ dfll_writel(td, val, DFLL_OUTPUT_LUT + i * 4);
+ }
+
+ val = td->out_map[td->lut_max]->reg_value;
+ for (; i < td->num_voltages; i++)
+ dfll_writel(td, val, DFLL_OUTPUT_LUT + i * 4);
+
+ dfll_wmb(td);
+}



- Paul

2013-06-07 10:18:50

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH] regulator: core: add regulator_get_linear_step()

On Fri, Jun 07, 2013 at 10:03:22AM +0000, Paul Walmsley wrote:

> The IP block has an I2C controller embedded in it that autonomously sends
> set-voltage commands (across a range of voltages) to the PMIC. To enable
> that, the driver must first initialize the IP block with a
> voltage-to-selector table.

Ah, right - a one of those! OK, makes sense, I'll apply.


Attachments:
(No filename) (368.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments