All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
To: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
	festevam-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Cc: lgirdwood-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	pawel.moll-5wv7dgnIgG8@public.gmane.org,
	mark.rutland-5wv7dgnIgG8@public.gmane.org,
	ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org,
	galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org
Subject: Re: [PATCH 2/2] regulator: add mxs regulator driver
Date: Tue, 30 Sep 2014 08:40:26 +0200	[thread overview]
Message-ID: <542A505A.5030109@i2se.com> (raw)
In-Reply-To: <20140929171314.GW16977-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>

Am 29.09.2014 um 19:13 schrieb Mark Brown:
> On Mon, Sep 29, 2014 at 08:38:51AM +0200, Stefan Wahren wrote:
>
>> I'm searching for a good regulator implementation example.
>> Does it apply to ti-abb-regulator.c and twl-regulator.c?
> Possibly.  But bear in mind that it's important to understand the
> hardware you're trying to support.

The question refer more to the devicetree binding and it's implementation.

>
>>> This really needs a comment to explain what on earth is going on here -
>>> the whole thing with writing the same thing twice with two delays is
>>> more than a little odd.  It looks like the driver is trying to busy wait
>>> in cases where the change happens quickly but the comments about "fast"
>>> and "normal" mode make this unclear.
>> The regulator driver polls for the DC_OK bit in the power status register.
>> Quote for reference manual (p. 935): "High when switching DC-DC
>> converter control loop has stabilized after a voltage target change."
>> The two loops comes from the different regulator modes
>> (REGULATOR_MODE_FAST, REGULATOR_MODE_NORMAL).
>> In REGULATOR_MODE_FAST the voltage steping is disabled and changing
>> voltage should be fast. In REGULATOR_MODE_NORMAL voltage steping is
>> enabled and it's take a while for reaching the target voltage.
> I don't think you've fully understood what the different modes mean
> here, that's not normally how a buck convertor works.  The different
> modes would typically control the ability of the regulator to respond
> quickly to changes in load without drifting off regulation, fast mode
> makes the regulator less efficient but more responsive to load changes
> (probably marginally with modern regulators).  It should have relatively
> little to do with the ability to ramp the voltage and certainly not on
> the scale there.
>
>> Do you see more a problem with the two different loops or the redundant
>> register write?
> Both.  The code right now just looks really obscure.

That leads me to the conclusion to drop both mode functions. My
intention is to get the cpufreq-cpu0 aka cpufreq-dt working on i.MX28,
not to build up the complete power system.

@Fabio, @Shawn: What is your opinion?

Best regards

Stefan


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

WARNING: multiple messages have this Message-ID (diff)
From: Stefan Wahren <stefan.wahren@i2se.com>
To: Mark Brown <broonie@kernel.org>,
	shawn.guo@linaro.org, festevam@gmail.com
Cc: lgirdwood@gmail.com, robh+dt@kernel.org, pawel.moll@arm.com,
	mark.rutland@arm.com, ijc+devicetree@hellion.org.uk,
	galak@codeaurora.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, kernel@pengutronix.de
Subject: Re: [PATCH 2/2] regulator: add mxs regulator driver
Date: Tue, 30 Sep 2014 08:40:26 +0200	[thread overview]
Message-ID: <542A505A.5030109@i2se.com> (raw)
In-Reply-To: <20140929171314.GW16977@sirena.org.uk>

Am 29.09.2014 um 19:13 schrieb Mark Brown:
> On Mon, Sep 29, 2014 at 08:38:51AM +0200, Stefan Wahren wrote:
>
>> I'm searching for a good regulator implementation example.
>> Does it apply to ti-abb-regulator.c and twl-regulator.c?
> Possibly.  But bear in mind that it's important to understand the
> hardware you're trying to support.

The question refer more to the devicetree binding and it's implementation.

