From: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> To: Kevin Hilman <khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>, Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> Cc: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>, Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, Alexandre Courbot <gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, "Rafael J. Wysocki" <rjw-LthD3rsA81gm4RdzfppkhA@public.gmane.org>, Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>, Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>, Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>, Ian Campbell <ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>, Kumar Gala <galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>, "linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, "linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, "devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" <linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org> Subject: Re: [PATCH V5 11/14] soc: tegra: pmc: Add generic PM domain support Date: Mon, 15 Feb 2016 11:27:39 +0000 [thread overview] Message-ID: <56C1B62B.5060708@nvidia.com> (raw) In-Reply-To: <7hh9hdzflv.fsf-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> On 12/02/16 23:14, Kevin Hilman wrote: > Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> writes: > >> On 28 January 2016 at 17:33, Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote: >>> Adds generic PM support to the PMC driver where the PM domains are >>> populated from device-tree and the PM domain consumer devices are >>> bound to their relevant PM domains via device-tree as well. >>> >>> Update the tegra_powergate_sequence_power_up() API so that internally >>> it calls the same tegra_powergate_xxx functions that are used by the >>> tegra generic power domain code for consistency. >>> >>> This is based upon work by Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> >>> and Vince Hsu <vinceh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>. >>> >>> Signed-off-by: Jon Hunter <jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> > > [...] > >>> +static void tegra_powergate_disable_clocks(struct tegra_powergate *pg) >>> +{ >>> + unsigned int i; >>> + >>> + for (i = 0; i < pg->num_clks; i++) >>> + clk_disable_unprepare(pg->clks[i]); >>> +} >>> + >>> +static int tegra_powergate_enable_clocks(struct tegra_powergate *pg) >>> +{ >>> + unsigned int i; >>> + int err; >>> + >>> + for (i = 0; i < pg->num_clks; i++) { >>> + err = clk_prepare_enable(pg->clks[i]); >>> + if (err) >>> + goto out; >>> + } >>> + >>> + return 0; >>> + >>> +out: >>> + while (i--) >>> + clk_disable_unprepare(pg->clks[i]); >>> + >>> + return err; >>> +} >> >> I have seen similar code around in other PM domains, dealing with >> enabling/disabling a *list* of clocks. >> Perhaps we should invent a new clock API that helps with this to >> prevents code duplication!? > > What about the pm_clk_* API which was built for tracking clocks > associated with devices for runtime PM. > > IOW, you could pm_clk_add(pg->pmc->dev, pg->clks[i]) and then your > _enable_clocks() would become pm_clk_suspend() an dyour > _disable_clocks() would become pm_clk_resume(). Very interesting, I was not aware of this. > I might not be following the mapping between PMC and PGs though so not > sure pg->pmc->dev is the right struct device, but you get the idea. Yes, so this will not work here as-is, because the pmc->dev is common to all pm-domains (it is the device that creates all the pm-domains). So to make this work, I would need to create a device for each pm-domain and add the clocks to that. I see that this works very well for normal drivers, but it does not feel so natural for pm-domains where we don't have a device struct today. By the way, the rockchip pm-domains implementation is very much in the same boat as tegra, where there are multiple clocks per pm-domain and it is handled by a simple list. So I am not sure if you think that we should be turning all pm-domains registered by pm_genpd_init() into a device and then we can make use of these pm_clk_XXXX() APIs? I have implemented the generic clk APIs that Ulf and I discussed for handling multiple clocks, but if we think that this is a better way, then I will hold off for now. Cheers Jon
WARNING: multiple messages have this Message-ID (diff)
From: jonathanh@nvidia.com (Jon Hunter) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH V5 11/14] soc: tegra: pmc: Add generic PM domain support Date: Mon, 15 Feb 2016 11:27:39 +0000 [thread overview] Message-ID: <56C1B62B.5060708@nvidia.com> (raw) In-Reply-To: <7hh9hdzflv.fsf@baylibre.com> On 12/02/16 23:14, Kevin Hilman wrote: > Ulf Hansson <ulf.hansson@linaro.org> writes: > >> On 28 January 2016 at 17:33, Jon Hunter <jonathanh@nvidia.com> wrote: >>> Adds generic PM support to the PMC driver where the PM domains are >>> populated from device-tree and the PM domain consumer devices are >>> bound to their relevant PM domains via device-tree as well. >>> >>> Update the tegra_powergate_sequence_power_up() API so that internally >>> it calls the same tegra_powergate_xxx functions that are used by the >>> tegra generic power domain code for consistency. >>> >>> This is based upon work by Thierry Reding <treding@nvidia.com> >>> and Vince Hsu <vinceh@nvidia.com>. >>> >>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com> > > [...] > >>> +static void tegra_powergate_disable_clocks(struct tegra_powergate *pg) >>> +{ >>> + unsigned int i; >>> + >>> + for (i = 0; i < pg->num_clks; i++) >>> + clk_disable_unprepare(pg->clks[i]); >>> +} >>> + >>> +static int tegra_powergate_enable_clocks(struct tegra_powergate *pg) >>> +{ >>> + unsigned int i; >>> + int err; >>> + >>> + for (i = 0; i < pg->num_clks; i++) { >>> + err = clk_prepare_enable(pg->clks[i]); >>> + if (err) >>> + goto out; >>> + } >>> + >>> + return 0; >>> + >>> +out: >>> + while (i--) >>> + clk_disable_unprepare(pg->clks[i]); >>> + >>> + return err; >>> +} >> >> I have seen similar code around in other PM domains, dealing with >> enabling/disabling a *list* of clocks. >> Perhaps we should invent a new clock API that helps with this to >> prevents code duplication!? > > What about the pm_clk_* API which was built for tracking clocks > associated with devices for runtime PM. > > IOW, you could pm_clk_add(pg->pmc->dev, pg->clks[i]) and then your > _enable_clocks() would become pm_clk_suspend() an dyour > _disable_clocks() would become pm_clk_resume(). Very interesting, I was not aware of this. > I might not be following the mapping between PMC and PGs though so not > sure pg->pmc->dev is the right struct device, but you get the idea. Yes, so this will not work here as-is, because the pmc->dev is common to all pm-domains (it is the device that creates all the pm-domains). So to make this work, I would need to create a device for each pm-domain and add the clocks to that. I see that this works very well for normal drivers, but it does not feel so natural for pm-domains where we don't have a device struct today. By the way, the rockchip pm-domains implementation is very much in the same boat as tegra, where there are multiple clocks per pm-domain and it is handled by a simple list. So I am not sure if you think that we should be turning all pm-domains registered by pm_genpd_init() into a device and then we can make use of these pm_clk_XXXX() APIs? I have implemented the generic clk APIs that Ulf and I discussed for handling multiple clocks, but if we think that this is a better way, then I will hold off for now. Cheers Jon
next prev parent reply other threads:[~2016-02-15 11:27 UTC|newest] Thread overview: 98+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-01-28 16:33 [PATCH V5 00/14] Add generic PM domain support for Tegra Jon Hunter 2016-01-28 16:33 ` Jon Hunter 2016-01-28 16:33 ` [PATCH V5 02/14] soc: tegra: pmc: Protect public functions from potential race conditions Jon Hunter 2016-01-28 16:33 ` Jon Hunter 2016-01-29 16:20 ` Mathieu Poirier 2016-01-29 16:20 ` Mathieu Poirier 2016-02-01 13:42 ` Jon Hunter 2016-02-01 13:42 ` Jon Hunter 2016-01-28 16:33 ` [PATCH V5 03/14] soc: tegra: pmc: Change powergate and rail IDs to be an unsigned type Jon Hunter 2016-01-28 16:33 ` Jon Hunter 2016-01-28 16:33 ` [PATCH V5 04/14] soc: tegra: pmc: Fix testing of powergate state Jon Hunter 2016-01-28 16:33 ` Jon Hunter 2016-01-28 16:33 ` [PATCH V5 07/14] soc: tegra: pmc: Ensure partitions can be toggled on/off by PMC Jon Hunter 2016-01-28 16:33 ` Jon Hunter 2016-01-28 16:33 ` [PATCH V5 08/14] PM / Domains: Add function to remove a pm-domain Jon Hunter 2016-01-28 16:33 ` Jon Hunter [not found] ` <1453998832-27383-9-git-send-email-jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2016-02-02 15:35 ` Ulf Hansson 2016-02-02 15:35 ` Ulf Hansson [not found] ` <CAPDyKFqJLdoee4a9819XukXTmYyd3pue452K_zbiV6XhfA=fTw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2016-02-03 10:51 ` Jon Hunter 2016-02-03 10:51 ` Jon Hunter [not found] ` <1453998832-27383-1-git-send-email-jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2016-01-28 16:33 ` [PATCH V5 01/14] soc: tegra: pmc: Restore base address on probe failure Jon Hunter 2016-01-28 16:33 ` Jon Hunter 2016-01-28 16:33 ` [PATCH V5 05/14] soc: tegra: pmc: Wait for powergate state to change Jon Hunter 2016-01-28 16:33 ` Jon Hunter 2016-01-29 16:58 ` Mathieu Poirier 2016-01-29 16:58 ` Mathieu Poirier [not found] ` <CANLsYkycbEo+wyMX8RJ9H-S5kDTjQR4nnDZc5gvf2kShOZAv9w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2016-02-01 13:44 ` Jon Hunter 2016-02-01 13:44 ` Jon Hunter [not found] ` <56AF613A.1000909-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2016-02-03 9:20 ` Jon Hunter 2016-02-03 9:20 ` Jon Hunter [not found] ` <56B1C647.4060504-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2016-02-03 15:58 ` Mathieu Poirier 2016-02-03 15:58 ` Mathieu Poirier 2016-01-28 16:33 ` [PATCH V5 06/14] soc: tegra: pmc: Fix checking of valid partitions Jon Hunter 2016-01-28 16:33 ` Jon Hunter [not found] ` <1453998832-27383-7-git-send-email-jonathanh-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2016-01-29 17:08 ` Mathieu Poirier 2016-01-29 17:08 ` Mathieu Poirier [not found] ` <CANLsYkxY5P2wQxGev0veN39nD-1cTVkZCVpX9jca7da39JJpWg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2016-02-01 13:45 ` Jon Hunter 2016-02-01 13:45 ` Jon Hunter 2016-01-28 16:33 ` [PATCH V5 09/14] Documentation: DT: bindings: Update NVIDIA PMC for Tegra Jon Hunter 2016-01-28 16:33 ` Jon Hunter 2016-01-29 16:08 ` Rob Herring 2016-01-29 16:08 ` Rob Herring 2016-01-28 16:33 ` [PATCH V5 10/14] Documentation: DT: bindings: Add power domain info for NVIDIA PMC Jon Hunter 2016-01-28 16:33 ` Jon Hunter 2016-01-29 16:06 ` Rob Herring 2016-01-29 16:06 ` Rob Herring 2016-02-03 11:02 ` Jon Hunter 2016-02-03 11:02 ` Jon Hunter [not found] ` <56B1DE40.7080403-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2016-02-03 15:48 ` Rob Herring 2016-02-03 15:48 ` Rob Herring [not found] ` <CAL_JsqLcoKW2znNNvM=sYLmZ6O6ZWqn7+aXspkXoONw6-O1ygg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2016-02-10 10:57 ` Jon Hunter 2016-02-10 10:57 ` Jon Hunter [not found] ` <56BB1787.4050801-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2016-02-10 14:06 ` Rob Herring 2016-02-10 14:06 ` Rob Herring 2016-01-28 16:33 ` [PATCH V5 11/14] soc: tegra: pmc: Add generic PM domain support Jon Hunter 2016-01-28 16:33 ` Jon Hunter 2016-02-04 15:44 ` Ulf Hansson 2016-02-04 15:44 ` Ulf Hansson 2016-02-10 18:01 ` Jon Hunter 2016-02-10 18:01 ` Jon Hunter [not found] ` <56BB7AF4.8040708-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2016-02-10 18:25 ` Ulf Hansson 2016-02-10 18:25 ` Ulf Hansson [not found] ` <CAPDyKFrZ6tWBsQC0tyWWeChiZja3h_zcbaiX25ak-Zyp4MzqVw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2016-02-11 9:13 ` Jon Hunter 2016-02-11 9:13 ` Jon Hunter 2016-02-11 9:57 ` Ulf Hansson 2016-02-11 9:57 ` Ulf Hansson [not found] ` <CAPDyKFrdmufsMqNL0U7q5gPEUqsg3SrkrNChcziQjEOjvd30Ng-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2016-02-11 10:13 ` Jon Hunter 2016-02-11 10:13 ` Jon Hunter [not found] ` <56BC5EE0.2040804-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2016-02-11 10:26 ` Jon Hunter 2016-02-11 10:26 ` Jon Hunter 2016-02-11 10:37 ` Ulf Hansson 2016-02-11 10:37 ` Ulf Hansson 2016-02-11 10:52 ` Jon Hunter 2016-02-11 10:52 ` Jon Hunter 2016-02-11 10:28 ` Ulf Hansson 2016-02-11 10:28 ` Ulf Hansson [not found] ` <CAPDyKFq_0t4tcvkgMBW8p8ubJDALWMjdhgGM+_Z6auRxEkSPdA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2016-02-11 16:38 ` Jon Hunter 2016-02-11 16:38 ` Jon Hunter [not found] ` <56BCB90C.8000302-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2016-02-18 15:06 ` Ulf Hansson 2016-02-18 15:06 ` Ulf Hansson 2016-02-12 23:14 ` Kevin Hilman 2016-02-12 23:14 ` Kevin Hilman [not found] ` <7hh9hdzflv.fsf-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> 2016-02-15 11:27 ` Jon Hunter [this message] 2016-02-15 11:27 ` Jon Hunter [not found] ` <56C1B62B.5060708-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 2016-02-18 16:00 ` Ulf Hansson 2016-02-18 16:00 ` Ulf Hansson [not found] ` <CAPDyKFoPrFoMOFxC37zXX4L3VdLKknaw_LUTw7ycr9mfa_=7_A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2016-02-18 16:31 ` Jon Hunter 2016-02-18 16:31 ` Jon Hunter 2016-02-24 0:03 ` Kevin Hilman 2016-02-24 0:03 ` Kevin Hilman 2016-01-28 16:33 ` [PATCH V5 12/14] clk: tegra210: Add the APB2APE audio clock Jon Hunter 2016-01-28 16:33 ` Jon Hunter 2016-02-02 14:37 ` Thierry Reding 2016-02-02 14:37 ` Thierry Reding 2016-01-28 16:33 ` [PATCH V5 13/14] ARM64: tegra: Add audio PM domain device node for Tegra210 Jon Hunter 2016-01-28 16:33 ` Jon Hunter 2016-01-28 16:33 ` [PATCH V5 14/14] ARM64: tegra: select PM_GENERIC_DOMAINS Jon Hunter 2016-01-28 16:33 ` Jon Hunter
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=56C1B62B.5060708@nvidia.com \ --to=jonathanh-ddmlm1+adcrqt0dzr+alfa@public.gmane.org \ --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org \ --cc=gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \ --cc=ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org \ --cc=khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org \ --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \ --cc=linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=mark.rutland-5wv7dgnIgG8@public.gmane.org \ --cc=pawel.moll-5wv7dgnIgG8@public.gmane.org \ --cc=rjw-LthD3rsA81gm4RdzfppkhA@public.gmane.org \ --cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \ --cc=swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org \ --cc=thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \ --cc=ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@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.