[PATCH v3 6/7] PM / devfreq: rockchip: add devfreq driver for rk3399 dmc

[Lin Huang]

base on dfi result, we do ddr frequency scaling, register
dmc driver to devfreq framework, and use simple-ondemand
policy.

Signed-off-by: Lin Huang <h...@rock-chips.com>
---
Changes in v3:
- operate dram setting through sip call
- imporve set rate flow

Changes in v2:
- None

Changes in v1:
- move dfi controller to event
- fix set voltage sequence when set rate fail
- change Kconfig type from tristate to bool
- move unuse EXPORT_SYMBOL_GPL()

drivers/devfreq/Kconfig | 1 +
drivers/devfreq/Makefile | 1 +
drivers/devfreq/rockchip/Kconfig | 15 +
drivers/devfreq/rockchip/Makefile | 2 +
drivers/devfreq/rockchip/rk3399_dmc.c | 482 ++++++++++++++++++++++++++++++++
drivers/devfreq/rockchip/rockchip_dmc.c | 143 ++++++++++
include/soc/rockchip/rockchip_dmc.h | 45 +++
7 files changed, 689 insertions(+)
create mode 100644 drivers/devfreq/rockchip/Kconfig
create mode 100644 drivers/devfreq/rockchip/Makefile
create mode 100644 drivers/devfreq/rockchip/rk3399_dmc.c
create mode 100644 drivers/devfreq/rockchip/rockchip_dmc.c
create mode 100644 include/soc/rockchip/rockchip_dmc.h

diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
index a5be56e..cb67246 100644
--- a/drivers/devfreq/Kconfig
+++ b/drivers/devfreq/Kconfig
@@ -101,5 +101,6 @@ config ARM_TEGRA_DEVFREQ
operating frequencies and voltages with OPP support.

source "drivers/devfreq/event/Kconfig"
+source "drivers/devfreq/rockchip/Kconfig"

endif # PM_DEVFREQ
diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
index 09f11d9..48e2ae6 100644
--- a/drivers/devfreq/Makefile
+++ b/drivers/devfreq/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_DEVFREQ_GOV_PASSIVE) += governor_passive.o
# DEVFREQ Drivers
obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ) += exynos-bus.o
obj-$(CONFIG_ARM_TEGRA_DEVFREQ) += tegra-devfreq.o
+obj-$(CONFIG_ARCH_ROCKCHIP) += rockchip/

