From: Kevin Hilman <khilman@linaro.org>
To: linux-arm-kernel@lists.infradead.org
Subject: Re: [RFC PATCH 0/3] PM / clock_ops: allow to specify custom pm_clk_notifier callback
Date: Mon, 08 Sep 2014 21:37:29 +0000 [thread overview]
Message-ID: <7hr3zln692.fsf@linaro.org> (raw)
In-Reply-To: <1406313096-29761-1-git-send-email-grygorii.strashko@ti.com> (Grygorii Strashko's message of "Fri, 25 Jul 2014 21:31:33 +0300")
Grygorii Strashko <grygorii.strashko@ti.com> writes:
> The CLK PM domain (clock_ops.c) assumes that platform code should always provide
> list of Connection IDs of the clock (con_id) in pm_clk_notifier_block structure.
> Then CLK PM domain uses these con_ids to setup list of clocks per device.
>
> Such approach is inconvenient when devices can have different number
> of clocks. For example, if maximum number of clocks used by
> devices is 4 the pm_clk_notifier_block structure will look like:
>
> static struct pm_clk_notifier_block platform_domain_notifier = {
> .pm_domain = &keystone_pm_domain,
> .con_ids = { "fck", "opt1", "opt2", "opt3", NULL },
> };
>
> More over, clocks in DT have to be named using only above con_ids:
> clocks = <&paclk13>, <&clkcpgmac>, <&chipclk12>;
> clock-names = "fck", "opt1", "opt2";
>
> It makes hard to enable/support Runtime PM in case when some HW modules are used
> by different SoCs or there are few version of the same SoC, because clock trees
> can be changed significantly in all such cases.
>
> This patch set allows to specify custom pm_clk_notifier callback from
> platform code and in such way makes CLK PM domain initialization
> more flexible.
Replacing the pm_clk notifier is essentially replacing the guts of the
clock_ops code.
I think your platform may have left the realm of "simple" platforms that
the clock_ops was intended for.
Since you're essentially gutting clock_ops, have you considered
migrating to genpd and having your own pm_domain ops that manage your clocks?
> Also, It updates Keystone 2 platform code to provide custom
> callback which fills list of clocks for Device with all clocks assigned to this
> Device in DT.
> More over, It's safe for Keystone 2 to perform CLK PM domain initialization
> at device's binding time instead of device's creation time.
>
> I've posted these patches because I need fully enable Runtime PM on Keystone 2
> where all HW modules are controlled using clocks only. That's why CLK PM domain
> fits our needs perfectly.
Heh, doesn't seem like a perfect fit to me ;)
> Also, on Keystone 2 clocks are initialized early and
> they are available at device creation time. The main problems from my side are:
> - every time when support for new HW modules is added the ".con_ids" array
> need to be checked and updated;
> - restriction for clock's names in DT - to be named using only above con_ids
> It's ugly.
>
> I hope, this approach to be accepted at least as temporal solution until
> the generic solution is not found.
Sorry if I missed it, but is there ongoing discussion of a more generic
solution other than this one:
> [1] Previously, same problem was discussed in, but no final solution was accepted:
> "[PATCH/RFC 0/4] of: Register clocks for Runtime PM with PM core"
> https://lkml.org/lkml/2014/4/24/1118
Personally, I still like Geert's approach better.
Kevin
WARNING: multiple messages have this Message-ID (diff)
From: Kevin Hilman <khilman@linaro.org>
To: Grygorii Strashko <grygorii.strashko@ti.com>
Cc: <santosh.shilimkar@ti.com>,
"Rafael J. Wysocki" <rjw@rjwysocki.net>,
Geert Uytterhoeven <geert+renesas@glider.be>,
<linux-pm@vger.kernel.org>, <ben.dooks@codethink.co.uk>,
<laurent.pinchart@ideasonboard.com>, <grant.likely@secretlab.ca>,
<ulf.hansson@linaro.org>, <linux-arm-kernel@lists.infradead.org>,
<linux-kernel@vger.kernel.org>, <linux-sh@vger.kernel.org>
Subject: Re: [RFC PATCH 0/3] PM / clock_ops: allow to specify custom pm_clk_notifier callback
Date: Mon, 08 Sep 2014 14:37:29 -0700 [thread overview]
Message-ID: <7hr3zln692.fsf@linaro.org> (raw)
In-Reply-To: <1406313096-29761-1-git-send-email-grygorii.strashko@ti.com> (Grygorii Strashko's message of "Fri, 25 Jul 2014 21:31:33 +0300")
Grygorii Strashko <grygorii.strashko@ti.com> writes:
> The CLK PM domain (clock_ops.c) assumes that platform code should always provide
> list of Connection IDs of the clock (con_id) in pm_clk_notifier_block structure.
> Then CLK PM domain uses these con_ids to setup list of clocks per device.
>
> Such approach is inconvenient when devices can have different number
> of clocks. For example, if maximum number of clocks used by
> devices is 4 the pm_clk_notifier_block structure will look like:
>
> static struct pm_clk_notifier_block platform_domain_notifier = {
> .pm_domain = &keystone_pm_domain,
> .con_ids = { "fck", "opt1", "opt2", "opt3", NULL },
> };
>
> More over, clocks in DT have to be named using only above con_ids:
> clocks = <&paclk13>, <&clkcpgmac>, <&chipclk12>;
> clock-names = "fck", "opt1", "opt2";
>
> It makes hard to enable/support Runtime PM in case when some HW modules are used
> by different SoCs or there are few version of the same SoC, because clock trees
> can be changed significantly in all such cases.
>
> This patch set allows to specify custom pm_clk_notifier callback from
> platform code and in such way makes CLK PM domain initialization
> more flexible.
Replacing the pm_clk notifier is essentially replacing the guts of the
clock_ops code.
I think your platform may have left the realm of "simple" platforms that
the clock_ops was intended for.
Since you're essentially gutting clock_ops, have you considered
migrating to genpd and having your own pm_domain ops that manage your clocks?
> Also, It updates Keystone 2 platform code to provide custom
> callback which fills list of clocks for Device with all clocks assigned to this
> Device in DT.
> More over, It's safe for Keystone 2 to perform CLK PM domain initialization
> at device's binding time instead of device's creation time.
>
> I've posted these patches because I need fully enable Runtime PM on Keystone 2
> where all HW modules are controlled using clocks only. That's why CLK PM domain
> fits our needs perfectly.
Heh, doesn't seem like a perfect fit to me ;)
> Also, on Keystone 2 clocks are initialized early and
> they are available at device creation time. The main problems from my side are:
> - every time when support for new HW modules is added the ".con_ids" array
> need to be checked and updated;
> - restriction for clock's names in DT - to be named using only above con_ids
> It's ugly.
>
> I hope, this approach to be accepted at least as temporal solution until
> the generic solution is not found.
Sorry if I missed it, but is there ongoing discussion of a more generic
solution other than this one:
> [1] Previously, same problem was discussed in, but no final solution was accepted:
> "[PATCH/RFC 0/4] of: Register clocks for Runtime PM with PM core"
> https://lkml.org/lkml/2014/4/24/1118
Personally, I still like Geert's approach better.
Kevin
WARNING: multiple messages have this Message-ID (diff)
From: Kevin Hilman <khilman@linaro.org>
To: Grygorii Strashko <grygorii.strashko@ti.com>
Cc: santosh.shilimkar@ti.com, "Rafael J. Wysocki" <rjw@rjwysocki.net>,
Geert Uytterhoeven <geert+renesas@glider.be>,
linux-pm@vger.kernel.org, ben.dooks@codethink.co.uk,
laurent.pinchart@ideasonboard.com, grant.likely@secretlab.ca,
ulf.hansson@linaro.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-sh@vger.kernel.org
Subject: Re: [RFC PATCH 0/3] PM / clock_ops: allow to specify custom pm_clk_notifier callback
Date: Mon, 08 Sep 2014 14:37:29 -0700 [thread overview]
Message-ID: <7hr3zln692.fsf@linaro.org> (raw)
In-Reply-To: <1406313096-29761-1-git-send-email-grygorii.strashko@ti.com> (Grygorii Strashko's message of "Fri, 25 Jul 2014 21:31:33 +0300")
Grygorii Strashko <grygorii.strashko@ti.com> writes:
> The CLK PM domain (clock_ops.c) assumes that platform code should always provide
> list of Connection IDs of the clock (con_id) in pm_clk_notifier_block structure.
> Then CLK PM domain uses these con_ids to setup list of clocks per device.
>
> Such approach is inconvenient when devices can have different number
> of clocks. For example, if maximum number of clocks used by
> devices is 4 the pm_clk_notifier_block structure will look like:
>
> static struct pm_clk_notifier_block platform_domain_notifier = {
> .pm_domain = &keystone_pm_domain,
> .con_ids = { "fck", "opt1", "opt2", "opt3", NULL },
> };
>
> More over, clocks in DT have to be named using only above con_ids:
> clocks = <&paclk13>, <&clkcpgmac>, <&chipclk12>;
> clock-names = "fck", "opt1", "opt2";
>
> It makes hard to enable/support Runtime PM in case when some HW modules are used
> by different SoCs or there are few version of the same SoC, because clock trees
> can be changed significantly in all such cases.
>
> This patch set allows to specify custom pm_clk_notifier callback from
> platform code and in such way makes CLK PM domain initialization
> more flexible.
Replacing the pm_clk notifier is essentially replacing the guts of the
clock_ops code.
I think your platform may have left the realm of "simple" platforms that
the clock_ops was intended for.
Since you're essentially gutting clock_ops, have you considered
migrating to genpd and having your own pm_domain ops that manage your clocks?
> Also, It updates Keystone 2 platform code to provide custom
> callback which fills list of clocks for Device with all clocks assigned to this
> Device in DT.
> More over, It's safe for Keystone 2 to perform CLK PM domain initialization
> at device's binding time instead of device's creation time.
>
> I've posted these patches because I need fully enable Runtime PM on Keystone 2
> where all HW modules are controlled using clocks only. That's why CLK PM domain
> fits our needs perfectly.
Heh, doesn't seem like a perfect fit to me ;)
> Also, on Keystone 2 clocks are initialized early and
> they are available at device creation time. The main problems from my side are:
> - every time when support for new HW modules is added the ".con_ids" array
> need to be checked and updated;
> - restriction for clock's names in DT - to be named using only above con_ids
> It's ugly.
>
> I hope, this approach to be accepted at least as temporal solution until
> the generic solution is not found.
Sorry if I missed it, but is there ongoing discussion of a more generic
solution other than this one:
> [1] Previously, same problem was discussed in, but no final solution was accepted:
> "[PATCH/RFC 0/4] of: Register clocks for Runtime PM with PM core"
> https://lkml.org/lkml/2014/4/24/1118
Personally, I still like Geert's approach better.
Kevin
WARNING: multiple messages have this Message-ID (diff)
From: khilman@linaro.org (Kevin Hilman)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH 0/3] PM / clock_ops: allow to specify custom pm_clk_notifier callback
Date: Mon, 08 Sep 2014 14:37:29 -0700 [thread overview]
Message-ID: <7hr3zln692.fsf@linaro.org> (raw)
In-Reply-To: <1406313096-29761-1-git-send-email-grygorii.strashko@ti.com> (Grygorii Strashko's message of "Fri, 25 Jul 2014 21:31:33 +0300")
Grygorii Strashko <grygorii.strashko@ti.com> writes:
> The CLK PM domain (clock_ops.c) assumes that platform code should always provide
> list of Connection IDs of the clock (con_id) in pm_clk_notifier_block structure.
> Then CLK PM domain uses these con_ids to setup list of clocks per device.
>
> Such approach is inconvenient when devices can have different number
> of clocks. For example, if maximum number of clocks used by
> devices is 4 the pm_clk_notifier_block structure will look like:
>
> static struct pm_clk_notifier_block platform_domain_notifier = {
> .pm_domain = &keystone_pm_domain,
> .con_ids = { "fck", "opt1", "opt2", "opt3", NULL },
> };
>
> More over, clocks in DT have to be named using only above con_ids:
> clocks = <&paclk13>, <&clkcpgmac>, <&chipclk12>;
> clock-names = "fck", "opt1", "opt2";
>
> It makes hard to enable/support Runtime PM in case when some HW modules are used
> by different SoCs or there are few version of the same SoC, because clock trees
> can be changed significantly in all such cases.
>
> This patch set allows to specify custom pm_clk_notifier callback from
> platform code and in such way makes CLK PM domain initialization
> more flexible.
Replacing the pm_clk notifier is essentially replacing the guts of the
clock_ops code.
I think your platform may have left the realm of "simple" platforms that
the clock_ops was intended for.
Since you're essentially gutting clock_ops, have you considered
migrating to genpd and having your own pm_domain ops that manage your clocks?
> Also, It updates Keystone 2 platform code to provide custom
> callback which fills list of clocks for Device with all clocks assigned to this
> Device in DT.
> More over, It's safe for Keystone 2 to perform CLK PM domain initialization
> at device's binding time instead of device's creation time.
>
> I've posted these patches because I need fully enable Runtime PM on Keystone 2
> where all HW modules are controlled using clocks only. That's why CLK PM domain
> fits our needs perfectly.
Heh, doesn't seem like a perfect fit to me ;)
> Also, on Keystone 2 clocks are initialized early and
> they are available at device creation time. The main problems from my side are:
> - every time when support for new HW modules is added the ".con_ids" array
> need to be checked and updated;
> - restriction for clock's names in DT - to be named using only above con_ids
> It's ugly.
>
> I hope, this approach to be accepted at least as temporal solution until
> the generic solution is not found.
Sorry if I missed it, but is there ongoing discussion of a more generic
solution other than this one:
> [1] Previously, same problem was discussed in, but no final solution was accepted:
> "[PATCH/RFC 0/4] of: Register clocks for Runtime PM with PM core"
> https://lkml.org/lkml/2014/4/24/1118
Personally, I still like Geert's approach better.
Kevin
next prev parent reply other threads:[~2014-09-08 21:37 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-25 17:47 [RFC PATCH 0/3] PM / clock_ops: allow to specify custom pm_clk_notifier callback Grygorii Strashko
2014-07-25 18:31 ` Grygorii Strashko
2014-07-25 18:31 ` Grygorii Strashko
2014-07-25 18:31 ` Grygorii Strashko
2014-07-25 17:46 ` [RFC PATCH 2/3] " Grygorii Strashko
2014-07-25 18:31 ` Grygorii Strashko
2014-07-25 18:31 ` Grygorii Strashko
2014-07-25 18:31 ` Grygorii Strashko
2014-07-25 17:47 ` [RFC PATCH 3/3] ARM: keystone: pm_domain: setup clk pm domain clocks from DT Grygorii Strashko
2014-07-25 18:31 ` Grygorii Strashko
2014-07-25 18:31 ` Grygorii Strashko
2014-07-25 18:31 ` Grygorii Strashko
2014-09-08 20:47 ` Kevin Hilman
2014-09-08 20:47 ` Kevin Hilman
2014-09-08 20:47 ` Kevin Hilman
2014-09-08 20:47 ` Kevin Hilman
2014-07-25 17:47 ` [RFC PATCH 1/3] PM / clock_ops: Add pm_clk_add_clk() Grygorii Strashko
2014-07-25 18:31 ` Grygorii Strashko
2014-07-25 18:31 ` Grygorii Strashko
2014-07-25 18:31 ` Grygorii Strashko
2014-09-08 21:37 ` Kevin Hilman [this message]
2014-09-08 21:37 ` [RFC PATCH 0/3] PM / clock_ops: allow to specify custom pm_clk_notifier callback Kevin Hilman
2014-09-08 21:37 ` Kevin Hilman
2014-09-08 21:37 ` Kevin Hilman
2014-09-09 13:41 ` Grygorii Strashko
2014-09-09 13:41 ` Grygorii Strashko
2014-09-09 13:41 ` Grygorii Strashko
2014-09-09 13:41 ` Grygorii Strashko
2014-09-10 12:34 ` Geert Uytterhoeven
2014-09-10 12:34 ` Geert Uytterhoeven
2014-09-10 12:34 ` Geert Uytterhoeven
2014-09-10 12:34 ` Geert Uytterhoeven
2014-09-17 15:03 ` Geert Uytterhoeven
2014-09-17 15:03 ` Geert Uytterhoeven
2014-09-17 15:03 ` Geert Uytterhoeven
2014-09-17 15:03 ` Geert Uytterhoeven
2014-09-17 16:04 ` Grygorii Strashko
2014-09-17 16:04 ` Grygorii Strashko
2014-09-17 16:04 ` Grygorii Strashko
2014-09-17 16:04 ` Grygorii Strashko
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=7hr3zln692.fsf@linaro.org \
--to=khilman@linaro.org \
--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 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.