linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: hdegoede@redhat.com (Hans de Goede)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] sunxi: a10-lime: add regulator nodes
Date: Sat, 04 Apr 2015 15:05:44 +0200	[thread overview]
Message-ID: <551FE1A8.9060709@redhat.com> (raw)
In-Reply-To: <20150404125645.GK14618@lukather>

Hi,

On 04-04-15 14:56, Maxime Ripard wrote:
> Hi Mark,
>
> On Sat, Apr 04, 2015 at 01:33:02PM +0100, Mark Brown wrote:
>> On Sat, Apr 04, 2015 at 02:18:12PM +0200, Javier Martinez Canillas wrote:
>>> On Tue, Mar 31, 2015 at 12:23 AM, Maxime Ripard
>>
>>>> No, it's defining which regulators are provided by the regulator, and
>>>> the voltage boundaries they have. It doesn't make any assumption with
>>>> regards to what is connected to what, and if a particular regulator is
>>>> connected to something. That's something that the board DTS should
>>>> describe as accurately as possible.
>>
>> This is broken - think about what this means.  If you are defining a
>> voltage (or any other constraint) you're saying that it's safe to use on
>> a given board. If you provide a voltage constraint saying that the
>> maximum allowable range for a voltage regulator is safe.  That's
>> unlikely to be true on any given board, usually only a limited set of
>> regulators can vary voltages at runtime safely at all and then rarely
>> over their full supported range.  Similarly for other constraints, for
>> example allowing a regulator to be disabled when there are driverless
>> things (or drivers without regulator support or mappings for that board)
>> relying on it is going to break.
>>
>> Providing a list of the regulators is safe but not really doing a huge
>> amount.
>>
>> Note that I've not seen the original patch so it's possible it's doing
>> something different.
>
> If this model is broken, then I don't see how the full regulator
> support is not broken.
>
> Our PMIC DTSI sets up the regulators with the ranges supported by the
> PMIC. We have two cases then:
>    - Either the regulator is not used on the board, and it will be
>      disabled by the framework.
>    - Or the regulator is actually used on the board, it will be defined
>      in the DTS, possibly with additional constraints.


Note that we've already stopped defining any constraints in the dtsi as
that indeed does not make much sense, if the dts does not provide
any constraints but does use / enable the regulator then:
1) That is a bug in the dts really
2) It will end up with the actual hardware constraints anyways as those
are coded into the driver.

>What I do see though, is that if we drop the DTSI, all our
> boards will have to define all the PMIC regulators, even if they're
> not used at all on the board, just to have the right behaviour. And
> that's clearly a no-go for me.

Right the advantage of the dtsi is that it lasts ALL regulators,
so that unused ones actually get turned off, where as a dts not using
the dtsi and only containing those it actually uses will result in
the unused regulators being untouched (since they are not listed)
rather then turned off.

Regards,

Hans

  reply	other threads:[~2015-04-04 13:05 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-27 10:58 [PATCH] sunxi: a10-lime: add regulator nodes Iain Paton
2015-03-27 11:02 ` Hans de Goede
2015-03-28 11:53   ` Iain Paton
2015-03-28 23:50     ` Hans de Goede
2015-04-04 10:20       ` Iain Paton
2015-03-30 22:23     ` Maxime Ripard
2015-04-04 10:30       ` Iain Paton
2015-04-04 11:06         ` Hans de Goede
2015-04-04 12:18       ` Javier Martinez Canillas
2015-04-04 12:33         ` Mark Brown
2015-04-04 12:56           ` Maxime Ripard
2015-04-04 13:05             ` Hans de Goede [this message]
2015-04-04 18:54               ` Mark Brown
2015-04-04 18:47             ` Mark Brown
2015-04-07 10:17               ` Maxime Ripard
2015-04-08 12:35                 ` Mark Brown
2015-04-13  8:15                   ` Maxime Ripard
2015-04-13 11:44                     ` Mark Brown
2015-04-14 16:27                       ` Maxime Ripard
2015-04-14 20:06                         ` Mark Brown
2015-04-04 12:58           ` Hans de Goede
2015-04-04 18:52             ` Mark Brown

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=551FE1A8.9060709@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=linux-arm-kernel@lists.infradead.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).