# DEVFREQ Event Drivers
obj-$(CONFIG_PM_DEVFREQ_EVENT) += event/
diff --git a/drivers/devfreq/rockchip/Kconfig b/drivers/devfreq/rockchip/Kconfig
new file mode 100644
index 0000000..7fb1cff
--- /dev/null
+++ b/drivers/devfreq/rockchip/Kconfig
@@ -0,0 +1,15 @@
+config ARM_ROCKCHIP_DMC_DEVFREQ
+ bool "ARM ROCKCHIP DMC DEVFREQ Driver"
+ depends on ARCH_ROCKCHIP
+ help
+ This adds the DEVFREQ driver framework for the rockchip dmc.
+
+config ARM_RK3399_DMC_DEVFREQ
+ bool "ARM RK3399 DMC DEVFREQ Driver"
+ depends on ARM_ROCKCHIP_DMC_DEVFREQ
+ select PM_OPP
+ select DEVFREQ_GOV_SIMPLE_ONDEMAND
+ help
+ This adds the DEVFREQ driver for the RK3399 dmc. It sets the frequency
+ for the memory controller and reads the usage counts from hardware.
+
diff --git a/drivers/devfreq/rockchip/Makefile b/drivers/devfreq/rockchip/Makefile
new file mode 100644
index 0000000..caca525
--- /dev/null
+++ b/drivers/devfreq/rockchip/Makefile
@@ -0,0 +1,2 @@
+obj-$(CONFIG_ARM_ROCKCHIP_DMC_DEVFREQ) += rockchip_dmc.o
+obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ) += rk3399_dmc.o
diff --git a/drivers/devfreq/rockchip/rk3399_dmc.c b/drivers/devfreq/rockchip/rk3399_dmc.c
new file mode 100644
index 0000000..f1d6120
--- /dev/null
+++ b/drivers/devfreq/rockchip/rk3399_dmc.c
@@ -0,0 +1,482 @@
+/*
+ * Copyright (c) 2016, Fuzhou Rockchip Electronics Co., Ltd
+ * Author: Lin Huang <h...@rock-chips.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ */
+
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/devfreq.h>
+#include <linux/devfreq-event.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_opp.h>
+#include <linux/regulator/consumer.h>
+#include <linux/rwsem.h>
+#include <linux/suspend.h>
+#include <linux/syscore_ops.h>
+
+#include <soc/rockchip/rockchip_dmc.h>
+#include "../../firmware/rockchip_sip.h"
+
+struct dram_timing {
+ unsigned int ddr3_speed_bin;
+ unsigned int pd_idle;
+ unsigned int sr_idle;
+ unsigned int sr_mc_gate_idle;
+ unsigned int srpd_lite_idle;
+ unsigned int standby_idle;
+ unsigned int dram_dll_dis_freq;
+ unsigned int phy_dll_dis_freq;
+ unsigned int ddr3_odt_dis_freq;
+ unsigned int ddr3_drv;
+ unsigned int ddr3_odt;
+ unsigned int phy_ddr3_ca_drv;
+ unsigned int phy_ddr3_dq_drv;
+ unsigned int phy_ddr3_odt;
+ unsigned int lpddr3_odt_dis_freq;
+ unsigned int lpddr3_drv;
+ unsigned int lpddr3_odt;
+ unsigned int phy_lpddr3_ca_drv;
+ unsigned int phy_lpddr3_dq_drv;
+ unsigned int phy_lpddr3_odt;
+ unsigned int lpddr4_odt_dis_freq;
+ unsigned int lpddr4_drv;
+ unsigned int lpddr4_dq_odt;
+ unsigned int lpddr4_ca_odt;
+ unsigned int phy_lpddr4_ca_drv;
+ unsigned int phy_lpddr4_ck_cs_drv;
+ unsigned int phy_lpddr4_dq_drv;
+ unsigned int phy_lpddr4_odt;
+};
+
+struct rk3399_dmcfreq {
+ struct device *dev;
+ struct devfreq *devfreq;
+ struct devfreq_simple_ondemand_data ondemand_data;
+ struct clk *dmc_clk;
+ struct devfreq_event_dev *edev;
+ struct mutex lock;
+ struct notifier_block dmc_nb;
+ struct dram_timing *timing;
+ wait_queue_head_t wait_dcf_queue;
+ int irq;
+ int wait_dcf_flag;
+ struct regulator *vdd_center;
+ unsigned long rate, target_rate;
+ unsigned long volt, target_volt;
+};
+
+static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
+ u32 flags)
+{
+ struct platform_device *pdev = container_of(dev, struct platform_device,
+ dev);
+ struct rk3399_dmcfreq *dmcfreq = platform_get_drvdata(pdev);
+ struct dev_pm_opp *opp;
+ unsigned long old_clk_rate = dmcfreq->rate;
+ unsigned long target_volt, target_rate;
+ int err;
+
+ rcu_read_lock();
+ opp = devfreq_recommended_opp(dev, freq, flags);
+ if (IS_ERR(opp)) {
+ rcu_read_unlock();
+ return PTR_ERR(opp);
+ }
+ target_rate = dev_pm_opp_get_freq(opp);
+ target_volt = dev_pm_opp_get_voltage(opp);
+ opp = devfreq_recommended_opp(dev, &dmcfreq->rate, flags);
+ if (IS_ERR(opp)) {
+ rcu_read_unlock();
+ return PTR_ERR(opp);
+ }
+ dmcfreq->volt = dev_pm_opp_get_voltage(opp);
+ rcu_read_unlock();
+
+ if (dmcfreq->rate == target_rate)
+ return 0;
+
+ mutex_lock(&dmcfreq->lock);
+ /*
+ * if frequency scaling from low to high, adjust voltage first;
+ * if frequency scaling from high to low, adjuset frequency first;
+ */
+ if (old_clk_rate < target_rate) {
+ err = regulator_set_voltage(dmcfreq->vdd_center, target_volt,
+ target_volt);
+ if (err) {
+ dev_err(dev, "Unable to set vol %lu\n", target_volt);
+ goto out;
+ }
+ }
+
+ dmc_event(DMCFREQ_ADJUST);
+ dmcfreq->wait_dcf_flag = 1;
+
+ err = clk_set_rate(dmcfreq->dmc_clk, target_rate);
+ if (err) {
+ dev_err(dev,
+ "Unable to set freq %lu. Current freq %lu. Error %d\n",
+ target_rate, old_clk_rate, err);
+ regulator_set_voltage(dmcfreq->vdd_center, dmcfreq->volt,
+ dmcfreq->volt);
+ dmc_event(DMCFREQ_FINISH);
+ goto out;
+ }
+
+ /*
+ * wait until bcf irq happen, it means freq scaling finish in bl31,
+ * use 100ms as timeout time
+ */
+ wait_event_timeout(dmcfreq->wait_dcf_queue,
+ !dmcfreq->wait_dcf_flag, HZ / 10);
+
+ dmc_event(DMCFREQ_FINISH);
+
+ /*
+ * check the dpll rate
+ * there only two result we will get,
+ * 1. ddr frequency scaling fail, we still get the old rate
+ * 2, ddr frequency scaling sucessful, we get the rate we set
+ */
+ dmcfreq->rate = clk_get_rate(dmcfreq->dmc_clk);
+
+ /* if get the incorrect rate, set voltage to old value */
+ if (dmcfreq->rate != target_rate) {
+ dev_err(dev, "get wrong ddr frequency, Request freq %lu,\
+ Current freq %lu\n", target_rate, dmcfreq->rate);
+ regulator_set_voltage(dmcfreq->vdd_center, dmcfreq->volt,
+ dmcfreq->volt);
+ } else if (old_clk_rate > target_rate)
+ err = regulator_set_voltage(dmcfreq->vdd_center, target_volt,
+ target_volt);
+ if (err)
+ dev_err(dev, "Unable to set vol %lu\n", target_volt);
+
+out:
+ mutex_unlock(&dmcfreq->lock);
+ return err;
+}
+
+static int rk3399_dmcfreq_get_dev_status(struct device *dev,
+ struct devfreq_dev_status *stat)
+{
+ struct platform_device *pdev = container_of(dev, struct platform_device,
+ dev);
+ struct rk3399_dmcfreq *dmcfreq = platform_get_drvdata(pdev);
+ struct devfreq_event_data edata;
+
+ devfreq_event_get_event(dmcfreq->edev, &edata);
+
+ stat->current_frequency = dmcfreq->rate;
+ stat->busy_time = edata.load_count;
+ stat->total_time = edata.total_count;
+
+ return 0;
+}
+
+static int rk3399_dmcfreq_get_cur_freq(struct device *dev, unsigned long *freq)
+{
+ struct platform_device *pdev = container_of(dev, struct platform_device,
+ dev);
+ struct rk3399_dmcfreq *dmcfreq = platform_get_drvdata(pdev);
+
+ *freq = dmcfreq->rate;
+
+ return 0;
+}
+
+static void rk3399_dmcfreq_exit(struct device *dev)
+{
+ struct platform_device *pdev = container_of(dev, struct platform_device,
+ dev);
+ struct rk3399_dmcfreq *dmcfreq = platform_get_drvdata(pdev);
+
+ devfreq_unregister_opp_notifier(dev, dmcfreq->devfreq);
+}
+
+static struct devfreq_dev_profile rk3399_devfreq_dmc_profile = {
+ .polling_ms = 200,
+ .target = rk3399_dmcfreq_target,
+ .get_dev_status = rk3399_dmcfreq_get_dev_status,
+ .get_cur_freq = rk3399_dmcfreq_get_cur_freq,
+ .exit = rk3399_dmcfreq_exit,
+};
+
+static __maybe_unused int rk3399_dmcfreq_suspend(struct device *dev)
+{
+ rockchip_dmc_disable();
+ return 0;
+}
+
+static __maybe_unused int rk3399_dmcfreq_resume(struct device *dev)
+{
+ rockchip_dmc_enable();
+ return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(rk3399_dmcfreq_pm, rk3399_dmcfreq_suspend,
+ rk3399_dmcfreq_resume);
+
+static int rk3399_dmc_enable_notify(struct notifier_block *nb,
+ unsigned long event, void *data)
+{
+ struct rk3399_dmcfreq *dmcfreq =
+ container_of(nb, struct rk3399_dmcfreq, dmc_nb);
+ unsigned long freq = ULONG_MAX;
+
+ if (event == DMC_ENABLE) {
+ devfreq_event_enable_edev(dmcfreq->edev);
+ devfreq_resume_device(dmcfreq->devfreq);
+ return NOTIFY_OK;
+ } else if (event == DMC_DISABLE) {
+ devfreq_event_disable_edev(dmcfreq->edev);
+ devfreq_suspend_device(dmcfreq->devfreq);
+
+ /* when disable dmc, set sdram to max frequency */
+ rk3399_dmcfreq_target(dmcfreq->dev, &freq, 0);
+ return NOTIFY_OK;
+ }
+
+ return NOTIFY_DONE;
+}
+
+static irqreturn_t rk3399_dmc_irq(int irq, void *dev_id)
+{
+ struct rk3399_dmcfreq *dmcfreq = dev_id;
+
+ dmcfreq->wait_dcf_flag = 0;
+ wake_up(&dmcfreq->wait_dcf_queue);
+
+ /* clr dcf irq */
+ sip_smc_clr_ddr_irq();
+
+ return IRQ_HANDLED;
+}
+
+static int of_do_get_timing(struct device_node *np,
+ struct dram_timing *timing)
+{
+ int ret;
+
+ ret = of_property_read_u32(np, "ddr3_speed_bin",
+ &timing->ddr3_speed_bin);
+ ret |= of_property_read_u32(np, "pd_idle", &timing->pd_idle);
+ ret |= of_property_read_u32(np, "sr_idle", &timing->sr_idle);
+ ret |= of_property_read_u32(np, "sr_mc_gate_idle",
+ &timing->sr_mc_gate_idle);
+ ret |= of_property_read_u32(np, "srpd_lite_idle",
+ &timing->srpd_lite_idle);
+ ret |= of_property_read_u32(np, "standby_idle", &timing->standby_idle);
+ ret |= of_property_read_u32(np, "dram_dll_dis_freq",
+ &timing->dram_dll_dis_freq);
+ ret |= of_property_read_u32(np, "phy_dll_dis_freq",
+ &timing->phy_dll_dis_freq);
+ ret |= of_property_read_u32(np, "ddr3_odt_dis_freq",
+ &timing->ddr3_odt_dis_freq);
+ ret |= of_property_read_u32(np, "ddr3_drv", &timing->ddr3_drv);
+ ret |= of_property_read_u32(np, "ddr3_odt", &timing->ddr3_odt);
+ ret |= of_property_read_u32(np, "phy_ddr3_ca_drv",
+ &timing->phy_ddr3_ca_drv);
+ ret |= of_property_read_u32(np, "phy_ddr3_dq_drv",
+ &timing->phy_ddr3_dq_drv);
+ ret |= of_property_read_u32(np, "phy_ddr3_odt", &timing->phy_ddr3_odt);
+ ret |= of_property_read_u32(np, "lpddr3_odt_dis_freq",
+ &timing->lpddr3_odt_dis_freq);
+ ret |= of_property_read_u32(np, "lpddr3_drv", &timing->lpddr3_drv);
+ ret |= of_property_read_u32(np, "lpddr3_odt", &timing->lpddr3_odt);
+ ret |= of_property_read_u32(np, "phy_lpddr3_ca_drv",
+ &timing->phy_lpddr3_ca_drv);
+ ret |= of_property_read_u32(np, "phy_lpddr3_dq_drv",
+ &timing->phy_lpddr3_dq_drv);
+ ret |= of_property_read_u32(np, "phy_lpddr3_odt",
+ &timing->phy_lpddr3_odt);
+ ret |= of_property_read_u32(np, "lpddr4_odt_dis_freq",
+ &timing->lpddr4_odt_dis_freq);
+ ret |= of_property_read_u32(np, "lpddr4_drv",
+ &timing->lpddr4_drv);
+ ret |= of_property_read_u32(np, "lpddr4_dq_odt",
+ &timing->lpddr4_dq_odt);
+ ret |= of_property_read_u32(np, "lpddr4_ca_odt",
+ &timing->lpddr4_ca_odt);
+ ret |= of_property_read_u32(np, "phy_lpddr4_ca_drv",
+ &timing->phy_lpddr4_ca_drv);
+ ret |= of_property_read_u32(np, "phy_lpddr4_ck_cs_drv",
+ &timing->phy_lpddr4_ck_cs_drv);
+ ret |= of_property_read_u32(np, "phy_lpddr4_dq_drv",
+ &timing->phy_lpddr4_dq_drv);
+ ret |= of_property_read_u32(np, "phy_lpddr4_odt",
+ &timing->phy_lpddr4_odt);
+
+ return ret;
+}
+
+static struct dram_timing *of_get_ddr_timings(struct device *dev,
+ struct device_node *np)
+{
+ struct dram_timing *timing = NULL;
+ struct device_node *np_tim;
+
+ np_tim = of_parse_phandle(np, "ddr_timing", 0);
+ if (np_tim) {
+ timing = devm_kzalloc(dev, sizeof(*timing), GFP_KERNEL);
+ if (!timing)
+ goto err;
+ if (of_do_get_timing(np_tim, timing)) {
+ devm_kfree(dev, timing);
+ goto err;
+ }
+ return timing;
+ }
+
+err:
+ if (timing) {
+ devm_kfree(dev, timing);
+ timing = NULL;
+ }
+
+ return timing;
+}
+
+static int rk3399_dmcfreq_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct rk3399_dmcfreq *data;
+ int ret, irq, index;
+ uint64_t param = 0;
+ uint32_t *timing;
+ struct device_node *np = pdev->dev.of_node;
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0) {
+ dev_err(&pdev->dev, "no dmc irq resource\n");
+ return -EINVAL;
+ }
+
+ data = devm_kzalloc(dev, sizeof(struct rk3399_dmcfreq), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ mutex_init(&data->lock);
+
+ data->vdd_center = devm_regulator_get(dev, "center");
+ if (IS_ERR(data->vdd_center)) {
+ dev_err(dev, "Cannot get the regulator \"center\"\n");
+ return PTR_ERR(data->vdd_center);
+ }
+
+ data->dmc_clk = devm_clk_get(dev, "dmc_clk");
+ if (IS_ERR(data->dmc_clk)) {
+ dev_err(dev, "Cannot get the clk dmc_clk\n");
+ return PTR_ERR(data->dmc_clk);
+ };
+
+ data->edev = devfreq_event_get_edev_by_phandle(dev, 0);
+ if (IS_ERR(data->edev))
+ return -EPROBE_DEFER;
+
+ ret = devfreq_event_enable_edev(data->edev);
+ if (ret < 0) {
+ dev_err(dev, "failed to enable devfreq-event devices\n");
+ return ret;
+ }
+
+ /*
+ * We add a devfreq driver to our parent since it has a device tree node
+ * with operating points.
+ */
+ if (dev_pm_opp_of_add_table(dev)) {
+ dev_err(dev, "Invalid operating-points in device tree.\n");
+ return -EINVAL;
+ }
+
+ of_property_read_u32(np, "upthreshold",
+ &data->ondemand_data.upthreshold);
+
+ of_property_read_u32(np, "downdifferential",
+ &data->ondemand_data.downdifferential);
+
+ data->rate = clk_get_rate(data->dmc_clk);
+ rk3399_devfreq_dmc_profile.initial_freq = data->rate;
+ data->devfreq = devfreq_add_device(dev,
+ &rk3399_devfreq_dmc_profile,
+ "simple_ondemand",
+ &data->ondemand_data);
+ if (IS_ERR(data->devfreq))
+ return PTR_ERR(data->devfreq);
+
+ devfreq_register_opp_notifier(dev, data->devfreq);
+
+ data->dmc_nb.notifier_call = rk3399_dmc_enable_notify;
+ dmc_register_notifier(&data->dmc_nb);
+ data->irq = irq;
+ ret = devm_request_irq(dev, irq, rk3399_dmc_irq, 0,
+ dev_name(dev), data);
+ if (ret) {
+ dev_err(dev, "failed to request dmc irq: %d\n", ret);
+ return ret;
+ }
+
+ /* get dram timing and pass it to bl31 */
+ data->timing = of_get_ddr_timings(dev, np);
+ if (data->timing) {
+ timing = (uint32_t *)data->timing;
+ for (index = 0; index < (sizeof(struct dram_timing) / 4);
+ index++) {
+ param = index;
+ param = param << 32 | *timing++;
+ sip_smc_set_ddr_param(param);
+ param = 0;
+ }
+ }
+ sip_smc_ddr_init();
+
+ init_waitqueue_head(&data->wait_dcf_queue);
+ data->wait_dcf_flag = 0;
+
+ data->dev = dev;
+ platform_set_drvdata(pdev, data);
+
+ return 0;
+}
+
+static int rk3399_dmcfreq_remove(struct platform_device *pdev)
+{
+ struct rk3399_dmcfreq *dmcfreq = platform_get_drvdata(pdev);
+
+ devfreq_remove_device(dmcfreq->devfreq);
+ regulator_put(dmcfreq->vdd_center);
+
+ return 0;
+}
+
+static const struct of_device_id rk3399dmc_devfreq_of_match[] = {
+ { .compatible = "rockchip,rk3399-dmc" },
+ { },
+};
+
+static struct platform_driver rk3399_dmcfreq_driver = {
+ .probe = rk3399_dmcfreq_probe,
+ .remove = rk3399_dmcfreq_remove,
+ .driver = {
+ .name = "rk3399-dmc-freq",
+ .pm = &rk3399_dmcfreq_pm,
+ .of_match_table = rk3399dmc_devfreq_of_match,
+ },
+};
+module_platform_driver(rk3399_dmcfreq_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("RK3399 dmcfreq driver with devfreq framework");
diff --git a/drivers/devfreq/rockchip/rockchip_dmc.c b/drivers/devfreq/rockchip/rockchip_dmc.c
new file mode 100644
index 0000000..78baee5
--- /dev/null
+++ b/drivers/devfreq/rockchip/rockchip_dmc.c
@@ -0,0 +1,143 @@
+/*
+ * Copyright (c) 2016, Fuzhou Rockchip Electronics Co., Ltd
+ * Author: Lin Huang <h...@rock-chips.com>
+ * Base on: https://chromium-review.googlesource.com/#/c/231477/
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ */
+
+#include <linux/mutex.h>
+#include <soc/rockchip/rockchip_dmc.h>
+
+static int num_wait;
+static int num_disable;
+static BLOCKING_NOTIFIER_HEAD(dmc_notifier_list);
+static DEFINE_MUTEX(dmc_en_lock);
+static DEFINE_MUTEX(dmc_sync_lock);
+
+/**
+ * rockchip_dmc_enabled - Returns true if dmc freq is enabled, false otherwise.
+ */
+bool rockchip_dmc_enabled(void)
+{
+ return num_disable <= 0 && num_wait <= 1;
+}
+
+/**
+ * rockchip_dmc_enable - Enable dmc frequency scaling. Will only enable
+ * frequency scaling if there are 1 or fewer notifiers. Call to undo
+ * rockchip_dmc_disable.
+ */
+void rockchip_dmc_enable(void)
+{
+ mutex_lock(&dmc_en_lock);
+ num_disable--;
+ WARN_ON(num_disable < 0);
+ if (rockchip_dmc_enabled())
+ dmc_event(DMC_ENABLE);
+ mutex_unlock(&dmc_en_lock);
+}
+
+/**
+ * rockchip_dmc_disable - Disable dmc frequency scaling. Call when something
+ * cannot coincide with dmc frequency scaling.
+ */
+void rockchip_dmc_disable(void)
+{
+ mutex_lock(&dmc_en_lock);
+ if (rockchip_dmc_enabled())
+ dmc_event(DMC_DISABLE);
+ num_disable++;
+ mutex_unlock(&dmc_en_lock);
+}
+
+void dmc_event(int event)
+{
+ mutex_lock(&dmc_sync_lock);
+ blocking_notifier_call_chain(&dmc_notifier_list, event, NULL);
+ mutex_unlock(&dmc_sync_lock);
+}
+
+/**
+ * dmc_register_notifier - register a driver to dmc chain
+ * @nb: notifier function to register
+ */
+int dmc_register_notifier(struct notifier_block *nb)
+{
+ int ret;
+
+ if (!nb)
+ return -EINVAL;
+
+ ret = blocking_notifier_chain_register(&dmc_notifier_list, nb);
+
+ return ret;
+}
+
+/**
+ * dmc_unregister_notifier - unregister a driver from dmc chain
+ * @nb: remove notifier function
+ */
+int dmc_unregister_notifier(struct notifier_block *nb)
+{
+ int ret;
+
+ if (!nb)
+ return -EINVAL;
+
+ ret = blocking_notifier_chain_unregister(&dmc_notifier_list, nb);
+
+ return ret;
+}
+
+/**
+ * rockchip_dmc_get - Register the notifier block for the dmc chain.
+ * @nb The dmc notifier block to register
+ */
+int rockchip_dmc_get(struct notifier_block *nb)
+{
+ if (!nb)
+ return -EINVAL;
+
+ mutex_lock(&dmc_en_lock);
+
+ /*
+ * if have two notifier(enable two vop etc),
+ * need to disable dmc
+ */
+ if (num_wait == 1 && num_disable <= 0)
+ dmc_event(DMC_DISABLE);
+ num_wait++;
+ dmc_register_notifier(nb);
+ mutex_unlock(&dmc_en_lock);
+
+ return 0;
+}
+
+/**
+ * rockchip_dmc_put - Remove the notifier block from the dmc chain.
+ * @nb The dmc notifier block to unregister
+ */
+int rockchip_dmc_put(struct notifier_block *nb)
+{
+ if (!nb)
+ return -EINVAL;
+
+ mutex_lock(&dmc_en_lock);
+ num_wait--;
+
+ /* if notifier from 2 back to 1, enable dmc again */
+ if (num_wait == 1 && num_disable <= 0)
+ dmc_event(DMC_ENABLE);
+ dmc_unregister_notifier(nb);
+ mutex_unlock(&dmc_en_lock);
+
+ return 0;
+}
diff --git a/include/soc/rockchip/rockchip_dmc.h b/include/soc/rockchip/rockchip_dmc.h
new file mode 100644
index 0000000..3f69cbf
--- /dev/null
+++ b/include/soc/rockchip/rockchip_dmc.h
@@ -0,0 +1,45 @@
+/*
+ * Copyright (c) 2016, Fuzhou Rockchip Electronics Co., Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ */
+
+#ifndef __SOC_ROCKCHIP_DMC_H
+#define __SOC_ROCKCHIP_DMC_H
+
+#include <linux/notifier.h>
+
+#define DMC_ENABLE 0
+#define DMC_DISABLE 1
+#define DMCFREQ_ADJUST 2
+#define DMCFREQ_FINISH 3
+
+#if IS_ENABLED(CONFIG_ARM_ROCKCHIP_DMC_DEVFREQ)
+int rockchip_dmc_get(struct notifier_block *nb);
+int rockchip_dmc_put(struct notifier_block *nb);
+#else
+static inline int rockchip_dmc_get(struct notifier_block *nb)
+{
+ return 0;
+}
+
+static inline int rockchip_dmc_put(struct notifier_block *nb)
+{
+ return 0;
+}
+#endif
+
+void dmc_event(int event);
+int dmc_register_notifier(struct notifier_block *nb);
+int dmc_unregister_notifier(struct notifier_block *nb);
+void rockchip_dmc_enable(void);
+void rockchip_dmc_disable(void);
+bool rockchip_dmc_enabled(void);
+#endif
--
2.6.6

[Paul Gortmaker]

Since you are using bool Kconfigs for your driver, please do not use
module.h or MODULE_<xyz> tags in your driver, and use the builtin
register function.

Alternatively if there really is a use case for it to be a modular driver
then use a tristate Kconfig.

THanks,
Paul.
--

[hl]

Hi Paul,

Thanks for pointing it, will fix it next version.

> THanks,
> Paul.
> --
>
>> + depends on ARM_ROCKCHIP_DMC_DEVFREQ
>> + select PM_OPP
>> + select DEVFREQ_GOV_SIMPLE_ONDEMAND
>> + help
>> + This adds the DEVFREQ driver for the RK3399 dmc. It sets the frequency
>> + for the memory controller and reads the usage counts from hardware.
>> +

> _______________________________________________
> Linux-rockchip mailing list
> Linux-r...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>
>
>

--
Lin Huang

[Chanwoo Choi]

Hi Lin,

I'm glad to support the for dmc ddr clock scaling with devfreq/devfreq-event.

But, I think that you have to use the standard interface.
As I already mentioned[1] on previous mail, devfreq fwk support
the standard DEVFREQ_TRANSITION_NOTIFIER notifier which has the two notifications.
- DEVFREQ_PRECHANGE
- DEVFREQ_POSTCHANGE
[1] https://patchwork.kernel.org/patch/9194305/

You can use the DEVFREQ_PRECHANGE instead of DMCFREQ_ADJUST
and use the DEVFREQ_POSTCHANGE instead of DMCFREQ_FINISH.

You don't need to send the notification in target function.
When you use the DEVFREQ_TRANSITION_NOTIFIER notifier,
devfreq send the notification before calling the target() and after calling the target().

> + dmcfreq->wait_dcf_flag = 1;
> +
> + err = clk_set_rate(dmcfreq->dmc_clk, target_rate);
> + if (err) {
> + dev_err(dev,
> + "Unable to set freq %lu. Current freq %lu. Error %d\n",
> + target_rate, old_clk_rate, err);
> + regulator_set_voltage(dmcfreq->vdd_center, dmcfreq->volt,
> + dmcfreq->volt);
> + dmc_event(DMCFREQ_FINISH);

ditto.

> + goto out;
> + }
> +
> + /*
> + * wait until bcf irq happen, it means freq scaling finish in bl31,
> + * use 100ms as timeout time
> + */
> + wait_event_timeout(dmcfreq->wait_dcf_queue,
> + !dmcfreq->wait_dcf_flag, HZ / 10);
> +
> + dmc_event(DMCFREQ_FINISH);

ditto.

Regards,
Chanwoo Choi

[hl]

Hi Chanwoo Choi,

I checked DEVFREQ_TRANSISTION_NOTIFIER notifier before, but i have some
question:
Now i want to use
DEVFREQ_PRECHANGE and DEVFREQ_POSTCHANGE in vop driver, but it hard
to get the *devfreq
in vop driver, or i can register a DEVFREQ_TRANSITION_NOTIFIER notifier
in rk3399_dmc.c and
when get message, post DEVFREQ_PRECHANGE or DEVFREQ_POSTCHANGE to vop
driver through dmc_event(),
do you have a better way to do it?

[Chanwoo Choi]

Hi Lin,

Firstly,
The devfreq has the devfreq_get_devfreq_by_phandle() function
to get the devfreq pointer in device tree. The vop_bind()
get the IORESOURCE_MEM and interrupt. So, I think that you
would get the devfreq pointer in the vop_bind() function.
So, DEVFREQ_TRANSITION_NOTIFIER can alter the following two notifications:
- dmc_event(DMCFREQ_ADJUST);
- dmc_event(DMCFREQ_FINISH);


Also,
devfreq-event fwk provide the devfreq_event_get_edev_by_phandle() funtcion
to get the devfreq-event pointer in the device tree.

You could call the devfreq_event_[enable/disable[_edev()
and devfreq_[resume/suspend]_device in the drivers/gpu/drm/rockchip/rockchip_drm_vop.c
instead of dmc_event(DMC_ENABLE) / dmc_event(DMC_DISABLE).

dmc_event(DMC_ENABLE);
devfreq_event_enable_edev(dmcfreq->edev);
devfreq_resume_device(dmcfreq->devfreq);

dmc_event(DMC_DISABLE);
devfreq_event_disable_edev(dmcfreq->edev);
devfreq_suspend_device(dmcfreq->devfreq);

I think that we have to use the standard interfaces and
functions providing by framework. It is not appropriate way
to define the specific functions for only one device driver
between other framework.

Regards,
Chanwoo Choi

[hl]

Hi Chanwoo Choi,

Thanks for you suggestion, this may be a huge update, i will try to
implement it.

--
Lin Huang

[Lin Huang]

base on dfi result, we do ddr frequency scaling, register
dmc driver to devfreq framework, and use simple-ondemand
policy.

Signed-off-by: Lin Huang <h...@rock-chips.com>
---

Changes in v4:
- use arm_smccc_smc() function talk to bl31
- delete rockchip_dmc.c file and config
- delete dmc_notify
- adjust probe order

Changes in v3:
- operate dram setting through sip call
- imporve set rate flow

Changes in v2:
- None

Changes in v1:
- move dfi controller to event
- fix set voltage sequence when set rate fail
- change Kconfig type from tristate to bool
- move unuse EXPORT_SYMBOL_GPL()

drivers/devfreq/Kconfig | 1 +
drivers/devfreq/Makefile | 1 +

drivers/devfreq/rockchip/Kconfig | 8 +
drivers/devfreq/rockchip/Makefile | 1 +
drivers/devfreq/rockchip/rk3399_dmc.c | 473 ++++++++++++++++++++++++++++++++++
5 files changed, 484 insertions(+)

create mode 100644 drivers/devfreq/rockchip/Kconfig
create mode 100644 drivers/devfreq/rockchip/Makefile
create mode 100644 drivers/devfreq/rockchip/rk3399_dmc.c

diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
index 64281bb..acb2a57 100644
--- a/drivers/devfreq/Kconfig
+++ b/drivers/devfreq/Kconfig
@@ -99,5 +99,6 @@ config ARM_TEGRA_DEVFREQ

operating frequencies and voltages with OPP support.

source "drivers/devfreq/event/Kconfig"
+source "drivers/devfreq/rockchip/Kconfig"

endif # PM_DEVFREQ
diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile

index 5134f9e..d844e23 100644
--- a/drivers/devfreq/Makefile
+++ b/drivers/devfreq/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_DEVFREQ_GOV_USERSPACE) += governor_userspace.o
obj-$(CONFIG_ARM_EXYNOS4_BUS_DEVFREQ) += exynos/
obj-$(CONFIG_ARM_EXYNOS5_BUS_DEVFREQ) += exynos/

obj-$(CONFIG_ARM_TEGRA_DEVFREQ) += tegra-devfreq.o
+obj-$(CONFIG_ARCH_ROCKCHIP) += rockchip/

# DEVFREQ Event Drivers
obj-$(CONFIG_PM_DEVFREQ_EVENT) += event/
diff --git a/drivers/devfreq/rockchip/Kconfig b/drivers/devfreq/rockchip/Kconfig
new file mode 100644

index 0000000..d8f9e66
--- /dev/null
+++ b/drivers/devfreq/rockchip/Kconfig
@@ -0,0 +1,8 @@
+config ARM_RK3399_DMC_DEVFREQ
+ tristate "ARM RK3399 DMC DEVFREQ Driver"

+ select PM_OPP
+ select DEVFREQ_GOV_SIMPLE_ONDEMAND
+ help
+ This adds the DEVFREQ driver for the RK3399 dmc. It sets the frequency
+ for the memory controller and reads the usage counts from hardware.
+
diff --git a/drivers/devfreq/rockchip/Makefile b/drivers/devfreq/rockchip/Makefile
new file mode 100644

index 0000000..c62c105
--- /dev/null
+++ b/drivers/devfreq/rockchip/Makefile
@@ -0,0 +1 @@

+obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ) += rk3399_dmc.o
diff --git a/drivers/devfreq/rockchip/rk3399_dmc.c b/drivers/devfreq/rockchip/rk3399_dmc.c
new file mode 100644

index 0000000..527aa11
--- /dev/null
+++ b/drivers/devfreq/rockchip/rk3399_dmc.c
@@ -0,0 +1,473 @@

+/*
+ * Copyright (c) 2016, Fuzhou Rockchip Electronics Co., Ltd
+ * Author: Lin Huang <h...@rock-chips.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ */
+

+#include <linux/arm-smccc.h>

+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/devfreq.h>
+#include <linux/devfreq-event.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_opp.h>
+#include <linux/regulator/consumer.h>
+#include <linux/rwsem.h>
+#include <linux/suspend.h>
+#include <linux/syscore_ops.h>
+

+#include <soc/rockchip/rockchip_sip.h>

+ dmcfreq->wait_dcf_flag = 1;
+ err = clk_set_rate(dmcfreq->dmc_clk, target_rate);
+ if (err) {
+ dev_err(dev,
+ "Unable to set freq %lu. Current freq %lu. Error %d\n",
+ target_rate, old_clk_rate, err);
+ regulator_set_voltage(dmcfreq->vdd_center, dmcfreq->volt,
+ dmcfreq->volt);

+ goto out;
+ }
+
+ /*
+ * wait until bcf irq happen, it means freq scaling finish in bl31,
+ * use 100ms as timeout time
+ */

+ if (!wait_event_timeout(dmcfreq->wait_dcf_queue,
+ !dmcfreq->wait_dcf_flag, HZ / 10))
+ dev_warn(dev, "Timeout waiting for dcf irq\n");

+ struct platform_device,

+ dev);
+ struct rk3399_dmcfreq *dmcfreq = platform_get_drvdata(pdev);
+
+ devfreq_unregister_opp_notifier(dev, dmcfreq->devfreq);
+}
+
+static struct devfreq_dev_profile rk3399_devfreq_dmc_profile = {
+ .polling_ms = 200,
+ .target = rk3399_dmcfreq_target,
+ .get_dev_status = rk3399_dmcfreq_get_dev_status,
+ .get_cur_freq = rk3399_dmcfreq_get_cur_freq,
+ .exit = rk3399_dmcfreq_exit,
+};
+
+static __maybe_unused int rk3399_dmcfreq_suspend(struct device *dev)
+{

+ struct platform_device *pdev = container_of(dev,

+ struct platform_device,

+ dev);
+ struct rk3399_dmcfreq *dmcfreq = platform_get_drvdata(pdev);
+

+ devfreq_event_disable_edev(dmcfreq->edev);
+ devfreq_suspend_device(dmcfreq->devfreq);
+

+ return 0;
+}
+
+static __maybe_unused int rk3399_dmcfreq_resume(struct device *dev)
+{

+ struct platform_device *pdev = container_of(dev,

+ struct platform_device,

+ dev);
+ struct rk3399_dmcfreq *dmcfreq = platform_get_drvdata(pdev);
+

+ devfreq_event_enable_edev(dmcfreq->edev);
+ devfreq_resume_device(dmcfreq->devfreq);
+

+ return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(rk3399_dmcfreq_pm, rk3399_dmcfreq_suspend,
+ rk3399_dmcfreq_resume);
+

+static irqreturn_t rk3399_dmc_irq(int irq, void *dev_id)
+{
+ struct rk3399_dmcfreq *dmcfreq = dev_id;

+ struct arm_smccc_res res;

+
+ dmcfreq->wait_dcf_flag = 0;
+ wake_up(&dmcfreq->wait_dcf_queue);
+
+ /* clr dcf irq */

+ arm_smccc_smc(SIP_DDR_FREQ, 0, 0, CONFIG_DRAM_CLR_IRQ,
+ 0, 0, 0, 0, &res);

+ struct arm_smccc_res res;

+ struct device *dev = &pdev->dev;

+ struct device_node *np = pdev->dev.of_node;

+ uint64_t param = 0;
+ struct rk3399_dmcfreq *data;
+ int ret, irq, index;

+ uint32_t *timing;

+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0) {
+ dev_err(&pdev->dev, "no dmc irq resource\n");
+ return -EINVAL;
+ }
+ data = devm_kzalloc(dev, sizeof(struct rk3399_dmcfreq), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ mutex_init(&data->lock);
+
+ data->vdd_center = devm_regulator_get(dev, "center");
+ if (IS_ERR(data->vdd_center)) {
+ dev_err(dev, "Cannot get the regulator \"center\"\n");
+ return PTR_ERR(data->vdd_center);
+ }
+
+ data->dmc_clk = devm_clk_get(dev, "dmc_clk");
+ if (IS_ERR(data->dmc_clk)) {
+ dev_err(dev, "Cannot get the clk dmc_clk\n");
+ return PTR_ERR(data->dmc_clk);
+ };
+

+ data->irq = irq;
+ ret = devm_request_irq(dev, irq, rk3399_dmc_irq, 0,
+ dev_name(dev), data);
+ if (ret) {
+ dev_err(dev, "failed to request dmc irq: %d\n", ret);
+ return ret;
+ }
+
+ /* get dram timing and pass it to bl31 */
+ data->timing = of_get_ddr_timings(dev, np);
+ if (data->timing) {
+ timing = (uint32_t *)data->timing;
+ for (index = 0; index < (sizeof(struct dram_timing) / 4);
+ index++) {
+ param = index;
+ param = param << 32 | *timing++;

+ arm_smccc_smc(SIP_DDR_FREQ, param, 0,
+ CONFIG_DRAM_SET_PARAM, 0, 0, 0, 0, &res);
+ if (res.a0) {
+ dev_err(dev, "failed to set dram param: %ld\n",
+ res.a0);
+ return -EINVAL;
+ }

+ param = 0;
+ }
+ }
+

+ arm_smccc_smc(SIP_DDR_FREQ, 0, 0, CONFIG_DRAM_INIT,
+ 0, 0, 0, 0, &res);

+
+ init_waitqueue_head(&data->wait_dcf_queue);
+ data->wait_dcf_flag = 0;
+

--
1.9.1

[Chanwoo Choi]

Hi Lin,

As I mentioned on patch5, you better to make the documentation as following:
- Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt
And, I add the comments.

This patch include the only one patch. So, I think that
you don't need to create the 'rockchip' directory.

>
> endif # PM_DEVFREQ
> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
> index 5134f9e..d844e23 100644
> --- a/drivers/devfreq/Makefile
> +++ b/drivers/devfreq/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_DEVFREQ_GOV_USERSPACE) += governor_userspace.o
> obj-$(CONFIG_ARM_EXYNOS4_BUS_DEVFREQ) += exynos/
> obj-$(CONFIG_ARM_EXYNOS5_BUS_DEVFREQ) += exynos/
> obj-$(CONFIG_ARM_TEGRA_DEVFREQ) += tegra-devfreq.o
> +obj-$(CONFIG_ARCH_ROCKCHIP) += rockchip/

ditto.

>
> # DEVFREQ Event Drivers
> obj-$(CONFIG_PM_DEVFREQ_EVENT) += event/
> diff --git a/drivers/devfreq/rockchip/Kconfig b/drivers/devfreq/rockchip/Kconfig
> new file mode 100644
> index 0000000..d8f9e66
> --- /dev/null
> +++ b/drivers/devfreq/rockchip/Kconfig
> @@ -0,0 +1,8 @@
> +config ARM_RK3399_DMC_DEVFREQ
> + tristate "ARM RK3399 DMC DEVFREQ Driver"
> + select PM_OPP
> + select DEVFREQ_GOV_SIMPLE_ONDEMAND
> + help
> + This adds the DEVFREQ driver for the RK3399 dmc. It sets the frequency

If you add the full description for 'dmc' as following,
it is easy to understand the operation of this device driver.
- DMC (Dynamic Memory Controller)

> + for the memory controller and reads the usage counts from hardware.
> +
> diff --git a/drivers/devfreq/rockchip/Makefile b/drivers/devfreq/rockchip/Makefile
> new file mode 100644
> index 0000000..c62c105
> --- /dev/null
> +++ b/drivers/devfreq/rockchip/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ) += rk3399_dmc.o
> diff --git a/drivers/devfreq/rockchip/rk3399_dmc.c b/drivers/devfreq/rockchip/rk3399_dmc.c
> new file mode 100644
> index 0000000..527aa11
> --- /dev/null
> +++ b/drivers/devfreq/rockchip/rk3399_dmc.c
> @@ -0,0 +1,473 @@
> +/*
> + * Copyright (c) 2016, Fuzhou Rockchip Electronics Co., Ltd

You miss the '.' at the end of the copylight.
When you use an abbreviation, you should add '.' for Ltd.
- s/Ltd/Ltd.

> + * Author: Lin Huang <h...@rock-chips.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + */
> +
> +#include <linux/arm-smccc.h>
> +#include <linux/clk.h>
> +#include <linux/completion.h>

