[v2,2/6] clk: tegra: DT align parameter for CVB calculation

Message ID 1516797938-32044-4-git-send-email-pdeschrijver@nvidia.com
State Superseded
Headers show
Series
  • Untitled series #25120
Related show

Commit Message

Peter De Schrijver Jan. 24, 2018, 12:45 p.m.
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@nvidia.com>
---
 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(-)

Comments

Jon Hunter Jan. 31, 2018, 10:43 a.m. | #1
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@nvidia.com>
> ---
>  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
Peter De Schrijver Feb. 1, 2018, 10:30 a.m. | #2
On Wed, Jan 31, 2018 at 10:43:04AM +0000, Jon Hunter wrote:
> 
> 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@nvidia.com>
> > ---
> >  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?

We fall back to the original behaviour in case the properties are missing, so
it should work.

> 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?!?
> 

The DT updates are indeed missing.

> >  	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

It's a struct copy, not a pointer. And both values in the struct will indeed
be set to 0 then.

> 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.
> 

The table to be used is determined by tegra_cvb_add_opp_table() based on speedo
and process ID. So this logic would have to be duplicated in
tegra124_dfll_fcpu_probe() then. In retrospect it was probably a mistake to
put these regulator parameters in the CVB table, but I think we're stuck
with this unless we want to force people to also update their DT.

> >  	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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jon Hunter Feb. 1, 2018, 10:54 a.m. | #3
On 01/02/18 10:30, Peter De Schrijver wrote:
> On Wed, Jan 31, 2018 at 10:43:04AM +0000, Jon Hunter wrote:
>>
>> 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@nvidia.com>
>>> ---
>>>  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?
> 
> We fall back to the original behaviour in case the properties are missing, so
> it should work.

Ah yes. However, on Tegra124 now I see all this prints. Can't we only
read these properties if using PWM?

Cheers
Jon
Peter De Schrijver Feb. 1, 2018, 11:02 a.m. | #4
On Thu, Feb 01, 2018 at 10:54:15AM +0000, Jon Hunter wrote:
> 
> On 01/02/18 10:30, Peter De Schrijver wrote:
> > On Wed, Jan 31, 2018 at 10:43:04AM +0000, Jon Hunter wrote:
> >>
> >> 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@nvidia.com>
> >>> ---
> >>>  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?
> > 
> > We fall back to the original behaviour in case the properties are missing, so
> > it should work.
> 
> Ah yes. However, on Tegra124 now I see all this prints. Can't we only
> read these properties if using PWM?
> 

I think it would make more sense then to get these parameters from the
regulator object in case of i2c. If that works on Tegra124, we can eliminate
using the CVB table align values entirely.

Peter.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

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;
+	}
+
 	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;
+
 	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);