[PATCH v14 2/3] power-domain: rockchip: add power domain driver
Caesar Wang
sasukewxt at 163.com
Sat Jun 13 20:15:34 PDT 2015
Ulf,
Thanks for your comments.:-)
在 2015年05月28日 18:38, Ulf Hansson 写道:
> On 24 April 2015 at 10:07, Caesar Wang <wxt at rock-chips.com> wrote:
>> In order to meet high performance and low power requirements, a power
>> management unit is designed or saving power when RK3288 in low power
>> mode.
>> The RK3288 PMU is dedicated for managing the power ot the whole chip.
>>
>> Signed-off-by: jinkun.hong <jinkun.hong at rock-chips.com>
>> Signed-off-by: Caesar Wang <wxt at rock-chips.com>
>> ---
>>
>> arch/arm/mach-rockchip/Kconfig | 1 +
>> arch/arm/mach-rockchip/Makefile | 1 +
>> arch/arm/mach-rockchip/pm_domains.c | 506 ++++++++++++++++++++++++++++++++++++
>> 3 files changed, 508 insertions(+)
>> create mode 100644 arch/arm/mach-rockchip/pm_domains.c
>>
>> diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
>> index ae4eb7c..578206b 100644
>> --- a/arch/arm/mach-rockchip/Kconfig
>> +++ b/arch/arm/mach-rockchip/Kconfig
>> @@ -15,6 +15,7 @@ config ARCH_ROCKCHIP
>> select ROCKCHIP_TIMER
>> select ARM_GLOBAL_TIMER
>> select CLKSRC_ARM_GLOBAL_TIMER_SCHED_CLOCK
>> + select PM_GENERIC_DOMAINS if PM
>> help
>> Support for Rockchip's Cortex-A9 Single-to-Quad-Core-SoCs
>> containing the RK2928, RK30xx and RK31xx series.
>> diff --git a/arch/arm/mach-rockchip/Makefile b/arch/arm/mach-rockchip/Makefile
>> index 5c3a9b2..9c902d3 100644
>> --- a/arch/arm/mach-rockchip/Makefile
>> +++ b/arch/arm/mach-rockchip/Makefile
>> @@ -1,5 +1,6 @@
>> CFLAGS_platsmp.o := -march=armv7-a
>>
>> obj-$(CONFIG_ARCH_ROCKCHIP) += rockchip.o
>> +obj-$(CONFIG_PM_GENERIC_DOMAINS) += pm_domains.o
>> obj-$(CONFIG_PM_SLEEP) += pm.o sleep.o
>> obj-$(CONFIG_SMP) += headsmp.o platsmp.o
>> diff --git a/arch/arm/mach-rockchip/pm_domains.c b/arch/arm/mach-rockchip/pm_domains.c
>> new file mode 100644
>> index 0000000..92d2569
>> --- /dev/null
>> +++ b/arch/arm/mach-rockchip/pm_domains.c
>> @@ -0,0 +1,506 @@
>> +/*
>> + * Rockchip Generic power domain support.
>> + *
>> + * Copyright (c) 2014 ROCKCHIP, Co. Ltd.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/io.h>
>> +#include <linux/err.h>
>> +#include <linux/pm_clock.h>
>> +#include <linux/pm_domain.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/clk.h>
>> +#include <linux/clk-provider.h>
> clk-provider.h, why?
The following is needed.
_clk_get_name(clk)
>
>> +#include <linux/regmap.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <dt-bindings/power-domain/rk3288.h>
> Same comment as in patch1. I don't find this header.
OK.
Perhaps, Making a mistake. the missing the patch in
here.(https://patchwork.kernel.org/patch/6266881/)
>> +
>> +struct rockchip_domain_info {
>> + int pwr_mask;
>> + int status_mask;
>> + int req_mask;
>> + int idle_mask;
>> + int ack_mask;
>> +};
>> +
>> +struct rockchip_pmu_info {
>> + u32 pwr_offset;
>> + u32 status_offset;
>> + u32 req_offset;
>> + u32 idle_offset;
>> + u32 ack_offset;
>> +
>> + u32 core_pwrcnt_offset;
>> + u32 gpu_pwrcnt_offset;
>> +
>> + unsigned int core_power_transition_time;
>> + unsigned int gpu_power_transition_time;
>> +
>> + int num_domains;
>> + const struct rockchip_domain_info *domain_info;
>> +};
>> +
>> +struct rockchip_pm_domain {
>> + struct generic_pm_domain genpd;
>> + const struct rockchip_domain_info *info;
>> + struct rockchip_pmu *pmu;
>> + int num_clks;
>> + struct clk *clks[];
>> +};
>> +
>> +struct rockchip_pmu {
>> + struct device *dev;
>> + struct regmap *regmap;
>> + const struct rockchip_pmu_info *info;
>> + struct mutex mutex; /* mutex lock for pmu */
>> + struct genpd_onecell_data genpd_data;
>> + struct generic_pm_domain *domains[];
>> +};
>> +
>> +#define to_rockchip_pd(gpd) container_of(gpd, struct rockchip_pm_domain, genpd)
>> +
>> +#define DOMAIN(pwr, status, req, idle, ack) \
>> +{ \
>> + .pwr_mask = BIT(pwr), \
>> + .status_mask = BIT(status), \
>> + .req_mask = BIT(req), \
>> + .idle_mask = BIT(idle), \
>> + .ack_mask = BIT(ack), \
>> +}
>> +
>> +#define DOMAIN_RK3288(pwr, status, req) \
>> + DOMAIN(pwr, status, req, req, (req) + 16)
>> +
>> +static bool rockchip_pmu_domain_is_idle(struct rockchip_pm_domain *pd)
>> +{
>> + struct rockchip_pmu *pmu = pd->pmu;
>> + const struct rockchip_domain_info *pd_info = pd->info;
>> + unsigned int val;
>> +
>> + regmap_read(pmu->regmap, pmu->info->idle_offset, &val);
>> + return (val & pd_info->idle_mask) == pd_info->idle_mask;
>> +}
>> +
>> +static int rockchip_pmu_set_idle_request(struct rockchip_pm_domain *pd,
>> + bool idle)
>> +{
>> + const struct rockchip_domain_info *pd_info = pd->info;
>> + struct rockchip_pmu *pmu = pd->pmu;
>> + unsigned int val;
>> +
>> + regmap_update_bits(pmu->regmap, pmu->info->req_offset,
>> + pd_info->req_mask, idle ? -1U : 0);
>> +
>> + dsb();
>> +
>> + do {
>> + regmap_read(pmu->regmap, pmu->info->ack_offset, &val);
>> + } while ((val & pd_info->ack_mask) != (idle ? pd_info->ack_mask : 0));
>> +
>> + while (rockchip_pmu_domain_is_idle(pd) != idle)
>> + cpu_relax();
>> +
>> + return 0;
>> +}
>> +
>> +static bool rockchip_pmu_domain_is_on(struct rockchip_pm_domain *pd)
>> +{
>> + struct rockchip_pmu *pmu = pd->pmu;
>> + unsigned int val;
>> +
>> + regmap_read(pmu->regmap, pmu->info->status_offset, &val);
>> +
>> + /* 1'b0: power on, 1'b1: power off */
>> + return !(val & pd->info->status_mask);
>> +}
>> +
>> +static void rockchip_do_pmu_set_power_domain(struct rockchip_pm_domain *pd,
>> + bool on)
>> +{
>> + struct rockchip_pmu *pmu = pd->pmu;
>> +
>> + regmap_update_bits(pmu->regmap, pmu->info->pwr_offset,
>> + pd->info->pwr_mask, on ? 0 : -1U);
>> +
>> + dsb();
>> +
>> + while (rockchip_pmu_domain_is_on(pd) != on)
>> + cpu_relax();
>> +}
>> +
>> +static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
>> +{
>> + int i;
>> +
>> + mutex_lock(&pd->pmu->mutex);
> I don't quite understand why you need a lock, what are you protecting and why?
Says:
[ 3551.678762] mali ffa30000.gpu: starting device in power domain 'pd_gpu'
[ 3551.899180] mali ffa30000.gpu: stopping device in power domain 'pd_gpu'
[ 3551.905958] mali ffa30000.gpu: starting device in power domain 'pd_gpu'
[ 3551.912832] mali ffa30000.gpu: stopping device in power domain 'pd_gpu'
[ 3551.919730] rockchip_pd_power:139: mutex_lock
[ 3551.924130] rockchip_pd_power:167,mutex_unlock
[ 3563.827295] rockchip_pd_power:139: mutex_lock
[ 3563.831753] rockchip_pd_power:167,mutex_unlock
[ 3563.836216] mali ffa30000.gpu: starting device in power domain 'pd_gpu'
[ 3564.058989] mali ffa30000.gpu: stopping device in power domain 'pd_gpu'
[ 3564.065659] mali ffa30000.gpu: starting device in power domain 'pd_gpu'
[ 3564.072354] mali ffa30000.gpu: stopping device in power domain 'pd_gpu'
[ 3564.079047] rockchip_pd_power:139: mutex_lock
[ 3564.083426] rockchip_pd_power:167,mutex_unlock
[ 3611.665726] rockchip_pd_power:139: mutex_lock
[ 3611.670206] rockchip_pd_power:167,mutex_unlock
[ 3611.674692] mali ffa30000.gpu: starting device in power domain 'pd_gpu'
[ 3611.899160] mali ffa30000.gpu: stopping device in power domain 'pd_gpu'
[ 3611.905938] mali ffa30000.gpu: starting device in power domain 'pd_gpu'
[ 3611.912818] mali ffa30000.gpu: stopping device in power domain 'pd_gpu'
[ 3611.919689] rockchip_pd_power:139: mutex_lock
[ 3611.924090] rockchip_pd_power:167,mutex_unlock
[ 3623.848296] rockchip_pd_power:139: mutex_lock
[ 3623.852752] rockchip_pd_power:167,mutex_unlock
>> +
>> + if (rockchip_pmu_domain_is_on(pd) != power_on) {
>> + for (i = 0; i < pd->num_clks; i++)
>> + clk_enable(pd->clks[i]);
>> +
>> + if (!power_on) {
>> + /* FIXME: add code to save AXI_QOS */
>> +
>> + /* if powering down, idle request to NIU first */
>> + rockchip_pmu_set_idle_request(pd, true);
>> + }
>> +
>> + rockchip_do_pmu_set_power_domain(pd, power_on);
>> +
>> + if (power_on) {
>> + /* if powering up, leave idle mode */
>> + rockchip_pmu_set_idle_request(pd, false);
>> +
>> + /* FIXME: add code to restore AXI_QOS */
>> + }
>> +
>> + for (i = pd->num_clks - 1; i >= 0; i--)
>> + clk_disable(pd->clks[i]);
>> + }
>> +
>> + mutex_unlock(&pd->pmu->mutex);
>> + return 0;
>> +}
>> +
>> +static int rockchip_pd_power_on(struct generic_pm_domain *domain)
>> +{
>> + struct rockchip_pm_domain *pd = to_rockchip_pd(domain);
>> +
>> + return rockchip_pd_power(pd, true);
>> +}
>> +
>> +static int rockchip_pd_power_off(struct generic_pm_domain *domain)
>> +{
>> + struct rockchip_pm_domain *pd = to_rockchip_pd(domain);
>> +
>> + return rockchip_pd_power(pd, false);
>> +}
>> +
>> +static int rockchip_pd_attach_dev(struct generic_pm_domain *genpd,
>> + struct device *dev)
>> +{
>> + struct clk *clk;
>> + int i;
>> + int error;
>> +
>> + dev_dbg(dev, "attaching to power domain '%s'\n", genpd->name);
>> +
>> + error = pm_clk_create(dev);
>> + if (error) {
>> + dev_err(dev, "pm_clk_create failed %d\n", error);
>> + return error;
>> + }
>> +
>> + i = 0;
>> + while ((clk = of_clk_get(dev->of_node, i++)) && !IS_ERR(clk)) {
> This loop adds all available device clocks to the PM clock list. I
> wonder if this could be considered as a common thing and if so, we
> might want to extend the pm_clk API with this.
There are several reasons as follows:
Firstly, the clocks need be turned off to save power when
the system enter the suspend state. So we need to enumerate the clocks
in the dts. In order to power domain can turn on and off.
Secondly, the reset-circuit should reset be synchronous on rk3288,
then sync revoked.
So we need to enable clocks of all devices.
>> + dev_dbg(dev, "adding clock '%s' to list of PM clocks\n",
>> + __clk_get_name(clk));
>> + error = pm_clk_add_clk(dev, clk);
>> + clk_put(clk);
>> + if (error) {
>> + dev_err(dev, "pm_clk_add_clk failed %d\n", error);
>> + pm_clk_destroy(dev);
>> + return error;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void rockchip_pd_detach_dev(struct generic_pm_domain *genpd,
>> + struct device *dev)
>> +{
>> + dev_dbg(dev, "detaching from power domain '%s'\n", genpd->name);
>> +
>> + pm_clk_destroy(dev);
>> +}
>> +
>> +static int rockchip_pd_start_dev(struct device *dev)
>> +{
>> + struct generic_pm_domain *genpd = pd_to_genpd(dev->pm_domain);
>> +
>> + dev_dbg(dev, "starting device in power domain '%s'\n", genpd->name);
>> +
>> + return pm_clk_resume(dev);
>> +}
>> +
>> +static int rockchip_pd_stop_dev(struct device *dev)
>> +{
>> + struct generic_pm_domain *genpd = pd_to_genpd(dev->pm_domain);
>> +
>> + dev_dbg(dev, "stopping device in power domain '%s'\n", genpd->name);
>> +
>> + return pm_clk_suspend(dev);
>> +}
>> +
>> +static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
>> + struct device_node *node)
>> +{
>> + const struct rockchip_domain_info *pd_info;
>> + struct rockchip_pm_domain *pd;
>> + struct clk *clk;
>> + int clk_cnt;
>> + int i;
>> + u32 id;
>> + int error;
>> +
>> + error = of_property_read_u32(node, "reg", &id);
>> + if (error) {
>> + dev_err(pmu->dev,
>> + "%s: failed to retrieve domain id (reg): %d\n",
>> + node->name, error);
>> + return -EINVAL;
>> + }
>> +
>> + if (id >= pmu->info->num_domains) {
>> + dev_err(pmu->dev, "%s: invalid domain id %d\n",
>> + node->name, id);
>> + return -EINVAL;
>> + }
>> +
>> + pd_info = &pmu->info->domain_info[id];
>> + if (!pd_info) {
>> + dev_err(pmu->dev, "%s: undefined domain id %d\n",
>> + node->name, id);
>> + return -EINVAL;
>> + }
>> +
>> + clk_cnt = of_count_phandle_with_args(node, "clocks", "#clock-cells");
>> + pd = devm_kzalloc(pmu->dev,
>> + sizeof(*pd) + clk_cnt * sizeof(pd->clks[0]),
>> + GFP_KERNEL);
>> + if (!pd)
>> + return -ENOMEM;
>> +
>> + pd->info = pd_info;
>> + pd->pmu = pmu;
>> +
>> + for (i = 0; i < clk_cnt; i++) {
> This loop is similar to the one when creates the PM clock list in the
> rockchip_pd_attach_dev().
>
> What's the reason you don't want to use pm clk for these clocks?
>
Ditto.
>> + clk = of_clk_get(node, i);
>> + if (IS_ERR(clk)) {
>> + error = PTR_ERR(clk);
>> + dev_err(pmu->dev,
>> + "%s: failed to get clk %s (index %d): %d\n",
>> + node->name, __clk_get_name(clk), i, error);
>> + goto err_out;
>> + }
>> +
>> + error = clk_prepare(clk);
>> + if (error) {
>> + dev_err(pmu->dev,
>> + "%s: failed to prepare clk %s (index %d): %d\n",
>> + node->name, __clk_get_name(clk), i, error);
>> + clk_put(clk);
>> + goto err_out;
>> + }
>> +
>> + pd->clks[pd->num_clks++] = clk;
>> +
>> + dev_dbg(pmu->dev, "added clock '%s' to domain '%s'\n",
>> + __clk_get_name(clk), node->name);
>> + }
>> +
>> + error = rockchip_pd_power(pd, true);
>> + if (error) {
>> + dev_err(pmu->dev,
>> + "failed to power on domain '%s': %d\n",
>> + node->name, error);
>> + goto err_out;
>> + }
>> +
>> + pd->genpd.name = node->name;
>> + pd->genpd.power_off = rockchip_pd_power_off;
>> + pd->genpd.power_on = rockchip_pd_power_on;
>> + pd->genpd.attach_dev = rockchip_pd_attach_dev;
>> + pd->genpd.detach_dev = rockchip_pd_detach_dev;
>> + pd->genpd.dev_ops.start = rockchip_pd_start_dev;
>> + pd->genpd.dev_ops.stop = rockchip_pd_stop_dev;
> Instead of assigning the ->stop|start() callbacks, which do
> pm_clk_suspend|resume(), just set the genpd->flags to
> GENPD_FLAG_PM_CLK, and genpd will fix this for you.
>
Ditto.
>> + pm_genpd_init(&pd->genpd, NULL, false);
>> +
>> + pmu->genpd_data.domains[id] = &pd->genpd;
>> + return 0;
>> +
>> +err_out:
>> + while (--i >= 0) {
>> + clk_unprepare(pd->clks[i]);
>> + clk_put(pd->clks[i]);
>> + }
>> + return error;
>> +}
>> +
>> +static void rockchip_pm_remove_one_domain(struct rockchip_pm_domain *pd)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < pd->num_clks; i++) {
>> + clk_unprepare(pd->clks[i]);
> If you converted these clocks to be dealt with via the PM clk API,
> pm_clk_destoy() would have replaced this loop.
>
>> + clk_put(pd->clks[i]);
>> + }
>> +
>> + /* devm will free our memory */
>> +}
>> +
>> +static void rockchip_pm_domain_cleanup(struct rockchip_pmu *pmu)
>> +{
>> + struct generic_pm_domain *genpd;
>> + struct rockchip_pm_domain *pd;
>> + int i;
>> +
>> + for (i = 0; i < pmu->genpd_data.num_domains; i++) {
>> + genpd = pmu->genpd_data.domains[i];
>> + if (genpd) {
>> + pd = to_rockchip_pd(genpd);
>> + rockchip_pm_remove_one_domain(pd);
>> + }
>> + }
>> +
>> + /* devm will free our memory */
>> +}
>> +
>> +static void rockchip_configure_pd_cnt(struct rockchip_pmu *pmu,
>> + u32 domain_reg_offset,
>> + unsigned int count)
>> +{
>> + /* First configure domain power down transition count ... */
>> + regmap_write(pmu->regmap, domain_reg_offset, count);
>> + /* ... and then power up count. */
>> + regmap_write(pmu->regmap, domain_reg_offset + 4, count);
>> +}
>> +
>> +static int rockchip_pm_domain_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct device_node *np = dev->of_node;
>> + struct device_node *node;
>> + struct rockchip_pmu *pmu;
>> + const struct of_device_id *match;
>> + const struct rockchip_pmu_info *pmu_info;
>> + int error;
>> +
>> + if (!np) {
>> + dev_err(dev, "device tree node not found\n");
>> + return -ENXIO;/
> Nitpick:
> It's more common to return -ENODEV here, you might want to change.
OK.
Thanks,
Caesar
>> + }
>> +
>> + match = of_match_device(dev->driver->of_match_table, dev);
>> + if (!match || !match->data) {
>> + dev_err(dev, "missing pmu data\n");
>> + return -EINVAL;
>> + }
>> +
>> + pmu_info = match->data;
>> +
>> + pmu = devm_kzalloc(dev,
>> + sizeof(*pmu) +
>> + pmu_info->num_domains * sizeof(pmu->domains[0]),
>> + GFP_KERNEL);
>> + if (!pmu)
>> + return -ENOMEM;
>> +
>> + pmu->dev = &pdev->dev;
>> + mutex_init(&pmu->mutex);
>> +
>> + pmu->info = pmu_info;
>> +
>> + pmu->genpd_data.domains = pmu->domains;
>> + pmu->genpd_data.num_domains = pmu_info->num_domains;
>> +
>> + node = of_parse_phandle(np, "rockchip,pmu", 0);
>> + pmu->regmap = syscon_node_to_regmap(node);
>> + of_node_put(node);
>> + if (IS_ERR(pmu->regmap)) {
>> + error = PTR_ERR(pmu->regmap);
>> + dev_err(dev, "failed to get PMU regmap: %d\n", error);
>> + return error;
>> + }
>> +
>> + /*
>> + * Configure power up and down transition delays for core
>> + * and GPU domains.
>> + */
>> + rockchip_configure_pd_cnt(pmu, pmu_info->core_pwrcnt_offset,
>> + pmu_info->core_power_transition_time);
>> + rockchip_configure_pd_cnt(pmu, pmu_info->gpu_pwrcnt_offset,
>> + pmu_info->gpu_power_transition_time);
>> +
>> + error = -ENXIO;
>> +
>> + for_each_available_child_of_node(np, node) {
>> + error = rockchip_pm_add_one_domain(pmu, node);
>> + if (error) {
>> + dev_err(dev, "failed to handle node %s: %d\n",
>> + node->name, error);
>> + goto err_out;
>> + }
>> + }
>> +
>> + if (error) {
>> + dev_dbg(dev, "no power domains defined\n");
>> + goto err_out;
>> + }
>> +
>> + of_genpd_add_provider_onecell(np, &pmu->genpd_data);
>> +
>> + return 0;
>> +
>> +err_out:
>> + rockchip_pm_domain_cleanup(pmu);
>> + return error;
>> +}
>> +
>> +static const struct rockchip_domain_info rk3288_pm_domains[] = {
>> + [RK3288_PD_GPU] = DOMAIN_RK3288(9, 9, 2),
>> + [RK3288_PD_VIO] = DOMAIN_RK3288(7, 7, 4),
>> + [RK3288_PD_VIDEO] = DOMAIN_RK3288(8, 8, 3),
>> + [RK3288_PD_HEVC] = DOMAIN_RK3288(14, 10, 9),
>> +};
>> +
>> +static const struct rockchip_pmu_info rk3288_pmu = {
>> + .pwr_offset = 0x08,
>> + .status_offset = 0x0c,
>> + .req_offset = 0x10,
>> + .idle_offset = 0x14,
>> + .ack_offset = 0x14,
>> +
>> + .core_pwrcnt_offset = 0x34,
>> + .gpu_pwrcnt_offset = 0x3c,
>> +
>> + .core_power_transition_time = 24, /* 1us */
>> + .gpu_power_transition_time = 24, /* 1us */
>> +
>> + .num_domains = ARRAY_SIZE(rk3288_pm_domains),
>> + .domain_info = rk3288_pm_domains,
>> +};
>> +
>> +static const struct of_device_id rockchip_pm_domain_dt_match[] = {
>> + {
>> + .compatible = "rockchip,rk3288-power-controller",
>> + .data = (void *)&rk3288_pmu,
>> + },
>> + { /* sentinel */ },
>> +};
>> +
>> +static struct platform_driver rockchip_pm_domain_driver = {
>> + .probe = rockchip_pm_domain_probe,
>> + .driver = {
>> + .name = "rockchip-pm-domain",
>> + .of_match_table = rockchip_pm_domain_dt_match,
>> + /*
>> + * We can't forcibly eject devices form power domain,
>> + * so we can't really remove power domains once they
>> + * were added.
>> + */
>> + .suppress_bind_attrs = true,
>> + },
>> +};
>> +
>> +static int __init rockchip_pm_domain_drv_register(void)
>> +{
>> + return platform_driver_register(&rockchip_pm_domain_driver);
>> +}
>> +postcore_initcall(rockchip_pm_domain_drv_register);
>> --
>> 1.9.1
>>
> Kind regards
> Uffe
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
More information about the Linux-rockchip
mailing list