You don't need to include the "completion.h".
Without "completion.h", the build is working.

> +#include <linux/delay.h>
> +#include <linux/devfreq.h>
> +#include <linux/devfreq-event.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_opp.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/rwsem.h>
> +#include <linux/suspend.h>
> +#include <linux/syscore_ops.h>

You don't need to include the "syscore_ops.h".
Without "syscore_ops.h", the build is working.

The dram_timing is used for SMC (Secure Monitor Call)?
I recommend that you add the detailed description about this property
on Documentation.

> +
> +struct rk3399_dmcfreq {
> + struct device *dev;
> + struct devfreq *devfreq;
> + struct devfreq_simple_ondemand_data ondemand_data;
> + struct clk *dmc_clk;
> + struct devfreq_event_dev *edev;
> + struct mutex lock;
> + struct dram_timing *timing;
> + wait_queue_head_t wait_dcf_queue;
> + int irq;
> + int wait_dcf_flag;

I want to add the full name and description of 'dcf'.
It is unknown word.

> + struct regulator *vdd_center;
> + unsigned long rate, target_rate;
> + unsigned long volt, target_volt;

I think that if you add 'curr_opp' variable,
you can reduce the calling of devfreq_recommended_opp() in rk3399_dmcfreq_target()
struct dev_pm_opp *curr_opp;

> +};
> +
> +static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
> + u32 flags)
> +{
> + struct platform_device *pdev = container_of(dev, struct platform_device,
> + dev);
> + struct rk3399_dmcfreq *dmcfreq = platform_get_drvdata(pdev);

You can use the 'dev_get_drvdata()' to simplify it instead of 'platform_get_drvdata()'.

struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(dev);

> + struct dev_pm_opp *opp;
> + unsigned long old_clk_rate = dmcfreq->rate;
> + unsigned long target_volt, target_rate;
> + int err;
> +
> + rcu_read_lock();
> + opp = devfreq_recommended_opp(dev, freq, flags);
> + if (IS_ERR(opp)) {
> + rcu_read_unlock();
> + return PTR_ERR(opp);
> + }
> +
> + target_rate = dev_pm_opp_get_freq(opp);
> + target_volt = dev_pm_opp_get_voltage(opp);
> + opp = devfreq_recommended_opp(dev, &dmcfreq->rate, flags);
> + if (IS_ERR(opp)) {
> + rcu_read_unlock();
> + return PTR_ERR(opp);
> + }
> + dmcfreq->volt = dev_pm_opp_get_voltage(opp);

If you add the 'curr_opp' variable to struct rk3399_dmcfreq,
you can remove the calling of devfreq_recommended_opp().
dmcfreq->rate = dev_pm_opp_get_freq(dmcfreq->curr_opp);
dmcfreq->volt = dev_pm_opp_get_freq(dmcfreq->curr_opp);

Because the current rate and voltage is already decided on previous polling cycle,
So we don't need to get the opp with devfreq_recommended_opp().

> + rcu_read_unlock();
> +
> + if (dmcfreq->rate == target_rate)
> + return 0;
> +
> + mutex_lock(&dmcfreq->lock);
> +
> + /*
> + * if frequency scaling from low to high, adjust voltage first;
> + * if frequency scaling from high to low, adjuset frequency first;
> + */

s/adjuset/adjust

I recommend that you use a captital letter for first character and use the '.'
instead of ';'.

> + if (old_clk_rate < target_rate) {
> + err = regulator_set_voltage(dmcfreq->vdd_center, target_volt,
> + target_volt);
> + if (err) {
> + dev_err(dev, "Unable to set vol %lu\n", target_volt);

To readability, you better to use the corrent word to pass the precise the log message.
- s/vol/voltage

And, this patch uses the 'Unable to' or 'Cannot' to show the error log.
I recommend that you use the consistent expression if there is not any specific reason.

dev_err(dev, "Cannot set the voltage %lu uV\n", target_volt);

> + goto out;
> + }
> + }
> + dmcfreq->wait_dcf_flag = 1;
> + err = clk_set_rate(dmcfreq->dmc_clk, target_rate);
> + if (err) {
> + dev_err(dev,
> + "Unable to set freq %lu. Current freq %lu. Error %d\n",
> + target_rate, old_clk_rate, err);

dev_err(dev, "Cannot set the frequency %lu (%d)\n", target_rate, err);

> + regulator_set_voltage(dmcfreq->vdd_center, dmcfreq->volt,
> + dmcfreq->volt);
> + goto out;
> + }
> +
> + /*
> + * wait until bcf irq happen, it means freq scaling finish in bl31,

ditto.

> + * use 100ms as timeout time

s/time/time.

> + */
> + if (!wait_event_timeout(dmcfreq->wait_dcf_queue,
> + !dmcfreq->wait_dcf_flag, HZ / 10))
> + dev_warn(dev, "Timeout waiting for dcf irq\n");

If the timeout happen, are there any problem?

After setting the frequency and voltage, store the current opp entry on .curr_opp.
dmcfreq->curr_opp = opp;

> + /*
> + * check the dpll rate
> + * there only two result we will get,
> + * 1. ddr frequency scaling fail, we still get the old rate
> + * 2, ddr frequency scaling sucessful, we get the rate we set
> + */
> + dmcfreq->rate = clk_get_rate(dmcfreq->dmc_clk);
> +
> + /* if get the incorrect rate, set voltage to old value */
> + if (dmcfreq->rate != target_rate) {
> + dev_err(dev, "get wrong ddr frequency, Request freq %lu,\
> + Current freq %lu\n", target_rate, dmcfreq->rate);
> + regulator_set_voltage(dmcfreq->vdd_center, dmcfreq->volt,
> + dmcfreq->volt);

[Without force, it is just my opion about this code:]
I think that this checking code it is un-needed.
If this case occur, the rk3399_dmc.c never set the specific frequency
because the rk3399 clock don't support the specific frequency for rk3399 dmc.

If you want to set the correct frequency,
When verifying the rk3399 dmc driver, you should check the rk3399 clock driver.

Basically, if the the clock driver don't support the correct same frequency ,
CCF(Common Clock Framework) set the close frequency. It is not a bad thing.

> + } else if (old_clk_rate > target_rate)
> + err = regulator_set_voltage(dmcfreq->vdd_center, target_volt,
> + target_volt);
> + if (err)
> + dev_err(dev, "Unable to set vol %lu\n", target_volt);

ditto. You better to use the consistent expression for error log as following:
dev_err(dev, "Cannot set the voltage %lu uV\n", target_volt);

> +
> +out:
> + mutex_unlock(&dmcfreq->lock);
> + return err;
> +}
> +
> +static int rk3399_dmcfreq_get_dev_status(struct device *dev,
> + struct devfreq_dev_status *stat)
> +{
> + struct platform_device *pdev = container_of(dev, struct platform_device,
> + dev);
> + struct rk3399_dmcfreq *dmcfreq = platform_get_drvdata(pdev);
> + struct devfreq_event_data edata;

ditto. Use the dev_get_drvdata(dev).

> +
> + devfreq_event_get_event(dmcfreq->edev, &edata);

You need to check the return value for exception handling.

> +
> + stat->current_frequency = dmcfreq->rate;
> + stat->busy_time = edata.load_count;
> + stat->total_time = edata.total_count;
> +
> + return 0;
> +}
> +
> +static int rk3399_dmcfreq_get_cur_freq(struct device *dev, unsigned long *freq)
> +{
> + struct platform_device *pdev = container_of(dev, struct platform_device,
> + dev);
> + struct rk3399_dmcfreq *dmcfreq = platform_get_drvdata(pdev);

ditto. Use the dev_get_drvdata(dev).

> +
> + *freq = dmcfreq->rate;
> +
> + return 0;
> +}
> +
> +static void rk3399_dmcfreq_exit(struct device *dev)
> +{
> + struct platform_device *pdev = container_of(dev,
> + struct platform_device,
> + dev);
> + struct rk3399_dmcfreq *dmcfreq = platform_get_drvdata(pdev);
> +
> + devfreq_unregister_opp_notifier(dev, dmcfreq->devfreq);

Use the devm_devfreq_register_opp_notifier(). You don't need to handle it.

> +}
> +
> +static struct devfreq_dev_profile rk3399_devfreq_dmc_profile = {
> + .polling_ms = 200,
> + .target = rk3399_dmcfreq_target,
> + .get_dev_status = rk3399_dmcfreq_get_dev_status,
> + .get_cur_freq = rk3399_dmcfreq_get_cur_freq,
> + .exit = rk3399_dmcfreq_exit,
> +};
> +
> +static __maybe_unused int rk3399_dmcfreq_suspend(struct device *dev)
> +{
> + struct platform_device *pdev = container_of(dev,
> + struct platform_device,
> + dev);
> + struct rk3399_dmcfreq *dmcfreq = platform_get_drvdata(pdev);

ditto. Use the dev_get_drvdata(dev).

> +
> + devfreq_event_disable_edev(dmcfreq->edev);
> + devfreq_suspend_device(dmcfreq->devfreq);

ditto. you need to check the return value.

> +
> + return 0;
> +}
> +
> +static __maybe_unused int rk3399_dmcfreq_resume(struct device *dev)
> +{
> + struct platform_device *pdev = container_of(dev,
> + struct platform_device,
> + dev);
> + struct rk3399_dmcfreq *dmcfreq = platform_get_drvdata(pdev);

ditto.

> +
> + devfreq_event_enable_edev(dmcfreq->edev);
> + devfreq_resume_device(dmcfreq->devfreq);

ditto.

> +
> + return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(rk3399_dmcfreq_pm, rk3399_dmcfreq_suspend,
> + rk3399_dmcfreq_resume);
> +
> +static irqreturn_t rk3399_dmc_irq(int irq, void *dev_id)
> +{
> + struct rk3399_dmcfreq *dmcfreq = dev_id;
> + struct arm_smccc_res res;
> +
> + dmcfreq->wait_dcf_flag = 0;
> + wake_up(&dmcfreq->wait_dcf_queue);
> +
> + /* clr dcf irq */

s/ clr dcf irq -> "Clear the DCF Interrupt"

As I already mentioned, you need to add the detailed documetation about the property.

> + if (np_tim) {
> + timing = devm_kzalloc(dev, sizeof(*timing), GFP_KERNEL);
> + if (!timing)
> + goto err;
> + if (of_do_get_timing(np_tim, timing)) {

I recommend that you better to squash following two functions
because of_do_get_timing() is not used on other point of this driver.
- of_do_get_timing()
- of_get_ddr_timings()

> + devm_kfree(dev, timing);
> + goto err;
> + }

You're missing the 'of_node_put'.
of_node_put(np_tim);

> + return timing;
> + }
> +
> +err:
> + if (timing) {
> + devm_kfree(dev, timing);
> + timing = NULL;
> + }
> +

ditto.
of_node_put(np_tim);

> + return timing;
> +}
> +
> +static int rk3399_dmcfreq_probe(struct platform_device *pdev)
> +{
> + struct arm_smccc_res res;
> + struct device *dev = &pdev->dev;
> + struct device_node *np = pdev->dev.of_node;
> + uint64_t param = 0;
> + struct rk3399_dmcfreq *data;
> + int ret, irq, index;
> + uint32_t *timing;
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0) {
> + dev_err(&pdev->dev, "no dmc irq resource\n");

We need to maintain the consistent expression for error log.
dev_err(&pdev->dev, ""Cannot get the interrupt resource"\n");

ditto. "Failed to request the dmc irq" or "Cannot request the dmc irq"

> + return ret;
> + }
> +
> + /* get dram timing and pass it to bl31 */

I want to add the more detailed description why this code is necessary.
Because the SMC is usually black box. So, If you add the more explanation,
it help people to understand this code.

> + data->timing = of_get_ddr_timings(dev, np);
> + if (data->timing) {
> + timing = (uint32_t *)data->timing;
> + for (index = 0; index < (sizeof(struct dram_timing) / 4);
> + index++) {

You better to use following code. It is better way to use
the only one line for 'for' statement.

size = sizeof(struct dram_timing) / 4;
for (index = 0; index < size; index++) {

> + param = index;
> + param = param << 32 | *timing++;
> + arm_smccc_smc(SIP_DDR_FREQ, param, 0,
> + CONFIG_DRAM_SET_PARAM, 0, 0, 0, 0, &res);
> + if (res.a0) {
> + dev_err(dev, "failed to set dram param: %ld\n",
> + res.a0);
> + return -EINVAL;
> + }
> + param = 0;
> + }
> + }
> +
> + arm_smccc_smc(SIP_DDR_FREQ, 0, 0, CONFIG_DRAM_INIT,
> + 0, 0, 0, 0, &res);

ditto. You need to add the description.

> +
> + init_waitqueue_head(&data->wait_dcf_queue);
> + data->wait_dcf_flag = 0;
> +
> + data->edev = devfreq_event_get_edev_by_phandle(dev, 0);
> + if (IS_ERR(data->edev))
> + return -EPROBE_DEFER;
> +
> + ret = devfreq_event_enable_edev(data->edev);
> + if (ret < 0) {
> + dev_err(dev, "failed to enable devfreq-event devices\n");

s/failed/Failed. Use a capital letter for the first char.

> + return ret;
> + }
> +
> + /*
> + * We add a devfreq driver to our parent since it has a device tree node
> + * with operating points.
> + */

You should wrap the rcu lock before calling the dev_pm_opp_of_add_table().

> + if (dev_pm_opp_of_add_table(dev)) {
> + dev_err(dev, "Invalid operating-points in device tree.\n");
> + return -EINVAL;
> + }
> + of_property_read_u32(np, "upthreshold",
> + &data->ondemand_data.upthreshold);
> + of_property_read_u32(np, "downdifferential",
> + &data->ondemand_data.downdifferential);

Need one blank line.
Maybe this code to get the properfy for ondemand governor,
the devfreq will support in the future.

> + data->rate = clk_get_rate(data->dmc_clk);
> + rk3399_devfreq_dmc_profile.initial_freq = data->rate;

Need one blank line.

> + data->devfreq = devfreq_add_device(dev,
> + &rk3399_devfreq_dmc_profile,
> + "simple_ondemand",
> + &data->ondemand_data);
> + if (IS_ERR(data->devfreq))
> + return PTR_ERR(data->devfreq);
> + devfreq_register_opp_notifier(dev, data->devfreq);

Use the devm_devfreq_register_opp_notifier().

You need to add the MODULE_AUTHOR information.

Regards,
Chanwoo Choi

[hl]

Hi Chanwoo Choi,

Thanks for reviewing so carefully. And i have some question:

I prefer the way now use, since we get the dmcfreq->rate use
clk_get_rate() after,
Base on that, i do not care the set_rate success or fail. use curr_opp
i need to
care about set_rate status, when fail, i must set some rate, when
success i must
set other rate.

When timeout happen , may be we miss interrupt, but it do not affect this
process, since we will check the rate whether success later.

> After setting the frequency and voltage, store the current opp entry on .curr_opp.
> dmcfreq->curr_opp = opp;
>
>> + /*
>> + * check the dpll rate
>> + * there only two result we will get,
>> + * 1. ddr frequency scaling fail, we still get the old rate
>> + * 2, ddr frequency scaling sucessful, we get the rate we set
>> + */
>> + dmcfreq->rate = clk_get_rate(dmcfreq->dmc_clk);
>> +
>> + /* if get the incorrect rate, set voltage to old value */
>> + if (dmcfreq->rate != target_rate) {
>> + dev_err(dev, "get wrong ddr frequency, Request freq %lu,\
>> + Current freq %lu\n", target_rate, dmcfreq->rate);
>> + regulator_set_voltage(dmcfreq->vdd_center, dmcfreq->volt,
>> + dmcfreq->volt);
> [Without force, it is just my opion about this code:]
> I think that this checking code it is un-needed.
> If this case occur, the rk3399_dmc.c never set the specific frequency
> because the rk3399 clock don't support the specific frequency for rk3399 dmc.
>
> If you want to set the correct frequency,
> When verifying the rk3399 dmc driver, you should check the rk3399 clock driver.
>
> Basically, if the the clock driver don't support the correct same frequency ,
> CCF(Common Clock Framework) set the close frequency. It is not a bad thing.

May be i should remove the regulator_set_voltage() here, but still need to
check whether we get the right frequency, since if we do not get the
right frequency,
we should send a warn message, to remind that maybe you pass a frequency
which
do not support in bl31.

--
Lin Huang

[Chanwoo Choi]

Hi Lin,

On the next version, I'd like you to add the 'linu...@vger.kernel.org'
because devfreq is a subsystem of power management.

[snip]

I think that it is not good to get the alrady decided opp
by devfreq_recommended_opp(). Usually, devfreq_recommended_opp() is used
to get the proper opp which get the close frequency (dmcfreq->rate).

Also, When finishing the rk3399_dmcfreq_target(), the rk3399_dmc.c
have to know the current opp or rate without any finding sequence.
The additional finding procedure is un-needed.

OK. But I'd like you to modify the warning message.

One more thing, is the dcf interrupt related to the change of clock rate?
When the clock rate is changed, the dcf interrupt happen?

When calling clk_set_rate(), the final frequency only depend on the rk3399 clock driver.
But, if you want to check the new rate, I think that you should move this code
right after clk_set_rate() when there is any dependency of dcf interrupt.

> we should send a warn message, to remind that maybe you pass a frequency which
> do not support in bl31.

Also, I'd like you to explain the 'bl31' and add the description on next version.

[snip]

Regards,
Chanwoo Choi

[hl]

Hi Chanwoo Choi,

On 2016年08月02日 12:21, Chanwoo Choi wrote:
> Hi Lin,
>

> On the next version, I'd like you to add the 'linu...@vger.kernel.org'
> because devfreq is a subsystem of power management.

Sure, will do it next version.

Yes, when clock rate changed sucessful, it will trigger a irq in bl31.

it should be after the wait_event, since it indicate the clk_set_rate
finish,

>
>> we should send a warn message, to remind that maybe you pass a frequency which
>> do not support in bl31.
> Also, I'd like you to explain the 'bl31' and add the description on next version.
>
> [snip]
>
> Regards,
> Chanwoo Choi
>

[Chanwoo Choi]

Hi Lin,

OK.

OK.

>>
>>> we should send a warn message, to remind that maybe you pass a frequency which
>>> do not support in bl31.
>> Also, I'd like you to explain the 'bl31' and add the description on next version.
>>
>> [snip]
>>
>> Regards,
>> Chanwoo Choi

Regards,
Chanwoo Choi

[Lin Huang]

base on dfi result, we do ddr frequency scaling, register
dmc driver to devfreq framework, and use simple-ondemand
policy.

Signed-off-by: Lin Huang <h...@rock-chips.com>
---

Changes in v5:
- improve dmc driver suggest by Chanwoo Choi

Changes in v4:
- use arm_smccc_smc() function talk to bl31
- delete rockchip_dmc.c file and config
- delete dmc_notify
- adjust probe order

Changes in v3:
- operate dram setting through sip call
- imporve set rate flow

Changes in v2:
- None

Changes in v1:
- move dfi controller to event
- fix set voltage sequence when set rate fail
- change Kconfig type from tristate to bool
- move unuse EXPORT_SYMBOL_GPL()

drivers/devfreq/Kconfig | 9 +
drivers/devfreq/Makefile | 1 +
drivers/devfreq/rk3399_dmc.c | 512 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 522 insertions(+)
create mode 100644 drivers/devfreq/rk3399_dmc.c

diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
index a5be56e..749499d 100644
--- a/drivers/devfreq/Kconfig
+++ b/drivers/devfreq/Kconfig
@@ -100,6 +100,15 @@ config ARM_TEGRA_DEVFREQ
It reads ACTMON counters of memory controllers and adjusts the

operating frequencies and voltages with OPP support.

+config ARM_RK3399_DMC_DEVFREQ
+ tristate "ARM RK3399 DMC DEVFREQ Driver"
+ select PM_OPP
+ select DEVFREQ_GOV_SIMPLE_ONDEMAND
+ help

+ This adds the DEVFREQ driver for the RK3399 dmc(Dynamic Memory Controller).
+ It sets the frequency for the memory controller and reads the usage counts
+ from hardware.
+
source "drivers/devfreq/event/Kconfig"

endif # PM_DEVFREQ
diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile

index 09f11d9..70d9549 100644
--- a/drivers/devfreq/Makefile
+++ b/drivers/devfreq/Makefile

@@ -9,6 +9,7 @@ obj-$(CONFIG_DEVFREQ_GOV_PASSIVE) += governor_passive.o
# DEVFREQ Drivers
obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ) += exynos-bus.o
obj-$(CONFIG_ARM_TEGRA_DEVFREQ) += tegra-devfreq.o

+obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ) += rk3399_dmc.o

# DEVFREQ Event Drivers
obj-$(CONFIG_PM_DEVFREQ_EVENT) += event/

diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
new file mode 100644
index 0000000..c1157ba
--- /dev/null
+++ b/drivers/devfreq/rk3399_dmc.c
@@ -0,0 +1,512 @@
+/*
+ * Copyright (c) 2016, Fuzhou Rockchip Electronics Co., Ltd.

+ * Author: Lin Huang <h...@rock-chips.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ */
+
+#include <linux/arm-smccc.h>
+#include <linux/clk.h>

+#include <linux/delay.h>
+#include <linux/devfreq.h>
+#include <linux/devfreq-event.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_opp.h>
+#include <linux/regulator/consumer.h>
+#include <linux/rwsem.h>
+#include <linux/suspend.h>

+
+struct rk3399_dmcfreq {
+ struct device *dev;
+ struct devfreq *devfreq;
+ struct devfreq_simple_ondemand_data ondemand_data;
+ struct clk *dmc_clk;
+ struct devfreq_event_dev *edev;
+ struct mutex lock;
+ struct dram_timing *timing;

+
+ /*
+ * DDR Converser of Frequency (DCF) is used to implement DDR frequency
+ * conversion without the participation of CPU, we will implement and
+ * control it in arm trust firmware.
+ */

+ wait_queue_head_t wait_dcf_queue;
+ int irq;
+ int wait_dcf_flag;

+ struct regulator *vdd_center;
+ unsigned long rate, target_rate;
+ unsigned long volt, target_volt;

+ struct dev_pm_opp *curr_opp;
+};

+
+static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
+ u32 flags)
+{

+ struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(dev);

+ struct dev_pm_opp *opp;
+ unsigned long old_clk_rate = dmcfreq->rate;
+ unsigned long target_volt, target_rate;
+ int err;
+
+ rcu_read_lock();
+ opp = devfreq_recommended_opp(dev, freq, flags);
+ if (IS_ERR(opp)) {
+ rcu_read_unlock();
+ return PTR_ERR(opp);
+ }
+
+ target_rate = dev_pm_opp_get_freq(opp);
+ target_volt = dev_pm_opp_get_voltage(opp);
+

+ dmcfreq->rate = dev_pm_opp_get_freq(dmcfreq->curr_opp);
+ dmcfreq->volt = dev_pm_opp_get_voltage(dmcfreq->curr_opp);
+

+ rcu_read_unlock();
+
+ if (dmcfreq->rate == target_rate)
+ return 0;
+
+ mutex_lock(&dmcfreq->lock);
+
+ /*

+ * If frequency scaling from low to high, adjust voltage first.
+ * If frequency scaling from high to low, adjust frequency first.
+ */

+ if (old_clk_rate < target_rate) {
+ err = regulator_set_voltage(dmcfreq->vdd_center, target_volt,
+ target_volt);
+ if (err) {

+ dev_err(dev, "Cannot to set voltage %lu uV\n",
+ target_volt);

+ goto out;
+ }
+ }
+ dmcfreq->wait_dcf_flag = 1;
+
+ err = clk_set_rate(dmcfreq->dmc_clk, target_rate);
+ if (err) {

+ dev_err(dev, "Cannot to set frequency %lu (%d)\n",
+ target_rate, err);

+ regulator_set_voltage(dmcfreq->vdd_center, dmcfreq->volt,
+ dmcfreq->volt);
+ goto out;
+ }
+
+ /*

+ * Wait until bcf irq happen, it means freq scaling finish in
+ * arm trust firmware, use 100ms as timeout time.

+ */
+ if (!wait_event_timeout(dmcfreq->wait_dcf_queue,
+ !dmcfreq->wait_dcf_flag, HZ / 10))

+ dev_warn(dev, "Timeout waiting for dcf interrupt\n");
+
+ /*
+ * Check the dpll rate,
+ * There only two result we will get,
+ * 1. Ddr frequency scaling fail, we still get the old rate.
+ * 2. Ddr frequency scaling sucessful, we get the rate we set.

+ */
+ dmcfreq->rate = clk_get_rate(dmcfreq->dmc_clk);
+

+ /* If get the incorrect rate, set voltage to old value. */

+ if (dmcfreq->rate != target_rate) {

+ dev_err(dev, "Get wrong ddr frequency, Request frequency %lu,\
+ Current frequency %lu\n", target_rate, dmcfreq->rate);

+ regulator_set_voltage(dmcfreq->vdd_center, dmcfreq->volt,
+ dmcfreq->volt);

+ goto out;
+ } else if (old_clk_rate > target_rate)

+ err = regulator_set_voltage(dmcfreq->vdd_center, target_volt,
+ target_volt);
+ if (err)

+ dev_err(dev, "Cannot to set vol %lu uV\n", target_volt);
+
+ dmcfreq->curr_opp = opp;

+out:
+ mutex_unlock(&dmcfreq->lock);
+ return err;
+}
+
+static int rk3399_dmcfreq_get_dev_status(struct device *dev,
+ struct devfreq_dev_status *stat)
+{

+ struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(dev);
+ struct devfreq_event_data edata;
+ int ret = 0;
+
+ ret = devfreq_event_get_event(dmcfreq->edev, &edata);
+ if (ret < 0)
+ return ret;

+
+ stat->current_frequency = dmcfreq->rate;
+ stat->busy_time = edata.load_count;
+ stat->total_time = edata.total_count;
+

+ return ret;

+}
+
+static int rk3399_dmcfreq_get_cur_freq(struct device *dev, unsigned long *freq)
+{

+ struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(dev);

+
+ *freq = dmcfreq->rate;
+

+ return 0;
+}
+

