From: Joseph Lo <josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> To: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> Cc: "linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" <linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org> Subject: Re: [PATCH 2/3] ARM: tegra: refactor the pmc_pm_set function Date: Thu, 14 Mar 2013 09:44:16 +0800 [thread overview] Message-ID: <1363225456.3308.23.camel@jlo-ubuntu-64.nvidia.com> (raw) In-Reply-To: <5140BCF9.2070900-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> On Thu, 2013-03-14 at 01:52 +0800, Stephen Warren wrote: > On 03/13/2013 02:27 AM, Joseph Lo wrote: > > The pmc_pm_set function was designed for SoC to configure the related > > settings when system going to some low power modes (e.g. platform > > suspend or CPU idle powered-down mode). We refactor the function to make > > it work on the usage. > > > diff --git a/arch/arm/mach-tegra/pmc.c b/arch/arm/mach-tegra/pmc.c > > > +#include <linux/clk-provider.h> > > This code isn't a clock-provider, and hence shouldn't include that > header file. > > > +void tegra_pmc_pm_set(enum tegra_suspend_mode mode) > > > + switch (mode) { > > + case TEGRA_SUSPEND_LP2: > > + rate = __clk_get_rate(tegra_pclk); > > Doesn't regular clk_get_rate() work here? > Actually it works. So I will use clk_get_rate() here and remove clk-provider. > > @@ -234,6 +229,9 @@ void tegra_pmc_suspend_init(void) > > { > > u32 reg; > > > > + tegra_pclk = clk_get_sys(NULL, "pclk"); > > + WARN_ON(IS_ERR(tegra_pclk)); > > Shouldn't this instead error out and/or disable any system suspend > modes? Otherwise, tegra_pmc_pm_set() is going to call > clk_get_rate(tegra_pclk), and tegra_pclk won't be a valid clock object. OK. > > Also, can you use regular clk_get() rather than clk_get_sys()? That'd > need a clocks property in the PMC DT node. OK. > I guess I see now that this series does actually depend on the suspend > series. However, stuff like: > > > -u32 tegra_pmc_get_cpu_good_time(void) > > -{ > > - return pmc_pm_data.cpu_good_time; > > -} > > - > > -u32 tegra_pmc_get_cpu_off_time(void) > > -{ > > - return pmc_pm_data.cpu_off_time; > > -} > > ... was actually added in that series, so if you put this series first, > or merged the two series with these patches first, you could end up > avoiding some churn. Let me check how to reorganize them. Thanks, Joseph
WARNING: multiple messages have this Message-ID (diff)
From: josephl@nvidia.com (Joseph Lo) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH 2/3] ARM: tegra: refactor the pmc_pm_set function Date: Thu, 14 Mar 2013 09:44:16 +0800 [thread overview] Message-ID: <1363225456.3308.23.camel@jlo-ubuntu-64.nvidia.com> (raw) In-Reply-To: <5140BCF9.2070900@wwwdotorg.org> On Thu, 2013-03-14 at 01:52 +0800, Stephen Warren wrote: > On 03/13/2013 02:27 AM, Joseph Lo wrote: > > The pmc_pm_set function was designed for SoC to configure the related > > settings when system going to some low power modes (e.g. platform > > suspend or CPU idle powered-down mode). We refactor the function to make > > it work on the usage. > > > diff --git a/arch/arm/mach-tegra/pmc.c b/arch/arm/mach-tegra/pmc.c > > > +#include <linux/clk-provider.h> > > This code isn't a clock-provider, and hence shouldn't include that > header file. > > > +void tegra_pmc_pm_set(enum tegra_suspend_mode mode) > > > + switch (mode) { > > + case TEGRA_SUSPEND_LP2: > > + rate = __clk_get_rate(tegra_pclk); > > Doesn't regular clk_get_rate() work here? > Actually it works. So I will use clk_get_rate() here and remove clk-provider. > > @@ -234,6 +229,9 @@ void tegra_pmc_suspend_init(void) > > { > > u32 reg; > > > > + tegra_pclk = clk_get_sys(NULL, "pclk"); > > + WARN_ON(IS_ERR(tegra_pclk)); > > Shouldn't this instead error out and/or disable any system suspend > modes? Otherwise, tegra_pmc_pm_set() is going to call > clk_get_rate(tegra_pclk), and tegra_pclk won't be a valid clock object. OK. > > Also, can you use regular clk_get() rather than clk_get_sys()? That'd > need a clocks property in the PMC DT node. OK. > I guess I see now that this series does actually depend on the suspend > series. However, stuff like: > > > -u32 tegra_pmc_get_cpu_good_time(void) > > -{ > > - return pmc_pm_data.cpu_good_time; > > -} > > - > > -u32 tegra_pmc_get_cpu_off_time(void) > > -{ > > - return pmc_pm_data.cpu_off_time; > > -} > > ... was actually added in that series, so if you put this series first, > or merged the two series with these patches first, you could end up > avoiding some churn. Let me check how to reorganize them. Thanks, Joseph
next prev parent reply other threads:[~2013-03-14 1:44 UTC|newest] Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top 2013-03-13 8:27 [PATCH 1/3] ARM: tegra: moving the CPU power timer function to PMC driver Joseph Lo 2013-03-13 8:27 ` Joseph Lo [not found] ` <1363163246-26368-1-git-send-email-josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2013-03-13 8:27 ` [PATCH 2/3] ARM: tegra: refactor the pmc_pm_set function Joseph Lo 2013-03-13 8:27 ` Joseph Lo [not found] ` <1363163246-26368-2-git-send-email-josephl-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2013-03-13 17:52 ` Stephen Warren 2013-03-13 17:52 ` Stephen Warren [not found] ` <5140BCF9.2070900-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> 2013-03-14 1:44 ` Joseph Lo [this message] 2013-03-14 1:44 ` Joseph Lo 2013-03-13 8:27 ` [PATCH 3/3] ARM: tegra: cpuidle: remove redundant parameters for powered-down mode Joseph Lo 2013-03-13 8:27 ` Joseph Lo 2013-03-13 17:40 ` [PATCH 1/3] ARM: tegra: moving the CPU power timer function to PMC driver Stephen Warren 2013-03-13 17:40 ` Stephen Warren [not found] ` <5140B9F6.3060604-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> 2013-03-14 1:34 ` Joseph Lo 2013-03-14 1:34 ` Joseph Lo
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=1363225456.3308.23.camel@jlo-ubuntu-64.nvidia.com \ --to=josephl-ddmlm1+adcrqt0dzr+alfa@public.gmane.org \ --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \ --cc=linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=swarren-3lzwWm7+Weoh9ZMKESR00Q@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: linkBe 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.