LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Mark Brown <broonie@kernel.org>
To: Bjorn Andersson <bjorn.andersson@sonymobile.com>
Cc: Chanwoo Choi <cw00.choi@samsung.com>,
	Ian Campbell <ijc+devicetree@hellion.org.uk>,
	Krzysztof Kozlowski <k.kozlowski@samsung.com>,
	Kumar Gala <galak@codeaurora.org>,
	Lee Jones <lee.jones@linaro.org>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Pawel Moll <pawel.moll@arm.com>, Rob Herring <robh+dt@kernel.org>,
	Stephen Boyd <sboyd@codeaurora.org>,
	Andy Gross <agross@codeaurora.org>,
	Srinivas Kandagatla <srinivas.kandagatla@linaro.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-arm-msm@vger.kernel.org" <linux-arm-msm@vger.kernel.org>
Subject: Re: [PATCH 2/4] regulator: core: Expose init_data to of_parse_cb
Date: Thu, 5 Mar 2015 00:42:20 +0000	[thread overview]
Message-ID: <20150305004220.GB21293@sirena.org.uk> (raw)
In-Reply-To: <20150303161541.GE26334@sonymobile.com>

[-- Attachment #1: Type: text/plain, Size: 1993 bytes --]

On Tue, Mar 03, 2015 at 08:15:41AM -0800, Bjorn Andersson wrote:
> On Tue 03 Mar 04:50 PST 2015, Mark Brown wrote:

> > Why would the driver need to do that?  I guess I'll see later on in the
> > series but the changelog should make that clear.  Drivers aren't
> > supposed to ever need to look at their init data, modifying the init
> > data from what the machine provied is usually a red flag.

> I think what you're getting at is that the init_data should come from a
> board file or device tree. With the reworkings done in patch 4 this

Yes, that's the entire purpose of the init data.

> As this matches a regulator, there is no way for the driver (or anyone
> else to affect the init_data).

That's a goal not a problem.  There is an interface for the machine
description to configure the system integration which neither the
regulator driver nor the consumer driver should be a part of.

> The problem at hand is that there is nothing in this code path telling
> the core that we can do DRMS - something we can figure out in the driver
> by basically looking at the ops for the registering regulator.

No, that way lies people doing all the crap they usually do with putting
the default constraints for their reference design or the maximum limits
for their PMIC into the constraints and then getting upset when drivers
then go and use this to make their CPU catch fire or whatever.

You need to provide a way for the machine description to say DRMS (or
really setting the load which is what's actually happening here now we
have that op) is safe on a given system.  I think that means adding a
new permission to DT and then using that; it's more sensible to want to
use this feature in the context where we're working with an external
processor which also has a chance to have system tuning than when we're
working with the PMIC directly so a permission that enable load setting
seems good.

We could even kill the existing DRMS permission, there's one user in a
legacy STE platform.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

  reply	other threads:[~2015-03-05  0:42 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-03  4:25 [PATCH 0/4] Refactor Qualcomm RPM regulator to single platform_device Bjorn Andersson
2015-03-03  4:25 ` [PATCH 1/4] mfd: devicetree: bindings: Add Qualcomm RPM regulator subnodes Bjorn Andersson
2015-03-03 12:47   ` Mark Brown
2015-03-03 16:02     ` Bjorn Andersson
2015-03-05  0:33       ` Mark Brown
2015-03-03 18:53   ` Stephen Boyd
2015-03-03 21:54     ` Bjorn Andersson
2015-03-03 22:02       ` Stephen Boyd
2015-03-03 22:17         ` Bjorn Andersson
2015-03-03 23:25           ` Stephen Boyd
2015-03-03  4:25 ` [PATCH 2/4] regulator: core: Expose init_data to of_parse_cb Bjorn Andersson
2015-03-03 12:50   ` Mark Brown
2015-03-03 16:15     ` Bjorn Andersson
2015-03-05  0:42       ` Mark Brown [this message]
2015-03-03  4:25 ` [PATCH 3/4] regulator: qcom: Refactor of-parsing code Bjorn Andersson
2015-03-03 14:13   ` Mark Brown
2015-03-03 16:26     ` Bjorn Andersson
2015-03-03 18:56   ` Stephen Boyd
2015-03-03 22:07     ` Bjorn Andersson
2015-03-03  4:25 ` [PATCH 4/4] regulator: qcom: Rework to single platform device Bjorn Andersson
2015-03-03 22:09   ` Stephen Boyd
2015-03-03 22:32     ` Bjorn Andersson
2015-03-03 23:52       ` Mark Brown
2015-03-04  0:01         ` Stephen Boyd
2015-03-04  0:09           ` Mark Brown
2015-03-04 19:35   ` Stephen Boyd
2015-03-04 23:51     ` Bjorn Andersson
2015-03-05  0:56       ` Mark Brown
2015-03-05  0:30     ` Mark Brown
2015-03-05  1:46       ` Stephen Boyd
2015-03-05 10:38         ` 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=20150305004220.GB21293@sirena.org.uk \
    --to=broonie@kernel.org \
    --cc=agross@codeaurora.org \
    --cc=bjorn.andersson@sonymobile.com \
    --cc=cw00.choi@samsung.com \
    --cc=devicetree@vger.kernel.org \
    --cc=galak@codeaurora.org \
    --cc=ijc+devicetree@hellion.org.uk \
    --cc=k.kozlowski@samsung.com \
    --cc=lee.jones@linaro.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=pawel.moll@arm.com \
    --cc=robh+dt@kernel.org \
    --cc=sboyd@codeaurora.org \
    --cc=srinivas.kandagatla@linaro.org \
    --subject='Re: [PATCH 2/4] regulator: core: Expose init_data to of_parse_cb' \
    /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

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