+static void rk3399_dmcfreq_exit(struct device *dev)

+{
+ struct platform_device *pdev = container_of(dev,

+ struct platform_device,

+ dev);
+ struct rk3399_dmcfreq *dmcfreq = platform_get_drvdata(pdev);

+
+ devfreq_unregister_opp_notifier(dev, dmcfreq->devfreq);

+}
+
+static struct devfreq_dev_profile rk3399_devfreq_dmc_profile = {
+ .polling_ms = 200,
+ .target = rk3399_dmcfreq_target,
+ .get_dev_status = rk3399_dmcfreq_get_dev_status,
+ .get_cur_freq = rk3399_dmcfreq_get_cur_freq,
+ .exit = rk3399_dmcfreq_exit,
+};
+
+static __maybe_unused int rk3399_dmcfreq_suspend(struct device *dev)
+{

+ struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(dev);
+ int ret = 0;
+
+ ret = devfreq_event_disable_edev(dmcfreq->edev);
+ if (ret < 0) {
+ dev_err(dev, "failed to disable the devfreq-event devices\n");

+ return ret;
+ }
+

+ ret = devfreq_suspend_device(dmcfreq->devfreq);
+ if (ret < 0) {
+ dev_err(dev, "failed to suspend the devfreq devices\n");

+ return ret;
+ }
+

+ return 0;
+}
+

