Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1439594 > unrolled thread

Re: [PATCH v3 12/14] regulator: pwm: Retrieve correct voltage

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v3 12/14] regulator: pwm: Retrieve correct voltage Thierry Reding <thierry.reding@gmail.com> - 2016-07-08 17:50 +0200
    Re: [PATCH v3 12/14] regulator: pwm: Retrieve correct voltage Mark Brown <broonie@kernel.org> - 2016-07-09 11:50 +0200
      Re: [PATCH v3 12/14] regulator: pwm: Retrieve correct voltage Thierry Reding <thierry.reding@gmail.com> - 2016-07-11 09:10 +0200
        Re: [PATCH v3 12/14] regulator: pwm: Retrieve correct voltage Boris Brezillon <boris.brezillon@free-electrons.com> - 2016-07-11 09:30 +0200
        Re: [PATCH v3 12/14] regulator: pwm: Retrieve correct voltage Doug Anderson <dianders@google.com> - 2016-07-11 19:00 +0200

#1439594 — Re: [PATCH v3 12/14] regulator: pwm: Retrieve correct voltage

[Multipart message — attachments visible in raw view] — view raw

On Tue, Jun 14, 2016 at 11:13:20AM +0200, Boris Brezillon wrote:
> The continuous PWM voltage regulator is caching the voltage value in
> the ->volt_uV field. While most of the time this value should reflect the
> real voltage, sometime it can be sightly different if the PWM device
> rounded the set_duty_cycle request.
> Moreover, this value is not valid until someone has modified the regulator
> output.
> 
> Remove the ->volt_uV field and always rely on the PWM state to calculate
> the regulator output.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> Reviewed-by: Brian Norris <briannorris@chromium.org>
> Tested-by: Brian Norris <briannorris@chromium.org>
> Tested-by: Heiko Stuebner <heiko@sntech.de>
> ---
> Mark,
> 
> I know you already added your Tested-by/Acked-by tags on this patch
> but this version has slightly change and is now making use of the
> pwm_get_relative_duty_cycle() helper instead of manually converting
> the absolute duty_cycle value into a relative one.
> ---
>  drivers/regulator/pwm-regulator.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
> index 2000118..80d083f 100644
> --- a/drivers/regulator/pwm-regulator.c
> +++ b/drivers/regulator/pwm-regulator.c
> @@ -35,9 +35,6 @@ struct pwm_regulator_data {
>  	struct regulator_ops ops;
>  
>  	int state;
> -
> -	/* Continuous voltage */
> -	int volt_uV;
>  };
>  
>  struct pwm_voltages {
> @@ -135,8 +132,13 @@ static int pwm_regulator_is_enabled(struct regulator_dev *dev)
>  static int pwm_regulator_get_voltage(struct regulator_dev *rdev)
>  {
>  	struct pwm_regulator_data *drvdata = rdev_get_drvdata(rdev);
> +	int min_uV = rdev->constraints->min_uV;
> +	int diff = rdev->constraints->max_uV - min_uV;
> +	struct pwm_state pstate;
>  
> -	return drvdata->volt_uV;
> +	pwm_get_state(drvdata->pwm, &pstate);
> +
> +	return min_uV + pwm_get_relative_duty_cycle(&pstate, diff);
>  }
>  
>  static int pwm_regulator_set_voltage(struct regulator_dev *rdev,
> @@ -162,8 +164,6 @@ static int pwm_regulator_set_voltage(struct regulator_dev *rdev,
>  		return ret;
>  	}
>  
> -	drvdata->volt_uV = min_uV;
> -
>  	/* Delay required by PWM regulator to settle to the new voltage */
>  	usleep_range(ramp_delay, ramp_delay + 1000);
>  

This hunk has a minor conflict with the regulator tree and the commit
830583004e61 ("regulator: pwm: Drop unneeded pwm_enable() call") that
it contains.

Mark, do you want me to provide a stable branch with the PWM regulator
patches and resolve that conflict in your tree? Or would you rather take
the whole set based on a stable branch from the PWM tree? Or maybe yet
another possibility would be to base the PWM tree on a stable branch
from the regulator tree containing the above commit.

Or we can let Linus sort out the conflict, it's really quite trivial.

Thierry

[toc] | [next] | [standalone]

#1439933

[Multipart message — attachments visible in raw view] — view raw

On Fri, Jul 08, 2016 at 05:43:02PM +0200, Thierry Reding wrote:

> Mark, do you want me to provide a stable branch with the PWM regulator
> patches and resolve that conflict in your tree? Or would you rather take
> the whole set based on a stable branch from the PWM tree? Or maybe yet
> another possibility would be to base the PWM tree on a stable branch
> from the regulator tree containing the above commit.

Probably easiest to use this signed tag and resolve it in your tree:

The following changes since commit 1a695a905c18548062509178b98bc91e67510864:

  Linux 4.7-rc1 (2016-05-29 09:29:24 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git tags/pwm-modernization

for you to fetch changes up to c2588393e6315ab68207323d37d2a73713d6bc81:

  regulator: pwm: Fix regulator ramp delay for continuous mode (2016-07-07 11:45:06 +0200)

----------------------------------------------------------------
regulator: Provide a branch for moderninzation of the PWM code

There's a new, improved PWM API which allows a lot of improvements in
the PWM regulator driver.  Since the bulk of the changes are in the PWM
API this is being managed in the PWM tree, merge pending regulator API
changes to allow this to be resolved more easily.

----------------------------------------------------------------
Alexandre Courbot (1):
      regulator: pwm: Support for enable GPIO

Boris Brezillon (1):
      regulator: pwm: Drop unneeded pwm_enable() call

Douglas Anderson (1):
      regulator: pwm: Fix regulator ramp delay for continuous mode

 .../bindings/regulator/pwm-regulator.txt           |  7 +++-
 drivers/regulator/pwm-regulator.c                  | 40 ++++++++++++++++++----
 2 files changed, 39 insertions(+), 8 deletions(-)

[toc] | [prev] | [next] | [standalone]

#1440305

[Multipart message — attachments visible in raw view] — view raw

On Sat, Jul 09, 2016 at 11:47:18AM +0200, Mark Brown wrote:
> On Fri, Jul 08, 2016 at 05:43:02PM +0200, Thierry Reding wrote:
> 
> > Mark, do you want me to provide a stable branch with the PWM regulator
> > patches and resolve that conflict in your tree? Or would you rather take
> > the whole set based on a stable branch from the PWM tree? Or maybe yet
> > another possibility would be to base the PWM tree on a stable branch
> > from the regulator tree containing the above commit.
> 
> Probably easiest to use this signed tag and resolve it in your tree:
> 
> The following changes since commit 1a695a905c18548062509178b98bc91e67510864:
> 
>   Linux 4.7-rc1 (2016-05-29 09:29:24 -0700)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git tags/pwm-modernization
> 
> for you to fetch changes up to c2588393e6315ab68207323d37d2a73713d6bc81:
> 
>   regulator: pwm: Fix regulator ramp delay for continuous mode (2016-07-07 11:45:06 +0200)
> 
> ----------------------------------------------------------------
> regulator: Provide a branch for moderninzation of the PWM code
> 
> There's a new, improved PWM API which allows a lot of improvements in
> the PWM regulator driver.  Since the bulk of the changes are in the PWM
> API this is being managed in the PWM tree, merge pending regulator API
> changes to allow this to be resolved more easily.
> 
> ----------------------------------------------------------------
> Alexandre Courbot (1):
>       regulator: pwm: Support for enable GPIO
> 
> Boris Brezillon (1):
>       regulator: pwm: Drop unneeded pwm_enable() call
> 
> Douglas Anderson (1):
>       regulator: pwm: Fix regulator ramp delay for continuous mode
> 
>  .../bindings/regulator/pwm-regulator.txt           |  7 +++-
>  drivers/regulator/pwm-regulator.c                  | 40 ++++++++++++++++++----
>  2 files changed, 39 insertions(+), 8 deletions(-)

Merged into for-4.8/regulator of the PWM tree and rebased Boris'
pwm-regulator patches on top.

Boris, everything looks right to me, but can you take a quick look to
see if it all matches up with what you expect?

Thanks,
Thierry

[toc] | [prev] | [next] | [standalone]

#1440315

On Mon, 11 Jul 2016 09:02:51 +0200
Thierry Reding <thierry.reding@gmail.com> wrote:

> On Sat, Jul 09, 2016 at 11:47:18AM +0200, Mark Brown wrote:
> > On Fri, Jul 08, 2016 at 05:43:02PM +0200, Thierry Reding wrote:
> >   
> > > Mark, do you want me to provide a stable branch with the PWM regulator
> > > patches and resolve that conflict in your tree? Or would you rather take
> > > the whole set based on a stable branch from the PWM tree? Or maybe yet
> > > another possibility would be to base the PWM tree on a stable branch
> > > from the regulator tree containing the above commit.  
> > 
> > Probably easiest to use this signed tag and resolve it in your tree:
> > 
> > The following changes since commit 1a695a905c18548062509178b98bc91e67510864:
> > 
> >   Linux 4.7-rc1 (2016-05-29 09:29:24 -0700)
> > 
> > are available in the git repository at:
> > 
> >   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git tags/pwm-modernization
> > 
> > for you to fetch changes up to c2588393e6315ab68207323d37d2a73713d6bc81:
> > 
> >   regulator: pwm: Fix regulator ramp delay for continuous mode (2016-07-07 11:45:06 +0200)
> > 
> > ----------------------------------------------------------------
> > regulator: Provide a branch for moderninzation of the PWM code
> > 
> > There's a new, improved PWM API which allows a lot of improvements in
> > the PWM regulator driver.  Since the bulk of the changes are in the PWM
> > API this is being managed in the PWM tree, merge pending regulator API
> > changes to allow this to be resolved more easily.
> > 
> > ----------------------------------------------------------------
> > Alexandre Courbot (1):
> >       regulator: pwm: Support for enable GPIO
> > 
> > Boris Brezillon (1):
> >       regulator: pwm: Drop unneeded pwm_enable() call
> > 
> > Douglas Anderson (1):
> >       regulator: pwm: Fix regulator ramp delay for continuous mode
> > 
> >  .../bindings/regulator/pwm-regulator.txt           |  7 +++-
> >  drivers/regulator/pwm-regulator.c                  | 40 ++++++++++++++++++----
> >  2 files changed, 39 insertions(+), 8 deletions(-)  
> 
> Merged into for-4.8/regulator of the PWM tree and rebased Boris'
> pwm-regulator patches on top.
> 
> Boris, everything looks right to me, but can you take a quick look to
> see if it all matches up with what you expect?

It looks good, but I won't be able to test it on real hardware until
next week.
Heiko, Brian, can you try this pwm/for-4.8/regulator branch?

Thanks,

Boris

[toc] | [prev] | [next] | [standalone]

#1440769

Hi,

On Mon, Jul 11, 2016 at 12:02 AM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> On Sat, Jul 09, 2016 at 11:47:18AM +0200, Mark Brown wrote:
>> On Fri, Jul 08, 2016 at 05:43:02PM +0200, Thierry Reding wrote:
>>
>> > Mark, do you want me to provide a stable branch with the PWM regulator
>> > patches and resolve that conflict in your tree? Or would you rather take
>> > the whole set based on a stable branch from the PWM tree? Or maybe yet
>> > another possibility would be to base the PWM tree on a stable branch
>> > from the regulator tree containing the above commit.
>>
>> Probably easiest to use this signed tag and resolve it in your tree:
>>
>> The following changes since commit 1a695a905c18548062509178b98bc91e67510864:
>>
>>   Linux 4.7-rc1 (2016-05-29 09:29:24 -0700)
>>
>> are available in the git repository at:
>>
>>   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git tags/pwm-modernization
>>
>> for you to fetch changes up to c2588393e6315ab68207323d37d2a73713d6bc81:
>>
>>   regulator: pwm: Fix regulator ramp delay for continuous mode (2016-07-07 11:45:06 +0200)
>>
>> ----------------------------------------------------------------
>> regulator: Provide a branch for moderninzation of the PWM code
>>
>> There's a new, improved PWM API which allows a lot of improvements in
>> the PWM regulator driver.  Since the bulk of the changes are in the PWM
>> API this is being managed in the PWM tree, merge pending regulator API
>> changes to allow this to be resolved more easily.
>>
>> ----------------------------------------------------------------
>> Alexandre Courbot (1):
>>       regulator: pwm: Support for enable GPIO
>>
>> Boris Brezillon (1):
>>       regulator: pwm: Drop unneeded pwm_enable() call
>>
>> Douglas Anderson (1):
>>       regulator: pwm: Fix regulator ramp delay for continuous mode
>>
>>  .../bindings/regulator/pwm-regulator.txt           |  7 +++-
>>  drivers/regulator/pwm-regulator.c                  | 40 ++++++++++++++++++----
>>  2 files changed, 39 insertions(+), 8 deletions(-)
>
> Merged into for-4.8/regulator of the PWM tree and rebased Boris'
> pwm-regulator patches on top.
>
> Boris, everything looks right to me, but can you take a quick look to
> see if it all matches up with what you expect?

As I mentioned in the other thread about the linuxnext conflict,
pwm_regulator_set_voltage() is wrong.

You have:

  ramp_delay = DIV_ROUND_UP(abs(min_uV - old_uV), ramp_delay);

You should have:

  ramp_delay = DIV_ROUND_UP(abs(req_min_uV - old_uV), ramp_delay);

-Doug

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web