>
>>> This really needs a comment to explain what on earth is going on here -
>>> the whole thing with writing the same thing twice with two delays is
>>> more than a little odd.  It looks like the driver is trying to busy wait
>>> in cases where the change happens quickly but the comments about "fast"
>>> and "normal" mode make this unclear.
>> The regulator driver polls for the DC_OK bit in the power status register.
>> Quote for reference manual (p. 935): "High when switching DC-DC
>> converter control loop has stabilized after a voltage target change."
>> The two loops comes from the different regulator modes
>> (REGULATOR_MODE_FAST, REGULATOR_MODE_NORMAL).
>> In REGULATOR_MODE_FAST the voltage steping is disabled and changing
>> voltage should be fast. In REGULATOR_MODE_NORMAL voltage steping is
>> enabled and it's take a while for reaching the target voltage.
> I don't think you've fully understood what the different modes mean
> here, that's not normally how a buck convertor works.  The different
> modes would typically control the ability of the regulator to respond
> quickly to changes in load without drifting off regulation, fast mode
> makes the regulator less efficient but more responsive to load changes
> (probably marginally with modern regulators).  It should have relatively
> little to do with the ability to ramp the voltage and certainly not on
> the scale there.
>
>> Do you see more a problem with the two different loops or the redundant
>> register write?
> Both.  The code right now just looks really obscure.

That leads me to the conclusion to drop both mode functions. My
intention is to get the cpufreq-cpu0 aka cpufreq-dt working on i.MX28,
not to build up the complete power system.

@Fabio, @Shawn: What is your opinion?

Best regards

Stefan



  parent reply	other threads:[~2014-09-30  6:40 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-27  0:59 [PATCH 0/2] regulator: add support for mxs regulator Stefan Wahren
2014-09-27  0:59 ` [PATCH 1/2] DT: add binding " Stefan Wahren
     [not found]   ` <1411779588-22031-2-git-send-email-stefan.wahren-eS4NqCHxEME@public.gmane.org>
2014-09-28 10:22     ` Mark Brown
2014-09-28 10:22       ` Mark Brown
     [not found]       ` <20140928102202.GM27755-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2014-09-29  6:00         ` Stefan Wahren
2014-09-29  6:00           ` Stefan Wahren
     [not found]           ` <5428F592.3020809-eS4NqCHxEME@public.gmane.org>
2014-09-29 10:23             ` Mark Brown
2014-09-29 10:23               ` Mark Brown
2014-09-29 11:09     ` Mark Rutland
2014-09-29 11:09       ` Mark Rutland
2014-09-29 11:53       ` Stefan Wahren
2014-09-29 11:53         ` Stefan Wahren
     [not found]         ` <54294832.8000702-eS4NqCHxEME@public.gmane.org>
2014-09-29 12:41           ` Mark Rutland
2014-09-29 12:41             ` Mark Rutland
2014-09-29 13:10             ` Stefan Wahren
2014-09-29 13:10               ` Stefan Wahren
2014-09-29 13:23               ` Mark Rutland
2014-10-03 11:46                 ` Stefan Wahren
2014-10-03 11:46                   ` Stefan Wahren
2014-09-27  0:59 ` [PATCH 2/2] regulator: add mxs regulator driver Stefan Wahren
2014-09-28 10:16   ` Mark Brown
2014-09-29  6:38     ` Stefan Wahren
     [not found]       ` <5428FE7B.8060700-eS4NqCHxEME@public.gmane.org>
2014-09-29 17:13         ` Mark Brown
2014-09-29 17:13           ` Mark Brown
     [not found]           ` <20140929171314.GW16977-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2014-09-30  6:40             ` Stefan Wahren [this message]
2014-09-30  6:40               ` Stefan Wahren
2014-10-01 13:23             ` Stefan Wahren
2014-10-01 13:23               ` Stefan Wahren
2014-10-01 15:57               ` Mark Brown
2014-10-02 16:18   ` Fabio Estevam

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=542A505A.5030109@i2se.com \
    --to=stefan.wahren-es4nqchxeme@public.gmane.org \
    --cc=broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=festevam-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \
    --cc=ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org \
    --cc=kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org \
    --cc=lgirdwood-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=mark.rutland-5wv7dgnIgG8@public.gmane.org \
    --cc=pawel.moll-5wv7dgnIgG8@public.gmane.org \
    --cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.