+static __maybe_unused int rk3399_dmcfreq_resume(struct device *dev)
+{

+ struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(dev);
+ int ret = 0;
+
+ ret = devfreq_event_enable_edev(dmcfreq->edev);
+ if (ret < 0) {
+ dev_err(dev, "failed to enable the devfreq-event devices\n");

+ return ret;
+ }
+

+ ret = devfreq_resume_device(dmcfreq->devfreq);
+ if (ret < 0) {
+ dev_err(dev, "failed to resume the devfreq devices\n");

+ return ret;
+ }
+ return ret;
+}
+

+static SIMPLE_DEV_PM_OPS(rk3399_dmcfreq_pm, rk3399_dmcfreq_suspend,
+ rk3399_dmcfreq_resume);
+
+static irqreturn_t rk3399_dmc_irq(int irq, void *dev_id)
+{
+ struct rk3399_dmcfreq *dmcfreq = dev_id;
+ struct arm_smccc_res res;

+

+ dmcfreq->wait_dcf_flag = 0;
+ wake_up(&dmcfreq->wait_dcf_queue);
+

+ /* Clear the DCF interrupt */

+ arm_smccc_smc(SIP_DDR_FREQ, 0, 0, CONFIG_DRAM_CLR_IRQ,
+ 0, 0, 0, 0, &res);
+
+ return IRQ_HANDLED;
+}
+

