All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Hilman <khilman@kernel.org>
To: "Heiko Stübner" <heiko@sntech.de>
Cc: Caesar Wang <wxt@rock-chips.com>,
	tomasz.figa@gmail.com, linux-arm-kernel@lists.infradead.org,
	linus.walleij@linaro.org, robh+dt@kernel.org, pawel.moll@arm.com,
	mark.rutland@arm.com, galak@codeaurora.org,
	grant.likely@linaro.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, rdunlap@infradead.org,
	linux-doc@vger.kernel.org, dianders@chromium.org,
	linux-rockchip@lists.infradead.org, ulf.hansson@linaro.org,
	dmitry.torokhov@gmail.com, broonie@kernel.org,
	ijc+devicetree@hellion.org.uk, linux@arm.linux.org.uk
Subject: Re: [PATCH v14 0/3] ARM: rk3288: Add PM Domain support
Date: Mon, 27 Apr 2015 11:28:42 -0700	[thread overview]
Message-ID: <7h383lzaqt.fsf@deeprootsystems.com> (raw)
In-Reply-To: <3550912.kR0pejLP0h@diego> ("Heiko Stübner"'s message of "Sat, 25 Apr 2015 20:47:49 +0200")

Heiko Stübner <heiko@sntech.de> writes:

> Am Freitag, 24. April 2015, 16:07:45 schrieb Caesar Wang:
>>     Add power domain drivers based on generic power domain for
>> Rockchip platform, and support RK3288.
>> 
>>     Verified on url =
>>     https://chromium.googlesource.com/chromiumos/third_party/kernel.
>> 
>>     At the moment,there are mass of products are using the driver.
>> I believe the driver can happy work for next kernel.
>
> I've taken a look at the driver and here are some global remarks:
>
> (1) You provide dt-bindings/power-domain/rk3288.h in patch 3. This breaks
> bisectability, as the driver itself in patch 2 also includes the header and
> would thus fail to compile if the later patch 3 is missing.
> Ideally I think the header addition should be a separate patch itself, so that
> we can possibly share it between driver and dts branches.
> So 1: binding doc, 2: binding-header, 3: driver, 4: dts-changes.
>
>
> (2) The dts-changes in patch 3 should also add any necessary power-domain
> assignment on devices if they're still missing, so that we don't introduce
> regressions. In my case my work-in-progress edp died because the powerdomain
> was turned off automatically it seems.
>
>
> (3) more like wondering @Kevin or so, is there some more generic place for a
> power-domain driver nowadays?

I think the preference has been to put these under drivers/soc/<vendor> for now,
so they can shared across arm32 and arm64.

> (4) As Tomasz remarked previously the dts should represent the hardware and
> the power-domains are part of the pmu. There is a recent addition from Linus
> Walleij, called simple-mfd [a] that is supposed to get added real early for
> kernel 4.2. So I'd think the power-domains should use that and the patchset
> modified to include the changes shown below [b]?
>
> (5) Keven Hilman and Tomasz had reservations about all the device clocks
> being listed in the power-domains itself in the previous versions. I don't see
> a comment from them yet changing that view.

Correct.  

> Their wish was to get the clocks by reading the clocks from the device nodes,
> though I see a problem on how to handle devices that do not have any bindings
> at all yet.
>
> Kevin, Tomasz any new thoughts?

I don't see any issues with devices that don't have bindings, as all
that would be needed would be to simple device nodes with a clock
property.  I wouldn't even matter if those devices had device drivers.

Kevin

WARNING: multiple messages have this Message-ID (diff)
From: khilman@kernel.org (Kevin Hilman)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v14 0/3] ARM: rk3288: Add PM Domain support
Date: Mon, 27 Apr 2015 11:28:42 -0700	[thread overview]
Message-ID: <7h383lzaqt.fsf@deeprootsystems.com> (raw)
In-Reply-To: <3550912.kR0pejLP0h@diego> ("Heiko Stübner"'s message of "Sat, 25 Apr 2015 20:47:49 +0200")

Heiko St?bner <heiko@sntech.de> writes:

> Am Freitag, 24. April 2015, 16:07:45 schrieb Caesar Wang:
>>     Add power domain drivers based on generic power domain for
>> Rockchip platform, and support RK3288.
>> 
>>     Verified on url =
>>     https://chromium.googlesource.com/chromiumos/third_party/kernel.
>> 
>>     At the moment,there are mass of products are using the driver.
>> I believe the driver can happy work for next kernel.
>
> I've taken a look at the driver and here are some global remarks:
>
> (1) You provide dt-bindings/power-domain/rk3288.h in patch 3. This breaks
> bisectability, as the driver itself in patch 2 also includes the header and
> would thus fail to compile if the later patch 3 is missing.
> Ideally I think the header addition should be a separate patch itself, so that
> we can possibly share it between driver and dts branches.
> So 1: binding doc, 2: binding-header, 3: driver, 4: dts-changes.
>
>
> (2) The dts-changes in patch 3 should also add any necessary power-domain
> assignment on devices if they're still missing, so that we don't introduce
> regressions. In my case my work-in-progress edp died because the powerdomain
> was turned off automatically it seems.
>
>
> (3) more like wondering @Kevin or so, is there some more generic place for a
> power-domain driver nowadays?

I think the preference has been to put these under drivers/soc/<vendor> for now,
so they can shared across arm32 and arm64.

> (4) As Tomasz remarked previously the dts should represent the hardware and
> the power-domains are part of the pmu. There is a recent addition from Linus
> Walleij, called simple-mfd [a] that is supposed to get added real early for
> kernel 4.2. So I'd think the power-domains should use that and the patchset
> modified to include the changes shown below [b]?
>
> (5) Keven Hilman and Tomasz had reservations about all the device clocks
> being listed in the power-domains itself in the previous versions. I don't see
> a comment from them yet changing that view.

Correct.  

> Their wish was to get the clocks by reading the clocks from the device nodes,
> though I see a problem on how to handle devices that do not have any bindings
> at all yet.
>
> Kevin, Tomasz any new thoughts?

I don't see any issues with devices that don't have bindings, as all
that would be needed would be to simple device nodes with a clock
property.  I wouldn't even matter if those devices had device drivers.

Kevin

  reply	other threads:[~2015-04-27 18:28 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-24  8:07 [PATCH v14 0/3] ARM: rk3288: Add PM Domain support Caesar Wang
2015-04-24  8:07 ` [PATCH v14 1/3] dt-bindings: add document of Rockchip power domain Caesar Wang
2015-05-28  9:02   ` Ulf Hansson
2015-05-28  9:02     ` Ulf Hansson
2015-05-28  9:02     ` Ulf Hansson
2015-04-24  8:07 ` [PATCH v14 2/3] power-domain: rockchip: add power domain driver Caesar Wang
2015-05-28 10:38   ` Ulf Hansson
2015-05-28 10:38     ` Ulf Hansson
2015-05-28 10:38     ` Ulf Hansson
     [not found]     ` <CAPDyKFrBzZjOVZzq9unQQwm2BSA0kzeT7EE8_kSpH_udMKjo6A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-06-14  3:15       ` Caesar Wang
2015-06-14  3:15         ` Caesar Wang
2015-06-14  3:15         ` Caesar Wang
2015-06-25 15:33         ` Ulf Hansson
2015-06-25 15:33           ` Ulf Hansson
2015-06-25 15:33           ` Ulf Hansson
2015-06-29  8:16           ` Caesar Wang
2015-06-29  8:16             ` Caesar Wang
2015-06-29  8:16             ` Caesar Wang
2015-04-24  8:07 ` [PATCH v14 3/3] ARM: dts: add RK3288 power-domain node Caesar Wang
2015-04-25 18:47 ` [PATCH v14 0/3] ARM: rk3288: Add PM Domain support Heiko Stübner
2015-04-25 18:47   ` Heiko Stübner
2015-04-27 18:28   ` Kevin Hilman [this message]
2015-04-27 18:28     ` Kevin Hilman
2015-06-12  5:11     ` Caesar Wang
2015-06-12  5:11       ` Caesar Wang

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=7h383lzaqt.fsf@deeprootsystems.com \
    --to=khilman@kernel.org \
    --cc=broonie@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dianders@chromium.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=galak@codeaurora.org \
    --cc=grant.likely@linaro.org \
    --cc=heiko@sntech.de \
    --cc=ijc+devicetree@hellion.org.uk \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=linux@arm.linux.org.uk \
    --cc=mark.rutland@arm.com \
    --cc=pawel.moll@arm.com \
    --cc=rdunlap@infradead.org \
    --cc=robh+dt@kernel.org \
    --cc=tomasz.figa@gmail.com \
    --cc=ulf.hansson@linaro.org \
    --cc=wxt@rock-chips.com \
    /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.