On 06/02/18 16:34, Peter De Schrijver wrote:
> The CVB table contains calibration data for the CPU DFLL based on
> process charaterization. The regulator step and offset parameters depend
> on the regulator supplying vdd-cpu , not on the specific Tegra SKU.
> Hence than hardcoding those regulator parameters in the CVB table,
> retrieve them from the regulator framework and store them as part of the
> tegra_dfll_soc_data struct.
> 
> Signed-off-by: Peter De Schrijver <[email protected]>
> ---
>  drivers/clk/tegra/clk-dfll.h               |  2 ++
>  drivers/clk/tegra/clk-tegra124-dfll-fcpu.c | 42 
> +++++++++++++++++++++++++-----
>  drivers/clk/tegra/cvb.c                    | 16 +++++++++---
>  drivers/clk/tegra/cvb.h                    |  6 ++---
>  4 files changed, 53 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/clk/tegra/clk-dfll.h b/drivers/clk/tegra/clk-dfll.h
> index 83352c8..e7cbc5b 100644
> --- a/drivers/clk/tegra/clk-dfll.h
> +++ b/drivers/clk/tegra/clk-dfll.h
> @@ -21,6 +21,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/reset.h>
>  #include <linux/types.h>
> +#include "cvb.h"
>  
>  /**
>   * struct tegra_dfll_soc_data - SoC-specific hooks/integration for the DFLL 
> driver
> @@ -35,6 +36,7 @@ struct tegra_dfll_soc_data {
>       struct device *dev;
>       unsigned long max_freq;
>       const struct cvb_table *cvb;
> +     struct rail_alignment alignment;
>  
>       void (*init_clock_trimmers)(void);
>       void (*set_clock_trimmers_high)(void);
> diff --git a/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c 
> b/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
> index 269d359..e2dbb79 100644
> --- a/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
> +++ b/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
> @@ -22,6 +22,7 @@
>  #include <linux/kernel.h>
>  #include <linux/init.h>
>  #include <linux/platform_device.h>
> +#include <linux/regulator/consumer.h>
>  #include <soc/tegra/fuse.h>
>  
>  #include "clk.h"
> @@ -42,9 +43,6 @@
>               .process_id = -1,
>               .min_millivolts = 900,
>               .max_millivolts = 1260,
> -             .alignment = {
> -                     .step_uv = 10000, /* 10mV */
> -             },
>               .speedo_scale = 100,
>               .voltage_scale = 1000,
>               .entries = {
> @@ -82,6 +80,34 @@
>       },
>  };
>  
> +static int get_alignment_from_regulator(struct device *dev,
> +                                     struct rail_alignment *align)
> +{
> +     int min_uV, max_uV, n_voltages, ret;
> +     struct regulator *reg;
> +
> +     reg = devm_regulator_get(dev, "vdd-cpu");
> +     if (IS_ERR(reg))
> +             return PTR_ERR(reg);
> +
> +     ret = regulator_get_constraint_voltages(reg, &min_uV, &max_uV);
> +     if (!ret)
> +             align->offset_uv = min_uV;
> +     else
> +             return ret;

Nit-pick ... looks a bit odd, why not ...

        if (ret)
                return ret;

        align->offset_uv = min_uV;

> +
> +     align->step_uv = regulator_get_linear_step(reg);
> +     if (!align->step_uv && !ret) {

Do you need to test 'ret' here?

> +             n_voltages = regulator_count_voltages(reg);
> +             if (n_voltages > 1)
> +                     align->step_uv = (max_uV - min_uV) / (n_voltages - 1);

Later in the patch !align->step_uv is treated as an error, so if
n_voltages should always be greater 1 then why not return an error here?
Seems that this should not happen?

Cheers
Jon

-- 
nvpublic

Reply via email to