+static struct dram_timing *of_get_ddr_timings(struct device *dev,
+ struct device_node *np)
+{
+ struct dram_timing *timing = NULL;
+ struct device_node *np_tim;

+ int ret;

+
+ np_tim = of_parse_phandle(np, "ddr_timing", 0);

+ if (np_tim) {
+ timing = devm_kzalloc(dev, sizeof(*timing), GFP_KERNEL);
+ if (!timing)
+ goto err;
+

+ ret = of_property_read_u32(np_tim, "ddr3_speed_bin",
+ &timing->ddr3_speed_bin);
+ ret |= of_property_read_u32(np_tim, "pd_idle",
+ &timing->pd_idle);
+ ret |= of_property_read_u32(np_tim, "sr_idle",
+ &timing->sr_idle);
+ ret |= of_property_read_u32(np_tim, "sr_mc_gate_idle",
+ &timing->sr_mc_gate_idle);
+ ret |= of_property_read_u32(np_tim, "srpd_lite_idle",
+ &timing->srpd_lite_idle);
+ ret |= of_property_read_u32(np_tim, "standby_idle",
+ &timing->standby_idle);
+ ret |= of_property_read_u32(np_tim, "dram_dll_dis_freq",
+ &timing->dram_dll_dis_freq);
+ ret |= of_property_read_u32(np_tim, "phy_dll_dis_freq",
+ &timing->phy_dll_dis_freq);
+ ret |= of_property_read_u32(np_tim, "ddr3_odt_dis_freq",
+ &timing->ddr3_odt_dis_freq);
+ ret |= of_property_read_u32(np_tim, "ddr3_drv",
+ &timing->ddr3_drv);
+ ret |= of_property_read_u32(np_tim, "ddr3_odt",
+ &timing->ddr3_odt);
+ ret |= of_property_read_u32(np_tim, "phy_ddr3_ca_drv",
+ &timing->phy_ddr3_ca_drv);
+ ret |= of_property_read_u32(np_tim, "phy_ddr3_dq_drv",
+ &timing->phy_ddr3_dq_drv);
+ ret |= of_property_read_u32(np_tim, "phy_ddr3_odt",
+ &timing->phy_ddr3_odt);
+ ret |= of_property_read_u32(np_tim, "lpddr3_odt_dis_freq",
+ &timing->lpddr3_odt_dis_freq);
+ ret |= of_property_read_u32(np_tim, "lpddr3_drv",
+ &timing->lpddr3_drv);
+ ret |= of_property_read_u32(np_tim, "lpddr3_odt",
+ &timing->lpddr3_odt);
+ ret |= of_property_read_u32(np_tim, "phy_lpddr3_ca_drv",
+ &timing->phy_lpddr3_ca_drv);
+ ret |= of_property_read_u32(np_tim, "phy_lpddr3_dq_drv",
+ &timing->phy_lpddr3_dq_drv);
+ ret |= of_property_read_u32(np_tim, "phy_lpddr3_odt",
+ &timing->phy_lpddr3_odt);
+ ret |= of_property_read_u32(np_tim, "lpddr4_odt_dis_freq",
+ &timing->lpddr4_odt_dis_freq);
+ ret |= of_property_read_u32(np_tim, "lpddr4_drv",
+ &timing->lpddr4_drv);
+ ret |= of_property_read_u32(np_tim, "lpddr4_dq_odt",
+ &timing->lpddr4_dq_odt);
+ ret |= of_property_read_u32(np_tim, "lpddr4_ca_odt",
+ &timing->lpddr4_ca_odt);
+ ret |= of_property_read_u32(np_tim, "phy_lpddr4_ca_drv",
+ &timing->phy_lpddr4_ca_drv);
+ ret |= of_property_read_u32(np_tim, "phy_lpddr4_ck_cs_drv",
+ &timing->phy_lpddr4_ck_cs_drv);
+ ret |= of_property_read_u32(np_tim, "phy_lpddr4_dq_drv",
+ &timing->phy_lpddr4_dq_drv);
+ ret |= of_property_read_u32(np_tim, "phy_lpddr4_odt",
+ &timing->phy_lpddr4_odt);
+ if (ret) {

+ devm_kfree(dev, timing);
+ goto err;
+ }

+ of_node_put(np_tim);

+ return timing;
+ }
+
+err:
+ if (timing) {
+ devm_kfree(dev, timing);
+ timing = NULL;
+ }

+ of_node_put(np_tim);

+ return timing;
+}
+
+static int rk3399_dmcfreq_probe(struct platform_device *pdev)
+{
+ struct arm_smccc_res res;
+ struct device *dev = &pdev->dev;
+ struct device_node *np = pdev->dev.of_node;
+ uint64_t param = 0;
+ struct rk3399_dmcfreq *data;

+ int ret, irq, index, size;
+ uint32_t *timing;

+ struct dev_pm_opp *opp;
+

+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0) {

+ dev_err(&pdev->dev, "Cannot get the dmc interrupt resource\n");

+ return -EINVAL;
+ }
+ data = devm_kzalloc(dev, sizeof(struct rk3399_dmcfreq), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ mutex_init(&data->lock);
+
+ data->vdd_center = devm_regulator_get(dev, "center");
+ if (IS_ERR(data->vdd_center)) {
+ dev_err(dev, "Cannot get the regulator \"center\"\n");
+ return PTR_ERR(data->vdd_center);
+ }
+
+ data->dmc_clk = devm_clk_get(dev, "dmc_clk");
+ if (IS_ERR(data->dmc_clk)) {
+ dev_err(dev, "Cannot get the clk dmc_clk\n");
+ return PTR_ERR(data->dmc_clk);
+ };
+
+ data->irq = irq;
+ ret = devm_request_irq(dev, irq, rk3399_dmc_irq, 0,
+ dev_name(dev), data);
+ if (ret) {

+ dev_err(dev, "Failed to request dmc irq: %d\n", ret);
+ return ret;
+ }
+
+ /*
+ * Get dram timing and pass it to arm trust firmware,
+ * the dram drvier in arm trust firmware will get these
+ * timing and to do dram initial.
+ */

+ data->timing = of_get_ddr_timings(dev, np);
+ if (data->timing) {
+ timing = (uint32_t *)data->timing;

+ size = sizeof(struct dram_timing) / 4;
+ for (index = 0; index < size; index++) {

+ param = index;
+ param = param << 32 | *timing++;
+ arm_smccc_smc(SIP_DDR_FREQ, param, 0,
+ CONFIG_DRAM_SET_PARAM, 0, 0, 0, 0, &res);
+ if (res.a0) {

+ dev_err(dev, "Failed to set dram param: %ld\n",

+ res.a0);
+ return -EINVAL;
+ }
+ param = 0;
+ }
+ }
+
+ arm_smccc_smc(SIP_DDR_FREQ, 0, 0, CONFIG_DRAM_INIT,
+ 0, 0, 0, 0, &res);

+
+ init_waitqueue_head(&data->wait_dcf_queue);
+ data->wait_dcf_flag = 0;
+
+ data->edev = devfreq_event_get_edev_by_phandle(dev, 0);
+ if (IS_ERR(data->edev))
+ return -EPROBE_DEFER;
+
+ ret = devfreq_event_enable_edev(data->edev);
+ if (ret < 0) {
+ dev_err(dev, "failed to enable devfreq-event devices\n");

+ return ret;
+ }
+
+ /*

+ * We add a devfreq driver to our parent since it has a device tree node
+ * with operating points.
+ */

+ if (dev_pm_opp_of_add_table(dev)) {
+ dev_err(dev, "Invalid operating-points in device tree.\n");

+ rcu_read_unlock();

+ return -EINVAL;
+ }
+

+ of_property_read_u32(np, "upthreshold",
+ &data->ondemand_data.upthreshold);
+ of_property_read_u32(np, "downdifferential",
+ &data->ondemand_data.downdifferential);

+

+ data->rate = clk_get_rate(data->dmc_clk);

+
+ rcu_read_lock();
+ opp = devfreq_recommended_opp(dev, &data->rate, 0);

+ if (IS_ERR(opp)) {
+ rcu_read_unlock();
+ return PTR_ERR(opp);
+ }

+ rcu_read_unlock();
+ data->curr_opp = opp;
+

+ rk3399_devfreq_dmc_profile.initial_freq = data->rate;

+

+ data->devfreq = devfreq_add_device(dev,
+ &rk3399_devfreq_dmc_profile,
+ "simple_ondemand",
+ &data->ondemand_data);
+ if (IS_ERR(data->devfreq))
+ return PTR_ERR(data->devfreq);
+ devfreq_register_opp_notifier(dev, data->devfreq);

+
+ data->dev = dev;
+ platform_set_drvdata(pdev, data);
+

+ return 0;
+}
+

