On 24/01/18 12:45, Peter De Schrijver wrote:
> When using a PWM controlled regulator, the voltage step and offset are
> determined by the regulator IC in use. This is specified in DT rather
> than fixed in the CVB table. Hence pass this information to the CVB table
> calculation function. For backwards compatibility the table values are used
> if the corresponding parameter is 0.
>
> Signed-off-by: Peter De Schrijver <pdeschrijver@xxxxxxxxxx>
> ---
> drivers/clk/tegra/clk-tegra124-dfll-fcpu.c | 23 +++++++++++++++++++++--
> drivers/clk/tegra/cvb.c | 18 ++++++++++++++----
> drivers/clk/tegra/cvb.h | 5 +++--
> 3 files changed, 38 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c b/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
> index 440eb8d..6205ce1 100644
> --- a/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
> +++ b/drivers/clk/tegra/clk-tegra124-dfll-fcpu.c
> @@ -111,6 +111,7 @@ static int tegra124_dfll_fcpu_probe(struct platform_device *pdev)
> struct tegra_dfll_soc_data *soc;
> const struct of_device_id *of_id;
> const struct dfll_fcpu_data *fcpu_data;
> + struct rail_alignment align;
>
> of_id = of_match_device(tegra124_dfll_fcpu_of_match, &pdev->dev);
> fcpu_data = of_id->data;
> @@ -135,12 +136,30 @@ static int tegra124_dfll_fcpu_probe(struct platform_device *pdev)
> return -ENODEV;
> }
>
> + err = of_property_read_u32(pdev->dev.of_node, "nvidia,align-offset-uv",
> + &align.offset_uv);
> + if (err < 0) {
> + dev_err(&pdev->dev,
> + "offset uv not found, default to table value\n");
> + align.offset_uv = 0;
> + }
> +
> + err = of_property_read_u32(pdev->dev.of_node, "nvidia,align-step-uv",
> + &align.step_uv);
> + if (err < 0) {
> + dev_err(&pdev->dev,
> + "step uv not found, default to table value\n");
> + align.step_uv = 0;
> + }
> +
I am a bit confused by this ...
1. Isn't this going to break Tegra124 DFLL support?
2. These DT properties are not defined anywhere (so the binding doc
needs updating).
3. I don't see any patches in this series that populate these properties
in DT.
So I am not sure how this works?!?
> soc->max_freq = fcpu_data->cpu_max_freq_table[speedo_id];
>
> soc->cvb = tegra_cvb_add_opp_table(soc->dev, fcpu_data->cpu_cvb_tables,
> fcpu_data->cpu_cvb_tables_size,
> - process_id, speedo_id, speedo_value,
> - soc->max_freq);
> + &align, process_id, speedo_id,
> + speedo_value, soc->max_freq);
> + soc->alignment = align;
> +
This is not defined yet and so breaks compile.
drivers/clk/tegra/clk-tegra124-dfll-fcpu.c: In function
‘tegra124_dfll_fcpu_probe’:
drivers/clk/tegra/clk-tegra124-dfll-fcpu.c:161:5: error: ‘struct
tegra_dfll_soc_data’ has no member named ‘alignment’
soc->alignment = align;
^
What about Tegra124? This is set to NULL? Can we not set this before
calling tegra_cvb_add_opp_table() and then just pass soc->alignment for
both Tegra124 and Tegra210? Then you can avoid the if-statements in
build_opp_table.
> if (IS_ERR(soc->cvb)) {
> dev_err(&pdev->dev, "couldn't add OPP table: %ld\n",
> PTR_ERR(soc->cvb));
> diff --git a/drivers/clk/tegra/cvb.c b/drivers/clk/tegra/cvb.c
> index da9e8e7..561012a 100644
> --- a/drivers/clk/tegra/cvb.c
> +++ b/drivers/clk/tegra/cvb.c
> @@ -62,11 +62,19 @@ static int round_voltage(int mv, const struct rail_alignment *align, int up)
> }
>
> static int build_opp_table(struct device *dev, const struct cvb_table *table,
> + struct rail_alignment *align,
> int speedo_value, unsigned long max_freq)
> {
> - const struct rail_alignment *align = &table->alignment;
> int i, ret, dfll_mv, min_mv, max_mv;
>
> + if (!align->step_uv)
> + align->step_uv = table->alignment.step_uv;
> + if (!align->step_uv)
> + return -EINVAL;
> +
> + if (!align->offset_uv)
> + align->offset_uv = table->alignment.offset_uv;
> +
> min_mv = round_voltage(table->min_millivolts, align, UP);
> max_mv = round_voltage(table->max_millivolts, align, DOWN);
>
> @@ -109,8 +117,9 @@ static int build_opp_table(struct device *dev, const struct cvb_table *table,
> */
> const struct cvb_table *
> tegra_cvb_add_opp_table(struct device *dev, const struct cvb_table *tables,
> - size_t count, int process_id, int speedo_id,
> - int speedo_value, unsigned long max_freq)
> + size_t count, struct rail_alignment *align,
> + int process_id, int speedo_id, int speedo_value,
> + unsigned long max_freq)
> {
> size_t i;
> int ret;
> @@ -124,7 +133,8 @@ static int build_opp_table(struct device *dev, const struct cvb_table *table,
> if (table->process_id != -1 && table->process_id != process_id)
> continue;
>
> - ret = build_opp_table(dev, table, speedo_value, max_freq);
> + ret = build_opp_table(dev, table, align, speedo_value,
> + max_freq);
> return ret ? ERR_PTR(ret) : table;
> }
>
> diff --git a/drivers/clk/tegra/cvb.h b/drivers/clk/tegra/cvb.h
> index c1f0779..cfa110f 100644
> --- a/drivers/clk/tegra/cvb.h
> +++ b/drivers/clk/tegra/cvb.h
> @@ -59,8 +59,9 @@ struct cvb_table {
>
> const struct cvb_table *
> tegra_cvb_add_opp_table(struct device *dev, const struct cvb_table *cvb_tables,
> - size_t count, int process_id, int speedo_id,
> - int speedo_value, unsigned long max_freq);
> + size_t count, struct rail_alignment *align,
> + int process_id, int speedo_id, int speedo_value,
> + unsigned long max_freq);
> void tegra_cvb_remove_opp_table(struct device *dev,
> const struct cvb_table *table,
> unsigned long max_freq);
>
Cheers
Jon
--
nvpublic
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