+static int rk3399_dmcfreq_remove(struct platform_device *pdev)
+{

+ struct rk3399_dmcfreq *dmcfreq = platform_get_drvdata(pdev);

+
+ devfreq_remove_device(dmcfreq->devfreq);
+ regulator_put(dmcfreq->vdd_center);
+

+ return 0;
+}
+

+static const struct of_device_id rk3399dmc_devfreq_of_match[] = {
+ { .compatible = "rockchip,rk3399-dmc" },
+ { },
+};
+
+static struct platform_driver rk3399_dmcfreq_driver = {
+ .probe = rk3399_dmcfreq_probe,
+ .remove = rk3399_dmcfreq_remove,
+ .driver = {
+ .name = "rk3399-dmc-freq",
+ .pm = &rk3399_dmcfreq_pm,
+ .of_match_table = rk3399dmc_devfreq_of_match,
+ },
+};
+module_platform_driver(rk3399_dmcfreq_driver);
+
+MODULE_LICENSE("GPL v2");

+MODULE_AUTHOR("Lin Huang <h...@rock-chips.com>");

+MODULE_DESCRIPTION("RK3399 dmcfreq driver with devfreq framework");

--
1.9.1

[kbuild test robot]

drivers/devfreq/rk3399_dmc.c:393:2-3: Unneeded semicolon


Remove unneeded semicolon.

Generated by: scripts/coccinelle/misc/semicolon.cocci

CC: Lin Huang <h...@rock-chips.com>
Signed-off-by: Fengguang Wu <fenggu...@intel.com>
---

rk3399_dmc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/devfreq/rk3399_dmc.c
+++ b/drivers/devfreq/rk3399_dmc.c
@@ -390,7 +390,7 @@ static int rk3399_dmcfreq_probe(struct p
if (IS_ERR(data->dmc_clk)) {

dev_err(dev, "Cannot get the clk dmc_clk\n");

return PTR_ERR(data->dmc_clk);
- };

+ }

data->irq = irq;

[kbuild test robot]

Hi Lin,

[auto build test WARNING on v4.8-rc1]
[also build test WARNING on next-20160809]
[cannot apply to rockchip/for-next devfreq/for-rafael linux/master]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Lin-Huang/rk3399-support-ddr-frequency-scaling/20160810-114433


coccinelle warnings: (new ones prefixed by >>)

>> drivers/devfreq/rk3399_dmc.c:393:2-3: Unneeded semicolon

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

[Chanwoo Choi]

Hi Lin,

I add the some comment on below.

If you modify them, feel free to add the my reviewed tag on next version:
Reviewed-by: Chanwoo Choi <cw00...@samsung.com>

depend on ARCH_ROCKCHIP ?

> + select PM_OPP
> + select DEVFREQ_GOV_SIMPLE_ONDEMAND

This entry needs the following command in Kconfig:
select DEVFREQ_EVENT_ROCKCHIP_DFI

> + help
> + This adds the DEVFREQ driver for the RK3399 dmc(Dynamic Memory Controller).

Use a capital letter for and abbreviation.

s/dmc -> DMC

> + It sets the frequency for the memory controller and reads the usage counts
> + from hardware.
> +
> source "drivers/devfreq/event/Kconfig"
>
> endif # PM_DEVFREQ
> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
> index 09f11d9..70d9549 100644
> --- a/drivers/devfreq/Makefile
> +++ b/drivers/devfreq/Makefile
> @@ -9,6 +9,7 @@ obj-$(CONFIG_DEVFREQ_GOV_PASSIVE) += governor_passive.o
> # DEVFREQ Drivers
> obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ) += exynos-bus.o
> obj-$(CONFIG_ARM_TEGRA_DEVFREQ) += tegra-devfreq.o
> +obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ) += rk3399_dmc.o

The entry would be positioned
The position of CONFIG_ARM_RK3399_DMC_DEVFREQ would be good on
between EXYNOS_BUS_DEVFREQ and TEGRA_DEVFREQ because of the alphabetical order.

As I mentioned on previous patches on v4,
You'd better to use the devm_devfreq_register_opp_notifier().
If you use devm_devfreq_register_opp_notifier(), you don't need
to handle the unregistration operation.

ditto. devm_devfreq_register_opp_notifier()

> +
> + data->dev = dev;
> + platform_set_drvdata(pdev, data);
> +
> + return 0;
> +}
> +
> +static int rk3399_dmcfreq_remove(struct platform_device *pdev)
> +{
> + struct rk3399_dmcfreq *dmcfreq = platform_get_drvdata(pdev);
> +
> + devfreq_remove_device(dmcfreq->devfreq);

I think that you don't need to call this function.

> + regulator_put(dmcfreq->vdd_center);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id rk3399dmc_devfreq_of_match[] = {
> + { .compatible = "rockchip,rk3399-dmc" },
> + { },
> +};
> +
> +static struct platform_driver rk3399_dmcfreq_driver = {
> + .probe = rk3399_dmcfreq_probe,
> + .remove = rk3399_dmcfreq_remove,
> + .driver = {
> + .name = "rk3399-dmc-freq",
> + .pm = &rk3399_dmcfreq_pm,
> + .of_match_table = rk3399dmc_devfreq_of_match,
> + },
> +};
> +module_platform_driver(rk3399_dmcfreq_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Lin Huang <h...@rock-chips.com>");
> +MODULE_DESCRIPTION("RK3399 dmcfreq driver with devfreq framework");
>

Regards,
Chanwoo Choi